[pull] main from microsoft:main#1207
Merged
Merged
Conversation
Two regressions from the merge of #287050: 1. preview.ts: The merge retained `this.#isScrolling = false` inside the early-return guard of `scrollTo()`, which was intentionally removed in the original PR. This resets the timer-based flag on the very first forward-sync call, allowing subsequent editor scroll events to re-trigger forward sync while reverse sync is in progress, causing the editor to jump back up. 2. index.ts: The PR converted `onUpdateView` from a decrement-counter to a timer-based approach but left initialization and resize handlers still using `scrollDisabledCount += 1` without a corresponding timer reset. The old scroll handler decremented the counter naturally; the new handler only returns early. As a result, after page load `scrollDisabledCount` stays at 1 indefinitely, blocking all preview-to-editor sync until the user scrolls the editor once. Fixes: - Remove the erroneous `this.#isScrolling = false` from scrollTo() - Apply the same timer-reset pattern (200ms) to initialization and resize handlers so scrollDisabledCount is always auto-cleared Fixes #307762
…l-sync-regressions markdown: fix scroll sync regressions introduced in #287050
… in web (#313575) * chat: add real BrowserPluginGitCommandService for adding plugins in web Replaces the throwing stub at src/vs/workbench/contrib/chat/browser/pluginGitCommandService.ts with a real implementation that lets browser/web clients install agent plugins from public (and authenticated) GitHub repositories without a local git binary or AHP server. How it works - Resolve the requested ref to a commit SHA via GitHub's /repos/{owner}/{repo}/commits/{ref} endpoint. - Download the tarball at that SHA, decompress with the platform DecompressionStream, and stream-extract the USTAR archive directly into the workbench virtual file system at the caller's targetDir. The standard GitHub-archive wrapper directory ({repo}-{shortSha}/) is stripped so consumers see a clean tree, and any prior contents are wiped first so files removed upstream don't linger. - Persist {owner, repo, ref, sha, fetchedAt} per-target via IStorageService (chat.plugins.browserCache.v1). This lets revParse() answer locally and lets pull()/checkout() short-circuit when the upstream SHA matches the cached one -- which also feeds CustomizationRef.nonce so the AHP server's plugin manager dedupes. - Best-effort silent IAuthenticationService lookup attaches a GitHub token when one is already available, enabling private-repo installs; public repos still work with no session. 401/403 surfaces a typed GitHubAuthRequiredError so future UI can drive sign-in. - checkout() handles SHA-pinned plugin sources (the AbstractGitPluginSource path): no-op when the SHA matches, otherwise fetches the tarball at the requested SHA. Branches/tags/short SHAs resolve through the commits API. - Non-GitHub clone URLs throw an actionable localized error directing users to the desktop client or a remote agent host. - TAR extraction validates entry paths (rejects '.', '..', empty, NUL, leading-slash segments and double-checks isEqualOrParent) so a malicious archive cannot escape targetDir. The DI singleton registration in chat.contribution.ts already wires IPluginGitService -> BrowserPluginGitCommandService; the new constructor parameters are injected automatically. Tests - New src/vs/workbench/contrib/chat/test/browser/pluginGitCommandService.test.ts covers URL parsing (canonical / trailing-slash / extra-segment / malformed), tarball fetch+extract, no-op pull on SHA match, re-download on SHA change, stale-file cleanup, path-traversal entry rejection, checkout no-op / re-extract / no-metadata, and the auth-required error path. Test fixtures build a minimal valid USTAR + gzip via CompressionStream so bytes round-trip through the production DecompressionStream. Reuse notes - Uses isSuccess / isClientError / asJson from platform/request rather than rolling status-code checks. - Uses dirname / isEqualOrParent / joinPath from base/common/resources for path arithmetic and traversal defence. - GitHubApiClient (sessions/contrib/github) was considered but is layering-isolated, JSON-only, and forces sign-in -- wrong semantics for best-effort silent auth and binary tarball download. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chat: address council review on browser plugin git service Council review surfaced ~12 follow-ups across correctness, security, and parser robustness. This commit addresses the actionable ones. Correctness - Stage extraction in a sibling `.staging-{uuid}` directory and swap into place via `IFileService.move(..., overwrite=true)` only on success. If anything throws (network, gunzip, malformed tar, cancellation, FS write error), the staging dir is cleaned up and the existing `targetDir` is left untouched. Previously the target was wiped *before* extraction began, so a mid-flight failure left the persisted SHA cache pointing at an empty directory. (consensus C1) - `pull()` now wraps its catch block with `_maybeLogTransientError` for parity with `cloneRepository` / `checkout`. (S6) - `revParse(repoDir, ref)` no longer silently ignores `ref`: when asked for a 40-hex SHA that does not match the cached one, it throws instead of lying. (S3) Rate-limit detection - New `GitHubRateLimitError`, distinct from `GitHubAuthRequiredError`, thrown when GitHub returns 403 with `X-RateLimit-Remaining: 0` or a `Retry-After` header. Higher-layer UI can present "wait" rather than "sign in". `_maybeLogTransientError` logs the retry-after window. (C2) Auth + redirects - Drop the dead `followRedirects: 5` option (browser fetch ignores it per IRequestService impl). Add a comment on `fetchAndExtractGitHubTarball` explaining the codeload signed-URL flow: GitHub's tarball endpoint 302s to a URL whose authorization is encoded in the URL itself, so private-repo downloads survive the cross-origin Authorization-header strip. (C3 cleanup) - Document the multi-account auth-session selection limitation in `_lookupGitHubToken` rather than try to solve it here -- account selection is the auth provider's responsibility. (C6) Parser robustness - `readOctal` -> `readNumericField`. Now handles: - leading whitespace (legal POSIX padding) -- previously truncated to 0 - GNU base-256 binary encoding (high bit of byte 0 set) -- previously silently mis-aligned subsequent block offsets - Invalid fields throw rather than silently returning 0, so corrupt tarballs surface as errors instead of producing empty entries. - USTAR `prefix` field now joined unconditionally per spec (`${prefix}/${name}`); previous heuristic skipped the prefix when `name.startsWith(prefix)` which is non-standard. The GNU LongLink path correctly bypasses prefix join via a `fromLongLink` flag. (C5) - `stripArchiveRoot` rejects absolute paths instead of silently rebasing them under `targetDir`. (S4) - `safeJoinUnderTarget` also rejects backslash-bearing segments to defend against Windows-style separators on tar entries that would escape `targetDir` when materialised through a Windows AHP server's `agent-client:` provider. (S1) URL parsing - Reorder normalisation in `parseGitHubCloneUrl` so trailing slashes are stripped before the `.git` suffix; URLs like `https://github.com/o/r.git/` now parse correctly. (S2) Cache hygiene - Cache-key on `getComparisonKey(targetDir, ignoreFragment=true)` instead of `URI.toString()`, so callers passing equivalent URIs with different trailing-slash / percent-encoding don't silently miss the cache. (S5) - On first cache load, kick off a fire-and-forget sweep that drops entries whose `targetDir` no longer exists on disk. Bounds the storage map size when external code (e.g. `cleanupPluginSource`) deletes a plugin directory without notifying us. (C4) Tests - Rate-limit (`GitHubRateLimitError`) on `403 + X-RateLimit-Remaining: 0`. - Failed extraction leaves the previous `targetDir` intact and the cache reporting the previous SHA. - Backslash-traversal entry rejected. - USTAR prefix split + GNU LongLink paths via a new `makeGzippedTarWithSpecial` test fixture. - `parseGitHubCloneUrl` accepts `https://github.com/o/r.git/`. - `revParse` throws on unrelated full SHA, accepts cached one. Council items not addressed in this commit - Multi-account session selection (C6): documented as a VS Code-wide auth UX concern, not a plugin-git issue. - Cross-origin redirect end-to-end test (C3): the unit-test stub doesn't simulate redirects; the fix is a real-world smoke test against vscode.dev which is out of scope for this commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chat: keep default plugin toolbar actions * chat: refine plugin add actions * chat: reuse signed in github session for plugin clone * chat: fallback to anonymous plugin clone * chat: fetch plugin repos via tree+raw to bypass CORS GitHub's /tarball/ endpoint 302-redirects to codeload.github.com, which returns no CORS headers. Browser fetch() therefore fails the preflight check with TypeError: Failed to fetch before any of the existing auth-retry logic in BrowserPluginGitCommandService can run, so even public repos with a signed-in session cannot be installed. Replace the tarball flow with two CORS-friendly endpoints: - GET /repos/{owner}/{repo}/git/trees/{sha}?recursive=1 returns the full file listing. - GET https://raw.githubusercontent.com/{owner}/{repo}/{sha}/{path} returns each blob's bytes (and accepts the same Authorization header for private repos). Drop the now-unused gunzip + tar parser. Update tests to stub the new tree + raw responses instead of building synthetic gzipped tar archives. * chat: fetch plugin blobs via api.github.com to bypass CORS raw.githubusercontent.com refuses the OPTIONS preflight that an Authorization: Bearer header forces, so the previous tree+raw flow still failed the CORS check (TypeError: Failed to fetch). Switch the per-blob download to api.github.com's /git/blobs/{sha} endpoint, which is properly CORS-enabled and accepts auth headers. The blob SHA already comes back from the tree response, so no extra round-trips are needed; content is base64-encoded JSON which we decode via decodeBase64. Also add a loggedRequest wrapper that re-throws transport-level errors with the URL we were trying to reach. Without it, browser fetch CORS / DNS failures bubble up as a bare 'TypeError: Failed to fetch' that hides which request actually failed. Surface the same context through _maybeLogTransientError in the install path. * chat: drop unused bytesResponse test helper * chat: rename githubTarballFetcher to githubRepoFetcher The file no longer contains tarball logic — it fetches the repo tree and individual blobs via api.github.com. Rename to match. * chat: trim verbose comments in plugin git service * chat: tidy plugin git service auth ladder and comments - Flatten the nested try/catch in cloneRepository into a linear loop over the auth-ladder rungs (signed-in token → anonymous → fresh repo session). - Trim further verbose comments in the fetcher and cache helpers. * chat: surface locally-installed plugin items in remote-harness view When the active harness has both an itemProvider and a syncProvider (remote agent host), fetchItems blends remote items with local items. The local pass only included PromptsStorage.local / .user files, so files contributed by locally-installed agent plugins (e.g. one just cloned into vscode-userdata:/User/agent-plugins/...) never reached the customizations UI. Widen the local pass to also include PromptsStorage.plugin files. Plugin files are not individually syncable — the plugin is the unit of sync — so they're returned without the syncable marker. They still get the right grouping via the normalizer's plugin-URI check. * chat: refresh customizations debug output - Stage 6: render fromMarketplace as name@version (marketplace, type) instead of [object Object]. - Stage 3: surface syncable count and per-item syncable / pluginUri flags so the local syncable vs locally-installed plugin split (added in fetchLocalSyncableItems) is visible. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <copilot@github.com>
The local agent host starters (Electron utility process and Node child
process) only forwarded --logsPath. Inside agentHostMain.ts, parseArgs
therefore never saw --user-data-dir, so the agent host's
NativeEnvironmentService always resolved userDataPath to the platform
default for the build's quality.
As a result, all agent host state -- SessionDataService
({userData}/agentSessionData), AgentPluginManager
({userData}/agentPlugins), and the root agent-host-config.json under
appSettingsHome -- was written to the default location instead of the
custom user data dir the parent app was launched with.
Forward the parent process's resolved userDataPath to the child as
--user-data-dir so the child's parseArgs picks it up and all derived
paths live inside the custom dir.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* agent-host: restore sessions without listing catalog Use direct provider metadata lookup when restoring an existing session so local Agent Host session switches do not block on enumerating the full Copilot session catalog. Keep the catalog fallback for providers without the direct path or transient direct lookup failures. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agent-host: preserve protocol error data on restore fallback Keep structured protocol error data when wrapping a failed catalog fallback after direct metadata restore fails. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* sessions: remember tunnel disconnects Persist explicit tunnel disconnects so startup and background reconnect paths skip those tunnels until the user connects again. Route tunnel remove/reconnect UI through the provider lifecycle so the suppression state is updated consistently. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * sessions: hide disconnected tunnels Keep user-disconnected tunnels out of the workspace picker until the user explicitly reconnects them, and route inline remove actions through the same remote removal helper as the options picker. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The shared ActionList renderer falls back to a "{keybinding} to Apply"
tooltip when an item has no explicit tooltip/hover. That hint is misleading
in the session workspace picker, where items aren't code actions.
Add a new IActionListOptions.hideDefaultKeybindingTooltip flag and opt the
session workspace picker into it (for both the flat and tabbed
presentations).
(Written by Copilot)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* chat: allow text selection in tool confirmation message The tool confirmation widget lives in the chat input area, which inherits user-select: none from the list widget. Override it on the message body so users can select and copy text like file paths shown in tool confirmations. (Written by Copilot) Co-authored-by: Copilot <223556219+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> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…313837) The Copilot SDK spills oversized tool results to a file under os.tmpdir() (named like `copilot-tool-output-*.txt`) and asks the model to read it back in a follow-up turn. These reads were prompting for permission even though the file was just written by the SDK on our behalf. Mirrors the existing session-state auto-approval pattern: only auto- approves `read` requests whose path lives directly in os.tmpdir() and whose basename matches the SDK's two known naming layouts. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* agent host: eagerly create sessions when a folder is picked
Previously the agent-host backend session was only created when the user
sent their first message. Move creation up to folder-pick time so the
new-chat view interacts with a real session URI throughout: model
selections, session-config picks, etc. all dispatch against a known
session, and the chat handler avoids a duplicate createSession round-trip
on the user's first send.
This is particularly in advance of support for completions, which necessarily
are derived in the context of a workspace, agent customizations, etc. While
the work for these new 'provisional sessions' is trending towards gnarly
internally, the protocol is kept pretty clean with the only change being
semantic guidance that sessions with no messages should be garbage collected.
To make eager creation cheap, sessions are ephemeral in the agent
until the first message lands — no SDK session, no worktree, no on-disk
metadata. Materialization happens inside `sendMessage` and fires
`onDidMaterializeSession`, at which point the agent service emits the
deferred `notify/sessionAdded` and transitions lifecycle to `Ready`.
Switching workspaces or closing the new-chat view disposes the
provisional record (`disposeSession` over the wire); a 30s server-side
empty-session GC backstops crashes and dropped disconnects.
The new-session bookkeeping in the sessions provider previously lived
across 11 loose `_currentNewSession*` fields and 4 keyed-by-sessionId
maps. Bundle them into a single `NewSession` class held as a
`MutableDisposable` — assigning a new value automatically tears down
the previous one (subscription release + disposeSession RPC).
Subtle wire-ordering note: `NewSession.eagerCreate` awaits
`createSession` *before* opening the state subscription. Reversing the
order races the wire — the server sees `subscribe` for an unknown
session, returns `AHP_SESSION_NOT_FOUND`, and the client subscription
enters an unrecoverable error state. New unit tests pin the ordering
and the bail-out behaviour for workspace-switch-mid-flight.
---
Architecture (provisional → real session):
```mermaid
sequenceDiagram
participant U as User
participant SP as SessionsProvider
participant H as ChatHandler
participant A as Agent (CopilotAgent)
participant S as StateManager
U->>SP: pick folder
SP->>A: createSession(uri)
A->>S: createSession(emitNotification=false)
Note over A: provisional record<br/>(no SDK, no worktree, no DB)
SP-->>U: session.resource
U->>SP: pick model / config (optional)
SP->>A: SessionModelChanged / SessionConfigChanged
Note over A: updates provisional record
U->>H: send first message
H->>A: sendMessage()
A->>A: _materializeProvisional<br/>(create worktree,<br/> SDK session, persist DB)
A->>S: onDidMaterializeSession
S-->>U: notify/sessionAdded
S->>S: SessionReady
U-)SP: switch workspace
SP->>A: disposeSession (old uri)
Note over A: drops provisional record<br/>cancels worktree creation
```
* review
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.
8 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 : )