sessions: add developer command to kill SSH remote agent host#312193
sessions: add developer command to kill SSH remote agent host#312193joshspicer wants to merge 1 commit intomainfrom
Conversation
Add a Developer: Kill Remote Agent Host (SSH)... command (workbench.action.sessions.killRemoteAgentHostSSH) that lets developers kill a wedged remote agent host process and tear down the SSH tunnel. - Add killRemoteAgentHost(host) to ISSHRemoteAgentHostService and ISSHRemoteAgentHostMainService interfaces - Implement in SSHRemoteAgentHostMainService using the existing cleanupRemoteAgentHost helper, then dispose the connection - Proxy through the renderer-side service and add a no-op to the null browser implementation - Register a Developer category action that quick-picks from active SSH connections and calls killRemoteAgentHost Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a developer “escape hatch” command in the Sessions window to force-kill a wedged SSH remote agent host by invoking a new killRemoteAgentHost(host) API on the SSH remote agent host services (renderer + shared process), then tearing down the SSH tunnel.
Changes:
- Introduces
killRemoteAgentHost(host: string): Promise<void>onISSHRemoteAgentHostServiceandISSHRemoteAgentHostMainService. - Implements the kill behavior in
SSHRemoteAgentHostMainService(cleanup state-file-tracked PID, then dispose connection). - Registers Developer: Kill Remote Agent Host (SSH)... and wires renderer IPC forwarding + browser no-op implementation.
Show a summary per file
| File | Description |
|---|---|
| src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostActions.ts | Adds the Developer command and quick-pick UI to select an active SSH connection and invoke the kill API. |
| src/vs/platform/agentHost/node/sshRemoteAgentHostService.ts | Implements killRemoteAgentHost in the shared process and disposes the SSH connection/tunnel. |
| src/vs/platform/agentHost/electron-browser/sshRemoteAgentHostServiceImpl.ts | Forwards killRemoteAgentHost calls from renderer to the shared process via IPC proxy. |
| src/vs/platform/agentHost/common/sshRemoteAgentHost.ts | Extends the common interfaces with the new kill API and documents intended behavior. |
| src/vs/platform/agentHost/browser/nullSshRemoteAgentHostService.ts | Adds a no-op killRemoteAgentHost for browser contexts without SSH. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 3
| } | ||
| conn.dispose(); | ||
| return; | ||
| } | ||
| } | ||
| this._logService.warn(`${LOG_PREFIX} killRemoteAgentHost: no active SSH connection found for ${host}`); |
There was a problem hiding this comment.
killRemoteAgentHost swallows cleanupRemoteAgentHost errors and also only logs (then returns) when no connection matches. Because this method always resolves, the renderer action will always show a success notification even when nothing was killed. Consider propagating failures (rethrow) and throwing when the connection isn't found so the UI can show the intended error notification (or change the API to return a boolean/result that the UI can interpret).
| } | |
| conn.dispose(); | |
| return; | |
| } | |
| } | |
| this._logService.warn(`${LOG_PREFIX} killRemoteAgentHost: no active SSH connection found for ${host}`); | |
| throw err; | |
| } finally { | |
| conn.dispose(); | |
| } | |
| return; | |
| } | |
| } | |
| const message = localize('sshRemoteAgentHost.killRemoteAgentHost.noActiveConnection', "No active SSH connection found for {0}.", host); | |
| this._logService.warn(`${LOG_PREFIX} killRemoteAgentHost: ${message}`); | |
| throw new Error(message); |
| try { | ||
| await cleanupRemoteAgentHost(exec, this._logService, this._quality); | ||
| } catch (err) { | ||
| this._logService.warn(`${LOG_PREFIX} Error killing remote agent host for ${key}: ${err}`); |
There was a problem hiding this comment.
The warn log interpolates the raw caught value (${err}), which can produce unhelpful output like [object Object] and may omit stack traces. Use the same pattern as elsewhere in this file (e.g., err instanceof Error ? err.message : String(err)) or a shared helper like toErrorMessage so logs are actionable.
| this._logService.warn(`${LOG_PREFIX} Error killing remote agent host for ${key}: ${err}`); | |
| this._logService.warn(`${LOG_PREFIX} Error killing remote agent host for ${key}: ${err instanceof Error ? err.message : String(err)}`); |
| } | ||
| conn.dispose(); | ||
| return; | ||
| } | ||
| } | ||
| this._logService.warn(`${LOG_PREFIX} killRemoteAgentHost: no active SSH connection found for ${host}`); |
There was a problem hiding this comment.
New behavior in killRemoteAgentHost isn't covered by the existing SSHRemoteAgentHostMainService unit tests. Adding a test that (1) connects, (2) calls killRemoteAgentHost, and (3) asserts cleanup commands ran and the connection was disposed (and that errors/not-found cases surface to callers) would help prevent regressions in this developer escape hatch.
| } | |
| conn.dispose(); | |
| return; | |
| } | |
| } | |
| this._logService.warn(`${LOG_PREFIX} killRemoteAgentHost: no active SSH connection found for ${host}`); | |
| throw err; | |
| } finally { | |
| conn.dispose(); | |
| } | |
| return; | |
| } | |
| } | |
| const error = new Error(`${LOG_PREFIX} killRemoteAgentHost: no active SSH connection found for ${host}`); | |
| this._logService.warn(error.message); | |
| throw error; |
Summary
Adds a Developer: Kill Remote Agent Host (SSH)... command (
workbench.action.sessions.killRemoteAgentHostSSH) as a developer escape hatch for when a remote agent host process becomes wedged. It kills the remote process via SSH and tears down the tunnel.Changes
ISSHRemoteAgentHostService/ISSHRemoteAgentHostMainService(common interface)killRemoteAgentHost(host: string): Promise<void>to both interfaces.SSHRemoteAgentHostMainService(shared process, node)killRemoteAgentHost: finds the matchingSSHConnectionby key or connectionId, callscleanupRemoteAgentHost(existing helper that reads the remote state file, kills the PID, and removes the state file) over the connection's SSH client, then disposes the connection to tear down the tunnel.Renderer proxy & null impl
SSHRemoteAgentHostService(electron-browser): forwards to the main service via the existing IPC proxy.NullSSHRemoteAgentHostService(browser): no-op.Developer command (
remoteAgentHostActions.ts)killRemoteAgentHostand shows a success/error notification.