Skip to content

orb(review): public-surface publish runs outside the per-PR mutex — duplicate gate check-runs and placeholder overwriting a real verdict #9013

Description

@JSONbored

Two passes can publish the same PR concurrently — duplicate gate check-runs, verdict thrash

maybePublishPrPublicSurface runs entirely outside the per-PR actuation mutex. Both entry points (sweep src/queue/processors.ts ~3762, webhook ~6644) call it unlocked; only its AI-review sub-section takes ai-review-lock (~10119-10141) and only the later maybeRunAgentMaintenance takes pr-actuation-lock (~2447). The file's own comment (~2439-2447) acknowledges concurrent same-PR passes are reachable.

Observed live on PR #8965 (2026-07-26): two github_app.pr_public_surface_published events 13 seconds apart (14:55:11 and 14:55:24) with a type_label_decision | denied | lock_contended between them — i.e. pass B lost the type-label lock, logged it, and continued publishing anyway (~8985: logTypeLabelSkip(...) then falls through).

Two concrete harms:

  1. Duplicate gate check-runs. createOrUpdatePendingGateCheckRun (~9138) → createOrUpdateNamedCheckRun with no checkRunId does GET-latest-by-name-and-SHA then POST-if-absent (src/github/app.ts ~1057-1080). On the first publish for a head, both passes GET (find nothing) and both POST → two check-runs named "LoopOver Gate" on one commit with divergent conclusions. Unlike panel comments — which self-heal via deleteDuplicateMarkerComments (src/github/comments.ts ~177-195) — there is no dedup for check-runs; later passes PATCH only filter:latest, so the stale duplicate persists forever and can hold the PR under strict branch protection.

  2. A lock-loser overwrites a real verdict with a placeholder. The AI-review lock loser takes aiReviewLockContendedResult (~10126-10127) and keeps going to publish a full surface carrying the ai_review_inconclusive hold. Because the bodies differ, the byte-identical no-op guard (comments.ts ~143-147) doesn't absorb it: whichever pass PATCHes last owns the panel. If the loser lands second, a genuine verdict is replaced by "AI review already in progress". Both passes also record pr_public_surface_published, double-counting public stats.

Fix

  1. Hold one pass-level per-PR mutex across publish and disposition (they should be one unit — this also closes the orb(selfhost): deploy restarts kill passes between surface-publish and disposition; repair sweep can't see the orphan #8997 crash gap), with the loser deferring rather than proceeding.
  2. Promote the type-label contention from skip-and-continue to a deferral of the whole pass.
  3. A contended AI-review pass must not publish a placeholder over an existing same-head surface.
  4. Thread a durably-stored gate check-run id (and add duplicate-check-run cleanup mirroring the comment dedup).

Acceptance

  • Two concurrent passes on one PR produce exactly one gate check-run and one panel comment.
  • A lock-losing pass never overwrites a published verdict.

Refs #8997, #9008.

Metadata

Metadata

Assignees

Labels

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

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions