Skip to content

Commit 04d4087

Browse files
sestinjsestinj
authored
fix: show error when cn -p doesnt have tool access (#8045)
* fix: show error when cn -p doesnt have tool access * fix: type error * fix: type error * fix: delete line * fix: lint
1 parent 2d50cfd commit 04d4087

File tree

3 files changed

+291
-11
lines changed

3 files changed

+291
-11
lines changed

‎extensions/cli/src/e2e/headless-permission-error.test.ts‎

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
/**
2+
* E2E test for headless mode permission errors
3+
* Verifies that when a tool requires permission in headless mode,
4+
* an appropriate error message is displayed suggesting the --auto flag
5+
*/
6+
import * as http from "http";
7+
8+
import { afterEach, beforeEach, describe, expect, it } from "vitest";
9+
10+
import {
11+
cleanupTestContext,
12+
createTestContext,
13+
runCLI,
14+
} from "../test-helpers/cli-helpers.js";
15+
import {
16+
cleanupMockLLMServer,
17+
setupMockLLMTest,
18+
type MockLLMServer,
19+
} from "../test-helpers/mock-llm-server.js";
20+
21+
describe("E2E: Headless Mode Permission Errors", () => {
22+
let context: any;
23+
let mockServer: MockLLMServer;
24+
25+
beforeEach(async () => {
26+
context = await createTestContext();
27+
});
28+
29+
afterEach(async () => {
30+
if (mockServer) {
31+
await cleanupMockLLMServer(mockServer);
32+
}
33+
await cleanupTestContext(context);
34+
});
35+
it("should display error message with --auto suggestion when tool requires permission", async () => {
36+
// Set up mock LLM to return a tool call that requires permission (Write tool)
37+
mockServer = await setupMockLLMTest(context, {
38+
customHandler: (req: http.IncomingMessage, res: http.ServerResponse) => {
39+
let body = "";
40+
req.on("data", (chunk) => {
41+
body += chunk.toString();
42+
});
43+
44+
req.on("end", () => {
45+
if (req.method === "POST" && req.url === "/chat/completions") {
46+
res.writeHead(200, {
47+
"Content-Type": "text/event-stream",
48+
"Cache-Control": "no-cache",
49+
Connection: "keep-alive",
50+
});
51+
52+
// Send a Write tool call (requires permission by default)
53+
res.write(
54+
`data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"call_write","type":"function","function":{"name":"Write"}}]},"index":0}]}\n\n`,
55+
);
56+
res.write(
57+
`data: {"choices":[{"delta":{"tool_calls":[{"index":0,"function":{"arguments":"{\\"filepath\\":\\"test.txt\\",\\"content\\":\\"hello\\"}"}}]},"index":0}]}\n\n`,
58+
);
59+
60+
// Send usage data
61+
res.write(
62+
`data: {"usage":{"prompt_tokens":10,"completion_tokens":20}}\n\n`,
63+
);
64+
65+
// End the stream
66+
res.write(`data: [DONE]\n\n`);
67+
res.end();
68+
} else {
69+
res.writeHead(404);
70+
res.end("Not found");
71+
}
72+
});
73+
},
74+
});
75+
76+
// Run CLI in headless mode without permission flags
77+
const result = await runCLI(context, {
78+
args: ["-p", "--config", context.configPath, "Create a file test.txt"],
79+
timeout: 15000,
80+
});
81+
82+
// Expect non-zero exit code
83+
expect(result.exitCode).toBe(1);
84+
85+
// Verify the error message contains the expected information
86+
expect(result.stderr).toContain("requires permission");
87+
expect(result.stderr).toContain("headless mode");
88+
expect(result.stderr).toContain("--auto");
89+
expect(result.stderr).toContain("--allow");
90+
expect(result.stderr).toContain("--exclude");
91+
}, 20000);
92+
93+
it("should succeed with --auto flag when tool requires permission", async () => {
94+
// Set up mock LLM to return a Write tool call
95+
mockServer = await setupMockLLMTest(context, {
96+
customHandler: (req: http.IncomingMessage, res: http.ServerResponse) => {
97+
let body = "";
98+
req.on("data", (chunk) => {
99+
body += chunk.toString();
100+
});
101+
102+
req.on("end", () => {
103+
if (req.method === "POST" && req.url === "/chat/completions") {
104+
res.writeHead(200, {
105+
"Content-Type": "text/event-stream",
106+
"Cache-Control": "no-cache",
107+
Connection: "keep-alive",
108+
});
109+
110+
// Send a Write tool call
111+
res.write(
112+
`data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"call_write","type":"function","function":{"name":"Write"}}]},"index":0}]}\n\n`,
113+
);
114+
res.write(
115+
`data: {"choices":[{"delta":{"tool_calls":[{"index":0,"function":{"arguments":"{\\"filepath\\":\\"test.txt\\",\\"content\\":\\"hello\\"}"}}]},"index":0}]}\n\n`,
116+
);
117+
118+
// Send usage data
119+
res.write(
120+
`data: {"usage":{"prompt_tokens":10,"completion_tokens":20}}\n\n`,
121+
);
122+
123+
// End the stream
124+
res.write(`data: [DONE]\n\n`);
125+
res.end();
126+
} else {
127+
res.writeHead(404);
128+
res.end("Not found");
129+
}
130+
});
131+
},
132+
});
133+
134+
// Run CLI with --auto flag
135+
const result = await runCLI(context, {
136+
args: [
137+
"-p",
138+
"--auto",
139+
"--config",
140+
context.configPath,
141+
"Create a file test.txt",
142+
],
143+
timeout: 15000,
144+
});
145+
146+
// Should succeed
147+
expect(result.exitCode).toBe(0);
148+
expect(result.stderr).not.toContain("requires permission");
149+
}, 20000);
150+
151+
it("should succeed with --allow flag for specific tool", async () => {
152+
// Set up mock LLM to return a Write tool call
153+
mockServer = await setupMockLLMTest(context, {
154+
customHandler: (req: http.IncomingMessage, res: http.ServerResponse) => {
155+
let body = "";
156+
req.on("data", (chunk) => {
157+
body += chunk.toString();
158+
});
159+
160+
req.on("end", () => {
161+
if (req.method === "POST" && req.url === "/chat/completions") {
162+
res.writeHead(200, {
163+
"Content-Type": "text/event-stream",
164+
"Cache-Control": "no-cache",
165+
Connection: "keep-alive",
166+
});
167+
168+
// Send a Write tool call
169+
res.write(
170+
`data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"call_write","type":"function","function":{"name":"Write"}}]},"index":0}]}\n\n`,
171+
);
172+
res.write(
173+
`data: {"choices":[{"delta":{"tool_calls":[{"index":0,"function":{"arguments":"{\\"filepath\\":\\"test.txt\\",\\"content\\":\\"hello\\"}"}}]},"index":0}]}\n\n`,
174+
);
175+
176+
// Send usage data
177+
res.write(
178+
`data: {"usage":{"prompt_tokens":10,"completion_tokens":20}}\n\n`,
179+
);
180+
181+
// End the stream
182+
res.write(`data: [DONE]\n\n`);
183+
res.end();
184+
} else {
185+
res.writeHead(404);
186+
res.end("Not found");
187+
}
188+
});
189+
},
190+
});
191+
192+
// Run CLI with --allow Write flag
193+
const result = await runCLI(context, {
194+
args: [
195+
"-p",
196+
"--allow",
197+
"Write",
198+
"--config",
199+
context.configPath,
200+
"Create a file test.txt",
201+
],
202+
timeout: 15000,
203+
});
204+
205+
// Should succeed
206+
expect(result.exitCode).toBe(0);
207+
expect(result.stderr).not.toContain("requires permission");
208+
}, 20000);
209+
});

‎extensions/cli/src/stream/streamChatResponse.helpers.test.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
3+
import type { PreprocessedToolCall } from "../tools/types.js";
4+
5+
import { handleHeadlessPermission } from "./streamChatResponse.helpers.js";
6+
7+
describe("streamChatResponse.helpers", () => {
8+
describe("handleHeadlessPermission", () => {
9+
it("should display error message and exit when tool requires permission in headless mode", async () => {
10+
// Mock the tool call
11+
const toolCall: PreprocessedToolCall = {
12+
id: "call_123",
13+
name: "Write",
14+
arguments: { filepath: "test.txt", content: "hello" },
15+
argumentsStr: '{"filepath":"test.txt","content":"hello"}',
16+
startNotified: false,
17+
tool: {
18+
name: "Write",
19+
displayName: "Write",
20+
description: "Write to a file",
21+
parameters: {
22+
type: "object",
23+
properties: {},
24+
},
25+
run: vi.fn(),
26+
isBuiltIn: true,
27+
},
28+
};
29+
30+
// Mock safeStderr to capture output
31+
const stderrOutputs: string[] = [];
32+
vi.doMock("../init.js", () => ({
33+
safeStderr: (message: string) => {
34+
stderrOutputs.push(message);
35+
},
36+
}));
37+
38+
// Mock gracefulExit to prevent actual process exit
39+
let exitCode: number | undefined;
40+
vi.doMock("../util/exit.js", () => ({
41+
gracefulExit: async (code: number) => {
42+
exitCode = code;
43+
},
44+
}));
45+
46+
// Call the function (it should exit gracefully)
47+
try {
48+
await handleHeadlessPermission(toolCall);
49+
} catch (error) {
50+
// Expected to throw after exit
51+
}
52+
53+
// Verify error message was displayed
54+
const fullOutput = stderrOutputs.join("");
55+
expect(fullOutput).toContain("requires permission");
56+
expect(fullOutput).toContain("headless mode");
57+
expect(fullOutput).toContain("--auto");
58+
expect(fullOutput).toContain("--allow");
59+
expect(fullOutput).toContain("--exclude");
60+
expect(fullOutput).toContain("Write");
61+
62+
// Verify it tried to exit with code 1
63+
expect(exitCode).toBe(1);
64+
});
65+
});
66+
});

‎extensions/cli/src/stream/streamChatResponse.helpers.ts‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,25 +51,30 @@ export function handlePermissionDenied(
5151
}
5252

5353
// Helper function to handle headless mode permission
54-
export function handleHeadlessPermission(
54+
export async function handleHeadlessPermission(
5555
toolCall: PreprocessedToolCall,
56-
): never {
56+
): Promise<never> {
5757
const allBuiltinTools = getAllBuiltinTools();
5858
const tool = allBuiltinTools.find((t) => t.name === toolCall.name);
5959
const toolName = tool?.displayName || toolCall.name;
6060

61-
console.error(
62-
`Error: Tool '${toolName}' requires permission but cn is running in headless mode.`,
61+
// Import safeStderr to bypass console blocking in headless mode
62+
const { safeStderr } = await import("../init.js");
63+
safeStderr(
64+
`Error: Tool '${toolName}' requires permission but cn is running in headless mode.\n`,
6365
);
64-
console.error(`If you want to allow this tool, use --allow ${toolName}.`);
65-
console.error(
66-
`If you don't want the tool to be included, use --exclude ${toolName}.`,
66+
safeStderr(
67+
`If you want to allow all tools without asking, use cn -p --auto "your prompt".\n`,
68+
);
69+
safeStderr(`If you want to allow this tool, use --allow ${toolName}.\n`);
70+
safeStderr(
71+
`If you don't want the tool to be included, use --exclude ${toolName}.\n`,
6772
);
6873

6974
// Use graceful exit to flush telemetry even in headless denial
70-
// Note: We purposely trigger an async exit without awaiting in this sync path
71-
import("../util/exit.js").then(({ gracefulExit }) => gracefulExit(1));
72-
// Throw to satisfy the never return type; process will exit shortly
75+
const { gracefulExit } = await import("../util/exit.js");
76+
await gracefulExit(1);
77+
// This line will never be reached, but TypeScript needs it for the 'never' return type
7378
throw new Error("Exiting due to headless permission requirement");
7479
}
7580

@@ -133,7 +138,7 @@ export async function checkToolPermissionApproval(
133138
return { approved: true };
134139
} else if (permissionCheck.permission === "ask") {
135140
if (isHeadless) {
136-
handleHeadlessPermission(toolCall);
141+
await handleHeadlessPermission(toolCall);
137142
}
138143
const userApproved = await requestUserPermission(toolCall, callbacks);
139144
return userApproved

0 commit comments

Comments
 (0)