diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index 0d0c8919c2..06f0ddd1f9 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -19030,6 +19030,58 @@ } ] } + }, + "/v1/repos/{owner}/{repo}/issue-plan-drafts/generate": { + "post": { + "summary": "AI-plan repo issue drafts from a maintainer goal", + "parameters": [ + { + "schema": { + "type": "string" + }, + "required": true, + "name": "owner", + "in": "path" + }, + { + "schema": { + "type": "string" + }, + "required": true, + "name": "repo", + "in": "path" + } + ], + "responses": { + "200": { + "description": "AI-plan a small set of GitHub issue drafts from a maintainer-supplied planning goal (dry-run by default)", + "content": { + "application/json": { + "schema": { + "type": "object", + "additionalProperties": { + "nullable": true + } + } + } + } + }, + "400": { + "description": "Invalid request or explicit create without dryRun false" + }, + "403": { + "description": "Insufficient role" + } + }, + "security": [ + { + "LoopOverBearer": [] + }, + { + "LoopOverSessionCookie": [] + } + ] + } } }, "servers": [ diff --git a/packages/loopover-mcp/bin/loopover-mcp.ts b/packages/loopover-mcp/bin/loopover-mcp.ts index 3bad6ac30a..734e277397 100644 --- a/packages/loopover-mcp/bin/loopover-mcp.ts +++ b/packages/loopover-mcp/bin/loopover-mcp.ts @@ -2,9 +2,10 @@ // Phase 3 of #7291 / #7330 — design decision: single-file 1:1 JS→TS conversion (not a split). // Seams exist, but splitting would be a separate refactor; this PR only routes the CLI through tsc. import { createHash } from "node:crypto"; -import { closeSync, constants as fsConstants, existsSync, fstatSync, mkdirSync, openSync, readdirSync, readFileSync, readSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { closeSync, constants as fsConstants, existsSync, fstatSync, mkdirSync, openSync, readdirSync, readFileSync, readSync, realpathSync, rmSync, statSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { delimiter, dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; import { McpServer, ResourceTemplate } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; import { buildFeasibilityVerdict, buildPrTextLint, buildGateDispositions, buildPublicPrBodyDraft } from "@loopover/engine"; @@ -72,6 +73,27 @@ const decisionPackCacheMaxBytes = 512 * 1024; const cliTextFileMaxBytes = 1024 * 1024; const changelogPath = new URL("../CHANGELOG.md", import.meta.url); const cliArgs = process.argv.slice(2); + +// #7764: true only when this file is the process entrypoint (`node .../loopover-mcp.js`, incl. via the npm +// `.bin` symlink -- realpathSync resolves it), false when it is imported in-process (e.g. by a vitest unit +// test that exercises the CLI dispatcher + stdio tools directly). The two top-level side effects below -- the +// CLI dispatch and the stdio `server.connect()` -- are gated on it so an in-process importer neither hijacks +// the test runner's argv into runCli() nor binds a StdioServerTransport to the shared stdin. This is the same +// "testable-export refactor" path bin/loopover-miner-mcp.ts's createMinerMcpServer already took (see +// codecov.yml's CLI-dispatcher note); subprocess invocation is unchanged (argv[1] is this file, so it stays +// true) and every mcp-cli-*.test.ts harness run continues to hit the real dispatcher. +function isProcessEntrypoint() { + const entry = process.argv[1]; + /* v8 ignore next -- argv[1] is always populated for a spawned Node process; the guard is belt-and-suspenders */ + if (!entry) return false; + try { + return realpathSync(entry) === realpathSync(fileURLToPath(import.meta.url)); + } catch { + /* v8 ignore next -- defensive: a realpath failure (renamed/removed entry) just means "not the launched CLI" */ + return false; + } +} +const runAsCliEntrypoint = isProcessEntrypoint(); const defaultProfileName = "default"; // Single source of truth for shell-completion: top-level command -> its subcommands (if any). const CLI_COMMAND_SPEC = { @@ -107,7 +129,7 @@ const CLI_COMMAND_SPEC = { profile: ["list", "create", "switch", "remove"], cache: ["status", "clear", "list"], agent: ["plan", "status", "explain", "packet"], - maintain: ["status", "queue", "propose", "approve", "reject", "pause", "resume", "set-level", "precision", "outcome-calibration", "onboarding-pack", "audit-feed", "automation-state", "refresh-docs", "generate-issue-drafts"], + maintain: ["status", "queue", "propose", "approve", "reject", "pause", "resume", "set-level", "precision", "outcome-calibration", "onboarding-pack", "audit-feed", "automation-state", "refresh-docs", "generate-issue-drafts", "plan-issues"], }; const COMPLETION_SHELLS = ["bash", "zsh", "fish", "powershell"]; const AGENT_PROFILE_IDS = ["miner-planner", "miner-auto-dev", "maintainer-triage", "repo-owner-intake"]; @@ -921,6 +943,20 @@ const gatePrecisionShape = { windowDays: z.number().int().positive().optional(), }; +// #7764: mirrors the remote loopover_plan_repo_issues tool's input (src/mcp/server.ts's planRepoIssuesShape), +// minus the create-only `milestone` which this proxy (and the `maintain plan-issues` CLI) does not expose -- +// forwarded to POST /v1/repos/:owner/:repo/issue-plan-drafts/generate. `goal` is the required maintainer +// planning goal; dryRun/create carry the route's create-safety (create alone is rejected there). `limit` is +// capped at 10, matching the route, because every draft costs real LLM spend. +const planRepoIssuesShape = { + owner: z.string().min(1), + repo: z.string().min(1), + goal: z.string().min(1).max(2000), + dryRun: z.boolean().optional().default(true), + create: z.boolean().optional().default(false), + limit: z.number().int().min(1).max(10).optional().default(5), +}; + // Single source of truth for stdio tool name + one-line description (#2233). // Registration and `loopover-mcp tools` both read this list. const STDIO_TOOL_DESCRIPTORS = [ @@ -1302,6 +1338,12 @@ const STDIO_TOOL_DESCRIPTORS = [ category: "maintainer", description: "Return per-gate-type false-positive precision for a repo's recorded gate blocks — blocked / blocked-then-merged counts and false-positive rates with low-sample guards. Optionally bounded by windowDays. Maintainer-authenticated; measurement only.", }, + { + name: "loopover_plan_repo_issues", + category: "maintainer", + description: + "AI-plan a small set of concrete GitHub issue drafts for a repo from a maintainer-supplied free-form goal, same as `loopover-mcp maintain plan-issues --goal ...`. Dry-run BY DEFAULT: only previews the drafted title/body/labels unless the caller passes BOTH create:true and dryRun:false, so it can never silently open issues. Maintainer access required.", + }, { name: "loopover_open_pr", category: "agent", @@ -1373,7 +1415,9 @@ function stdioToolDescription(name: any) { return tool.description; } -if (cliArgs[0] && cliArgs[0] !== "--stdio") { +/* v8 ignore next 8 -- the CLI dispatch runs only in the launched process (runAsCliEntrypoint); an in-process + unit importer keeps it false and drives runCli/maintainCli directly instead (mcp-cli-plan-issues.test.ts). */ +if (runAsCliEntrypoint && cliArgs[0] && cliArgs[0] !== "--stdio") { try { const exitCode = await runCli(cliArgs); process.exit(typeof exitCode === "number" ? exitCode : 0); @@ -1382,7 +1426,7 @@ if (cliArgs[0] && cliArgs[0] !== "--stdio") { } } -const server = new McpServer({ +export const server = new McpServer({ name: "loopover-local", version: packageVersion, }); @@ -2623,6 +2667,25 @@ registerStdioTool( return toolResult(`Gate precision for ${owner}/${repo}.`, payload); }, ); + +registerStdioTool( + "loopover_plan_repo_issues", + { + description: stdioToolDescription("loopover_plan_repo_issues"), + inputSchema: planRepoIssuesShape, + }, + async ({ owner, repo, goal, dryRun, create, limit }: any) => { + // #7764: proxies POST {repoBase}/issue-plan-drafts/generate (the REST mirror of this same tool id). The + // route re-applies its own explicit_create_requires_dry_run_false guard, so forwarding the schema-defaulted + // dryRun/create verbatim keeps the create-safety exact: `create` alone (dryRun still true) is rejected; + // only an explicit {create:true, dryRun:false} reaches the write path. + const payload = await apiPost(`${toolRepoBase(owner, repo)}/issue-plan-drafts/generate`, { goal, dryRun, create, limit }); + return toolResult( + `Issue plan for ${owner}/${repo} (status=${payload.status}, dryRun=${payload.dryRun}): ${payload.proposed ?? 0} proposed, ${payload.created ?? 0} created.`, + payload, + ); + }, +); // ── Write-tools (#6149): pure LOCAL-execution spec builders. loopover NEVER performs the write -- each tool // returns a spec the caller runs with its OWN gh creds. Brings the local stdio server to parity with the // miner-auto-dev profile's recommendedTools, using the same @loopover/engine builders as the remote server. @@ -3167,7 +3230,10 @@ server.registerPrompt( }), ); -await server.connect(new StdioServerTransport()); +// #7764: only bind the shared stdin/stdout transport when actually launched as the CLI/stdio process. An +// in-process unit-test importer holds the exported `server` and connects it to an in-memory transport instead. +/* v8 ignore next -- only the launched stdio process binds the real transport; unit tests connect in-memory. */ +if (runAsCliEntrypoint) await server.connect(new StdioServerTransport()); async function withClientWorkspaceRoots(input: any) { return withWorkspaceRoots(input, await clientWorkspaceRoots()); @@ -3228,6 +3294,9 @@ function printMaintainHelp() { " generate-issue-drafts Preview contributor issue drafts (dry-run). Never creates without --create.", " [--create] Actually open the drafted issues (requires repo write access).", " [--limit N] Cap the drafts generated (1-20, default 5).", + ' plan-issues --goal "..." AI-plan issue drafts from a free-form goal (dry-run). Never creates without --create.', + " [--create] Actually open the drafted issues (requires repo write access).", + " [--limit N] Cap the drafts generated (1-10, default 5).", "", "Pass --json for machine-readable output.", ].join("\n") + "\n", @@ -3236,7 +3305,7 @@ function printMaintainHelp() { // #784 maintainer CLI controls — thin proxies over the agent approval-queue API (#779) and the maintainer // settings kill-switch (#130). The API enforces maintainer authorization; the CLI never decides locally. -async function maintainCli(args: any) { +export async function maintainCli(args: any) { const subcommand = args[0]; if (!subcommand || subcommand === "--help" || subcommand === "help") return printMaintainHelp(); const positional = args[1] && !args[1].startsWith("--") ? args[1] : undefined; @@ -3465,8 +3534,32 @@ async function maintainCli(args: any) { emit(payload, lines.join("\n")); return; } + if (subcommand === "plan-issues") { + // #7764: session-authenticated mirror of POST {repoBase}/issue-plan-drafts/generate (and the remote + // loopover_plan_repo_issues tool). Requires --goal (the maintainer's free-form planning goal). Dry-run BY + // DEFAULT — only a bare `--create` opts into the write path, forwarded as {create:true, dryRun:false}, the + // exact shape the route's explicit_create_requires_dry_run_false guard demands. A plain `plan-issues` can + // never create. + const goal = typeof options.goal === "string" ? options.goal.trim() : ""; + if (!goal) throw new Error('Pass the planning goal: loopover-mcp maintain plan-issues --repo owner/repo --goal "...".'); + const create = options.create === true; + const parsedLimit = Number(options.limit); + const body = { goal, create, dryRun: !create, ...(Number.isFinite(parsedLimit) ? { limit: parsedLimit } : {}) }; + const payload = await apiPost(`${repoBase}/issue-plan-drafts/generate`, body); + const mode = payload.dryRun ? "dry-run" : "create"; + const lines = [ + `Issue plan for ${repoFullName} (${mode}, status=${sanitizePlainTextTerminalOutput(payload.status)}): ${payload.proposed ?? 0} proposed, ${payload.created ?? 0} created, ${payload.skippedDuplicate ?? 0} duplicate, ${payload.skippedDeclined ?? 0} declined, ${payload.skippedUnsafe ?? 0} unsafe, ${payload.skippedCreateFailed ?? 0} create-failed.`, + // draft.title/body are AI-generated free text, so the plain-text path is sanitized (#6261). + ...(payload.drafts ?? []).map((draft: any) => { + const ref = draft.issue ? ` -> #${draft.issue.number} ${draft.issue.url}` : ""; + return `- [${sanitizePlainTextTerminalOutput(draft.status)}] ${sanitizePlainTextTerminalOutput(draft.title)}${sanitizePlainTextTerminalOutput(ref)}`; + }), + ]; + emit(payload, lines.join("\n")); + return; + } throw new Error( - `Unknown maintain subcommand: ${subcommand}. Use status | queue | propose | approve | reject | pause | resume | set-level | precision | outcome-calibration | onboarding-pack | audit-feed | automation-state | refresh-docs | generate-issue-drafts.`, + `Unknown maintain subcommand: ${subcommand}. Use status | queue | propose | approve | reject | pause | resume | set-level | precision | outcome-calibration | onboarding-pack | audit-feed | automation-state | refresh-docs | generate-issue-drafts | plan-issues.`, ); } diff --git a/src/api/routes.ts b/src/api/routes.ts index 5b56a3c904..2943336199 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -320,6 +320,7 @@ import { resolveRepositorySettings } from "../settings/repository-settings"; import { loadPublicRepoFocusManifest, loadRepoFocusManifest, upsertRepoFocusManifest } from "../signals/focus-manifest-loader"; import { buildRepoOnboardingPackPreviewForRepo } from "../services/repo-onboarding-pack"; import { generateContributorIssueDrafts } from "../services/contributor-issue-draft"; +import { generateIssuePlanDrafts } from "../services/issue-plan-draft"; import { buildRepoSettingsPreview, PUBLIC_SURFACE_SKIP_REASONS, skippedPrAuditRemediation } from "../signals/settings-preview"; import { buildGittensorConfigRecommendation, @@ -954,6 +955,17 @@ const contributorIssueDraftGenerateSchema = z.object({ limit: z.number().int().min(1).max(20).optional().default(5), }); +// #7764: REST mirror of the loopover_plan_repo_issues MCP tool (src/mcp/server.ts's planRepoIssuesShape). +// Unlike the contributor-issue-draft schema above, `goal` is a REQUIRED maintainer-supplied free-form string +// and `limit` is capped lower (10, not 20): every draft here costs real LLM spend, unlike that tool's zero-cost +// static signals. dryRun/create keep the same create-safety contract (create alone is rejected below). +const issuePlanDraftGenerateSchema = z.object({ + goal: z.string().trim().min(1).max(2000), + dryRun: z.boolean().optional().default(true), + create: z.boolean().optional().default(false), + limit: z.number().int().min(1).max(10).optional().default(5), +}); + const settingsPreviewSchema = z.object({ sample: z .object({ @@ -2793,6 +2805,43 @@ export function createApp() { ); }); + // #7764: REST mirror of the loopover_plan_repo_issues MCP tool (src/mcp/server.ts) and the + // `maintain plan-issues` CLI. Gated EXACTLY like the sibling contributor-issue-drafts route above + // (requireAppRole maintainer/owner/operator, then per-repo requireSessionRepoAccess for sessions), and it + // preserves the same create-safety: dry-run by default, and the write path is entered only when the caller + // passes BOTH create:true and dryRun:false -- which additionally requires live repo write access. The + // required `goal` is a maintainer-supplied free-form planning goal the service turns into issue drafts. + app.post("/v1/repos/:owner/:repo/issue-plan-drafts/generate", async (c) => { + const fullName = `${c.req.param("owner")}/${c.req.param("repo")}`; + const forbidden = await requireAppRole(c, ["maintainer", "owner", "operator"]); + if (forbidden) return forbidden; + const identity = await authenticateRequestIdentity(c); + const repo = await getRepository(c.env, fullName); + if (identity?.kind === "session") { + const repoForbidden = await requireSessionRepoAccess(c, identity, fullName, repo); + if (repoForbidden) return repoForbidden; + } + const body = await c.req.json().catch(() => null); + if (body === null) return c.json({ error: "invalid_json" }, 400); + const parsed = issuePlanDraftGenerateSchema.safeParse(body); + if (!parsed.success) return c.json({ error: "invalid_issue_plan_draft_request", issues: parsed.error.issues }, 400); + if (parsed.data.create && parsed.data.dryRun !== false) { + return c.json({ error: "explicit_create_requires_dry_run_false" }, 400); + } + if (parsed.data.create && parsed.data.dryRun === false) { + const writeForbidden = await requireRepoWriteAccess(c, fullName); + if (writeForbidden instanceof Response) return writeForbidden; + } + return c.json( + await generateIssuePlanDrafts(c.env, fullName, parsed.data.goal, { + dryRun: parsed.data.dryRun, + create: parsed.data.create, + limit: parsed.data.limit, + requestedBy: identity?.kind === "session" ? identity.actor : "api", + }), + ); + }); + // Repo loopover settings (gate config, AI-review mode/provider/model — NON-secret; the BYOK key is // never here). Maintainer DATA: session callers must be a verified maintainer of THIS repo (per-repo // scope), so a maintainer of repo A cannot read repo B's config. Server-to-server tokens are exempt. @@ -6393,6 +6442,7 @@ function canSessionAccessPath(env: Env, identity: Extract { expect(ps).toContain("[System.Management.Automation.CompletionResult]::new"); expect(ps).toContain("$commands = @('login', 'logout'"); expect(ps).toContain( - "'maintain' = @('status', 'queue', 'propose', 'approve', 'reject', 'pause', 'resume', 'set-level', 'precision', 'outcome-calibration', 'onboarding-pack', 'audit-feed', 'automation-state', 'refresh-docs', 'generate-issue-drafts')", + "'maintain' = @('status', 'queue', 'propose', 'approve', 'reject', 'pause', 'resume', 'set-level', 'precision', 'outcome-calibration', 'onboarding-pack', 'audit-feed', 'automation-state', 'refresh-docs', 'generate-issue-drafts', 'plan-issues')", ); }); diff --git a/test/unit/mcp-cli-maintain.test.ts b/test/unit/mcp-cli-maintain.test.ts index 1f06d989ae..a9b3656415 100644 --- a/test/unit/mcp-cli-maintain.test.ts +++ b/test/unit/mcp-cli-maintain.test.ts @@ -123,6 +123,36 @@ describe("loopover-mcp CLI — maintain (#784)", () => { expect(json).toMatchObject({ dryRun: true, createRequested: false }); }); + it("plan-issues requires --goal and dry-runs by default, never forwarding create (#7764)", async () => { + const bodies: Array<{ goal?: string; dryRun?: boolean; create?: boolean; limit?: number }> = []; + const e = await env({ onPlanIssuesRequest: (b) => bodies.push(b) }); + // Missing --goal fails before any request is made. + await expect(runAsync(["maintain", "plan-issues", "--repo", "owner/repo"], e)).rejects.toThrow(/planning goal/); + const out = await runAsync(["maintain", "plan-issues", "--repo", "owner/repo", "--goal", "Improve docs"], e); + // A bare invocation must send {create:false, dryRun:true} — the CLI can never silently create. + expect(bodies[0]).toMatchObject({ goal: "Improve docs", create: false, dryRun: true }); + expect(out).toMatch(/Issue plan for owner\/repo \(dry-run, status=ok\): 1 proposed, 0 created/); + // The AI-generated draft title carries an ANSI escape; the plain-text path must strip it (#6261). + expect(out).toContain("Add cursor pagination"); + expect(out).not.toContain("[31m"); + }); + + it("plan-issues --create forwards {create:true, dryRun:false} and reports created issues (#7764)", async () => { + const bodies: Array<{ goal?: string; dryRun?: boolean; create?: boolean; limit?: number }> = []; + const e = await env({ onPlanIssuesRequest: (b) => bodies.push(b) }); + const out = await runAsync(["maintain", "plan-issues", "--repo", "owner/repo", "--goal", "Ship it", "--create", "--limit", "3"], e); + // --create maps to the exact {create:true, dryRun:false} shape the route's create-safety guard demands, + // and --limit is forwarded as a number. + expect(bodies[0]).toMatchObject({ goal: "Ship it", create: true, dryRun: false, limit: 3 }); + expect(out).toMatch(/\(create, status=ok\): 0 proposed, 1 created/); + expect(out).toMatch(/#51 https:\/\/github\.com\/owner\/repo\/issues\/51/); + const json = JSON.parse(await runAsync(["maintain", "plan-issues", "--repo", "owner/repo", "--goal", "x", "--json"], e)) as { + dryRun: boolean; + createRequested: boolean; + }; + expect(json).toMatchObject({ dryRun: true, createRequested: false }); + }); + it("outcome-calibration reports slop-band merge rates + recommendation outcomes (plain + json), passing the window through (#6735)", async () => { const e = await env(); const out = await runAsync(["maintain", "outcome-calibration", "--repo", "owner/repo"], e); diff --git a/test/unit/mcp-cli-plan-issues.test.ts b/test/unit/mcp-cli-plan-issues.test.ts new file mode 100644 index 0000000000..f5eaa3ac8c --- /dev/null +++ b/test/unit/mcp-cli-plan-issues.test.ts @@ -0,0 +1,145 @@ +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, vi } from "vitest"; +import { closeFixtureServer, startFixtureServer } from "./support/mcp-cli-harness"; + +// #7764: in-process coverage for the `maintain plan-issues` CLI dispatcher AND the loopover_plan_repo_issues +// stdio tool in packages/loopover-mcp/bin/loopover-mcp.ts. The bin dispatcher/stdio server is otherwise only +// exercised via subprocess spawn (mcp-cli-*.test.ts), which v8 cannot instrument -- the #7764 entrypoint guard +// (isProcessEntrypoint) is what lets a test import the module without it hijacking the runner's argv or binding +// stdin, so these new lines get real Codecov-measured coverage. Only the committed .ts source is imported: since +// #7705 the compiled .js is a gitignored build artifact, so the .ts is the sole source this PR adds and grades. +const MODULES = ["../../packages/loopover-mcp/bin/loopover-mcp.ts"] as const; + +type BinModule = { + maintainCli: (args: string[]) => Promise; + server: { connect: (transport: unknown) => Promise }; +}; + +let tempDir = ""; +const planRequests: Array<{ goal?: string; dryRun?: boolean; create?: boolean; limit?: number }> = []; +const loaded = new Map(); + +beforeAll(async () => { + tempDir = mkdtempSync(join(tmpdir(), "loopover-plan-issues-")); + const apiUrl = await startFixtureServer({ onPlanIssuesRequest: (body) => planRequests.push(body) }); + // The bin reads LOOPOVER_API_URL at module load, so set the env BEFORE importing (hence the dynamic import). + 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"; + for (const specifier of MODULES) { + loaded.set(specifier, (await import(specifier)) as unknown as BinModule); + } +}); + +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; +}); + +async function captureStdout(fn: () => Promise): Promise { + const chunks: string[] = []; + const spy = vi.spyOn(process.stdout, "write").mockImplementation((chunk: string | Uint8Array): boolean => { + chunks.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8")); + return true; + }); + try { + await fn(); + } finally { + spy.mockRestore(); + } + return chunks.join(""); +} + +describe("bin maintain plan-issues CLI (in-process, #7764)", () => { + it.each(MODULES)("previews drafts (dry-run) and sanitizes the AI title — %s", async (specifier) => { + planRequests.length = 0; + const mod = loaded.get(specifier)!; + const out = await captureStdout(() => mod.maintainCli(["plan-issues", "--repo", "owner/repo", "--goal", "Improve docs", "--limit", "4"])); + expect(planRequests[0]).toMatchObject({ goal: "Improve docs", create: false, dryRun: true, limit: 4 }); + expect(out).toMatch(/Issue plan for owner\/repo \(dry-run, status=ok\): 1 proposed, 0 created/); + expect(out).toContain("Add cursor pagination"); + expect(out).not.toContain("[31m"); + }); + + it.each(MODULES)("--create forwards {create:true, dryRun:false} and reports the created issue ref — %s", async (specifier) => { + planRequests.length = 0; + const mod = loaded.get(specifier)!; + const out = await captureStdout(() => mod.maintainCli(["plan-issues", "--repo", "owner/repo", "--goal", "Ship it", "--create"])); + expect(planRequests[0]).toMatchObject({ goal: "Ship it", create: true, dryRun: false }); + expect(out).toMatch(/\(create, status=ok\): 0 proposed, 1 created/); + expect(out).toMatch(/#51 https:\/\/github\.com\/owner\/repo\/issues\/51/); + }); + + it.each(MODULES)("emits machine-readable JSON with --json — %s", async (specifier) => { + const mod = loaded.get(specifier)!; + const out = await captureStdout(() => mod.maintainCli(["plan-issues", "--repo", "owner/repo", "--goal", "x", "--json"])); + const payload = JSON.parse(out) as { status: string; dryRun: boolean; proposed: number }; + expect(payload).toMatchObject({ status: "ok", dryRun: true, proposed: 1 }); + }); + + it.each(MODULES)("rejects a missing --goal before any request — %s", async (specifier) => { + const mod = loaded.get(specifier)!; + await expect(mod.maintainCli(["plan-issues", "--repo", "owner/repo"])).rejects.toThrow(/planning goal/); + }); + + it.each(MODULES)("falls through past plan-issues to the unknown-subcommand error — %s", async (specifier) => { + const mod = loaded.get(specifier)!; + // Exercises the false side of `subcommand === "plan-issues"` and the updated unknown-subcommand throw. + await expect(mod.maintainCli(["not-a-real-subcommand", "--repo", "owner/repo"])).rejects.toThrow(/Unknown maintain subcommand.*plan-issues/); + }); + + it.each(MODULES)("falls back to zero counts and no draft lines on a countless posture — %s", async (specifier) => { + const mod = loaded.get(specifier)!; + // "__bare__" makes the fixture return a disabled posture with no counts/drafts, exercising the CLI's + // defensive `?? 0` / `?? []` fallbacks (a real `disabled`/`unavailable` short-circuit has no counts). + const out = await captureStdout(() => mod.maintainCli(["plan-issues", "--repo", "owner/repo", "--goal", "__bare__"])); + expect(out).toMatch(/status=disabled\): 0 proposed, 0 created, 0 duplicate, 0 declined, 0 unsafe, 0 create-failed\./); + expect(out).not.toContain("- ["); + }); + + it.each(MODULES)("documents plan-issues in the maintain --help output — %s", async (specifier) => { + // Covers printMaintainHelp's new plan-issues entry (#7764) and guards the command staying documented. + const mod = loaded.get(specifier)!; + const out = await captureStdout(() => mod.maintainCli(["--help"])); + expect(out).toContain('plan-issues --goal "..."'); + expect(out).toContain("AI-plan issue drafts from a free-form goal"); + }); +}); + +describe("bin loopover_plan_repo_issues stdio tool (in-process, #7764)", () => { + it.each(MODULES)("proxies the plan route and returns the counts + posture — %s", async (specifier) => { + planRequests.length = 0; + const mod = loaded.get(specifier)!; + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await mod.server.connect(serverTransport); + const client = new Client({ name: "plan-repo-issues-test", version: "0.1.0" }, { capabilities: {} }); + await client.connect(clientTransport); + try { + const result = await client.callTool({ + name: "loopover_plan_repo_issues", + arguments: { owner: "owner", repo: "repo", goal: "Improve docs" }, + }); + expect(result.isError).toBeFalsy(); + expect(planRequests[0]).toMatchObject({ goal: "Improve docs", dryRun: true, create: false }); + expect(result.structuredContent).toMatchObject({ repoFullName: "owner/repo", status: "ok", dryRun: true, proposed: 1 }); + + // A countless (disabled) posture exercises the summary's defensive `?? 0` fallbacks. + const bare = await client.callTool({ name: "loopover_plan_repo_issues", arguments: { owner: "owner", repo: "repo", goal: "__bare__" } }); + expect(bare.isError).toBeFalsy(); + const bareText = (bare.content as Array<{ type: string; text: string }>)[0]!.text; + expect(bareText).toMatch(/status=disabled, dryRun=true\): 0 proposed, 0 created\./); + } finally { + await client.close(); + } + }); +}); diff --git a/test/unit/mcp-tool-rename-aliases.test.ts b/test/unit/mcp-tool-rename-aliases.test.ts index 5088d983c7..b1195bc93d 100644 --- a/test/unit/mcp-tool-rename-aliases.test.ts +++ b/test/unit/mcp-tool-rename-aliases.test.ts @@ -22,6 +22,7 @@ // (#6747 registered the loopover_pr_outcome CLI mirror, taking the count from 77 to 78.) // (#6980 registered the loopover_explain_review_risk CLI mirror, taking the count from 78 to 79.) // (#7758 registered the loopover_get_outcome_calibration stdio tool, taking the count from 79 to 80.) +// (#7764 registered the loopover_plan_repo_issues stdio + CLI + REST tool, taking the count from 80 to 81.) import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; import { mkdtempSync, rmSync } from "node:fs"; @@ -69,14 +70,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { }); afterEach(disconnect); - it("lists exactly 80 loopover_ tools and zero gittensory_-prefixed aliases", async () => { + it("lists exactly 81 loopover_ tools and zero gittensory_-prefixed aliases", async () => { const { tools } = await client.listTools(); const names = tools.map((t) => t.name); const primary = names.filter((n) => n.startsWith("loopover_")); const legacy = names.filter((n) => n.startsWith("gittensory_")); - expect(primary.length).toBe(80); + expect(primary.length).toBe(81); expect(legacy.length).toBe(0); - expect(names.length).toBe(80); + expect(names.length).toBe(81); }); it("no loopover_ tool's description carries a stale deprecation notice", async () => { @@ -88,14 +89,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { } }); - it("`loopover-mcp tools --json` reports the same 80-tool count the live server registers", async () => { + it("`loopover-mcp tools --json` reports the same 81-tool count the live server registers", async () => { const { tools } = await client.listTools(); const payload = JSON.parse(run(["tools", "--json"])) as { count: number; tools: Array<{ name: string }>; }; expect(payload.count).toBe(tools.length); - expect(payload.count).toBe(80); + expect(payload.count).toBe(81); expect([...payload.tools.map((t) => t.name)].sort()).toEqual( [...tools.map((t) => t.name)].sort(), ); diff --git a/test/unit/routes-issue-plan-draft.test.ts b/test/unit/routes-issue-plan-draft.test.ts new file mode 100644 index 0000000000..1c7ed17f35 --- /dev/null +++ b/test/unit/routes-issue-plan-draft.test.ts @@ -0,0 +1,224 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { createApp } from "../../src/api/routes"; +import { createSessionForGitHubUser } from "../../src/auth/security"; +import { upsertInstallation, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; +import { getRepositoryCollaboratorPermission } from "../../src/github/app"; +import { createTestEnv } from "../helpers/d1"; + +// #7764: REST mirror of the loopover_plan_repo_issues MCP tool. Structure mirrors +// routes-contributor-issue-draft.test.ts exactly (same requireAppRole + requireSessionRepoAccess + +// requireRepoWriteAccess gate), plus the required free-form `goal` and the write-GRANTED branch that lets the +// create path fall through to the service (still dry-run-safe: createTestEnv leaves env.AI unset, so the +// service short-circuits to a `disabled` posture before any GitHub write). +const PLAN_PATH = "/v1/repos/JSONbored/loopover/issue-plan-drafts/generate"; +const OWNED_REPO_PATH = "/v1/repos/repo-owner/owned-repo/issue-plan-drafts/generate"; + +vi.mock("../../src/github/app", async (importOriginal) => ({ + ...(await importOriginal()), + getRepositoryCollaboratorPermission: vi.fn(), +})); +const mockedPermission = vi.mocked(getRepositoryCollaboratorPermission); + +function apiHeaders(env: Env): Record { + return { + authorization: `Bearer ${env.LOOPOVER_API_TOKEN}`, + "content-type": "application/json", + }; +} + +async function seedRegisteredInstalledRepo(env: Env, installationId: number, owner: string, name: string): Promise { + await upsertInstallation(env, { + installation: { + id: installationId, + account: { login: owner, id: installationId, type: "User" }, + repository_selection: "selected", + permissions: { metadata: "read", contents: "read" }, + events: ["repository"], + }, + }); + await upsertRepositoryFromGitHub( + env, + { name, full_name: `${owner}/${name}`, private: false, owner: { login: owner } }, + installationId, + ); + await env.DB.prepare("UPDATE repositories SET is_registered = 1 WHERE full_name = ?") + .bind(`${owner}/${name}`) + .run(); +} + +describe("issue-plan-drafts route auth (#7764)", () => { + beforeEach(() => mockedPermission.mockReset()); + + it("rejects unauthenticated access", async () => { + const app = createApp(); + const env = createTestEnv(); + const response = await app.request(PLAN_PATH, { method: "POST", body: "{}" }, env); + expect(response.status).toBe(401); + await expect(response.json()).resolves.toMatchObject({ error: "unauthorized" }); + }); + + it("rejects unauthorized session access", async () => { + const app = createApp(); + const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "jsonbored" }); + const { token } = await createSessionForGitHubUser(env, { login: "new-user", id: 2468 }); + const response = await app.request( + PLAN_PATH, + { method: "POST", headers: { cookie: `loopover_session=${token}`, "content-type": "application/json" }, body: "{}" }, + env, + ); + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ error: "insufficient_role" }); + }); + + it("allows same-repo owner sessions to preview dry-run drafts", async () => { + const app = createApp(); + const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "" }); + await seedRegisteredInstalledRepo(env, 301, "repo-owner", "owned-repo"); + const { token } = await createSessionForGitHubUser(env, { login: "repo-owner", id: 301 }); + const response = await app.request( + OWNED_REPO_PATH, + { + method: "POST", + headers: { cookie: `loopover_session=${token}`, "content-type": "application/json" }, + body: JSON.stringify({ goal: "Improve the onboarding docs", dryRun: true, limit: 1 }), + }, + env, + ); + expect(response.status).toBe(200); + await expect(response.json()).resolves.toMatchObject({ + repoFullName: "repo-owner/owned-repo", + dryRun: true, + createRequested: false, + drafts: expect.any(Array), + }); + }); + + it("requires live GitHub write permission before session issue creation", async () => { + const app = createApp(); + const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "" }); + await seedRegisteredInstalledRepo(env, 301, "repo-owner", "owned-repo"); + await upsertPullRequestFromGitHub(env, "repo-owner/owned-repo", { + number: 5, + title: "cached collaborator scope", + state: "open", + user: { login: "reader" }, + author_association: "COLLABORATOR", + head: { sha: "a1", ref: "f" }, + base: { ref: "main" }, + labels: [], + }); + mockedPermission.mockResolvedValue("read"); + const { token } = await createSessionForGitHubUser(env, { login: "reader", id: 777 }); + + const response = await app.request( + OWNED_REPO_PATH, + { + method: "POST", + headers: { cookie: `loopover_session=${token}`, "content-type": "application/json" }, + body: JSON.stringify({ goal: "Ship a queue fix", dryRun: false, create: true, limit: 1 }), + }, + env, + ); + + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ error: "insufficient_repo_permission" }); + expect(mockedPermission).toHaveBeenCalledWith(env, 301, "repo-owner/owned-repo", "reader"); + }); + + it("lets a write-granted owner session reach the (still-gated) create path", async () => { + const app = createApp(); + const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "" }); + await seedRegisteredInstalledRepo(env, 301, "repo-owner", "owned-repo"); + // env.AI is unset in createTestEnv, so the service returns a `disabled` posture instead of calling any + // model or opening any issue -- this test's job is only to cover the write-GRANTED branch of the route's + // requireRepoWriteAccess gate (the negative branch is covered by the test above). + mockedPermission.mockResolvedValue("admin"); + const { token } = await createSessionForGitHubUser(env, { login: "repo-owner", id: 301 }); + const response = await app.request( + OWNED_REPO_PATH, + { + method: "POST", + headers: { cookie: `loopover_session=${token}`, "content-type": "application/json" }, + body: JSON.stringify({ goal: "Plan the next milestone", dryRun: false, create: true, limit: 1 }), + }, + env, + ); + expect(response.status).toBe(200); + await expect(response.json()).resolves.toMatchObject({ + repoFullName: "repo-owner/owned-repo", + createRequested: true, + }); + }); + + it("rejects cross-repo owner sessions with forbidden_repo", async () => { + const app = createApp(); + const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "" }); + await seedRegisteredInstalledRepo(env, 301, "repo-owner", "owned-repo"); + await seedRegisteredInstalledRepo(env, 302, "other-owner", "other-repo"); + const { token } = await createSessionForGitHubUser(env, { login: "other-owner", id: 302 }); + const response = await app.request( + OWNED_REPO_PATH, + { + method: "POST", + headers: { cookie: `loopover_session=${token}`, "content-type": "application/json" }, + body: JSON.stringify({ goal: "Anything", dryRun: true, limit: 1 }), + }, + env, + ); + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ error: "forbidden_repo" }); + }); + + it("rejects malformed JSON with 400", async () => { + const app = createApp(); + const env = createTestEnv(); + const response = await app.request( + PLAN_PATH, + { method: "POST", headers: { ...apiHeaders(env), "content-type": "application/json" }, body: "not-json" }, + env, + ); + expect(response.status).toBe(400); + await expect(response.json()).resolves.toMatchObject({ error: "invalid_json" }); + }); + + it("rejects explicit create without dryRun false", async () => { + const app = createApp(); + const env = createTestEnv(); + const response = await app.request( + PLAN_PATH, + { method: "POST", headers: apiHeaders(env), body: JSON.stringify({ goal: "Improve tests", create: true }) }, + env, + ); + expect(response.status).toBe(400); + await expect(response.json()).resolves.toMatchObject({ error: "explicit_create_requires_dry_run_false" }); + }); + + it("rejects invalid request bodies (missing goal)", async () => { + const app = createApp(); + const env = createTestEnv(); + const response = await app.request( + PLAN_PATH, + { method: "POST", headers: apiHeaders(env), body: JSON.stringify({ limit: 2 }) }, + env, + ); + expect(response.status).toBe(400); + await expect(response.json()).resolves.toMatchObject({ error: "invalid_issue_plan_draft_request" }); + }); + + it("returns dry-run drafts for authorized static-token callers", async () => { + const app = createApp(); + const env = createTestEnv({ LOOPOVER_DRIFT_ISSUE_REPO: "JSONbored/loopover" }); + const response = await app.request( + PLAN_PATH, + { method: "POST", headers: apiHeaders(env), body: JSON.stringify({ goal: "Reduce reviewer noise", dryRun: true, limit: 2 }) }, + env, + ); + expect(response.status).toBe(200); + await expect(response.json()).resolves.toMatchObject({ + repoFullName: "JSONbored/loopover", + dryRun: true, + createRequested: false, + drafts: expect.any(Array), + }); + }); +}); diff --git a/test/unit/support/mcp-cli-harness.ts b/test/unit/support/mcp-cli-harness.ts index 2f26c988c5..93b92ef427 100644 --- a/test/unit/support/mcp-cli-harness.ts +++ b/test/unit/support/mcp-cli-harness.ts @@ -187,6 +187,7 @@ export async function startFixtureServer( prTextLintStatus?: number; onPacketRequest?: (body: unknown) => void; onIssueDraftRequest?: (body: { dryRun?: boolean; create?: boolean; limit?: number }) => void; + onPlanIssuesRequest?: (body: { goal?: string; dryRun?: boolean; create?: boolean; limit?: number }) => void; onWatchRequest?: (req: { method: string; body: { repoFullName?: string; labels?: string[] } }) => void; onApiRequest?: (request: IncomingMessage) => void; validateConfigWarnings?: string[]; @@ -704,6 +705,45 @@ export async function startFixtureServer( ); return; } + // #7764: issue-plan-drafts generation. Echoes the forwarded {goal, dryRun, create, limit} so the CLI/stdio + // test can assert the exact body it sent. The draft title carries an ANSI escape to prove the plain-text + // path is sanitized (#6261); a created run returns the issue ref like the contributor handler above. + if (request.url === "/v1/repos/owner/repo/issue-plan-drafts/generate" && request.method === "POST") { + const requestBody = (await readJsonRequest(request)) as { goal?: string; dryRun?: boolean; create?: boolean; limit?: number }; + options.onPlanIssuesRequest?.(requestBody); + // Sentinel goal "__bare__" returns a minimal disabled-posture response (no counts/drafts), so the CLI + + // stdio proxies' defensive `?? 0` / `?? []` fallbacks are exercised (the service really can short-circuit + // to a countless `disabled`/`unavailable` posture when AI is off). + if (requestBody.goal === "__bare__") { + response.end(JSON.stringify({ repoFullName: "owner/repo", generatedAt: "2026-05-30T00:00:00.000Z", status: "disabled", dryRun: true, createRequested: false })); + return; + } + response.end( + JSON.stringify({ + repoFullName: "owner/repo", + generatedAt: "2026-05-30T00:00:00.000Z", + status: "ok", + dryRun: requestBody.dryRun ?? true, + createRequested: requestBody.create ?? false, + proposed: requestBody.create ? 0 : 1, + skippedDuplicate: 0, + skippedDeclined: 0, + skippedUnsafe: 0, + created: requestBody.create ? 1 : 0, + skippedCreateFailed: 0, + drafts: [ + { + status: requestBody.create ? "created" : "proposed", + title: "Add cursor pagination", + body: "body", + labels: [], + ...(requestBody.create ? { issue: { number: 51, url: "https://github.com/owner/repo/issues/51" } } : {}), + }, + ], + }), + ); + return; + } const onboardingPackUrl = new URL(request.url ?? "/", "http://localhost"); if (onboardingPackUrl.pathname === "/v1/repos/owner/repo/onboarding-pack/preview" && request.method === "GET") { const refresh = onboardingPackUrl.searchParams.get("refresh");