From f6eb68cac464d09f26614f79d089fa930319871f Mon Sep 17 00:00:00 2001 From: phamngocquy Date: Fri, 31 Jul 2026 21:52:15 +0800 Subject: [PATCH] mcp(stdio): restore the CurrentBranchInput narrowing on the eight local-branch tools Fixes #10034 --- .../src/tools/local-branch.ts | 8 ++ packages/loopover-mcp/bin/loopover-mcp.ts | 10 +++ test/unit/contract-registry.test.ts | 57 ++++++++++++ .../unit/mcp-cli-current-branch-input.test.ts | 89 +++++++++++++++++++ 4 files changed, 164 insertions(+) create mode 100644 test/unit/mcp-cli-current-branch-input.test.ts diff --git a/packages/loopover-contract/src/tools/local-branch.ts b/packages/loopover-contract/src/tools/local-branch.ts index aa7e897b8..101465500 100644 --- a/packages/loopover-contract/src/tools/local-branch.ts +++ b/packages/loopover-contract/src/tools/local-branch.ts @@ -239,9 +239,17 @@ export const reviewPrBeforePushTool = defineTool({ output: PreflightCurrentBranchOutput, }); +/** What the STDIO server serves: resolves login/repo from session and reads branch metadata from the checkout. */ +export const StdioLocalBranchAnalysisInput = CurrentBranchInput; + export const DraftPrBodyInput = LocalBranchAnalysisInput.extend({ format: z.enum(["json", "markdown"]).optional(), }); + +/** What the STDIO server serves for draft_pr_body: the branch narrowing plus optional format. */ +export const StdioDraftPrBodyInput = CurrentBranchInput.extend({ + format: z.enum(["json", "markdown"]).optional(), +}); export const draftPrBodyTool = defineTool({ name: "loopover_draft_pr_body", title: "Draft PR body", diff --git a/packages/loopover-mcp/bin/loopover-mcp.ts b/packages/loopover-mcp/bin/loopover-mcp.ts index 3cf119ca7..cf2b00964 100644 --- a/packages/loopover-mcp/bin/loopover-mcp.ts +++ b/packages/loopover-mcp/bin/loopover-mcp.ts @@ -141,6 +141,8 @@ import { LocalStatusStructuredInput, MarkNotificationsReadInput, StdioMarkNotificationsReadInput, + StdioLocalBranchAnalysisInput, + StdioDraftPrBodyInput, MonitorOpenPrsInput, OpenPrInput, PlanRepoIssuesInput, @@ -1670,6 +1672,7 @@ registerStdioTool( workspaceIntelligence: publicSafeWorkspaceIntelligence(result.analysis.workspaceIntelligence), }); }, + { input: StdioLocalBranchAnalysisInput }, ); registerStdioTool( @@ -1689,6 +1692,7 @@ registerStdioTool( recommendedRerunCondition: result.analysis.recommendedRerunCondition, }); }, + { input: StdioLocalBranchAnalysisInput }, ); registerStdioTool( @@ -1697,6 +1701,7 @@ registerStdioTool( const result = await analyzeCurrentBranch(await withClientWorkspaceRoots(input)); return toolResult("LoopOver local next-action ranking.", { local: result.local, nextActions: result.analysis.nextActions, rewardRisk: result.analysis.rewardRisk, recommendedRerunCondition: result.analysis.recommendedRerunCondition }); }, + { input: StdioLocalBranchAnalysisInput }, ); registerStdioTool( @@ -1713,6 +1718,7 @@ registerStdioTool( recommendedRerunCondition: result.analysis.recommendedRerunCondition, }); }, + { input: StdioLocalBranchAnalysisInput }, ); registerStdioTool( @@ -1723,6 +1729,7 @@ registerStdioTool( const { localScorerStatus: _localScorerStatus, ...body } = payload; return toolResult("LoopOver remediation plan.", await apiPost("/v1/local/remediation-plan", body)); }, + { input: StdioLocalBranchAnalysisInput }, ); registerStdioTool( @@ -1731,6 +1738,7 @@ registerStdioTool( const result = await analyzeCurrentBranch(await withClientWorkspaceRoots(input)); return toolResult("LoopOver public-safe PR packet.", { local: result.local, prPacket: result.analysis.prPacket }); }, + { input: StdioLocalBranchAnalysisInput }, ); // #6741: CLI stdio mirror of loopover_draft_pr_body — same analyzeCurrentBranch fetch as prepare_pr_packet, @@ -1755,6 +1763,7 @@ registerStdioTool( draft, ); }, + { input: StdioDraftPrBodyInput }, ); registerStdioTool( @@ -1823,6 +1832,7 @@ registerStdioTool( registerStdioTool( "loopover_agent_prepare_pr_packet", async (input: z.infer) => toolResult("LoopOver base-agent public-safe PR packet.", await agentPreparePrPacket(await withClientWorkspaceRoots(input))), + { input: StdioLocalBranchAnalysisInput }, ); // Only this tool declares an outputSchema today; every other tool returns text + unschematized diff --git a/test/unit/contract-registry.test.ts b/test/unit/contract-registry.test.ts index 06e9907e4..9805fabe2 100644 --- a/test/unit/contract-registry.test.ts +++ b/test/unit/contract-registry.test.ts @@ -37,6 +37,12 @@ import { CLAIM_STATUSES as MINER_CLAIM_STATUSES } from "../../packages/loopover- import { RUN_STATES as LIVE_MINER_RUN_STATES } from "../../packages/loopover-miner/lib/run-state"; import { LocalStatusStructuredInput } from "@loopover/contract/tools"; import { GetRepoContextInput } from "@loopover/contract/tools"; +import { + DraftPrBodyInput, + LocalBranchAnalysisInput, + StdioDraftPrBodyInput, + StdioLocalBranchAnalysisInput, +} from "@loopover/contract/tools"; import { PREFLIGHT_LIMITS as ENGINE_PREFLIGHT_LIMITS } from "../../packages/loopover-engine/src/signals/preflight-limits"; import { PUBLIC_SURFACE_SKIP_REASONS as SERVER_PUBLIC_SURFACE_SKIP_REASONS } from "../../src/signals/settings-preview"; import { AUTONOMY_LEVELS as ENGINE_AUTONOMY_LEVELS, AGENT_ACTION_CLASSES as ENGINE_AGENT_ACTION_CLASSES } from "../../packages/loopover-engine/src/settings/autonomy"; @@ -97,6 +103,57 @@ describe("contract tool registry", () => { expect(contract!.input.safeParse({ owner: "o", repo: "r", title: "" }).success).toBe(false); expect(contract!.input.safeParse({ owner: "o", repo: "r", title: "t" }).success).toBe(true); }); + + it("declares stdio local-branch narrowings derived from LocalBranchAnalysisInput (#10034)", () => { + const contractRequired = new Set( + ((z.toJSONSchema(LocalBranchAnalysisInput, { target: "draft-2020-12" }) as { required?: string[] }).required ?? []), + ); + const eightTools = [ + "loopover_preflight_current_branch", + "loopover_preview_current_branch_score", + "loopover_rank_local_next_actions", + "loopover_explain_local_blockers", + "loopover_remediation_plan", + "loopover_prepare_pr_packet", + "loopover_draft_pr_body", + "loopover_agent_prepare_pr_packet", + ] as const; + const narrowingByTool: Record<(typeof eightTools)[number], z.ZodObject> = { + loopover_preflight_current_branch: StdioLocalBranchAnalysisInput, + loopover_preview_current_branch_score: StdioLocalBranchAnalysisInput, + loopover_rank_local_next_actions: StdioLocalBranchAnalysisInput, + loopover_explain_local_blockers: StdioLocalBranchAnalysisInput, + loopover_remediation_plan: StdioLocalBranchAnalysisInput, + loopover_prepare_pr_packet: StdioLocalBranchAnalysisInput, + loopover_draft_pr_body: StdioDraftPrBodyInput, + loopover_agent_prepare_pr_packet: StdioLocalBranchAnalysisInput, + }; + for (const toolName of eightTools) { + const contract = getToolContract(toolName); + expect(contract, toolName).toBeDefined(); + const contractSchema = z.toJSONSchema(contract!.input, { target: "draft-2020-12" }) as { + properties?: Record; + required?: string[]; + }; + const narrowing = narrowingByTool[toolName]; + const narrowingSchema = z.toJSONSchema(narrowing, { target: "draft-2020-12" }) as { + properties?: Record; + required?: string[]; + }; + const narrowingRequired = narrowingSchema.required ?? []; + expect(narrowingRequired, toolName).not.toContain("login"); + expect(narrowingRequired, toolName).not.toContain("repoFullName"); + for (const required of narrowingRequired) { + expect(contractRequired, `${toolName}.${required}`).toContain(required); + } + for (const property of Object.keys(narrowingSchema.properties ?? {})) { + expect(contractSchema.properties ?? {}, `${toolName}.${property}`).toHaveProperty(property); + } + } + expect(DraftPrBodyInput.safeParse({ login: "a", repoFullName: "o/r", format: "json" }).success).toBe(true); + expect(StdioDraftPrBodyInput.safeParse({ format: "markdown" }).success).toBe(true); + expect(StdioDraftPrBodyInput.safeParse({}).success).toBe(true); + }); }); describe("contract projection", () => { diff --git a/test/unit/mcp-cli-current-branch-input.test.ts b/test/unit/mcp-cli-current-branch-input.test.ts new file mode 100644 index 000000000..0a5cb83a7 --- /dev/null +++ b/test/unit/mcp-cli-current-branch-input.test.ts @@ -0,0 +1,89 @@ +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { closeFixtureServer, startFixtureServer } from "./support/mcp-cli-harness"; + +const EIGHT_CURRENT_BRANCH_TOOLS = [ + "loopover_preflight_current_branch", + "loopover_preview_current_branch_score", + "loopover_rank_local_next_actions", + "loopover_explain_local_blockers", + "loopover_remediation_plan", + "loopover_prepare_pr_packet", + "loopover_draft_pr_body", + "loopover_agent_prepare_pr_packet", +] as const; + +type BinModule = { + server: { connect: (transport: unknown) => Promise }; +}; + +let tempDir = ""; +let loaded: BinModule; + +beforeAll(async () => { + tempDir = mkdtempSync(join(tmpdir(), "loopover-current-branch-input-")); + const apiUrl = await startFixtureServer(); + process.env.LOOPOVER_API_URL = apiUrl; + process.env.LOOPOVER_API_TOKEN = "in-process-token"; + process.env.LOOPOVER_API_TIMEOUT_MS = "2000"; + process.env.LOOPOVER_CONFIG_DIR = tempDir; + process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK = "1"; + process.env.LOOPOVER_LOGIN = "JSONbored"; + loaded = (await import("../../packages/loopover-mcp/bin/loopover-mcp")) as unknown as BinModule; +}, 120_000); + +afterAll(async () => { + await closeFixtureServer(); + if (tempDir) rmSync(tempDir, { recursive: true, force: true }); + delete process.env.LOOPOVER_API_URL; + delete process.env.LOOPOVER_API_TOKEN; + delete process.env.LOOPOVER_CONFIG_DIR; + delete process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK; + delete process.env.LOOPOVER_LOGIN; +}); + +async function connectClient(): Promise { + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await loaded.server.connect(serverTransport); + const client = new Client({ name: "current-branch-input-test", version: "0.1.0" }, { capabilities: {} }); + await client.connect(clientTransport); + return client; +} + +describe("REGRESSION: the stdio branch tools must not demand a login the CLI resolves itself", () => { + it("advertises no required login or repoFullName on the eight current-branch tools", async () => { + const client = await connectClient(); + try { + const { tools } = await client.listTools(); + for (const toolName of EIGHT_CURRENT_BRANCH_TOOLS) { + const tool = tools.find((entry) => entry.name === toolName); + expect(tool, toolName).toBeDefined(); + const required = (tool!.inputSchema as { required?: string[] }).required ?? []; + expect(required, toolName).not.toContain("login"); + expect(required, toolName).not.toContain("repoFullName"); + } + } finally { + await client.close().catch(() => undefined); + } + }); + + it("accepts an empty argument object without schema rejection for login or repoFullName", async () => { + const client = await connectClient(); + try { + for (const toolName of EIGHT_CURRENT_BRANCH_TOOLS) { + const result = await client.callTool({ name: toolName, arguments: {} }); + const serialized = JSON.stringify(result); + expect(serialized, toolName).not.toMatch(/required property ['"]login['"]/i); + expect(serialized, toolName).not.toMatch(/required property ['"]repoFullName['"]/i); + expect(serialized, toolName).not.toMatch(/-32602.*login/i); + expect(serialized, toolName).not.toMatch(/-32602.*repoFullName/i); + } + } finally { + await client.close().catch(() => undefined); + } + }); +});