fix(lock): re-check the config under the lock; never break a lock mid-write (PHARN-03)

.dev/features/lock-read-modify-write/GRILL.md

@@ -0,0 +1,34 @@
+# GRILL — lock-read-modify-write
+
+Plan: `.dev/features/lock-read-modify-write/PLAN.md` · spec-hash `bca940a5…d729d3c4e` matches live
+`ARCHITECTURE.md`. Registered grillers: `{"registered":0,"grillers":[]}` → inline axes only.
+
+## Findings
+
+```yaml
+- type: FINDING
+  rule_id: 'P0'
+  severity: important
+  file: '.dev/features/lock-read-modify-write/PLAN.md:67'
+  problem: 'The stress test can show the race is absent in N rounds but cannot prove it; the floor claim must rest on the mtime-grace unit test, with the stress test labeled advisory evidence (the plan does so).'
+  evidence: '"zero overlapping holders" → demonstrated by the stress test (advisory evidence, not a proof).'
+- type: FINDING
+  rule_id: 'P5'
+  severity: minor
+  file: '.dev/features/lock-read-modify-write/PLAN.md:33'
+  problem: 'Comparing JSON.stringify of two parsed configs is exact only because both sides go through the same readPharnConfig; a snapshot mutated in memory before the re-check would false-positive. The check must run before any in-memory mutation of `config`.'
+  evidence: '`JSON.stringify` of two `readPharnConfig` results over the same schema'
+- type: FINDING
+  rule_id: 'P1'
+  severity: minor
+  file: '.dev/features/lock-read-modify-write/PLAN.md:41'
+  problem: 'A multi-process stress test inside vitest adds wall-clock time and potential CI flakiness; keep rounds bounded and assert only overlap = 0.'
+  evidence: 'a multi-process stress test (N child processes × rounds'
+```
+
+## Summary
+
+Both halves reduce to deterministic compares under the existing O_EXCL lock. The residuals (a >10 s
+stalled `tryCreate`, non-pharn writers) are named in the plan.
+
+ADVISORY VERDICT: 3 concerns raised (0 blocking-severity, 3 advisory) — for the human to weigh before /pharn-dev-build.

.dev/features/lock-read-modify-write/PLAN.md

