fix: return noop subscription from rootState when no protocol client (fixes #318222)#318233
Open
vs-code-engineering[bot] wants to merge 1 commit into
Open
Conversation
…ixes #318222) The EditorRemoteAgentHostServiceClient.rootState getter unconditionally called _requireClient() which throws when the agent host is disabled or no remote connection exists. Other getters (clientId, onDidNotification, onDidAction) already handle this gracefully with ?. and ??. When the agentHostSandboxForwarder subscribes to rootState.onDidChange or reads rootState.value on a config change, the throw becomes an unhandled error in telemetry. Return a static noop IAgentSubscription (value: undefined, events: Event.None) when _protocolClient is absent, matching the existing guard in _pushToConnection that early-returns on undefined values. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
🔧 Error Fix
Summary
The
EditorRemoteAgentHostServiceClient.rootStategetter unconditionally called_requireClient()which throwsError: Remote agent host is not enabled or no remote connection is available.when the agent host is disabled or no remote connection exists. TheagentHostSandboxForwarder(introduced in f32c055) subscribes torootState.onDidChangeand readsrootState.valueon configuration changes, triggering this unhandled error in telemetry across all platforms.Fixes #318222
Recommended reviewer:
@chrmartiCulprit Commit
f32c0553"Sandboxing for Copilot SDK integration (Support sandboxing for the Copilot SDK integration #317981)" by@chrmarti(2026-05-22)agentHostSandboxForwarder.tswhich eagerly accessesrootStateon the injectedIRemoteAgentHostService(backed byEditorRemoteAgentHostServiceClient) without handling the no-client case. Combined withb4b5c20c("Enable agent host by default for insiders") which increased the surface area.Code Flow
sequenceDiagram participant CS as ConfigurationService participant SF as SandboxForwarder participant RAHSC as EditorRemoteAgentHostServiceClient CS->>SF: onDidChangeConfiguration (sandbox key) SF->>SF: _pushToAllConnections() SF->>RAHSC: connection.rootState (getter) RAHSC->>RAHSC: _requireClient() Note over RAHSC: _protocolClient is undefined (disabled/no remote) RAHSC-->>SF: throws Error Note over SF: Unhandled error reaches telemetryAffected Files
src/vs/workbench/services/agentHost/browser/editorRemoteAgentHostServiceClient.tsrootStategetter throws instead of returning a safe fallbacksrc/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/agentHostSandboxForwarder.tsRepro Steps
onDidChangeConfigurationagentHostSandboxForwarder._pushToAllConnections()firesrootStateon theEditorRemoteAgentHostServiceClientthrows because_protocolClientisundefinedHow the Fix Works
Chosen approach (
editorRemoteAgentHostServiceClient.ts): Replace the throwing_requireClient()call in therootStategetter withthis._protocolClient?.rootState ?? this._noopRootState, where_noopRootStateis a static object implementingIAgentSubscription<RootState>withvalue: undefinedand all events asEvent.None. This fixes the error at the data producer — the getter that produces an exception instead of the safe fallback that the interface contract allows (.valuecan beundefinedper the interface docs). The consumer (_pushToConnection) already guards againstundefinedvalues at line 92, so it correctly no-ops.Alternatives considered: Adding a try/catch in
_pushToConnectionwould mask the error at the crash site without fixing the inconsistency in the service client; the getter is the only one among its siblings (clientId,onDidNotification,onDidAction) that throws instead of returning a safe default.Recommended Owner
@chrmarti— authored theagentHostSandboxForwarder.tsthat exposed this latent bug, and owns the sandboxing integration for Copilot SDK.