Skip to content

sessions: restore X-button removal of SSH remote agent host entries#318262

Merged
roblourens merged 2 commits into
mainfrom
agents/vsckb-implement-the-x-button-on-the-remote-385e7265
May 26, 2026
Merged

sessions: restore X-button removal of SSH remote agent host entries#318262
roblourens merged 2 commits into
mainfrom
agents/vsckb-implement-the-x-button-on-the-remote-385e7265

Conversation

@roblourens
Copy link
Copy Markdown
Member

The X button on remote agent host SSH entries in the workspace picker disconnected the SSH tunnel but did not remove the entry from configured storage: on the next reconcile pass (or on reload) _reconnectSSHEntries found the entry still configured and immediately auto-reconnected it.

This regressed in #316810, which routed SSH disconnect through the provider's _disconnectOnDemand hook. The provider's disconnect() returns early when that hook is set, so it no longer called removeRemoteAgentHost for SSH entries (the pre-#316810 behavior). Combined with the synchronous reconcile that fires from _sshService.disconnect's own close event, the entry was reconnected before the disconnect even returned.

Fix

Call removeRemoteAgentHost from _disconnectSSHOnDemand before the _sshService.disconnect await so the entry is gone before the close-event chain triggers _reconnectSSHEntries. removeRemoteAgentHost also runs the SSH transport disposable, which itself tears down the main-process SSH tunnel; the explicit _sshService.disconnect(connectionKey) is kept as belt-and-suspenders.

Behavior

Before #316810 After #316810 (bug) After this fix
Entry removed from storage
SSH tunnel torn down ( broken differently)
Stays disconnected on reload

(Written by Copilot)

…Written by Copilot)

The X button on remote agent host SSH entries in the workspace picker
disconnected the SSH tunnel but did not remove the entry from configured
storage: on the next reconcile pass (or on reload) `_reconnectSSHEntries`
found the entry still configured and immediately auto-reconnected it.

This regressed in [#316810](#316810),
which routed SSH disconnect through the provider's `_disconnectOnDemand`
hook. The provider's `disconnect()` returns early when that hook is set,
so it no longer called `removeRemoteAgentHost` for SSH entries (the
pre-#316810 behavior). Combined with the synchronous reconcile that fires
from `_sshService.disconnect`'s own close event, the entry was reconnected
before the disconnect even returned.

Call `removeRemoteAgentHost` from `_disconnectSSHOnDemand` before the
`_sshService.disconnect` await so the entry is gone before the close-event
chain triggers `_reconnectSSHEntries`. `removeRemoteAgentHost` also runs
the SSH transport disposable, which itself tears down the main-process
SSH tunnel; the explicit `_sshService.disconnect(connectionKey)` is kept
as belt-and-suspenders.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 25, 2026 23:38
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a regression in the Sessions “Remote Agent Host” workspace picker where removing an SSH host via the X button would disconnect the tunnel but leave the SSH entry persisted, causing it to auto-reconnect on the next reconcile pass or reload.

Changes:

  • Reorders SSH “disconnect on demand” to remove the configured SSH entry from storage before tearing down the SSH tunnel.
  • Adds detailed inline rationale about the synchronous close-event → reconcile → auto-reconnect chain and why ordering matters.
Show a summary per file
File Description
src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHost.contribution.ts Ensures SSH host removal updates persisted configuration before disconnect triggers synchronous reconciliation that could otherwise immediately reconnect.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 1

@roblourens roblourens marked this pull request as ready for review May 26, 2026 00:02
@roblourens roblourens enabled auto-merge (squash) May 26, 2026 00:02
Extracts the SSH disconnect ordering from _disconnectSSHOnDemand into an
exported `disconnectSSHEntry` helper (plus an `sshConnectionKey` helper
for the per-connection key used by ISSHRemoteAgentHostService.disconnect).

The new tests in remoteAgentHost.contribution.test.ts guard against the
regression fixed in the previous commit:

  - removeRemoteAgentHost must run BEFORE the SSH tunnel teardown so the
    synchronous onDidChangeConnections fired by _sshService.disconnect
    doesn't trigger _reconcile -> _reconnectSSHEntries against the entry
    we are tearing down.
  - sshConnectionKey produces the same keys the SSH service stores
    connections under (ssh:<configHost> or user@host:port).

(Written by Copilot)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@roblourens roblourens merged commit 2bcf105 into main May 26, 2026
25 checks passed
@roblourens roblourens deleted the agents/vsckb-implement-the-x-button-on-the-remote-385e7265 branch May 26, 2026 00:51
@vs-code-engineering vs-code-engineering Bot added this to the 1.122.0 milestone May 26, 2026
anthonykim1 added a commit that referenced this pull request May 26, 2026
Squashed cherry-pick of 10 commits from main that are included in the
Insiders build (183159e) people are verifying:

- agentHost: show fetched URL for web_fetch (#318240)
- Fix SSH remote agent host passphrase auth (#318244)
- agentHost: add setting to disable worktreeCreated task auto-dispatch (#318243)
- Agent host: clearer worktree git timeout errors and 60s budget (#318242)
- Normalize LF to CRLF in agent host terminal tool output (#318257)
- sessions: restore X-button removal of SSH remote agent host entries (#318262)
- chat: fix duplicate command registration for agent-host-copilotcli (#318273)
- launch: build copilot in compile; wait for CDP in launch.sh (#318272)
- Preserve unread state across remote host disconnect (#318267)
- Add more codenotify for terminal (#318285)
dileepyavan pushed a commit that referenced this pull request May 27, 2026
Squashed cherry-pick of 10 commits from main that are included in the
Insiders build (183159e) people are verifying:

- agentHost: show fetched URL for web_fetch (#318240)
- Fix SSH remote agent host passphrase auth (#318244)
- agentHost: add setting to disable worktreeCreated task auto-dispatch (#318243)
- Agent host: clearer worktree git timeout errors and 60s budget (#318242)
- Normalize LF to CRLF in agent host terminal tool output (#318257)
- sessions: restore X-button removal of SSH remote agent host entries (#318262)
- chat: fix duplicate command registration for agent-host-copilotcli (#318273)
- launch: build copilot in compile; wait for CDP in launch.sh (#318272)
- Preserve unread state across remote host disconnect (#318267)
- Add more codenotify for terminal (#318285)
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