Rob Lourens c7d320d48e
agentHost: dedupe concurrent _resumeSession calls per sessionId (#318636)
* agentHost: dedupe concurrent _resumeSession calls per sessionId

Concurrent _resumeSession(id) callers (e.g. an outdated-config refresh in
sendMessage plus a getSessionMessages subscribe) each constructed a fresh
CopilotAgentSession and ran it through _createAgentSession, whose
this._sessions.set(id, …) on a DisposableMap synchronously disposed the
first entry mid-initializeSession(). The result was a tight loop of
'Trying to add a disposable to a DisposableStore that has already been
disposed' warnings (~550 in one repro) and a half-initialised session
with no event subscriptions — the chat widget opens, but sending a
message goes nowhere.

Fix:
- Add an _resumingSessions: Map<string, Promise<CopilotAgentSession>>;
  _resumeSession is now a thin wrapper that returns the in-flight promise
  when one exists, else delegates to a new _doResumeSession and memoizes
  the promise until it settles.
- Stop registering sessions in _sessions inside _createAgentSession.
  Both callers (_doResumeSession and _materializeProvisional) now set
  _sessions only after initializeSession() succeeds, and dispose the
  freshly-constructed agentSession if it throws. This removes the
  DisposableMap.set footgun that disposed an in-flight entry from under
  its own init.

Adds focused tests for the dedup contract:
- concurrent calls for the same id share one promise + one _doResumeSession
- inflight entry is cleared after resolution (next call re-invokes)
- inflight entry is cleared after rejection (next call retries)
- different ids resolve independently

(Written by Copilot)

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

* agentHost: guard post-init _sessions.set against shutdown race

Address Copilot review on #318636: now that _sessions.set is deferred
until after initializeSession() resolves, an in-flight _resumeSession /
_materializeProvisional whose init resolves AFTER dispose() ->
shutdown() -> super.dispose() has run would call _sessions.set(...) on
a disposed DisposableMap, leaking the session and reproducing the
'Trying to add a disposable to a DisposableStore that has already been
disposed' warning this PR exists to eliminate.

Extract a small _registerInitializedSession helper that bails (dispose
+ CancellationError) when _shutdownPromise is already set, and route
both call sites through it. The provisional path additionally still
removes its created worktree on cancel via the existing catch block.

Add a focused unit test that exercises the helper directly with a fake
session, simulating the shutdown-started state.

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

* launch skill: call out the transpile-client → preLaunch-skips-compile trap

When 'out/' already exists from a prior 'npm run transpile-client',
'node build/lib/preLaunch.ts' will skip 'npm run compile' and you'll
get a launched window whose built-in extensions fail to activate with
'Cannot find module .../extensions/.../out/...'. Document the trap in
both prerequisites and troubleshooting, and add a 'node_modules / npm
install' prereq.

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

* launch skill: tighten compile prereq + sign-in nudge

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-05-28 00:06:56 +00:00
..