diff --git a/packages/loopover-mcp/bin/loopover-mcp.js b/packages/loopover-mcp/bin/loopover-mcp.js index 7928263cc6..55187ea4b0 100644 --- a/packages/loopover-mcp/bin/loopover-mcp.js +++ b/packages/loopover-mcp/bin/loopover-mcp.js @@ -98,7 +98,7 @@ const CLI_COMMAND_SPEC = { profile: ["list", "create", "switch", "remove"], cache: ["status", "clear", "list"], agent: ["plan", "status", "explain", "packet"], - maintain: ["status", "queue", "approve", "reject", "pause", "resume", "set-level", "precision", "onboarding-pack", "audit-feed"], + maintain: ["status", "queue", "propose", "approve", "reject", "pause", "resume", "set-level", "precision", "onboarding-pack", "audit-feed"], }; const COMPLETION_SHELLS = ["bash", "zsh", "fish", "powershell"]; const AGENT_PROFILE_IDS = ["miner-planner", "miner-auto-dev", "maintainer-triage", "repo-owner-intake"]; @@ -120,6 +120,10 @@ const AGENT_PROFILE_IDS = ["miner-planner", "miner-auto-dev", "maintainer-triage // purpose. Do not "sync" it to the engine list. const MAINTAIN_ACTION_CLASSES = ["review", "request_changes", "approve", "merge", "close", "label"]; const MAINTAIN_AUTONOMY_LEVELS = ["observe", "auto_with_approval", "auto"]; +// #6744: the loopover_propose_action / POST .../agent/pending-actions action-class enum. A superset of +// MAINTAIN_ACTION_CLASSES (adds review_state_label) — kept separate so `maintain propose` accepts exactly what the +// route + MCP tool accept, while set-level keeps its own autonomy-configurable subset above. +const PROPOSE_ACTION_CLASSES = ["review", "request_changes", "approve", "merge", "close", "label", "review_state_label"]; // #6150 — plan-DAG step tracking for loopover_build_plan/loopover_plan_status/loopover_record_step_result. // Hand-duplicated from src/services/plan-dag.ts (packages/loopover-engine/src/services/plan-dag.ts is NOT @@ -3012,6 +3016,9 @@ function printMaintainHelp() { "Subcommands:", " status List the agent approval queue (auto_with_approval actions awaiting a decision).", " queue List pending actions (id, kind, target) for approve/reject. Alias: pending.", + " propose Stage a new auto_with_approval action for a maintainer to approve later.", + ` classes: ${PROPOSE_ACTION_CLASSES.join(", ")}`, + " opts: --reason, --label, --review-body, --merge-method, --close-comment.", " approve Approve a staged action -> execute it.", " reject Reject a staged action -> cancel it.", " pause Pause ALL agent actions on the repo (kill-switch).", @@ -3091,6 +3098,26 @@ async function maintainCli(args) { emit(payload, `${subcommand === "approve" ? "Accepted" : "Rejected"} ${positional}: ${payload.status ?? "ok"}${payload.executionOutcome ? ` (${payload.executionOutcome})` : ""}.`); return; } + if (subcommand === "propose") { + const actionClass = positional; + const pullArg = args[2] && !args[2].startsWith("--") ? args[2] : undefined; + if (!actionClass || !pullArg) { + throw new Error("Usage: loopover-mcp maintain propose --repo owner/repo [--reason ...] [--label ...] [--review-body ...] [--merge-method merge|squash|rebase] [--close-comment ...]."); + } + if (!PROPOSE_ACTION_CLASSES.includes(actionClass)) throw new Error(`Unknown action class: ${actionClass}. Use ${PROPOSE_ACTION_CLASSES.join(", ")}.`); + const pullNumber = Number(pullArg); + if (!Number.isInteger(pullNumber) || pullNumber <= 0) throw new Error(`Invalid pull number: ${pullArg}. Pass a positive integer.`); + const payload = await apiPost( + queueBase, + stripUndefined({ pullNumber, actionClass, reason: options.reason, label: options.label, reviewBody: options.reviewBody, mergeMethod: options.mergeMethod, closeComment: options.closeComment }), + ); + const action = payload.action ?? {}; + emit( + payload, + `${payload.created ? "Staged" : "Already staged"} ${sanitizePlainTextTerminalOutput(action.actionClass ?? actionClass)} on ${repoFullName}#${pullNumber} (${sanitizePlainTextTerminalOutput(action.status ?? "pending")}), id ${sanitizePlainTextTerminalOutput(action.id ?? "?")}.`, + ); + return; + } if (subcommand === "pause" || subcommand === "resume") { const payload = await apiFetch(`${repoBase}/settings`, { method: "PUT", body: JSON.stringify({ agentPaused: subcommand === "pause" }) }); emit(payload, `Agent actions ${subcommand === "pause" ? "paused" : "resumed"} for ${repoFullName}.`); @@ -3174,7 +3201,7 @@ async function maintainCli(args) { return; } throw new Error( - `Unknown maintain subcommand: ${subcommand}. Use status | queue | approve | reject | pause | resume | set-level | precision | onboarding-pack | audit-feed.`, + `Unknown maintain subcommand: ${subcommand}. Use status | queue | propose | approve | reject | pause | resume | set-level | precision | onboarding-pack | audit-feed.`, ); } diff --git a/src/api/routes.ts b/src/api/routes.ts index 9edc02215e..ee8909c13c 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -53,6 +53,7 @@ import { getRepoQueueTrendSnapshot, getRepositorySettings, getPendingAgentAction, + createPendingAgentActionIfAbsent, listAgentAuditEvents, listAuditEventsForTarget, listPendingAgentActions, @@ -495,6 +496,19 @@ const evaluateEscalationSchema = z.object({ killRequested: z.boolean().optional(), }); +// #6744: mirrors proposeActionShape in src/mcp/server.ts VERBATIM, minus owner/repo (they are path params), so +// POST /v1/repos/:owner/:repo/agent/pending-actions can never stage an action the loopover_propose_action MCP +// tool would reject, or vice versa. actionClass stays the 7-value propose set (a subset of AgentActionClass). +const proposePendingActionSchema = z.object({ + pullNumber: z.number().int().positive(), + actionClass: z.enum(["review", "request_changes", "approve", "merge", "close", "label", "review_state_label"]), + reason: z.string().max(500).optional(), + label: z.string().min(1).max(100).optional(), + reviewBody: z.string().max(60000).optional(), + mergeMethod: z.enum(["merge", "squash", "rebase"]).optional(), + closeComment: z.string().max(60000).optional(), +}); + // #6755: mirrors intakeIdeaShape in src/mcp/server.ts VERBATIM. Fields are deliberately LOOSE here for the same // reason they are on the tool: the engine's validateIdeaSubmission owns the real bounds/format checks and returns // the actionable error list, so an empty/malformed submission must reach the handler rather than be rejected @@ -2689,6 +2703,42 @@ export function createApp() { return c.json(result); }); + // #6744 propose: the CREATE side of the approval queue the list (GET) + decision (POST /:id/:decision) routes + // already cover. Stages an auto_with_approval action for a maintainer to later accept/reject; it never executes + // one. Mirrors the loopover_propose_action MCP tool (src/mcp/server.ts:proposeAction) VERBATIM — same + // requireRepoWriteAccess gate as the decision route, same head-SHA pinning (#2255), same { created, action } shape. + app.post("/v1/repos/:owner/:repo/agent/pending-actions", async (c) => { + const fullName = `${c.req.param("owner")}/${c.req.param("repo")}`; + const gate = await requireRepoWriteAccess(c, fullName); + /* v8 ignore next -- unauthorized requests are rejected by the auth middleware before reaching the handler. */ + if (gate instanceof Response) return gate; + const body = await c.req.json().catch(() => null); + const parsed = proposePendingActionSchema.safeParse(body); + if (!parsed.success) return c.json({ error: "invalid_propose_action_request", issues: parsed.error.issues }, 400); + const repo = await getRepository(c.env, fullName); + if (!repo?.installationId) return c.json({ error: "app_not_installed", detail: "The LoopOver App is not installed on this repository." }, 409); + // Pin the staged action to the head the proposer saw, so the accept path's force-push freshness guard can + // catch an unreviewed force-push between proposal and accept (matches proposeAction, #2255). + const pr = await getPullRequest(c.env, fullName, parsed.data.pullNumber); + const params = { + ...(parsed.data.label !== undefined ? { label: parsed.data.label } : {}), + ...(parsed.data.reviewBody !== undefined ? { reviewBody: parsed.data.reviewBody } : {}), + ...(parsed.data.mergeMethod !== undefined ? { mergeMethod: parsed.data.mergeMethod } : {}), + ...(parsed.data.closeComment !== undefined ? { closeComment: parsed.data.closeComment } : {}), + ...(pr?.headSha ? { expectedHeadSha: pr.headSha } : {}), + }; + const { action, created } = await createPendingAgentActionIfAbsent(c.env, { + repoFullName: fullName, + pullNumber: parsed.data.pullNumber, + installationId: repo.installationId, + actionClass: parsed.data.actionClass, + autonomyLevel: "auto_with_approval", + params, + reason: parsed.data.reason ?? null, + }); + return c.json({ created, action: { id: action.id, actionClass: action.actionClass, pullNumber: action.pullNumber, status: action.status, reason: action.reason } }); + }); + // #784 audit feed: the agent's executed actions + approval-queue decisions for this repo. Maintainer-scoped, // read-only, public-safe (action posture only — no trust/score metadata). `?since=ISO&limit=N` (max 200). // `?pull=N` opts into the unfiltered sibling query (listAuditEventsForTarget): every audit_events row for @@ -5984,7 +6034,7 @@ function canSessionAccessPath(env: Env, identity: Extract { expect(payload.echoedQuery).toEqual({ since: null, limit: null, pull: null }); }); + it("propose stages a new action (plain + json), POSTing to the bare pending-actions path", async () => { + const requests: Array<{ url: string; method: string }> = []; + const e = await env((request) => requests.push({ url: request.url ?? "", method: request.method ?? "" })); + const plain = await runAsync(["maintain", "propose", "review", "7", "--repo", "owner/repo", "--reason", "needs a look"], e); + expect(plain).toMatch(/Staged review on owner\/repo#7 \(pending\), id pa-1\./); + // The bare create path (no trailing slash) — distinct from the decision `/:id/:decision` POST. + expect(requests.at(-1)).toEqual({ url: "/v1/repos/owner/repo/agent/pending-actions", method: "POST" }); + const json = JSON.parse(await runAsync(["maintain", "propose", "merge", "7", "--repo", "owner/repo", "--merge-method", "squash", "--json"], e)) as { + created: boolean; + action: { actionClass: string; pullNumber: number }; + }; + expect(json).toMatchObject({ created: true, action: { actionClass: "merge", pullNumber: 7 } }); + }); + + it("propose validates the action class and pull number before any request", async () => { + const e = await env(); + await expect(runAsync(["maintain", "propose", "--repo", "owner/repo"], e)).rejects.toThrow(/Usage: loopover-mcp maintain propose/); + await expect(runAsync(["maintain", "propose", "review", "--repo", "owner/repo"], e)).rejects.toThrow(/Usage: loopover-mcp maintain propose/); + await expect(runAsync(["maintain", "propose", "bogus", "7", "--repo", "owner/repo"], e)).rejects.toThrow(/Unknown action class/); + await expect(runAsync(["maintain", "propose", "review", "0", "--repo", "owner/repo"], e)).rejects.toThrow(/Invalid pull number/); + await expect(runAsync(["maintain", "propose", "review", "1.5", "--repo", "owner/repo"], e)).rejects.toThrow(/Invalid pull number/); + }, 45_000); + it("validates inputs: --repo required, id required for approve, known subcommand + action/level", async () => { const e = await env(); await expect(runAsync(["maintain", "status"], e)).rejects.toThrow(/Pass --repo/); @@ -192,6 +215,7 @@ describe("loopover-mcp CLI — maintain (#784)", () => { const out = await runAsync(["maintain"], e); expect(out).toMatch(/Usage: loopover-mcp maintain/); expect(out).toMatch(/approve /); + expect(out).toMatch(/propose /); expect(out).toMatch(/queue/); expect(out).toMatch(/pause/); expect(out).toMatch(/onboarding-pack/); diff --git a/test/unit/routes-agent-approval.test.ts b/test/unit/routes-agent-approval.test.ts index 2e77b20fe8..6fff60182e 100644 --- a/test/unit/routes-agent-approval.test.ts +++ b/test/unit/routes-agent-approval.test.ts @@ -189,6 +189,85 @@ describe("agent approval-queue routes (#779)", () => { }); }); +describe("agent propose route (#6744) — POST create side of the approval queue", () => { + // Seed the repo + installation (and optionally a PR) WITHOUT staging an action — the route is what creates it. + async function seedRepo(env: Env, pr?: { number: number; headSha: string }) { + await upsertInstallation(env, { + installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", contents: "write", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, + repositories: [{ name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }], + }); + // upsertInstallation registers the installation; the repo row's own installationId is set by this call — the + // route's `repo?.installationId` check (mirroring proposeAction) reads that column. + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); + if (pr) await upsertPullRequestFromGitHub(env, "owner/repo", { number: pr.number, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: pr.headSha }, labels: [], body: "x" }); + } + const post = (env: Env, body: unknown) => app.request("/v1/repos/owner/repo/agent/pending-actions", { method: "POST", headers: headers(env), body: JSON.stringify(body) }, env); + + it("stages a minimal action, pinning the PR head SHA and defaulting the rest (parity with the MCP tool's data.action)", async () => { + const env = createTestEnv(); + await seedRepo(env, { number: 7, headSha: "h7" }); + const res = await post(env, { pullNumber: 7, actionClass: "review" }); + expect(res.status).toBe(200); + // The route returns EXACTLY the { created, action:{ id, actionClass, pullNumber, status, reason } } shape the + // loopover_propose_action MCP tool returns in data — no extra/missing fields. + const json = (await res.json()) as { created: boolean; action: { id: string; actionClass: string; pullNumber: number; status: string; reason: string | null } }; + expect(json.created).toBe(true); + expect(json.action).toEqual({ id: expect.any(String), actionClass: "review", pullNumber: 7, status: "pending", reason: null }); + // Head-SHA pinned; no optional params carried when none were sent. + const stored = await getPendingAgentAction(env, json.action.id); + expect(stored?.params).toEqual({ expectedHeadSha: "h7" }); + }); + + it("carries every optional param and omits the head pin when the PR is unknown", async () => { + const env = createTestEnv(); + await seedRepo(env); // installation only — PR #8 is deliberately not seeded, so there is no head to pin + const res = await post(env, { pullNumber: 8, actionClass: "merge", reason: "stale base", label: "needs-rebase", reviewBody: "please rebase", mergeMethod: "squash", closeComment: "closing stale" }); + expect(res.status).toBe(200); + const json = (await res.json()) as { created: boolean; action: { id: string; reason: string | null } }; + expect(json.created).toBe(true); + expect(json.action.reason).toBe("stale base"); + const stored = await getPendingAgentAction(env, json.action.id); + expect(stored?.params).toEqual({ label: "needs-rebase", reviewBody: "please rebase", mergeMethod: "squash", closeComment: "closing stale" }); + }); + + it("is idempotent: a second identical propose returns created:false", async () => { + const env = createTestEnv(); + await seedRepo(env, { number: 7, headSha: "h7" }); + const first = (await (await post(env, { pullNumber: 7, actionClass: "review" })).json()) as { created: boolean; action: { id: string } }; + expect(first.created).toBe(true); + const second = (await (await post(env, { pullNumber: 7, actionClass: "review" })).json()) as { created: boolean; action: { id: string } }; + expect(second.created).toBe(false); + expect(second.action.id).toBe(first.action.id); + }); + + it("rejects a schema-invalid or unparseable body with 400", async () => { + const env = createTestEnv(); + await seedRepo(env, { number: 7, headSha: "h7" }); + for (const body of [{ actionClass: "review" }, { pullNumber: 7, actionClass: "bogus" }, { pullNumber: -1, actionClass: "review" }]) { + const res = await post(env, body); + expect(res.status, JSON.stringify(body)).toBe(400); + await expect(res.json()).resolves.toMatchObject({ error: "invalid_propose_action_request" }); + } + const malformed = await app.request("/v1/repos/owner/repo/agent/pending-actions", { method: "POST", headers: headers(env), body: "{not json" }, env); + expect(malformed.status).toBe(400); + }); + + it("409s when the LoopOver App is not installed on the repo", async () => { + const env = createTestEnv(); // no installation seeded → getRepository has no installationId + const res = await post(env, { pullNumber: 7, actionClass: "review" }); + expect(res.status).toBe(409); + await expect(res.json()).resolves.toMatchObject({ error: "app_not_installed" }); + }); + + it("forbids a non-maintainer session from staging an action", async () => { + const env = createTestEnv(); + await seedRepo(env, { number: 7, headSha: "h7" }); + const { token } = await createSessionForGitHubUser(env, { login: "rando", id: 555 }); + const res = await app.request("/v1/repos/owner/repo/agent/pending-actions", { method: "POST", headers: { authorization: `Bearer ${token}`, "content-type": "application/json" }, body: JSON.stringify({ pullNumber: 7, actionClass: "review" }) }, env); + expect([401, 403]).toContain(res.status); + }); +}); + describe("agent audit-feed route (#784)", () => { async function seedAudit(env: Env) { await recordAuditEvent(env, { eventType: "agent.action.merge", actor: "loopover", targetKey: "owner/repo#7", outcome: "completed", detail: "merged", createdAt: "2026-06-18T10:00:00.000Z" }); diff --git a/test/unit/support/mcp-cli-harness.ts b/test/unit/support/mcp-cli-harness.ts index ff124b8a02..a2be07acd5 100644 --- a/test/unit/support/mcp-cli-harness.ts +++ b/test/unit/support/mcp-cli-harness.ts @@ -439,6 +439,15 @@ export async function startFixtureServer( ); return; } + // #6744 propose: the CREATE side of the approval queue — bare path POST (no trailing slash), so it does NOT + // collide with the decision `.../pending-actions/:id/:decision` POST stub below. Echoes the posted actionClass + // + pullNumber so a test can assert the CLI serialized the right body. + if (request.url === "/v1/repos/owner/repo/agent/pending-actions" && request.method === "POST") { + const body = (await readJsonRequest(request)) as { pullNumber?: number; actionClass?: string; reason?: string | null }; + const action = { id: "pa-1", actionClass: body.actionClass ?? "merge", pullNumber: body.pullNumber ?? 7, status: "pending", reason: body.reason ?? null }; + response.end(JSON.stringify({ created: true, action: options.terminalInjection ? { ...action, actionClass: options.terminalInjection } : action })); + return; + } if (request.url === "/v1/repos/owner/repo/maintainer-noise" && request.method === "GET") { response.end( JSON.stringify({