From d33e8f3c08788d29e9127b4b6b62b2ba5d64fc84 Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 00:06:12 +0000 Subject: [PATCH 1/4] test(terminal): let cleanup fixture exit on request --- .../src/terminal/worker-terminal-manager.test.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/server/src/terminal/worker-terminal-manager.test.ts b/packages/server/src/terminal/worker-terminal-manager.test.ts index d953d6cb69..21edc39ecd 100644 --- a/packages/server/src/terminal/worker-terminal-manager.test.ts +++ b/packages/server/src/terminal/worker-terminal-manager.test.ts @@ -771,10 +771,16 @@ it("never adopts a live worker's terminals after a kill cleanup it could not fin temporaryDirs.push(cwd); const first = await startRealWorkerManager({ paseoHome }); manager = first; + const exitRequestPath = join(cwd, "exit-requested"); const ending = await first.createTerminal({ workspaceId: "ws-unfinished-kill", cwd, - ...nodeTerminalCommand("setInterval(() => {}, 1000);"), + ...nodeTerminalCommand(` + const { existsSync } = require("node:fs"); + setInterval(() => { + if (existsSync(${JSON.stringify(exitRequestPath)})) process.exit(0); + }, 25); + `), }); const remaining = await first.createTerminal({ workspaceId: "ws-unfinished-kill", @@ -782,7 +788,6 @@ it("never adopts a live worker's terminals after a kill cleanup it could not fin ...nodeTerminalCommand("setInterval(() => {}, 1000);"), }); const pids = await recordedTerminalPids(paseoHome); - const endingPid = pids.get(ending.id)!; const remainingPid = pids.get(remaining.id)!; // A kill cleanup whose process table read fails signals nothing and leaves the worker live. @@ -797,7 +802,7 @@ it("never adopts a live worker's terminals after a kill cleanup it could not fin expect((await readUnconfirmedTerminalCleanup(paseoHome)).length).toBeGreaterThan(0); // One terminal ends on its own: the live worker rewrites its record from memory. - process.kill(endingPid, "SIGKILL"); + writeFileSync(exitRequestPath, ""); await waitForCondition( async () => !(await recordedTerminalPids(paseoHome)).has(ending.id), 10000, From 94349a133e5b45470f3e62d4e33b097247fa2c36 Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 00:31:37 +0000 Subject: [PATCH 2/4] test(terminal): capture the Windows record rewrite failure --- .../src/terminal/terminal-worker-process.ts | 7 ++++- .../terminal/worker-terminal-manager.test.ts | 29 +++++++++++++++---- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/packages/server/src/terminal/terminal-worker-process.ts b/packages/server/src/terminal/terminal-worker-process.ts index 6c13ba04d5..8bf306d2e5 100644 --- a/packages/server/src/terminal/terminal-worker-process.ts +++ b/packages/server/src/terminal/terminal-worker-process.ts @@ -223,7 +223,12 @@ function runWorker(argv: string[]): void { ...record, terminals: record.terminals.filter((entry) => sessionsById.has(entry.id)), }; - writeTerminalWorkerRecord(args.paseoHome, record); + try { + writeTerminalWorkerRecord(args.paseoHome, record); + } catch (error) { + console.error("CI terminal record write failed:", (error as NodeJS.ErrnoException).code); + throw error; + } } function removeOwnFiles(): void { diff --git a/packages/server/src/terminal/worker-terminal-manager.test.ts b/packages/server/src/terminal/worker-terminal-manager.test.ts index 21edc39ecd..1758822fce 100644 --- a/packages/server/src/terminal/worker-terminal-manager.test.ts +++ b/packages/server/src/terminal/worker-terminal-manager.test.ts @@ -43,6 +43,7 @@ import { listTerminalWorkerRecords, readTerminalWorkerRecord, terminalWorkerDirectory, + terminalWorkerLogPath, type TerminalWorkerRecord, } from "./terminal-worker-record.js"; import { TERMINAL_WORKER_PROTOCOL_OVERRIDE_ENV } from "./terminal-worker-protocol.js"; @@ -772,13 +773,17 @@ it("never adopts a live worker's terminals after a kill cleanup it could not fin const first = await startRealWorkerManager({ paseoHome }); manager = first; const exitRequestPath = join(cwd, "exit-requested"); + const exitObservedPath = join(cwd, "exit-observed"); const ending = await first.createTerminal({ workspaceId: "ws-unfinished-kill", cwd, ...nodeTerminalCommand(` - const { existsSync } = require("node:fs"); + const { existsSync, writeFileSync } = require("node:fs"); setInterval(() => { - if (existsSync(${JSON.stringify(exitRequestPath)})) process.exit(0); + if (existsSync(${JSON.stringify(exitRequestPath)})) { + writeFileSync(${JSON.stringify(exitObservedPath)}, ""); + process.exit(0); + } }, 25); `), }); @@ -788,6 +793,7 @@ it("never adopts a live worker's terminals after a kill cleanup it could not fin ...nodeTerminalCommand("setInterval(() => {}, 1000);"), }); const pids = await recordedTerminalPids(paseoHome); + const endingPid = pids.get(ending.id)!; const remainingPid = pids.get(remaining.id)!; // A kill cleanup whose process table read fails signals nothing and leaves the worker live. @@ -803,10 +809,21 @@ it("never adopts a live worker's terminals after a kill cleanup it could not fin // One terminal ends on its own: the live worker rewrites its record from memory. writeFileSync(exitRequestPath, ""); - await waitForCondition( - async () => !(await recordedTerminalPids(paseoHome)).has(ending.id), - 10000, - ); + try { + await waitForCondition( + async () => !(await recordedTerminalPids(paseoHome)).has(ending.id), + 10000, + ); + } catch (error) { + const workerLog = readFileSync(terminalWorkerLogPath(paseoHome), "utf8"); + console.error("CI terminal exit diagnostics", { + exitObserved: existsSync(exitObservedPath), + endingAlive: isProcessAlive(endingPid), + remainingAlive: isProcessAlive(remainingPid), + recordWriteErrors: workerLog.match(/CI terminal record write failed: \w+/g) ?? [], + }); + throw error; + } expect((await readUnconfirmedTerminalCleanup(paseoHome)).length).toBeGreaterThan(0); manager = await startRealWorkerManager({ paseoHome }); From 117cda20b60f3a271fa0460f2c96f2ae296c9ed3 Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 03:06:42 +0000 Subject: [PATCH 3/4] test(terminal): repeat Windows cleanup exit diagnostics --- .../terminal/worker-terminal-manager.test.ts | 115 +++++++++--------- 1 file changed, 59 insertions(+), 56 deletions(-) diff --git a/packages/server/src/terminal/worker-terminal-manager.test.ts b/packages/server/src/terminal/worker-terminal-manager.test.ts index 1758822fce..4206678913 100644 --- a/packages/server/src/terminal/worker-terminal-manager.test.ts +++ b/packages/server/src/terminal/worker-terminal-manager.test.ts @@ -766,18 +766,20 @@ async function recordedTerminalPids(paseoHome: string): Promise { - const paseoHome = createTemporaryHome(); - const cwd = mkdtempSync(join(tmpdir(), "worker-terminal-manager-unfinished-kill-")); - temporaryDirs.push(cwd); - const first = await startRealWorkerManager({ paseoHome }); - manager = first; - const exitRequestPath = join(cwd, "exit-requested"); - const exitObservedPath = join(cwd, "exit-observed"); - const ending = await first.createTerminal({ - workspaceId: "ws-unfinished-kill", - cwd, - ...nodeTerminalCommand(` +it.each(Array.from({ length: process.platform === "win32" ? 20 : 1 }, (_, index) => index))( + "never adopts a live worker's terminals after a kill cleanup it could not finish, even once the worker rewrites its record (diagnostic %i)", + async () => { + const paseoHome = createTemporaryHome(); + const cwd = mkdtempSync(join(tmpdir(), "worker-terminal-manager-unfinished-kill-")); + temporaryDirs.push(cwd); + const first = await startRealWorkerManager({ paseoHome }); + manager = first; + const exitRequestPath = join(cwd, "exit-requested"); + const exitObservedPath = join(cwd, "exit-observed"); + const ending = await first.createTerminal({ + workspaceId: "ws-unfinished-kill", + cwd, + ...nodeTerminalCommand(` const { existsSync, writeFileSync } = require("node:fs"); setInterval(() => { if (existsSync(${JSON.stringify(exitRequestPath)})) { @@ -786,53 +788,54 @@ it("never adopts a live worker's terminals after a kill cleanup it could not fin } }, 25); `), - }); - const remaining = await first.createTerminal({ - workspaceId: "ws-unfinished-kill", - cwd, - ...nodeTerminalCommand("setInterval(() => {}, 1000);"), - }); - const pids = await recordedTerminalPids(paseoHome); - const endingPid = pids.get(ending.id)!; - const remainingPid = pids.get(remaining.id)!; - - // A kill cleanup whose process table read fails signals nothing and leaves the worker live. - const inspector = createTerminalWorkerProcessInspector(); - await expect( - reapTerminalWorkers({ - paseoHome, - inspector: { ...inspector, listProcesses: () => Promise.reject(new Error("ps timed out")) }, - }), - ).rejects.toBeInstanceOf(TerminalCleanupNotConfirmedError); - await first.detach(); - expect((await readUnconfirmedTerminalCleanup(paseoHome)).length).toBeGreaterThan(0); - - // One terminal ends on its own: the live worker rewrites its record from memory. - writeFileSync(exitRequestPath, ""); - try { - await waitForCondition( - async () => !(await recordedTerminalPids(paseoHome)).has(ending.id), - 10000, - ); - } catch (error) { - const workerLog = readFileSync(terminalWorkerLogPath(paseoHome), "utf8"); - console.error("CI terminal exit diagnostics", { - exitObserved: existsSync(exitObservedPath), - endingAlive: isProcessAlive(endingPid), - remainingAlive: isProcessAlive(remainingPid), - recordWriteErrors: workerLog.match(/CI terminal record write failed: \w+/g) ?? [], }); - throw error; - } - expect((await readUnconfirmedTerminalCleanup(paseoHome)).length).toBeGreaterThan(0); + const remaining = await first.createTerminal({ + workspaceId: "ws-unfinished-kill", + cwd, + ...nodeTerminalCommand("setInterval(() => {}, 1000);"), + }); + const pids = await recordedTerminalPids(paseoHome); + const endingPid = pids.get(ending.id)!; + const remainingPid = pids.get(remaining.id)!; - manager = await startRealWorkerManager({ paseoHome }); + // A kill cleanup whose process table read fails signals nothing and leaves the worker live. + const inspector = createTerminalWorkerProcessInspector(); + await expect( + reapTerminalWorkers({ + paseoHome, + inspector: { ...inspector, listProcesses: () => Promise.reject(new Error("ps timed out")) }, + }), + ).rejects.toBeInstanceOf(TerminalCleanupNotConfirmedError); + await first.detach(); + expect((await readUnconfirmedTerminalCleanup(paseoHome)).length).toBeGreaterThan(0); - expect(manager.getTerminal(remaining.id)).toBeUndefined(); - expect(await manager.getTerminals(cwd)).toEqual([]); - await waitForCondition(() => !isProcessAlive(remainingPid), 10000); - expect(await readUnconfirmedTerminalCleanup(paseoHome)).toEqual([]); -}); + // One terminal ends on its own: the live worker rewrites its record from memory. + writeFileSync(exitRequestPath, ""); + try { + await waitForCondition( + async () => !(await recordedTerminalPids(paseoHome)).has(ending.id), + 10000, + ); + } catch (error) { + const workerLog = readFileSync(terminalWorkerLogPath(paseoHome), "utf8"); + console.error("CI terminal exit diagnostics", { + exitObserved: existsSync(exitObservedPath), + endingAlive: isProcessAlive(endingPid), + remainingAlive: isProcessAlive(remainingPid), + recordWriteErrors: workerLog.match(/CI terminal record write failed: \w+/g) ?? [], + }); + throw error; + } + expect((await readUnconfirmedTerminalCleanup(paseoHome)).length).toBeGreaterThan(0); + + manager = await startRealWorkerManager({ paseoHome }); + + expect(manager.getTerminal(remaining.id)).toBeUndefined(); + expect(await manager.getTerminals(cwd)).toEqual([]); + await waitForCondition(() => !isProcessAlive(remainingPid), 10000); + expect(await readUnconfirmedTerminalCleanup(paseoHome)).toEqual([]); + }, +); // The worker's own `ps`, delayed: a program that exits while the worker reads its birth time // lands its exit in the middle of creation. Only the worker launched while this PATH is set sees From 2197805b3da1b2efdce323e884594c9459eadcdb Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 03:37:17 +0000 Subject: [PATCH 4/4] test(terminal): isolate record reader contention on exit --- .../src/terminal/terminal-worker-process.ts | 1 + .../terminal/worker-terminal-manager.test.ts | 28 +++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/packages/server/src/terminal/terminal-worker-process.ts b/packages/server/src/terminal/terminal-worker-process.ts index 8bf306d2e5..576993da30 100644 --- a/packages/server/src/terminal/terminal-worker-process.ts +++ b/packages/server/src/terminal/terminal-worker-process.ts @@ -223,6 +223,7 @@ function runWorker(argv: string[]): void { ...record, terminals: record.terminals.filter((entry) => sessionsById.has(entry.id)), }; + console.error("CI terminal record write attempted"); try { writeTerminalWorkerRecord(args.paseoHome, record); } catch (error) { diff --git a/packages/server/src/terminal/worker-terminal-manager.test.ts b/packages/server/src/terminal/worker-terminal-manager.test.ts index 4206678913..65eb079839 100644 --- a/packages/server/src/terminal/worker-terminal-manager.test.ts +++ b/packages/server/src/terminal/worker-terminal-manager.test.ts @@ -2,7 +2,9 @@ import { afterEach, expect, it } from "vitest"; import { isPlatform } from "../test-utils/platform.js"; import { EventEmitter } from "node:events"; import { + closeSync, existsSync, + openSync, mkdirSync, mkdtempSync, readdirSync, @@ -44,6 +46,7 @@ import { readTerminalWorkerRecord, terminalWorkerDirectory, terminalWorkerLogPath, + terminalWorkerRecordPath, type TerminalWorkerRecord, } from "./terminal-worker-record.js"; import { TERMINAL_WORKER_PROTOCOL_OVERRIDE_ENV } from "./terminal-worker-protocol.js"; @@ -766,9 +769,9 @@ async function recordedTerminalPids(paseoHome: string): Promise index))( - "never adopts a live worker's terminals after a kill cleanup it could not finish, even once the worker rewrites its record (diagnostic %i)", - async () => { +it.each([false, true])( + "never adopts a live worker's terminals after a kill cleanup it could not finish, even once the worker rewrites its record (held reader: %s)", + async (holdReader) => { const paseoHome = createTemporaryHome(); const cwd = mkdtempSync(join(tmpdir(), "worker-terminal-manager-unfinished-kill-")); temporaryDirs.push(cwd); @@ -797,6 +800,7 @@ it.each(Array.from({ length: process.platform === "win32" ? 20 : 1 }, (_, index) const pids = await recordedTerminalPids(paseoHome); const endingPid = pids.get(ending.id)!; const remainingPid = pids.get(remaining.id)!; + const [record] = await workerRecords(paseoHome); // A kill cleanup whose process table read fails signals nothing and leaves the worker live. const inspector = createTerminalWorkerProcessInspector(); @@ -809,9 +813,16 @@ it.each(Array.from({ length: process.platform === "win32" ? 20 : 1 }, (_, index) await first.detach(); expect((await readUnconfirmedTerminalCleanup(paseoHome)).length).toBeGreaterThan(0); - // One terminal ends on its own: the live worker rewrites its record from memory. - writeFileSync(exitRequestPath, ""); + // Diagnostic: keep the same read handle the poller uses open across the exit write. + const recordFd = holdReader + ? openSync(terminalWorkerRecordPath(paseoHome, record!.pid), "r") + : null; + const attemptsBeforeExit = + readFileSync(terminalWorkerLogPath(paseoHome), "utf8").match( + /CI terminal record write attempted/g, + )?.length ?? 0; try { + writeFileSync(exitRequestPath, ""); await waitForCondition( async () => !(await recordedTerminalPids(paseoHome)).has(ending.id), 10000, @@ -819,12 +830,19 @@ it.each(Array.from({ length: process.platform === "win32" ? 20 : 1 }, (_, index) } catch (error) { const workerLog = readFileSync(terminalWorkerLogPath(paseoHome), "utf8"); console.error("CI terminal exit diagnostics", { + holdReader, + attemptsAfterExit: + (workerLog.match(/CI terminal record write attempted/g)?.length ?? 0) - + attemptsBeforeExit, + uncaughtExceptionLogged: workerLog.includes("Terminal worker uncaught exception"), exitObserved: existsSync(exitObservedPath), endingAlive: isProcessAlive(endingPid), remainingAlive: isProcessAlive(remainingPid), recordWriteErrors: workerLog.match(/CI terminal record write failed: \w+/g) ?? [], }); throw error; + } finally { + if (recordFd !== null) closeSync(recordFd); } expect((await readUnconfirmedTerminalCleanup(paseoHome)).length).toBeGreaterThan(0);