Skip to content

Commit 7d4117c

Browse files
authored
feat(commands): wire @gittensory review and resume PR-comment commands (#4175)
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.
1 parent 6713609 commit 7d4117c

2 files changed

Lines changed: 380 additions & 66 deletions

File tree

src/queue/processors.ts

Lines changed: 104 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5618,7 +5618,9 @@ async function processGitHubWebhook(
56185618

56195619
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; }
56205620
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; }
5621+
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; }
56215622
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; }
5623+
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; }
56225624
if (
56235625
eventName === "issue_comment" &&
56245626
(await maybeProcessConfigurationCommand(env, deliveryId, payload))
@@ -11088,6 +11090,52 @@ async function maybeProcessResolveCommand(env: Env, deliveryId: string, payload:
1108811090
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 } : {}) } });
1108911091
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; }
1109011092

11093+
/**
11094+
* `@gittensory review` (#2163, part of #1960, alias `re-review`): a maintainer/collaborator/confirmed-miner
11095+
* asks for a fresh AUTO-REVIEW pass on this PR. AUTO-REVIEW SCOPE ONLY, same hard constraint as pause/resolve/
11096+
* explain (#1960): this dispatches to the EXISTING reReviewStoredPullRequest path with `force: true` (bypasses
11097+
* the AI-review cache/dedup, since a maintainer explicitly typing the command wants a fresh verdict, not a
11098+
* cached one) — it never touches the Gate check-run, the AgentActionMode, or the one-shot disposition directly;
11099+
* whatever reReviewStoredPullRequest's own gate evaluation produces is exactly what a scheduled sweep pass
11100+
* would produce. If the PR is currently paused (hasAutoreviewPausedMarker), reReviewStoredPullRequest's own
11101+
* existing skipAiReview-on-pause behavior still applies — this command does not special-case or bypass pause;
11102+
* it is a re-review trigger, not a resume. Mirrors maybeProcessResolveCommand's classify → authorize → dispatch
11103+
* shape. Returns true once it owns the event.
11104+
*/
11105+
async function maybeProcessReviewCommand(env: Env, deliveryId: string, payload: GitHubWebhookPayload): Promise<boolean> {
11106+
const command = parseGittensoryMentionCommand(payload.comment?.body);
11107+
if (!command || command.name !== "review") return false;
11108+
const { classifyPrCommandRequest } = await import("../github/pr-command-request");
11109+
const req = classifyPrCommandRequest(payload, getInstallationId(payload));
11110+
if (!req.ok) {
11111+
await recordReviewCommandSkip(env, deliveryId, req.repoFullName, req.targetKey, req.actor, req.reason);
11112+
return true;
11113+
}
11114+
const targetKey = `${req.repoFullName}#${req.pr.number}`;
11115+
const [pr, settings] = await Promise.all([getPullRequest(env, req.repoFullName, req.pr.number), resolveRepositorySettings(env, req.repoFullName)]);
11116+
if (!pr) {
11117+
await recordReviewCommandSkip(env, deliveryId, req.repoFullName, targetKey, req.actor, "cached_pr_missing");
11118+
return true;
11119+
}
11120+
const { authorization } = await authorizePrActionActor({ env, deliveryId, installationId: req.installationId, repoFullName: req.repoFullName, issue: payload.issue!, actor: req.actor, commandName: "review" as GittensoryMentionCommandName, settings, pr });
11121+
if (!authorization.authorized) {
11122+
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") } });
11123+
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") } });
11124+
return true;
11125+
}
11126+
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"));
11127+
await createIssueComment(env, req.installationId, req.repoFullName, req.pr.number, confirmation);
11128+
await reReviewStoredPullRequest(env, deliveryId, req.installationId, req.repoFullName, req.pr.number, undefined, { force: true });
11129+
await recordAuditEvent(env, { eventType: "github_app.review_command_completed", actor: req.actor, targetKey, outcome: "completed", detail: "Re-review dispatched.", metadata: { deliveryId, repoFullName: req.repoFullName } });
11130+
await recordGithubProductUsage(env, "review_command_completed", { actor: req.actor, repoFullName: req.repoFullName, targetKey, outcome: "completed", metadata: { actorKind: authorization.actorKind } });
11131+
return true;
11132+
}
11133+
11134+
async function recordReviewCommandSkip(env: Env, deliveryId: string, repoFullName: string | null, targetKey: string | null, actor: string | null, reason: string): Promise<void> {
11135+
await recordAuditEvent(env, { eventType: "github_app.review_command_skipped", actor, targetKey, outcome: "completed", detail: reason, metadata: { deliveryId, repoFullName, reason } });
11136+
await recordGithubProductUsage(env, "review_command_skipped", { actor, repoFullName, targetKey, outcome: "skipped", metadata: { reason } });
11137+
}
11138+
1109111139
/**
1109211140
* `@gittensory pause` (#2164, part of #1960): a maintainer pauses AUTO-REVIEW for THIS PR only by recording a
1109311141
* 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
1113611184
await recordGithubProductUsage(env, "autoreview_paused_skipped", { actor, repoFullName, targetKey, outcome: "skipped", metadata: { reason } });
1113711185
}
1113811186

11187+
/**
11188+
* `@gittensory resume` (#2165, part of #1960): the inverse of pause — clears the per-PR auto-review-paused
11189+
* marker by recording a `github_app.autoreview_resumed` event that SUPERSEDES an earlier pause (see
11190+
* hasAutoreviewPausedMarker below, which now reads the MOST RECENT of {paused, resumed} rather than merely
11191+
* checking pause existence — see that function's own doc comment for why the old existence-only check made
11192+
* resume a no-op). Same hard constraint as pause: AUTO-REVIEW SCOPE ONLY, never touches the Gate check-run,
11193+
* AgentActionMode, or the one-shot disposition. Mirrors maybeProcessPauseCommand's classify → authorize →
11194+
* record shape exactly. Returns true once it owns the event.
11195+
*/
11196+
async function maybeProcessResumeCommand(env: Env, deliveryId: string, payload: GitHubWebhookPayload): Promise<boolean> {
11197+
const command = parseGittensoryMentionCommand(payload.comment?.body);
11198+
if (!command || command.name !== "resume") return false;
11199+
const { classifyPrCommandRequest } = await import("../github/pr-command-request");
11200+
const req = classifyPrCommandRequest(payload, getInstallationId(payload));
11201+
if (!req.ok) {
11202+
await recordAutoreviewResumedSkip(env, deliveryId, req.repoFullName, req.targetKey, req.actor, req.reason);
11203+
return true;
11204+
}
11205+
const targetKey = `${req.repoFullName}#${req.pr.number}`;
11206+
const [pr, settings] = await Promise.all([getPullRequest(env, req.repoFullName, req.pr.number), resolveRepositorySettings(env, req.repoFullName)]);
11207+
if (!pr) {
11208+
await recordAutoreviewResumedSkip(env, deliveryId, req.repoFullName, targetKey, req.actor, "cached_pr_missing");
11209+
return true;
11210+
}
11211+
const { authorization } = await authorizePrActionActor({ env, deliveryId, installationId: req.installationId, repoFullName: req.repoFullName, issue: payload.issue!, actor: req.actor, commandName: "resume" as GittensoryMentionCommandName, settings, pr });
11212+
if (!authorization.authorized) {
11213+
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") } });
11214+
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") } });
11215+
return true;
11216+
}
11217+
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"));
11218+
await createIssueComment(env, req.installationId, req.repoFullName, req.pr.number, confirmation);
11219+
await recordAuditEvent(env, { eventType: "github_app.autoreview_resumed", actor: req.actor, targetKey, outcome: "completed", detail: "Auto-review resumed.", metadata: { deliveryId, repoFullName: req.repoFullName } });
11220+
await recordGithubProductUsage(env, "autoreview_resumed", { actor: req.actor, repoFullName: req.repoFullName, targetKey, outcome: "completed", metadata: { actorKind: authorization.actorKind } });
11221+
return true;
11222+
}
11223+
11224+
async function recordAutoreviewResumedSkip(env: Env, deliveryId: string, repoFullName: string | null, targetKey: string | null, actor: string | null, reason: string): Promise<void> {
11225+
await recordAuditEvent(env, { eventType: "github_app.autoreview_resumed_skipped", actor, targetKey, outcome: "completed", detail: reason, metadata: { deliveryId, repoFullName, reason } });
11226+
await recordGithubProductUsage(env, "autoreview_resumed_skipped", { actor, repoFullName, targetKey, outcome: "skipped", metadata: { reason } });
11227+
}
11228+
11229+
/** True when the MOST RECENT of {autoreview_paused, autoreview_resumed} for this target is a pause (#2165
11230+
* fix): the original version of this check only tested for the EXISTENCE of any autoreview_paused row ever
11231+
* recorded, so a resume command could parse/authorize/post its confirmation but silently fail to actually
11232+
* resume auto-review -- the very next re-review pass would still read the stale pause as active forever.
11233+
* Ordering by created_at DESC across BOTH event types and checking which one is latest lets a resume
11234+
* genuinely supersede an earlier pause, while a later pause after a resume still re-pauses correctly.
11235+
* `created_at` is millisecond-precision text, so two rows written within the same millisecond (a real
11236+
* possibility for back-to-back commands) would tie under created_at alone -- `rowid DESC` (audit_events'
11237+
* implicit insertion-order column; `id` itself is a non-chronological TEXT primary key) breaks the tie by
11238+
* true write order, not timestamp precision. */
1113911239
async function hasAutoreviewPausedMarker(env: Env, repoFullName: string, prNumber: number): Promise<boolean> {
1114011240
try {
1114111241
const row = await env.DB.prepare(
11142-
"select 1 from audit_events where event_type = ? and target_key = ? and outcome = ? order by created_at desc limit 1",
11242+
"select event_type from audit_events where event_type in (?, ?) and target_key = ? and outcome = ? order by created_at desc, rowid desc limit 1",
1114311243
)
11144-
.bind("github_app.autoreview_paused", `${repoFullName}#${prNumber}`, "completed")
11145-
.first();
11146-
return Boolean(row);
11244+
.bind("github_app.autoreview_paused", "github_app.autoreview_resumed", `${repoFullName}#${prNumber}`, "completed")
11245+
.first<{ event_type: string }>();
11246+
return row?.event_type === "github_app.autoreview_paused";
1114711247
} catch {
1114811248
/* v8 ignore next -- audit lookup failures fail open so a stale/corrupt ledger cannot wedge review processing. */
1114911249
return false;

0 commit comments

Comments
 (0)