Fix retained sub-agent session leaks and close_agent false success - #623
Closed
TheGreatAxios wants to merge 1 commit into
Closed
Conversation
Retained sessions were exempt from any cap/TTL and never released on cancelAll/clear; close_agent during agent setup returned false success over an unreleasable session; a disposed salvage still looked resumable; and the parent-abort listener was torn down on the persist path, stranding a retained session's LSP sidecars, reactor, and @intx/agent lock entry. - session-store.ts: fold retained-completed sessions into the existing maxCompleted bound (no separate cap needed) and release their close handle on eviction; cancelAll and clear() now invoke closeHandles for every still-open retained session; closeOne waits (bounded) for registerClose during the setup window instead of reporting shutdown with nothing to release, and reports the honest in-progress status if the window never closes in time; complete() only leaves a session resumable when the caller confirms the agent actually survived the turn (agentRetained), so a disposed salvage can't look reusable. - run.ts: keep the run controller's parent-abort forwarding alive for a persisted session so a later parent cancel still reaches the registered close handle; the close handle itself fully disposes the controller once the session actually closes. - types.ts / agent-fleet.ts: thread agentRetained through RunSubAgentResult so the store can tell a clean, still-open completion apart from a resolved-but-disposed salvage.
Collaborator
Author
|
Superseded by the same commits on a correctly-named branch — this one would have auto-closed CL-7001 instead of CL-7002. |
2 tasks
TheGreatAxios
deleted the
cl-7001-retained-sessions-never-release-and-close-agent-can-false-succeed
branch
August 28, 2026 00:09
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.
Summary
Four resource-leak / false-success defects in the CL-6943 retained-session lifecycle (session-store.ts, agent-fleet.ts, run.ts):
pruneCompletedno longer exempts retained-completed sessions from any bound — they now share the samemaxCompletedcap every other finished session already used, and eviction releases the session's close handle (LSP sidecars, reactor,@intx/agentlock entry) instead of abandoning it.cancelAllandclear()now also release every still-open retained session directly (not gated onstatus === "running", which a retained "done" session never satisfies).closeOnenow waits (bounded by the same deadline) forregisterCloseto fire if it hasn't yet, instead of finding no handle and returning a false "shutdown". If the window never closes in time it returns the session's honest in-progress status so a retry can still find it.complete()takes anagentRetainedflag (threaded fromRunSubAgentResult.agentRetained, set only on run.ts's clean-completion path whenpersistactually skipped teardown); a deadline/cancel salvage resolves the same promise but always disposed its agent, so it now clearsretainedinstead of leaving the sessionresumeOne-eligible.createSubAgentRunController.dispose()takes akeepParentListeneroption; a persisting run keeps the forwarding listener alive so a later parent cancel still reaches the registered close handle (which fully disposes the controller once the session actually closes).Test plan
bun test src/subagent/— 273 pass (includes newsrc/subagent/retain-salvage.test.ts, copied from the provided starting point and extended with setup-window / clear() / genuinely-resumable cases)bun run check(lint + typecheck + build + full test suite) — 5342 pass, 0 fail