diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4667994f10..c6cf69765f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -132,7 +132,7 @@ jobs: exit 0 - run: bun install --frozen-lockfile if: needs.changes.outputs.draft != 'true' - - run: bun test ./server/claude-store.test.ts ./server/process-start.test.ts ./server/grok.test.ts + - run: bun test ./server/claude-store.test.ts ./server/process-start.test.ts ./server/darwin-process.test.ts ./server/grok.test.ts if: needs.changes.outputs.draft != 'true' rust-pty-host: # The Rust PTY host (crates/pty-sidecar) is the default where it is installed or built, and its diff --git a/changelog.d/macos-no-sync-subprocess.md b/changelog.d/macos-no-sync-subprocess.md new file mode 100644 index 0000000000..c7463b1eb5 --- /dev/null +++ b/changelog.d/macos-no-sync-subprocess.md @@ -0,0 +1,5 @@ +### Fixed +- On macOS the server no longer pauses every connection while `ps` tells when an agent's process + started or which terminal it runs in: it reads both from the kernel instead. +- Opening a file an agent named without its folder no longer pauses the server while git lists a + large repository. diff --git a/docs/muse-chat.md b/docs/muse-chat.md index eb1222c741..e19740106e 100644 --- a/docs/muse-chat.md +++ b/docs/muse-chat.md @@ -36,8 +36,8 @@ pane, so a session Muse opens after a failed scan can take that long to show. On 4096 entries, is not read, and Chat uses Terminal. The data directory comes from the server's `XDG_DATA_HOME`; a Muse started with another one -uses Terminal. A process's start time comes from `ps` on macOS (whole seconds, so a three-second -margin) and `/proc` on Linux (to 10 ms, so a 20 ms margin). Windows uses Terminal. +uses Terminal. A process's start time comes from the kernel's process record on macOS (with a +three-second margin) and `/proc` on Linux (to 10 ms, so a 20 ms margin). Windows uses Terminal. ## What is read diff --git a/server/claude-store.test.ts b/server/claude-store.test.ts index bacdc91341..c30f8a5812 100644 --- a/server/claude-store.test.ts +++ b/server/claude-store.test.ts @@ -3,7 +3,8 @@ import { chmodSync, mkdirSync, mkdtempSync, readFileSync, renameSync, rmSync, sy import * as fsPromises from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, win32 } from "node:path"; -import { AmbiguousClaudeStore, claudeAttachArgs, claudeAttachedJob, claudeAttachSession, claudeForegroundSession, claudeJobSession, claudeProcessSession, claudeSessionTempDir, claudeStoreKey, claudeStoreOf, claudeProjectDir, claudeTranscriptFile, claudeRecordOnlyIn, configDirInPsLine, darwinProcess, forgetClaudeSessions, herdrLaunchSource, isClaudeProcess, paneClaudeSource, processClaudeConfigDir, processClaudeSource, procArgsEnvironment, procStartTime } from "./claude-store.ts"; +import { AmbiguousClaudeStore, claudeAttachArgs, claudeAttachedJob, claudeAttachSession, claudeForegroundSession, claudeJobSession, claudeProcessSession, claudeSessionTempDir, claudeStoreKey, claudeStoreOf, claudeProjectDir, claudeTranscriptFile, claudeRecordOnlyIn, configDirInPsLine, darwinProcess, forgetClaudeSessions, herdrLaunchSource, isClaudeProcess, paneClaudeSource, processClaudeConfigDir, processClaudeSource, procArgsEnvironment } from "./claude-store.ts"; +import { procStartTime } from "./darwin-process.ts"; import type { ProcessRow } from "./windows-processes.ts"; const NATIVE = process.platform === "linux" || process.platform === "darwin"; diff --git a/server/claude-store.ts b/server/claude-store.ts index 1d4d99de1e..23654e5183 100644 --- a/server/claude-store.ts +++ b/server/claude-store.ts @@ -13,13 +13,13 @@ * id is a UUID herdr reports, so at most one file answers to it. */ -import { dlopen, FFIType, ptr } from "bun:ffi"; import { createHash } from "node:crypto"; import { constants, type Dirent } from "node:fs"; import { open, readFile, readdir, readlink, stat } from "node:fs/promises"; import { tmpdir } from "node:os"; import nodePath, { basename, dirname, isAbsolute, join, type PlatformPath } from "node:path"; import { directoryKey, withoutVerbatimPrefix } from "./codex.ts"; +import { darwinStartTime, sysctlCall } from "./darwin-process.ts"; import { recentProcessTable } from "./gjc-runtime.ts"; import { processAlive, processStartUpperBoundAsync } from "./process-start.ts"; import type { ProcessRow } from "./windows-processes.ts"; @@ -194,20 +194,6 @@ export async function claudeRecordOnlyIn( return holders.length === 1 && holders[0] === wanted; } -/** sysctl(3) from libSystem, opened once, on macOS only. */ -let darwinSysctl: ((name: Int32Array, out: Uint8Array) => number | null) | undefined; - -function sysctlCall(): (name: Int32Array, out: Uint8Array) => number | null { - if (darwinSysctl) return darwinSysctl; - // a system library's C function: nothing native of node's is loaded into Bun - const lib = dlopen("/usr/lib/libSystem.B.dylib", { sysctl: { args: [FFIType.ptr, FFIType.u32, FFIType.ptr, FFIType.ptr, FFIType.ptr, FFIType.u64], returns: FFIType.i32 } }); - darwinSysctl = (name, out) => { - const size = new BigUint64Array([BigInt(out.byteLength)]); - return lib.symbols.sysctl(ptr(name), name.length, ptr(out), ptr(size), null, 0) === 0 ? Number(size[0]) : null; - }; - return darwinSysctl; -} - /** * The environment in a `KERN_PROCARGS2` answer, one entry each: an int argc, the executable's path, * its NUL padding, argc arguments, then the environment, each NUL-terminated, up to an empty one or @@ -249,23 +235,6 @@ function darwinEnvironment(pid: number): string | null { return entries === null ? null : entries.join("\0"); } -/** kern.proc.pid begins with extern_proc.p_starttime, a 64-bit Darwin timeval. */ -export function procStartTime(answer: Uint8Array): bigint | null { - if (answer.byteLength < 16) return null; - const view = new DataView(answer.buffer, answer.byteOffset, answer.byteLength); - const seconds = view.getBigInt64(0, true); - const micros = view.getInt32(8, true); - return seconds > 0n && micros >= 0 && micros < 1_000_000 ? seconds * 1_000_000n + BigInt(micros) : null; -} - -/** The kernel's process birth time, including microseconds that `ps -o lstart` drops. */ -function darwinStartTime(pid: number): bigint | null { - if (!Number.isSafeInteger(pid) || pid <= 0) return null; - const answer = new Uint8Array(4096); - const length = sysctlCall()(new Int32Array([1, 14, 1, pid]), answer); // CTL_KERN, KERN_PROC, KERN_PROC_PID - return length === null ? null : procStartTime(answer.subarray(0, length)); -} - export const darwinProcess = { environment: darwinEnvironment, startTime: darwinStartTime }; /** A named store: an absolute path to a directory, else null. */ @@ -921,7 +890,7 @@ async function findAttachedJob(configDir: string, { ref, byName }: ClaudeAttach, */ const attachedJobs = new Map(); const MAX_ATTACHED_JOBS = 256; -/** Two reads of one process's start differ by up to a second: macOS's ps counts whole seconds. */ +/** Two reads of one process's start: Linux's differ by a tick or two; macOS's are the kernel's own, kept to the margin they had when `ps` counted whole seconds. */ const SAME_START_MS = 2000; const ATTACH_MISS_MS = 10_000; diff --git a/server/conversation.test.ts b/server/conversation.test.ts index 7cc9a71f25..9e45d65fd8 100644 --- a/server/conversation.test.ts +++ b/server/conversation.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test"; +import * as darwinProcess from "./darwin-process.ts"; import { Database } from "bun:sqlite"; import { appendFileSync, copyFileSync, linkSync, mkdirSync, mkdtempSync, readFileSync, renameSync, rmSync, symlinkSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; @@ -2743,7 +2744,7 @@ describe("a Codex pane that reported no session yet", () => { }); // any client snapshot starts the Claude subagent refresh that reads this, also the one a pending message is checked against (#1545) -it("reads a Claude pane's process start through a ps child on macOS, never a synchronous ps", async () => { +it("reads a Claude pane's process start from the kernel on macOS, with no ps for it", async () => { const platform = Object.getOwnPropertyDescriptor(process, "platform")!; const home = mkdtempSync(join(tmpdir(), "claude-darwin-start-")); const previousHome = process.env["HOME"]; @@ -2755,23 +2756,22 @@ it("reads a Claude pane's process start through a ps child on macOS, never a syn ? { agent: { agent_session: { value: session } } } : { process_info: { foreground_processes: [{ pid: 4242, name: "claude", argv: ["claude"] }] } }) as unknown as typeof herdr.herdrRpc); const blocking = spyOn(Bun, "spawnSync"); - const probes: string[][] = []; - const awaited = spyOn(Bun, "spawn").mockImplementation(((argv: string[]) => { - probes.push(argv); - return { stdout: new Response("01:00\n").body, exited: Promise.resolve(0), signalCode: null, kill: () => {} }; - }) as unknown as typeof Bun.spawn); + // the kernel's process record answers the start; no ps asks it, awaited or not (#1404). Claude's own + // store may still ask an awaited ps for the process's environment on a real Mac. + const kernel = spyOn(darwinProcess, "darwinStartTime").mockReturnValue(1_000_000n); + const awaited = spyOn(Bun, "spawn"); process.env["HOME"] = home; Object.defineProperty(process, "platform", { value: "darwin" }); try { const found = await claudePaneSession({ pane_id: "claude-darwin", cwd } as HerdrPane); expect(found?.pid).toBe(4242); - expect(found?.startedAt).toBeNumber(); - expect(probes).toContainEqual(["/bin/ps", "-o", "etime=", "-p", "4242"]); + expect(found?.startedAt).toBe(1_000); + expect((awaited.mock.calls as unknown as [string[]][]).filter(([argv]) => argv.includes("etime="))).toEqual([]); expect(blocking).not.toHaveBeenCalled(); } finally { Object.defineProperty(process, "platform", platform); if (previousHome === undefined) delete process.env["HOME"]; else process.env["HOME"] = previousHome; - awaited.mockRestore(); blocking.mockRestore(); rpc.mockRestore(); + awaited.mockRestore(); blocking.mockRestore(); kernel.mockRestore(); rpc.mockRestore(); rmSync(home, { recursive: true, force: true }); } }); diff --git a/server/core/pending.ts b/server/core/pending.ts index 56cfc61aa8..38da661fbf 100644 --- a/server/core/pending.ts +++ b/server/core/pending.ts @@ -707,7 +707,7 @@ export function createPendingDelivery({ options, clients, attachments, sight, se && front !== undefined && launchedName !== undefined && front !== launchedName; if (identity.agent !== null && !shellBack) return false; return identity.processPid !== launched - // macOS reads a birth to the whole second (samePendingIdentity) + // macOS allows a second, as samePendingIdentity does || (item.identity.processBirth != null && identity.processBirth != null && Math.abs(identity.processBirth - item.identity.processBirth) > (process.platform === "darwin" ? 1000 : 0)); } diff --git a/server/darwin-process.test.ts b/server/darwin-process.test.ts new file mode 100644 index 0000000000..32d2ffc9b3 --- /dev/null +++ b/server/darwin-process.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from "bun:test"; +import { darwinStartTime, darwinTerminal, procTerminalDevice } from "./darwin-process.ts"; + +/** A 648-byte kinfo_proc that started at `seconds` (plus `micros`) on terminal device `device`. */ +function kinfo(seconds: number, micros: number, device: number): Uint8Array { + const answer = new Uint8Array(648); + const view = new DataView(answer.buffer); + view.setBigInt64(0, BigInt(seconds), true); + view.setInt32(8, micros, true); + view.setInt32(572, device, true); + return answer; +} + +describe("macOS kernel process record", () => { + it("reads the controlling terminal's device from kinfo_proc, and none for NODEV or a short answer", () => { + const answer = new Uint8Array(648).fill(0xff); + const view = new DataView(answer.buffer); + view.setInt32(572, 0x1000001, true); + expect(procTerminalDevice(answer)).toBe(0x1000001); + view.setInt32(572, -1, true); + expect(procTerminalDevice(answer)).toBeNull(); + expect(procTerminalDevice(answer.subarray(0, 575))).toBeNull(); + expect(procTerminalDevice(new Uint8Array(0))).toBeNull(); + }); + + it("takes a terminal and its start from one record, so a PID reused between reads cannot pair two processes", () => { + // the first read sees process A on ttys001; any later read of the PID sees process B on ttys002 + const records = [kinfo(1_793_000_000, 250_000, 1), kinfo(1_793_000_100, 0, 2)]; + let reads = 0; + const read = () => records[Math.min(reads++, 1)]!; + const name = (device: number) => `ttys00${device}`; + expect(darwinTerminal(42, read, name)).toEqual({ tty: "ttys001", startedAt: 1_793_000_000_250 }); + expect(reads).toBe(1); + // no terminal, no start or no process: nothing is paired + expect(darwinTerminal(42, () => kinfo(1_793_000_000, 0, -1), name)).toBeNull(); + expect(darwinTerminal(42, () => kinfo(0, 0, 1), name)).toBeNull(); + expect(darwinTerminal(42, () => new Uint8Array(0), name)).toBeNull(); + expect(darwinTerminal(42, () => null, name)).toBeNull(); + }); + + it.skipIf(process.platform !== "darwin")("tells what `ps` told, for a live process and none for a missing one", async () => { + const child = Bun.spawn([process.execPath, "-e", "await Bun.sleep(5000)"]); + try { + const ps = Bun.spawnSync(["/bin/ps", "-o", "tty=", "-o", "lstart=", "-p", String(child.pid)], { env: { ...process.env, LC_ALL: "C", TZ: "UTC" } }); + const [tty, ...lstart] = ps.stdout.toString().trim().split(/\s+/); + const started = darwinStartTime(child.pid); + expect(started).not.toBeNull(); + expect(Math.floor(Number(started! / 1000n) / 1000) * 1000).toBe(Date.parse(`${lstart.join(" ")} UTC`)); + const record = darwinTerminal(child.pid); + expect(record?.tty ?? null).toBe(tty === "??" ? null : tty!); + if (record) expect(record.startedAt).toBe(Number(started! / 1000n)); + } finally { child.kill(); } + expect(darwinStartTime(2147483647)).toBeNull(); + expect(darwinTerminal(2147483647)).toBeNull(); + }); +}); diff --git a/server/darwin-process.ts b/server/darwin-process.ts new file mode 100644 index 0000000000..fa09b801f6 --- /dev/null +++ b/server/darwin-process.ts @@ -0,0 +1,87 @@ +/** + * What macOS's kernel keeps about a process, read in the server's own thread through libSystem: + * `ps` asked the same kernel, but as a child the event loop had to wait for, and that wait stalled + * every connection (#1404). Each read takes microseconds and never starts a process. + */ +import { dlopen, FFIType, ptr } from "bun:ffi"; + +/** libSystem's sysctl(3) and devname(3), opened once, on macOS only. */ +let libSystem: ReturnType | undefined; + +function openLibSystem() { + // a system library's C functions: nothing native of node's is loaded into Bun + return dlopen("/usr/lib/libSystem.B.dylib", { + sysctl: { args: [FFIType.ptr, FFIType.u32, FFIType.ptr, FFIType.ptr, FFIType.ptr, FFIType.u64], returns: FFIType.i32 }, + devname: { args: [FFIType.i32, FFIType.u16], returns: FFIType.cstring }, + }).symbols; +} + +/** sysctl(3) for `name`, into `out`: the answer's length, or null when the call failed. */ +export function sysctlCall(): (name: Int32Array, out: Uint8Array) => number | null { + const symbols = libSystem ??= openLibSystem(); + return (name, out) => { + const size = new BigUint64Array([BigInt(out.byteLength)]); + return symbols.sysctl(ptr(name), name.length, ptr(out), ptr(size), null, 0) === 0 ? Number(size[0]) : null; + }; +} + +/** kern.proc.pid begins with extern_proc.p_starttime, a 64-bit Darwin timeval. */ +export function procStartTime(answer: Uint8Array): bigint | null { + if (answer.byteLength < 16) return null; + const view = new DataView(answer.buffer, answer.byteOffset, answer.byteLength); + const seconds = view.getBigInt64(0, true); + const micros = view.getInt32(8, true); + return seconds > 0n && micros >= 0 && micros < 1_000_000 ? seconds * 1_000_000n + BigInt(micros) : null; +} + +/** Where kinfo_proc keeps kp_eproc.e_tdev, the controlling terminal's device (648-byte kinfo_proc, arm64 and x86_64). */ +const E_TDEV_OFFSET = 572; +/** dev_t's NODEV: the process has no controlling terminal (`ps` prints `??`). */ +const NODEV = -1; + +/** The controlling terminal's device in a kern.proc.pid answer, or null when it has none. */ +export function procTerminalDevice(answer: Uint8Array): number | null { + if (answer.byteLength < E_TDEV_OFFSET + 4) return null; + const device = new DataView(answer.buffer, answer.byteOffset, answer.byteLength).getInt32(E_TDEV_OFFSET, true); + return device === NODEV ? null : device; +} + +/** CTL_KERN, KERN_PROC, KERN_PROC_PID: the process's kinfo_proc, empty when no such process runs. */ +function kinfoProc(pid: number): Uint8Array | null { + if (!Number.isSafeInteger(pid) || pid <= 0) return null; + const answer = new Uint8Array(4096); + const length = sysctlCall()(new Int32Array([1, 14, 1, pid]), answer); + return length === null ? null : answer.subarray(0, length); +} + +/** The kernel's process birth time in microseconds, including those `ps -o lstart` drops. */ +export function darwinStartTime(pid: number): bigint | null { + const answer = kinfoProc(pid); + return answer === null ? null : procStartTime(answer); +} + +/** A character device's name under /dev (`ttys003`), or null when it has none. */ +function deviceName(device: number): string | null { + const symbols = libSystem ??= openLibSystem(); + const name = symbols.devname(device, 0o020000); // S_IFCHR + return name === null ? null : String(name) || null; +} + +/** + * The process's controlling terminal under /dev (`ttys003`, as `ps -o tty=` prints it) and its start + * in ms, both from one kinfo_proc: two reads could describe two processes when the PID is reused + * between them. Null when no such process runs or it has no terminal. `read` and `name` stand in for + * the kernel in tests. + */ +export function darwinTerminal( + pid: number, + read: (pid: number) => Uint8Array | null = kinfoProc, + name: (device: number) => string | null = deviceName, +): { tty: string; startedAt: number } | null { + const answer = read(pid); + if (answer === null) return null; + const started = procStartTime(answer); + const device = procTerminalDevice(answer); + const tty = started === null || device === null ? null : name(device); + return tty === null ? null : { tty, startedAt: Number(started! / 1000n) }; +} diff --git a/server/file-view.test.ts b/server/file-view.test.ts index bc1d5d89b1..9083ff3587 100644 --- a/server/file-view.test.ts +++ b/server/file-view.test.ts @@ -1044,8 +1044,8 @@ describe("locateFile", () => { writeFileSync(path, "# nfd"); const distinguishes = !existsSync(join(root, "문서.md")); for (const found of [ - locateFile(join(root, "문서.md".normalize("NFC")), null, true), - locateFile("문서.md", root, true), + await locateFile(join(root, "문서.md".normalize("NFC")), null, true), + await locateFile("문서.md", root, true), await fileView.findFile("문서.md", root, async () => null), ]) { expect(found).not.toBeNull(); @@ -1054,7 +1054,7 @@ describe("locateFile", () => { expect(readFileSync(found.info.path, "utf8")).toBe("# nfd"); if (distinguishes) expect(found.info.path).toBe(path); } - if (distinguishes) expect(locateFile("문서.md", root)).toBeNull(); + if (distinguishes) expect(await locateFile("문서.md", root)).toBeNull(); }); it("(b) opens a path whose folder and filename use different forms", async () => { @@ -1065,7 +1065,7 @@ describe("locateFile", () => { writeFileSync(path, "mixed"); const requested = join(root, "회의자료", "메모.md"); for (const found of [ - locateFile(requested, null, true), + await locateFile(requested, null, true), await fileView.findFile(requested, null, async () => null), ]) { expect(found).not.toBeNull(); @@ -1114,32 +1114,32 @@ describe("locateFile", () => { rmSync(path); mkdirSync(path); expect(await fileView.findFile(path, null, async () => null)).toBeNull(); - expect(locateFile(path, null, true)).toBeNull(); + expect(await locateFile(path, null, true)).toBeNull(); }); - it("(f) resolves scratch names across forms without allowing symlink escapes", () => { + it("(f) resolves scratch names across forms without allowing symlink escapes", async () => { const root = temp(); const folder = join(root, "scratch"); mkdirSync(folder); const outside = join(root, "outside.txt"); writeFileSync(outside, "outside"); symlinkSync(outside, join(folder, "링크.txt".normalize("NFD"))); - expect(locateFileInFolder("링크.txt", folder, true)).toBeNull(); + expect(await locateFileInFolder("링크.txt", folder, true)).toBeNull(); const path = join(folder, "문서.md".normalize("NFD")); writeFileSync(path, "inside"); - const found = locateFileInFolder("문서.md", folder, true); + const found = await locateFileInFolder("문서.md", folder, true); expect(found).not.toBeNull(); if (found === null || !("info" in found)) throw new Error("expected one file"); expect(found.info.name.normalize("NFC")).toBe("문서.md"); expect(readFileSync(found.info.path, "utf8")).toBe("inside"); - if (!existsSync(join(folder, "문서.md"))) expect(locateFileInFolder("문서.md", folder)).toBeNull(); + if (!existsSync(join(folder, "문서.md"))) expect(await locateFileInFolder("문서.md", folder)).toBeNull(); }); it("(g) returns null for missing ASCII names even beneath a non-ASCII folder", async () => { const root = temp(); mkdirSync(join(root, "자료".normalize("NFD"))); for (const path of [join(root, "missing.txt"), join(root, "자료", "missing.txt")]) { - expect(locateFile(path, null, true)).toBeNull(); + expect(await locateFile(path, null, true)).toBeNull(); expect(await fileView.findFile(path, null, async () => null)).toBeNull(); } }); @@ -1167,11 +1167,11 @@ describe("locateFile", () => { const found = await fileView.findFile("메모.md", root, async () => null); if (distinguishes) { expect(found).toMatchObject({ info: { path: exact } }); - expect(filesNamed(root, "메모.md")).toEqual(["a/메모.md"]); + expect(await filesNamed(root, "메모.md")).toEqual(["a/메모.md"]); } else { expect(found).not.toBeNull(); } - const matches = filesNamed(root, "메모.md", 10, true).sort(); + const matches = (await filesNamed(root, "메모.md", 10, true)).sort(); if (distinguishes) expect(matches).toEqual(["a/메모.md", `b/${variant}`].sort()); else expect(matches.map((path) => path.normalize("NFC"))).toEqual(["a/메모.md", "b/메모.md"]); }); @@ -1202,31 +1202,31 @@ describe("locateFile", () => { expect(await fileView.findFile("메모.md", cwd, async () => null)).toEqual(found); }); - it("refuses parent traversal outside the Claude scratch fallback folder", () => { + it("refuses parent traversal outside the Claude scratch fallback folder", async () => { const root = mkdtempSync(join(tmpdir(), "herdr-locate-")); roots.push(root); const folder = join(root, "session"); mkdirSync(folder); writeFileSync(join(root, "outside.txt"), "outside"); - expect(locateFileInFolder("../outside.txt", folder)).toBeNull(); + expect(await locateFileInFolder("../outside.txt", folder)).toBeNull(); }); - it("resolves an in-folder Claude scratch fallback link", () => { + it("resolves an in-folder Claude scratch fallback link", async () => { const folder = mkdtempSync(join(tmpdir(), "herdr-locate-")); roots.push(folder); mkdirSync(join(folder, "scratchpad")); const path = join(folder, "scratchpad", "notes.txt"); writeFileSync(path, "notes"); - expect(locateFileInFolder("scratchpad/notes.txt", folder)).toMatchObject({ info: { path } }); + expect(await locateFileInFolder("scratchpad/notes.txt", folder)).toMatchObject({ info: { path } }); }); - it("refuses absolute paths and symlink escapes from the scratch fallback", () => { + it("refuses absolute paths and symlink escapes from the scratch fallback", async () => { const root = mkdtempSync(join(tmpdir(), "herdr-locate-")); roots.push(root); const folder = join(root, "session"); mkdirSync(folder); const outside = join(root, "outside.txt"); writeFileSync(outside, "outside"); symlinkSync(outside, join(folder, "linked.txt")); - expect(locateFileInFolder(outside, folder)).toBeNull(); - expect(locateFileInFolder("linked.txt", folder)).toBeNull(); + expect(await locateFileInFolder(outside, folder)).toBeNull(); + expect(await locateFileInFolder("linked.txt", folder)).toBeNull(); }); it("finds a bare name deeper in the pane's folder, and lists several of that name", async () => { @@ -1241,12 +1241,12 @@ describe("locateFile", () => { writeFileSync(join(root, "a/notes.md"), "a"); writeFileSync(join(root, "b/notes.md"), "b"); clearFileCache(); - expect(locateFile("demo.mp4", root)).toMatchObject({ info: { path: join(root, "docs/screenshots/demo.mp4") } }); - expect(locateFile("screenshots/demo.mp4", root)).toMatchObject({ info: { name: "demo.mp4" } }); - expect(locateFile("notes.md", root)).toEqual({ candidates: [join(root, "a/notes.md"), join(root, "b/notes.md")] }); - expect(locateFile("missing.txt", root)).toBeNull(); + expect(await locateFile("demo.mp4", root)).toMatchObject({ info: { path: join(root, "docs/screenshots/demo.mp4") } }); + expect(await locateFile("screenshots/demo.mp4", root)).toMatchObject({ info: { name: "demo.mp4" } }); + expect(await locateFile("notes.md", root)).toEqual({ candidates: [join(root, "a/notes.md"), join(root, "b/notes.md")] }); + expect(await locateFile("missing.txt", root)).toBeNull(); // an absolute or ~ path is taken as written, never searched for - expect(locateFile("/nowhere/demo.mp4", root)).toBeNull(); + expect(await locateFile("/nowhere/demo.mp4", root)).toBeNull(); }); // the walk goes files first, then folders in name order, and stops at 5,000 entries @@ -1255,7 +1255,7 @@ describe("locateFile", () => { for (let index = 0; index < 5_100; index += 1) writeFileSync(join(root, "a-data", `page-${index}.txt`), ""); }; - it("finds a tracked file past an ignored folder too large to walk", () => { + it("finds a tracked file past an ignored folder too large to walk", async () => { const root = temp(); bigFolder(root); mkdirSync(join(root, "docs/design"), { recursive: true }); @@ -1263,10 +1263,10 @@ describe("locateFile", () => { writeFileSync(join(root, ".gitignore"), "a-data/\n"); expect(Bun.spawnSync(["git", "init", "-q", root]).exitCode).toBe(0); clearFileCache(); - expect(locateFile("11_two-version-design.md", root)).toMatchObject({ info: { path: join(root, "docs/design/11_two-version-design.md") } }); + expect(await locateFile("11_two-version-design.md", root)).toMatchObject({ info: { path: join(root, "docs/design/11_two-version-design.md") } }); }); - it("finds an ignored file past an ignored folder too large to walk", () => { + it("finds an ignored file past an ignored folder too large to walk", async () => { const root = temp(); bigFolder(root); mkdirSync(join(root, "recordings")); @@ -1274,10 +1274,10 @@ describe("locateFile", () => { writeFileSync(join(root, ".gitignore"), "a-data/\nrecordings/\n"); expect(Bun.spawnSync(["git", "init", "-q", root]).exitCode).toBe(0); clearFileCache(); - expect(locateFile("demo.mp4", root)).toMatchObject({ info: { path: join(root, "recordings/demo.mp4") } }); + expect(await locateFile("demo.mp4", root)).toMatchObject({ info: { path: join(root, "recordings/demo.mp4") } }); }); - it("opens an explicitly named ignored paste", () => { + it("opens an explicitly named ignored paste", async () => { const root = temp(); expect(Bun.spawnSync(["git", "init", "-q", root]).exitCode).toBe(0); mkdirSync(join(root, ".ovis")); @@ -1288,25 +1288,25 @@ describe("locateFile", () => { // ignored keeps it out of Git status, not out of reach: the absolute path the upload answered // and the project-relative one both open it expect(Bun.spawnSync(["git", "-C", root, "check-ignore", "-q", path]).exitCode).toBe(0); - expect(locateFile(path, root)).toMatchObject({ info: { path } }); - expect(locateFile(".ovis/notes-20261021-120000-0123abcd.json", root)).toMatchObject({ info: { path } }); + expect(await locateFile(path, root)).toMatchObject({ info: { path } }); + expect(await locateFile(".ovis/notes-20261021-120000-0123abcd.json", root)).toMatchObject({ info: { path } }); }); - it("walks the subfolders of a folder holding thousands of files outside a repository", () => { + it("walks the subfolders of a folder holding thousands of files outside a repository", async () => { const root = temp(); for (let index = 0; index < 1_500; index += 1) writeFileSync(join(root, `page-${index}.txt`), ""); mkdirSync(join(root, "docs")); writeFileSync(join(root, "docs/notes.md"), "notes"); clearFileCache(); - expect(locateFile("notes.md", root)).toMatchObject({ info: { path: join(root, "docs/notes.md") } }); + expect(await locateFile("notes.md", root)).toMatchObject({ info: { path: join(root, "docs/notes.md") } }); }); - it("finds a file past a folder too large to walk outside a repository", () => { + it("finds a file past a folder too large to walk outside a repository", async () => { const root = temp(); bigFolder(root); mkdirSync(join(root, "docs")); writeFileSync(join(root, "docs/notes.md"), "notes"); clearFileCache(); - expect(locateFile("notes.md", root)).toMatchObject({ info: { path: join(root, "docs/notes.md") } }); + expect(await locateFile("notes.md", root)).toMatchObject({ info: { path: join(root, "docs/notes.md") } }); }); }); diff --git a/server/file-view.ts b/server/file-view.ts index 34b98de2cf..1b1b966304 100644 --- a/server/file-view.ts +++ b/server/file-view.ts @@ -90,7 +90,7 @@ function pathsByName(path: string): string[] { * file under the pane's folder whose path ends in it (agents often name `demo.mp4` without * `docs/screenshots/`). Several such files come back as candidates to choose from. */ -export function locateFile(input: string, cwd: string | null, anyForm = false): { info: FileInfo } | { candidates: string[] } | null { +export async function locateFile(input: string, cwd: string | null, anyForm = false): Promise<{ info: FileInfo } | { candidates: string[] } | null> { const path = resolveFilePath(input, cwd); const info = anyForm || path === null ? null : fileInfo(path); if (info !== null) return { info }; @@ -102,7 +102,7 @@ export function locateFile(input: string, cwd: string | null, anyForm = false): const relative = input.trim(); if (cwd === null || relative === "" || isAbsolute(relative) || relative.startsWith("~") || relative.startsWith("../")) return null; if (anyForm && !/[^\x00-\x7f]/.test(relative)) return null; - const found = filesNamed(cwd, relative, 10, anyForm).map((match) => resolve(cwd, match)); + const found = (await filesNamed(cwd, relative, 10, anyForm)).map((match) => resolve(cwd, match)); if (found.length === 1) { const only = fileInfo(found[0]!); return only === null ? null : { info: only }; @@ -111,10 +111,10 @@ export function locateFile(input: string, cwd: string | null, anyForm = false): } /** A relative fallback stays in its own folder, including after following symlinks. */ -export function locateFileInFolder(input: string, folder: string, anyForm = false): ReturnType { +export async function locateFileInFolder(input: string, folder: string, anyForm = false): ReturnType { const relative = input.trim(); if (isAbsolute(relative) || relative.startsWith("~") || !resolve(folder, relative).startsWith(resolve(folder) + sep)) return null; - const found = locateFile(relative, folder, anyForm); + const found = await locateFile(relative, folder, anyForm); if (found === null) return null; try { const root = realpathSync(folder) + sep; @@ -124,18 +124,18 @@ export function locateFileInFolder(input: string, folder: string, anyForm = fals } /** Try every exact place before opening a name stored in another form. */ -export async function findFile(input: string, cwd: string | null, scratchFolder: () => Promise): Promise> { - const exact = locateFile(input, cwd); +export async function findFile(input: string, cwd: string | null, scratchFolder: () => Promise): ReturnType { + const exact = await locateFile(input, cwd); if (exact !== null) return exact; let folder: string | null = null; if (cwd !== null && !isAbsolute(input.trim()) && !input.trim().startsWith("~")) { folder = await scratchFolder(); if (folder !== null) { - const scratch = locateFileInFolder(input, folder); + const scratch = await locateFileInFolder(input, folder); if (scratch !== null) return scratch; } } - const found = locateFile(input, cwd, true); + const found = await locateFile(input, cwd, true); if (found !== null) return found; return folder === null ? null : locateFileInFolder(input, folder, true); } diff --git a/server/files.test.ts b/server/files.test.ts index e86f5e7a81..f850d0af74 100644 --- a/server/files.test.ts +++ b/server/files.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it, spyOn } from "bun:test"; import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { delimiter, join } from "node:path"; @@ -218,7 +219,7 @@ describe("git inventories", () => { // a server started with these set (from a hook, say) inherits them; Bun.spawn ignores later edits to process.env const script = `import { filesNamed, paneFiles } from ${JSON.stringify(join(import.meta.dir, "files.ts"))}; const root = process.env.FILES_ROOT; - console.log(JSON.stringify({ listed: await paneFiles(root, "", 100), foreign: filesNamed(root, "foreign-only.txt"), api: filesNamed(root, "api.ts") }));`; + console.log(JSON.stringify({ listed: await paneFiles(root, "", 100), foreign: await filesNamed(root, "foreign-only.txt"), api: await filesNamed(root, "api.ts") }));`; const child = Bun.spawn([process.execPath, "-e", script], { stdin: "ignore", stdout: "pipe", @@ -256,10 +257,10 @@ describe("git inventories", () => { expect(listed).toContain("src/api.ts"); expect(listed).toContain(buried); expect(listed).not.toContain("README.md"); - expect(filesNamed(root, "buried.txt")).toEqual([buried]); + expect(await filesNamed(root, "buried.txt")).toEqual([buried]); }); - it("hands both subprocess APIs the inert argv, environment, cap and deadline", async () => { + it("hands both inventories the inert argv and environment, never through a synchronous spawn", async () => { const root = fixture(); gitRepository(root); process.env["GIT_DIR"] = join(root, "elsewhere"); @@ -267,16 +268,18 @@ describe("git inventories", () => { const spawnSync = spyOn(Bun, "spawnSync"); try { await paneFiles(root, "", 100); - filesNamed(root, "api.ts"); + await filesNamed(root, "api.ts"); const argv = ["git", ...INERT_ARGV, "-C", root, "ls-files", "-z", "-co", "--exclude-standard"]; - const calls = [spawn.mock.calls[0], spawnSync.mock.calls[0]] as unknown as [string[], { stdin?: string; env?: Record }][]; + const calls = spawn.mock.calls as unknown as [string[], { stdin?: string; env?: Record }][]; + expect(calls).toHaveLength(2); for (const [command, options] of calls) { expect(command).toEqual(argv); expect(options.stdin).toBe("ignore"); expect(options.env?.["GIT_DIR"]).toBeUndefined(); expect(options.env).toMatchObject({ GIT_TERMINAL_PROMPT: "0", GIT_OPTIONAL_LOCKS: "0", LC_ALL: "C" }); } - expect(spawnSync.mock.calls[0]![1]).toMatchObject({ timeout: GIT_LIST_DEADLINE_MS, maxBuffer: GIT_LIST_MAX_BYTES }); + // a synchronous listing holds the whole server's event loop for up to its deadline (#1404) + expect(spawnSync).not.toHaveBeenCalled(); expect(GIT_LIST_MAX_BYTES).toBe(16 * 1024 * 1024); expect(GIT_LIST_DEADLINE_MS).toBe(3_000); } finally { @@ -302,7 +305,7 @@ describe("git inventories", () => { const files = await paneFiles(root, "", 100); expect(files).toContain("src/components/PromptCard.tsx"); expect(files).not.toContain("README.md"); - expect(filesNamed(root, "PromptCard.tsx")).toEqual(["src/components/PromptCard.tsx"]); + expect(await filesNamed(root, "PromptCard.tsx")).toEqual(["src/components/PromptCard.tsx"]); expect(existsSync(join(hook.folder, "ran"))).toBe(false); expect(existsSync(join(onPath, "ran"))).toBe(false); expect(readFileSync(join(root, ".git", "config"), "utf8")).toBe(config); @@ -322,14 +325,14 @@ describe("git inventories", () => { expect(readFileSync(listing).byteLength).toBe(bytes); clearFileCache(); const listed = await paneFiles(root, "", 100); - const named = filesNamed(root, second); + const named = await filesNamed(root, second); if (bytes <= cap) { expect(listed).toEqual([second, "ghost-first"].sort()); expect(named).toEqual([second]); } else { expect(listed).toEqual(["README.md", "src/api.ts", "src/components/PromptCard.tsx"]); expect(named).toEqual([]); - expect(filesNamed(root, "ghost-first")).toEqual([]); + expect(await filesNamed(root, "ghost-first")).toEqual([]); } } }); @@ -345,9 +348,9 @@ describe("git inventories", () => { writeFileSync(listing, output); clearFileCache(); expect(await paneFiles(root, "", 100)).toEqual(["README.md", "src/api.ts", "src/components/PromptCard.tsx"]); - expect(filesNamed(root, "ghost-partial")).toEqual([]); - expect(filesNamed(root, "ghost-whole")).toEqual([]); - expect(filesNamed(root, "api.ts")).toEqual(["src/api.ts"]); + expect(await filesNamed(root, "ghost-partial")).toEqual([]); + expect(await filesNamed(root, "ghost-whole")).toEqual([]); + expect(await filesNamed(root, "api.ts")).toEqual(["src/api.ts"]); } }); @@ -379,18 +382,61 @@ describe("git inventories", () => { await asyncHolder.end(); } - const syncHolder = heldGit(); - setGitListOptions({ git: syncHolder.git, deadlineMs }); + const namedHolder = heldGit(); + setGitListOptions({ git: namedHolder.git, deadlineMs }); clearFileCache(); try { const started = performance.now(); - expect(filesNamed(root, "ghost-held")).toEqual([]); + expect(await within(filesNamed(root, "ghost-held"), deadlineMs + 1_000, "the name lookup")).toEqual([]); expect(performance.now() - started).toBeLessThan(deadlineMs + 1_000); // the holder reported in while the lookup ran, and still holds stdout after it answered - await within(syncHolder.ready, 1_000, "the holder's readiness"); - expect(syncHolder.holding()).toBe(true); + await within(namedHolder.ready, 1_000, "the holder's readiness"); + expect(namedHolder.holding()).toBe(true); } finally { - await syncHolder.end(); + await namedHolder.end(); } }); + + it("runs other work while a name lookup waits for a slow git", async () => { + const root = fixture(); + // a git that answers only once the test lets it go, through a FIFO no one else writes + const { git, folder } = fakeGit((dir) => `read _ < "${dir}/go"\nprintf 'slow-name\\0'`); + expect(Bun.spawnSync(["mkfifo", join(folder, "go")]).exitCode).toBe(0); + setGitListOptions({ git }); + const spawn = spyOn(Bun, "spawn"); + try { + let answered = false; + const named = filesNamed(root, "slow-name").finally(() => { answered = true; }); + // a second lookup of the folder meanwhile waits for the same listing, not a git of its own + const again = filesNamed(root, "api.ts"); + // a timer comes due while git still waits: the lookup holds no one else's turn (#1404) + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(answered).toBe(false); + await within(writeFile(join(folder, "go"), "\n"), 5_000, "git's release"); + expect(await within(named, 5_000, "the name lookup")).toEqual(["slow-name"]); + expect(await within(again, 5_000, "the second lookup")).toEqual(["src/api.ts"]); + expect(spawn).toHaveBeenCalledTimes(1); + } finally { + spawn.mockRestore(); + } + }); + + it("keeps a listing begun before clearFileCache out of the cache it cleared", async () => { + const root = fixture(); + // the old git answers only once released; the new one answers at once + const slow = fakeGit((dir) => `read _ < "${dir}/go"\nprintf 'old-name\\0'`); + expect(Bun.spawnSync(["mkfifo", join(slow.folder, "go")]).exitCode).toBe(0); + const fast = fakeGit(() => `printf 'new-name\\0'`); + setGitListOptions({ git: slow.git }); + // filesNamed starts its git before its first await, so the old listing is under way here + const old = filesNamed(root, "old-name"); + clearFileCache(); + setGitListOptions({ git: fast.git }); + expect(await within(filesNamed(root, "new-name"), 5_000, "the new lookup")).toEqual(["new-name"]); + await within(writeFile(join(slow.folder, "go"), "\n"), 5_000, "the old git's release"); + // the old lookup still answers its caller, but leaves the newer listing in the cache + expect(await within(old, 5_000, "the old lookup")).toEqual(["old-name"]); + expect(await filesNamed(root, "old-name")).toEqual([]); + expect(await filesNamed(root, "new-name")).toEqual(["new-name"]); + }); }); diff --git a/server/files.ts b/server/files.ts index fcde657b35..c68e356087 100644 --- a/server/files.ts +++ b/server/files.ts @@ -53,7 +53,8 @@ function listedNames(output: Uint8Array): string[] | null { return Buffer.from(output.buffer, output.byteOffset, output.byteLength).toString("utf8").split("\0").filter(Boolean); } -async function gitFiles(cwd: string): Promise { +/** The listing, at most `limit` names of it; null when git failed, overran its cap or its deadline. */ +async function gitFiles(cwd: string, limit = MAX_GIT_ENTRIES): Promise { const { deadlineMs, maxBytes } = gitList; let proc: Bun.Subprocess<"ignore", "pipe", "ignore">; try { @@ -86,7 +87,7 @@ async function gitFiles(cwd: string): Promise { const exitCode = await proc.exited; if (stopped || exitCode !== 0) return null; const names = listedNames(Buffer.concat(chunks)); - return names && [...new Set(names)].slice(0, MAX_GIT_ENTRIES); + return names && [...new Set(names)].slice(0, limit); } finally { clearTimeout(timer); } @@ -172,44 +173,41 @@ export async function paneFiles(cwd: string, query = "", limit = 20): Promise(); - -/** `gitFiles` for a lookup that answers at once, not cut to MAX_GIT_ENTRIES: a name is matched against every path. */ -function gitFilesNow(cwd: string): string[] | null { - const { deadlineMs, maxBytes } = gitList; - let proc; - try { - proc = Bun.spawnSync(gitListArgv(cwd), { - stdin: "ignore", - stdout: "pipe", - stderr: "ignore", - env: gitEnvironment(), - timeout: deadlineMs, - maxBuffer: maxBytes, - killSignal: "SIGKILL", - }); - } catch { - return null; - } - // a listing past its cap or deadline may end in half a name, even when git exited 0 - if (proc.exitCode !== 0 || proc.exitedDueToTimeout || proc.exitedDueToMaxBuffer || proc.stdout.byteLength > maxBytes) return null; - return listedNames(proc.stdout); -} +/** One listing per folder at a time: lookups that arrive while it runs share it rather than start their own git. */ +const listing = new Map>(); +/** Raised by clearFileCache: a listing begun before it neither fills the cache nor removes a later listing. */ +let generation = 0; /** * The files under `cwd` whose path ends in `name` (a bare `demo.mp4`, or `screenshots/demo.mp4`), * for a path an agent wrote without its folders. The git inventory comes first, whole, so a * folder of thousands of files cannot hide the one named; the bounded walk follows, since that - * inventory leaves out ignored files (build output, recordings). + * inventory leaves out ignored files (build output, recordings). The listing is awaited, not + * waited for: a slow git in a large repository must not hold every other request (#1404). */ -export function filesNamed(cwd: string, name: string, limit = 10, anyForm = false): string[] { +export async function filesNamed(cwd: string, name: string, limit = 10, anyForm = false): Promise { const suffix = anyForm ? name.replace(/^\.\//, "").normalize("NFC") : name.replace(/^\.\//, ""); - let hit = walked.get(cwd); - if (!hit || hit.expires <= Date.now()) { - hit = { expires: Date.now() + CACHE_MS, files: [...new Set([...gitFilesNow(cwd) ?? [], ...walkFiles(cwd)])] }; - walked.set(cwd, hit); - if (walked.size > 16) walked.delete(walked.keys().next().value!); + const hit = walked.get(cwd); + let files = hit && hit.expires > Date.now() ? hit.files : undefined; + if (files === undefined) { + let shared = listing.get(cwd); + if (shared === undefined) { + const begun = generation; + // not cut to MAX_GIT_ENTRIES: a name is matched against every path + const pending: Promise = gitFiles(cwd, Infinity).then((listed) => { + const found = [...new Set([...listed ?? [], ...walkFiles(cwd)])]; + if (begun === generation) { + walked.set(cwd, { expires: Date.now() + CACHE_MS, files: found }); + if (walked.size > 16) walked.delete(walked.keys().next().value!); + } + return found; + }).finally(() => { if (listing.get(cwd) === pending) listing.delete(cwd); }); + shared = pending; + listing.set(cwd, pending); + } + files = await shared; } - return hit.files.filter((path) => { + return files.filter((path) => { const compared = anyForm ? path.normalize("NFC") : path; return compared === suffix || compared.endsWith(`/${suffix}`); }).sort().slice(0, limit); @@ -218,4 +216,6 @@ export function filesNamed(cwd: string, name: string, limit = 10, anyForm = fals export function clearFileCache(): void { cache.clear(); walked.clear(); + listing.clear(); + generation += 1; } diff --git a/server/gjc-runtime.test.ts b/server/gjc-runtime.test.ts index 7a7298d6d5..cc5d44556b 100644 --- a/server/gjc-runtime.test.ts +++ b/server/gjc-runtime.test.ts @@ -1,10 +1,11 @@ import { expect, it, spyOn } from "bun:test"; +import * as darwinProcess from "./darwin-process.ts"; import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, win32 } from "node:path"; import { GJC_NATIVE_DIRECTORY, type GjcNativeBinding } from "../shared/gjc-native.ts"; -import { forgetGjcPane, forgetGjcState, gjcPidUnderShell, gjcSessionFile, gjcTranscriptForPane, isGjcProcess, parseGjcPs, recentProcessTable, storeRelative, terminalBreadcrumb } from "./gjc-runtime.ts"; +import { forgetGjcPane, forgetGjcState, gjcPidUnderShell, gjcSessionFile, gjcTranscriptForPane, isGjcProcess, recentProcessTable, storeRelative, terminalBreadcrumb } from "./gjc-runtime.ts"; const tempDir = (prefix: string) => realpathSync(mkdtempSync(join(tmpdir(), prefix))); @@ -19,12 +20,6 @@ it("recognizes native and interpreter-launched gjc, not look-alikes", () => { expect(isGjcProcess([])).toBe(false); }); -it("reads macOS terminal/process identity without /proc", () => { - expect(parseGjcPs("ttys003 Mon Sep 28 10:00:00 2026\n")?.id).toBe("ttys003"); - expect(parseGjcPs("?? Mon Sep 28 10:00:00 2026")).toBeNull(); - expect(parseGjcPs("ttys003 invalid")).toBeNull(); -}); - it("validates breadcrumbs against process age, canonical cwd and the native session store", () => { const home = tempDir("gjc-breadcrumb-"); try { @@ -396,33 +391,32 @@ it("cancels a running identity read when its pane ends", async () => { }); /** macOS, where a start is read with `ps`: answered by an awaited child, and any synchronous spawn recorded */ -async function onDarwinPs(elapsed: string, run: (probes: string[][], blocking: { mock: { calls: unknown[] } }) => Promise): Promise { +async function onDarwinKernel(startedMs: number, run: (spawned: { mock: { calls: unknown[] } }, blocking: { mock: { calls: unknown[] } }) => Promise): Promise { const platform = Object.getOwnPropertyDescriptor(process, "platform")!; Object.defineProperty(process, "platform", { value: "darwin" }); + // the kernel's process record answers; no ps runs, awaited or not (#1404) + const kernel = spyOn(darwinProcess, "darwinStartTime").mockReturnValue(BigInt(startedMs) * 1000n); const blocking = spyOn(Bun, "spawnSync"); - const probes: string[][] = []; - const awaited = spyOn(Bun, "spawn").mockImplementation(((argv: string[]) => { - probes.push(argv); - return { stdout: new Response(`${elapsed}\n`).body, exited: Promise.resolve(0), signalCode: null, kill: () => {} }; - }) as unknown as typeof Bun.spawn); - try { return await run(probes, blocking); } finally { - awaited.mockRestore(); + const spawned = spyOn(Bun, "spawn"); + try { return await run(spawned, blocking); } finally { + spawned.mockRestore(); blocking.mockRestore(); + kernel.mockRestore(); Object.defineProperty(process, "platform", platform); } } // GJC discovery runs this lookup for every snapshot a pending message is checked against (#1545) -it("reads the GJC process's birth through an awaited ps on macOS, never a synchronous one", async () => { +it("reads the GJC process's birth from the kernel on macOS, with no ps child", async () => { const f = nativeFixture(); const clock = spyOn(Date, "now").mockReturnValue(61_000); try { const path = f.sessionFile("session-darwin"); f.publish(f.record(200, "session-darwin", path)); const { processBirth: _sync, ...options } = f.options; - await onDarwinPs("01:00", async (probes, blocking) => { + await onDarwinKernel(1_000, async (spawned, blocking) => { expect(await gjcTranscriptForPane("pane", f.cwd, f.home, { ...options, platform: "darwin" })).toBe(realpathSync(path)); - expect(probes).toContainEqual(["/bin/ps", "-o", "etime=", "-p", "200"]); + expect(spawned.mock.calls).toHaveLength(0); expect(blocking.mock.calls).toHaveLength(0); }); } finally { clock.mockRestore(); f.done(); } diff --git a/server/gjc-runtime.ts b/server/gjc-runtime.ts index 70705c0ba5..5f35a457e4 100644 --- a/server/gjc-runtime.ts +++ b/server/gjc-runtime.ts @@ -1,10 +1,10 @@ /** GJC transcript identity is published by its lifecycle extension; shared terminal/store helpers are also used by omp. */ -import { execFileSync } from "node:child_process"; import { closeSync, openSync, readdirSync, readFileSync, readlinkSync, readSync, realpathSync, statSync } from "node:fs"; import nodePath, { join, type PlatformPath } from "node:path"; import { readGjcNativeBinding, validateGjcNativeTranscript } from "./gjc-native.ts"; +import { darwinTerminal } from "./darwin-process.ts"; import { herdrRpc } from "./herdr/client.ts"; import { processStartedAt, processStartedAtAsync } from "./process-start.ts"; import { descendantArgv, windowsProcessTable, type ProcessRow } from "./windows-processes.ts"; @@ -143,25 +143,15 @@ export function gjcTerminal(pid: number, environ?: readonly string[] | null): Gj return { id: tty.slice(5).replaceAll("/", "-"), startedAt }; } if (process.platform === "darwin") { - // lstart is local time without a zone: print it in the zone Date.parse reads it in, - // which is not always $TZ (bun test runs in UTC without setting it) - const zone = Intl.DateTimeFormat().resolvedOptions().timeZone; - const output = execFileSync("ps", ["-p", String(pid), "-o", "tty=", "-o", "lstart="], { - encoding: "utf8", timeout: 1500, maxBuffer: 4096, env: { ...process.env, LC_ALL: "C", TZ: zone }, stdio: ["ignore", "pipe", "ignore"], - }); - return parseGjcPs(output); + // the terminal and the start from one kernel record, as `ps -o tty= -o lstart=` read them, without a + // child process that holds the event loop + const record = darwinTerminal(pid); + return record !== null && /^ttys\d+$/.test(record.tty) ? { id: record.tty, startedAt: record.startedAt } : null; } } catch { /* process exited or its terminal metadata is unavailable */ } return null; } -export function parseGjcPs(output: string): GjcTerminal | null { - const match = output.trim().match(/^(?:\/dev\/)?(ttys\d+)\s+(.+)$/); - if (!match) return null; - const startedAt = Date.parse(match[2]!); - return Number.isFinite(startedAt) ? { id: match[1]!, startedAt } : null; -} - /** * The cwd a gjc or omp transcript names in its header (`{"type":"session",...}`), or null when * the file is not one. omp writes a title record before the header. Read bounded: only the diff --git a/server/gjc-status.test.ts b/server/gjc-status.test.ts index 007edf6ab0..b97297c129 100644 --- a/server/gjc-status.test.ts +++ b/server/gjc-status.test.ts @@ -1,4 +1,5 @@ import { afterAll, describe, expect, it, spyOn } from "bun:test"; +import * as darwinProcess from "./darwin-process.ts"; import { appendFileSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -732,36 +733,35 @@ describe("the gjc process a pane's transcript and start are read from", () => { }); /** macOS, where a start is read with `ps`: answered by an awaited child, and any synchronous spawn recorded */ -async function onDarwinPs(elapsed: string, run: (probes: string[][], blocking: { mock: { calls: unknown[] } }) => Promise): Promise { +async function onDarwinKernel(startedMs: number, run: (spawned: { mock: { calls: unknown[] } }, blocking: { mock: { calls: unknown[] } }) => Promise): Promise { const platform = Object.getOwnPropertyDescriptor(process, "platform")!; Object.defineProperty(process, "platform", { value: "darwin" }); + // the kernel's process record answers; no ps runs, awaited or not (#1404) + const kernel = spyOn(darwinProcess, "darwinStartTime").mockReturnValue(BigInt(startedMs) * 1000n); const blocking = spyOn(Bun, "spawnSync"); - const probes: string[][] = []; - const awaited = spyOn(Bun, "spawn").mockImplementation(((argv: string[]) => { - probes.push(argv); - return { stdout: new Response(`${elapsed}\n`).body, exited: Promise.resolve(0), signalCode: null, kill: () => {} }; - }) as unknown as typeof Bun.spawn); - try { return await run(probes, blocking); } finally { - awaited.mockRestore(); + const spawned = spyOn(Bun, "spawn"); + try { return await run(spawned, blocking); } finally { + spawned.mockRestore(); blocking.mockRestore(); + kernel.mockRestore(); Object.defineProperty(process, "platform", platform); } } // every snapshot a pending message is checked against runs GJC discovery (#1545) -it("reads a GJC pane's process start through an awaited ps on macOS, never a synchronous one", async () => { +it("reads a GJC pane's process start from the kernel on macOS, with no ps child", async () => { const clock = spyOn(Date, "now").mockReturnValue(61_000); const pane = { pane_id: "gjc-darwin", workspace_id: "w", tab_id: "t", terminal_id: "term", focused: false, revision: 0, agent: "gjc", agent_status: "idle", cwd: root } as HerdrPane; try { - await onDarwinPs("01:00", async (probes, blocking) => { + await onDarwinKernel(1_000, async (spawned, blocking) => { const found = await gjcPanes([pane], { platform: "darwin", processInfo: async () => ({ process_info: { pane_id: pane.pane_id, foreground_processes: [{ pid: 300, name: "gjc", argv: ["/usr/bin/gjc"] }] } }), transcript: async () => null, }); expect(found.get(pane.pane_id)).toEqual({ path: null, startedAt: 1_000 }); - expect(probes).toEqual([["/bin/ps", "-o", "etime=", "-p", "300"]]); + expect(spawned.mock.calls).toHaveLength(0); expect(blocking.mock.calls).toHaveLength(0); }); } finally { clock.mockRestore(); } diff --git a/server/hermes.ts b/server/hermes.ts index 7262636c3d..bd8553b4ae 100644 --- a/server/hermes.ts +++ b/server/hermes.ts @@ -1,5 +1,4 @@ import { Database } from "bun:sqlite"; -import { execFileSync } from "node:child_process"; import { createHash } from "node:crypto"; import { existsSync, readFileSync, readlinkSync, realpathSync, statSync } from "node:fs"; import { isAbsolute, join } from "node:path"; @@ -10,6 +9,7 @@ import { herdrRpc } from "./herdr/client.ts"; import { answerVersion, ConversationNotStarted, ConversationUnavailable, type ConversationPage, HistoryChanged, type RecognizedConversation, TRANSCRIPT_WINDOW_BYTES } from "./conversation-shared.ts"; import { toolSummary } from "./transcript-records.ts"; import { TOOL_OUTPUT_CHARS, trimOutput } from "./tool-output.ts"; +import { darwinTerminal } from "./darwin-process.ts"; import { processStartedAt } from "./process-start.ts"; import { hermesSkillPrompt } from "./skill-activity.ts"; @@ -135,20 +135,24 @@ export function isHermesProcess(entry: { name?: string; argv0?: string; argv?: r /** Resolves process terminal identity to Hermes breadcrumb filename convention. */ export function hermesTerminalId(pid: number): string | null { + return hermesTerminal(pid)?.id ?? null; +} + +/** hermesTerminalId with the start of the process it names; on macOS both come from one kernel record. */ +function hermesTerminal(pid: number): { id: string; startedAt: number | null } | null { if (!Number.isSafeInteger(pid) || pid <= 0) return null; try { if (process.platform === "linux") { const tty = readlinkSync(`/proc/${pid}/fd/0`); if (!/^\/dev\/(?:pts\/\d+|tty[\w-]+)$/.test(tty)) return null; - return `tty-${tty.slice(1).replaceAll("/", "-")}`; + return { id: `tty-${tty.slice(1).replaceAll("/", "-")}`, startedAt: processStartedAt(pid) }; } if (process.platform === "darwin") { - const output = execFileSync("ps", ["-p", String(pid), "-o", "tty="], { - encoding: "utf8", timeout: 1500, maxBuffer: 4096, stdio: ["ignore", "pipe", "ignore"], - }).trim(); - if (!output || output === "??") return null; - const device = output.startsWith("/dev/") ? output.slice(5) : output.startsWith("ttys") ? `dev-${output}` : output; - return `tty-${device.replaceAll("/", "-")}`; + // the kernel's record, as `ps -o tty=` read it, without a child process that holds the event loop + const record = darwinTerminal(pid); + if (!record) return null; + const device = record.tty.startsWith("ttys") ? `dev-${record.tty}` : record.tty; + return { id: `tty-${device.replaceAll("/", "-")}`, startedAt: record.startedAt }; } } catch { /* process exited or terminal unavailable */ } return null; @@ -209,10 +213,9 @@ export async function hermesTranscriptForPane( const hermesHome = configuredHome ?? processHome ?? defaultHermesHome(); if (sessionId === null && hermesProcess !== undefined) { - const termId = hermesTerminalId(hermesProcess.pid); - const startedAt = processStartedAt(hermesProcess.pid); - if (termId !== null && startedAt !== null) { - sessionId = hermesBreadcrumbSession(hermesHome, termId, cwd, startedAt); + const terminal = hermesTerminal(hermesProcess.pid); + if (terminal !== null && terminal.startedAt !== null) { + sessionId = hermesBreadcrumbSession(hermesHome, terminal.id, cwd, terminal.startedAt); } } diff --git a/server/muse-store.ts b/server/muse-store.ts index 802d6a35f2..3e476a5ffe 100644 --- a/server/muse-store.ts +++ b/server/muse-store.ts @@ -13,8 +13,9 @@ import { processStartUpperBoundAsync } from "./process-start.ts"; const SESSION = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; /** - * A process's start is an upper bound: up to two seconds late on macOS (whole seconds of `ps` - * elapsed time, plus one), and up to two 10 ms ticks late on Linux. + * A process's start is an upper bound: up to a second late on macOS (the kernel's start, plus one), + * and up to two 10 ms ticks late on Linux. macOS keeps the three seconds it had when `ps` told the + * start in whole seconds. */ const START_SLACK_MS = process.platform === "darwin" ? 3000 : 20; const TAIL_BYTES = 64 * 1024; diff --git a/server/omo.test.ts b/server/omo.test.ts index a11f099580..f98701a073 100644 --- a/server/omo.test.ts +++ b/server/omo.test.ts @@ -1,4 +1,5 @@ import { expect, it, spyOn } from "bun:test"; +import * as darwinProcess from "./darwin-process.ts"; import { mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -504,7 +505,7 @@ it("names the OmO process holding a session, not the bundled SDK child beside it }); // every OmO status refresh reads these, so every snapshot a pending message is checked against does (#1545) -it("reads OmO panes' process starts through ps children on macOS, never a synchronous ps", async () => { +it("reads OmO panes' process starts from the kernel on macOS, with no ps child", async () => { const platform = Object.getOwnPropertyDescriptor(process, "platform")!; Object.defineProperty(process, "platform", { value: "darwin" }); const home = mkdtempSync(join(tmpdir(), "omo-darwin-starts-")); @@ -513,18 +514,19 @@ it("reads OmO panes' process starts through ps children on macOS, never a synchr spyOn(client, "herdrRpc").mockImplementation((async () => ({ process_info: { pane_id: pane.pane_id, foreground_processes: [{ pid: 4242, name: "node", argv: ["node", `${OMO_AI}/bin/omo`], cwd: home }] } })) as unknown as typeof client.herdrRpc); const blocking = spyOn(Bun, "spawnSync"); - const probes: string[][] = []; - const awaited = spyOn(Bun, "spawn").mockImplementation(((argv: string[]) => { - probes.push(argv); - return { stdout: new Response("01:00\n").body, exited: Promise.resolve(0), signalCode: null, kill: () => {} }; - }) as unknown as typeof Bun.spawn); + // the kernel's process record answers; no ps runs, awaited or not (#1404) + const kernel = spyOn(darwinProcess, "darwinStartTime").mockReturnValue(1_000_000n); + const awaited = spyOn(Bun, "spawn"); try { - expect((await omoPanes([pane], home)).has(pane.pane_id)).toBe(true); - expect(probes).toContainEqual(["/bin/ps", "-o", "etime=", "-p", "4242"]); + // the kernel's start reaches the pane's OmO result: a read that skipped the kernel would leave it unknown + expect((await omoPanes([pane], home)).get(pane.pane_id)?.startedAt).toBe(1_000); + expect(kernel).toHaveBeenCalledWith(4242); + expect(awaited).not.toHaveBeenCalled(); expect(blocking).not.toHaveBeenCalled(); } finally { awaited.mockRestore(); blocking.mockRestore(); + kernel.mockRestore(); (client.herdrRpc as unknown as { mockRestore(): void }).mockRestore(); Object.defineProperty(process, "platform", platform); rmSync(home, { recursive: true, force: true }); diff --git a/server/omo.ts b/server/omo.ts index 0b337dd42e..fae2972696 100644 --- a/server/omo.ts +++ b/server/omo.ts @@ -347,7 +347,7 @@ const processInfo = async (paneId: string): Promise => { /** * When each foreground process of `infos` started, read before the checks that compare it, without holding the - * event loop: on macOS each read runs `ps` (processStartedAtAsync), a few at a time. Every OmO status refresh, and + * event loop: on macOS each read is the kernel's process record (processStartedAtAsync), a few at a time. Every OmO status refresh, and * so every snapshot a pending message is checked against, reads these (#1545). */ async function processStarts(infos: Iterable): Promise<(pid: number) => number | null> { diff --git a/server/pending-input.ts b/server/pending-input.ts index d0d143fb74..7e503e879c 100644 --- a/server/pending-input.ts +++ b/server/pending-input.ts @@ -18,7 +18,7 @@ export function samePendingIdentity(before: PendingIdentity, after: PendingIdent && !changed(before.session, after.session) && !changed(before.omoSession, after.omoSession) && !changed(before.claudeSession, after.claudeSession) && !changed(before.processPid, after.processPid) - // macOS derives birth from whole elapsed seconds; reads of one PID drift by up to a second. + // macOS allows a second: its birth was once read from `ps`'s whole elapsed seconds, now from the kernel exactly. && (before.processBirth == null || after.processBirth == null || (platform === "darwin" ? Math.abs(before.processBirth - after.processBirth) <= 1000 : before.processBirth === after.processBirth)); } diff --git a/server/process-start.test.ts b/server/process-start.test.ts index 1bebc2c73e..3d8282722b 100644 --- a/server/process-start.test.ts +++ b/server/process-start.test.ts @@ -1,120 +1,26 @@ import { describe, expect, it, spyOn } from "bun:test"; -import { createDarwinStartReader, parseElapsed, processStartedAt, processStartedAtAsync, processStartLowerBound, processStartUpperBound, processStartUpperBoundAsync } from "./process-start.ts"; - -describe("bounded macOS process start reads", () => { - it("returns unknown when the bounded spawn times out even with partial output", () => { - // Given a probe that reached its timeout after writing valid elapsed time. - const probes: { argv: string[]; timeout: number; killSignal: string }[] = []; - const read = createDarwinStartReader((argv, options) => { - probes.push({ argv, timeout: options.timeout, killSignal: options.killSignal }); - return { exitCode: 0, stdout: "00:05", exitedDueToTimeout: true }; - }); - // When the process start is requested, then timeout output grants no identity. - expect(read(42)).toBeNull(); - expect(probes).toEqual([{ argv: ["/bin/ps", "-o", "etime=", "-p", "42"], timeout: 1000, killSignal: "SIGKILL" }]); - }); - - it.each([1, 0])("returns unknown for failed or unreadable output with exit code %i", (exitCode) => { - // Given a failed exit with valid output, or a successful exit with unreadable output. - const read = createDarwinStartReader(() => ({ exitCode, stdout: exitCode === 0 ? "" : "00:05" })); - // When read, then neither case grants a process identity. - expect(read(42)).toBeNull(); - }); - - it("returns unknown when spawning throws", () => { - // Given an unavailable subprocess. - const read = createDarwinStartReader(() => { throw new TypeError("spawn unavailable"); }); - // When read, then the existing failed-read result is preserved. - expect(read(42)).toBeNull(); - }); - - it("preserves the before and after clocks for the lower bound and start estimate", () => { - // Given a probe that takes 20 ms. - let now = 10_000; - const read = createDarwinStartReader(() => { - now += 20; - return { exitCode: 0, stdout: "00:05" }; - }, () => now); - // When read, then its lower bound uses the earlier clock, not the later estimate. - expect(read(42)).toEqual({ started: 5020, lower: 4000 }); - }); -}); +import { processStartedAt, processStartedAtAsync, processStartLowerBound, processStartUpperBound, processStartUpperBoundAsync } from "./process-start.ts"; describe("process start read without holding the event loop", () => { - it("reads macOS's start from ps run as an awaited child, never a synchronous one", async () => { - const platform = Object.getOwnPropertyDescriptor(process, "platform")!; - Object.defineProperty(process, "platform", { value: "darwin" }); + it.skipIf(process.platform !== "linux" && process.platform !== "darwin")("reads the same identity the synchronous read gives, with no child process", async () => { + const child = Bun.spawn([process.execPath, "-e", "await Bun.sleep(5000)"]); + // a ps here, even an awaited one, is what #1404 removed: the kernel answers on both platforms + const spawn = spyOn(Bun, "spawn"); const blocking = spyOn(Bun, "spawnSync"); - const probes: { argv: string[]; timeout?: number; killSignal?: string }[] = []; - const awaited = spyOn(Bun, "spawn").mockImplementation(((argv: string[], options?: { timeout?: number; killSignal?: string }) => { - probes.push({ argv, timeout: options?.timeout, killSignal: options?.killSignal }); - return { stdout: new Response(" 00:05\n").body, exited: Promise.resolve(0), signalCode: null, kill: () => {} }; - }) as unknown as typeof Bun.spawn); try { - const before = Date.now(); - const started = await processStartedAtAsync(42); - expect(started).toBeGreaterThanOrEqual(before - 5000); - expect(started).toBeLessThanOrEqual(Date.now() - 5000); - expect(probes).toEqual([{ argv: ["/bin/ps", "-o", "etime=", "-p", "42"], timeout: 1000, killSignal: "SIGKILL" }]); + expect(await processStartedAtAsync(child.pid)).toBe(processStartedAt(child.pid)); + expect(await processStartedAtAsync(2147483647)).toBeNull(); + expect(spawn).not.toHaveBeenCalled(); expect(blocking).not.toHaveBeenCalled(); } finally { - awaited.mockRestore(); + spawn.mockRestore(); blocking.mockRestore(); - Object.defineProperty(process, "platform", platform); + child.kill(); } }); - - it("counts a macOS birth from when ps started, so a late answer does not move it", async () => { - const platform = Object.getOwnPropertyDescriptor(process, "platform")!; - Object.defineProperty(process, "platform", { value: "darwin" }); - // the clock moves 900 ms while ps runs and the event loop is busy before its answer is taken - let now = 100_000; - const clock = spyOn(Date, "now").mockImplementation(() => now); - const awaited = spyOn(Bun, "spawn").mockImplementation((() => { - now += 900; - return { stdout: new Response("00:10\n").body, exited: Promise.resolve(0), signalCode: null, kill: () => {} }; - }) as unknown as typeof Bun.spawn); - try { - expect(await processStartedAtAsync(42)).toBe(100_000 - 10_000); - // the bound from above stays one: counted from after the answer - expect(await processStartUpperBoundAsync(42)).toBe(now - 10_000 + 1000); - } finally { - awaited.mockRestore(); - clock.mockRestore(); - Object.defineProperty(process, "platform", platform); - } - }); - - it("says nothing for a macOS ps that was killed at its deadline", async () => { - const platform = Object.getOwnPropertyDescriptor(process, "platform")!; - Object.defineProperty(process, "platform", { value: "darwin" }); - const awaited = spyOn(Bun, "spawn").mockImplementation((() => ({ stdout: new Response("00:05\n").body, exited: Promise.resolve(0), signalCode: "SIGKILL", kill: () => {} })) as unknown as typeof Bun.spawn); - try { - expect(await processStartedAtAsync(42)).toBeNull(); - } finally { - awaited.mockRestore(); - Object.defineProperty(process, "platform", platform); - } - }); - - it.skipIf(process.platform !== "linux")("reads Linux's start as the same identity the synchronous read gives", async () => { - const child = Bun.spawn([process.execPath, "-e", "await Bun.sleep(5000)"]); - try { - expect(await processStartedAtAsync(child.pid)).toBe(processStartedAt(child.pid)); - expect(await processStartedAtAsync(2147483647)).toBeNull(); - } finally { child.kill(); } - }); }); describe("process start time", () => { - it("reads every form of ps's elapsed time", () => { - expect(parseElapsed(" 00:07\n")).toBe(7); - expect(parseElapsed("12:34")).toBe(12 * 60 + 34); - expect(parseElapsed("01:02:03")).toBe(3723); - expect(parseElapsed("2-01:02:03")).toBe(2 * 86400 + 3723); - expect(parseElapsed("")).toBeNull(); - }); - it("tells when a running process started, to the second", async () => { const child = Bun.spawn([process.execPath, "-e", "await Bun.sleep(5000)"]); try { @@ -150,9 +56,8 @@ describe("process start time", () => { expect(lower).not.toBeNull(); // never after the process started (it started by the time spawn returned) ... expect(lower!).toBeLessThanOrEqual(spawned); - // ... and close: on Linux within btime's whole second and a tick, on macOS the second `ps` counts - // and the one it may round away - expect(lower!).toBeGreaterThanOrEqual(before - (process.platform === "linux" ? 1030 : 2100)); + // ... and close: on Linux within btime's whole second and a tick, on macOS the kernel's own start + expect(lower!).toBeGreaterThanOrEqual(before - (process.platform === "linux" ? 1030 : 5)); expect(processStartLowerBound(2147483647)).toBeNull(); } finally { child.kill(); } }); diff --git a/server/process-start.ts b/server/process-start.ts index 3e22ce5f03..5618c1e6f7 100644 --- a/server/process-start.ts +++ b/server/process-start.ts @@ -1,49 +1,26 @@ import { readFileSync } from "node:fs"; +import { darwinStartTime } from "./darwin-process.ts"; -/** `ps -o etime=`: `[[dd-]hh:]mm:ss` since the process started. */ -export function parseElapsed(text: string): number | null { - const match = /^\s*(?:(?:(\d+)-)?(\d+):)?(\d+):(\d+)\s*$/.exec(text); - if (!match) return null; - const [, days = "0", hours = "0", minutes, seconds] = match; - return ((Number(days) * 24 + Number(hours)) * 60 + Number(minutes!)) * 60 + Number(seconds!); -} - -type StartReading = { readonly started: number; readonly lower: number }; -type StartSpawn = (argv: string[], options: { - readonly stdout: "pipe"; readonly stderr: "ignore"; readonly timeout: number; readonly killSignal: "SIGKILL"; -}) => { readonly exitCode: number; readonly stdout: { toString(): string }; readonly exitedDueToTimeout?: boolean }; - -/** Read a fresh macOS start on every call: a live PID may belong to a replacement process. */ -export function createDarwinStartReader( - spawn: StartSpawn = (argv, options) => Bun.spawnSync(argv, options), - now: () => number = Date.now, -): (pid: number) => StartReading | null { - return (pid) => { - const before = now(); - try { - const ps = spawn(["/bin/ps", "-o", "etime=", "-p", String(pid)], { - stdout: "pipe", stderr: "ignore", timeout: 1000, killSignal: "SIGKILL", - }); - const elapsed = ps.exitCode === 0 && !ps.exitedDueToTimeout ? parseElapsed(ps.stdout.toString()) : null; - if (elapsed === null) return null; - return { started: now() - elapsed * 1000, lower: before - (elapsed + 1) * 1000 }; - } catch { - return null; - } - }; +/** + * The kernel's own start on macOS, to the millisecond, read fresh on every call (a live PID may belong + * to a replacement process) and without a child process: a `ps` here held the event loop (#1404). + */ +function darwinStart(pid: number): number | null { + try { + const micros = darwinStartTime(pid); + return micros === null ? null : Number(micros / 1000n); + } catch { + return null; + } } -const darwinStart = createDarwinStartReader(); - /** * When a process started, in ms since the epoch: Linux counts it in /proc (USER_HZ - * ticks after boot); macOS has no /proc, so there `ps` tells how long it has run (to the - * second, which is all the callers compare). null where neither answers. + * ticks after boot); macOS has no /proc, so there the kernel's process record tells it + * (sysctl). null where neither answers. */ export function processStartedAt(pid: number): number | null { - if (process.platform === "darwin") { - return darwinStart(pid)?.started ?? null; - } + if (process.platform === "darwin") return darwinStart(pid); return linuxBirth(pid); } @@ -73,44 +50,20 @@ export function processAlive(pid: number): boolean { } /** - * macOS: how long `pid` has run, in whole seconds, from `ps` run as a child that is awaited, never on the event - * loop; null when it cannot say. Bounded as the synchronous reader is: a `ps` past a second is killed and says nothing. - * `spawned`: the clock as `ps` was started, which it read the elapsed time after by no more than its own start-up; - * the clock once the answer is awaited can be later by however long the event loop was busy. - */ -async function darwinElapsedAsync(pid: number): Promise<{ elapsed: number; spawned: number } | null> { - try { - const spawned = Date.now(); - const ps = Bun.spawn(["/bin/ps", "-o", "etime=", "-p", String(pid)], { stdout: "pipe", stderr: "ignore", timeout: 1000, killSignal: "SIGKILL" }); - const [text, code] = await Promise.all([new Response(ps.stdout).text(), ps.exited]); - const elapsed = code === 0 && ps.signalCode === null ? parseElapsed(text) : null; - return elapsed === null ? null : { elapsed, spawned }; - } catch { - return null; - } -} - -/** - * processStartedAt without holding the event loop: the same reading (Linux's /proc birth; macOS's start to the - * second, from `ps` run as a child that is awaited). null where neither answers. + * processStartedAt for a caller that awaits it: the same reading (Linux's /proc birth; macOS's kernel record), + * which no platform needs a child process for any more (#1404). null where neither answers. */ export async function processStartedAtAsync(pid: number): Promise { - if (process.platform !== "darwin") return linuxBirth(pid); - // counted from when `ps` was started: a late answer must not move the birth (samePendingIdentity allows a second) - const read = await darwinElapsedAsync(pid); - return read === null ? null : read.spawned - read.elapsed * 1000; + return processStartedAt(pid); } /** - * processStartUpperBound without holding the event loop: on macOS `ps` runs as a child that is awaited. + * processStartUpperBound for a caller that awaits it; every platform's read is synchronous now. * Where no start time is known (Windows), null. */ export async function processStartUpperBoundAsync(pid: number): Promise { - if (process.platform === "linux") return processStartUpperBound(pid); - if (process.platform !== "darwin") return null; - const read = await darwinElapsedAsync(pid); - // from above, so counted from the clock after the answer: `ps` read the elapsed time no later than that - return read === null ? null : Date.now() - read.elapsed * 1000 + 1000; + if (process.platform !== "linux" && process.platform !== "darwin") return null; + return processStartUpperBound(pid); } /** An upper bound for comparing a new thread's timestamp, without btime's whole-second loss. */ @@ -147,15 +100,13 @@ function linuxReadStart(pid: number): number | null { /** * A time no later than the process started, in ms since the epoch: what a thread begun since * must be no older than. Codex 0.163 creates its thread as its process starts, often in the same - * millisecond tick on Linux and always in the same second as macOS's `ps` reports; the bound from + * millisecond tick on Linux and always in the same second on macOS; the bound from * above (processStartUpperBound, still what a send's process check compares) left such a thread * out, and two Codex panes of one directory with short answers both said they could not tell. An older thread let in with it makes two begun * since, and two tell nothing. */ export function processStartLowerBound(pid: number): number | null { - if (process.platform === "darwin") { - return darwinStart(pid)?.lower ?? null; - } + if (process.platform === "darwin") return darwinStart(pid); if (process.platform !== "linux") return null; // the earlier of both readings, less one tick: no later than the real start (the uptime's), nor // than the start processStartedAt tells (btime's), so a thread made as the process starts is in diff --git a/server/usage.ts b/server/usage.ts index 793bbbcbff..7d5254d93e 100644 --- a/server/usage.ts +++ b/server/usage.ts @@ -854,7 +854,9 @@ async function readKeychainInLoginSession(service: string, account?: string): Pr const plist = join(dir, "job.plist"); let pipe: FileHandle | undefined; try { - if (Bun.spawnSync(["mkfifo", "-m", "600", fifo]).exitCode !== 0) return null; + // awaited, not waited for: a stalled mkfifo must not hold every other request (#1404) + const mkfifo = Bun.spawn(["mkfifo", "-m", "600", fifo], { stdin: "ignore", stdout: "ignore", stderr: "ignore", timeout: COMMAND_TIMEOUT_MS, killSignal: "SIGKILL" }); + if (await mkfifo.exited !== 0) return null; writeFileSync(plist, ` Label${xml(label)}ProgramArguments${args.map((a) => `${xml(a)}`).join("")}StandardOutPath${xml(fifo)}StandardErrorPath/dev/nullRunAtLoad