Render sandbox command approvals as terminal confirmations - #333883
Merged
Osvaldo Ortega (osortega) merged 3 commits intoSep 2, 2026
Merged
Render sandbox command approvals as terminal confirmations#333883Osvaldo Ortega (osortega) merged 3 commits into
Osvaldo Ortega (osortega) merged 3 commits into
Conversation
Copilot started reviewing on behalf of
Osvaldo Ortega (osortega)
September 1, 2026 22:11
View session
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Remote write labels and cross-platform paths remain incorrect, and newly editable inputs are silently ignored.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 3
New issues introduced by this change (3)
| Severity | Finding |
|---|---|
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts — The normalized write data does not update the invocation label. AgentSessionApprovalModel reads… |
|
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts — This converts a remote filesystem path with URI.file inside getEditFileMessage, whose separator… |
|
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationSubPart.ts — This makes the JSON editor appear for plain-string Agent Host confirmations, but that editor is… |
What changed in this PR
Improves Agent Host sandbox permission prompts so command and file approvals use specialized renderers.
Changes:
- Infers rendering metadata from remote permission requests.
- Normalizes write-permission messages and inputs.
- Adds regression tests for command, path, and write prompts.
| File | Description |
|---|---|
stateToProgressAdapter.test.ts |
Tests remote permission rendering. |
chatToolConfirmationSubPart.ts |
Renders structured input alongside plain messages. |
stateToProgressAdapter.ts |
Adapts command and write permission metadata. |
copilotToolDisplay.ts |
Reuses shared edit-message formatting. |
streamingToolCallDisplay.ts |
Adds shared edit-message helper. |
sessionReducers.ts |
Infers tool kinds from permission metadata. |
agentPermissionRequestMeta.ts |
Parses remote permission metadata. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Translate the write permission's invocationMessage, not just the confirmation message. AgentSessionApprovalModel reads invocationMessage for non-terminal approvals, so the sessions list showed the host's generic "Edit file" while the confirmation card showed the file. Both now derive from the same value. Normalize a Windows host path before formatting the link. URI.file only rewrites separators when the client runs Windows, so a Windows host paired with a non-Windows client collapsed C:\repo\file.ts into a single-segment basename and an unresolvable URI. Revert the chatToolConfirmationSubPart change. It surfaced the raw input for plain-string confirmations, but that editor is writable while agent-host approval dispatch never sends editedToolInput, so edits were silently discarded. Translating the message removes the need for it, and this no longer touches shared rendering code. Also stop emitting the raw input for writes: it repeated the file the message already names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Osvaldo Ortega (osortega)
marked this pull request as ready for review
September 1, 2026 23:22
The write path restated the host's file name as a markdown link so the confirmation card and sessions list would show a file pill. That duplicates information a host can supply directly, so it would have become dead code rather than a lasting fix. Tracked separately with the agent host team. The command path stays. It is not compensating for a host deviation: the Agent Host Protocol has no field that identifies a pending tool call as a shell command, so `_meta.toolKind` (a VS Code-private hint a remote host has no reason to set) is the only signal, and without the fallback a sandbox command approval shows the agent's intention instead of the command being approved. Reverts the getEditFileMessage extraction, the write branch and its Windows path normalization, the write fields on the permission meta reader, and the three write tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Osvaldo Ortega (osortega)
force-pushed
the
osortega/agents/sandbox-permission-prompt-review
branch
from
September 1, 2026 23:24
4bec2ea to
8a642e4
Compare
Osvaldo Ortega (osortega)
enabled auto-merge (squash)
September 2, 2026 00:02
roblourens
approved these changes
Sep 2, 2026
Osvaldo Ortega (osortega)
deleted the
osortega/agents/sandbox-permission-prompt-review
branch
September 2, 2026 02:02
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.

Sandbox command approvals render as a plain text card instead of the terminal confirmation, so the user approves a command they cannot see.
Before / after
A shell-command permission request from a cloud sandbox session:
shellscript.Why it happens
isTerminalToolCallhas three ways to recognize a terminal call, but two of them only apply once the tool isRunning/Completed. At confirmation time — the moment that matters — the only live signal is_meta.toolKind === 'terminal'.toolKindis a VS Code-private rendering hint, not part of the Agent Host Protocol. Our owngetToolKindsays so: "This is not part of the protocol and is injected by the agent adapter." Our in-process adapters (mapSessionEvents.ts,claudeToolDisplay.ts) stamp it; a remote agent host is a separate implementation and has no reason to know about it. With notoolKind, the call falls through to the generic tool card.Note the contrast with subagents:
isSubagentToolalready has a tool-name fallback for exactly this "server didn't send_meta" case. Terminal had none.The change
getToolKindfalls back to the permission metadata a remote host does provide on a pending confirmation, mapping it to the rendering hint:terminalreadEverything downstream is untouched: the existing terminal confirmation renderer takes over, reading the command from
toolInput, which was already available. Putting the fallback ingetToolKindrather than inisTerminalToolCallmeans all eight call sites agree — confirmation, live rendering, and history replay.Path-batched requests are deliberately not mapped to
terminal. Their subject is a list of paths rather than a command line, so rendering one as a command would misdescribe what is being approved. Covered by a regression test.Why this isn't temporary
The Agent Host Protocol has no field that identifies a pending tool call as a shell command — not
toolName,intention,confirmationTitle, or anything else onToolCallPendingConfirmationState. The only in-protocol signal is aTerminalcontent block, which does not exist until the tool is already running, i.e. after approval. So this gap is not something a host-side change alone can close today, and the fallback is expected to be long-lived.An earlier revision of this PR also adapted file-write permissions (restating the target file as a markdown link so the card and sessions list showed a file pill). That has been removed: it duplicated information a host can supply directly, so it would have become dead code rather than a lasting fix. Tracked separately with the agent host team.
Testing
Three unit tests in
stateToProgressAdapter.test.ts, built from a real pending-confirmation payload:Verified the first two fail without the change. Full suite: 814 passing across the adapter, chat contribution, client tools, confirmations, approval model and tool-display suites; typecheck and layer checks clean.
Risk
The fallback only fires when
_meta.toolKindis absent, which is only ever a remote host — local sessions cannot reach it. No shared VS Code rendering code is modified; the adapter's own diff is a documentation comment plus the delegation togetToolKind.