feat(#717): consolidate the worker into a single lifecycle owner - #722
Closed
zgeoff wants to merge 3 commits into
Closed
feat(#717): consolidate the worker into a single lifecycle owner#722zgeoff wants to merge 3 commits into
zgeoff wants to merge 3 commits into
Conversation
Three deletions of accidental complexity in the idle worker, each
paying for itself:
- continuations become durable start intents: a same-row race or a
transport failure holds a {avatarID, scope, startKey} record in the
preferences store, delivered idempotently at reconnect and resync
entry before the snapshot is fetched — the row it mints attaches
like any other active row. planResync loses its continue plan and
pending-continuation input, the continue apply path is deleted, and
a held continuation now survives worker restarts, which the
in-memory record never did. A player stop or a fresh player start
clears the held intent.
- lifecycle mailbox: start, resync, and continuation flows run
strictly one at a time on a shared chain; stops stay concurrent and
their epoch bump is what queued flows re-check. The resync
single-flight drop, the continuation's entry-state ownership
capture, and the per-flow chain plumbing collapse into one utility,
and a refused install under a landed stop stops the row back at the
one install boundary instead of per caller.
- never-null simulation: the worker holds a simulation from boot —
"no run" is an empty one — deleting the null guards, the lazy
creation in initialize and start, and the null-uninstall in the
continuation failure paths, which now swap in a fresh instance like
every other clear.
Closes #717
Findings from an adversarial review of the branch: - an expired or insufficient session holds the durable start intent instead of dropping it with a spurious fault — losing it there defeated the durability the intent exists for - a park whose durable write races a concurrent stop's clear is removed by a post-park epoch re-check, keeping the promise that a stopped activity is never resumed - the reconnect comment matches the activity-null guard's actual behavior - an externally raised install takes a lifecycle-chain slot like every other flow, and cross-kind chain serialization is covered by a test
|
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:
📝 WalkthroughWalkthroughThe worker replaces in-memory pending continuations with durable start intents, serializes lifecycle operations through one promise chain, keeps a non-null simulation, and updates resync and stop handling for continuation races and interrupted installations. ChangesWorker lifecycle ownership
Estimated code review effort: 4 (Complex) | ~60 minutes 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/game/idle-client/src/resync/plan-resync.test.ts (1)
30-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the duplicate capped-rebase test.
Lines 7-20 already use the same input and assert the same result, so this block adds no coverage.
🤖 Prompt for 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. In `@libs/game/idle-client/src/resync/plan-resync.test.ts` around lines 30 - 47, Remove the duplicate test case named “it rebases a capped activity” from plan-resync.test.ts, keeping the existing equivalent capped-rebase test and its coverage unchanged.
🤖 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.
Outside diff comments:
In `@libs/game/idle-client/src/resync/plan-resync.test.ts`:
- Around line 30-47: Remove the duplicate test case named “it rebases a capped
activity” from plan-resync.test.ts, keeping the existing equivalent
capped-rebase test and its coverage unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8d0ca734-08d1-4d1e-ab4d-f6d8dff9b51b
📒 Files selected for processing (35)
docs/architecture/game/game-simulation.mdlibs/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/submission/constants.tslibs/game/idle-client/src/submission/read-pending-start-intent.test.tslibs/game/idle-client/src/submission/read-pending-start-intent.tslibs/game/idle-client/src/submission/remove-pending-start-intent.test.tslibs/game/idle-client/src/submission/remove-pending-start-intent.tslibs/game/idle-client/src/submission/types.tslibs/game/idle-client/src/submission/write-pending-start-intent.test.tslibs/game/idle-client/src/submission/write-pending-start-intent.tslibs/game/idle-client/src/test-utils/create-stub-worker-context.test.tslibs/game/idle-client/src/test-utils/create-stub-worker-context.tslibs/game/idle-client/src/worker/create-worker-runtime.tslibs/game/idle-client/src/worker/flush-pending-start.test.tslibs/game/idle-client/src/worker/flush-pending-start.tslibs/game/idle-client/src/worker/handle-client-message.tslibs/game/idle-client/src/worker/handle-initialize-message.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.test.tslibs/game/idle-client/src/worker/handle-set-activity-message.tslibs/game/idle-client/src/worker/handle-simulation-update.tslibs/game/idle-client/src/worker/handle-start-activity-message.test.tslibs/game/idle-client/src/worker/handle-start-activity-message.tslibs/game/idle-client/src/worker/handle-stop-activity-message.test.tslibs/game/idle-client/src/worker/handle-stop-activity-message.tslibs/game/idle-client/src/worker/report-worker-fault.tslibs/game/idle-client/src/worker/run-continuation.test.tslibs/game/idle-client/src/worker/run-continuation.tslibs/game/idle-client/src/worker/run-on-lifecycle-chain.test.tslibs/game/idle-client/src/worker/run-on-lifecycle-chain.tslibs/game/idle-client/src/worker/types.ts
💤 Files with no reviewable changes (5)
- libs/game/idle-client/src/worker/handle-simulation-update.ts
- libs/game/idle-client/src/worker/handle-set-activity-message.ts
- libs/game/idle-client/src/resync/run-resync.ts
- libs/game/idle-client/src/resync/types.ts
- libs/game/idle-client/src/worker/handle-set-activity-message.test.ts
Owner
Author
|
CodeRabbit's one finding (duplicate capped-rebase test in plan-resync.test.ts) fixed in 8c15d78. |
Owner
Author
|
Closing in favor of the parallel implementation on |
4 tasks
This was referenced Jul 20, 2026
4 tasks
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 #717
Three deletions of accidental complexity in the idle worker: continuations become durable start intents, lifecycle flows share one serialized chain, and the simulation handle is never null. Each pays for itself — net code shrinks while a held continuation now survives worker restarts.
planResyncloses its continue kind; the continue apply path is deleted.Testing
bun run typecheckpassesbun run testpassesbun run lintpasses