From e613d9dcf616fc29a40885780c71f4718c720cd5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 09:28:30 +0000 Subject: [PATCH] fix(init): a re-run init backs up your edits and keeps your manual adds (PHARN-11) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reproduced: init, `pharn add a11y`, edit pharn-spec.md, init again, confirm -> the prompt listed 10 of 417 paths, no backup was made, the edit was lost, and a11y vanished from the config while its files stayed. The CLI itself points users at `init` as a repair, so this is a reachable path. - The overwrite prompt lists the files that differ from upstream first, marked "(edited)", and says they will be backed up. - runInstallArchetype re-scans right before the copy and copies every edited file to .pharn-backup// (the scanDest + createBackup pair `add` uses), naming the directory at creation. - init reads the config it replaces tolerantly and carries over its `source: manual` capabilities that upstream still ships — installed again and recorded `manual`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TvcuVhk8hTeDskp5pAJhnc --- .dev/features/reinit-preserve-edits/GRILL.md | 29 ++++++ .dev/features/reinit-preserve-edits/PLAN.md | 73 ++++++++++++++ .../reinit-preserve-edits/REGRESSION.md | 30 ++++++ .dev/features/reinit-preserve-edits/REVIEW.md | 40 ++++++++ .dev/features/reinit-preserve-edits/SHIP.md | 17 ++++ .dev/features/reinit-preserve-edits/VERIFY.md | 27 ++++++ .../regression-report.json | 26 +++++ .../reinit-preserve-edits/verify-report.json | 17 ++++ .pharn/writes-scope.json | 6 +- CLAUDE.md | 2 +- docs/commands/init.md | 10 +- src/commands/init.ts | 76 ++++++++++++++- src/steps/install-archetype.ts | 48 +++++++++- src/steps/overwrite-check.ts | 31 +++++- tests/init-archetype.test.ts | 96 +++++++++++++++++++ tests/init.test.ts | 94 +++++++++++++++++- tests/overwrite-check.test.ts | 50 ++++++++++ 17 files changed, 654 insertions(+), 18 deletions(-) create mode 100644 .dev/features/reinit-preserve-edits/GRILL.md create mode 100644 .dev/features/reinit-preserve-edits/PLAN.md create mode 100644 .dev/features/reinit-preserve-edits/REGRESSION.md create mode 100644 .dev/features/reinit-preserve-edits/REVIEW.md create mode 100644 .dev/features/reinit-preserve-edits/SHIP.md create mode 100644 .dev/features/reinit-preserve-edits/VERIFY.md create mode 100644 .dev/features/reinit-preserve-edits/regression-report.json create mode 100644 .dev/features/reinit-preserve-edits/verify-report.json diff --git a/.dev/features/reinit-preserve-edits/GRILL.md b/.dev/features/reinit-preserve-edits/GRILL.md new file mode 100644 index 00000000..3db35afb --- /dev/null +++ b/.dev/features/reinit-preserve-edits/GRILL.md @@ -0,0 +1,29 @@ +# GRILL — reinit-preserve-edits + +Plan: `.dev/features/reinit-preserve-edits/PLAN.md` · spec-hash `bca940a5…d729d3c4e` matches live +`ARCHITECTURE.md`. Registered grillers: `{"registered":0,"grillers":[]}` → inline axes only. + +## Findings + +```yaml +- type: FINDING + rule_id: 'P3' + severity: minor + file: '.dev/features/reinit-preserve-edits/PLAN.md:32' + problem: 'Reading the previous config and deciding manual carry-over is a new concern for init.ts; it should stay a small, named helper so the command keeps one axis (orchestration).' + evidence: 'reads the existing config tolerantly' +- type: FINDING + rule_id: 'P5' + severity: important + file: '.dev/features/reinit-preserve-edits/PLAN.md:28' + problem: "The scan in the prompt and the scan before the copy run at different times; the backup must use its OWN scan (the one immediately before the write), not the prompt's, or an edit made while the prompt was open is lost." + evidence: 'before `installCapabilities`, `scanDest` over the expected install paths' +- type: FINDING + rule_id: 'P1' + severity: minor + file: '.dev/features/reinit-preserve-edits/PLAN.md:41' + problem: 'pharn.config.json itself is overwritten by re-init; it is not in the install manifest, so it is not backed up. Say so (the manual entries are carried over instead).' + evidence: 'copies those files to `.pharn-backup//`' +``` + +ADVISORY VERDICT: 3 concerns raised (0 blocking-severity, 3 advisory) — for the human to weigh before /pharn-dev-build. diff --git a/.dev/features/reinit-preserve-edits/PLAN.md b/.dev/features/reinit-preserve-edits/PLAN.md new file mode 100644 index 00000000..1de58a43 --- /dev/null +++ b/.dev/features/reinit-preserve-edits/PLAN.md @@ -0,0 +1,73 @@ +# PLAN — reinit-preserve-edits (PHARN-11: re-running `init` must back up edits and keep manual adds) + +- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4 +- increment: on a re-install over an existing project, `init` (1) lists the existing files that DIFFER + from upstream first in its overwrite prompt, saying they will be backed up; (2) copies those files to + `.pharn-backup//` (the same `scanDest` + `createBackup` `add` uses) before the first write, naming + the directory at creation; and (3) keeps every `source: 'manual'` capability of a readable existing + archetype config that is still in the fetched index — installing it and recording it as `manual` — + instead of rewriting the config from archetype resolution alone. +- layer(s): the CLI itself (`src/steps/overwrite-check.ts`, `src/steps/install-archetype.ts`, `src/commands/init.ts`) +- constitution_refs: [P0, P1, P3, P4, P5, P7] + +## Discovery — verified this run (P6) + +Reproduced (`repro/reinit`): `init`, `add a11y`, edit `pharn-spec.md`, `init` again, confirm → the +prompt shows 10 of 417 paths ("…and 407 more"), no `.pharn-backup/` is made, the edit is lost, and +`a11y` drops out of the config while its files stay. The CLI itself points users at `init` as the +repair (`LEGACY_CONFIG_MESSAGE`, `ConfigParseError`'s last resort). `steps/overwrite-check.ts` caps the +list at `MAX_LISTED = 10` with no edit classification; `steps/install-archetype.ts` builds the config from +scratch, every entry `source: 'auto'`. `lib/dest-drift.ts` `scanDest` + `lib/backup.ts` `createBackup` are +the existing primitives `add` already uses for exactly this. + +## Files + +- `src/steps/overwrite-check.ts` — `confirmWriteTargets` runs `scanDest` over the conflicting install + paths: the drifted ones are listed first under a heading saying they differ from upstream and will be + backed up to `.pharn-backup/` before being overwritten; the rest follow; the cap applies to the total — layer CLI/step +- `src/steps/install-archetype.ts` — before `installCapabilities`, `scanDest` over the expected install + paths; a non-empty `drifted` (and an empty `unsafe` — a symlinked destination is refused by the install + pre-flight anyway) goes to `createBackup`, logged at creation; the config marks the capabilities named in + a new `manualKeys` argument `source: 'manual'` — layer CLI/step +- `src/commands/init.ts` — reads the existing config tolerantly (absent / unreadable / invalid / legacy → + none; `init` is the recovery path and must never be blocked by the file it replaces); its + `source: 'manual'` entries still present in the fetched index and not already selected are added to the + install selection (named in one info line) and passed as `manualKeys` — layer CLI/command +- `tests/overwrite-check.test.ts` — drifted paths listed first with the backup note; identical files are + not called edits +- `tests/init-archetype.test.ts` — re-init backs up an edited file (bytes preserved under + `.pharn-backup/`, pointer printed) and keeps a manual capability (installed, `source: 'manual'`); a + corrupt existing config does not block re-init; a manual entry gone from the index is not resurrected +- `tests/init.test.ts` — the `runInstallArchetype` call gains its `manualKeys` argument +- `docs/commands/init.md` — re-install behavior (P4) +- `CLAUDE.md` — init step 5 (P4) + +## Contracts satisfied + +- The product's three edit protections (init's prompt, update's skips, `--force`'s backup) — init's is no + longer a bare "overwrite?" but names the edits and backs them up, as `add` does. +- `merge-capabilities.ts` "sticky manual" semantics — now honoured by `init` too. + +## Evals to write (P1) + +- listed above; backup, manual-keep and edit-first listing fail on the base source. + +## Guarantee audit (P0) + +- "an edited file overwritten by re-init is first copied to `.pharn-backup/`" → floor: sha256 inequality + (scanDest) + `createBackup` before `installCapabilities` (same primitives as `add`). +- "a manual capability survives re-init" → set membership over the existing config and the fetched index. +- Residual (named): `init` still overwrites after confirmation — the protection is the backup, not a skip. + +## Trust audit (P2) + +- The existing config is local, hand-editable input read through `readPharnConfig` (validated); failures + degrade to "no previous config", never to trusting unvalidated entries. + +## Determinism audit (P5) + +- Hash inequality, key membership; a failed read of the previous config → the documented fresh-install path. + +## Open questions (HALT) + +- none diff --git a/.dev/features/reinit-preserve-edits/REGRESSION.md b/.dev/features/reinit-preserve-edits/REGRESSION.md new file mode 100644 index 00000000..d5375bba --- /dev/null +++ b/.dev/features/reinit-preserve-edits/REGRESSION.md @@ -0,0 +1,30 @@ +# REGRESSION — reinit-preserve-edits + +The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment. + +## Base and partition + +- **base:** `770adad9a9213f43ced51c8e5b7ec3f22a2598e2` (`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`, `docs/commands/init.md`, `src/commands/init.ts`, `src/steps/install-archetype.ts`, `src/steps/overwrite-check.ts`, `tests/init-archetype.test.ts`, `tests/init.test.ts`, `tests/overwrite-check.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. diff --git a/.dev/features/reinit-preserve-edits/REVIEW.md b/.dev/features/reinit-preserve-edits/REVIEW.md new file mode 100644 index 00000000..de7eaf31 --- /dev/null +++ b/.dev/features/reinit-preserve-edits/REVIEW.md @@ -0,0 +1,40 @@ +# REVIEW — reinit-preserve-edits + +Floor first: `node .dev/floor/validate.mjs .` → exit 0 (GREEN). Everything below is **advisory**. + +## Floor-gate findings (blocking) + +None. + +- **L-floor (P0):** "an edited file is copied before re-init overwrites it" reduces to sha256 inequality + (`scanDest`) + `createBackup` before `installCapabilities` — the backup takes its OWN scan right before + the copy (grill #2), not the prompt's. "A manual capability survives re-init" reduces to key membership + over the previous config and the fetched index. +- **L-eval (P1):** 4 cases fail on the base source (edited-first prompt, backup + pointer, manual + recorded, manual carried by the command); controls: no backup when nothing was edited, no "(edited)" + when byte-identical, corrupt previous config does not block init. Coverage 97.14% (gate 97%). +- **L-trust (P2):** the previous config is read through `readPharnConfig` (validated); any failure + degrades to "nothing carried over". +- **L-axis (P3):** the carry-over is one named helper in `init.ts`; the backup lives in the install step + it protects; the prompt only reorders and annotates. + +## Advisory findings + +```yaml +- type: FINDING + rule_id: 'P7' + severity: minor + file: 'src/steps/install-archetype.ts' + problem: "pharn.config.json itself is not in the install manifest, so it is not backed up; its manual entries are carried over instead, but a hand-edited models/seam block is still reset to defaults by re-init (pre-existing, and named in ConfigParseError's message)." + evidence: 'models: DEFAULT_MODEL_ROUTING,' +- 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, 2 advisory findings. No lesson proposed for canon. diff --git a/.dev/features/reinit-preserve-edits/SHIP.md b/.dev/features/reinit-preserve-edits/SHIP.md new file mode 100644 index 00000000..382836ec --- /dev/null +++ b/.dev/features/reinit-preserve-edits/SHIP.md @@ -0,0 +1,17 @@ +# SHIP — reinit-preserve-edits + +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. diff --git a/.dev/features/reinit-preserve-edits/VERIFY.md b/.dev/features/reinit-preserve-edits/VERIFY.md new file mode 100644 index 00000000..c0acf311 --- /dev/null +++ b/.dev/features/reinit-preserve-edits/VERIFY.md @@ -0,0 +1,27 @@ +# VERIFY — reinit-preserve-edits + +## 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. diff --git a/.dev/features/reinit-preserve-edits/regression-report.json b/.dev/features/reinit-preserve-edits/regression-report.json new file mode 100644 index 00000000..ffd4cc2d --- /dev/null +++ b/.dev/features/reinit-preserve-edits/regression-report.json @@ -0,0 +1,26 @@ +{ + "base": "770adad9a9213f43ced51c8e5b7ec3f22a2598e2", + "inside": [ + "CLAUDE.md", + "docs/commands/init.md", + "src/commands/init.ts", + "src/steps/install-archetype.ts", + "src/steps/overwrite-check.ts", + "tests/init-archetype.test.ts", + "tests/init.test.ts", + "tests/overwrite-check.test.ts" + ], + "outside_gates": { + "tests": { + "base": 0, + "head": 0 + }, + "validate": { + "base": 0, + "head": 0 + } + }, + "regressions": [], + "pre_existing": [], + "verdict": "no-regressions" +} diff --git a/.dev/features/reinit-preserve-edits/verify-report.json b/.dev/features/reinit-preserve-edits/verify-report.json new file mode 100644 index 00000000..e573900f --- /dev/null +++ b/.dev/features/reinit-preserve-edits/verify-report.json @@ -0,0 +1,17 @@ +{ + "feature": "reinit-preserve-edits", + "gates": { + "format:check": 0, + "lint": 0, + "lint:md": 0, + "test": 0, + "typecheck": 0, + "validate": 0 + }, + "verdict": "PASS", + "failing_gates": [], + "verifiers": { + "registered": 0, + "findings": [] + } +} diff --git a/.pharn/writes-scope.json b/.pharn/writes-scope.json index 164c0a3b..9a69ffe1 100644 --- a/.pharn/writes-scope.json +++ b/.pharn/writes-scope.json @@ -1,7 +1,7 @@ { "scope": [ - ".dev/features/interrupt-exit-code/REVIEW.md" + ".dev/features/reinit-preserve-edits/SHIP.md" ], - "set_by": ".claude/commands/pharn-dev-review.md", - "set_at": "2026-09-24T08:58:56.214Z" + "set_by": ".claude/commands/pharn-dev-ship.md", + "set_at": "2026-09-24T09:28:29.312Z" } diff --git a/CLAUDE.md b/CLAUDE.md index 138da640..eef77edb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,7 +51,7 @@ ESM-only (`"type": "module"`, NodeNext). **Relative imports must use `.js` exten 2. `detectArchetypesFromProject` (`lib/detect-archetype.ts`) — merges `package.json` dependency names + a bounded, symlink-safe file-tree walk into an `Archetype[]` (`ssr`/`backend`/`spa`/`lib`). 3. `fetchRepo` (`lib/repo.ts`) — ONE SHA resolve, then download pharn-oss's tarball from codeload and extract it with `lib/tar-extract.ts` into a temp dir (cleaned up in a `finally`; every `process.exit`/`cancelAndExit` happens AFTER it). No fetch dependency, no `git` binary, no cache. 4. `parseCapabilityIndex` (`lib/capability-index.ts`) + `resolveCapabilities` (`lib/resolve-capabilities.ts`) — select capabilities whose `applies` is `universal` or intersects the detected archetypes; skip the rest with a reason. -5. `runArchetypeSummary` (selected + skipped) → `install` / `cancel`; on `install`, `confirmWriteTargets` (`steps/overwrite-check.ts`) warns if any of the install's actual write targets already exist in the cwd — listing them (capped), default No, silent when none; it subsumes the old `confirmOverwriteIfExists` (its set includes `pharn.config.json`) and derives the target set from `lib/install-manifest.ts` (the shared install manifest, also used by `status`'s `diff.ts`). Then `runInstallArchetype` (`steps/install-archetype.ts` → `lib/install-capabilities.ts`) copies the capabilities + fixed product surfaces (including the layout-resolved product-loop boundary contract (`resolveFeaturesReadme`)) into the mirrored layout (flat OR `pharn/`) and writes the archetype `pharn.config.json` (`archetypes`, `capabilities`, `layout`, `skillsVersion` read from the fetched `SKILLS_VERSION` file via `readSkillsVersion`, `modules: []`, plus `models`/`seam` defaults; canonical `CONSTITUTION.md` copied verbatim). The `--archetype` CLI flag is a retained no-op alias for one release. +5. `runArchetypeSummary` (selected + skipped) → `install` / `cancel`; on `install`, `confirmWriteTargets` (`steps/overwrite-check.ts`) warns if any of the install's actual write targets already exist in the cwd — listing them (capped), default No, silent when none; it subsumes the old `confirmOverwriteIfExists` (its set includes `pharn.config.json`) and derives the target set from `lib/install-manifest.ts` (the shared install manifest, also used by `status`'s `diff.ts`). On a re-install it lists the files that differ from upstream (`scanDest`) first, marked `(edited)`; `runInstallArchetype` re-scans immediately before the copy and backs those up with `createBackup` (the same pair `add` uses), naming the directory at creation; and `init` reads the config it replaces TOLERANTLY (any failure → nothing carried over) to keep its `source: 'manual'` entries that the index still has — installed again and recorded `manual` via `manualKeys`. Then `runInstallArchetype` (`steps/install-archetype.ts` → `lib/install-capabilities.ts`) copies the capabilities + fixed product surfaces (including the layout-resolved product-loop boundary contract (`resolveFeaturesReadme`)) into the mirrored layout (flat OR `pharn/`) and writes the archetype `pharn.config.json` (`archetypes`, `capabilities`, `layout`, `skillsVersion` read from the fetched `SKILLS_VERSION` file via `readSkillsVersion`, `modules: []`, plus `models`/`seam` defaults; canonical `CONSTITUTION.md` copied verbatim). The `--archetype` CLI flag is a retained no-op alias for one release. **`lib/install-capabilities.ts`** is the shared capability copy core — `installCapabilityDirs` (`add`) and `installCapabilities` (`init`); `update` applies the manifest per file instead. `installCapabilityDirs(repoDir, projectRoot, capabilities, paths?)` pre-flights **every** selected capability source (validated name via `CAPABILITY_NAME_RE` + `safeJoin` + existence + symlink rejection) before any write — no partial installs — then copies each griller/lens dir into the mirrored layout. `installCapabilities` additionally copies the fixed product surfaces: product `pharn-*` commands (excluding `pharn-dev-*`), `.cjs` hooks (excluding `*.test.cjs`), `settings.json` (**never** overwritten), the trusted docs, the layout-resolved product-loop boundary contract (`resolveFeaturesReadme`) (layout-invariant; guarded by `findSymlinkComponent`, not just the leaf `isSymlink`, since it is the one root-relative copy with an intermediate directory), `pharn-contracts/`, and `.dev/floor/` minus test files and its `test-fixtures/` subtree (a floor-RELATIVE segment match, single-sourced as `FLOOR_TEST_FIXTURES_DIR` and anchored at the floor root on both sides — an unanchored absolute-path match would prune the whole floor copy under an ancestor of that name). Copying from the untrusted clone is symlink-guarded (`isSymlink` reject / `noSymlinks` filter) and `safeJoin`-contained; file contents are copied verbatim, never executed. The DESTINATION is guarded too: before the first write, `installCapabilities` walks every path the install manifest (`collectExpectedInstallPaths`) says it writes, plus `.claude/settings.json`, with `findSymlinkComponent` and refuses the whole install naming each symlinked component (`cpSync` follows a symlinked `.claude/commands` or `pharn/` out of the project, and `safeJoin` is lexical). The install set is resolved by `lib/capability-index.ts` (`parseCapabilityIndex` — the untrusted-frontmatter → typed `CapabilityIndex` fetch boundary, reading only `name`/`role`/`applies` via a strict field reader) + `lib/resolve-capabilities.ts` (select where `applies` is `universal` or intersects the detected archetypes). The commit SHA is threaded from `fetchRepo` (`repo.sha`) — no separate GitHub fetch (closes the resolve/fetch TOCTOU). diff --git a/docs/commands/init.md b/docs/commands/init.md index de989dae..cb31b82e 100644 --- a/docs/commands/init.md +++ b/docs/commands/init.md @@ -128,7 +128,15 @@ Lists the **selected** capabilities (name, role, and why — `universal` or the | Yes, install | Copy the capabilities + product surfaces and write config | | Cancel | Exit 0; nothing written | -After you choose **install**, `init` checks which of its **actual write targets** (the selected capability dirs, product `pharn-*` commands, `.cjs` hooks, `pharn/features/README.md`, the contracts, core and floor dirs, the trusted docs, pharn's `LICENSE` copy, and `pharn.config.json`) already exist in your project. If any do, it lists them (capped at 10, then "…and N more") and asks you to confirm before overwriting — default **no**. When `pharn.config.json` is one of them, the warning also names the `skillsVersion` your existing config records, so you can see which version you are about to replace (read locally, never fetched; the clause is simply omitted if that file cannot be read). If none do, there is no prompt (zero friction). `.claude/settings.json` is never overwritten, so it is excluded from the check. The target set is derived from the fetched clone's layout + your resolved selection (`lib/install-manifest.ts`), so it is exact — not a git-history heuristic. +After you choose **install**, `init` checks which of its **actual write targets** (the selected capability dirs, product `pharn-*` commands, `.cjs` hooks, `pharn/features/README.md`, the contracts, core and floor dirs, the trusted docs, pharn's `LICENSE` copy, and `pharn.config.json`) already exist in your project. If any do, it lists them (capped at 10, then "…and N more") and asks you to confirm before overwriting — default **no**. When `pharn.config.json` is one of them, the warning also names the `skillsVersion` your existing config records, so you can see which version you are about to replace (read locally, never fetched; the clause is simply omitted if that file cannot be read). If none do, there is no prompt (zero friction). `.claude/settings.json` is never overwritten, so it is excluded from the check. + +**Re-running `init` over an existing install.** Existing files whose bytes **differ from upstream** — your +edits — are listed **first** and marked `(edited)`, and the prompt says how many there are. If you +continue, `init` copies each of them to `.pharn-backup//` **before** the first write and prints +that directory as soon as it is created (byte-identical files are not edits and are not backed up). +Capabilities you added by hand with `pharn add` (`source: "manual"` in `pharn.config.json`) are **kept**: +`init` installs them again and records them as `manual`, as long as upstream still ships them. An +unreadable or invalid existing config never blocks `init` — nothing is carried over from it. The target set is derived from the fetched clone's layout + your resolved selection (`lib/install-manifest.ts`), so it is exact — not a git-history heuristic. ### 7. Install diff --git a/src/commands/init.ts b/src/commands/init.ts index 883fba50..701c8af5 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -23,6 +23,12 @@ import { runGitPrereq } from '../steps/prereqs.js'; import { confirmWriteTargets } from '../steps/overwrite-check.js'; import { runArchetypeSummary } from '../steps/archetype-summary.js'; import { runInstallArchetype } from '../steps/install-archetype.js'; +import { readPharnConfig } from '../lib/pharn-config.js'; +import type { + CapabilityIndex, + InstalledCapability, + Selection, +} from '../types.js'; export async function runInit(): Promise { showBanner(); @@ -124,7 +130,30 @@ async function runInitArchetype(): Promise { // immediately after the parse and BEFORE the summary the user acts on. const unknownWarning = unknownCapabilitiesWarning(index.unknown); if (unknownWarning) log.warn(unknownWarning); - const selection = resolveCapabilities(archetypes, index); + const resolved = resolveCapabilities(archetypes, index); + // A re-run init must not silently drop what the user added by hand + // (`pharn add` → `source: 'manual'`): carry those entries over, install + // them with the rest, and keep their provenance. + const manual = carriedManualCapabilities(cwd, index, resolved); + if (manual.length) { + log.info( + `Keeping ${manual.length} capabilit${manual.length === 1 ? 'y' : 'ies'} you added by hand: ${manual.map((c) => `${c.role}:${c.name}`).join(', ')}.`, + ); + } + const selection: Selection = manual.length + ? { + ...resolved, + selected: [ + ...resolved.selected, + ...manual.map((c) => ({ + name: c.name, + role: c.role, + matched: [], + })), + ], + } + : resolved; + const manualKeys = new Set(manual.map((c) => `${c.role}:${c.name}`)); const action = await runArchetypeSummary(archetypes, selection); // Both prompts return a VALUE and neither exits, so `outcome` stays @@ -166,7 +195,14 @@ async function runInitArchetype(): Promise { // the bootstrap command, run once, interactively, and it is where a // concurrent-writer collision is least likely. await withProjectLock(cwd, 'init', () => - runInstallArchetype(repo.dir, cwd, archetypes, selection, commit), + runInstallArchetype( + repo.dir, + cwd, + archetypes, + selection, + commit, + manualKeys, + ), ); outcome = 'installed'; } @@ -193,3 +229,39 @@ async function runInitArchetype(): Promise { } if (outcome === 'cancelled') cancelAndExit(); } + +/** + * The `source: 'manual'` capabilities of the config a re-run init is about to + * replace that the fetched index still has and archetype resolution did not + * already select. Read TOLERANTLY: init is the command every other one points + * at for recovery, so an absent, unreadable, invalid or pre-archetype config + * simply means "nothing to carry over" — never a refusal. A manual entry the + * index no longer has is not resurrected (the `dropped-gone` rule `update` + * applies in lib/merge-capabilities.ts). + */ +function carriedManualCapabilities( + cwd: string, + index: CapabilityIndex, + resolved: Selection, +): InstalledCapability[] { + let previous: InstalledCapability[]; + try { + previous = readPharnConfig(cwd)?.capabilities ?? []; + } catch { + return []; + } + const key = (c: { name: string; role: string }): string => + `${c.role}:${c.name}`; + const inIndex = new Set(index.capabilities.map(key)); + const selected = new Set(resolved.selected.map(key)); + const seen = new Set(); + return previous.filter((c) => { + const k = key(c); + if (c.source !== 'manual' || !inIndex.has(k) || selected.has(k)) { + return false; + } + if (seen.has(k)) return false; + seen.add(k); + return true; + }); +} diff --git a/src/steps/install-archetype.ts b/src/steps/install-archetype.ts index 1c1b1e7a..1841ded8 100644 --- a/src/steps/install-archetype.ts +++ b/src/steps/install-archetype.ts @@ -3,7 +3,9 @@ import pc from 'picocolors'; import { FIRST_FEATURE_COMMAND, REPO_URL } from '../lib/constants.js'; import { installCapabilities } from '../lib/install-capabilities.js'; import { collectExpectedInstallPaths } from '../lib/install-manifest.js'; -import { layoutPaths } from '../lib/layout.js'; +import { detectLayout, layoutPaths } from '../lib/layout.js'; +import { scanDest } from '../lib/dest-drift.js'; +import { createBackup } from '../lib/backup.js'; import { buildRecords, writeRecords } from '../lib/install-records.js'; import { DEFAULT_MODEL_ROUTING } from '../lib/model-routing.js'; import { formatModelRoutingLines } from '../lib/model-routing-format.js'; @@ -30,9 +32,39 @@ export async function runInstallArchetype( archetypes: Archetype[], selection: Selection, commit: string | null, + // `role:name` keys of capabilities the user added by hand in the config this + // install replaces (commands/init.ts carries them over). Recorded `manual`, + // so `pharn update` keeps them — every other entry is `auto`. + manualKeys: ReadonlySet = new Set(), ): Promise { const startedAt = Date.now(); + // Back up every existing file this install is about to overwrite whose bytes + // DIFFER from upstream — a re-run `init` used to discard local edits with no + // copy anywhere. The same scan + backup `pharn add` uses (lib/dest-drift.ts, + // lib/backup.ts), taken HERE, immediately before the copy, not at the prompt: + // an edit made while the prompt was open must be covered too. Byte-identical + // files are not edits. A path through a symlinked directory is left out — the + // install's own pre-flight refuses the whole run over it (install-capabilities.ts). + const scan = scanDest({ + repoDir, + projectRoot: cwd, + rels: [ + ...collectExpectedInstallPaths({ + repoDir, + capabilities: selection.selected, + layout: detectLayout(repoDir), + }).keys(), + ], + }); + if (scan.drifted.length > 0 && scan.unsafe.length === 0) { + const backupDir = createBackup(cwd, scan.drifted); + // Named at creation, so the pointer survives anything that fails after it. + log.info( + `Backed up ${scan.drifted.length} edited file(s) to ${backupDir} before overwriting.`, + ); + } + const s = spinner(); s.start('Installing capabilities'); let capabilities: InstalledCapability[]; @@ -94,12 +126,18 @@ export async function runInstallArchetype( // Seam-resolution policy, written on every fresh install (P7 — additive). seam: DEFAULT_SEAM_CONFIG, archetypes, - // Every entry a fresh install writes came from archetype resolution, so it is - // `auto` — `pharn update` owns it and may drop it when the archetypes stop - // selecting it. `pharn add` is the only thing that writes `manual`. Tagged at + // An entry from archetype resolution is `auto` — `pharn update` owns it and + // may drop it when the archetypes stop selecting it. `manual` is what + // `pharn add` writes, and what a re-run init carries over from the config it + // replaces (`manualKeys`), so a hand-added capability survives. Tagged at // the WRITE site so lib/install-capabilities.ts (the copy routine) stays // unaware of provenance, which is not its axis (P3). - capabilities: capabilities.map((c) => ({ ...c, source: 'auto' as const })), + capabilities: capabilities.map((c) => ({ + ...c, + source: manualKeys.has(`${c.role}:${c.name}`) + ? ('manual' as const) + : ('auto' as const), + })), // The layout mirrored from the fetched clone (flat OR pharn/) — status/remove // read this back to address the project the same way (lib/layout.ts). layout, diff --git a/src/steps/overwrite-check.ts b/src/steps/overwrite-check.ts index 29a5a497..6ef48f42 100644 --- a/src/steps/overwrite-check.ts +++ b/src/steps/overwrite-check.ts @@ -5,6 +5,8 @@ import { PHARN_CONFIG_FILE, } from '../lib/install-manifest.js'; import { detectLayout } from '../lib/layout.js'; +import { scanDest } from '../lib/dest-drift.js'; +import { BACKUP_DIR } from '../lib/backup.js'; import { safeJoin, VERSION_RE } from '../lib/validate.js'; import type { Selection } from '../types.js'; @@ -103,10 +105,31 @@ export async function confirmWriteTargets( }); if (conflicts.length === 0) return 'proceed'; // zero friction — nothing to overwrite - const shown = conflicts.slice(0, MAX_LISTED); - const more = conflicts.length - shown.length; - const list = shown.map((p) => ` • ${p}`).join('\n'); + // Which of those differ from upstream — the user's EDITS. They are listed + // first (a 400-path list capped at 10 used to hide them) and named as backed + // up: runInstallArchetype copies them to .pharn-backup/ before the first + // write, re-scanning then rather than trusting this snapshot. + const drifted = new Set( + scanDest({ + repoDir, + projectRoot: cwd, + rels: conflicts.filter((p) => p !== PHARN_CONFIG_FILE), + }).drifted, + ); + const ordered = [ + ...conflicts.filter((p) => drifted.has(p)), + ...conflicts.filter((p) => !drifted.has(p)), + ]; + const shown = ordered.slice(0, MAX_LISTED); + const more = ordered.length - shown.length; + const list = shown + .map((p) => (drifted.has(p) ? ` • ${p} (edited)` : ` • ${p}`)) + .join('\n'); const tail = more > 0 ? `\n …and ${more} more` : ''; + const editsLine = + drifted.size > 0 + ? `\n${drifted.size} of them differ from upstream (your edits) and will be copied to ${BACKUP_DIR}/ before being overwritten.` + : ''; // Only when the config itself is at risk — i.e. a re-install over an existing // one — and only AFTER the zero-conflict return above, so a conflict-free // project still reaches none of this (P5: the branch stays `conflicts.length @@ -122,7 +145,7 @@ export async function confirmWriteTargets( // steps/archetype-summary.ts: every helper there ends in cancelAndExit, and // this stage's whole contract is that it does not exit. log.warn( - `${intro} These paths already exist and may be overwritten:\n${list}${tail}`, + `${intro} These paths already exist and may be overwritten:\n${list}${tail}${editsLine}`, ); const result = await confirm({ message: 'Continue and overwrite?', diff --git a/tests/init-archetype.test.ts b/tests/init-archetype.test.ts index 2718750f..166c0e1c 100644 --- a/tests/init-archetype.test.ts +++ b/tests/init-archetype.test.ts @@ -1,6 +1,7 @@ import { existsSync, mkdirSync, + readdirSync, readFileSync, rmSync, writeFileSync, @@ -387,3 +388,98 @@ describe('archetype install (fixture e2e)', () => { expect(warned).not.toContain('not installed'); }); }); + +// PHARN-11: re-running `init` over an existing install used to overwrite local +// edits with no copy anywhere, and rewrote the config from archetype +// resolution alone — dropping every capability the user had added by hand. +describe('re-running init over an existing install (PHARN-11)', () => { + const tmp = useTmpDir(); + + async function firstInstall(repo: string, proj: string) { + scaffoldRepo(repo); + write( + join(proj, 'package.json'), + JSON.stringify({ dependencies: { next: '14.0.0' } }), + ); + const { archetypes } = detectArchetypesFromProject(proj); + const index = parseCapabilityIndex(repo); + const selection = resolveCapabilities(archetypes, index); + await runInstallArchetype(repo, proj, archetypes, selection, 'sha123'); + return { archetypes, selection }; + } + + it('copies an edited file to .pharn-backup/ before overwriting it, and names the directory', async () => { + const repo = join(tmp.path(), 'repo'); + const proj = join(tmp.path(), 'proj'); + const { archetypes, selection } = await firstInstall(repo, proj); + write(join(proj, '.claude/commands/pharn-plan.md'), 'MY LOCAL EDIT'); + vi.mocked(prompts.log.info).mockClear(); + + await runInstallArchetype(repo, proj, archetypes, selection, 'sha123'); + + const backups = readdirSync(join(proj, '.pharn-backup')); + expect(backups).toHaveLength(1); + expect( + readFileSync( + join( + proj, + '.pharn-backup', + backups[0]!, + '.claude/commands/pharn-plan.md', + ), + 'utf8', + ), + ).toBe('MY LOCAL EDIT'); + expect( + readFileSync(join(proj, '.claude/commands/pharn-plan.md'), 'utf8'), + ).toBe('plan'); + const info = vi + .mocked(prompts.log.info) + .mock.calls.map((c) => String(c[0])) + .join('\n'); + expect(info).toContain(`.pharn-backup/${backups[0]!}`); + }); + + it('makes NO backup when nothing was edited (byte-identical is not an edit)', async () => { + const repo = join(tmp.path(), 'repo'); + const proj = join(tmp.path(), 'proj'); + const { archetypes, selection } = await firstInstall(repo, proj); + + await runInstallArchetype(repo, proj, archetypes, selection, 'sha123'); + + expect(existsSync(join(proj, '.pharn-backup'))).toBe(false); + }); + + it('records the carried-over manual capabilities as `manual`', async () => { + const repo = join(tmp.path(), 'repo'); + const proj = join(tmp.path(), 'proj'); + const { archetypes, selection } = await firstInstall(repo, proj); + const withManual = { + ...selection, + selected: [ + ...selection.selected, + { name: 'path-traversal', role: 'lens' as const, matched: [] }, + ], + }; + + await runInstallArchetype( + repo, + proj, + archetypes, + withManual, + 'sha123', + new Set(['lens:path-traversal']), + ); + + const caps = readPharnConfig(proj)!.capabilities!; + expect(caps).toContainEqual({ + name: 'path-traversal', + role: 'lens', + source: 'manual', + }); + expect(caps.filter((c) => c.source === 'manual')).toHaveLength(1); + expect( + existsSync(join(proj, 'pharn-review/path-traversal/path-traversal.md')), + ).toBe(true); + }); +}); diff --git a/tests/init.test.ts b/tests/init.test.ts index 71ad2c06..60eea11e 100644 --- a/tests/init.test.ts +++ b/tests/init.test.ts @@ -1,8 +1,14 @@ -import { readFileSync, readdirSync } from 'node:fs'; +import { readFileSync, readdirSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { ProcessExit, restoreTTY, setTTY, stubProcessExit } from './helpers.js'; +import { + ProcessExit, + restoreTTY, + setTTY, + stubProcessExit, + useTmpDir, +} from './helpers.js'; import type { CapabilityIndex } from '../src/types.js'; // Archetype is now the DEFAULT (and only) init flow. runInit() drives it with no @@ -95,6 +101,89 @@ describe('runInit (archetype default)', () => { .mock.calls.map((c) => String(c[0])) .join('\n'); + // PHARN-11: a re-run init carries the user's hand-added capabilities over + // (still in the index, not already selected) instead of dropping them. + describe('carrying manual capabilities over', () => { + const tmp = useTmpDir(); + const writeConfig = (dir: string, value: unknown) => + writeFileSync(join(dir, 'pharn.config.json'), JSON.stringify(value)); + const baseConfig = { + pharnVersion: '0.5.0', + skillsVersion: '1.0.0', + repo: 'pharn-dev/pharn-oss', + commit: null, + modules: [], + installedAt: '2026-09-24T00:00:00.000Z', + archetypes: ['ssr'], + }; + + beforeEach(() => { + runArchetypeSummary.mockResolvedValue('install'); + confirmWriteTargets.mockResolvedValue('proceed'); + parseCapabilityIndex.mockReturnValue({ + capabilities: [ + { name: 'a11y', role: 'griller', applies: 'universal' }, + { name: 'path-traversal', role: 'lens', applies: ['backend'] }, + ], + unknown: [], + }); + resolveCapabilities.mockReturnValue({ + selected: [{ name: 'a11y', role: 'griller', matched: 'universal' }], + skipped: [], + } as never); + }); + afterEach(() => { + // Implementations survive vi.clearAllMocks — reset what this block set so + // the rest of the file sees its defaults. + resolveCapabilities.mockReturnValue({ selected: [], skipped: [] }); + vi.mocked(process.cwd).mockRestore(); + }); + + it('installs and flags a manual entry that the index still has', async () => { + const dir = tmp.path(); + vi.spyOn(process, 'cwd').mockReturnValue(dir); + writeConfig(dir, { + ...baseConfig, + capabilities: [ + { name: 'a11y', role: 'griller', source: 'auto' }, + { name: 'path-traversal', role: 'lens', source: 'manual' }, + { name: 'gone-now', role: 'lens', source: 'manual' }, + ], + }); + + await runInit(); + + const [, , , selection, , manualKeys] = runInstallArchetype.mock + .calls[0] as unknown as [ + unknown, + unknown, + unknown, + { selected: { name: string }[] }, + unknown, + Set, + ]; + expect(selection.selected.map((c) => c.name)).toEqual([ + 'a11y', + 'path-traversal', + ]); + expect([...manualKeys]).toEqual(['lens:path-traversal']); + expect(informed()).toContain('lens:path-traversal'); + }); + + it('a corrupt existing config never blocks init — nothing is carried over', async () => { + const dir = tmp.path(); + vi.spyOn(process, 'cwd').mockReturnValue(dir); + writeFileSync(join(dir, 'pharn.config.json'), '{ not json'); + + await runInit(); + + expect(runInstallArchetype).toHaveBeenCalledTimes(1); + const manualKeys = runInstallArchetype.mock.calls[0]![5 as never] as + Set | undefined; + expect([...(manualKeys ?? [])]).toEqual([]); + }); + }); + it('drives the archetype flow and installs — no module/manifest fetch', async () => { runArchetypeSummary.mockResolvedValue('install'); confirmWriteTargets.mockResolvedValue('proceed'); @@ -122,6 +211,7 @@ describe('runInit (archetype default)', () => { ['ssr'], { selected: [], skipped: [] }, 'sha123', + new Set(), ); expect(cleanup).toHaveBeenCalledTimes(1); }); diff --git a/tests/overwrite-check.test.ts b/tests/overwrite-check.test.ts index 8179fef0..c098cd43 100644 --- a/tests/overwrite-check.test.ts +++ b/tests/overwrite-check.test.ts @@ -266,3 +266,53 @@ describe('confirmWriteTargets', () => { expect(lastWarning()).not.toContain('skills v'); }); }); + +// PHARN-11: with hundreds of existing paths capped at MAX_LISTED, the user's +// EDITS were buried in "…and N more". They are listed first, marked, and the +// prompt says they will be backed up before being overwritten. +describe('confirmWriteTargets — edited files first (PHARN-11)', () => { + const tmp = useTmpDir(); + const sel: Selection = { + selected: [ + { name: 'a11y', role: 'griller', matched: ['ssr'] }, + { name: 'n-plus-one', role: 'lens', matched: ['ssr'] }, + ], + skipped: [], + }; + + it('lists the edited file first, marked, and announces the backup', async () => { + const repo = join(tmp.path(), 'repo'); + const proj = join(tmp.path(), 'proj'); + scaffoldRepo(repo); + // Everything already installed, byte-identical… + installCapabilities(repo, proj, sel); + // …except one file the user edited. `z…` would sort LAST alphabetically. + write(join(proj, 'pharn-review/n-plus-one/n-plus-one.md'), 'MY EDIT'); + vi.mocked(prompts.confirm).mockResolvedValueOnce(false); + + await confirmWriteTargets(repo, proj, sel); + + const warning = lastWarning(); + const firstListed = warning + .split('\n') + .find((l) => l.trimStart().startsWith('•')); + expect(firstListed).toContain( + 'pharn-review/n-plus-one/n-plus-one.md (edited)', + ); + expect(warning).toContain('1 of them differ from upstream (your edits)'); + expect(warning).toContain('.pharn-backup/'); + }); + + it('calls nothing an edit when every existing file is byte-identical', async () => { + const repo = join(tmp.path(), 'repo'); + const proj = join(tmp.path(), 'proj'); + scaffoldRepo(repo); + installCapabilities(repo, proj, sel); + vi.mocked(prompts.confirm).mockResolvedValueOnce(false); + + await confirmWriteTargets(repo, proj, sel); + + expect(lastWarning()).not.toContain('(edited)'); + expect(lastWarning()).not.toContain('your edits'); + }); +});