diff --git a/server/src/__tests__/heartbeat-workspace-branch-containment.test.ts b/server/src/__tests__/heartbeat-workspace-branch-containment.test.ts index 48e483b8ce..e8032b9847 100644 --- a/server/src/__tests__/heartbeat-workspace-branch-containment.test.ts +++ b/server/src/__tests__/heartbeat-workspace-branch-containment.test.ts @@ -1,6 +1,6 @@ import { execFile } from "node:child_process"; import { createHash, randomUUID } from "node:crypto"; -import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, realpath, rm, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { promisify } from "node:util"; @@ -131,7 +131,9 @@ async function readGit(cwd: string, args: string[]) { } async function createGitRepo() { - const repoRoot = await mkdtemp(path.join(os.tmpdir(), "paperclip-branch-containment-repo-")); + // realpath: on macOS os.tmpdir() is a symlink (/tmp -> /private/tmp) and the + // runtime persists resolved worktree paths, so unresolved fixtures never match. + const repoRoot = await realpath(await mkdtemp(path.join(os.tmpdir(), "paperclip-branch-containment-repo-"))); await runGit(repoRoot, ["init"]); await runGit(repoRoot, ["config", "user.email", "paperclip-test@example.com"]); await runGit(repoRoot, ["config", "user.name", "Paperclip Test"]); diff --git a/server/src/__tests__/workspace-runtime.test.ts b/server/src/__tests__/workspace-runtime.test.ts index 795bdfc828..802d4cd94c 100644 --- a/server/src/__tests__/workspace-runtime.test.ts +++ b/server/src/__tests__/workspace-runtime.test.ts @@ -54,6 +54,7 @@ import { isLocalServiceRegistryCwdCompatible, isLocalServiceProcessOwnedBy, isLocalServiceProcessInWorkspace, + readLocalServiceProcessCwd, readLocalServicePortOwner, writeLocalServiceRegistryRecord, } from "../services/local-service-supervisor.ts"; @@ -5277,7 +5278,46 @@ describe("readLocalServicePortOwner", () => { await expect(isLocalServiceProcessInWorkspace(serviceCwd, workspace)).resolves.toBe(true); }); - it("keeps a live registry record adoptable when cwd inspection is unsupported", async () => { + it("preserves newlines and trailing whitespace from Darwin lsof cwd output", async () => { + const fakeBin = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-runtime-lsof-tools-")); + const previousPath = process.env.PATH; + const reportedCwd = path.join(os.tmpdir(), "paperclip-runtime-line\nbreak "); + const output = `p${process.pid}\0fcwd\0n${reportedCwd}\0\n`; + await fs.writeFile( + path.join(fakeBin, "lsof"), + `#!${process.execPath}\nprocess.stdout.write(${JSON.stringify(output)});\n`, + { mode: 0o755 }, + ); + Object.defineProperty(process, "platform", { value: "darwin" }); + process.env.PATH = fakeBin; + + try { + await expect(readLocalServiceProcessCwd(process.pid)).resolves.toBe(reportedCwd); + } finally { + if (previousPath === undefined) delete process.env.PATH; + else process.env.PATH = previousPath; + await fs.rm(fakeBin, { recursive: true, force: true }); + } + }); + + it("returns null for invalid PIDs and a missing Darwin lsof binary", async () => { + await expect(readLocalServiceProcessCwd(-1)).resolves.toBeNull(); + + const fakeBin = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-runtime-missing-lsof-")); + const previousPath = process.env.PATH; + Object.defineProperty(process, "platform", { value: "darwin" }); + process.env.PATH = fakeBin; + + try { + await expect(readLocalServiceProcessCwd(process.pid)).resolves.toBeNull(); + } finally { + if (previousPath === undefined) delete process.env.PATH; + else process.env.PATH = previousPath; + await fs.rm(fakeBin, { recursive: true, force: true }); + } + }); + + it("keeps a live registry record adoptable when Darwin cwd inspection confirms it", async () => { try { await execFileAsync("lsof", ["-v"]); } catch { @@ -5329,16 +5369,19 @@ describe("readLocalServicePortOwner", () => { } }); - it("trusts unavailable cwd for registry records only off Linux", async () => { + it("trusts unavailable cwd for registry records only on unsupported platforms", async () => { Object.defineProperty(process, "platform", { value: "darwin" }); - await expect(isLocalServiceRegistryCwdCompatible(null, process.cwd())).resolves.toBe(true); + await expect(isLocalServiceRegistryCwdCompatible(null, process.cwd())).resolves.toBe(false); Object.defineProperty(process, "platform", { value: "linux" }); await expect(isLocalServiceRegistryCwdCompatible(null, process.cwd())).resolves.toBe(false); + + Object.defineProperty(process, "platform", { value: "win32" }); + await expect(isLocalServiceRegistryCwdCompatible(null, process.cwd())).resolves.toBe(true); }); it("refuses to adopt a listener whose real cwd belongs to another workspace", async () => { - if (process.platform !== "linux") return; + if (process.platform !== "linux" && process.platform !== "darwin") return; try { await execFileAsync("lsof", ["-v"]); } catch { @@ -5429,6 +5472,114 @@ describe("readLocalServicePortOwner", () => { await fs.rm(paperclipHome, { recursive: true, force: true }); } }); + + it("adopts a port owner running inside the workspace when the registry record is gone", async () => { + if (process.platform !== "linux" && process.platform !== "darwin") return; + try { + await execFileAsync("lsof", ["-v"]); + } catch { + return; + } + + const workspace = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-runtime-adopt-")); + const paperclipHome = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-runtime-home-")); + process.env.PAPERCLIP_HOME = paperclipHome; + process.env.PAPERCLIP_INSTANCE_ID = `adopt-port-owner-${randomUUID()}`; + const serviceKey = `adopt-port-owner-${randomUUID()}`; + // Detach, because managed runtime services also start detached + // (`detached: process.platform !== "win32"`). An attached child shares the + // runner's process group, so adoption would record the group leader — pnpm + // or a shell — and the command check would read that leader instead. + const child = spawn( + process.execPath, + [ + "-e", + "const server=require('node:http').createServer((req,res)=>res.end('ok')); server.listen(0, '127.0.0.1', () => console.log(server.address().port));", + ], + { cwd: workspace, stdio: ["ignore", "pipe", "inherit"], detached: true }, + ); + const port = await new Promise((resolve, reject) => { + let output = ""; + child.stdout?.on("data", (chunk) => { + output += String(chunk); + const value = Number.parseInt(output.trim(), 10); + if (Number.isInteger(value) && value > 0) resolve(value); + }); + child.once("error", reject); + child.once("exit", (code) => reject(new Error(`Port owner exited before listening: ${code ?? "unknown"}`))); + }); + + try { + // No registry record is written: this is the "registry lost, service still + // running" path that falls through to adoptLocalServiceFromPortOwner. + await expect(findAdoptableLocalService({ + serviceKey, + serviceName: "node", + command: "node", + cwd: workspace, + port, + })).resolves.toMatchObject({ pid: expect.any(Number), port }); + } finally { + child.kill("SIGTERM"); + await new Promise((resolve) => child.once("exit", () => resolve())); + await fs.rm(paperclipHome, { recursive: true, force: true }); + await fs.rm(workspace, { recursive: true, force: true }); + } + }); + + it("refuses to adopt a listener whose cwd differs only by trailing whitespace", async () => { + if (process.platform !== "linux" && process.platform !== "darwin") return; + try { + await execFileAsync("lsof", ["-v"]); + } catch { + return; + } + + const workspace = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-runtime-ws-")); + // A sibling directory whose name is the workspace name plus one space. + // These are different directories, so a listener in one must not be + // adopted into the other. + const lookalike = `${workspace} `; + await fs.mkdir(lookalike, { recursive: true }); + const paperclipHome = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-runtime-home-")); + process.env.PAPERCLIP_HOME = paperclipHome; + process.env.PAPERCLIP_INSTANCE_ID = `adopt-whitespace-${randomUUID()}`; + const serviceKey = `adopt-whitespace-${randomUUID()}`; + const child = spawn( + process.execPath, + [ + "-e", + "const server=require('node:http').createServer((req,res)=>res.end('ok')); server.listen(0, '127.0.0.1', () => console.log(server.address().port));", + ], + { cwd: lookalike, stdio: ["ignore", "pipe", "inherit"], detached: true }, + ); + const port = await new Promise((resolve, reject) => { + let output = ""; + child.stdout?.on("data", (chunk) => { + output += String(chunk); + const value = Number.parseInt(output.trim(), 10); + if (Number.isInteger(value) && value > 0) resolve(value); + }); + child.once("error", reject); + child.once("exit", (code) => reject(new Error(`Port owner exited before listening: ${code ?? "unknown"}`))); + }); + + try { + await expect(findAdoptableLocalService({ + serviceKey, + serviceName: "node", + command: "node", + cwd: workspace, + port, + })).resolves.toBeNull(); + } finally { + child.kill("SIGTERM"); + await new Promise((resolve) => child.once("exit", () => resolve())); + await fs.rm(paperclipHome, { recursive: true, force: true }); + await fs.rm(lookalike, { recursive: true, force: true }); + await fs.rm(workspace, { recursive: true, force: true }); + } + }); }); describeEmbeddedPostgres("workspace dirty quarantine branch repair", () => { diff --git a/server/src/services/local-service-supervisor.ts b/server/src/services/local-service-supervisor.ts index 51c329a816..0eee27ea3d 100644 --- a/server/src/services/local-service-supervisor.ts +++ b/server/src/services/local-service-supervisor.ts @@ -634,13 +634,50 @@ export async function readLocalServicePortOwner(port: number) { } } +/** + * Resolve a running process's working directory. + * + * Linux reads it straight off procfs. macOS has no procfs, so it asks `lsof` + * for the process's `cwd` descriptor — the same tool this module already shells + * out to for port ownership, so this adds no new dependency. `-d cwd` narrows + * the output to the working directory. + * + * `-F0n` terminates each field with NUL instead of a newline. The parser does + * not trim the path or split it on newlines. That matters because the caller + * compares this value against a workspace root: a directory name may contain + * leading or trailing spaces, or even a newline, and changing the path would + * report a different directory than the one the process runs in. + * + * Returning a real path on macOS is what lets `adoptLocalServiceFromPortOwner` + * verify a listener actually belongs to the workspace. While this returned + * null off Linux, that check could never pass, so port-owner adoption always + * failed there and still-running services were reconciled to `stopped`. + */ export async function readLocalServiceProcessCwd(pid: number) { - if (!Number.isInteger(pid) || pid <= 0 || process.platform !== "linux") return null; - try { - return await fs.readlink(`/proc/${pid}/cwd`); - } catch { - return null; + if (!Number.isInteger(pid) || pid <= 0) return null; + if (process.platform === "linux") { + try { + return await fs.readlink(`/proc/${pid}/cwd`); + } catch { + return null; + } } + if (process.platform === "darwin") { + try { + const { stdout } = await execFileAsync("lsof", ["-a", "-d", "cwd", "-p", String(pid), "-F0n"]); + // Each field ends with NUL. The newline that ends a field set carries + // into the next field, so drop it before reading the `n` tag; anything + // after the tag is the path exactly as lsof reported it. + const cwdField = stdout + .split("\0") + .map((field) => field.replace(/^\n+/, "")) + .find((field) => field.startsWith("n")); + return cwdField ? cwdField.slice(1) || null : null; + } catch { + return null; + } + } + return null; } export async function isLocalServiceProcessInWorkspace(processCwd: string, workspaceCwd: string) { @@ -657,7 +694,7 @@ export async function isLocalServiceProcessInWorkspace(processCwd: string, works } export async function isLocalServiceRegistryCwdCompatible(processCwd: string | null, workspaceCwd: string) { - if (!processCwd) return process.platform !== "linux"; + if (!processCwd) return process.platform !== "linux" && process.platform !== "darwin"; return isLocalServiceProcessInWorkspace(processCwd, workspaceCwd); }