mcp: enforce that a server may only NARROW the contract, and pin the last four restatements (#9662)

[JSONbored]

Closes #9662, #9661 and #9660.

Stacked on #9815 → #9812. Review the third commit; this rebases onto main as they merge.

The rule nothing enforced

registerStdioTool's override is documented as one-way — "a server may serve LESS than the contract when its own route cannot honour a field… never used to widen" — and overrides.input is typed as any z.ZodObject at all. No existing check could see a violation: diffToolSets compares name sets, checkAdvertisedShape asks only whether the advertised input is object-typed, and the smoke arguments are synthesized from the advertised schema itself, so a widened schema simply gets widened arguments and passes.

checkInputNarrowing closes that, for all three surfaces.

What it found on its first run

Not hypothetical — five hand-written shapes in src/mcp/server.ts, feeding ten tools that advertised eight input properties their contract never declared, plus two that demanded a field the catalog calls optional:

loopover_preflight_current_branch advertises input property baseSha, which its contract does not declare
loopover_preflight_current_branch requires input property login, which its contract does not require
… ×10 tools

So listToolDefinitions() — and with it the OpenAI/Anthropic spec builders and the .well-known catalogs — described a tool that rejects what the remote accepts, and a caller following the published schema got a -32602.

The fix is the rule the epic already states

The contract holds the wider surface; each server declares its narrowing there, not in a literal beside its registration:

Contract	Serves
LocalBranchAnalysisInput (was localBranchAnalysisShape, 8 tools)	remote takes it whole; stdio narrows to CurrentBranchInput — it reads the shas, the diff and the scorer probe off the checkout rather than taking them from the caller
LocalScorePreviewInput (union of scorePreviewShape + LocalScoreInput)	remote narrows to RemoteLocalScorePreviewInput — no cwd, no scorer command, because it has no checkout to run them against
MarkNotificationsReadInput.login / WatchIssuesInput.login now required	stdio narrows via Stdio*Input — it resolves the login from the active session

Deleted from the server: localBranchAnalysisShape, scorePreviewShape, linkedIssueContextShape, markNotificationsReadShape, watchIssuesShape, callerBranchEligibilitySchema, branchEligibilityShape, focusManifestInputSchema and isJsonByteLengthWithinLimit — 173 lines, all of them the hand-maintained kind this epic exists to remove.

#9661 — a lock that asserted a value against itself

checkVersionLock was called with serverInfoVersion: packageVersion — the same expression — for the one leg its own doc calls "the one that can drift". Six lines later the same file warns about exactly this: "a version lock that only compares constants to each other stays green while the thing that is supposed to update them has stopped running."

Both legs are read off a connected client now (client.getServerVersion()), the miner server gains its own lock (it was unlocked entirely), and an absent version is its own failure rather than being reported as a mismatch — an empty string must not read as a version. The remote's hardcoded 0.1.0 stays out of the lock deliberately; making it derivable is #9526's.

The RFAIL debug loop and the duplicated comment paragraph that shipped with #9520 are deleted.

#9660 — the last four unpinned restatements

TEST_FRAMEWORKS, QUEUE_STATUSES, CLAIM_STATUSES and MINER_RUN_STATES move to enums.ts (three of them lived in tools/miner.ts, outside even the convention that a shared vocabulary lives in one file) and gain pins against their live sources. Two appear in output schemas, where a new value makes a tool's real structuredContent fail the schema that same tool advertises.

Pinning the run states required a fix first: the miner's RUN_STATES was a cast over a literal with RunState written out separately, so adding a state to one and not the other compiled fine. The type is derived from the list now, which is what makes the pin mean anything.

FEASIBILITY_VERDICTS is imported at the two places it was re-inlined inside the package whose stated purpose is removing hand-copies.

And the epic's headline outcome

A second commit finishes the job the first one started: grep -cE "^const \w+Shape = \{" src/mcp/server.ts now returns 0.

Four shapes were still a registration's inputSchema (issueRagShape, findOpportunitiesShape, localBranchVariantsShape, variantsShape) — agreeing with their contract entries only by history, with nothing keeping them agreeing. The other fifteen had stopped being registered but survived as the type of their handler (z.infer<z.ZodObject<typeof preflightShape>>), which is the same defect one step removed: a handler typed off a hand-written shape can drift from the contract its tool registers with, and the compiler is happy either way.

Both variant tools also took the narrower element type — ComparePrVariantsInput.variants was an array of the stdio server's LocalScoreInput while the remote's own handler expected the wider one, so the remote accepted variants its registry entry rejected. The element is the union now, with Stdio* narrowings, exactly as their singular siblings got.

Validation

• npx vitest run --changed=origin/main: 2991 passed, 194 files, with zero hand-written shapes left in the server. tsc --noEmit clean.
• validate:mcp green across all four surfaces: remote (139 tools with admin), stdio (102), miner (21).
• checkInputNarrowing unit-tested for: agreement, serving strictly less, an undeclared property, an optional field demanded, and the two quiet cases. checkVersionLock for all three agreeing, each leg drifting, an absent version, and the server label.
• docs:drift-check, manifest:drift-check, ui:openapi:check, command-reference:check, contract:api-schemas:check all clean (the last regenerated to pick up feat(gate): gate.ignoredCheckRuns — exclude a third-party check from CI resolution entirely #9813's ignoredCheckRuns on the settings response).

[loopover-orb]

Important

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏳ LoopOver is waiting…

LoopOver has seen this pull request and is waiting on CI checks to finish before reviewing it. This comment will update once the review runs.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟨 Waiting}

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.64%. Comparing base (f881645) to head (b077b0f).
⚠️ Report is 5 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9833   +/-   ##
=======================================
  Coverage   91.64%   91.64%           
=======================================
  Files         916      916           
  Lines      112761   112734   -27     
  Branches    27077    27077           
=======================================
- Hits       103341   103319   -22     
+ Misses       8130     8126    -4     
+ Partials     1290     1289    -1     

Flag	Coverage Δ
backend	95.67% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
packages/loopover-contract/src/api-schemas.ts	100.00% <ø> (ø)
packages/loopover-contract/src/enums.ts	100.00% <100.00%> (ø)
packages/loopover-contract/src/tools/agent.ts	100.00% <ø> (ø)
packages/loopover-contract/src/tools/branch.ts	100.00% <100.00%> (ø)
...ckages/loopover-contract/src/tools/local-branch.ts	100.00% <100.00%> (ø)
packages/loopover-contract/src/tools/miner.ts	100.00% <ø> (ø)
packages/loopover-mcp/bin/loopover-mcp.ts	69.95% <ø> (ø)
packages/loopover-miner/lib/run-state.ts	100.00% <100.00%> (ø)
src/mcp/server.ts	97.66% <ø> (+0.30%)	⬆️