[v0.8 Core 7] overlap refinement drain with kernel teardown - #1335
Conversation
Reconstruct the unique net delta from PR #1265, excluding propagation merges.
jonaowen
left a comment
There was a problem hiding this comment.
The overlap is unsafe without a complete admission fence. disposeAsync() sets _asyncTeardownStarted and immediately starts kernel disposal while deliberately leaving _disposing === false until both kernel disposal and refinement drain settle. But normal action admission, queue pumping, subagent spawning, and most lifecycle guards still check only _disposed || _disposing (for example _assertSessionActionAdmissionAvailable, _admitSessionInput, and _spawnRlmChild). A concurrent prompt/message/heartbeat can therefore be admitted and begin using the authoritative kernel while it is being snapshotted/disposed. The patch gates only reload() and _buildRuntime(), so it does not establish exclusivity.
Please add one admission predicate that treats _asyncTeardownStarted as closed for every external/new-work entry point and queue pump, while allowing only the already-owned refinement drain/finalizers to complete. Then add a behavioral test that holds kernel disposal and refinement drain separately, attempts prompt/steer/message/subagent/reload during each boundary, and proves zero action admission/provider/kernel use plus one terminal teardown outcome. Also cover synchronous dispose() racing after async teardown starts.
Replacement scope
This PR reconstructs and supersedes the unique implementation delta reviewed in #1265 without rewriting that historical branch. The original PR remains the immutable discussion record: #1265
v080/core-split-c4-sibling-validationv080/core-split-c7-lifecycle2cab0cd1003432619b9e253dd58f01febae93a4f77b188b92dc91365cb2bc41bdb46a50669d104a8) is the shared foundation. For reconstructed deltas it is a proven tree-compatible base, not an ancestry claim about the historical PR stack.Validation
tsgo --noEmit: passNo original PR was retargeted, closed, merged, or otherwise mutated.
Note
Medium Risk
Changes session shutdown ordering and error surfacing for kernel teardown and concurrent dispose callers; behavior is well covered by new tests but affects core lifecycle paths.
Overview
AgentSession.disposeAsyncis reworked so graceful shutdown overlaps refinement drain and IPython kernel disposal instead of awaiting them strictly in sequence. Both start before the firstawait, run underPromise.allSettled, and rejections surface as a single error or anAggregateErrorwhen both fail—while synchronous cleanup still runs viadispose()in afinallypath.Concurrent and late
disposeAsynccallers now always join the same in-flight_disposeAsyncPromise, including when_disposedis already true, so they observe the same terminal outcome (success or failure). Kerneldispose()is no longer invoked from_disposeAsyncOnce(where failures were previously swallowed).A new
_asyncTeardownStartedflag is set at the start of async teardown;reload()and_buildRuntime()throw if called while disposal is underway, preventing the runtime from replacing the kernel mid-teardown.Tests in
agent-session-concurrent.test.tscover parallel start order, failure propagation, single finalization, late callers, reload blocking, and dual failure aggregation.Reviewed by Cursor Bugbot for commit 3055505. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Overlap refinement drain with kernel teardown in
AgentSession.disposeAsyncdisposeAsyncnow runs_drainPendingRefinementForDisposal()and_ipythonKernelProvisioner?.dispose()concurrently viaPromise.allSettled, aggregating failures into anAggregateErrorwhen both reject._disposedis already true) share the same terminal promise result, including any rejection._disposeAsyncOnce, which previously silently swallowed kernel disposal errors._asyncTeardownStartedflag causes_buildRuntimeandreloadto throw immediately if called after async teardown has begun.Macroscope summarized 3055505.