fold/docs/research/convergence-code-review-2026-07-29.md

5.2 KiB

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.