diff --git a/.dev/features/reinit-preserve-edits/GRILL.md b/.dev/features/reinit-preserve-edits/GRILL.md new file mode 100644 index 0000000..3db35af --- /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 0000000..1de58a4 --- /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 0000000..d5375bb --- /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 0000000..de7eaf3 --- /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 0000000..382836e --- /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 0000000..c0acf31 --- /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 0000000..ffd4cc2 --- /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 0000000..e573900 --- /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 164c0a3..9a69ffe 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 138da64..eef77ed 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 de989da..cb31b82 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 883fba5..701c8af 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 1c1b1e7..1841ded 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 29a5a49..6ef48f4 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 2718750..166c0e1 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 71ad2c0..60eea11 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 8179fef..c098cd4 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'); + }); +});