[Dumbris]

Summary

Spec 120 (specs/120-scan-structural-gate/spec.md), follow-up to #1740 (Spec 119) and Phase 0 of the maintainer decision posted on #1713 on 2026-10-25 after the reviews by @james-crowley and @kaspermuutra-dotcom. Must merge before the release that carries the scan default (#1740): the two ship together.

Under trust_mode: scan a changed tool contract is approved when the offline TPA scan is clean. The scanner reads a schema as text and never sees the annotations, so a new required bcc parameter, an openWorldHint flipped to false, or a defaulted webhook_url all passed a clean scan. This PR puts a deterministic structural gate in front of that verdict.

• contractDelta(approved, live) []Signal (internal/runtime/tool_contract_delta.go) replaces the spec-119 safer-flip rule at both checkToolApprovals gate sites. Any signal holds the change whatever the verdict; signals are prepended so MaxToolHeldSignals never drops them. Rules:
  • GATE-HINT-CHANGE:<hint>:<old>>:<new>: any effective change of the four safety hints, either direction, MCP defaults applied (GATE-HINT-SAFER-FLIP kept as the label of the two safer flips).
  • GATE-SCHEMA-SHAPE:<reason>:<path>: recursive keyword-path diff of the input schema (new required, type/$ref, enum member added or enum dropped, pattern/format, loosened or removed bound, additionalProperties relaxed, default/const, new property whose subtree carries default/const/enum, anyOf/oneOf grew, allOf shrank, items form, unknown approved schema). Narrowing edits pass.
  • GATE-OUTPUT-SCHEMA:added|removed|changed (text keywords stripped).
  • GATE-HINT-CHANGE:dispatch:<old>>:<new> (FR-009): the hash folds hints in by effective value while DeriveCallWith reads an explicit default differently from an absent hint, so {readOnlyHint:false, destructiveHint:true} → no annotations kept the hash and moved the tool from call_tool_destructive to call_tool_read. The failing test was written first (TestHashEqualDispatchDrop_Bites); it bit, so the hash-match, formula and content shortcuts are closed for a dispatch drift and the revert proof requires the variant too.
  • Explicitly not built: keyword lists, edit distance, classifier, holding every new optional property.
• Audit: tool_auto_approved for a changed contract (scan-approved and auto_approve_tool_changes) now carries old_hash, old_description, old_schema, old_output_schema, old_annotations, the new values, approved_by, gate_signals, scan_verdict, scan_bundle.
• Upgrade visibility: one-time startup WARN (marker scan_default_notice_done, nothing on a fresh install) and a mcpproxy doctor finding name every enabled server with no trust_mode.
• Contracts and UI: terminology family hold_signal (Go, golden, generated TS consts, Web labels on the hold-evidence chips). Shared fixture internal/runtime/testdata/contract_delta_cases.json: Go asserts exact signals (51 cases), vitest asserts every held schema case yields a review line.
• Pilot: qapilot.Contract.Shape, fixture BenignWide, QP-20, scenarios S28/S29, mutations M15–M19.
• Docs: security-quarantine.md "Structural changes are held" with the residual risk stated plainly (paraphrase, one-word edit, text drift, behaviour behind an unchanged definition); spec 119 FR-007 amended; CHANGELOG Breaking Changes entry extended.

Behaviour changes to existing tests (intentional)

• Spec 119's "less-safe flip is scan-approved" is now held (any hint change holds).
• A legacy changed record with no recorded previous input schema is held (approved-unknown), not scan-approved; a v4 collision whose live schema changes type is held under scan too.
• Spec 117 fixtures that used "add an explicit destructiveHint: true on a tool approved without hints" as a hash-equal, still-approved change now use a title-only annotation change; that hint move is exactly the FR-009 hold.
• Live mcp-go fixtures seed the explicit default hints mcp-go's NewTool serves.

Verification

