pharn-dev · PrzemekGalarowicz · Sep 25, 2026 · Sep 25, 2026
@@ -0,0 +1,55 @@
+# GRILL — init-reinstall-safety
+
+Plan: `.dev/features/init-reinstall-safety/PLAN.md`. Spec hash recomputed:
+`bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e` — **matches**. Registered
+grillers: `{"registered":0,"grillers":[]}` → inline axes only. The plan is `trust: untrusted`;
+nothing in it read as an instruction.
+
+## Findings
+
+### Eval coverage (P1)
+
+```yaml
+- type: FINDING
+  rule_id: 'P1'
+  severity: important
+  file: '.dev/features/init-reinstall-safety/PLAN.md:104'
+  problem: 'tests/init.test.ts mocks the install step and the prompt, so a spy there only sees init.ts''s own call. The "once per init" claim spans init → prompt → install → pre-flight → records, which only a test running the REAL steps can count. That is init-archetype.test.ts, with the manifest module partially mocked to pass through while counting. Otherwise the F28 test passes while four of the five calls survive below it.'
+  evidence: '`tests/init.test.ts` — layer tests. The manifest builder runs once per init (spy;'
+```
+
+### Guarantee audit (P0)
+
+```yaml
+- type: FINDING
+  rule_id: 'P0'
+  severity: minor
+  file: '.dev/features/init-reinstall-safety/PLAN.md:83'
+  problem: 'The records store is keyed at the layout of the install that wrote it. A re-install whose clone changed layout (flat → pharn/) finds no record at the new paths, so every differing file is labelled "pharn has no record of them" and backed up. That is conservative and correct, but name it in the docs, so a layout migration''s long backup list is not read as a bug.'
+  evidence: 'records baseline (`carry.previousStamp`) → backup → copy (F20). The records keys'
+- type: FINDING
+  rule_id: 'P0'
+  severity: minor
+  file: '.dev/features/init-reinstall-safety/PLAN.md:86'
+  problem: 'The prompt classifies before the lock and the backup re-classifies inside it, so the two can disagree if the records store changes in between. The lock and init''s config-fingerprint re-check make that rare, and the backup is the one that acts: state that the backup scan is authoritative and the prompt''s labels are advisory.'
+  evidence: 'from the same classifier, worded per label: "changed since pharn wrote them"; "pharn'
+```
+
+### Checked, no finding
+
+- Determinism (P5): classification is `decideFileAction`, already table-tested for update. The
+  fallback with no usable store is "back it up", never a guess.
+- Trust (P2): source paths come from the manifest, already `safeJoin`-contained. Records are local,
+  stamp-checked data.
+- Axis (P3): the scan widens in `dest-drift.ts` (its owner), the pre-flight moves within
+  `install-capabilities.ts`, and the ordering changes in the step that owns it.
+
+## Summary
+
+The design follows update's decision table and closes each finding with a failing-on-base case.
+The important gap is test placement: the F28 count must be taken where the real steps run, or it
+proves nothing. Two documentation points follow from the design and should be written down: the
+layout-migration backups, and which scan is authoritative.
+
+**ADVISORY VERDICT: 3 concerns raised (0 blocking-severity, 1 important, 2 minor) — for the human to
+weigh before /pharn-dev-build.**
@@ -0,0 +1,167 @@
+# PLAN — init-reinstall-safety (a re-run init calls only YOUR edits "edited", backs up every one of them, refuses before it backs up, and computes its manifest once)
+
+- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4
+- increment: `init`'s install path, six findings.
+  - **Edited = changed since pharn wrote it (F9).** A re-install calls a file "edited", and backs it
+    up, only when `update` would have skipped it: by `pharn.records.json`, through update's own
+    `decideFileAction`, not "differs from upstream".
+  - **Scan the real source (F10).** The scan compares each destination with its real SOURCE, so
+    `PHARN-LICENSE` / `pharn/LICENSE` ← `LICENSE` is covered.
+  - **Refuse before the backup (F20).** Every pre-flight runs before the backup. A refused install
+    creates no `.pharn-backup/`.
+  - **Config and records paths (F19).** The type pre-flight also covers `pharn.config.json` and
+    `pharn.records.json`.
+  - **A live symlinked settings.json is fine (F8, init half).** init never writes an existing file
+    there, so only a dangling link is refused.
+  - **One manifest per run (F28).** It is computed once and passed down.
+- layer(s): the CLI itself (`src/commands/init.ts`, `src/steps/overwrite-check.ts`,
+  `src/steps/install-archetype.ts`, `src/lib/install-capabilities.ts`, `src/lib/dest-drift.ts`),
+  docs
+- constitution_refs: [P0, P1, P2, P5, P6]
+
+## Discovery — verified this run (P6), code read on HEAD `abb274a`
+
+- **F9.** `confirmWriteTargets` (`overwrite-check.ts:108-155`) labels `(edited)` the conflicts
+  whose bytes differ from the NEW upstream (`scanDest`). It says "N of them differ from upstream
+  (your edits)". `runInstallArchetype` (`install-archetype.ts:83-101`) backs up the same set. So
+  after any upstream bump, untouched pharn files are called your edits and copied to
+  `.pharn-backup/`. The review reproduced 14 of them with zero edits.
+  - `update` already has the right table: `decideFileAction` (`update-decision.ts:73+`). A disk
+    hash equal to the record is a clean upgrade with no backup. A modified, unrecorded or
+    unverifiable file gets `backup: true` under `force`, which is init's position, since init always
+    overwrites.
+  - init already reads the previous config's `(skillsVersion, commit)` as `carry.previousStamp`
+    (`install-archetype.ts:54`, `init.ts:345`).
+- **F10.** `collectExpectedInstallPaths` returns `Map<dest, sourcePath>`, and LICENSE is its one
+  entry where they differ (`install-manifest.ts:172-181`). Both scans pass only `.keys()`, and
+  `scanDest` (`dest-drift.ts`) joins the same `rel` on both sides. The clone has no `PHARN-LICENSE`,
+  so an edited copy is never compared, never backed up, and silently overwritten (reproduced by the
+  review, both layouts).
+- **F20.** The backup (`install-archetype.ts:83-101`) runs before `installCapabilities`. That
+  function's pre-flights (`install-capabilities.ts:160-171`: symlinked destinations, type
+  collisions) then refuse with "Nothing was written". The user sees "Backed up…", and each retry
+  adds `.pharn-backup/<ts>-2`, `-3`.
+- **F19.** `assertDestinationTypes` (`:384-412`) walks only manifest paths. A DIRECTORY at
+  `pharn.records.json` passes every check, with no prompt (records are not a conflict), so every file
+  is copied and then the records rename fails (EISDIR): no records and no config.
+  `pharn.config.json/` does the same after the confirm.
+- **F8, init half.** `assertDestinationsInProject` (`:342-375`) walks `.claude/settings.json` with
+  `findSymlinkComponent`, so a symlinked LEAF refuses the whole install. Its advice ("replace it
+  with a real directory") is also wrong for a file. Yet `settingsPreserved = existsSync(settingsTo)`
+  (`:196`) follows a live link, so an existing settings file is never written. Only a DANGLING leaf
+  would be written through (`cpSync` follows it and creates the target outside the project).
+- **F28.** `collectExpectedInstallPaths` runs 5× per init:
+  1. `conflictingWriteTargets` (`install-manifest.ts:228`);
+  2. the backup scan (`install-archetype.ts:88`);
+  3. `assertDestinationsInProject` (`install-capabilities.ts:349`);
+  4. `assertDestinationTypes` (`:392`);
+  5. the records keys (`install-archetype.ts:203`).
+
+  Every call returns the same map: it depends only on the clone, the selection and the layout, and
+  none of those change during a run. The re-scan HASHING (prompt time, then again before the copy)
+  is deliberate, because it catches an edit made while the prompt was open, and it stays.
+
+## Files
+
+- `src/lib/dest-drift.ts` — layer CLI/lib. Changes:
+  - The scan takes `(dest → source)` pairs instead of plain rels; `add` passes identity pairs, so
+    its behaviour stays as it is (F10).
+  - An optional records baseline: a differing file is "drifted" exactly when
+    `decideFileAction({…, force: true}).backup` holds — update's own table (F9). With no baseline,
+    today's rule (differs from upstream) stands.
+  - Each drifted file carries its label: modified, unrecorded, or unverifiable.
+- `src/lib/install-capabilities.ts` — layer CLI/lib. An exported pre-flight wraps the symlink and
+  type walks over a precomputed manifest:
+  - The type walk also covers `pharn.config.json` and `pharn.records.json`, which must never be a
+    directory (F19).
+  - The settings leaf is refused only when it is a DANGLING link (F8, GATE 1 answer 2 → a). A
+    symlinked `.claude/` is still refused, and the message distinguishes a linked file from a
+    linked directory.
+  - The install function (`installCapabilities`) accepts the precomputed manifest and still runs
+    the pre-flight itself.
+- `src/lib/install-manifest.ts` — layer CLI/lib. `conflictingWriteTargets` accepts an optional
+  precomputed manifest, so the prompt reuses init's one computation instead of building its own
+  (F28; amendment during build — the alternative was duplicating its logic in the prompt).
+- `src/steps/install-archetype.ts` — layer CLI/steps. Order becomes pre-flight → scan with the
+  records baseline (`carry.previousStamp`) → backup → copy (F20). The records keys come from the
+  same manifest (F28).
+- `src/steps/overwrite-check.ts` — layer CLI/steps. The `(edited)` marker and the edits line come
+  from the same classifier, worded per label: "changed since pharn wrote them"; "pharn has no
+  record of them"; and, with no usable store, "differ from upstream — pharn has no record to tell
+  your edits from upstream changes". A file equal to its record is a clean upgrade: never marked,
+  never backed up (GATE 1 answer 1 → a).
+- `src/commands/init.ts` — layer CLI/commands. Computes the manifest ONCE after resolution and
+  passes it to the prompt and the install (F28).
+- `tests/dest-drift.test.ts` — layer tests. A file equal to its record but not to upstream → not
+  drifted (FAILS on base); an unrecorded differing file → drifted, unrecorded; a dest≠src pair →
+  compared with its source (FAILS on base).
+- `tests/init-archetype.test.ts` — layer tests. An upstream bump with no edits → no `(edited)`, no
+  backup (FAILS on base); one real edit → exactly that file listed and backed up; an edited
+  `PHARN-LICENSE` (flat) or `pharn/LICENSE` → marked and backed up (FAILS on base); an edited file
+  plus a type collision → refused, no `.pharn-backup/`, no "Backed up" line (FAILS on base). The manifest builder runs
+  once per init, counted through the REAL steps with a pass-through spy (FAILS on base with 5;
+  grill finding 1).
+- `tests/install-capabilities.test.ts` — layer tests. A directory at `pharn.records.json` or
+  `pharn.config.json` → refused before the first write (FAILS on base); a live
+  `.claude/settings.json` symlink → the install proceeds and the link target stays untouched (FAILS
+  on base); a dangling one → refused, target never created (guard); a symlinked `.claude/` →
+  still refused (guard).
+- `tests/init.test.ts` — layer tests. Only the call-shape assertions change (the prompt and the
+  install now receive the manifest). The once-per-init count lives in init-archetype.test.ts,
+  where the real steps run (grill finding 1).
+- `tests/overwrite-check.test.ts` — layer tests. The three wordings.
+- `docs/commands/init.md` — layer docs. The re-install paragraph (what "edited" means, the
+  no-records fallback, LICENSE included) and the refusals (config/records directories; the
+  dangling `settings.json` link).
+- `docs/troubleshooting.md` — layer docs. A symlinked `.claude/settings.json` is fine and a dangling
+  one is refused; the directory-at-config/records refusal.
+- `CLAUDE.md` — layer docs. The init step-5 passage (prompt / scan / backup order) and the
+  `installCapabilities` DESTINATION-guard sentence (settings.json rule, config/records in the type
+  walk, manifest passed in).
+- `CHANGELOG.md` — `[Unreleased]` → `### Fixed`, one entry per finding
+
+## Contracts satisfied
+
+- `update`'s decision table (`lib/update-decision.ts`) becomes the single definition of "your
+  edit" for both commands (cited, P4).
+- PHARN-16's "no half-install": now true for the two CLI-owned files too.
+
+## Evals to write (P1)
+
+- Listed under Files. Eight cases FAIL on the base.
+
+## Guarantee audit (P0)
+
+- "a pristine pharn file is never called your edit when records are usable" → floor:
+  `decideFileAction` (already tested) plus the scan tests. Without usable records → advisory,
+  conservative (every difference is backed up), and the prompt says so.
+- "every file init overwrites that update would have skipped is backed up first, LICENSE included"
+  → floor: the pair scan plus tests.
+- "a refused install writes nothing, not even a backup" → floor: pre-flight before backup, plus a
+  test. Residual: a filesystem change between pre-flight and copy (TOCTOU), the same residual
+  `update` names.
+- "the manifest is computed once" → floor: a spy test (a performance claim, not a safety one).
+
+## Trust audit (P2)
+
+- The clone is untrusted. The scan joins source paths from the manifest, which are already
+  `safeJoin`-contained. Records are local, stamp-checked data (a stale or foreign store is ignored
+  → the conservative path). Nothing new is printed except CLI-owned wording and manifest paths,
+  as today.
+
+## Determinism audit (P5)
+
+- Hash equality and set membership through the existing decision table. The fallback with no usable
+  store is the conservative "back it up", never a guess.
+
+## Open questions (HALT)
+
+None open. Resolved at GATE 1 (human, 2026-09-25): every question below → **(a)**, the
+recommended answer. Kept for the record:
+
+1. A file that equals its record but differs from upstream (pharn's own bytes, just outdated).
+   (a) Treat it as a clean upgrade: no `(edited)`, no backup, exactly as `update` does —
+   recommended. (b) Keep backing it up, noisy but zero-risk.
+2. `.claude/settings.json` as a symlink during init. (a) A live link is allowed (it is preserved, as
+   today) and a dangling one is refused with a file-specific message — recommended. (b) Allow a
+   dangling link too, by skipping the settings write and warning.
@@ -0,0 +1,43 @@
+# REGRESSION — init-reinstall-safety
+
+The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment.
+
+## Base and partition
+
+- **base:** `5b63e313ed74c5d875039685227c68ea76c1ea67` (`HEAD` — `origin/main` after #222; the
+  build is an uncommitted working tree on top of it).
+- **inside** (each declared in `PLAN.md` `## Files`):
+  - `src/commands/init.ts`, `src/lib/dest-drift.ts`, `src/lib/install-capabilities.ts`,
+    `src/lib/install-manifest.ts`, `src/steps/install-archetype.ts`, `src/steps/overwrite-check.ts`
+  - `tests/dest-drift.test.ts`, `tests/init-archetype.test.ts`, `tests/init.test.ts`,
+    `tests/install-capabilities.test.ts`, `tests/overwrite-check.test.ts`
+  - `docs/commands/init.md`, `docs/troubleshooting.md`
+  - `CLAUDE.md`, `CHANGELOG.md`
+- **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 46 stdlib `*.test.mjs` / `*.test.cjs` files `scope` returned (754 tests) +
+  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.
+- **environment:** both sides ran with no proxy variables and as root **without**
+  `CAP_DAC_OVERRIDE` / `CAP_DAC_READ_SEARCH` / `CAP_FOWNER` (`setpriv`), the CI-equivalent of this
+  root sandbox.
+
+## 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.