fix(test): wait for interactive PTY sessions to end during cleanup

integration-tests/test-helper.test.ts

@@ -17,6 +17,12 @@ function isProcessAlive(pid: number): boolean {
   }
 }
 
+// How long the stand-in below stays alive after it is signalled. The real CLI
+// traps SIGHUP and exits only once its own exit-cleanup chain has drained, so
+// a stand-in that dies on the default action would let cleanup() return early
+// and still look correct.
+const STAND_IN_EXIT_DELAY_MS = 750;
+
 describe('TestRig', () => {
   const originalKeepOutput = process.env['KEEP_OUTPUT'];
 
@@ -63,7 +69,7 @@ describe('TestRig', () => {
     expect(existsSync(testDir)).toBe(true);
   });
 
-  it('kills an interactive session a test never closed during cleanup', async () => {
+  it('waits for an interactive session a test never closed to end', async () => {
     // KEEP_OUTPUT is what CI sets, and it makes cleanup() keep the test
     // directory — the spawned child must not survive that path either.
     process.env['KEEP_OUTPUT'] = 'true';
@@ -72,15 +78,30 @@ describe('TestRig', () => {
     // Stands in for the CLI bundle: what is under test is that cleanup ends
     // whatever runInteractive spawned, not what the CLI itself does.
     rig.bundlePath = rig.createFile(
-      'idle-cli.js',
-      'setInterval(() => {}, 1000);\n',
+      'slow-exit-cli.js',
+      'process.on("SIGHUP", () => setTimeout(() => process.exit(129), ' +
+        `${STAND_IN_EXIT_DELAY_MS}));\n` +
+        'setInterval(() => {}, 1000);\n' +
+        'process.stdout.write("STAND_IN_READY\\n");\n',
     );
 
     const { ptyProcess } = rig.runInteractive();
     expect(isProcessAlive(ptyProcess.pid)).toBe(true);
+    // Signal before the handler is installed and the default action ends the
+    // child at once, measuring nothing. A real session is booted by the time
+    // its test ends, so wait for the stand-in to report itself up.
+    expect(await rig.waitForText('STAND_IN_READY', 30_000)).toBe(true);
 
+    const cleanupStartedAt = Date.now();
     await rig.cleanup();
-
+    const cleanupTookMs = Date.now() - cleanupStartedAt;
+
+    // Signalling alone returns straight through the delay above, leaving the
+    // child forwarding PTY bytes into a worker vitest is tearing down.
+    expect(
+      cleanupTookMs,
+      'cleanup() returned before the interactive CLI child exited',
+    ).toBeGreaterThanOrEqual(STAND_IN_EXIT_DELAY_MS);
     await expect
       .poll(() => isProcessAlive(ptyProcess.pid), {
         message: 'the interactive CLI child outlived cleanup()',

integration-tests/test-helper.ts

@@ -124,6 +124,26 @@ export function validateModelOutput(
   return true;
 }
 
+// The CLI traps SIGHUP and exits only once `runExitCleanup()` has drained, a
+// chain it bounds at 5s (packages/cli/src/utils/cleanup.ts). Waiting longer
+// than that bound is what makes cleanup() return with the child actually gone.
+const INTERACTIVE_EXIT_GRACE_MS = 10_000;
+
+// Resolves when `promise` settles, or after `ms` if it never does. The timer
+// is cleared and unrefed so a won race leaves no handle holding the worker's
+// event loop open.
+function settleWithin(promise: Promise<unknown>, ms: number): Promise<void> {
+  return new Promise((resolve) => {
+    const timer = setTimeout(resolve, ms);
+    timer.unref();
+    const settle = () => {
+      clearTimeout(timer);
+      resolve();
+    };
+    void promise.then(settle, settle);
+  });
+}
+
 // Simulates typing a string one character at a time to avoid paste detection.
 export async function type(ptyProcess: pty.IPty, text: string) {
   const delay = 5;
@@ -200,7 +220,10 @@ export class TestRig {
   testName?: string;
   _lastRunStdout?: string;
   _interactiveOutput = '';
-  private readonly interactiveProcesses: pty.IPty[] = [];
+  private readonly interactiveProcesses: Array<{
+    ptyProcess: pty.IPty;
+    exited: Promise<unknown>;
+  }> = [];
 
   constructor() {
     this.bundlePath = join(__dirname, '..', 'dist/cli.js');
@@ -496,13 +519,16 @@ export class TestRig {
   async cleanup() {
     // A session a test never closed keeps its CLI child forwarding PTY bytes
     // into this worker's stdout; after vitest tears the worker down those
-    // writes EPIPE and fail an otherwise all-green run (#10969).
-    for (const ptyProcess of this.interactiveProcesses.splice(0)) {
+    // writes EPIPE and fail an otherwise all-green run (#10969). Signalling
+    // alone still returns with the child alive and writing, so wait for it to
+    // actually go away (#10990).
+    for (const { ptyProcess, exited } of this.interactiveProcesses.splice(0)) {
       try {
         ptyProcess.kill();
       } catch {
         // Process may have already exited
       }
+      await settleWithin(exited, INTERACTIVE_EXIT_GRACE_MS);
     }
 
     // Clean up test directory
@@ -944,7 +970,10 @@ export class TestRig {
         ...e2eRendererEnv(renderer),
       } as { [key: string]: string },
     });
-    this.interactiveProcesses.push(ptyProcess);
+    const exited = new Promise<void>((resolve) => {
+      ptyProcess.onExit(() => resolve());
+    });
+    this.interactiveProcesses.push({ ptyProcess, exited });
 
     ptyProcess.onData((data) => {
       this._interactiveOutput += data;