diff --git a/.dev/features/init-preflight-first/GRILL.md b/.dev/features/init-preflight-first/GRILL.md new file mode 100644 index 0000000..985dbe3 --- /dev/null +++ b/.dev/features/init-preflight-first/GRILL.md @@ -0,0 +1,72 @@ +# GRILL — init-preflight-first + +Plan: `.dev/features/init-preflight-first/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. + +The plan's discovery was read on `9d2d574`; `main` is now `33861f1` (#225). #225 touched +`src/steps/install-archetype.ts` (`readCarriedEntries`) and `src/steps/overwrite-check.ts` +(`recordedSkillsVersion`) only in their config reads. The lines the plan cites (`init.ts:197-205`) +are unchanged. + +## Findings + +### Guarantee audit (P0) + +```yaml +- type: FINDING + rule_id: 'P0' + severity: minor + file: '.dev/features/init-preflight-first/PLAN.md:4' + problem: 'The claim is "refused without first asking", but the summary prompt (install / cancel) still comes before the refusal: the manifest is built only after an install answer, and a test pins that a cancelled summary builds none. That order is right (cancelling stays free), but the docs and CHANGELOG must say "before the overwrite prompt", never "before any prompt".' + evidence: 'so a project the install cannot finish in is refused without first asking "Continue and overwrite?"' +- type: FINDING + rule_id: 'P0' + severity: minor + file: '.dev/features/init-preflight-first/PLAN.md:26' + problem: 'The pre-flight will run three times per install: this new early run, the one under the lock, and the one in installCapabilities. Each is a read-only lstat walk over the manifest paths. Say why the later two stay (the lock-time one is the authoritative one; installCapabilities keeps it so no caller can copy without it), so nobody later removes one as a duplicate.' + evidence: 'a thin call to `prepareInstall` over the manifest init built' +``` + +### Eval coverage (P1) + +```yaml +- type: FINDING + rule_id: 'P1' + severity: minor + file: '.dev/features/init-preflight-first/PLAN.md:31' + problem: 'Pin the refusal''s report shape as the existing init failure cases do (the message on stderr, the clone cleaned up before the report). Also extend the cancelled-summary case: the pre-flight must not run when the summary is cancelled.' + evidence: 'A refusal exits 1 without calling `confirmWriteTargets` or `runInstallArchetype`, and still cleans up the clone.' +``` + +### Honest scope (P7) + +```yaml +- type: FINDING + rule_id: 'P7' + severity: minor + file: '.dev/features/init-preflight-first/PLAN.md:41' + problem: '`[Unreleased]` already has "A project `init` cannot finish installing into is refused before the first write." Since the CHANGELOG was restructured into net-since-0.5.0 entries, a second entry about the same refusal would split one change in two. Extend that entry instead.' + evidence: '`CHANGELOG.md` — `[Unreleased]` → `### Fixed`, one entry.' +``` + +### Checked, no finding + +- **Trust (P2).** No new input. The early run reads the same destination paths the lock-time run reads. +- **One axis (P3).** `preflightInstall` goes in the step module init already imports, the same way + `installManifest` does. That keeps `init.ts` clear of `*manifest.js` (the static guard) and of + `install-capabilities.js`. +- **Determinism (P5).** No new branch: the pre-flight's existing throw lands in init's existing + catch. It gets the same report, exit 1 and cleanup as when it throws under the lock today. +- **TOCTOU.** The plan says outright that the tree can change while the prompt is open, and that the + lock-time checks stay the authoritative ones. + +## Summary + +A small, well-scoped plan. Keep the claim to the overwrite prompt, not every prompt. Say why all +three pre-flight runs stay. Pin the report shape and the cancelled-summary case. Extend the existing +CHANGELOG entry rather than add a second one. + +**ADVISORY VERDICT: 4 concerns raised (0 blocking-severity, 0 important, 4 minor) — for the human to +weigh before /pharn-dev-build.** diff --git a/.dev/features/init-preflight-first/PLAN.md b/.dev/features/init-preflight-first/PLAN.md new file mode 100644 index 0000000..c7beb13 --- /dev/null +++ b/.dev/features/init-preflight-first/PLAN.md @@ -0,0 +1,68 @@ +# PLAN — init-preflight-first (refuse an unfinishable install before asking to overwrite) + +- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4 +- increment: `pharn init` runs its read-only destination pre-flight before the overwrite prompt, + so a project the install cannot finish in is refused without first asking "Continue and + overwrite?". The checks that already run under the lock stay; they remain the authoritative ones. +- layer(s): the CLI itself (`src/commands/init.ts`, `src/steps/install-archetype.ts`), docs +- constitution_refs: [P0, P5, P6] + +## Discovery — verified this run (P6), code read on HEAD `9d2d574` + +- `init.ts:197-205` builds the install manifest once (`installManifest`), then calls + `confirmWriteTargets`, and only after a yes calls `runInstallArchetype` under the lock. +- `runInstallArchetype` runs `prepareInstall` first (#223), and `installCapabilities` runs it again + just before copying. Both are after the prompt, so a user who answers yes can still be told + "Refusing to install: … is in the way" — the question was never real. +- `prepareInstall` (`install-capabilities.ts`) is read-only and throws `ManifestValidationError`, + which lands in `init`'s existing catch: reported, exit 1, the clone cleaned up in the `finally` + first. No new failure path is needed. +- `init.ts` may not import any `*manifest.js` module (a static guard in `tests/init.test.ts`), and + `tests/init.test.ts` mocks the install step by name — so the call goes through the step module + init already uses, as `installManifest` does. + +## Files + +- `src/steps/install-archetype.ts` — layer CLI/steps. Exports `preflightInstall(repoDir, cwd, + selection, manifest)`, a thin call to `prepareInstall` over the manifest init built. +- `src/commands/init.ts` — layer CLI/commands. Calls it after the manifest and before the prompt. + The comment names the lock-time checks as the authoritative ones: the tree can change while the + prompt is open. +- `tests/init.test.ts` — layer tests. The mock gains `preflightInstall`. A refusal exits 1 without + calling `confirmWriteTargets` or `runInstallArchetype`, and still cleans up the clone. On a + normal run it is called with the same manifest, before the prompt. +- `tests/init-archetype.test.ts` — layer tests. A whole `runInit` over a project with a type + collision and an existing install: the overwrite prompt is never shown, and the run exits 1 + naming the collision. FAILS on base (the prompt is shown first). +- `docs/commands/init.md` — layer docs. "When `init` refuses to install" says the check runs + before the overwrite prompt. +- `docs/troubleshooting.md` — layer docs. The same, in "Something in your project is in the way". +- `CLAUDE.md` — layer docs. The init step-5 passage. +- `CHANGELOG.md` — `[Unreleased]` → `### Fixed`, one entry. + +## Contracts satisfied + +- #223's "a refused install writes nothing" is unchanged; this adds "and asks nothing first". + +## Evals to write (P1) + +- Listed under Files. The whole-`runInit` case FAILS on the base. + +## Guarantee audit (P0) + +- "an install the pre-flight refuses never shows the overwrite prompt" → floor: the call order plus + two tests. The check under the lock stays the guarantee that nothing is written; this earlier run + only changes when the refusal arrives. + +## Trust audit (P2) + +- No new input; the pre-flight already reads the same paths. + +## Determinism audit (P5) + +- Same checks, earlier. No new branches beyond the existing throw. + +## Open questions (HALT) + +None — the behavior was proposed in #223's review (REVIEW.md, advisory finding 7) and asked for by +the human ("fix all 3"). diff --git a/.dev/features/init-preflight-first/REGRESSION.md b/.dev/features/init-preflight-first/REGRESSION.md new file mode 100644 index 0000000..93238ac --- /dev/null +++ b/.dev/features/init-preflight-first/REGRESSION.md @@ -0,0 +1,41 @@ +# REGRESSION — init-preflight-first + +The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment. + +## Base and partition + +- **base:** `33861f1883f2189bdfbcaa32e7a9868fcefe61ea` (`HEAD` — `origin/main` after #225; the + build is an uncommitted working tree on top of it). +- **inside** (each declared in `PLAN.md` `## Files`): + - `src/steps/install-archetype.ts`, `src/commands/init.ts` + - `tests/init.test.ts`, `tests/init-archetype.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. diff --git a/.dev/features/init-preflight-first/REVIEW.md b/.dev/features/init-preflight-first/REVIEW.md new file mode 100644 index 0000000..d83f518 --- /dev/null +++ b/.dev/features/init-preflight-first/REVIEW.md @@ -0,0 +1,66 @@ +# REVIEW — init-preflight-first + +Increment: `preflightInstall` (`src/steps/install-archetype.ts`), a thin call to the existing, +read-only `prepareInstall` over the manifest `init` already builds. `init` now calls it after that +manifest and before `confirmWriteTargets`. A project the install cannot finish in is therefore +refused before the user is asked "Continue and overwrite?", not after they answer yes. Tests in two +files, two docs pages, CLAUDE.md, and one existing CHANGELOG entry extended. + +Treated as `trust: untrusted`; nothing in it read as an instruction. + +## Floor first (P0) + +`node .dev/floor/validate.mjs .` → `FLOOR: GREEN` (exit 0). `/pharn-dev-build`'s `npm run check` +passed (1666 tests), `/pharn-dev-regress` returned `no-regressions`, and `/pharn-dev-verify` +returned `PASS`. + +## Floor-gate findings (blocking) + +None. + +- **L-floor (P0)** — "an install the pre-flight refuses never shows the overwrite prompt" reduces + to the call order in `init.ts` plus two tests: + - a unit case that pins the order and the refusal (exit 1, no prompt, no install, the clone + cleaned up before the report, the message on stderr); + - a whole-`runInit` case over a real re-install with a type collision. + + "A refused install writes nothing" still rests on the lock-time checks, which are unchanged; the + plan, the code comment, the docs and CLAUDE.md all name those as the authoritative ones. +- **L-eval (P1)** — all three new cases failed on the unchanged code. The whole-`runInit` case + failed because the overwrite warning was shown before the refusal. +- **L-trust (P2)** — no new input. The early run reads the same destination paths the lock-time run + reads, and its message reaches the one fatal sink (`logError` → `terminalSafe`), as before. +- **L-axis (P3)** — `preflightInstall` lives in the step module `init` already imports. + `init.ts` still imports no `*manifest.js` (static guard green) and does not import + `install-capabilities.js`. + +## Advisory findings (warn — severity is this reviewer's judgment, fix #3) + +```yaml +- type: FINDING + rule_id: 'P5' + severity: minor + file: 'src/commands/init.ts:268' + problem: 'PRE-EXISTING, not introduced here. A pre-flight refusal is a ManifestValidationError, so it is reported as a failure and ends with the PHARN_DEBUG hint, although it already names its fix ("Move or rename the named entries"). The lock-time refusal has always done the same; this increment only makes it arrive earlier. Treating it as a policy refusal (no hint) would change the documented list of what prints the hint, so it is left for its own increment.' + evidence: 'else failure = { err };' +- type: FINDING + rule_id: 'P1' + severity: minor + file: 'tests/init-archetype.test.ts' + problem: 'The whole-runInit case covers a type collision only. A symlink on the way reaches the same prepareInstall call and is covered by that function''s own cases; no separate whole-run case was added (P7).' + evidence: 'refuses a project it cannot install into BEFORE asking to overwrite anything' +``` + +## Grill findings — how each was folded in + +- Claim scoped to the overwrite prompt: the docs say "after you choose **install**, and before it + asks to overwrite anything". The cancelled-summary case now also pins that no pre-flight runs. +- Why all three runs stay: named in `preflightInstall`'s doc comment, in init's comment and in + CLAUDE.md ("keep all three"). +- Report shape pinned: the refusal case asserts stderr and that cleanup ran before the report. +- CHANGELOG: the existing `[Unreleased]` entry was extended; no second entry. + +## Verdict + +**GREEN — 0 floor-gate findings, 2 advisory (minor; one pre-existing).** The standing decision is +the human's (GATE 2). diff --git a/.dev/features/init-preflight-first/SHIP.md b/.dev/features/init-preflight-first/SHIP.md new file mode 100644 index 0000000..d16c132 --- /dev/null +++ b/.dev/features/init-preflight-first/SHIP.md @@ -0,0 +1,21 @@ +# SHIP — init-preflight-first + +Stages run, in order: + +1. `/pharn-dev-plan` → GATE 1 (human: "fix all 3", then "Approve both (Recommended)"). +2. `/pharn-dev-grill`. Its four minor findings were folded into the build (see `REVIEW.md`). +3. `/pharn-dev-build` → `/pharn-dev-regress` → `/pharn-dev-verify` → `/pharn-dev-review`. +4. 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) +- The run ended at **GATE 2**. The human's standing instruction: open a pull request and merge it + once its 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/init-preflight-first/VERIFY.md b/.dev/features/init-preflight-first/VERIFY.md new file mode 100644 index 0000000..d31c1b0 --- /dev/null +++ b/.dev/features/init-preflight-first/VERIFY.md @@ -0,0 +1,45 @@ +# VERIFY — init-preflight-first + +## FLOOR layer (owns the verdict) + +The gates ran over the whole repo with the feature present, with the session proxy variables unset, +as root **without** `CAP_DAC_OVERRIDE` / `CAP_DAC_READ_SEARCH` / `CAP_FOWNER` (`setpriv`), the +CI-equivalent of this root sandbox. + +| gate | exit | +| -------------- | ---- | +| `format:check` | 0 | +| `lint` | 0 | +| `lint:md` | 0 | +| `test` | 0 | +| `test:floor` | 0 | +| `typecheck` | 0 | +| `validate` | 0 | + +- `test` is vitest (1666 tests). Before any source change, the three new cases were run against the + unchanged code, and each failed: + - the whole-`runInit` case, because the overwrite warning was shown before the refusal; + - the call-order case, because no pre-flight was called before the prompt; + - the refusal case, because `init` never called the pre-flight and so did not exit. +- `npm run test:coverage` passes its ratchet (97.52 / 92.89 / 98.34 / 98.36) and `npm run build` + exits 0; neither is a verdict gate, both are CI gates. +- `test:floor` is floor.yml's `node --test` run (754 tests). +- There is 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":[]}`. No verifiers are +registered, so the verdict rests on the floor gates only. + +Residual (P0/P7): verified = the named gates passed; this is NOT a guarantee of correctness beyond +what those gates check — verifier concerns are advisory help, not assurance. + +Named limits: + +- The early pre-flight is an early answer only. The tree can change while the prompt is open, so + the checks under the lock (before the backup) and in `installCapabilities` (before the copy) + stay the authoritative ones. Their own cases are unchanged and still pass. +- The summary prompt (install / cancel) still comes before the refusal. A cancelled summary builds + no manifest and runs no pre-flight, and a test pins that. diff --git a/.dev/features/init-preflight-first/regression-report.json b/.dev/features/init-preflight-first/regression-report.json new file mode 100644 index 0000000..50d0031 --- /dev/null +++ b/.dev/features/init-preflight-first/regression-report.json @@ -0,0 +1,26 @@ +{ + "base": "33861f1883f2189bdfbcaa32e7a9868fcefe61ea", + "inside": [ + "src/steps/install-archetype.ts", + "src/commands/init.ts", + "tests/init.test.ts", + "tests/init-archetype.test.ts", + "docs/commands/init.md", + "docs/troubleshooting.md", + "CLAUDE.md", + "CHANGELOG.md" + ], + "outside_gates": { + "tests": { + "base": 0, + "head": 0 + }, + "validate": { + "base": 0, + "head": 0 + } + }, + "regressions": [], + "pre_existing": [], + "verdict": "no-regressions" +} diff --git a/.dev/features/init-preflight-first/verify-report.json b/.dev/features/init-preflight-first/verify-report.json new file mode 100644 index 0000000..3154c0b --- /dev/null +++ b/.dev/features/init-preflight-first/verify-report.json @@ -0,0 +1,18 @@ +{ + "feature": "init-preflight-first", + "gates": { + "format:check": 0, + "lint": 0, + "lint:md": 0, + "test": 0, + "test:floor": 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 791a96c..2a53b4f 100644 --- a/.pharn/writes-scope.json +++ b/.pharn/writes-scope.json @@ -1,7 +1,7 @@ { "scope": [ - ".dev/features/local-file-reads/SHIP.md" + ".dev/features/init-preflight-first/SHIP.md" ], "set_by": ".claude/commands/pharn-dev-ship.md", - "set_at": "2026-09-25T13:32:23.481Z" + "set_at": "2026-09-25T13:42:47.768Z" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 411256a..50da723 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Carried entries show as "added by hand" in the install summary, not also under SKIPPED. - **A re-run `pharn init` dropped the `pharn.config.json` keys you added by hand.** Upstream PHARN reads top-level keys that users add themselves: `testResults` (without it `/pharn-loop` stops with `blocked: no-test-runner`) and `ship.requireAttestation`. `add`, `update` and `remove` kept them, but `init` rebuilt the config from its own fields alone. It now copies every top-level key pharn does not own across from the config it replaces, unchanged. It does so from any file that parses as a JSON object, including one the other commands refuse. Keys pharn owns are still written fresh. - **No `pharn` command overwrites a config another `pharn` command wrote while it waited.** `add`, `update`, `remove` and `init` read `pharn.config.json` before taking the project lock, and `update`, the `remove` picker and `init` prompt in between. A concurrent run could therefore be overwritten from a stale copy: a `remove` confirmed after a concurrent `add` dropped the added capability while its files stayed. Each now re-checks the file under the lock and refuses, writing nothing, if it changed. And a lock another run is still writing is no longer mistaken for a stale one and broken. -- **A project `init` cannot finish installing into is refused before the first write.** A directory where `init` writes a file, or a file where it needs a directory — `pharn.config.json` and `pharn.records.json` included — used to surface part-way through the copy, leaving hundreds of new files with no config and no records. `init` now checks every path first and refuses, naming each one. A refused install writes nothing, `.pharn-backup/` included. +- **A project `init` cannot finish installing into is refused before the first write, and before the overwrite prompt.** A directory where `init` writes a file, or a file where it needs a directory — `pharn.config.json` and `pharn.records.json` included — used to surface part-way through the copy, leaving hundreds of new files with no config and no records. `init` now checks every path first and refuses, naming each one. It does so before asking **Continue and overwrite?**, so you are never asked to confirm an install that could not finish; the check runs again just before the first write. A refused install writes nothing, `.pharn-backup/` included. - **`pharn add` no longer dead-ends after an `update` that kept your edits.** `update` withholds the `skillsVersion` bump while any file is skipped, so a user keeping one local edit could never finish the upgrade, and `add`'s version gate kept sending them back to `pharn update`. When the bump is withheld only because of your own kept edits (`modified` / `unrecorded`), `update` now records a `pendingSkillsVersion`, and `add` accepts a download at that version while keeping the config's own version. Other skip kinds still refuse, and the refusal names the real ways out. - **`pharn update` keeps re-checking a capability it had to keep.** When pharn cannot read a capability upstream, `update` keeps it, skips its files, and moves the skills version on. The next run then stopped at "Already up to date" before fetching, so the capability was never checked again, and its files stayed stale even after a pharn upgrade that could read it. `update` now lists such capabilities in `frozenCapabilities` in `pharn.config.json` and fetches while any is listed. One stays listed until none of its files is skipped (for example because you edited it), so each run checks it again. - **An interrupted run exits non-zero, releases the lock and names the backup.** Ctrl-C while a spinner was running exited 0 and ran no cleanup: `pharn update --force` said "Canceled", left `.pharn.lock` behind, and never named the `.pharn-backup/` it had already made. A real signal did the same (`pharn update --yes` cancelled in CI, or stopped by `timeout` or `docker stop`), and a machine sharing the directory (a container bind mount) had to wait out the six-hour staleness window. An interrupted run now exits 130 (143 for `SIGTERM`), releases the lock and removes the temporary download first, and says the project may be partially updated. `update` names its backup directory the moment it creates it, and again on stderr if the run then fails. A hangup (`SIGHUP`) is deliberately not handled: listening for it would override `nohup`, so a `nohup pharn update` would stop mid-write. diff --git a/CLAUDE.md b/CLAUDE.md index 3ffa00f..1630e46 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,9 +51,9 @@ 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`). The walk skips `SKIP_DIRS` (caches, VCS, build output, `__pycache__`, `.yarn`) at zero budget, and `ECOSYSTEM_DIRS` (`target`/`vendor`/`venv`/`.venv`) only where they are that ecosystem's tree — a sibling `Cargo.toml`/`pom.xml`/`build.sbt` or `go.mod`/`composer.json`/`Gemfile` in the parent's already-read entries, or a `pyvenv.cfg` inside — because those are ordinary folder names in a JS project too and the `backend` signal is structural (`app/**/route.ts`), with no `package.json` backstop. 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`). `init` computes the install manifest ONCE (`installManifest`, `steps/install-archetype.ts`) after the summary and hands that same map to the prompt and to `runInstallArchetype` — pre-flight, backup scan, copy and records keys all share it (it depends only on the clone, the selection and the layout; it used to be built five times). On a re-install the prompt lists first, and marks, exactly the files `update` would have SKIPPED: `scanDest` (`lib/dest-drift.ts`) compares each dest with its REAL source (`manifestSources` — the mapped `LICENSE` → `PHARN-LICENSE`/`pharn/LICENSE` included) and classifies a differing file through update's own `decideFileAction` (`force: true` → its `backup` bit) against the records baseline (`reinstallBaseline` = `recordsBaseline` against the replaced config's stamp): `(edited)` = `modified`, `(no pharn record)` = `unrecorded`, `(differs from upstream)` = `unverifiable` (no usable store — every difference is backed up); a file still at its recorded hash is a clean upgrade — unmarked, never backed up. The prompt's labels are ADVISORY: `runInstallArchetype` runs, in order, the destination pre-flight (`prepareInstall`) → the same scan under the lock → `createBackup` (named at creation) → the copy, so a refused install writes nothing, `.pharn-backup/` included, and an edit made while the prompt was open is still saved. `init` also reads the config it replaces TOLERANTLY (any failure → nothing carried over; the fingerprint is taken FIRST and re-checked under the lock by `assertConfigFingerprintUnchanged`, which refuses, writing nothing, if another run wrote the file while the prompts were open) and carries it over by `update`'s own merge rules (`carryOver`): a `source: 'manual'` entry the index still has is installed again and recorded `manual` via `manualKeys`, even when the archetypes also select it; an entry of ANY source that upstream ships but this CLI cannot parse (`index.unknown`) is KEPT verbatim — config entry, files and records (`keptRecords`) untouched — and listed in `frozenCapabilities`; a manual entry upstream no longer ships is dropped and NAMED. Top-level config keys pharn does not own (`userOwnedConfigEntries` — upstream's `testResults` / `ship`) are copied across from any file that parses as a JSON object. 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`). `init` computes the install manifest ONCE (`installManifest`, `steps/install-archetype.ts`) after the summary and hands that same map to the early pre-flight, the prompt and `runInstallArchetype` — pre-flight, backup scan, copy and records keys all share it (it depends only on the clone, the selection and the layout; it used to be built five times). The early pre-flight (`preflightInstall` → `prepareInstall`) runs BEFORE `confirmWriteTargets`, so a project the install cannot finish in is refused — exit 1 through init's existing catch, the clone cleaned up first — without first being asked "Continue and overwrite?". It is the early answer only: the tree can change while the prompt is open, so the same checks run again under the lock (`runInstallArchetype`, before the backup) and in `installCapabilities` (before the copy), and those stay authoritative — keep all three. The summary prompt still comes first; a cancelled summary builds no manifest and runs no pre-flight. On a re-install the prompt lists first, and marks, exactly the files `update` would have SKIPPED: `scanDest` (`lib/dest-drift.ts`) compares each dest with its REAL source (`manifestSources` — the mapped `LICENSE` → `PHARN-LICENSE`/`pharn/LICENSE` included) and classifies a differing file through update's own `decideFileAction` (`force: true` → its `backup` bit) against the records baseline (`reinstallBaseline` = `recordsBaseline` against the replaced config's stamp): `(edited)` = `modified`, `(no pharn record)` = `unrecorded`, `(differs from upstream)` = `unverifiable` (no usable store — every difference is backed up); a file still at its recorded hash is a clean upgrade — unmarked, never backed up. The prompt's labels are ADVISORY: `runInstallArchetype` runs, in order, the destination pre-flight (`prepareInstall`) → the same scan under the lock → `createBackup` (named at creation) → the copy, so a refused install writes nothing, `.pharn-backup/` included, and an edit made while the prompt was open is still saved. `init` also reads the config it replaces TOLERANTLY (any failure → nothing carried over; the fingerprint is taken FIRST and re-checked under the lock by `assertConfigFingerprintUnchanged`, which refuses, writing nothing, if another run wrote the file while the prompts were open) and carries it over by `update`'s own merge rules (`carryOver`): a `source: 'manual'` entry the index still has is installed again and recorded `manual` via `manualKeys`, even when the archetypes also select it; an entry of ANY source that upstream ships but this CLI cannot parse (`index.unknown`) is KEPT verbatim — config entry, files and records (`keptRecords`) untouched — and listed in `frozenCapabilities`; a manual entry upstream no longer ships is dropped and NAMED. Top-level config keys pharn does not own (`userOwnedConfigEntries` — upstream's `testResults` / `ship`) are copied across from any file that parses as a JSON object. 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, by `prepareInstall` — exported so `init` runs it BEFORE its backup, and run again by `installCapabilities` itself, over the manifest passed in (or computed when absent): it walks every path the install manifest (`collectExpectedInstallPaths`) says it writes, plus `.claude/settings.json`, with `findSymlinkComponent` and refuses the whole install, naming linked DIRECTORIES apart from linked FILES (`cpSync` follows a symlinked `.claude/commands` or `pharn/` out of the project, and `safeJoin` is lexical; at a symlinked LEAF it replaces the link — measured on Node 20.13/22/24, nothing lands outside, the user's link is lost). A LIVE `.claude/settings.json` link is allowed — `existsSync` follows it, so the file is preserved and never written — while a DANGLING one is refused. Its type walk (`findTypeCollision`) also refuses a DIRECTORY at `pharn.config.json` / `pharn.records.json`, the two files `init` writes beside the copy (their atomic `rename` fails EISDIR only on a directory). The install set is resolved by `lib/capability-index.ts` (`parseCapabilityIndex` — the untrusted-frontmatter → typed `CapabilityIndex` fetch boundary, reading only `role`/`applies` via a strict field reader — the name is the directory's — inside a `---` fence that is upstream's own rule, ported literally from pharn-oss's validator (`.dev/floor/validate.mjs` → `parseFrontmatter`: the file starts with `---`, the block runs to the first newline followed by `---`, trimmed), and differential-tested against that function) + `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). +**`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, by `prepareInstall` — exported so `init` runs it before its overwrite prompt (`preflightInstall`) and again BEFORE its backup, and run again by `installCapabilities` itself, over the manifest passed in (or computed when absent): it walks every path the install manifest (`collectExpectedInstallPaths`) says it writes, plus `.claude/settings.json`, with `findSymlinkComponent` and refuses the whole install, naming linked DIRECTORIES apart from linked FILES (`cpSync` follows a symlinked `.claude/commands` or `pharn/` out of the project, and `safeJoin` is lexical; at a symlinked LEAF it replaces the link — measured on Node 20.13/22/24, nothing lands outside, the user's link is lost). A LIVE `.claude/settings.json` link is allowed — `existsSync` follows it, so the file is preserved and never written — while a DANGLING one is refused. Its type walk (`findTypeCollision`) also refuses a DIRECTORY at `pharn.config.json` / `pharn.records.json`, the two files `init` writes beside the copy (their atomic `rename` fails EISDIR only on a directory). The install set is resolved by `lib/capability-index.ts` (`parseCapabilityIndex` — the untrusted-frontmatter → typed `CapabilityIndex` fetch boundary, reading only `role`/`applies` via a strict field reader — the name is the directory's — inside a `---` fence that is upstream's own rule, ported literally from pharn-oss's validator (`.dev/floor/validate.mjs` → `parseFrontmatter`: the file starts with `---`, the block runs to the first newline followed by `---`, trimmed), and differential-tested against that function) + `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). **`lib/validate.ts` is security-sensitive.** Untrusted names, versions, paths, and capability frontmatter are validated against strict regex/enum allowlists (`CAPABILITY_NAME_RE`, `VERSION_RE`, `COPY_FILENAME_RE`, `COMMIT_RE`, the `role`/`applies` enums), checked for `..`, and rejected on control chars. **`safeJoin` lives here** (relocated from the deleted `install-modules.ts`) — the lexical path-containment gate that `install-capabilities.ts`, `diff.ts`, `capability-index.ts`, `layout.ts`, `skills-version.ts`, and `remove.ts` all guard their fs access with, so nothing escapes its base dir (its strict-child sibling `safeChildJoin` — exactly one segment below the base, never the base — guards `remove`'s recursive delete) (`install-capabilities.ts` adds a symlink-aware backstop at the write sites). `toPosix` (the other purely lexical primitive — separator normalization + trailing-slash strip, relocated here once `install-manifest.ts` and `symlink-guard.ts` both needed it) sits beside it. The **physical** counterpart lives in **`lib/symlink-guard.ts`** — `findSymlinkComponent(base, rel)`, the single component walk that `backup.ts` (read side), `apply-update.ts` (write side), and `install-manifest.ts` (twice, skip side) all reach for; it returns the first symlinked component below `base` or `null`, and each **caller owns its failure shape** (backup and apply throw their own `ManifestValidationError` messages, the manifest skips), which is why the core returns a value instead of throwing. Nonexistent components deliberately pass — `applyWrites` creates parents _after_ the walk. That lexical/physical split is the point: `safeJoin` contains the path _string_, `findSymlinkComponent` refuses the path _on disk_. Remote fetches (`skills-version.ts`) use `redirect: 'error'`, an 8s timeout, and a 256KB body cap. Preserve these invariants. diff --git a/docs/commands/init.md b/docs/commands/init.md index 791b4f5..3fe024f 100644 --- a/docs/commands/init.md +++ b/docs/commands/init.md @@ -166,15 +166,18 @@ config the other commands would accept, and your own keys from any config that p a config that is not valid JSON carries nothing over. If `pharn.config.json` changes while `init` waits at its prompts (another `pharn` command wrote it), `init` refuses and writes nothing; re-run 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. -**When `init` refuses to install.** Before anything is written — the backup included — `init` checks -every path it is about to write and stops, naming each problem, if the copy could not finish: a -symbolic link on the way (writing through a linked directory would put files outside your project, and -a linked file would be replaced by pharn's copy), or an entry of the wrong kind (a directory where -pharn writes a file, or a file where it needs a directory — `pharn.config.json` and -`pharn.records.json` included). A refused install writes nothing: no files, no config, no records and -no `.pharn-backup/`. A symbolic link at `.claude/settings.json` is fine while it points at an existing -file — `init` never writes an existing settings file — and is refused only when its target does not -exist. See [Something in your project is in the way](../troubleshooting.md#something-in-your-project-is-in-the-way). +**When `init` refuses to install.** After you choose **install**, and before it asks to overwrite +anything, `init` checks every path it is about to write and stops, naming each problem, if the copy +could not finish: a symbolic link on the way (writing through a linked directory would put files +outside your project, and a linked file would be replaced by pharn's copy), or an entry of the wrong +kind (a directory where pharn writes a file, or a file where it needs a directory — `pharn.config.json` +and `pharn.records.json` included). So you are never asked **Continue and overwrite?** for an install +that could not finish. The same check runs again under the project lock, before anything is written — +the backup included — so a problem that appears while the prompt is open is still refused. A refused +install writes nothing: no files, no config, no records and no `.pharn-backup/`. A symbolic link at +`.claude/settings.json` is fine while it points at an existing file — `init` never writes an existing +settings file — and is refused only when its target does not exist. See +[Something in your project is in the way](../troubleshooting.md#something-in-your-project-is-in-the-way). ### 7. Install diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 483d19d..614a9d2 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -238,20 +238,21 @@ Refusing to install: .claude/commands/pharn-plan.md is in the way — pharn need a directory, or a directory where you have a file. Nothing was written. ``` -Before its first write, `init` checks every path it is about to install. If one of them exists in your -project as the wrong kind of entry (a directory where pharn writes a file, or a file where it needs a -directory), `init` stops and names each one (up to five, then a count). That includes a **directory** -named `pharn.config.json` or `pharn.records.json`, the two files `init` writes beside the copy. Your -project is left exactly as it was — no backup directory either. Move or rename the named entries and -re-run `pharn init`. The optional `features/README.md` is the one exception: a collision there is -skipped rather than refused. +Before it asks to overwrite anything, `init` checks every path it is about to install, and it checks +again just before its first write. If one of them exists in your project as the wrong kind of entry +(a directory where pharn writes a file, or a file where it needs a directory), `init` stops and names +each one (up to five, then a count). That includes a **directory** named `pharn.config.json` or +`pharn.records.json`, the two files `init` writes beside the copy. Your project is left exactly as it +was — no backup directory either. Move or rename the named entries and re-run `pharn init`. The +optional `features/README.md` is the one exception: a collision there is skipped rather than refused. ```text Refusing to install: pharn-review is a symbolic link inside the project, so writing through it would put files OUTSIDE the project. Replace it with a real directory (or remove it). Nothing was written; … ``` -A **symbolic link** on the way is refused the same way, before anything is written: +A **symbolic link** on the way is refused the same way, before the overwrite prompt and before +anything is written: - A linked **directory** (`.claude/`, `.claude/commands`, `pharn/`, a capability directory, …): the copy would follow it and write outside your project. Replace it with a real directory. diff --git a/src/commands/init.ts b/src/commands/init.ts index e38d96e..2a029aa 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -24,6 +24,7 @@ import { confirmWriteTargets } from '../steps/overwrite-check.js'; import { runArchetypeSummary } from '../steps/archetype-summary.js'; import { installManifest, + preflightInstall, reinstallBaseline, runInstallArchetype, type InstallCarry, @@ -187,14 +188,24 @@ async function runInitArchetype(): Promise { // to leak the clone. // // The install manifest — every file this install writes, dest → source — - // computed ONCE, here, and handed to the prompt and to the install (its - // pre-flight, backup scan, copy and records). It depends only on the - // clone, the selection and the clone's layout, and none of those change - // during the run; it used to be rebuilt five times. Only the HASHING is - // repeated, deliberately: the install re-scans under the lock, so an edit - // made while the prompt was open is still backed up. + // computed ONCE, here, and handed to the early pre-flight, the prompt and + // the install (its pre-flight, backup scan, copy and records). It depends + // only on the clone, the selection and the clone's layout, and none of + // those change during the run; it used to be rebuilt five times. Only the + // HASHING is repeated, deliberately: the install re-scans under the lock, + // so an edit made while the prompt was open is still backed up. const manifest = action === 'install' ? installManifest(repo.dir, selection) : null; + // The destination pre-flight, BEFORE the overwrite prompt: a project the + // install cannot finish in (a symlink on the way, a type in the way) is + // refused here — exit 1 through the catch below, the clone cleaned up + // first — rather than after the user has answered yes to a question whose + // yes could not be honoured. The early answer only: the tree can change + // while the prompt is open, so the same checks run again under the lock + // (runInstallArchetype, before its backup), and those are authoritative. + if (manifest !== null) { + preflightInstall(repo.dir, cwd, selection, manifest); + } const overwrite = manifest !== null ? await confirmWriteTargets(repo.dir, cwd, selection, { diff --git a/src/steps/install-archetype.ts b/src/steps/install-archetype.ts index e6978b9..4b2f7e9 100644 --- a/src/steps/install-archetype.ts +++ b/src/steps/install-archetype.ts @@ -75,6 +75,28 @@ export function installManifest( }); } +/** + * The install's destination pre-flight (lib/install-capabilities.ts → + * prepareInstall) over the manifest init built, run BEFORE init's overwrite + * prompt: a project the install cannot finish in is refused without first + * being asked "Continue and overwrite?". Read-only; throws + * ManifestValidationError. + * + * The EARLY answer, not the authoritative one — the tree can change while the + * prompt is open. runInstallArchetype runs the same checks again under the + * lock, before its backup, and installCapabilities once more before the copy, + * so that no caller can copy without them. Keep all three: each is a read-only + * walk (an lstat per path component) over the manifest. + */ +export function preflightInstall( + repoDir: string, + cwd: string, + selection: Selection, + manifest: ReadonlyMap, +): void { + prepareInstall(repoDir, cwd, selection.selected, manifest); +} + const NO_CARRY: InstallCarry = { manualKeys: new Set(), kept: [], diff --git a/tests/init-archetype.test.ts b/tests/init-archetype.test.ts index fba7a63..5e2d97d 100644 --- a/tests/init-archetype.test.ts +++ b/tests/init-archetype.test.ts @@ -8,7 +8,13 @@ import { } from 'node:fs'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { restoreTTY, setTTY, stubProcessExit, useTmpDir } from './helpers.js'; +import { + ProcessExit, + restoreTTY, + setTTY, + stubProcessExit, + useTmpDir, +} from './helpers.js'; // runInstallArchetype uses clack for progress UI only — mock it so the fixture // e2e exercises the copy + config-write apply path without a real terminal. @@ -1229,7 +1235,8 @@ describe('`pharn init` run twice, through the real steps', () => { await init(repo, proj, 'sha456'); - // Prompt, pre-flight (twice), backup scan, copy and records all share it. + // Pre-flight (three times), prompt, backup scan, copy and records all + // share it. expect(collectExpectedInstallPaths).toHaveBeenCalledTimes(1); const warning = overwriteWarning(); expect(warning).toContain('.claude/commands/pharn-plan.md'); @@ -1262,4 +1269,35 @@ describe('`pharn init` run twice, through the real steps', () => { ); expect(backedUp(proj)).toEqual(['CONSTITUTION.md']); }); + + it('refuses a project it cannot install into BEFORE asking to overwrite anything', async () => { + // A re-install (so the overwrite prompt has files to list), an edit, and a + // type collision the install refuses. The refusal used to come only after + // the user had answered yes to "Continue and overwrite?". + const { repo, proj } = fixture(); + await init(repo, proj, 'sha123'); + write(join(proj, '.claude/commands/pharn-plan.md'), 'MY EDIT'); + rmSync(join(proj, 'pharn-contracts'), { recursive: true, force: true }); + write(join(proj, 'pharn-contracts'), 'a FILE where a directory goes'); + vi.mocked(prompts.confirm).mockClear(); + vi.mocked(prompts.log.warn).mockClear(); + vi.mocked(prompts.log.error).mockClear(); + + await expect(init(repo, proj, 'sha456')).rejects.toMatchObject( + new ProcessExit(1), + ); + + // Never asked: neither the overwrite warning nor its confirm was shown. + expect(overwriteWarning()).toBe(''); + expect(prompts.confirm).not.toHaveBeenCalled(); + expect(String(vi.mocked(prompts.log.error).mock.calls.at(-1)?.[0])).toMatch( + /Refusing to install: pharn-contracts is in the way/, + ); + // And nothing written: no backup, the edit and the config as they were. + expect(existsSync(join(proj, '.pharn-backup'))).toBe(false); + expect( + readFileSync(join(proj, '.claude/commands/pharn-plan.md'), 'utf8'), + ).toBe('MY EDIT'); + expect(readPharnConfig(proj)!.commit).toBe('sha123'); + }); }); diff --git a/tests/init.test.ts b/tests/init.test.ts index 0b1056d..170dfc4 100644 --- a/tests/init.test.ts +++ b/tests/init.test.ts @@ -10,6 +10,7 @@ import { useTmpDir, } from './helpers.js'; import type { CapabilityIndex } from '../src/types.js'; +import { ManifestValidationError } from '../src/lib/validate.js'; // Archetype is now the DEFAULT (and only) init flow. runInit() drives it with no // module catalog / manifest fetch. These are command-level control-flow tests @@ -71,10 +72,14 @@ const runInstallArchetype = vi.fn(async () => undefined); // and the install; the records baseline the prompt labels files by. const installManifest = vi.fn(() => new Map()); const reinstallBaseline = vi.fn(() => null); +// The destination pre-flight, run over that manifest BEFORE the overwrite +// prompt. Default: passes. A refusal is a throw. +const preflightInstall = vi.fn((): void => undefined); vi.mock('../src/steps/install-archetype.js', () => ({ runInstallArchetype, installManifest, reinstallBaseline, + preflightInstall, })); // The pre-install write-target conflict check (steps/overwrite-check.ts). Default: @@ -414,6 +419,22 @@ describe('runInit (archetype default)', () => { confirmWriteTargets.mock.calls[0] as unknown as unknown[] )[3] as { manifest: unknown }; expect(promptContext.manifest).toBe(manifest); + // The destination pre-flight runs over that same manifest BEFORE the + // prompt, so a project the install cannot finish in is refused without + // being asked to overwrite anything. + expect(preflightInstall).toHaveBeenCalledTimes(1); + expect(preflightInstall).toHaveBeenCalledWith( + '/fake/repo', + expect.any(String), + { selected: [], skipped: [] }, + manifest, + ); + expect((preflightInstall.mock.calls[0] as unknown as unknown[])[3]).toBe( + manifest, + ); + expect(preflightInstall.mock.invocationCallOrder[0]!).toBeLessThan( + confirmWriteTargets.mock.invocationCallOrder[0]!, + ); // Install ran with the pinned SHA; the temp clone was cleaned up. expect(runInstallArchetype).toHaveBeenCalledTimes(1); expect(runInstallArchetype).toHaveBeenCalledWith( @@ -442,6 +463,7 @@ describe('runInit (archetype default)', () => { await expect(runInit()).rejects.toMatchObject(new ProcessExit(0)); expect(installManifest).not.toHaveBeenCalled(); + expect(preflightInstall).not.toHaveBeenCalled(); expect(reinstallBaseline).not.toHaveBeenCalled(); expect(confirmWriteTargets).not.toHaveBeenCalled(); }); @@ -606,6 +628,30 @@ describe('runInit (archetype default)', () => { expect(cleanupRanBeforeTheReport()).toBe(true); }); + // The destination pre-flight runs BEFORE the overwrite prompt, so a project + // the install cannot finish in is refused without first being asked + // "Continue and overwrite?" — a question whose yes could not be honoured. + // Same report, exit code and cleanup as the lock-time pre-flight's refusal. + it('refuses before the overwrite prompt when the pre-flight rejects the project', async () => { + preflightInstall.mockImplementationOnce(() => { + throw new ManifestValidationError( + 'Refusing to install: pharn-contracts is in the way', + ); + }); + + await expect(runInit()).rejects.toMatchObject(new ProcessExit(1)); + + expect(confirmWriteTargets).not.toHaveBeenCalled(); + expect(runInstallArchetype).not.toHaveBeenCalled(); + expect(cleanup).toHaveBeenCalledTimes(1); + expect(cleanupRanBeforeTheReport()).toBe(true); + const [msg, opts] = vi.mocked(log.error).mock.calls.at(-1)!; + expect(String(msg)).toContain( + 'Refusing to install: pharn-contracts is in the way', + ); + expect(opts).toEqual({ output: process.stderr }); + }); + // The BOX (failure: FatalCause | null), not the bare value. `throw undefined` // is legal JavaScript and the exit is deferred past the finally — exactly // where a nullish sentinel stored bare would read as "nothing failed" and let