Skip to content

Commit d5a5a8f

Browse files
authored
feat(mcp): migrate the stdio MCP server's 102 tools to @loopover/contract (#9537) (#9565)
* feat(contract): add the stdio server's local-git tool contracts and converge the hand-mirrored inputs Adds the 19 tools the remote migration could not cover, which is every tool the stdio server registers that the remote one either does not have or declares differently. All 102 stdio tools now have a contract entry. Thirteen of these existed on both servers with two hand-mirrored input schemas that had already drifted -- the stdio copies of find_opportunities, retrieve_issue_context and mark_notifications_read carried no length bounds at all. The contract takes the wider bound in every such case, and resolves the eligibility-transform question #9518 documented rather than solved: the contract describes what a caller may SEND, so the pre-transform shape is the one that belongs here and the remote server's downgrade stays a server-side control over the parsed value. * feat(mcp): register every stdio tool from @loopover/contract All 102 stdio tools now take their title, description, category, annotations, and BOTH schemas from the registry. ~85 hand-mirrored input shapes and the 560-line STDIO_TOOL_DESCRIPTORS table are gone (the bin file drops ~1,100 lines), every handler is typed via z.infer instead of `(input: any)`, and 97 of the 102 gain a real output schema where they had none. The registration helper passes the schema OBJECTS, not their .shape. The SDK accepts either, but a raw shape is re-wrapped in a plain z.object, which discards the catchall -- so every output modelled as a looseObject would be advertised and enforced as additionalProperties:false, and any field the payload carries beyond the modelled set becomes a -32602 the caller cannot act on. Turning the output schemas on surfaced six real defects, which is what they are for: - the agent audit feed models pullNumber/actor/detail as nullable, but the route OMITS them rather than sending null, so every real feed failed validation; - plan_repo_issues and generate_contributor_issue_drafts declared their six counters required, but the service short-circuits to a countless disabled/unavailable posture -- which is exactly why the CLI proxies carry '?? 0' fallbacks; - three test fixtures described payloads the services do not produce (laneFit as a lane name rather than a fit score, autoMaintain as a string rather than the settings object, a pending-action row missing four ledger columns). Fixtures fixed, not schemas. Four input divergences converged, each by widening so no live caller breaks: get_repo_onboarding_pack gains stdio's 'refresh'; preflight_local_diff and explain_score_breakdown gain stdio's local-diff fields; and get_pr_ai_review_findings -- the one tool whose two servers disagreed on a FIELD NAME -- accepts both 'number' (canonical, and what every other PR-scoped tool uses) and 'pullNumber' (the remote's alias). list_pending_actions is the single deliberate narrowing: its route hardcodes status 'pending' and cannot honour a filter, so the stdio server registers ListPendingActionsStdioInput -- derived from the contract entry with .omit() -- rather than advertising a filter that would silently do nothing. Also restores .strict() on callerBranchEligibilitySchema, dropped when it was relocated in #9518: both servers wrapped it strictly before the migration, and without it a caller inventing an eligibility field is silently stripped instead of rejected. * fix(ci): stop import-specifiers:check reporting three false positives and one real miss The checker exits 1 on main today. Two of the three reports are its own false positives and the third is mine. - check-dead-source-files-script.test.ts embeds import-statement TEXT as string fixtures for its script's injectable readFile, and a js-suffixed relative specifier is the exact case that test exists to pin -- so its fixtures read as violations. Allowlisted, the same way this checker already allowlists its own test file for the same reason. - scripts/actionlint.ts runs under 'node --experimental-strip-types', whose loader does no extension resolution: the literal .ts is the only form that starts. That is a fact about how package.json invokes it, not something readable from the source, so the entrypoint is listed explicitly. - The three relative imports #9517/#9518 added to contract-registry.test.ts carried .js in a Bundler-resolved zone. Extension dropped. * feat(engine): collapse the duplicated plan-DAG state machine into @loopover/engine Completes #9537's last requirement, and fixes the build failure the first push of PR #9565 hit. THE BUILD FIX: `ToolContract` was imported from @loopover/contract/tools, which only imports that type internally and never re-exports it. The local typecheck resolves the package through a src alias and passed; the real package build resolves the export map and failed at 'Build MCP'. Now imported from the package root, which does export it. THE PLAN-DAG DEDUP: buildPlanDag/validatePlanDag/the step state machine existed in src/services/plan-dag.ts AND hand-copied, untyped, into the stdio MCP bin -- whose own comment explained why: it resolves @loopover/engine through the published package, whose export map did not surface them. It does now. Moving it turned up a THIRD partial copy already in the engine: plan-export.ts holds the step/plan types, plan-step-readiness.ts holds nextReadySteps/isDone, and plan-overall-status.ts holds the status vocabulary, each decomposed into its own module. The moved file imports all three rather than restating them -- a fourth PlanDag would have defeated the point -- and contributes only the mutating half (build, validate, advance) that had no engine home. src/services/plan-dag.ts stays as the Worker's import path, now a re-export, so nothing else had to move. Release ordering, which is why this looked blocked: both publish workflows are workflow_dispatch-only, so nothing auto-publishes on merge and an operator publishes the engine before the CLI, as usual. The engine takes a minor bump for the new export, with packages/loopover-miner/expected-engine.version and both consumers' dependency ranges moved to ^3.16.0 in the same commit so a published CLI can never resolve an engine without it. Also declares @loopover/contract#build as an edge of the root typecheck task. That was working only because ci.yml happens to run a contract build first; the edge makes a bare 'turbo run typecheck' correct on its own. * fix(mcp,typecheck): prune the imports the contract migration orphaned, and make the root typecheck catch them The stdio migration left ~40 imports and shape constants unreferenced once every tool started resolving its schemas through getToolContract. @loopover/mcp's own tsconfig sets noUnusedLocals, so its package build failed on all of them -- twice -- while 'npm run typecheck' and the full unit suite stayed green locally. That gap is the real bug, and it is fixed here rather than worked around: the root 'typecheck' script now runs the root tsconfig AND each package whose config is stricter than it (@loopover/contract, @loopover/mcp, @loopover/miner). A bare 'npm run typecheck' now covers the same surface CI's build steps do, so this class of failure cannot pass locally and fail in CI again. Also defaults preflight_local_diff's baseRef at the call site. It is optional in the shared contract input -- the remote server has no checkout to read -- so the local diff collector needs the 'HEAD' default the stdio-only shape used to bake into the schema. * chore(release): sync the release-please manifest to the engine's 3.16.0 bump The engine version bump that gives the stdio server a published plan-DAG export has to be reflected in .release-please-manifest.json, which release-please reads to decide what it is releasing. Caught by release-manifest:sync:check. * test(mcp): cover the pr-ai-review-findings field-name alias and its two caller-error paths codecov/patch flagged the new branches in getPrAiReviewFindings: every existing test supplies pullNumber, so the canonical 'number' side of the alias and both 'required field missing' throws were unexercised. * chore: move the #9520 validator files out of this PR They were swept in by a git add -A while both branches were in flight; they belong to the contract-validator issue, not the stdio migration. * test(engine): move the plan-DAG behavioural tests in with the implementation codecov/patch flagged packages/loopover-engine/src/plan-dag.ts at 40%: the engine uploads its own coverage from its own node:test suite, so the root suite's test/unit/plan-dag.test.ts -- which exercises the same code thoroughly through the Worker's re-export -- contributes nothing to it. The tests move with the implementation. 100% statements and branches on the moved module, verified with c8 against the engine's own dist.
1 parent 3bffb87 commit d5a5a8f

30 files changed

Lines changed: 1427 additions & 1946 deletions

.release-please-manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"packages/loopover-mcp": "3.15.2",
3-
"packages/loopover-engine": "3.15.3",
3+
"packages/loopover-engine": "3.16.0",
44
"packages/loopover-miner": "3.15.2",
55
"packages/loopover-ui-kit": "1.2.0"
66
}

