diff --git a/.dev/features/init-dest-type-preflight/GRILL.md b/.dev/features/init-dest-type-preflight/GRILL.md new file mode 100644 index 0000000..52d176d --- /dev/null +++ b/.dev/features/init-dest-type-preflight/GRILL.md @@ -0,0 +1,29 @@ +# GRILL — init-dest-type-preflight + +Plan: `.dev/features/init-dest-type-preflight/PLAN.md` · spec-hash `bca940a5…d729d3c4e` matches live +`ARCHITECTURE.md`. Registered grillers: `{"registered":0,"grillers":[]}` → inline axes only. + +## Findings + +```yaml +- type: FINDING + rule_id: 'P5' + severity: important + file: '.dev/features/init-dest-type-preflight/PLAN.md:5' + problem: 'Stop the walk at the FIRST non-directory component of each path: deeper components cannot be lstat-ed (ENOTDIR) and must not throw out of the pre-flight; report that component once even when many paths share it.' + evidence: '`an existing intermediate component that is not a directory`' +- type: FINDING + rule_id: 'P5' + severity: minor + file: '.dev/features/init-dest-type-preflight/PLAN.md:22' + problem: '`.claude/settings.json` is never overwritten (existsSync → skip), so it must stay OUT of the type walk — a directory there is not a collision the copy would hit.' + evidence: '`for every path the install manifest says init writes`' +- type: FINDING + rule_id: 'P1' + severity: minor + file: '.dev/features/init-dest-type-preflight/PLAN.md:25' + problem: 'Assert "zero files written" by snapshotting the project tree before and after, not by checking one path, or a partial write elsewhere passes.' + evidence: '`ZERO files written`' +``` + +ADVISORY VERDICT: 3 concerns raised (0 blocking-severity, 3 advisory) — all folded into the build. diff --git a/.dev/features/init-dest-type-preflight/PLAN.md b/.dev/features/init-dest-type-preflight/PLAN.md new file mode 100644 index 0000000..af46c80 --- /dev/null +++ b/.dev/features/init-dest-type-preflight/PLAN.md @@ -0,0 +1,57 @@ +# PLAN — init-dest-type-preflight (PHARN-16: init refuses a file/dir collision BEFORE its first write) + +- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4 +- increment: `installCapabilities`' destination pre-flight also checks TYPES: for every path the install + manifest says `init` writes, an existing intermediate component that is not a directory, or an + existing leaf that is not a regular file, is a collision. Any collision refuses the whole install with + one `ManifestValidationError` naming the colliding paths (capped) — "Nothing was written" — instead of + `cpSync` failing part-way and leaving ~300 files with no `pharn.config.json` and no records. The + optional `features/README.md` keeps its documented skip-on-collision (a leaf that is not a regular file + now skips too, instead of throwing). +- layer(s): the CLI itself (`src/lib/install-capabilities.ts`) +- constitution_refs: [P1, P2, P4, P5] + +## Discovery — verified this run (P6) + +Review repro (`repro/init-dir-collision`): a directory at `.claude/commands/pharn-spec.md/` makes +`init` abort mid-copy; the project keeps 296 new files, no config, no records. The pre-flight +(`assertDestinationsInProject`, `install-capabilities.ts`) walks the same manifest set with +`findSymlinkComponent` only and deliberately leaves ENOTDIR "to the copy". `features/README.md` is +already skip-on-collision via `destAcceptsWrite` (tests at `install-capabilities.test.ts:396-427`). + +## Files + +- `src/lib/install-capabilities.ts` — type walk in the destination pre-flight (after the symlink check, + so a symlink is still reported as a symlink); features rel excluded; `destAcceptsWrite` also refuses a + non-regular-file leaf — layer CLI/lib +- `src/lib/symlink-guard.ts` — `findTypeCollision` lives beside `findSymlinkComponent` (the repo pins every + path-component walk to this one module — `tests/symlink-guard.test.ts`; amendment approved) — layer CLI/lib +- `tests/install-capabilities.test.ts` — directory at a file target → throws naming it, ZERO files + written; regular file at a directory component → same; a re-install over existing regular files still + succeeds; directory at `features/README.md` → skipped, install completes +- `docs/troubleshooting.md` — the new refusal and how to fix it (P4) + +## Contracts satisfied + +- "no partial installs" (CLAUDE.md, `installCapabilityDirs` pre-flight) — now also for destination types. + +## Evals to write (P1) + +- listed above; the collision cases leave files behind on the base source. + +## Guarantee audit (P0) + +- "init never starts writing into a tree whose types it cannot write" → floor: `lstat` of every + manifest path component before the first write. + +## Trust audit (P2) + +- Only `lstat` on project paths already `safeJoin`-contained; nothing read or executed. + +## Determinism audit (P5) + +- Sorted, capped list of colliding paths. + +## Open questions (HALT) + +- none diff --git a/.dev/features/init-dest-type-preflight/REGRESSION.md b/.dev/features/init-dest-type-preflight/REGRESSION.md new file mode 100644 index 0000000..1159e3a --- /dev/null +++ b/.dev/features/init-dest-type-preflight/REGRESSION.md @@ -0,0 +1,30 @@ +# REGRESSION — init-dest-type-preflight + +The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment. + +## Base and partition + +- **base:** `6920b79be17ff0f6e65bcbce9a7ab48d68550400` (`origin/main` at build time; the build is an uncommitted working tree on top of it). +- **inside** (each declared in `PLAN.md` `## Files`): `docs/troubleshooting.md`, `src/lib/install-capabilities.ts`, `src/lib/symlink-guard.ts`, `tests/install-capabilities.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/init-dest-type-preflight/REVIEW.md b/.dev/features/init-dest-type-preflight/REVIEW.md new file mode 100644 index 0000000..a05bf5b --- /dev/null +++ b/.dev/features/init-dest-type-preflight/REVIEW.md @@ -0,0 +1,39 @@ +# REVIEW — init-dest-type-preflight + +Floor first: `node .dev/floor/validate.mjs .` → exit 0 (GREEN). Everything below is **advisory**. + +## Floor-gate findings (blocking) + +None. + +- **L-floor (P0):** before the first write, every manifest path is walked with `lstat`; an existing + non-directory ancestor or non-regular-file leaf refuses the whole install. The walk stops at the first + offender (grill #1); `.claude/settings.json` and the optional features README stay out (grill #2). +- **L-eval (P1):** 6 cases fail on the base source (four collision shapes, the sorted multi-name + message, and the features-README directory, which used to throw from `cpSync`); "nothing written" is a + whole-tree snapshot (grill #3). A re-install over regular files still succeeds. +- **L-trust (P2):** `lstat` only, on `safeJoin`-contained project paths. +- **L-axis (P3):** the walk lives in `symlink-guard.ts` beside `findSymlinkComponent` — the repo pins + every path-component walk to that module (`tests/symlink-guard.test.ts`); this was a plan amendment, + approved before the move. The caller owns the failure shape, as for the symlink walk. + +## Advisory findings + +```yaml +- type: FINDING + rule_id: 'P5' + severity: minor + file: 'src/lib/install-capabilities.ts' + problem: 'An entry created between the pre-flight and the copy (TOCTOU) is not covered — the same residual the symlink pre-flight names.' + evidence: 'assertDestinationTypes(' +- 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/init-dest-type-preflight/SHIP.md b/.dev/features/init-dest-type-preflight/SHIP.md new file mode 100644 index 0000000..0a7649c --- /dev/null +++ b/.dev/features/init-dest-type-preflight/SHIP.md @@ -0,0 +1,17 @@ +# SHIP — init-dest-type-preflight + +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/init-dest-type-preflight/VERIFY.md b/.dev/features/init-dest-type-preflight/VERIFY.md new file mode 100644 index 0000000..dcdfaac --- /dev/null +++ b/.dev/features/init-dest-type-preflight/VERIFY.md @@ -0,0 +1,27 @@ +# VERIFY — init-dest-type-preflight + +## 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/init-dest-type-preflight/regression-report.json b/.dev/features/init-dest-type-preflight/regression-report.json new file mode 100644 index 0000000..2d5491c --- /dev/null +++ b/.dev/features/init-dest-type-preflight/regression-report.json @@ -0,0 +1,22 @@ +{ + "base": "6920b79be17ff0f6e65bcbce9a7ab48d68550400", + "inside": [ + "docs/troubleshooting.md", + "src/lib/install-capabilities.ts", + "src/lib/symlink-guard.ts", + "tests/install-capabilities.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/init-dest-type-preflight/verify-report.json b/.dev/features/init-dest-type-preflight/verify-report.json new file mode 100644 index 0000000..407a503 --- /dev/null +++ b/.dev/features/init-dest-type-preflight/verify-report.json @@ -0,0 +1,17 @@ +{ + "feature": "init-dest-type-preflight", + "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 95d0f33..207a504 100644 --- a/.pharn/writes-scope.json +++ b/.pharn/writes-scope.json @@ -1,7 +1,7 @@ { "scope": [ - ".dev/features/bounded-package-json/SHIP.md" + ".dev/features/init-dest-type-preflight/SHIP.md" ], "set_by": ".claude/commands/pharn-dev-ship.md", - "set_at": "2026-09-24T10:22:32.356Z" + "set_at": "2026-09-24T10:53:06.579Z" } diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 96616d8..9646597 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -223,6 +223,19 @@ Causes include a fetch failure (network/GitHub), an archive `pharn` refused to e PHARN_DEBUG=1 npx @pharn-dev/pharn init ``` +### Something in your project is in the way + +```text +Refusing to install: .claude/commands/pharn-plan.md is in the way — pharn needs a file where you have +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). Your project is left exactly as +it was. 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. + ### When the `PHARN_DEBUG` hint appears Every fatal error that came from an **exception** ends with diff --git a/src/lib/install-capabilities.ts b/src/lib/install-capabilities.ts index 6a73087..1b429dc 100644 --- a/src/lib/install-capabilities.ts +++ b/src/lib/install-capabilities.ts @@ -24,7 +24,7 @@ import { type LayoutPaths, } from './layout.js'; import { collectExpectedInstallPaths } from './install-manifest.js'; -import { findSymlinkComponent } from './symlink-guard.js'; +import { findSymlinkComponent, findTypeCollision } from './symlink-guard.js'; import type { InstalledCapability, Layout, Selection } from '../types.js'; // --------------------------------------------------------------------------- @@ -160,6 +160,16 @@ export function installCapabilities( // Destination pre-flight, before the FIRST write: nothing below may follow a // symlinked project directory out of the project (see assertDestinationsInProject). assertDestinationsInProject(repoDir, projectRoot, selection.selected, paths); + // ...and nothing may start writing into a tree whose TYPES it cannot write: + // a collision found by `cpSync` part-way leaves a half-installed project with + // no config and no records (see assertDestinationTypes). + assertDestinationTypes( + repoDir, + projectRoot, + selection.selected, + paths, + featuresRel, + ); // Copy the selected capability dirs (pre-flighted; no partial installs). const capabilities = installCapabilityDirs( @@ -368,6 +378,48 @@ function assertDestinationsInProject( const MAX_LINKED_SHOWN = 5; +/** + * Refuse the whole install when any path it would write collides by TYPE with + * what is already in the project: an existing component on the way that is not + * a directory, or an existing leaf that is not a regular file. Without this, + * `cpSync` meets the collision part-way (ENOTDIR / ERR_FS_CP_NON_DIR_TO_DIR) + * and the project is left with hundreds of new files but no `pharn.config.json` + * and no records. Runs AFTER assertDestinationsInProject, so a symlink is still + * reported as a symlink. + * + * Out of the walk: `.claude/settings.json` (never overwritten — `existsSync` + * skips it) and the optional features README (`destAcceptsWrite` skips it on a + * collision, by contract). Each path's walk stops at its FIRST non-directory + * component: nothing below it can be lstat-ed. + */ +function assertDestinationTypes( + repoDir: string, + projectRoot: string, + capabilities: InstalledCapability[], + paths: LayoutPaths, + featuresRel: string, +): void { + const colliding = new Set(); + for (const rel of collectExpectedInstallPaths({ + repoDir, + capabilities, + layout: paths.layout, + }).keys()) { + if (rel === featuresRel) continue; + const hit = findTypeCollision(projectRoot, rel); + if (hit !== null) colliding.add(hit); + } + if (colliding.size === 0) return; + const shown = [...colliding].sort(); + const list = + shown.length > MAX_LINKED_SHOWN + ? `${shown.slice(0, MAX_LINKED_SHOWN).join(', ')} and ${shown.length - MAX_LINKED_SHOWN} more` + : shown.join(', '); + throw new ManifestValidationError( + `Refusing to install: ${list} ${shown.length === 1 ? 'is in the way' : 'are in the way'} — pharn needs a file where you have a directory, or a directory where you have a file. Nothing was written. Move or rename ${shown.length === 1 ? 'it' : 'them'} and re-run \`pharn init\`.`, + ); +} + /** * May the install write `rel` under `projectRoot`? False when any component * below the root is a symlink — following one writes OUTSIDE the project, which @@ -384,7 +436,10 @@ const MAX_LINKED_SHOWN = 5; */ function destAcceptsWrite(projectRoot: string, rel: string): boolean { try { - return findSymlinkComponent(projectRoot, rel) === null; + return ( + findSymlinkComponent(projectRoot, rel) === null && + findTypeCollision(projectRoot, rel) === null + ); } catch { return false; } diff --git a/src/lib/symlink-guard.ts b/src/lib/symlink-guard.ts index 5a64553..b3a59f3 100644 --- a/src/lib/symlink-guard.ts +++ b/src/lib/symlink-guard.ts @@ -14,7 +14,8 @@ import { safeJoin, toPosix } from './validate.js'; // safeJoin-contained, and the returned value is DATA (a path string a caller // interpolates into a message or tests for null). // -// One axis (P3): detecting a symlinked path component. +// One axis (P3): walking a path's components on disk — a symlinked component +// (findSymlinkComponent) or one whose type blocks a write (findTypeCollision). // --------------------------------------------------------------------------- /** @@ -65,3 +66,26 @@ export function findSymlinkComponent(base: string, rel: string): string | null { } return null; } + +/** + * The first component of `rel` below `base` whose existing type blocks + * the write, or `null`. Intermediate components must be directories; the leaf + * must be a regular file (or absent). Components that do not exist pass — + * the copy creates them. The walk stops at the first offender, so a path below + * a regular file is never lstat-ed (no ENOTDIR). Used by `init`'s destination + * pre-flight (install-capabilities.ts), which owns the failure shape. + */ +export function findTypeCollision(base: string, rel: string): string | null { + const segments = toPosix(rel).split('/').filter(Boolean); + let current = ''; + for (const [i, segment] of segments.entries()) { + current = current ? `${current}/${segment}` : segment; + const stat = lstatSync(safeJoin(base, current), { + throwIfNoEntry: false, + }); + if (stat === undefined) return null; + const isLeaf = i === segments.length - 1; + if (isLeaf ? !stat.isFile() : !stat.isDirectory()) return current; + } + return null; +} diff --git a/tests/install-capabilities.test.ts b/tests/install-capabilities.test.ts index 001054c..48fa95a 100644 --- a/tests/install-capabilities.test.ts +++ b/tests/install-capabilities.test.ts @@ -1044,3 +1044,78 @@ describe('installCapabilities — symlinked destination pre-flight (PHARN-02)', ).toBe(true); }); }); + +// PHARN-16: a TYPE collision (a directory where the install writes a file, or a +// file where it needs a directory) used to surface from cpSync part-way, +// leaving ~300 new files with no config and no records. It is now refused +// before the first write, and the whole tree is left byte-identical. +describe('installCapabilities — destination type pre-flight (PHARN-16)', () => { + const tmp = useTmpDir(); + + function tree(root: string): string[] { + if (!existsSync(root)) return []; + return (readdirSync(root, { recursive: true }) as string[]) + .map((p) => p.split('\\').join('/')) + .sort(); + } + + it.each([ + ['a directory at a file target', '.claude/commands/pharn-plan.md', 'dir'], + [ + 'a directory at a capability file', + 'pharn-pipeline/grillers/a11y/a11y.md', + 'dir', + ], + ['a file at a directory component', 'pharn-contracts', 'file'], + ['a file at a capability dir', 'pharn-review/n-plus-one', 'file'], + ])('refuses %s; nothing written anywhere', (_label, rel, kind) => { + const repo = join(tmp.path(), 'repo'); + const proj = join(tmp.path(), 'proj'); + scaffoldRepo(repo); + if (kind === 'dir') { + write(join(proj, rel, 'keep.txt'), 'USER'); + } else { + write(join(proj, rel), 'USER FILE'); + } + const before = tree(proj); + + expect(() => installCapabilities(repo, proj, selection())).toThrow( + `Refusing to install: ${rel} is in the way`, + ); + expect(tree(proj)).toEqual(before); + }); + + it('names every colliding path once, sorted', () => { + const repo = join(tmp.path(), 'repo'); + const proj = join(tmp.path(), 'proj'); + scaffoldRepo(repo); + write(join(proj, 'pharn-review'), 'a file where lenses go'); + write(join(proj, 'LIMITS.md/inner.txt'), 'a dir where a doc goes'); + + expect(() => installCapabilities(repo, proj, selection())).toThrow( + /Refusing to install: LIMITS\.md, pharn-review are in the way/, + ); + }); + + it('still re-installs over existing regular files', () => { + const repo = join(tmp.path(), 'repo'); + const proj = join(tmp.path(), 'proj'); + scaffoldRepo(repo); + installCapabilities(repo, proj, selection()); + + expect(() => installCapabilities(repo, proj, selection())).not.toThrow(); + }); + + it('skips (does not refuse) a directory at the optional features/README.md', () => { + const repo = join(tmp.path(), 'repo'); + const proj = join(tmp.path(), 'proj'); + scaffoldRepo(repo); + write(join(proj, 'features/README.md/keep.txt'), 'USER'); + + expect(() => installCapabilities(repo, proj, selection())).not.toThrow(); + expect( + readFileSync(join(proj, 'features/README.md/keep.txt'), 'utf8'), + ).toBe('USER'); + expect(existsSync(join(proj, 'CONSTITUTION.md'))).toBe(true); + }); +});