test: caller-driven replica-lag + idempotency guards (stacked on #4284)#4285
Merged
Conversation
…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.
|
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 (62)
🚧 Files skipped from review as they are similar to previous changes (55)
📜 Recent review details⏰ Context from checks skipped due to timeout. (17)
🧰 Additional context used📓 Path-based instructions (9)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{test,spec}.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.ts📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/**/*.test.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/**/*.{test,spec}.{ts,tsx}📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
🧠 Learnings (18)📚 Learning: 2026-03-22T13:26:12.060ZApplied to files:
📚 Learning: 2026-03-22T19:24:14.403ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-06-13T19:53:13.759ZApplied to files:
📚 Learning: 2026-06-17T17:13:49.929ZApplied to files:
📚 Learning: 2026-06-23T13:04:21.413ZApplied to files:
📚 Learning: 2026-05-07T12:25:18.271ZApplied to files:
📚 Learning: 2026-05-28T20:02:10.647ZApplied to files:
📚 Learning: 2026-05-12T21:04:05.815ZApplied to files:
📚 Learning: 2026-06-25T18:21:51.905ZApplied to files:
📚 Learning: 2026-07-03T17:10:21.498ZApplied to files:
📚 Learning: 2026-05-18T14:40:02.173ZApplied to files:
📚 Learning: 2026-06-04T18:16:35.386ZApplied to files:
📚 Learning: 2026-06-09T17:58:04.699ZApplied to files:
📚 Learning: 2026-06-16T09:19:47.637ZApplied to files:
📚 Learning: 2026-07-18T14:01:28.319ZApplied to files:
📚 Learning: 2026-07-18T13:08:51.075ZApplied to files:
🔇 Additional comments (8)
WalkthroughAdds extensive Vitest integration coverage for replica-lag behavior across webapp routes, presenters, realtime services, run-engine operations, and run-store views. Tests simulate missing or stale replica rows, verify primary read fallbacks and primary-only mutation reads, validate tolerated null/empty responses, and cover cross-database global idempotency and waitpoint ownership behavior. 🚥 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 |
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
force-pushed
the
fix/wait-until-idempotency-retry-rewait-tests
branch
from
July 18, 2026 16:35
b9faca3 to
69fe2db
Compare
d-cs
marked this pull request as ready for review
July 18, 2026 16:39
d-cs
force-pushed
the
fix/wait-until-idempotency-retry-rewait-tests
branch
2 times, most recently
from
July 18, 2026 17:17
e7b4143 to
1abf430
Compare
![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.
d-cs
force-pushed
the
fix/wait-until-idempotency-retry-rewait-tests
branch
3 times, most recently
from
July 18, 2026 18:32
c0b053f to
eac4192
Compare
Collaborator
Author
|
Follow-up on the shared- Rather than leave it for a later PR, I'm sweeping the remaining replica-lag test files in this PR that still carry a local lag-proxy onto the shared Scope: ~18 remaining files across Exceptions that will stay on a local proxy (with an inline note), because the shared primitive intercepts only configured Prisma model reads and can't express them:
Each migrated file is verified green individually; pushing as an amend to the tests commit shortly. |
d-cs
force-pushed
the
fix/wait-until-idempotency-retry-rewait-tests
branch
from
July 18, 2026 18:46
eac4192 to
47a6984
Compare
Collaborator
Author
|
Shared- Migrated onto the shared
Each now calls Kept on a local proxy — the shared primitive can't express these (with reason):
No change needed: |
d-cs
force-pushed
the
fix/wait-until-idempotency-retry-rewait-tests
branch
2 times, most recently
from
July 18, 2026 19:16
1a6af28 to
7102b75
Compare
Collaborator
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
d-cs
force-pushed
the
fix/wait-until-idempotency-retry-rewait-tests
branch
from
July 18, 2026 19:52
7102b75 to
551354a
Compare
Contributor
|
The suite genuinely guards the regression — caller-driven RED-on-revert across every fix category, and the global-idempotency race is exercised for real over Redis + a two-DB store with a live
|
ericallam
approved these changes
Jul 19, 2026
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
…tency guards Guards for the production fixes in the base PR. Each fixed site has a caller-driven test that drives the real exported route/presenter/service/engine caller against a real Postgres with the owning replica frozen (shared laggingReplica primitive), and goes RED when the fix is reverted; tolerated read-view sites carry a caller-driven GREEN proof of self-healing. The global-scope idempotency guard drives the real dedup + claim path through a real MollifierBuffer over a Redis testcontainer, incl. the cross-DB andWait and the expired/failed clear-and-recreate reacquire cases.
…w fixes Caller-driven / unit guards for the production hardening in the base PR: claim-TTL floor, publishClaim CAS result, reacquire fail-closed 503, expired/failed global-scope recreate re-serialisation, and the realtime batch primary re-read. Split out of the base PR so the production change stays reviewable on its own.
d-cs
force-pushed
the
fix/wait-until-idempotency-retry-rewait-tests
branch
from
July 19, 2026 15:11
551354a to
d1f1a85
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
- cancelRun / idempotencyReset NEW-ksuid cases: use a run-ops-shaped friendlyId so the by-friendlyId read routes to NEW single-store (the fan-out that masked the fake LEGACY-classifying friendlyId is gone; a real NEW run's friendlyId classifies NEW). - routesBatchGet realtime.v1.batches case: assert the loader now recovers a stale-on-replica batch via the owning-primary re-read (reaches streamBatch, 200) instead of a resource-gate 404.
d-cs
added a commit
that referenced
this pull request
Jul 19, 2026
…e run-ops split (#4284) ## 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`/`attempt` scope embed the run id and never contend. ## Stacked for review This is the **base** of a 2-PR stack, split so review is easier: - **This PR** — production code only (34 files). - **Stacked tests PR → #4285 — the caller-driven guards (55 test files) on top of this branch. ## 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.
d-cs
enabled auto-merge (squash)
July 19, 2026 16:58
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.
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:
.call(), service, or engine method) against a real Postgres with the owning replica frozen via the sharedlaggingReplicatestcontainer primitive — never a store-seam reimplementation.MollifierBufferover 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.