Parent: #9492
Summary
#9312 added a webhook-redelivery guard to the @loopover command handlers:
const redeliverySinceIso = new Date(Date.now() - COMMAND_RATE_LIMIT_REDELIVERY_WINDOW_MS).toISOString();
if (await hasAuditEventForDelivery(env, req.actor, "<completed event type>", targetKey, deliveryId, redeliverySinceIso)) return true;
It exists because GitHub can redeliver the same issue_comment, and the job queue's max_retries: 3 plus the DLQ re-drive reuse the identical deliveryId. Two handlers were missed.
The ingress dedupe does not cover this vector: src/github/webhook.ts:188 suppresses a re-POST whose row is already processed, but a queue retry re-enters processGitHubWebhook directly, and recordWebhookEvent is an upsert written after the handler returns. There is no global delivery-level dedupe.
1. maybeProcessResolveCommand — duplicate permanent suppression rows
A redelivered @loopover resolve re-runs recordReviewSuppression (once per selected finding), writing a second permanent review-memory suppression row per finding, and re-posts its confirmation.
This is the same command family as the five #9312 already covers — it was missed because its five siblings sit 100–300 lines below it in a different formatting style (a single minified line vs. multi-line bodies), so a grep for the guard's shape does not surface it as a sixth site.
2. maybeProcessGateOverrideCommand — the override can retarget onto a newer commit
The write side is mostly idempotent (createOrUpdateOverriddenGateCheckRun PATCHes the existing run; the confirmation is an in-place marker comment; markGateOutcomeOverridden is a plain set overridden = true).
The real damage is target drift. resolveOverrideHeadSha deliberately re-fetches the live head at execution time — fetchLivePullRequestHeadSha, whose doc comment states it bypasses the cache to get "the literal current commit". If a commit lands between the original delivery and the replay, the replay neutralizes the Gate on a newer commit the maintainer never overrode, breaking the "no permanent bypass, this commit only" invariant the handler's own doc comment states. Duplicate github_app.gate_overridden audit and product-usage rows follow.
Explicitly NOT affected (verified, for the record)
maybeProcessConfigurationCommand — SAFE. Its only effect is createOrUpdateAgentCommandComment with a deterministic body (summarizeEffectiveConfig), so a replay produces a byte-identical body and the PATCH is skipped. Duplicate telemetry rows only.
maybeProcessPlanCommand — SAFE. isPlanCommandCoolingDown already short-circuits a replay into recordPlanSkip("cooldown_active"). Its key (actor + repo) is strictly broader than the guard's (actor + targetKey + deliveryId), and ISSUE_PLAN_COOLDOWN_MS is 10 * 60 * 1000 — identical to COMMAND_RATE_LIMIT_REDELIVERY_WINDOW_MS, so the coverage window matches exactly.
The PR-panel twins are tracked separately in the sibling issue, because their replay cost is a paid model call rather than a duplicate row.
Parent: #9492
Summary
#9312 added a webhook-redelivery guard to the
@loopovercommand handlers:It exists because GitHub can redeliver the same
issue_comment, and the job queue'smax_retries: 3plus the DLQ re-drive reuse the identicaldeliveryId. Two handlers were missed.The ingress dedupe does not cover this vector:
src/github/webhook.ts:188suppresses a re-POST whose row is alreadyprocessed, but a queue retry re-entersprocessGitHubWebhookdirectly, andrecordWebhookEventis an upsert written after the handler returns. There is no global delivery-level dedupe.1.
maybeProcessResolveCommand— duplicate permanent suppression rowsA redelivered
@loopover resolvere-runsrecordReviewSuppression(once per selected finding), writing a second permanent review-memory suppression row per finding, and re-posts its confirmation.This is the same command family as the five #9312 already covers — it was missed because its five siblings sit 100–300 lines below it in a different formatting style (a single minified line vs. multi-line bodies), so a grep for the guard's shape does not surface it as a sixth site.
2.
maybeProcessGateOverrideCommand— the override can retarget onto a newer commitThe write side is mostly idempotent (
createOrUpdateOverriddenGateCheckRunPATCHes the existing run; the confirmation is an in-place marker comment;markGateOutcomeOverriddenis a plainset overridden = true).The real damage is target drift.
resolveOverrideHeadShadeliberately re-fetches the live head at execution time —fetchLivePullRequestHeadSha, whose doc comment states it bypasses the cache to get "the literal current commit". If a commit lands between the original delivery and the replay, the replay neutralizes the Gate on a newer commit the maintainer never overrode, breaking the "no permanent bypass, this commit only" invariant the handler's own doc comment states. Duplicategithub_app.gate_overriddenaudit and product-usage rows follow.Explicitly NOT affected (verified, for the record)
maybeProcessConfigurationCommand— SAFE. Its only effect iscreateOrUpdateAgentCommandCommentwith a deterministic body (summarizeEffectiveConfig), so a replay produces a byte-identical body and the PATCH is skipped. Duplicate telemetry rows only.maybeProcessPlanCommand— SAFE.isPlanCommandCoolingDownalready short-circuits a replay intorecordPlanSkip("cooldown_active"). Its key (actor + repo) is strictly broader than the guard's (actor + targetKey + deliveryId), andISSUE_PLAN_COOLDOWN_MSis10 * 60 * 1000— identical toCOMMAND_RATE_LIMIT_REDELIVERY_WINDOW_MS, so the coverage window matches exactly.The PR-panel twins are tracked separately in the sibling issue, because their replay cost is a paid model call rather than a duplicate row.