446 lines
17 KiB
Markdown
446 lines
17 KiB
Markdown
# CLI/MCP Surface Convergence Audit and Plan
|
|
|
|
Date: 2026-07-28
|
|
|
|
## Goal
|
|
|
|
Make Fold expose one coherent scan contract across CLI, worker, and MCP; reduce
|
|
agent-facing tool duplication; add a grep-like finding inspection operation;
|
|
and update agent skills only after the built surfaces are verified.
|
|
|
|
This plan preserves source/binary backend boundaries, finding provenance, the
|
|
existing scan exit codes, and compatibility aliases during a documented
|
|
migration window.
|
|
|
|
## Audit findings
|
|
|
|
### 1. MCP schemas are descriptive, not enforced (high)
|
|
|
|
`ToolRegistry::call` dispatches raw arguments directly to handlers without
|
|
validating the advertised `inputSchema` (`src/mcp/tool_registry.cpp:234-251`). A
|
|
built-server check confirmed that `fold.get_scan_context` accepts an unknown
|
|
`definitely_not_an_option` property and returns success.
|
|
|
|
**Impact:** misspelled or stale agent parameters can be silently ignored. The
|
|
request appears successful but executes with defaults.
|
|
|
|
**Recommendation:** validate every call before dispatch, set
|
|
`additionalProperties: false`, and keep compatibility aliases in an explicit
|
|
allow-list rather than a permissive fallback.
|
|
|
|
### 2. `ScanOptions` is centralized, but its public contract is not (high)
|
|
|
|
`ScanOptions` contains the shared runtime values
|
|
(`include/fold/cli/scan_options.hpp:15-90`), but CLI registration is handwritten
|
|
inside the 991-line `src/cli/main.cpp`, while MCP schema and JSON parsing are
|
|
handwritten separately in `src/mcp/tool_registry_scan.cpp:22-112` and
|
|
`src/mcp/tool_registry.cpp:148-212`.
|
|
|
|
The built CLI advertises 53 long `scan` options. The general MCP `fold.scan`
|
|
schema advertises 21 properties, including MCP-only queue controls, and maps
|
|
only a subset into `ScanOptions`.
|
|
|
|
**Impact:** adding a field to `ScanOptions` does not add or validate it on either
|
|
public surface despite the header comment claiming a single addition point.
|
|
|
|
**Recommendation:** add a declarative scan-option catalog and generate MCP
|
|
schema/parsing and contract metadata from it. Keep CLI11 special callbacks only
|
|
where parsing genuinely differs.
|
|
|
|
### 3. Equivalent options have different names and capabilities (high)
|
|
|
|
Examples:
|
|
|
|
- CLI `--binary` versus MCP `binary_path`.
|
|
- CLI `--binary-max-functions` versus MCP `max_functions`.
|
|
- CLI `--scope`, presets, exclusions, analysis depth, complexity/clone tuning,
|
|
incremental controls, and several safety bounds have no general MCP mapping.
|
|
- MCP parses legacy `input_type`, but the general schema does not advertise it.
|
|
|
|
**Impact:** users and agents cannot transfer a known-good invocation between
|
|
surfaces, and unsupported values may be ignored rather than rejected.
|
|
|
|
**Recommendation:** use one canonical internal/JSON key per option. CLI keeps
|
|
idiomatic kebab-case flags and positional paths; MCP uses the same words in
|
|
snake_case. Existing MCP names remain temporary deprecated aliases.
|
|
|
|
### 4. MCP exposes duplicate and competing operations (high)
|
|
|
|
The built server advertises 21 tools. Exact duplicate implementations exist for
|
|
`fold.get_job_findings` / `fold.scan_job_findings` and
|
|
`fold.activate_scan_job` / `fold.scan_activate`
|
|
(`src/mcp/tool_registry_scan.cpp:148-252`). `fold.scan` also accepts
|
|
`background: true` while `fold.scan_start` separately represents the queued
|
|
operation (`src/mcp/tool_registry_scan.cpp:22-112`). General scan competes with
|
|
`scan_project`, language/binary scan wrappers, and four `find_*` wrappers.
|
|
|
|
**Impact:** tool selection becomes harder for models and humans, documentation
|
|
must explain synonyms, and duplicated handlers/schemas drift.
|
|
|
|
**Recommendation:** advertise one canonical tool per intent. Keep old names
|
|
callable but hidden from `tools/list` during migration.
|
|
|
|
### 5. `--format json` is not a native Fold JSON format (high)
|
|
|
|
`writeReport` routes both `sarif` and `json` to `writeSarif`
|
|
(`src/cli/main.cpp:70-95`). The skill and usage documentation describe JSON as
|
|
if it were a distinct finding representation.
|
|
|
|
**Impact:** scripts requesting JSON may unknowingly depend on SARIF structure,
|
|
while MCP returns a different native `findingToJson` shape.
|
|
|
|
**Recommendation:** add an explicitly named, versioned `findings-json` format.
|
|
Keep `json` as a deprecated SARIF alias until a major-version migration; do not
|
|
silently change its shape in a minor release.
|
|
|
|
### 6. User help contains invalid or missing recovery references (medium)
|
|
|
|
CLI help says `--paths` is required even though paths are positional
|
|
(`src/cli/main.cpp:400-403`), and Rust/Swift router errors use the same
|
|
nonexistent flag. Root help points to `docs/mcp-tools.md`, which does not exist
|
|
(`src/cli/main.cpp:924-926`). Quality help still says "focused tests" after the
|
|
impacted/full-suite work.
|
|
|
|
**Impact:** recovery guidance cannot be copied successfully, violating Fold's
|
|
own CLI philosophy.
|
|
|
|
**Recommendation:** test every recovery command against the built binary and
|
|
generate MCP/option docs from canonical metadata.
|
|
|
|
### 7. Accepted hybrid strategies exceed implemented behavior (medium)
|
|
|
|
CLI and MCP advertise `binary-first`, `source-first`, and `bidirectional`, but
|
|
the router warns and falls back to merge-only (`src/cli/scan_router.cpp:604-672`).
|
|
|
|
**Impact:** an invocation can succeed without performing the strategy the user
|
|
requested.
|
|
|
|
**Recommendation:** advertise and accept only implemented strategies. Retain
|
|
`auto` only if its current merge-only meaning is explicit.
|
|
|
|
### 8. Finding inspection is incomplete (medium)
|
|
|
|
CLI scans can write reports but cannot list, show, filter, or grep them. MCP can
|
|
page the current context by backend only; it cannot query message, rule,
|
|
confidence, path, function, suppression, or evidence fields. MCP state is also
|
|
session-local.
|
|
|
|
**Impact:** users must pipe SARIF through ad hoc `jq` expressions, while agents
|
|
page and post-filter potentially large finding sets.
|
|
|
|
**Recommendation:** implement one shared finding-query engine and expose it as a
|
|
grep-like report operation plus one MCP query tool.
|
|
|
|
### 9. Skill sources already drift (medium)
|
|
|
|
`skills/fold-sast/SKILL.md` is the intended skill, the embedded header differs
|
|
from it in two lines, and `docs/internal/fold-skill.md` is a separate 116-line
|
|
older guide. There is no byte-parity test. The skill also recommends
|
|
`--format json`, inheriting the SARIF/native-JSON ambiguity.
|
|
|
|
**Impact:** installed, embedded, and repository guidance can describe different
|
|
commands and output contracts.
|
|
|
|
**Recommendation:** make `skills/fold-sast/SKILL.md` authoritative and generate
|
|
or verify every mirror.
|
|
|
|
## Target surface
|
|
|
|
### CLI
|
|
|
|
```sh
|
|
# One router-backed scan operation
|
|
fold scan [PATH...] --scope security --format sarif
|
|
fold scan --mode binary --binary ./app --binary-max-functions 64
|
|
fold scan --mode hybrid --compile-db build --binary ./app
|
|
|
|
# Inspect existing Fold findings, not arbitrary source text
|
|
fold grep PATTERN report.sarif
|
|
fold grep PATTERN report.sarif --field message --field path --ignore-case
|
|
fold grep report.sarif --rule hardcoded-credential --backend binary
|
|
```
|
|
|
|
`fold grep` searches finding reports or stdin. It does not replace ripgrep and
|
|
does not scan source code. Querying preserves finding IDs and backend
|
|
provenance and never promotes confidence.
|
|
|
|
Grep output defaults to:
|
|
|
|
```text
|
|
path:line:column: [rule/confidence/backend] message
|
|
```
|
|
|
|
Structured output uses versioned native finding JSON. Grep exit status follows
|
|
Fold's existing actionable-finding contract: `0` no actionable matches, `1`
|
|
actionable matches, `2` invalid query/input/report failure.
|
|
|
|
**Decision gate before implementation:** confirm that "grep" means finding
|
|
reports/current MCP contexts, not raw source text. Finding reports are the
|
|
recommended scope because raw text search duplicates ripgrep and bypasses Fold
|
|
analysis/evidence semantics.
|
|
|
|
### Canonical advertised MCP tools
|
|
|
|
```text
|
|
fold.scan synchronous router scan
|
|
fold.scan_start queued router scan
|
|
fold.scan_status one job status
|
|
fold.scan_jobs list jobs
|
|
fold.scan_job_findings page one completed job
|
|
fold.scan_activate activate one completed job
|
|
fold.get_scan_context current context
|
|
fold.get_findings page current findings
|
|
fold.query_findings grep/filter current context or completed job
|
|
fold.explain inspect one finding
|
|
fold.suggest_merge clone/equivalence merge candidate
|
|
```
|
|
|
|
`fold.scan` becomes synchronous only. `fold.scan_start` becomes the only queued
|
|
entrypoint. Queue limits move to MCP server configuration instead of each scan
|
|
request.
|
|
|
|
## Phased implementation
|
|
|
|
### Phase 0 — Contract fixtures before refactoring
|
|
|
|
Add built-surface snapshots and behavioral tests for:
|
|
|
|
- root and scan help;
|
|
- positional paths and current recovery errors;
|
|
- `tools/list` names and schemas;
|
|
- unknown MCP argument acceptance (a red test for the fix);
|
|
- source, binary, Rust, Swift, and merge-only hybrid parity;
|
|
- current aliases and `json`-as-SARIF behavior.
|
|
|
|
Likely files:
|
|
|
|
- `tests/integration/test_cli_paths.cpp`
|
|
- `tests/integration/test_mcp_stdio.cpp`
|
|
- new `tests/contracts/` fixtures for CLI/MCP metadata
|
|
|
|
Acceptance: the test suite records current compatibility behavior before any
|
|
handler or name moves.
|
|
|
|
### Phase 1 — Canonical scan option catalog
|
|
|
|
Add a small metadata model, for example:
|
|
|
|
```cpp
|
|
struct ScanOptionSpec {
|
|
ScanOptionId id;
|
|
std::string_view cli_name;
|
|
std::string_view json_name;
|
|
OptionType type;
|
|
OptionExposure exposure; // common, advanced, CLI-only, MCP-safe
|
|
// enum/bounds/default/help/group/deprecation metadata
|
|
};
|
|
```
|
|
|
|
Generate from the catalog:
|
|
|
|
1. MCP `inputSchema`;
|
|
2. validated JSON-to-`ScanOptions` parsing;
|
|
3. option documentation/contract JSON;
|
|
4. parity checks for CLI registration.
|
|
|
|
CLI11 registration can still use member pointers and focused callbacks for
|
|
comma-separated lists, positional paths, and the transitional `--profile`
|
|
syntax. Do not build a general reflection framework.
|
|
|
|
Likely files:
|
|
|
|
- `include/fold/cli/scan_option_catalog.hpp` (new)
|
|
- `src/cli/scan_option_catalog.cpp` (new)
|
|
- `src/cli/scan_command.cpp` (new; moves scan binding out of `main.cpp`)
|
|
- `src/mcp/scan_schema.cpp` (new)
|
|
- `include/fold/cli/scan_options.hpp`
|
|
- `src/cli/main.cpp`
|
|
- `src/mcp/tool_registry.cpp`
|
|
|
|
Acceptance:
|
|
|
|
- every advertised MCP property maps to one `ScanOptions` field or one declared
|
|
server-only field;
|
|
- defaults/enums/bounds match on CLI and MCP;
|
|
- canonical names are `binary` and `binary_max_functions`; old MCP names warn as
|
|
aliases;
|
|
- no duplicated scan schema literals remain.
|
|
|
|
### Phase 2 — Strict MCP boundary and filesystem policy
|
|
|
|
Validate schema before handler dispatch:
|
|
|
|
- `additionalProperties: false`;
|
|
- exact types and enums;
|
|
- integer, string, array, and path-count bounds;
|
|
- no NUL-containing paths;
|
|
- deterministic invalid-params errors naming the field and expected value.
|
|
|
|
Classify options by MCP safety. Do not expose CLI output/fix/profile paths,
|
|
arbitrary Clang plugin flags, cache/quarantine writes, or
|
|
`rust_allow_build_code` by default. Add server-level repeated allowed roots and
|
|
queue/resource limits; canonicalize paths and reject symlink escapes.
|
|
|
|
Likely files:
|
|
|
|
- `include/fold/mcp/schema_validator.hpp` (new)
|
|
- `src/mcp/schema_validator.cpp` (new)
|
|
- `include/fold/mcp/server_policy.hpp` (new)
|
|
- `src/mcp/tool_registry.cpp`
|
|
- `src/cli/main.cpp` or a new `serve_command.cpp`
|
|
|
|
Acceptance:
|
|
|
|
- unknown/mistyped/out-of-range arguments fail before handler execution;
|
|
- all filesystem inputs remain beneath configured roots;
|
|
- MCP cannot enable Rust build scripts or inject arbitrary compiler/plugin
|
|
flags;
|
|
- valid CLI/MCP requests normalize to the same option fingerprint.
|
|
|
|
### Phase 3 — MCP operation consolidation
|
|
|
|
Extend `Tool` with advertised/deprecation metadata. `tools/list` returns only
|
|
canonical tools; `call` continues to route hidden aliases for a documented
|
|
migration window.
|
|
|
|
- Keep `scan_job_findings` and `scan_activate` canonical.
|
|
- Hide `get_job_findings` and `activate_scan_job` as aliases.
|
|
- Hide `scan_project`, `scan_rust`, `scan_swift`, `scan_binary`, and `find_*`
|
|
wrappers; forward them to `fold.scan` with mode/scope/detectors.
|
|
- Stop advertising `background` on `fold.scan`; temporarily accept it as a
|
|
deprecated queueing alias, then remove it in the next major release.
|
|
- Move `queue_depth` to MCP server configuration.
|
|
|
|
Acceptance:
|
|
|
|
- advertised tools drop from 21 to the canonical set above;
|
|
- aliases remain callable and return a structured deprecation notice;
|
|
- scan and scan-start schemas share one generated analysis schema;
|
|
- no duplicate handlers remain.
|
|
|
|
### Phase 4 — CLI cleanup and truthful formats
|
|
|
|
- Move scan registration/dispatch out of `main.cpp`.
|
|
- Keep common options in normal help; add a discoverable advanced-help path for
|
|
detector tuning, profiling, and quarantine controls without removing flags.
|
|
- Replace every `--paths` reference with positional examples.
|
|
- Restrict hybrid strategy values to implemented behavior.
|
|
- Add `findings-json` with a versioned envelope:
|
|
|
|
```json
|
|
{"output_version": 1, "findings": []}
|
|
```
|
|
|
|
- Keep `json` as a deprecated SARIF alias through the compatibility window.
|
|
- Generate `docs/mcp-tools.md` and option tables from catalogs.
|
|
|
|
Acceptance:
|
|
|
|
- all recovery commands execute against the built binary;
|
|
- `sarif`, `findings-json`, and markdown have distinct tested contracts;
|
|
- old flags preserve behavior or emit one deterministic deprecation warning;
|
|
- scan help is shorter without hiding how to discover advanced options.
|
|
|
|
### Phase 5 — Shared finding query and `fold grep`
|
|
|
|
Add report-layer components:
|
|
|
|
- `FindingReportReader`: SARIF plus versioned native finding JSON;
|
|
- `FindingQuery`: fixed/regex text matching and typed filters;
|
|
- deterministic sorting/pagination/truncation metadata.
|
|
|
|
Initial query fields:
|
|
|
|
- message, ID, rule/kind, path, function, and evidence text;
|
|
- rule, confidence, backend, path glob, baseline/suppression status, and
|
|
actionable-only filters.
|
|
|
|
Use exactly the same engine from CLI and MCP:
|
|
|
|
- `fold grep [PATTERN] [REPORT...]`;
|
|
- `fold.query_findings` with current context or `job_id`.
|
|
|
|
If regex support cannot be bounded safely, ship fixed-string matching first and
|
|
make regex an explicit later capability rather than using unbounded
|
|
`std::regex` on attacker-controlled MCP input.
|
|
|
|
Likely files:
|
|
|
|
- `include/fold/report/finding_query.hpp` (new)
|
|
- `src/report/finding_query.cpp` (new)
|
|
- `include/fold/report/finding_report_reader.hpp` (new)
|
|
- `src/report/finding_report_reader.cpp` (new)
|
|
- `src/cli/grep_command.cpp` (new)
|
|
- `src/mcp/tool_registry_findings.cpp`
|
|
|
|
Acceptance:
|
|
|
|
- CLI and MCP queries over the same fixture return identical ordered finding
|
|
IDs and totals;
|
|
- source/binary/Rust/Swift provenance remains unchanged;
|
|
- literal, case, field, glob, pagination, no-match, malformed-report, and
|
|
suppressed-finding cases are covered;
|
|
- complete results remain available as artifacts while summaries stay bounded.
|
|
|
|
### Phase 6 — Compatibility removal checkpoint
|
|
|
|
Do not remove aliases based only on elapsed time. Before removal, require:
|
|
|
|
- repository/CI searches show no first-party use;
|
|
- deprecation telemetry or release feedback is reviewed where available;
|
|
- migration docs contain old-to-new examples;
|
|
- one major-version boundary is selected explicitly.
|
|
|
|
Until then, aliases stay hidden but callable.
|
|
|
|
### Phase 7 — Skills and generated documentation (last)
|
|
|
|
Make `skills/fold-sast/SKILL.md` the sole authored skill source.
|
|
|
|
- Generate `include/fold/cli/embedded_skill.hpp` during the build.
|
|
- Replace `docs/internal/fold-skill.md` with a pointer or generated mirror.
|
|
- Add a byte-parity test: built `fold skill` equals the canonical file.
|
|
- Update the skill only after built CLI help and MCP schemas pass, adding:
|
|
- canonical scan names;
|
|
- `fold grep` / `fold.query_findings` recipes;
|
|
- native JSON versus SARIF guidance;
|
|
- async scan lifecycle;
|
|
- deprecated aliases and removal window;
|
|
- MCP allowed-root/resource policy.
|
|
- Run agent-facing prompt/skill examples against source, binary, hybrid, queued,
|
|
and grep workflows. Record failures before adopting/publishing the revision.
|
|
- Any Sage library/publish/activation step remains a separate, explicit
|
|
authority-bearing action after local validation.
|
|
|
|
## Validation matrix
|
|
|
|
Focused checks during development:
|
|
|
|
```sh
|
|
ninja -C build/release
|
|
meson test -C build/release \
|
|
fold:cli_paths fold:mcp_stdio fold:scan_e2e \
|
|
fold:binary_backend fold:subprocess_fanout \
|
|
--print-errorlogs
|
|
```
|
|
|
|
Built-surface checks:
|
|
|
|
```sh
|
|
./build/release/src/cli/fold --help
|
|
./build/release/src/cli/fold scan --help
|
|
./build/release/src/cli/fold grep --help
|
|
printf '<initialize/tools-list fixture>' \
|
|
| ./build/release/src/cli/fold --quiet mcp
|
|
```
|
|
|
|
Required full validation for CLI/router/MCP changes:
|
|
|
|
```sh
|
|
ninja -C build/release
|
|
meson test -C build/release --print-errorlogs
|
|
git diff --check
|
|
```
|
|
|
|
For every phase, preserve source/binary lane separation, finding backend
|
|
provenance, complete forensic artifacts, and exit codes `0`/`1`/`2`.
|