Repository navigation
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const next = this.subsessionBindingWriteQueue.then(() => | ||
| this.writeSubsessionBindings(), | ||
| ); | ||
| this.subsessionBindingWriteQueue = next.catch(() => {}); |
There was a problem hiding this comment.
🔴 Failed writes corrupt End snapshots
When a binding write fails during concurrent End, subsessionBindingWriteQueue can snapshot the failed caller's rollback. createReserved deletes the marker before End's queued snapshot runs. The resulting tombstone lacks a marker, so the next init() rejects the registry as malformed.
Learn more
The stored queue tail converts the active write's rejection into fulfillment. A later End write waits on that tail. When the active write rejects, the caller's catch can roll back memory before the later writer snapshots it. createReserved deletes its marker without checking whether End advanced the close generation; resumePrepared similarly restores the old marker. End has already added the session to userClosedSubsessions, and loadSubsessionBindings rejects any closed ID without a marker.
Example: A reserved create starts marker write A. End adds the closed tombstone and queues write B. Write A fails, so the create catch deletes the marker. Write B then snapshots { markers: {}, closedSessionIds: [id] }; the next launch rejects this file as malformed.
Recommended fix: Make every binding rollback close-generation-aware, as restartFreshPrepared already does, so it cannot overwrite End's in-memory state. Add failure-and-End race tests for both createReserved and resumeBound, including reloading the persisted registry.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Primary change type
Problem and motivation
A held older Terminal binding write could land after End removed its marker, and failed restart compensation was swallowed.
Summary and scope
Serialize actual sidecar writes independently from launch preparation and snapshot current registry state when the queued write runs. Await and surface repair failure. Return HTTP409 for Resume/Start Terminal during shutdown.
Related work
Related issue: https://linear.app/sapiom/issue/SAP-3292
Depends on #1021. Native stack target: main.
Validation
385 tests passed across five SessionManager suites and REST. Five meaningful red cases reproduced the old failures before correction. Scoped lint and diff checks passed; full Harness typecheck also passed after concurrent source edits settled.
Tests and documentation
Cover End ordering behind held marker writes, ENOSPC compensation before/after commit, successful retry, and shutdown mapping for both Resume and dormant Terminal activation. Updated assistant-history.md. Increment:134 changed lines including tests/docs/changeset; SDK pr:size is absent.
Compatibility and release impact
Security
AI assistance
Checklist