Skip to content

Commit bbb2d39

Browse files
committed
fix(acp): classify the executable before choosing the client terminal
The terminal selector matched only the argument shape and the non-interactive env, so any other caller spawning `<binary> -c <script>` with the same env was routed to the client terminal, which can refuse to run that binary. Require the executable to be a shell as well.
1 parent c7935dc commit bbb2d39

3 files changed

Lines changed: 59 additions & 3 deletions

File tree

.changeset/acp-local-execution-and-stdio-mcp.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
"@pymodel/pythinker-code": patch
33
---
44

5-
Run shell commands locally when an ACP client provides no terminal, accept stdio MCP servers in ACP sessions, and let a reloaded ACP session bind its runtime again.
5+
Run a command locally when an ACP client provides no terminal or the command is not a shell, accept stdio MCP servers in ACP sessions, and let a reloaded ACP session bind its runtime again.

packages/acp-server/src/acp-terminal/acpTerminalRunner.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,25 @@ const OUTPUT_BYTE_LIMIT = 4 * 1024 * 1024;
2424
const OUTPUT_POLL_MS = 250;
2525
let nextGeneration = 1;
2626

27-
function isBashToolInvocation(args: readonly string[], options?: HostProcessOptions): boolean {
27+
const SHELL_EXECUTABLES = new Set(['sh', 'bash', 'zsh', 'dash', 'ksh', 'fish']);
28+
29+
/**
30+
* The Bash tool always spawns the configured shell. Classifying the executable
31+
* keeps another caller's `-c` invocation — `python -c ...` carrying the same
32+
* non-interactive env — on the local path, where the client cannot refuse it.
33+
*/
34+
function isShellExecutable(command: string): boolean {
35+
const base = (command.split(/[\\/]/).pop() ?? command).toLowerCase();
36+
return SHELL_EXECUTABLES.has(base.endsWith('.exe') ? base.slice(0, -4) : base);
37+
}
38+
39+
function isBashToolInvocation(
40+
command: string,
41+
args: readonly string[],
42+
options?: HostProcessOptions,
43+
): boolean {
2844
return (
45+
isShellExecutable(command) &&
2946
args.length === 2 &&
3047
args[0] === '-c' &&
3148
options?.env?.['NO_COLOR'] === '1' &&
@@ -55,7 +72,7 @@ class AcpProcessService implements IHostProcessService {
5572
args: readonly string[] = [],
5673
options?: HostProcessOptions,
5774
): Promise<IHostProcess> {
58-
if (!this.connection.terminalEnabled || !isBashToolInvocation(args, options)) {
75+
if (!this.connection.terminalEnabled || !isBashToolInvocation(command, args, options)) {
5976
return this.local.spawn(command, args, { ...options, cwd: options?.cwd ?? this.cwd });
6077
}
6178

packages/acp-server/test/acp-terminal.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,45 @@ describe('AcpProcessService local fallback', () => {
175175
});
176176
});
177177

178+
it('falls back to local execution for a non-shell -c command carrying the Bash env', async () => {
179+
let created = 0;
180+
const connection = makeConnection({
181+
terminalEnabled: true,
182+
createTerminal: () => {
183+
created += 1;
184+
return makeTerminalHandle();
185+
},
186+
});
187+
const { local, calls } = makeLocalProcessService();
188+
const runtime = await bindRuntime(makeEnvironment(), { connection, local });
189+
190+
await runtime.process!.spawn('python', ['-c', 'print(1)'], { env: { ...bashEnv } });
191+
192+
expect(created).toBe(0);
193+
expect(calls).toHaveLength(1);
194+
expect(calls[0]).toMatchObject({ command: 'python', args: ['-c', 'print(1)'] });
195+
});
196+
197+
it('routes a shell spawn to the terminal regardless of the shell binary or its path', async () => {
198+
for (const shell of ['/bin/zsh', '/usr/local/bin/fish', 'C:\\Program Files\\Git\\bin\\bash.exe']) {
199+
let created = 0;
200+
const connection = makeConnection({
201+
terminalEnabled: true,
202+
createTerminal: () => {
203+
created += 1;
204+
return makeTerminalHandle();
205+
},
206+
});
207+
const { local, calls } = makeLocalProcessService();
208+
const runtime = await bindRuntime(makeEnvironment(), { connection, local });
209+
210+
await runtime.process!.spawn(shell, ['-c', 'echo hi'], { env: { ...bashEnv } });
211+
212+
expect(created, shell).toBe(1);
213+
expect(calls, shell).toHaveLength(0);
214+
}
215+
});
216+
178217
it('falls back to local execution for non-Bash spawns even with the terminal capability', async () => {
179218
let created = 0;
180219
const connection = makeConnection({

0 commit comments

Comments
 (0)