@@ -0,0 +1,78 @@
+# PLAN — lock-read-modify-write (PHARN-03: the project lock must cover the config snapshot and admit one holder)
+
+- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4
+- increment: (a) `add` / `update` / `remove` re-read `pharn.config.json` as the first step INSIDE the
+  project lock and refuse (exit 1, nothing written) when it differs from the snapshot they planned
+  against; (b) a lock file that exists but is not a parseable payload and is younger than a short
+  grace window is treated as LIVE (a holder mid-`tryCreate`), not stale.
+- layer(s): the CLI itself (`src/lib/project-lock.ts`, `src/lib/pharn-config.ts`, `src/commands/{add,update,remove}.ts`)
+- constitution_refs: [P0, P1, P3, P5, P6]
+
+## Discovery — verified this run (P6)
+
+Base = `main` after PHARN-02. Reproduced in the review:
+
+- (a) `repro/stale-config`: `remove` picker waits at its confirm; a concurrent `add a11y` succeeds; the
+  confirmed `remove` writes its pre-prompt snapshot → `a11y` vanishes from the config, its files and 13
+  records stay, both exit 0. `update` has the same shape (config read `update.ts:81`, confirm, lock
+  `:292`); `remove` named/picker read `:50`, lock `:275`/`:380`; `add` read `:65`, lock `:174`/`:297`.
+- (b) `repro/lock-race`: `tryCreate` (`project-lock.ts`) opens with `O_EXCL` then writes the payload,
+  so the file is briefly EMPTY; `parsePayload('') → null`, `isStale(null) → true`, and a concurrent
+  process breaks the live lock. 8 processes × 150 rounds → 15 rounds with 2–5 simultaneous holders.
+
+## Files
+
+- `src/lib/project-lock.ts` — `ProjectChangedError extends ProjectLockedError` (so every existing
+  catch site reports it as a named refusal + exit 1 with cleanup); `isStale` takes the lock file's
+  mtime and returns `false` for an unparseable payload younger than `MALFORMED_GRACE_MS` (10 s) —
+  an older or unreadable-and-unstattable one is still broken (never wedge) — layer CLI/lib
+- `src/lib/pharn-config.ts` — `assertConfigUnchanged(cwd, snapshot, command)`: re-reads the config and
+  throws `ProjectChangedError` when it is absent or its parsed form differs (`JSON.stringify` of two
+  `readPharnConfig` results over the same schema) — layer CLI/lib
+- `src/commands/add.ts` — call `assertConfigUnchanged` first inside both `withProjectLock` callbacks — layer CLI/command
+- `src/commands/update.ts` — same, first inside its `withProjectLock` callback — layer CLI/command
+- `src/commands/remove.ts` — same, in both lock callbacks — layer CLI/command
+- `tests/project-lock.test.ts` — empty lock with fresh mtime → refused, fn not run, lock untouched;
+  empty lock with mtime past the grace → broken and acquired (the multi-process race is measured
+  out-of-suite with the review's `repro/lock-race` harness, 150 rounds × 8 processes, before/after)
+- `tests/pharn-config.test.ts` — `assertConfigUnchanged`: identical → no throw; changed / deleted →
+  `ProjectChangedError` naming the file and the command
+- `tests/remove.test.ts` — picker: config rewritten (a capability added) between load and confirm →
+  exit 1, nothing deleted, the concurrently-added entry preserved
+- `tests/update.test.ts` — config changed between load and lock → exit 1, nothing written
+- `tests/add.test.ts` — config changed between load and lock → exit 1, nothing written
+- `tests/project-lock-commands.test.ts` — its `pharn-config.js` mock gains a no-op `assertConfigUnchanged` stub
+- `README.md` — "One writer at a time" paragraph: a run whose config changed underneath it refuses (P4)
+- `CLAUDE.md` — project-lock / command paragraphs (P4)
+
+## Contracts satisfied
+
+- README "One writer at a time… a second run refuses" and the `project-lock.ts` header's stated threat
+  (planning against a stale snapshot) — now true for the prompt window too.
+
+## Evals to write (P1)
+
+- listed per test file above; each must fail on the base source.
+
+## Guarantee audit (P0)
+
+- "a command never persists a config planned against a stale snapshot" → floor: exact comparison of
+  the re-read config against the snapshot, under the O_EXCL lock. Residual (named): an edit landing
+  AFTER the re-check but while the lock is held by a non-pharn writer (a human editor) is not covered.
+- "a live lock is never broken while its payload is being written" → floor: mtime-age comparison
+  against `MALFORMED_GRACE_MS`. Residual (advisory): a holder stalled > 10 s between `open` and
+  `write` (practically impossible for a two-syscall window) could still be broken.
+- "zero overlapping holders" → demonstrated by the stress test (advisory evidence, not a proof).
+
+## Trust audit (P2)
+
+- The lock file and config are local, user-editable; mtime is read via `statSync` only. No new
+  untrusted remote input.
+
+## Determinism audit (P5)
+
+- Branches: string equality of two serialized configs; integer age compare. Terminal = named refusal.
+
+## Open questions (HALT)
+
+- none

.dev/features/lock-read-modify-write/REGRESSION.md

@@ -0,0 +1,30 @@
+# REGRESSION — lock-read-modify-write
+
+The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment.
+
+## Base and partition
+
+- **base:** `31f15cedcad80465396e2532f617f8afb60b3012` (`origin/main` at build time; the build is an uncommitted working tree on top of it).
+- **inside** (each declared in `PLAN.md` `## Files`): `CLAUDE.md`, `README.md`, `src/commands/add.ts`, `src/commands/remove.ts`, `src/commands/update.ts`, `src/lib/pharn-config.ts`, `src/lib/project-lock.ts`, `tests/add.test.ts`, `tests/pharn-config.test.ts`, `tests/project-lock-commands.test.ts`, `tests/project-lock.test.ts`, `tests/remove.test.ts`, `tests/update.test.ts`.
+- **scope partition:** `check-regress.mjs scope` exited **0**, `escaped: []`. `.pharn/` (hook scratch) and this
+  feature's own stage artifacts are not build output.
+- **outside gates:** the stdlib `*.test.mjs` / `*.test.cjs` files + whole-repo `validate`; 0 committed eval pairs.
+- **style-gate skip:** `inside` touches no shared style config, so `lint` / `format:check` / `lint:md` are absent from both maps.
+
+## Per-gate exit codes
+
+| gate       | base | head | flipped? |
+| ---------- | ---- | ---- | -------- |
+| `tests`    | 0    | 0    | no       |
+| `validate` | 0    | 0    | no       |
+
+- `regressions[]`: **empty**
+- `pre_existing[]`: **empty**
+
+## Verdict
+
+**REGRESSIONS: none — no deterministically-detectable breakage outside the feature.**
+(`regression-report.json` `.verdict` = `no-regressions`.)
+
+Residual (P0/P7): this catches exactly what its suite catches. The vitest suite exercising `src/**` is
+owned by `/pharn-dev-build`'s floor and `/pharn-dev-verify`. This certifies the comparison, never the increment.

.dev/features/lock-read-modify-write/REVIEW.md

@@ -0,0 +1,50 @@
+# REVIEW — lock-read-modify-write
+
+Floor first: `node .dev/floor/validate.mjs .` → exit 0 (GREEN). Everything below is **advisory**.
+
+## Floor-gate findings (blocking)
+
+None.
+
+- **L-floor (P0):** (a) reduces to an exact string compare of two `readPharnConfig` parses under the
+  O_EXCL lock; (b) to an integer mtime-age compare against `MALFORMED_GRACE_MS`. Residuals named in code:
+  a non-pharn writer editing after the re-check, and a holder stalled > 10 s inside `tryCreate`.
+- **L-eval (P1):** 8 new cases fail on the base source: 2 fresh-unparseable-lock refusals, 3
+  `assertConfigUnchanged` units, and the `remove` picker / `remove` named / `update` / `add`
+  interleave cases in `project-lock-commands.test.ts` (plus a control). Nine existing lock tests now
+  backdate their planted malformed lock past the grace window — that is their intent (a leftover), not a
+  weakening.
+- **Out-of-suite evidence (advisory):** the review's `repro/lock-race` harness (8 processes started in the
+  same millisecond, 150 rounds) measured **26/150** rounds with >1 simultaneous holder on the base lock and
+  **0/150** on this build. An in-suite version at CI-affordable size (6×12) did not reproduce the race on
+  the base either, so it was dropped rather than shipped as a test that proves nothing.
+- **L-trust (P2):** no new untrusted input; the lock/config are local files read defensively.
+- **L-axis (P3):** `pharn-config.ts` imports `ProjectChangedError` from `project-lock.ts` (lib→lib, no
+  cycle: `project-lock.ts` imports only `validate.ts`). The lock module stays config-agnostic.
+
+## Advisory findings
+
+```yaml
+- type: FINDING
+  rule_id: 'P5'
+  severity: minor
+  file: 'src/lib/pharn-config.ts'
+  problem: 'The comparison covers pharn.config.json only; a concurrent writer that changed pharn.records.json but not the config would not be detected. Every pharn writer that touches records also rewrites the config, so this is reachable only by a non-pharn edit.'
+  evidence: 'assertConfigUnchanged(cwd, snapshot, command)'
+- type: FINDING
+  rule_id: 'P7'
+  severity: minor
+  file: 'src/commands/update.ts'
+  problem: "A user who hand-edits pharn.config.json while `update`'s confirm is open is now refused (exit 1) instead of having the edit silently overwritten — intended, but user-visible."
+  evidence: "assertConfigUnchanged(cwd, config, 'update');"
+- type: FINDING
+  rule_id: 'P4'
+  severity: minor
+  file: 'CHANGELOG.md:8'
+  problem: "No CHANGELOG `[Unreleased]` entry (not in the plan's `## Files`)."
+  evidence: '## [Unreleased]'
+```
+
+## Verdict
+
+**GREEN** — 0 floor-gate findings, 3 advisory findings (minor). No lesson proposed for canon.

.dev/features/lock-read-modify-write/SHIP.md

@@ -0,0 +1,17 @@
+# SHIP — lock-read-modify-write
+
+Stages run, in order: `/pharn-dev-plan` → GATE 1 (human: **Approve as written**) → `/pharn-dev-grill` →
+`/pharn-dev-build` → `/pharn-dev-regress` → `/pharn-dev-verify` → `/pharn-dev-review` → GATE 2.
+
+| stage                | structural verdict (verbatim)                          |
+| -------------------- | ------------------------------------------------------ |
+| `/pharn-dev-build`   | `node .dev/floor/validate.mjs .` exit `0`              |
+| `/pharn-dev-regress` | `regression-report.json` `.verdict` = `no-regressions` |
+| `/pharn-dev-verify`  | `verify-report.json` `.verdict` = `PASS`               |
+
+- Review: [`REVIEW.md`](REVIEW.md) · Grill (advisory): [`GRILL.md`](GRILL.md)
+- Run ended at **GATE 2**. The human's standing instruction for this batch: open a PR and merge it only if
+  its CI checks are green.
+
+chain ran; the named floor verdicts are as shown — this is NOT a judgment that the increment is good or
+wise; that is the human's call at the post-review gate.

.dev/features/lock-read-modify-write/VERIFY.md

@@ -0,0 +1,27 @@
+# VERIFY — lock-read-modify-write
+
+## FLOOR layer (owns the verdict)
+
+Gates run over the whole repo with the feature present, as a non-root user on node 22 with the session
+proxy variables unset (as root with the proxy set, 5 pre-existing tests in `init.test.ts` /
+`update.test.ts` fail for environmental reasons, identically at the baseline).
+
+| gate           | exit |
+| -------------- | ---- |
+| `format:check` | 0    |
+| `lint`         | 0    |
+| `lint:md`      | 0    |
+| `test`         | 0    |
+| `typecheck`    | 0    |
+| `validate`     | 0    |
+
+No `structural:*` gate — the increment ships no eval-actual pair.
+
+**VERDICT: PASS** (`.dev/floor/check-verify.mjs`, `failing_gates: []`).
+
+## ADVISORY layer
+
+`node .dev/floor/count-verifiers.mjs .` → `{"registered":0,"verifiers":[]}` — floor gates only.
+
+Residual (P0/P7): "verified" means the named gates passed — not that the feature is correct in any sense
+the suite does not encode.

.dev/features/lock-read-modify-write/regression-report.json

@@ -0,0 +1,31 @@
+{
+  "base": "31f15cedcad80465396e2532f617f8afb60b3012",
+  "inside": [
+    "CLAUDE.md",
+    "README.md",
+    "src/commands/add.ts",
+    "src/commands/remove.ts",
+    "src/commands/update.ts",
+    "src/lib/pharn-config.ts",
+    "src/lib/project-lock.ts",
+    "tests/add.test.ts",
+    "tests/pharn-config.test.ts",
+    "tests/project-lock-commands.test.ts",
+    "tests/project-lock.test.ts",
+    "tests/remove.test.ts",
+    "tests/update.test.ts"
+  ],
+  "outside_gates": {
+    "tests": {
+      "base": 0,
+      "head": 0
+    },
+    "validate": {
+      "base": 0,
+      "head": 0
+    }
+  },
+  "regressions": [],
+  "pre_existing": [],
+  "verdict": "no-regressions"
+}

.dev/features/lock-read-modify-write/verify-report.json

@@ -0,0 +1,17 @@
+{
+  "feature": "lock-read-modify-write",
+  "gates": {
+    "format:check": 0,
+    "lint": 0,
+    "lint:md": 0,
+    "test": 0,
+    "typecheck": 0,
+    "validate": 0
+  },
+  "verdict": "PASS",
+  "failing_gates": [],
+  "verifiers": {
+    "registered": 0,
+    "findings": []
+  }
+}

.pharn/writes-scope.json

@@ -1,12 +1,7 @@
 {
   "scope": [
-    "src/lib/install-capabilities.ts",
-    "src/commands/remove.ts",
-    "tests/install-capabilities.test.ts",
-    "tests/remove.test.ts",
-    "README.md",
-    "CLAUDE.md"
+    ".dev/features/lock-read-modify-write/SHIP.md"
   ],
-  "set_by": ".dev/features/dest-symlink-guard-init-remove/PLAN.md",
-  "set_at": "2026-09-24T07:58:14.028Z"
+  "set_by": ".claude/commands/pharn-dev-ship.md",
+  "set_at": "2026-09-24T08:06:32.625Z"
 }