Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions .dev/features/init-preflight-first/GRILL.md
Original file line number Diff line number Diff line change
@@ -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.**
68 changes: 68 additions & 0 deletions .dev/features/init-preflight-first/PLAN.md
Original file line number Diff line number Diff line change
@@ -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").
41 changes: 41 additions & 0 deletions .dev/features/init-preflight-first/REGRESSION.md
Original file line number Diff line number Diff line change
@@ -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.
66 changes: 66 additions & 0 deletions .dev/features/init-preflight-first/REVIEW.md
Original file line number Diff line number Diff line change
@@ -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).
21 changes: 21 additions & 0 deletions .dev/features/init-preflight-first/SHIP.md
Original file line number Diff line number Diff line change
@@ -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.
45 changes: 45 additions & 0 deletions .dev/features/init-preflight-first/VERIFY.md
Original file line number Diff line number Diff line change
@@ -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.
26 changes: 26 additions & 0 deletions .dev/features/init-preflight-first/regression-report.json
Original file line number Diff line number Diff line change
@@ -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"
}
18 changes: 18 additions & 0 deletions .dev/features/init-preflight-first/verify-report.json
Original file line number Diff line number Diff line change
@@ -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": []
}
}
4 changes: 2 additions & 2 deletions .pharn/writes-scope.json
Original file line number Diff line number Diff line change
@@ -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"
}
Loading
Loading