feat(contract): migrate every remote MCP tool contract to @loopover/contract (#9518) - #9559
Conversation
…opover/contract squashed; rewritten below
…ontract Completes #9518. All seven remote-server categories -- admin, maintainer, review, branch, discovery, utility and agent -- now take their input and output schemas from the contract package instead of ~120 declarations local to src/mcp/server.ts. The remote server is the second of the three to be migrated to completion, after the AMS miner (#9542). A handful of inputs deliberately stay server-side, each for a stated reason: schemas carrying a .transform() or .default() (a caller must not be able to assert its own branch eligibility into its own score), one input shared verbatim with a REST route, and three bounded by constants a zod-only leaf package cannot import. Their outputs migrate either way. Two fixes fall out of the move: - The changed-file and validation-entry schemas behind loopover_run_local_scorer and loopover_preflight_local_diff, and both nested schemas behind loopover_suggest_boundary_tests, lost their .strict() in an earlier batch of this migration. That strictness is the no-upload boundary: without it a caller that smuggles a patch/content field gets a silently-stripped call instead of a rejected one, and believes the upload succeeded. Restored, with regression tests on both surfaces. - PLAN_STEP_STATUSES said 'in_progress' where both real surfaces -- the remote plan DAG and the miner's plan store -- say 'running'. Nothing consumed it yet, so nothing broke, but the first consumer would have rejected every running step the store has ever persisted. Corrected, aliased from the miner contract so there is one vocabulary, and pinned by a test. The REST<->MCP parity guards now read the contract rather than server-local declarations that no longer exist. Tool descriptions are relocated verbatim.
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-28 09:26:21 UTC
Review summary Nits — 6 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionPartially addressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | dde50d1 | Commit Preview URL Branch Preview URL |
Jul 28 2026, 09:07 AM |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9559 +/- ##
==========================================
+ Coverage 89.60% 89.62% +0.01%
==========================================
Files 861 866 +5
Lines 110629 110784 +155
Branches 26334 26334
==========================================
+ Hits 99133 99288 +155
Misses 10231 10231
Partials 1265 1265
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Bundle ReportBundle size has no change ✅ |
![loopover-orb[bot]](https://avatars.githubusercontent.com/in/4139483?s=60&v=4){kind=small}
1 participant
Closes #9518.
What changed
Every tool the remote MCP server registers now takes its input and output schema from
@loopover/contractinstead of a declaration local tosrc/mcp/server.ts. All seven categories are done —admin,maintainer,review,branch,discovery,utility,agent— which makes the remote server the second of the three to be migrated to completion, after the AMS miner (#9542). ~120 shape/output-schema declarations and their supporting sub-schemas are deleted from the server; what remains there is the handler logic and the auth checks.New contract families:
maintainer.ts(27 tools),review.ts(12),branch.ts(13),discovery-utility.ts(22),agent.ts(29), plus theadminadditions.What deliberately did not move
A shared contract describes the wire shape a caller may send; the emitted JSON Schema cannot round-trip a runtime coercion. So these inputs stay server-side and only their outputs migrate:
callerBranchEligibilitySchema— a.transform()that downgrades a caller-claimedeligibletounknown. Relocating it would have advertised the post-transform shape and silently dropped the downgrade, letting a caller assert its own eligibility into its own score. That isexplain_score_breakdown,get_eligibility_plan, and elevenbranchtools.watch_issues, whose input carries.default("list").simulate_open_pr_pressure, whose input is exported and reused verbatim byPOST /v1/lint/open-pr-pressure(REST + CLI mirror for loopover_simulate_open_pr_pressure #6751) and is built from.passthrough()sub-schemas.find_opportunities,retrieve_issue_contextandmark_notifications_read, bounded by constants owned by modules a zod-only leaf package cannot import.Tool descriptions are relocated verbatim — a description is part of the surface an agent selects on, so rewording one would be a behaviour change wearing a refactor's clothes.
Two bugs fixed on the way
The no-upload boundary had been quietly disabled. The changed-file and validation-entry schemas behind
loopover_run_local_scorer/loopover_preflight_local_diff, and both nested schemas behindloopover_suggest_boundary_tests, lost their.strict()in an earlier batch of this same migration. That strictness is the boundary: with it, a caller that smuggles apatch/contentfield gets a rejected call; without it, zod v4 strips the field and returns success, so an agent uploading source text is told it worked. Restored on all four, with regression tests on both surfaces.test/unit/mcp-suggest-boundary-tests.test.ts's existing case is what caught it.PLAN_STEP_STATUSESnamed a state that does not exist. It saidin_progresswhere both real surfaces — the remote plan DAG and the miner's plan store — sayrunning. Nothing consumed it yet, so nothing broke, but the first consumer would have rejected every running step the store has ever persisted. Corrected, aliased from the miner contract so there is exactly one vocabulary, and pinned by a test that also asserts the plan-level statuses are a subset.Validation
npx vitest run test/unit test/contract— 23361 passed, 6 skipped, 0 failed.npx tsc --noEmitclean across the workspace.ui:openapi:check,docs:drift-check,manifest:drift-check,engine-parity:drift-check,build:mcpall green.openapi.jsonregenerates byte-identical, which is the intended result: this is a relocation, not a surface change.test/unit/contract-registry.test.tsforSCENARIO_LIMITS(against the server's live values) and the plan-step vocabulary.test/unit/openapi.test.tsnow read the contract instead of server-local exports that no longer exist; their assertions are unchanged.