From a93956fc0fd6948caa4ac4d405012d37b74bc952 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 8 Jul 2026 02:39:05 -0700 Subject: [PATCH] feat(commands): wire @gittensory review and resume PR-comment commands Adds maybeProcessReviewCommand (#2163) and maybeProcessResumeCommand (#2165), the last two handlers in the @gittensory PR-comment command surface (#1960). Both mirror the existing classify -> authorize -> dispatch shape used by pause/resolve/explain. review dispatches to the existing reReviewStoredPullRequest path with force:true so a maintainer gets a fresh verdict instead of a cached one; it never touches the Gate check-run or one-shot disposition. resume fixes hasAutoreviewPausedMarker, which previously only checked whether a pause row had EVER been recorded, so a resume command could authorize and confirm but never actually un-pause anything. It now reads the most recent of {paused, resumed} for the target, with a rowid tiebreaker for same-millisecond writes. --- src/queue/processors.ts | 108 ++++++++++++- test/unit/queue.test.ts | 338 ++++++++++++++++++++++++++++++++-------- 2 files changed, 380 insertions(+), 66 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 33022e912c..01eeccc05e 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -5618,7 +5618,9 @@ async function processGitHubWebhook( if (eventName === "issue_comment" && (await maybeProcessResolveCommand(env, deliveryId, payload))) { await recordWebhookEvent(env, { deliveryId, eventName, action: payload.action, installationId: payload.installation?.id, repositoryFullName: payload.repository?.full_name, payloadHash: "processed", status: "processed" }); return; } if (eventName === "issue_comment" && (await maybeProcessExplainCommand(env, deliveryId, payload))) { await recordWebhookEvent(env, { deliveryId, eventName, action: payload.action, installationId: payload.installation?.id, repositoryFullName: payload.repository?.full_name, payloadHash: "processed", status: "processed" }); return; } + if (eventName === "issue_comment" && (await maybeProcessReviewCommand(env, deliveryId, payload))) { await recordWebhookEvent(env, { deliveryId, eventName, action: payload.action, installationId: payload.installation?.id, repositoryFullName: payload.repository?.full_name, payloadHash: "processed", status: "processed" }); return; } if (eventName === "issue_comment" && (await maybeProcessPauseCommand(env, deliveryId, payload))) { await recordWebhookEvent(env, { deliveryId, eventName, action: payload.action, installationId: payload.installation?.id, repositoryFullName: payload.repository?.full_name, payloadHash: "processed", status: "processed" }); return; } + if (eventName === "issue_comment" && (await maybeProcessResumeCommand(env, deliveryId, payload))) { await recordWebhookEvent(env, { deliveryId, eventName, action: payload.action, installationId: payload.installation?.id, repositoryFullName: payload.repository?.full_name, payloadHash: "processed", status: "processed" }); return; } if ( eventName === "issue_comment" && (await maybeProcessConfigurationCommand(env, deliveryId, payload)) @@ -11088,6 +11090,52 @@ async function maybeProcessResolveCommand(env: Env, deliveryId: string, payload: await recordAuditEvent(env, { eventType: "github_app.finding_resolved", actor: req.actor, targetKey, outcome: "completed", detail: `Marked ${resolvedLabel} as resolved.`, metadata: { deliveryId, repoFullName: req.repoFullName, scope: findingRef.scope, resolvedWarningCount: selection.findings.length, recordedSuppressionCount, ...(findingRef.scope === "single" ? { findingCode: findingRef.findingCode } : {}) } }); await recordGithubProductUsage(env, "finding_resolved", { actor: req.actor, repoFullName: req.repoFullName, targetKey, outcome: "completed", metadata: { scope: findingRef.scope, resolvedWarningCount: selection.findings.length, recordedSuppressionCount, ...(findingRef.scope === "single" ? { findingCode: findingRef.findingCode } : {}) } }); return true; } +/** + * `@gittensory review` (#2163, part of #1960, alias `re-review`): a maintainer/collaborator/confirmed-miner + * asks for a fresh AUTO-REVIEW pass on this PR. AUTO-REVIEW SCOPE ONLY, same hard constraint as pause/resolve/ + * explain (#1960): this dispatches to the EXISTING reReviewStoredPullRequest path with `force: true` (bypasses + * the AI-review cache/dedup, since a maintainer explicitly typing the command wants a fresh verdict, not a + * cached one) — it never touches the Gate check-run, the AgentActionMode, or the one-shot disposition directly; + * whatever reReviewStoredPullRequest's own gate evaluation produces is exactly what a scheduled sweep pass + * would produce. If the PR is currently paused (hasAutoreviewPausedMarker), reReviewStoredPullRequest's own + * existing skipAiReview-on-pause behavior still applies — this command does not special-case or bypass pause; + * it is a re-review trigger, not a resume. Mirrors maybeProcessResolveCommand's classify → authorize → dispatch + * shape. Returns true once it owns the event. + */ +async function maybeProcessReviewCommand(env: Env, deliveryId: string, payload: GitHubWebhookPayload): Promise { + const command = parseGittensoryMentionCommand(payload.comment?.body); + if (!command || command.name !== "review") return false; + const { classifyPrCommandRequest } = await import("../github/pr-command-request"); + const req = classifyPrCommandRequest(payload, getInstallationId(payload)); + if (!req.ok) { + await recordReviewCommandSkip(env, deliveryId, req.repoFullName, req.targetKey, req.actor, req.reason); + return true; + } + const targetKey = `${req.repoFullName}#${req.pr.number}`; + const [pr, settings] = await Promise.all([getPullRequest(env, req.repoFullName, req.pr.number), resolveRepositorySettings(env, req.repoFullName)]); + if (!pr) { + await recordReviewCommandSkip(env, deliveryId, req.repoFullName, targetKey, req.actor, "cached_pr_missing"); + return true; + } + const { authorization } = await authorizePrActionActor({ env, deliveryId, installationId: req.installationId, repoFullName: req.repoFullName, issue: payload.issue!, actor: req.actor, commandName: "review" as GittensoryMentionCommandName, settings, pr }); + if (!authorization.authorized) { + await recordAuditEvent(env, { eventType: "github_app.review_command_denied", actor: req.actor, targetKey, outcome: "denied", detail: authorization.reason, metadata: { deliveryId, repoFullName: req.repoFullName, allowedRoles: commandAuthorizationAllowedRoles(settings.commandAuthorization, "review") } }); + await recordGithubProductUsage(env, "review_command_denied", { actor: req.actor, repoFullName: req.repoFullName, targetKey, outcome: "denied", metadata: { reason: authorization.reason, actorKind: authorization.actorKind, allowedRoles: commandAuthorizationAllowedRoles(settings.commandAuthorization, "review") } }); + return true; + } + const confirmation = sanitizePublicComment([AGENT_COMMAND_COMMENT_MARKER, "", "> [!NOTE]", `> **Re-review triggered by @${req.actor}**`, "> Re-running auto-review for this PR. The Gate check-run and one-shot disposition are produced the same way a scheduled pass would.", "", "---", gittensoryFooter()].join("\n")); + await createIssueComment(env, req.installationId, req.repoFullName, req.pr.number, confirmation); + await reReviewStoredPullRequest(env, deliveryId, req.installationId, req.repoFullName, req.pr.number, undefined, { force: true }); + await recordAuditEvent(env, { eventType: "github_app.review_command_completed", actor: req.actor, targetKey, outcome: "completed", detail: "Re-review dispatched.", metadata: { deliveryId, repoFullName: req.repoFullName } }); + await recordGithubProductUsage(env, "review_command_completed", { actor: req.actor, repoFullName: req.repoFullName, targetKey, outcome: "completed", metadata: { actorKind: authorization.actorKind } }); + return true; +} + +async function recordReviewCommandSkip(env: Env, deliveryId: string, repoFullName: string | null, targetKey: string | null, actor: string | null, reason: string): Promise { + await recordAuditEvent(env, { eventType: "github_app.review_command_skipped", actor, targetKey, outcome: "completed", detail: reason, metadata: { deliveryId, repoFullName, reason } }); + await recordGithubProductUsage(env, "review_command_skipped", { actor, repoFullName, targetKey, outcome: "skipped", metadata: { reason } }); +} + /** * `@gittensory pause` (#2164, part of #1960): a maintainer pauses AUTO-REVIEW for THIS PR only by recording a * per-PR `github_app.autoreview_paused` marker (an audit event keyed to repo#pr) that the sweep/webhook re-review @@ -11136,14 +11184,66 @@ async function recordAutoreviewPausedSkip(env: Env, deliveryId: string, repoFull await recordGithubProductUsage(env, "autoreview_paused_skipped", { actor, repoFullName, targetKey, outcome: "skipped", metadata: { reason } }); } +/** + * `@gittensory resume` (#2165, part of #1960): the inverse of pause — clears the per-PR auto-review-paused + * marker by recording a `github_app.autoreview_resumed` event that SUPERSEDES an earlier pause (see + * hasAutoreviewPausedMarker below, which now reads the MOST RECENT of {paused, resumed} rather than merely + * checking pause existence — see that function's own doc comment for why the old existence-only check made + * resume a no-op). Same hard constraint as pause: AUTO-REVIEW SCOPE ONLY, never touches the Gate check-run, + * AgentActionMode, or the one-shot disposition. Mirrors maybeProcessPauseCommand's classify → authorize → + * record shape exactly. Returns true once it owns the event. + */ +async function maybeProcessResumeCommand(env: Env, deliveryId: string, payload: GitHubWebhookPayload): Promise { + const command = parseGittensoryMentionCommand(payload.comment?.body); + if (!command || command.name !== "resume") return false; + const { classifyPrCommandRequest } = await import("../github/pr-command-request"); + const req = classifyPrCommandRequest(payload, getInstallationId(payload)); + if (!req.ok) { + await recordAutoreviewResumedSkip(env, deliveryId, req.repoFullName, req.targetKey, req.actor, req.reason); + return true; + } + const targetKey = `${req.repoFullName}#${req.pr.number}`; + const [pr, settings] = await Promise.all([getPullRequest(env, req.repoFullName, req.pr.number), resolveRepositorySettings(env, req.repoFullName)]); + if (!pr) { + await recordAutoreviewResumedSkip(env, deliveryId, req.repoFullName, targetKey, req.actor, "cached_pr_missing"); + return true; + } + const { authorization } = await authorizePrActionActor({ env, deliveryId, installationId: req.installationId, repoFullName: req.repoFullName, issue: payload.issue!, actor: req.actor, commandName: "resume" as GittensoryMentionCommandName, settings, pr }); + if (!authorization.authorized) { + await recordAuditEvent(env, { eventType: "github_app.autoreview_resumed_denied", actor: req.actor, targetKey, outcome: "denied", detail: authorization.reason, metadata: { deliveryId, repoFullName: req.repoFullName, allowedRoles: commandAuthorizationAllowedRoles(settings.commandAuthorization, "resume") } }); + await recordGithubProductUsage(env, "autoreview_resumed_denied", { actor: req.actor, repoFullName: req.repoFullName, targetKey, outcome: "denied", metadata: { reason: authorization.reason, actorKind: authorization.actorKind, allowedRoles: commandAuthorizationAllowedRoles(settings.commandAuthorization, "resume") } }); + return true; + } + const confirmation = sanitizePublicComment([AGENT_COMMAND_COMMENT_MARKER, "", "> [!NOTE]", `> **Auto-review resumed by @${req.actor}**`, "> Auto-review is resumed for this PR. Gate enforcement and the one-shot disposition were never affected by pause.", "", "---", gittensoryFooter()].join("\n")); + await createIssueComment(env, req.installationId, req.repoFullName, req.pr.number, confirmation); + await recordAuditEvent(env, { eventType: "github_app.autoreview_resumed", actor: req.actor, targetKey, outcome: "completed", detail: "Auto-review resumed.", metadata: { deliveryId, repoFullName: req.repoFullName } }); + await recordGithubProductUsage(env, "autoreview_resumed", { actor: req.actor, repoFullName: req.repoFullName, targetKey, outcome: "completed", metadata: { actorKind: authorization.actorKind } }); + return true; +} + +async function recordAutoreviewResumedSkip(env: Env, deliveryId: string, repoFullName: string | null, targetKey: string | null, actor: string | null, reason: string): Promise { + await recordAuditEvent(env, { eventType: "github_app.autoreview_resumed_skipped", actor, targetKey, outcome: "completed", detail: reason, metadata: { deliveryId, repoFullName, reason } }); + await recordGithubProductUsage(env, "autoreview_resumed_skipped", { actor, repoFullName, targetKey, outcome: "skipped", metadata: { reason } }); +} + +/** True when the MOST RECENT of {autoreview_paused, autoreview_resumed} for this target is a pause (#2165 + * fix): the original version of this check only tested for the EXISTENCE of any autoreview_paused row ever + * recorded, so a resume command could parse/authorize/post its confirmation but silently fail to actually + * resume auto-review -- the very next re-review pass would still read the stale pause as active forever. + * Ordering by created_at DESC across BOTH event types and checking which one is latest lets a resume + * genuinely supersede an earlier pause, while a later pause after a resume still re-pauses correctly. + * `created_at` is millisecond-precision text, so two rows written within the same millisecond (a real + * possibility for back-to-back commands) would tie under created_at alone -- `rowid DESC` (audit_events' + * implicit insertion-order column; `id` itself is a non-chronological TEXT primary key) breaks the tie by + * true write order, not timestamp precision. */ async function hasAutoreviewPausedMarker(env: Env, repoFullName: string, prNumber: number): Promise { try { const row = await env.DB.prepare( - "select 1 from audit_events where event_type = ? and target_key = ? and outcome = ? order by created_at desc limit 1", + "select event_type from audit_events where event_type in (?, ?) and target_key = ? and outcome = ? order by created_at desc, rowid desc limit 1", ) - .bind("github_app.autoreview_paused", `${repoFullName}#${prNumber}`, "completed") - .first(); - return Boolean(row); + .bind("github_app.autoreview_paused", "github_app.autoreview_resumed", `${repoFullName}#${prNumber}`, "completed") + .first<{ event_type: string }>(); + return row?.event_type === "github_app.autoreview_paused"; } catch { /* v8 ignore next -- audit lookup failures fail open so a stale/corrupt ledger cannot wedge review processing. */ return false; diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 9d47d17b8b..0ea063d51a 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -10865,6 +10865,17 @@ describe("queue processors", () => { await setupPlannerRepo(env); await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 77, title: "Add a retry to the fetch helper", state: "open", user: { login: "reporter" }, head: { sha: "h1" }, labels: [], body: "b" }); } + // Mirrors hasAutoreviewPausedMarker's own MOST-RECENT-of-{paused,resumed} query (#2165) via a raw read, + // rather than exporting that internal helper just for tests -- same pattern the pre-existing pause tests + // already use (raw audit_events queries) instead of importing processors.ts internals. + async function isCurrentlyPaused(env: Env, repoFullName: string, prNumber: number): Promise { + const row = await env.DB.prepare( + "select event_type from audit_events where event_type in (?, ?) and target_key = ? and outcome = ? order by created_at desc, rowid desc limit 1", + ) + .bind("github_app.autoreview_paused", "github_app.autoreview_resumed", `${repoFullName}#${prNumber}`, "completed") + .first<{ event_type: string }>(); + return row?.event_type === "github_app.autoreview_paused"; + } it("pause (#2164): a maintainer @gittensory pause records the autoreview-paused marker and posts a public-safe confirmation", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); @@ -10992,6 +11003,271 @@ describe("queue processors", () => { expect(paused).toBeFalsy(); }); + const reviewIssue = { number: 78, title: "Draft feature for review command", state: "open", user: { login: "reporter" }, body: "b", pull_request: { url: "https://api.github.com/repos/JSONbored/gittensory/pulls/78" } }; + async function seedReviewPr(env: Env, options: { draft?: boolean } = {}): Promise { + await setupPlannerRepo(env); + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { review: { auto_review: { skip_drafts: true } } }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 78, title: "Draft feature for review command", state: "open", draft: options.draft ?? true, user: { login: "reporter" }, head: { sha: "r78" }, labels: [], body: "b" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 78, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + } + function reviewCommandFetchStub(): (input: RequestInfo | URL, init?: RequestInit) => Promise { + const seen: string[] = []; + return async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + seen.push(url); + const method = init?.method ?? "GET"; + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "admin" }); // maintainer + if (url.includes("/pulls/78/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]); + if (url.endsWith("/pulls/78")) return Response.json({ number: 78, title: "Draft feature for review command", state: "open", draft: true, user: { login: "reporter" }, head: { sha: "r78" }, labels: [], body: "b", mergeable_state: "clean" }); + if (url.includes("/commits/r78/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/r78/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + if (url.includes("/issues/78/comments") && method === "POST") return Response.json({ id: 78 }, { status: 201 }); + if (url.includes("/issues/78/comments")) return Response.json([]); + if (url.includes("/check-runs") && (method === "POST" || method === "PATCH")) return Response.json({ id: 981 }, { status: method === "POST" ? 201 : 200 }); + return Response.json({}); + }; + } + + it("review (#2163): an authorized @gittensory review posts a confirmation, dispatches a REAL re-review (proven by a live PR resync fetch inside reReviewStoredPullRequest, not just the command's own comment post), and records review_command_completed", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedReviewPr(env); + let postedCommentBody: string | undefined; + let liveResyncFetched = false; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("/issues/78/comments") && method === "POST") { + postedCommentBody = init?.body ? JSON.parse(init.body.toString()).body : undefined; + return Response.json({ id: 78 }, { status: 201 }); + } + // reReviewStoredPullRequest's own live-head resync (#sweep-resync) GETs the PR fresh before reviewing -- + // this only happens INSIDE that function, never in the command handler's own classify/authorize/confirm + // steps, so seeing it proves the dispatch call genuinely reached the real re-review path. + if (url.endsWith("/pulls/78") && method === "GET") liveResyncFetched = true; + return reviewCommandFetchStub()(input, init); + }); + await processJob(env, plannerWebhook("@gittensory review", "maintainer1", reviewIssue)); + expect(postedCommentBody).toContain("Re-review triggered by @maintainer1"); + expect(liveResyncFetched).toBe(true); // proves the real reReviewStoredPullRequest path ran, unlike pause + const audit = await env.DB.prepare("select outcome, detail from audit_events where event_type = ?").bind("github_app.review_command_completed").first<{ outcome: string; detail: string }>(); + expect(audit?.outcome).toBe("completed"); + const usage = await env.DB.prepare("select outcome from product_usage_events where event_name = ?").bind("review_command_completed").first<{ outcome: string }>(); + expect(usage?.outcome).toBe("completed"); + // The command itself never writes repository_settings -- it only triggers a fresh eval through the same + // path a scheduled sweep would take (#2163's hard constraint: never reimplements/flips the disposition). + const settingsRow = await env.DB.prepare("select 1 from repository_settings where repo_full_name = ?").bind("JSONbored/gittensory").first(); + expect(settingsRow).toBeFalsy(); + }); + + it("review: the 're-review' alias resolves to the same handler", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedReviewPr(env); + let postedCommentBody: string | undefined; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("/issues/78/comments") && method === "POST") { + postedCommentBody = init?.body ? JSON.parse(init.body.toString()).body : undefined; + return Response.json({ id: 78 }, { status: 201 }); + } + return reviewCommandFetchStub()(input, init); + }); + await processJob(env, plannerWebhook("@gittensory re-review", "maintainer1", reviewIssue)); + expect(postedCommentBody).toContain("Re-review triggered by @maintainer1"); + }); + + it("review: a non-maintainer/collaborator/confirmed-miner is denied — nothing posted, no re-review dispatched", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedReviewPr(env); + let posted = false; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "read" }); // not authorized + if (url.includes("/comments")) posted = true; + return new Response("not found", { status: 404 }); + }); + await processJob(env, plannerWebhook("@gittensory review", "outsider", reviewIssue)); + expect(posted).toBe(false); + const denied = await env.DB.prepare("select outcome from audit_events where event_type = ?").bind("github_app.review_command_denied").first<{ outcome: string }>(); + expect(denied?.outcome).toBe("denied"); + const completed = await env.DB.prepare("select 1 from audit_events where event_type = ?").bind("github_app.review_command_completed").first(); + expect(completed).toBeFalsy(); + }); + + it("review: a review command on a PR with no cached record is recorded as a cached_pr_missing skip, never posted", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await setupPlannerRepo(env); // repo + installation, but deliberately NO cached PR record + let posted = false; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "admin" }); + if (url.includes("/comments")) posted = true; + return new Response("not found", { status: 404 }); + }); + await processJob(env, plannerWebhook("@gittensory review", "maintainer1", reviewIssue)); + expect(posted).toBe(false); + const skip = await env.DB.prepare("select detail from audit_events where event_type = ?").bind("github_app.review_command_skipped").first<{ detail: string }>(); + expect(skip?.detail).toBe("cached_pr_missing"); + }); + + it("review: a bot-authored @gittensory review is recorded as a classifier skip, never posted", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedReviewPr(env); + let posted = false; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + if (input.toString().includes("/comments")) posted = true; + return new Response("not found", { status: 404 }); + }); + await processJob(env, { + type: "github-webhook", + deliveryId: "review-bot", + eventName: "issue_comment", + payload: { + action: "created", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + issue: reviewIssue, + comment: { body: "@gittensory review", user: { login: "some-bot[bot]", type: "Bot" } }, + sender: { login: "some-bot[bot]", type: "Bot" }, + }, + } as unknown as Parameters[1]); + expect(posted).toBe(false); + const skip = await env.DB.prepare("select detail from audit_events where event_type = ?").bind("github_app.review_command_skipped").first<{ detail: string }>(); + expect(skip?.detail).toBe("bot_author"); + }); + + it("resume (#2165): an authorized @gittensory resume clears an earlier pause and posts a public-safe confirmation", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedPausePr(env); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "admin" }); + if (url.includes("/issues/77/comments") && (init?.method ?? "GET") === "POST") return Response.json({ id: 5 }, { status: 201 }); + if (url.includes("/issues/77/comments")) return Response.json([]); + return new Response("not found", { status: 404 }); + }); + // Pause first, matching real usage: a resume without a prior pause is still valid (idempotent), but this + // proves the SUPERSEDE behavior, not just that resume can run standalone. + await processJob(env, plannerWebhook("@gittensory pause", "maintainer1", pauseIssue)); + expect(await isCurrentlyPaused(env, "JSONbored/gittensory", 77)).toBe(true); + + let postedBody: string | undefined; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "admin" }); + if (url.includes("/issues/77/comments")) { + postedBody = init?.body ? JSON.parse(init.body.toString()).body : undefined; + return Response.json({ id: 6 }, { status: 201 }); + } + return new Response("not found", { status: 404 }); + }); + await processJob(env, plannerWebhook("@gittensory resume", "maintainer1", pauseIssue)); + expect(postedBody).toContain("Auto-review resumed by @maintainer1"); + const audit = await env.DB.prepare("select outcome from audit_events where event_type = ?").bind("github_app.autoreview_resumed").first<{ outcome: string }>(); + expect(audit?.outcome).toBe("completed"); + // The core bug fix (#2165): hasAutoreviewPausedMarker now reads the MOST RECENT of {paused, resumed}, so + // resume actually supersedes the earlier pause instead of silently no-opping forever. + expect(await isCurrentlyPaused(env, "JSONbored/gittensory", 77)).toBe(false); + }); + + it("resume: a LATER pause after a resume still re-pauses correctly (ordering, not just existence)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedPausePr(env); + const adminFetch = (): ((input: RequestInfo | URL, init?: RequestInit) => Promise) => async (input, init) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "admin" }); + if (url.includes("/issues/77/comments") && (init?.method ?? "GET") === "POST") return Response.json({ id: 5 }, { status: 201 }); + if (url.includes("/issues/77/comments")) return Response.json([]); + return new Response("not found", { status: 404 }); + }; + vi.stubGlobal("fetch", adminFetch()); + await processJob(env, plannerWebhook("@gittensory pause", "maintainer1", pauseIssue)); + expect(await isCurrentlyPaused(env, "JSONbored/gittensory", 77)).toBe(true); + vi.stubGlobal("fetch", adminFetch()); + await processJob(env, plannerWebhook("@gittensory resume", "maintainer1", pauseIssue)); + expect(await isCurrentlyPaused(env, "JSONbored/gittensory", 77)).toBe(false); + vi.stubGlobal("fetch", adminFetch()); + await processJob(env, plannerWebhook("@gittensory pause again", "maintainer1", pauseIssue)); + expect(await isCurrentlyPaused(env, "JSONbored/gittensory", 77)).toBe(true); + }); + + it("resume: a non-maintainer/collaborator is denied — nothing posted and the pause marker is untouched", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedPausePr(env); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "admin" }); + if (url.includes("/issues/77/comments") && (init?.method ?? "GET") === "POST") return Response.json({ id: 5 }, { status: 201 }); + if (url.includes("/issues/77/comments")) return Response.json([]); + return new Response("not found", { status: 404 }); + }); + await processJob(env, plannerWebhook("@gittensory pause", "maintainer1", pauseIssue)); + let posted = false; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "read" }); // not authorized + if (url.includes("/comments")) posted = true; + return new Response("not found", { status: 404 }); + }); + await processJob(env, plannerWebhook("@gittensory resume", "outsider", pauseIssue)); + expect(posted).toBe(false); + const denied = await env.DB.prepare("select outcome from audit_events where event_type = ?").bind("github_app.autoreview_resumed_denied").first<{ outcome: string }>(); + expect(denied?.outcome).toBe("denied"); + expect(await isCurrentlyPaused(env, "JSONbored/gittensory", 77)).toBe(true); // still paused + }); + + it("resume: a resume on a PR with no cached record is recorded as a cached_pr_missing skip, never posted", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await setupPlannerRepo(env); // repo + installation, but deliberately NO cached PR record + let posted = false; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "admin" }); + if (url.includes("/comments")) posted = true; + return new Response("not found", { status: 404 }); + }); + await processJob(env, plannerWebhook("@gittensory resume", "maintainer1", pauseIssue)); + expect(posted).toBe(false); + const skip = await env.DB.prepare("select detail from audit_events where event_type = ?").bind("github_app.autoreview_resumed_skipped").first<{ detail: string }>(); + expect(skip?.detail).toBe("cached_pr_missing"); + }); + + it("resume: a bot-authored @gittensory resume is recorded as a classifier skip, never posted", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedPausePr(env); + let posted = false; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + if (input.toString().includes("/comments")) posted = true; + return new Response("not found", { status: 404 }); + }); + await processJob(env, { + type: "github-webhook", + deliveryId: "resume-bot", + eventName: "issue_comment", + payload: { + action: "created", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + issue: pauseIssue, + comment: { body: "@gittensory resume", user: { login: "some-bot[bot]", type: "Bot" } }, + sender: { login: "some-bot[bot]", type: "Bot" }, + }, + } as unknown as Parameters[1]); + expect(posted).toBe(false); + const skip = await env.DB.prepare("select detail from audit_events where event_type = ?").bind("github_app.autoreview_resumed_skipped").first<{ detail: string }>(); + expect(skip?.detail).toBe("bot_author"); + }); + it("REGRESSION (#audit-draft-maintenance): a clean DRAFT PR is never auto-merged/approved/closed (drafts are WIP)", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await upsertInstallation(env, { @@ -24342,68 +24618,6 @@ describe("queue processors", () => { }); }); - it("a #1960 action-command verb with no dispatch handler wired yet (e.g. resume) is bailed out of the Q&A answer-card path, not misrendered as help (#2160)", async () => { - const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); - await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); - await upsertRepositorySettings(env, { - repoFullName: "JSONbored/gittensory", - commentMode: "off", - publicSurface: "off", - autoLabelEnabled: false, - checkRunMode: "off", - gateCheckMode: "enabled", - linkedIssueGateMode: "off", - }); - await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { - number: 93, - title: "Not yet wired", - state: "open", - user: { login: "contributor" }, - author_association: "CONTRIBUTOR", - head: { sha: "action-verb-scaffold" }, - labels: [], - body: "Validation: npm test", - }); - const calls = { token: 0, permission: 0, comments: 0 }; - vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { - const url = input.toString(); - if (url.includes("/access_tokens")) { - calls.token += 1; - return Response.json({ token: "installation-token" }); - } - if (url.includes("/collaborators/")) { - calls.permission += 1; - return Response.json({ permission: "admin" }); - } - if (url.includes("/comments")) { - calls.comments += 1; - return Response.json([]); - } - return new Response("not found", { status: 404 }); - }); - - await processJob(env, { - type: "github-webhook", - deliveryId: "action-verb-scaffold", - eventName: "issue_comment", - payload: { - action: "created", - installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, - repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, - issue: { number: 93, title: "Not yet wired", state: "open", user: { login: "contributor" }, pull_request: {} }, - comment: { id: 900, body: "@gittensory resume", author_association: "OWNER", user: { login: "maintainer", type: "User" } }, - sender: { login: "maintainer", type: "User" }, - }, - }); - - // No handler claims a bare "resume" comment yet (its dispatch lands in a follow-up bounty -- unlike - // "pause"/"resolve"/"configuration", which now have their own handlers), so the Q&A answer-card path must bail - // rather than post a stray "help" card or any other Q&A comment. - expect(calls.comments).toBe(0); - const feedback = await env.DB.prepare("select id from audit_events where event_type = ?").bind("github_app.agent_command_feedback_prompted").first<{ id: string }>(); - expect(feedback ?? null).toBeNull(); - }); - it("ops-alerts job no-ops when GITTENSORY_REVIEW_OPS is OFF (does no anomaly scan)", async () => { const env = createTestEnv(); // flag unset → OFF await env.DB.prepare("INSERT INTO repositories (full_name, owner, name, is_installed, is_registered) VALUES (?, ?, ?, 1, 1)")