From 362ed36792d165825b502703917110509ebbf852 Mon Sep 17 00:00:00 2001 From: 404-Page-Found <139850808+404-Page-Found@users.noreply.github.com> Date: Wed, 23 Sep 2026 11:03:22 +1000 Subject: [PATCH 1/5] fix: bound e2e child and server cleanup for #323 --- tests/e2e/suggest-smoke.test.mjs | 90 +++++++++++++++++++++++++++----- 1 file changed, 76 insertions(+), 14 deletions(-) diff --git a/tests/e2e/suggest-smoke.test.mjs b/tests/e2e/suggest-smoke.test.mjs index 46c84c6..95fe3bd 100644 --- a/tests/e2e/suggest-smoke.test.mjs +++ b/tests/e2e/suggest-smoke.test.mjs @@ -12,13 +12,51 @@ function listen(server) { }); } +async function closeServer(server) { + if (!server.listening) return; + + const close = new Promise((resolve, reject) => { + server.close((error) => { + if (error) { + reject(error); + } else { + resolve(); + } + }); + }); + + server.closeAllConnections(); + await close; +} + function onceExit(child) { return new Promise((resolve, reject) => { + if (child.exitCode !== null || child.signalCode !== null) { + resolve({ code: child.exitCode, signal: child.signalCode }); + return; + } + child.on('error', reject); child.on('exit', (code, signal) => resolve({ code, signal })); }); } +function wait(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +async function stopChild(child, graceMs = 1000) { + if (child.exitCode !== null || child.signalCode !== null) return; + + const exit = onceExit(child); + child.kill('SIGINT'); + + if (!(await Promise.race([exit, wait(graceMs)]))) { + child.kill('SIGKILL'); + await Promise.race([exit, wait(graceMs)]); + } +} + function stripAnsi(text) { return text.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, ''); } @@ -87,9 +125,8 @@ function runSuggestUntil(args, { cwd, env, text }) { if (!settled && stdout.includes(text)) { settled = true; clearTimeout(timeout); - child.kill('SIGINT'); try { - await onceExit(child); + await stopChild(child); resolve({ stdout, stderr }); } catch (err) { reject(err); @@ -266,7 +303,7 @@ async function setupShowDiffFixture( const { requests, server } = createChatCompletionServer({ content, streamContent, requireStream }); const port = await listen(server); t.after(async () => { - server.close(); + await closeServer(server); await rm(root, { recursive: true, force: true }); }); @@ -284,6 +321,31 @@ async function setupShowDiffFixture( return { home, repo, requests }; } +test('stubborn child processes are forcibly cleaned up after SIGINT', async () => { + const child = spawn(process.execPath, [ + '-e', + "process.stdout.write('ready\\n'); process.on('SIGINT', () => {}); setInterval(() => {}, 1000);", + ], { + stdio: ['ignore', 'pipe', 'ignore'], + }); + + await new Promise((resolve, reject) => { + child.stdout.setEncoding('utf8'); + child.stdout.once('data', (chunk) => { + if (chunk.includes('ready')) { + resolve(); + } else { + reject(new Error('Stubborn child did not signal readiness')); + } + }); + child.once('error', reject); + }); + + await stopChild(child); + + assert.ok(child.exitCode !== null || child.signalCode !== null); +}); + test('suggest smoke test boots the CLI, loads config, and prints suggestions', async (t) => { const root = await mkdtemp(join(tmpdir(), 'commit-echo-e2e-')); const { home, repo, configDir } = await setupRepo(root); @@ -311,7 +373,7 @@ test('suggest smoke test boots the CLI, loads config, and prints suggestions', a }); const port = await listen(server); t.after(async () => { - server.close(); + await closeServer(server); await rm(root, { recursive: true, force: true }); }); @@ -401,7 +463,7 @@ test('suggest --auto selects the first suggestion like --yes without committing' }); const port = await listen(server); t.after(async () => { - server.close(); + await closeServer(server); await rm(root, { recursive: true, force: true }); }); @@ -456,7 +518,7 @@ test('top-level --auto commits the first suggestion like --yes', async (t) => { }); const port = await listen(server); t.after(async () => { - server.close(); + await closeServer(server); await rm(root, { recursive: true, force: true }); }); @@ -502,7 +564,7 @@ test('suggest --commit --yes refuses a staged diff changed during analysis', asy }); const port = await listen(server); t.after(async () => { - server.close(); + await closeServer(server); await rm(root, { recursive: true, force: true }); }); @@ -529,7 +591,7 @@ test('suggest --commit rejects a staged diff changed during interactive confirma const { server } = createChatCompletionServer({ content: '1. feat: reject interactive changed diff' }); const port = await listen(server); t.after(async () => { - server.close(); + await closeServer(server); await rm(root, { recursive: true, force: true }); }); @@ -590,7 +652,7 @@ test('suggest reports beforeResponse failures instead of timing out', async (t) }); const port = await listen(server); t.after(async () => { - server.close(); + await closeServer(server); await rm(root, { recursive: true, force: true }); }); @@ -685,7 +747,7 @@ test('suggest --model overrides configured model for one invocation and -m is an }); const port = await listen(server); t.after(async () => { - server.close(); + await closeServer(server); await rm(root, { recursive: true, force: true }); }); @@ -926,7 +988,7 @@ test('suggest --stream prints incremental SSE output', async (t) => { }); const port = await listen(server); t.after(async () => { - server.close(); + await closeServer(server); await rm(root, { recursive: true, force: true }); }); @@ -1006,7 +1068,7 @@ test('suggest --stream prints incremental Anthropic SSE output', async (t) => { }); const port = await listen(server); t.after(async () => { - server.close(); + await closeServer(server); await rm(root, { recursive: true, force: true }); }); @@ -1080,7 +1142,7 @@ test('suggest --stream --yes streams output and auto-commits the first suggestio }); const port = await listen(server); t.after(async () => { - server.close(); + await closeServer(server); await rm(root, { recursive: true, force: true }); }); @@ -1197,7 +1259,7 @@ test('suggest --stream reports parse failure for unparseable streamed output', a }); const port = await listen(server); t.after(async () => { - server.close(); + await closeServer(server); await rm(root, { recursive: true, force: true }); }); From 2ff357a1c917808be6f8cc4ebdd93656c6a50908 Mon Sep 17 00:00:00 2001 From: 404-Page-Found <139850808+404-Page-Found@users.noreply.github.com> Date: Wed, 23 Sep 2026 11:04:02 +1000 Subject: [PATCH 2/5] fix: clean up timed out e2e children for #323 --- tests/e2e/suggest-smoke.test.mjs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/e2e/suggest-smoke.test.mjs b/tests/e2e/suggest-smoke.test.mjs index 95fe3bd..063b015 100644 --- a/tests/e2e/suggest-smoke.test.mjs +++ b/tests/e2e/suggest-smoke.test.mjs @@ -116,9 +116,11 @@ function runSuggestUntil(args, { cwd, env, text }) { let stderr = ''; let settled = false; const timeout = setTimeout(() => { + if (settled) return; settled = true; - child.kill('SIGINT'); - reject(new Error(`Timed out waiting for ${text}. stdout: ${stdout} stderr: ${stderr}`)); + void stopChild(child).finally(() => { + reject(new Error(`Timed out waiting for ${text}. stdout: ${stdout} stderr: ${stderr}`)); + }); }, 5000); child.stdout.on('data', async (chunk) => { stdout += chunk.toString(); @@ -164,9 +166,13 @@ function runNodeProcess(args, { cwd, env }, label) { const child = spawn(process.execPath, args, { cwd, env, stdio: ['ignore', 'pipe', 'pipe'] }); let stdout = ''; let stderr = ''; + let settled = false; const timeout = setTimeout(() => { - child.kill('SIGINT'); - reject(new Error(`Timed out running ${label}. stdout: ${stdout} stderr: ${stderr}`)); + if (settled) return; + settled = true; + void stopChild(child).finally(() => { + reject(new Error(`Timed out running ${label}. stdout: ${stdout} stderr: ${stderr}`)); + }); }, 8000); child.stdout.on('data', (chunk) => { stdout += chunk.toString(); @@ -175,10 +181,14 @@ function runNodeProcess(args, { cwd, env }, label) { stderr += chunk.toString(); }); child.on('error', (err) => { + if (settled) return; + settled = true; clearTimeout(timeout); reject(err); }); child.on('exit', (code, signal) => { + if (settled) return; + settled = true; clearTimeout(timeout); resolve({ code, signal, stdout, stderr }); }); From 7fedacdb8314b1d6320d64b08964bd587a5044a9 Mon Sep 17 00:00:00 2001 From: 404-Page-Found <139850808+404-Page-Found@users.noreply.github.com> Date: Wed, 23 Sep 2026 12:11:17 +1000 Subject: [PATCH 3/5] fix: resolve e2e cleanup review comments --- tests/e2e/suggest-smoke.test.mjs | 34 +++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/tests/e2e/suggest-smoke.test.mjs b/tests/e2e/suggest-smoke.test.mjs index 063b015..11a8b87 100644 --- a/tests/e2e/suggest-smoke.test.mjs +++ b/tests/e2e/suggest-smoke.test.mjs @@ -118,9 +118,11 @@ function runSuggestUntil(args, { cwd, env, text }) { const timeout = setTimeout(() => { if (settled) return; settled = true; - void stopChild(child).finally(() => { - reject(new Error(`Timed out waiting for ${text}. stdout: ${stdout} stderr: ${stderr}`)); - }); + void stopChild(child) + .catch(() => undefined) + .finally(() => { + reject(new Error(`Timed out waiting for ${text}. stdout: ${stdout} stderr: ${stderr}`)); + }); }, 5000); child.stdout.on('data', async (chunk) => { stdout += chunk.toString(); @@ -170,9 +172,11 @@ function runNodeProcess(args, { cwd, env }, label) { const timeout = setTimeout(() => { if (settled) return; settled = true; - void stopChild(child).finally(() => { - reject(new Error(`Timed out running ${label}. stdout: ${stdout} stderr: ${stderr}`)); - }); + void stopChild(child) + .catch(() => undefined) + .finally(() => { + reject(new Error(`Timed out running ${label}. stdout: ${stdout} stderr: ${stderr}`)); + }); }, 8000); child.stdout.on('data', (chunk) => { stdout += chunk.toString(); @@ -340,15 +344,31 @@ test('stubborn child processes are forcibly cleaned up after SIGINT', async () = }); await new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + void stopChild(child) + .catch(() => undefined) + .finally(() => { + reject(new Error('Timed out waiting for stubborn child readiness')); + }); + }, 5000); + child.stdout.setEncoding('utf8'); child.stdout.once('data', (chunk) => { + clearTimeout(timeout); if (chunk.includes('ready')) { resolve(); } else { reject(new Error('Stubborn child did not signal readiness')); } }); - child.once('error', reject); + child.once('error', (error) => { + clearTimeout(timeout); + reject(error); + }); + child.once('exit', (code, signal) => { + clearTimeout(timeout); + reject(new Error(`Stubborn child exited before signaling readiness (code: ${code}, signal: ${signal})`)); + }); }); await stopChild(child); From 87e4644c1ef642535435b924d2afd3e2a94462c6 Mon Sep 17 00:00:00 2001 From: 404-Page-Found <139850808+404-Page-Found@users.noreply.github.com> Date: Wed, 23 Sep 2026 14:21:02 +1000 Subject: [PATCH 4/5] fix: retry transient Windows history lock errors --- src/history/store.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/history/store.ts b/src/history/store.ts index 50358c4..66c63d4 100644 --- a/src/history/store.ts +++ b/src/history/store.ts @@ -131,6 +131,15 @@ function hasErrorCode(error: unknown, code: string): boolean { return typeof error === 'object' && error !== null && 'code' in error && error.code === code; } +function isRetryableHistoryLockError(error: unknown): boolean { + if (hasErrorCode(error, 'EEXIST')) return true; + if (process.platform !== 'win32') return false; + + // Windows can briefly report sharing violations while another handle is closing + // or a lock file is being removed. Treat those as contention and retry. + return hasErrorCode(error, 'EPERM') || hasErrorCode(error, 'EACCES'); +} + function waitForHistoryLockRetry(): Promise { return new Promise((resolve) => setTimeout(resolve, HISTORY_LOCK_RETRY_MS)); } @@ -292,9 +301,11 @@ async function acquireHistoryLock(historyPath: string): Promise<() => Promise= deadline) { throw new Error(`Timed out waiting to update commit history: ${historyPath}`); } From d20c58c37e75379fe69260e6109c2e61857e137a Mon Sep 17 00:00:00 2001 From: 404-Page-Found <139850808+404-Page-Found@users.noreply.github.com> Date: Wed, 23 Sep 2026 14:28:48 +1000 Subject: [PATCH 5/5] fix: narrow Windows history lock retries --- src/history/store.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/history/store.ts b/src/history/store.ts index 66c63d4..7fe809e 100644 --- a/src/history/store.ts +++ b/src/history/store.ts @@ -135,9 +135,9 @@ function isRetryableHistoryLockError(error: unknown): boolean { if (hasErrorCode(error, 'EEXIST')) return true; if (process.platform !== 'win32') return false; - // Windows can briefly report sharing violations while another handle is closing - // or a lock file is being removed. Treat those as contention and retry. - return hasErrorCode(error, 'EPERM') || hasErrorCode(error, 'EACCES'); + // libuv maps Windows sharing violations to EBUSY. Keep ordinary permission + // failures as errors instead of misclassifying them as lock contention. + return hasErrorCode(error, 'EBUSY'); } function waitForHistoryLockRetry(): Promise { @@ -215,8 +215,9 @@ async function hasHistoryLockTakeover(lockPath: string): Promise { } return true; } catch (error) { - if (!hasErrorCode(error, 'ENOENT')) throw error; - return false; + if (hasErrorCode(error, 'ENOENT')) return false; + if (isRetryableHistoryLockError(error)) return true; + throw error; } } @@ -233,9 +234,8 @@ async function removeStaleHistoryLock(lockPath: string): Promise { await takeover.writeFile(lockSnapshot.ownerToken, 'utf-8'); await takeover.close(); } catch (error) { - if (hasErrorCode(error, 'EEXIST')) return; - if (!hasErrorCode(error, 'ENOENT')) throw error; - return; + if (hasErrorCode(error, 'ENOENT') || isRetryableHistoryLockError(error)) return; + throw error; } try { @@ -261,12 +261,12 @@ async function removeStaleHistoryLock(lockPath: string): Promise { } } } catch (error) { - if (!hasErrorCode(error, 'ENOENT')) throw error; + if (!hasErrorCode(error, 'ENOENT') && !isRetryableHistoryLockError(error)) throw error; } finally { await unlink(takeoverPath).catch(() => {}); } } catch (error) { - if (!hasErrorCode(error, 'ENOENT')) throw error; + if (!hasErrorCode(error, 'ENOENT') && !isRetryableHistoryLockError(error)) throw error; } }