From f13083642b257f6feef4d61c1b039f9c20cdedb1 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 27 Jul 2026 23:04:42 -0700 Subject: [PATCH 1/5] 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 --- package-lock.json | 34 +++ packages/loopover-contract/README.md | 87 ++++++ packages/loopover-contract/package.json | 64 +++++ packages/loopover-contract/src/agent-specs.ts | 80 ++++++ packages/loopover-contract/src/enums.ts | 74 ++++++ packages/loopover-contract/src/index.ts | 37 +++ packages/loopover-contract/src/limits.ts | 42 +++ packages/loopover-contract/src/shared.ts | 45 ++++ .../loopover-contract/src/tool-definition.ts | 154 +++++++++++ .../src/tools/admin-config.ts | 59 +++++ packages/loopover-contract/src/tools/index.ts | 51 ++++ .../src/tools/local-status.ts | 50 ++++ .../src/tools/pr-reviewability.ts | 72 +++++ .../src/tools/predict-gate.ts | 68 +++++ .../src/tools/preflight-pr.ts | 60 +++++ .../src/tools/repo-context.ts | 186 +++++++++++++ packages/loopover-contract/tsconfig.json | 28 ++ test/unit/contract-registry.test.ts | 250 ++++++++++++++++++ turbo.json | 8 + vitest.config.ts | 4 + 20 files changed, 1453 insertions(+) create mode 100644 packages/loopover-contract/README.md create mode 100644 packages/loopover-contract/package.json create mode 100644 packages/loopover-contract/src/agent-specs.ts create mode 100644 packages/loopover-contract/src/enums.ts create mode 100644 packages/loopover-contract/src/index.ts create mode 100644 packages/loopover-contract/src/limits.ts create mode 100644 packages/loopover-contract/src/shared.ts create mode 100644 packages/loopover-contract/src/tool-definition.ts create mode 100644 packages/loopover-contract/src/tools/admin-config.ts create mode 100644 packages/loopover-contract/src/tools/index.ts create mode 100644 packages/loopover-contract/src/tools/local-status.ts create mode 100644 packages/loopover-contract/src/tools/pr-reviewability.ts create mode 100644 packages/loopover-contract/src/tools/predict-gate.ts create mode 100644 packages/loopover-contract/src/tools/preflight-pr.ts create mode 100644 packages/loopover-contract/src/tools/repo-context.ts create mode 100644 packages/loopover-contract/tsconfig.json create mode 100644 test/unit/contract-registry.test.ts diff --git a/package-lock.json b/package-lock.json index 058e32c0a0..22accdc20c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4262,6 +4262,10 @@ "@lezer/lr": "^1.4.0" } }, + "node_modules/@loopover/contract": { + "resolved": "packages/loopover-contract", + "link": true + }, "node_modules/@loopover/discovery-index": { "resolved": "packages/discovery-index", "link": true @@ -5013,6 +5017,22 @@ "node": ">=8" } }, + "node_modules/@posthog/cli/node_modules/prettier": { + "version": "3.8.3", + "resolved": "https://registry.npmjs.org/prettier/-/prettier-3.8.3.tgz", + "integrity": "sha512-7igPTM53cGHMW8xWuVTydi2KO233VFiTNyF5hLJqpilHfmn8C8gPf+PS7dUT64YcXFbiMGZxS9pCSxL/Dxm/Jw==", + "extraneous": true, + "license": "MIT", + "bin": { + "prettier": "bin/prettier.cjs" + }, + "engines": { + "node": ">=14" + }, + "funding": { + "url": "https://github.com/prettier/prettier?sponsor=1" + } + }, "node_modules/@posthog/core": { "version": "1.45.1", "resolved": "https://registry.npmjs.org/@posthog/core/-/core-1.45.1.tgz", @@ -22595,6 +22615,20 @@ } } }, + "packages/loopover-contract": { + "name": "@loopover/contract", + "version": "0.1.0", + "license": "AGPL-3.0-only", + "dependencies": { + "zod": "^4.4.3" + }, + "devDependencies": { + "typescript": "^5.9.3" + }, + "engines": { + "node": ">=22.0.0 <23.0.0" + } + }, "packages/loopover-engine": { "name": "@loopover/engine", "version": "3.15.3", diff --git a/packages/loopover-contract/README.md b/packages/loopover-contract/README.md new file mode 100644 index 0000000000..865c7c6395 --- /dev/null +++ b/packages/loopover-contract/README.md @@ -0,0 +1,87 @@ +# @loopover/contract + +The single zod source of truth for LoopOver's MCP tool and API contracts. + +LoopOver runs three MCP servers — the hosted/self-host remote server (`src/mcp/server.ts`), the +stdio contributor wrapper (`@loopover/mcp`), and the AMS miner server (`@loopover/miner`) — plus a +REST API and a UI that all describe the same data. Before this package, each of those declared its +own zod shapes, and the copies drifted: the stdio server's shapes were hand-mirrored from the remote +server's (their own comments said so), enum literals were hand-copied out of the engine, and +responses were consumed as `any`. + +This package is the one place those contracts live. **A shape declared here is never restated +elsewhere.** + +## Why a separate package + +It is a **leaf**: its only runtime dependency is `zod`, and it imports no node builtins, so it is +safe in the Cloudflare Workers bundle. That is what lets every surface depend on it — the Worker, +both published stdio bins, the miner, the control plane, and the UI — without dragging the engine +along behind it. Sharing these schemas through `@loopover/engine` was considered and rejected: +`@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 (#6153). + +## Layout + +| Path | Holds | +|---|---| +| `src/tool-definition.ts` | The `ToolContract` model, `defineTool`, and `projectToolDefinitions` — the single projection point | +| `src/tools/*.ts` | One file per tool family; the contracts themselves | +| `src/tools/index.ts` | `TOOL_CONTRACTS`, `listToolDefinitions()`, `getToolContract()` | +| `src/enums.ts` | Shared enum vocabularies (autonomy levels, action classes, …) | +| `src/shared.ts` | Shapes reused by **three or more** contracts | +| `src/agent-specs.ts` | OpenAI / Anthropic / agent-index projections | + +## Conventions + +These are enforced by meta-tests in `test/unit/contract-registry.test.ts`, not just documented. + +**Naming.** One file per tool family. Within it, export `Input` and +`Output` schemas plus the `defineTool(...)` contract. Derive types with +`z.infer` — never hand-write an interface that mirrors a schema. + +**Inputs are closed; outputs are open.** Input schemas use `z.object`, which emits +`additionalProperties: false`. Output schemas use `z.looseObject`, which emits open +`additionalProperties`. An MCP output schema is a *floor*, not a fence: a server that starts +returning an extra field must not retroactively invalidate a client validating against the older +schema. + +> **Known gap:** zod's `z.object` *strips* unknown keys at runtime rather than rejecting them, so a +> typo'd argument is silently dropped even though the advertised JSON Schema says it should be +> refused. Switching to `z.strictObject` would close the gap but is a wire-visible tightening, so it +> is a recorded decision on #9518 rather than a drive-by change. A meta-test pins the current +> behavior so the switch cannot happen by accident. + +**Output schemas may be shallower than their REST counterparts, and that is deliberate.** Reusing a +strict REST response schema for an MCP tool *tightens* the wire contract and is a regression — the +exact constraint metagraphed hit during its own migration. Reuse a REST schema only when it is +field-for-field equal to what the tool actually returns. What is never acceptable is a top-level +`z.unknown()` standing in for a real object. + +**Hoist to `shared.ts` at the third consumer, not the second.** Two contracts sharing fields today +is usually coincidence; coupling them early means a later divergence has to be un-shared under +pressure. + +**Metadata is a declaration, not a hint.** Every contract states its `auth`, `locality`, and +`availability`, and runtimes enforce them: + +- `locality` — where the state physically lives (`remote`, `local-git`, `miner`). This is why + LoopOver cannot collapse to one MCP process: `local-git` tools read the caller's uncommitted + working tree and `miner` tools read the miner box's stores, neither reachable from a Worker. +- `availability` — `cloud`, `selfhost`, or `both`. Self-host-only tools depend on capabilities the + Workers bundle cannot provide (fs-backed config, a redeploy socket). +- `auth` — the identity kind `src/auth/security.ts` must authenticate before the tool runs. + +**Nothing reads `TOOL_CONTRACTS` directly.** Consumers call `listToolDefinitions()` (optionally +filtered), so cross-cutting concerns are applied exactly once. + +## Adding a tool + +1. Add `src/tools/.ts` with input + output schemas and a `defineTool(...)` entry. +2. Export it from `src/tools/index.ts`. +3. Register it in whichever runtimes can serve its locality, using `contract.input.shape` / + `contract.output.shape` for the MCP SDK. +4. The contract validator (#9520) requires a smoke call per tool — a tool with no call fails CI. + +Generated docs, agent tool specs, and the tool-reference tables pick it up automatically. If you +find yourself hand-editing a tool table, that table is a bug. diff --git a/packages/loopover-contract/package.json b/packages/loopover-contract/package.json new file mode 100644 index 0000000000..562f0d3ce0 --- /dev/null +++ b/packages/loopover-contract/package.json @@ -0,0 +1,64 @@ +{ + "name": "@loopover/contract", + "version": "0.1.0", + "license": "AGPL-3.0-only", + "type": "module", + "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.", + "repository": { + "type": "git", + "url": "git+https://github.com/JSONbored/loopover.git", + "directory": "packages/loopover-contract" + }, + "homepage": "https://github.com/JSONbored/loopover#readme", + "bugs": { + "url": "https://github.com/JSONbored/loopover/issues" + }, + "keywords": [ + "loopover", + "mcp", + "model-context-protocol", + "zod", + "openapi", + "schema" + ], + "publishConfig": { + "access": "public" + }, + "main": "dist/index.js", + "types": "dist/index.d.ts", + "exports": { + ".": { + "types": "./dist/index.d.ts", + "default": "./dist/index.js" + }, + "./enums": { + "types": "./dist/enums.d.ts", + "default": "./dist/enums.js" + }, + "./tools": { + "types": "./dist/tools/index.d.ts", + "default": "./dist/tools/index.js" + }, + "./agent-specs": { + "types": "./dist/agent-specs.d.ts", + "default": "./dist/agent-specs.js" + }, + "./package.json": "./package.json" + }, + "files": [ + "dist", + "CHANGELOG.md" + ], + "scripts": { + "build": "tsc -p tsconfig.json" + }, + "dependencies": { + "zod": "^4.4.3" + }, + "devDependencies": { + "typescript": "^5.9.3" + }, + "engines": { + "node": ">=22.0.0 <23.0.0" + } +} diff --git a/packages/loopover-contract/src/agent-specs.ts b/packages/loopover-contract/src/agent-specs.ts new file mode 100644 index 0000000000..5e05444125 --- /dev/null +++ b/packages/loopover-contract/src/agent-specs.ts @@ -0,0 +1,80 @@ +// Non-MCP projections of the same tool registry (#9517). +// +// The hosted maintainer chat (#9183) and hosted AMS chat (#9184) both need a "grounding tool +// catalog" to hand an LLM. That catalog is this registry in a different envelope -- not a second +// list to maintain. Each builder is a pure function over already-projected definitions, so a tool +// added to the contract appears in every surface at once, and a validator can assert the served +// bytes equal a fresh build. +import type { JsonSchemaLike, McpToolDefinition } from "./tool-definition.js"; + +export type OpenAIToolSpec = { + type: "function"; + function: { + name: string; + description: string; + parameters: JsonSchemaLike; + }; +}; + +export type AnthropicToolSpec = { + name: string; + description: string; + input_schema: JsonSchemaLike; +}; + +export function buildOpenAIToolSpecs(tools: readonly McpToolDefinition[]): OpenAIToolSpec[] { + return tools.map((tool) => ({ + type: "function", + function: { + name: tool.name, + description: tool.description, + parameters: tool.inputSchema, + }, + })); +} + +export function buildAnthropicToolSpecs(tools: readonly McpToolDefinition[]): AnthropicToolSpec[] { + return tools.map((tool) => ({ + name: tool.name, + description: tool.description, + input_schema: tool.inputSchema, + })); +} + +export type AgentToolsIndex = { + schema_version: number; + title: string; + description: string; + executor: { + transport: "mcp-streamable-http"; + endpoint: string; + jsonrpc_method: "tools/call"; + }; + specs: { + openai: OpenAIToolSpec[]; + anthropic: AnthropicToolSpec[]; + }; + tools: string[]; +}; + +/** Bumped only when the index's own envelope changes shape -- not when tools are added. */ +export const AGENT_TOOLS_INDEX_SCHEMA_VERSION = 1; + +export function buildAgentToolsIndex(tools: readonly McpToolDefinition[], options: { endpoint: string }): AgentToolsIndex { + return { + schema_version: AGENT_TOOLS_INDEX_SCHEMA_VERSION, + title: "LoopOver agent tools", + description: + "Tool specifications for LoopOver's MCP server, projected for OpenAI- and Anthropic-shaped tool-calling clients. Every tool is executed through the same MCP endpoint via tools/call.", + executor: { + transport: "mcp-streamable-http", + endpoint: options.endpoint, + jsonrpc_method: "tools/call", + }, + specs: { + openai: buildOpenAIToolSpecs(tools), + anthropic: buildAnthropicToolSpecs(tools), + }, + tools: tools.map((tool) => tool.name), + }; +} diff --git a/packages/loopover-contract/src/enums.ts b/packages/loopover-contract/src/enums.ts new file mode 100644 index 0000000000..0d5d4fc28b --- /dev/null +++ b/packages/loopover-contract/src/enums.ts @@ -0,0 +1,74 @@ +// Shared enum vocabularies (#9517). +// +// These existed as hand-copied literals in packages/loopover-mcp/bin/loopover-mcp.ts because that +// package resolves @loopover/engine through the PUBLISHED package, whose export map never surfaced +// them -- so importing the canonical list would have meant widening the engine's public API +// (#6153). The comment on those copies noted the drift it invited "has bitten once already": the +// stdio list carried "suggest"/"propose" for the whole life of #4620 after the server dropped +// them, turning a clear client-side error into a confusing 400 from the API. +// +// This package is the answer that was missing: a zod-only leaf with no dependencies, so every +// surface (Worker, both stdio bins, miner, UI) can import the same values without pulling the +// engine in behind them. +// +// SCOPE NOTE: packages/loopover-engine/src/settings/autonomy.ts still declares its own +// AUTONOMY_LEVELS / AGENT_ACTION_CLASSES, because it is an engine-parity twin of +// src/settings/autonomy.ts and inverting that pair to import from here is #9518's batch work, not +// the keystone's. Until then the values below are pinned against the engine's by a meta-test, so +// the two cannot silently disagree. + +/** + * Per-action-class autonomy levels. Mirrors the engine's `AUTONOMY_LEVELS`. + * + * `observe` records what it would have done; `auto_with_approval` stages an action for a human + * decision; `auto` acts directly. + */ +export const AUTONOMY_LEVELS = ["observe", "auto_with_approval", "auto"] as const; +export type AutonomyLevel = (typeof AUTONOMY_LEVELS)[number]; + +/** + * The action classes an operator may configure autonomy for. + * + * Deliberately NOT the engine's full `AGENT_ACTION_CLASSES` -- it is the operator-settable subset + * the maintain surface exposes, matching `MAINTAIN_AUTONOMY_ACTION_CLASSES` in src/mcp/server.ts. + * Do not "sync" this to the engine's list; the difference is the point. + */ +export const MAINTAIN_ACTION_CLASSES = ["review", "request_changes", "approve", "merge", "close", "label"] as const; +export type MaintainActionClass = (typeof MAINTAIN_ACTION_CLASSES)[number]; + +/** + * Action classes accepted when proposing an action into the approval queue (#6744). + * + * Derived rather than restated: it is exactly the operator-settable set plus `review_state_label`, + * and expressing that as code means the two can never drift apart the way three independent + * literal lists did. + */ +export const PROPOSE_ACTION_CLASSES = [...MAINTAIN_ACTION_CLASSES, "review_state_label"] as const; +export type ProposeActionClass = (typeof PROPOSE_ACTION_CLASSES)[number]; + +/** Test frameworks the boundary-test and test-evidence surfaces recognize by name. */ +export const TEST_FRAMEWORKS = ["vitest", "jest", "pytest", "go-test", "rspec", "cargo-test"] as const; +export type TestFramework = (typeof TEST_FRAMEWORKS)[number]; + +/** Lifecycle states of a plan step in the stateless plan-DAG tools. */ +export const PLAN_STEP_STATUSES = ["pending", "in_progress", "completed", "skipped", "failed"] as const; +export type PlanStepStatus = (typeof PLAN_STEP_STATUSES)[number]; + +/** Verdicts the pre-start feasibility surfaces return. */ +export const FEASIBILITY_VERDICTS = ["go", "raise", "avoid"] as const; +export type FeasibilityVerdict = (typeof FEASIBILITY_VERDICTS)[number]; + +/** + * Scope selector for WRITING the self-hosted private config: the deployment-wide default layer, or + * one repository's override layer. Only real files are writable. + */ +export const CONFIG_ADMIN_WRITE_SCOPES = ["global", "repo"] as const; +export type ConfigAdminWriteScope = (typeof CONFIG_ADMIN_WRITE_SCOPES)[number]; + +/** + * Scope selector for READING it. A superset of the write scopes: `effective` is the merged view + * (shared base + global default + per-repo override) that no single file corresponds to, which is + * exactly why it can be read but never written. + */ +export const CONFIG_ADMIN_READ_SCOPES = ["effective", ...CONFIG_ADMIN_WRITE_SCOPES] as const; +export type ConfigAdminReadScope = (typeof CONFIG_ADMIN_READ_SCOPES)[number]; diff --git a/packages/loopover-contract/src/index.ts b/packages/loopover-contract/src/index.ts new file mode 100644 index 0000000000..b4551d9eee --- /dev/null +++ b/packages/loopover-contract/src/index.ts @@ -0,0 +1,37 @@ +// @loopover/contract — the single zod source of truth for LoopOver's MCP tool and API contracts. +// +// Import from here (or from the ./enums, ./tools, ./agent-specs subpaths) rather than restating a +// shape locally. Every schema in this package is consumed by more than one runtime; a local copy +// is drift waiting to happen, which is the entire reason the package exists (#9515, #9517). +export { + TOOL_CATEGORIES, + TOOL_AUTH_LEVELS, + TOOL_LOCALITIES, + TOOL_AVAILABILITIES, + defineTool, + projectToolDefinitions, + toJsonSchema, + type ToolCategory, + type ToolAuthLevel, + type ToolLocality, + type ToolAvailability, + type ToolAnnotations, + type ToolContract, + type ToolFilter, + type JsonSchemaLike, + type McpToolDefinition, +} from "./tool-definition.js"; + +export * from "./enums.js"; +export { PREFLIGHT_LIMITS, PREDICT_GATE_MAX_CHANGED_PATHS, PREDICT_GATE_MAX_CHANGED_PATH_CHARS } from "./limits.js"; +export { ownerRepoInput, ownerRepoPullInput, freshnessFields, toolErrorFields } from "./shared.js"; +export { TOOL_CONTRACTS, listToolDefinitions, getToolContract } from "./tools/index.js"; +export { + AGENT_TOOLS_INDEX_SCHEMA_VERSION, + buildOpenAIToolSpecs, + buildAnthropicToolSpecs, + buildAgentToolsIndex, + type OpenAIToolSpec, + type AnthropicToolSpec, + type AgentToolsIndex, +} from "./agent-specs.js"; diff --git a/packages/loopover-contract/src/limits.ts b/packages/loopover-contract/src/limits.ts new file mode 100644 index 0000000000..f8e8044eb6 --- /dev/null +++ b/packages/loopover-contract/src/limits.ts @@ -0,0 +1,42 @@ +// Input bounds shared by the preflight/predictor surfaces (#9517). +// +// These exist in packages/loopover-engine/src/signals/preflight-limits.ts as PREFLIGHT_LIMITS, but +// this package cannot import the engine (it is a zod-only leaf, which is the property that lets +// every other surface depend on it). Restated here as the contract's own canonical copy and pinned +// against the engine's by a meta-test, so a bound cannot be raised on one side only -- the failure +// mode being a schema that rejects input the server would have accepted, or accepts input the +// server then truncates. +export const PREFLIGHT_LIMITS = { + repoFullNameChars: 200, + contributorLoginChars: 100, + titleChars: 300, + bodyChars: 20_000, + labelChars: 100, + changedFileChars: 300, + testChars: 300, + authorAssociationChars: 100, + labels: 50, + changedFiles: 200, + linkedIssues: 100, + tests: 50, +} as const; + +/** + * Cap on `changedPaths` for the gate predictor specifically. + * + * Deliberately larger than `PREFLIGHT_LIMITS.changedFiles` (200): paths are the cheapest possible + * metadata and a large refactor legitimately touches more files than the preflight surface bounds, + * so the predictor accepts a wider list than preflight does. Not a copy of that limit -- a + * different limit for a different reason. + */ +export const PREDICT_GATE_MAX_CHANGED_PATHS = 500; + +/** + * Per-path character cap for the gate predictor. + * + * 400 rather than `PREFLIGHT_LIMITS.changedFileChars` (300) because the two servers disagreed: the + * remote MCP server bounded these at 300 and the stdio server at 400. A shared contract takes the + * WIDER of two historical bounds -- narrowing it would start rejecting input one of the two + * servers accepts today, which is the one thing an input schema may never do. + */ +export const PREDICT_GATE_MAX_CHANGED_PATH_CHARS = 400; diff --git a/packages/loopover-contract/src/shared.ts b/packages/loopover-contract/src/shared.ts new file mode 100644 index 0000000000..b04b5e42a1 --- /dev/null +++ b/packages/loopover-contract/src/shared.ts @@ -0,0 +1,45 @@ +// Shapes reused by three or more tool contracts (#9517). +// +// The bar for hoisting here is deliberate and worth stating, because a "shared" file with no +// admission rule becomes a junk drawer: a shape earns a place here at its THIRD consumer, not its +// second. Two tools that happen to take the same fields today are usually a coincidence, and +// prematurely coupling them means a later divergence has to be un-shared under pressure. +import { z } from "zod"; + +/** The owner/repo pair virtually every repo-scoped tool takes. */ +export const ownerRepoInput = z.object({ + owner: z.string().min(1), + repo: z.string().min(1), +}); + +/** owner/repo plus a pull-request number. */ +export const ownerRepoPullInput = ownerRepoInput.extend({ + number: z.number().int().positive(), +}); + +/** + * The freshness marker cached advisory payloads carry. + * + * Optional on purpose: a freshly-computed response has no cache metadata to report, and the older + * REST responses these tools wrap omit the field entirely rather than sending a null. + */ +export const freshnessFields = { + generatedAt: z.string().optional(), + cached: z.boolean().optional(), + stale: z.boolean().optional(), +}; + +/** + * Fields an error envelope carries when a tool fails in a way the caller can act on. + * + * `code` is drawn from a closed, developer-defined set rather than free text so telemetry can + * break failures down by cause (#9525) and never ingests a caller-derived string. + */ +export const toolErrorFields = { + error: z + .object({ + code: z.string().min(1), + message: z.string().min(1), + }) + .optional(), +}; diff --git a/packages/loopover-contract/src/tool-definition.ts b/packages/loopover-contract/src/tool-definition.ts new file mode 100644 index 0000000000..a23b19bead --- /dev/null +++ b/packages/loopover-contract/src/tool-definition.ts @@ -0,0 +1,154 @@ +// The tool-contract model (#9517). One entry per MCP tool, carrying its schemas AND the metadata +// every runtime needs to decide whether it may serve that tool at all. +// +// Two rules this file exists to enforce, both learned from surfaces that drifted: +// +// 1. Schemas live ON the entry, never in a name-keyed side map. metagraphed keeps its output +// schemas in a `TOOL_OUTPUT_SCHEMAS[name]` lookup, where a typo'd key silently drops the +// schema and nothing fails. Here a tool without an output schema is a type error. +// 2. `listToolDefinitions()` is the only projection. Nothing downstream reads TOOL_CONTRACTS +// directly, so cross-cutting concerns (JSON Schema conversion, annotation defaults) are +// applied exactly once instead of per consumer. +import { z } from "zod"; + +/** Categories a tool can belong to, mirroring the ids the servers already advertise as + * `_meta.category` and the stdio CLI groups its `tools` output by. */ +export const TOOL_CATEGORIES = ["maintainer", "review", "branch", "discovery", "agent", "utility", "admin"] as const; +export type ToolCategory = (typeof TOOL_CATEGORIES)[number]; + +/** Who a caller must be for a tool to run. Mirrors the identity kinds `src/auth/security.ts` + * actually authenticates -- this is the declaration a runtime enforces against, not a hint. */ +export const TOOL_AUTH_LEVELS = ["public", "token", "session", "maintainer", "operator", "mcp-admin", "internal"] as const; +export type ToolAuthLevel = (typeof TOOL_AUTH_LEVELS)[number]; + +/** Where the state a tool reads physically lives. This is why LoopOver runs more than one MCP + * server and cannot collapse to one process: `local-git` tools read the caller's uncommitted + * working tree, `miner` tools read the miner box's local stores, and neither is reachable from + * a hosted Worker. A runtime registers only the localities it can actually serve. */ +export const TOOL_LOCALITIES = ["remote", "local-git", "miner"] as const; +export type ToolLocality = (typeof TOOL_LOCALITIES)[number]; + +/** Which deployments expose the tool. `selfhost`-only tools depend on capabilities the Cloudflare + * bundle has no way to provide (fs-backed config, a redeploy socket); `cloud`-only tools depend + * on fleet/tenant state a single self-hosted instance does not have. */ +export const TOOL_AVAILABILITIES = ["cloud", "selfhost", "both"] as const; +export type ToolAvailability = (typeof TOOL_AVAILABILITIES)[number]; + +/** MCP tool annotations advertised in `tools/list`. Defaults are applied in the projection, so an + * entry only states what differs from "safe, read-only". */ +export type ToolAnnotations = { + readOnlyHint: boolean; + destructiveHint: boolean; +}; + +/** + * One tool's complete contract. + * + * `input`/`output` are `ZodObject`s rather than raw shapes because both consumers need something + * different from them and each conversion must happen in exactly one place: the MCP SDK's + * `registerTool` wants `.shape`, while the agent-spec builders and any Ajv validator want JSON + * Schema via `z.toJSONSchema`. Storing the object keeps both derivable; storing a bare shape + * would not. + * + * `output` is intentionally typed as a loose object (`z.looseObject`, i.e. `additionalProperties` + * open). An MCP output schema is a *floor*, not a fence: a server that starts returning an extra + * field must not retroactively invalidate a client validating against the older schema. This is + * the wire-compatibility constraint metagraphed hit head-on when it tried to reuse its strict REST + * schemas for MCP output and found the tighter contract was a regression. + */ +export type ToolContract = { + name: string; + title: string; + description: string; + category: ToolCategory; + auth: ToolAuthLevel; + locality: ToolLocality; + availability: ToolAvailability; + annotations?: Partial; + input: z.ZodObject; + output: z.ZodObject; +}; + +/** JSON Schema as emitted by `z.toJSONSchema` -- structurally open because draft-2020-12 allows + * keywords we neither enumerate nor interpret here. */ +export type JsonSchemaLike = Record; + +/** A tool as advertised over the wire: schemas converted to JSON Schema, annotations defaulted. */ +export type McpToolDefinition = { + name: string; + title: string; + description: string; + category: ToolCategory; + auth: ToolAuthLevel; + locality: ToolLocality; + availability: ToolAvailability; + annotations: ToolAnnotations; + inputSchema: JsonSchemaLike; + outputSchema: JsonSchemaLike; +}; + +/** Read-only, non-destructive: the default posture. A tool that mutates anything must say so. */ +const DEFAULT_ANNOTATIONS: ToolAnnotations = { readOnlyHint: true, destructiveHint: false }; + +/** `target` is pinned to draft-2020-12 because that is the dialect the MCP spec's `outputSchema` + * is validated under, and the dialect Ajv2020 compiles in the contract validator. */ +export function toJsonSchema(schema: z.ZodObject): JsonSchemaLike { + return z.toJSONSchema(schema, { target: "draft-2020-12" }) as JsonSchemaLike; +} + +/** + * Filter for a runtime's own slice of the registry. + * + * Omitting a field means "do not filter on it" rather than "match nothing", so a caller asks only + * about the axes it actually constrains -- the self-host MCP server filters on availability but + * serves every locality it can reach, while the stdio server filters on locality but runs against + * both deployments. + */ +export type ToolFilter = { + locality?: readonly ToolLocality[]; + availability?: readonly ToolAvailability[]; + category?: readonly ToolCategory[]; +}; + +function matchesFilter(contract: ToolContract, filter: ToolFilter): boolean { + if (filter.locality && !filter.locality.includes(contract.locality)) return false; + // `both` satisfies any availability constraint -- it is the absence of a restriction, not a + // third deployment, so a `selfhost` filter must still return it. + if (filter.availability && contract.availability !== "both" && !filter.availability.includes(contract.availability)) { + return false; + } + if (filter.category && !filter.category.includes(contract.category)) return false; + return true; +} + +/** + * The single projection point. Every consumer -- MCP registration on all three servers, the + * OpenAI/Anthropic spec builders, the generated tool-reference docs, the contract validator -- + * derives from this and never from the raw contract array. + */ +export function projectToolDefinitions(contracts: readonly ToolContract[], filter: ToolFilter = {}): McpToolDefinition[] { + return contracts.filter((contract) => matchesFilter(contract, filter)).map((contract) => ({ + name: contract.name, + title: contract.title, + description: contract.description, + category: contract.category, + auth: contract.auth, + locality: contract.locality, + availability: contract.availability, + annotations: { ...DEFAULT_ANNOTATIONS, ...contract.annotations }, + inputSchema: toJsonSchema(contract.input), + outputSchema: toJsonSchema(contract.output), + })); +} + +/** + * Helper for authoring a contract entry. + * + * The explicit `ToolContract` return annotation is load-bearing, not decoration: without it TS + * infers each call's `input`/`output` as its own narrow `ZodObject<{...}>` literal, and an array + * of those heterogeneous types collapses to an unsatisfiable intersection the moment anything + * maps over it. metagraphed documents the same lesson on its own registry array. + */ +export function defineTool(contract: ToolContract): ToolContract { + return contract; +} diff --git a/packages/loopover-contract/src/tools/admin-config.ts b/packages/loopover-contract/src/tools/admin-config.ts new file mode 100644 index 0000000000..979d8a2396 --- /dev/null +++ b/packages/loopover-contract/src/tools/admin-config.ts @@ -0,0 +1,59 @@ +// loopover_admin_get_config (#9517 pilot). +// +// The self-host operator surface, and the pilot's proof that `auth` and `availability` are real +// enforcement inputs rather than documentation. This tool is registered only when +// LOOPOVER_MCP_ADMIN_ENABLED is truthy AND requires actor === "mcp-admin" at call time -- defense in +// depth, so enabling the flag alone grants nothing to a caller holding only the ordinary MCP token. +// +// availability "selfhost" is physics, not policy: reading the private config needs an fs-backed +// capability the Cloudflare Workers bundle cannot provide, which is why it reaches the filesystem +// through the nullable registry in src/mcp/private-config-admin-registry.ts that only the self-host +// Node entry ever fills. +import { z } from "zod"; +import { defineTool } from "../tool-definition.js"; +import { CONFIG_ADMIN_READ_SCOPES } from "../enums.js"; + +/** + * `repoFullName` is CONDITIONALLY required -- mandatory for scope "effective" and "repo", ignored + * for "global" -- and that dependency is enforced at runtime by a throw rather than by this schema. + * Expressing it here (a discriminated union, or a refinement) would change the emitted JSON Schema + * from a flat object into a composed one, which is a wire change to the advertised inputSchema. + * Left flat on purpose; the runtime check remains the authority. + */ +export const AdminGetConfigInput = z.object({ + scope: z.enum(CONFIG_ADMIN_READ_SCOPES), + repoFullName: z.string().min(3).max(200).optional(), +}); + +/** + * Three distinct payloads share this shape, which is why only `configured` is required: + * + * 1. not configured (LOOPOVER_REPO_CONFIG_DIR unset) -- `{ configured: false }` and nothing else; + * 2. scope "effective" -- adds found/path/content/warnings, with `path` always null because a + * merged view corresponds to no single file; + * 3. scope "global" | "repo" -- adds found/path/content, and never `warnings` (there is no merge + * or parse step in a raw file read to warn about). + */ +export const AdminGetConfigOutput = z.looseObject({ + configured: z.boolean(), + found: z.boolean().optional(), + path: z.string().nullable().optional(), + content: z.string().nullable().optional(), + warnings: z.array(z.string()).optional(), +}); + +export type AdminGetConfigInput = z.infer; +export type AdminGetConfigOutput = z.infer; + +export const adminGetConfigTool = defineTool({ + name: "loopover_admin_get_config", + title: "Read private instance config", + description: + "Self-hosted-operator only. Read this instance's own private .loopover.yml config: the merged effective config for a repo (shared base + global default + per-repo override), or just the raw global-default layer, or just the raw per-repo layer. Requires LOOPOVER_MCP_ADMIN_TOKEN. Returns configured=false if LOOPOVER_REPO_CONFIG_DIR is unset.", + category: "admin", + auth: "mcp-admin", + locality: "remote", + availability: "selfhost", + input: AdminGetConfigInput, + output: AdminGetConfigOutput, +}); diff --git a/packages/loopover-contract/src/tools/index.ts b/packages/loopover-contract/src/tools/index.ts new file mode 100644 index 0000000000..2cd00698b0 --- /dev/null +++ b/packages/loopover-contract/src/tools/index.ts @@ -0,0 +1,51 @@ +// The tool registry (#9517). +// +// Every MCP tool LoopOver serves, from any of its three servers, has exactly one entry here. A +// runtime registers the slice it can actually serve by filtering on locality/availability -- it +// does not keep its own list. +import { projectToolDefinitions, type McpToolDefinition, type ToolContract, type ToolFilter } from "../tool-definition.js"; +import { getRepoContextTool } from "./repo-context.js"; +import { getPrReviewabilityTool } from "./pr-reviewability.js"; +import { predictGateTool } from "./predict-gate.js"; +import { preflightPrTool } from "./preflight-pr.js"; +import { localStatusStructuredTool } from "./local-status.js"; +import { adminGetConfigTool } from "./admin-config.js"; + +/** + * Pilot batch (#9517). The remaining ~110 remote / 96 stdio / 11 miner tools migrate in #9518's + * category batches; this set was chosen to exercise every axis of the model at least once -- + * remote and local-git locality, cloud/selfhost/both availability, and the token/session/ + * maintainer/mcp-admin auth levels. + */ +export const TOOL_CONTRACTS: readonly ToolContract[] = [ + getRepoContextTool, + getPrReviewabilityTool, + predictGateTool, + preflightPrTool, + localStatusStructuredTool, + adminGetConfigTool, +]; + +const CONTRACTS_BY_NAME: ReadonlyMap = new Map( + TOOL_CONTRACTS.map((contract) => [contract.name, contract]), +); + +/** The single projection every consumer reads. Nothing downstream touches TOOL_CONTRACTS. */ +export function listToolDefinitions(filter: ToolFilter = {}): McpToolDefinition[] { + return projectToolDefinitions(TOOL_CONTRACTS, filter); +} + +/** Lookup for a runtime that needs the zod objects themselves (`.shape` for the MCP SDK, or + * `.parse` to validate a response) rather than the JSON Schema projection. */ +export function getToolContract(name: string): ToolContract | undefined { + return CONTRACTS_BY_NAME.get(name); +} + +// Re-export each family wholesale so consumers can reach the individual input/output schemas (and +// the shared sub-shapes like laneAdviceSchema) without importing deep paths. +export * from "./repo-context.js"; +export * from "./pr-reviewability.js"; +export * from "./predict-gate.js"; +export * from "./preflight-pr.js"; +export * from "./local-status.js"; +export * from "./admin-config.js"; diff --git a/packages/loopover-contract/src/tools/local-status.ts b/packages/loopover-contract/src/tools/local-status.ts new file mode 100644 index 0000000000..8071d13ac7 --- /dev/null +++ b/packages/loopover-contract/src/tools/local-status.ts @@ -0,0 +1,50 @@ +// loopover_local_status_structured (#9517 pilot). +// +// stdio-only, and the one tool that already had a real zod outputSchema before this package existed +// -- it was declared inline in packages/loopover-mcp/bin/loopover-mcp.ts. Relocated here verbatim in +// shape so the migration is provably behavior-preserving for the one tool where a before/after +// comparison is possible. +// +// `locality: "local-git"` is the load-bearing metadata: this reads the caller's own checkout and +// local config, which no hosted Worker can see. It is the reason a gateway cannot simply proxy +// everything to the remote server. +import { z } from "zod"; +import { defineTool } from "../tool-definition.js"; + +export const LocalStatusStructuredInput = z.object({ + cwd: z.string().optional(), + baseRef: z.string().optional(), + repoFullName: z.string().min(3).optional(), +}); + +export const LocalStatusStructuredOutput = z.looseObject({ + apiUrl: z.string(), + package: z.looseObject({ name: z.string(), version: z.string() }), + hasToken: z.boolean(), + // Left as an open record, matching the shape shipped today. The concrete payload + // (profilePublicState) has a known set of keys, but tightening an OUTPUT schema is the direction + // that breaks clients, so it stays as-is until a deliberate widening pass. + profile: z.record(z.string(), z.unknown()), + authLogin: z.string().nullable(), + sessionExpiresAt: z.string().nullable(), + sourceUploadDefault: z.boolean(), + sourceUploadSupported: z.boolean(), + // Either collectLocalBranchMetadata's result or { error } when git inspection failed -- the + // handler catches and reports rather than throwing, so both shapes are legitimate. + git: z.record(z.string(), z.unknown()), +}); + +export type LocalStatusStructuredInput = z.infer; +export type LocalStatusStructuredOutput = z.infer; + +export const localStatusStructuredTool = defineTool({ + name: "loopover_local_status_structured", + title: "Local MCP status (structured)", + description: "Return local LoopOver MCP status with a validated structured output schema.", + category: "utility", + auth: "public", + locality: "local-git", + availability: "both", + input: LocalStatusStructuredInput, + output: LocalStatusStructuredOutput, +}); diff --git a/packages/loopover-contract/src/tools/pr-reviewability.ts b/packages/loopover-contract/src/tools/pr-reviewability.ts new file mode 100644 index 0000000000..d491c0ead5 --- /dev/null +++ b/packages/loopover-contract/src/tools/pr-reviewability.ts @@ -0,0 +1,72 @@ +// loopover_get_pr_reviewability (#9517 pilot). +// +// KNOWN DIVERGENCE, modelled deliberately (same class as get_repo_context's): +// +// - The remote server wraps the report in a freshness envelope -- +// { status, source, repoFullName, generatedAt, report } -- and can answer with +// status "forbidden" or "not_found" and no report at all. +// - The stdio server proxies GET /v1/repos/:owner/:repo/pulls/:number/reviewability, which returns +// the BARE PullRequestReviewability object with no envelope around it. +// +// The union below therefore makes every envelope field optional and allows the report's own fields +// to appear at the top level. Converging the two is #9518's work for this category; this schema is +// what keeps the divergence visible and validated rather than hidden behind an unknown. +// +// One drift found while writing this and worth fixing separately: the remote's shared +// `freshnessResponseOutputSchema` advertises a `freshness` field that this handler never emits. +import { z } from "zod"; +import { defineTool } from "../tool-definition.js"; +import { ownerRepoPullInput } from "../shared.js"; + +/** `PullRequestReviewability` (packages/loopover-engine/src/reward-risk.ts). All ten fields are + * required and non-nullable on the type; `PullRequestReviewabilitySchema` in src/openapi/schemas.ts + * is a field-for-field match, so REST and MCP genuinely agree on this shape. */ +export const pullRequestReviewabilitySchema = z.looseObject({ + repoFullName: z.string(), + pullNumber: z.number(), + generatedAt: z.string(), + score: z.number(), + action: z.enum(["review_now", "needs_author", "likely_duplicate", "close_or_redirect", "watch", "maintainer_lane"]), + noiseSources: z.array(z.string()), + whyThisHelps: z.array(z.string()), + maintainerNextSteps: z.array(z.string()), + privateSummary: z.string(), +}); + +export const GetPrReviewabilityInput = ownerRepoPullInput; + +export const GetPrReviewabilityOutput = z.looseObject({ + // Envelope (remote only). `status` also carries the two no-report answers: "forbidden" when the + // caller cannot see the repo, "not_found" when the repo or PR is unknown. + status: z.enum(["ready", "forbidden", "not_found"]).optional(), + source: z.enum(["snapshot", "computed"]).optional(), + repoFullName: z.string().optional(), + generatedAt: z.string().optional(), + report: pullRequestReviewabilitySchema.optional(), + // Bare-report fields (stdio path). Optional because the remote path nests them under `report`. + pullNumber: z.number().optional(), + score: z.number().optional(), + action: z + .enum(["review_now", "needs_author", "likely_duplicate", "close_or_redirect", "watch", "maintainer_lane"]) + .optional(), + noiseSources: z.array(z.string()).optional(), + whyThisHelps: z.array(z.string()).optional(), + maintainerNextSteps: z.array(z.string()).optional(), + privateSummary: z.string().optional(), +}); + +export type GetPrReviewabilityInput = z.infer; +export type GetPrReviewabilityOutput = z.infer; + +export const getPrReviewabilityTool = defineTool({ + name: "loopover_get_pr_reviewability", + title: "Get pull-request reviewability", + description: + "Return the cached or freshly-computed reviewability report for an open PR: how ready it is to review/merge, the blocking or advisory signals against it, and its lane/duplicate/linked-issue context. Metadata-only, repo-scoped, no GitHub writes.", + category: "review", + auth: "maintainer", + locality: "remote", + availability: "both", + input: GetPrReviewabilityInput, + output: GetPrReviewabilityOutput, +}); diff --git a/packages/loopover-contract/src/tools/predict-gate.ts b/packages/loopover-contract/src/tools/predict-gate.ts new file mode 100644 index 0000000000..17a4fde2cd --- /dev/null +++ b/packages/loopover-contract/src/tools/predict-gate.ts @@ -0,0 +1,68 @@ +// loopover_predict_gate (#9517 pilot). +// +// The remote server's output schema declared blockers/warnings/funnel as bare `z.unknown()`, which +// is precisely the information a caller needs most: a predicted FAIL is only actionable if the +// client can read the blocker list. Modelled here from `PredictedGateVerdict` +// (packages/loopover-engine/src/predicted-gate.ts). +import { z } from "zod"; +import { defineTool } from "../tool-definition.js"; +import { PREDICT_GATE_MAX_CHANGED_PATH_CHARS, PREDICT_GATE_MAX_CHANGED_PATHS } from "../limits.js"; + +/** A predicted blocker or warning. Identical shape for both, per the engine type. */ +export const gateFindingSchema = z.looseObject({ + code: z.string(), + title: z.string(), + detail: z.string(), + action: z.string().optional(), +}); + +export const PredictGateInput = z.object({ + login: z.string().min(1), + owner: z.string().min(1), + repo: z.string().min(1), + title: z.string().min(1), + body: z.string().optional(), + labels: z.array(z.string()).optional(), + linkedIssues: z.array(z.number().int().positive()).optional(), + // Changed file PATHS only -- metadata, never source content. Supplying them lets the predictor + // also evaluate the focus-manifest path policy and path-gated pre-merge checks. + // 400, not PREFLIGHT_LIMITS.changedFileChars (300): the remote server bounded paths at 300 while + // the stdio server bounded them at 400, and a shared contract may only widen an input, never + // tighten one -- picking 300 would start rejecting paths the stdio server accepts today. The + // engine bounds these again on the way in regardless. + changedPaths: z.array(z.string().min(1).max(PREDICT_GATE_MAX_CHANGED_PATH_CHARS)).max(PREDICT_GATE_MAX_CHANGED_PATHS).optional(), +}); + +export const PredictGateOutput = z.looseObject({ + predicted: z.boolean(), + basis: z.string(), + pack: z.enum(["gittensor", "oss-anti-slop"]), + conclusion: z.string(), + title: z.string(), + summary: z.string(), + // Null when the pack does not compute a readiness score, distinct from an absent field. + readinessScore: z.number().nullable(), + confirmedContributor: z.boolean().optional(), + blockers: z.array(gateFindingSchema), + warnings: z.array(gateFindingSchema), + // Present only under the `oss-anti-slop` pack; explicitly null under `gittensor`, where the + // contributor is already registered and has no conversion path to offer. + funnel: z.looseObject({ message: z.string(), registerUrl: z.string() }).nullable(), + note: z.string(), +}); + +export type PredictGateInput = z.infer; +export type PredictGateOutput = z.infer; + +export const predictGateTool = defineTool({ + name: "loopover_predict_gate", + title: "Predict gate disposition", + description: + "Predict how the LoopOver gate would dispose of a planned pull request, from the repo's public .loopover.yml config plus safe defaults: the conclusion, readiness score, and the specific blockers and warnings it would raise. Metadata-only — never receives diff content, so the slop score is not evaluated.", + category: "review", + auth: "token", + locality: "remote", + availability: "both", + input: PredictGateInput, + output: PredictGateOutput, +}); diff --git a/packages/loopover-contract/src/tools/preflight-pr.ts b/packages/loopover-contract/src/tools/preflight-pr.ts new file mode 100644 index 0000000000..7a708a1b3a --- /dev/null +++ b/packages/loopover-contract/src/tools/preflight-pr.ts @@ -0,0 +1,60 @@ +// loopover_preflight_pr (#9517 pilot). +// +// Both servers return the same payload here -- the engine's `PreflightResult` -- and the REST route +// behind the stdio server already has a field-for-field zod schema for it +// (`PreflightResultSchema`, src/openapi/schemas.ts). That makes this the one pilot tool where the +// MCP output schema and the REST response schema genuinely agree, so the shape below is modelled +// directly on both. +// +// The input bounds are the REST route's (`preflightSchema`, src/api/routes.ts), which the stdio +// server did NOT apply -- it declared every field unbounded. That is not a tightening in any +// meaningful sense: the server rejects an over-long title with a 400 regardless, so the only effect +// is that the caller now gets a clear client-side error instead of a confusing API error two hops +// away. This is the same failure #6153 documented when the stdio autonomy enum drifted looser than +// the server's. +import { z } from "zod"; +import { defineTool } from "../tool-definition.js"; +import { PREFLIGHT_LIMITS } from "../limits.js"; +import { advisoryFindingSchema, laneAdviceSchema, collisionClusterSchema } from "./repo-context.js"; + +export const PreflightPrInput = z.object({ + repoFullName: z.string().min(3).max(PREFLIGHT_LIMITS.repoFullNameChars), + contributorLogin: z.string().min(1).max(PREFLIGHT_LIMITS.contributorLoginChars).optional(), + title: z.string().min(1).max(PREFLIGHT_LIMITS.titleChars), + body: z.string().max(PREFLIGHT_LIMITS.bodyChars).optional(), + labels: z.array(z.string().max(PREFLIGHT_LIMITS.labelChars)).max(PREFLIGHT_LIMITS.labels).optional(), + changedFiles: z.array(z.string().max(PREFLIGHT_LIMITS.changedFileChars)).max(PREFLIGHT_LIMITS.changedFiles).optional(), + linkedIssues: z.array(z.number().int().positive()).max(PREFLIGHT_LIMITS.linkedIssues).optional(), + tests: z.array(z.string().max(PREFLIGHT_LIMITS.testChars)).max(PREFLIGHT_LIMITS.tests).optional(), + authorAssociation: z.string().max(PREFLIGHT_LIMITS.authorAssociationChars).optional(), +}); + +/** `PreflightResult` (packages/loopover-engine/src/signals/engine.ts). All eight fields are + * required and non-nullable on the type, and the builder always populates them. Note `collisions` + * is a flat cluster array here, NOT the `CollisionReport` envelope get_repo_context returns. */ +export const PreflightPrOutput = z.looseObject({ + repoFullName: z.string(), + generatedAt: z.string(), + status: z.enum(["ready", "needs_work", "hold"]), + lane: laneAdviceSchema, + reviewBurden: z.enum(["low", "medium", "high"]), + linkedIssues: z.array(z.number()), + findings: z.array(advisoryFindingSchema), + collisions: z.array(collisionClusterSchema), +}); + +export type PreflightPrInput = z.infer; +export type PreflightPrOutput = z.infer; + +export const preflightPrTool = defineTool({ + name: "loopover_preflight_pr", + title: "Preflight a planned pull request", + description: + "Preflight planned pull-request metadata against the repo's lane, duplicate clusters, linked-issue policy, test evidence, and review burden before any code is pushed. Metadata-only: accepts titles, labels, file paths, and test names, never source content.", + category: "discovery", + auth: "token", + locality: "remote", + availability: "both", + input: PreflightPrInput, + output: PreflightPrOutput, +}); diff --git a/packages/loopover-contract/src/tools/repo-context.ts b/packages/loopover-contract/src/tools/repo-context.ts new file mode 100644 index 0000000000..cb499eb07e --- /dev/null +++ b/packages/loopover-contract/src/tools/repo-context.ts @@ -0,0 +1,186 @@ +// loopover_get_repo_context (#9517 pilot). +// +// Replaces the remote server's placeholder output schema, which declared six of its eight fields +// as bare `z.unknown().optional()` -- structurally advertising nothing a client could rely on. +// Every shape below is modelled from the engine types the handler actually returns: +// LaneAdvice, CollisionReport, QueueHealth, and ConfigQuality in +// packages/loopover-engine/src/signals/engine.ts. +import { z } from "zod"; +import { defineTool } from "../tool-definition.js"; +import { ownerRepoInput } from "../shared.js"; + +/** `AdvisorySeverity` / `AdvisoryFinding` (reward-risk-types.ts). The advisory finding shape every + * signal surface emits. */ +export const advisoryFindingSchema = z.looseObject({ + code: z.string(), + title: z.string(), + severity: z.enum(["info", "warning", "critical"]), + detail: z.string(), + action: z.string().optional(), + publicText: z.string().optional(), + confidence: z.number().optional(), +}); + +/** `ParticipationLane` -- which contribution lane a repo's activity actually follows. */ +export const participationLaneSchema = z.enum(["direct_pr", "issue_discovery", "split", "inactive", "unknown"]); + +/** `LaneAdvice`. The share fields are genuinely optional: a repo with no measurable activity + * yields a lane of "unknown" with neither share computed. */ +export const laneAdviceSchema = z.looseObject({ + lane: participationLaneSchema, + repoFullName: z.string(), + issueDiscoveryShare: z.number().optional(), + directPrShare: z.number().optional(), + summary: z.string(), + contributorGuidance: z.string(), + maintainerGuidance: z.string(), +}); + +/** `CollisionItem`. Nullable-and-optional fields mirror the type exactly: GitHub omits some of + * these and explicitly nulls others, and the distinction survives into the payload. */ +export const collisionItemSchema = z.looseObject({ + type: z.enum(["issue", "pull_request", "recent_merged_pull_request"]), + number: z.number(), + title: z.string(), + authorLogin: z.string().nullish(), + htmlUrl: z.string().nullish(), + labels: z.array(z.string()).optional(), + linkedIssues: z.array(z.number()).optional(), + linkedIssueClaimedAt: z.string().nullish(), + changedFiles: z.array(z.string()).optional(), + body: z.string().nullish(), +}); + +/** `CollisionCluster`. Exported separately because loopover_preflight_pr returns a flat array of + * these rather than the full report envelope below. */ +export const collisionClusterSchema = z.looseObject({ + id: z.string(), + risk: z.enum(["low", "medium", "high"]), + reason: z.string(), + items: z.array(collisionItemSchema), +}); + +/** `CollisionReport` -- duplicate-work detection across issues and PRs. */ +export const collisionReportSchema = z.looseObject({ + repoFullName: z.string(), + generatedAt: z.string(), + summary: z.looseObject({ + clusterCount: z.number(), + highRiskCount: z.number(), + itemsReviewed: z.number(), + }), + clusters: z.array(collisionClusterSchema), +}); + +/** `QueueHealth`. `signals` carries the raw counters; the two `*Flagged*` counts are deliberately + * public-safe flag counts rather than any scoring detail. */ +export const queueHealthSchema = z.looseObject({ + repoFullName: z.string(), + generatedAt: z.string(), + burdenScore: z.number(), + level: z.enum(["low", "medium", "high", "critical"]), + summary: z.string(), + signals: z.looseObject({ + openIssues: z.number(), + openPullRequests: z.number(), + unlinkedPullRequests: z.number(), + stalePullRequests: z.number(), + draftPullRequests: z.number(), + maintainerAuthoredPullRequests: z.number(), + collisionClusters: z.number(), + slopFlaggedPullRequests: z.number(), + duplicateFlaggedPullRequests: z.number(), + ageBuckets: z.looseObject({ + under7Days: z.number(), + days7To30: z.number(), + over30Days: z.number(), + }), + likelyReviewablePullRequests: z.number(), + cachedOpenPullRequests: z.number().optional(), + likelyReviewablePullRequestsSource: z.enum(["cache", "sampled_cache", "authoritative"]).optional(), + }), + findings: z.array(advisoryFindingSchema), + rankedPullRequests: z + .array( + z.looseObject({ + number: z.number(), + title: z.string(), + authorLogin: z.string(), + recommendation: z.string(), + }), + ) + .optional(), +}); + +/** `ConfigQuality` -- how well a repo's configured labels match what it actually uses. */ +export const configQualitySchema = z.looseObject({ + repoFullName: z.string(), + generatedAt: z.string(), + score: z.number(), + level: z.enum(["excellent", "good", "needs_attention", "fragile"]), + lane: laneAdviceSchema, + configuredLabels: z.array(z.string()), + observedLabels: z.array(z.string()), + notObservedConfiguredLabels: z.array(z.string()), + findings: z.array(advisoryFindingSchema), +}); + +export const GetRepoContextInput = ownerRepoInput; + +/** + * KNOWN DIVERGENCE, modelled deliberately rather than papered over. + * + * The two servers do not return the same payload for this tool name today: + * + * - The remote server's handler builds eight keys itself (repoFullName, repo, lane, queueHealth, + * queueTrends, collisions, configQuality, dataQuality). + * - The stdio server proxies GET /v1/repos/:owner/:repo/intelligence, whose response additionally + * carries status, source, generatedAt, labelAudit, maintainerLane, maintainerCutReadiness, + * contributorIntakeHealth and (optionally) burdenForecast -- and whose snapshot branch omits + * `collisions` entirely. + * + * So the only fields that can be REQUIRED here are the ones both paths always emit. Everything + * either side may omit is optional, making this schema the honest union rather than a description + * of one server that the other would fail. Converging the two payloads is a wire change with its + * own blast radius and belongs to #9518's batch for this category, not to the keystone -- this + * schema is what makes the divergence visible in the meantime. + */ +export const GetRepoContextOutput = z.looseObject({ + repoFullName: z.string(), + // `getRepository` returns null for a repo LoopOver has never synced, and the handler passes that + // through rather than failing -- the rest of the context is still useful. + repo: z.looseObject({}).nullable().optional(), + lane: laneAdviceSchema.optional(), + queueHealth: queueHealthSchema.nullable().optional(), + // Either a stored trend snapshot's payload or buildUnavailableQueueTrendReport's stand-in, which + // is why this stays loose rather than modelling one of the two. + queueTrends: z.looseObject({}).nullable().optional(), + // Absent on the REST snapshot branch the stdio server proxies. + collisions: collisionReportSchema.optional(), + configQuality: configQualitySchema.nullable().optional(), + dataQuality: z.looseObject({}).optional(), + // REST-envelope fields the stdio path carries and the remote path does not. + status: z.string().optional(), + source: z.string().optional(), + generatedAt: z.string().optional(), + labelAudit: z.looseObject({}).nullable().optional(), + maintainerLane: z.looseObject({}).nullable().optional(), + maintainerCutReadiness: z.looseObject({}).nullable().optional(), + contributorIntakeHealth: z.looseObject({}).nullable().optional(), +}); + +export type GetRepoContextInput = z.infer; +export type GetRepoContextOutput = z.infer; + +export const getRepoContextTool = defineTool({ + name: "loopover_get_repo_context", + title: "Get repo context", + description: + "Return LoopOver repo context: registration, lane, queue health, collisions, and config quality.", + category: "maintainer", + auth: "token", + locality: "remote", + availability: "both", + input: GetRepoContextInput, + output: GetRepoContextOutput, +}); diff --git a/packages/loopover-contract/tsconfig.json b/packages/loopover-contract/tsconfig.json new file mode 100644 index 0000000000..f4d72e67c5 --- /dev/null +++ b/packages/loopover-contract/tsconfig.json @@ -0,0 +1,28 @@ +{ + "extends": "../../tsconfig.json", + "compilerOptions": { + "module": "NodeNext", + "moduleResolution": "NodeNext", + // No "types" entry on purpose: this package must stay Workers-safe, so it never reaches for + // node builtins and has no need for @types/node. Adding it would let a node: import compile + // here and only fail later, in the Cloudflare bundle. + "types": [], + "declaration": true, + "sourceMap": true, + "rootDir": "src", + "outDir": "dist", + "noEmit": false, + // Dead schemas are exactly what this package exists to prevent, so the compiler rejects them + // here from day one (#9516 enabled the same pair for packages/loopover-mcp after five dead + // output-schema literals accumulated there unnoticed). + "noUnusedLocals": true, + "noUnusedParameters": true, + // Without this, the inherited root value ("./.tsbuildinfo") resolves relative to the ROOT + // config's location, not this one -- every package would then read/write the same cache file + // at the repo root and corrupt each other's incremental state (same collision documented in + // packages/loopover-engine/tsconfig.json and packages/loopover-mcp/tsconfig.json). + "tsBuildInfoFile": "./.tsbuildinfo" + }, + "include": ["src"], + "exclude": [] +} diff --git a/test/unit/contract-registry.test.ts b/test/unit/contract-registry.test.ts new file mode 100644 index 0000000000..50760a421f --- /dev/null +++ b/test/unit/contract-registry.test.ts @@ -0,0 +1,250 @@ +// Meta-tests for @loopover/contract's registry (#9517). +// +// These assert the CONVENTIONS the package documents, so a contract that violates one fails here +// rather than being discovered as drift months later. They are the cheap, always-run half of the +// enforcement; the contract validator (#9520) is the expensive half that actually calls each tool +// and validates its structuredContent against the schema advertised below. +import { describe, expect, it } from "vitest"; +import { Ajv2020 } from "ajv/dist/2020.js"; +import { z } from "zod"; +import { + TOOL_CATEGORIES, + TOOL_AUTH_LEVELS, + TOOL_LOCALITIES, + TOOL_AVAILABILITIES, + TOOL_CONTRACTS, + listToolDefinitions, + getToolContract, + buildOpenAIToolSpecs, + buildAnthropicToolSpecs, + buildAgentToolsIndex, + AGENT_TOOLS_INDEX_SCHEMA_VERSION, + AUTONOMY_LEVELS, + MAINTAIN_ACTION_CLASSES, + PROPOSE_ACTION_CLASSES, + PREFLIGHT_LIMITS, +} from "@loopover/contract"; +import { LocalStatusStructuredInput } from "@loopover/contract/tools"; +import { GetRepoContextInput } from "@loopover/contract/tools"; +import { PREFLIGHT_LIMITS as ENGINE_PREFLIGHT_LIMITS } from "../../packages/loopover-engine/src/signals/preflight-limits.js"; +import { AUTONOMY_LEVELS as ENGINE_AUTONOMY_LEVELS, AGENT_ACTION_CLASSES as ENGINE_AGENT_ACTION_CLASSES } from "../../packages/loopover-engine/src/settings/autonomy.js"; + +describe("contract tool registry", () => { + it("registers at least one tool", () => { + expect(TOOL_CONTRACTS.length).toBeGreaterThan(0); + }); + + it("gives every tool a unique name", () => { + const names = TOOL_CONTRACTS.map((contract) => contract.name); + expect(new Set(names).size).toBe(names.length); + }); + + it("names every tool with the loopover_ prefix", () => { + for (const contract of TOOL_CONTRACTS) { + expect(contract.name, contract.name).toMatch(/^loopover_[a-z0-9_]+$/); + } + }); + + it("gives every tool a substantive description and a title", () => { + for (const contract of TOOL_CONTRACTS) { + expect(contract.description.length, contract.name).toBeGreaterThan(20); + expect(contract.title.trim().length, contract.name).toBeGreaterThan(0); + } + }); + + it("draws category, auth, locality, and availability from the declared vocabularies", () => { + for (const contract of TOOL_CONTRACTS) { + expect(TOOL_CATEGORIES, contract.name).toContain(contract.category); + expect(TOOL_AUTH_LEVELS, contract.name).toContain(contract.auth); + expect(TOOL_LOCALITIES, contract.name).toContain(contract.locality); + expect(TOOL_AVAILABILITIES, contract.name).toContain(contract.availability); + } + }); + + it("declares input and output as zod objects, never bare unknowns", () => { + for (const contract of TOOL_CONTRACTS) { + expect(contract.input, contract.name).toBeInstanceOf(z.ZodObject); + expect(contract.output, contract.name).toBeInstanceOf(z.ZodObject); + } + }); + + it("resolves a contract by name and returns undefined for an unknown one", () => { + const first = TOOL_CONTRACTS[0]!; + expect(getToolContract(first.name)).toBe(first); + expect(getToolContract("loopover_not_a_real_tool")).toBeUndefined(); + }); +}); + +describe("contract projection", () => { + const definitions = listToolDefinitions(); + + it("projects every contract exactly once", () => { + expect(definitions.map((tool) => tool.name).sort()).toEqual(TOOL_CONTRACTS.map((contract) => contract.name).sort()); + }); + + it("emits object-typed JSON Schema that Ajv can compile for both input and output", () => { + // strict:false mirrors the contract validator's own Ajv setup: draft-2020-12 output schemas + // legitimately carry keywords Ajv's strict mode would reject as unknown. + const ajv = new Ajv2020({ strict: false }); + for (const tool of definitions) { + expect(tool.inputSchema.type, tool.name).toBe("object"); + expect(tool.outputSchema.type, tool.name).toBe("object"); + expect(() => ajv.compile(tool.inputSchema), `${tool.name} input`).not.toThrow(); + expect(() => ajv.compile(tool.outputSchema), `${tool.name} output`).not.toThrow(); + } + }); + + it("keeps output schemas open so an added server field cannot invalidate an older client", () => { + // The wire-compatibility rule the README states: an MCP output schema is a floor, not a fence. + // z.looseObject is what produces the open additionalProperties; a plain z.object would emit + // `false` here and silently make every future field addition a breaking change. + for (const tool of definitions) { + expect(tool.outputSchema.additionalProperties, tool.name).not.toBe(false); + } + }); + + it("advertises closed input schemas, matching the shape the MCP SDK already published", () => { + // z.object emits additionalProperties:false. Note this is the ADVERTISED contract only: zod's + // runtime STRIPS unknown keys rather than rejecting them (see the test below), so the two do + // not agree today. Whether to close that gap with z.strictObject is a deliberate wire decision + // recorded on #9518, not something to change silently during a behavior-preserving migration. + for (const tool of listToolDefinitions()) { + expect(tool.inputSchema.additionalProperties, tool.name).toBe(false); + } + }); + + it("strips rather than rejects an unknown input key, preserving today's runtime behavior", () => { + // Pinned deliberately. The migration promised no wire-visible change, and this is the one place + // where the advertised schema and the runtime disagree -- pinning it means a future switch to + // strict parsing has to be an intentional edit to this assertion, not an accident. + const parsed = LocalStatusStructuredInput.safeParse({ definitelyNotAField: 1 }); + expect(parsed.success).toBe(true); + expect(parsed.success && parsed.data).toEqual({}); + }); + + it("rejects a known input field carrying the wrong type", () => { + expect(GetRepoContextInput.safeParse({ owner: "a", repo: 5 }).success).toBe(false); + expect(GetRepoContextInput.safeParse({ owner: "", repo: "b" }).success).toBe(false); + expect(GetRepoContextInput.safeParse({ owner: "a", repo: "b" }).success).toBe(true); + }); + + it("defaults annotations to read-only and non-destructive", () => { + const readOnly = definitions.find((tool) => !TOOL_CONTRACTS.find((c) => c.name === tool.name)?.annotations); + expect(readOnly?.annotations).toEqual({ readOnlyHint: true, destructiveHint: false }); + }); + + it("marks every destructive tool as not read-only", () => { + // A tool cannot coherently claim to both mutate state and be read-only; this catches an entry + // that sets destructiveHint without clearing the read-only default. + for (const tool of definitions) { + if (tool.annotations.destructiveHint) expect(tool.annotations.readOnlyHint, tool.name).toBe(false); + } + }); + + it("filters by locality, availability, and category without mutating the registry", () => { + const remote = listToolDefinitions({ locality: ["remote"] }); + expect(remote.every((tool) => tool.locality === "remote")).toBe(true); + expect(remote.length).toBeLessThanOrEqual(definitions.length); + + const selfhost = listToolDefinitions({ availability: ["selfhost"] }); + // `both` is the absence of a restriction, not a third deployment, so it must survive a + // selfhost filter -- the bug this asserts against is treating it as a distinct value. + expect(selfhost.every((tool) => tool.availability === "selfhost" || tool.availability === "both")).toBe(true); + expect(selfhost.some((tool) => tool.availability === "both")).toBe(true); + + const cloud = listToolDefinitions({ availability: ["cloud"] }); + expect(cloud.some((tool) => tool.availability === "selfhost")).toBe(false); + + const admin = listToolDefinitions({ category: ["admin"] }); + expect(admin.every((tool) => tool.category === "admin")).toBe(true); + + // Combined filters intersect rather than union. + const none = listToolDefinitions({ locality: ["miner"], availability: ["cloud"] }); + expect(none.every((tool) => tool.locality === "miner")).toBe(true); + + expect(TOOL_CONTRACTS.length).toBeGreaterThan(0); + }); +}); + +describe("agent tool specs", () => { + const definitions = listToolDefinitions(); + + it("projects OpenAI specs from the same definitions", () => { + const specs = buildOpenAIToolSpecs(definitions); + expect(specs).toHaveLength(definitions.length); + for (const [index, spec] of specs.entries()) { + const tool = definitions[index]!; + expect(spec.type).toBe("function"); + expect(spec.function.name).toBe(tool.name); + expect(spec.function.description).toBe(tool.description); + expect(spec.function.parameters).toEqual(tool.inputSchema); + } + }); + + it("projects Anthropic specs from the same definitions", () => { + const specs = buildAnthropicToolSpecs(definitions); + expect(specs).toHaveLength(definitions.length); + for (const [index, spec] of specs.entries()) { + const tool = definitions[index]!; + expect(spec.name).toBe(tool.name); + expect(spec.description).toBe(tool.description); + expect(spec.input_schema).toEqual(tool.inputSchema); + } + }); + + it("builds an agent-tools index carrying both spec flavors and the executor contract", () => { + const index = buildAgentToolsIndex(definitions, { endpoint: "https://api.loopover.ai/mcp" }); + expect(index.schema_version).toBe(AGENT_TOOLS_INDEX_SCHEMA_VERSION); + expect(index.executor).toEqual({ + transport: "mcp-streamable-http", + endpoint: "https://api.loopover.ai/mcp", + jsonrpc_method: "tools/call", + }); + expect(index.tools).toEqual(definitions.map((tool) => tool.name)); + expect(index.specs.openai).toHaveLength(definitions.length); + expect(index.specs.anthropic).toHaveLength(definitions.length); + }); + + it("returns empty specs for an empty tool list", () => { + expect(buildOpenAIToolSpecs([])).toEqual([]); + expect(buildAnthropicToolSpecs([])).toEqual([]); + const index = buildAgentToolsIndex([], { endpoint: "https://example.test/mcp" }); + expect(index.tools).toEqual([]); + expect(index.specs.openai).toEqual([]); + }); +}); + +describe("contract enums", () => { + it("pins autonomy levels against the engine's live enum", () => { + // packages/loopover-engine/src/settings/autonomy.ts still declares its own copy because it is + // an engine-parity twin of src/settings/autonomy.ts; inverting that pair to import from the + // contract is #9518's batch work. Until then this pin is what makes the two impossible to + // drift apart -- the same technique test/unit/mcp-cli-maintain.test.ts used for the stdio + // hand-copy this package replaces. + expect([...AUTONOMY_LEVELS]).toEqual([...ENGINE_AUTONOMY_LEVELS]); + }); + + it("keeps the operator-settable action classes a strict subset of the engine's full list", () => { + // MAINTAIN_ACTION_CLASSES is deliberately NOT the engine's full AGENT_ACTION_CLASSES -- it is + // the subset the maintain surface exposes. Asserting subset-ness (rather than equality) pins + // the intended relationship without re-coupling them. + for (const actionClass of MAINTAIN_ACTION_CLASSES) { + expect(ENGINE_AGENT_ACTION_CLASSES as readonly string[], actionClass).toContain(actionClass); + } + expect(MAINTAIN_ACTION_CLASSES.length).toBeLessThan(ENGINE_AGENT_ACTION_CLASSES.length); + }); + + it("derives the propose set as the maintain set plus review_state_label", () => { + expect([...PROPOSE_ACTION_CLASSES]).toEqual([...MAINTAIN_ACTION_CLASSES, "review_state_label"]); + for (const actionClass of PROPOSE_ACTION_CLASSES) { + expect(ENGINE_AGENT_ACTION_CLASSES as readonly string[], actionClass).toContain(actionClass); + } + }); + + it("pins the preflight input bounds against the engine's live limits", () => { + // The contract restates PREFLIGHT_LIMITS because it cannot import the engine (zod-only leaf). + // Without this pin, raising a bound on one side only would produce a schema that either rejects + // input the server accepts, or accepts input the server silently truncates. + expect(PREFLIGHT_LIMITS).toEqual(ENGINE_PREFLIGHT_LIMITS); + }); +}); diff --git a/turbo.json b/turbo.json index b7dfeaf22a..365f8f9ad0 100644 --- a/turbo.json +++ b/turbo.json @@ -46,6 +46,14 @@ ], "outputs": [] }, + // Zod-only leaf: no ^build edge because it depends on nothing in the workspace, which is the + // property that lets every other package (Worker, both stdio bins, miner, UI) depend on IT + // without dragging a build graph along. .tsbuildinfo restored alongside dist/ for the same + // reason @loopover/mcp#build declares it: a tsc run seeing a stale .tsbuildinfo with a missing + // dist/ silently no-ops, believing the build is already done. + "@loopover/contract#build": { + "outputs": ["dist/**", ".tsbuildinfo"] + }, "@loopover/engine#build": { "outputs": ["dist/**"] }, diff --git a/vitest.config.ts b/vitest.config.ts index 94b6124740..fa6ad941bf 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -58,6 +58,10 @@ export default defineConfig({ // export the way bin/loopover-miner-mcp.ts already was. "packages/loopover-miner/bin/**/*.ts", "packages/discovery-index/src/**/*.ts", + // @loopover/contract is pure schema + pure projection functions with no I/O of any kind, imported + // in-process by test/unit/contract-*.test.ts -- fully unit-coverable, so it is graded like any + // other src surface rather than exempted. + "packages/loopover-contract/src/**/*.ts", // All 5 packages/loopover-mcp/lib/*.ts files (format-table/local-branch/redact-local-path/ // telemetry/cli-error) are imported in-process by test/unit/*.test.ts (cli-error's own // test/unit/mcp-cli-error.test.ts landed in #7409), so a PR touching any of them is covered by From 60fb0d57e2336132efbe670747a90b51cbb651da Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 27 Jul 2026 23:10:39 -0700 Subject: [PATCH 2/5] 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 --- apps/loopover-ui/public/openapi.json | 67 ++++++++++++++++++++++- package-lock.json | 2 + package.json | 1 + packages/loopover-mcp/bin/loopover-mcp.ts | 45 ++++++++------- packages/loopover-mcp/package.json | 1 + src/mcp/server.ts | 36 ++++++++---- src/openapi/schemas.ts | 28 +++++++++- 7 files changed, 148 insertions(+), 32 deletions(-) diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index 1be226d847..cf691ed367 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -1417,6 +1417,15 @@ "sampled_cache", "authoritative" ] + }, + "draftPullRequests": { + "type": "number" + }, + "slopFlaggedPullRequests": { + "type": "number" + }, + "duplicateFlaggedPullRequests": { + "type": "number" } }, "required": [ @@ -1424,8 +1433,11 @@ "openPullRequests", "unlinkedPullRequests", "stalePullRequests", + "draftPullRequests", "maintainerAuthoredPullRequests", "collisionClusters", + "slopFlaggedPullRequests", + "duplicateFlaggedPullRequests", "ageBuckets", "likelyReviewablePullRequests" ] @@ -1435,6 +1447,32 @@ "items": { "$ref": "#/components/schemas/Finding" } + }, + "rankedPullRequests": { + "type": "array", + "items": { + "type": "object", + "properties": { + "number": { + "type": "number" + }, + "title": { + "type": "string" + }, + "authorLogin": { + "type": "string" + }, + "recommendation": { + "type": "string" + } + }, + "required": [ + "number", + "title", + "authorLogin", + "recommendation" + ] + } } }, "required": [ @@ -1527,7 +1565,8 @@ "type": "string", "enum": [ "issue", - "pull_request" + "pull_request", + "recent_merged_pull_request" ] }, "number": { @@ -1543,6 +1582,32 @@ "htmlUrl": { "type": "string", "nullable": true + }, + "labels": { + "type": "array", + "items": { + "type": "string" + } + }, + "linkedIssues": { + "type": "array", + "items": { + "type": "number" + } + }, + "linkedIssueClaimedAt": { + "type": "string", + "nullable": true + }, + "changedFiles": { + "type": "array", + "items": { + "type": "string" + } + }, + "body": { + "type": "string", + "nullable": true } }, "required": [ diff --git a/package-lock.json b/package-lock.json index 22accdc20c..be752261a9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,6 +16,7 @@ "@asteasolutions/zod-to-openapi": "^8.5.0", "@cloudflare/puppeteer": "^1.1.0", "@hono/node-server": "^2.0.11", + "@loopover/contract": "^0.1.0", "@loopover/engine": "*", "@modelcontextprotocol/sdk": "1.29.0", "@octokit/core": "^7.0.6", @@ -22670,6 +22671,7 @@ "version": "3.15.2", "license": "AGPL-3.0-only", "dependencies": { + "@loopover/contract": "^0.1.0", "@loopover/engine": "^3.15.2", "@modelcontextprotocol/sdk": "1.29.0", "posthog-node": "^5.46.1", diff --git a/package.json b/package.json index 4348f973f6..ee87e97d9b 100644 --- a/package.json +++ b/package.json @@ -124,6 +124,7 @@ "@asteasolutions/zod-to-openapi": "^8.5.0", "@cloudflare/puppeteer": "^1.1.0", "@hono/node-server": "^2.0.11", + "@loopover/contract": "^0.1.0", "@loopover/engine": "*", "@modelcontextprotocol/sdk": "1.29.0", "@octokit/core": "^7.0.6", diff --git a/packages/loopover-mcp/bin/loopover-mcp.ts b/packages/loopover-mcp/bin/loopover-mcp.ts index f661042705..f9927e9f8d 100644 --- a/packages/loopover-mcp/bin/loopover-mcp.ts +++ b/packages/loopover-mcp/bin/loopover-mcp.ts @@ -40,6 +40,21 @@ import { buildProgressSnapshot } from "@loopover/engine"; // #6755: the same pure bridge the remote MCP tool + /v1/loop/intake-idea both call. import { validateIdeaSubmission, buildTaskGraph, buildClaimPlan } from "@loopover/engine"; import { z } from "zod"; +// #9517: pilot-tool schemas come from the shared contract, replacing shapes this file previously +// hand-mirrored from src/mcp/server.ts. Output schemas arrive with them, so these tools stop +// returning unschematized structured content. +import { + GetPrReviewabilityInput, + GetPrReviewabilityOutput, + GetRepoContextInput, + GetRepoContextOutput, + LocalStatusStructuredInput, + LocalStatusStructuredOutput, + PredictGateInput, + PredictGateOutput, + PreflightPrInput, + PreflightPrOutput, +} from "@loopover/contract/tools"; import { buildBranchAnalysisPayload, collectLocalDiff, collectLocalBranchMetadata, probeLocalScorer, referenceScorePreviewExample, resolveScorePreviewCommand, resolveWorkspaceCwd, sanitizeLocalScorerStatus, setupGuidanceForLocalScorer, isTestFile } from "../lib/local-branch.js"; import { formatTable } from "../lib/format-table.js"; import { argsWantJson, describeCliError, reportCliFailure } from "../lib/cli-error.js"; @@ -1747,7 +1762,8 @@ registerStdioTool( "loopover_get_repo_context", { description: stdioToolDescription("loopover_get_repo_context"), - inputSchema: ownerRepoShape, + inputSchema: GetRepoContextInput.shape, + outputSchema: GetRepoContextOutput.shape, }, async ({ owner, repo }: any) => { const prefix = `/v1/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}`; @@ -1759,7 +1775,8 @@ registerStdioTool( "loopover_get_pr_reviewability", { description: stdioToolDescription("loopover_get_pr_reviewability"), - inputSchema: ownerRepoPullShape, + inputSchema: GetPrReviewabilityInput.shape, + outputSchema: GetPrReviewabilityOutput.shape, }, async ({ owner, repo, number }: any) => { const prefix = `/v1/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}`; @@ -1987,7 +2004,8 @@ registerStdioTool( "loopover_preflight_pr", { description: stdioToolDescription("loopover_preflight_pr"), - inputSchema: preflightShape, + inputSchema: PreflightPrInput.shape, + outputSchema: PreflightPrOutput.shape, }, async (input: any) => toolResult("LoopOver PR preflight.", await apiPost("/v1/preflight/pr", input)), ); @@ -2294,7 +2312,8 @@ registerStdioTool( "loopover_predict_gate", { description: stdioToolDescription("loopover_predict_gate"), - inputSchema: predictGateShape, + inputSchema: PredictGateInput.shape, + outputSchema: PredictGateOutput.shape, }, async (input: any) => { const body = { @@ -2962,22 +2981,8 @@ registerStdioTool( "loopover_local_status_structured", { description: stdioToolDescription("loopover_local_status_structured"), - inputSchema: { - cwd: z.string().optional(), - baseRef: z.string().optional(), - repoFullName: z.string().min(3).optional(), - }, - outputSchema: z.object({ - apiUrl: z.string(), - package: z.object({ name: z.string(), version: z.string() }), - hasToken: z.boolean(), - profile: z.record(z.string(), z.unknown()), - authLogin: z.string().nullable(), - sessionExpiresAt: z.string().nullable(), - sourceUploadDefault: z.boolean(), - sourceUploadSupported: z.boolean(), - git: z.record(z.string(), z.unknown()), - }), + inputSchema: LocalStatusStructuredInput.shape, + outputSchema: LocalStatusStructuredOutput.shape, }, async (input: any) => { let git = null; diff --git a/packages/loopover-mcp/package.json b/packages/loopover-mcp/package.json index c7357a1622..b1ecdc8339 100644 --- a/packages/loopover-mcp/package.json +++ b/packages/loopover-mcp/package.json @@ -45,6 +45,7 @@ "build:verify": "node scripts/check-syntax.mjs" }, "dependencies": { + "@loopover/contract": "^0.1.0", "@loopover/engine": "^3.15.2", "@modelcontextprotocol/sdk": "1.29.0", "posthog-node": "^5.46.1", diff --git a/src/mcp/server.ts b/src/mcp/server.ts index a46341b2a0..5dc4d44d04 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -4,6 +4,22 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import type { RequestHandlerExtra } from "@modelcontextprotocol/sdk/shared/protocol.js"; import { ElicitResultSchema, type ServerNotification, type ServerRequest } from "@modelcontextprotocol/sdk/types.js"; import { z } from "zod"; +// #9517: the pilot tools' schemas come from the shared contract instead of being declared here. +// `.shape` is what the MCP SDK's registerTool wants; the same ZodObject also drives the JSON Schema +// the agent-tool specs and the contract validator read, so there is one definition per tool rather +// than one per server. +import { + AdminGetConfigInput, + AdminGetConfigOutput, + GetPrReviewabilityInput, + GetPrReviewabilityOutput, + GetRepoContextInput, + GetRepoContextOutput, + PredictGateInput, + PredictGateOutput, + PreflightPrInput, + PreflightPrOutput, +} from "@loopover/contract/tools"; import { MAX_FIND_OPPORTUNITIES_LANGUAGE_LENGTH, MAX_FIND_OPPORTUNITIES_LANGUAGES, @@ -2180,8 +2196,8 @@ export class LoopoverMcp { "loopover_get_repo_context", { description: "Return LoopOver repo context: registration, lane, queue health, collisions, and config quality.", - inputSchema: ownerRepoShape, - outputSchema: repoContextOutputSchema, + inputSchema: GetRepoContextInput.shape, + outputSchema: GetRepoContextOutput.shape, }, async (input) => this.toolResult(await this.getRepoContext(input)), ); @@ -2454,8 +2470,8 @@ export class LoopoverMcp { { description: "Predict whether a planned PR would pass the repo's LoopOver gate, from its PUBLIC .loopover.yml only — an agent-native pre-submission self-check that works on ANY repo (no Gittensor account). Under the oss-anti-slop pack the verdict applies to any author; self-scoped to the authenticated login.", - inputSchema: predictGateShape, - outputSchema: predictGateOutputSchema, + inputSchema: PredictGateInput.shape, + outputSchema: PredictGateOutput.shape, }, async (input) => this.toolResult(await this.predictGate(input)), ); @@ -2650,8 +2666,8 @@ export class LoopoverMcp { "loopover_preflight_pr", { description: "Preflight a planned PR for lane correctness, duplicate risk, linked issues, and review burden.", - inputSchema: preflightShape, - outputSchema: preflightResultOutputSchema, + inputSchema: PreflightPrInput.shape, + outputSchema: PreflightPrOutput.shape, }, async (input) => this.toolResult(await this.preflightPr(input)), ); @@ -2742,8 +2758,8 @@ export class LoopoverMcp { { description: "Return the cached or freshly-computed reviewability report for an open PR: how ready it is to review/merge, the blocking or advisory signals against it, and its lane/duplicate/linked-issue context. Metadata-only, repo-scoped, no GitHub writes.", - inputSchema: ownerRepoPullShape, - outputSchema: freshnessResponseOutputSchema, + inputSchema: GetPrReviewabilityInput.shape, + outputSchema: GetPrReviewabilityOutput.shape, }, async (input) => this.toolResult(await this.getPrReviewability(input)), ); @@ -3298,8 +3314,8 @@ export class LoopoverMcp { { description: "Self-hosted-operator only. Read this instance's own private .loopover.yml config: the merged effective config for a repo (shared base + global default + per-repo override), or just the raw global-default layer, or just the raw per-repo layer. Requires LOOPOVER_MCP_ADMIN_TOKEN. Returns configured=false if LOOPOVER_REPO_CONFIG_DIR is unset.", - inputSchema: adminConfigScopeShape, - outputSchema: adminGetConfigOutputSchema, + inputSchema: AdminGetConfigInput.shape, + outputSchema: AdminGetConfigOutput.shape, }, async (input) => this.toolResult(await this.adminGetConfig(input)), ); diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index bd0dc34935..d638997608 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -265,13 +265,22 @@ export const LaneAdviceSchema = z }) .openapi("LaneAdvice"); +// #9517: `recent_merged_pull_request` was missing from `type` even though buildCollisionReport is +// handed recent merged PRs and emits items with exactly that discriminant -- so a client validating +// a real response against the published spec rejected it. The remaining fields were absent too, +// which understated what this endpoint actually returns. export const CollisionItemSchema = z .object({ - type: z.enum(["issue", "pull_request"]), + type: z.enum(["issue", "pull_request", "recent_merged_pull_request"]), number: z.number(), title: z.string(), authorLogin: z.string().nullable().optional(), htmlUrl: z.string().nullable().optional(), + labels: z.array(z.string()).optional(), + linkedIssues: z.array(z.number()).optional(), + linkedIssueClaimedAt: z.string().nullable().optional(), + changedFiles: z.array(z.string()).optional(), + body: z.string().nullable().optional(), }) .openapi("CollisionItem"); @@ -304,13 +313,20 @@ export const QueueHealthSchema = z burdenScore: z.number(), level: z.enum(["low", "medium", "high", "critical"]), summary: z.string(), + // #9517: draftPullRequests, slopFlaggedPullRequests and duplicateFlaggedPullRequests are all + // REQUIRED on the QueueHealth type and always emitted by buildQueueHealth, but were missing + // here -- the published spec understated the response. The two flagged counts are deliberately + // public-safe counts, carrying no score or ranking detail. signals: z.object({ openIssues: z.number(), openPullRequests: z.number(), unlinkedPullRequests: z.number(), stalePullRequests: z.number(), + draftPullRequests: z.number(), maintainerAuthoredPullRequests: z.number(), collisionClusters: z.number(), + slopFlaggedPullRequests: z.number(), + duplicateFlaggedPullRequests: z.number(), ageBuckets: z.object({ under7Days: z.number(), days7To30: z.number(), @@ -321,6 +337,16 @@ export const QueueHealthSchema = z likelyReviewablePullRequestsSource: z.enum(["cache", "sampled_cache", "authoritative"]).optional(), }), findings: z.array(FindingSchema), + rankedPullRequests: z + .array( + z.object({ + number: z.number(), + title: z.string(), + authorLogin: z.string(), + recommendation: z.string(), + }), + ) + .optional(), }) .openapi("QueueHealth"); From 0e09101abc691842e3a309c6cc3a4cc3b101f530 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 27 Jul 2026 23:23:49 -0700 Subject: [PATCH 3/5] 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 --- .github/workflows/ci.yml | 8 ++++++++ .github/workflows/release-selfhost.yml | 5 +++++ .github/workflows/selfhost.yml | 5 +++++ Dockerfile | 4 ++++ 4 files changed, 22 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0ee5b9dd7e..ce22fd7f20 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -467,6 +467,14 @@ jobs: key: turbo-code-${{ hashFiles('package-lock.json') }}-${{ github.run_id }} restore-keys: | turbo-code-${{ hashFiles('package-lock.json') }}- + # @loopover/contract's "types" resolve to packages/loopover-contract/dist/index.d.ts, and src/ + + # packages/loopover-mcp both import it -- so like the engine below, typecheck cannot run until it + # has been built. Unconditional rather than gated on a `contract` path filter: every consumer of it + # (backend, mcp, miner, ui) can pull it into the typecheck surface, and building a zod-only leaf + # package with no dependencies of its own is cheap enough that gating it would buy nothing but a + # class of skipped-build failures. + - name: Build contract package + run: npx turbo run build --filter=@loopover/contract # mcp/miner are in this gate because "Typecheck" below already runs for them, and typecheck's real # surface reaches @loopover/engine -- test/** imports it directly, and its "types" resolve to # packages/loopover-engine/dist/index.d.ts, which only exists once this step has run. An mcp-only or diff --git a/.github/workflows/release-selfhost.yml b/.github/workflows/release-selfhost.yml index 0e47034cee..82a843e94b 100644 --- a/.github/workflows/release-selfhost.yml +++ b/.github/workflows/release-selfhost.yml @@ -129,6 +129,11 @@ jobs: # That package's dist/ is gitignored and only exists after this build step -- the regular CI smoke # test's narrower (non --all) build never hits this import chain, so it never caught the gap that # ci.yml's own validate-code job hit for ordinary backend PRs (fixed there separately). + # Built before the engine for the same reason the Dockerfile does: src/'s import graph reaches + # @loopover/contract, whose package exports resolve to dist/, so anything type-checking or + # bundling src/ needs it emitted first. Zod-only leaf, no workspace dependencies of its own. + - name: Build contract package + run: npm run build --workspace @loopover/contract - name: Build engine package run: npm run build --workspace @loopover/engine diff --git a/.github/workflows/selfhost.yml b/.github/workflows/selfhost.yml index 15fcffc01b..22b1a3f58d 100644 --- a/.github/workflows/selfhost.yml +++ b/.github/workflows/selfhost.yml @@ -77,6 +77,11 @@ jobs: # dist/ is gitignored and only exists after this build step, so the test fails to resolve the # package's exports without it -- this workflow never needed the engine package built before, so it # never had this step; it does now. + # Built before the engine for the same reason the Dockerfile does: src/'s import graph reaches + # @loopover/contract, whose package exports resolve to dist/, so anything type-checking or + # bundling src/ needs it emitted first. Zod-only leaf, no workspace dependencies of its own. + - name: Build contract package + run: npm run build --workspace @loopover/contract - name: Build engine package run: npm run build --workspace @loopover/engine diff --git a/Dockerfile b/Dockerfile index 4ffe7db2e6..feafb6c8e3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -19,6 +19,10 @@ COPY . . # --ignore-scripts: no native builds are needed (SQLite is the built-in node:sqlite; @hono/node-server is # pure JS; esbuild ships its binary as an optional dependency, not a script). RUN npm ci --ignore-scripts +# @loopover/contract before the engine: src/ imports it, and its package exports resolve to dist/, +# so esbuild in build-selfhost.ts below cannot resolve the import until it has been emitted. A +# zod-only leaf with no workspace dependencies, so it builds first and standalone. +RUN npm --workspace @loopover/contract run build RUN npm --workspace @loopover/engine run build # --all: bundle every dependency into one self-contained dist/server.mjs, so the runtime image needs no # node_modules (≈10× smaller). The bundle has zero `cloudflare:*` imports (stubbed at build), so no loader. From d837d1a8775829eeee22ff9b7f63db2a4a690be8 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 00:04:08 -0700 Subject: [PATCH 4/5] 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. --- .../src/tools/pr-reviewability.ts | 11 +++-- test/unit/mcp-pr-reviewability.test.ts | 43 +++++++++++++++++-- test/unit/mcp-predict-gate.test.ts | 6 ++- test/unit/support/mcp-cli-harness.ts | 10 +++++ 4 files changed, 63 insertions(+), 7 deletions(-) diff --git a/packages/loopover-contract/src/tools/pr-reviewability.ts b/packages/loopover-contract/src/tools/pr-reviewability.ts index d491c0ead5..34d57a3880 100644 --- a/packages/loopover-contract/src/tools/pr-reviewability.ts +++ b/packages/loopover-contract/src/tools/pr-reviewability.ts @@ -19,12 +19,17 @@ import { defineTool } from "../tool-definition.js"; import { ownerRepoPullInput } from "../shared.js"; /** `PullRequestReviewability` (packages/loopover-engine/src/reward-risk.ts). All ten fields are - * required and non-nullable on the type; `PullRequestReviewabilitySchema` in src/openapi/schemas.ts - * is a field-for-field match, so REST and MCP genuinely agree on this shape. */ + * required and non-nullable on the type when freshly computed; `PullRequestReviewabilitySchema` in + * src/openapi/schemas.ts is a field-for-field match there, so REST and a fresh MCP computation + * genuinely agree on this shape. `generatedAt` is optional here specifically because the cached + * path can serve an older persisted snapshot row: the remote handler's own fallback chain + * (`cached.generatedAt || payload.generatedAt || new Date().toISOString()`) only exists because a + * stored payload's `generatedAt` is not guaranteed -- the schema has to describe that real, + * defended-against case, not just the freshly-computed one. */ export const pullRequestReviewabilitySchema = z.looseObject({ repoFullName: z.string(), pullNumber: z.number(), - generatedAt: z.string(), + generatedAt: z.string().optional(), score: z.number(), action: z.enum(["review_now", "needs_author", "likely_duplicate", "close_or_redirect", "watch", "maintainer_lane"]), noiseSources: z.array(z.string()), diff --git a/test/unit/mcp-pr-reviewability.test.ts b/test/unit/mcp-pr-reviewability.test.ts index 8cdace7270..5d521abbb2 100644 --- a/test/unit/mcp-pr-reviewability.test.ts +++ b/test/unit/mcp-pr-reviewability.test.ts @@ -77,7 +77,21 @@ describe("MCP loopover_get_pr_reviewability (#6154)", () => { targetKey: "owner/repo#7", repoFullName: "owner/repo", generatedAt: "2026-05-30T00:00:00.000Z", - payload: { repoFullName: "owner/repo", pullNumber: 7, generatedAt: "2026-05-30T00:00:00.000Z", summary: "cached" }, + // #9517: a persisted pr-reviewability snapshot is always a full PullRequestReviewability -- + // src/api/routes.ts's reviewability route persists the whole computed object, never a partial + // one -- so the fixture matches that shape now that the tool validates it against a real + // output schema instead of z.unknown(). + payload: { + repoFullName: "owner/repo", + pullNumber: 7, + generatedAt: "2026-05-30T00:00:00.000Z", + score: 0.5, + action: "review_now", + noiseSources: [], + whyThisHelps: [], + maintainerNextSteps: [], + privateSummary: "cached", + }, }); const client = await connect(env); const result = await client.callTool({ name: "loopover_get_pr_reviewability", arguments: { owner: "owner", repo: "repo", number: 7 } }); @@ -93,7 +107,19 @@ describe("MCP loopover_get_pr_reviewability (#6154)", () => { `insert into signal_snapshots (id, signal_type, target_key, repo_full_name, payload_json, generated_at) values ('reviewability-payload-generated', 'pr-reviewability', 'owner/repo#7', 'owner/repo', ?, '')`, ) - .bind(JSON.stringify({ repoFullName: "owner/repo", pullNumber: 7, generatedAt: "2026-05-29T00:00:00.000Z", summary: "payload" })) + .bind( + JSON.stringify({ + repoFullName: "owner/repo", + pullNumber: 7, + generatedAt: "2026-05-29T00:00:00.000Z", + score: 0.5, + action: "review_now", + noiseSources: [], + whyThisHelps: [], + maintainerNextSteps: [], + privateSummary: "payload", + }), + ) .run(); const client = await connect(env); const result = await client.callTool({ name: "loopover_get_pr_reviewability", arguments: { owner: "owner", repo: "repo", number: 7 } }); @@ -108,7 +134,18 @@ describe("MCP loopover_get_pr_reviewability (#6154)", () => { `insert into signal_snapshots (id, signal_type, target_key, repo_full_name, payload_json, generated_at) values ('reviewability-no-generated', 'pr-reviewability', 'owner/repo#7', 'owner/repo', ?, '')`, ) - .bind(JSON.stringify({ repoFullName: "owner/repo", pullNumber: 7, summary: "no-timestamp" })) + .bind( + JSON.stringify({ + repoFullName: "owner/repo", + pullNumber: 7, + score: 0.5, + action: "review_now", + noiseSources: [], + whyThisHelps: [], + maintainerNextSteps: [], + privateSummary: "no-timestamp", + }), + ) .run(); const client = await connect(env); const result = await client.callTool({ name: "loopover_get_pr_reviewability", arguments: { owner: "owner", repo: "repo", number: 7 } }); diff --git a/test/unit/mcp-predict-gate.test.ts b/test/unit/mcp-predict-gate.test.ts index 61a5617259..29000256ff 100644 --- a/test/unit/mcp-predict-gate.test.ts +++ b/test/unit/mcp-predict-gate.test.ts @@ -51,9 +51,13 @@ describe("MCP loopover_predict_gate", () => { const env = createTestEnv(); const client = await connect(env); + // #9517: the per-path cap is now @loopover/contract's PREDICT_GATE_MAX_CHANGED_PATH_CHARS + // (400), the wider of the two servers' historical bounds (this server's own predictGateShape + // used PREFLIGHT_LIMITS.changedFileChars, 300; the stdio server used 400) -- a shared input + // schema may only widen a bound, never tighten one, so 400 is what both servers accept today. const result = await client.callTool({ name: "loopover_predict_gate", - arguments: { login: "miner1", owner: "acme", repo: "widgets", title: "Huge path", changedPaths: [`src/${"a".repeat(301)}.ts`] }, + arguments: { login: "miner1", owner: "acme", repo: "widgets", title: "Huge path", changedPaths: [`src/${"a".repeat(401)}.ts`] }, }); expect(result.isError).toBe(true); diff --git a/test/unit/support/mcp-cli-harness.ts b/test/unit/support/mcp-cli-harness.ts index 7976bdb7cf..c38f340d43 100644 --- a/test/unit/support/mcp-cli-harness.ts +++ b/test/unit/support/mcp-cli-harness.ts @@ -1128,7 +1128,15 @@ export function localBranchAnalysisFixture() { rerunWhen: "Rerun after account/queue maturity blockers clear.", }, dataQuality: { signalFidelity: { status: "complete" } }, + // #9517: predicted/basis/funnel/note were missing here, though PredictedGateVerdict + // (packages/loopover-engine/src/predicted-gate.ts) declares all four as required and + // buildPredictedGateVerdict always emits them -- the fixture had drifted from the real payload, + // which only surfaced once loopover_predict_gate gained an output schema to validate against. + // funnel is null under the `gittensor` pack (the contributor is already registered); it is only + // populated under `oss-anti-slop`. predictedGate: { + predicted: true, + basis: "public_config", pack: "gittensor", conclusion: "advisory_pass", title: "Predicted gate: advisory pass", @@ -1136,6 +1144,8 @@ export function localBranchAnalysisFixture() { readinessScore: 72, blockers: [], warnings: [{ code: "missing_tests", title: "Missing tests", detail: "No test files accompany the changed paths." }], + funnel: null, + note: "Predicted from the repo's public .loopover.yml gate config + safe defaults.", }, }; } From 6192bad7d2c6721b5178d7cd7d9d6766dbe6480b Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 00:56:36 -0700 Subject: [PATCH 5/5] 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%. --- vitest.config.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/vitest.config.ts b/vitest.config.ts index fa6ad941bf..616d508567 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -10,6 +10,25 @@ export default defineConfig({ alias: { "cloudflare:email": new URL("./test/stubs/cloudflare-email.ts", import.meta.url).pathname, "cloudflare:workers": new URL("./test/stubs/cloudflare-workers.ts", import.meta.url).pathname, + // @loopover/contract is consumed everywhere by its package specifier, which resolves through + // node_modules via that package's own "exports" map -- straight to dist/*.js, since (unlike + // packages/loopover-{mcp,miner}'s RELATIVE "../lib/foo.js" imports) there's no missing-file + // fallback to a sibling .ts for a package-specifier resolution that already found a real + // compiled file. Left unaliased, v8 coverage instruments and attributes every hit to the + // gitignored dist/ output instead of packages/loopover-contract/src/**/*.ts -- exactly what + // coverage.include below expects -- so Codecov sees 0% patch coverage on real, tested source + // (confirmed live on #9530). Aliasing straight to source is what the mcp/miner packages get + // for free from their relative-import resolution; this closes the same gap for a package + // that's actually installed as a workspace dependency. + // + // "/tools" MUST be listed 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 -- so with the bare entry + // first, it silently intercepted "@loopover/contract/tools" too and rewrote it to a bogus + // "/tools" that resolved nowhere. Confirmed by reproducing the failure with a + // throwaway probe test before reordering, not assumed from reading Vite's docs alone. + "@loopover/contract/tools": new URL("./packages/loopover-contract/src/tools/index.ts", import.meta.url).pathname, + "@loopover/contract": new URL("./packages/loopover-contract/src/index.ts", import.meta.url).pathname, }, }, test: {