Skip to content

Commit 5408992

Browse files
codeaholicguycodeaholicguy
committed
chore(cli): fix execAsync with string interpolation in TerminalFocusManager
1 parent 7bd2b8a commit 5408992

5 files changed

Lines changed: 93 additions & 84 deletions

File tree

‎packages/agent-manager/src/__tests__/utils/process.test.ts‎

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44

55
import { describe, it, expect, jest, beforeEach } from '@jest/globals';
6-
import { execSync } from 'child_process';
6+
import { execFileSync } from 'child_process';
77
import {
88
listAgentProcesses,
99
batchGetProcessCwds,
@@ -12,18 +12,18 @@ import {
1212
} from '../../utils/process';
1313

1414
jest.mock('child_process', () => ({
15-
execSync: jest.fn(),
15+
execFileSync: jest.fn(),
1616
}));
1717

18-
const mockedExecSync = execSync as jest.MockedFunction<typeof execSync>;
18+
const mockedExecFileSync = execFileSync as jest.MockedFunction<typeof execFileSync>;
1919

2020
describe('listAgentProcesses', () => {
2121
beforeEach(() => {
22-
mockedExecSync.mockReset();
22+
mockedExecFileSync.mockReset();
2323
});
2424

2525
it('should parse ps aux | grep output and post-filter by executable name', () => {
26-
mockedExecSync.mockReturnValue(
26+
mockedExecFileSync.mockReturnValue(
2727
'user 78070 1.0 0.5 485636016 245952 s018 S+ 11:18PM 1:55.14 claude\n' +
2828
'user 55106 0.1 0.4 485620368 72496 s015 S+ 9Mar26 8:06.36 claude\n',
2929
);
@@ -38,7 +38,7 @@ describe('listAgentProcesses', () => {
3838
});
3939

4040
it('should filter out non-matching executables', () => {
41-
mockedExecSync.mockReturnValue(
41+
mockedExecFileSync.mockReturnValue(
4242
'user 100 0.0 0.0 0 0 s001 S 1:00PM 0:00 claude\n' +
4343
'user 200 0.0 0.0 0 0 s002 S 1:00PM 0:00 claude-helper --pid 100\n' +
4444
'user 300 0.0 0.0 0 0 s003 S 1:00PM 0:00 /usr/bin/claude\n',
@@ -50,46 +50,46 @@ describe('listAgentProcesses', () => {
5050
});
5151

5252
it('should return empty array on command failure', () => {
53-
mockedExecSync.mockImplementation(() => { throw new Error('fail'); });
53+
mockedExecFileSync.mockImplementation(() => { throw new Error('fail'); });
5454
expect(listAgentProcesses('claude')).toEqual([]);
5555
});
5656

5757
it('should handle empty output', () => {
58-
mockedExecSync.mockReturnValue('');
58+
mockedExecFileSync.mockReturnValue('');
5959
expect(listAgentProcesses('claude')).toEqual([]);
6060
});
6161

6262
it('should reject empty pattern', () => {
6363
expect(listAgentProcesses('')).toEqual([]);
64-
expect(mockedExecSync).not.toHaveBeenCalled();
64+
expect(mockedExecFileSync).not.toHaveBeenCalled();
6565
});
6666

6767
it('should reject patterns with shell injection characters', () => {
6868
expect(listAgentProcesses('claude; rm -rf /')).toEqual([]);
6969
expect(listAgentProcesses("claude' || true")).toEqual([]);
7070
expect(listAgentProcesses('$(whoami)')).toEqual([]);
71-
expect(mockedExecSync).not.toHaveBeenCalled();
71+
expect(mockedExecFileSync).not.toHaveBeenCalled();
7272
});
7373

7474
it('should accept valid patterns with dashes and underscores', () => {
75-
mockedExecSync.mockReturnValue('');
75+
mockedExecFileSync.mockReturnValue('');
7676
listAgentProcesses('claude-code');
77-
expect(mockedExecSync).toHaveBeenCalled();
77+
expect(mockedExecFileSync).toHaveBeenCalled();
7878

79-
mockedExecSync.mockReset();
80-
mockedExecSync.mockReturnValue('');
79+
mockedExecFileSync.mockReset();
80+
mockedExecFileSync.mockReturnValue('');
8181
listAgentProcesses('my_agent');
82-
expect(mockedExecSync).toHaveBeenCalled();
82+
expect(mockedExecFileSync).toHaveBeenCalled();
8383
});
8484
});
8585

8686
describe('batchGetProcessCwds', () => {
8787
beforeEach(() => {
88-
mockedExecSync.mockReset();
88+
mockedExecFileSync.mockReset();
8989
});
9090

9191
it('should parse batched lsof output', () => {
92-
mockedExecSync.mockReturnValue(
92+
mockedExecFileSync.mockReturnValue(
9393
'p78070\nn/Users/user/ai-devkit\np55106\nn/Users/user/other-project\n',
9494
);
9595

@@ -104,7 +104,7 @@ describe('batchGetProcessCwds', () => {
104104

105105
it('should return partial results when lsof succeeds for some PIDs', () => {
106106
// lsof might not return entries for dead processes
107-
mockedExecSync.mockReturnValue(
107+
mockedExecFileSync.mockReturnValue(
108108
'p78070\nn/Users/user/ai-devkit\n',
109109
);
110110

@@ -114,7 +114,7 @@ describe('batchGetProcessCwds', () => {
114114
});
115115

116116
it('should return empty map on total failure', () => {
117-
mockedExecSync.mockImplementation(() => { throw new Error('fail'); });
117+
mockedExecFileSync.mockImplementation(() => { throw new Error('fail'); });
118118
const cwds = batchGetProcessCwds([78070]);
119119
// Falls through to pwdx fallback which also fails
120120
expect(cwds.size).toBe(0);
@@ -123,11 +123,11 @@ describe('batchGetProcessCwds', () => {
123123

124124
describe('batchGetProcessStartTimes', () => {
125125
beforeEach(() => {
126-
mockedExecSync.mockReset();
126+
mockedExecFileSync.mockReset();
127127
});
128128

129129
it('should parse ps lstart output', () => {
130-
mockedExecSync.mockReturnValue(
130+
mockedExecFileSync.mockReturnValue(
131131
' 78070 Wed Mar 18 23:18:01 2026\n' +
132132
' 55106 Mon Mar 9 21:41:42 2026\n',
133133
);
@@ -143,7 +143,7 @@ describe('batchGetProcessStartTimes', () => {
143143
});
144144

145145
it('should skip lines with unparseable dates', () => {
146-
mockedExecSync.mockReturnValue(
146+
mockedExecFileSync.mockReturnValue(
147147
' 78070 Wed Mar 18 23:18:01 2026\n' +
148148
' 99999 INVALID_DATE\n',
149149
);
@@ -154,20 +154,20 @@ describe('batchGetProcessStartTimes', () => {
154154
});
155155

156156
it('should return empty map on failure', () => {
157-
mockedExecSync.mockImplementation(() => { throw new Error('fail'); });
157+
mockedExecFileSync.mockImplementation(() => { throw new Error('fail'); });
158158
expect(batchGetProcessStartTimes([78070])).toEqual(new Map());
159159
});
160160
});
161161

162162
describe('enrichProcesses', () => {
163163
beforeEach(() => {
164-
mockedExecSync.mockReset();
164+
mockedExecFileSync.mockReset();
165165
});
166166

167167
it('should populate cwd and startTime on processes', () => {
168168
// First call: batchGetProcessCwds (lsof)
169169
// Second call: batchGetProcessStartTimes (ps lstart)
170-
mockedExecSync
170+
mockedExecFileSync
171171
.mockReturnValueOnce('p100\nn/projects/app\n')
172172
.mockReturnValueOnce(' 100 Wed Mar 18 23:18:01 2026\n');
173173

@@ -182,12 +182,12 @@ describe('enrichProcesses', () => {
182182

183183
it('should return empty array for empty input', () => {
184184
expect(enrichProcesses([])).toEqual([]);
185-
expect(mockedExecSync).not.toHaveBeenCalled();
185+
expect(mockedExecFileSync).not.toHaveBeenCalled();
186186
});
187187

188188
it('should handle partial failures', () => {
189189
// lsof succeeds, ps lstart fails
190-
mockedExecSync
190+
mockedExecFileSync
191191
.mockReturnValueOnce('p100\nn/projects/app\n')
192192
.mockImplementationOnce(() => { throw new Error('fail'); });
193193

‎packages/agent-manager/src/terminal/TerminalFocusManager.ts‎

Lines changed: 38 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
1-
import { exec, execFile } from 'child_process';
1+
import { execFile } from 'child_process';
22
import { promisify } from 'util';
33
import { getProcessTty } from '../utils/process';
44

5-
const execAsync = promisify(exec);
65
const execFileAsync = promisify(execFile);
76

87
export enum TerminalType {
@@ -18,6 +17,10 @@ export interface TerminalLocation {
1817
tty: string; // e.g., "/dev/ttys030"
1918
}
2019

20+
function escapeAppleScript(text: string): string {
21+
return text.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
22+
}
23+
2124
export class TerminalFocusManager {
2225
/**
2326
* Find the terminal location (emulator info) for a given process ID
@@ -67,17 +70,16 @@ export class TerminalFocusManager {
6770
default:
6871
return false;
6972
}
70-
} catch (error) {
73+
} catch {
7174
return false;
7275
}
7376
}
7477

7578
private async findTmuxPane(tty: string): Promise<TerminalLocation | null> {
7679
try {
77-
// List all panes with their TTYs and identifiers
78-
// Format: /dev/ttys001|my-session:1.1
79-
// using | as separator to handle spaces in session names
80-
const { stdout } = await execAsync("tmux list-panes -a -F '#{pane_tty}|#{session_name}:#{window_index}.#{pane_index}'");
80+
const { stdout } = await execFileAsync('tmux', [
81+
'list-panes', '-a', '-F', '#{pane_tty}|#{session_name}:#{window_index}.#{pane_index}'
82+
]);
8183

8284
const lines = stdout.trim().split('\n');
8385
for (const line of lines) {
@@ -91,7 +93,7 @@ export class TerminalFocusManager {
9193
};
9294
}
9395
}
94-
} catch (error) {
96+
} catch {
9597
// tmux might not be installed or running
9698
}
9799
return null;
@@ -100,15 +102,19 @@ export class TerminalFocusManager {
100102
private async findITerm2Session(tty: string): Promise<TerminalLocation | null> {
101103
try {
102104
// Check if iTerm2 is running first to avoid launching it
103-
const { stdout: isRunning } = await execAsync('pgrep -x iTerm2 || echo "no"');
104-
if (isRunning.trim() === "no") return null;
105+
await execFileAsync('pgrep', ['-x', 'iTerm2']);
106+
} catch {
107+
return null;
108+
}
105109

110+
try {
111+
const escapedTty = escapeAppleScript(tty);
106112
const script = `
107113
tell application "iTerm"
108114
repeat with w in windows
109115
repeat with t in tabs of w
110116
repeat with s in sessions of t
111-
if tty of s is "${tty}" then
117+
if tty of s is "${escapedTty}" then
112118
return "found"
113119
end if
114120
end repeat
@@ -117,48 +123,52 @@ export class TerminalFocusManager {
117123
end tell
118124
`;
119125

120-
const { stdout } = await execAsync(`osascript -e '${script}'`);
126+
const { stdout } = await execFileAsync('osascript', ['-e', script]);
121127
if (stdout.trim() === "found") {
122128
return {
123129
type: TerminalType.ITERM2,
124130
identifier: tty,
125131
tty
126132
};
127133
}
128-
} catch (error) {
129-
// iTerm2 not found or script failed
134+
} catch {
135+
// iTerm2 script failed
130136
}
131137
return null;
132138
}
133139

134140
private async findTerminalAppWindow(tty: string): Promise<TerminalLocation | null> {
135141
try {
136-
// Check if Terminal is running
137-
const { stdout: isRunning } = await execAsync('ps -eo pid=,comm= | grep "Terminal.app" || echo "no"');
138-
if (isRunning.trim() === "no") return null;
142+
// Check if Terminal.app is running
143+
await execFileAsync('pgrep', ['-x', 'Terminal']);
144+
} catch {
145+
return null;
146+
}
139147

148+
try {
149+
const escapedTty = escapeAppleScript(tty);
140150
const script = `
141151
tell application "Terminal"
142152
repeat with w in windows
143153
repeat with t in tabs of w
144-
if tty of t is "${tty}" then
154+
if tty of t is "${escapedTty}" then
145155
return "found"
146156
end if
147157
end repeat
148158
end repeat
149159
end tell
150160
`;
151161

152-
const { stdout } = await execAsync(`osascript -e '${script}'`);
162+
const { stdout } = await execFileAsync('osascript', ['-e', script]);
153163
if (stdout.trim() === "found") {
154164
return {
155165
type: TerminalType.TERMINAL_APP,
156166
identifier: tty,
157167
tty
158168
};
159169
}
160-
} catch (error) {
161-
// Terminal not found or script failed
170+
} catch {
171+
// Terminal.app script failed
162172
}
163173
return null;
164174
}
@@ -167,19 +177,20 @@ export class TerminalFocusManager {
167177
try {
168178
await execFileAsync('tmux', ['switch-client', '-t', identifier]);
169179
return true;
170-
} catch (error) {
180+
} catch {
171181
return false;
172182
}
173183
}
174184

175185
private async focusITerm2Session(tty: string): Promise<boolean> {
186+
const escapedTty = escapeAppleScript(tty);
176187
const script = `
177188
tell application "iTerm"
178189
activate
179190
repeat with w in windows
180191
repeat with t in tabs of w
181192
repeat with s in sessions of t
182-
if tty of s is "${tty}" then
193+
if tty of s is "${escapedTty}" then
183194
select s
184195
return "true"
185196
end if
@@ -188,17 +199,18 @@ export class TerminalFocusManager {
188199
end repeat
189200
end tell
190201
`;
191-
const { stdout } = await execAsync(`osascript -e '${script}'`);
202+
const { stdout } = await execFileAsync('osascript', ['-e', script]);
192203
return stdout.trim() === "true";
193204
}
194205

195206
private async focusTerminalAppWindow(tty: string): Promise<boolean> {
207+
const escapedTty = escapeAppleScript(tty);
196208
const script = `
197209
tell application "Terminal"
198210
activate
199211
repeat with w in windows
200212
repeat with t in tabs of w
201-
if tty of t is "${tty}" then
213+
if tty of t is "${escapedTty}" then
202214
set index of w to 1
203215
set selected tab of w to t
204216
return "true"
@@ -207,7 +219,7 @@ export class TerminalFocusManager {
207219
end repeat
208220
end tell
209221
`;
210-
const { stdout } = await execAsync(`osascript -e '${script}'`);
222+
const { stdout } = await execFileAsync('osascript', ['-e', script]);
211223
return stdout.trim() === "true";
212224
}
213225
}

0 commit comments

Comments
 (0)