diff options
| author | amankumarsingh77 <amankumarsingh7702@gmail.com> | 2026-07-28 19:46:57 +0530 |
|---|---|---|
| committer | Aman Kumar <145339995+amankumarsingh77@users.noreply.github.com> | 2026-08-02 19:12:46 +0530 |
| commit | ce1240fa9b3b74983cc22171469c24d0708b10ea (patch) | |
| tree | bed14eb1351f6b4e1114f64638bda63906a614ff | |
| parent | 226e814d47a5e1acd4d308619921ca1012723b88 (diff) | |
fix(tool): stop the socket gate rejecting every tool call on Windows
The tool gate ran access() on DAEMON_SOCKET_PATH, which on Windows is the named
pipe \\.\pipe\pi-browser-daemon. Named pipes are not filesystem entries, so
access() always fails there and every browser_* call returned "Browser harness
not initialized" no matter how healthy the daemon was. spawn.ts already skipped
the identical check for this reason; that knowledge is now one shared
daemonSocketMayExist() used by both call sites, with platform and path as
parameters so the branch is testable off-platform.
Report: docs/superpowers/reports/2026-07-26-bug-audit.md#f1
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| -rw-r--r-- | src/daemon/spawn.ts | 34 | ||||
| -rw-r--r-- | src/util/tool.ts | 7 | ||||
| -rw-r--r-- | test/util/daemon-socket-test.ts | 87 |
3 files changed, 113 insertions, 15 deletions
diff --git a/src/daemon/spawn.ts b/src/daemon/spawn.ts index d051fcc..165abcd 100644 --- a/src/daemon/spawn.ts +++ b/src/daemon/spawn.ts @@ -33,22 +33,36 @@ const cleanupStaleSocket = (): void => { }; /** + * Whether the daemon endpoint could plausibly exist, judged without connecting. + * + * On POSIX a missing socket file means the daemon definitely isn't running, so + * callers can short-circuit. Windows named pipes are NOT filesystem entries — + * access() always fails on `\\.\pipe\…` even while the daemon is serving it — so + * on Windows the answer is always "maybe" and the caller must probe instead. + * + * Platform and path are parameters so the branch is testable off-platform. + */ +export const daemonSocketMayExist = async ( + platform: NodeJS.Platform = process.platform, + socketPath: string = DAEMON_SOCKET_PATH, +): Promise<boolean> => { + if (platform === "win32") return true; + try { + await access(socketPath); + return true; + } catch { + return false; + } +}; + +/** * Check whether the daemon is running by probing its Unix socket. * A file-existence check is unreliable — a crashed daemon leaves a stale socket. * We connect, send a register, wait for the registered ack, then disconnect. * On failure, clean up the stale socket so ensureDaemon can spawn a fresh one. */ export const isDaemonRunning = async (timeoutMs = 2_000): Promise<boolean> => { - // On POSIX, a missing socket file means the daemon definitely isn't running, - // so we can short-circuit. Windows named pipes are not filesystem entries — - // access() always fails there — so skip the pre-check and probe directly. - if (!isWindows) { - try { - await access(DAEMON_SOCKET_PATH); - } catch { - return false; - } - } + if (!(await daemonSocketMayExist())) return false; return new Promise((resolve) => { let settled = false; diff --git a/src/util/tool.ts b/src/util/tool.ts index e4bb5ba..76087bc 100644 --- a/src/util/tool.ts +++ b/src/util/tool.ts @@ -8,8 +8,7 @@ import type { ToolRenderResultOptions, } from "@mariozechner/pi-coding-agent"; import type { Component } from "@mariozechner/pi-tui"; -import { access } from "node:fs/promises"; -import { DAEMON_SOCKET_PATH } from "../daemon/protocol"; +import { daemonSocketMayExist } from "../daemon/spawn"; import type { BrowserClient } from "../client"; import type { Result } from "./result"; @@ -141,9 +140,7 @@ export const registerBrowserTool = ( // Check daemon socket exists before attempting browser control. // If missing, the user hasn't run /browser-setup yet. Return a clear // error so the agent knows to direct the user to initialize. - try { - await access(DAEMON_SOCKET_PATH); - } catch { + if (!(await daemonSocketMayExist())) { return toToolResult( { success: false, diff --git a/test/util/daemon-socket-test.ts b/test/util/daemon-socket-test.ts new file mode 100644 index 0000000..eee0500 --- /dev/null +++ b/test/util/daemon-socket-test.ts @@ -0,0 +1,87 @@ +import { test, describe } from "node:test"; +import assert from "node:assert/strict"; +import { fileURLToPath } from "node:url"; +import { Type } from "typebox"; +import type { ExtensionAPI, ToolDefinition } from "@mariozechner/pi-coding-agent"; +import type { TSchema } from "typebox"; +import { daemonSocketMayExist } from "../../src/daemon/spawn"; +import { type AnyBrowserToolDefinition, registerBrowserTool } from "../../src/util/tool"; +import type { BrowserClient } from "../../src/client"; +import { ok } from "../../src/util/result"; + +const WINDOWS_PIPE = "\\\\.\\pipe\\pi-browser-daemon"; +const MISSING_SOCKET = "/pi-browser-harness/definitely/not/here.sock"; +const thisFile = fileURLToPath(import.meta.url); + +describe("daemon socket presence check", () => { + test("a Windows named pipe is never judged absent from the filesystem", async () => { + assert.equal(await daemonSocketMayExist("win32", WINDOWS_PIPE), true); + }); + + test("a missing POSIX socket path is judged absent", async () => { + assert.equal(await daemonSocketMayExist("linux", MISSING_SOCKET), false); + }); + + test("an existing POSIX path is judged present", async () => { + assert.equal(await daemonSocketMayExist("linux", thisFile), true); + }); + + test("darwin uses the filesystem like linux", async () => { + assert.equal(await daemonSocketMayExist("darwin", MISSING_SOCKET), false); + assert.equal(await daemonSocketMayExist("darwin", thisFile), true); + }); +}); + +type GateProbe = { readonly ran: ReadonlyArray<string>; readonly text: string }; + +const runGate = async (ensureAlive: boolean | undefined): Promise<GateProbe> => { + const ran: string[] = []; + const client = { + mutationMutex: () => ({ acquire: async () => () => {} }), + ensureAlive: async () => ok(undefined), + } as unknown as BrowserClient; + const def = { + name: "browser_probe", + label: "Probe", + description: "probe", + promptSnippet: "probe", + promptGuidelines: [], + parameters: Type.Object({}), + concurrency: "parallel", + ...(ensureAlive !== undefined ? { ensureAlive } : {}), + handler: async () => { + ran.push("handler"); + return ok({ text: "done" }); + }, + } as AnyBrowserToolDefinition; + + let registered: ToolDefinition<TSchema> | undefined; + const pi = { + registerTool: (td: ToolDefinition<TSchema>) => { + registered = td; + }, + } as unknown as ExtensionAPI; + registerBrowserTool(pi, client, def); + assert.ok(registered, "tool was not registered"); + const result = await registered.execute?.("id", {}, undefined, undefined, undefined as never); + const first = result?.content[0]; + return { ran, text: first !== undefined && first.type === "text" ? first.text : "" }; +}; + +describe("tool gate on the daemon socket", () => { + test("ensureAlive:false bypasses the gate entirely", async () => { + const probe = await runGate(false); + assert.deepEqual([...probe.ran], ["handler"]); + assert.equal(probe.text, "done"); + }); + + test("a gated tool agrees with the socket check about reaching the handler", async () => { + const reachable = await daemonSocketMayExist(); + const probe = await runGate(undefined); + if (reachable) assert.deepEqual([...probe.ran], ["handler"]); + else { + assert.deepEqual([...probe.ran], []); + assert.match(probe.text, /not initialized/); + } + }); +}); |
