agentHost: release idle sessions from memory (SDK session + state)#324854
Draft
roblourens wants to merge 4 commits into
Draft
agentHost: release idle sessions from memory (SDK session + state)#324854roblourens wants to merge 4 commits into
roblourens wants to merge 4 commits into
Conversation
… state Re-enable idle-session eviction so an idle session with no remaining subscribers releases all of its in-memory footprint: both the cached AHP state and the provider's SDK session/connection. The previous eviction only dropped the lightweight state cache and never released the SDK session, so it freed the wrong thing while breaking observability, which is why it was disabled. Add a non-destructive `IAgent.releaseSession` implemented by CopilotAgent that disconnects the SDK session and disposes the in-memory entry while preserving all durable data (SDK session log, session database, worktree), so the session resumes transparently on the next access. Eviction is guarded against in-flight restores and active turns, and restore-from-disk is verified lossless by unit and integration tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extend idle-session release to the Claude and Codex providers so eviction frees their in-memory footprint while preserving durable state for a lossless resume, matching the Copilot behavior. - Claude: releaseSession tears down the session entry (disposing the live SDK subprocess and its MCP-server children via the pipeline) but keeps the on-disk session for _resumeSession. Guarded against provisional (never-materialized) sessions and in-flight turns via a new hasActiveTurn signal on the pipeline/session. - Codex: releaseSession runs the same in-memory teardown as disposeSession (thread/unsubscribe on the shared process, keeping the resumable on-disk rollout), extracted into a shared helper. Guarded against provisional (unstarted thread) sessions and in-flight turns. Both providers' disposeSession was already non-destructive at the provider level; the destructive difference (deleting durable data) stays in the orchestrator, which only invokes it on dispose. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Re-enables idle-session eviction in the agent host and corrects the original implementation. When an idle session loses its last subscriber, the orchestrator now drops the cached AHP state and asks the provider to release its heavy in-memory SDK resources, while preserving all durable data (SDK session log, session database, worktree) so the session resumes transparently on next access. This inverts the prior (buggy) behavior that dropped only the lightweight state cache that clients still observed.
Changes:
- Adds a non-destructive, optional
IAgent.releaseSession(session)distinct from destructivedisposeSession, implemented for Copilot, Claude, and Codex by sharing each provider's existing in-memory teardown. - Re-enables
_maybeEvictIdleSessionwith a restore-in-flight guard and a providerreleaseSessioncall in lockstep with the state-cache drop; addshasActiveTurndefensive re-check signals. - Re-enables/updates the four eviction unit tests, adds lossless-restore coverage, and adds an end-to-end unsubscribe → eviction → re-subscribe integration test.
Show a summary per file
| File | Description |
|---|---|
src/vs/platform/agentHost/common/agentService.ts |
Adds optional releaseSession? to the IAgent interface with documented non-destructive semantics. |
src/vs/platform/agentHost/node/agentService.ts |
Re-enables _maybeEvictIdleSession; adds restore-in-flight guard and fire-and-forget provider.releaseSession. |
src/vs/platform/agentHost/node/copilot/copilotAgent.ts |
Adds releaseSession; extracts _releaseSessionResources from _destroyAndDisposeSession, keeping worktree reaping only on dispose. |
src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts |
Adds hasActiveTurn getter for the active-turn guard. |
src/vs/platform/agentHost/node/claude/claudeAgent.ts |
Adds releaseSession reusing _teardownEntry with provisional/active-turn guards. |
src/vs/platform/agentHost/node/claude/claudeAgentSession.ts |
Adds hasActiveTurn getter delegating to the pipeline. |
src/vs/platform/agentHost/node/claude/claudeSdkPipeline.ts |
Adds hasActiveTurn getter based on queue emptiness. |
src/vs/platform/agentHost/node/codex/codexAgent.ts |
Adds releaseSession; extracts _teardownSessionInMemory shared with disposeSession. |
src/vs/platform/agentHost/test/node/mockAgent.ts |
Adds MockAgent.releaseSession recording calls non-destructively. |
src/vs/platform/agentHost/test/node/agentService.test.ts |
Re-enables/updates eviction tests and adds lossless re-subscribe restore coverage. |
src/vs/platform/agentHost/test/node/protocol/sessionLifecycle.integrationTest.ts |
Adds an end-to-end release-and-restore round-trip over the real protocol server. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 0
- Review effort level: Medium
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
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.
Fixes #324856
What & why
Re-enables idle-session eviction in the agent host and fixes what the original implementation got wrong. When an idle session loses its last subscriber, we now release all of its in-memory footprint — both the cached AHP state and the provider's SDK session/connection — while preserving all durable data (SDK session log, session database, worktree) so the session resumes transparently on the next access.
Idle eviction was previously disabled (#323824) because the old
_maybeEvictIdleSessiononly dropped the lightweight AHP state cache via_stateManager.removeSession(...)and never released the heavy provider resources (the cached session entry, the live SDK session, active clients, MCP subscriptions). So it freed the wrong thing and dropped observable state out from under clients that still needed it. This change inverts that: it releases the expensive SDK resources, and the state-cache drop is safe because restore-from-disk is lossless.How
IAgent.releaseSession(session)— distinct from the destructivedisposeSession. It disconnects/tears down the session's in-memory SDK resources but never deletes anything on disk._maybeEvictIdleSessionis re-enabled: after confirming no subscribers remain on the root or any subagent descendant, no restore is in flight, and there is no active turn, it drops the cached state for the root + its subagent entries and callsprovider.releaseSession(root)._destroyAndDisposeSessioninto_releaseSessionResources(SDKsession.disconnect()+ entry/active-client/MCP-subscription disposal, no worktree removal, nodeleteSession). The destructivedisposeSessionkeeps calling it then reaps the worktree.thread/unsubscribeon the shared codex process (keeping the resumable on-disk rollout), reusing the existingdisposeSessionteardown.hasActiveTurnsignal backs the Copilot/Claude defensive re-check.The destructive difference (deleting durable data) stays in the orchestrator, which only invokes it on dispose — so provider
releaseSessionanddisposeSessionshare the same in-memory teardown.Notes for reviewers
session.destroy->session.dispose()->mcpHost.stopServers()for Copilot; pipeline dispose -> CLI subprocess teardown for Claude;thread/unsubscribefor Codex) thatdisposeSessionalready used, so no new subprocess-leak risk is introduced._pendingSessionGctimer pattern andAgentHostChangesetStateCacheLRU) in a follow-up.Tests
agentService.test.tsand added coverage thatreleaseSessionfires, that dispose is not called on eviction, and that a subsequent subscribe restores turns losslessly.sessionLifecycle.integrationTest.tsover the real protocol server (mock provider).typecheck-clientand hygiene clean.(Written by Copilot)