Skip to content

fix(fetch): stop repository conflicts from failing whole PR batches - #62

Merged
ewhauser merged 2 commits into
mainfrom
coordinator-repo-conflict-livelock
Aug 7, 2026
Merged

fix(fetch): stop repository conflicts from failing whole PR batches#62
ewhauser merged 2 commits into
mainfrom
coordinator-repo-conflict-livelock

Conversation

@ewhauser

@ewhauser ewhauser commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes #60.

A same-repository PR fanout could livelock: concurrent batches raced the incidental parent-repository apply, the losing batch's ErrObservationSuperseded failed every prepared PR from that repository, and retrySupersededObservation's immediate retries kept competing workers phase-locked while River re-ran the larger jobs.

Approach

The key observation is that UpsertRepositoryWriteIfNewer already keeps the repository row monotonic by GitHub's own updated_at clock, so a superseded observation snapshot only proves local interleaving — never that the fetched repository representation is stale. That makes a conflict on the incidental parent write recoverable without refetching:

  • The coordinator's repository apply now retries in place against a fresh observation snapshot (no remote refetch). Each retry absorbs one competing commit, so a batch racing N-1 concurrent same-repository batches converges without ever failing an item.
  • Only sustained churn that survives every in-place retry escalates to the old behavior — failing the depending items so the handler refetches parent and PR together. That escalation is load-bearing: the incidental apply must eventually land (or refetch), otherwise a later reconcile-source refresh could claim domain changes and provenance that the webhook observation already carried (caught by TestOrderIndependenceFinalCacheState in CI on an earlier revision of this branch that skipped the apply entirely).
  • The refresh-generation fence path (ErrRefreshGenerationSuperseded) is unchanged.
  • Batch items that explicitly skipped the parent observation (branch-page reconciliation) are no longer failed by a repository-level error they never depended on.
  • retrySupersededObservation now applies full jitter between attempts so competing same-repository workers desynchronize instead of re-conflicting in lockstep.

Acceptance criteria from #60

  • A repository observation conflict no longer fails the losing batch's PR observations (TestCoordinatorRepositoryConflictDoesNotFailBatchPRs, asserts zero extra GraphQL fetches and both PR writes landing).
  • Concurrent same-repository fanout converges with exactly one authoritative fetch per worker and no retry stampede (TestCoordinatorSameRepositoryFanoutConvergesWithoutRefetch).
  • Repository metadata cannot regress past the competing writer (asserted via the monotonic upsert).

Both tests reproduce the reported failure (apply repository: observation superseded + refetch amplification) when run against the previous coordinator behavior.

Testing

  • go test ./internal/fetch/ (full package, local Postgres), plus 5× stress of the new tests together with TestOrderIndependenceFinalCacheState
  • Full suite go test ./... except cmd/loadgen, matching CI's split
  • make lint clean

🤖 Generated with Claude Code

ewhauser and others added 2 commits August 7, 2026 11:52
A same-repository fanout could livelock: when concurrent batches raced the
incidental parent-repository apply, the losing batch's observation conflict
failed every prepared PR from that repository, and the handler's immediate
retries kept competing workers phase-locked while River re-ran the larger
jobs (issue #60).

The repository upsert is already monotonic by GitHub's update clock, so a
superseded snapshot only proves interleaving, never staleness. The
coordinator now retries the incidental repository apply against a fresh
observation without refetching, treats an exhausted conflict as non-fatal
for the independently fenced PR observations, and no longer fails
branch-page items that skipped the parent observation. The handler-level
superseded retry gains full jitter so competing refetches desynchronize.

Fixes #60

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Silently skipping the incidental parent apply after exhausted snapshot
retries could drop a webhook observation's repository domain changes,
letting a later reconcile-source refresh claim them (and their provenance)
— observed as an order-independence divergence in CI. In-place retries
still absorb each competing commit without refetching, so the issue #60
livelock stays fixed; only sustained churn now escalates to the handler's
jittered refetch, which guarantees the parent representation eventually
lands with its original provenance.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ewhauser
ewhauser merged commit 545fe4a into main Aug 7, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Same-repository PR batches can livelock on optimistic repository observation conflicts

1 participant