go test ./internal/runtime ./internal/qapilot ./internal/config ./internal/telemetry ./internal/storage ./internal/contracts, internal/httpapi Spec 109 goldens, server-tag run of internal/server, httpapi, serveredition, cmd with the CI skip regex, ./scripts/test-api-e2e.sh, qapilot conformance + qapilot report gate, frontend vitest (2762), both golangci-lint configs (only pre-existing findings outside this diff), check-upstream-write-sites.sh, check-no-unquarantine-callers.sh. Cross-model review: opencode github-copilot/gpt-6.1-sol, three chunks; findings and fixes listed in the first comment.

🤖 Generated with Claude Code

[cloudflare-workers-and-pages]

Deploying mcpproxy-docs with    Cloudflare Pages

Latest commit:	5b268c5
Status:	✅  Deploy successful!
Preview URL:	https://deed93e1.mcpproxy-docs.pages.dev
Branch Preview URL:	https://120-scan-structural-gate.mcpproxy-docs.pages.dev

View logs

[Dumbris]

Cross-model review log (opencode github-copilot/gpt-6.1-sol, three file-named chunks)

Chunk A — tool_contract_delta.go + shared fixture. Round 1: FINDINGS 5, all verified and fixed in 08688e0 → fix(quarantine): review round 1:

1. P1 numbers decoded as float64 hid bound/enum widenings beyond 2^53 → schemas decode with json.Number, bounds compare via math/big.
2. P1 stripText removed output properties named description/title → schema-aware stripping (property-name maps are names, not keywords).
3. P2 carriesSteering saw a property named default or an example containing one as steering → schema-aware walk.
4. P2 a description-only change inside not was held → stripped forms compared.
5. P2 additionalProperties: {} treated as constraining → {} is unconstrained like true.

Round 2: FINDINGS 5, fixed in 850036a → fix(quarantine): review round 2:
6. P1 {} → {default: …} under additionalProperties passed → two schemas always recurse.
7. P1 steering inside a newly added tuple-form items passed → carriesSteering handles schema lists.
8. P2 enum [1,2] → [1.0] held as "added" → members identified by numeric value.
9. P2 trailing content after the first JSON value decoded as valid → rejected (unparsable).
10. P2 prefixItems descriptions were structural → added to the subschema keywords.

Round 3: FINDINGS 4, fixed in 1e7f538 → fix(quarantine): review round 3:
11. P1 an additionalProperties schema introduced from absent/true with a default passed → held as new-property:additionalProperties when it carries steering (a plain constraining one passes).
12. P2 nested numbers in enum members compared by spelling → canonical numbers at any depth.
13. P2 a default-only additionalProperties schema counted as a constraint → ignored.
14. P2 draft-04 dependencies not stripped → name-map keyword.

Round 4: FINDINGS 2, fixed in a046118 → fix(quarantine): review round 4:
15. P1 the numeric canonical form collided with a string member spelled "n:1" → injective tagged canonical form (canonicalValue) for every value comparison.
16. P2 multipleOf/default/const compared by spelling → sameJSON compares canonical values.

Round 5: FINDINGS 1, fixed in 594f0eb: P2 required: [] / properties: {} counted as constraints in schemaIsUnconstrained.

Round 6: FINDINGS 2, fixed in 307fc40:
17. P1 a growing allOf skipped the pairwise comparison of retained branches → common-prefix recursion, growth rule on top.
18. P2 unknown annotation keywords (x-note) made a schema look constraining → allowlist of validation keywords.

Round 7: FINDINGS 4, fixed in a424a6e:
19. P1 const: [] exempted as "empty list" → const/enum/not/ always constrain.
20. P2 {additionalProperties: true} / {properties: {x: {}}} nested schemas read as constraining → semantic recursion (subschemaUnconstrained).
21. P2 removing a non-final combinator branch shifted the index pairing → surviving branches matched by value first (unmatchedBranches); residual pairing by index can over-report, accepted.
22. P2 output schemas compared numbers by spelling → canonical values.

Round 8: FINDINGS 2, fixed in df93544:
23. P1 appended allOf branches were never inspected → new branches checked for steering.
24. P1 oneOf with an unconstrained branch read as unconstrained → only a single unconstrained branch qualifies.

