Skip to content

Commit 8a642e4

Browse files
osortegaCopilot
andcommitted
Drop the write permission adaptation
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>
1 parent 965b5fb commit 8a642e4

5 files changed

Lines changed: 15 additions & 153 deletions

File tree

src/vs/platform/agentHost/common/meta/agentPermissionRequestMeta.ts

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,10 @@ export const enum AgentPermissionRequestKind {
2626
Commands = 'commands',
2727
/** Read a single file. */
2828
Read = 'read',
29-
/** Write to a single file. */
30-
Write = 'write',
3129
}
3230

3331
export interface IAgentPermissionRequestMeta {
3432
readonly kind?: AgentPermissionRequestKind;
35-
/** Absolute path of the file a write request targets. */
36-
readonly fileName?: string;
3733
}
3834

3935
/**
@@ -50,17 +46,16 @@ function normalizeKind(value: unknown): AgentPermissionRequestKind | undefined {
5046
return AgentPermissionRequestKind.Commands;
5147
case 'read':
5248
return AgentPermissionRequestKind.Read;
53-
case 'write':
54-
return AgentPermissionRequestKind.Write;
5549
default:
5650
return undefined;
5751
}
5852
}
5953

60-
function readRequest(value: unknown): Record<string, unknown> | undefined {
61-
return value && typeof value === 'object' && !Array.isArray(value)
62-
? value as Record<string, unknown>
63-
: undefined;
54+
function readKind(value: unknown): AgentPermissionRequestKind | undefined {
55+
if (!value || typeof value !== 'object' || Array.isArray(value)) {
56+
return undefined;
57+
}
58+
return normalizeKind((value as Record<string, unknown>)['kind']);
6459
}
6560

6661
/**
@@ -75,14 +70,6 @@ export function readAgentPermissionRequestMeta(source: IHasPermissionRequestMeta
7570
if (!meta) {
7671
return {};
7772
}
78-
const prompt = readRequest(meta['promptRequest']);
79-
const permission = readRequest(meta['permissionRequest']);
80-
const kind = normalizeKind(prompt?.['kind']) ?? normalizeKind(permission?.['kind']);
81-
if (!kind) {
82-
return {};
83-
}
84-
const fileName = prompt?.['fileName'] ?? permission?.['fileName'];
85-
return typeof fileName === 'string' && fileName
86-
? { kind, fileName }
87-
: { kind };
73+
const kind = readKind(meta['promptRequest']) ?? readKind(meta['permissionRequest']);
74+
return kind ? { kind } : {};
8875
}

src/vs/platform/agentHost/common/streamingToolCallDisplay.ts

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,20 +51,6 @@ export function streamingToolTextLineCount(value: unknown): number | undefined {
5151
return typeof value === 'string' ? splitLines(value).length : undefined;
5252
}
5353

54-
/**
55-
* The message naming an edit's target file, e.g. `Edit [index.js](file:///…)`.
56-
*
57-
* Shared so every agent host presents an edit the same way: the local adapters
58-
* build it from the tool's `path` argument, and the client rebuilds it for a
59-
* remote host that names the file separately from its message.
60-
*/
61-
export function getEditFileMessage(path: unknown, resolvePath: ToolPathResolver = identityPathResolver): StringOrMarkdown {
62-
const file = formatPath(path, resolvePath);
63-
return file
64-
? markdown(localize('toolInvoke.editFile', "Edit {0}", file))
65-
: localize('toolInvoke.edit', "Edit file");
66-
}
67-
6854
export function getStreamingEditMessage(
6955
path: unknown,
7056
lineCount: number | undefined,

src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { stripRedundantCdPrefix } from '../../common/commandLineHelpers.js';
1616
import { parsePartialToolInput } from '../../common/partialToolInput.js';
1717
import { StringOrMarkdown } from '../../common/state/protocol/state.js';
1818
import { basename, extUriBiasedIgnorePathCase } from '../../../../base/common/resources.js';
19-
import { getStreamingCreateMessage, getStreamingInsertMessage, getStreamingPatchMessage, getStreamingReplaceMessage, getEditFileMessage, streamingToolTextLineCount, type ToolPathResolver } from '../../common/streamingToolCallDisplay.js';
19+
import { getStreamingCreateMessage, getStreamingInsertMessage, getStreamingPatchMessage, getStreamingReplaceMessage, streamingToolTextLineCount, type ToolPathResolver } from '../../common/streamingToolCallDisplay.js';
2020
import { getServerToolDisplay } from '../shared/serverToolGroups.js';
2121

2222
// =============================================================================
@@ -690,7 +690,10 @@ export function getInvocationMessage(toolName: string, displayName: string, para
690690
case CopilotToolName.Edit:
691691
case CopilotToolName.StrReplace: {
692692
const args = parameters as ICopilotFileToolArgs | undefined;
693-
return getEditFileMessage(args?.path, resolvePath);
693+
if (typeof args?.path === 'string' && args.path) {
694+
return md(localize('toolInvoke.editFile', "Edit {0}", formatPathAsMarkdownLink(resolvePath(args.path))));
695+
}
696+
return localize('toolInvoke.edit', "Edit file");
694697
}
695698
case CopilotToolName.Insert: {
696699
const args = parameters as ICopilotFileToolArgs | undefined;

src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts

Lines changed: 3 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ import { buildSubagentChatUri, getTurnError, isMessageHiddenFromTranscript, Mess
1717
import type { ChatInputRequestWithPlanReview, IAgentHostPlanReview } from '../../../../../../platform/agentHost/common/agentHostPlanReview.js';
1818
import { getToolKind } from '../../../../../../platform/agentHost/common/state/sessionReducers.js';
1919
import { readToolCallMeta } from '../../../../../../platform/agentHost/common/meta/agentToolCallMeta.js';
20-
import { AgentPermissionRequestKind, readAgentPermissionRequestMeta } from '../../../../../../platform/agentHost/common/meta/agentPermissionRequestMeta.js';
21-
import { getEditFileMessage } from '../../../../../../platform/agentHost/common/streamingToolCallDisplay.js';
2220
import { getChatErrorDetailsFromMeta, IChatErrorContext } from '../../../common/chatErrorMessages.js';
2321
import { AGENT_HOST_SCHEME, createAgentHostResourceUriMapper, type IAgentHostResourceUriMapper, toAgentHostContentUri, toAgentHostUri } from '../../../../../../platform/agentHost/common/agentHostUri.js';
2422
import { AgentHostElementAttachmentDisplayKind, getElementAttachmentCorrelationId } from '../../../../../../platform/agentHost/common/meta/agentElementAttachments.js';
@@ -2334,9 +2332,7 @@ export function toolCallStateToInvocation(tc: ToolCallState, subAgentInvocationI
23342332
};
23352333
} else if (getToolKind(tc) === 'terminal' && getInlineToolInput(tc.toolInput)) {
23362334
toolSpecificData = buildTerminalToolSpecificData(tc, sessionResource);
2337-
} else if (!remoteWriteInvocationMessage(tc)) {
2338-
// A write's only argument is the file the message already names,
2339-
// so there is nothing left for the raw input view to add.
2335+
} else {
23402336
const toolInput = getInlineToolInput(tc.toolInput);
23412337
if (toolInput) {
23422338
let rawInput: unknown;
@@ -2347,7 +2343,7 @@ export function toolCallStateToInvocation(tc: ToolCallState, subAgentInvocationI
23472343

23482344
return new ChatToolInvocation(
23492345
{
2350-
invocationMessage: stringOrMarkdownToString(remoteWriteInvocationMessage(tc) ?? tc.invocationMessage, connectionAuthority),
2346+
invocationMessage: stringOrMarkdownToString(tc.invocationMessage, connectionAuthority),
23512347
confirmationMessages,
23522348
presentation: ToolInvocationPresentation.HiddenAfterComplete,
23532349
toolSpecificData,
@@ -2413,28 +2409,6 @@ export function toolCallStateToInvocation(tc: ToolCallState, subAgentInvocationI
24132409
return invocation;
24142410
}
24152411

2416-
/**
2417-
* The invocation message for a remote host's write permission.
2418-
*
2419-
* A remote host names the target file in `_meta` and sends a plain-text
2420-
* message, where a local host sends the shared `Edit <file link>` message.
2421-
* Restating it in that form is what gives both hosts the same file pill, in
2422-
* the confirmation card and anywhere else the message is summarized.
2423-
*
2424-
* The path is normalized the same way {@link parseAbsoluteFileLinkTarget}
2425-
* normalizes a remote link: `URI.file` only rewrites separators when the
2426-
* *client* runs Windows, so a Windows host paired with a non-Windows client
2427-
* would otherwise keep `C:\repo\file.ts` verbatim and yield a single-segment
2428-
* basename. The link itself stays a plain `file:` URI, exactly as a local host
2429-
* emits it; {@link stringOrMarkdownToString} rewrites it to address the host.
2430-
*/
2431-
function remoteWriteInvocationMessage(tc: ToolCallPendingConfirmationState): StringOrMarkdown | undefined {
2432-
const { kind, fileName } = readAgentPermissionRequestMeta(tc);
2433-
return kind === AgentPermissionRequestKind.Write && fileName
2434-
? getEditFileMessage(fileName, path => win32.isAbsolute(path) ? path.replaceAll('\\', '/') : path)
2435-
: undefined;
2436-
}
2437-
24382412
export function toolCallConfirmationMessages(tc: ToolCallPendingConfirmationState, connectionAuthority: string): IToolConfirmationMessages {
24392413
const riskAssessment = tc.riskAssessment;
24402414
let approvalReason: IToolConfirmationMessages['approvalReason'];
@@ -2453,7 +2427,7 @@ export function toolCallConfirmationMessages(tc: ToolCallPendingConfirmationStat
24532427
: stringOrMarkdownToString(tc.confirmationTitle, connectionAuthority) ?? tc.displayName,
24542428
message: isViewUnreviewedCommentsTool(tc.toolName)
24552429
? localize('agentFeedback.reviewMessage', "Choose which comments to reveal to the agent. Unchecked comments stay hidden.")
2456-
: stringOrMarkdownToString(remoteWriteInvocationMessage(tc) ?? tc.invocationMessage, connectionAuthority),
2430+
: stringOrMarkdownToString(tc.invocationMessage, connectionAuthority),
24572431
approvalReason,
24582432
...(tc.options ? { customOptions: tc.options } : {}),
24592433
};

src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts

Lines changed: 0 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,94 +1613,6 @@ suite('stateToProgressAdapter', () => {
16131613
assert.strictEqual(invocation.toolSpecificData?.kind, 'input');
16141614
});
16151615

1616-
test('restates a remote host write permission in the local shape', () => {
1617-
// A remote host names the file in `_meta` and sends a plain-text
1618-
// message; a local host sends the shared `Edit <link>` message.
1619-
// Both the confirmation card and the sessions list read
1620-
// `invocationMessage`, so it must carry the file too.
1621-
const tc: ToolCallPendingConfirmationState = {
1622-
toolCallId: 'tc-write',
1623-
toolName: 'str_replace_editor',
1624-
displayName: 'Edit',
1625-
invocationMessage: 'Edit file',
1626-
status: ToolCallStatus.PendingConfirmation,
1627-
confirmationTitle: 'Edit file',
1628-
toolInput: '/workspaces/simple-server/index.js',
1629-
_meta: {
1630-
requestId: 'req-w',
1631-
promptRequest: { kind: 'write', fileName: '/workspaces/simple-server/index.js' },
1632-
},
1633-
};
1634-
1635-
const invocation = toolCallStateToInvocation(tc);
1636-
const state = invocation.state.get();
1637-
const message = state.type === IChatToolInvocation.StateKind.WaitingForConfirmation
1638-
? state.confirmationMessages?.message
1639-
: undefined;
1640-
1641-
assert.deepStrictEqual({
1642-
invocationMessage: typeof invocation.invocationMessage === 'string' ? invocation.invocationMessage : invocation.invocationMessage?.value,
1643-
markdown: typeof message === 'string' ? message : message?.value,
1644-
toolSpecificData: invocation.toolSpecificData,
1645-
}, {
1646-
// Link rewriting drops the label so the renderer shows a file
1647-
// widget; a local host's message collapses to the same form.
1648-
invocationMessage: 'Edit [](file:///workspaces/simple-server/index.js)',
1649-
markdown: 'Edit [](file:///workspaces/simple-server/index.js)',
1650-
// The message names the file, so raw input would only repeat it.
1651-
toolSpecificData: undefined,
1652-
});
1653-
});
1654-
1655-
test('normalizes a Windows host path regardless of the client platform', () => {
1656-
// `URI.file` only rewrites separators when the *client* runs
1657-
// Windows, so a Windows host paired with a POSIX client would
1658-
// otherwise yield a single-segment basename.
1659-
const tc: ToolCallPendingConfirmationState = {
1660-
toolCallId: 'tc-write-win',
1661-
toolName: 'str_replace_editor',
1662-
displayName: 'Edit',
1663-
invocationMessage: 'Edit file',
1664-
status: ToolCallStatus.PendingConfirmation,
1665-
toolInput: 'C:\\repo\\src\\index.ts',
1666-
_meta: { requestId: 'req-w2', promptRequest: { kind: 'write', fileName: 'C:\\repo\\src\\index.ts' } },
1667-
};
1668-
1669-
const invocation = toolCallStateToInvocation(tc);
1670-
const message = invocation.invocationMessage;
1671-
1672-
assert.strictEqual(
1673-
typeof message === 'string' ? message : message?.value,
1674-
'Edit [](file:///c%3A/repo/src/index.ts)',
1675-
);
1676-
});
1677-
1678-
test('keeps the host message for a write permission that names no file', () => {
1679-
const tc: ToolCallPendingConfirmationState = {
1680-
toolCallId: 'tc-write-nofile',
1681-
toolName: 'str_replace_editor',
1682-
displayName: 'Edit',
1683-
invocationMessage: 'Edit file',
1684-
status: ToolCallStatus.PendingConfirmation,
1685-
toolInput: '{"path":"/tmp/a.txt"}',
1686-
_meta: { requestId: 'req-w3', promptRequest: { kind: 'write' } },
1687-
};
1688-
1689-
const invocation = toolCallStateToInvocation(tc);
1690-
const state = invocation.state.get();
1691-
const message = state.type === IChatToolInvocation.StateKind.WaitingForConfirmation
1692-
? state.confirmationMessages?.message
1693-
: undefined;
1694-
1695-
assert.deepStrictEqual({
1696-
message,
1697-
toolSpecificData: invocation.toolSpecificData,
1698-
}, {
1699-
message: 'Edit file',
1700-
toolSpecificData: { kind: 'input', rawInput: { path: '/tmp/a.txt' } },
1701-
});
1702-
});
1703-
17041616
test('sets subagent toolSpecificData from _meta for subagent toolKind', () => {
17051617
const tc = createToolCallState({
17061618
_meta: { toolKind: 'subagent', subagentDescription: 'Review code', subagentAgentName: 'code-reviewer' },

0 commit comments

Comments
 (0)