row 202 round 2 (#1700): lock refuses and never reclaims; fs seam with temp cleanup; BusinessError exits 3; refusals before context (Duckworth B1, Sage sage-1429, sage-1437)

- Lock: a dead-pid, empty or unreadable lock refuses and names the path (and pid); a human removes it. No reclaim, so the reclaim race is gone.
- writeAtomic takes the fs calls through deps.fs; a failed write removes its temp file and leaves the old bytes.
- A BusinessError from loadAccess or loadBusiness exits 3 with the file and key path; no stack trace.
- contextFor is built after the agent and tester refusals and the human proof.
- Tests: 21 in access.test.mjs, single file green. New: dead-pid refuses, failed rename, 0664 file, refusal before context.

agents/quackerjack/work/queue-202/BUILD.md

@@ -11,7 +11,7 @@ Brief: `docs/plans/2026-10-20_federation-r1-r2-r5.md`, "Brief: FED R5". Reviewer
 - `scripts/mosaic`: sends `access` to the CLI.
 - `packages/cli/README.md`: the verbs, the human proof, the lock, the `instances` limit, the removal order,
   and the limits on peers and subjects.
-- `packages/cli/tests/access.test.mjs`: 18 tests. Single file, green on this branch.
+- `packages/cli/tests/access.test.mjs`: 21 tests (round 2; 18 in round 1). Single file, green on this branch.
 
 R1 (`packages/business/src/access.mjs`) is merged in unchanged at 80fe31071, the Duckworth-approved
 candidate. 44e36fc4c, the next commit on that branch, only removes a stray pyc file.
@@ -24,25 +24,45 @@ candidate. 44e36fc4c, the next commit on that branch, only removes a stray pyc f
 - A second writer that finds the lock reads the pid. If the pid is alive (or the pid is reused by a live
   process), it refuses with `refused: ... being written by another command; try again` (exit 3). Nothing
   is written.
-- A lock whose pid is gone (`kill(pid, 0)` returns ESRCH) is stale. The next writer removes it and takes
-  the lock. This is the crash case: the holder died mid-write.
+- A lock whose pid is gone (`kill(pid, 0)` returns ESRCH) also refuses, and nothing reclaims it. The
+  refusal names the path and the pid. A human checks that no write is running and removes the file. This
+  is the crash case: the holder died mid-write, and a human clears it.
 - An empty lock, or one that is not a pid, refuses. Its holder may still be between create and write. A
-  holder that died in that gap leaves an empty lock; remove it by hand.
+  holder that died in that gap leaves an empty lock; a human removes it.
+
+Reclaim was removed at Sage's ruling (sage-1429, 10-20). It was the one design with a race, so the
+race below is gone, not accepted.
+
+Former limit, now removed: reclaim race. The old design let a writer take a stale lock by unlinking it
+and retaking it. The case that lost: writers A and B both read the same dead pid N. A unlinks the lock
+and takes it. B then unlinks A's live lock and takes its own. Both read the same revision, both write
+it, and the second rename silently replaces the first. The revision check did not catch it, because both
+had read the old revision before either wrote.
 
 Known limits, stated plainly:
-1. Reclaim is not atomic. Two writers that both find the same stale lock can both reclaim it, and then one
-   can remove the other's new lock. The window opens only after a crash. The revision check still refuses
-   a second write that read the old revision, but two writers that both read the new revision can race.
-   Fixing it needs a lock that the kernel releases with the holder (`flock`). That is a decision for Sage.
+1. A dead holder's lock blocks every write until a human removes it. That is the fail-closed price of
+   no reclaim. The refusal says which pid and which path.
 2. A reused pid reads as alive, so a stale lock refuses. The fix is to remove the file by hand.
 3. The lock is per access file. It does not cover the bus's own reads.
+4. Crash-safety of the write path: the temp file is removed on any failure before or at the rename. The
+   directory is not fsynced after the rename, so a power loss right after a rename can lose the rename.
+   The bytes are never mixed.
 
 Tests for the lock (all in `access.test.mjs`):
 - "a live holder's lock refuses and leaves the file and the lock alone": pid = this process.
 - "an empty or unreadable lock refuses": empty file, and `not-a-pid`.
-- "a stale lock from a dead pid is reclaimed and the write goes through": a child that has exited.
+- "a lock left by a dead pid refuses, names the pid, and is not reclaimed": a child that has exited. The
+  file, the lock and the receipt are unchanged.
 - "the lock is mode 0600 while held and removed after a refused write".
 
+Tests for the write path and the refusals (added in this round):
+- "a failed rename leaves the old bytes, removes the temp file, and writes no receipt": the fs seam
+  (`deps.fs`) makes `renameSync` throw. Checks the old bytes, no `.access-*.tmp`, no lock, no receipt.
+- "a group-writable access file refuses with exit 3, names the file, and changes nothing": a 0664 file.
+  Exit 3, the file path in the message, nothing written.
+- "an agent or tester caller is refused before the context or the access file is read": `contextFor`
+  throws if called, and the file is 0664. The refusal must come first.
+
 ## The human proof, and the bus behavior behind test 10
 
 The proof is one `project.list` call through `humanTransport`. The bus gates it with `#human`
@@ -81,16 +101,22 @@ reads `console` through `checkConsole`.
 | M2 | remove the `--tester` check | "--tester is refused on every verb" |
 | M3 | the proof's error is swallowed | "a bus that refuses the human proof blocks the write" |
 | M4 | the revision compare is removed | "a stale revision refuses with revision-conflict" |
-| M5 | `holderAlive` always returns false (always reclaim) | "a live holder's lock refuses" |
-| M6 | `holderAlive` always returns true (never reclaim) | "a stale lock from a dead pid is reclaimed" |
+| M5 | `takeLock` unlinks a dead-pid lock and retakes it (the reclaim this round removed) | "a lock left by a dead pid refuses, names the pid, and is not reclaimed" |
+| M6 | `pidAlive` always returns true (a dead pid reads as live) | "a lock left by a dead pid refuses, names the pid, and is not reclaimed" |
+| M11 | `writeAtomic` writes straight to access.json, no temp file or rename | "a failed rename leaves the old bytes, removes the temp file, and writes no receipt" |
+| M12 | the BusinessError catch in `accessCommand` is removed | "a group-writable access file refuses with exit 3, names the file, and changes nothing" |
+| M13 | `contextFor()` is called before the agent and tester refusals | "an agent or tester caller is refused before the context or the access file is read" |
 | M7 | `checkAccess` is skipped before the write | "a grant naming an unknown instance is refused before any write" |
 | M8 | the receipt append is removed | "grant writes ... a receipt line" |
 | M9 | a project name is written as given (no lookup) | "a project name resolves to its id" |
 | M10 | the lock is not taken | "a live holder's lock refuses" |
 
-Known gap, not killed by any test: a direct `writeFileSync` in place of the temp file and rename. No test
-makes the rename fail. Adding one needs a seam in `writeAtomic`, which is not exported. Sage to decide
-whether that goes in this row or a follow-up.
+Round 2 closed the direct-write gap: `writeAtomic` takes the fs calls through `deps.fs` (default: the real
+`node:fs` functions), and a rename that throws is tested (M11 is the direct-write mutant). Not yet run:
+the mutant batch, the cli suite and `scripts/test-*.sh`. All wait on Sage's slot.
+
+Round 2 also changed the file's refusal path: a `BusinessError` from `loadAccess` or `loadBusiness` now
+exits 3 with the file path and key path on stderr, no stack trace (Duckworth B1, Sage sage-1437).
 
 ## Not done in this row
 

packages/cli/README.md

@@ -1022,9 +1022,15 @@ call through the human transport: the bus gates it with `#human` (`packages/bus/
 the `project.list` case and `#human`). An agent, or `--tester`, is refused before any write. Each
 write runs the file's checks on the new document first, bumps the revision by one, and renames
 atomically. A `--revision` that is not the file's current revision refuses with `revision-conflict`.
-A second writer that finds the lock refuses at once. Each write appends one receipt line to
+A writer that finds the lock refuses at once. A lock whose pid is gone, or that is empty or
+unreadable, also refuses, and nothing reclaims it: check that no write is running, then remove
+`<configDir>/access.json.lock` by hand. Each write appends one receipt line to
 `<dataRoot>/state/access-log.jsonl`.
 
+The command runs as the instance's uid. `access.json` must belong to that uid and must not be
+writable by group or other. If it is not, or a business file fails to load, the command refuses
+with exit 3, names the file and the key path, and writes nothing.
+
 A grant's `instances` is the real limit. The granted instance runs with the owner's role, so a
 collaborator's message drives it with that authority. To cut a user off, revoke first: it acts at
 once. Removing a provider from `console.sso` takes effect only after a Console restart.

packages/cli/src/access.mjs

@@ -4,7 +4,7 @@
 // receipt line to <dataRoot>/state/access-log.jsonl. The human proof is one call
 // through humanTransport: a bus that does not see a human shell refuses it, so
 // no agent and no tester can write. The context (providers, peers, businesses,
-// ssoSubjects) comes in as a parameter; the caller decides where it comes from.
+// ssoSubjects) comes in as a function; the caller decides where it comes from.
 
 import { appendFileSync, mkdirSync, readdirSync, readFileSync, renameSync, openSync, writeSync, fsyncSync, closeSync, existsSync, unlinkSync } from "node:fs";
 import { randomBytes } from "node:crypto";
@@ -35,7 +35,9 @@ To cut a user off, revoke them here first. It acts at once. Removing a provider
 from console.sso takes effect only after a Console restart, so do that second.
 Peer homes are not read yet (row 203 adds the federation key), so a grant that
 names a peer home refuses. A grant that names a sign-in subject needs console.sso
-in the system config; without it every such grant refuses.`;
+in the system config; without it every such grant refuses.
+The command runs as the instance's uid: access.json must belong to it and must
+not be writable by group or other, or the command refuses (exit 3) and writes nothing.`;
 
 // The instance's context for checkAccess, read from the system config and the business
 // files. providers and ssoSubjects come from console.sso (row 192's shape: each provider
@@ -79,19 +81,31 @@ function readCurrent(file, context) {
   return { document, revision: document.revision };
 }
 
+// The file calls the write path makes. A test passes a stand-in through deps.fs.
+const realFs = { openSync, writeSync, fsyncSync, closeSync, renameSync, unlinkSync };
+
 // Atomic write: a temp file in the same directory, 0600, fsynced, then renamed over
-// the old file. A reader sees the old bytes or the new bytes, never a mix.
-function writeAtomic(file, text) {
-  const dir = dirname(file);
-  const tmp = join(dir, `.access-${randomBytes(8).toString("hex")}.tmp`);
-  const fd = openSync(tmp, "wx", 0o600);
+// the old file. A reader sees the old bytes or the new bytes, never a mix. A failure
+// before or at the rename removes the temp file and leaves the old bytes in place.
+function writeAtomic(file, text, fs = realFs) {
+  const tmp = join(dirname(file), `.access-${randomBytes(8).toString("hex")}.tmp`);
   try {
-    writeSync(fd, text);
-    fsyncSync(fd);
-  } finally {
-    closeSync(fd);
+    const fd = fs.openSync(tmp, "wx", 0o600);
+    try {
+      fs.writeSync(fd, text);
+      fs.fsyncSync(fd);
+    } finally {
+      fs.closeSync(fd);
+    }
+    fs.renameSync(tmp, file);
+  } catch (error) {
+    try {
+      fs.unlinkSync(tmp);
+    } catch {
+      // Already gone: the temp file was never created.
+    }
+    throw error;
   }
-  renameSync(tmp, file);
 }
 
 function receipt(dataRoot, line) {
@@ -102,21 +116,23 @@ function receipt(dataRoot, line) {
 
 // Two writers never interleave: the read, the check and the rename happen under an
 // exclusive lock file, `<access.json>.lock`, mode 0600. The file holds the holder's
-// pid and a newline. A lock whose pid is no longer alive (ESRCH) is stale: the
-// holder died mid-write, so the next writer removes it and takes the lock. A lock
-// whose pid is alive, or that is empty or unreadable, refuses (fail closed). An
-// empty lock is a holder that died between create and write; remove it by hand.
-// A reused pid reads as alive, so that also refuses; remove the file by hand then.
+// pid and a newline. A lock whose holder is alive refuses. A lock that is empty,
+// unreadable, or whose pid is gone also refuses, and nothing reclaims it: a human
+// removes it once no write is running. Fail closed, with no reclaim race.
 const BUSY = "refused: the access file is being written by another command; try again";
 
-function holderAlive(lock) {
-  let pid;
+function lockPid(lock) {
+  let text;
   try {
-    pid = Number(readFileSync(lock, "utf8").trim());
+    text = readFileSync(lock, "utf8");
   } catch {
-    return true;
+    return null;
   }
-  if (!Number.isSafeInteger(pid) || pid < 1) return true;
+  const pid = Number(text.trim());
+  return Number.isSafeInteger(pid) && pid >= 1 ? pid : null;
+}
+
+function pidAlive(pid) {
   try {
     process.kill(pid, 0);
     return true;
@@ -125,34 +141,34 @@ function holderAlive(lock) {
   }
 }
 
-function takeLock(lock) {
+function lockRefusal(lock) {
+  const pid = lockPid(lock);
+  if (pid !== null && pidAlive(pid)) return BUSY;
+  if (pid === null) return `refused: lock ${lock} is unreadable or not a pid; if no write is running, remove it by hand and try again`;
+  return `refused: lock ${lock} is left by pid ${pid}, which is not running; if no write is running, remove it by hand and try again`;
+}
+
+function takeLock(lock, fs) {
   try {
-    return openSync(lock, "wx", 0o600);
+    return fs.openSync(lock, "wx", 0o600);
   } catch (error) {
     if (error.code !== "EEXIST") throw error;
   }
-  if (holderAlive(lock)) throw new CliError(BUSY, 3);
-  unlinkSync(lock);
-  try {
-    return openSync(lock, "wx", 0o600);
-  } catch (error) {
-    if (error.code === "EEXIST") throw new CliError(BUSY, 3);
-    throw error;
-  }
+  throw new CliError(lockRefusal(lock), 3);
 }
 
-export function withLock(file, fn) {
+export function withLock(file, fn, fs = realFs) {
   const lock = `${file}.lock`;
-  const fd = takeLock(lock);
+  const fd = takeLock(lock, fs);
   try {
-    writeSync(fd, `${process.pid}\n`);
+    fs.writeSync(fd, `${process.pid}\n`);
   } finally {
-    closeSync(fd);
+    fs.closeSync(fd);
   }
   try {
     return fn();
   } finally {
-    unlinkSync(lock);
+    fs.unlinkSync(lock);
   }
 }
 
@@ -174,9 +190,20 @@ function resolveProjects(items, rows) {
   });
 }
 
-// deps: { system: { dataRoot }, transport(business) -> call(verb, args), context, configDir?, now? }
+// deps: { system: { dataRoot }, transport(business) -> call(verb, args), contextFor() -> context, configDir?, now?, fs? }
 export async function accessCommand(args, io, deps) {
   refuseInsideAgent(io.env);
+  try {
+    return await accessVerb(args, io, deps);
+  } catch (error) {
+    // A file or business doc the checks refuse to load (group-writable, a wrong uid, a
+    // hand edit) is a refusal with exit 3 and the file and key path, not a crash.
+    if (error instanceof BusinessError) throw new CliError(`refused: ${error.message}`, 3);
+    throw error;
+  }
+}
+
+async function accessVerb(args, io, deps) {
   const parsed = parse(args);
   const sub = parsed.positional[0];
   if (!["list", "grant", "revoke"].includes(sub) || parsed.positional.length !== 1) throw new CliError(ACCESS_HELP, 4);
@@ -186,7 +213,8 @@ export async function accessCommand(args, io, deps) {
 
   const rows = await proveHuman(deps.transport, business);
   const file = deps.file ?? accessFilePath(deps.configDir ?? configDir(io.env));
-  const context = deps.context;
+  // Built after the human proof, so a refused caller never reads the business files.
+  const context = deps.contextFor();
 
   if (sub === "list") {
     const { document } = readCurrent(file, context);
@@ -232,16 +260,10 @@ export async function accessCommand(args, io, deps) {
       next = { ...document, revision: revision + 1, grants: document.grants.filter((g) => !drop(g)) };
     }
     // Every check runs on the new document before a byte is written.
-    let checked;
-    try {
-      checked = checkAccess(next, context);
-    } catch (error) {
-      if (error instanceof BusinessError) throw new CliError(`refused: ${error.message}`, 2);
-      throw error;
-    }
-    writeAtomic(file, `${JSON.stringify(checked, null, 2)}\n`);
+    const checked = checkAccess(next, context);
+    writeAtomic(file, `${JSON.stringify(checked, null, 2)}\n`, deps.fs);
     return { checked, revision, touched };
-  });
+  }, deps.fs);
 
   receipt(deps.system.dataRoot, {
     at: (deps.now ?? (() => new Date()))().toISOString(),

packages/cli/src/cli.mjs

@@ -579,7 +579,7 @@ export async function main(argv, io = { env: process.env, stdin: process.stdin,
     return accessCommand(rest, io, {
       system,
       transport: (b) => humanTransport({ socket: socketPath(system.dataRoot), business: b, env: io.env }),
-      context: deps.accessContext ?? accessContextFor(system, io.env),
+      contextFor: () => deps.accessContext ?? accessContextFor(system, io.env),
       now: deps.now,
     });
   }

packages/cli/tests/access.test.mjs

@@ -5,7 +5,8 @@
 import test from "node:test";
 import assert from "node:assert/strict";
 import { spawnSync } from "node:child_process";
-import { existsSync, mkdirSync, readFileSync, readdirSync, statSync, writeFileSync } from "node:fs";
+import * as nodeFs from "node:fs";
+import { chmodSync, existsSync, mkdirSync, readFileSync, readdirSync, statSync, writeFileSync } from "node:fs";
 import { join } from "node:path";
 import { startBroker } from "../../bus/src/runtime.mjs";
 import { accessCommand, accessContextFor, withLock } from "../src/access.mjs";
@@ -53,7 +54,7 @@ function setup(t, { mode = "ok", doc = DOC } = {}) {
   const deps = {
     system: { dataRoot },
     configDir: config,
-    context: CONTEXT,
+    contextFor: () => CONTEXT,
     now: () => new Date("2026-10-20T20:00:00Z"),
     transport: (business) => humanTransport({ socket: join(root, "bus.sock"), business, env, cli, timeoutMs: 5000 }),
   };
@@ -128,19 +129,53 @@ test("a live holder's lock refuses and leaves the file and the lock alone", asyn
 test("an empty or unreadable lock refuses, since its holder may still be writing", async (t) => {
   const s = setup(t);
   writeFileSync(`${s.file}.lock`, "");
-  await refusal(run([...GRANT, "--revision", "1"], s.deps), /being written by another command/);
+  await refusal(run([...GRANT, "--revision", "1"], s.deps), /is unreadable or not a pid/);
   writeFileSync(`${s.file}.lock`, "not-a-pid\n");
-  await refusal(run([...GRANT, "--revision", "1"], s.deps), /being written by another command/);
+  await refusal(run([...GRANT, "--revision", "1"], s.deps), /is unreadable or not a pid/);
   assert.equal(existsSync(`${s.file}.lock`), true);
 });
 
-test("a stale lock from a dead pid is reclaimed and the write goes through", async (t) => {
+test("a lock left by a dead pid refuses, names the pid, and is not reclaimed", async (t) => {
   const s = setup(t);
   const dead = spawnSync(process.execPath, ["-e", ""]);
   writeFileSync(`${s.file}.lock`, `${dead.pid}\n`, { mode: 0o600 });
-  await run([...GRANT, "--revision", "1"], s.deps);
-  assert.equal(s.read().revision, 2);
+  const before = readFileSync(s.file, "utf8");
+  await refusal(run([...GRANT, "--revision", "1"], s.deps), new RegExp(`${s.file}\\.lock is left by pid ${dead.pid}, which is not running`));
+  assert.equal(readFileSync(s.file, "utf8"), before);
+  assert.equal(readFileSync(`${s.file}.lock`, "utf8"), `${dead.pid}\n`);
+  assert.equal(s.receipts(), "");
+});
+
+test("a failed rename leaves the old bytes, removes the temp file, and writes no receipt", async (t) => {
+  const s = setup(t);
+  const before = readFileSync(s.file, "utf8");
+  const fs = { ...nodeFs, renameSync: () => { throw Object.assign(new Error("injected rename failure"), { code: "EXDEV" }); } };
+  await assert.rejects(run([...GRANT, "--revision", "1"], { ...s.deps, fs }), /injected rename failure/);
+  assert.equal(readFileSync(s.file, "utf8"), before);
+  assert.deepEqual(readdirSync(s.config).filter((f) => f.startsWith(".access-")), []);
   assert.equal(existsSync(`${s.file}.lock`), false);
+  assert.equal(s.receipts(), "");
+});
+
+test("a group-writable access file refuses with exit 3, names the file, and changes nothing", async (t) => {
+  const s = setup(t);
+  chmodSync(s.file, 0o664);
+  const before = readFileSync(s.file, "utf8");
+  const err = await refusal(run([...GRANT, "--revision", "1"], s.deps), /must not be writable by group or other/);
+  assert.equal(err.exitCode, 3);
+  assert.match(err.message, new RegExp(s.file.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")));
+  assert.equal(readFileSync(s.file, "utf8"), before);
+  assert.equal(s.receipts(), "");
+  assert.equal(existsSync(`${s.file}.lock`), false);
+});
+
+test("an agent or tester caller is refused before the context or the access file is read", async (t) => {
+  const s = setup(t);
+  chmodSync(s.file, 0o664);
+  const deps = { ...s.deps, contextFor: () => { throw new Error("context was read"); } };
+  const env = { ...humanEnv(), MOSAIC_RUN_ID: "run-1" };
+  await refusal(run([...GRANT, "--revision", "1"], deps, env), /human shell/);
+  await refusal(run(["list", "--business", "acme", "--tester"], deps), /not --tester/);
 });
 
 test("the lock is mode 0600 while held and removed after a refused write", async (t) => {
@@ -257,7 +292,7 @@ test("the real human-cli, run from agent ancestry, refuses and writes nothing",
   const deps = {
     system: { dataRoot: f.dataRoot },
     configDir: config,
-    context: CONTEXT,
+    contextFor: () => CONTEXT,
     transport: (business) => humanTransport({ socket: socketPath(f.dataRoot), business, env: agentEnv, cli: HUMAN_CLI, timeoutMs: 10000 }),
   };
   // The bus closes the socket on a failed human proof, so the client reports