feat(contract): add @loopover/contract, the single zod source for tool and API schemas - #9530
Merged
Merged
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 6192bad | Commit Preview URL Branch Preview URL |
Jul 28 2026, 07:59 AM |
Contributor
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-28 08:16:46 UTC
Review summary Nits — 7 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. Decision record
Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy. Scroll preview
A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show. 🟩 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.
|
5 tasks
Contributor
|
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 #9530 +/- ##
=======================================
Coverage 89.58% 89.59%
=======================================
Files 846 858 +12
Lines 110384 110452 +68
Branches 26295 26298 +3
=======================================
+ Hits 98889 98957 +68
Misses 10231 10231
Partials 1264 1264
Flags with carried forward coverage won't be shown. Click here to find out more.
|
JSONbored
force-pushed
the
feat/loopover-contract-package-9517
branch
from
July 28, 2026 06:29
fb4788b to
4fd1261
Compare
Bundle ReportChanges will increase total bundle size by 2.5kB (0.03%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
JSONbored
force-pushed
the
feat/loopover-contract-package-9517
branch
from
July 28, 2026 07:04
4fd1261 to
d9cea33
Compare
JSONbored
added a commit
that referenced
this pull request
Jul 28, 2026
…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.
24 tasks
…l 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
… 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
…dles 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
…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.
…ttributes 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%.
JSONbored
force-pushed
the
feat/loopover-contract-package-9517
branch
from
July 28, 2026 07:56
d9cea33 to
6192bad
Compare
JSONbored
added a commit
that referenced
this pull request
Jul 28, 2026
…s that transitively need it validate-code failed on this PR with 'Cannot find module @loopover/contract/tools' plus five downstream implicit-any errors in src/mcp/server.ts -- none of which this PR touches. #9530 added @loopover/contract, and src/mcp/server.ts imports @loopover/contract/tools. Three turbo typecheck tasks pull that file into their program without any build edge to the package: - @loopover/ui#typecheck: apps/loopover-ui/tsconfig.json includes $TURBO_ROOT$/worker-configuration.d.ts, which imports './src/index' -- so the ENTIRE Worker is in the UI's typecheck program (src/index.ts -> src/api/routes.ts -> src/mcp/server.ts -> @loopover/contract/tools). Confirmed with tsc --explainFiles, not inferred. apps/loopover-ui has no package.json dependency on contract, so ^build never builds it. - //#typecheck: a root task, where a root package.json dependency creates no build edge. - @loopover/ui-miner#typecheck: reaches packages/loopover-miner/lib/**, which imports the package, and miner-ui has no dependency on it either. Each already carries an explicit @loopover/engine#build edge for precisely this reason -- turbo.json's own comment there describes the same scheduling race, observed intermittently in validate-code, that bit here. Cache-dependent, which is why it looked like flakiness: turbo caches these tasks, so the failure only appears on a cache MISS. Other open PRs are green on cache hits. Verified both directions from a clean contract build state (dist/ and .tsbuildinfo both removed, --force): - with the edges: 4 tasks successful, contract built first, typecheck passes - without them: the identical six errors CI reported
JSONbored
added a commit
that referenced
this pull request
Jul 28, 2026
…s that transitively need it validate-code failed on this PR with 'Cannot find module @loopover/contract/tools' plus five downstream implicit-any errors in src/mcp/server.ts -- none of which this PR touches. #9530 added @loopover/contract, and src/mcp/server.ts imports @loopover/contract/tools. Three turbo typecheck tasks pull that file into their program without any build edge to the package: - @loopover/ui#typecheck: apps/loopover-ui/tsconfig.json includes $TURBO_ROOT$/worker-configuration.d.ts, which imports './src/index' -- so the ENTIRE Worker is in the UI's typecheck program (src/index.ts -> src/api/routes.ts -> src/mcp/server.ts -> @loopover/contract/tools). Confirmed with tsc --explainFiles, not inferred. apps/loopover-ui has no package.json dependency on contract, so ^build never builds it. - //#typecheck: a root task, where a root package.json dependency creates no build edge. - @loopover/ui-miner#typecheck: reaches packages/loopover-miner/lib/**, which imports the package, and miner-ui has no dependency on it either. Each already carries an explicit @loopover/engine#build edge for precisely this reason -- turbo.json's own comment there describes the same scheduling race, observed intermittently in validate-code, that bit here. Cache-dependent, which is why it looked like flakiness: turbo caches these tasks, so the failure only appears on a cache MISS. Other open PRs are green on cache hits. Verified both directions from a clean contract build state (dist/ and .tsbuildinfo both removed, --force): - with the edges: 4 tasks successful, contract built first, typecheck passes - without them: the identical six errors CI reported
JSONbored
added a commit
that referenced
this pull request
Jul 28, 2026
…cabulary, and make inert config queryable (#9554, #9555, #9433) (#9556) * fix(gate,review,ops): add glob exclusions, tier the public-comment vocabulary, and make inert config queryable (#9554, #9555, #9433) * test: cover the exclusion matcher from both import identities so shard attribution cannot miss it * test(gate): cover the presence-mode staleness checkpoint and two long-untested reject arms Brings packages/loopover-engine/src/review/screenshot-table-gate.ts and src/signals/change-guardrail.ts (and both re-export shims) to 100% statement, branch and function coverage. Three branches had no test on any identity: - evaluateScreenshotTableGate's PRESENCE-mode freshness checkpoint. Matrix mode's identical correlation was pinned by #8866's tests, but presence mode runs it through a separate return site and never saw a headSha in a test. Presence mode is the DEFAULT shape, so the miss meant one table pasted on push #1 could hold the gate green for every later push -- the exact regression the checkpoint exists to stop. Six tests: first-push checkpoint, stale-on-new-head (asserting no checkpoint is re-issued, which would launder the staleness away after one extra push), re-affirmation with fresh URLs, custom message on the stale path, no-headSha degradation, and same-head replay. - guardrailPathMatches' empty-path skip, a deliberate divergence from matchesAny's fail-safe: the boolean form matches an empty path under an over-complex glob, the structured form must not, because its output is rendered verbatim into public review text and audit metadata. - extractTableRows' non-table reject arm, which is what stops ordinary PR prose (and a shell pipe inside backticks) from parsing as table rows. Mirrored across both import identities so a sharded, flag-merged coverage upload cannot report a branch as uncovered on one copy. * fix(ci): declare @loopover/contract#build on the three typecheck tasks that transitively need it validate-code failed on this PR with 'Cannot find module @loopover/contract/tools' plus five downstream implicit-any errors in src/mcp/server.ts -- none of which this PR touches. #9530 added @loopover/contract, and src/mcp/server.ts imports @loopover/contract/tools. Three turbo typecheck tasks pull that file into their program without any build edge to the package: - @loopover/ui#typecheck: apps/loopover-ui/tsconfig.json includes $TURBO_ROOT$/worker-configuration.d.ts, which imports './src/index' -- so the ENTIRE Worker is in the UI's typecheck program (src/index.ts -> src/api/routes.ts -> src/mcp/server.ts -> @loopover/contract/tools). Confirmed with tsc --explainFiles, not inferred. apps/loopover-ui has no package.json dependency on contract, so ^build never builds it. - //#typecheck: a root task, where a root package.json dependency creates no build edge. - @loopover/ui-miner#typecheck: reaches packages/loopover-miner/lib/**, which imports the package, and miner-ui has no dependency on it either. Each already carries an explicit @loopover/engine#build edge for precisely this reason -- turbo.json's own comment there describes the same scheduling race, observed intermittently in validate-code, that bit here. Cache-dependent, which is why it looked like flakiness: turbo caches these tasks, so the failure only appears on a cache MISS. Other open PRs are green on cache hits. Verified both directions from a clean contract build state (dist/ and .tsbuildinfo both removed, --force): - with the edges: 4 tasks successful, contract built first, typecheck passes - without them: the identical six errors CI reported
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
@loopover/contract— the single zod source of truth for LoopOver's MCP tool and API contracts, and the keystone of #9515.The problem. The same contracts are declared in four unshared places today:
src/openapi/schemas.ts(~150 response schemas, spec-only),src/api/routes.ts(38 inline request schemas),src/mcp/server.ts(tool shapes), andpackages/loopover-mcp/bin/loopover-mcp.ts(~85 shapes whose own comments say they mirror the remote server's, e.g.:376). Enum literals are hand-copied out of the engine with a comment noting the drift "has bitten once already" (:154-157).The shape of the fix. A zod-only leaf package with no node builtins — that is what lets the Worker, both published stdio bins, the miner, and the UI all depend on it without pulling the engine in behind them. Sharing these through
@loopover/enginewas considered and rejected once already (#6153):@loopover/mcpresolves the engine through its published export map, which never surfaced the enums.Key model decisions, all documented in the package README and enforced by meta-tests:
TOOL_OUTPUT_SCHEMAS[name]lookup where a typo'd key silently drops the schema; here a tool without an output schema is a type error.projectToolDefinitions()is the single projection point — MCP registration, the OpenAI/Anthropic spec builders Epic: ORB maintainer chat platform — conversational command center for maintainers and repo owners (hosted-ready) #9183/Epic: hosted AMS chat platform — sign up, connect GitHub, converse with your miner #9184 need, and the generated docs to come all derive from it.auth,locality,availability.localityis what makes explicit why LoopOver cannot collapse to one MCP process:local-gittools read the caller's uncommitted working tree andminertools read the miner box's stores, neither reachable from a Worker.z.looseObject). An MCP output schema is a floor, not a fence — a server returning an extra field must not retroactively invalidate a client on the older schema.Six pilot contracts, modelled from the engine types the handlers actually return rather than from the placeholder schemas advertised today. The remote server declares six of
get_repo_context's eight fields — and all ofpredict_gate'sblockers,warnings, andfunnel— as barez.unknown().optional(), which is precisely the information a caller needs most (a predicted FAIL is only actionable if the blocker list is readable).Defects surfaced while writing this
Three real findings, all documented in code where they live:
loopover_get_repo_contextreturns different payloads from the two servers. The remote handler builds eight keys; the stdio server proxiesGET /v1/repos/:owner/:repo/intelligence, which carries seven additional keys and omitscollisionsentirely on its snapshot branch. Modelled here as an honest union (only fields both paths always emit are required). Converging the payloads is a wire change and belongs to contract(remote): migrate the remote MCP server's tools to @loopover/contract — typed handlers, real output schemas #9518's batch for this category.loopover_get_pr_reviewabilitylikewise. The remote wraps the report in a freshness envelope and can answerforbidden/not_foundwith no report; the stdio server returns the barePullRequestReviewability. Also modelled as a union. Related drift found in passing: the remote's sharedfreshnessResponseOutputSchemaadvertises afreshnessfield the handler never emits.predict_gate'schangedPathselement cap was 300 remote / 400 stdio. The contract takes the wider bound — a shared contract may widen an input but never tighten one, and narrowing would start rejecting paths the stdio server accepts today.A fourth was caught by my own meta-test rather than by reading: zod v4's
z.objectstrips unknown keys rather than rejecting them, so the advertisedadditionalProperties: falseand the runtime behavior disagree. I did not change it — that would be a wire-visible tightening inside a migration that promised none. The current behavior is pinned by a test and the decision is recorded for #9518.Scope note recorded on the issue
packages/loopover-engine/src/settings/autonomy.tsstill declares its ownAUTONOMY_LEVELS/AGENT_ACTION_CLASSESbecause it is an engine-parity twin ofsrc/settings/autonomy.ts(guarded byengine-parity:drift-check). Inverting that pair to import from the contract is #9518's batch work. Until then the values are pinned against the engine's by meta-tests, so they cannot silently disagree — the same techniquetest/unit/mcp-cli-maintain.test.tsalready used for the stdio hand-copy this package replaces.PREFLIGHT_LIMITSis pinned the same way.Server wiring (second commit)
Both servers now register the six pilot tools from the contract. The stdio server's hand-mirrored shapes are gone for these tools, and four of them gain a real
outputSchemafor the first time — previouslyloopover_local_status_structuredwas the only tool in that entire package declaring one.Pre-existing OpenAPI drift fixed while here
Modelling the real payloads surfaced two published-spec lies, both fixed with
openapi.jsonregenerated:CollisionItemSchema.typeomitted"recent_merged_pull_request"— a discriminantbuildCollisionReportgenuinely emits, since it is handed recent merged PRs. A client validating a real response against the published spec rejected it. Itslabels,linkedIssues,linkedIssueClaimedAt,changedFilesandbodyfields were missing too.QueueHealthSchemaomitteddraftPullRequests,slopFlaggedPullRequests,duplicateFlaggedPullRequests— all required on theQueueHealthtype and always emitted — plus the optionalrankedPullRequestsarray.One more is recorded but not fixed here:
LocalBranchAnalysisSchemadoes not declare thepredictedGatefield the route emits. Fixing it means authoring the fullPredictedGateVerdictresponse schema, which now exists in the contract package and lands with #9519's route work.Closes #9517
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally — see note belownpm run test:workersnpm run build:mcp(unaffected; contract not yet consumed)npm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderatetest/unit/contract-registry.test.ts— 24 tests, all green: registry invariants (unique names, description length, vocabulary membership), Ajv-compilability of every emitted input and output schema, the open-output-schema wire-compat rule, filter behavior on all three axes including thatbothsurvives an availability filter, agent-spec projection parity, empty-list edges, and the engine pin tests for autonomy levels, action-class subset-ness, andPREFLIGHT_LIMITS.If any required check was skipped, explain why:
packages/loopover-contract/src/**is added tovitest.config.ts's coverage include list rather than exempted, so it is graded like any other source surface.Safety
authfield here is a declaration the runtimes will enforce in a later PR, not an enforcement change.)UI Evidencesection below. (n/a — no visible change)Notes
Part of #9515. Wired into
turbo.json(leaf build task, no^buildedge) andvitest.config.tscoverage. Release-please registration and thetest:contract-packallowlist land with the first PR that actually publishes a consumer, so the package is not released before anything depends on it.