fix(#582): resume the continuation once a held terminal append drains - #661
Merged
Conversation
runContinuation loses the "wanted to start next" intent on a same-row CONFLICT or a transport failure, stranding play once the terminal append it was racing eventually drains. Record it on the worker context instead, clearing it once a player picks a new activity, and extend the reconnect trigger to resync for a pending continuation as well as a remembered resyncAvatarID.
A non-active row matching a pending continuation's target row plans 'continue' instead of 'none', so a resync can start the row the worker wanted once the terminal append that closed it is acknowledged. A capped row still rebases, and a pending naming a different row is stale and ignored.
applyContinue starts the row a pending continuation wanted once its target reads closed: a budget already spent halts at the boundary, a fresh never-appended CONFLICT row is adopted directly, any other defined error clears the pending record and reports through the resync failure status, and a transport failure keeps it for the next reconnect. Cover both of the issue's repro sequences end to end against the mocked stateful backend.
Sorts the merged ORPCError/isDefinedError import and teaches pickLatestActivityID about the continue plan's activity-shaped context, both surfaced by rebasing onto main's independent failure-action and checkpoint-sweep work.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesPending continuation recovery
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/game/idle-client/src/worker/create-worker-runtime.ts`:
- Around line 258-261: Update the avatarID selection in the reconnect flow
around handleRequestResyncMessage to prefer pendingContinuation?.avatarID over
resyncAvatarID, while retaining null as the fallback. Add coverage for the case
where resyncAvatarID is avatar A and the pending continuation is avatar B,
asserting that avatar B is used for resynchronization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 35dbb4c1-ec1f-422f-8e74-440792ddf9bd
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock,!bun.lock
📒 Files selected for processing (15)
docs/architecture/game-simulation.mdlibs/game/idle-client/package.jsonlibs/game/idle-client/src/resync/plan-resync.test.tslibs/game/idle-client/src/resync/plan-resync.tslibs/game/idle-client/src/resync/run-resync.tslibs/game/idle-client/src/resync/types.tslibs/game/idle-client/src/test-utils/create-stub-worker-context.tslibs/game/idle-client/src/worker/create-worker-runtime.test.tslibs/game/idle-client/src/worker/create-worker-runtime.tslibs/game/idle-client/src/worker/handle-request-resync-message.test.tslibs/game/idle-client/src/worker/handle-request-resync-message.tslibs/game/idle-client/src/worker/handle-set-activity-message.tslibs/game/idle-client/src/worker/run-continuation.test.tslibs/game/idle-client/src/worker/run-continuation.tslibs/game/idle-client/src/worker/types.ts
The pending-continuation record no longer snapshots the dying simulation's failure action: the worker context now holds the durable, reconciled value, so a preference changed while the intent waited still applies, and the fragile read-after-stop ordering disappears. Also covers the continue apply's CONFLICT-adopt branch with a test.
zgeoff
changed the title
The remembered resync avatar can predate a pending continuation recorded for another avatar; picking it first would strand the continuation on every reconnect.
The worker runtime takes a now option beside timestep and client, so the fast-clock test stand-in drives the tick loop by injection instead of patching the global performance.now — the restore-ordering footgun goes with it. The fail-first transport handler moves to test-utils per the stand-in convention, and the authed-client helper dissolves into the per-test inline sequence its sibling suites already use.
This was referenced Jul 20, 2026
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
1 participant
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.
Description
Closes #582
A continuation that couldn't start after a terminal checkpoint (a same-row race, or a transport
failure on the follow-up
startActivity) now leaves a pending record instead of dead-ending theavatar; the next resync consults it and starts the row once it reads closed.
WorkerContextgains aPendingContinuationrecord;runContinuationsets it on a same-rowCONFLICTor a transport failure, and clears it when the player picks a new activity.continueResyncPlankind targets a non-active row matching the pending continuation(a stale pending naming a different row is ignored, and a cap still wins); reconnect now also
resyncs when a pending continuation exists, not only a remembered
resyncAvatarID.continue: budget gate mirrors halt-at-boundary, adopts a freshCONFLICTrow likerunContinuationitself, and clears/reports through the existingresync-failure and offline paths on other outcomes.
docs/architecture/game-simulation.md's offline-progress section.Testing
bun run typecheckpassesbun run testpassesbun run lintpasses