From 86927552c23defcbe9d3118ba0511bb6c8311101 Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Sat, 3 Oct 2026 14:30:44 +0000 Subject: [PATCH 1/2] fix: let the sidecar tests exit when a killed bash outlives its SIGHUP tool-reap.test.js kills a PTY right as bash redraws its prompt after Ctrl+C; bash occasionally loses that SIGHUP and stays alive, and its PTY handle keeps the node --test file process open forever. Run the sidecar suite with --test-force-exit, and bound CI's Test step so any future never-exiting test file fails in minutes instead of hours. --- .github/workflows/ci.yml | 3 +++ standalone/sidecar/package.json | 2 +- standalone/sidecar/tool-reap.test.js | 3 +++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7bd3d1e4a..28ec494c1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -81,7 +81,10 @@ jobs: echo "::error::could not install zsh; the shell-integration suite would silently cover only bash" exit 1 + # It finishes in about 4 minutes. Without a bound, a test file that never + # exits holds the job until the 6-hour default. - name: Test + timeout-minutes: 20 run: pnpm test - name: Build diff --git a/standalone/sidecar/package.json b/standalone/sidecar/package.json index 712efbf0f..3a0c4e74a 100644 --- a/standalone/sidecar/package.json +++ b/standalone/sidecar/package.json @@ -4,7 +4,7 @@ "version": "0.1.0", "main": "main.js", "scripts": { - "test": "node --test" + "test": "node --test --test-force-exit" }, "dependencies": { "detect-libc": "2.1.2", diff --git a/standalone/sidecar/tool-reap.test.js b/standalone/sidecar/tool-reap.test.js index cac415209..5262bcfc1 100644 --- a/standalone/sidecar/tool-reap.test.js +++ b/standalone/sidecar/tool-reap.test.js @@ -94,6 +94,9 @@ function session(shell = BASH) { const seen = await waitFor(mark, (s) => prompts(s) >= 1, 'prompt after Ctrl+C'); return /\x1b\]367;dehydrate;([^\x07]*)\x07/.exec(seen)?.[1] ?? null; }, + // A SIGHUP that lands as bash redraws its prompt is occasionally lost, + // leaving the shell alive and its PTY holding the test process open; + // package.json's `--test-force-exit` ends the file anyway. kill() { mgr.kill('tool'); }, close() { mgr.killAll(); From be189acadf0820ffd54bbdf96524403098d4f043 Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Sat, 3 Oct 2026 14:38:07 +0000 Subject: [PATCH 2/2] fix: settle tool-reap's two other flakes: early Ctrl+C and history writes The test Tool printed HANDED= before installing its SIGINT handler, and the test sends Ctrl+C as soon as it reads HANDED=, so the signal could kill Node before any dehydrate payload (red on this PR's first CI run, zsh). The session's kill() and close() also removed the bare home while the killed shell was still writing its history (ENOTEMPTY). Install the handler first, and wait (bounded) for the shell's exit before respawning or removing. --- standalone/sidecar/tool-reap.test.js | 41 ++++++++++++++++++---------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/standalone/sidecar/tool-reap.test.js b/standalone/sidecar/tool-reap.test.js index 5262bcfc1..3d9dc571d 100644 --- a/standalone/sidecar/tool-reap.test.js +++ b/standalone/sidecar/tool-reap.test.js @@ -31,13 +31,14 @@ const raw = process.env.DORMOUSE_DEHYDRATE; let handed = null; try { const parsed = JSON.parse(raw); if (parsed && parsed.v === 1 && parsed.state != null) handed = parsed.state; } catch {} const state = handed ?? { stops: 0 }; -process.stdout.write('\\x1b]367;serve;{"dehydrate":true,"v":1}\\x07'); -process.stdout.write('HANDED=' + JSON.stringify(handed) + '\\n'); +// The handler goes in before HANDED: the test sends Ctrl+C as soon as it reads it. process.on('SIGINT', () => { state.stops += 1; process.stdout.write('\\x1b]367;dehydrate;' + JSON.stringify({ v: 1, state }) + '\\x07'); process.exit(0); }); +process.stdout.write('\\x1b]367;serve;{"dehydrate":true,"v":1}\\x07'); +process.stdout.write('HANDED=' + JSON.stringify(handed) + '\\n'); setInterval(() => {}, 1000); `; @@ -46,8 +47,11 @@ function session(shell = BASH) { writeFileSync(path.join(home, 'tool.js'), TOOL); let output = ''; const waiters = new Set(); + let onExit = null; const mgr = create((event, data) => { - if (event !== 'data' || data.id !== 'tool') return; + if (data.id !== 'tool') return; + if (event === 'exit') onExit?.(); + if (event !== 'data') return; output += data.data; for (const waiter of waiters) waiter(); }, require('node-pty')); @@ -94,13 +98,22 @@ function session(shell = BASH) { const seen = await waitFor(mark, (s) => prompts(s) >= 1, 'prompt after Ctrl+C'); return /\x1b\]367;dehydrate;([^\x07]*)\x07/.exec(seen)?.[1] ?? null; }, - // A SIGHUP that lands as bash redraws its prompt is occasionally lost, - // leaving the shell alive and its PTY holding the test process open; - // package.json's `--test-force-exit` ends the file anyway. - kill() { mgr.kill('tool'); }, - close() { - mgr.killAll(); - // A dying zsh can still be writing its history into the bare home. + /** Kill the shell and wait for it to exit: a dying shell writes its + * history into the bare home. A SIGHUP that lands as bash redraws its + * prompt is occasionally lost, so the wait is bounded; the shell left + * alive holds the test process open, which package.json's + * `--test-force-exit` ends. */ + async kill() { + if (!mgr.hasPty('tool')) return; + let timer; + const exited = new Promise((resolve) => { onExit = resolve; timer = setTimeout(resolve, 3_000); }); + mgr.kill('tool'); + await exited; + clearTimeout(timer); + onExit = null; + }, + async close() { + await this.kill(); rmSync(home, { recursive: true, force: true, maxRetries: 10, retryDelay: 50 }); }, }; @@ -114,7 +127,7 @@ for (const [name, shell] of [['bash', BASH], ['zsh', ZSH]]) test(`${name}: a Too assert.equal(await s.run(), null); const payload = await s.stop(); assert.equal(payload, JSON.stringify({ v: 1, state: { stops: 1 } })); - s.kill(); + await s.kill(); // The rehydrate: a fresh shell carrying the payload to its first command. await s.spawn(payload); @@ -124,7 +137,7 @@ for (const [name, shell] of [['bash', BASH], ['zsh', ZSH]]) test(`${name}: a Too assert.equal(await s.run(), null); await s.stop(); } finally { - s.close(); + await s.close(); } }); @@ -135,9 +148,9 @@ test('a missing, garbage, or oversized payload restarts the Tool from its args', await s.spawn(dehydrate); assert.equal(await s.run(), null, `handed for ${String(dehydrate).slice(0, 20)}`); await s.stop(); - s.kill(); + await s.kill(); } } finally { - s.close(); + await s.close(); } });