refactor(#912): collapse resync serialization into the lifecycle mailbox - #943
Merged
Conversation
Three stacked mechanisms — the lifecycle tail, the resync in-flight flag, and the 1-deep held-claim queue — served one serialization policy. Collapse them into create-lifecycle-mailbox.ts, a single closure-owned mailbox exposed through WorkerContext.getMailbox(). Behavior is unchanged: turns still run strictly one at a time and never reject, and resync coalescing still drops a non-claiming call during the drop window and holds one claiming call, latest wins.
Wait for the inline resync's broadcast before the external call arrives, so the new pinning test actually lands mid-window instead of racing ahead of the queued turn's own microtask start. Replace the three conditional gate selections in the mailbox coalescing tests with a per-avatar gate lookup, per the no-branching-in-test-bodies rule.
The pinning test for behavior (e) waited on the inline resync's own terminal broadcast before firing the external call, so by the time waitFor returned the inline flow had already settled — the arrival never actually landed inside the drop window a regression could leave open. Gate the stub submitter's flushHeld on a test-controlled promise instead, and wait for it to be called: that pauses the inline flow strictly inside its own execution, so the external call now arrives while the mailbox's resync ticket is genuinely still held.
|
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
💤 Files with no reviewable changes (2)
Included review availability: 4 reviews are currently available. Based on recent review activity, included reviews refill at 5 per hour. 📝 WalkthroughWalkthroughThe idle worker now uses a lifecycle mailbox to serialize lifecycle turns, report turn errors, and coalesce resync requests. Worker context, runtime handlers, resync execution, simulations, and tests now use the mailbox API. ChangesLifecycle mailbox consolidation
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to This refactor is merge-ready after normal checks and review; no actionable merge-blocking risk remains. Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 11 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
A caller-supplied plan that throws while the ticket is set would otherwise drop every later resync for the worker's lifetime; the accept-through-settle span now clears the ticket in a finally, pinned by a new mailbox test. Also rewords the factory JSDoc to stop naming another declaration.
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 #912
Introduces
createLifecycleMailbox(), collapsingwith-lifecycle-turn.ts's tail-queue-and-report-fault turn wrapper and the resync single-flight/held-claim bookkeeping into one owner. Resync coalescing becomes a private ticket inside the mailbox: a non-claiming call is dropped while one is queued or running, a claiming call is held one deep with latest-arrival-wins, and the queued body is prepared synchronously at both accept and requeue so cancellation signals are captured fresh each run.WorkerContextdrops its six lifecycle/resync accessors for onegetMailbox(): LifecycleMailboxrun-resync-turn.tsis now a thin binder delegating tocontext.getMailbox().runResyncTurn(...), its exported signature unchangedrun-simulation.ts,handle-start-activity-message.ts, and the runtime'sonEvictedcallback callcontext.getMailbox().runTurn(...)with-lifecycle-turn.tsand its test are deleted, and the stub worker context now builds a real mailboxcreate-lifecycle-mailbox.test.tscovers queue ordering, cross-kind serialization, fault reporting, and the full coalescing policyrun-resync-turn.test.tsswaps itsisResyncInFlight()postconditions for behavioral probes and adds a case for an external resync queuing behind an inline resync flowTesting
bun run typecheckpassesbun run testpassesbun run lintpasses