Skip to content

queue: the two PR-panel handlers replay a paid model call on webhook redelivery (no #9312 guard) #9562

Description

@JSONbored

Parent: #9492

Summary

The two PR-panel checkbox handlers have no webhook-redelivery guard, and unlike the text-command handlers in the sibling issue, their replay cost is a second paid model call, not a duplicate row.

Both fire on payload.action === "edited" and read the payload snapshot (isCheckedPrPanelRetrigger(comment.body) / isCheckedPrPanelGenerateTests(comment.body)) — a plain regex over the body string in the delivered payload. So the usual "the panel rewrote its own checkbox, therefore a replay no longer matches" argument does not apply: a redelivered payload still carries the checked body regardless of what the live panel looks like now.

1. maybeProcessPrPanelRetrigger — a guaranteed duplicate paid LLM review

A replay re-runs refreshPullRequestDetails(..., { force: true }), a full gate evaluation, and maybePublishPrPublicSurface with forceAiReview: true. That flag defeats both mechanisms that would otherwise collapse the duplicate:

  • const cachedReview = webhook.forceAiReview === true ? null : (await getCachedAiReview(...)) — the AI-review cache and the cross-head fingerprint cache are bypassed entirely.
  • claimAiReviewLock(..., { steal: webhook.forceAiReview === true }) — the lock is deliberately stolen, with the comment "a duplicate LLM call is the explicitly accepted cost".

That trade-off is correct for a maintainer deliberately re-ticking the box. It is not correct for a queue retry the maintainer never asked for. Because LLM output is non-deterministic, the second verdict can also overwrite the first in the published surface — a user-visible verdict flip.

startActiveReviewTracking is tracking-only and short-circuits nothing.

2. maybeProcessPrPanelGenerateTests — duplicate generation and a duplicate public comment

A replay re-enters runE2eTestGenerationAndDeliver, which runs runLoopOverE2eTestGeneration (a second real AI generation, BYOK or shared budget) and then createIssueComment — a brand-new public comment every time (the fallback path posts too).

commitE2eTestToPrBranch is partially self-limiting in commit delivery mode: src/github/e2e-test-commit.ts declines when liveHead.sha !== args.headSha, which the first commit itself made true. So the duplicate push is usually declined — but the duplicate generation and the duplicate comment always happen.

The existing hasAuditEventForHeadSha guard does not help: it is explicitly scoped to the auto-trigger, with the comment "The explicit command deliberately does NOT consult this guard."

The fix is a drop-in for at least one of them

maybeProcessPrPanelGenerateTests's text-command twin, maybeProcessGenerateTestsCommand, already guards the identical event type and targetKey:

hasAuditEventForDelivery(env, req.actor, "github_app.e2e_tests_generation", targetKey, deliveryId, redeliverySinceIso)

and both paths write that same audit row through the same runE2eTestGenerationAndDeliver. The panel twin simply never got the guard — the same "two panel twins drifting inside one file" family #9541 describes.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.orbGittensory Orb related - maintainer self-hosting analytics.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions