Skip to content

Commit 95f1524

Browse files
authored
feat(contract): add @loopover/contract, the single zod source for tool and API schemas (#9530)
* feat(contract): add @loopover/contract, the single zod source for tool and API schemas LoopOver declares the same contracts in four unshared places: src/openapi/schemas.ts (responses, spec-only), src/api/routes.ts (request bodies), src/mcp/server.ts (tool shapes), and packages/loopover-mcp/bin/loopover-mcp.ts (~85 shapes whose own comments say they mirror the remote server's). This package is the one place they can live. It is a zod-only leaf with no node builtins, which is the property that lets the Worker, both published stdio bins, the miner, and the UI depend on it. Sharing these through @loopover/engine was rejected once already (#6153): @loopover/mcp resolves the engine through its published export map, which never surfaced the enums, so importing them would have meant widening the engine's public API. The model carries schemas ON each entry rather than in a name-keyed side map, so a tool without an output schema is a type error rather than a silently dropped lookup, and projectToolDefinitions is the single point every consumer derives from -- MCP registration, the OpenAI/Anthropic spec builders that #9183/#9184 need, and the generated docs to come. Each entry declares auth, locality, and availability; locality is what makes explicit why LoopOver cannot collapse to one MCP process. Six pilot contracts, modelled from the engine types the handlers actually return rather than from the placeholder schemas they advertise today -- the remote server declared six of get_repo_context's eight fields, and all of predict_gate's blockers, warnings, and funnel, as bare z.unknown(). Writing them surfaced two divergences that are modelled honestly as unions and left for #9518 to converge: get_repo_context and get_pr_reviewability return DIFFERENT payloads from the remote and stdio servers. Input bounds take the wider of the two servers' historical limits where they disagreed, since a shared contract may widen an input but never tighten one. Refs #9517 * feat(mcp): register the pilot tools from the shared contract, and fix three drifted OpenAPI schemas Both MCP servers now take the six pilot tools' input AND output schemas from @loopover/contract instead of declaring them locally. The stdio server's hand-mirrored shapes are gone for these tools, and four of them gain a real outputSchema for the first time -- previously only loopover_local_status_structured declared one anywhere in that package. Fixes found while modelling the real payloads, all cases of the published spec disagreeing with what the code actually returns: - CollisionItemSchema's `type` enum omitted "recent_merged_pull_request", which buildCollisionReport genuinely emits (it is handed recent merged PRs). A client validating a real response against the spec rejected it. Its labels, linkedIssues, linkedIssueClaimedAt, changedFiles and body fields were missing too. - QueueHealthSchema omitted draftPullRequests, slopFlaggedPullRequests and duplicateFlaggedPullRequests -- all required on the QueueHealth type and always emitted -- plus the optional rankedPullRequests array. openapi.json is regenerated to match. Refs #9517 * build: emit @loopover/contract before anything that typechecks or bundles src/ src/mcp/server.ts and packages/loopover-mcp both import @loopover/contract, whose package exports resolve to dist/ -- so every surface that typechecks or bundles src/ needs it emitted first, and nothing did. CI's Typecheck and UI typecheck failed on exactly this, and the selfhost image would have failed at esbuild resolution: the Dockerfile builds the engine but had no step for contract, so build-selfhost.ts could not have resolved the import. Adds the build to ci.yml (before the engine step), the Dockerfile, selfhost.yml and release-selfhost.yml. Unconditional rather than path-gated: every consumer (backend, mcp, miner, ui) can pull it into the typecheck surface, and a zod-only leaf with no workspace dependencies is cheap enough that gating buys nothing but a class of skipped-build failures. Same shape as the engine build-order fix, applied before that step for the same reason. Verified by running build-selfhost.ts --all against a clean contract build. Refs #9517 * fix(mcp): repair test fixtures and bounds that drifted from the real payload Rebasing #9530 onto main after the engine-build-order fix (#9516) surfaced tests whose fixtures no longer matched what the tools' new output schemas actually require -- each one a real drift the placeholder z.unknown() schemas had been hiding, not a false positive from the schema itself. mcp-cli-plan-scorer-tools.test.ts and mcp-cli-harness.ts's shared predictedGate fixture omitted predicted/basis/funnel/note, all required on PredictedGateVerdict and always emitted by buildPredictedGateVerdict -- the fixture had silently drifted since #9517's placeholder schema never caught it. mcp-pr-reviewability.test.ts persisted partial cached snapshot payloads (missing score/action/noiseSources/whyThisHelps/maintainerNextSteps/privateSummary), but src/api/routes.ts's reviewability route always persists the full computed object -- made the three fixtures match what production actually writes. One genuine schema gap this surfaced: generatedAt needed to become optional in the report schema, because the remote handler's own fallback chain (cached.generatedAt || payload.generatedAt || new Date().toISOString()) proves a stored payload's generatedAt is not guaranteed -- the schema now describes that real, defended-against case instead of only the freshly-computed one. mcp-predict-gate.test.ts pinned the OLD 300-char per-path bound the remote server used before #9517 unified it to 400 (the wider of the two servers' historical limits, per that issue's documented never-tighten-an-input rule) -- updated to exercise the new bound rather than reverting it. Full unit suite: 23041 tests, 1208 files, all green. * fix(coverage): alias @loopover/contract to source so patch coverage attributes correctly @loopover/contract is consumed via its package specifier everywhere, which resolves through node_modules' exports map straight to compiled dist/*.js -- unlike packages/loopover-mcp/miner's RELATIVE ../lib/foo.js imports, which fall back to a sibling .ts when nothing exists at the .js path. A package-specifier resolution that already finds a real compiled file has no such fallback, so v8 instrumented dist/ (gitignored, invisible to Codecov) instead of packages/loopover-contract/src/**/*.ts. Confirmed live: #9530 reported 0% patch coverage on fully-tested contract source. Aliased both import specifiers straight to source. The /tools subpath alias has to be declared BEFORE the bare @loopover/contract entry: Vite's string-find alias matcher treats a plain string as matching both the exact specifier and anything starting with find + "/", first-match-wins in declaration order -- with the bare entry first it silently intercepted the /tools import too and rewrote it to a bogus path, breaking resolution outright. Found by reproducing the failure with a throwaway probe test before reordering, not assumed. Verified: a scoped coverage run against contract-registry.test.ts + the pilot tool tests now shows 100% statement/branch/function/line coverage on the contract package, where before the alias it was flat 0%.
1 parent 8751b9a commit 95f1524

33 files changed

Lines changed: 1702 additions & 36 deletions

.github/workflows/ci.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,14 @@ jobs:
467467
key: turbo-code-${{ hashFiles('package-lock.json') }}-${{ github.run_id }}
468468
restore-keys: |
469469
turbo-code-${{ hashFiles('package-lock.json') }}-
470+
# @loopover/contract's "types" resolve to packages/loopover-contract/dist/index.d.ts, and src/ +
471+
# packages/loopover-mcp both import it -- so like the engine below, typecheck cannot run until it
472+
# has been built. Unconditional rather than gated on a `contract` path filter: every consumer of it
473+
# (backend, mcp, miner, ui) can pull it into the typecheck surface, and building a zod-only leaf
474+
# package with no dependencies of its own is cheap enough that gating it would buy nothing but a
475+
# class of skipped-build failures.
476+
- name: Build contract package
477+
run: npx turbo run build --filter=@loopover/contract
470478
# mcp/miner are in this gate because "Typecheck" below already runs for them, and typecheck's real
471479
# surface reaches @loopover/engine -- test/** imports it directly, and its "types" resolve to
472480
# packages/loopover-engine/dist/index.d.ts, which only exists once this step has run. An mcp-only or

.github/workflows/release-selfhost.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,11 @@ jobs:
129129
# That package's dist/ is gitignored and only exists after this build step -- the regular CI smoke
130130
# test's narrower (non --all) build never hits this import chain, so it never caught the gap that
131131
# ci.yml's own validate-code job hit for ordinary backend PRs (fixed there separately).
132+
# Built before the engine for the same reason the Dockerfile does: src/'s import graph reaches
133+
# @loopover/contract, whose package exports resolve to dist/, so anything type-checking or
134+
# bundling src/ needs it emitted first. Zod-only leaf, no workspace dependencies of its own.
135+
- name: Build contract package
136+
run: npm run build --workspace @loopover/contract
132137
- name: Build engine package
133138
run: npm run build --workspace @loopover/engine
134139

.github/workflows/selfhost.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,11 @@ jobs:
7777
# dist/ is gitignored and only exists after this build step, so the test fails to resolve the
7878
# package's exports without it -- this workflow never needed the engine package built before, so it
7979
# never had this step; it does now.
80+
# Built before the engine for the same reason the Dockerfile does: src/'s import graph reaches
81+
# @loopover/contract, whose package exports resolve to dist/, so anything type-checking or
82+
# bundling src/ needs it emitted first. Zod-only leaf, no workspace dependencies of its own.
83+
- name: Build contract package
84+
run: npm run build --workspace @loopover/contract
8085
- name: Build engine package
8186
run: npm run build --workspace @loopover/engine
8287

Dockerfile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ COPY . .
1919
# --ignore-scripts: no native builds are needed (SQLite is the built-in node:sqlite; @hono/node-server is
2020
# pure JS; esbuild ships its binary as an optional dependency, not a script).
2121
RUN npm ci --ignore-scripts
22+
# @loopover/contract before the engine: src/ imports it, and its package exports resolve to dist/,
23+
# so esbuild in build-selfhost.ts below cannot resolve the import until it has been emitted. A
24+
# zod-only leaf with no workspace dependencies, so it builds first and standalone.
25+
RUN npm --workspace @loopover/contract run build
2226
RUN npm --workspace @loopover/engine run build
2327
# --all: bundle every dependency into one self-contained dist/server.mjs, so the runtime image needs no
2428
# node_modules (≈10× smaller). The bundle has zero `cloudflare:*` imports (stubbed at build), so no loader.

apps/loopover-ui/public/openapi.json

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1417,15 +1417,27 @@
14171417
"sampled_cache",
14181418
"authoritative"
14191419
]
1420+
},
1421+
"draftPullRequests": {
1422+
"type": "number"
1423+
},
1424+
"slopFlaggedPullRequests": {
1425+
"type": "number"
1426+
},
1427+
"duplicateFlaggedPullRequests": {
1428+
"type": "number"
14201429
}
14211430
},
14221431
"required": [
14231432
"openIssues",
14241433
"openPullRequests",
14251434
"unlinkedPullRequests",
14261435
"stalePullRequests",
1436+
"draftPullRequests",
14271437
"maintainerAuthoredPullRequests",
14281438
"collisionClusters",
1439+
"slopFlaggedPullRequests",
1440+
"duplicateFlaggedPullRequests",
14291441
"ageBuckets",
14301442
"likelyReviewablePullRequests"
14311443
]
@@ -1435,6 +1447,32 @@
14351447
"items": {
14361448
"$ref": "#/components/schemas/Finding"
14371449
}
1450+
},
1451+
"rankedPullRequests": {
1452+
"type": "array",
1453+
"items": {
1454+
"type": "object",
1455+
"properties": {
1456+
"number": {
1457+
"type": "number"
1458+
},
1459+
"title": {
1460+
"type": "string"
1461+
},
1462+
"authorLogin": {
1463+
"type": "string"
1464+
},
1465+
"recommendation": {
1466+
"type": "string"
1467+
}
1468+
},
1469+
"required": [
1470+
"number",
1471+
"title",
1472+
"authorLogin",
1473+
"recommendation"
1474+
]
1475+
}
14381476
}
14391477
},
14401478
"required": [
@@ -1527,7 +1565,8 @@
15271565
"type": "string",
15281566
"enum": [
15291567
"issue",
1530-
"pull_request"
1568+
"pull_request",
1569+
"recent_merged_pull_request"
15311570
]
15321571
},
15331572
"number": {
@@ -1543,6 +1582,32 @@
15431582
"htmlUrl": {
15441583
"type": "string",
15451584
"nullable": true
1585+
},
1586+
"labels": {
1587+
"type": "array",
1588+
"items": {
1589+
"type": "string"
1590+
}
1591+
},
1592+
"linkedIssues": {
1593+
"type": "array",
1594+
"items": {
1595+
"type": "number"
1596+
}
1597+
},
1598+
"linkedIssueClaimedAt": {
1599+
"type": "string",
1600+
"nullable": true
1601+
},
1602+
"changedFiles": {
1603+
"type": "array",
1604+
"items": {
1605+
"type": "string"
1606+
}
1607+
},
1608+
"body": {
1609+
"type": "string",
1610+
"nullable": true
15461611
}
15471612
},
15481613
"required": [

package-lock.json

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

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@
124124
"@asteasolutions/zod-to-openapi": "^8.5.0",
125125
"@cloudflare/puppeteer": "^1.1.0",
126126
"@hono/node-server": "^2.0.11",
127+
"@loopover/contract": "^0.1.0",
127128
"@loopover/engine": "*",
128129
"@modelcontextprotocol/sdk": "1.29.0",
129130
"@octokit/core": "^7.0.6",
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
# @loopover/contract
2+
3+
The single zod source of truth for LoopOver's MCP tool and API contracts.
4+
5+
LoopOver runs three MCP servers — the hosted/self-host remote server (`src/mcp/server.ts`), the
6+
stdio contributor wrapper (`@loopover/mcp`), and the AMS miner server (`@loopover/miner`) — plus a
7+
REST API and a UI that all describe the same data. Before this package, each of those declared its
8+
own zod shapes, and the copies drifted: the stdio server's shapes were hand-mirrored from the remote
9+
server's (their own comments said so), enum literals were hand-copied out of the engine, and
10+
responses were consumed as `any`.
11+
12+
This package is the one place those contracts live. **A shape declared here is never restated
13+
elsewhere.**
14+
15+
## Why a separate package
16+
17+
It is a **leaf**: its only runtime dependency is `zod`, and it imports no node builtins, so it is
18+
safe in the Cloudflare Workers bundle. That is what lets every surface depend on it — the Worker,
19+
both published stdio bins, the miner, the control plane, and the UI — without dragging the engine
20+
along behind it. Sharing these schemas through `@loopover/engine` was considered and rejected:
21+
`@loopover/mcp` resolves the engine through its *published* export map, which never surfaced the
22+
enums, so importing them would have meant widening the engine's public API (#6153).
23+
24+
## Layout
25+
26+
| Path | Holds |
27+
|---|---|
28+
| `src/tool-definition.ts` | The `ToolContract` model, `defineTool`, and `projectToolDefinitions` — the single projection point |
29+
| `src/tools/*.ts` | One file per tool family; the contracts themselves |
30+
| `src/tools/index.ts` | `TOOL_CONTRACTS`, `listToolDefinitions()`, `getToolContract()` |
31+
| `src/enums.ts` | Shared enum vocabularies (autonomy levels, action classes, …) |
32+
| `src/shared.ts` | Shapes reused by **three or more** contracts |
33+
| `src/agent-specs.ts` | OpenAI / Anthropic / agent-index projections |
34+
35+
## Conventions
36+
37+
These are enforced by meta-tests in `test/unit/contract-registry.test.ts`, not just documented.
38+
39+
**Naming.** One file per tool family. Within it, export `<ToolNamePascal>Input` and
40+
`<ToolNamePascal>Output` schemas plus the `defineTool(...)` contract. Derive types with
41+
`z.infer<typeof X>` — never hand-write an interface that mirrors a schema.
42+
43+
**Inputs are closed; outputs are open.** Input schemas use `z.object`, which emits
44+
`additionalProperties: false`. Output schemas use `z.looseObject`, which emits open
45+
`additionalProperties`. An MCP output schema is a *floor*, not a fence: a server that starts
46+
returning an extra field must not retroactively invalidate a client validating against the older
47+
schema.
48+
49+
> **Known gap:** zod's `z.object` *strips* unknown keys at runtime rather than rejecting them, so a
50+
> typo'd argument is silently dropped even though the advertised JSON Schema says it should be
51+
> refused. Switching to `z.strictObject` would close the gap but is a wire-visible tightening, so it
52+
> is a recorded decision on #9518 rather than a drive-by change. A meta-test pins the current
53+
> behavior so the switch cannot happen by accident.
54+
55+
**Output schemas may be shallower than their REST counterparts, and that is deliberate.** Reusing a
56+
strict REST response schema for an MCP tool *tightens* the wire contract and is a regression — the
57+
exact constraint metagraphed hit during its own migration. Reuse a REST schema only when it is
58+
field-for-field equal to what the tool actually returns. What is never acceptable is a top-level
59+
`z.unknown()` standing in for a real object.
60+
61+
**Hoist to `shared.ts` at the third consumer, not the second.** Two contracts sharing fields today
62+
is usually coincidence; coupling them early means a later divergence has to be un-shared under
63+
pressure.
64+
65+
**Metadata is a declaration, not a hint.** Every contract states its `auth`, `locality`, and
66+
`availability`, and runtimes enforce them:
67+
68+
- `locality` — where the state physically lives (`remote`, `local-git`, `miner`). This is why
69+
LoopOver cannot collapse to one MCP process: `local-git` tools read the caller's uncommitted
70+
working tree and `miner` tools read the miner box's stores, neither reachable from a Worker.
71+
- `availability``cloud`, `selfhost`, or `both`. Self-host-only tools depend on capabilities the
72+
Workers bundle cannot provide (fs-backed config, a redeploy socket).
73+
- `auth` — the identity kind `src/auth/security.ts` must authenticate before the tool runs.
74+
75+
**Nothing reads `TOOL_CONTRACTS` directly.** Consumers call `listToolDefinitions()` (optionally
76+
filtered), so cross-cutting concerns are applied exactly once.
77+
78+
## Adding a tool
79+
80+
1. Add `src/tools/<family>.ts` with input + output schemas and a `defineTool(...)` entry.
81+
2. Export it from `src/tools/index.ts`.
82+
3. Register it in whichever runtimes can serve its locality, using `contract.input.shape` /
83+
`contract.output.shape` for the MCP SDK.
84+
4. The contract validator (#9520) requires a smoke call per tool — a tool with no call fails CI.
85+
86+
Generated docs, agent tool specs, and the tool-reference tables pick it up automatically. If you
87+
find yourself hand-editing a tool table, that table is a bug.
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
{
2+
"name": "@loopover/contract",
3+
"version": "0.1.0",
4+
"license": "AGPL-3.0-only",
5+
"type": "module",
6+
"description": "Single zod source of truth for LoopOver's MCP tool and API contracts — schemas, tool metadata, and the projections every server and client derives from.",
7+
"repository": {
8+
"type": "git",
9+
"url": "git+https://github.com/JSONbored/loopover.git",
10+
"directory": "packages/loopover-contract"
11+
},
12+
"homepage": "https://github.com/JSONbored/loopover#readme",
13+
"bugs": {
14+
"url": "https://github.com/JSONbored/loopover/issues"
15+
},
16+
"keywords": [
17+
"loopover",
18+
"mcp",
19+
"model-context-protocol",
20+
"zod",
21+
"openapi",
22+
"schema"
23+
],
24+
"publishConfig": {
25+
"access": "public"
26+
},
27+
"main": "dist/index.js",
28+
"types": "dist/index.d.ts",
29+
"exports": {
30+
".": {
31+
"types": "./dist/index.d.ts",
32+
"default": "./dist/index.js"
33+
},
34+
"./enums": {
35+
"types": "./dist/enums.d.ts",
36+
"default": "./dist/enums.js"
37+
},
38+
"./tools": {
39+
"types": "./dist/tools/index.d.ts",
40+
"default": "./dist/tools/index.js"
41+
},
42+
"./agent-specs": {
43+
"types": "./dist/agent-specs.d.ts",
44+
"default": "./dist/agent-specs.js"
45+
},
46+
"./package.json": "./package.json"
47+
},
48+
"files": [
49+
"dist",
50+
"CHANGELOG.md"
51+
],
52+
"scripts": {
53+
"build": "tsc -p tsconfig.json"
54+
},
55+
"dependencies": {
56+
"zod": "^4.4.3"
57+
},
58+
"devDependencies": {
59+
"typescript": "^5.9.3"
60+
},
61+
"engines": {
62+
"node": ">=22.0.0 <23.0.0"
63+
}
64+
}

0 commit comments

Comments
 (0)