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. 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 058e32c0a0..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", @@ -4262,6 +4263,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 +5018,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 +22616,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", @@ -22636,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-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..34d57a3880 --- /dev/null +++ b/packages/loopover-contract/src/tools/pr-reviewability.ts @@ -0,0 +1,77 @@ +// 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 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().optional(), + 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/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"); 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/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.", }, }; } 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..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: { @@ -58,6 +77,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