From 36fcb7c547147efaa146500fe7896d59f4ed010b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20Such=C3=A1nek?= Date: Mon, 12 Oct 2026 16:23:09 +0200 Subject: [PATCH 1/4] fix(server): archiving a thread stops only its own agent-browser daemon, over its socket Archiving or deleting a thread stopped its agent-browser daemon with SIGTERM to the process its .pid named, after checking that the command line was agent-browser's. A stale .pid whose number another session's daemon has now would stop that daemon and could release its tab, and the check and the signal raced (found in LazurioPlatform#348, which had the same path). stopSessionDaemon now sends agent-browser's own graceful shutdown (__agent_browser_internal_shutdown, as its CLI does before restarting a daemon) on the session's own socket, t3-.sock, and waits up to 5 s for the .pid to go. Nothing listening means no daemon; a refusal or no answer is "failed" and is logged. No process id is signalled. The tests run a live process named agent-browser whose id a stale t3-a.pid names, with only t3-b's daemon listening: archiving touches neither. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/lazurio/holders.test.ts | 101 ++++++++++--------- apps/server/src/lazurio/holders.ts | 112 ++++++++++++++++----- apps/server/src/lazurio/threadTabs.test.ts | 36 ++++--- apps/server/src/lazurio/threadTabs.ts | 2 +- docs/operations/lazurio-fork-release.md | 16 +-- 5 files changed, 178 insertions(+), 89 deletions(-) diff --git a/apps/server/src/lazurio/holders.test.ts b/apps/server/src/lazurio/holders.test.ts index 89faeeb20f232..e34e2c135acb2 100644 --- a/apps/server/src/lazurio/holders.test.ts +++ b/apps/server/src/lazurio/holders.test.ts @@ -1,5 +1,7 @@ // @effect-diagnostics nodeBuiltinImport:off globalTimers:off - the holder files are real files in a temporary runtime directory, and the watch is the operating system's. +import * as NodeChildProcess from "node:child_process"; import * as NodeFSP from "node:fs/promises"; +import * as NodeNet from "node:net"; import * as NodeOS from "node:os"; import * as NodePath from "node:path"; import { afterEach, describe, expect, it } from "vite-plus/test"; @@ -408,53 +410,62 @@ describe("T3 Code's holder files", () => { }); describe("stopping a thread's agent-browser daemon", () => { - const setup = async (pid: string | null) => { + // LazurioPlatform#348: the stop goes to the session's own socket and never signals a process + // id, so a stale .pid whose number another session's daemon (or any process named + // agent-browser) has now is never touched. + it("asks only the session's own daemon, on its socket, and signals no process", async () => { const sockets = await temporary("holders-daemon-"); - if (pid !== null) await NodeFSP.writeFile(NodePath.join(sockets, "t3-a.pid"), pid); - const terminated: number[] = []; - return { sockets, terminated, terminate: (target: number) => terminated.push(target) }; - }; - - it("sends SIGTERM only to a live agent-browser process", async () => { - const { sockets, terminated, terminate } = await setup("4242\n"); - const commandLine = async () => "/home/u/.local/bin/agent-browser-linux-x64\0"; - expect( - await stopSessionDaemon(sockets, "t3-a", { alive: () => true, commandLine, terminate }), - ).toBe("stopped"); - expect(terminated).toEqual([4242]); + const heard: Record = { "t3-a": [], "t3-b": [] }; + const servers: NodeNet.Server[] = []; + const daemon = (session: "t3-a" | "t3-b") => + new Promise((resolve) => { + const server = NodeNet.createServer((socket) => { + socket.on("data", async (chunk) => { + const line = chunk.toString("utf8").trim(); + heard[session]?.push(line); + const { id } = JSON.parse(line) as { id: string }; + socket.write(`${JSON.stringify({ id, success: true, data: { closed: true } })}\n`); + await NodeFSP.rm(NodePath.join(sockets, `${session}.pid`), { force: true }); + }); + }); + servers.push(server); + server.listen(NodePath.join(sockets, `${session}.sock`), () => resolve()); + }); + // A live process named agent-browser whose id t3-a's stale .pid names. + const named = NodePath.join(sockets, "agent-browser"); + await NodeFSP.symlink("/bin/sleep", named); + const sleeper = NodeChildProcess.spawn(named, ["30"], { stdio: "ignore" }); + try { + expect(await stopSessionDaemon(sockets, "t3-a")).toBe("none"); + await NodeFSP.writeFile(NodePath.join(sockets, "t3-a.pid"), `${sleeper.pid}\n`); + await daemon("t3-b"); + expect(await stopSessionDaemon(sockets, "t3-a")).toBe("none"); + expect(heard["t3-b"]).toEqual([]); + expect(sleeper.exitCode).toBeNull(); + expect(sleeper.signalCode).toBeNull(); + // A socket file nobody listens on is no daemon either. + await NodeFSP.writeFile(NodePath.join(sockets, "t3-c.sock"), ""); + expect(await stopSessionDaemon(sockets, "t3-c")).toBe("none"); + // t3-a's own daemon: one shutdown line, then its .pid is gone. + await daemon("t3-a"); + expect(await stopSessionDaemon(sockets, "t3-a")).toBe("stopped"); + expect(heard["t3-a"]?.map((line) => JSON.parse(line))).toEqual([ + { id: "lazurio-browser-done", action: "__agent_browser_internal_shutdown" }, + ]); + await expect(NodeFSP.access(NodePath.join(sockets, "t3-a.pid"))).rejects.toThrow(); + expect(sleeper.signalCode).toBeNull(); + } finally { + sleeper.kill(); + for (const server of servers) server.close(); + } }); - it("never signals a reused pid, an ended one or an unclear one", async () => { - const reused = await setup("4242"); - expect( - await stopSessionDaemon(reused.sockets, "t3-a", { - alive: () => true, - commandLine: async () => "/usr/bin/python3\0server.py\0", - terminate: reused.terminate, - }), - ).toBe("not-agent-browser"); - expect(reused.terminated).toEqual([]); - for (const [pid, alive] of [ - ["4242", false], - ["1", true], - ["", true], - ["x", true], - ] as const) { - const fixture = await setup(pid); - expect( - await stopSessionDaemon(fixture.sockets, "t3-a", { - alive: () => alive, - commandLine: async () => "agent-browser\0", - terminate: fixture.terminate, - }), - ).toBe("none"); - expect(fixture.terminated).toEqual([]); - } - // No .pid: no daemon runs. - const none = await setup(null); - expect(await stopSessionDaemon(none.sockets, "t3-a", { terminate: none.terminate })).toBe( - "none", - ); - expect(none.terminated).toEqual([]); + it("a daemon that refuses or does not answer is a failure that changes nothing", async () => { + const sockets = await temporary("holders-daemon-"); + const refusing = { ask: async () => ({ kind: "reply", line: '{"success":false}' }) as const }; + expect(await stopSessionDaemon(sockets, "t3-a", refusing)).toBe("failed"); + const silent = { ask: async () => ({ kind: "no-answer" }) as const }; + expect(await stopSessionDaemon(sockets, "t3-a", silent)).toBe("failed"); + expect(await stopSessionDaemon(sockets, "../t3-a")).toBe("none"); }); }); diff --git a/apps/server/src/lazurio/holders.ts b/apps/server/src/lazurio/holders.ts index 12583e7b8bf71..06a8bab26e77f 100644 --- a/apps/server/src/lazurio/holders.ts +++ b/apps/server/src/lazurio/holders.ts @@ -17,12 +17,14 @@ * parseHolderRecords, replaceOwnerFile, processAlive and `watchDirectories` (the Platform's * `watchHolds`). holders.test.ts runs the Platform's vectors, so a drift between the copies fails * there. Added here: the hub's rule for a daemon that runs (`daemonWorking`, from `readHolds`), - * the bindings of agent-browser's sessions, and the stop of a thread's daemon. + * the bindings of agent-browser's sessions, and the stop of a thread's daemon (the Platform's + * `stopSessionDaemon`, over the session's socket). */ import * as NodeCrypto from "node:crypto"; import * as NodeFS from "node:fs"; import * as NodeFSP from "node:fs/promises"; +import * as NodeNet from "node:net"; import * as NodePath from "node:path"; /** A strip's name is a file name: agent-browser's session alphabet, as T3 @@ -355,43 +357,105 @@ export async function readSessionBindings( return bindings; } -/** What the daemon stop needs of the host: overridable in tests (a fake process table). */ +/** What a session's daemon answered on its socket: nothing listens there (`absent`), a reply + * line, or no reply in time. */ +export type DaemonAnswer = + | Readonly<{ kind: "absent" }> + | Readonly<{ kind: "reply"; line: string }> + | Readonly<{ kind: "no-answer" }>; + +/** What the daemon stop and the bindings need of the host: overridable in tests. */ export type DaemonSeams = Readonly<{ alive?: (pid: number) => boolean; - /** `/proc//cmdline`, NUL-separated. */ - commandLine?: (pid: number) => Promise; - terminate?: (pid: number) => void; read?: (path: string) => Promise; + /** Sends one command line to a socket and answers the reply line. */ + ask?: (socketPath: string, line: string) => Promise; + /** Whether a file is still there, polled while the daemon exits. */ + exists?: (path: string) => Promise; + exitWithinMs?: number; }>; -/** The daemon is agent-browser's own executable, re-run with AGENT_BROWSER_DAEMON=1 and no - * arguments (agent-browser `cli/src/connection.rs`), so its command line names agent-browser. */ -const namesAgentBrowser = (commandLine: string) => - /^agent-browser(?:[-_.][A-Za-z0-9_.-]*)?$/.test( - NodePath.basename(commandLine.split("\0")[0] ?? ""), +/** agent-browser's own graceful shutdown of a session's daemon, as its CLI sends it before + * restarting a daemon (`request_graceful_daemon_shutdown` in agent-browser + * `cli/src/connection.rs`): the daemon runs its close, which only disconnects from a browser it + * did not launch, answers and exits, removing its `.pid`. The Platform's + * `agentBrowserShutdownAction`. */ +export const agentBrowserShutdownAction = "__agent_browser_internal_shutdown"; + +const daemonAnswerMs = 5_000; +const maxDaemonReplyBytes = 64 * 1024; + +/** One command line to a Unix socket and its reply line. Nothing listening (no socket, a stale + * one) is `absent`; no reply within 5 s is `no-answer`. The Platform's `askDaemonSocket`. */ +export function askDaemonSocket(socketPath: string, line: string): Promise { + return new Promise((resolve) => { + let settled = false; + let connected = false; + let buffer = ""; + const socket = NodeNet.createConnection({ path: socketPath }); + const done = (answer: DaemonAnswer) => { + if (settled) return; + settled = true; + clearTimeout(timer); + socket.destroy(); + resolve(answer); + }; + const timer = setTimeout(() => done({ kind: "no-answer" }), daemonAnswerMs); + socket.on("connect", () => { + connected = true; + socket.write(line); + }); + socket.on("data", (chunk) => { + buffer += chunk.toString("utf8"); + const end = buffer.indexOf("\n"); + if (end >= 0) done({ kind: "reply", line: buffer.slice(0, end) }); + else if (buffer.length > maxDaemonReplyBytes) done({ kind: "no-answer" }); + }); + socket.on("end", () => + done(buffer === "" ? { kind: "no-answer" } : { kind: "reply", line: buffer }), + ); + socket.on("error", () => done(connected ? { kind: "no-answer" } : { kind: "absent" })); + }); +} + +const fileExists = (path: string) => + NodeFSP.access(path).then( + () => true, + () => false, ); /** - * Stops a session's agent-browser daemon: SIGTERM to the process its `.pid` names, only when - * that is a well-formed id of a live process whose command line is agent-browser's. On SIGTERM - * the daemon of an attached browser disconnects without closing a tab and removes its `.pid`, so - * its binding no longer counts for the hub. Never `agent-browser close`, which first spawns a - * daemon when none runs. Resolves what happened; a failed signal is thrown. + * Stops a session's agent-browser daemon: asks it on that session's own socket + * (`/.sock`) to shut down, then waits up to 5 s for its `.pid` to go. The + * daemon of an attached browser disconnects without closing a tab, so its binding no longer + * counts for the hub. Nothing is signalled by a process id: a stale `.pid` whose number another + * process now has, even another session's daemon, is never touched (LazurioPlatform#348). Never + * `agent-browser close`, which first starts a daemon when none runs. `none`: nothing listens on + * the session's socket; `failed`: the daemon did not answer or refused. The Platform's + * `stopSessionDaemon`. */ export async function stopSessionDaemon( socketDirectory: string, session: string, seams: DaemonSeams = {}, -): Promise<"stopped" | "none" | "not-agent-browser"> { +): Promise<"stopped" | "none" | "failed"> { if (!isStrip(session)) return "none"; - const pidText = await fileText(NodePath.join(socketDirectory, `${session}.pid`), seams.read); - const pid = pidText === null ? null : daemonProcessId(pidText); - if (pid === null || !(seams.alive ?? processAlive)(pid)) return "none"; - const commandLine = await ( - seams.commandLine ?? ((id) => NodeFSP.readFile(`/proc/${id}/cmdline`, "utf8")) - )(pid).catch(() => ""); - if (!namesAgentBrowser(commandLine)) return "not-agent-browser"; - (seams.terminate ?? ((id) => process.kill(id, "SIGTERM")))(pid); + const answer = await (seams.ask ?? askDaemonSocket)( + NodePath.join(socketDirectory, `${session}.sock`), + `${JSON.stringify({ id: "lazurio-browser-done", action: agentBrowserShutdownAction })}\n`, + ); + if (answer.kind === "absent") return "none"; + if (answer.kind === "no-answer") return "failed"; + let success = false; + try { + success = (JSON.parse(answer.line) as { success?: unknown } | null)?.success === true; + } catch {} + if (!success) return "failed"; + const exists = seams.exists ?? fileExists; + const pid = NodePath.join(socketDirectory, `${session}.pid`); + const deadline = Date.now() + (seams.exitWithinMs ?? daemonAnswerMs); + while ((await exists(pid)) && Date.now() < deadline) + await new Promise((resolve) => setTimeout(resolve, 100)); return "stopped"; } diff --git a/apps/server/src/lazurio/threadTabs.test.ts b/apps/server/src/lazurio/threadTabs.test.ts index 4e4e27a131065..70feea8e2681c 100644 --- a/apps/server/src/lazurio/threadTabs.test.ts +++ b/apps/server/src/lazurio/threadTabs.test.ts @@ -54,7 +54,9 @@ async function world(options: { readonly declared?: boolean; readonly ready?: bo const listReads: number[] = []; /** The fake process table: pid → command line. */ const processes = new Map(); - const terminated: number[] = []; + /** Sessions whose agent-browser daemon listens on its socket, and those asked to stop. */ + const listening = new Set(); + const stopped: string[] = []; const logs: string[] = []; const watched: Array> = []; let retries = 0; @@ -92,10 +94,13 @@ async function world(options: { readonly declared?: boolean; readonly ready?: bo }, daemon: { alive: (pid) => processes.has(pid), - commandLine: async (pid) => processes.get(pid) ?? "", - terminate: (pid) => { - terminated.push(pid); - processes.delete(pid); + ask: async (socketPath) => { + const session = NodePath.basename(socketPath, ".sock"); + if (!listening.has(session)) return { kind: "absent" }; + listening.delete(session); + stopped.push(session); + await NodeFSP.rm(NodePath.join(sockets, `${session}.pid`), { force: true }); + return { kind: "reply", line: '{"success":true}' }; }, }, watch: (directories, onChange) => { @@ -132,7 +137,8 @@ async function world(options: { readonly declared?: boolean; readonly ready?: bo events, threads, processes, - terminated, + listening, + stopped, logs, watched, listReads, @@ -501,22 +507,25 @@ describe("holds and releases", () => { }); describe("archive and delete", () => { - it("removes the thread's file and stops only its live agent-browser daemon, never a reused pid", async () => { + it("removes the thread's file and asks only its own agent-browser daemon to stop, on its socket", async () => { const w = await world(); await w.core.hold(threadA, id(1)); await w.core.hold(threadB, id(2)); await w.bind(stripA, id(1), 4242); await w.bind(stripB, id(2), 4343); w.processes.set(4242, "/home/u/.local/bin/agent-browser\0"); - w.processes.set(4343, "/usr/bin/vim\0notes.txt\0"); + // B's .pid is stale: its number is another live agent-browser's, and nothing listens on B's + // socket. + w.processes.set(4343, "/home/u/.local/bin/agent-browser\0"); + w.listening.add(stripA); await w.core.lifecycle({ type: "retired", threadId: threadA }); expect(await w.stored()).toBeNull(); - expect(w.terminated).toEqual([4242]); + expect(w.stopped).toEqual([stripA]); expect(tabIds(w.lastSnapshot())).toEqual([]); - // The pid in B's .pid is another program now: nothing is killed. await w.core.lifecycle({ type: "retired", threadId: threadB }); - expect(w.terminated).toEqual([4242]); - expect(w.logs).toContain("thread-tabs-daemon-not-agent-browser"); + expect(w.stopped).toEqual([stripA]); + expect(w.processes.has(4343)).toBe(true); + expect(w.logs).not.toContain("thread-tabs-daemon-stop-failed"); }); it("closes no tab itself: a tab another file holds stays held there", async () => { @@ -592,10 +601,11 @@ describe("the startup sweep", () => { { id: id(6), recordedAt: at(50) }, ]); for (const pid of [5001, 5002, 5003]) w.processes.set(pid, "agent-browser\0"); + for (const session of [stripGone, stripDaemon, "codex-1"]) w.listening.add(session); await w.list([id(4), id(6)], at(100)); await w.declare(); expect(await w.stored(stripGone)).toBeNull(); - expect(w.terminated.sort()).toEqual([5001, 5002]); + expect(w.stopped.toSorted()).toEqual([stripGone, stripDaemon].toSorted()); expect(await w.stored("launchpad")).toEqual([{ id: id(4), recordedAt: at(50) }]); // Live threads' files are kept and reconciled. expect(await w.stored()).toEqual([{ id: id(6), recordedAt: at(50) }]); diff --git a/apps/server/src/lazurio/threadTabs.ts b/apps/server/src/lazurio/threadTabs.ts index 8f22ac1b980be..b6510d5c8e17e 100644 --- a/apps/server/src/lazurio/threadTabs.ts +++ b/apps/server/src/lazurio/threadTabs.ts @@ -465,7 +465,7 @@ export function createThreadTabs(seams: ThreadTabsCoreSeams): ThreadTabsCore { if (seams.socketDirectory === undefined) return; try { const outcome = await stopSessionDaemon(seams.socketDirectory, session, seams.daemon); - if (outcome === "not-agent-browser") log("thread-tabs-daemon-not-agent-browser", { session }); + if (outcome === "failed") log("thread-tabs-daemon-stop-failed", { session }); } catch (error) { log("thread-tabs-daemon-stop-failed", { session, cause: String(error) }); } diff --git a/docs/operations/lazurio-fork-release.md b/docs/operations/lazurio-fork-release.md index 383acf193e635..f36fc8d93d0e8 100644 --- a/docs/operations/lazurio-fork-release.md +++ b/docs/operations/lazurio-fork-release.md @@ -467,12 +467,16 @@ T3 žádný prohlížeč nemá, a tak overlay dělá tohle: `rel=noopener`) na tutéž URL: přihlašovací stránka brány se po vypršení přihlášení v rámu nevykreslí. - **Archivace a smazání.** Archivace i smazání vlákna smaže jeho soubor - držitele (i nečitelný) a démonovi sezení agent-browser vlákna pošle SIGTERM: - jen živému procesu z `/t3-.pid`, jehož `/proc//cmdline` - je agent-browser, nikdy znovu použitému pid. Démon připojeného prohlížeče se - odpojí, žádnou záložku nezavře a smaže svůj `.pid`, takže vazba pro hub - přestane počítat; `agent-browser close` se nevolá, protože by démona nejdřív - spustil. T3 nikdy nezavírá záložku sám: záložka, kterou drží Apps nebo jiné + držitele (i nečitelný) a démona sezení agent-browser vlákna požádá o + ukončení na jeho vlastním socketu `/t3-.sock` + (`__agent_browser_internal_shutdown`, které posílá i CLI agent-browseru před + restartem démona). Žádnému procesu nepošle signál podle pid, takže zastaralý + `.pid`, jehož číslo má teď jiný proces nebo démon jiného sezení, nic + nezastaví (LazurioPlatform#348). Když na socketu nikdo neposlouchá, démon + neběží. Démon připojeného prohlížeče se odpojí, žádnou záložku nezavře a + smaže svůj `.pid`, takže vazba pro hub přestane počítat. Příkaz + `agent-browser close` se nevolá, protože by démona nejdřív spustil. + T3 nikdy nezavírá záložku sám: záložka, kterou drží Apps nebo jiné vlákno nebo ji někdo sleduje, žije dál, jiná skončí po 8 s milosti hubu. Obnovení z archivu nic nevrátí. Podržení a archivace jdou jednou frontou zápisů a podržení se těsně před zápisem znovu ujistí, že vlákno žije a není From 965c470db0dc567006a8bd3fad33d5e8ebd67067 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20Such=C3=A1nek?= Date: Mon, 12 Oct 2026 16:30:25 +0200 Subject: [PATCH 2/4] fix(server): holders.ts may use timers and the clock outside the Effect runtime The socket shutdown waits for the daemon's answer and for its .pid to go with setTimeout and Date.now, as the Platform's copy does. The fork's typecheck flags both (globalTimers, globalDate) unless the file opts out; holders.ts now does, like the other overlay files that run outside the Effect runtime. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/lazurio/holders.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/server/src/lazurio/holders.ts b/apps/server/src/lazurio/holders.ts index 06a8bab26e77f..9c5d782cf5506 100644 --- a/apps/server/src/lazurio/holders.ts +++ b/apps/server/src/lazurio/holders.ts @@ -1,4 +1,4 @@ -// @effect-diagnostics nodeBuiltinImport:off - The holder files are the Platform's, read and written as it does. +// @effect-diagnostics nodeBuiltinImport:off globalDate:off globalTimers:off - The holder files and the daemon's socket are the Platform's, read, written and asked outside the Effect runtime as it does. /** * Lazurio overlay (plan DEV-6646, task 501; LazurioPlatform decision F47 points 4 and 5): who From be5e8e551c64543b05ab67470a2fa4d9b76dceb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20Such=C3=A1nek?= Date: Mon, 12 Oct 2026 16:42:09 +0200 Subject: [PATCH 3/4] fix(server): a thread's daemon counts as stopped only once its .pid is gone As in LazurioPlatform#348 after Pablo's review: an acknowledged shutdown whose .pid is still there after the wait, or a lost answer after the command went out, is "unconfirmed" (the daemon may still hold the tab), and a daemon that answers it did not shut down is "refused". Both are logged as thread-tabs-daemon-stop-failed with the outcome; only a gone .pid is "stopped". Co-Authored-By: Claude Opus 5.5 --- apps/server/src/lazurio/holders.test.ts | 13 ++++++++++--- apps/server/src/lazurio/holders.ts | 19 ++++++++++++------- apps/server/src/lazurio/threadTabs.ts | 3 ++- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/apps/server/src/lazurio/holders.test.ts b/apps/server/src/lazurio/holders.test.ts index e34e2c135acb2..b1f7f11af271d 100644 --- a/apps/server/src/lazurio/holders.test.ts +++ b/apps/server/src/lazurio/holders.test.ts @@ -460,12 +460,19 @@ describe("stopping a thread's agent-browser daemon", () => { } }); - it("a daemon that refuses or does not answer is a failure that changes nothing", async () => { + it("is stopped only once the .pid is gone: a refusal changes nothing, a lost answer or a lingering daemon is unconfirmed", async () => { const sockets = await temporary("holders-daemon-"); const refusing = { ask: async () => ({ kind: "reply", line: '{"success":false}' }) as const }; - expect(await stopSessionDaemon(sockets, "t3-a", refusing)).toBe("failed"); + expect(await stopSessionDaemon(sockets, "t3-a", refusing)).toBe("refused"); const silent = { ask: async () => ({ kind: "no-answer" }) as const }; - expect(await stopSessionDaemon(sockets, "t3-a", silent)).toBe("failed"); + expect(await stopSessionDaemon(sockets, "t3-a", silent)).toBe("unconfirmed"); + // Acknowledged, but the daemon has not exited: it may still hold the tab. + await NodeFSP.writeFile(NodePath.join(sockets, "t3-a.pid"), "4242\n"); + const lingering = { + ask: async () => ({ kind: "reply", line: '{"success":true}' }) as const, + exitWithinMs: 150, + }; + expect(await stopSessionDaemon(sockets, "t3-a", lingering)).toBe("unconfirmed"); expect(await stopSessionDaemon(sockets, "../t3-a")).toBe("none"); }); }); diff --git a/apps/server/src/lazurio/holders.ts b/apps/server/src/lazurio/holders.ts index 9c5d782cf5506..f71b98b015ea8 100644 --- a/apps/server/src/lazurio/holders.ts +++ b/apps/server/src/lazurio/holders.ts @@ -430,32 +430,37 @@ const fileExists = (path: string) => * daemon of an attached browser disconnects without closing a tab, so its binding no longer * counts for the hub. Nothing is signalled by a process id: a stale `.pid` whose number another * process now has, even another session's daemon, is never touched (LazurioPlatform#348). Never - * `agent-browser close`, which first starts a daemon when none runs. `none`: nothing listens on - * the session's socket; `failed`: the daemon did not answer or refused. The Platform's - * `stopSessionDaemon`. + * `agent-browser close`, which first starts a daemon when none runs. `stopped`: the daemon + * answered and its `.pid` is gone; `none`: nothing listens on the session's socket; `refused`: + * the daemon answered that it did not shut down; `unconfirmed`: no answer after the command went + * out, or the `.pid` still there after the wait, so the daemon may still hold the tab. The + * Platform's `stopSessionDaemon`. */ export async function stopSessionDaemon( socketDirectory: string, session: string, seams: DaemonSeams = {}, -): Promise<"stopped" | "none" | "failed"> { +): Promise<"stopped" | "none" | "refused" | "unconfirmed"> { if (!isStrip(session)) return "none"; const answer = await (seams.ask ?? askDaemonSocket)( NodePath.join(socketDirectory, `${session}.sock`), `${JSON.stringify({ id: "lazurio-browser-done", action: agentBrowserShutdownAction })}\n`, ); if (answer.kind === "absent") return "none"; - if (answer.kind === "no-answer") return "failed"; + // The command went out: the daemon may have taken it even if its answer was lost. + if (answer.kind === "no-answer") return "unconfirmed"; let success = false; try { success = (JSON.parse(answer.line) as { success?: unknown } | null)?.success === true; } catch {} - if (!success) return "failed"; + if (!success) return "refused"; const exists = seams.exists ?? fileExists; const pid = NodePath.join(socketDirectory, `${session}.pid`); const deadline = Date.now() + (seams.exitWithinMs ?? daemonAnswerMs); - while ((await exists(pid)) && Date.now() < deadline) + while (await exists(pid)) { + if (Date.now() >= deadline) return "unconfirmed"; await new Promise((resolve) => setTimeout(resolve, 100)); + } return "stopped"; } diff --git a/apps/server/src/lazurio/threadTabs.ts b/apps/server/src/lazurio/threadTabs.ts index b6510d5c8e17e..c37da31d53142 100644 --- a/apps/server/src/lazurio/threadTabs.ts +++ b/apps/server/src/lazurio/threadTabs.ts @@ -465,7 +465,8 @@ export function createThreadTabs(seams: ThreadTabsCoreSeams): ThreadTabsCore { if (seams.socketDirectory === undefined) return; try { const outcome = await stopSessionDaemon(seams.socketDirectory, session, seams.daemon); - if (outcome === "failed") log("thread-tabs-daemon-stop-failed", { session }); + if (outcome === "refused" || outcome === "unconfirmed") + log("thread-tabs-daemon-stop-failed", { session, outcome }); } catch (error) { log("thread-tabs-daemon-stop-failed", { session, cause: String(error) }); } From 4988c8ff423c453cca82037f09a436bd3a0fb1fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20Such=C3=A1nek?= Date: Mon, 12 Oct 2026 17:14:27 +0200 Subject: [PATCH 4/4] fix(server): a socket or .pid that cannot be reached never counts as no daemon or stopped Pablo's review of #56: every connection error before connect counted as "nothing listens", so a daemon whose socket cannot be opened (EACCES) was reported as none; and every failure of fs.access counted as "the .pid is gone", so an acknowledged shutdown whose .pid cannot be checked was reported as stopped. Only ENOENT, ECONNREFUSED and ENOTSOCK now prove that nothing listens, and only ENOENT proves the .pid gone; any other error is unconfirmed. Tests cover a listening socket of mode 000 and a .pid in a directory that cannot be read. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/lazurio/holders.test.ts | 32 +++++++++++++++++++++++++ apps/server/src/lazurio/holders.ts | 31 +++++++++++++++++++----- 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/apps/server/src/lazurio/holders.test.ts b/apps/server/src/lazurio/holders.test.ts index b1f7f11af271d..5b85a0c3cfff2 100644 --- a/apps/server/src/lazurio/holders.test.ts +++ b/apps/server/src/lazurio/holders.test.ts @@ -475,4 +475,36 @@ describe("stopping a thread's agent-browser daemon", () => { expect(await stopSessionDaemon(sockets, "t3-a", lingering)).toBe("unconfirmed"); expect(await stopSessionDaemon(sockets, "../t3-a")).toBe("none"); }); + + // Pablo's review of #56: a failure that proves nothing is never "none" or "stopped". + it("a socket or a .pid that cannot be reached is unconfirmed, never none or stopped", async () => { + const sockets = await temporary("holders-daemon-"); + const server = NodeNet.createServer(() => undefined); + await new Promise((resolve) => + server.listen(NodePath.join(sockets, "t3-a.sock"), () => resolve()), + ); + try { + // A daemon listens, but its socket cannot be opened. + await NodeFSP.chmod(NodePath.join(sockets, "t3-a.sock"), 0o000); + expect(await stopSessionDaemon(sockets, "t3-a")).toBe("unconfirmed"); + } finally { + server.close(); + } + // Acknowledged, and the .pid's directory cannot be read: not proven gone. + const pidDirectory = await temporary("holders-pid-"); + await NodeFSP.writeFile(NodePath.join(pidDirectory, "t3-a.pid"), "4242\n"); + const acknowledged = { + ask: async () => { + await NodeFSP.chmod(pidDirectory, 0o000); + return { kind: "reply", line: '{"success":true}' } as const; + }, + exitWithinMs: 150, + }; + try { + expect(await stopSessionDaemon(pidDirectory, "t3-a", acknowledged)).toBe("unconfirmed"); + } finally { + await NodeFSP.chmod(pidDirectory, 0o700); + } + expect(await NodeFSP.readFile(NodePath.join(pidDirectory, "t3-a.pid"), "utf8")).toBe("4242\n"); + }); }); diff --git a/apps/server/src/lazurio/holders.ts b/apps/server/src/lazurio/holders.ts index f71b98b015ea8..2339be80c80da 100644 --- a/apps/server/src/lazurio/holders.ts +++ b/apps/server/src/lazurio/holders.ts @@ -385,8 +385,14 @@ export const agentBrowserShutdownAction = "__agent_browser_internal_shutdown"; const daemonAnswerMs = 5_000; const maxDaemonReplyBytes = 64 * 1024; +/** Connection errors that prove nothing listens on a socket path: no file, a socket nobody + * listens on (Linux), a file that is no socket (macOS). Anything else, a permission above all, + * proves nothing. */ +const absentSocketCodes = new Set(["ENOENT", "ECONNREFUSED", "ENOTSOCK"]); + /** One command line to a Unix socket and its reply line. Nothing listening (no socket, a stale - * one) is `absent`; no reply within 5 s is `no-answer`. The Platform's `askDaemonSocket`. */ + * one) is `absent`; no reply within 5 s, or a socket that cannot be reached for any other reason, + * is `no-answer`. The Platform's `askDaemonSocket`. */ export function askDaemonSocket(socketPath: string, line: string): Promise { return new Promise((resolve) => { let settled = false; @@ -414,14 +420,25 @@ export function askDaemonSocket(socketPath: string, line: string): Promise done(buffer === "" ? { kind: "no-answer" } : { kind: "reply", line: buffer }), ); - socket.on("error", () => done(connected ? { kind: "no-answer" } : { kind: "absent" })); + socket.on("error", (error: NodeJS.ErrnoException) => + done( + !connected && absentSocketCodes.has(error.code ?? "") + ? { kind: "absent" } + : { kind: "no-answer" }, + ), + ); }); } +/** Whether a file is there: only "no such file" is gone; any other failure is thrown, since it + * proves nothing. */ const fileExists = (path: string) => NodeFSP.access(path).then( () => true, - () => false, + (error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT") return false; + throw error; + }, ); /** @@ -457,11 +474,13 @@ export async function stopSessionDaemon( const exists = seams.exists ?? fileExists; const pid = NodePath.join(socketDirectory, `${session}.pid`); const deadline = Date.now() + (seams.exitWithinMs ?? daemonAnswerMs); - while (await exists(pid)) { - if (Date.now() >= deadline) return "unconfirmed"; + for (;;) { + // Only a .pid proven gone counts; one that cannot be checked is not. + const present = await exists(pid).catch(() => null); + if (present === false) return "stopped"; + if (present === null || Date.now() >= deadline) return "unconfirmed"; await new Promise((resolve) => setTimeout(resolve, 100)); } - return "stopped"; } /** Calls `changed` whenever something in one of the directories changes.