Skip to content

agentHost: observe capabilities lazily in session adapters - #330853

Merged
Sandeep Somavarapu (sandy081) merged 1 commit into
mainfrom
alexd/agent-host-lazy-capability-observer
Aug 17, 2026
Merged

agentHost: observe capabilities lazily in session adapters#330853
Sandeep Somavarapu (sandy081) merged 1 commit into
mainfrom
alexd/agent-host-lazy-capability-observer

Conversation

@alexdima

@alexdima Alexandru Dima (alexdima) commented Aug 14, 2026

Copy link
Copy Markdown
Member

I generated this by scanning my logs, pls take if it's good/useful


🤖

Found while self-hosting Insiders: scanning my recent session logs for errors and leaked-disposable warnings surfaced a listener leak originating in the Agent Host sessions provider.

The symptom

~/Library/Application Support/Code - Insiders/logs/*/window1/renderer.log:

2026-08-10 11:15:47.896 [error] potential listener LEAK detected, popular: Error
    at s.create (.../sessions.desktop.main.js)
    at ML.onFirstObserverAdded (.../sessions.desktop.main.js)
    at dS._computeFn (.../sessions.desktop.main.js)
    ...
    at dce.createAdapter (.../sessions.desktop.main.js)
    at dce._refreshSessions (.../sessions.desktop.main.js)

The minified frames deobfuscate to BaseAgentHostSessionsProvider._refreshSessionscreateAdapternew AgentHostSessionAdapter → an autorun adding an observer to a shared observable.

Root cause

AgentHostSessionAdapter's constructor registered an autorun that reads this.capabilities, which is derived from the provider-wide agentCapabilities observable. Every adapter therefore attached an observer to that one shared observable at construction time.

_refreshSessions builds an adapter for every session the host lists, so a window restoring a large session list installed one observer per session and crossed the leak-detector threshold.

The subscription was also mostly pointless. That autorun exists only to re-apply a chat catalog when capabilities hydrate late (the race #323625 addressed, where a multi-chat SessionState is processed before root state advertises supportsMultipleChats). Its body no-ops unless _lastCatalogState is set — and _lastCatalogState is only ever assigned by applyChatCatalog. Adapters that never received a catalog (the overwhelming majority in a large list) held a live observer that could never do work.

The fix

Install the capabilities observer on the first applyChatCatalog call rather than in the constructor, held in a MutableDisposable. Adapters with catalog state to reconcile subscribe; adapters without one cost nothing.

This is a natural follow-on to #328031, which collapsed N root-state event listeners into one shared lookup. This removes the remaining N per-adapter observers on that shared lookup.

Behavior preserved

Late-hydrating capabilities still re-expand a collapsed peer catalog. The existing test a peer catalog collapsed while capabilities were absent re-expands when they hydrate covers exactly that path and passes unchanged — the observer is installed by applyChatCatalog, which necessarily runs before any catalog needs reapplying.

Validation

  • New test session adapters observe capabilities only after receiving a chat catalog builds 200 adapters and asserts 0 capability observers before any catalog, exactly 1 after, and correct peer-chat expansion on late hydration.
  • Targeted suite (incl. the two pre-existing capability/listener tests): 3 passing.
  • npm run typecheck-client and npm run valid-layers-check pass. (typecheck-client reports two pre-existing assignmentService.ts errors that also reproduce on a pristine main — unrelated to this change.)

Review note

Sandeep Somavarapu (@sandy081) flagging you since you authored applyChatCatalog, _lastCatalogState, and the "Make ISession.capabilities observable so late-hydrating capabilities reconcile" change that introduced this autorun. The one assumption worth your eyes: that an adapter which never receives a chat catalog genuinely has nothing to reconcile when capabilities hydrate late. Everything else here is listener bookkeeping.

Every cached AgentHostSessionAdapter eagerly subscribed to the shared
agent-capabilities observable, so a window restoring hundreds of sessions
installed hundreds of observers and tripped the listener leak detector.

Most of those observers had nothing to do: the autorun only re-applies a
chat catalog, and an adapter that never received one has no catalog to
reconcile. Install the observer on the first applyChatCatalog call instead,
so only adapters with catalog state to reapply subscribe. Late-hydrating
capabilities still re-expand a collapsed peer catalog.

Found while self-hosting Insiders.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Lazily observes Agent Host capabilities only when a session adapter has a chat catalog requiring reconciliation, preventing unnecessary observers.

Changes:

  • Moves capability observation from construction to first catalog application.
  • Adds regression coverage for lazy observation and late hydration.
  • Documents the updated architecture.
Show a summary per file
File Description
baseAgentHostSessionsProvider.ts Lazily installs the capability observer.
localAgentHostSessionsProvider.test.ts Tests observer counts and catalog hydration.
AGENT_HOST_SESSIONS_PROVIDER.md Documents lazy capability observation.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Balanced

@sandy081
Sandeep Somavarapu (sandy081) merged commit 156357c into main Aug 17, 2026
28 checks passed
@sandy081
Sandeep Somavarapu (sandy081) deleted the alexd/agent-host-lazy-capability-observer branch August 17, 2026 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants