fix(workspaces): read process cwd on macOS so port-owner adoption works (#11763)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A workspace can run a shared local service, such as a dev server, on an automatic port > - Paperclip adopts a live service again after it loses the runtime registry state > - Paperclip must first prove that the port owner runs inside the workspace > - Linux reads the process working directory from `/proc/<pid>/cwd` > - macOS has no `/proc`, so the check returned `null` and adoption always failed > - This pull request reads the process working directory with `lsof` on macOS > - The benefit is that macOS keeps a healthy live service after startup reconciliation, instead of recording it as stopped ## Linked Issues or Issue Description Closes #9911. That pull request reports the same defect and was opened first, on 2026-07-20. Its checks have been red since that day, because its inline issue description does not use the label format the gate parses. It has had no author activity since. This pull request keeps that author's test-fixture commit, with the author unchanged, and adds NUL-delimited parsing, adoption-boundary tests, and fail-closed Darwin registry handling. Maintainers may prefer to land #9911 instead. I will close this one again if they do. This pull request replaces #11600, which I closed earlier as a duplicate. It carries the same work, rebased onto current `master`, with the review feedback from that pull request applied. No public issue exists. The problem follows. **What happened?** On macOS, `readLocalServiceProcessCwd` returned `null`. Startup reconciliation found a live port owner, but it could not verify the working directory. It rejected the candidate and recorded the live service as stopped. **Expected behavior** Paperclip adopts a healthy port owner when the working directory is inside the requested workspace. Paperclip rejects the process when the working directory is outside the workspace, or when it cannot be read. **Steps to reproduce** 1. Build Paperclip from source on macOS. 2. Start a shared workspace runtime service on an automatic port. 3. Remove the runtime registry state while the service stays alive. 4. Run startup reconciliation. 5. Read the result. Unpatched `master` reports `adopted: 0` and `stopped: 1`. **Paperclip version or commit** This branch is based on `master` at `7c8064da1b35527865c1d523c9f0016e304ae46d`. **Deployment mode** Local development from source. **Installation method** Built from source with pnpm. **Operating system** macOS 26.4, Darwin 25.4.0, arm64. **Node.js version** Node.js 22.22.2 on macOS. Node.js 24.19.0 on Linux. pnpm 9.15.4. ## Darwin Registry Adoption Now Fails Closed This pull request changes one existing Darwin registry-adoption behavior in addition to enabling port-owner adoption. Before this change, `readLocalServiceProcessCwd` always returned `null` on Darwin. `isLocalServiceRegistryCwdCompatible` treated a null cwd as compatible on every non-Linux platform, so a service with an existing registry record could still be adopted when its port owner, process group, and command matched, even though Paperclip had not verified the process's real working directory. Darwin can now inspect the process cwd through `lsof`. If that inspection returns `null` — including a missing `lsof`, a command failure, or missing cwd output — registry-backed adoption now fails closed and the stale registry record is removed. This is a deliberate behavior change. It prevents a failed Darwin cwd probe from silently falling back to trusting stored registry metadata. The no-registry port-owner path already rejected a null cwd before this pull request, so its failure behavior has not changed. ## What Changed - Add a Darwin branch to `readLocalServiceProcessCwd`. - Run `lsof -a -d cwd -p <pid> -F0n` to read the process working directory. - Parse the NUL-delimited field output. - Do not trim the path. Do not split it on newlines. A directory name can contain a trailing space or a newline, and a changed path would name a different directory. - Keep the Linux `/proc/<pid>/cwd` path unchanged. - Return `null` for an invalid pid, a missing `lsof`, a command error, or missing output. - Reject a Darwin registry record when the working directory cannot be read. Darwin can now read it, so a failed read means the check failed. It no longer means the platform has no way to check. - Keep the registry fallback only on platforms that cannot read a process working directory. - Run the existing foreign-workspace rejection test on macOS. - Add a test: Paperclip adopts a port owner inside the workspace when no registry record exists. - Add a test: Paperclip rejects a listener in a sibling directory that differs only by a trailing space. - Add helper tests for newline and whitespace parsing, an invalid pid, and a missing `lsof` binary. - Resolve the branch-containment temporary repository root before the path comparison. This test-only commit comes from #9911 and keeps its author. ## Verification Head of this branch: `2be1b74746d8a0db4b680062f0c57995a6ff3912`. **Linux, on this head** ```sh pnpm --filter @paperclipai/server exec vitest run \ src/__tests__/workspace-runtime.test.ts \ src/__tests__/heartbeat-workspace-branch-containment.test.ts ``` Result: 138/138 pass. `workspace-runtime.test.ts` is 132/132. `heartbeat-workspace-branch-containment.test.ts` is 6/6. **macOS, on this head** macOS 26.4, Darwin 25.4.0, arm64, Node.js 22.22.2, pnpm 9.15.4. - Controlled baseline: `workspace runtime startup reconciliation > adopts a live auto-port shared service after runtime state is reset` fails on the rebase base `7c8064da1b35527865c1d523c9f0016e304ae46d` and reports `adopted: 0`, `stopped: 1`. The same test passes on this head. That test uses the normal managed start path, which starts the service detached. - Focused working-directory, registry, adoption, and boundary tests: 8/8 pass. - `heartbeat-workspace-branch-containment.test.ts`: 6/6 pass. Two assertions failed before the fixture change, because `/var/...` and `/private/var/...` name the same macOS directory. - Server typecheck: pass. - Full `workspace-runtime.test.ts`: 131/132 pass. The one failure is `realizeExecutionWorkspace > records teardown and cleanup operations when a recorder is provided`: ```text expected: /var/folders/... received: /private/var/folders/... ``` I ran that same test alone on the rebase base `7c8064da`, with no patch applied, and got the identical failure. It is a pre-existing macOS fixture that builds a path from `os.tmpdir()` and compares it against a realpath. It does not run the changed adoption path. This description does not claim the whole file is green on macOS. **macOS listener evidence** In the `adopts a port owner running inside the workspace when the registry record is gone` scenario, the auto-port listener bound port `54360`: ```text COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME node 6808 <local-user> 12u IPv4 0xee4e36b2c8c094cf 0t0 TCP 127.0.0.1:54360 (LISTEN) ``` To hold the listener open long enough to capture this, that one diagnostic run added a temporary pause, which exceeded the Vitest timeout. The pause was reverted, the unmodified test was run again on this head, and it passed 1/1. The process and the port were then released. Note for maintainers: an existing test already covered this defect. That test never runs on macOS, because CI runs on Linux. A macOS job would have caught it in July. ## Risks Low risk. - Linux keeps the existing procfs implementation. - Other platforms keep the existing registry fallback. - macOS makes one extra `lsof` call, and only when it must read a process working directory. - A probe failure returns `null`. - Darwin port-owner adoption and Darwin registry adoption both fail closed. - The parser keeps significant whitespace and embedded newlines. - There is no database migration and no API change. ## Model Used Claude Opus 5 (`claude-opus-5`), with extended thinking, tool use, and code execution. It wrote the original implementation and the adoption tests, reviewed the branch, ran the Linux test suite, rebased onto current `master`, and prepared this text. OpenAI GPT-5.6-sol, through Hermes Agent, added the failure-mode coverage and ran the macOS checks. A human reviewed the change and controls publication. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass (see Verification for the one disclosed macOS baseline failure) - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes (this change affects an internal helper and tests only) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: tim <tf00185077@i-mps.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: scbailey-build <scott@bequall.com>
This commit is contained in:
tf00185077
GitHub
parent
417336f8be
commit
14027df09e
|
|
@ -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"]);
|
||||
|
|
|
|||
|
|
@ -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<number>((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<void>((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<number>((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<void>((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", () => {
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue