Parent: #9492
Summary
Eleven functions in src/queue/processors.ts share the identical signature:
(env: Env, deliveryId: string, payload: GitHubWebhookPayload): Promise<boolean>
Each is a webhook-owned entry point, and each must decide whether it needs #9312's redelivery guard. Three of them were found missing it (#9561, #9562) — one writing duplicate permanent suppression rows, two spending a second paid model call.
Nothing prevents a twelfth handler from repeating this. The invariant is currently maintained by "whoever adds one remembers to grep for the guard", which is exactly the failure mode that produced all three misses.
Why a grep does not catch it
The eleven handlers are written in two different formatting styles — some declare the signature on one line, some across five:
async function maybeProcessResolveCommand(env: Env, deliveryId: string, payload: GitHubWebhookPayload): Promise<boolean> { ...async function maybeProcessGateOverrideCommand(
env: Env,
deliveryId: string,
payload: GitHubWebhookPayload,
): Promise<boolean> {A grep for either shape silently misses the other half of the family. That formatting split is the mechanical cause of the drift, not carelessness — the same "a guard is added to the instances someone greps for" observation #9541 opens with.
Proposal
A scripts/check-command-redelivery-guards.ts in the established idiom of check-dead-source-files.ts and check-regate-sort-key.ts:
- normalize whitespace so the signature matches regardless of formatting, and find every handler in the family
- require each to either call
hasAuditEventForDelivery, or be listed in an allowlist with a stated reason
- injectable
readFile seam so the checker is directly testable without touching the tree
- bound each handler's body by brace depth, not a fixed line window (a fixed window produced a real false negative while
check-regate-sort-key.ts was being written: one producer's field satisfied the scan for a neighbour's omission)
The allowlist carries the two verified-safe handlers and the mechanism that makes each safe, so "this one is fine" is a stated claim that a reviewer can check rather than an absence someone has to re-derive:
maybeProcessConfigurationCommand — idempotent createOrUpdateAgentCommandComment with a deterministic bodymaybeProcessPlanCommand — isPlanCommandCoolingDown, whose window is identical to COMMAND_RATE_LIMIT_REDELIVERY_WINDOW_MS and whose key is strictly broader
This is the "an exception must be stated, not inferred from absence" shape both existing checkers already use.
Parent: #9492
Summary
Eleven functions in
src/queue/processors.tsshare the identical signature:Each is a webhook-owned entry point, and each must decide whether it needs #9312's redelivery guard. Three of them were found missing it (#9561, #9562) — one writing duplicate permanent suppression rows, two spending a second paid model call.
Nothing prevents a twelfth handler from repeating this. The invariant is currently maintained by "whoever adds one remembers to grep for the guard", which is exactly the failure mode that produced all three misses.
Why a grep does not catch it
The eleven handlers are written in two different formatting styles — some declare the signature on one line, some across five:
A grep for either shape silently misses the other half of the family. That formatting split is the mechanical cause of the drift, not carelessness — the same "a guard is added to the instances someone greps for" observation #9541 opens with.
Proposal
A
scripts/check-command-redelivery-guards.tsin the established idiom ofcheck-dead-source-files.tsandcheck-regate-sort-key.ts:hasAuditEventForDelivery, or be listed in an allowlist with a stated reasonreadFileseam so the checker is directly testable without touching the treecheck-regate-sort-key.tswas being written: one producer's field satisfied the scan for a neighbour's omission)The allowlist carries the two verified-safe handlers and the mechanism that makes each safe, so "this one is fine" is a stated claim that a reviewer can check rather than an absence someone has to re-derive:
maybeProcessConfigurationCommand— idempotentcreateOrUpdateAgentCommandCommentwith a deterministic bodymaybeProcessPlanCommand—isPlanCommandCoolingDown, whose window is identical toCOMMAND_RATE_LIMIT_REDELIVERY_WINDOW_MSand whose key is strictly broaderThis is the "an exception must be stated, not inferred from absence" shape both existing checkers already use.