Skip to content

fix(server): a retried pull-request-state delivery never fails with 500 - #225

Merged
tusharbhardwaj-bk merged 2 commits into
expbkmainfrom
t3code/perf-12-pr-state-retry
Sep 27, 2026
Merged

tusharbhardwaj-bk merged 2 commits into
expbkmainfrom
t3code/perf-12-pr-state-retry

Conversation

@tusharbhardwaj-bk

Copy link
Copy Markdown
Collaborator

Problem

A stress run on expbkt3 (10 concurrent pull-request-state writers against 5 threads on one branch) got HTTP 500 on 27 of 160 writes:

pull request state write failed
  _tag: 'OrchestrationCommandPreviouslyRejectedError'
  commandId: 'bridge:pr-state:stress-1-0:perf-stress-thread-…:branch'
  detail: 'Orchestration command invariant failed (thread.pull-request.sync): thread … changed before pull request discovery'

The endpoint (#217) treats a thread that changed between its read and the dispatch as stale. But a retry of the same delivery reuses the same command id (bridge:pr-state:<deliveryId>:<threadId>:…), and the engine answers a command it already rejected with OrchestrationCommandPreviouslyRejectedError. That surfaced as a 500. The bridge retries failed deliveries, so it would have retried that delivery forever. The thread.pull-request-link.sync dispatch had no race handling at all.

Fix

Both dispatches now report either OrchestrationCommandInvariantError or OrchestrationCommandPreviouslyRejectedError as ignored: stale with HTTP 200. That matches the contract: "a write that lost to the thread's own changes is ignored".

Evidence

  • New test treats a retried delivery whose update was rejected as stale, not a failure: it pre-records a rejected command under the retry's command id, then applies the delivery. It fails without the fix and passes with it. The suite passes (7).
  • vp run typecheck in apps/server and lint are clean.

After it deploys I'll re-run the stress test on expbkt3 and expect 0 errors.

Model/harness: Claude Opus 5.5 via Claude Code in T3 Code.

🤖 Generated with Claude Code

The endpoint treated a thread that changed between read and dispatch as
"stale", but a retry of the same delivery reuses the command id, and the
engine answers a command it already rejected with
OrchestrationCommandPreviouslyRejectedError. That surfaced as a 500, so the
bridge would retry the delivery forever. The link-sync path had no race
handling at all.

Both dispatches now report either rejection as "stale". Found by a stress
run on expbkt3 (27 of 160 concurrent writes returned 500).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S labels Sep 26, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added size:M and removed size:S labels Sep 26, 2026
@github-actions

Copy link
Copy Markdown

Thread transfer impact

⚠️ The latest CI run did not produce a thread transfer result for ee20f2a.

This comment will update automatically after the next completed run.

@tusharbhardwaj-bk
tusharbhardwaj-bk merged commit 84180b2 into expbkmain Sep 27, 2026
12 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants