fold/docs/research/cli-mcp-surface-convergence-plan-2026-07-28.md

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`.