From 98bf81b1f78abc88fb6d286700a793757aa4c8e0 Mon Sep 17 00:00:00 2001 From: bitfathers94 <237535319+bitfathers94@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:47:06 +0000 Subject: [PATCH] feat(mcp): add loopover_file_incident_report write tool Mirror POST /v1/repos/:owner/:repo/pulls/:number/incident-reports over the MCP surface so a maintainer-authenticated client can file a post-merge incident report on a harmful rented-loop PR, closing the write-side gap next to the already-wrapped maintainer-packet/reviewability read tools. The handler replays the REST route exactly: maintainer-manage auth, the PR-must-exist-and-be-merged validation, then recordPostMergeIncidentReport with reporterKind "customer" and the calling actor, returning the same { ok, repoFullName, pullNumber, ...report } shape. The input body fields (description/severity/mergedSha) are declared inline rather than spread from routes.ts's postMergeIncidentReportSchema.shape, because routes.ts imports the MCP server module before that schema is defined -- dereferencing .shape at module-init would hit the circular-import temporal dead zone. --- src/mcp/server.ts | 78 +++++++++++++ test/unit/mcp-file-incident-report.test.ts | 122 +++++++++++++++++++++ 2 files changed, 200 insertions(+) create mode 100644 test/unit/mcp-file-incident-report.test.ts diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 93e87c09a0..06a6285e9e 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -80,6 +80,7 @@ import { MAX_NOTIFICATION_MARK_READ_IDS, markNotificationDeliveriesRead, recordAuditEvent, + recordPostMergeIncidentReport, recordProductUsageEvent, } from "../db/repositories"; import { decidePendingAgentAction } from "../services/agent-approval-queue"; @@ -258,6 +259,20 @@ const clearSelftuneOverrideShape = { confirm: z.literal(true), }; +// (#9298) owner/repo/pull (mirrors ownerRepoPullShape) plus the same body fields the REST route's +// postMergeIncidentReportSchema validates. Declared inline rather than spread from that schema's `.shape` +// because src/api/routes.ts imports this module before it defines postMergeIncidentReportSchema, so reading +// `.shape` at module-init time would dereference `undefined` (circular-import temporal dead zone). +const fileIncidentReportShape = { + ...ownerRepoPullShape, + description: z.string().min(1).max(4000), + severity: z.enum(["low", "medium", "high", "critical"]), + mergedSha: z + .string() + .regex(/^[0-9a-f]{7,40}$/i) + .optional(), +}; + const windowOnlyShape = { windowDays: z.number().int().positive().optional(), }; @@ -1134,6 +1149,17 @@ const clearSelftuneOverrideOutputSchema = { cleared: z.boolean().optional(), }; +// (#9298) mirrors the REST incident-report route's response: `{ ok: true, repoFullName, pullNumber, ...report }` +// on success, or `{ ok: false, error }` when the PR is missing/unmerged (the REST route's 404/409 bodies). +const fileIncidentReportOutputSchema = { + ok: z.boolean(), + repoFullName: z.string(), + pullNumber: z.number().int().positive(), + id: z.string().optional(), + createdAt: z.string().optional(), + error: z.enum(["pull_request_not_found", "pull_request_not_merged"]).optional(), +}; + // #5825 - maintainer-authenticated skipped-PR audit trail, mirroring GET /v1/app/skipped-pr-audit's // filters (all optional: a bare call returns the caller's own repo-scoped feed). No owner/repo shape // here on purpose: unlike ownerRepoShape tools this report can legitimately span every repo the caller @@ -2028,6 +2054,7 @@ export const MCP_TOOL_CATEGORIES: Record = { loopover_get_gate_precision: "maintainer", loopover_get_selftune_override_audit: "maintainer", loopover_clear_selftune_override: "maintainer", + loopover_file_incident_report: "maintainer", loopover_get_skipped_pr_audit: "maintainer", loopover_get_fleet_analytics: "maintainer", loopover_get_recommendation_quality: "maintainer", @@ -2333,6 +2360,20 @@ export class LoopoverMcp { async (input) => this.toolResult(await this.clearSelftuneOverride(input)), ); + // (#9298) MCP mirror of POST /v1/repos/:owner/:repo/pulls/:number/incident-reports (#5672): the missing + // write tool next to the already-wrapped PR read surfaces (maintainer-packet, reviewability). Same + // maintainer-manage boundary and recordPostMergeIncidentReport persistence path as the REST route. + register( + "loopover_file_incident_report", + { + description: + "File a post-merge incident report on an already-merged rented-loop PR later found harmful, mirroring POST /v1/repos/:owner/:repo/pulls/:number/incident-reports. Persists an audit_events row keyed to the PR; the PR must exist and be merged. Maintainer access required.", + inputSchema: fileIncidentReportShape, + outputSchema: fileIncidentReportOutputSchema, + }, + async (input) => this.toolResult(await this.fileIncidentReport(input)), + ); + register( "loopover_get_skipped_pr_audit", { @@ -4338,6 +4379,43 @@ export class LoopoverMcp { }; } + // (#9298) Mirrors POST /v1/repos/:owner/:repo/pulls/:number/incident-reports (#5672): maintainer-manage + // gate, then the REST route's exact PR-must-exist-and-be-merged validation, then the same + // recordPostMergeIncidentReport persistence (reporterKind "customer", the calling actor) and response shape. + // Missing/unmerged PRs return the route's 404/409 error codes as a normal `{ ok: false, error }` tool result. + private async fileIncidentReport(input: z.infer>): Promise { + const fullName = `${input.owner}/${input.repo}`; + await this.requireRepoManageAccess(fullName); + const pullRequest = await getPullRequest(this.env, fullName, input.number); + if (!pullRequest) { + return { + summary: `No pull request ${fullName}#${input.number} to file a post-merge incident report against.`, + data: { ok: false, error: "pull_request_not_found", repoFullName: fullName, pullNumber: input.number }, + }; + } + if (!pullRequest.mergedAt) { + return { + summary: `Pull request ${fullName}#${input.number} is not merged; a post-merge incident report cannot be filed.`, + data: { ok: false, error: "pull_request_not_merged", repoFullName: fullName, pullNumber: input.number }, + }; + } + const actor = this.identity.kind === "session" ? this.identity.actor : "mcp"; + const report = await recordPostMergeIncidentReport(this.env, { + repoFullName: fullName, + pullNumber: input.number, + description: input.description, + severity: input.severity, + mergedSha: input.mergedSha, + reporterKind: "customer", + actor, + route: `/v1/repos/${input.owner}/${input.repo}/pulls/${input.number}/incident-reports`, + }); + return { + summary: `Filed a post-merge incident report on ${fullName}#${input.number} (severity ${input.severity}).`, + data: { ok: true, repoFullName: fullName, pullNumber: input.number, ...report }, + }; + } + // #5825 - repo-scope resolution for the skipped-PR audit tool. Mirrors skippedPrAuditRepoScope in // src/api/routes.ts (same underlying loadControlPanelRoleSummary/loadControlPanelAccessScope calls, // same maintainer/owner/operator role gate, same "no filter -> caller's own scoped repos" fallback), diff --git a/test/unit/mcp-file-incident-report.test.ts b/test/unit/mcp-file-incident-report.test.ts new file mode 100644 index 0000000000..d573a50d72 --- /dev/null +++ b/test/unit/mcp-file-incident-report.test.ts @@ -0,0 +1,122 @@ +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { LoopoverMcp } from "../../src/mcp/server"; +import { getRepositoryCollaboratorPermission } from "../../src/github/app"; +import { listAuditEventsForTarget, upsertInstallation, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; +import type { AuthIdentity } from "../../src/auth/security"; +import { createTestEnv } from "../helpers/d1"; + +// #9298: MCP mirror of POST /v1/repos/:owner/:repo/pulls/:number/incident-reports (#5672). The write itself +// persists through the same recordPostMergeIncidentReport helper into a PR-keyed `audit_events` row, read +// back here through listAuditEventsForTarget -- the exact `repo#number` target the REST route documents (the +// agent-audit-feed tool is deliberately scoped to `agent.action.%`/`agent.pending_action.%`, not this event). + +vi.mock("../../src/github/app", async (importOriginal) => ({ + ...(await importOriginal()), + getRepositoryCollaboratorPermission: vi.fn(), +})); +const mockedPermission = vi.mocked(getRepositoryCollaboratorPermission); + +beforeEach(() => { + mockedPermission.mockReset(); + mockedPermission.mockResolvedValue("write"); +}); + +async function connect(env: Env, identity?: AuthIdentity) { + const server = new LoopoverMcp(env, identity).createServer(); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + const client = new Client({ name: "loopover-file-incident-report-test", version: "0.1.0" }, { capabilities: {} }); + await client.connect(clientTransport); + return client; +} + +async function seedRepoWithPulls(env: Env) { + 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"] }, + }); + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "Merged PR", state: "closed", merged_at: "2026-06-18T10:00:00.000Z", user: { login: "a-miner" }, head: { sha: "deadbeef" }, labels: [], body: "x" }); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 8, title: "Open PR", state: "open", user: { login: "a-miner" }, head: { sha: "open-sha" }, labels: [], body: "x" }); +} + +async function metadataRow(env: Env): Promise<{ target_key: string; actor: string; detail: string; metadata_json: string } | null> { + return env.DB.prepare( + "select target_key, actor, detail, metadata_json from audit_events where event_type = 'agent.post_merge_incident_reported' order by created_at desc limit 1", + ).first<{ target_key: string; actor: string; detail: string; metadata_json: string }>(); +} + +describe("MCP loopover_file_incident_report (#9298)", () => { + it("files a report on a merged PR for the shared mcp token, and it reads back on the PR's audit target", async () => { + const env = createTestEnv(); + await seedRepoWithPulls(env); + const client = await connect(env); // default identity: { kind: "static", actor: "mcp" } + + const result = await client.callTool({ name: "loopover_file_incident_report", arguments: { owner: "owner", repo: "repo", number: 7, description: "broke prod config", severity: "high", mergedSha: "deadbeef" } }); + expect(result.isError).toBeFalsy(); + const data = result.structuredContent as { ok: boolean; repoFullName: string; pullNumber: number; id: string; createdAt: string }; + expect(data).toMatchObject({ ok: true, repoFullName: "owner/repo", pullNumber: 7 }); + expect(typeof data.id).toBe("string"); + expect(typeof data.createdAt).toBe("string"); + expect(JSON.stringify(result.content)).toContain("Filed a post-merge incident report on owner/repo#7"); + + // Regression: the recorded incident is one `audit_events` row keyed to the PR (`repo#number`), readable + // back through the same listAuditEventsForTarget path recordPostMergeIncidentReport documents. + const events = await listAuditEventsForTarget(env, { repoFullName: "owner/repo", pullNumber: 7 }); + expect(events).toHaveLength(1); + expect(events[0]).toMatchObject({ eventType: "agent.post_merge_incident_reported", outcome: "completed", actor: "mcp", detail: "broke prod config" }); + const row = await metadataRow(env); + expect(row?.target_key).toBe("owner/repo#7"); + expect(JSON.parse(row!.metadata_json)).toMatchObject({ severity: "high", mergedSha: "deadbeef", reporterKind: "customer" }); + }); + + it("records the reporting maintainer's own login as actor for a session caller, and omits mergedSha as null", async () => { + const env = createTestEnv(); + await seedRepoWithPulls(env); + const client = await connect(env, { kind: "session", actor: "owner" } as AuthIdentity); + + const result = await client.callTool({ name: "loopover_file_incident_report", arguments: { owner: "owner", repo: "repo", number: 7, description: "silent data loss", severity: "critical" } }); + expect(result.isError).toBeFalsy(); + expect(result.structuredContent).toMatchObject({ ok: true, repoFullName: "owner/repo", pullNumber: 7 }); + + const events = await listAuditEventsForTarget(env, { repoFullName: "owner/repo", pullNumber: 7 }); + expect(events).toHaveLength(1); + expect(events[0]?.actor).toBe("owner"); + const row = await metadataRow(env); + expect(JSON.parse(row!.metadata_json)).toMatchObject({ severity: "critical", mergedSha: null, reporterKind: "customer" }); + }); + + it("returns pull_request_not_found for an unknown PR without recording anything", async () => { + const env = createTestEnv(); + await seedRepoWithPulls(env); + const client = await connect(env); + + const result = await client.callTool({ name: "loopover_file_incident_report", arguments: { owner: "owner", repo: "repo", number: 999, description: "x", severity: "low" } }); + expect(result.isError).toBeFalsy(); // a business rejection is a normal tool result, not an MCP-level error + expect(result.structuredContent).toMatchObject({ ok: false, error: "pull_request_not_found", repoFullName: "owner/repo", pullNumber: 999 }); + expect(await metadataRow(env)).toBeFalsy(); + }); + + it("returns pull_request_not_merged for an open PR without recording anything", async () => { + const env = createTestEnv(); + await seedRepoWithPulls(env); + const client = await connect(env); + + const result = await client.callTool({ name: "loopover_file_incident_report", arguments: { owner: "owner", repo: "repo", number: 8, description: "x", severity: "low" } }); + expect(result.isError).toBeFalsy(); + expect(result.structuredContent).toMatchObject({ ok: false, error: "pull_request_not_merged", repoFullName: "owner/repo", pullNumber: 8 }); + expect(await metadataRow(env)).toBeFalsy(); + }); + + it("rejects a caller lacking maintainer-manage access, recording nothing", async () => { + const env = createTestEnv({ MCP_ACTUATION_REPO_ALLOWLIST: "" }); + await seedRepoWithPulls(env); + const client = await connect(env); // default static mcp identity, no actuation allowlist + + const result = await client.callTool({ name: "loopover_file_incident_report", arguments: { owner: "owner", repo: "repo", number: 7, description: "x", severity: "low" } }); + expect(result.isError).toBe(true); + expect(JSON.stringify(result)).toMatch(/MCP_ACTUATION_REPO_ALLOWLIST/); + expect(await metadataRow(env)).toBeFalsy(); + }); +});