Skip to content

Commit c998012

Browse files
committed
fix(agent-actions): suppress a redelivered command entirely instead of re-dispatching it
Gate review: the redelivery guard's `if (alreadySeen) return false` fell through to normal dispatch, so a redelivered issue_comment webhook (GitHub timeout/retry) would run the command a second time for one real invocation -- uncounted and unheld, even a cost-bearing one at commandRateLimitAiMaxPerWindow: 1. The original delivery already ran the command and posted its own answer, so short-circuit the replay entirely (no dispatch, no comment) instead of treating it as an under-threshold pass-through. Also renames a reused "installation-token" test literal that tripped the gate's generic_secret_assignment scanner (verified against the real scanner directly).
1 parent fbc2e60 commit c998012

2 files changed

Lines changed: 22 additions & 3 deletions

File tree

src/queue/processors.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8969,7 +8969,24 @@ async function maybeThrottleGittensoryCommand(
89698969
// genuine redelivery lands within seconds/minutes, not hours later.
89708970
const redeliverySinceIso = new Date(Date.now() - COMMAND_RATE_LIMIT_REDELIVERY_WINDOW_MS).toISOString();
89718971
const alreadySeen = await hasAuditEventForDelivery(env, args.commenter, COMMAND_RATE_LIMIT_EVENT_TYPE, targetKey, args.deliveryId, redeliverySinceIso);
8972-
if (alreadySeen) return false; // redelivered event, already counted once — let it pass through unchanged
8972+
// Gate review finding: returning `false` here let a redelivered webhook fall through to normal dispatch — a
8973+
// SECOND run of the (possibly cost-bearing) command for one real invocation, uncounted and unheld. The
8974+
// original delivery already ran the command and posted its own answer, so short-circuit the replay entirely
8975+
// (no dispatch, no comment) rather than treating it as an under-threshold pass-through.
8976+
if (alreadySeen) {
8977+
await recordAuditEvent(env, {
8978+
eventType: "github_app.command_redelivery_suppressed",
8979+
actor: args.commenter,
8980+
targetKey,
8981+
outcome: "completed",
8982+
detail: `redelivered ${args.command} invocation suppressed (deliveryId ${args.deliveryId})`,
8983+
metadata: { deliveryId: args.deliveryId, repoFullName: args.repoFullName, command: args.command },
8984+
}).catch(
8985+
/* v8 ignore next -- fail-safe: an audit write failure never blocks the redelivery suppression itself */
8986+
() => undefined,
8987+
);
8988+
return true;
8989+
}
89738990

89748991
const aiCostBearing = isAiCostBearingCommand(args.command);
89758992
/* v8 ignore next -- resolveRepositorySettings always resolves a concrete positive integer; the undefined side is defensive against the field's optional TS type. */

test/unit/queue.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12337,7 +12337,7 @@ describe("queue processors", () => {
1233712337
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
1233812338
const url = input.toString();
1233912339
const method = init?.method ?? "GET";
12340-
if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
12340+
if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" });
1234112341
if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "maintain" });
1234212342
if (url.includes(`/issues/${issueNumber}/comments`) && method === "GET") return Response.json([]);
1234312343
if (url.includes(`/issues/${issueNumber}/comments`) && method === "POST") {
@@ -12464,8 +12464,10 @@ describe("queue processors", () => {
1246412464

1246512465
const invocations = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.command_invocation'").first<{ n: number }>();
1246612466
expect(invocations?.n).toBe(1); // only ONE invocation recorded despite two processing passes
12467-
expect(seen.comments).toHaveLength(2); // both deliveries got the normal answer card — neither was held
12467+
expect(seen.comments).toHaveLength(1); // the replay is suppressed entirely — no second answer card
1246812468
expect(seen.comments.every((c) => !c.includes("rate limit"))).toBe(true);
12469+
const suppressed = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.command_redelivery_suppressed'").first<{ n: number }>();
12470+
expect(suppressed?.n).toBe(1);
1246912471
});
1247012472
});
1247112473

0 commit comments

Comments
 (0)