refactor(#730): restate the checkpoint submitter as an xstate statechart - #743
Merged
Conversation
Replace the per-activity mutable flag records, hand-scheduled flush timers, and p-retry backoff loop with declared xstate v5 machines: a parent spawning one child actor per activity, a child owning the idle/scheduled/flushing/retrying/invalid/evicted states, and a thin adapter preserving the exact CheckpointSubmitter interface so the existing behavioral suite runs unmodified. - the flush attempt is one invoked fromPromise internalizing the oRPC call, queue/cursor settlement, and callbacks, resolving a discriminated outcome the transitions route on - backoff parity with the p-retry config (factor 2, 10s..300s, no jitter, first retry waits a full window); retryTimings parameterizes the delay function via machine input - scheduled waits on an external flush-due event so the injected scheduleFlush test closures keep their awaitable, superseded-safe semantics - the CHECKPOINT_INVALID tombstone is a retained ordinary state; the parent's evictedActivityIDs answers isEvicted after a child stops - buildMachineTypes<T>() owns the single phantom-type assertion xstate's setup() requires, keeping machine definitions cast-free - allow xstate's AnyActorRef in the readonly-parameter-types allow list (live actor handle, no readonly form) Known accepted edge: flushNow during an in-flight registration seed is a no-op where it previously flushed on the partially-seeded cursor. Closes #730 Claude-Session: https://claude.ai/code/session_01CmZyk7wqzP8KtwQBVZg4ha
- reset the backoff attempt counter on success and conflict outcomes so a later outage starts at the base window, with a regression test - carry the settled appended head into the callback-failed outcome so a throwing ack callback no longer strands a stale expected head - fire onHeld on the fold-in branches that re-flush without passing through retrying, matching the per-held-attempt cadence - rename listenForShutdownAbort/makeScheduleProgressFlush/findChild to taxonomy verbs; reword a comment that named other declarations Claude-Session: https://claude.ai/code/session_01CmZyk7wqzP8KtwQBVZg4ha
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
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 checkpoint submitter is refactored from local imperative state into XState parent and per-activity child machines. The adapter forwards queue, flush, retry, terminal, abort, and eviction events, with comprehensive tests covering state transitions and failure handling. ChangesCheckpoint submission statecharts
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🤖 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/submission/checkpoint-activity-machine.ts`:
- Around line 273-281: Update emitSettlementToParent to avoid accessing the
internal args.self._parent handle; use XState’s supported sendParent mechanism
or pass an explicit parentRef and send through sendTo, while preserving the
existing CHILD_SETTLED payload and settlement behavior.
In `@libs/game/idle-client/src/submission/create-checkpoint-submitter.ts`:
- Around line 164-169: Update the WriteCursor interface to add one inline
readonly-exception directive explaining that nextVersion, prevHash, and
previousNextSeed are intentionally mutated in submit; keep startChainIndex
readonly and avoid adding separate directives for each field.
🪄 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: ccb20374-3554-4f27-a28d-bd5fd92d7c38
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock,!bun.lock
📒 Files selected for processing (8)
.oxlintrc.jsonlibs/game/idle-client/package.jsonlibs/game/idle-client/src/submission/build-machine-types.tslibs/game/idle-client/src/submission/checkpoint-activity-machine.test.tslibs/game/idle-client/src/submission/checkpoint-activity-machine.tslibs/game/idle-client/src/submission/checkpoint-submitter-machine.tslibs/game/idle-client/src/submission/create-checkpoint-submitter.tspackage.json
Replace the child's private _parent access with the documented pattern: the submitter machine hands its own ref to each spawned child via input, and settlement is sent to that ref — a machine started without one reports settlement nowhere. Also note WriteCursor's deliberate in-place chain-field mutation in its doc. Claude-Session: https://claude.ai/code/session_01CmZyk7wqzP8KtwQBVZg4ha
Every property of xstate's SetupTypes is optional, so an empty object is assignable to Partial<T> with no cast and inference through the optional properties still names the machine shapes exactly.
This was referenced Jul 21, 2026
This was referenced Aug 15, 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 #730
Replace the checkpoint submitter's per-activity mutable flag records, hand-scheduled timers, and p-retry loop with declared XState v5 machines — a parent spawning one child actor per activity — behind the unchanged
CheckpointSubmitterinterface, so the existing behavioral suite passes unmodified.scheduledwaits on an external flush-due event, keeping the injectedscheduleFlushtest closures awaitable and superseded-safe.CHECKPOINT_INVALIDtombstone is a retained ordinary state; the parent'sevictedActivityIDsanswersisEvictedafter a child stops.buildMachineTypes<T>()owns the one phantom-type assertionsetup()requires.flushNowduring an in-flight registration seed is now a no-op (previously flushed the partially-seeded cursor).Testing
bun run typecheckpassesbun run testpassesbun run lintpasses