Round 9: FINDINGS 1, fixed in fb03e9b: P1 a new allOf branch carrying required made an optional parameter mandatory unseen → new branches are scanned for required names anywhere (requiredAnywhere).

Round 10 (the per-PR cap): see below.

Chunk B — gate wiring in tool_quarantine.go / tool_quarantine_scan_approve.go. FINDINGS 2: one duplicate of A2; one P2: more than 16 gate signals were cut by MaxToolHeldSignals (evidence loss, still held) → gate signals are compacted per rule with a <rule>:more:+n marker (compactGateSignals, spec FR-002 amended). The reviewer traced FR-009 (no remaining bypass for a hash-equal dispatch drift; revert restores only when the explicit hints return), the signal-cap/reason handling, the failed-write rollback and the audit before-state at both sites, and manual-mode scope: no other findings.

Chunk C — upgrade visibility, qapilot model, terminology/Web. VERDICT: CLEAN.

Every fix carries a fixture row in internal/runtime/testdata/contract_delta_cases.json (102 rows; web_invisible marks the two Go-only precision cases the Web differ cannot show).

Round 10 — FINDINGS 2, UNRESOLVED (per-PR round cap reached; the maintainer decides). Both are real by FR-004's letter and share one cause: newBranchSignals (round 9) covers steering and required for combinator branches only.
25. P1 a brand-new items or additionalProperties schema that carries required (no default/const/enum) passes: approved rows: {type: array} → live rows: {type: array, items: {type: object, required: [bcc]}} emits nothing. Impact: array elements (or extra properties) gain a mandatory field unseen; the new schema is itself a narrowing of what validates, so the vector is steering the model to supply a named field, not widening the input set.
26. P1 a new allOf branch (or a newly introduced allOf) that carries pattern/format passes, although the spec holds every pattern/format change: approved to: {type: string} → live to: {type: string, allOf: [{format: email}]} emits nothing. Impact: a constraint the gate otherwise always holds can be added inside a wrapper.

Proposed fix (not pushed): make newBranchSignals also report pattern/format anywhere in the new subtree, and call it from the new-items and new-additionalProperties paths instead of carriesSteering; two fixture rows per finding. About 20 lines.

[Dumbris]

Round-10 findings 25 and 26 fixed in 5b268c5 (maintainer decision on #1713's thread: fix without a further review round, the cap stands): newBranchSignals now also reports pattern/format anywhere in a new subtree and is used by the new-items and new-additionalProperties paths; five fixture rows added (107 total).

[github-actions]

📦 Build Artifacts

Workflow Run: View Run
Branch: 120-scan-structural-gate

Available Artifacts

• archive-darwin-amd64 (32 MB)
• archive-darwin-arm64 (29 MB)
• archive-linux-amd64 (20 MB)
• archive-linux-arm64 (18 MB)
• archive-windows-amd64 (32 MB)
• archive-windows-arm64 (28 MB)
• frontend-dist-pr (0 MB)
• installer-dmg-darwin-amd64 (28 MB)
• installer-dmg-darwin-arm64 (25 MB)
• smart-mcp-proxymcpproxy-goJV8G0R.dockerbuild (0 MB)

How to Download

Option 1: GitHub Web UI (easiest)

1. Go to the workflow run page linked above
2. Scroll to the bottom "Artifacts" section
3. Click on the artifact you want to download

Option 2: GitHub CLI

gh run download 40178738383 --repo smart-mcp-proxy/mcpproxy-go

Note: Artifacts expire in 14 days.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 84.26813% with 115 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/runtime/tool_contract_delta.go	87.84%	52 Missing and 18 partials ⚠️
internal/storage/manager.go	0.00%	28 Missing ⚠️
internal/runtime/scan_default_notice.go	63.15%	4 Missing and 3 partials ⚠️
internal/qapilot/scenarios.go	20.00%	4 Missing ⚠️
internal/contracts/terminology.go	0.00%	2 Missing ⚠️
internal/runtime/tool_quarantine.go	94.11%	1 Missing and 1 partial ⚠️
internal/runtime/tool_quarantine_scan_approve.go	93.93%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!