Skip to content

Commit 30573d2

Browse files
authored
refactor(core): remove eslint-disable and type-asserts from shellExecutionService (#28862)
1 parent ba4296c commit 30573d2

2 files changed

Lines changed: 219 additions & 101 deletions

File tree

packages/core/src/services/shellExecutionService.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,6 +1247,92 @@ describe('ShellExecutionService', () => {
12471247
// The catch block must call destroy() on spawnedPty to prevent fd leak
12481248
expect(destroySpy).toHaveBeenCalled();
12491249
});
1250+
1251+
it('should dispose of PTY event listeners on process exit', async () => {
1252+
const dataDisposeSpy = vi.fn();
1253+
const exitDisposeSpy = vi.fn();
1254+
1255+
mockPtyProcess.onData.mockReturnValue({ dispose: dataDisposeSpy });
1256+
mockPtyProcess.onExit.mockReturnValue({ dispose: exitDisposeSpy });
1257+
1258+
await simulateExecution('ls -l', (pty) => {
1259+
pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null });
1260+
});
1261+
1262+
expect(dataDisposeSpy).toHaveBeenCalled();
1263+
expect(exitDisposeSpy).toHaveBeenCalled();
1264+
});
1265+
1266+
it('should dispose of PTY event listeners on abort', async () => {
1267+
const dataDisposeSpy = vi.fn();
1268+
const exitDisposeSpy = vi.fn();
1269+
1270+
mockPtyProcess.onData.mockReturnValue({ dispose: dataDisposeSpy });
1271+
mockPtyProcess.onExit.mockReturnValue({ dispose: exitDisposeSpy });
1272+
1273+
const abortController = new AbortController();
1274+
const handle = await ShellExecutionService.execute(
1275+
'long-running',
1276+
'/test/dir',
1277+
onOutputEventMock,
1278+
abortController.signal,
1279+
true,
1280+
shellExecutionConfig,
1281+
);
1282+
1283+
await new Promise((resolve) => process.nextTick(resolve));
1284+
abortController.abort();
1285+
1286+
// Simulate PTY process exit resulting from abort/SIGKILL
1287+
mockPtyProcess.onExit.mock.calls[0][0]({ exitCode: 1, signal: 9 });
1288+
await handle.result;
1289+
1290+
expect(dataDisposeSpy).toHaveBeenCalled();
1291+
expect(exitDisposeSpy).toHaveBeenCalled();
1292+
});
1293+
1294+
it('should fall back to child_process when PTY creation fails with ENXIO', async () => {
1295+
const ptyError = new Error('posix_openpt failed: Device not configured');
1296+
// @ts-expect-error adding custom code property
1297+
ptyError.code = 'ENXIO';
1298+
mockPtySpawn.mockImplementationOnce(() => {
1299+
throw ptyError;
1300+
});
1301+
1302+
// Mock child process fallback
1303+
const mockFallbackChild = new EventEmitter() as unknown as ChildProcess;
1304+
Object.defineProperty(mockFallbackChild, 'stdout', {
1305+
value: new EventEmitter(),
1306+
});
1307+
Object.defineProperty(mockFallbackChild, 'stderr', {
1308+
value: new EventEmitter(),
1309+
});
1310+
Object.defineProperty(mockFallbackChild, 'kill', {
1311+
value: vi.fn(),
1312+
});
1313+
Object.defineProperty(mockFallbackChild, 'pid', {
1314+
value: 9999,
1315+
});
1316+
mockCpSpawn.mockReturnValueOnce(mockFallbackChild);
1317+
1318+
const abortController = new AbortController();
1319+
const handle = await ShellExecutionService.execute(
1320+
'test-fallback',
1321+
'/test/dir',
1322+
onOutputEventMock,
1323+
abortController.signal,
1324+
true,
1325+
shellExecutionConfig,
1326+
);
1327+
1328+
// Simulate exit of standard child process fallback to allow handle to resolve
1329+
mockFallbackChild.emit('exit', 0, null);
1330+
mockFallbackChild.emit('close', 0, null);
1331+
1332+
const result = await handle.result;
1333+
expect(result.executionMethod).toBe('child_process');
1334+
expect(mockCpSpawn).toHaveBeenCalled();
1335+
});
12501336
});
12511337
});
12521338

0 commit comments

Comments
 (0)