agentHost: resolve user shell environment for agent host process#312199
Merged
roblourens merged 2 commits intomainfrom Apr 23, 2026
Merged
agentHost: resolve user shell environment for agent host process#312199roblourens merged 2 commits intomainfrom
roblourens merged 2 commits intomainfrom
Conversation
Spawn the agent host with the user's resolved shell environment merged in (PATH and friends from the login shell), matching what other VS Code processes do via getResolvedShellEnv. Without this, tools and terminals launched by the agent host on macOS/Linux GUI launches don't see the user's PATH. Both ElectronAgentHostStarter and NodeAgentHostStarter now resolve the shell env before spawning. IAgentHostStarter.start() is now async; the process managers await it and guard against being disposed mid-await. In the Electron starter, the renderer's createMessageChannel request could race ahead of the now-async start() and call utilityProcess.connect() before utilityProcess.start() had run, silently dropping the MessagePort and leaving the renderer with no agents. _onWindowConnection now awaits a DeferredPromise that completes once the utility process has actually been spawned. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the agent host startup flow to incorporate the user’s resolved login-shell environment (e.g., PATH on macOS/Linux GUI launches) and adjusts the agent host starter/manager APIs to support async startup safely.
Changes:
- Make
IAgentHostStarter.start()async and update the agent host process managers toawaitstartup and dispose connections if the manager was disposed mid-start. - Resolve and merge
getResolvedShellEnv(...)into the spawned agent host process environment (Electron utility process and Node child-process starter). - Update server agent host manager tests to accommodate async startup timing.
Show a summary per file
| File | Description |
|---|---|
| src/vs/server/test/node/serverAgentHostManager.test.ts | Updates tests for async starter signature and event wiring timing. |
| src/vs/server/node/serverAgentHostManager.ts | Awaits async starter and adds disposed-guard after startup. |
| src/vs/platform/agentHost/node/nodeAgentHostStarter.ts | Resolves shell env and passes it to the forked agent host process; starter becomes async. |
| src/vs/platform/agentHost/node/agentHostService.ts | Awaits async starter and adds disposed-guard; adjusts _started timing. |
| src/vs/platform/agentHost/electron-main/electronAgentHostStarter.ts | Resolves shell env for the utility process, makes startup async, and adds a deferred gate for connect timing. |
| src/vs/platform/agentHost/common/agent.ts | Changes IAgentHostStarter.start() to return a promise. |
| src/vs/code/electron-main/app.ts | Passes IConfigurationService into ElectronAgentHostStarter. |
Copilot's findings
Comments suppressed due to low confidence (2)
src/vs/platform/agentHost/electron-main/electronAgentHostStarter.ts:123
- Because
_onWindowConnectionis nowasync, any exception/rejection inside it will turn into a rejected Promise. Since the IPC registration uses a plainvalidatedIpcMain.on(...)listener (which does not await Promises), rejections can become unhandled promise rejections. Consider ensuring_onWindowConnectionnever rejects (wrap body in try/catch + log) or have the IPC listener explicitly handle the Promise result.
private async _onWindowConnection(e: IpcMainEvent, nonce: string): Promise<void> {
this._onRequestConnection.fire();
// Wait for utilityProcess.start() to actually run before calling connect(),
// otherwise the MessagePort posted via connect() is silently dropped.
await this.utilityProcessStarted?.p;
src/vs/platform/agentHost/electron-main/electronAgentHostStarter.ts:69
utilityProcessStartedis awaited in_onWindowConnection, but it’s only completed afterutilityProcess.start(...). IfUtilityProcess.startthrows (e.g. fork failure), the deferred promise will never settle and future window connections can hang awaiting it. Consider settling the deferred in atry/finally(and callingerror(...)on failure) so_onWindowConnectioncan proceed to the “process not running” path instead of waiting indefinitely.
This issue also appears on line 117 of the same file.
this.utilityProcess.start({
type: 'agentHost',
name: 'agent-host',
entryPoint: 'vs/platform/agentHost/node/agentHostMain',
- Files reviewed: 7/7 changed files
- Comments generated: 3
- ElectronAgentHostStarter: spread shellEnv after process.env so the resolved login shell PATH actually wins over the GUI-launched env. (NodeAgentHostStarter is fine as-is: ipc.cp.Client merges process.env before the options env, so shellEnv there already wins.) - AgentHostProcessManager._start / ServerAgentHostManager._start: wrap the body in try/catch so a rejection from starter.start() doesn't surface as an unhandled promise rejection. Reset state on failure so future starts can retry. Server manager applies the same MaxRestarts policy as the unexpected-exit path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @deepak1556Matched files:
|
zhichli
approved these changes
Apr 23, 2026
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.
Spawn the agent host with the user's resolved shell environment merged in (PATH and friends from the login shell), matching what other VS Code processes already do via
getResolvedShellEnv. Without this, tools and terminals launched by the agent host on macOS/Linux GUI launches don't see the user's PATH.What changed
ElectronAgentHostStarterandNodeAgentHostStarternow callgetResolvedShellEnvand merge the result into the spawned agent host's env. The shell env resolution is cached at module scope inshellEnv.ts, so reusing the same call the main process already made is essentially free.IAgentHostStarter.start()is now async (Promise<IAgentHostConnection>).AgentHostProcessManager._start()andServerAgentHostManager._start()await it and guard against the manager being disposed mid-await.IConfigurationService.Race fix in
_onWindowConnectionOnce
start()became async (with anawaiton shell-env resolution), the renderer'svscode:createAgentHostMessageChannelIPC could race ahead and callutilityProcess.connect()beforeutilityProcess.start()had actually run.connect()callspostMessage(port)which silently returns false when the underlying process isn't started — the renderer's MessagePort got dropped, and the renderer never saw any agents advertised._onWindowConnectionnow awaits aDeferredPromise<void>(utilityProcessStarted) that completes onceutilityProcess.start()has run.Tests
IAgentHostStarterinserverAgentHostManager.test.tsupdated for the async signature; all tests now await awaitForStart()microtask before firing channel events.Why pty host doesn't need this
Pty host doesn't pre-resolve shell env into the spawned child — it just inherits
process.env. Resolved shell env is used later, on demand, only for terminal profile detection and per-terminal launch configs (which are sent to the pty host via IPC). The agent host is different: tools it spawns internally need PATH baked into its own process env, because they aren't routed through a workbench-built launch config.(Written by Copilot)