fix: read-your-writes + global-scope idempotency correctness under the run-ops split#4284
Merged
Merged
Conversation
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (9)
📜 Recent review details⏰ Context from checks skipped due to timeout. (24)
WalkthroughThe changes add primary fallbacks for replica-miss run, batch, waitpoint, and session lookups across API, realtime, resource, presenter, routing, and execution paths. Batch routes also mark not-found responses as retryable. Run-store routing now uses owning-store reads for classifiable runs and colocated dedup probes. Split-mode global idempotency handling adds claim TTL calculation, compare-and-delete reset, cross-database winner resolution, bounded cleared-winner reacquisition, and explicit claim publish outcomes. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…bal-scope idempotency correctness under the run-ops split Production code only — the guarding tests are in the stacked PR. 1) Read-your-writes: run-store reads that gate a mutation or feed a public GET / realtime response were routed to a lagging read replica, so a just-written run/waitpoint/batch could spuriously miss under replica lag. Route those reads to the owning primary (findRun/findWaitpoint/findBatchTaskRunByFriendlyId -> *OnPrimary, a primary re-read on a miss, or a retryable 404 where the SDK polls). Additive: the happy path is unchanged; a primary read happens only on a miss. 2) Global-scope idempotency across the split: a global-scope key carries no per-run salt, so the same (env, task, key) triggered concurrently from parents resident on different run-ops DBs could dedup-miss on each DB and create a duplicate run (the per-DB unique index can't enforce cross-DB uniqueness). Serialize such triggers (global scope, or scope-absent, while split is active) through the existing Redis idempotency claim, resolve the winner by id across both DBs, and reacquire the claim on the expired/failed clear-and-recreate path. run/attempt scope embed the run id in their hash and never contend.
d-cs
force-pushed
the
fix/wait-until-idempotency-retry-rewait
branch
from
July 18, 2026 16:26
bf4ee4f to
e4c9b5c
Compare
d-cs
changed the title
Dead since #4272 inlined per-write residency routing at each call site — the private helper had zero callers (only doc-comments referenced it by name), tripping eslint no-unused-private-class-members. Remove it and reword the comments that referenced it.
d-cs
marked this pull request as ready for review
July 18, 2026 16:37
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Reviewed the read-your-writes routing and the global-scope idempotency claim, plus a completeness sweep of run/waitpoint/batch/snapshot read-after-write paths across the engine and webapp. Core routing looks correct and complete — no read that must observe its own write still reads the replica, and the readOnlyPrisma → prisma change in runAttemptSystem is a genuine correctness fix. Four inline notes: two edges worth tightening (both re-open the duplicate/stale-read class this PR closes) and two minors. None a blocker.
(Note: the third inline note is attached as a file-level comment on internal-packages/run-store/src/runOpsStore.ts because its target — the findRunOnPrimary() classifiable-id path around lines 811-821 — is unchanged code outside the PR's diff hunks.)
ericallam
approved these changes
Jul 19, 2026
d-cs
force-pushed
the
fix/wait-until-idempotency-retry-rewait
branch
from
July 19, 2026 14:06
1c6e6ff to
2941d71
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
d-cs
force-pushed
the
fix/wait-until-idempotency-retry-rewait
branch
from
July 19, 2026 14:13
2941d71 to
6ae9b8a
Compare
…me batch hardening Production follow-ups from the CodeRabbit/Claude/Devin review. Caller-driven guard tests are in the stacked tests PR; the two test edits here are coupled to the production change (the publishClaim signature change invalidates a main assertion; the fan-out removal obsoletes a main test). - Claim TTL floor (TRIGGER_MOLLIFIER_CLAIM_MIN_TTL_SECONDS, default 5) independent of the customer key TTL, so a short key TTL can't expire the claim mid-pipeline and let a loser re-claim. - publishClaim returns the buffer CAS result; the trigger success path detects + logs a no-op'd publish. - reacquireClearedGlobalWinner fails closed with a retryable 503 (exhaustion / unfindable winner); the expired/failed clear-and-recreate path routes through it too (Devin), so that recreate is serialised. - Realtime batch route re-reads the owning primary on a replica miss (backend-agnostic; closes the Electric ShapeStream permanent-404 for self-hosters). - Remove the classifiable-id cross-store fan-out from findRun: a run's id-shape fixes its residency for life, so the single-store read is correct and the fan-out was dead code.
d-cs
force-pushed
the
fix/wait-until-idempotency-retry-rewait
branch
from
July 19, 2026 15:10
568b3bf to
1c625e1
Compare
d-cs
added a commit
that referenced
this pull request
Jul 19, 2026
… (#4285) ## Stacked on #4284 — tests only This PR contains **only the tests** that guard the production fixes in #4284 (its base). Review #4284 first; this branch adds no production code. ## What Caller-driven replica-lag and idempotency guards for every fixed site: - Each guard **drives the real exported caller** (route loader/action, presenter `.call()`, service, or engine method) against a **real Postgres** with the owning replica frozen via the shared `laggingReplica` testcontainer primitive — never a store-seam reimplementation. - For a **fixed** site the guard goes **RED when the production change is reverted**; for a **tolerated read-view** site it's a caller-driven **GREEN** proof the miss self-heals (returns null/empty, no mutation, row live on primary). - The **global-scope idempotency** guard drives the real dedup + claim path through a **real `MollifierBuffer` over a Redis testcontainer** (real SETNX/poll/publish), and covers the cross-DB **andWait** waitpoint wiring and the **expired/failed clear-and-recreate** reacquire cases. Run with `vitest --no-file-parallelism` (testcontainers). Verified GREEN, and revert→RED verified per fixed site.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Two related correctness fixes for the run-ops DB split. Under the split, run-store reads can route to a lagging read replica; a just-written run/waitpoint/batch can then be missed, causing a wrong decision.
1. Read-your-writes → owning primary. Surfaced first as an intermittent
wait.until({ idempotencyKey })re-wait on retry. Auditing the run-store read surface found the same class at sibling sites (some gating mutations or returning spurious 404s, others tolerable/self-healing). Reads that must observe their own writes now route to the owning primary (findRun/findWaitpoint/findBatchTaskRunByFriendlyId→*OnPrimary, a primary re-read on a miss, or a retryable 404 where the SDK polls). Read-view reads stay on the replica. All additive — the happy path is unchanged.2. Global-scope idempotency across the split. A
global-scope key carries no per-run salt, so the same(env, task, key)triggered concurrently from parents resident on different run-ops DBs could dedup-miss on each DB and create a duplicate (the per-DB unique index can't enforce cross-DB uniqueness). Such triggers (global scope, or scope-absent, while split is active) are serialized through the existing Redis idempotency claim, the loser resolves the winner by id across both DBs, and the claim is reacquired on the expired/failed clear-and-recreate path.run/attemptscope embed the run id and never contend.Stacked for review
This is the base of a 2-PR stack, split so review is easier:
Validation
Local run-ops split, both 2-DB and 3-DB, fresh boot on this branch: SDK canary 64/71 (only the known concurrency/input-streams/s3 failures), quarantine sweep 0 unexpected (340 pass / 16 known / 4 local) in each topology, dashboard e2e 0 failed. No product regressions.