diff --git a/src/github/resolve-command.ts b/src/github/resolve-command.ts new file mode 100644 index 0000000000..31aa0e5509 --- /dev/null +++ b/src/github/resolve-command.ts @@ -0,0 +1,21 @@ +// #2166 `@gittensory resolve []` — pure finding-reference normalization for the resolve dispatch +// scaffold. A maintainer marks a posted review finding (or the whole PR's findings) as resolved; suppression +// semantics that feed a future review pass are maintainer-owned (#1964). This module only validates the optional +// trailing argument so the processor can record `github_app.finding_resolved` with a stable finding key. + +const RESOLVE_FINDING_CODE = /^[a-z][a-z0-9_]{0,199}$/; + +export type ResolveFindingRef = + | { ok: true; scope: "whole_pr" } + | { ok: true; scope: "single"; findingCode: string } + | { ok: false; reason: "malformed_finding_id" }; + +/** Normalize the optional trailing text from `@gittensory resolve []`. Empty/absent ⇒ whole-PR ack; + * a present token must be a public-safe finding code (snake_case, optional `finding-` prefix). PURE. */ +export function normalizeResolveFindingRef(raw: string | null | undefined): ResolveFindingRef { + const trimmed = (raw ?? "").trim(); + if (trimmed.length === 0) return { ok: true, scope: "whole_pr" }; + const normalized = trimmed.toLowerCase().replace(/^finding-/, ""); + if (!RESOLVE_FINDING_CODE.test(normalized)) return { ok: false, reason: "malformed_finding_id" }; + return { ok: true, scope: "single", findingCode: normalized }; +} diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 3bc8dc42a4..a5c37ca3af 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -171,6 +171,8 @@ import { parseGittensoryMentionCommand, sanitizePublicComment, } from "../github/commands"; +import { classifyPrCommandRequest } from "../github/pr-command-request"; +import { normalizeResolveFindingRef } from "../github/resolve-command"; import { ensurePullRequestLabel, removePullRequestLabel, @@ -5508,6 +5510,22 @@ 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 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 maybeProcessPlanCommand(env, deliveryId, payload)) @@ -10420,6 +10438,208 @@ async function maybeProcessResolveCommand(env: Env, deliveryId: string, payload: await createOrUpdateAgentCommandComment(env, req.installationId, req.repoFullName, req.pr.number, confirmation, mode); 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 resolve []` (#2166 dispatch scaffold). A maintainer records that a posted review + * finding (or every finding on the PR when no id is supplied) is resolved so it stops re-surfacing in future + * passes. Contributor scope stops at authorization + `github_app.finding_resolved` audit/usage + a public + * confirmation — suppression semantics that feed the next review are maintainer-owned (#1964). + */ +async function maybeProcessResolveCommand( + env: Env, + deliveryId: string, + payload: GitHubWebhookPayload, +): Promise { + const command = parseGittensoryMentionCommand(payload.comment?.body); + if (!command || command.name !== "resolve") return false; + + const req = classifyPrCommandRequest(payload, getInstallationId(payload)); + if (!req.ok) { + await recordFindingResolvedSkip( + 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 recordFindingResolvedSkip( + 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: "resolve" as GittensoryMentionCommandName, + settings, + pr, + }); + if (!authorization.authorized) { + await recordAuditEvent(env, { + eventType: "github_app.finding_resolved_denied", + actor: req.actor, + targetKey, + outcome: "denied", + detail: authorization.reason, + metadata: { + deliveryId, + repoFullName: req.repoFullName, + allowedRoles: commandAuthorizationAllowedRoles( + settings.commandAuthorization, + "resolve", + ), + }, + }); + await recordGithubProductUsage(env, "finding_resolved_denied", { + actor: req.actor, + repoFullName: req.repoFullName, + targetKey, + outcome: "denied", + metadata: { + reason: authorization.reason, + actorKind: authorization.actorKind, + allowedRoles: commandAuthorizationAllowedRoles( + settings.commandAuthorization, + "resolve", + ), + }, + }); + return true; + } + + const findingRef = normalizeResolveFindingRef(command.reason); + if (!findingRef.ok) { + await recordFindingResolvedSkip( + env, + deliveryId, + req.repoFullName, + targetKey, + req.actor, + findingRef.reason, + ); + return true; + } + + const mode = resolveAgentActionMode({ + globalPaused: isGlobalAgentPause(env) || (await isGlobalAgentFrozen(env)), + agentPaused: settings.agentPaused, + agentDryRun: settings.agentDryRun, + }); + const resolvedLabel = + findingRef.scope === "whole_pr" + ? "all review findings on this pull request" + : `\`${findingRef.findingCode}\``; + const confirmation = sanitizePublicComment( + [ + AGENT_COMMAND_COMMENT_MARKER, + "", + "> [!NOTE]", + `> **Review finding resolved by @${req.actor}**`, + `> Marked ${resolvedLabel} as resolved for this PR. The Gate check-run is unchanged.`, + "", + "---", + gittensoryFooter(), + ].join("\n"), + ); + await createOrUpdateAgentCommandComment( + env, + req.installationId, + req.repoFullName, + req.pr.number, + confirmation, + mode, + ); + if (mode === "live") { + 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, + ...(findingRef.scope === "single" + ? { findingCode: findingRef.findingCode } + : {}), + }, + }); + await recordGithubProductUsage(env, "finding_resolved", { + actor: req.actor, + repoFullName: req.repoFullName, + targetKey, + outcome: "completed", + metadata: { + scope: findingRef.scope, + ...(findingRef.scope === "single" + ? { findingCode: findingRef.findingCode } + : {}), + }, + }); + } else { + await recordFindingResolvedSkip( + env, + deliveryId, + req.repoFullName, + targetKey, + req.actor, + mode === "dry_run" ? "dry_run" : "agent_paused", + mode, + ); + } + return true; +} + +async function recordFindingResolvedSkip( + env: Env, + deliveryId: string, + repoFullName: string | null | undefined, + targetKey: string | null | undefined, + actor: string | null, + reason: string, + mode?: "dry_run" | "paused", +): Promise { + await recordAuditEvent(env, { + eventType: "github_app.finding_resolved_skipped", + actor, + targetKey, + outcome: "completed", + detail: reason, + metadata: { + deliveryId, + repoFullName: repoFullName ?? null, + reason, + ...(mode ? { mode } : {}), + }, + }); + await recordGithubProductUsage(env, "finding_resolved_skipped", { + actor, + repoFullName, + targetKey, + outcome: "skipped", + metadata: { reason, ...(mode ? { mode } : {}) }, + }); +} + /** * `@gittensory plan` (#issue-coding-plan, flag-gated by GITTENSORY_REVIEW_PLANNER). On a MAINTAINER's comment on * an ISSUE (not a PR), generate a concise implementation plan from the issue text via Workers AI and post it as an diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index b10a3ff189..3a461cb281 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -22716,12 +22716,12 @@ describe("queue processors", () => { 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 pause waiting on design sign-off", author_association: "OWNER", user: { login: "maintainer", type: "User" } }, + comment: { id: 900, body: "@gittensory configuration", author_association: "OWNER", user: { login: "maintainer", type: "User" } }, sender: { login: "maintainer", type: "User" }, }, }); - // No handler claims a bare "pause" comment yet (its dispatch lands in a follow-up bounty), so the Q&A + // No handler claims a bare "configuration" comment yet (its dispatch lands in a follow-up bounty), 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 }>(); diff --git a/test/unit/resolve-command.test.ts b/test/unit/resolve-command.test.ts new file mode 100644 index 0000000000..fe18cf3889 --- /dev/null +++ b/test/unit/resolve-command.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from "vitest"; +import { normalizeResolveFindingRef } from "../../src/github/resolve-command"; + +describe("normalizeResolveFindingRef (#2166)", () => { + it("treats empty/absent trailing text as a whole-PR ack", () => { + expect(normalizeResolveFindingRef(undefined)).toEqual({ ok: true, scope: "whole_pr" }); + expect(normalizeResolveFindingRef("")).toEqual({ ok: true, scope: "whole_pr" }); + expect(normalizeResolveFindingRef(" ")).toEqual({ ok: true, scope: "whole_pr" }); + }); + + it("accepts a bare finding code and the optional finding- prefix", () => { + expect(normalizeResolveFindingRef("missing_linked_issue")).toEqual({ + ok: true, + scope: "single", + findingCode: "missing_linked_issue", + }); + expect(normalizeResolveFindingRef("finding-missing_linked_issue")).toEqual({ + ok: true, + scope: "single", + findingCode: "missing_linked_issue", + }); + }); + + it("rejects malformed finding references", () => { + expect(normalizeResolveFindingRef("../escape")).toEqual({ ok: false, reason: "malformed_finding_id" }); + expect(normalizeResolveFindingRef("Bad-Hyphen")).toEqual({ ok: false, reason: "malformed_finding_id" }); + expect(normalizeResolveFindingRef("has space")).toEqual({ ok: false, reason: "malformed_finding_id" }); + expect(normalizeResolveFindingRef("9starts_with_digit")).toEqual({ ok: false, reason: "malformed_finding_id" }); + }); +});