[pull] main from microsoft:main - #1654
Merged
Merged
Conversation
Migrates the remaining connection kinds — WSL, dev tunnels (desktop, web and browser), cloud sandbox, and Dev Container — onto `IRemoteAgentHostConnectionFactory`, so `RemoteAgentHostService` builds, handshakes, classifies, retries and disposes every remote agent host connection. `addManagedConnection`, the path that let an owning service hand over an already-connected client, is removed; with it gone the registry cannot be bypassed and the nine copies of the handshake-and-classify dance collapse to one. Eager and on-demand remotes are now distinguished by data rather than by which code path built the connection. `dialedFromEntries` (renamed from `dialableByService`) marks kinds reconciled from a factory's entries; on-demand kinds stage an entry, ask for an explicit connect, and still self-heal after a drop. `autoConnectGated` records which kinds respect `chat.remoteAgentHostsAutoConnect`, which the service previously ignored entirely — that gating lived only in the contributions, so moving dialing into the service would have auto-connected hosts users had opted out of. Fixes found along the way: - Cloud sandbox's sealed GitHub token is now applied by the protocol client between `initialize` and reporting connected, so no consumer can send an unauthenticated request. - Cached tunnels persist their `protocolVersion`. Reconstruction previously assumed v5, silently skipping gateway selection for v6+ tunnels. - WSL handles own no channel teardown: its `disconnect` is distro-scoped, so a stale transport teardown could kill a freshly established reconnect. - SSH re-establishes through the main service's `reconnect`, not `connect` with credential-stripped config, which could never succeed for password- or key-authenticated hosts. - Dev Container reconnects no longer gate on the initiating operation's cancellation token, whose scope ends when the first connect returns. Also converts `TunnelAgentHostStorage` to `observableMemento` and configuration reads to `observableConfigValue`, and adds VERIFICATION.md covering the shared connection scenarios and each remote's own behaviour. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-connection-factories
A cold WSL connect downloads a ~220 MB server inside the distro, but the readiness deadline was a single 60s total timeout, so the attempt was guaranteed to fail and be retried. Each retry spawned another bootstrap that started its own concurrent download; they then corrupted each other and failed with "error renaming downloaded server: Directory not empty". Fix the whole chain: - WSL waits on output rather than a wall clock. The readiness deadline is now an idle timeout rearmed on every line the bootstrap prints, with a separate absolute ceiling, so a slow download no longer looks stuck. Also surface a spawn `error`, which previously hung until the timeout. - WSL dedupes in-flight connects per distro, reserving synchronously before the first await so concurrent callers cannot both spawn. - The WSL launch carries `--idle-timeout`, matching the SSH spawn path, so an agent host abandoned by a failed attempt reaps itself instead of lingering. The timer is paused while a client is connected. - SSH and Dev Containers size their endpoint-registration wait to a cold server download instead of ~10s, reporting progress while they wait. The installer now reports whether it installed, so the long budget applies only to a cold start. - An automatic retry no longer clears the reconnect budget it depends on, and a reconnect for an address already being dialed joins that attempt rather than starting a competing one. - The CLI download cache stages per download instead of sharing one `.staging` directory it wipes first, serializes concurrent downloaders behind a file lock, treats a lost rename race as success rather than a spurious "please retry", and cleans up staging on every exit path. Separately, `RemoteAgentHostContribution` registered a sessions provider for every configured entry, including kinds whose own contribution already owns one. That threw and aborted the rest of the reconcile, so connections were never wired: the filesystem authority went unregistered and root state was never observed, leaving hosts that appeared connected but reported no models and could not read files. Restrict it to the kinds it owns and decouple provider registration from connection wiring. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`RemoteAgentHostContribution` owned two unrelated jobs: shared connection wiring, and SSH's provider plus reconnect machinery. Because the second lived there, it iterated every configured entry and registered a provider for kinds that already have their own contribution, throwing "Sessions provider '...' is already registered". A stopgap allowlist kept that in check but had to be updated by hand whenever a kind was added. Make provider ownership uniform instead, so the allowlist is unnecessary: - Add `EntryDrivenProviderContribution`, holding the provider registry and the entry-driven reconcile/wire/status loop that WSL, SSH and WebSocket each had their own copy of. Subclasses supply only the entry type they own and the provider options for an entry. - `ManagedReconnectAgentHostContribution` extends it, so WebSocket gets provider ownership without inheriting a reconnect state machine it has no use for. - Extract SSH into its own contribution and migrate it onto the shared managed-reconnect loop, which it had a near-duplicate of. Host-key denial still pauses in a way only an explicit user reconnect clears, now expressed as `requiresUserInitiatedResume` on the shared state and defaulted off, so WSL is unaffected. The periodic resume sweep stays SSH-specific. - Reduce the shared contribution to connection wiring: filesystem authority, root state, agent discovery, terminals and authentication. Note that WSL does not clear a provider's connection when that connection vanishes, unlike SSH and WebSocket. That difference predates this change, so it is preserved behind an opt-in rather than altered here. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removing a tunnel appeared to work but the tunnel returned as soon as the picker was reopened, for two independent reasons. `_disconnectTunnel` suppressed auto-connect and closed the relay but left the tunnel cached, and the provider list is built from the cache, so the provider was recreated on the next reconcile. WSL and SSH both drop their entry on disconnect; tunnels did not. Even with that fixed the tunnel came back, because discovery caches every tunnel on the account that is not already cached — and `cacheTunnel` clears auto-connect suppression, so re-caching also dropped that flag. Persist an explicit dismissal instead. Removing a tunnel records it, drops it from the cache and disconnects; the provider list and the discovery pass both skip dismissed tunnels, so the removal survives re-discovery. Only an explicit user connection clears a dismissal. Dismissal is kept separate from auto-connect suppression rather than reusing it: suppression marks tunnels this machine hosts, which must stay listed in the picker, and it is deliberately cleared by `cacheTunnel`. With removal no longer suppressing, suppression is now only about hosted tunnels, so its documentation is updated to say so. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…osts
Removing an SSH host from the picker brought it straight back:
Removing contribution for ssh:wsl-agent-test
Reconciling: desired=[ssh:wsl-agent-test], current=[]
Host key verification for wsl-agent-test
`removeRemoteAgentHost` only clears in-memory connection state; the
persisted entry is dropped by the kind's own `disconnect`. Both SSH and
WSL called them in that order, so tearing the connection down fired
`onDidChangeConnections` while the entry was still stored. Reconciliation
saw a desired-but-disconnected host and immediately re-dialled it, and
the removal appeared to do nothing.
Drop the persisted entry first so the address is no longer desired, and
the teardown's own reconcile becomes a no-op.
The previous order was deliberate, guarding against the entry being
"surfaced again before teardown completes" — but the entry was still
stored during that first step too, so it guarded the wrong half of the
sequence.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`cargo clippy -- -D warnings` failed on the new download-cache code:
`OpenOptions::create` without an explicit `truncate`, and a
`std::io::Error::new(ErrorKind::Other, _)` in a test.
Also reaps `{name}.staging-*` directories from earlier attempts. Staging
cleanup runs from a drop guard, which a killed or crashed process never
runs, and unlike the previous fixed staging path nothing else removed
them — so each crash leaked a partial server download. Reaping is safe
here specifically because it happens while holding that entry's download
lock, so no live attempt can own a matching directory.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- A transport drop during the initial `authenticate` RPC was wrapped as `InitialAuthenticationError`, which both connect paths classify as terminally incompatible. A momentary blip therefore stopped recovery permanently; connection-closed errors now stay reconnectable. - Cloud sandbox returned `undefined` when the sealed token was missing or not a sealed envelope, so the client skipped authentication and still reported connected — the silent, always-failing connection the documented contract says must not happen. It now fails the connection. - Staging a tunnel publishes its entry synchronously, so reconciliation could begin dialing before the caller's explicit reconnect; joining that pending dial meant the user's first connect ran as a background attempt with interactive auth and gateway selection suppressed. The initiation mode is now staged in the factory and consumed by the first connect, matching WSL. - A refused SSH host key or a cancelled connect was retried by the shared service, which re-prompted the person who had just declined. Both are now terminal: cancellation in the service, host-key denial converted by the SSH factory into the existing non-reconnectable error. - The auto-connect setting description omitted SSH, which it now gates. - Corrected the ownership split in the provider specification: the shared contribution, not the service, owns filesystem, discovery, model, terminal and authentication wiring. Also drops VERIFICATION.md from this change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Bump xterm to 6.1.0-beta.303 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3f172890-b580-41e9-a79d-c7af8ee372c2 * Enable custom glyphs in auxiliary terminals Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fd37bd2a-fb74-4d09-a0bb-64a3ac18cad4 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3f172890-b580-41e9-a79d-c7af8ee372c2 Copilot-Session: fd37bd2a-fb74-4d09-a0bb-64a3ac18cad4
Wrapping a refused SSH host key in NonReconnectableTransportError stopped the shared retry but dropped the SSHHostKeyDenied name that isSSHHostKeyDeniedError matches across IPC, so telemetry and the contribution's pause policy saw an ordinary failure. Preserve the name on the wrapper so the error is both terminal and identifiable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix missing changes for migrated legacy Copilot CLI sessions (#333642) * Feedback updates
Split Electron unit and integration test jobs Run Electron unit and node tests independently from integration tests on Linux, macOS, and Windows while preserving existing integration check and artifact names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Retries were silently disabled for on-demand kinds. Cloud sandbox and Dev Container cleared their staged connection whenever the address left `connections`, but the service withdraws an entry *before* arming a retry, so the staging a retry depends on was deleted and `_scheduleReconnect` then found nothing configured. Both kinds got exactly one attempt. The heuristic also duplicated cleanup that `_establish` already performs on the genuinely terminal paths, so remove it: staging is now cleared only by an explicit unstage. A WSL distro started outside VS Code never reconnected. A stopped distro fails terminally, so no retry stays armed, and this branch had dropped the background poll that noticed one had started. Restore it in the WSL contribution, which owns discovery; the dial still goes through the service. Turning auto-connect off disconnected live tunnels. The three tunnel factories filtered `entries` by the setting, so entries vanished and reconciliation tore the connections down. The shared service already gates tunnel dialing through `autoConnectGated`, so the filter was both redundant and harmful — a startup preference must not close a live connection. Also: - `waitForConnection` could hang forever: a discarded late dial returned without settling the address's waiter, and callers following an in-flight dial deliberately have no timeout to fall back on. - `reconnect()` withdrew and disposed an entry without announcing it, so consumers could keep integrations bound to a disposed client. - A protocol-v6 tunnel with no saved location lost its prompt on startup: cached dials are not user-initiated, so gateway selection returned nothing and was treated as terminal. Use `getAutoConnectMode`, which encodes exactly that decision. - The web tunnel factory was registered before `_discoveryProvider` was assigned, so a persisted tunnel could dial into a missing provider and fail terminally with nothing to retrigger it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ction-factories agentHost: build every remote connection through a registered factory
Avoid duplicate Copilot static checks Run platform-independent type checking and linting only in the Linux Copilot job while preserving the Windows compile and test coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A canceled setInput operation can outlive editor cleanup and try to resolve an already disposed SessionChangesEditorInput. Stop before model resolution and cover the race with a regression test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove unused inline edits displayLine setting Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Increase the history depth to 150 and skip screenshot comparison with an explicit warning if the merge base is still unavailable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7defd909-5470-4f45-97dc-09706d9c6570
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Split Electron unit and integration test jobs Run Electron unit and node tests independently from integration tests on Linux, macOS, and Windows while preserving existing integration check and artifact names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Limit test runtime downloads Download Electron only for Electron and Remote jobs, and install only the headless Playwright browsers used by Browser jobs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Align browser setup with split test inputs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
#333727) * First cut of GrepResultService * Wire up a TS7 implementation * WIP * WIP * WIP * Add TS6 implementation * WIP * WIP * Align implementation * Polish telemetry and fix tests * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Enable developer action only for typescript and javascript * More review comments fixed * Handle dispose correctly * Correct parent for property access --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: vs-code-engineering[bot] <vs-code-engineering[bot]@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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )