# CLI/MCP convergence cumulative code review — 2026-07-29

## Scope and method

This review covers the cumulative uncommitted quality-harness, CLI/router,
report/query, MCP-policy/job, documentation-generation, and compatibility
changes relative to `669b4b4`. The tracked diff was approximately 3,763
insertions and 2,100 deletions across 50 files before review fixes, plus the new
catalog/query/policy/harness files.

The review was findings-first and risk-weighted. It combined:

- targeted diff and history inspection;
- separate reviews of MCP/query, CLI/report, and quality/CI surfaces;
- schema/handler and report round-trip checks;
- focused regression tests followed by the complete Meson suite;
- Python harness tests, source-structure checks, diagnostics, generated-file
  parity, and whitespace validation.

Task 32 retains the separate bounded Fold self-scan so analyzer/tool failures do
not get conflated with this manual differential review.

## Confirmed findings resolved

| Severity | Finding | Resolution and regression evidence |
| --- | --- | --- |
| High | MCP accepted workspace compile databases containing Clang/plugin load flags, permitting analyzer-process code execution outside the intended path policy. | Parse the compilation database with Clang, reject response files and plugin/pass-plugin arguments before dispatch, and cover `-Xclang -load` in `mcp_input_policy`. |
| High | Project-evaluation and optimization runs could consume stale report/profile/run artifacts from a prior invocation. | Remove every expected lane artifact before launch, require profiled runs to produce a profile, use a fresh optimization run directory, and add a stale-artifact regression. |
| Medium | CLI numeric options did not enforce the bounds encoded by the shared option catalog. | Add CLI11 checks for clone levels/thresholds, solver/process/time limits, and candidate/function caps; add CLI rejection coverage. |
| Medium | The Fold SARIF reader accepted malformed nested message/location shapes as empty actionable findings. | Validate Fold SARIF message and physical/logical location shapes and add malformed nested-field tests. |
| Medium | The `binary` query filter excluded granular `binary-ir` and `binary-pcode-ssa` provenance. | Treat canonical backend filters as evidence-lane filters while preserving exact provenance; test P-Code matching. |
| Medium | Scan, grep, profile, FixIt, and embedded-skill writers did not consistently detect post-open stream failures. | Flush and check output state before returning a success/finding exit status. |
| Medium | SARIF round-trips omitted proof/bail evidence, failed to normalize ROP `stackOffset`, and did not declare every emitted rule. | Emit and read the missing evidence, generate driver rules from the one canonical rule table, and add foreign/security rule plus round-trip tests. |
| Medium | Completed-job queries copied the full finding vector while holding the queue mutex and copied it again before bounded pagination. | Store immutable job findings behind a shared pointer, release the lock before querying, avoid the second copy, use overflow-safe paging, and enforce the server work-item ceiling. |
| Medium | Evidence queries omitted gadget, ROP-chain, and fact-provenance strings. | Search all string-bearing exploit and fact-provenance fields; cover gadget text. |
| Medium | Unknown future/foreign wire finding kinds aborted the entire report. | Map them to `UnknownForeign`, retain the original kind in correlation evidence, emit a warning, and add forward-compatibility coverage. |
| Medium | Newline/non-ASCII Git paths could be C-quoted and evade differential source/test selection. | Use NUL-delimited Git output and filesystem decoding; add an unusual-filename regression. |
| Medium | Lint comparison collapsed multiple same-identity warnings in one file into one accepted occurrence. | De-duplicate only identical source locations and compare identity multiplicity; test that an added occurrence is new debt. |
| Medium | `fold quality` inherited a hidden five-minute outer subprocess timeout despite two-hour CI tiers and tighter inner timeouts. | Align the outer wrapper with the declared two-hour workflow ceiling while preserving inner test/scan limits and timeout classification. |

## Reviewed decisions retained

- Scan exit status remains based on the filtered/deduplicated findings visible in
  every selected report format. `findings-json` is field-preserving and versioned;
  it is not a bypass around report visibility policy.
- `fold grep` intentionally requires `PATTERN REPORT...`; filters-only invocation
  is outside the frozen CLI contract.
- A positive fixture may also carry the `regression` coverage label: `positive`
  describes expected detector behavior, while `regression` marks the assertion
  as a locked prior behavior. Negative coverage remains a separate assertion.
- Completed jobs remain session-scoped. The queue depth limits active work;
  immutable snapshots and query work ceilings bound per-query amplification.

## Residual review limits

- The bounded production-only Fold self-scan is intentionally deferred to task
  32 because the full workspace compilation database includes unsupported
  vendored/multi-command entries.
- This review did not claim whole-project sanitizer or fuzz coverage beyond the
  existing Meson suite.