Skip to content

Commit 731e1d6

Browse files
committed
fix(pty): validate command exists before spawning agent
Add validateCommand() that checks whether a binary exists (via `which` for PATH names, `fs.accessSync` for absolute paths) before pty.spawn(). This gives users a clear "not found in PATH" error instead of the cryptic "Process exited (?)" when a CLI agent is not installed. Closes #6
1 parent 8666fca commit 731e1d6

3 files changed

Lines changed: 70 additions & 1 deletion

File tree

electron/ipc/pty.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { validateCommand } from './pty.js';
3+
4+
describe('validateCommand', () => {
5+
it('does not throw for a command found in PATH', () => {
6+
// /bin/sh always exists on macOS/Linux
7+
expect(() => validateCommand('/bin/sh')).not.toThrow();
8+
});
9+
10+
it('throws a descriptive error for a missing command', () => {
11+
expect(() => validateCommand('nonexistent-binary-xyz')).toThrow(
12+
/not found in PATH/,
13+
);
14+
});
15+
16+
it('throws a descriptive error naming the command', () => {
17+
expect(() => validateCommand('nonexistent-binary-xyz')).toThrow(
18+
/nonexistent-binary-xyz/,
19+
);
20+
});
21+
22+
it('throws for a nonexistent absolute path', () => {
23+
expect(() => validateCommand('/nonexistent/path/binary')).toThrow(
24+
/not found or not executable/,
25+
);
26+
});
27+
28+
it('does not throw for a bare command found in PATH', () => {
29+
expect(() => validateCommand('sh')).not.toThrow();
30+
});
31+
32+
it('throws for an empty command string', () => {
33+
expect(() => validateCommand('')).toThrow(/must not be empty/);
34+
});
35+
36+
it('throws for a whitespace-only command string', () => {
37+
expect(() => validateCommand(' ')).toThrow(/must not be empty/);
38+
});
39+
});

electron/ipc/pty.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import * as pty from 'node-pty';
2+
import { execFileSync } from 'child_process';
3+
import fs from 'fs';
24
import type { BrowserWindow } from 'electron';
35
import { RingBuffer } from '../remote/ring-buffer.js';
46

@@ -43,6 +45,32 @@ const BATCH_INTERVAL = 8; // ms
4345
const TAIL_CAP = 8 * 1024;
4446
const MAX_LINES = 50;
4547

48+
/** Verify that a command exists in PATH. Throws a descriptive error if not found. */
49+
export function validateCommand(command: string): void {
50+
if (!command || !command.trim()) {
51+
throw new Error('Command must not be empty.');
52+
}
53+
// Absolute paths: check directly via filesystem
54+
if (command.startsWith('/')) {
55+
try {
56+
fs.accessSync(command, fs.constants.X_OK);
57+
return;
58+
} catch {
59+
throw new Error(
60+
`Command '${command}' not found or not executable. Check that it is installed.`,
61+
);
62+
}
63+
}
64+
// Bare names: resolve via `which` (execFileSync — no shell interpolation)
65+
try {
66+
execFileSync('which', [command], { encoding: 'utf8', timeout: 3000 });
67+
} catch {
68+
throw new Error(
69+
`Command '${command}' not found in PATH. Make sure it is installed and available in your terminal.`,
70+
);
71+
}
72+
}
73+
4674
export function spawnAgent(
4775
win: BrowserWindow,
4876
args: {
@@ -68,6 +96,8 @@ export function spawnAgent(
6896
throw new Error(`Command contains disallowed characters: ${command}`);
6997
}
7098

99+
validateCommand(command);
100+
71101
const filteredEnv: Record<string, string> = {};
72102
for (const [k, v] of Object.entries(process.env)) {
73103
if (v !== undefined) filteredEnv[k] = v;

vitest.config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@ import { defineConfig } from 'vitest/config';
22

33
export default defineConfig({
44
test: {
5-
include: ['src/**/*.test.ts'],
5+
include: ['src/**/*.test.ts', 'electron/**/*.test.ts'],
66
},
77
});

0 commit comments

Comments
 (0)