From 7e47d439f81a74cbfe8ebda1112dfd826bd3dba0 Mon Sep 17 00:00:00 2001 From: mp Date: Mon, 26 Oct 2026 17:53:26 +0900 Subject: [PATCH 1/6] fix(server): read macOS process starts and terminals from the kernel, and await name lookups' git answered in under a millisecond. #1748 bounded those waits at 1 s; they still held every connection while they ran. - process-start.ts, gjc-runtime.ts and hermes.ts no longer run ps on macOS: the process's start and controlling terminal come from the kernel's kinfo_proc (sysctl KERN_PROC_PID and devname through libSystem), the read claude-store.ts already made for Claude. It stays synchronous, so no caller and no unknown-identity rule changes, and the start is exact rather than ps's whole seconds. The sysctl helper moves to darwin-process.ts. - files.ts: filesNamed awaits the streaming, capped and deadlined git listing paneFiles already used, instead of Bun.spawnSync for up to 3 s; locateFile follows. - usage.ts: the keychain FIFO's mkfifo is awaited and killed after 5 s. Startup-only probes stay synchronous: process-owner.ts (owner guard, persisted lstart birth), managed.ts, remote-entry.ts, pty/host.ts and pty/sidecar.ts. Refs #1404, #1545 --- changelog.d/macos-no-sync-subprocess.md | 5 + docs/muse-chat.md | 4 +- server/claude-store.test.ts | 3 +- server/claude-store.ts | 33 +------ server/darwin-process.test.ts | 29 ++++++ server/darwin-process.ts | 71 ++++++++++++++ server/file-view.test.ts | 70 +++++++------- server/file-view.ts | 16 ++-- server/files.test.ts | 54 +++++++---- server/files.ts | 35 ++----- server/gjc-runtime.test.ts | 8 +- server/gjc-runtime.ts | 20 +--- server/hermes.ts | 9 +- server/muse-store.ts | 5 +- server/process-start.test.ts | 119 +++--------------------- server/process-start.ts | 95 +++++-------------- server/usage.ts | 4 +- 17 files changed, 248 insertions(+), 332 deletions(-) create mode 100644 changelog.d/macos-no-sync-subprocess.md create mode 100644 server/darwin-process.test.ts create mode 100644 server/darwin-process.ts 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..13c3da1e08 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. */ diff --git a/server/darwin-process.test.ts b/server/darwin-process.test.ts new file mode 100644 index 0000000000..2be18f08ab --- /dev/null +++ b/server/darwin-process.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "bun:test"; +import { darwinStartTime, darwinTerminal, procTerminalDevice } from "./darwin-process.ts"; + +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.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`)); + expect(darwinTerminal(child.pid)).toBe(tty === "??" ? null : tty!); + } 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..f2da1c18fd --- /dev/null +++ b/server/darwin-process.ts @@ -0,0 +1,71 @@ +/** + * 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); +} + +/** The name of the process's controlling terminal under /dev (`ttys003`), as `ps -o tty=` prints it; null without one. */ +export function darwinTerminal(pid: number): string | null { + const answer = kinfoProc(pid); + const device = answer === null ? null : procTerminalDevice(answer); + if (device === null) return null; + const symbols = libSystem ??= openLibSystem(); + const name = symbols.devname(device, 0o020000); // S_IFCHR + return name === null ? null : String(name) || null; +} 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..642a6d3af5 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,33 @@ 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 }); + let answered = false; + const named = filesNamed(root, "slow-name").finally(() => { answered = true; }); + // 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"]); + }); }); diff --git a/server/files.ts b/server/files.ts index fcde657b35..f5cdf955b8 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); } @@ -173,39 +174,19 @@ 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); -} - /** * 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)])] }; + // not cut to MAX_GIT_ENTRIES: a name is matched against every path + hit = { expires: Date.now() + CACHE_MS, files: [...new Set([...await gitFiles(cwd, Infinity) ?? [], ...walkFiles(cwd)])] }; walked.set(cwd, hit); if (walked.size > 16) walked.delete(walked.keys().next().value!); } diff --git a/server/gjc-runtime.test.ts b/server/gjc-runtime.test.ts index 7a7298d6d5..823327ddeb 100644 --- a/server/gjc-runtime.test.ts +++ b/server/gjc-runtime.test.ts @@ -4,7 +4,7 @@ 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 +19,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 { diff --git a/server/gjc-runtime.ts b/server/gjc-runtime.ts index 70705c0ba5..420afa635f 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 kernel's record, as `ps -o tty=` read it, without a child process that holds the event loop + const tty = darwinTerminal(pid); + const startedAt = processStartedAt(pid); + return tty !== null && /^ttys\d+$/.test(tty) && startedAt !== null ? { id: tty, 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/hermes.ts b/server/hermes.ts index 7262636c3d..c909e12229 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"; @@ -143,10 +143,9 @@ export function hermesTerminalId(pid: number): string | null { return `tty-${tty.slice(1).replaceAll("/", "-")}`; } 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; + // the kernel's record, as `ps -o tty=` read it, without a child process that holds the event loop + const output = darwinTerminal(pid); + if (!output) return null; const device = output.startsWith("/dev/") ? output.slice(5) : output.startsWith("ttys") ? `dev-${output}` : output; return `tty-${device.replaceAll("/", "-")}`; } 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/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 From caa768d84dc09ccf8d1e977bbddec9aea930f341 Mon Sep 17 00:00:00 2001 From: mp Date: Mon, 26 Oct 2026 18:03:03 +0900 Subject: [PATCH 2/6] fix(server): share one name-lookup listing per folder, run the kernel record test on macOS CI Review of the previous commit (omo-native-code-reviewer): - files.ts: lookups of one folder that arrive while its git listing runs wait for that listing instead of each starting a git of its own (the old synchronous lookup ran them one after another). - ci.yml: the macOS job runs darwin-process.test.ts, the only check that kinfo_proc's e_tdev offset and devname match `ps`. - Comments that still gave ps's whole seconds as the reason for macOS start tolerances. Refs #1404 --- .github/workflows/ci.yml | 2 +- server/claude-store.ts | 2 +- server/core/pending.ts | 2 +- server/files.test.ts | 23 ++++++++++++++++------- server/files.ts | 26 +++++++++++++++++++------- server/pending-input.ts | 2 +- 6 files changed, 39 insertions(+), 18 deletions(-) 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/server/claude-store.ts b/server/claude-store.ts index 13c3da1e08..23654e5183 100644 --- a/server/claude-store.ts +++ b/server/claude-store.ts @@ -890,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/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/files.test.ts b/server/files.test.ts index 642a6d3af5..c9fb781678 100644 --- a/server/files.test.ts +++ b/server/files.test.ts @@ -403,12 +403,21 @@ describe("git inventories", () => { const { git, folder } = fakeGit((dir) => `read _ < "${dir}/go"\nprintf 'slow-name\\0'`); expect(Bun.spawnSync(["mkfifo", join(folder, "go")]).exitCode).toBe(0); setGitListOptions({ git }); - let answered = false; - const named = filesNamed(root, "slow-name").finally(() => { answered = true; }); - // 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"]); + 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(); + } }); }); diff --git a/server/files.ts b/server/files.ts index f5cdf955b8..fd72c128d1 100644 --- a/server/files.ts +++ b/server/files.ts @@ -173,6 +173,8 @@ export async function paneFiles(cwd: string, query = "", limit = 20): Promise(); +/** 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>(); /** * The files under `cwd` whose path ends in `name` (a bare `demo.mp4`, or `screenshots/demo.mp4`), @@ -183,14 +185,23 @@ const walked = new Map(); */ 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()) { - // not cut to MAX_GIT_ENTRIES: a name is matched against every path - hit = { expires: Date.now() + CACHE_MS, files: [...new Set([...await gitFiles(cwd, Infinity) ?? [], ...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) { + // not cut to MAX_GIT_ENTRIES: a name is matched against every path + shared = gitFiles(cwd, Infinity).then((listed) => { + const found = [...new Set([...listed ?? [], ...walkFiles(cwd)])]; + walked.set(cwd, { expires: Date.now() + CACHE_MS, files: found }); + if (walked.size > 16) walked.delete(walked.keys().next().value!); + return found; + }).finally(() => listing.delete(cwd)); + listing.set(cwd, shared); + } + 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); @@ -199,4 +210,5 @@ export async function filesNamed(cwd: string, name: string, limit = 10, anyForm export function clearFileCache(): void { cache.clear(); walked.clear(); + listing.clear(); } 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)); } From 0af983705f1793224a9d167a12f256c86e24740a Mon Sep 17 00:00:00 2001 From: mp Date: Mon, 26 Oct 2026 19:04:08 +0900 Subject: [PATCH 3/6] fix(server): take a macOS terminal and its start from one kernel record Astra gate B1 on #1980: gjcTerminal (and Hermes's breadcrumb lookup) read the controlling terminal and the process start in two kernel queries, so a PID reused between them could pair one process's terminal with another's start. darwinTerminal now returns the tty and the start from a single kinfo_proc, and both callers take both fields from it. The new test feeds two different records to one lookup and checks that only the first is read; reading them twice fails it. Refs #1404 --- server/darwin-process.test.ts | 29 ++++++++++++++++++++++++++++- server/darwin-process.ts | 26 +++++++++++++++++++++----- server/gjc-runtime.ts | 8 ++++---- server/hermes.ts | 22 +++++++++++++--------- 4 files changed, 66 insertions(+), 19 deletions(-) diff --git a/server/darwin-process.test.ts b/server/darwin-process.test.ts index 2be18f08ab..32d2ffc9b3 100644 --- a/server/darwin-process.test.ts +++ b/server/darwin-process.test.ts @@ -1,6 +1,16 @@ 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); @@ -13,6 +23,21 @@ describe("macOS kernel process record", () => { 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 { @@ -21,7 +46,9 @@ describe("macOS kernel process record", () => { const started = darwinStartTime(child.pid); expect(started).not.toBeNull(); expect(Math.floor(Number(started! / 1000n) / 1000) * 1000).toBe(Date.parse(`${lstart.join(" ")} UTC`)); - expect(darwinTerminal(child.pid)).toBe(tty === "??" ? null : tty!); + 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 index f2da1c18fd..fa09b801f6 100644 --- a/server/darwin-process.ts +++ b/server/darwin-process.ts @@ -60,12 +60,28 @@ export function darwinStartTime(pid: number): bigint | null { return answer === null ? null : procStartTime(answer); } -/** The name of the process's controlling terminal under /dev (`ttys003`), as `ps -o tty=` prints it; null without one. */ -export function darwinTerminal(pid: number): string | null { - const answer = kinfoProc(pid); - const device = answer === null ? null : procTerminalDevice(answer); - if (device === null) return null; +/** 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/gjc-runtime.ts b/server/gjc-runtime.ts index 420afa635f..5f35a457e4 100644 --- a/server/gjc-runtime.ts +++ b/server/gjc-runtime.ts @@ -143,10 +143,10 @@ export function gjcTerminal(pid: number, environ?: readonly string[] | null): Gj return { id: tty.slice(5).replaceAll("/", "-"), startedAt }; } if (process.platform === "darwin") { - // the kernel's record, as `ps -o tty=` read it, without a child process that holds the event loop - const tty = darwinTerminal(pid); - const startedAt = processStartedAt(pid); - return tty !== null && /^ttys\d+$/.test(tty) && startedAt !== null ? { id: tty, startedAt } : null; + // 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; diff --git a/server/hermes.ts b/server/hermes.ts index c909e12229..bd8553b4ae 100644 --- a/server/hermes.ts +++ b/server/hermes.ts @@ -135,19 +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") { // the kernel's record, as `ps -o tty=` read it, without a child process that holds the event loop - const output = darwinTerminal(pid); - if (!output) return null; - const device = output.startsWith("/dev/") ? output.slice(5) : output.startsWith("ttys") ? `dev-${output}` : output; - return `tty-${device.replaceAll("/", "-")}`; + 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; @@ -208,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); } } From 1f152bdfed4cc7584d2dde344bda5c0a9f02e606 Mon Sep 17 00:00:00 2001 From: mp Date: Mon, 26 Oct 2026 21:00:39 +0900 Subject: [PATCH 4/6] test(server): check #1986's macOS reads against the kernel record, not an awaited ps #1986 read a macOS process start through an awaited ps, and its OmO, Claude and GJC tests mocked that ps. This branch reads the start from the kernel instead, so those tests now stub darwinStartTime and check that no process is spawned at all. Refs #1404 --- server/conversation.test.ts | 18 +++++++++--------- server/gjc-runtime.test.ts | 22 +++++++++++----------- server/gjc-status.test.ts | 22 +++++++++++----------- server/omo.test.ts | 14 +++++++------- 4 files changed, 38 insertions(+), 38 deletions(-) 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/gjc-runtime.test.ts b/server/gjc-runtime.test.ts index 823327ddeb..cc5d44556b 100644 --- a/server/gjc-runtime.test.ts +++ b/server/gjc-runtime.test.ts @@ -1,4 +1,5 @@ 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"; @@ -390,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-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/omo.test.ts b/server/omo.test.ts index a11f099580..bbd2b0f0ac 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,17 @@ 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"]); + 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 }); From 2da269d41c96a40d043a9c44da310a6697baacc2 Mon Sep 17 00:00:00 2001 From: mp Date: Mon, 26 Oct 2026 21:13:30 +0900 Subject: [PATCH 5/6] test(omo): check that the kernel's start reaches the OmO result on macOS Astra gate B2 on #1980: the adapted OmO test still passed when processStartedAtAsync skipped the kernel and answered null. It now checks that the pane's OmO result carries the kernel's start and that the kernel was asked for that pid; making the read answer null fails it, as it fails the Claude and GJC tests. Also corrects omo.ts's comment that said each macOS read runs ps. Refs #1404 --- server/omo.test.ts | 4 +++- server/omo.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/server/omo.test.ts b/server/omo.test.ts index bbd2b0f0ac..f98701a073 100644 --- a/server/omo.test.ts +++ b/server/omo.test.ts @@ -518,7 +518,9 @@ it("reads OmO panes' process starts from the kernel on macOS, with no ps child", 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); + // 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 { 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> { From 6cd2d28972c178adfc440a6a3a33b74f0008fbb2 Mon Sep 17 00:00:00 2001 From: mp Date: Tue, 27 Oct 2026 04:14:33 +0900 Subject: [PATCH 6/6] fix(files): keep a name lookup begun before clearFileCache out of the cache CodeRabbit on #1980: a listing still waiting on git when clearFileCache ran could write its result into the cleared cache, and its cleanup could drop a newer listing for the same folder. Each listing now carries the cache generation it began in and fills the cache, or removes its entry, only while that generation is current. Refs #1404 --- server/files.test.ts | 19 +++++++++++++++++++ server/files.ts | 17 ++++++++++++----- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/server/files.test.ts b/server/files.test.ts index c9fb781678..f850d0af74 100644 --- a/server/files.test.ts +++ b/server/files.test.ts @@ -420,4 +420,23 @@ describe("git inventories", () => { 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 fd72c128d1..c68e356087 100644 --- a/server/files.ts +++ b/server/files.ts @@ -175,6 +175,8 @@ export async function paneFiles(cwd: string, query = "", limit = 20): Promise(); /** 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`), @@ -190,14 +192,18 @@ export async function filesNamed(cwd: string, name: string, limit = 10, anyForm 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 - shared = gitFiles(cwd, Infinity).then((listed) => { + const pending: Promise = gitFiles(cwd, Infinity).then((listed) => { const found = [...new Set([...listed ?? [], ...walkFiles(cwd)])]; - walked.set(cwd, { expires: Date.now() + CACHE_MS, files: found }); - if (walked.size > 16) walked.delete(walked.keys().next().value!); + 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(() => listing.delete(cwd)); - listing.set(cwd, shared); + }).finally(() => { if (listing.get(cwd) === pending) listing.delete(cwd); }); + shared = pending; + listing.set(cwd, pending); } files = await shared; } @@ -211,4 +217,5 @@ export function clearFileCache(): void { cache.clear(); walked.clear(); listing.clear(); + generation += 1; }