Skip to content

Commit cca2dca

Browse files
tpansinotpansino
authored
fix: show commands in Zed approval prompts (#735)
Addresses zed-industries/zed#65727 Command approval requests replaced the command title with “Run command,” hiding what Zed was asking the user to approve. Use the command minus the shell bits as the approval title, including when approval arrives before the command’s start event. Add ACP v1/v2 regression coverage and update the documented approval contract. Validation: - Typecheck, build, 206 focused tests, and all 19 real-Codex integration tests passed. - Confirmed fixed in Zed (v1.24.2 on macOS 26.6.2)
1 parent 519c267 commit cca2dca

18 files changed

Lines changed: 199 additions & 10 deletions

‎docs/air-extensions.md‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ Only these differences are allowed:
5959
After a cancelled or failed permission request, the next update carries every field again.
6060
ACP defines no merge for `_meta` keys, so the `_meta` of each report keeps every key that the adapter sent before.
6161
- Bug fixes: a unique MCP startup tool call id, the result of a dynamic tool in `content`,
62-
no output after a tool call ended, and a terminal status for a replayed image generation.
62+
no output after a tool call ended, a terminal status for a replayed image generation,
63+
and the command as the title of a command approval.
6364
- Feature changes for every client: the `/mcp` command has a new description and an input hint.
6465
- The client gets no AIR-only key.
6566
That is no `_meta.jetbrains.air` key and none of the earlier keys in [Removed keys](#removed-keys).
@@ -359,7 +360,7 @@ The other clients get the same fields as before the AIR extensions.
359360
| Turn plan (`turn/plan/updated`) | standard `plan` with entries | The same. |
360361
| `contextCompaction` | `compaction_update` when the client declares `session.compaction`. Otherwise a synthetic tool call with `_meta.jetbrains.air.contextCompaction`. | The same, without `_meta`. |
361362
| `agentMessage` | `agent_message_chunk` with `_meta.jetbrains.air.phase` when Codex reports a phase. | No `_meta`. |
362-
| Command, file change, or sandbox permission request | See [Tool call of the request](#tool-call-of-the-request). | `kind`, `status: pending`, and a generic title such as `Run command` or `Edit files`, also for a started tool call. No `_meta`. |
363+
| Command, file change, or sandbox permission request | See [Tool call of the request](#tool-call-of-the-request). | `kind`, `status: pending`, and a title, also for a started tool call. Command approvals keep the started title or show the command; other requests use a generic title or a network title. No `_meta`. |
363364
| `imageView`, fuzzy search, MCP startup | standard shape. A fuzzy search that finds no file sends `locations: []`. | The same. |
364365

365366
## Diff patch
@@ -542,7 +543,10 @@ The `toolCall` is an ACP `ToolCallUpdate`. The client merges it into the stored
542543
A client without `rawInputRendering` also gets it as text in `content`.
543544

544545
A client that is not AIR gets the request tool call of the adapter before the AIR extensions.
545-
It always carries `kind` and `status: pending`, and a generic title such as `Run command`, `Edit files`, or a network title.
546+
It always carries `kind` and `status: pending`. A started command keeps its title, including a parsed read, search, or list title.
547+
An approval without a start uses the command without its shell wrapper as the title, so Zed keeps the command visible.
548+
When Codex supplies no command, the title describes the parsed action or falls back to `Run command`.
549+
File changes and network approvals keep their generic or network titles.
546550
Its `locations` hold every path of the Codex command actions.
547551

548552
Command approvals use `kind: execute`. File changes use `kind: edit`.

‎src/CommandUtils.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
11
export function stripShellPrefix(command: string): string {
2-
const withoutShell = command.replace(/^(?:\/bin\/)?(?:bash|zsh|sh)\s+(?:-[lc]+\s+)?/, "");
3-
if (withoutShell.startsWith("'") && withoutShell.endsWith("'")) {
2+
const shell = /^(?:\/(?:[^\s/]+\/)*)?(?:bash|zsh|sh)\s+(?:-[lc]+\s+)?/.exec(command);
3+
if (!shell) return command;
4+
const withoutShell = command.slice(shell[0].length);
5+
// Keep invocations with shell options that this display cleanup does not recognize.
6+
if (withoutShell.startsWith("-")) return command;
7+
if (/^'[^']*'$/.test(withoutShell)) {
48
return withoutShell.slice(1, -1);
59
}
10+
if (/^"(?:[^"\\]|\\[\s\S])*"$/.test(withoutShell)) {
11+
return withoutShell.slice(1, -1).replace(/\\([$`"\\\n])/g,
12+
(_escape, character: string) => character === "\n" ? "" : character);
13+
}
614
return withoutShell;
715
}

‎src/__tests__/CodexACPAgent/data/permissions-v2-command-allowed.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
"subject": {
1313
"toolCall": {
1414
"toolCallId": "cmd-item-1",
15-
"title": "Run command",
15+
"title": "ls -la",
1616
"kind": "execute",
1717
"status": "pending",
1818
"rawInput": {

‎src/__tests__/CodexACPAgent/data/permissions-v2-command-rejected.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
"subject": {
1313
"toolCall": {
1414
"toolCallId": "cmd-item-1",
15-
"title": "Run command",
15+
"title": "ls -la",
1616
"kind": "execute",
1717
"status": "pending",
1818
"rawInput": {

‎src/__tests__/CodexACPAgent/terminal-output-events.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,21 @@ describe('CodexEventHandler - terminal output events', () => {
6060
{ command: 'npm install', expected: 'npm install' },
6161
{ command: "/bin/bash -lc './tests.cmd -Darg=value'", expected: './tests.cmd -Darg=value' },
6262
{ command: "/bin/zsh -c 'echo hello'", expected: 'echo hello' },
63+
{
64+
command: String.raw`/private/var/run/current-system/sw/bin/zsh -lc "printf '%s\\n' 'codex-acp approval display test'"`,
65+
expected: String.raw`printf '%s\n' 'codex-acp approval display test'`,
66+
},
67+
{ command: "/run/current-system/sw/bin/sh -c 'npm test'", expected: 'npm test' },
68+
{ command: '/usr/local/bin/bash -lc "npm test"', expected: 'npm test' },
69+
{
70+
command: String.raw`/nix/store/example-bash/bin/bash -lc "printf '%s\\n' \"\$HOME\""`,
71+
expected: String.raw`printf '%s\n' "$HOME"`,
72+
},
73+
{ command: String.raw`zsh -c "printf 'line\n'"`, expected: String.raw`printf 'line\n'` },
74+
{ command: "'printf' '%s\\n' 'hello'", expected: "'printf' '%s\\n' 'hello'" },
75+
{ command: '/usr/local/bin/my-zsh -lc "npm test"', expected: '/usr/local/bin/my-zsh -lc "npm test"' },
76+
{ command: 'zsh --version', expected: 'zsh --version' },
77+
{ command: 'bash -x -c "npm test"', expected: 'bash -x -c "npm test"' },
6378
])('should strip shell prefix from "$command"', async ({ command, expected }) => {
6479
const commandStartNotification: ServerNotification = {
6580
method: 'item/started',

‎src/__tests__/scenarios/baseline.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,15 @@ const DYNAMIC_TOOL_RESULTS: Record<string, Record<string, string>> = {
105105
* The bug fixes of the compatibility rule that change the messages of the scenarios:
106106
* - the MCP startup tool call id is unique, so it ends with a random id;
107107
* - the report that completes a dynamic tool call has the result of the tool in `content`.
108+
* - a command approval keeps the command visible in the title.
108109
*/
109110
function withBugFixes(scenario: string, messages: RecordedMessage[]): RecordedMessage[] {
110111
const results = {...DYNAMIC_TOOL_RESULTS[scenario]};
111112
return messages.map(message => {
113+
if (scenario === "command-approval" && message.method === "session/request_permission") {
114+
const params = message.params as Json;
115+
return {...message, params: {...params, toolCall: {...(params["toolCall"] as Json), title: "npm install"}}};
116+
}
112117
const update = sessionUpdate(message);
113118
if (update === undefined || typeof update["toolCallId"] !== "string") return message;
114119
const toolCallId = update["toolCallId"];

‎src/__tests__/scenarios/client-profiles.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,13 +292,13 @@ describe("clients that are not AIR", () => {
292292
}]);
293293
});
294294

295-
it(`${profile}: gets the pre-contract permission request of a started command`, () => {
295+
it(`${profile}: keeps the command visible when requesting permission for a started command`, () => {
296296
expect(permissionRequests(profile, "command-approval")[0]).toEqual(expect.objectContaining({
297297
toolCall: {
298298
toolCallId: "cmd-a",
299299
kind: "execute",
300300
status: "pending",
301-
title: "Run command",
301+
title: "npm install",
302302
rawInput: {command: "npm install", cwd: "/workspace"},
303303
},
304304
}));

‎src/__tests__/tool-calls/data/command-approval-missing.json‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
{
2+
"toolCallId": "cmd",
3+
"kind": "execute",
4+
"status": "pending",
5+
"title": "Read file",
6+
"rawInput": {
7+
"cwd": "/w"
8+
},
9+
"locations": [
10+
{
11+
"path": "/w/a.txt"
12+
}
13+
]
14+
}

‎src/__tests__/tool-calls/data/command-approval-multiline.json‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"toolCallId": "cmd",
3+
"kind": "execute",
4+
"status": "pending",
5+
"title": "printf 'first\\n'\nprintf 'second\\n'",
6+
"rawInput": {
7+
"command": "printf 'first\\n'\nprintf 'second\\n'",
8+
"cwd": "/w"
9+
}
10+
}

‎src/__tests__/tool-calls/data/command-approval-network.json‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
{
2+
"toolCallId": "cmd",
3+
"kind": "execute",
4+
"status": "pending",
5+
"title": "https network access to example.com",
6+
"rawInput": {
7+
"command": "curl https://example.com",
8+
"cwd": "/w",
9+
"url": "https://example.com"
10+
},
11+
"content": [
12+
{
13+
"type": "content",
14+
"content": {
15+
"type": "text",
16+
"text": "https access to example.com"
17+
}
18+
}
19+
]
20+
}

0 commit comments

Comments
 (0)