[pull] main from microsoft:main - #1602
Merged
Merged
Conversation
`text-transform: capitalize` on the shared group header re-cased strings that were already cased correctly, and got them wrong: "Included Based on Context" rendered as "Included Based On Context" and "Loaded on Demand" as "Loaded On Demand", against this repo's own rule that short prepositions stay lowercase, and "Built-in" rendered as "Built-In". Per-word capitalization is also not a transform that survives translation, so the rule was wrong for every locale rather than just awkward in English. The header is shared, so this reaches every customizations tab -- Agents, Skills, Instructions, Hooks, Prompts, Plugins and MCP Servers. Every group label on those tabs is already a correctly cased localized string, so the only rendered differences are the three the transform was getting wrong: four screenshot baselines change, on the Instructions and MCP Servers tabs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
`updateStatus` began by clearing the row's action disposables and emptying its actions node, then rebuilt them. It runs from an autorun over the server's connection state, and an erroring server re-runs it about twice a second while producing byte-identical content: measured in a real Code OSS build, 9-10 rebuilds per 5 seconds, every one of them a no-op. A DOM node replaced between mousedown and mouseup never receives the click, so the inline `Show Output` button did nothing on precisely the rows that needed it -- the failing ones, which are the only rows that offer it at all. The row's actions are now rebuilt only when something about them changed. `getMcpStatusRenderSignature` reduces them to a comparable value covering both what they render and what they act on; leaving anything out would drop an update that matters, so it is a pure exported function whose test fails to compile if a field is added without being covered. The list re-splices on every customizations change, so `renderElement` would otherwise undo this by clearing the actions itself. It now keys on the row's content identity rather than the entry object, which is recreated on every refresh and therefore says nothing about whether this is the same row. Both guards are pinned by tests that drive the renderer itself, since that is the only place the failure is observable: an erroring server's update is fired repeatedly with unchanged content, and the button must be the same node afterwards, still attached, and still reach `showMcpServerLog` when clicked. A third test asserts a real status change does still rebuild, so the guard cannot be satisfied by never updating. Reverting either guard fails them. `McpServerItemRenderer` is exported for this. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A session can carry two customizations for a single MCP server. The agent host
publishes the declaration as a child of whatever declared it -- a plugin, or
the .mcp.json VS Code syncs into the agent -- and separately mints a top-level
customization for any server the SDK reports before that child can be resolved
by name. `McpCustomizationController._applyOne` never retires the minted entry
once the child becomes resolvable ("Once promoted to a top-level entry, stay
top-level for the session"), so both remain in state:
notion -> file:///.../vscode-synced-customization-.../.mcp.json#mcp=notion
state: stopped <- the declaration
notion -> mcp-top-level:copilotcli:<session>:notion
state: ready, channel: mcp:// <- the live one
Every consumer of getMcpServers saw both, so the servers list rendered the same
server twice with contradictory status. It was worse than a repeat: the list's
matcher only matches when exactly one candidate answers a key, so with two
copies the server's local row could not adopt either, and both fell through as
extra rows.
getMcpServers now drops a child that a top-level customization already speaks
for. The top-level copy wins because it is the one the host treats as live: it
carries the running state and channel, and its id is what the host resolves for
lifecycle and enablement. Position in the tree is the signal, not the shape of
the minted id, which is the host's own business -- and not the absence of an
owning plugin either, since a directory-declared child has none. That last one
is a live trap rather than a hypothetical, so it has its own test.
Nothing else is collapsed. Two plugins that each declare a server named
`search` stay two rows, because they are two servers. Only the presentation
path dedupes; log, diagnostics and id lookups still walk every customization,
so an id from either copy continues to resolve.
The host-side repair is tracked in #330893.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…hat-prompt-race chat: fix signed-in model prompt on startup
* sessions: Synchronize live last turn changes Use active-turn chat edits for Last Turn Changes until the finalized host changeset is available, and route the live changes pill to the same changeset. Keep historical turn selections workspace-scoped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * sessions: Address live turn review feedback Keep live turn changes until the finalized host changeset is ready, tighten workspace filtering, and prune per-chat observable cache entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A provider can end a Responses stream with a terminal event whose error
object omits the code and message the API contract requires. Serializing
that object produced {"code":0,"message":"","metadata":{}}, which the BYOK
endpoint promotes to the entire user-facing reason, so an outage and a
malformed request looked identical and neither users nor maintainers could
tell them apart.
Describe such failures instead, naming the terminal event, response status,
and response id so the failure stays correlatable with the provider, and
omit absent metadata keys so serialization cannot collapse them to an empty
object. Keep the fetcher's original reason when a stream error carries no
message, so a hollow struct never replaces an actionable one.
The mapping predates the reports; the Agent Host BYOK proxy moving from
Chat Completions to Responses in 1.132.0 is what first exposed it in the
Agents Window. This restores diagnosability rather than stopping the
upstream failure, so it does not close the report on its own.
Related to #330408
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Route both synthesized failure descriptions through @vscode/l10n with placeholders for the provider code and diagnostic details, matching the localized endpoint error pattern in imageLimits.ts. These strings reach the user as the BYOK failure reason, so a non-English install should not receive untranslated text. The bracketed identifiers stay verbatim so they remain greppable and pasteable into a provider support request. Revert the Agent Host integration test expansion. It exercised parameterless tool schemas and tool-call continuations, which ruled out two alternative causes while investigating, but no production tool handling changes here and that coverage does not validate this fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two ways a failed browser tool still read as success. `ChatToolInvocation.didExecuteTool` only replaced the past-tense message when the result carried one of its own, so a tool that failed kept the message prepared before it ran. A screenshot that failed with "No browser page found" still rendered as "Captured browser screenshot". Drop the prepared message on failure and let the error stand on its own. `invokeFunctionResultToToolResult` recorded an evaluated-code error only in `toolResultDetails.isError`, so the tool call itself was still reported as successful. Set `toolResultError` as well.
Review feedback: the codebase sets a past-tense message for a failed tool call rather than clearing it — the codex mapper does it per result, and the agent host does it for client tool calls that outlive their client. Doing it in ChatToolInvocation changed the past-tense behavior of every tool that reports an error in order to fix browser ones. The browser helpers now supply the failure label alongside the error, so the shared model is untouched. These tools declare only an invocationMessage and no pastTenseMessage, which is why a failed call reused the present-tense one and read as a success. The error check is on presence rather than truthiness, so an empty message from `throw ''` or `new Error()` still reports a failure. Coverage moves to browserToolHelpers, which nothing exercised before.
* agentHost: make startup session discovery registry-first (#331155) A 2026-08-17 Insiders reproduction with 941 SDK sessions and 1,437 per-session databases (~946 MB) showed the ~60s delay is not migration — `sessionRegistryBackfilled:copilotcli` was already true and migration was skipped. The time went to per-session SQLite work during discovery and listing: classification took 66.9s (113 external, 342 adoptable extension-host, 486 already known), registration of the 113 emitted candidates another 21.9s, and the first two AHP `listSessions` calls 109.8s and 91.0s, with a third 73.2s window after discovery. Each measured phase is addressed: - Classification (66.9s): discovery is now registry-first. `IAgent` gains an optional `setKnownSessionsFilter` seam that `AgentService` installs at provider registration; it answers, in one registry query for the whole candidate set, which sessions the host already owns. `CopilotAgent` drops those candidates instead of opening a session database each, so the 486 already-known sessions cost no DB opens. Tombstoned sessions are absent from the registry and therefore never reported as known, so an explicitly deleted session still reaches `register`, whose atomic tombstone check declines it. Provenance of a registered row stays owned by the explicit create/restore paths. - Classification (the 342 adoptable rows): while migrate-legacy is off, adoptable extension-host candidates are never emitted, so their Git-touching project resolution is now skipped entirely instead of being computed and then filtered away. `_emitCopilotChats` keeps its filter as a re-check, since the setting can flip mid-pass. - Registration (21.9s): `_registerDiscoveredChats` rejects an already-registered candidate with unchanged provenance before `_isChatBacking()` or any other per-session I/O. - Listing (109.8s / 91.0s / 73.2s): `listSessions()` coalesces concurrent computations per external-sessions mode, so the burst a multi-window restore produces shares one registry traversal. The shared entry records the registry epoch it started at and every registry mutation invalidates it, so a caller arriving after a mutation starts a fresh pass rather than joining a possibly pre-mutation one. Each caller gets its own array; rejections are shared only with callers already waiting. - `_readStoredSessionMetadata` / `_readSessionMetadata` now issue one bulk `getMetadataObject()` query instead of nine and six single-key reads, shrinking the cost of the fallback path that runs when no host filter is installed. Deferred deliberately: seeding a newly discovered external session as read still creates its database purely to hold one flag. Dropping the write without a durable default would flip every discovered external session to unread, because the list overlay only applies `IsRead` when the key is present. The correct default belongs on the registry row and is left to the registry list-projection change; the reasoning is recorded on `_initializeExternalSessionReadState`. Removal of `_awaitInitialProviderMigration()` is likewise not attempted: the reproduction proves migration was already skipped. Tests assert call/open counts rather than wall-clock thresholds: registry-known candidates cause zero session DB opens; disabled adoptable candidates resolve no projects; re-registering a known discovered chat performs no per-session I/O; the known-sessions filter reports registered sessions only and leaves tombstones to registration; concurrent list calls share one computation but not their arrays; a mutation during an in-flight list is not served from it; and stored metadata is read with a single bulk query. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: address discovery review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: update discovery perf fixture Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…byok-responses-error chat: describe BYOK Responses failures that omit error details
* agentHost: scope server tools to exact chats Pass exact Agent Host chat channels through every provider and derive owning session context centrally for session-scoped tools. This fixes rename_chat targeting for Claude and Codex peer chats while preserving Agent Merge and feedback behavior.\n\nFixes #331165\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: test Claude peer server tool routing Capture Claude SDK server-tool handlers in the provider test double and verify a materialized peer chat forwards its exact Agent Host chat URI.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix Anthropic refusal error handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Handle refusal edge cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Simplify refusal error handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Avoid logging refusal explanations Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Allow local dictation without Copilot entitlement Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d367acf7-ee13-488e-bf9f-4aef2d274190 * Allow cloud dictation without paid entitlement Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d367acf7-ee13-488e-bf9f-4aef2d274190 --------- Copilot-Session: d367acf7-ee13-488e-bf9f-4aef2d274190
…orting Do not present a failed tool call as a successful one
* Scope dictation indicator to owning chat Fixes #331206 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d367acf7-ee13-488e-bf9f-4aef2d274190 * Track dictation editor ownership changes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d367acf7-ee13-488e-bf9f-4aef2d274190 * Register dictation service in inline chat fixtures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d367acf7-ee13-488e-bf9f-4aef2d274190 --------- Copilot-Session: d367acf7-ee13-488e-bf9f-4aef2d274190
* Render quick chats consistently in automation history * Announce quick chats in automation history * Test quick chat presentation in flat session lists * Clarify session status timestamp behavior * signing commit --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* agenthost: speed up rename chat tool Avoid enumerating every session when resolving a rename target. Add a targeted registry lookup so rename validation and persistence remain awaited while lookup cost stays constant. Fixes #331110 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agenthost: share live session metadata overlay Reuse one helper for targeted session lookup and listSessions so both paths apply live state consistently. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agenthost: make rename chat non-blocking Return to the agent before session lookup and persistence complete. Serialize queued renames and log deferred failures while retaining targeted session resolution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agenthost: update merged rename test Wait for the background rename independently while expecting the immediate tool result. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agenthost: simplify background rename Start each rename independently without a sequencer and keep the existing tool description unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- 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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
12 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 : )