package-lock.json

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,9 @@
9797
"changelog:check:mcp": "node --experimental-strip-types scripts/check-changelog.ts --mcp",
9898
"mcp:release-due": "tsx scripts/check-mcp-release-due.ts --json",
9999
"mcp:release-candidate": "tsx scripts/check-mcp-release-candidate.ts",
100-
"typecheck": "tsc --noEmit",
100+
"typecheck": "npm run typecheck:root && npm run typecheck:packages",
101+
"typecheck:root": "tsc --noEmit",
102+
"typecheck:packages": "tsc -p packages/loopover-contract/tsconfig.json --noEmit && tsc -p packages/loopover-mcp/tsconfig.json --noEmit && tsc -p packages/loopover-miner/tsconfig.json --noEmit",
101103
"check-node-version": "node --experimental-strip-types scripts/check-node-version.ts",
102104
"pretest": "npm run check-node-version",
103105
"test": "vitest run",

packages/loopover-contract/src/tools/agent.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,17 @@ export const ListPendingActionsOutput = z.looseObject({
598598
status: z.string().optional(),
599599
pendingActions: z.array(pendingActionEntrySchema).optional(),
600600
});
601+
/**
602+
* The stdio server's narrowed variant, DERIVED rather than restated.
603+
*
604+
* `GET /agent/pending-actions` takes no query parameters and hardcodes status "pending", so the
605+
* stdio server cannot honour the filter its remote counterpart offers. An agent reads the published
606+
* schema to decide what to send, so advertising a filter that silently does nothing is worse than
607+
* not advertising it -- this is the one place in the migration where a server deliberately serves
608+
* LESS than the contract, and it says so in code rather than by omission.
609+
*/
610+
export const ListPendingActionsStdioInput = ListPendingActionsInput.omit({ status: true });
611+
601612
export const listPendingActionsTool = defineTool({
602613
name: "loopover_list_pending_actions",
603614
title: "List pending actions",
@@ -644,12 +655,15 @@ export const GetAgentAuditFeedOutput = z.looseObject({
644655
repoFullName: z.string().optional(),
645656
events: z
646657
.array(
658+
// `.nullish()`, not `.nullable()`: the REST route this proxies OMITS these for an event that
659+
// has none rather than sending an explicit null, and modelling them as merely nullable made
660+
// every real audit feed fail output validation (#9537).
647661
z.looseObject({
648662
eventType: z.string(),
649-
pullNumber: z.number().nullable(),
663+
pullNumber: z.number().nullish(),
650664
outcome: z.string(),
651-
actor: z.string().nullable(),
652-
detail: z.string().nullable(),
665+
actor: z.string().nullish(),
666+
detail: z.string().nullish(),
653667
createdAt: z.string(),
654668
}),
655669
)

packages/loopover-contract/src/tools/branch.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ const validationEntrySchema = z.strictObject({
4949
// ── preflight local diff (input + output: no transform) ─────────────────────────────────────────
5050

5151
export const PreflightLocalDiffInput = PreflightPrInput.extend({
52+
// #9537: `cwd`/`baseRef` name a checkout only the stdio server can read; the remote server
53+
// ignores them. Widening the shared input is the safe direction.
54+
cwd: z.string().optional(),
55+
baseRef: z.string().optional(),
5256
changedLineCount: z.number().int().min(0).optional(),
5357
testFiles: z.array(z.string().max(PREFLIGHT_LIMITS.changedFileChars)).max(PREFLIGHT_LIMITS.changedFiles).optional(),
5458
commitMessage: z.string().max(PREFLIGHT_LIMITS.bodyChars).optional(),

packages/loopover-contract/src/tools/discovery-utility.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -484,12 +484,46 @@ export const validateConfigTool = defineTool({
484484
output: ValidateConfigOutput,
485485
});
486486

487-
export const LocalStatusInput = noInput;
487+
/**
488+
* A THIRD divergence, found while migrating the stdio server (#9537) -- one the issue did not name,
489+
* because it is not a payload that drifted but two different tools that collided on one name:
490+
*
491+
* - the remote server answers "what does this MCP endpoint support" (reachability, the supported
492+
* endpoint, the tool surface it advertises);
493+
* - the stdio server answers "what is the state of THIS CLI on THIS machine" (api url, package
494+
* version, token/session presence, workspace roots, and the local git checkout).
495+
*
496+
* Neither can answer the other's question -- the remote has no checkout to inspect, and the CLI has
497+
* no endpoint surface to report -- so unlike get_repo_context and get_pr_reviewability there is no
498+
* payload to converge on. The real fix is a rename, which breaks every caller of whichever side
499+
* loses the name, so it is filed rather than done in flight. The union below keeps both wires
500+
* working, gives both a validated schema instead of none, and keeps the collision visible.
501+
*
502+
* `cwd`/`baseRef`/`repoFullName` on the input are the stdio side's; the remote server ignores them.
503+
* Widening an input is always the safe direction.
504+
*/
505+
export const LocalStatusInput = z.object({
506+
cwd: z.string().optional(),
507+
baseRef: z.string().optional(),
508+
repoFullName: z.string().min(3).optional(),
509+
});
488510
export const LocalStatusOutput = z.looseObject({
511+
// Remote fields.
489512
apiAvailable: z.boolean().optional(),
490-
sourceUploadDefault: z.boolean().optional(),
491513
supportedEndpoint: z.string().optional(),
492514
supportedTools: z.unknown().optional(),
515+
// Shared.
516+
sourceUploadDefault: z.boolean().optional(),
517+
// stdio fields.
518+
apiUrl: z.string().optional(),
519+
package: z.looseObject({ name: z.string(), version: z.string() }).optional(),
520+
hasToken: z.boolean().optional(),
521+
profile: z.record(z.string(), z.unknown()).optional(),
522+
authLogin: z.string().nullable().optional(),
523+
sessionExpiresAt: z.string().nullable().optional(),
524+
sourceUploadSupported: z.boolean().optional(),
525+
workspaceRoots: z.unknown().optional(),
526+
git: z.record(z.string(), z.unknown()).optional(),
493527
});
494528
export const localStatusTool = defineTool({
495529
name: "loopover_local_status",

packages/loopover-contract/src/tools/index.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,27 @@ import {
116116
agentStartRunTool,
117117
agentGetRunTool,
118118
} from "./agent.js";
119+
import {
120+
preflightCurrentBranchTool,
121+
previewCurrentBranchScoreTool,
122+
rankLocalNextActionsTool,
123+
explainLocalBlockersTool,
124+
remediationPlanTool,
125+
preparePrPacketTool,
126+
agentPreparePrPacketTool,
127+
reviewPrBeforePushTool,
128+
draftPrBodyTool,
129+
compareLocalVariantsTool,
130+
previewLocalPrScoreTool,
131+
getEligibilityPlanTool,
132+
comparePrVariantsTool,
133+
feasibilityGateTool,
134+
markNotificationsReadTool,
135+
watchIssuesTool,
136+
findOpportunitiesTool,
137+
retrieveIssueContextTool,
138+
simulateOpenPrPressureTool,
139+
} from "./local-branch.js";
119140

120141
/**
121142
* #9517's pilot batch, the full AMS miner server (#9536, all 11 tools), and the remote server's
@@ -218,6 +239,25 @@ export const TOOL_CONTRACTS: readonly ToolContract[] = [
218239
agentExplainNextActionTool,
219240
agentStartRunTool,
220241
agentGetRunTool,
242+
preflightCurrentBranchTool,
243+
previewCurrentBranchScoreTool,
244+
rankLocalNextActionsTool,
245+
explainLocalBlockersTool,
246+
remediationPlanTool,
247+
preparePrPacketTool,
248+
agentPreparePrPacketTool,
249+
reviewPrBeforePushTool,
250+
draftPrBodyTool,
251+
compareLocalVariantsTool,
252+
previewLocalPrScoreTool,
253+
getEligibilityPlanTool,
254+
comparePrVariantsTool,
255+
feasibilityGateTool,
256+
markNotificationsReadTool,
257+
watchIssuesTool,
258+
findOpportunitiesTool,
259+
retrieveIssueContextTool,
260+
simulateOpenPrPressureTool,
221261
minerPingTool,
222262
minerPortfolioDashboardTool,
223263
minerManageStatusTool,
@@ -259,4 +299,5 @@ export * from "./review.js";
259299
export * from "./branch.js";
260300
export * from "./discovery-utility.js";
261301
export * from "./agent.js";
302+
export * from "./local-branch.js";
262303
export * from "./miner.js";

0 commit comments

Comments
 (0)