From 892c1133fa8cdd7898a4075280968385bfe726fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Galarowicz?= Date: Sat, 26 Sep 2026 00:37:57 +0200 Subject: [PATCH 1/2] =?UTF-8?q?feat(models):=20pharn-oss=20owns=20the=20mo?= =?UTF-8?q?dels=20block=20=E2=80=94=20init=20copies=20it,=20update=20migra?= =?UTF-8?q?tes=20it,=20status=20labels=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Roadmap Phase 2.0. The CLI hardcoded its own `models` default (ids `opus-4-8`/`sonnet-5`/…, a top-level `default`) that pharn-oss's own checker REDs three times, that `claude --model sonnet-5` rejects, and that disagreed with the installed command frontmatter. - pharn-oss now owns the `models` schema and defaults (CLAUDE.md amended). `init` copies pharn-oss's root `models` block verbatim — none when it ships none — checked at the ingest boundary. - The CLI's MODEL_IDS / EFFORT_LEVELS / PIPELINE_STAGES are retired. `lib/model-config.ts` is a copy of pharn-oss's `check-model-config.mjs` validate/resolve rules, pinned by `tests/model-config-parity.test.ts` against the vendored checker (sha256, constants read from its source, 93-case verdict + RED-line + resolve parity). A copy, not a call: the CLI never executes a file it installs (THREAT-MODEL §1). - `update` decides the block with the per-file rows (`decideFileAction`, over the block's hash, recorded as `pharn.config.json#/models`): absent → restored, pharn's (record or an old default) → updated, the user's → kept, `--force` → config backed up, then replaced. An edited pre-0.7.0 block is converted (default → stages.default, old ids → aliases), never reset; anything unconvertible is named and left as is. The migration reaches current installs; the gate is bounded. - `status` and `init` show every product stage resolved, under a truthful label: Claude Code applies each command's frontmatter; the block is the source of truth it is held to. - `readPharnConfig` no longer validates `models`, so no command refuses to run over a block this CLI does not own. Pipeline: .dev/features/models-pharn-oss-owned/ (PLAN, GRILL, REGRESSION, VERIFY, REVIEW, SHIP). Floor verdicts (iteration 2): validate=0, regress=no-regressions, verify=PASS, check-ship=STOP_GREEN. GATE 1 and GATE 2 were decided by the model under the maintainer's delegation of 2026-09-25. Co-Authored-By: Claude Opus 5.5 --- .dev/features/models-pharn-oss-owned/GRILL.md | 104 ++++ .dev/features/models-pharn-oss-owned/PLAN.md | 304 +++++++++++ .../models-pharn-oss-owned/REGRESSION.md | 45 ++ .../features/models-pharn-oss-owned/REVIEW.md | 160 ++++++ .dev/features/models-pharn-oss-owned/SHIP.md | 74 +++ .../features/models-pharn-oss-owned/VERIFY.md | 49 ++ .../regression-report.json | 60 +++ .../models-pharn-oss-owned/verify-report.json | 17 + .pharn/writes-scope.json | 4 +- CHANGELOG.md | 18 + CLAUDE.md | 10 +- docs/commands/add.md | 2 +- docs/commands/init.md | 7 +- docs/commands/list.md | 2 +- docs/commands/status.md | 15 +- docs/commands/update.md | 52 +- docs/contributing.md | 32 +- docs/reference/pharn-config.md | 130 +++-- docs/reference/pharn-records.md | 19 +- docs/roadmap.md | 4 +- docs/troubleshooting.md | 40 +- src/commands/list.ts | 6 +- src/commands/status.ts | 72 ++- src/commands/update.ts | 178 ++++++- src/lib/install-records.ts | 26 +- src/lib/model-config-format.ts | 62 +++ src/lib/model-config.ts | 202 ++++++++ src/lib/model-routing-format.ts | 36 -- src/lib/model-routing.ts | 170 ------ src/lib/models-update.ts | 301 +++++++++++ src/lib/pharn-config.ts | 42 +- src/lib/proxy-env.ts | 2 +- src/lib/upstream-models.ts | 92 ++++ src/steps/install-archetype.ts | 80 ++- src/steps/overwrite-check.ts | 2 +- src/types.ts | 52 +- .../fixtures/pharn-oss/check-model-config.mjs | 424 +++++++++++++++ tests/init-archetype.test.ts | 169 +++++- tests/install-records.test.ts | 31 ++ tests/list.test.ts | 10 +- tests/model-config-format.test.ts | 90 ++++ tests/model-config-parity.test.ts | 338 ++++++++++++ tests/model-config.test.ts | 109 ++++ tests/model-routing-format.test.ts | 50 -- tests/model-routing.test.ts | 216 -------- tests/models-update.test.ts | 484 ++++++++++++++++++ tests/overwrite-check.test.ts | 16 +- tests/pharn-config.test.ts | 64 ++- tests/status.test.ts | 109 +++- tests/update.test.ts | 382 +++++++++++++- tests/upstream-models.test.ts | 142 +++++ 51 files changed, 4376 insertions(+), 729 deletions(-) create mode 100644 .dev/features/models-pharn-oss-owned/GRILL.md create mode 100644 .dev/features/models-pharn-oss-owned/PLAN.md create mode 100644 .dev/features/models-pharn-oss-owned/REGRESSION.md create mode 100644 .dev/features/models-pharn-oss-owned/REVIEW.md create mode 100644 .dev/features/models-pharn-oss-owned/SHIP.md create mode 100644 .dev/features/models-pharn-oss-owned/VERIFY.md create mode 100644 .dev/features/models-pharn-oss-owned/regression-report.json create mode 100644 .dev/features/models-pharn-oss-owned/verify-report.json create mode 100644 src/lib/model-config-format.ts create mode 100644 src/lib/model-config.ts delete mode 100644 src/lib/model-routing-format.ts delete mode 100644 src/lib/model-routing.ts create mode 100644 src/lib/models-update.ts create mode 100644 src/lib/upstream-models.ts create mode 100644 tests/fixtures/pharn-oss/check-model-config.mjs create mode 100644 tests/model-config-format.test.ts create mode 100644 tests/model-config-parity.test.ts create mode 100644 tests/model-config.test.ts delete mode 100644 tests/model-routing-format.test.ts delete mode 100644 tests/model-routing.test.ts create mode 100644 tests/models-update.test.ts create mode 100644 tests/upstream-models.test.ts diff --git a/.dev/features/models-pharn-oss-owned/GRILL.md b/.dev/features/models-pharn-oss-owned/GRILL.md new file mode 100644 index 00000000..945659a4 --- /dev/null +++ b/.dev/features/models-pharn-oss-owned/GRILL.md @@ -0,0 +1,104 @@ +# GRILL — models-pharn-oss-owned + +Plan: `.dev/features/models-pharn-oss-owned/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 was written on `c40a40c` (= `origin/main`). + +## Findings + +### Guarantee audit (P0) + +```yaml +- type: FINDING + rule_id: 'P0' + severity: minor + file: '.dev/features/models-pharn-oss-owned/PLAN.md:77' + problem: 'The parity claim rests on a corpus. The plan labels the structural equivalence beyond the corpus advisory, but it does not say the corpus must reach every RED branch the checker has. Make that a checked property: the set of RED kinds the checker emits over the corpus must cover every kind its validate path can emit (shape, default, stage, entry, model, effort, and json for the file-level read), read from the checker''s own red("…") call sites, so a new upstream branch cannot slip past a corpus that never exercises it.' + evidence: 'its verdicts and RED lines are compared with the copy''s over a corpus. A divergent copy cannot pass.' +- type: FINDING + rule_id: 'P0' + severity: minor + file: '.dev/features/models-pharn-oss-owned/PLAN.md:77' + problem: 'The sha256 pin proves the vendored file was not edited; it cannot prove it equals live upstream. The plan names that residual, but nothing tells the next maintainer how to refresh the copy. Write the procedure down (docs/contributing.md): copy the file from pharn-oss, update the pin and its commit, run the parity test, and update model-config.ts until it passes.' + evidence: 'That the vendored copy equals LIVE upstream is advisory (checked when it is refreshed)' +``` + +### Eval coverage (P1) + +```yaml +- type: FINDING + rule_id: 'P1' + severity: minor + file: '.dev/features/models-pharn-oss-owned/PLAN.md:128' + problem: 'The same-version migration re-opens update''s early return. Pin both halves: the first run converts, and a second run at the same version returns "Already up to date" without fetching. A block whose only leftover is a top-level `default` that could not move (stages.default already set) must not keep the gate open forever.' + evidence: 'The same-version early return is skipped while the block still holds something to convert (`needsModelsConversion`), so a current install is migrated; once converted it no longer re-opens the gate.' +- type: FINDING + rule_id: 'P1' + severity: minor + file: '.dev/features/models-pharn-oss-owned/PLAN.md:193' + problem: 'The coverage ratchet (97/92/97/97) applies to src/. Four new modules and a rewritten update path need every branch exercised, including `models: null`, a `stages` that is not an object during conversion, and a `__proto__` stage key.' + evidence: 'rows 1-6 and `--force` on a real tree' +``` + +### Trust propagation (P2) + +```yaml +- type: FINDING + rule_id: 'P2' + severity: minor + file: '.dev/features/models-pharn-oss-owned/PLAN.md:229' + problem: 'RED details quote untrusted keys and values through JSON.stringify, which escapes C0 controls but not C1 controls or Unicode format characters (U+202E). The plan routes them through terminalSafe; make sure the update MODELS note and the init warning do too, not only status.' + evidence: 'RED details for invalid input go through `terminalSafe`.' +``` + +### One axis of change (P3) + +```yaml +- type: FINDING + rule_id: 'P3' + severity: minor + file: '.dev/features/models-pharn-oss-owned/PLAN.md:162' + problem: 'models-update.ts holds the record key and hash, but init writes that record too, so init would import update''s module. The key is a key of pharn.records.json: keep it, and the hash that fills it, in lib/install-records.ts (the store''s own vocabulary, which init already imports). models-update.ts then changes only for update''s treatment of the block.' + evidence: '`src/lib/models-update.ts` — new, lib: the old format (ids, historical defaults), conversion, the record key + hash, and the update decision' +- type: FINDING + rule_id: 'P3' + severity: important + file: '.dev/features/models-pharn-oss-owned/PLAN.md:241' + problem: 'CONSTITUTION.md P3 states "this CLI owns the pharn.config.json schema". The maintainer''s decision makes that false for `models`. P3''s enforceable clause (one change-reason per file, no sibling leaf imports) is still met — pharn-oss''s rules sit in their own file, as `testResults`/`ship` already sit outside CLI_OWNED_KEYS — but the sentence is now stale. It is human-only; the plan surfaces it, and SHIP.md, the PR body and the final report must too.' + evidence: '`CONSTITUTION.md` P3: "this CLI owns the `pharn.config.json` schema" — now true for every key except `models`' +``` + +### Determinism (P5) + +```yaml +- type: FINDING + rule_id: 'P5' + severity: minor + file: '.dev/features/models-pharn-oss-owned/PLAN.md:113' + problem: '"Byte-equal" is defined as JSON.stringify equality, so a block with the old default''s values in a different key order reads as edited and is converted rather than replaced. That is the conservative direction (the values are kept) and matches the brief''s wording, but say it in the docs so nobody reads it as a bug.' + evidence: '"Byte-equal" is `JSON.stringify` of the parsed block — the exact serialization pharn wrote, key order included' +``` + +### Honest scope (P7) + +```yaml +- type: FINDING + rule_id: 'P7' + severity: minor + file: '.dev/features/models-pharn-oss-owned/PLAN.md:83' + problem: 'A re-run `init` still replaces an edited models block with pharn-oss''s (the documented reset), while `update` now keeps it. That asymmetry is inherited, not introduced, and the brief does not ask to change it. Name it in the docs and leave it as a follow-up rather than extend this increment.' + evidence: '`init` writes the block **verbatim** (a JSON deep copy) on `ok`' +``` + +## Summary + +The plan is grounded: the failure is measured against the checker this run, the delegation choice is +argued from two trusted documents rather than preference, and the per-file rows are reused instead +of copied. The concerns are about pinning what the plan already claims — a branch-complete parity +corpus, a written refresh procedure, both halves of the same-version migration — plus moving the +record key to the store it belongs to, and carrying one human-only reconciliation (CONSTITUTION P3) +all the way to the PR. + +ADVISORY VERDICT: 9 concerns raised (0 blocking-severity, 1 important, 8 minor) — for the human to +weigh before /pharn-dev-build. Folded into the build under the plan-approval delegation. diff --git a/.dev/features/models-pharn-oss-owned/PLAN.md b/.dev/features/models-pharn-oss-owned/PLAN.md new file mode 100644 index 00000000..3891b826 --- /dev/null +++ b/.dev/features/models-pharn-oss-owned/PLAN.md @@ -0,0 +1,304 @@ +# PLAN — models-pharn-oss-owned (pharn-oss owns the `models` schema; init copies it, update migrates it) + +- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4 +- increment: roadmap Phase 2.0 (token-reduction roadmap, approved by the maintainer 2026-09-25). + The `models` block in `pharn.config.json` stops being a CLI-owned schema. `pharn init` copies + pharn-oss's own root `models` block instead of writing a hardcoded default, `pharn update` + migrates the block the CLI used to write, the CLI's own model/effort/stage enums are retired in + favour of a copy of pharn-oss's checker rules that a parity test pins to that checker, and + `pharn status` shows the resolved per-stage values under a truthful label. +- layer(s): the CLI itself (`src/lib`, `src/steps`, `src/commands`), tests, docs +- constitution_refs: [P0, P1, P2, P3, P4, P5, P6, P7] + +## Why (P7 — a real failure, measured) + +Pre-check P2 (maintainer, 2026-09-25, pharn-cli 0.5.0; unchanged in 0.6.0). Re-verified this run +against pharn-oss `main` @ `767bf61` (SKILLS_VERSION 6.22.0), local clone equal to `origin/main`: + +- `src/lib/model-routing.ts` hardcodes `MODEL_IDS` (`opus-4-8`, `sonnet-5`, `fable-5`, `haiku-4-5`), + `EFFORT_LEVELS` (`low`, `high`, `max`), `PIPELINE_STAGES` (7 dev stages) and + `DEFAULT_MODEL_ROUTING`; `src/steps/install-archetype.ts:229` writes that default into every + user's config. +- pharn-oss's checker, which ships into every install as `pharn/floor/check-model-config.mjs`, REDs + that default **three times** — measured: `missing required default stage entry`, and `stage + "plan"` / `stage "review"` `model "opus-4-8" is not an alias {sonnet, opus, haiku, fable, + inherit} nor a claude-* id` (exit 1). pharn-oss's own root block is GREEN (12 stages). +- `claude --model sonnet-5` → 404 `unrecognized_model` (maintainer's pre-check). +- The block disagrees with the installed command frontmatter (pharn-oss: plan = `opus`/`high`; the + CLI: `opus-4-8 · max`), so `pharn status` prints a table that nothing applies. + +## Discovery — verified this run (P6) + +- pharn-oss's checker (`pharn/floor/check-model-config.mjs`, 424 lines, dependency-free, runs + `main()` at import and `process.exit`s): `validate` reads `models.stages` only; `models` or + `stages` absent/null → GREEN by design; `default` required inside `stages`; stage keys ⊆ + `PRODUCT_STAGES` (11: spec, plan, grill, build, regress, verify, ship, loop, review, + memory-promote, ac-test) ∪ {`default`}; `model` ∈ {sonnet, opus, haiku, fable, inherit} or + `/^claude-[a-z0-9][a-z0-9-]*$/`; `effort` ∈ {low, medium, high, xhigh, max}; extra keys inside an + entry or beside `stages` are ignored; resolution is `Object.hasOwn(stages, s) ? stages[s] : + stages.default`. It passes this repo's eslint unchanged (measured). +- pharn-oss's root `pharn.config.json` carries `models.stages` for all 11 stages + `default`; the + codeload tarball the CLI already downloads contains it. Nothing in pharn-oss reads the block at + run time; Claude Code applies each command's static `model:`/`effort:` frontmatter, and the + checker's `agreement` mode holds the two equal (pharn-oss `LIMITS.md` §8). +- Two defaults were ever written by this CLI (`git log -- src/lib/model-routing.ts`): `74c5653` + (2026-07-07) … `a8131a2~1` wrote `review: fable-5/max`; `a8131a2` (#57, 2026-07-23) onward writes + `review: opus-4-8/high`. Both: `default: sonnet-5/high`, `plan: opus-4-8/max`. Every published + release (0.3.0+) carries the second. +- `readPharnConfig` validates `models` strictly and THROWS `ModelRoutingError`; every command + refuses to run on a bad block, and the new pharn-oss format would itself be refused. +- `update` returns "Already up to date" at the same skills version before fetching, so a + migration that only runs past that return would never reach a current install. +- `pharn.records.json` keys are compared, never path-joined (`install-records.ts` header); `add` + and `remove` spread/prefix-filter `files`, `update` rebuilds it from the manifest. +- THREAT-MODEL §1: pharn never executes the files it installs. §3.1: every network-derived field is + validated at its ingest boundary before it reaches the config write. + +## The validation choice (task item 3) — a pinned copy, not delegation + +**Chosen: keep a local copy of pharn-oss's `validate`/`resolve` rules, pinned to pharn-oss's checker +by a parity test.** Delegating to the installed `pharn/floor/check-model-config.mjs` would mean +the CLI EXECUTES a file from the user's project (or, at `init`, from the downloaded clone): + +1. THREAT-MODEL §1 (human-only) states "pharn never executes them". Delegation contradicts a + trusted doc this loop may not edit. +2. It would change a real security property: `pharn status --strict` runs in CI on pull requests, + and today it only reads and hashes. Executing `pharn/floor/*.mjs` there runs whatever a PR put in + that file. +3. At `init` the checker exists only in the untrusted clone; running it would execute downloaded + code before the user has seen anything. + +One owner per rule is kept by construction: the CLI keeps **no rule of its own** (`MODEL_IDS`, +`EFFORT_LEVELS`, `PIPELINE_STAGES` are deleted outright). The copy lives in its own file +(`src/lib/model-config.ts`, one axis: pharn-oss's models rules), and the parity test makes pharn-oss +the owner of every rule in it: the vendored checker is byte-pinned (sha256 + upstream commit), the +stage/alias/regex/effort constants are READ OUT of its source and compared, and its verdicts and RED +lines are compared with the copy's over a corpus. A divergent copy cannot pass. This is the repo's +existing pattern for an upstream rule: the capability-index frontmatter fence ("upstream's own rule, +ported literally, differential-tested") and the seam lockstep test. + +**The residual, named (LIMITS §3e):** released CLIs read `main` HEAD. If pharn-oss widens its rules +(a new stage, a new alias) without a `MIN_CLI` bump, an older CLI's copy rejects the new block. The +blast radius is bounded the §3e way: the block is **not applied** and the rejection is **named** +(with "upgrade pharn"); the install/update otherwise completes, and the next `pharn update` after an +upgrade restores it. The lever is upstream's `MIN_CLI`. + +## Design + +### The block's rules — `src/lib/model-config.ts` (pure; the pinned copy) + +`PRODUCT_STAGES` (the 11 keys, upstream's order), `MODEL_ALIASES`, `MODEL_ID_RE`, `EFFORT_LEVELS`, +`checkModelsBlock(value)` → `no-stages` | `valid {stages}` | `invalid {reds[]}` (a literal port of +`stagesOf` + `validateStages`, same RED kinds, same detail text, same order), and +`resolveStageModel(stages, stage)` (the own-property pick). + +### What `init` writes — `src/lib/upstream-models.ts` + `src/steps/install-archetype.ts` + +`readUpstreamModels(repoDir)` reads the clone's root `pharn.config.json` through `readBoundedFile` +(the clone holds no symlinks: `tar-extract.ts` rejects them) → `absent` (no file, or no/null `models`) +| `ok {block}` | `invalid {reds}` (unreadable, not JSON, not an object, or the pinned copy REDs it). +`init` writes the block **verbatim** (a JSON deep copy) on `ok`, writes **no** `models` key on +`absent` (never invents one), and on `invalid` writes none and warns, naming each RED. The written +block's hash goes into `pharn.records.json` under the key `pharn.config.json#/models`, so a later +`update` can tell pharn's block from the user's. The outro's "Models per stage" becomes the resolved +view under the truthful label below. + +### How `update` treats the block — `src/lib/models-update.ts` (pure) + `src/commands/update.ts` + +The CLI's knowledge of its OWN old format lives here (it did own that format): the id map +`opus-4-8→opus`, `sonnet-5→sonnet`, `fable-5→fable`, `haiku-4-5→haiku`, and the two historical +defaults. `decideModelsUpdate` **reuses `decideFileAction`** (lib/update-decision.ts) with the +block's hash as the "file" hash — so the rows are the per-file rows by construction, not a copy: + +| row | the user's block | default | `--force` | +| --- | ------------------------------------------------------- | --------------------------------- | ------------------------------ | +| 1 | absent (`models` key missing) | write pharn-oss's — `restored` | same | +| 2 | equal to pharn-oss's | no-op — `ok` | same | +| 3 | byte-equal to a default an older pharn wrote | replace with pharn-oss's — `updated` | same | +| 3 | equal to the recorded hash (pharn wrote it) | replace with pharn-oss's — `updated` | same | +| 4-6 | edited / no record / no usable records | KEPT (`modified`/`unrecorded`/`unverifiable`) | back up `pharn.config.json`, then replace | + +"Byte-equal" is `JSON.stringify` of the parsed block — the exact serialization pharn wrote, key order +included; indentation is not an edit. A historical default is proof of authorship that needs no +records file, so it is fed to the table as the record. + +A KEPT block in the old format is **converted, never reset**: a top-level `default` moves into +`stages.default` (unless `stages.default` exists, or `stages` is not an object — then it stays and is +named), each old id maps to its alias, everything else is copied verbatim. The converted block is then +checked with the pinned copy, and each remaining RED is reported by name ("could not convert — left +as is"). Nothing is dropped, nothing is guessed. A converted or kept block carries its previous record +forward, never a fresh one, so the next run still reads it as the user's. + +When pharn-oss's block is absent or fails the pinned copy, the user's block is kept (converted if it +is in the old format) and the failure is named. The block's outcome never withholds the +`skillsVersion` bump — it is configuration, not bytes of a version, and a customised block is a +supported steady state. The same-version early return is skipped while the block still holds +something to convert (`needsModelsConversion`), so a current install is migrated; once converted it +no longer re-opens the gate. `update` prints a `MODELS` note for every outcome except `ok`. + +### What `status` shows — `src/lib/model-config-format.ts` + `src/commands/status.ts` + +Every product stage, resolved (its own entry, or `default` marked as such), under the label: Claude +Code applies each `/pharn-*` command's own `model:`/`effort:` frontmatter, not this block; the block +is the source of truth that frontmatter is held to — `node /check-model-config.mjs agreement` +checks the two. An old-format block says `pharn update` converts it; an invalid block lists its REDs +(through `terminalSafe`). Absent → no note (unchanged). `--strict` does not gate on this section: +pharn-oss's checker owns that verdict. + +### The config loader — `src/lib/pharn-config.ts` + +`models` is read and written back **verbatim** and no longer validated at load, so the old format +(which `update` must be able to load to migrate), pharn-oss's format, and a future pharn-oss format +all load, and no unrelated command refuses to run over a block this CLI does not own. +`ModelRoutingError` leaves `isConfigValidationError`. `models` stays in `CLI_OWNED_KEYS`: pharn still +WRITES the key (init copies it, update manages it), so a re-run `init` rewrites it rather than +carrying a stale one — pharn-oss owns what goes inside. + +## Files + +- `src/lib/model-config.ts` — new, lib: the pinned copy of pharn-oss's models rules (validate, resolve). +- `src/lib/model-config-format.ts` — new, lib: the resolved-per-stage display lines (pure). +- `src/lib/models-update.ts` — new, lib: the old format (ids, historical defaults), conversion, and + the update decision (reusing `decideFileAction`). +- `src/lib/upstream-models.ts` — new, lib: `readUpstreamModels(repoDir)`, the fetch-boundary read. +- `src/lib/model-routing.ts` — deleted (its enums are retired). +- `src/lib/model-routing-format.ts` — deleted (replaced by `model-config-format.ts`). +- `src/types.ts` — the CLI-owned model types go; `models?: unknown`, documented as pharn-oss's. +- `src/lib/pharn-config.ts` — no load-time `models` validation; comments. +- `src/lib/install-records.ts` — the models record key (`pharn.config.json#/models`) and the hash + that fills it — the store's own vocabulary, which `init` already imports (grill, P3); the header + names the one non-path key. +- `src/lib/proxy-env.ts` — a comment names the old module; repointed. +- `src/steps/install-archetype.ts` — copy + record pharn-oss's block; truthful outro. +- `src/steps/overwrite-check.ts` — a comment names `ModelRoutingError`; corrected. +- `src/commands/update.ts` — the decision, the gate, the backup, the records, the `MODELS` note. +- `src/commands/status.ts` — the `MODELS` note. +- `src/commands/list.ts` — a comment names the `models` error; corrected. +- `tests/fixtures/pharn-oss/check-model-config.mjs` — new: pharn-oss's checker, byte-for-byte + (`767bf61`), the parity reference. +- `tests/model-config.test.ts` — new: the copy's rules. +- `tests/model-config-parity.test.ts` — new: sha256 pin, constants read from the checker's source, + verdict + RED-line + resolve parity over a corpus (the checker run as a subprocess). +- `tests/model-config-format.test.ts` — new: the display. +- `tests/models-update.test.ts` — new: conversion + every decision row. +- `tests/upstream-models.test.ts` — new: the clone read. +- `tests/model-routing.test.ts` — deleted. +- `tests/model-routing-format.test.ts` — deleted. +- `tests/init-archetype.test.ts` — the block copied verbatim + recorded; none when upstream has none; + none + a named warning when it is invalid; the truthful outro. +- `tests/update.test.ts` — the rows end to end on a real tree, the same-version migration, `--force` + backing up the config, conversion reporting. +- `tests/status.test.ts` — the resolved view, the label, the old-format and invalid notes. +- `tests/pharn-config.test.ts` — a bad or old-format `models` block loads verbatim. +- `tests/list.test.ts` — its config-error stand-in moves from `ModelRoutingError` to `SeamConfigError`. +- `tests/overwrite-check.test.ts` — its throwing-config stand-in moves from `models` to `seam`. +- `tests/install-records.test.ts` — the models record key is a valid store key. +- `docs/reference/pharn-config.md` — the `models` section rewritten (pharn-oss's schema, what each + command does, the truthful label); the table row; `init` re-run wording. +- `docs/reference/pharn-records.md` — the `pharn.config.json#/models` key. +- `docs/commands/init.md` — what init writes for `models`. +- `docs/commands/update.md` — the `models` rows and the old-format migration. +- `docs/commands/status.md` — the `MODELS` section. +- `docs/commands/add.md` — a bad `models` block no longer stops the command. +- `docs/commands/list.md` — same. +- `docs/roadmap.md` — the two `models` rows. +- `docs/troubleshooting.md` — the `models` error entry replaced. +- `docs/contributing.md` — module list and test table. +- `CLAUDE.md` — "This CLI still owns the `pharn.config.json` schema" amended: every key except + `models`; the architecture notes follow the code. +- `CHANGELOG.md` — `[Unreleased]`. + +### Not touched + +- `CONSTITUTION.md`, `THREAT-MODEL.md`, `ARCHITECTURE.md`, `LIMITS.md` — human-only; see the + reconciliations below. +- `src/lib/update-decision.ts` — reused as is. +- `.claude/settings.json` handling, `seam` — unchanged. + +## Contracts satisfied + +- pharn-oss `pharn/floor/check-model-config.mjs` (`validate`, `resolve`) — the schema, cited and + pinned; not restated as a CLI contract (P4). +- The per-file update decision (`lib/update-decision.ts`) — reused, not re-implemented. +- THREAT-MODEL §3.1 — the new network-derived field is validated at its ingest boundary. + +## Evals to write (P1) + +- Parity: for every corpus config, the copy's verdict and RED lines equal the checker's + (`node check-model-config.mjs validate --config `), and `resolve` output is equal for every + product stage + `default` + inherited names (`constructor`, `toString`, `__proto__`). +- Parity: the checker's `PRODUCT_STAGES` keys, `MODEL_ALIASES`, `MODEL_ID_RE`, `EFFORT_ENUM`, + evaluated from its source, equal the copy's constants; the vendored file's sha256 equals the pin. +- Both historical defaults: the checker REDs them (the failure this fixes); `update` replaces them. +- An edited old-format block → converted: ids mapped, `default` moved, values kept; an unmappable + model / unknown stage / conflicting `default` → reported by name, left verbatim. +- `init`: upstream block copied byte-for-byte (JSON) + recorded; absent → no `models` key; invalid → + no key + a warning naming the RED; the written block passes the checker. +- `update`: rows 1-6 and `--force` on a real tree; the same-version run migrates, and the next one + returns "Already up to date"; `--force` backs up `pharn.config.json`; a kept block never withholds + the version; the models record survives `planUpdate`'s manifest-keyed rebuild. +- `status`: all 11 stages resolved with `(default)` marks; the label; old-format and invalid notes. +- `readPharnConfig`: bad and old-format `models` blocks load and round-trip verbatim. + +## Guarantee audit (P0) + +- "The CLI's models rules are pharn-oss's" → floor: content-hash (the vendored checker's sha256 + pin) + enum equality (constants read out of its source) + exit-code equality over a corpus. + Beyond the corpus the structural equivalence is advisory. That the vendored copy equals LIVE + upstream is advisory (checked when it is refreshed); drift is the named §3e residual. +- "`init` writes pharn-oss's block, never an invented one" → floor: presence test + the pinned + enum/regex check at the ingest boundary. +- "`update` never overwrites a block the user edited without `--force`" → floor: sha256 equality + (block vs record) and exact string equality (block vs a historical default), through + `decideFileAction`. +- "Nothing unconvertible is dropped or guessed" → floor: the conversion maps only members of a fixed + 4-entry table and moves one key; every other value is copied verbatim; what remains is the pinned + checker's RED set, printed. +- "`--force` loses nothing" → floor: `createBackup` of `pharn.config.json` before any write. +- "The status label is truthful" → advisory (wording), pinned by a test. That Claude Code applies + frontmatter is pharn-oss's claim (its LIMITS §8), cited. + +## Trust audit (P2) + +- The clone's root `pharn.config.json` (untrusted): bounded, non-blocking read; parsed as data; + checked by the pinned copy before it can reach the config write; written with `JSON.stringify`. + Extra keys inside the block are carried verbatim and are inert (nothing reads them). A valid block's + printed values are enum/regex members (a safe charset); RED details for invalid input go through + `terminalSafe`. +- The user's `models` block (local, hand-editable, untrusted): hashed, compared, converted and shown — + never path-joined. Conversion builds objects with own-property semantics, so a `__proto__` key stays + data. +- The `pharn.config.json#/models` record key: compared, never path-joined (the store's rule). + +## Determinism audit (P5) + +- Every branch is a membership or equality test: key presence, hash equality, historical-default + string equality, legacy-id table membership, the pinned enums. The terminal fallback is "keep the + user's block and name what is wrong" — never a guessed mapping, never a silent drop. + +## Doc reconciliations for the human (never agent-edited) + +- `CONSTITUTION.md` P3: "this CLI owns the `pharn.config.json` schema" — now true for every key + except `models`, whose schema pharn-oss owns (the maintainer's decision). P3's enforceable clause + (one axis per file, no sibling imports) still holds: pharn-oss's rules live in their own file. + `testResults`/`ship` were already upstream-owned keys in the same file. +- `THREAT-MODEL.md` §3.1: lists `models` among the local-origin fields ("the `models`/`seam` + defaults"). It is now a fourth network-derived field, validated at ingest by `checkModelsBlock`. + +## Grill fold-ins (GRILL.md, all advisory; folded under the plan-approval delegation) + +- The record key and its hash live in `install-records.ts`, not `models-update.ts` (P3). +- The parity corpus must reach every RED kind the checker's validate path can emit, read from its own + `red("…")` call sites (P0). +- `docs/contributing.md` gets the refresh procedure for the vendored checker (P0). +- Both halves of the same-version migration are tested; a block whose only leftover cannot move does + not keep the gate open (P1). +- The `update` MODELS note and the `init` warning use `terminalSafe` too (P2). +- The docs say that a reordered old default reads as edited (P5), and that a re-run `init` still + replaces an edited block (P7, an inherited asymmetry left as a follow-up). + +## Open questions (HALT) + +None. The maintainer's brief (2026-09-25) fixed the ownership decision, the id map, the migration +rules and the release steps; the one open choice (delegate vs. pinned copy) is decided above with its +reasons, under the plan-approval delegation. diff --git a/.dev/features/models-pharn-oss-owned/REGRESSION.md b/.dev/features/models-pharn-oss-owned/REGRESSION.md new file mode 100644 index 00000000..9d14d3d9 --- /dev/null +++ b/.dev/features/models-pharn-oss-owned/REGRESSION.md @@ -0,0 +1,45 @@ +# REGRESSION — models-pharn-oss-owned + +**Iteration 2** (after the GATE 2 "fix" decision — see `SHIP.md`). Re-run over the fixed tree with the +same base and gate set; the numbers below are this run's, and they match iteration 1 exactly. + +The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment. + +## Base and partition + +- **base:** `c40a40c48555d6bce378e42567551f5b3ae26525` (`HEAD` = `origin/main` after #230; the build is + an uncommitted working tree on top of it, so `git status --porcelain` is non-empty → `base = HEAD`). +- **inside:** the 42 paths `git diff --name-only HEAD` + untracked-new files report, each declared in + `PLAN.md` `## Files` — 4 new `src/lib` modules, 2 deleted ones, 9 changed `src` files, the vendored + pharn-oss checker, 5 new test files, 2 deleted test files, 8 changed test files, 10 docs, `CLAUDE.md` + and `CHANGELOG.md` (full list in `regression-report.json` `.inside`). +- **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 (`eslint.config.mjs`, + `.prettierrc`, `.prettierignore`, `.markdownlint-cli2.jsonc`), so `lint` / `format:check` / + `lint:md` are absent from both maps. +- **environment:** the base ran in a fresh `git worktree add --detach` of the base SHA (no gitignored + `test-*/` trees on either side); both sides ran with the proxy variables unset. + +## Per-gate exit codes + +| gate | base | head | flipped? | +| ---------- | ---- | ---- | -------- | +| `tests` | 0 | 0 | no | +| `validate` | 0 | 0 | no | + +`node --test`: 754 tests, 754 pass, 0 fail on both sides. `validate`: `FLOOR: GREEN` on both sides. + +- `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/models-pharn-oss-owned/REVIEW.md b/.dev/features/models-pharn-oss-owned/REVIEW.md new file mode 100644 index 00000000..60a6cb5b --- /dev/null +++ b/.dev/features/models-pharn-oss-owned/REVIEW.md @@ -0,0 +1,160 @@ +# REVIEW — models-pharn-oss-owned (iteration 1) + +Floor first: `node .dev/floor/validate.mjs .` → `FLOOR: GREEN`. Everything below is **advisory**. The +increment was read as `trust: untrusted`; nothing in it read as an instruction. + +Two sources: the four inline lenses, and an independent read-only bug hunt (a subagent that probed +the built modules with `tsx` scripts). Its findings are marked **[probe]** where it confirmed them by +running code, and each was re-checked against the source before being listed here. + +## Floor-gate findings (blocking) + +None. No guarantee is claimed without a floor reduction or an `advisory` label, no eval binding is +missing, and no sibling-leaf import was added. + +## Advisory findings + +### Correctness + +```yaml +- type: FINDING + rule_id: 'P5' + severity: important + file: 'src/lib/models-update.ts:207' + problem: 'When pharn-oss ships no block, or one this pharn rejects, an unedited pre-0.7.0 default is converted but keeps the record it had — none, since no earlier pharn recorded the block. After conversion it no longer equals an old default and has no record, so every later update reads it as the user''s (`unrecorded`) and pharn-oss''s block is never applied without --force. [probe: run1 upstream-invalid, record null; run2 kept unrecorded]' + evidence: 'nextRecord: recorded,' +- type: FINDING + rule_id: 'P2' + severity: important + file: 'src/lib/upstream-models.ts:73' + problem: 'The checker ignores keys it does not read, so a block with a very deeply nested value under a sibling key passes, and JSON.stringify then overflows the stack — in init after the files are copied and before the config is written, leaving a half-installed project. The fetched block is untrusted; the read must refuse what it cannot serialize. [probe: kind ok, then RangeError from modelsRecordHash]' + evidence: 'return { kind: ''ok'', block };' +- type: FINDING + rule_id: 'P5' + severity: minor + file: 'src/commands/update.ts:227' + problem: 'The same-version gate re-opens while the block holds something convertible. If pharn-oss''s own block ever carried a top-level `default`, update would write it, find it convertible again, and re-open the gate on every run. Nothing bounds that loop. Close the gate for a block that is exactly the one pharn recorded writing. [probe: ok, needs-again true]' + evidence: 'const convertModels = needsModelsConversion(config.models);' +- type: FINDING + rule_id: 'P5' + severity: minor + file: 'src/commands/update.ts:640' + problem: 'The models record is written with the records, before the config. If the config write then fails while the stamp still matches (a withheld bump, or a same-version run), the next run compares the old block with the new record and keeps it as `modified`. This fails safe — no edit is ever overwritten — and it mirrors the existing records-before-config order. Accepted, named.' + evidence: '...(models.nextRecord !== null' +- type: FINDING + rule_id: 'P7' + severity: minor + file: 'src/commands/update.ts:548' + problem: '`--force` over an edited block backs up pharn.config.json; createBackup refuses a symlinked path, so a symlinked config now makes such a run abort, before any write, with a named message. Before this change it succeeded. Accepted: the refusal is createBackup''s deliberate posture, and it writes nothing.' + evidence: '? [...plan.backups, CONFIG_FILENAME]' +``` + +### Guarantee wording (L-floor → P0) + +```yaml +- type: FINDING + rule_id: 'P0' + severity: minor + file: 'src/lib/model-config.ts:18' + problem: '"Why it cannot drift unnoticed" overstates: the copy cannot drift from the VENDORED checker unnoticed, but the vendored checker can lag live upstream until someone refreshes it.' + evidence: 'Why it cannot drift unnoticed: tests/model-config-parity.test.ts runs the real checker' +- type: FINDING + rule_id: 'P0' + severity: minor + file: 'docs/reference/pharn-config.md:185' + problem: '"the copy never gains a rule of its own" is a discipline promise; the test catches an added rule only where the corpus exercises it. Say what the test does.' + evidence: 'and the copy never gains a rule of its own' +- type: FINDING + rule_id: 'P4' + severity: minor + file: 'src/commands/update.ts:243' + problem: 'The pre-confirm note says an edited models block will be OVERWRITTEN under --force, which is false when pharn-oss ships no block or one this pharn rejects — and the note is printed before the fetch that decides it. The MODELS note after the run is the accurate place; drop the claim here.' + evidence: '--force: files you changed, and an edited models block, will be OVERWRITTEN' +- type: FINDING + rule_id: 'P4' + severity: minor + file: 'docs/troubleshooting.md:503' + problem: 'The example cannot be produced: a block with `opus-4-8` takes status''s old-format branch, not the list of REDs. [probe]' + evidence: 'stage "plan" model "opus-4-8" is not an alias' +- type: FINDING + rule_id: 'P4' + severity: minor + file: 'src/steps/install-archetype.ts:421' + problem: 'Stale comment: readPharnConfig no longer throws on a bad `models` block.' + evidence: 'throws on a bad `models`/`seam` hand-edit' +``` + +### Eval coverage (L-eval → P1) + +```yaml +- type: FINDING + rule_id: 'P1' + severity: minor + file: 'src/commands/update.ts:250' + problem: 'The pre-confirm line announcing the conversion is not pinned by any test.' + evidence: 'Your models block is in the format pharn wrote before 0.7.0 — this run converts it.' +``` + +### Trust (L-trust → P2) + +Every untrusted string reaches the terminal through `terminalSafe` (stage names, values, upstream's +reasons); `resolvedStageLines` prints only enum/regex members. The vendored checker is executed only +by tests, never at run time, and is not in the published package (`files: ["dist"]`). The residual: +`terminalSafe` does not strip U+2028/2029 — pre-existing helper behavior, out of scope. + +### Axis (L-axis → P3) + +One reason to change per new file: pharn-oss's rules (`model-config.ts`), their presentation +(`model-config-format.ts`), update's treatment of the block (`models-update.ts`), the fetch-boundary +read (`upstream-models.ts`); the record key sits with the store it keys (`install-records.ts`). No +command→command or step→step import. + +## Human-only reconciliations (not agent-editable; carried to SHIP.md and the PR) + +- `CONSTITUTION.md` P3 — "this CLI owns the `pharn.config.json` schema" is now true for every key + except `models` (the maintainer's decision). P3's enforceable clause still holds. +- `THREAT-MODEL.md` §3.1 — `models` is listed among the local-origin fields; it is now a fourth + network-derived field, validated at ingest by `checkModelsBlock` (`readUpstreamModels`). + +## Follow-ups (out of scope) + +- `tests/seam-config.test.ts:61` names "model-routing's effort" in a test title — cosmetic, not in + this increment's `## Files`. +- A re-run `init` still replaces an edited `models` block while `update` keeps it (inherited, named in + the docs). + +## Verdict + +GREEN floor; **0 floor-gate findings**, 2 important + 9 minor advisory. Two advisory findings are real +defects (the lost authorship of a converted default; an unserializable upstream block) and three more +are cheap to fix — for the human at GATE 2. + +--- + +## Iteration 2 (after the GATE 2 "fix" decision) + +Floor first: `FLOOR: GREEN`. Re-read the fixes as `trust: untrusted`. Each fixed finding has a test that +fails with the fix reverted (checked by reverting it). + +| iteration-1 finding | resolution | +| ------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| converted old default loses its authorship (important) | Fixed — `models-update.ts`: a block that was pharn's (an old default, or the recorded block) takes the hash of what it became as its record. Unit test + an end-to-end update test. | +| unserializable upstream block half-installs (important) | Fixed — `upstream-models.ts` refuses a block `JSON.stringify` cannot serialize, before anything copies it. Unit test + an init test that finishes the install with no `models` key. | +| same-version gate unbounded (minor) | Fixed — `modelsMigrationPending`: a block that is exactly the recorded one does not re-open the gate. Unit test + an update test with a convertible upstream block, closed on the next run. | +| records written before the config (minor) | Accepted and named: fails safe (a block is kept, never overwritten); it follows the existing records-before-config order. | +| `--force` over a symlinked config aborts (minor) | Accepted and named: `createBackup`'s refusal, before any write. | +| "cannot drift unnoticed" (minor) | Fixed — the comment now names the vendored copy's lag. | +| "never gains a rule of its own" (minor) | Fixed — the doc says what the test does. | +| pre-confirm `--force` line over-promises (minor) | Fixed — the line no longer mentions the block; the MODELS note after the run is where it is decided and reported. | +| impossible troubleshooting example (minor) | Fixed — the example now shows a RED that `status` can print (`gpt-4`). | +| stale `readCarriedEntries` comment (minor) | Fixed. | +| pre-confirm conversion line untested (minor) | Fixed — pinned in the same-version migration test. | + +One residual of the gate fix, named: `status` still labels a block with a top-level `default` as "the +format pharn wrote before 0.7.0" even if pharn-oss itself shipped it — `status` does not read the +records file. pharn-oss's checker ignores a top-level `default`, so upstream has no reason to ship one. + +### Verdict (iteration 2) + +GREEN floor; **0 floor-gate findings**; the two important findings are fixed, the rest fixed or +accepted and named. Advisory — the decision is the human's (delegated; `SHIP.md`). diff --git a/.dev/features/models-pharn-oss-owned/SHIP.md b/.dev/features/models-pharn-oss-owned/SHIP.md new file mode 100644 index 00000000..f6c22ce5 --- /dev/null +++ b/.dev/features/models-pharn-oss-owned/SHIP.md @@ -0,0 +1,74 @@ +# SHIP — models-pharn-oss-owned + +Roadmap Phase 2.0 (token-reduction roadmap, approved by the maintainer 2026-09-25): pharn-oss owns the +`models` block; `init` copies it, `update` migrates it, `status` labels it truthfully. + +## Gate decisions — model decisions under delegation, NOT human approvals + +The maintainer delegated both human gates to the model in chat on 2026-09-25 ("Plan approval (GATE 1) +and the merge/fix decision (GATE 2) are delegated to you. Record them in SHIP.md as model decisions made +under that delegation, never as human approvals"). Every decision below was made by the model +(`claude-opus-5-5`) under that delegation. **No human approved any of them.** + +| gate | decision (model, under delegation) | +| --------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| GATE 1 | Plan approved as written, then amended once to fold the grill's findings (record key moved to `install-records.ts`; see `PLAN.md` "Grill fold-ins"). The one open choice — delegate to the installed checker vs. a pinned copy — decided in `PLAN.md`. | +| GATE 2, iter. 1 | **Fix.** `REVIEW.md` iteration 1 carried two important advisory defects (a converted old default losing its authorship; an unserializable upstream block half-installing) plus minor ones. The loop body re-ran within the plan's `## Files`. | +| GATE 2, iter. 2 | **Merge**, once the PR's required checks are green (the maintainer's standing instruction: `gh pr merge --squash`, `--admin` only if review is required; stop if the auto-mode classifier denies the merge). | + +## Stages run, in order + +| # | stage | outcome | +| --- | ------------------------- | ------------------------------------------------------------------------------------ | +| 1 | `/pharn-dev-plan` | `PLAN.md`; GATE 1 (delegated) | +| 2 | `/pharn-dev-grill` | `GRILL.md` — 9 advisory concerns (1 important), folded into the plan | +| 3 | `/pharn-dev-build` | 42 planned paths; `npm run check` GREEN | +| 4 | `/pharn-dev-regress` | `regression-report.json` | +| 5 | `/pharn-dev-verify` | `verify-report.json` | +| 6 | `/pharn-dev-review` | `REVIEW.md` iteration 1 (four lenses + an independent read-only probe); GATE 2 → fix | +| 7 | fix (within `## Files`) | 3 defects fixed, 6 wording/test gaps closed, 2 accepted and named | +| 8 | regress → verify → review | iteration 2; `check-ship.mjs --iter 2 --cap 3` → `STOP_GREEN`; GATE 2 → merge | + +## Structural verdicts read, verbatim + +| stage | verdict source | iteration 1 | iteration 2 | +| ------------------------- | ------------------------------------------ | ------------------ | ------------------ | +| `/pharn-dev-build` | `node .dev/floor/validate.mjs .` exit code | `0` | `0` | +| `/pharn-dev-regress` | `regression-report.json` `.verdict` | `"no-regressions"` | `"no-regressions"` | +| `/pharn-dev-verify` | `verify-report.json` `.verdict` | `"PASS"` | `"PASS"` | +| stop (`--loop` semantics) | `.dev/floor/check-ship.mjs` decision | — | `STOP_GREEN` | + +`.regressions[]`, `.pre_existing[]` and `.failing_gates[]` are empty; `verifiers.registered` is `0`. +`/pharn-dev-review` has no structural verdict and none was invented; it is read at GATE 2. + +## Stage models + +The maintainer asked that each stage run on the model this repo's `pharn.config.json` `models` block +assigns. This repo has no `pharn.config.json`, so no block assigns one: every stage ran on the session +model, `claude-opus-5-5`. (The stage commands' `model_tier:` is PHARN's own frontmatter field, inert +to the platform.) + +## Pointers + +- `REVIEW.md` — iteration 1 findings and the iteration 2 resolution table (read at GATE 2). +- `GRILL.md` (advisory), `REGRESSION.md` / `VERIFY.md` (human renders of the floor verdicts). + +## For the human — reconciliations only you can make + +The trusted docs are human-only (write-protected by `.claude/hooks/protect-trusted-paths.cjs`), so +this run did not edit them: + +- `CONSTITUTION.md` P3 — "this CLI owns the `pharn.config.json` schema" is now true for every key + except `models`, whose schema and defaults pharn-oss owns (your decision of 2026-09-25). P3's + enforceable clause (one change-reason per file, no sibling-leaf imports) still holds: pharn-oss's + rules sit in their own file, `src/lib/model-config.ts`. +- `THREAT-MODEL.md` §3.1 — lists `models` among the local-origin fields ("the `models`/`seam` + defaults"). It is now a fourth network-derived field, validated at its ingest boundary by + `checkModelsBlock` (`readUpstreamModels`). + +--- + +The 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, made here by the model under the +maintainer's explicit delegation and recorded as such. Nothing here is a `PHARN ✓ reviewed` seal, an +approval, or a self-issued "shipped". diff --git a/.dev/features/models-pharn-oss-owned/VERIFY.md b/.dev/features/models-pharn-oss-owned/VERIFY.md new file mode 100644 index 00000000..fce070c4 --- /dev/null +++ b/.dev/features/models-pharn-oss-owned/VERIFY.md @@ -0,0 +1,49 @@ +# VERIFY — models-pharn-oss-owned + +**Iteration 2** (after the GATE 2 "fix" decision — see `SHIP.md`): re-run over the fixed tree. + +The verdict below is computed by `.dev/floor/check-verify.mjs` from the gate exit codes; no verifier +judgment reaches it. + +## Floor layer — gate exit codes + +| gate | exit | +| -------------- | ---- | +| `test` | 0 | +| `validate` | 0 | +| `lint` | 0 | +| `format:check` | 0 | +| `lint:md` | 0 | +| `typecheck` | 0 | + +- `test` is the whole vitest suite with the feature in it: 69 files, 1818 passed, 1 skipped. It + includes `tests/model-config-parity.test.ts`, which runs pharn-oss's own checker + (`tests/fixtures/pharn-oss/check-model-config.mjs`, sha256-pinned to pharn-oss `767bf61`) over a + 93-case corpus and compares every verdict, RED line and resolution with this CLI's copy. +- `validate` is `FLOOR: GREEN` (no markdown capability was added, so it gates structure only). +- No committed eval pair ships with this feature, so there is no `structural:*` gate. +- Not in this map, measured separately during `/pharn-dev-build`: `npm run test:coverage` passes its + ratchet (97.61 / 93.32 / 98.43 / 98.44 against 97 / 92 / 97 / 97), and `npm run build` succeeds. + +**VERIFIED: floor gates PASS** (`verify-report.json` `.verdict` = `PASS`, `failing_gates: []`). + +## Advisory layer — verifiers + +No verifiers registered (`count-verifiers.mjs` → `{"registered":0,"verifiers":[]}`) — floor gates +only. + +## Beyond the gates (advisory, recorded for the human) + +A live run of the built CLI against pharn-oss `main` (`767bf61`, SKILLS_VERSION 6.22.0), in a scratch +project holding the default an earlier pharn wrote: `pharn status --no-drift` reported the old format; +`pharn update --yes` re-opened the same-version gate, replaced the block with pharn-oss's and recorded +it; the installed `pharn/floor/check-model-config.mjs` then printed GREEN for both `validate` (12 +stages) and `agreement` (11/11 product stages agree with command frontmatter); a second `update` +answered "Already up to date". An edited old-format block was converted with its values kept and its +unconvertible `deploy` stage named. The migration was repeated on the fixed build (iteration 2): +`agreement` GREEN, then "Already up to date". This is a demonstration, not a gate. + +--- + +Verified = the named gates passed; this is NOT a guarantee of correctness beyond what those gates +check — verifier concerns are advisory help, not assurance. diff --git a/.dev/features/models-pharn-oss-owned/regression-report.json b/.dev/features/models-pharn-oss-owned/regression-report.json new file mode 100644 index 00000000..7945aad4 --- /dev/null +++ b/.dev/features/models-pharn-oss-owned/regression-report.json @@ -0,0 +1,60 @@ +{ + "base": "c40a40c48555d6bce378e42567551f5b3ae26525", + "inside": [ + "CHANGELOG.md", + "CLAUDE.md", + "docs/commands/add.md", + "docs/commands/init.md", + "docs/commands/list.md", + "docs/commands/status.md", + "docs/commands/update.md", + "docs/contributing.md", + "docs/reference/pharn-config.md", + "docs/reference/pharn-records.md", + "docs/roadmap.md", + "docs/troubleshooting.md", + "src/commands/list.ts", + "src/commands/status.ts", + "src/commands/update.ts", + "src/lib/install-records.ts", + "src/lib/model-config-format.ts", + "src/lib/model-config.ts", + "src/lib/model-routing-format.ts", + "src/lib/model-routing.ts", + "src/lib/models-update.ts", + "src/lib/pharn-config.ts", + "src/lib/proxy-env.ts", + "src/lib/upstream-models.ts", + "src/steps/install-archetype.ts", + "src/steps/overwrite-check.ts", + "src/types.ts", + "tests/fixtures/pharn-oss/check-model-config.mjs", + "tests/init-archetype.test.ts", + "tests/install-records.test.ts", + "tests/list.test.ts", + "tests/model-config-format.test.ts", + "tests/model-config-parity.test.ts", + "tests/model-config.test.ts", + "tests/model-routing-format.test.ts", + "tests/model-routing.test.ts", + "tests/models-update.test.ts", + "tests/overwrite-check.test.ts", + "tests/pharn-config.test.ts", + "tests/status.test.ts", + "tests/update.test.ts", + "tests/upstream-models.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/models-pharn-oss-owned/verify-report.json b/.dev/features/models-pharn-oss-owned/verify-report.json new file mode 100644 index 00000000..20c4de90 --- /dev/null +++ b/.dev/features/models-pharn-oss-owned/verify-report.json @@ -0,0 +1,17 @@ +{ + "feature": "models-pharn-oss-owned", + "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 79e33b99..fceff001 100644 --- a/.pharn/writes-scope.json +++ b/.pharn/writes-scope.json @@ -1,7 +1,7 @@ { "scope": [ - ".dev/features/release-0-6-0/SHIP.md" + ".dev/features/models-pharn-oss-owned/SHIP.md" ], "set_by": ".claude/commands/pharn-dev-ship.md", - "set_at": "2026-09-25T14:32:21.445Z" + "set_at": "2026-09-25T22:37:12.910Z" } diff --git a/CHANGELOG.md b/CHANGELOG.md index df1752db..cf41a2fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **The `models` block in `pharn.config.json` now belongs to pharn-oss — its schema and its defaults.** `pharn init` copies pharn-oss's own block, verbatim, from the root `pharn.config.json` of the commit it installs, instead of writing a default of its own. When pharn-oss ships no block, `init` writes none. The CLI's own model ids, effort levels and stage list are gone. The block is checked against pharn-oss's rules instead, through a copy of pharn-oss's checker (`pharn/floor/check-model-config.mjs`) that a test runs against the real one: + - a `model` is an alias (`sonnet`, `opus`, `haiku`, `fable`, `inherit`) or a full `claude-*` id; + - an `effort` is `low`, `medium`, `high`, `xhigh` or `max`; + - `default` sits inside `stages`, beside pharn-oss's eleven product stages. + + No command refuses to run over the block any more; `pharn status` lists what pharn-oss's rules reject instead. If pharn-oss's rules move ahead of the copy in your `pharn`, the newer block is not applied, the reason is named, and upgrading `pharn` fixes it. +- **`pharn status` and `pharn init` show the block for what it is.** They list every product stage with the model and effort it resolves to, marking the stages that fall back to `default`. The label under the list says that Claude Code applies each `/pharn-*` command's own `model:`/`effort:` frontmatter, that the block is the source of truth that frontmatter is held to, and that `node pharn/floor/check-model-config.mjs agreement` checks the two. The block is no longer presented as routing. +- **`pharn update` manages the `models` block like a file.** pharn-oss's block replaces one `pharn` wrote, tracked by a new `pharn.config.json#/models` entry in `pharn.records.json`, and a block you changed is kept. `--force` replaces yours after backing up `pharn.config.json` to `.pharn-backup/`. A kept block never holds back the skills version, and a `MODELS` note says what happened. + +### Fixed + +- **The `models` block `pharn init` wrote was rejected by everything that reads it.** pharn-oss's checker, installed as `pharn/floor/check-model-config.mjs`, failed it three times: no `default` inside `stages`, and `opus-4-8` is not a model it accepts. `claude --model sonnet-5` fails with `unrecognized_model`. And its values disagreed with the installed commands' frontmatter (`plan` as `opus-4-8 · max` where the command says `opus`/`high`). The first `pharn update` with this release fixes an existing install, even at a current skills version: + - a block still exactly as an earlier `pharn` wrote it is replaced with pharn-oss's; + - a block you edited is converted with your values kept: `default` moves into `stages.default`, and `opus-4-8`, `sonnet-5`, `fable-5` and `haiku-4-5` become `opus`, `sonnet`, `fable` and `haiku`; + - anything that cannot be converted is listed by name and left as it is. + ## [0.6.0] — 2026-09-25 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index 1630e46b..7cbe0900 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,7 +6,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co `pharn` is an interactive CLI that installs [PHARN](https://github.com/pharn-dev/pharn-oss) — an audit-grade methodology for Claude Code — into an existing project (framework-agnostic). `pharn init` detects the project's **archetype(s)** (`ssr`/`backend`/`spa`/`lib`) and installs the applicable PHARN **capabilities** (grillers/lenses) from `pharn-dev/pharn-oss` via a codeload tarball download, copying them plus the fixed product surfaces into the mirrored layout (`.claude/` + `pharn/`) and writing `pharn.config.json`. No module catalog / `manifest.json` fetch. Published on npm as `@pharn-dev/pharn`, exposing a single `pharn` bin. Targets Claude Code today; Codex and Cursor are planned. -**Module model (removed).** Earlier releases installed by _modules_ driven by a repo-root `manifest.json` (schemaVersion 1/2 + a `wizard` block + per-module `module.json` `installs` maps), and kept a module/manifest fallback in `add`/`update`/`list`/`status`/`remove` for a pre-archetype config. **That subsystem is gone** (`lib/manifest.ts`, `install-modules.ts`, `installer.ts`, `wizard.ts` deleted) — live pharn-oss ships no `manifest.json`, so the fallback 404'd. **All commands are archetype-only.** A pre-archetype (module) `pharn.config.json` (one with `modules[]` but no `capabilities[]`) is detected by `isArchetypeConfig` and rejected up front by `loadArchetypeConfigOrExit` with a clear "re-run `pharn init`" message (`LEGACY_CONFIG_MESSAGE`) — never a fetch. **This CLI still owns the `pharn.config.json` schema**, which stays additive: a legacy config's now-unused fields (`modules`, `constitution`, `installedSkills`, `stackAnswers`) still LOAD (P7). +**Module model (removed).** Earlier releases installed by _modules_ driven by a repo-root `manifest.json` (schemaVersion 1/2 + a `wizard` block + per-module `module.json` `installs` maps), and kept a module/manifest fallback in `add`/`update`/`list`/`status`/`remove` for a pre-archetype config. **That subsystem is gone** (`lib/manifest.ts`, `install-modules.ts`, `installer.ts`, `wizard.ts` deleted) — live pharn-oss ships no `manifest.json`, so the fallback 404'd. **All commands are archetype-only.** A pre-archetype (module) `pharn.config.json` (one with `modules[]` but no `capabilities[]`) is detected by `isArchetypeConfig` and rejected up front by `loadArchetypeConfigOrExit` with a clear "re-run `pharn init`" message (`LEGACY_CONFIG_MESSAGE`) — never a fetch. **This CLI still owns the `pharn.config.json` schema — for every key EXCEPT `models`, whose schema and defaults pharn-oss owns** (maintainer decision, 2026-09-25; see the `models` paragraph below). The CLI-owned schema stays additive: a legacy config's now-unused fields (`modules`, `constitution`, `installedSkills`, `stackAnswers`) still LOAD (P7). ## Commands @@ -51,13 +51,13 @@ 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 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. +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 pharn-oss's `models` block — copied VERBATIM from the clone's root `pharn.config.json` by `readUpstreamModels` (none when upstream ships none, and none, with the reasons warned, when the pinned copy of pharn-oss's rules rejects it) and recorded under `MODELS_RECORD_KEY` — and the `seam` default; 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 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. -**`lib/pharn-config.ts`** reads/writes `pharn.config.json` (`pharnVersion`, `skillsVersion`, `repo`, `commit`, `installedAt`, and — for an archetype install — `archetypes[]`, `capabilities[]` (`{name, role}`), `layout`, plus the `models`/`seam` blocks). Every `capabilities[]` entry is validated at ingest: it must be an object whose `name` matches `CAPABILITY_NAME_RE` (no control chars) and whose `role` is in `ROLE_VALUES`, else `CapabilityEntryError` — the config is committed and hand-editable, and `name` is path-joined by `remove`'s recursive delete (a `{"name":"../.."}` entry used to delete the project root). A present-but-invalid `capabilities[].source` (anything outside `{auto, manual}`; ABSENT is legal, P7) is rejected by `CapabilitySourceError`. Both join `isConfigValidationError`'s union. A present-but-invalid `models`/`seam` block is caught by its own validator — `lib/model-routing.ts` (`validateModelRouting`/`ModelRoutingError`) and `lib/seam-config.ts` (`validateSeamConfig`/`SeamConfigError`) — and `readPharnConfig` lets that named error PROPAGATE (never collapsing a bad hand-edit into the "run init" path); `isConfigValidationError` + `loadConfigOrExit` catch and report it, and the validated/stripped blocks replace the raw ones. Schema is additive — a legacy config's now-unused `modules[]`/`constitution`/`stackAnswers`/`installedSkills[]` still load (P7). `isArchetypeConfig` (= `Array.isArray(config.capabilities)`) is the deterministic discriminator; **`loadArchetypeConfigOrExit`** is the shared load-or-reject surface for `add`/`update`/`status`/`remove` (a pre-archetype config → `LEGACY_CONFIG_MESSAGE` + exit(1), never a fetch; `list` keeps its own json-aware check so `--json` stderr stays clean). `add`/`update`/`remove` update the config in place, and each calls `assertConfigUnchanged` as the FIRST step inside its `.pharn.lock` (`lib/project-lock.ts`): the config is loaded before the lock (and across `update`'s / the `remove` picker's confirm), so a concurrent run could otherwise be overwritten from a stale snapshot; a mismatch throws `ProjectChangedError` (a `ProjectLockedError`, so every catch site reports it as a named refusal, exit 1, nothing written). The lock itself treats an unparseable lock file younger than `MALFORMED_GRACE_MS` (10 s) as LIVE — `tryCreate` writes its payload after the `O_EXCL` create, so a live lock is briefly empty — and only breaks older ones. All three files pharn keeps in a project — `pharn.config.json` (`readPharnConfig`, `configFingerprint`, and init's two tolerant reads), `pharn.records.json` (`readRecords`) and `.pharn.lock` (`readRawAt`) — are read through **`lib/bounded-read.ts`** (`readBoundedFile`): one descriptor opened `O_NONBLOCK`, `fstat`-checked as a regular file of at most `MAX_LOCAL_FILE_BYTES` (16 MiB), read from that same descriptor, never throwing. A FIFO, a device or a directory there is therefore each caller's existing "unreadable" outcome (config → `null` / `unreadable:`, records → a named `invalid`, lock → presumed live, then broken), never a hang in `open(2)` and never an unbounded read — the posture `hook-wiring.ts` and `detect-archetype.ts` already held for the files they read. +**`lib/pharn-config.ts`** reads/writes `pharn.config.json` (`pharnVersion`, `skillsVersion`, `repo`, `commit`, `installedAt`, and — for an archetype install — `archetypes[]`, `capabilities[]` (`{name, role}`), `layout`, plus the `seam` block and pharn-oss's `models` block). Every `capabilities[]` entry is validated at ingest: it must be an object whose `name` matches `CAPABILITY_NAME_RE` (no control chars) and whose `role` is in `ROLE_VALUES`, else `CapabilityEntryError` — the config is committed and hand-editable, and `name` is path-joined by `remove`'s recursive delete (a `{"name":"../.."}` entry used to delete the project root). A present-but-invalid `capabilities[].source` (anything outside `{auto, manual}`; ABSENT is legal, P7) is rejected by `CapabilitySourceError`. Both join `isConfigValidationError`'s union. A present-but-invalid `seam` block is caught by its own validator — `lib/seam-config.ts` (`validateSeamConfig`/`SeamConfigError`) — and `readPharnConfig` lets that named error PROPAGATE (never collapsing a bad hand-edit into the "run init" path); `isConfigValidationError` + `loadConfigOrExit` catch and report it, and the validated/stripped block replaces the raw one. The `models` block is NOT validated at load (it is pharn-oss's — see below): it rides the spread VERBATIM, so the pre-0.7.0 format `update` must migrate, pharn-oss's format and a newer one all load, and no command refuses to run over it. It stays in `CLI_OWNED_KEYS` because pharn still WRITES the key (a re-run `init` replaces it rather than carrying a stale one). Schema is additive — a legacy config's now-unused `modules[]`/`constitution`/`stackAnswers`/`installedSkills[]` still load (P7). `isArchetypeConfig` (= `Array.isArray(config.capabilities)`) is the deterministic discriminator; **`loadArchetypeConfigOrExit`** is the shared load-or-reject surface for `add`/`update`/`status`/`remove` (a pre-archetype config → `LEGACY_CONFIG_MESSAGE` + exit(1), never a fetch; `list` keeps its own json-aware check so `--json` stderr stays clean). `add`/`update`/`remove` update the config in place, and each calls `assertConfigUnchanged` as the FIRST step inside its `.pharn.lock` (`lib/project-lock.ts`): the config is loaded before the lock (and across `update`'s / the `remove` picker's confirm), so a concurrent run could otherwise be overwritten from a stale snapshot; a mismatch throws `ProjectChangedError` (a `ProjectLockedError`, so every catch site reports it as a named refusal, exit 1, nothing written). The lock itself treats an unparseable lock file younger than `MALFORMED_GRACE_MS` (10 s) as LIVE — `tryCreate` writes its payload after the `O_EXCL` create, so a live lock is briefly empty — and only breaks older ones. All three files pharn keeps in a project — `pharn.config.json` (`readPharnConfig`, `configFingerprint`, and init's two tolerant reads), `pharn.records.json` (`readRecords`) and `.pharn.lock` (`readRawAt`) — are read through **`lib/bounded-read.ts`** (`readBoundedFile`): one descriptor opened `O_NONBLOCK`, `fstat`-checked as a regular file of at most `MAX_LOCAL_FILE_BYTES` (16 MiB), read from that same descriptor, never throwing. A FIFO, a device or a directory there is therefore each caller's existing "unreadable" outcome (config → `null` / `unreadable:`, records → a named `invalid`, lock → presumed live, then broken), never a hang in `open(2)` and never an unbounded read — the posture `hook-wiring.ts` and `detect-archetype.ts` already held for the files they read. **`pharn add` addressing** (`commands/add.ts` + `lib/capability-address.ts`). `add ` or `add :` (e.g. `add a11y`, `add lens:n-plus-one`) installs one capability into an archetype project — a manual override of archetype auto-selection. It clones pharn-oss (SHA-pinned), then applies **the version gate**: a local `versionGate` helper, called ONCE per command from INSIDE each path's existing `try` (so `readSkillsVersion`'s throw still reaches the `finally` that cleans up the clone), refuses when `readSkillsVersion(repo.dir) !== config.skillsVersion` — reusing the existing `{kind:'error'}` outcome → `exit(1)`. It fires on `!==` (never `<`, so a rollback reads the same) — except that a clone at the config's additive `pendingSkillsVersion` also passes: `update` records that field only when it withheld the bump SOLELY because of `modified`/`unrecorded` skips (the user's kept edits; every other file is at that version) and clears it on the next complete run; at a pending version `add` keeps the config's own `(skillsVersion, commit)` pair (`addStamp`) so the records stamp stays consistent, fires **gate-first** (before the already-installed no-op and before the picker's `all-installed`, and before `groupMultiselect` renders), and writes nothing. A sibling **layout gate** (`layoutGate`) sits immediately after it at both call sites, `??`-chained (`versionGate(…) ?? layoutGate(…)`) so the **version** refusal wins when both mismatch — by short-circuit evaluation, not statement order: it refuses when `detectLayout(repo.dir) !== configLayout(config)`, naming both resolved layouts and `pharn update --force`. `add` copies at the CLONE's layout (`installCapabilityDirs`' default) and records at it (`mergeCapabilityRecords`), while `remove`/`status`/`diff.ts` address the project at `configLayout` — so without the gate a mismatched add lands files nothing ever looks at, and the next `remove` drops the config entry reporting "its files were already gone", orphaning the dir. `add` must NEVER record the clone's layout the way `update` does: `update` may only because it rewrites the WHOLE tree at that layout, whereas `add` writes one capability, so stamping `layout` here would re-address every other already-placed file. Comparing `configLayout(config)` (not the raw `config.layout`) is the point — agreement with the readers is the invariant, and it makes a garbage hand-edited value resolve to `flat` and fail closed. This is what keeps `update`'s `config.skillsVersion === latest` early-return honest: `add` must never stamp a newer `skillsVersion` over unchanged old bytes. `add` therefore refreshes `commit` but NEVER `skillsVersion` (the legal same-version-different-commit case). Past the gate it resolves the arg against `parseCapabilityIndex`, and if it uniquely names a not-yet-installed capability, copies it via `installCapabilityDirs` and **appends** to `capabilities` with `source: 'manual'` (never touches `archetypes`). That tag is what makes the override survive `update`, and it is written at BOTH entry-construction sites — `resolveArchetypeAdd` and the picker's threaded `cfg` mirror, which the next pick spreads into its own config write. Already-installed → no-op; unknown/ambiguous → lists the valid `role:name` addresses. **Before the copy, `add` backs up destination drift**: `capabilityCloneFiles` (`lib/install-manifest.ts`) enumerates the capability dir in the CLONE, `scanDest` (`lib/dest-drift.ts`) keeps the rels whose dest exists as a regular file and whose `sha256File` DIFFERS from the clone's, and a non-empty set goes to `createBackup` (`lib/backup.ts` — the same `.pharn-backup//` `update --force` writes) BEFORE the first `cpSync`, logged at creation so the pointer survives a later throw. `add` was the only write path with none of the product's three edit-protections (init's prompt, update's per-file skip, `--force`'s backup), and `update` itself manufactures the reachable sequence: a `dropped-unselected` capability's files are left on disk, the user edits them, and the later `add` is not a config no-op. Byte-identical is NOT drift (mirrors update's `identical → no-op`), so a drop-then-re-add of unedited files stays silent. The scan SKIPS an absent/symlinked/non-dir source rather than throwing, so `installCapabilityDirs`' curated `Capability "x" (griller) is missing at …` still wins — the same read-skips/write-throws split `install-manifest.ts`'s `addDir` already uses. `scanDest` returns a PARTITION, not just the drift set: a rel whose PROJECT-side path crosses a symlinked component is `unsafe`, and `add` **refuses the whole install** naming the component. That is measured, not defensive — on node v24.13.1 `cpSync` guards only the source, so a symlinked INTERMEDIATE dir under the capability dir is written straight THROUGH (bytes outside the project replaced), while a symlinked leaf is REPLACED and a symlinked capability root throws `ERR_FS_CP_DIR_TO_NON_DIR`. Skipping would be strictly worse than doing nothing (the copy writes through anyway while the backup omits the file), and backing up is impossible (`createBackup` refuses symlinked components; `copyFileSync` would save the link's TARGET). An ENOTDIR walk (a component below a regular file) is NOT unsafe — `cpSync` throws on that tree by itself — so it stays a skip, which is also what keeps a non-directory component out of the set `createBackup` consumes unwrapped. `add` also merges the capability's files into `pharn.records.json` (only extending an already-readable store; it never mints one), keyed by that SAME clone-derived list — never a walk of the destination, which used to sweep a user's own file inside a leftover capability dir into the store as pharn-written and let a later `update` read it as cleanly upgradeable instead of `modified`; the hashes are still taken at the DEST (`buildRecords`), so a record can never disagree with what landed. `CONSTITUTION.md` is **not** touched — `add` installs capability dirs only. `pharn update` re-resolves the **recorded archetypes** against the latest index and re-copies — drift-safely, see below. @@ -65,7 +65,9 @@ ESM-only (`"type": "module"`, NodeNext). **Relative imports must use `.js` exten **`pharn update` (`commands/update.ts`) is drift-safe by default.** It re-resolves the recorded archetypes and **unions** the result with the user's manual adds (`lib/merge-capabilities.ts` — the pure 9-row membership table: `next = resolve(archetypes) ∪ manual`; sticky manual; a manual entry gone from the index is dropped, its files left alone; a `source`-less legacy entry is inferred ONCE at merge time — in the resolved set → `auto`, outside it → `manual` — which is the ONLY place absence may be resolved). Every membership change is NAMED in a `CAPABILITIES` note (`added` / `dropped-unselected` / `dropped-gone` / `kept-manual`); zero changes print nothing. Resurrection of a removed capability is **reported, not prevented** (no tombstones). It then decides **per file** instead of re-copying wholesale: `lib/install-records.ts` holds `pharn.records.json` (a sha256 of every file an install wrote, hashed at the DEST, stamped with the config's `skillsVersion`/`commit` so a store left by another tool is detected and ignored); `lib/update-decision.ts` is the PURE 6-row table (`decideFileAction` + `planUpdate` — missing→restore, identical→no-op, equals-record→upgrade, else SKIP `modified`/`unrecorded`/`unverifiable`); `lib/apply-update.ts` executes the writes (dest-symlink refusal, parent `mkdir`, and an `ApplyError` carrying what was already written so those files are still recorded on a partial failure); `lib/backup.ts` copies every `--force` casualty to `.pharn-backup//` BEFORE any original is touched. Records are written BEFORE the config, and a run that skipped anything **withholds** the `skillsVersion`/`commit` bump so the recorded version stays true and the next run still has work. `--force` overwrites the skip buckets and bypasses the same-version early-return. Update **never deletes** and never touches `.claude/settings.json`. `CONSTITUTION.md` (from `paths.docs` in `lib/install-manifest.ts` — flat: root; pharn layout: `pharn/CONSTITUTION.md`) is in the expected trusted-doc set and follows the same per-file table: missing→restore, still-at-recorded-hash→upgrade, locally modified→skip (`modified`); `add`/`remove` never touch it. It records the layout detected in the CLONE (closing the latent drift where bytes landed at `pharn/` paths while the config still said `flat`). A capability it KEPT (upstream ships it, this CLI cannot parse it) is recorded in the additive `frozenCapabilities` (`role:name`, sorted, omitted when empty), and while that list is non-empty the same-version early return is skipped, so every run re-fetches and re-checks it; an entry leaves the list only once none of its files was skipped. When a run withholds the `skillsVersion` bump SOLELY for `modified`/`unrecorded` skips it records `pendingSkillsVersion` (see `add` above). With `--force`, the backup notice prints the moment the backup exists, with its spinner stopped first (a line logged under a running clack spinner is glued to its frame and, in a narrow terminal, erased by its stop); a later failure repeats only the part-way warning and the pointer, on stderr. -**`pharn status` (`commands/status.ts`) is strictly read-only** — it never writes, deletes, or overwrites (fixing is `update`/`add`). Two sections: a **version** check (installed `skillsVersion` vs the upstream `SKILLS_VERSION` file, plus an archetype + capability-count summary) and a **drift** check. Default clones `@main` once and reuses it for both; `--no-drift` skips the clone and uses `fetchRemoteSkillsVersion` for the version section only; `--strict` exits 1 on any outdated/modified/missing (CI gate, default exit 0). Cleanup runs in a `finally`, and every `process.exit` happens _after_ it. The pure (no I/O) engine is **`lib/diff.ts` → `diffInstalledCapabilities`**: it mirrors `installCapabilities` to derive the **expected** file set — the selected capability dirs + the fixed product surfaces, at the recorded `layout` — then `sha256`-compares each against the project root, returning `{modified, missing, okCount}`. Every read is `safeJoin`-guarded (from `lib/validate.ts`). `.claude/settings.json` is user-owned (preserved at install) and excluded from the file diff — but its `hooks` block is compared by `lib/hook-wiring.ts` (`diffHookWiring`: exact-string set difference over `Event · matcher · command · args`, against the UNION of the project's `.claude/settings.json` and `.claude/settings.local.json` — what Claude Code merges; user-level settings are never read. Every read is size-capped and opened `O_NONBLOCK` (a FIFO is refused, not waited on); upstream's file is symlink-refused at every component, while a project file may be a symlinked FILE (read through, as Claude Code does) but never sit under a symlinked `.claude/`. Only upstream entries are displayed, each as the hook's own JSON — exec vs shell form visible, paste-ready — with every `terminalSafe`-class character and U+2028/2029 turned into a `\u` JSON escape and each line capped at 300 chars); upstream hooks wired in neither file print a `HOOKS` note in `status` (and fail `--strict`, via `hookWiringFails`, as does an unreadable project file) and in `update` (report only — `update` still never writes the file); hooks wired only in `settings.local.json` are named as `local-only` and do NOT fail `--strict`; the copied-verbatim trusted docs (including `CONSTITUTION.md` / `pharn/CONSTITUTION.md`), hooks, contracts, and floor checkers ARE compared. Drift is derived live from the clone, always against `@main`, never the pinned `commit`. `status` does NOT read `pharn.records.json`, so it is a report, not a preview: a file it lists as differing may be cleanly upgraded OR skipped — only `update` can tell those apart. +**The `models` block is pharn-oss's** (schema and defaults: pharn-oss's `pharn/floor/check-model-config.mjs` and the block in its root `pharn.config.json`). Claude Code applies each `/pharn-*` command's static `model:`/`effort:` frontmatter; the block is the SOURCE OF TRUTH that frontmatter is held to (the checker's `agreement` mode) — never routing that happens, and every surface that prints it says so (`modelsLabelLines`, `lib/model-config-format.ts`). The CLI owns no model/effort/stage rule of its own. **`lib/model-config.ts` is a pinned COPY of the checker's `validate`/`resolve` rules** (`PRODUCT_STAGES`, `MODEL_ALIASES`, `MODEL_ID_RE`, `EFFORT_LEVELS`, `checkModelsBlock`, `resolveStageModel`) — a copy, not a call, because the CLI never executes a file it installs (THREAT-MODEL §1; `status --strict` runs in CI on PRs, and at `init` the only checker is inside the untrusted clone). `tests/model-config-parity.test.ts` makes pharn-oss the one owner: it runs the vendored checker (`tests/fixtures/pharn-oss/check-model-config.mjs`, byte-for-byte, sha256-pinned with its upstream commit) over a corpus and compares every verdict, RED line and resolution with the copy's, reads the four sets out of the checker's source, and fails if the corpus stops reaching one of the checker's validate-path RED kinds. Never add a rule the checker does not have; the refresh procedure is in `docs/contributing.md`. When upstream widens its sets before the copy follows, a block using the new member is NOT applied and the rejection is named — never fatal (LIMITS §3e); upstream's `MIN_CLI` is the lever. `lib/upstream-models.ts` (`readUpstreamModels`) is the fetch-boundary read of the clone's root config (bounded to `MAX_UPSTREAM_CONFIG_BYTES`, never throwing: `absent` / `ok` / `invalid` with reasons). **`lib/models-update.ts`** holds the CLI's knowledge of its OWN pre-0.7.0 format (the 4-id alias map `opus-4-8→opus`, `sonnet-5→sonnet`, `fable-5→fable`, `haiku-4-5→haiku`; the two defaults it ever wrote, compared byte-for-byte as `JSON.stringify`), `convertLegacyModels` (top-level `default` → `stages.default` unless that is set or `stages` is not an object — then named as a leftover; old ids → aliases; everything else verbatim; own-property rebuilds so a `__proto__` key stays data), `needsModelsConversion`, and `decideModelsUpdate`, which CALLS `decideFileAction` with the block's hash (`modelsRecordHash`, stored in `pharn.records.json` under the non-path key `MODELS_RECORD_KEY` = `pharn.config.json#/models`, defined in `lib/install-records.ts`) as the file hash and a historical default fed in as proof of authorship — so the rows ARE the per-file rows: absent → restored; equal to pharn-oss's → ok; still pharn's → updated; else KEPT (`modified`/`unrecorded`/`unverifiable`), converted if old, its previous record carried; `--force` → `pharn.config.json` backed up, then replaced. Upstream absent/invalid → the user's block stays (converted if old). The block's outcome never withholds the `skillsVersion` bump (configuration, not bytes of a version), and `needsModelsConversion` re-opens `update`'s same-version early return so a current install is migrated too. `update` prints a `MODELS` note for every outcome but `ok`. + +**`pharn status` (`commands/status.ts`) is strictly read-only** — it never writes, deletes, or overwrites (fixing is `update`/`add`). Two sections: a **version** check (installed `skillsVersion` vs the upstream `SKILLS_VERSION` file, plus an archetype + capability-count summary) and a **drift** check. Default clones `@main` once and reuses it for both; `--no-drift` skips the clone and uses `fetchRemoteSkillsVersion` for the version section only; `--strict` exits 1 on any outdated/modified/missing (CI gate, default exit 0). Cleanup runs in a `finally`, and every `process.exit` happens _after_ it. The pure (no I/O) engine is **`lib/diff.ts` → `diffInstalledCapabilities`**: it mirrors `installCapabilities` to derive the **expected** file set — the selected capability dirs + the fixed product surfaces, at the recorded `layout` — then `sha256`-compares each against the project root, returning `{modified, missing, okCount}`. Every read is `safeJoin`-guarded (from `lib/validate.ts`). `.claude/settings.json` is user-owned (preserved at install) and excluded from the file diff — but its `hooks` block is compared by `lib/hook-wiring.ts` (`diffHookWiring`: exact-string set difference over `Event · matcher · command · args`, against the UNION of the project's `.claude/settings.json` and `.claude/settings.local.json` — what Claude Code merges; user-level settings are never read. Every read is size-capped and opened `O_NONBLOCK` (a FIFO is refused, not waited on); upstream's file is symlink-refused at every component, while a project file may be a symlinked FILE (read through, as Claude Code does) but never sit under a symlinked `.claude/`. Only upstream entries are displayed, each as the hook's own JSON — exec vs shell form visible, paste-ready — with every `terminalSafe`-class character and U+2028/2029 turned into a `\u` JSON escape and each line capped at 300 chars); upstream hooks wired in neither file print a `HOOKS` note in `status` (and fail `--strict`, via `hookWiringFails`, as does an unreadable project file) and in `update` (report only — `update` still never writes the file); hooks wired only in `settings.local.json` are named as `local-only` and do NOT fail `--strict`; the copied-verbatim trusted docs (including `CONSTITUTION.md` / `pharn/CONSTITUTION.md`), hooks, contracts, and floor checkers ARE compared. Drift is derived live from the clone, always against `@main`, never the pinned `commit`. `status` does NOT read `pharn.records.json`, so it is a report, not a preview: a file it lists as differing may be cleanly upgraded OR skipped — only `update` can tell those apart. A `MODELS` note (both paths, no clone needed) shows the `models` block resolved for every product stage (`(default)` marks a fallback) under the truthful label, says `pharn update` converts an old-format block, or lists the pinned copy's REDs through `terminalSafe`; it is NOT a `--strict` input — the block's verdict is pharn-oss's checker's. ## Testing diff --git a/docs/commands/add.md b/docs/commands/add.md index dbb3d4c2..9a3a9807 100644 --- a/docs/commands/add.md +++ b/docs/commands/add.md @@ -13,7 +13,7 @@ pharn add # no arg, in a terminal: interactive multi-select pick ## Behavior 1. Reads `pharn.config.json`. If none exists — or it is a pre-archetype (module) config — it exits with - a hint to run `pharn init` first. A config that exists but is **invalid** (a bad `models`/`seam` + a hint to run `pharn init` first. A config that exists but is **invalid** (a bad `seam` block, an out-of-enum `capabilities[].source`, unparseable JSON) is reported by its own named error and exits 1 — deliberately not the "run `pharn init`" hint, which would tell you to overwrite it. 2. Takes the project lock (`.pharn.lock`) — **before** any download, so a run that will be refused pays diff --git a/docs/commands/init.md b/docs/commands/init.md index 3fe024f3..e3510709 100644 --- a/docs/commands/init.md +++ b/docs/commands/init.md @@ -189,6 +189,7 @@ settings file — and is refused only when its target does not exist. See | Mirror the layout | Whichever layout the fetched clone uses is mirrored verbatim; the CLI never rewrites copied file contents. Today that is `pharn/pharn-contracts/`, `pharn/pharn-core/`, `pharn/floor/`; the legacy flat layout is `pharn-contracts/`, `pharn-core/`, `.dev/floor/` | | Pin commit SHA | Best-effort (the SHA the tree was pinned to; `null` if unavailable) | | Write `pharn.config.json` | `pharnVersion`, `skillsVersion` (from `SKILLS_VERSION`), `repo`, `commit`, `installedAt`, `archetypes`, `capabilities` (`source: "auto"`, or `"manual"` for a kept `pharn add`), `layout`, `models`, `seam`, `modules: []`, `frozenCapabilities` (see above) | +| Copy the `models` block | pharn-oss's own, verbatim, from its root `pharn.config.json`; none when pharn-oss ships none, and none — with the reason — when it fails pharn-oss's rules as this pharn knows them ([Models](../reference/pharn-config.md#models)) | | Write `pharn.records.json` | A sha256 of every file the install wrote, so [`pharn update`](update.md) can keep your later edits ([reference](../reference/pharn-records.md)) | The install also copies pharn-oss's Apache-2.0 `LICENSE` — to `pharn/LICENSE`, or `PHARN-LICENSE` at @@ -198,7 +199,7 @@ publish carries the license grant for the ~450 Apache-2.0 files pharn put in it. The install copies pharn-oss's canonical `CONSTITUTION.md` verbatim — there is no privacy-posture / constitution-variant question in the archetype flow. Only capability contents are copied; the CLI never executes or parses them (your Claude Code runs them later). -On success, the CLI reports the capability count and suggests opening Claude Code and running `/pharn-spec` — intent capture for your first feature, which feeds `/pharn-plan`. +On success, the CLI reports the capability count, prints the `models` block it copied resolved per stage — under the label that Claude Code applies each command's own frontmatter, which the block is the source of truth for — and suggests opening Claude Code and running `/pharn-spec` — intent capture for your first feature, which feeds `/pharn-plan`. ## Concurrency @@ -217,9 +218,9 @@ the lock across an unanswered human prompt. And because both prompts sit inside `init` always writes an **archetype** config, and every command is archetype-only. A pre-archetype **module**-based `pharn.config.json` (one with `modules[]` but no `capabilities[]`, from a much older release) is no longer supported: `add`, `remove`, `list`, `update`, and `status` detect it up front and exit with a message to re-run `pharn init` — there is **no** module/manifest fallback (live pharn-oss ships no `manifest.json`). The config schema is additive, so a legacy config's now-unused fields (`modules`, `constitution`, `stackAnswers`, `installedSkills`) still parse; only the absence of `capabilities[]` triggers the rejection. -A config that is present but **invalid** — a malformed `models`/`seam` block, an out-of-enum +A config that is present but **invalid** — a malformed `seam` block, an out-of-enum `capabilities[].source`, or JSON that does not parse — is a different case with its own named error and -exit 1. It deliberately does **not** say "run `pharn init`", because that would tell you to overwrite +exit 1. (A `models` block is never one: it is pharn-oss's, and no command refuses to run over it.) It deliberately does **not** say "run `pharn init`", because that would tell you to overwrite the file you need to repair. See [A command rejects an invalid config](../troubleshooting.md#a-command-rejects-an-invalid-config-does-not-say-run-init). diff --git a/docs/commands/list.md b/docs/commands/list.md index a0360095..7e77783b 100644 --- a/docs/commands/list.md +++ b/docs/commands/list.md @@ -20,7 +20,7 @@ the human-readable output, not JSON. Pass `--json` bare. ## Behavior -1. Reads `pharn.config.json`. A config that is present but **invalid** (a malformed `models`/`seam` +1. Reads `pharn.config.json`. A config that is present but **invalid** (a malformed `seam` block, an out-of-enum `capabilities[].source`, unparseable JSON) gets its own named error and exit 1 — under `--json` it is written as plain text to stderr, so no clack chrome reaches a `2>&1` consumer. If none exists — or it is a pre-archetype (module) config — it exits with diff --git a/docs/commands/status.md b/docs/commands/status.md index 6bc3bbb5..f05cdaa7 100644 --- a/docs/commands/status.md +++ b/docs/commands/status.md @@ -57,11 +57,16 @@ which, and `status` cannot. or at any parent directory below your project root: reading through one would report it as merely "differs" when its target has other bytes, or say nothing at all when its target happens to match. - If none of the three, reports **No drift**. -6. **Models** — whenever `pharn.config.json` carries a `models` block, `status` prints it as a third - note on **both** paths (with and without `--drift`), each stage beside its configured model, under - the qualifier *"Recorded only — no installed stage reads this yet."* It is displayed so the - recorded intent stays legible; nothing resolves it for routing. See - [pharn.config.json](../reference/pharn-config.md#model-routing). +6. **Models** — whenever `pharn.config.json` carries a `models` block, `status` prints a `MODELS` note + on **both** paths (with and without `--drift`): every product stage beside the model and effort it + resolves to — its own entry, or `default`, marked `(default)`. Under the rows, the label that says + what they are: Claude Code applies each `/pharn-*` command's own `model:` / `effort:` frontmatter, + not this block; the block is the source of truth that frontmatter is held to, and + `node pharn/floor/check-model-config.mjs agreement` checks the two agree. A block still in the format + `pharn` wrote before 0.7.0 is reported as such (`pharn update` converts it), and a block pharn-oss's + rules reject has each problem listed by name. The note is read-only and needs no download, and it + is **not** a `--strict` input: the block is pharn-oss's, and so is the verdict on it. See + [pharn.config.json](../reference/pharn-config.md#models). The heading says "differs from", not "locally modified", on purpose: the comparison is against upstream `@main`, so a file can differ because **upstream moved**, not only because you edited it. diff --git a/docs/commands/update.md b/docs/commands/update.md index a24483ca..b8231fa3 100644 --- a/docs/commands/update.md +++ b/docs/commands/update.md @@ -28,14 +28,15 @@ wrote is upgraded. A file it cannot prove is untouched is **skipped and listed** 3. Fetches the latest `SKILLS_VERSION` from `pharn-dev/pharn-oss@main` (a lightweight check, no clone) and compares it to your recorded `skillsVersion`. 4. If they match, reports "Already up to date" and exits — **unless** you passed `--force`, which - re-applies upstream at the current version. + re-applies upstream at the current version, or your `models` block is still in the format `pharn` + wrote before 0.7.0, which this run converts (see [The `models` block](#the-models-block)). 5. Otherwise shows the version bump with a pointer to `CHANGELOG.md`, and asks for confirmation — unless you passed `--yes`, which skips that one prompt and nothing else. 6. On confirm, clones the repo (SHA-pinned) and **re-resolves your recorded `archetypes`** against the latest capability index, then **unions** that result with the capabilities you added by hand. -7. Decides each expected file with the table below, backs up anything `--force` is about to - overwrite, copies the files it may write, then updates `pharn.records.json` and - `pharn.config.json`. +7. Decides each expected file with the table below — and your `models` block the same way — backs up + anything `--force` is about to overwrite, copies the files it may write, then updates + `pharn.records.json` and `pharn.config.json`. Because `update` re-resolves your archetypes against the latest index, a capability upstream added for one of your archetypes since your last install is picked up, and one it removed is dropped — your @@ -191,6 +192,49 @@ cp .pharn-backup/20260807-091500/pharn/CONSTITUTION.md pharn/CONSTITUTION.md backups accumulate and are committable by accident. Delete them once you are happy, or add `.pharn-backup/` to your `.gitignore`. +## The `models` block + +`models` in `pharn.config.json` is **pharn-oss's** block: the source of truth each `/pharn-*` +command's `model:` / `effort:` frontmatter is held to (see +[pharn.config.json](../reference/pharn-config.md#models)). `update` decides it with the table above, +using the block's hash where a file would use its bytes, and prints a `MODELS` note for every outcome +except "already identical": + +| Your block | Default | With `--force` | +| --------------------------------------------------------------------------------- | ------------------------- | -------------------------------------------- | +| absent | pharn-oss's is written | same | +| identical to pharn-oss's | nothing to do | same | +| still what `pharn` wrote — its recorded hash, or a default an earlier pharn wrote | replaced with pharn-oss's | same | +| changed since `pharn` wrote it, never recorded, or no usable records file | **kept** | `pharn.config.json` backed up, then replaced | + +"What `pharn` wrote" is compared as `pharn` writes it: re-indenting the file is not an edit, but +reordering the block's keys is — so a reordered default counts as yours and is kept, never replaced. +The block's record lives in [`pharn.records.json`](../reference/pharn-records.md#the-models-block). + +**The format `pharn` wrote before 0.7.0.** Earlier releases wrote a block of their own — a top-level +`default`, and model ids that neither Claude Code nor pharn-oss's checker accepts. A block that is +exactly one of those defaults is replaced with pharn-oss's. An edited one is **converted, not reset**: + +- a top-level `default` moves into `stages.default`; +- `opus-4-8` becomes `opus`, `sonnet-5` becomes `sonnet`, `fable-5` becomes `fable`, and `haiku-4-5` + becomes `haiku`; +- everything else is kept exactly as it is. + +What pharn-oss's rules still reject after that — a model with no mapping, a stage that does not exist, +a `default` that could not move because `stages.default` is already set — is listed by name for you to +fix by hand. Nothing is dropped, and nothing is guessed. This conversion runs even when your skills +version is current: it is the one case where `update` goes past "Already up to date". Once your block +is converted it no longer does. + +A kept block never holds back the skills version, unlike a skipped file: your `models` block is +configuration, and a block you tuned is a steady state, not an unfinished upgrade. Keep in mind what +the block is for, though — if your values differ from the installed commands' frontmatter, +`node pharn/floor/check-model-config.mjs agreement` lists where. + +If pharn-oss ships no block, yours is left alone (converted, if it is in the old format). If pharn-oss +ships one this `pharn` cannot accept — pharn-oss's rules can move ahead of the copy of them in your +`pharn` — the block is not applied, the reasons are listed, and upgrading `pharn` fixes it. + ## Non-interactive use (CI, scripts, pipes) `update` confirms before it writes, so it needs either a terminal or your explicit consent. Off a TTY — diff --git a/docs/contributing.md b/docs/contributing.md index 9be105a9..3de2fa06 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -76,7 +76,7 @@ pharn-cli/ index.ts CLI entry, command routing commands/ init, add, remove, update, list, status steps/ init stages (prereqs, overwrite-check, archetype-summary, install-archetype) - lib/ install-capabilities, install-manifest, capability-index, resolve-capabilities, detect-archetype, archetype, layout, repo, tar-extract, diff, skills-version, min-cli-gate, semver, pharn-config, model-routing(-format), seam-config, install-records, update-decision, apply-update, merge-capabilities, dest-drift, backup, atomic-write, project-lock, proxy-env(-format), symlink-guard, capability-address, capability-groups, capability-picker, unknown-capabilities, report-error, hash, validate, constants, banner, confirm, format + lib/ install-capabilities, install-manifest, capability-index, resolve-capabilities, detect-archetype, archetype, layout, repo, tar-extract, diff, skills-version, min-cli-gate, semver, pharn-config, model-config(-format), models-update, upstream-models, seam-config, install-records, update-decision, apply-update, merge-capabilities, dest-drift, backup, atomic-write, project-lock, proxy-env(-format), symlink-guard, capability-address, capability-groups, capability-picker, unknown-capabilities, report-error, hash, validate, constants, banner, confirm, format types.ts Archetype / CapabilityEntry / Selection / PharnConfig tests/ vitest specs docs/ user + maintainer documentation @@ -94,6 +94,30 @@ See [`CLAUDE.md`](../CLAUDE.md) for the architecture in depth (the archetype ins - Remote fetches (`lib/skills-version.ts`) use `redirect: 'error'`, an 8s timeout, and a 256KB body cap. - The repo download (`lib/repo.ts`) has its own, larger bounds: a 60s timeout, a 32MB archive cap, a 128MB extracted cap, and a 20,000-entry cap — enforced twice, including at `gunzipSync`. +### pharn-oss's `models` rules — a pinned copy + +pharn-oss owns the `models` schema of `pharn.config.json`, and its checker, +`pharn/floor/check-model-config.mjs`, ships into every install. The CLI cannot call it — it never +executes a file it installs (`THREAT-MODEL.md` §1) — so `src/lib/model-config.ts` is a copy of that +checker's `validate` and `resolve` rules, and nothing more. `tests/model-config-parity.test.ts` keeps it +that way: it runs the real checker, vendored byte-for-byte at +`tests/fixtures/pharn-oss/check-model-config.mjs` and pinned by sha256, over a corpus, and fails on any +verdict, RED line or resolution that differs from the copy's. It also reads the stage, alias, id and +effort sets out of the checker's source, and fails if the corpus stops reaching one of the checker's +RED kinds. + +When pharn-oss changes its checker: + +1. Copy `pharn/floor/check-model-config.mjs` from pharn-oss `main` over + `tests/fixtures/pharn-oss/check-model-config.mjs`, unedited. +2. Update `PINNED_SHA256`, and the commit and `SKILLS_VERSION` in the comment above it, in + `tests/model-config-parity.test.ts`. +3. Run `npx vitest run tests/model-config-parity.test.ts`, and change `src/lib/model-config.ts` until it + passes: port pharn-oss's change, never a rule of your own. If the checker gained a branch, extend the + corpus to reach it. +4. If a released CLI would now reject pharn-oss's own block, release the new CLI first, then ask + pharn-oss to raise `MIN_CLI` to it. + ### The fetch boundary `pharn` has **no dependency that fetches or unpacks remote content**. `src/lib/repo.ts` resolves the @@ -139,7 +163,9 @@ Two rules that are easy to get wrong: | `skills-version.test.ts` | Read/fetch + validate `SKILLS_VERSION` | | `prereqs.test.ts` | `.git`-present gate | | `overwrite-check.test.ts` / `install-manifest.test.ts` | Pre-install write-target conflict check; the shared install manifest (mirror-pinned to `installCapabilities`) | -| `model-routing.test.ts` / `seam-config.test.ts` | `models` / `seam` config validation | +| `model-config.test.ts` / `model-config-parity.test.ts` | pharn-oss's `models` rules as copied; the copy pinned to pharn-oss's own checker | +| `models-update.test.ts` / `upstream-models.test.ts` | `update`'s rows for the `models` block and the old-format conversion; reading pharn-oss's block | +| `seam-config.test.ts` | `seam` config validation | | `confirm.test.ts` / `repo.test.ts` / `banner.test.ts` / `format.test.ts` | helpers; the codeload fetch boundary; banner; format | | `tar-extract.test.ts` | The ustar reader: strip-1, pax skip, prefix reassembly, and every rejection | | `index.test.ts` | argv dispatch: the per-command option allowlist, the arity gate, and their exit codes | @@ -153,7 +179,7 @@ Two rules that are easy to get wrong: | `unknown-capabilities.test.ts` / `constants.test.ts` | Control-char stripping for refused upstream names; the path constants | | `ci-workflow.test.ts` / `check-composition.test.ts` | The six CI job names + runner pair; `npm run check`'s composition | | `docs-install-tables.test.ts` / `lint-gate.test.ts` / `dev-script.test.ts` | The README + getting-started install tables; the lint gate; the dev script | -| `repo-signals.test.ts` / `model-routing-format.test.ts` | Fetch abort/cleanup signals; the model-routing render | +| `repo-signals.test.ts` / `model-config-format.test.ts` | Fetch abort/cleanup signals; the `models` render | `lib/hash.ts` is the one module with no matching test file. diff --git a/docs/reference/pharn-config.md b/docs/reference/pharn-config.md index 97c87095..bdd2d54b 100644 --- a/docs/reference/pharn-config.md +++ b/docs/reference/pharn-config.md @@ -20,7 +20,7 @@ archetypes/capabilities and the pinned commit). | `capabilities` | array | Installed capabilities, each `{ name, role, source? }` — see below | | | `layout` | string | Install layout your files are at: `flat` or `pharn` (absent → `flat`) | | | `modules` | array | Always `[]` for an archetype install (the install unit is capabilities) | | -| `models` | object | Per-stage routing — recorded, not yet read ([Coming soon](../roadmap.md)) | | +| `models` | object | pharn-oss's per-stage model/effort block ([Models](#models)) | | | `seam` | object | Seam-resolution policy ([`seam-config.ts`](../../src/lib/seam-config.ts)) | | `isArchetypeConfig` treats the presence of a `capabilities` array as the marker of an archetype install. @@ -116,61 +116,93 @@ from the list. A value that is not a list of } ``` -## Model routing +## Models -> **Coming soon** — see the [roadmap](../roadmap.md). -> -> The `models` block is **written, validated and displayed today — and consumed by nothing.** No -> stage `pharn init` installs reads it to pick a model, so editing it does **not** change which model -> a stage runs; it records the routing you want for when the consumer lands. The block _is_ read for -> two things that are not routing: `pharn init` and `pharn status` render it back to you, and -> `pharn` validates it on every command (a bad hand-edit still fails loudly). +The `models` block is **pharn-oss's**, not this CLI's: pharn-oss owns its schema and its defaults — +every other field on this page is pharn's. It declares a model and an effort for each PHARN product +stage, and it is the **source of truth** that each `/pharn-*` command's static `model:` / `effort:` +frontmatter is held to. -The block records a per-stage model + effort. It is **written on every fresh install** and is -**user-owned afterwards** — `pharn` never migrates it. Source of truth: -[`model-routing.ts`](../../src/lib/model-routing.ts). +> **What applies a model is the command frontmatter, not this block.** Claude Code runs each +> `/pharn-*` command on the `model:` / `effort:` in that command's own frontmatter; nothing reads this +> block to pick one. pharn-oss's checker, installed with PHARN, holds the two equal: +> `node pharn/floor/check-model-config.mjs agreement` (`.dev/floor/` in the legacy flat layout). So +> editing the block is half a change: update the command frontmatter too, or the checker tells you. A +> green check means two files agree, never that a stage ran on that model — pharn-oss states the +> bounds in its `LIMITS.md` §8. -The block is a required `default` plus per-stage overrides under `stages`. `default` is the fallback -for every stage without its own entry (`grill`, `build`, `regress`, `verify`, `ship`); a stage with no -entry — including an empty `stages` — resolves to `default`. - -Defaults written at install: - -| Stage | Model | Effort | -| --------- | ---------- | ------ | -| `default` | `sonnet-5` | `high` | -| `plan` | `opus-4-8` | `max` | -| `review` | `opus-4-8` | `high` | - -**Why `review` is `opus-4-8`/`high`, not `fable-5`/`max`.** Review is the fan-out stage — a backend -install ships ~22 lenses, so its cost multiplies per lens; a premium model at `max` effort across that -fan-out is the worst-case token multiplier, and it would apply silently. `opus-4-8`/`high` is the -spend-safe default. Cross-model review on `fable-5`/`max` has proven catch value, so recording it for -release audits is the intent the block exists to capture — set it explicitly under -`models.stages.review`. Until the consumer lands this changes nothing about the model your review -actually runs on; it is a note to your future self, and to whoever reads the config: +pharn-oss's block, as of pharn-oss 6.22.0: ```json { "models": { - "default": { "model": "sonnet-5", "effort": "high" }, "stages": { - "plan": { "model": "opus-4-8", "effort": "max" }, - "review": { "model": "fable-5", "effort": "max" } + "default": { "model": "sonnet", "effort": "high" }, + "spec": { "model": "opus", "effort": "high" }, + "plan": { "model": "opus", "effort": "high" }, + "grill": { "model": "opus", "effort": "high" }, + "build": { "model": "sonnet", "effort": "high" }, + "regress": { "model": "sonnet", "effort": "high" }, + "verify": { "model": "sonnet", "effort": "high" }, + "ship": { "model": "sonnet", "effort": "high" }, + "loop": { "model": "sonnet", "effort": "high" }, + "review": { "model": "opus", "effort": "high" }, + "memory-promote": { "model": "opus", "effort": "high" }, + "ac-test": { "model": "opus", "effort": "high" } } } } ``` -Valid `model` ids: `opus-4-8`, `sonnet-5`, `fable-5`, `haiku-4-5`. Valid `effort` levels: `low`, -`high`, `max`. A hand-edit with an unknown model, effort, or stage key is rejected loudly on the next -command — see [troubleshooting](../troubleshooting.md); `pharn` never silently falls back. +The rules are those of pharn-oss's checker (`check-model-config.mjs validate`): + +| Rule | What pharn-oss accepts | +| ----------- | ------------------------------------------------------------------------------------------------------------------------------------------- | +| Shape | `models.stages` maps a stage name to `{ "model": …, "effort": … }` | +| `default` | Required, **inside** `stages` — what every stage without its own entry resolves to | +| Stage names | `default`, or a product stage: `spec`, `plan`, `grill`, `build`, `regress`, `verify`, `ship`, `loop`, `review`, `memory-promote`, `ac-test` | +| `model` | An alias — `sonnet`, `opus`, `haiku`, `fable`, `inherit` — or a full id matching `claude-[a-z0-9][a-z0-9-]*` | +| `effort` | `low`, `medium`, `high`, `xhigh` or `max` | + +A config with no `models` block, or a block with no `stages`, declares nothing, and pharn-oss's +checker passes it by design. + +### What each command does with it + +- **`pharn init`** copies pharn-oss's block **verbatim** from the root `pharn.config.json` of the + commit it installs, and prints it resolved per stage. If pharn-oss ships no block, `init` writes + **none** — it never invents one. If pharn-oss ships a block this pharn cannot accept, `init` writes + none and says why (see below). A re-run `init` replaces your block with pharn-oss's, as it rewrites + every other key pharn writes. +- **`pharn update`** treats the block like a file: pharn-oss's block replaces one pharn wrote, and one + you changed is kept. It also converts the format earlier releases wrote. See + [update](../commands/update.md#the-models-block). +- **`pharn status`** prints every product stage beside the model and effort it resolves to — marked + `(default)` when the stage has no entry of its own — under the label above, and lists by name + anything pharn-oss's rules reject. See [status](../commands/status.md). +- **`add`, `remove` and `list`** leave the block exactly as it is. + +No command refuses to run over this block: `pharn` does not validate it when it loads the config, +because it is not `pharn`'s schema. `pharn` checks it where it copies, converts or shows it, with a +**copy** of pharn-oss's rules. A test in the pharn repository runs pharn-oss's checker beside that +copy over a corpus of cases and fails on any difference. If pharn-oss's rules move ahead of the copy in your pharn +(a new stage, a new alias), `pharn` does not apply the newer block, names what it could not accept, and +upgrading `pharn` fixes it. pharn-oss can also require the upgrade with `MIN_CLI` (see +[`pharn is too old for the current pharn-oss`](../commands/update.md#pharn-is-too-old-for-the-current-pharn-oss)). + +### The format `pharn` wrote before 0.7.0 + +Releases up to 0.6.0 wrote a block of their own: a top-level `default`, and model ids — `opus-4-8`, +`sonnet-5`, `fable-5`, `haiku-4-5` — that neither Claude Code nor pharn-oss's checker accepts. The +first `pharn update` with 0.7.0 or later fixes it, even when your skills version is current: an +unedited block is replaced with pharn-oss's, and an edited one is converted and kept. See +[update](../commands/update.md#the-models-block). ## Seam resolution -The `seam` block records how PHARN should resolve an unfamiliar integration point. Like `models`, it is -**written on every fresh install** and **user-owned afterwards** — `pharn` never migrates it — and it is -validated on every command, so a bad hand-edit fails loudly rather than being ignored. Source of truth: +The `seam` block records how PHARN should resolve an unfamiliar integration point. It is **written on +every fresh install** and **user-owned afterwards** — `pharn` never migrates it — and it is validated +on every command, so a bad hand-edit fails loudly rather than being ignored. Source of truth: [`seam-config.ts`](../../src/lib/seam-config.ts). The installed default: @@ -197,9 +229,11 @@ step in the order. ## Keys pharn does not own -Every field on this page is **pharn's**, including the [legacy fields](#legacy-fields-pre-archetype-configs-still-load) -it no longer writes. Any **other** top-level key is **yours**: `pharn` does not interpret or validate it, -and every command that writes this file keeps it. Upstream PHARN documents two that you add by hand: +Every field on this page is **pharn's** to write, including the +[legacy fields](#legacy-fields-pre-archetype-configs-still-load) it no longer writes — `models` too, +though pharn-oss defines what goes inside it. Any **other** top-level key is **yours**: `pharn` does not +interpret or validate it, and every command that writes this file keeps it. Upstream PHARN documents +two that you add by hand: | Key | Read by | What it sets | | ------------- | -------------------------------------------------------------- | ------------------------------------------------------------ | @@ -225,13 +259,15 @@ How each command keeps them: - `pharn init` writes the file afresh from its own fields, then **copies every key pharn does not own across from the config it replaces**, unchanged, after its own. It does this for any file that parses as a JSON object, including one the other commands refuse: a config missing its `modules` - array (the case where they tell you to run `pharn init`), or one with an invalid `models` or `seam` - block. A file that is **not valid JSON** carries nothing over. Move it aside first + array (the case where they tell you to run `pharn init`), or one with an invalid `seam` block. A + file that is **not valid JSON** carries nothing over. Move it aside first ([troubleshooting](../troubleshooting.md#the-config-is-not-valid-json)), then copy your keys back. `init` **never** carries over a key pharn owns. It writes `pharnVersion`, `skillsVersion`, `repo`, -`commit`, `installedAt`, `archetypes`, `capabilities`, `layout` and `modules` fresh. `models` and -`seam` go back to their defaults, so a hand-edit there does not survive a re-run `init`. +`commit`, `installedAt`, `archetypes`, `capabilities`, `layout` and `modules` fresh. `models` becomes +pharn-oss's block again (or is left out, when pharn-oss ships none) and `seam` goes back to its +default, so a hand-edit in either does not survive a re-run `init` — unlike `pharn update`, which +keeps an edited `models` block. `pendingSkillsVersion`, `frozenCapabilities` and the legacy fields below are dropped. (`capabilities` is rewritten too, but the entries you added with `pharn add` are kept as `manual` — see the [init command](../commands/init.md#6-summary).) diff --git a/docs/reference/pharn-records.md b/docs/reference/pharn-records.md index b73c7d31..0833e9da 100644 --- a/docs/reference/pharn-records.md +++ b/docs/reference/pharn-records.md @@ -5,7 +5,9 @@ A CLI-owned sidecar written next to [`pharn.config.json`](pharn-config.md) at yo capabilities collected by `runInstallArchetype` (`collectExpectedInstallPaths`), not every file `pharn` writes. [`pharn update`](../commands/update.md) compares against it to tell pharn's bytes from your edits and refuse to destroy the latter. `pharn.config.json`, this file, and `.claude/settings.json` -are excluded from the map. +are excluded from the map — with one exception that is not a file: `pharn.config.json#/models` records +the `models` block `pharn` wrote **inside** `pharn.config.json` (see +[The `models` block](#the-models-block)). Source: [`install-records.ts`](../../src/lib/install-records.ts). @@ -42,14 +44,25 @@ disagree with what is actually on disk. Keys are sorted, so the committed file h | Command | Effect | | -------- | ------------------------------------------------------------------------------------------------------ | -| `init` | Writes the full store — every file the install wrote | +| `init` | Writes the full store — every file the install wrote, and the `models` block when it wrote one | | `add` | Merges the added capability's files in. Only extends an **already readable** store; it never mints one | -| `update` | Rewrites it, keyed by the manifest it just applied (see [Pruning](#pruning)) | +| `update` | Rewrites it, keyed by the manifest it just applied (see [Pruning](#pruning)), and the `models` record | | `remove` | Prunes the removed capability's entries. Only edits an **already readable** store; it never mints one | `.claude/settings.json` is **never** recorded: it is yours, and the install only ever creates it when absent. +## The `models` block + +`pharn.config.json#/models` holds the sha256 of the `models` block `pharn` last wrote into +`pharn.config.json` — the block as `pharn` serializes it (`JSON.stringify`: keys in order, no +whitespace). So re-indenting your config is not an edit, while reordering the block's keys is. +`init` writes it with the block; `update` rewrites it when it writes pharn-oss's block, and carries it +unchanged when it keeps yours; `add` and `remove` carry it untouched. `update` compares your block +against it exactly as it compares a file against its record — see +[update](../commands/update.md#the-models-block). Like every key here, it is only compared, never used +as a path, and a store without it reads the block as `unrecorded`. + ## When the store is ignored (fail-closed) `update` treats the store as **unavailable** — and therefore skips every **present** file that diff --git a/docs/roadmap.md b/docs/roadmap.md index 293504b2..d84974b2 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -22,7 +22,8 @@ What PHARN CLI does today versus what is planned. | Proxy-environment warning — Node's `fetch` reads no proxy variable, so a configured one is reported as unused | Shipped | | Per-command option allowlist — an option a command does not take exits 1 instead of being ignored | Shipped | | Interactive-only `init`/`update` (exit 1 off a TTY; `update --yes` for CI) + the capability picker for bare `add`/`remove` | Shipped | -| `models` + `seam` blocks written and validated on every fresh install | Shipped | +| `seam` block written and validated on every fresh install | Shipped | +| pharn-oss's `models` block — copied by `init`, kept or upgraded by `update` (which converts the format `pharn` wrote before 0.7.0), shown resolved per stage by `status` | Shipped | ## Planned @@ -32,7 +33,6 @@ What PHARN CLI does today versus what is planned. | Stack scaffolding | Install npm packages / generate app code for a detected framework | | Migration for existing projects | Onboard repos with significant git history (today the CLI only requires a `.git` directory and hard-fails without one; it never inspects history and issues no warning) | | Orphaned-file detection in `pharn status` | `status` today reports modified, missing and unreadable PHARN-owned files; flagging files left orphaned after an upstream rename is not built yet | -| Per-stage model routing | `pharn init` writes and validates the `models` block and both `init` and `status` display it, but no installed stage consumes it for routing — editing it changes which model runs nowhere. When a consumer lands, drop this row and the **Coming soon** marker in [pharn.config.json](reference/pharn-config.md#model-routing) | | Other agents | Codex and Cursor in addition to Claude Code | ## Related diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 614a9d2a..ae6a5bca 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -449,15 +449,18 @@ If your `pharn.config.json` predates the archetype model (it has `modules` but n ## A command rejects an invalid config (does NOT say "run init") ```text -models.default has invalid model "gpt-4" (expected one of opus-4-8, sonnet-5, fable-5, haiku-4-5) +seam.resolutionOrder[1] has invalid step "guess" (expected one of official-skill, pinned-docs, fetch, model, ask) ``` If `pharn.config.json` **exists but was hand-edited into an invalid state**, `add` / `status` / `update` / `remove` / `list` print the loud, specific error above (naming the offending field) and exit non-zero — they do **not** say to run `pharn init` (the file is there; re-running init would -offer to clobber your edits). Fix the named field and re-run. The `models` / `seam` blocks reject an -out-of-enum value, an unknown key (e.g. a typo'd `stgaes` / `haltOnUnknwon`), a duplicate -`resolutionOrder` step, or a `modelConfidenceThreshold` with no `model` step to gate. +offer to clobber your edits). Fix the named field and re-run. The `seam` block rejects an out-of-enum +value, an unknown key (e.g. a typo'd `haltOnUnknwon`), a duplicate `resolutionOrder` step, or a +`modelConfidenceThreshold` with no `model` step to gate. + +The `models` block is **not** checked here: it is pharn-oss's, and no command refuses to run over it. +See [`pharn status` lists a problem with `models`](#pharn-status-lists-a-problem-with-models). ### The config is not valid JSON @@ -469,8 +472,9 @@ A syntax error — most often a trailing comma — is reported with the file's f JSON parser supplies one, the **line and column** of the offending byte. Open the file at that position and fix it; nothing else is needed, and nothing has been written. -**Do not reach for `pharn init` here.** It rewrites `pharn.config.json` wholesale: hand-edited -`models` / `seam` blocks go back to defaults, keys of your own such as `testResults` are not carried +**Do not reach for `pharn init` here.** It rewrites `pharn.config.json` wholesale: a hand-edited +`models` block becomes pharn-oss's again, `seam` goes back to its default, keys of your own such as +`testResults` are not carried over, and every capability is re-stamped `source: "auto"`, which discards the record of which capabilities you added by hand with `pharn add`. That record lives nowhere else, and `pharn update` reads it to keep your manual additions across upgrades. @@ -492,6 +496,30 @@ permissions problem, a directory, a FIFO or a device sitting at that path, or a still says [`No pharn.config.json found`](#add--update-say-to-run-init-first). +## `pharn status` lists a problem with `models` + +```text +pharn-oss's rules reject this block: + stage "plan" model "gpt-4" is not an alias {sonnet, opus, haiku, fable, inherit} nor a claude-* id +``` + +The `models` block in `pharn.config.json` is pharn-oss's (see +[Models](reference/pharn-config.md#models)), so no `pharn` command refuses to run over it. `status` +lists what pharn-oss's rules reject, and you fix it by hand. The checker installed with PHARN says the +same: `node pharn/floor/check-model-config.mjs validate` (`.dev/floor/` in the legacy flat layout). + +- **"In the format pharn wrote before 0.7.0"** — run `pharn update`. An unedited block is replaced + with pharn-oss's; an edited one is converted to pharn-oss's format, keeping your values. Anything it + cannot convert is listed for you to fix. +- **A stage, alias or effort pharn-oss added after your `pharn` was released** — upgrade `pharn`: the + copy of pharn-oss's rules it checks with predates the change. +- **Your values differ from the installed commands' frontmatter** — not an error here. + `node pharn/floor/check-model-config.mjs agreement` lists each difference. Claude Code applies the + frontmatter, not the block, so change both or neither. + +`pharn update` lists the same problems when it keeps your block, and `pharn init` names them when +pharn-oss's own block fails — it writes none then. + ## Unknown command ```text diff --git a/src/commands/list.ts b/src/commands/list.ts index c292c08e..6b99db9d 100644 --- a/src/commands/list.ts +++ b/src/commands/list.ts @@ -40,9 +40,9 @@ export async function runList(opts: { json?: boolean } = {}): Promise { try { config = readPharnConfig(cwd); } catch (e) { - // A present-but-invalid models/seam block: surface the loud, named error - // (json-aware, to stderr) + exit — never the "run init" lie (BUG 1). A - // non-config error is a bug: rethrow, never swallow. + // A present-but-invalid seam block or capabilities entry: surface the + // loud, named error (json-aware, to stderr) + exit — never the "run init" + // lie (BUG 1). A non-config error is a bug: rethrow, never swallow. if (isConfigValidationError(e)) { emitError(e.message, json); process.exit(1); diff --git a/src/commands/status.ts b/src/commands/status.ts index 23861fdc..02c8675d 100644 --- a/src/commands/status.ts +++ b/src/commands/status.ts @@ -13,9 +13,16 @@ import { diffInstalledCapabilities } from '../lib/diff.js'; import { parseCapabilityIndex } from '../lib/capability-index.js'; import { unknownCapabilitiesWarning } from '../lib/unknown-capabilities.js'; import type { InstallDiff } from '../lib/diff.js'; -import { configLayout } from '../lib/layout.js'; +import { configLayout, layoutPaths } from '../lib/layout.js'; import { row } from '../lib/format.js'; -import { formatModelRoutingLines } from '../lib/model-routing-format.js'; +import { checkModelsBlock } from '../lib/model-config.js'; +import { + modelsCheckerCommand, + modelsLabelLines, + resolvedStageLines, +} from '../lib/model-config-format.js'; +import { needsModelsConversion } from '../lib/models-update.js'; +import { terminalSafe } from '../lib/terminal-safe.js'; import { loadArchetypeConfigOrExit } from '../lib/pharn-config.js'; import { errorMessage, reportFatal } from '../lib/report-error.js'; import { @@ -95,7 +102,7 @@ async function runArchetypeStatus( process.exit(1); } const outdated = printArchetypeVersion(config, latest); - printModelRouting(config); + printModels(config); if (strict && outdated) process.exit(1); outro(pc.dim('Read-only — nothing changed (drift check skipped).')); return; @@ -116,7 +123,7 @@ async function runArchetypeStatus( let exitCode = 0; try { const outdated = printArchetypeVersion(config, readSkillsVersion(repo.dir)); - printModelRouting(config); + printModels(config); // Exclude FROZEN capabilities — the ones the fetch boundary could not parse // in this clone. `update` deliberately keeps their config entry but never // writes their files, so comparing them here would report drift that no @@ -183,23 +190,48 @@ function printArchetypeVersion(config: PharnConfig, latest: string): boolean { return outdated; } -// MODELS note: the per-stage routing recorded in pharn.config.json, rendered -// from the same config via formatModelRoutingLines (the init summary's "Models -// per stage" block, mirrored here). Omitted when `models` is absent — a -// pre-`models` archetype config (P7 additive/legacy). Read-only: display only — -// and the trailing qualifier keeps it honest: the block is written, validated -// and shown, but NO installed command reads it, so these lines report a -// recorded intent, not the model a stage will run (docs/roadmap.md, Planned). -function printModelRouting(config: PharnConfig): void { +// MODELS note: the `models` block, resolved per product stage — a declaration, +// labeled as one. Claude Code applies each /pharn-* command's own frontmatter; +// the block is the source of truth that frontmatter is held to, and pharn-oss's +// checker (installed under the floor dir) is what compares the two. Omitted +// when `models` is absent (P7 additive/legacy). Read-only, local (no clone), +// and NOT a `--strict` input: the block's verdict is pharn-oss's checker's to +// give — this note reports it, through this CLI's pinned copy of its rules. +function printModels(config: PharnConfig): void { if (config.models === undefined) return; - note( - [ - ...formatModelRoutingLines(config.models), - '', - pc.dim('Recorded only — no installed stage reads this yet.'), - ].join('\n'), - 'MODELS', - ); + const floor = layoutPaths(configLayout(config)).floor; + note(modelsNoteLines(config.models, floor).join('\n'), 'MODELS'); +} + +function modelsNoteLines(block: unknown, floor: string): string[] { + if (needsModelsConversion(block)) { + return [ + 'In the format pharn wrote before 0.7.0: a top-level `default` and', + "model ids such as `opus-4-8`, which Claude Code and pharn-oss's", + 'checker reject.', + pc.dim('`pharn update` converts it: a block pharn wrote and you never'), + pc.dim("changed becomes pharn-oss's; an edited one keeps your values."), + ]; + } + const check = checkModelsBlock(block); + if (check.kind === 'invalid') { + return [ + "pharn-oss's rules reject this block:", + ...check.reds.map((red) => ` ${terminalSafe(red.detail, { max: 300 })}`), + pc.dim('Fix it by hand. The checker installed with PHARN says the same:'), + pc.dim(modelsCheckerCommand(floor, 'validate')), + ]; + } + if (check.kind === 'no-stages') { + return [ + "Declares no stages: pharn-oss's checker reads that as none declared.", + ]; + } + return [ + ...resolvedStageLines(check.stages), + '', + ...modelsLabelLines(floor).map((line) => pc.dim(line)), + ]; } // DRIFT note: differing, missing and unreadable PHARN-owned files, or a clean diff --git a/src/commands/update.ts b/src/commands/update.ts index 7d8c6201..98ff1390 100644 --- a/src/commands/update.ts +++ b/src/commands/update.ts @@ -45,12 +45,21 @@ import { } from '../lib/layout.js'; import { buildRecords, + MODELS_RECORD_KEY, readRecords, recordsBaseline, recordsUnderCapabilities, RECORDS_FILE, writeRecords, } from '../lib/install-records.js'; +import { modelsLabelLines } from '../lib/model-config-format.js'; +import { + decideModelsUpdate, + modelsMigrationPending, + type ModelsUpdate, +} from '../lib/models-update.js'; +import { readUpstreamModels } from '../lib/upstream-models.js'; +import { terminalSafe } from '../lib/terminal-safe.js'; import { planUpdate, type UpdateLabel, @@ -66,6 +75,7 @@ import { import { row } from '../lib/format.js'; import { assertConfigUnchanged, + CONFIG_FILENAME, loadArchetypeConfigOrExit, writePharnConfig, } from '../lib/pharn-config.js'; @@ -145,6 +155,10 @@ interface UpdateOutcome { // Upstream hooks the project's settings.json does not wire. Reported only — // update never writes settings.json. Absent on paths that never saw a clone. hookWiring?: HookWiringDiff; + // What happened to the `models` block (lib/models-update.ts), and the floor + // dir its checker is installed under, for the MODELS note. + models: ModelsUpdate; + modelsFloor: string; } async function runArchetypeUpdate( @@ -206,7 +220,19 @@ async function runArchetypeUpdate( // repeat and its bytes would stay stale after a pharn upgrade that can parse it. const current = config.skillsVersion === latest; const recheckFrozen = (config.frozenCapabilities ?? []).length > 0; - if (current && !force && !recheckFrozen) { + // A `models` block still in the format pharn wrote before 0.7.0 re-opens the + // gate as well: converting it is this CLI's job, not upstream's, so it has to + // reach an install that is already current. Bounded: a run converts the block + // or writes pharn-oss's and records it, and either closes the gate + // (modelsMigrationPending). A local read — no round-trip. + const convertModels = modelsMigrationPending( + config.models, + recordsBaseline(readRecords(cwd), { + skillsVersion: config.skillsVersion, + commit: config.commit, + }).records?.[MODELS_RECORD_KEY] ?? null, + ); + if (current && !force && !recheckFrozen && !convertModels) { outro(`Already up to date (skills v${config.skillsVersion}).`); return; } @@ -226,6 +252,13 @@ async function runArchetypeUpdate( : pc.dim( ' Files you have changed are kept, not overwritten — they are listed at the end.', ), + ...(convertModels + ? [ + pc.dim( + ' Your models block is in the format pharn wrote before 0.7.0 — this run converts it.', + ), + ] + : []), pc.dim( ' Re-resolves your archetypes against the latest capabilities and re-copies them.', ), @@ -499,6 +532,17 @@ async function applyUpdate( const plan = planUpdate({ latestHashes, diskStates, records, force }); + // The `models` block goes through the same rows, over its hash instead of a + // file's (lib/models-update.ts): pharn-oss's block replaces one pharn wrote, + // and one the user edited is kept — converted when it is in the old format. + const models = decideModelsUpdate({ + current: config.models, + upstream: readUpstreamModels(repoDir), + recorded: records?.[MODELS_RECORD_KEY] ?? null, + recordsAvailable: records !== null, + force, + }); + // Back up EVERY about-to-be-clobbered file before a single original is // touched; a failure here aborts with the whole tree still intact. // @@ -506,15 +550,22 @@ async function applyUpdate( // pointer is handed UP here rather than only in the return value below: every // line after this one (the writes, the records, the config) can throw, and a // run that dies there has already moved the user's bytes into that directory. + // A `--force` over the user's models block backs up the whole config: the + // block lives inside it. + const backupRels = models.backup + ? [...plan.backups, CONFIG_FILENAME] + : plan.backups; const backup: Backup | null = - plan.backups.length > 0 - ? { dir: createBackup(cwd, plan.backups), count: plan.backups.length } + backupRels.length > 0 + ? { dir: createBackup(cwd, backupRels), count: backupRels.length } : null; if (backup) onBackup(backup); // A run that could not apply everything must not claim the new version: the // recorded version describes the last COMPLETE state, so the next `pharn - // update` still has work to do instead of early-returning forever. + // update` still has work to do instead of early-returning forever. A KEPT + // models block is not such a skip: it is configuration, not bytes of a + // version, and a block the user tuned is a steady state, not unfinished work. const versionWithheld = plan.counts.skipped > 0; const nextSkillsVersion = versionWithheld ? config.skillsVersion @@ -596,6 +647,11 @@ async function applyUpdate( ...frozenRecords, ...plan.nextRecords, ...buildRecords(cwd, written), + // `planUpdate` keys its records by the manifest, which holds no models + // block, so the block's record is written here. + ...(models.nextRecord !== null + ? { [MODELS_RECORD_KEY]: models.nextRecord } + : {}), }, }); // Only KEPT entries count: an unparseable capability this project never had @@ -641,6 +697,9 @@ async function applyUpdate( capabilities: configCapabilities, layout, installedAt: new Date().toISOString(), + // In place: the key keeps its position in the file, and `undefined` (there + // was none and there is none to write) writes no key. + models: models.next, }); return { @@ -658,6 +717,8 @@ async function applyUpdate( records !== null && FEATURES_README in records && resolveFeaturesReadme(repoDir, layout) !== FEATURES_README, + models, + modelsFloor: layoutPaths(layout).floor, }; } @@ -695,6 +756,7 @@ function reportOutcome(outcome: UpdateOutcome, force: boolean): void { if (recordsNote) log.warn(`⚠ ${recordsNote}`); reportCapabilityChanges(outcome.capabilityChanges); + reportModels(outcome.models, outcome.modelsFloor, outcome.backup); const hookLines = outcome.hookWiring ? hookWiringLines(outcome.hookWiring) @@ -875,6 +937,114 @@ function reportCapabilityChanges(changes: CapabilityChange[]): void { note(lines.join('\n'), 'CAPABILITIES'); } +// The MODELS note: what happened to the `models` block, for every outcome but +// `ok` (already pharn-oss's) and a quiet `upstream-absent` (nothing upstream, +// nothing of yours to convert or flag). Every string quoted from a block — +// stage names, values, upstream's reasons — goes through `terminalSafe`. The +// note's own lines stay within 70 columns, so an 80-column box never wraps +// one mid-sentence. +function reportModels( + models: ModelsUpdate, + floor: string, + backup: Backup | null, +): void { + const lines = modelsReportLines(models, backup); + if (lines.length === 0) return; + if (models.next !== undefined) { + lines.push('', ...modelsLabelLines(floor).map((line) => pc.dim(line))); + } + note(lines.join('\n'), 'MODELS'); +} + +function modelsReportLines( + models: ModelsUpdate, + backup: Backup | null, +): string[] { + const quoted = (text: string): string => + ` ${terminalSafe(text, { max: 300 })}`; + const lines: string[] = []; + switch (models.outcome) { + case 'ok': + return []; + case 'restored': + lines.push("Your config had no models block — wrote pharn-oss's."); + break; + case 'updated': + lines.push( + ...(models.replacedLegacyDefault + ? [ + "Replaced the models block an earlier pharn wrote with pharn-oss's.", + 'Its model ids, such as opus-4-8, are rejected by Claude Code', + "and by pharn-oss's checker.", + ] + : [ + "Moved your models block to pharn-oss's current one: you had", + 'not changed it since pharn wrote it.', + ]), + ); + break; + case 'forced': + lines.push( + "Replaced your models block with pharn-oss's (--force).", + `Your previous ${CONFIG_FILENAME} is in ${backup?.dir ?? `${BACKUP_DIR}/`}.`, + ); + break; + case 'kept': + lines.push(keptModelsHeading(models.label)); + break; + case 'upstream-invalid': + lines.push( + "pharn-oss's models block was not applied; this pharn rejects it:", + ...models.upstreamReasons.map(quoted), + 'If pharn-oss changed its models format, upgrade pharn', + '(npm i -g @pharn-dev/pharn@latest) and run `pharn update` again.', + ); + break; + case 'upstream-absent': + if (models.conversion === null && models.problems.length === 0) { + return []; + } + lines.push('pharn-oss ships no models block, so yours is kept.'); + break; + } + if (models.conversion !== null) { + lines.push( + '', + 'Converted your models block from the format pharn wrote before 0.7.0,', + 'keeping your values:', + ...models.conversion.changes.map(quoted), + ); + } + if (models.problems.length > 0) { + lines.push( + '', + "pharn-oss's rules still reject your models block — left as is,", + 'fix it by hand:', + ...models.problems.map(quoted), + ); + } + if (models.outcome === 'kept') { + lines.push( + '', + pc.dim("--force replaces it with pharn-oss's, after copying"), + pc.dim(`${CONFIG_FILENAME} to ${BACKUP_DIR}/.`), + ); + } + return lines; +} + +// Why a models block is the user's — `decideFileAction`'s skip labels. +function keptModelsHeading(label: ModelsUpdate['label']): string { + switch (label) { + case 'modified': + return 'Kept your models block — you changed it since pharn wrote it.'; + case 'unrecorded': + return 'Kept your models block — pharn has no record of writing it.'; + default: + return `Kept your models block — no usable ${RECORDS_FILE} to check it.`; + } +} + function skipHeading(label: string): string { switch (label) { case 'modified': diff --git a/src/lib/install-records.ts b/src/lib/install-records.ts index e0751c9a..2ae99628 100644 --- a/src/lib/install-records.ts +++ b/src/lib/install-records.ts @@ -1,3 +1,4 @@ +import { createHash } from 'node:crypto'; import { lstatSync } from 'node:fs'; import { writeJsonAtomic } from './atomic-write.js'; import { readBoundedFile } from './bounded-read.js'; @@ -24,7 +25,7 @@ import type { InstalledCapability } from '../types.js'; // KEY is never path-joined: consumers iterate the install manifest and look each // manifest-derived key up here, so a hostile key can never drive a filesystem // access. A corrupt store is reported BY NAME, never silently collapsed into -// "absent" (the same lesson lib/pharn-config.ts encodes for `models`/`seam`). +// "absent" (the same lesson lib/pharn-config.ts encodes for `seam`). // // THE STAMP: the store carries the `skillsVersion` + `commit` that // `pharn.config.json` holds after the operation that wrote it. Every pharn @@ -34,11 +35,34 @@ import type { InstalledCapability } from '../types.js'; // unavailable (fail closed) rather than trusting hashes that may describe bytes // nobody wrote (P5/P7). // +// ONE KEY IS NOT A FILE: `MODELS_RECORD_KEY` records the `models` block pharn +// wrote INSIDE pharn.config.json, so `pharn update` can tell pharn's block from +// the user's the same way it tells files apart (lib/models-update.ts). It is a +// JSON-Pointer fragment no install manifest can produce, and like every key it +// is compared, never path-joined. +// // One axis (P3): the install record store. // --------------------------------------------------------------------------- export const RECORDS_FILE = 'pharn.records.json'; +/** + * The record key for the `models` block pharn wrote into pharn.config.json. + * `add` and `remove` carry it untouched (they extend or prefix-filter `files`); + * `init` and `update` write it whenever they write the block. + */ +export const MODELS_RECORD_KEY = 'pharn.config.json#/models'; + +/** + * The recorded hash of a `models` block: sha256 of `JSON.stringify(block)` — + * the block as pharn serializes it, key order included, so re-indenting the + * file is not an edit and reordering keys is. Never called with `undefined` + * (an absent block has no record). + */ +export function modelsRecordHash(block: unknown): string { + return createHash('sha256').update(JSON.stringify(block)).digest('hex'); +} + // Exact-match schema discriminator (P5). A store written by a future CLI with a // different version is NOT guessed at — it reads as unavailable, and update // skips. Bumping this is the additive escape hatch P7 requires; the per-value diff --git a/src/lib/model-config-format.ts b/src/lib/model-config-format.ts new file mode 100644 index 00000000..62f09371 --- /dev/null +++ b/src/lib/model-config-format.ts @@ -0,0 +1,62 @@ +import { + PRODUCT_STAGES, + resolveStageModel, + type ModelsStages, +} from './model-config.js'; + +// --------------------------------------------------------------------------- +// Showing the `models` block — the rows `pharn status`, `pharn init` and +// `pharn update` print, and the one label they all print under them. +// +// The label is the point. Claude Code applies each /pharn-* command's own +// static `model:` / `effort:` frontmatter; nothing reads this block to pick a +// model. The block is the source of truth that frontmatter is HELD TO, by +// pharn-oss's checker (its `agreement` mode). So the rows are a declaration, +// and they are never presented as routing that happens. +// +// Pure: no color, no clack — callers add their own chrome. Every token in a +// row is a product stage (this CLI's pinned list) or a value pharn-oss's rules +// accepted (an alias, a `claude-*` id, an effort level), so a row carries no +// untrusted free text. One axis (P3): presenting the models block. +// --------------------------------------------------------------------------- + +/** The command that runs pharn-oss's checker in a project, in one mode. */ +export function modelsCheckerCommand( + floorDir: string, + mode: 'validate' | 'agreement', +): string { + return `node ${floorDir}/check-model-config.mjs ${mode}`; +} + +/** + * One row per product stage, resolved as pharn-oss resolves it, after the + * `default` row: a stage without its own entry shows the `default` it falls + * back to, marked `(default)`. The stage order is pharn-oss's. + */ +export function resolvedStageLines(stages: ModelsStages): string[] { + const rows: Array< + [label: string, model: string, effort: string, via: boolean] + > = [['default', stages.default!.model, stages.default!.effort, false]]; + for (const stage of PRODUCT_STAGES) { + const { model, effort } = resolveStageModel(stages, stage); + rows.push([stage, model, effort, !Object.hasOwn(stages, stage)]); + } + const width = Math.max(...rows.map(([label]) => label.length)) + 3; + return rows.map( + ([label, model, effort, via]) => + `${label.padEnd(width)}${model} · ${effort}${via ? ' (default)' : ''}`, + ); +} + +/** + * What the rows are, and what they are not. Each line stays within 70 + * columns, so a clack note box in an 80-column terminal never wraps one. + */ +export function modelsLabelLines(floorDir: string): string[] { + return [ + "Claude Code applies each /pharn-* command's own model:/effort:", + 'frontmatter, not this block. The block is the source of truth', + 'that frontmatter is held to; to check the two agree:', + modelsCheckerCommand(floorDir, 'agreement'), + ]; +} diff --git a/src/lib/model-config.ts b/src/lib/model-config.ts new file mode 100644 index 00000000..e6346a25 --- /dev/null +++ b/src/lib/model-config.ts @@ -0,0 +1,202 @@ +import { isPlainObject } from './validate.js'; + +// --------------------------------------------------------------------------- +// pharn-oss's rules for the `models` block — a COPY, and only a copy. +// +// pharn-oss owns the `models` schema of pharn.config.json and its defaults. +// Its checker, `pharn/floor/check-model-config.mjs`, ships into every install, +// and pharn-oss's own root pharn.config.json carries the block `pharn init` +// copies. This file is a literal port of that checker's `validate` and +// `resolve` rules (`stagesOf`, `validateStages`, `resolveStage`): the same +// stage set, the same model and effort sets, the same RED kinds with the same +// wording, in the same order. +// +// Why a copy and not a call: this CLI never executes a file it installs +// (THREAT-MODEL.md §1). `pharn status --strict` runs in CI on pull requests, +// and at `init` the only checker is the one inside the downloaded clone. +// +// Why it cannot drift from that checker unnoticed: +// tests/model-config-parity.test.ts runs the real checker — vendored +// byte-for-byte at tests/fixtures/pharn-oss/ and pinned by sha256 — over a +// corpus, compares every verdict and RED line with this file's, and reads the +// four sets below out of the checker's own source. The vendored checker can +// still lag live upstream until someone refreshes it (docs/contributing.md); +// that lag is the residual below. pharn-oss stays the one owner of every rule +// here: add nothing the checker does not have, and change nothing it has not +// changed. +// +// When upstream widens a set before this copy follows, a block using the new +// member is rejected here and NOT applied — named, never fatal (LIMITS.md §3e); +// upstream's MIN_CLI is the lever that asks users to upgrade first. +// +// Pure: no I/O. One axis (P3): pharn-oss's rules for the models block. +// --------------------------------------------------------------------------- + +/** + * The product stages a `models.stages` key may name besides `default`, in the + * checker's order (`PRODUCT_STAGES`, whose values are the command files; only + * the keys are a rule of the block). + */ +export const PRODUCT_STAGES = [ + 'spec', + 'plan', + 'grill', + 'build', + 'regress', + 'verify', + 'ship', + 'loop', + 'review', + 'memory-promote', + 'ac-test', +] as const; + +/** Model aliases the checker accepts (`MODEL_ALIASES`). */ +export const MODEL_ALIASES = [ + 'sonnet', + 'opus', + 'haiku', + 'fable', + 'inherit', +] as const; + +/** A full model id (`MODEL_ID_RE`), e.g. `claude-opus-4-8`. */ +export const MODEL_ID_RE = /^claude-[a-z0-9][a-z0-9-]*$/; + +/** Effort levels the checker accepts (`EFFORT_ENUM`). */ +export const EFFORT_LEVELS = ['low', 'medium', 'high', 'xhigh', 'max'] as const; + +/** One stage's declared model and effort, once the block has passed. */ +export interface StageEntry { + model: string; + effort: string; +} + +/** + * A `models.stages` map that passed `checkModelsBlock`: `default` is present, + * every key is `default` or a product stage, every entry carries a valid model + * and effort. Entries may carry other keys too (the checker ignores them). + */ +export type ModelsStages = Readonly>; + +/** The checker's RED kinds on this path — its own `red(kind, …)` names. */ +export type ModelsRedKind = + 'shape' | 'default' | 'stage' | 'entry' | 'model' | 'effort'; + +export interface ModelsRed { + kind: ModelsRedKind; + // The checker's own detail text. It quotes keys and values from the block + // with JSON.stringify, which escapes C0 controls but not C1 controls or + // Unicode format characters: pass it through `terminalSafe` before printing. + detail: string; +} + +export type ModelsCheck = + // No block, or a block with no `stages`: nothing is declared. The checker's + // "GREEN by design" — an install need not use the block. + | { kind: 'no-stages' } + | { kind: 'valid'; stages: ModelsStages } + | { kind: 'invalid'; reds: ModelsRed[] }; + +function isValidModel(model: unknown): boolean { + return ( + typeof model === 'string' && + ((MODEL_ALIASES as readonly string[]).includes(model) || + MODEL_ID_RE.test(model)) + ); +} + +function isValidEffort(effort: unknown): boolean { + return ( + typeof effort === 'string' && + (EFFORT_LEVELS as readonly string[]).includes(effort) + ); +} + +/** + * Check a `models` block (the value of the `models` key — `undefined` when + * the key is absent) against pharn-oss's rules: the checker's `validate`. + * Every RED is collected, not just the first, exactly as the checker prints + * them. + */ +export function checkModelsBlock(models: unknown): ModelsCheck { + // stagesOf + if (models === undefined || models === null) return { kind: 'no-stages' }; + if (!isPlainObject(models)) { + return invalid('shape', '`models` is present but is not an object'); + } + const stages = models.stages; + if (stages === undefined || stages === null) return { kind: 'no-stages' }; + if (!isPlainObject(stages)) { + return invalid('shape', '`models.stages` is present but is not an object'); + } + + // validateStages + const reds: ModelsRed[] = []; + if (!Object.hasOwn(stages, 'default')) { + reds.push({ + kind: 'default', + detail: + 'missing required `default` stage entry (the resolution fallback)', + }); + } + for (const [name, entry] of Object.entries(stages)) { + const stage = `stage ${JSON.stringify(name)}`; + if ( + name !== 'default' && + !(PRODUCT_STAGES as readonly string[]).includes(name) + ) { + reds.push({ + kind: 'stage', + detail: `${stage} is not a product stage — expected one of {${PRODUCT_STAGES.join(', ')}} or "default"`, + }); + continue; + } + if (!isPlainObject(entry)) { + reds.push({ + kind: 'entry', + detail: `${stage} is not an object with {model, effort}`, + }); + continue; + } + if (!Object.hasOwn(entry, 'model')) { + reds.push({ kind: 'model', detail: `${stage} missing \`model\`` }); + } else if (!isValidModel(entry.model)) { + reds.push({ + kind: 'model', + detail: `${stage} model ${JSON.stringify(entry.model)} is not an alias {${MODEL_ALIASES.join(', ')}} nor a claude-* id`, + }); + } + if (!Object.hasOwn(entry, 'effort')) { + reds.push({ kind: 'effort', detail: `${stage} missing \`effort\`` }); + } else if (!isValidEffort(entry.effort)) { + reds.push({ + kind: 'effort', + detail: `${stage} effort ${JSON.stringify(entry.effort)} not in {${EFFORT_LEVELS.join(', ')}}`, + }); + } + } + return reds.length > 0 + ? { kind: 'invalid', reds } + : { kind: 'valid', stages: stages as ModelsStages }; +} + +function invalid(kind: ModelsRedKind, detail: string): ModelsCheck { + return { kind: 'invalid', reds: [{ kind, detail }] }; +} + +/** + * The model and effort a stage resolves to — the checker's `resolve`: the + * stage's OWN entry, else `default`. An own-property test, so an inherited + * name (`constructor`, `toString`, `__proto__`) resolves to `default` rather + * than to something off the prototype. + */ +export function resolveStageModel( + stages: ModelsStages, + stage: string, +): StageEntry { + const entry = Object.hasOwn(stages, stage) ? stages[stage] : stages.default; + // `default` is present in every ModelsStages (checkModelsBlock's own RED). + const { model, effort } = entry!; + return { model, effort }; +} diff --git a/src/lib/model-routing-format.ts b/src/lib/model-routing-format.ts deleted file mode 100644 index 7226803a..00000000 --- a/src/lib/model-routing-format.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { PIPELINE_STAGES } from './model-routing.js'; -import type { ModelRouting, StageModel } from '../types.js'; - -/** - * Render a `models` block as aligned, one-line-per-entry display strings — the - * shared renderer behind the init summary's "Models per stage" block and - * `pharn status`'s MODELS note. `default` first, then each CONFIGURED stage in - * PIPELINE_STAGES order (a deterministic membership walk, P5 — never JSON key - * order). Pure: no color, no clack — callers add their own chrome; and pure of - * input too, so the same `ModelRouting` always renders the same lines (the test - * that feeds a non-default routing proves the block is rendered FROM the config, - * not re-hardcoded). Every emitted token is an allowlist member (model ∈ - * MODEL_IDS, effort ∈ EFFORT_LEVELS, stage ∈ PIPELINE_STAGES), so the output - * carries no untrusted free-text (P2). - * - * One axis (P3): PRESENTING the models block — split from model-routing.ts's - * schema/validation/resolution (REVIEW.md P3, so a display-format change no - * longer touches the validator). Reaches PIPELINE_STAGES via a `lib→lib` import - * (allowed — not a sibling-leaf reference). - */ -export function formatModelRoutingLines(routing: ModelRouting): string[] { - const entries: Array<[label: string, target: StageModel]> = [ - ['default', routing.default], - ]; - for (const stage of PIPELINE_STAGES) { - const target = routing.stages?.[stage]; - if (target) entries.push([stage, target]); - } - // Align the model column: pad every label to the longest + a 3-space gap - // (`default` is always present at length 7, so this is ≥ 10). Deterministic. - const width = Math.max(...entries.map(([label]) => label.length)) + 3; - return entries.map( - ([label, { model, effort }]) => - `${label.padEnd(width)}${model} · ${effort}`, - ); -} diff --git a/src/lib/model-routing.ts b/src/lib/model-routing.ts deleted file mode 100644 index 3d0ffec1..00000000 --- a/src/lib/model-routing.ts +++ /dev/null @@ -1,170 +0,0 @@ -import { isPlainObject } from './validate.js'; -import type { - EffortLevel, - ModelId, - ModelRouting, - PipelineStage, - StageModel, -} from '../types.js'; - -// Thrown when a `models` block fails validation. Names the offending -// model/effort/stage so a hand-edited pharn.config.json fails loudly, never -// silently (P5). -export class ModelRoutingError extends Error { - constructor(message: string) { - super(message); - this.name = 'ModelRoutingError'; - } -} - -// Runtime allowlists — the deterministic floor for the `models` block (enum -// membership, P5 / ARCHITECTURE.md §2 primitive #3). Each array is the runtime -// SoT paired with its types.ts union; `satisfies` keeps the two in lockstep (a -// typo or drift here is a compile error). -export const MODEL_IDS = [ - 'opus-4-8', - 'sonnet-5', - 'fable-5', - 'haiku-4-5', -] as const satisfies readonly ModelId[]; - -export const EFFORT_LEVELS = [ - 'low', - 'high', - 'max', -] as const satisfies readonly EffortLevel[]; - -export const PIPELINE_STAGES = [ - 'plan', - 'grill', - 'build', - 'regress', - 'verify', - 'review', - 'ship', -] as const satisfies readonly PipelineStage[]; - -// Sensible defaults written on every fresh install. Only stages that DEVIATE -// from `default` need an entry; the rest resolve to `default` at read time -// (resolveStageModel). Its stage keys are ⊆ PIPELINE_STAGES and its models ⊆ -// MODEL_IDS — asserted by validateModelRouting in the test suite. -export const DEFAULT_MODEL_ROUTING: ModelRouting = { - default: { model: 'sonnet-5', effort: 'high' }, - stages: { - plan: { model: 'opus-4-8', effort: 'max' }, // hardest reasoning - // review fans out across lenses (a backend install ships ~22), so its cost - // multiplies per lens — a premium model at max effort across that fan-out is - // the worst-case token multiplier, and it would apply silently. Default to - // opus-4-8/high (spend-safe); fable-5/max cross-model review has proven catch - // value but stays the documented release-audit OPT-IN (set - // models.stages.review in pharn.config.json). Effort calibration is gated on - // the fan-out cost measurement — no speculative knobs added here. - review: { model: 'opus-4-8', effort: 'high' }, - }, -}; - -// Known keys for the two shape checks below. An unknown sibling key (e.g. a -// typo'd `stgaes` beside `stages`, or a stray key inside a stage entry) is -// REJECTED, naming it (P5, fail-closed) — so a hand-edit typo can never silently -// disable per-stage routing. `satisfies` keeps each set honest against its type. -const ROUTING_KEYS = [ - 'default', - 'stages', -] as const satisfies readonly (keyof ModelRouting)[]; -const STAGE_MODEL_KEYS = [ - 'model', - 'effort', -] as const satisfies readonly (keyof StageModel)[]; - -// Reject any key not in `known`, naming the first offender (JSON-escaped so a -// control-char key is echoed as DATA, never raw — P2). Local to this axis. -function assertNoUnknownKeys( - obj: Record, - known: readonly string[], - label: string, -): void { - for (const key of Object.keys(obj)) { - if (!known.includes(key)) { - throw new ModelRoutingError( - `${label} has unknown key ${JSON.stringify(key)} (expected one of ${known.join(', ')})`, - ); - } - } -} - -// value ∈ allowlist. Mirrors validate.ts's assertRole/assertAppliesToken pattern -// (typeof guard + enum membership, not regex). Kept local to this axis. -function assertStageModel(value: unknown, label: string): StageModel { - if (!isPlainObject(value)) { - throw new ModelRoutingError(`${label} must be an object`); - } - assertNoUnknownKeys(value, STAGE_MODEL_KEYS, label); - const { model, effort } = value; - if ( - typeof model !== 'string' || - !(MODEL_IDS as readonly string[]).includes(model) - ) { - throw new ModelRoutingError( - `${label} has invalid model ${JSON.stringify(model)} (expected one of ${MODEL_IDS.join(', ')})`, - ); - } - if ( - typeof effort !== 'string' || - !(EFFORT_LEVELS as readonly string[]).includes(effort) - ) { - throw new ModelRoutingError( - `${label} has invalid effort ${JSON.stringify(effort)} (expected one of ${EFFORT_LEVELS.join(', ')})`, - ); - } - return { model: model as ModelId, effort: effort as EffortLevel }; -} - -/** - * Validate a `models` block (from pharn.config.json or DEFAULT_MODEL_ROUTING). - * Rejects — throwing ModelRoutingError, naming the offender — an invalid model - * string, an invalid effort, an unknown stage key, or a malformed shape. Returns - * the typed, validated routing. - * - * Deterministic floor (P0/P5): three fixed-set membership tests; fail-closed — - * a malformed block hard-fails, never a silent fallback. - */ -export function validateModelRouting(input: unknown): ModelRouting { - if (!isPlainObject(input)) { - throw new ModelRoutingError('models must be an object'); - } - // Reject unknown sibling keys (e.g. a typo'd `stgaes`) BEFORE validating the - // shape, so the typo — not a downstream "missing default" — is what is named. - assertNoUnknownKeys(input, ROUTING_KEYS, 'models'); - const routingDefault = assertStageModel(input.default, 'models.default'); - - const stages: Partial> = {}; - if (input.stages !== undefined) { - if (!isPlainObject(input.stages)) { - throw new ModelRoutingError('models.stages must be an object'); - } - for (const [stage, value] of Object.entries(input.stages)) { - if (!(PIPELINE_STAGES as readonly string[]).includes(stage)) { - throw new ModelRoutingError( - `models.stages has unknown stage ${JSON.stringify(stage)} (expected one of ${PIPELINE_STAGES.join(', ')})`, - ); - } - stages[stage as PipelineStage] = assertStageModel( - value, - `models.stages.${stage}`, - ); - } - } - return { default: routingDefault, stages }; -} - -/** - * Resolve the model + effort for a stage. A stage without an explicit entry - * (including an empty `stages`) resolves to `default` — deterministic membership - * + a single fallback, no guess (P5). - */ -export function resolveStageModel( - routing: ModelRouting, - stage: PipelineStage, -): StageModel { - return routing.stages?.[stage] ?? routing.default; -} diff --git a/src/lib/models-update.ts b/src/lib/models-update.ts new file mode 100644 index 00000000..50f70738 --- /dev/null +++ b/src/lib/models-update.ts @@ -0,0 +1,301 @@ +import { modelsRecordHash } from './install-records.js'; +import { checkModelsBlock } from './model-config.js'; +import { decideFileAction, type UpdateLabel } from './update-decision.js'; +import { isPlainObject } from './validate.js'; +import type { UpstreamModels } from './upstream-models.js'; + +// --------------------------------------------------------------------------- +// What `pharn update` does with the `models` block in pharn.config.json. +// +// The block's schema is pharn-oss's (lib/model-config.ts), but before 0.7.0 +// this CLI wrote a block of its own: a top-level `default`, and model ids +// (`opus-4-8`, `sonnet-5`, …) that neither Claude Code nor pharn-oss's checker +// accepts. That OLD format is this CLI's, so the knowledge of it lives here: +// the id map, the two defaults it ever wrote, and how an edited block +// converts. +// +// The decision is the per-file one — `decideFileAction`, called, not copied — +// with the block's hash standing in for a file's. So the rows are the rows: +// absent → restored; equal to pharn-oss's → ok; still what pharn wrote (its +// record, or one of the old defaults) → updated; anything else is the user's +// and is KEPT, unless `--force`, which backs up pharn.config.json first. A kept +// block in the old format is CONVERTED — never reset, never guessed at. +// +// Pure: no I/O. One axis (P3): update's treatment of the models block. +// --------------------------------------------------------------------------- + +/** The four model ids pharn wrote before 0.7.0 → the Claude Code alias. */ +const LEGACY_MODEL_ALIASES: ReadonlyMap = new Map([ + ['opus-4-8', 'opus'], + ['sonnet-5', 'sonnet'], + ['fable-5', 'fable'], + ['haiku-4-5', 'haiku'], +]); + +/** + * Every `models` block an earlier pharn wrote on a fresh install, serialized + * as pharn wrote it. A block equal to one of these was never edited — it is + * pharn's, records file or not. + */ +const LEGACY_DEFAULTS: readonly string[] = [ + // #57 (2026-07-23) through 0.6.0 — every published release. + JSON.stringify({ + default: { model: 'sonnet-5', effort: 'high' }, + stages: { + plan: { model: 'opus-4-8', effort: 'max' }, + review: { model: 'opus-4-8', effort: 'high' }, + }, + }), + // #24 (2026-07-07) until #57 — source builds only, never published. + JSON.stringify({ + default: { model: 'sonnet-5', effort: 'high' }, + stages: { + plan: { model: 'opus-4-8', effort: 'max' }, + review: { model: 'fable-5', effort: 'max' }, + }, + }), +]; + +/** + * Is this block, byte for byte as pharn serializes it, a default an earlier + * pharn wrote? `JSON.stringify` of the parsed block: re-indenting is not an + * edit, reordering keys is — the conservative reading, since an edited block + * is kept. + */ +export function isLegacyDefault(block: unknown): boolean { + return block !== undefined && LEGACY_DEFAULTS.includes(JSON.stringify(block)); +} + +export interface ModelsConversion { + // The converted block — a new object — or the input itself when nothing + // converted. + block: unknown; + // What was converted, one named line each. + changes: string[]; + // What could not be converted and was left exactly as it was. + leftovers: string[]; +} + +/** + * Convert a block from the format pharn wrote before 0.7.0 to pharn-oss's: a + * top-level `default` moves into `stages.default`, and each old model id + * becomes its alias. Everything else is copied verbatim. A `default` that + * cannot move (`stages.default` is already set, or `stages` is not an object) + * stays where it is and is named. Objects are rebuilt with spread and + * `Object.fromEntries`, which define own properties, so a `__proto__` key + * stays data. + */ +export function convertLegacyModels(block: unknown): ModelsConversion { + if (!isPlainObject(block)) return { block, changes: [], leftovers: [] }; + const changes: string[] = []; + const leftovers: string[] = []; + let next: Record = block; + + if (Object.hasOwn(next, 'default')) { + const { default: moved, ...rest } = next; + const stages = next.stages; + if (stages === undefined || stages === null) { + next = { ...rest, stages: { default: moved } }; + changes.push('moved `models.default` into `models.stages.default`'); + } else if (isPlainObject(stages) && !Object.hasOwn(stages, 'default')) { + next = { ...rest, stages: { default: moved, ...stages } }; + changes.push('moved `models.default` into `models.stages.default`'); + } else { + leftovers.push( + isPlainObject(stages) + ? '`models.default` was left where it is: `models.stages.default` is already set, and pharn-oss reads only that one' + : '`models.default` was left where it is: `models.stages` is not an object, so it cannot move into it', + ); + } + } + + const stages = next.stages; + if (isPlainObject(stages)) { + let mapped = false; + const converted = Object.fromEntries( + Object.entries(stages).map(([name, entry]) => { + if (!isPlainObject(entry) || typeof entry.model !== 'string') { + return [name, entry]; + } + const alias = LEGACY_MODEL_ALIASES.get(entry.model); + if (alias === undefined) return [name, entry]; + mapped = true; + changes.push( + `stage ${JSON.stringify(name)} model ${JSON.stringify(entry.model)} → ${JSON.stringify(alias)}`, + ); + return [name, { ...entry, model: alias }]; + }), + ); + if (mapped) next = { ...next, stages: converted }; + } + + return { block: changes.length > 0 ? next : block, changes, leftovers }; +} + +/** + * Does this block still hold something `convertLegacyModels` would convert? + * A block whose only leftover cannot move converts nothing. + */ +export function needsModelsConversion(block: unknown): boolean { + return convertLegacyModels(block).changes.length > 0; +} + +/** + * Should `update` go past its same-version early return to migrate this + * block? While it still needs converting — unless it is exactly the block + * pharn recorded writing (`recorded`: the store's MODELS_RECORD_KEY entry), + * which can only be pharn-oss's own and which `update` never converts. That + * exception is what bounds the gate: a run either converts the block, or + * writes pharn-oss's and records it, so the next run returns early. + */ +export function modelsMigrationPending( + block: unknown, + recorded: string | null, +): boolean { + return ( + needsModelsConversion(block) && + (recorded === null || modelsRecordHash(block) !== recorded) + ); +} + +export type ModelsOutcome = + // Your config had no block; pharn-oss's was written. + | 'restored' + // The block was still pharn's; it became pharn-oss's current one. + | 'updated' + // Already pharn-oss's current block. + | 'ok' + // Yours, kept (converted when it was in the old format). + | 'kept' + // Yours, replaced under `--force` after pharn.config.json was backed up. + | 'forced' + // pharn-oss ships no block, or one this CLI will not apply: yours stays. + | 'upstream-absent' + | 'upstream-invalid'; + +export interface ModelsUpdate { + outcome: ModelsOutcome; + // The per-file skip label behind `kept` / `forced` — why the block is the + // user's: `modified`, `unrecorded` or `unverifiable`. + label: UpdateLabel | null; + // `updated` / `forced` over a default an earlier pharn wrote. + replacedLegacyDefault: boolean; + // The block to write back; `undefined` means no `models` key. + next: unknown; + // The record to store under MODELS_RECORD_KEY; `null` means none. A block + // pharn did not just write carries its previous record, never a new one, so + // the next run still reads it as the user's. + nextRecord: string | null; + // Back up pharn.config.json before the config is written. + backup: boolean; + // Set when a kept block was converted from the old format. + conversion: ModelsConversion | null; + // Why the kept block still fails pharn-oss's rules — a `models.default` that + // could not move, then the checker's REDs. Untrusted text: `terminalSafe`. + problems: string[]; + // Why pharn-oss's block was not applied (`upstream-invalid`). + upstreamReasons: string[]; +} + +export function decideModelsUpdate(input: { + // config.models; `undefined` when the key is absent. + current: unknown; + upstream: UpstreamModels; + // The store's MODELS_RECORD_KEY entry, or null. + recorded: string | null; + // False when pharn.records.json is absent, invalid or stamped for another + // install state — `recordsBaseline`'s null. + recordsAvailable: boolean; + force: boolean; +}): ModelsUpdate { + const { current, upstream, recorded, recordsAvailable, force } = input; + const none = { + label: null, + replacedLegacyDefault: false, + backup: false, + conversion: null, + problems: [], + upstreamReasons: [], + }; + + // Nothing to compare with: the user's block stays, and the old format is + // still converted — it is rejected whatever pharn-oss ships. A block that was + // pharn's (an old default, or the block it recorded) is still pharn's once + // converted, so its record follows it: otherwise the conversion would erase + // the only evidence, and the next run — with pharn-oss's block usable again + // — would keep it as the user's forever. + if (upstream.kind !== 'ok') { + const kept = keep(current); + const pharns = + current !== undefined && + (isLegacyDefault(current) || + (recorded !== null && modelsRecordHash(current) === recorded)); + return { + ...none, + ...kept, + outcome: + upstream.kind === 'absent' ? 'upstream-absent' : 'upstream-invalid', + nextRecord: pharns ? modelsRecordHash(kept.next) : recorded, + upstreamReasons: upstream.kind === 'invalid' ? upstream.reasons : [], + }; + } + + const latestHash = modelsRecordHash(upstream.block); + const diskHash = current === undefined ? null : modelsRecordHash(current); + // A default an earlier pharn wrote is proof of authorship on its own, so it + // is fed to the table as the record — with or without a records file. + const legacyDefault = isLegacyDefault(current); + const decision = decideFileAction({ + diskHash, + latestHash, + recordedHash: legacyDefault ? diskHash : recorded, + recordsAvailable: legacyDefault || recordsAvailable, + force, + }); + + if (decision.action === 'write') { + return { + ...none, + outcome: decision.forced + ? 'forced' + : decision.label === 'restored' + ? 'restored' + : 'updated', + label: decision.forced ? decision.label : null, + replacedLegacyDefault: legacyDefault, + next: upstream.block, + nextRecord: latestHash, + backup: decision.backup, + }; + } + if (decision.action === 'noop') { + return { ...none, outcome: 'ok', next: current, nextRecord: latestHash }; + } + return { + ...none, + ...keep(current), + outcome: 'kept', + label: decision.label, + nextRecord: recorded, + }; +} + +// The user's block as it stays: converted when it is in the old format, and +// with whatever pharn-oss's rules still reject. +function keep( + current: unknown, +): Pick { + if (current === undefined) { + return { next: undefined, conversion: null, problems: [] }; + } + const conversion = convertLegacyModels(current); + const check = checkModelsBlock(conversion.block); + return { + next: conversion.block, + conversion: conversion.changes.length > 0 ? conversion : null, + problems: [ + ...conversion.leftovers, + ...(check.kind === 'invalid' ? check.reds.map((red) => red.detail) : []), + ], + }; +} diff --git a/src/lib/pharn-config.ts b/src/lib/pharn-config.ts index e781178d..c872035f 100644 --- a/src/lib/pharn-config.ts +++ b/src/lib/pharn-config.ts @@ -11,7 +11,6 @@ import { VERSION_RE, } from './validate.js'; import { ProjectChangedError } from './project-lock.js'; -import { validateModelRouting, ModelRoutingError } from './model-routing.js'; import { validateSeamConfig, SeamConfigError } from './seam-config.js'; import type { PharnConfig } from '../types.js'; @@ -24,7 +23,7 @@ const CAPABILITY_SOURCES = ['auto', 'manual']; /** * A `capabilities[].source` that is present but not in the allowlist — the FIRST * capabilities-entry check this config has ever had. Named + loud, following the - * `ModelRoutingError`/`SeamConfigError` pattern, so a hand-edit is reported as + * `SeamConfigError` pattern, so a hand-edit is reported as * the hand-edit it is and never collapsed into the "run `pharn init`" lie. * * It validates `source` ONLY; an entry's `name` and `role` are the separate @@ -202,8 +201,11 @@ export function configPath(cwd: string): string { /** * Every top-level key this CLI OWNS: exactly the keys `PharnConfig` declares — - * the ones it writes and validates, plus the module-era fields it no longer - * writes but that are still its own (`constitution`, `stackAnswers`, …). + * the ones it writes, plus the module-era fields it no longer writes but that + * are still its own (`constitution`, `stackAnswers`, …). One of them, `models`, + * is pharn's to WRITE but pharn-oss's to define: `init` copies pharn-oss's + * block into it and `update` manages it (lib/models-update.ts), so a re-run + * init replaces it rather than carrying a stale one across. * * `satisfies Record` makes the list exhaustive BY * CONSTRUCTION: declaring a field on `PharnConfig` without listing it here (or @@ -282,8 +284,13 @@ export function userOwnedConfigEntries( * something this CLI will not act on, and each throws its own NAMED error that * PROPAGATES rather than collapsing into the "run init" lie: `ConfigParseError` * (not JSON at all), and — validated OUTSIDE the null-returning try — - * `ModelRoutingError`/`SeamConfigError`/`CapabilityEntryError`/ - * `CapabilitySourceError` (BUG 1). + * `SeamConfigError`/`CapabilityEntryError`/`CapabilitySourceError` (BUG 1). + * + * `models` is NOT validated here. Its schema is pharn-oss's + * (lib/model-config.ts), and this CLI must load the format it wrote before + * 0.7.0 (so `update` can migrate it), pharn-oss's format, and whatever + * pharn-oss's next format is — so the block passes through VERBATIM and no + * command refuses to run over it. `status` reports it; `update` manages it. * * The parse split is the point. "File absent" and "file corrupt" used to be the * same `null`, so a stray comma was reported as a MISSING file and answered with @@ -291,8 +298,8 @@ export function userOwnedConfigEntries( * construction — the branch is `JSON.parse` throwing, not a second `existsSync` * guess at a call site. * - * On success the validated, typed `models`/`seam` (the validators' stripped - * return) replace the raw sub-blocks (BUG 3), while unknown TOP-LEVEL keys still + * On success the validated, typed `seam` (the validator's stripped return) + * replaces the raw sub-block (BUG 3), while unknown TOP-LEVEL keys still * pass through so a legacy config carrying a since-removed field still loads * (P7, additive) — and so a key the user owns (`userOwnedConfigEntries`) survives * every command that writes this object back. @@ -328,11 +335,10 @@ export function readPharnConfig(cwd: string): PharnConfig | null { if (typeof raw.skillsVersion !== 'string' || !Array.isArray(raw.modules)) { return null; } - // Present-but-invalid → the validators THROW (named) and the error propagates. - // Absent models/seam is legacy/valid (P7, additive). Use the validators' typed, - // stripped return for the sub-blocks (BUG 3). - const models = - raw.models !== undefined ? validateModelRouting(raw.models) : undefined; + // Present-but-invalid → the validator THROWS (named) and the error propagates. + // An absent seam is legacy/valid (P7, additive). Use the validator's typed, + // stripped return for the sub-block (BUG 3). `models` is not validated: it + // rides the spread below untouched (see the doc comment). const seam = raw.seam !== undefined ? validateSeamConfig(raw.seam) : undefined; // Same discipline for `capabilities[].source`: a present-but-invalid value @@ -343,7 +349,6 @@ export function readPharnConfig(cwd: string): PharnConfig | null { validateCapabilitySources(raw.capabilities); const config: PharnConfig = { ...(raw as unknown as PharnConfig), - ...(models !== undefined ? { models } : {}), ...(seam !== undefined ? { seam } : {}), }; // Additive `layout` (lib/layout.ts): coerce to the {pharn, flat} enum. A legacy @@ -497,9 +502,10 @@ export function assertConfigFingerprintUnchanged( /** * Is `err` a present-but-invalid-config error — a file that IS there and that - * the user has to fix (unparseable JSON, a hand-edited `models`/`seam` block, or - * a `capabilities[]` entry with an invalid `name`/`role`/`source`) — as opposed - * to a programming bug? + * the user has to fix (unparseable JSON, a hand-edited `seam` block, or a + * `capabilities[]` entry with an invalid `name`/`role`/`source`) — as opposed + * to a programming bug? (A `models` block is never one: it is pharn-oss's, and + * `readPharnConfig` carries it unvalidated.) * * The single definition of "config error" — used by `loadConfigOrExit` and by * `list`'s own `--json`-aware error path, so neither re-encodes the class @@ -511,13 +517,11 @@ export function isConfigValidationError( err: unknown, ): err is | ConfigParseError - | ModelRoutingError | SeamConfigError | CapabilityEntryError | CapabilitySourceError { return ( err instanceof ConfigParseError || - err instanceof ModelRoutingError || err instanceof SeamConfigError || err instanceof CapabilityEntryError || err instanceof CapabilitySourceError diff --git a/src/lib/proxy-env.ts b/src/lib/proxy-env.ts index 6f1d7f7e..00214041 100644 --- a/src/lib/proxy-env.ts +++ b/src/lib/proxy-env.ts @@ -2,7 +2,7 @@ * What a proxy set in the environment means for THIS run — the logic half * (detection). Message strings live in ./proxy-env-format.ts, which changes for * wording reasons while this file changes only when the transport does (P3, - * mirroring model-routing.ts / model-routing-format.ts). + * mirroring model-config.ts / model-config-format.ts). * * pharn fetches everything — the SHA resolve, the repo tarball, * `SKILLS_VERSION` — through Node's global `fetch`. By DEFAULT that fetch reads diff --git a/src/lib/upstream-models.ts b/src/lib/upstream-models.ts new file mode 100644 index 00000000..e921b2ba --- /dev/null +++ b/src/lib/upstream-models.ts @@ -0,0 +1,92 @@ +import { readBoundedFile } from './bounded-read.js'; +import { checkModelsBlock } from './model-config.js'; +import { isPlainObject, safeJoin } from './validate.js'; + +// --------------------------------------------------------------------------- +// pharn-oss's own `models` block — the one `pharn init` copies and `pharn +// update` moves an unedited block to. It lives in pharn-oss's ROOT +// pharn.config.json, which is inside the tarball every command already +// downloads. That file shares its name with the config pharn writes into a +// project and is a different file: pharn-oss's is read here, never copied. +// +// The block is NETWORK-DERIVED, so it is checked at this boundary, before it +// can reach a config write (THREAT-MODEL.md §3.1): against pharn-oss's rules, +// through this CLI's pinned copy of them (lib/model-config.ts). The read is +// bounded and non-blocking (lib/bounded-read.ts), and the clone holds no +// symlinks (lib/tar-extract.ts refuses them). Nothing here throws: every +// failure is a named `invalid`, and the caller applies nothing. +// +// One axis (P3): obtaining pharn-oss's models block from a fetched clone. +// --------------------------------------------------------------------------- + +// pharn-oss's root config. The same name as the project config, not the same +// file (see above). +const UPSTREAM_CONFIG_FILE = 'pharn.config.json'; + +// Upstream's file is about 2 KB. A cap far above that, and far below the +// 16 MiB pharn allows its own project files, bounds what a poisoned clone can +// make this read parse. +export const MAX_UPSTREAM_CONFIG_BYTES = 1024 * 1024; + +export type UpstreamModels = + // No root pharn.config.json, or one without a `models` block (absent or + // `null`, which pharn-oss's checker reads as nothing declared): there is + // nothing to copy, and pharn writes none rather than invent one. + | { kind: 'absent' } + // A block pharn-oss's rules accept — copied VERBATIM, other keys included. + | { kind: 'ok'; block: unknown } + // A block (or a file) this CLI will not apply, with why. The reasons quote + // upstream bytes: `terminalSafe` before printing. + | { kind: 'invalid'; reasons: string[] }; + +export function readUpstreamModels(repoDir: string): UpstreamModels { + const read = readBoundedFile( + safeJoin(repoDir, UPSTREAM_CONFIG_FILE), + MAX_UPSTREAM_CONFIG_BYTES, + ); + if (read.kind === 'absent') return { kind: 'absent' }; + if (read.kind === 'unusable') { + return { + kind: 'invalid', + reasons: [`pharn-oss's ${UPSTREAM_CONFIG_FILE} ${read.reason}`], + }; + } + let config: unknown; + try { + config = JSON.parse(read.bytes.toString('utf8')); + } catch { + // V8's message is not echoed: one of its shapes quotes raw file bytes + // (lib/pharn-config.ts, parseLocation). + return { + kind: 'invalid', + reasons: [`pharn-oss's ${UPSTREAM_CONFIG_FILE} is not valid JSON`], + }; + } + if (!isPlainObject(config)) { + return { + kind: 'invalid', + reasons: [ + `pharn-oss's ${UPSTREAM_CONFIG_FILE} is valid JSON but is not an object`, + ], + }; + } + const block = config.models; + if (block === undefined || block === null) return { kind: 'absent' }; + const check = checkModelsBlock(block); + if (check.kind === 'invalid') { + return { kind: 'invalid', reasons: check.reds.map((red) => red.detail) }; + } + // The checker reads only `stages`' entries, so a block can pass while a key + // it ignores nests deeper than JSON.stringify can go — and pharn serializes + // the block to record it and to write it. Refused here, before any copy, + // rather than thrown half-way through an install. + try { + JSON.stringify(block); + } catch { + return { + kind: 'invalid', + reasons: ["pharn-oss's models block is nested too deeply to copy"], + }; + } + return { kind: 'ok', block }; +} diff --git a/src/steps/install-archetype.ts b/src/steps/install-archetype.ts index 4b2f7e97..e8bf54f5 100644 --- a/src/steps/install-archetype.ts +++ b/src/steps/install-archetype.ts @@ -12,14 +12,21 @@ import { createBackup } from '../lib/backup.js'; import { readBoundedFile } from '../lib/bounded-read.js'; import { buildRecords, + MODELS_RECORD_KEY, + modelsRecordHash, readRecords, recordsBaseline, recordsUnderCapabilities, writeRecords, type FileRecords, } from '../lib/install-records.js'; -import { DEFAULT_MODEL_ROUTING } from '../lib/model-routing.js'; -import { formatModelRoutingLines } from '../lib/model-routing-format.js'; +import { checkModelsBlock } from '../lib/model-config.js'; +import { + modelsLabelLines, + resolvedStageLines, +} from '../lib/model-config-format.js'; +import { readUpstreamModels } from '../lib/upstream-models.js'; +import { terminalSafe } from '../lib/terminal-safe.js'; import { DEFAULT_SEAM_CONFIG } from '../lib/seam-config.js'; import { configPath, @@ -194,6 +201,28 @@ export async function runInstallArchetype( ); } + // The `models` block is pharn-oss's: copied VERBATIM from its root + // pharn.config.json, checked on the way in (lib/upstream-models.ts). None + // upstream → none written, never an invented one. One this CLI will not + // apply → none written, and each reason named: pharn-oss may have widened its + // rules past this CLI's copy of them (LIMITS.md §3e), which an upgrade fixes + // — `pharn update` then writes it (the block's `restored` row). + const upstreamModels = readUpstreamModels(repoDir); + if (upstreamModels.kind === 'invalid') { + log.warn( + [ + "pharn-oss's models block was not written; this pharn rejects it:", + ...upstreamModels.reasons.map( + (reason) => ` ${terminalSafe(reason, { max: 300 })}`, + ), + 'If pharn-oss changed its models format, upgrade pharn', + '(npm i -g @pharn-dev/pharn@latest) and run `pharn update`.', + ].join('\n'), + ); + } + const models = + upstreamModels.kind === 'ok' ? upstreamModels.block : undefined; + // Which trusted docs the fetched repo did NOT ship at their expected path. A // set difference over two CLI-owned string arrays — the expected list from the // layout resolver, the written list from the copy routine itself, so a doc can @@ -225,8 +254,8 @@ export async function runInstallArchetype( // CONSTITUTION.md verbatim. No modules: capabilities are the install unit. modules: [], installedAt: new Date().toISOString(), - // Per-stage model routing, written on every fresh install (P7 — additive). - models: DEFAULT_MODEL_ROUTING, + // pharn-oss's `models` block, or no key at all (`undefined` is not written). + models, // Seam-resolution policy, written on every fresh install (P7 — additive). seam: DEFAULT_SEAM_CONFIG, archetypes, @@ -268,6 +297,10 @@ export async function runInstallArchetype( files: { ...keptRecords(baseline, carry, layout), ...buildRecords(cwd, prepared.manifest.keys()), + // The block pharn just wrote, so `pharn update` can tell it from an edit. + ...(models !== undefined + ? { [MODELS_RECORD_KEY]: modelsRecordHash(models) } + : {}), }, }); // The config this install replaces may hold keys pharn does not own — @@ -288,18 +321,24 @@ export async function runInstallArchetype( const check = pc.green('✔'); const grillers = capabilities.filter((c) => c.role === 'griller').length; const lenses = capabilities.filter((c) => c.role === 'lens').length; - // Render the per-stage routing from the config just written (not a second - // hardcoded copy), so the recorded intent is legible right after install. - // config.models is set on every fresh install; the guard narrows its optional - // type (P7 legacy). The hint below says plainly that no installed STAGE - // consumes the block yet: it is written, validated and displayed — this line - // and status's MODELS note are two of its readers — but nothing reads it to - // PICK a model (docs/roadmap.md carries the Planned row). Claiming an edit - // here changes a stage's model would document unimplemented behavior - // (CLAUDE.md). - const modelLines = config.models - ? formatModelRoutingLines(config.models) - : []; + // The block just written, resolved per stage, under the label that says what + // it is: the source of truth each /pharn-* command's frontmatter is held to. + // Claude Code applies that frontmatter; nothing reads this block to pick a + // model, and the outro must not read as routing that happens. No block (or + // one that declares no stages) → no section. + const modelsCheck = checkModelsBlock(models); + const modelsSection = + modelsCheck.kind === 'valid' + ? [ + '', + pc.bold('Declared model per stage') + + ` ${pc.dim('(pharn.config.json → models, from pharn-oss)')}`, + ...resolvedStageLines(modelsCheck.stages).map((line) => ` ${line}`), + ...modelsLabelLines(layoutPaths(layout).floor).map( + (line) => ` ${pc.dim(line)}`, + ), + ] + : []; // Report the docs that LANDED, by name — never a count and never the expected // list. A count would hide exactly the silence this line exists to end, and // the names are what let a user see at a glance that (say) LIMITS.md is not @@ -314,10 +353,7 @@ export async function runInstallArchetype( docsLine, `${check} pharn.config.json written ${pc.dim(`(skills v${skillsVersion}, archetypes: ${archetypes.join(', ')})`)}`, `${pc.dim(`Done in ${elapsed}s`)}`, - '', - pc.bold('Models per stage'), - ...modelLines.map((line) => ` ${line}`), - ` ${pc.dim('Recorded in pharn.config.json → models.stages — no installed stage reads it yet')}`, + ...modelsSection, '', pc.bold('Next steps'), ` ${pc.cyan('1.')} ${pc.bold('claude')} ${pc.dim('open Claude Code')}`, @@ -382,8 +418,8 @@ function keptRecords( * * Deliberately NOT readPharnConfig, whose verdict is about the keys pharn OWNS: * it returns null for a config with no `modules` array — which every other - * command answers with "Run `pharn init` first" — and throws on a bad - * `models`/`seam` hand-edit. Neither says anything about the user's own keys, + * command answers with "Run `pharn init` first" — and throws on a bad `seam` + * hand-edit. Neither says anything about the user's own keys, * and reading through it would drop `testResults` on exactly the recovery path * pharn prescribes. So the one requirement is a JSON object at top level — the * same shape-only read steps/overwrite-check.ts makes for its one display diff --git a/src/steps/overwrite-check.ts b/src/steps/overwrite-check.ts index 655bedbd..7ca36881 100644 --- a/src/steps/overwrite-check.ts +++ b/src/steps/overwrite-check.ts @@ -45,7 +45,7 @@ export const MAX_LISTED = 10; // there is not one worth showing. Cosmetic input, cosmetic failure. // // DELIBERATELY NOT readPharnConfig (lib/pharn-config.ts). That reader lets -// ModelRoutingError / SeamConfigError / CapabilitySourceError PROPAGATE so a bad +// SeamConfigError / CapabilityEntryError / CapabilitySourceError PROPAGATE so a bad // hand-edit is never collapsed into the "run init" lie — correct for every // command that must not act on a config it failed to understand. But `init` IS // the command you run to REPAIR a broken config, and it has no recovery around diff --git a/src/types.ts b/src/types.ts index 79bfe7a8..9060dd06 100644 --- a/src/types.ts +++ b/src/types.ts @@ -46,41 +46,15 @@ export interface InstalledCapability { } // --------------------------------------------------------------------------- -// Model routing — the `models` block in pharn.config.json. The CLI owns this -// schema. Per-stage {model, effort} with a `default` fallback; a stage without -// an entry (incl. an empty `stages`) resolves to `default` (see -// src/lib/model-routing.ts, resolveStageModel). Realized via generated subagent -// frontmatter in a later increment — this is the config shape + validator only. +// The `models` block in pharn.config.json is pharn-oss's, not this CLI's: its +// schema and its defaults are owned upstream (pharn/floor/check-model-config.mjs, +// and the block in pharn-oss's root pharn.config.json). This CLI copies that +// block at `init`, carries it across `update` (src/lib/models-update.ts), shows +// it in `status`, and checks it with a pinned copy of pharn-oss's rules +// (src/lib/model-config.ts). So it has no type here: `PharnConfig.models` is +// `unknown` — whatever pharn-oss's format holds, and whatever a user wrote. // --------------------------------------------------------------------------- -// Effort level (brief: {low, high, max} — no "medium"). Runtime allowlist: -// EFFORT_LEVELS in src/lib/model-routing.ts. -export type EffortLevel = 'low' | 'high' | 'max'; - -// Valid model-id strings (short forms of the current models). Runtime allowlist: -// MODEL_IDS in src/lib/model-routing.ts. -export type ModelId = 'opus-4-8' | 'sonnet-5' | 'fable-5' | 'haiku-4-5'; - -// Known pipeline stage keys that may carry a model override — the dev-loop stage -// commands (pharn-dev-*), realized as subagents. NOT the ARCHITECTURE §6 spine -// (which omits `review` and includes `spec`). Runtime allowlist: PIPELINE_STAGES -// in src/lib/model-routing.ts. -export type PipelineStage = - 'plan' | 'grill' | 'build' | 'regress' | 'verify' | 'review' | 'ship'; - -// One routing target: which model runs a stage, at what effort. -export interface StageModel { - model: ModelId; - effort: EffortLevel; -} - -// The `models` block: a required `default` (the fallback for any stage without -// an explicit entry, incl. an empty `stages`) plus per-stage overrides. -export interface ModelRouting { - default: StageModel; - stages: Partial>; -} - // --------------------------------------------------------------------------- // Seam-resolution config — the `seam` block in pharn.config.json. The CLI owns // this schema; it conforms to pharn-contracts/seam-config.md (the SoT) and the @@ -99,7 +73,7 @@ export type ResolutionStep = 'official-skill' | 'pinned-docs' | 'fetch' | 'model' | 'ask'; // The confidence bar at the `model` step ({low, medium, high} — the seam-config -// contract's scale, NOT model-routing's effort enum). Runtime allowlist: +// contract's scale, NOT the models block's effort levels). Runtime allowlist: // SEAM_CONFIDENCE_LEVELS in src/lib/seam-config.ts. export type SeamConfidence = 'low' | 'medium' | 'high'; @@ -151,10 +125,12 @@ export interface PharnConfig { isMultiTenant?: boolean; modules: InstalledModule[]; installedAt: string; - // Per-stage model routing (the `models` block). Written on every fresh install - // with DEFAULT_MODEL_ROUTING; absent on legacy installs predating it (P7 — - // additive). Validated by validateModelRouting (src/lib/model-routing.ts). - models?: ModelRouting; + // The `models` block — pharn-oss's schema (see above), carried VERBATIM: never + // validated at load, so an old-format block `update` must migrate, pharn-oss's + // format, and a newer one all load. `init` writes pharn-oss's block (none when + // upstream has none); absent on installs that predate it (P7 — additive). The + // key stays pharn's to WRITE (CLI_OWNED_KEYS), so a re-run init replaces it. + models?: unknown; // Seam-resolution policy (the `seam` block). Written on every fresh install // with DEFAULT_SEAM_CONFIG; absent on legacy installs predating it (P7 — // additive). Validated by validateSeamConfig (src/lib/seam-config.ts). diff --git a/tests/fixtures/pharn-oss/check-model-config.mjs b/tests/fixtures/pharn-oss/check-model-config.mjs new file mode 100644 index 00000000..51ff755d --- /dev/null +++ b/tests/fixtures/pharn-oss/check-model-config.mjs @@ -0,0 +1,424 @@ +#!/usr/bin/env node +// pharn/floor/check-model-config.mjs — the deterministic PRODUCT-surface model/effort configuration +// checker: `pharn.config.json`'s `models.stages` block VALIDATED, RESOLVED per stage, and held in +// EQUALITY with the eleven `/pharn-*` product commands' platform `model:` / `effort:` frontmatter. +// +// ── THE MECHANISM, read live rather than assumed (P6) ──────────────────────────────────────────────── +// Claude Code selects a command's model through STATIC FRONTMATTER and nothing else. `model:` accepts +// the `/model` values (`sonnet` | `opus` | `haiku` | `fable` | a full `claude-*` id | `inherit`) and +// `effort:` accepts `low` | `medium` | `high` | `xhigh` | `max`. There is NO runtime routing hook: no +// command can read a JSON file and switch its own model, and nothing reads the `models.stages` BLOCK +// at run time — the FILE is read elsewhere (LIMITS.md §8). So `models.stages` cannot BE the runtime control — it can only be the +// SOURCE OF TRUTH the static frontmatter is held to, which is what this checker enforces. Simulating +// runtime routing (a command "consulting" the config in prose) would be the exact P0 disease: written in +// the config, therefore believed guaranteed. +// +// ── Floor primitives (ARCHITECTURE §2) ─────────────────────────────────────────────────────────────── +// #3 (enum / regex / presence) throughout: every `model` is bounded to the Claude model namespace (the +// closed alias set ∪ `inherit` ∪ an OPEN `claude-*` id regex — a NAMESPACE bound, NOT a closed +// allowlist: a non-real `claude-*` id validates), every `effort` ∈ the five-level enum, a `default` entry +// is present, resolution is the OWN-PROPERTY membership pick, and agreement is a deterministic EQUALITY +// between two repo files (the same drift-detection class as a content-hash, #2). +// NON-LLM, dependency-free (Node stdlib only). No network, no child_process, no eval, no dynamic import. +// +// ── Honest scope (P0) — what GREEN does and does NOT buy ───────────────────────────────────────────── +// FLOOR: the config is shape/enum-valid; a stage resolves deterministically; and each of the eleven product +// commands' static `model:` / `effort:` frontmatter EQUALS its config-resolved value, in both +// directions (no product command carries model/effort outside the map, no mapped command is missing). +// NOT guaranteed — and these are real holes, not formalities: +// • THE STAGE IS NOT PROVEN TO HAVE RUN UNDER THAT MODEL. Model and effort are applied by the Claude +// Code platform, invisible to any hook, hash or enum. "check-model-config GREEN" must NEVER read as +// "/pharn-plan ran on opus". That conflation is what this repo exists to prevent. +// • TURN SCOPE. The platform states the override "applies for the rest of the current turn". So it +// takes effect when a human invokes the stage command DIRECTLY (`/pharn-plan`). A stage invoked as a +// STEP INSIDE `/pharn-ship` or `/pharn-loop` runs inside the ORCHESTRATOR's turn — those stages do +// NOT get per-stage routing, and this checker cannot see the difference. +// • PLATFORM VETO. A value excluded by an organization's `availableModels` allowlist is not used, and +// in auto mode a model auto mode does not support is not used; the session silently keeps its +// current model. A GREEN here says nothing about either. +// • FRESH-INSTALL POSTURE. A target with no `pharn.config.json`, or a config with no `models.stages`, +// is GREEN BY DESIGN (the `check-lessons-index` NO_CANON / COLD precedent: the honest normal state of +// an install that does not use the block). The consequence is stated rather than hidden — a user who +// DELETES the block loses this check rather than failing it. +// • `model_tier:` IS A DIFFERENT FIELD and is deliberately untouched. It is PHARN's own capability +// frontmatter (ARCHITECTURE §3.1), inert to the platform. The frontmatter parser below matches the +// key EXACTLY, so `model_tier:` can never be read as `model:`; and nothing here requires the two to +// agree, because they answer different questions. +// +// ── Relationship to `.dev/floor/check-config.mjs` (L31) ────────────────────────────────────────────── +// That is the DEV-apparatus checker over the `pharn-dev-*` commands. This is NOT its copy-pair twin — +// unlike `check-provenance.mjs` / `lessons-index-core.mjs`, whose two copies are deliberate duplicates +// pinned to agree. The two files share an idea and almost no substance: a different stage→command map, a +// different filename prefix, a closed product enumeration where the dev side has a closed WIRED subset, +// and a different fresh-install posture (this one is GREEN with no config; that one is RED). The +// DISTINCT BASENAME is the signal — a reader must not go looking for a shared-constant obligation set +// that does not exist here. +// +// ── Trust (P2) ─────────────────────────────────────────────────────────────────────────────────────── +// `pharn.config.json` and command frontmatter are repo-local, human-authored DATA — parsed as JSON / +// YAML frontmatter, NEVER executed. The verdict ranges ONLY over enum-gated fields (model ∈ the bounded +// namespace, effort ∈ enum, resolved == frontmatter) and rejects any non-member; a poisoned config can at +// most select a DIFFERENT namespace-valid model/effort (a bounded, advisory blast radius), never inject +// an instruction. No guaranteed decision rests on any free-text field (mirrors fix #1). +// +// Usage: +// node pharn/floor/check-model-config.mjs [validate] [--config ] +// validate config shape + enums, and that every stage key is a member of the product stage set +// node pharn/floor/check-model-config.mjs resolve [--config ] +// print {"model":..,"effort":..} for , via the own-property pick with a `default` fallback +// node pharn/floor/check-model-config.mjs agreement [--config ] [--commands-dir ] +// validate + BIDIRECTIONAL config↔frontmatter agreement over the eleven product commands +// +// Exit: 1 on any RED / unreadable / malformed; 0 otherwise (including the two GREEN-by-design +// no-configuration states). + +import { readFileSync, readdirSync } from "node:fs"; +import { join } from "node:path"; + +// ── The enumeration IS the deliverable (L29 / L36) ─────────────────────────────────────────────────── +// The closed stage → product-command map. Every pass below iterates THIS object; no pass re-derives the +// set by pattern, and no pass asserts over whichever member its author happened to have in front of them. +// `default` is deliberately ABSENT: it is the resolution fallback, never a command. +const PRODUCT_STAGES = { + spec: "pharn-spec.md", + plan: "pharn-plan.md", + grill: "pharn-grill.md", + build: "pharn-build.md", + regress: "pharn-regress.md", + verify: "pharn-verify.md", + ship: "pharn-ship.md", + loop: "pharn-loop.md", + review: "pharn-review.md", + "memory-promote": "pharn-memory-promote.md", + // The one stage whose KEY is not its file stem (6.17.0): `test` would read as the `test` GATE id in the same + // pharn.config.json (`testResults.test`), so the stage is keyed `ac-test`. The map, not the file name, is the + // membership: the reverse pass below tests MAPPED_FILES, never a stem derived from a file name. + "ac-test": "pharn-test.md", +}; + +// The map's VALUES — the product command files the config governs. The reverse pass tests membership HERE. +const MAPPED_FILES = new Set(Object.values(PRODUCT_STAGES)); + +// Enums — every branch is a presence / enum / equality membership test (P5); the terminal fallback on any +// non-member is a loud RED, never a guess. These mirror the Claude Code command-frontmatter surface. +const MODEL_ALIASES = ["sonnet", "opus", "haiku", "fable", "inherit"]; +const MODEL_ID_RE = /^claude-[a-z0-9][a-z0-9-]*$/; // a full model id, e.g. claude-opus-4-8 +const EFFORT_ENUM = ["low", "medium", "high", "xhigh", "max"]; +const DEFAULT_CONFIG = "pharn.config.json"; +const DEFAULT_COMMANDS_DIR = join(".claude", "commands"); + +// A product command file is `pharn-.md` that is NOT `pharn-dev-*` — the product/dev boundary +// is the NAME PREFIX (CLAUDE.md, "Command-naming convention"), because `.claude/commands/` cannot nest. +const PRODUCT_CMD_RE = /^pharn-(?!dev-)(.+)\.md$/; + +const reds = []; +function red(kind, detail) { + reds.push({ kind, detail }); +} + +function isValidModel(m) { + return typeof m === "string" && (MODEL_ALIASES.includes(m) || MODEL_ID_RE.test(m)); +} +function isValidEffort(e) { + return typeof e === "string" && EFFORT_ENUM.includes(e); +} + +function fail() { + for (const r of reds) console.log(`RED — ${r.kind} failed: ${r.detail}`); + console.log(`\nRED — ${reds.length} model-config check(s) failed`); + return 1; +} + +// Read + JSON-parse the config. Returns {state, cfg}: `no-config` when the file is absent (a distinct, +// GREEN-by-design state, NOT an error), `error` on any other failure (fail-closed), else `ok`. +function readConfig(path) { + let text; + try { + text = readFileSync(path, "utf8"); + } catch (e) { + if (e && e.code === "ENOENT") return { state: "no-config" }; + red("input", `config unreadable (${path}): ${e.message}`); + return { state: "error" }; + } + let cfg; + try { + cfg = JSON.parse(text); + } catch (e) { + red("json", `config is not valid JSON (${path}): ${e.message}`); + return { state: "error" }; + } + // A parseable non-OBJECT (`null`, `[]`, `"hello"`, `123`) must NOT reach `stagesOf` — there, a missing + // `.models` is the GREEN-by-design "no block" state, so a scalar config would read as "the block is + // simply absent" and pass. A file that exists but is not a config object is a loud RED (fail-closed). + if (cfg === null || typeof cfg !== "object" || Array.isArray(cfg)) { + red( + "shape", + `config is valid JSON but is not an object (${path}): got ${Array.isArray(cfg) ? "array" : cfg === null ? "null" : typeof cfg}` + ); + return { state: "error" }; + } + return { state: "ok", cfg }; +} + +// The `models.stages` map. A config with NO `models` / no `stages` is `no-stages` (GREEN by design — the +// block is optional); a `stages` that is present but is not a plain object is a loud RED (fail-closed). +// `cfg` is guaranteed a plain object here — readConfig REDs on anything else. +function stagesOf(cfg) { + const models = cfg.models; + if (models === undefined || models === null) return { state: "no-stages" }; + if (typeof models !== "object" || Array.isArray(models)) { + red("shape", "`models` is present but is not an object"); + return { state: "error" }; + } + const stages = models.stages; + if (stages === undefined || stages === null) return { state: "no-stages" }; + if (typeof stages !== "object" || Array.isArray(stages)) { + red("shape", "`models.stages` is present but is not an object"); + return { state: "error" }; + } + return { state: "ok", stages }; +} + +// Validate every stage entry's shape + enums, and that every key is a PRODUCT stage (or `default`). +// An off-map key is a RED rather than a silent skip: on the product surface it governs nothing, so a +// typo (`bulid`) would otherwise sit in the config looking like a control while controlling nothing. +function validateStages(stages) { + if (!Object.hasOwn(stages, "default")) { + red("default", "missing required `default` stage entry (the resolution fallback)"); + } + for (const [name, entry] of Object.entries(stages)) { + if (name !== "default" && !Object.hasOwn(PRODUCT_STAGES, name)) { + red( + "stage", + `stage ${JSON.stringify(name)} is not a product stage — expected one of {${Object.keys(PRODUCT_STAGES).join(", ")}} or "default"` + ); + continue; + } + if (!entry || typeof entry !== "object" || Array.isArray(entry)) { + red("entry", `stage ${JSON.stringify(name)} is not an object with {model, effort}`); + continue; + } + if (!Object.hasOwn(entry, "model")) red("model", `stage ${JSON.stringify(name)} missing \`model\``); + else if (!isValidModel(entry.model)) + red( + "model", + `stage ${JSON.stringify(name)} model ${JSON.stringify(entry.model)} is not an alias {${MODEL_ALIASES.join(", ")}} nor a claude-* id` + ); + if (!Object.hasOwn(entry, "effort")) red("effort", `stage ${JSON.stringify(name)} missing \`effort\``); + else if (!isValidEffort(entry.effort)) + red("effort", `stage ${JSON.stringify(name)} effort ${JSON.stringify(entry.effort)} not in {${EFFORT_ENUM.join(", ")}}`); + } +} + +// resolve: `Object.hasOwn(stages, stage) ? stages[stage] : stages.default` — an OWN-PROPERTY membership +// test (P5, lessons-learned L15). An INHERITED member (toString / constructor / __proto__ / +// hasOwnProperty / valueOf) is truthy but is NOT a configured stage, so `||` / `??` would leak it and +// return {model: undefined, effort: undefined} → a silent `{}` at exit 0, which is the floor-tool-lies- +// quietly failure this repo exists to kill. Returns {model, effort}, or undefined + a RED. +function resolveStage(stages, stage) { + const entry = Object.hasOwn(stages, stage) ? stages[stage] : stages.default; + if (!entry) { + red("resolve", `stage ${JSON.stringify(stage)} has no entry and no \`default\` fallback`); + return undefined; + } + return { model: entry.model, effort: entry.effort }; +} + +// Parse a command file's frontmatter `model:` / `effort:`. Same `---` fence + `^([A-Za-z0-9_]+):\s*(.*)$` +// line parse + quote-strip as `count-grillers.mjs`'s `frontmatterRole` and `.dev/floor/check-config.mjs` +// — a STRUCTURED read of the structured location, never a substring grep (L6). The key match is EXACT, +// which is what keeps `model_tier:` from ever being read as `model:`. +function frontmatterModelEffort(text) { + if (!text.startsWith("---")) return {}; + const end = text.indexOf("\n---", 3); + if (end === -1) return {}; + const raw = text.slice(3, end).trim(); + const out = {}; + for (const line of raw.split("\n")) { + const m = line.match(/^([A-Za-z0-9_]+):\s*(.*)$/); + if (!m) continue; + if (m[1] === "model") out.model = m[2].trim().replace(/^["']|["']$/g, ""); + else if (m[1] === "effort") out.effort = m[2].trim().replace(/^["']|["']$/g, ""); + } + return out; +} + +// --- validate mode: config shape + enums + product-stage membership. --- +function doValidate(configPath) { + const c = readConfig(configPath); + if (c.state === "error") return fail(); + if (c.state === "no-config") { + console.log(`GREEN — no ${configPath}; nothing declares a per-stage model (GREEN by design: an install need not use the block)`); + return 0; + } + const s = stagesOf(c.cfg); + if (s.state === "error") return fail(); + if (s.state === "no-stages") { + console.log(`GREEN — ${configPath} has no \`models.stages\`; nothing declares a per-stage model (GREEN by design)`); + return 0; + } + validateStages(s.stages); + if (reds.length) return fail(); + console.log( + `GREEN — config valid; ${Object.keys(s.stages).length} stage(s); default present; every stage a product stage; every model/effort in enum` + ); + return 0; +} + +// --- resolve mode: print the resolved {model, effort} for a stage. --- +function doResolve(configPath, stage) { + if (!stage) { + console.log("RED — usage: node pharn/floor/check-model-config.mjs resolve [--config ]"); + return 1; + } + const c = readConfig(configPath); + if (c.state === "error") return fail(); + if (c.state === "no-config") { + red("resolve", `no ${configPath} — a stage cannot be resolved without one`); + return fail(); + } + const s = stagesOf(c.cfg); + if (s.state === "error") return fail(); + if (s.state === "no-stages") { + red("resolve", `${configPath} has no \`models.stages\` — a stage cannot be resolved without one`); + return fail(); + } + validateStages(s.stages); + if (reds.length) return fail(); + const r = resolveStage(s.stages, stage); + if (!r) return fail(); + process.stdout.write(JSON.stringify(r) + "\n"); + return 0; +} + +// --- agreement mode: validate + BIDIRECTIONAL config↔product-command-frontmatter consistency. --- +function doAgreement(configPath, commandsDir) { + const c = readConfig(configPath); + if (c.state === "error") return fail(); + if (c.state === "no-config") { + console.log(`GREEN — no ${configPath}; no config to hold the product commands to (GREEN by design)`); + return 0; + } + const s = stagesOf(c.cfg); + if (s.state === "error") return fail(); + if (s.state === "no-stages") { + console.log(`GREEN — ${configPath} has no \`models.stages\`; no config to hold the product commands to (GREEN by design)`); + return 0; + } + validateStages(s.stages); + if (reds.length) return fail(); + const stages = s.stages; + + // Forward pass (config → command), over the CLOSED product enumeration — every stage, whether it has + // its own config entry or resolves through `default`. That is what makes the config the source of + // truth for all eleven rather than only for the keys someone remembered to write down. + const checked = []; + for (const [stage, fileName] of Object.entries(PRODUCT_STAGES)) { + const cmdPath = join(commandsDir, fileName); + let text; + try { + text = readFileSync(cmdPath, "utf8"); + } catch (e) { + red("agreement", `stage ${JSON.stringify(stage)} → command file unreadable (${cmdPath}): ${e.message}`); + continue; + } + const fm = frontmatterModelEffort(text); + const want = resolveStage(stages, stage); + if (!want) continue; + const via = Object.hasOwn(stages, stage) ? "own entry" : "default"; + if (fm.model === undefined) red("agreement", `stage ${JSON.stringify(stage)} → ${cmdPath} frontmatter has no \`model:\``); + else if (fm.model !== want.model) + red( + "agreement", + `stage ${JSON.stringify(stage)} → ${cmdPath} model ${JSON.stringify(fm.model)} != config ${JSON.stringify(want.model)} (via ${via})` + ); + if (fm.effort === undefined) red("agreement", `stage ${JSON.stringify(stage)} → ${cmdPath} frontmatter has no \`effort:\``); + else if (fm.effort !== want.effort) + red( + "agreement", + `stage ${JSON.stringify(stage)} → ${cmdPath} effort ${JSON.stringify(fm.effort)} != config ${JSON.stringify(want.effort)} (via ${via})` + ); + checked.push(stage); + } + + // Reverse pass (command → config): a PRODUCT command that CARRIES `model:` / `effort:` but is NOT in + // the map would silently gain a model/effort the config never governs — invisible to the forward pass, + // which only walks the map. This is what CLOSES the enumeration rather than merely asserting presence + // over its members (L36). `pharn-dev-*` is excluded by the name regex: it is the other surface, owned + // by `.dev/floor/check-config.mjs`. Fail-closed: an unreadable commands dir is a loud RED. + let dirEntries; + try { + dirEntries = readdirSync(commandsDir); + } catch (e) { + red("agreement", `commands dir unreadable (${commandsDir}): ${e.message}`); + return fail(); + } + let productSeen = 0; + for (const fileName of dirEntries) { + const m = fileName.match(PRODUCT_CMD_RE); + if (!m) continue; + productSeen++; + if (MAPPED_FILES.has(fileName)) continue; // covered by the forward pass (by FILE, so a key ≠ stem still maps) + let text; + try { + text = readFileSync(join(commandsDir, fileName), "utf8"); + } catch { + continue; // vanished mid-scan; the forward pass owns mapped-command presence + } + const fm = frontmatterModelEffort(text); + if (fm.model !== undefined || fm.effort !== undefined) + red( + "agreement", + `command ${JSON.stringify(fileName)} carries \`model:\`/\`effort:\` frontmatter but is not a mapped product stage (unmapped-command drift)` + ); + } + + // A walk that discovered ZERO product commands has not passed — it has failed to look (L34). Every + // per-item assertion above is vacuously true over an empty set, so the emptiness is checked directly. + if (productSeen === 0) + red("agreement", `no product command (pharn-*.md, excluding pharn-dev-*) found in ${commandsDir} — the walk found nothing to check`); + + if (reds.length) return fail(); + console.log( + `GREEN — config valid; ${checked.length}/${Object.keys(PRODUCT_STAGES).length} product stage(s) agree with command frontmatter; ` + + `${productSeen} product command(s) scanned; none unmapped carries model:/effort: (bidirectional). ` + + `NOTE (P0): this is config↔frontmatter EQUALITY — never proof a stage RAN under that model.` + ); + return 0; +} + +// Read `--name `. A flag given with NO value (trailing, or immediately followed by another flag) +// is `null` — a distinct third state from "absent" (use the default), so main() can refuse it by name +// instead of letting `undefined` reach readFileSync and surface as a confusing TypeError. +function getOpt(args, name, dflt) { + const i = args.indexOf(name); + if (i === -1) return dflt; + const v = args[i + 1]; + return v === undefined || v.startsWith("--") ? null : v; +} + +function main() { + const args = process.argv.slice(2); + const configPath = getOpt(args, "--config", DEFAULT_CONFIG); + const commandsDir = getOpt(args, "--commands-dir", DEFAULT_COMMANDS_DIR); + for (const [flag, val] of [ + ["--config", configPath], + ["--commands-dir", commandsDir], + ]) { + if (val === null) { + console.log(`RED — ${flag} was given with no value`); + return 1; + } + } + const mode = args[0] && !args[0].startsWith("--") ? args[0] : "validate"; + if (mode === "validate") return doValidate(configPath); + if (mode === "resolve") { + const stage = args[1] && !args[1].startsWith("--") ? args[1] : undefined; + return doResolve(configPath, stage); + } + if (mode === "agreement") return doAgreement(configPath, commandsDir); + console.log(`RED — unknown mode ${JSON.stringify(mode)} (expected: validate | resolve | agreement)`); + return 1; +} + +process.exit(main()); diff --git a/tests/init-archetype.test.ts b/tests/init-archetype.test.ts index 5e2d97d3..67472cfd 100644 --- a/tests/init-archetype.test.ts +++ b/tests/init-archetype.test.ts @@ -6,6 +6,7 @@ import { rmSync, writeFileSync, } from 'node:fs'; +import { spawnSync } from 'node:child_process'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { @@ -59,8 +60,7 @@ const { resolveCapabilities } = const { runInstallArchetype } = await import('../src/steps/install-archetype.js'); const { readPharnConfig } = await import('../src/lib/pharn-config.js'); -const { DEFAULT_MODEL_ROUTING } = await import('../src/lib/model-routing.js'); -const { readRecords, writeRecords } = +const { MODELS_RECORD_KEY, modelsRecordHash, readRecords, writeRecords } = await import('../src/lib/install-records.js'); const { collectExpectedInstallPaths } = await import('../src/lib/install-manifest.js'); @@ -85,9 +85,20 @@ function cap(role: string, applies: string): string { return `---\nname: c\nrole: ${role}\napplies: ${applies}\n---\n# c\n`; } +// pharn-oss's own `models` block, as its root pharn.config.json carries it. +const UPSTREAM_MODELS = { + stages: { + default: { model: 'sonnet', effort: 'high' }, + plan: { model: 'opus', effort: 'high' }, + review: { model: 'opus', effort: 'high' }, + }, +}; + // A fake fetched pharn-oss clone: two grillers, two lenses (one backend-only, so -// it is skipped for an ssr project), the product + dev surfaces, and the root -// SKILLS_VERSION the archetype flow reads in place of a manifest. +// it is skipped for an ssr project), the product + dev surfaces, the root +// SKILLS_VERSION the archetype flow reads in place of a manifest, and +// pharn-oss's root pharn.config.json — whose `models` block init copies, and +// whose other keys it must not. function scaffoldRepo(repo: string): void { write( join(repo, 'pharn-pipeline/grillers/a11y/a11y.md'), @@ -114,6 +125,14 @@ function scaffoldRepo(repo: string): void { write(join(repo, '.dev/floor/validate.mjs'), 'floor'); write(join(repo, '.dev/features/x/PLAN.md'), 'DEVPLAN'); write(join(repo, 'SKILLS_VERSION'), '1.0.0\n'); + write( + join(repo, 'pharn.config.json'), + JSON.stringify({ + _models_stages_note: "pharn-oss's own note — not copied", + models: UPSTREAM_MODELS, + ship: { requireAttestation: true }, + }), + ); } describe('archetype install (fixture e2e)', () => { @@ -170,23 +189,29 @@ describe('archetype install (fixture e2e)', () => { expect(config!.commit).toBe('sha123'); expect(config!.modules).toEqual([]); expect(config!.constitution).toBeUndefined(); - // Model routing written on every fresh install (archetype path too). - expect(config!.models).toEqual(DEFAULT_MODEL_ROUTING); - // The spend-safe default: review resolves to opus-4-8/high, not fable-5/max. - expect(config!.models?.stages.review).toEqual({ - model: 'opus-4-8', - effort: 'high', - }); - // The outro still RENDERS the routing it just wrote... + // pharn-oss's `models` block, copied verbatim — and nothing else from its + // root config (its note and its `ship` are pharn-oss's, not the user's). + expect(config!.models).toEqual(UPSTREAM_MODELS); + const raw = JSON.parse( + readFileSync(join(proj, 'pharn.config.json'), 'utf8'), + ) as Record; + expect(raw._models_stages_note).toBeUndefined(); + expect(raw.ship).toBeUndefined(); + // The outro shows the block resolved per stage, under a label that says + // what it is: Claude Code applies each command's own frontmatter, and the + // block is the source of truth that frontmatter is held to. Both + // directions are pinned — the old claims gone AND the honest label present. const outro = outroBody(); - expect(outro).toContain('Models per stage'); - expect(outro).toContain('review'); - // ...but no longer invites an edit that would change which model a stage - // runs: nothing installed reads models.stages yet. Both directions are - // pinned — the old promise gone AND the honest replacement present — so a - // later copy edit cannot quietly re-promise the effect with the test green. + expect(outro).toContain('Declared model per stage'); + expect(outro).toContain('plan opus · high'); + expect(outro).toContain('ship sonnet · high (default)'); + expect(outro).toContain( + "Claude Code applies each /pharn-* command's own model:/effort:", + ); + expect(outro).toContain('node .dev/floor/check-model-config.mjs agreement'); + expect(outro).not.toContain('Models per stage'); expect(outro).not.toContain('Change per-stage routing anytime'); - expect(outro).toContain('no installed stage reads it yet'); + expect(outro).not.toMatch(/rout/i); // Everything a fresh install writes came from archetype resolution, so it is // `auto` — update owns it. Only `pharn add` writes `manual`. expect(config!.capabilities).toEqual([ @@ -220,7 +245,8 @@ describe('archetype install (fixture e2e)', () => { expect(read.kind).toBe('ok'); if (read.kind !== 'ok') return; - // Exactly the install manifest — nothing missing, nothing invented. + // Exactly the install manifest, plus the models block it wrote into the + // config — nothing missing, nothing invented. const expected = collectExpectedInstallPaths({ repoDir: repo, capabilities: selection.selected.map((c) => ({ @@ -230,7 +256,11 @@ describe('archetype install (fixture e2e)', () => { layout: 'flat', }); expect(Object.keys(read.store.files).sort()).toEqual( - [...expected.keys()].sort(), + [...expected.keys(), MODELS_RECORD_KEY].sort(), + ); + // The block's record is the block as written — read back from the config. + expect(read.store.files[MODELS_RECORD_KEY]).toBe( + modelsRecordHash(readPharnConfig(proj)!.models), ); // Each hash describes the bytes that actually LANDED (the dest), which is @@ -246,6 +276,103 @@ describe('archetype install (fixture e2e)', () => { expect(read.store.commit).toBe(config.commit); }); + // pharn-oss owns the `models` block. init copies pharn-oss's — and never + // invents one, never writes one pharn-oss's rules reject. + describe('the models block', () => { + async function installWith(rootConfig: string | null): Promise { + const repo = join(tmp.path(), 'repo'); + const proj = join(tmp.path(), 'proj'); + scaffoldRepo(repo); + if (rootConfig === null) rmSync(join(repo, 'pharn.config.json')); + else write(join(repo, 'pharn.config.json'), rootConfig); + write(join(proj, 'package.json'), '{}'); + const { archetypes } = detectArchetypesFromProject(proj); + const selection = resolveCapabilities( + archetypes, + parseCapabilityIndex(repo), + ); + await runInstallArchetype(repo, proj, archetypes, selection, 'sha123'); + return proj; + } + const rawConfig = (proj: string): Record => + JSON.parse(readFileSync(join(proj, 'pharn.config.json'), 'utf8')); + const recordKeys = (proj: string): string[] => { + const read = readRecords(proj); + return read.kind === 'ok' ? Object.keys(read.store.files) : []; + }; + + // Pre-check P2: pharn-oss's own checker RED-failed the block init used to + // write, three times. The block init writes now passes it. + it("writes a block pharn-oss's own checker passes", async () => { + const proj = await installWith( + JSON.stringify({ models: UPSTREAM_MODELS }), + ); + const checker = join( + import.meta.dirname, + 'fixtures/pharn-oss/check-model-config.mjs', + ); + const r = spawnSync( + process.execPath, + [checker, 'validate', '--config', join(proj, 'pharn.config.json')], + { encoding: 'utf8' }, + ); + expect(r.stdout).toMatch(/^GREEN/); + expect(r.status).toBe(0); + }); + + it('writes no models key when pharn-oss ships no block — never invents one', async () => { + for (const root of [null, '{"ship":{}}', '{"models":null}']) { + vi.mocked(prompts.outro).mockClear(); + const proj = await installWith(root); + expect('models' in rawConfig(proj)).toBe(false); + expect(recordKeys(proj)).not.toContain(MODELS_RECORD_KEY); + expect(outroBody()).not.toContain('Declared model per stage'); + rmSync(join(tmp.path(), 'proj'), { recursive: true, force: true }); + } + }); + + // Review finding (REVIEW.md, P2): a block too deep to serialize used to + // throw after the files were copied, leaving no config at all. + it('finishes the install, with no models key, over a block too deep to copy', async () => { + const depth = 100_000; + const proj = await installWith( + `{"models":{"stages":{"default":{"model":"opus","effort":"high"}},"deep":${'['.repeat(depth)}${']'.repeat(depth)}}}`, + ); + expect(existsSync(join(proj, 'pharn.config.json'))).toBe(true); + expect('models' in rawConfig(proj)).toBe(false); + expect(recordKeys(proj)).not.toContain(MODELS_RECORD_KEY); + }); + + it("writes none, and says why, when pharn-oss's block fails its rules", async () => { + const RLO = String.fromCharCode(0x202e); + vi.mocked(prompts.log.warn).mockClear(); + const proj = await installWith( + JSON.stringify({ + models: { + stages: { + default: { model: 'sonnet', effort: 'high' }, + triage: { model: 'opus', effort: 'high' }, + plan: { model: `${RLO}opus`, effort: 'high' }, + }, + }, + }), + ); + expect('models' in rawConfig(proj)).toBe(false); + expect(recordKeys(proj)).not.toContain(MODELS_RECORD_KEY); + const warning = vi + .mocked(prompts.log.warn) + .mock.calls.map((c) => String(c[0])) + .find((m) => m.includes('models block')); + expect(warning).toContain( + "pharn-oss's models block was not written; this pharn rejects it:", + ); + expect(warning).toContain('stage "triage" is not a product stage'); + expect(warning).toContain('stage "plan" model "opus" is not an alias'); + expect(warning).toContain('upgrade pharn'); + expect(warning).not.toContain(RLO); + }); + }); + it('does not record the user-owned .claude/settings.json', async () => { const repo = join(tmp.path(), 'repo'); const proj = join(tmp.path(), 'proj'); diff --git a/tests/install-records.test.ts b/tests/install-records.test.ts index d18426b7..4d4e0e5f 100644 --- a/tests/install-records.test.ts +++ b/tests/install-records.test.ts @@ -14,6 +14,8 @@ import { useTmpDir } from './helpers.js'; import { buildRecords, mergeRecords, + MODELS_RECORD_KEY, + modelsRecordHash, readRecords, recordsBaseline, RECORDS_FILE, @@ -268,6 +270,35 @@ describe('readRecords — validation is fail-closed and NAMES the failure', () = }); }); + // The one key that is not a file: the models block pharn wrote into + // pharn.config.json. The reader must accept it, or every store holding it + // would read as corrupt and degrade update to `unverifiable`. + it('round-trips the models record key', async () => { + const proj = tmp.path(); + const hash = modelsRecordHash({ stages: {} }); + await writeRecords(proj, { + ...STAMP, + files: { 'a.md': sha('a'), [MODELS_RECORD_KEY]: hash }, + }); + const read = readRecords(proj); + expect(read.kind).toBe('ok'); + expect(read.kind === 'ok' && read.store.files[MODELS_RECORD_KEY]).toBe( + hash, + ); + }); + + it('hashes a models block as pharn serializes it: whitespace no, key order yes', () => { + const block = { stages: { default: { model: 'opus', effort: 'low' } } }; + const reread: unknown = JSON.parse(JSON.stringify(block, null, 4)); + expect(modelsRecordHash(reread)).toBe(modelsRecordHash(block)); + expect(modelsRecordHash(block)).toBe(sha(JSON.stringify(block))); + expect( + modelsRecordHash({ + stages: { default: { effort: 'low', model: 'opus' } }, + }), + ).not.toBe(modelsRecordHash(block)); + }); + it('a non-string stamp invalidates the store', () => { const proj = tmp.path(); writeStore(proj, { ...validStore(), skillsVersion: 42 }); diff --git a/tests/list.test.ts b/tests/list.test.ts index 23905c6c..100b7b20 100644 --- a/tests/list.test.ts +++ b/tests/list.test.ts @@ -1,7 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { ProcessExit, stubProcessExit } from './helpers.js'; import type { PharnConfig } from '../src/types.js'; -import { ModelRoutingError } from '../src/lib/model-routing.js'; import { SeamConfigError } from '../src/lib/seam-config.js'; vi.mock('@clack/prompts', () => ({ @@ -21,8 +20,7 @@ vi.mock('../src/lib/pharn-config.js', () => ({ readPharnConfig, // Real discriminators so list's branches are exercised faithfully. isArchetypeConfig: (c: PharnConfig) => Array.isArray(c.capabilities), - isConfigValidationError: (e: unknown) => - e instanceof ModelRoutingError || e instanceof SeamConfigError, + isConfigValidationError: (e: unknown) => e instanceof SeamConfigError, LEGACY_CONFIG_MESSAGE: LEGACY, })); @@ -205,13 +203,15 @@ describe('runList --json', () => { it('emits the loud validator error to stderr (stdout clean) when config is invalid (BUG 1)', async () => { readPharnConfig.mockImplementationOnce(() => { - throw new ModelRoutingError('models.default has invalid model "gpt-4"'); + throw new SeamConfigError( + 'seam.resolutionOrder has unknown step "guess"', + ); }); await expect(runList({ json: true })).rejects.toMatchObject( new ProcessExit(1), ); expect(logSpy).not.toHaveBeenCalled(); expect(errSpy).toHaveBeenCalled(); - expect(String(errSpy.mock.calls[0]![0])).toMatch(/gpt-4/); + expect(String(errSpy.mock.calls[0]![0])).toMatch(/guess/); }); }); diff --git a/tests/model-config-format.test.ts b/tests/model-config-format.test.ts new file mode 100644 index 00000000..00f0f10d --- /dev/null +++ b/tests/model-config-format.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from 'vitest'; +import { + checkModelsBlock, + type ModelsStages, +} from '../src/lib/model-config.js'; +import { + modelsCheckerCommand, + modelsLabelLines, + resolvedStageLines, +} from '../src/lib/model-config-format.js'; + +const stagesOf = (stages: Record): ModelsStages => { + const check = checkModelsBlock({ stages }); + if (check.kind !== 'valid') throw new Error(JSON.stringify(check)); + return check.stages; +}; + +describe('resolvedStageLines', () => { + it('shows every product stage, resolved, after default — aligned', () => { + const lines = resolvedStageLines( + stagesOf({ + default: { model: 'sonnet', effort: 'high' }, + plan: { model: 'opus', effort: 'max' }, + review: { model: 'claude-opus-4-8', effort: 'xhigh' }, + }), + ); + expect(lines).toEqual([ + 'default sonnet · high', + 'spec sonnet · high (default)', + 'plan opus · max', + 'grill sonnet · high (default)', + 'build sonnet · high (default)', + 'regress sonnet · high (default)', + 'verify sonnet · high (default)', + 'ship sonnet · high (default)', + 'loop sonnet · high (default)', + 'review claude-opus-4-8 · xhigh', + 'memory-promote sonnet · high (default)', + 'ac-test sonnet · high (default)', + ]); + }); + + it('marks nothing when every stage has its own entry', () => { + const own = Object.fromEntries( + [ + 'default', + 'spec', + 'plan', + 'grill', + 'build', + 'regress', + 'verify', + 'ship', + 'loop', + 'review', + 'memory-promote', + 'ac-test', + ].map((stage) => [stage, { model: 'opus', effort: 'high' }]), + ); + expect( + resolvedStageLines(stagesOf(own)).some((l) => l.includes('(default)')), + ).toBe(false); + }); +}); + +describe('the label', () => { + it('says Claude Code applies the frontmatter, not the block', () => { + const lines = modelsLabelLines('pharn/floor'); + expect(lines.join(' ')).toContain( + "Claude Code applies each /pharn-* command's own model:/effort: frontmatter, not this block.", + ); + expect(lines.join(' ')).toContain( + 'The block is the source of truth that frontmatter is held to', + ); + expect(lines.at(-1)).toBe( + 'node pharn/floor/check-model-config.mjs agreement', + ); + // Never routing that happens. + expect(lines.join(' ')).not.toMatch(/rout/i); + // Within 70 columns: a clack note box in an 80-column terminal never + // wraps a line of it. + for (const line of lines) expect(line.length).toBeLessThanOrEqual(70); + }); + + it('names the checker under the install’s own floor dir', () => { + expect(modelsCheckerCommand('.dev/floor', 'validate')).toBe( + 'node .dev/floor/check-model-config.mjs validate', + ); + }); +}); diff --git a/tests/model-config-parity.test.ts b/tests/model-config-parity.test.ts new file mode 100644 index 00000000..eeddbc7d --- /dev/null +++ b/tests/model-config-parity.test.ts @@ -0,0 +1,338 @@ +import { spawnSync } from 'node:child_process'; +import { mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; +import { sha256File } from '../src/lib/hash.js'; +import { + checkModelsBlock, + EFFORT_LEVELS, + MODEL_ALIASES, + MODEL_ID_RE, + PRODUCT_STAGES, + resolveStageModel, + type ModelsStages, +} from '../src/lib/model-config.js'; +import { readUpstreamModels } from '../src/lib/upstream-models.js'; +import { useTmpDir } from './helpers.js'; + +// --------------------------------------------------------------------------- +// src/lib/model-config.ts is a COPY of pharn-oss's rules for the `models` +// block. This file is what keeps pharn-oss their one owner: it runs the real +// checker — pharn-oss's pharn/floor/check-model-config.mjs, vendored +// byte-for-byte — over a corpus, and fails on any verdict, RED line or +// resolution that differs from the copy's. The four value sets are read out +// of the checker's own source, so a set cannot drift either. Refreshing the +// vendored file: docs/contributing.md. +// --------------------------------------------------------------------------- + +const CHECKER = resolve( + dirname(fileURLToPath(import.meta.url)), + 'fixtures/pharn-oss/check-model-config.mjs', +); + +// The vendored checker, pinned: pharn-dev/pharn-oss `main` @ 767bf61 +// (SKILLS_VERSION 6.22.0), where the file last changed in 8ba9308. A refresh +// changes this line in the same commit as the file. +const PINNED_SHA256 = + '361bc3ce71d0c0b1e7caf1020ab80e7cc6ba8ee4990eff94de038baeeef82414'; + +function runChecker(args: string[]): { status: number | null; stdout: string } { + const r = spawnSync(process.execPath, [CHECKER, ...args], { + encoding: 'utf8', + }); + return { status: r.status, stdout: r.stdout }; +} + +// The checker prints one `RED — failed: ` line per RED. +// `[\s\S]` because a detail may quote any character but LF. +function checkerReds(stdout: string): string[] { + return stdout.split('\n').flatMap((line) => { + const m = /^RED — (\w+) failed: ([\s\S]*)$/.exec(line); + return m ? [`${m[1]}: ${m[2]}`] : []; + }); +} + +// pharn-oss's own root block at the pinned commit. +const UPSTREAM_BLOCK = { + stages: { + default: { model: 'sonnet', effort: 'high' }, + spec: { model: 'opus', effort: 'high' }, + plan: { model: 'opus', effort: 'high' }, + grill: { model: 'opus', effort: 'high' }, + build: { model: 'sonnet', effort: 'high' }, + regress: { model: 'sonnet', effort: 'high' }, + verify: { model: 'sonnet', effort: 'high' }, + ship: { model: 'sonnet', effort: 'high' }, + loop: { model: 'sonnet', effort: 'high' }, + review: { model: 'opus', effort: 'high' }, + 'memory-promote': { model: 'opus', effort: 'high' }, + 'ac-test': { model: 'opus', effort: 'high' }, + }, +}; + +const D = '"default":{"model":"sonnet","effort":"high"}'; +const entry = (model: string, effort = '"high"'): string => + `{"model":${model},"effort":${effort}}`; + +// [label, the `models` value as JSON text — or null for "no `models` key"]. +// JSON TEXT, not objects, so a `__proto__` key is an OWN key exactly as the +// checker's JSON.parse makes it. +const BLOCKS: [string, string | null][] = [ + ['no models key', null], + ['models: null', 'null'], + ['models: a number', '5'], + ['models: a string', '"x"'], + ['models: an array', '[]'], + ['models: true', 'true'], + ['models: {}', '{}'], + ['stages: null', '{"stages":null}'], + ['stages: an array', '{"stages":[]}'], + ['stages: a string', '{"stages":"x"}'], + ['stages: {}', '{"stages":{}}'], + ["pharn-oss's own block", JSON.stringify(UPSTREAM_BLOCK)], + [ + 'the default pharn wrote from #57 to 0.6.0', + '{"default":{"model":"sonnet-5","effort":"high"},"stages":{"plan":{"model":"opus-4-8","effort":"max"},"review":{"model":"opus-4-8","effort":"high"}}}', + ], + [ + 'the default pharn wrote before #57', + '{"default":{"model":"sonnet-5","effort":"high"},"stages":{"plan":{"model":"opus-4-8","effort":"max"},"review":{"model":"fable-5","effort":"max"}}}', + ], + ['only default', `{"stages":{${D}}}`], + ['an unknown stage', `{"stages":{${D},"deploy":${entry('"opus"')}}}`], + [ + 'inherited names as stage keys', + `{"stages":{${D},"constructor":${entry('"opus"')},"toString":${entry('"opus"')},"hasOwnProperty":${entry('"opus"')},"valueOf":${entry('"opus"')},"__proto__":${entry('"opus"')}}}`, + ], + ['default is a string', '{"stages":{"default":"sonnet"}}'], + ['default is null', '{"stages":{"default":null}}'], + ['default is an array', '{"stages":{"default":[]}}'], + ['a stage is a number', `{"stages":{${D},"plan":5}}`], + ['model missing', `{"stages":{${D},"plan":{"effort":"high"}}}`], + ['effort missing', `{"stages":{${D},"plan":{"model":"opus"}}}`], + ['both missing', `{"stages":{${D},"plan":{}}}`], + ...[ + '"opus-4-8"', + '"gpt-4"', + '"claude-"', + '"claude-Opus-4"', + '"CLAUDE-opus"', + '"claude-opus-4-8[1m]"', + '"Opus"', + '""', + '5', + 'null', + '{"x":1}', + '["opus"]', + ].map((model): [string, string] => [ + `model ${model}`, + `{"stages":{${D},"plan":${entry(model)}}}`, + ]), + ...[ + '"sonnet"', + '"opus"', + '"haiku"', + '"fable"', + '"inherit"', + '"claude-opus-4-8"', + '"claude-3"', + '"claude-x-"', + ].map((model): [string, string] => [ + `model ${model}`, + `{"stages":{${D},"plan":${entry(model)}}}`, + ]), + ...['"extreme"', '""', '"HIGH"', '3', 'null'].map( + (effort): [string, string] => [ + `effort ${effort}`, + `{"stages":{${D},"plan":${entry('"opus"', effort)}}}`, + ], + ), + ...['"low"', '"medium"', '"high"', '"xhigh"', '"max"'].map( + (effort): [string, string] => [ + `effort ${effort}`, + `{"stages":{${D},"plan":${entry('"opus"', effort)}}}`, + ], + ), + [ + 'extra keys inside an entry', + `{"stages":{${D},"plan":{"model":"opus","effort":"high","note":"x"}}}`, + ], + [ + 'extra keys beside stages, a top-level default among them', + `{"note":"x","default":{"model":"gpt-4","effort":"x"},"stages":{${D}}}`, + ], + [ + 'every RED at once, in order', + `{"stages":{"deploy":${entry('"opus"')},"plan":${entry('"gpt-4"', '"extreme"')},"review":"x","grill":{}}}`, + ], + [ + 'control and format characters quoted in details', + `{"stages":{${D},"pl\\u001ban":${entry('"opus"')},"plan":${entry('"\\u202eopus"', '"hi\\u009bgh"')}}}`, + ], +]; + +// Upstream's ROOT config, as `readUpstreamModels` reads it from a clone: +// [label, the file's text — or null for "no file"]. +const FILES: [string, string | null][] = [ + ['no file', null], + ['not JSON', '{"models":'], + ['JSON null', 'null'], + ['a JSON array', '[]'], + ['a JSON string', '"x"'], + ['a JSON number', '5'], + ['{}', '{}'], + ['models: null', '{"models":null}'], + ['models: {}', '{"models":{}}'], + ["pharn-oss's own file", JSON.stringify({ models: UPSTREAM_BLOCK })], + ['a bad block', '{"models":{"stages":{}}}'], +]; + +describe('the vendored checker is pharn-oss’s, unedited', () => { + it('matches the pinned sha256', () => { + expect(sha256File(CHECKER)).toBe(PINNED_SHA256); + }); +}); + +describe("model-config's sets are the checker's", () => { + // The checker runs main() at import, so the block of constant declarations + // is lifted out of its source and evaluated on its own. + const src = readFileSync(CHECKER, 'utf8'); + const start = src.indexOf('const PRODUCT_STAGES = {'); + const end = src.indexOf('const DEFAULT_CONFIG'); + const upstream = new Function( + `${src.slice(start, end)}; return { PRODUCT_STAGES, MODEL_ALIASES, MODEL_ID_RE, EFFORT_ENUM };`, + )() as { + PRODUCT_STAGES: Record; + MODEL_ALIASES: string[]; + MODEL_ID_RE: RegExp; + EFFORT_ENUM: string[]; + }; + + it('finds the declarations', () => { + expect(start).toBeGreaterThan(-1); + expect(end).toBeGreaterThan(start); + }); + + it('product stages — the keys, in order', () => { + expect([...PRODUCT_STAGES]).toEqual(Object.keys(upstream.PRODUCT_STAGES)); + }); + + it('model aliases', () => { + expect([...MODEL_ALIASES]).toEqual(upstream.MODEL_ALIASES); + }); + + it('the model id pattern', () => { + expect(MODEL_ID_RE.source).toBe(upstream.MODEL_ID_RE.source); + expect(MODEL_ID_RE.flags).toBe(upstream.MODEL_ID_RE.flags); + }); + + it('effort levels', () => { + expect([...EFFORT_LEVELS]).toEqual(upstream.EFFORT_ENUM); + }); +}); + +describe('validate: the copy gives the checker’s verdict and RED lines', () => { + const tmp = useTmpDir(); + const observedKinds = new Set(); + + it.each(BLOCKS)('%s', (_label, text) => { + const file = join(tmp.path(), 'pharn.config.json'); + writeFileSync(file, text === null ? '{}' : `{"models":${text}}`); + const value: unknown = text === null ? undefined : JSON.parse(text); + + const r = runChecker(['validate', '--config', file]); + const upstreamReds = checkerReds(r.stdout); + upstreamReds.forEach((red) => observedKinds.add(red.split(':')[0]!)); + + const check = checkModelsBlock(value); + expect(r.status).toBe(check.kind === 'invalid' ? 1 : 0); + expect( + check.kind === 'invalid' + ? check.reds.map((red) => `${red.kind}: ${red.detail}`) + : [], + ).toEqual(upstreamReds); + }); + + it.each(FILES)('reading a clone whose root config is %s', (_label, text) => { + const repo = join(tmp.path(), 'clone'); + mkdirSync(repo, { recursive: true }); + const file = join(repo, 'pharn.config.json'); + if (text !== null) writeFileSync(file, text); + + const r = runChecker(['validate', '--config', file]); + checkerReds(r.stdout).forEach((red) => + observedKinds.add(red.split(':')[0]!), + ); + const read = readUpstreamModels(repo); + // GREEN ⇔ there is nothing to refuse: no block, or a block that passed. + expect(r.status).toBe(read.kind === 'invalid' ? 1 : 0); + }); + + it('reading a clone whose root config is a directory', () => { + const repo = join(tmp.path(), 'clone-dir'); + mkdirSync(join(repo, 'pharn.config.json'), { recursive: true }); + const r = runChecker([ + 'validate', + '--config', + join(repo, 'pharn.config.json'), + ]); + checkerReds(r.stdout).forEach((red) => + observedKinds.add(red.split(':')[0]!), + ); + expect(r.status).toBe(1); + expect(readUpstreamModels(repo).kind).toBe('invalid'); + }); + + // The corpus is only as good as its reach. Every RED kind the checker's + // validate path can emit — read from its own red("…") call sites, minus the + // two modes this CLI does not copy — must have been produced above. + it('reached every RED kind the checker validates with', () => { + const src = readFileSync(CHECKER, 'utf8'); + const kinds = new Set( + [...src.matchAll(/\bred\(\s*"([a-z]+)"/g)].map((m) => m[1]!), + ); + kinds.delete('resolve'); + kinds.delete('agreement'); + expect([...kinds].sort()).toEqual( + [ + 'default', + 'effort', + 'entry', + 'input', + 'json', + 'model', + 'shape', + 'stage', + ].sort(), + ); + for (const kind of kinds) expect(observedKinds).toContain(kind); + }); +}); + +describe('resolve: the copy picks what the checker picks', () => { + const tmp = useTmpDir(); + const partial = `{"stages":{${D},"plan":${entry('"opus"', '"max"')},"review":{"model":"fable","effort":"xhigh","note":"x"}}}`; + + it.each([ + ...PRODUCT_STAGES, + 'default', + 'constructor', + 'toString', + '__proto__', + 'bogus', + ])('%s', (stage) => { + for (const text of [partial, JSON.stringify(UPSTREAM_BLOCK)]) { + const file = join(tmp.path(), 'pharn.config.json'); + writeFileSync(file, `{"models":${text}}`); + const r = runChecker(['resolve', stage, '--config', file]); + expect(r.status).toBe(0); + const check = checkModelsBlock(JSON.parse(text)); + expect(check.kind).toBe('valid'); + const stages = (check as { stages: ModelsStages }).stages; + expect(`${JSON.stringify(resolveStageModel(stages, stage))}\n`).toBe( + r.stdout, + ); + } + }); +}); diff --git a/tests/model-config.test.ts b/tests/model-config.test.ts new file mode 100644 index 00000000..ed8634c5 --- /dev/null +++ b/tests/model-config.test.ts @@ -0,0 +1,109 @@ +import { describe, expect, it } from 'vitest'; +import { + checkModelsBlock, + PRODUCT_STAGES, + resolveStageModel, + type ModelsStages, +} from '../src/lib/model-config.js'; + +// The copy's API. Its RULES are pinned to pharn-oss's checker by +// tests/model-config-parity.test.ts; this file pins the shape callers use. + +const valid = (stages: Record): ModelsStages => { + const check = checkModelsBlock({ stages }); + if (check.kind !== 'valid') throw new Error(JSON.stringify(check)); + return check.stages; +}; + +describe('checkModelsBlock', () => { + it('reads an absent block, and one with no stages, as nothing declared', () => { + expect(checkModelsBlock(undefined)).toEqual({ kind: 'no-stages' }); + expect(checkModelsBlock(null)).toEqual({ kind: 'no-stages' }); + expect(checkModelsBlock({})).toEqual({ kind: 'no-stages' }); + expect(checkModelsBlock({ stages: null })).toEqual({ kind: 'no-stages' }); + }); + + it('returns the stages it passed, by identity', () => { + const stages = { default: { model: 'sonnet', effort: 'high' } }; + const check = checkModelsBlock({ stages }); + expect(check).toEqual({ kind: 'valid', stages }); + expect((check as { stages: unknown }).stages).toBe(stages); + }); + + it('collects every RED, in order, each with its kind', () => { + const check = checkModelsBlock({ + stages: { + deploy: { model: 'opus', effort: 'high' }, + plan: { model: 'opus-4-8', effort: 'max' }, + review: 'x', + }, + }); + expect(check.kind).toBe('invalid'); + expect( + (check as { reds: { kind: string }[] }).reds.map((red) => red.kind), + ).toEqual(['default', 'stage', 'model', 'entry']); + }); + + it('names the product stages in a stage RED', () => { + const check = checkModelsBlock({ + stages: { default: { model: 'opus', effort: 'low' }, eval: {} }, + }); + expect(check).toEqual({ + kind: 'invalid', + reds: [ + { + kind: 'stage', + detail: `stage "eval" is not a product stage — expected one of {${PRODUCT_STAGES.join(', ')}} or "default"`, + }, + ], + }); + }); + + it('refuses a shape before it reads any stage', () => { + expect(checkModelsBlock([])).toEqual({ + kind: 'invalid', + reds: [ + { kind: 'shape', detail: '`models` is present but is not an object' }, + ], + }); + expect(checkModelsBlock({ stages: 'x' })).toEqual({ + kind: 'invalid', + reds: [ + { + kind: 'shape', + detail: '`models.stages` is present but is not an object', + }, + ], + }); + }); +}); + +describe('resolveStageModel', () => { + const stages = valid({ + default: { model: 'sonnet', effort: 'high' }, + plan: { model: 'opus', effort: 'max', note: 'kept, ignored' }, + }); + + it('picks the stage’s own entry — model and effort only', () => { + expect(resolveStageModel(stages, 'plan')).toEqual({ + model: 'opus', + effort: 'max', + }); + }); + + it('falls back to default for a stage without one', () => { + expect(resolveStageModel(stages, 'review')).toEqual({ + model: 'sonnet', + effort: 'high', + }); + }); + + it('never resolves through the prototype', () => { + for (const name of ['constructor', 'toString', '__proto__']) { + expect(resolveStageModel(stages, name)).toEqual({ + model: 'sonnet', + effort: 'high', + }); + } + }); +}); diff --git a/tests/model-routing-format.test.ts b/tests/model-routing-format.test.ts deleted file mode 100644 index 5fb64f30..00000000 --- a/tests/model-routing-format.test.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import { formatModelRoutingLines } from '../src/lib/model-routing-format.js'; -import { DEFAULT_MODEL_ROUTING } from '../src/lib/model-routing.js'; -import type { ModelRouting } from '../src/types.js'; - -describe('formatModelRoutingLines', () => { - it('renders DEFAULT_MODEL_ROUTING as aligned, default-first lines', () => { - expect(formatModelRoutingLines(DEFAULT_MODEL_ROUTING)).toEqual([ - 'default sonnet-5 · high', - 'plan opus-4-8 · max', - 'review opus-4-8 · high', - ]); - }); - - it('renders FROM the given config, not a hardcoded default (custom values flow through)', () => { - const custom: ModelRouting = { - default: { model: 'haiku-4-5', effort: 'low' }, - stages: { review: { model: 'fable-5', effort: 'max' } }, - }; - expect(formatModelRoutingLines(custom)).toEqual([ - 'default haiku-4-5 · low', - 'review fable-5 · max', - ]); - }); - - it('orders default first, then configured stages in PIPELINE_STAGES order (not JSON key order)', () => { - const routing: ModelRouting = { - default: { model: 'sonnet-5', effort: 'high' }, - // Deliberately out of pipeline order in the source object. - stages: { - review: { model: 'opus-4-8', effort: 'high' }, - build: { model: 'haiku-4-5', effort: 'low' }, - plan: { model: 'opus-4-8', effort: 'max' }, - }, - }; - const labels = formatModelRoutingLines(routing).map( - (line) => line.split(/\s{2,}/)[0], - ); - expect(labels).toEqual(['default', 'plan', 'build', 'review']); - }); - - it('renders a single default line when stages is empty', () => { - expect( - formatModelRoutingLines({ - default: { model: 'sonnet-5', effort: 'high' }, - stages: {}, - }), - ).toEqual(['default sonnet-5 · high']); - }); -}); diff --git a/tests/model-routing.test.ts b/tests/model-routing.test.ts deleted file mode 100644 index 8fb7c6d8..00000000 --- a/tests/model-routing.test.ts +++ /dev/null @@ -1,216 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import { - DEFAULT_MODEL_ROUTING, - EFFORT_LEVELS, - MODEL_IDS, - ModelRoutingError, - PIPELINE_STAGES, - resolveStageModel, - validateModelRouting, -} from '../src/lib/model-routing.js'; -import type { ModelRouting } from '../src/types.js'; - -// A minimal valid routing used as a base for the resolver + accept cases. -const valid: ModelRouting = { - default: { model: 'sonnet-5', effort: 'high' }, - stages: { - plan: { model: 'opus-4-8', effort: 'max' }, - review: { model: 'fable-5', effort: 'max' }, - }, -}; - -describe('validateModelRouting', () => { - it('accepts a valid routing (default + stages) and returns it typed', () => { - expect(validateModelRouting(valid)).toEqual(valid); - }); - - it('accepts a routing with stages omitted (default only)', () => { - expect( - validateModelRouting({ default: { model: 'haiku-4-5', effort: 'low' } }), - ).toEqual({ - default: { model: 'haiku-4-5', effort: 'low' }, - stages: {}, - }); - }); - - it('accepts an empty stages object', () => { - const out = validateModelRouting({ - default: { model: 'fable-5', effort: 'max' }, - stages: {}, - }); - expect(out.stages).toEqual({}); - }); - - it('rejects an invalid model string, naming the value', () => { - expect(() => - validateModelRouting({ default: { model: 'gpt-4', effort: 'high' } }), - ).toThrow(ModelRoutingError); - expect(() => - validateModelRouting({ default: { model: 'gpt-4', effort: 'high' } }), - ).toThrow(/gpt-4/); - }); - - it('rejects an invalid effort — "medium" is not in {low, high, max}', () => { - expect(() => - validateModelRouting({ - default: { model: 'sonnet-5', effort: 'medium' }, - }), - ).toThrow(/medium/); - }); - - it('rejects an unknown stage key, naming it', () => { - expect(() => - validateModelRouting({ - default: { model: 'sonnet-5', effort: 'high' }, - stages: { deploy: { model: 'sonnet-5', effort: 'low' } }, - }), - ).toThrow(/deploy/); - }); - - it('rejects a wrong-type (non-string) model or effort — the typeof guard', () => { - expect(() => - validateModelRouting({ default: { model: 123, effort: 'high' } }), - ).toThrow(ModelRoutingError); - expect(() => - validateModelRouting({ default: { model: 'sonnet-5', effort: null } }), - ).toThrow(ModelRoutingError); - }); - - it('rejects a missing or non-object default (fail-closed)', () => { - expect(() => validateModelRouting({})).toThrow(ModelRoutingError); - expect(() => validateModelRouting({ default: 'sonnet-5' })).toThrow( - ModelRoutingError, - ); - expect(() => validateModelRouting(null)).toThrow(ModelRoutingError); - expect(() => validateModelRouting('nope')).toThrow(ModelRoutingError); - }); - - it('rejects a non-object stages', () => { - expect(() => - validateModelRouting({ - default: { model: 'sonnet-5', effort: 'high' }, - stages: ['plan'], - }), - ).toThrow(ModelRoutingError); - }); - - it('accepts every model id × effort level in the allowlists', () => { - for (const model of MODEL_IDS) { - for (const effort of EFFORT_LEVELS) { - expect(() => - validateModelRouting({ default: { model, effort } }), - ).not.toThrow(); - } - } - }); - - it('accepts every known pipeline stage as a stages key', () => { - for (const stage of PIPELINE_STAGES) { - expect(() => - validateModelRouting({ - default: { model: 'sonnet-5', effort: 'high' }, - stages: { [stage]: { model: 'sonnet-5', effort: 'low' } }, - }), - ).not.toThrow(); - } - }); - - it('rejects an unknown sibling key (a typo\'d "stgaes"), naming it (BUG 2)', () => { - expect(() => - validateModelRouting({ - default: { model: 'sonnet-5', effort: 'high' }, - stgaes: { build: { model: 'sonnet-5', effort: 'low' } }, - }), - ).toThrow(/stgaes/); - expect(() => - validateModelRouting({ - default: { model: 'sonnet-5', effort: 'high' }, - stgaes: {}, - }), - ).toThrow(ModelRoutingError); - }); - - it('rejects an unknown key inside a stage entry (StageModel), naming it (BUG 2)', () => { - expect(() => - validateModelRouting({ - default: { model: 'sonnet-5', effort: 'high', modol: 'x' }, - }), - ).toThrow(/modol/); - }); -}); - -describe('DEFAULT_MODEL_ROUTING', () => { - it('is itself valid (passes validateModelRouting)', () => { - expect(() => validateModelRouting(DEFAULT_MODEL_ROUTING)).not.toThrow(); - }); - - it('uses only known stage keys and model ids', () => { - for (const stage of Object.keys(DEFAULT_MODEL_ROUTING.stages)) { - expect(PIPELINE_STAGES).toContain(stage); - } - const models: string[] = [DEFAULT_MODEL_ROUTING.default.model]; - for (const entry of Object.values(DEFAULT_MODEL_ROUTING.stages)) { - if (entry) models.push(entry.model); - } - for (const model of models) expect(MODEL_IDS).toContain(model); - }); - - it('routes review to opus-4-8/high — the spend-safe default (fable-5/max is the opt-in)', () => { - expect(DEFAULT_MODEL_ROUTING.stages.review).toEqual({ - model: 'opus-4-8', - effort: 'high', - }); - }); - - it('keeps plan opus-4-8/max and default sonnet-5/high', () => { - expect(DEFAULT_MODEL_ROUTING.stages.plan).toEqual({ - model: 'opus-4-8', - effort: 'max', - }); - expect(DEFAULT_MODEL_ROUTING.default).toEqual({ - model: 'sonnet-5', - effort: 'high', - }); - }); -}); - -describe('resolveStageModel', () => { - it('returns the stage entry when one exists', () => { - expect(resolveStageModel(valid, 'plan')).toEqual({ - model: 'opus-4-8', - effort: 'max', - }); - expect(resolveStageModel(valid, 'review')).toEqual({ - model: 'fable-5', - effort: 'max', - }); - }); - - it('falls back to default for a stage without an entry', () => { - expect(resolveStageModel(valid, 'build')).toEqual(valid.default); - expect(resolveStageModel(valid, 'verify')).toEqual(valid.default); - }); - - it('routes every stage to default when stages is empty (one-model config)', () => { - const oneModel: ModelRouting = { - default: { model: 'fable-5', effort: 'high' }, - stages: {}, - }; - for (const stage of PIPELINE_STAGES) { - expect(resolveStageModel(oneModel, stage)).toEqual({ - model: 'fable-5', - effort: 'high', - }); - } - }); - - it('falls back to default when stages is absent (unvalidated routing)', () => { - const noStages = { - default: { model: 'sonnet-5', effort: 'high' }, - } as unknown as ModelRouting; - expect(resolveStageModel(noStages, 'plan')).toEqual({ - model: 'sonnet-5', - effort: 'high', - }); - }); -}); diff --git a/tests/models-update.test.ts b/tests/models-update.test.ts new file mode 100644 index 00000000..c58000de --- /dev/null +++ b/tests/models-update.test.ts @@ -0,0 +1,484 @@ +import { describe, expect, it } from 'vitest'; +import { modelsRecordHash } from '../src/lib/install-records.js'; +import { checkModelsBlock } from '../src/lib/model-config.js'; +import { + convertLegacyModels, + decideModelsUpdate, + isLegacyDefault, + modelsMigrationPending, + needsModelsConversion, +} from '../src/lib/models-update.js'; +import type { UpstreamModels } from '../src/lib/upstream-models.js'; + +// The two defaults earlier pharns wrote, exactly as they wrote them. +const DEFAULT_057 = { + default: { model: 'sonnet-5', effort: 'high' }, + stages: { + plan: { model: 'opus-4-8', effort: 'max' }, + review: { model: 'opus-4-8', effort: 'high' }, + }, +}; +const DEFAULT_024 = { + default: { model: 'sonnet-5', effort: 'high' }, + stages: { + plan: { model: 'opus-4-8', effort: 'max' }, + review: { model: 'fable-5', effort: 'max' }, + }, +}; + +// Stands in for pharn-oss's block. +const UPSTREAM = { + stages: { + default: { model: 'sonnet', effort: 'high' }, + plan: { model: 'opus', effort: 'high' }, + review: { model: 'opus', effort: 'high' }, + }, +}; +const OK: UpstreamModels = { kind: 'ok', block: UPSTREAM }; +const LATEST = modelsRecordHash(UPSTREAM); + +// A block the user changed, already in pharn-oss's format. +const EDITED = { + stages: { + default: { model: 'haiku', effort: 'low' }, + review: { model: 'fable', effort: 'max' }, + }, +}; + +const decide = ( + over: Partial[0]>, +): ReturnType => + decideModelsUpdate({ + current: undefined, + upstream: OK, + recorded: null, + recordsAvailable: true, + force: false, + ...over, + }); + +describe('isLegacyDefault', () => { + it('knows both defaults earlier pharns wrote', () => { + expect(isLegacyDefault(DEFAULT_057)).toBe(true); + expect(isLegacyDefault(DEFAULT_024)).toBe(true); + }); + + it('compares the block as pharn serializes it: indentation is not an edit', () => { + const reread: unknown = JSON.parse(JSON.stringify(DEFAULT_057, null, 4)); + expect(isLegacyDefault(reread)).toBe(true); + }); + + it('reads a reordered or changed default as edited', () => { + expect( + isLegacyDefault({ + stages: DEFAULT_057.stages, + default: DEFAULT_057.default, + }), + ).toBe(false); + expect( + isLegacyDefault({ + ...DEFAULT_057, + default: { model: 'sonnet-5', effort: 'low' }, + }), + ).toBe(false); + expect(isLegacyDefault(undefined)).toBe(false); + expect(isLegacyDefault(UPSTREAM)).toBe(false); + }); +}); + +describe('convertLegacyModels', () => { + it('converts the old format: default moves into stages, ids become aliases', () => { + const edited = { + default: { model: 'haiku-4-5', effort: 'low' }, + stages: { + review: { model: 'fable-5', effort: 'max', note: 'mine' }, + build: { model: 'claude-sonnet-5', effort: 'high' }, + }, + }; + const out = convertLegacyModels(edited); + expect(out.block).toEqual({ + stages: { + default: { model: 'haiku', effort: 'low' }, + review: { model: 'fable', effort: 'max', note: 'mine' }, + build: { model: 'claude-sonnet-5', effort: 'high' }, + }, + }); + // `default` comes first, as pharn-oss writes it. + expect(Object.keys((out.block as { stages: object }).stages)[0]).toBe( + 'default', + ); + expect(out.changes).toEqual([ + 'moved `models.default` into `models.stages.default`', + 'stage "default" model "haiku-4-5" → "haiku"', + 'stage "review" model "fable-5" → "fable"', + ]); + expect(out.leftovers).toEqual([]); + // The result is a block pharn-oss's rules accept. + expect(checkModelsBlock(out.block).kind).toBe('valid'); + // The input is not mutated. + expect(edited.default.model).toBe('haiku-4-5'); + }); + + it('maps all four ids the old CLI knew', () => { + const out = convertLegacyModels({ + stages: { + default: { model: 'sonnet-5', effort: 'high' }, + plan: { model: 'opus-4-8', effort: 'high' }, + review: { model: 'fable-5', effort: 'high' }, + build: { model: 'haiku-4-5', effort: 'high' }, + }, + }); + expect(out.block).toEqual({ + stages: { + default: { model: 'sonnet', effort: 'high' }, + plan: { model: 'opus', effort: 'high' }, + review: { model: 'fable', effort: 'high' }, + build: { model: 'haiku', effort: 'high' }, + }, + }); + }); + + it('creates stages for a default with none, or with a null one', () => { + for (const block of [ + { default: { model: 'sonnet-5', effort: 'high' } }, + { default: { model: 'sonnet-5', effort: 'high' }, stages: null }, + ]) { + expect(convertLegacyModels(block).block).toEqual({ + stages: { default: { model: 'sonnet', effort: 'high' } }, + }); + } + }); + + it('leaves a default that cannot move where it is, and names it', () => { + const taken = convertLegacyModels({ + default: { model: 'opus-4-8', effort: 'max' }, + stages: { default: { model: 'sonnet-5', effort: 'high' } }, + }); + expect(taken.block).toEqual({ + default: { model: 'opus-4-8', effort: 'max' }, + stages: { default: { model: 'sonnet', effort: 'high' } }, + }); + expect(taken.leftovers).toEqual([ + '`models.default` was left where it is: `models.stages.default` is already set, and pharn-oss reads only that one', + ]); + + const notAnObject = convertLegacyModels({ + default: { model: 'opus-4-8', effort: 'max' }, + stages: ['x'], + }); + expect(notAnObject.block).toEqual({ + default: { model: 'opus-4-8', effort: 'max' }, + stages: ['x'], + }); + expect(notAnObject.changes).toEqual([]); + expect(notAnObject.leftovers).toEqual([ + '`models.default` was left where it is: `models.stages` is not an object, so it cannot move into it', + ]); + }); + + it('copies what has no mapping verbatim — never guesses', () => { + const out = convertLegacyModels({ + default: { model: 'gpt-4', effort: 'extreme' }, + stages: { deploy: { model: 'opus-4-8', effort: 'high' }, plan: 'x' }, + }); + expect(out.block).toEqual({ + stages: { + default: { model: 'gpt-4', effort: 'extreme' }, + deploy: { model: 'opus', effort: 'high' }, + plan: 'x', + }, + }); + }); + + it('returns a block with nothing to convert unchanged — the same object', () => { + for (const block of [UPSTREAM, EDITED, null, 5, 'x', [], undefined]) { + const out = convertLegacyModels(block); + expect(out.block).toBe(block); + expect(out.changes).toEqual([]); + } + }); + + it('keeps a `__proto__` stage key as data', () => { + const block: unknown = JSON.parse( + '{"default":{"model":"opus-4-8","effort":"max"},"stages":{"__proto__":{"model":"sonnet-5","effort":"high"}}}', + ); + const out = convertLegacyModels(block); + const stages = (out.block as { stages: object }).stages; + expect(Object.getPrototypeOf(stages)).toBe(Object.prototype); + expect(Object.hasOwn(stages, '__proto__')).toBe(true); + expect(Object.keys(stages)).toEqual(['default', '__proto__']); + }); +}); + +describe('modelsMigrationPending — what re-opens update’s early return', () => { + it('is pending while the block needs converting', () => { + expect(modelsMigrationPending(DEFAULT_057, null)).toBe(true); + expect( + modelsMigrationPending(DEFAULT_057, modelsRecordHash(UPSTREAM)), + ).toBe(true); + expect(modelsMigrationPending(UPSTREAM, null)).toBe(false); + expect(modelsMigrationPending(undefined, null)).toBe(false); + }); + + // Review finding (REVIEW.md, P5): a block pharn recorded writing can only be + // pharn-oss's own, which update never converts — so it must not hold the + // gate open, or a convertible upstream block would re-open it forever. + it('is not pending for the block pharn recorded writing', () => { + const upstreamShaped = { default: { model: 'sonnet', effort: 'high' } }; + expect(needsModelsConversion(upstreamShaped)).toBe(true); + expect( + modelsMigrationPending(upstreamShaped, modelsRecordHash(upstreamShaped)), + ).toBe(false); + }); +}); + +describe('needsModelsConversion', () => { + it('is true while something would convert, false once nothing would', () => { + expect(needsModelsConversion(DEFAULT_057)).toBe(true); + expect( + needsModelsConversion({ stages: { plan: { model: 'opus-4-8' } } }), + ).toBe(true); + const converted = convertLegacyModels(DEFAULT_057).block; + expect(needsModelsConversion(converted)).toBe(false); + expect(needsModelsConversion(UPSTREAM)).toBe(false); + expect(needsModelsConversion(undefined)).toBe(false); + }); + + it('does not hold the gate open for a default that cannot move', () => { + expect( + needsModelsConversion({ + default: { model: 'haiku', effort: 'low' }, + stages: { default: { model: 'sonnet', effort: 'high' } }, + }), + ).toBe(false); + }); +}); + +describe('decideModelsUpdate — the per-file rows, over the block', () => { + it('row 1: no block → pharn-oss’s is written (restored)', () => { + expect(decide({ current: undefined })).toMatchObject({ + outcome: 'restored', + next: UPSTREAM, + nextRecord: LATEST, + backup: false, + }); + }); + + it('row 2: already pharn-oss’s → nothing to do, record refreshed', () => { + expect(decide({ current: structuredClone(UPSTREAM) })).toMatchObject({ + outcome: 'ok', + next: UPSTREAM, + nextRecord: LATEST, + }); + }); + + it('row 3: a default an earlier pharn wrote → replaced, with no records at all', () => { + for (const current of [DEFAULT_057, DEFAULT_024]) { + expect( + decide({ current, recorded: null, recordsAvailable: false }), + ).toMatchObject({ + outcome: 'updated', + replacedLegacyDefault: true, + next: UPSTREAM, + nextRecord: LATEST, + backup: false, + }); + } + }); + + it('row 3: still the block pharn recorded writing → replaced', () => { + const previous = { stages: { default: { model: 'opus', effort: 'low' } } }; + expect( + decide({ current: previous, recorded: modelsRecordHash(previous) }), + ).toMatchObject({ + outcome: 'updated', + replacedLegacyDefault: false, + next: UPSTREAM, + nextRecord: LATEST, + }); + }); + + const STALE = modelsRecordHash({ stages: {} }); + it.each([ + ['modified', { recorded: STALE }, STALE], + ['unrecorded', { recorded: null }, null], + ['unverifiable', { recordsAvailable: false }, null], + ] as const)( + 'rows 4-6: %s → the user’s block is KEPT', + (label, over, carried) => { + const out = decide({ current: EDITED, ...over }); + expect(out).toMatchObject({ + outcome: 'kept', + label, + next: EDITED, + backup: false, + conversion: null, + problems: [], + }); + // The previous record is carried forward, never a fresh one. + expect(out.nextRecord).toBe(carried); + }, + ); + + it('--force: the user’s block is replaced, after a backup', () => { + expect( + decide({ + current: EDITED, + recorded: modelsRecordHash({ stages: {} }), + force: true, + }), + ).toMatchObject({ + outcome: 'forced', + label: 'modified', + next: UPSTREAM, + nextRecord: LATEST, + backup: true, + }); + }); + + it('--force over an old default is a plain update — nothing to back up', () => { + expect(decide({ current: DEFAULT_057, force: true })).toMatchObject({ + outcome: 'updated', + backup: false, + }); + }); + + it('a kept block in the old format is CONVERTED, not reset', () => { + const edited = { + default: { model: 'haiku-4-5', effort: 'low' }, + stages: { review: { model: 'fable-5', effort: 'max' } }, + }; + const out = decide({ current: edited }); + expect(out.outcome).toBe('kept'); + expect(out.next).toEqual({ + stages: { + default: { model: 'haiku', effort: 'low' }, + review: { model: 'fable', effort: 'max' }, + }, + }); + expect(out.conversion?.changes).toHaveLength(3); + expect(out.problems).toEqual([]); + expect(out.nextRecord).toBeNull(); + }); + + it('what cannot be converted is left as is, and named', () => { + const out = decide({ + current: { + default: { model: 'gpt-4', effort: 'high' }, + stages: { plan: { model: 'opus-4-8', effort: 'max' } }, + }, + }); + expect(out.next).toEqual({ + stages: { + default: { model: 'gpt-4', effort: 'high' }, + plan: { model: 'opus', effort: 'max' }, + }, + }); + expect(out.problems).toEqual([ + 'stage "default" model "gpt-4" is not an alias {sonnet, opus, haiku, fable, inherit} nor a claude-* id', + ]); + }); + + it('names a default that could not move, before the checker’s REDs', () => { + const out = decide({ + current: { + default: { model: 'opus-4-8', effort: 'max' }, + stages: { default: { model: 'sonnet', effort: 'high' }, x: {} }, + }, + }); + expect(out.problems[0]).toMatch(/^`models.default` was left where it is/); + expect(out.problems[1]).toMatch(/^stage "x" is not a product stage/); + }); + + it('keeps an explicit null as the user’s choice', () => { + expect(decide({ current: null })).toMatchObject({ + outcome: 'kept', + label: 'unrecorded', + next: null, + problems: [], + }); + }); + + it('no block upstream: the user’s block stays, converted if old', () => { + const recorded = modelsRecordHash({ stages: {} }); + expect( + decide({ current: undefined, upstream: { kind: 'absent' }, recorded }), + ).toMatchObject({ + outcome: 'upstream-absent', + next: undefined, + nextRecord: recorded, + conversion: null, + }); + // Not pharn's (it matches no record and no old default): converted, and + // its record — none of its own — is carried. + const edited = { + default: { model: 'haiku-4-5', effort: 'low' }, + stages: { review: { model: 'fable-5', effort: 'max' } }, + }; + const out = decide({ + current: edited, + upstream: { kind: 'absent' }, + recorded, + }); + expect(out.outcome).toBe('upstream-absent'); + expect(out.next).toEqual({ + stages: { + default: { model: 'haiku', effort: 'low' }, + review: { model: 'fable', effort: 'max' }, + }, + }); + expect(out.nextRecord).toBe(recorded); + }); + + // Review finding (REVIEW.md, P5): converting an old default must not erase + // the evidence that pharn wrote it — or once pharn-oss's block is usable + // again, the converted default would be kept as the user's forever. + it('a block that was pharn’s stays pharn’s across the conversion', () => { + const unusable: UpstreamModels[] = [ + { kind: 'absent' }, + { kind: 'invalid', reasons: ['x'] }, + ]; + for (const upstream of unusable) { + const out = decide({ current: DEFAULT_057, upstream, recorded: null }); + const converted = { + stages: { + default: { model: 'sonnet', effort: 'high' }, + plan: { model: 'opus', effort: 'max' }, + review: { model: 'opus', effort: 'high' }, + }, + }; + expect(out.next).toEqual(converted); + expect(out.nextRecord).toBe(modelsRecordHash(converted)); + // …so the next run, with pharn-oss's block back, replaces it. + expect( + decide({ current: out.next, recorded: out.nextRecord }), + ).toMatchObject({ outcome: 'updated', next: UPSTREAM }); + } + // A block pharn recorded writing keeps its record unchanged. + const recorded = modelsRecordHash(UPSTREAM); + expect( + decide({ + current: structuredClone(UPSTREAM), + upstream: { kind: 'absent' }, + recorded, + }).nextRecord, + ).toBe(recorded); + }); + + it('a block upstream this pharn will not apply: named, and yours stays', () => { + const out = decide({ + current: EDITED, + upstream: { + kind: 'invalid', + reasons: ['stage "triage" is not a product stage'], + }, + force: true, + }); + expect(out).toMatchObject({ + outcome: 'upstream-invalid', + next: EDITED, + backup: false, + upstreamReasons: ['stage "triage" is not a product stage'], + }); + }); +}); diff --git a/tests/overwrite-check.test.ts b/tests/overwrite-check.test.ts index 7e1d98f8..2d1635a9 100644 --- a/tests/overwrite-check.test.ts +++ b/tests/overwrite-check.test.ts @@ -18,7 +18,7 @@ const { installCapabilities } = await import('../src/lib/install-capabilities.js'); type Selection = import('../src/types.js').Selection; const { readPharnConfig } = await import('../src/lib/pharn-config.js'); -const { ModelRoutingError } = await import('../src/lib/model-routing.js'); +const { SeamConfigError } = await import('../src/lib/seam-config.js'); const { sha256File } = await import('../src/lib/hash.js'); // ESC built from its code point, so no literal control character lives in this @@ -29,10 +29,10 @@ const ESC = String.fromCharCode(27); // are raw strings so they are exactly what a hand-edit leaves behind. const CONFIG_2_3_4 = '{"skillsVersion":"2.3.4","modules":[]}'; const CONFIG_TRUNCATED = '{"skillsVersion":'; -// readPharnConfig THROWS ModelRoutingError on this one (unknown model id) — the -// exact class of config `init` has to stay able to repair. -const CONFIG_BAD_MODELS = - '{"skillsVersion":"2.3.4","modules":[],"models":{"default":{"model":"gpt-9","effort":"high"}}}'; +// readPharnConfig THROWS SeamConfigError on this one (unknown step) — the exact +// class of config `init` has to stay able to repair. +const CONFIG_BAD_SEAM = + '{"skillsVersion":"2.3.4","modules":[],"seam":{"resolutionOrder":["guess","ask"]}}'; // Valid JSON (stringify escapes the ESC), so JSON.parse SUCCEEDS and it is // VERSION_RE, not the parse guard, that drops the value. const CONFIG_ESCAPED_VERSION = JSON.stringify({ @@ -234,13 +234,13 @@ describe('confirmWriteTargets', () => { it('survives a config readPharnConfig would REJECT — init is the repair command', async () => { const { repo, proj } = dirs(scaffoldRepo); - write(join(proj, 'pharn.config.json'), CONFIG_BAD_MODELS); + write(join(proj, 'pharn.config.json'), CONFIG_BAD_SEAM); // Two-sided on purpose: prove the fixture really IS the dangerous class // before claiming the stage survives it. readPharnConfig lets - // ModelRoutingError PROPAGATE by design (lib/pharn-config.ts), and init has + // SeamConfigError PROPAGATE by design (lib/pharn-config.ts), and init has // no recovery around this stage — so an unguarded read here would make the // one command that repairs a broken config abort on it instead. - expect(() => readPharnConfig(proj)).toThrow(ModelRoutingError); + expect(() => readPharnConfig(proj)).toThrow(SeamConfigError); vi.mocked(prompts.confirm).mockResolvedValue(true); await expect(confirmWriteTargets(repo, proj, selection())).resolves.toBe( 'proceed', diff --git a/tests/pharn-config.test.ts b/tests/pharn-config.test.ts index 2c595b2b..1c8a11ff 100644 --- a/tests/pharn-config.test.ts +++ b/tests/pharn-config.test.ts @@ -30,10 +30,6 @@ import { } from '../src/lib/pharn-config.js'; import { tmpPathFor } from '../src/lib/atomic-write.js'; import type { PharnConfig } from '../src/types.js'; -import { - DEFAULT_MODEL_ROUTING, - ModelRoutingError, -} from '../src/lib/model-routing.js'; import { DEFAULT_SEAM_CONFIG, SeamConfigError, @@ -119,26 +115,39 @@ describe('pharn-config', () => { expect(readPharnConfig(tmp.path())).toBeNull(); }); - it('round-trips a config with a valid models block', async () => { + it("round-trips pharn-oss's models block", async () => { const withModels: PharnConfig = { ...sample, - models: DEFAULT_MODEL_ROUTING, + models: { + stages: { + default: { model: 'sonnet', effort: 'high' }, + plan: { model: 'opus', effort: 'high' }, + }, + }, }; await writePharnConfig(tmp.path(), withModels); expect(readPharnConfig(tmp.path())).toEqual(withModels); }); - it('THROWS (naming the offender), not null, when the models block is invalid (BUG 1)', () => { - writeFileSync( - join(tmp.path(), 'pharn.config.json'), - JSON.stringify({ - skillsVersion: '0.1.0', - modules: [], - models: { default: { model: 'gpt-4', effort: 'high' } }, - }), - ); - expect(() => readPharnConfig(tmp.path())).toThrow(ModelRoutingError); - expect(() => readPharnConfig(tmp.path())).toThrow(/gpt-4/); + // `models` is pharn-oss's schema, not this CLI's: it is carried VERBATIM, so + // the old format (which `update` must load to migrate), a block pharn-oss's + // rules reject, and a format newer than this CLI all load, unvalidated — + // and no command refuses to run over a block it does not own. + it.each([ + [ + 'the format pharn wrote before 0.7.0', + { + default: { model: 'sonnet-5', effort: 'high' }, + stages: { plan: { model: 'opus-4-8', effort: 'max' } }, + }, + ], + ['a block pharn-oss rejects', { default: { model: 'gpt-4', effort: 'x' } }], + ['a newer format', { stages: { triage: {} }, profiles: ['fast'] }], + ['not even an object', 'opus'], + ])('loads %s verbatim, without validating it', (_label, models) => { + const raw = { skillsVersion: '0.1.0', modules: [], models }; + writeFileSync(join(tmp.path(), 'pharn.config.json'), JSON.stringify(raw)); + expect(readPharnConfig(tmp.path())).toEqual(raw); }); it('round-trips a config with a valid seam block', async () => { @@ -569,13 +578,13 @@ describe('loadConfigOrExit', () => { stubProcessExit(); afterEach(() => vi.mocked(log.error).mockClear()); - it('exits(1) with the offender-naming message (NOT "run init") on invalid models (BUG 1)', () => { + it('exits(1) with the offender-naming message (NOT "run init") on an invalid seam block (BUG 1)', () => { writeFileSync( join(tmp.path(), 'pharn.config.json'), JSON.stringify({ skillsVersion: '0.1.0', modules: [], - models: { default: { model: 'gpt-4', effort: 'high' } }, + seam: { resolutionOrder: ['model', 'guess'] }, }), ); expect(() => loadConfigOrExit(tmp.path())).toThrow(ProcessExit); @@ -583,10 +592,21 @@ describe('loadConfigOrExit', () => { .mocked(log.error) .mock.calls.map((c) => String(c[0])) .join('\n'); - expect(msg).toMatch(/gpt-4/); + expect(msg).toMatch(/guess/); expect(msg).not.toMatch(/pharn init/); }); + it('loads a config whose models block pharn-oss would reject — not this CLI’s to refuse', () => { + const raw = { + skillsVersion: '0.1.0', + modules: [], + models: { default: { model: 'gpt-4', effort: 'high' } }, + }; + writeFileSync(join(tmp.path(), 'pharn.config.json'), JSON.stringify(raw)); + expect(loadConfigOrExit(tmp.path())).toEqual(raw); + expect(log.error).not.toHaveBeenCalled(); + }); + // The end-to-end shape of audit finding P-7, at the surface a user sees: a // corrupt config must NOT reach the "run init" branch, because that branch's // advice would overwrite the very file it is complaining about. @@ -635,8 +655,10 @@ describe('loadConfigOrExit', () => { describe('isConfigValidationError', () => { it('is true for the named validator errors, false for a plain Error (the config-vs-bug boundary)', () => { - expect(isConfigValidationError(new ModelRoutingError('x'))).toBe(true); expect(isConfigValidationError(new SeamConfigError('x'))).toBe(true); + expect(isConfigValidationError(new CapabilityEntryError('x'))).toBe(true); + expect(isConfigValidationError(new CapabilitySourceError('x'))).toBe(true); + expect(isConfigValidationError(new ConfigParseError('x'))).toBe(true); expect(isConfigValidationError(new Error('x'))).toBe(false); expect(isConfigValidationError('nope')).toBe(false); }); diff --git a/tests/status.test.ts b/tests/status.test.ts index c3801099..275f0ef3 100644 --- a/tests/status.test.ts +++ b/tests/status.test.ts @@ -505,12 +505,20 @@ describe('runStatus (archetype)', () => { expect(noteBody('DRIFT')).not.toContain('UNREADABLE'); }); - it('MODELS note renders the per-stage routing from config.models', async () => { + // The MODELS note shows pharn-oss's block resolved per product stage, under + // a label that says what it is: Claude Code applies each command's own + // frontmatter, and the block is the source of truth that frontmatter is held + // to. Never presented as routing that happens. + it('MODELS note resolves every product stage, labeled truthfully', async () => { loadArchetypeConfigOrExit.mockReturnValue( config({ + layout: 'pharn', models: { - default: { model: 'sonnet-5', effort: 'high' }, - stages: { review: { model: 'opus-4-8', effort: 'high' } }, + stages: { + default: { model: 'sonnet', effort: 'high' }, + plan: { model: 'opus', effort: 'high' }, + review: { model: 'claude-opus-4-8', effort: 'xhigh' }, + }, }, }), ); @@ -519,12 +527,95 @@ describe('runStatus (archetype)', () => { await runStatus({ drift: false }); const models = noteBody('MODELS'); - expect(models).toContain('default sonnet-5 · high'); - expect(models).toContain('review opus-4-8 · high'); - // The note reports what the config RECORDS, and says so: no installed - // command reads models.stages yet, so a reader must not take these lines - // as the model a stage will actually run. - expect(models).toContain('no installed stage reads this yet'); + expect(models).toContain('default sonnet · high'); + expect(models).toContain('plan opus · high'); + expect(models).toContain('review claude-opus-4-8 · xhigh'); + // A stage with no entry of its own shows the default it resolves to. + expect(models).toContain('memory-promote sonnet · high (default)'); + expect(models).toContain('ac-test sonnet · high (default)'); + expect(models).toContain( + "Claude Code applies each /pharn-* command's own model:/effort:", + ); + expect(models).toContain( + 'frontmatter, not this block. The block is the source of truth', + ); + expect(models).toContain( + 'node pharn/floor/check-model-config.mjs agreement', + ); + expect(models).not.toMatch(/rout/i); + expect(models).not.toContain('no installed stage reads'); + }); + + it('points a flat install at the checker under .dev/floor', async () => { + loadArchetypeConfigOrExit.mockReturnValue( + config({ + models: { stages: { default: { model: 'opus', effort: 'low' } } }, + }), + ); + fetchRemoteSkillsVersion.mockResolvedValue('1.0.0'); + await runStatus({ drift: false }); + expect(noteBody('MODELS')).toContain( + 'node .dev/floor/check-model-config.mjs agreement', + ); + }); + + it('says a block in the old pharn format is converted by `pharn update`', async () => { + loadArchetypeConfigOrExit.mockReturnValue( + config({ + models: { + default: { model: 'sonnet-5', effort: 'high' }, + stages: { + plan: { model: 'opus-4-8', effort: 'max' }, + review: { model: 'opus-4-8', effort: 'high' }, + }, + }, + }), + ); + fetchRemoteSkillsVersion.mockResolvedValue('1.0.0'); + await runStatus({ drift: false }); + const models = noteBody('MODELS'); + expect(models).toContain('In the format pharn wrote before 0.7.0'); + expect(models).toContain('`pharn update` converts it'); + for (const line of models.split('\n')) { + expect(line.length).toBeLessThanOrEqual(70); + } + expect(models).not.toContain('sonnet-5 · high'); + }); + + it("lists what pharn-oss's rules reject — terminal-safe — and does not fail --strict", async () => { + const ESC = String.fromCharCode(27); + const RLO = String.fromCharCode(0x202e); + loadArchetypeConfigOrExit.mockReturnValue( + config({ + models: { + stages: { + default: { model: 'sonnet', effort: 'high' }, + [`pl${ESC}[2Kan`]: { model: 'opus', effort: 'high' }, + review: { model: `${RLO}opus`, effort: 'high' }, + }, + }, + }), + ); + fetchRemoteSkillsVersion.mockResolvedValue('1.0.0'); + + // --strict gates on the install, not on this block: pharn-oss's checker + // owns that verdict. Current version + no drift checked → no exit. + await runStatus({ strict: true, drift: false }); + + const models = noteBody('MODELS'); + expect(models).toContain("pharn-oss's rules reject this block:"); + expect(models).toContain('is not a product stage'); + expect(models).toContain('is not an alias'); + expect(models).toContain('node .dev/floor/check-model-config.mjs validate'); + expect(models).not.toContain(RLO); + expect(models).not.toContain(`${ESC}[2K`); + }); + + it('says so when the block declares no stages', async () => { + loadArchetypeConfigOrExit.mockReturnValue(config({ models: {} })); + fetchRemoteSkillsVersion.mockResolvedValue('1.0.0'); + await runStatus({ drift: false }); + expect(noteBody('MODELS')).toContain('Declares no stages'); }); it('omits the MODELS note when config.models is absent (legacy archetype config)', async () => { diff --git a/tests/update.test.ts b/tests/update.test.ts index 32d66114..b63edaec 100644 --- a/tests/update.test.ts +++ b/tests/update.test.ts @@ -82,11 +82,17 @@ vi.mock('../src/lib/pharn-config.js', async () => { const { runUpdate } = await import('../src/commands/update.js'); const prompts = await import('@clack/prompts'); -const { readRecords, writeRecords, RECORDS_FILE } = - await import('../src/lib/install-records.js'); +const { + MODELS_RECORD_KEY, + modelsRecordHash, + readRecords, + writeRecords, + RECORDS_FILE, +} = await import('../src/lib/install-records.js'); const { sha256File } = await import('../src/lib/hash.js'); const { BACKUP_DIR } = await import('../src/lib/backup.js'); -const { readPharnConfig } = await import('../src/lib/pharn-config.js'); +const { readPharnConfig, writePharnConfig } = + await import('../src/lib/pharn-config.js'); // --------------------------------------------------------------------------- // Real-filesystem fixture: a fake clone + a real project root. The command's @@ -2261,6 +2267,376 @@ describe('runUpdate (drift-safe)', () => { expect(cleanup).toHaveBeenCalled(); }); + // The `models` block is pharn-oss's. `update` moves a block pharn wrote to + // pharn-oss's current one and keeps one the user edited — the per-file rows, + // over the block (lib/models-update.ts) — and converts the format pharn + // wrote before 0.7.0. + describe('the models block', () => { + const UPSTREAM = { + stages: { + default: { model: 'sonnet', effort: 'high' }, + plan: { model: 'opus', effort: 'high' }, + review: { model: 'opus', effort: 'high' }, + }, + }; + // The default every published pharn through 0.6.0 wrote. + const OLD_DEFAULT = { + default: { model: 'sonnet-5', effort: 'high' }, + stages: { + plan: { model: 'opus-4-8', effort: 'max' }, + review: { model: 'opus-4-8', effort: 'high' }, + }, + }; + const models = () => readPharnConfig(proj)!.models; + const needsConversionAfter = () => + JSON.stringify(models()).includes('opus-4-8'); + const modelsNote = () => + (vi + .mocked(prompts.note) + .mock.calls.find((c) => c[1] === 'MODELS')?.[0] as + string | undefined) ?? ''; + // The config on disk too: `--force` backs it up, and the position test + // reads it raw. + async function installedWith( + block: unknown, + recorded?: unknown, + ): Promise { + const config = await installed( + block === undefined ? {} : { models: block }, + ); + await writePharnConfig(proj, config); + if (recorded !== undefined) { + const read = readRecords(proj); + if (read.kind !== 'ok') throw new Error('no records'); + await writeRecords(proj, { + skillsVersion: config.skillsVersion, + commit: config.commit, + files: { + ...read.store.files, + [MODELS_RECORD_KEY]: modelsRecordHash(recorded), + }, + }); + } + return config; + } + + beforeEach(() => { + write( + join(repo, 'pharn.config.json'), + JSON.stringify({ models: UPSTREAM, ship: {} }), + ); + }); + + it("replaces the default an earlier pharn wrote with pharn-oss's block", async () => { + await installedWith(OLD_DEFAULT); + + await runUpdate(); + + expect(models()).toEqual(UPSTREAM); + expect(records()?.[MODELS_RECORD_KEY]).toBe(modelsRecordHash(UPSTREAM)); + expect(modelsNote()).toContain( + "Replaced the models block an earlier pharn wrote with pharn-oss's.", + ); + expect(modelsNote()).toContain( + 'node .dev/floor/check-model-config.mjs agreement', + ); + }); + + it('converts an edited block in the old format and keeps its values — and the version still advances', async () => { + await installedWith({ + default: { model: 'haiku-4-5', effort: 'low' }, + stages: { review: { model: 'fable-5', effort: 'max' } }, + }); + + await runUpdate(); + + expect(models()).toEqual({ + stages: { + default: { model: 'haiku', effort: 'low' }, + review: { model: 'fable', effort: 'max' }, + }, + }); + // Converted by pharn, still the user's: no record claims it. + expect(records()?.[MODELS_RECORD_KEY]).toBeUndefined(); + const note = modelsNote(); + expect(note).toContain( + 'Kept your models block — pharn has no record of writing it.', + ); + expect(note).toContain( + 'Converted your models block from the format pharn wrote before 0.7.0,', + ); + expect(note).toContain('stage "review" model "fable-5" → "fable"'); + expect(note).toContain("--force replaces it with pharn-oss's"); + // The note's own lines (not the quoted, indented details) stay within + // 70 columns: an 80-column note box never wraps one mid-sentence. + for (const line of note.split('\n')) { + if (!line.startsWith(' ')) expect(line.length).toBeLessThanOrEqual(70); + } + // A kept block is configuration, not unfinished work. + expect(readPharnConfig(proj)!.skillsVersion).toBe('1.1.0'); + }); + + it('names what cannot be converted and leaves it as it is', async () => { + await installedWith({ + default: { model: 'gpt-4', effort: 'high' }, + stages: { plan: { model: 'opus-4-8', effort: 'max' } }, + }); + + await runUpdate(); + + expect(models()).toEqual({ + stages: { + default: { model: 'gpt-4', effort: 'high' }, + plan: { model: 'opus', effort: 'max' }, + }, + }); + expect(modelsNote()).toContain( + "pharn-oss's rules still reject your models block — left as is,", + ); + expect(modelsNote()).toContain('stage "default" model "gpt-4"'); + }); + + // The conversion is this CLI's job, so it must reach an install that is + // already current — and must not keep re-opening the gate once done. + it('migrates at the same skills version, then is up to date', async () => { + fetchRemoteSkillsVersion.mockResolvedValue('1.0.0'); + write(join(repo, 'SKILLS_VERSION'), '1.0.0\n'); + await installedWith(OLD_DEFAULT); + + await runUpdate(); + + expect(fetchRepo).toHaveBeenCalledTimes(1); + expect(models()).toEqual(UPSTREAM); + // The confirm note said why a current install is being re-applied. + expect(String(vi.mocked(prompts.note).mock.calls[0]?.[0])).toContain( + 'Your models block is in the format pharn wrote before 0.7.0 — this run converts it.', + ); + + loadArchetypeConfigOrExit.mockReturnValue(readPharnConfig(proj)); + vi.mocked(prompts.outro).mockClear(); + await runUpdate(); + + expect(fetchRepo).toHaveBeenCalledTimes(1); + expect(vi.mocked(prompts.outro).mock.calls.at(-1)?.[0]).toBe( + 'Already up to date (skills v1.0.0).', + ); + }); + + // Review finding (REVIEW.md, P5): the gate must close even when what update + // wrote is itself convertible — here a block pharn-oss might ship with a + // top-level `default`, which its checker ignores. + it('closes the gate once it has written the block, even a convertible one', async () => { + const shaped = { + default: { model: 'sonnet', effort: 'high' }, + stages: UPSTREAM.stages, + }; + write( + join(repo, 'pharn.config.json'), + JSON.stringify({ models: shaped }), + ); + fetchRemoteSkillsVersion.mockResolvedValue('1.0.0'); + write(join(repo, 'SKILLS_VERSION'), '1.0.0\n'); + await installedWith(OLD_DEFAULT); + + await runUpdate(); + expect(models()).toEqual(shaped); + + loadArchetypeConfigOrExit.mockReturnValue(readPharnConfig(proj)); + await runUpdate(); + + expect(fetchRepo).toHaveBeenCalledTimes(1); + expect(vi.mocked(prompts.outro).mock.calls.at(-1)?.[0]).toBe( + 'Already up to date (skills v1.0.0).', + ); + }); + + // Review finding (REVIEW.md, P5), end to end: an old default converted + // while pharn-oss's block was unusable is still pharn's afterwards. + it('replaces an old default it had to convert, once pharn-oss’s block is usable', async () => { + write( + join(repo, 'pharn.config.json'), + JSON.stringify({ models: { stages: { triage: {} } } }), + ); + await installedWith(OLD_DEFAULT); + + await runUpdate(); + expect(needsConversionAfter()).toBe(false); + + // pharn-oss fixes its block in its next release. + write( + join(repo, 'pharn.config.json'), + JSON.stringify({ models: UPSTREAM }), + ); + write(join(repo, 'SKILLS_VERSION'), '1.2.0\n'); + fetchRemoteSkillsVersion.mockResolvedValue('1.2.0'); + loadArchetypeConfigOrExit.mockReturnValue(readPharnConfig(proj)); + vi.mocked(prompts.note).mockClear(); + await runUpdate(); + + expect(models()).toEqual(UPSTREAM); + expect(modelsNote()).toContain( + "Moved your models block to pharn-oss's current one: you had", + ); + }); + + it("restores a missing block with pharn-oss's", async () => { + await installedWith(undefined); + + await runUpdate(); + + expect(models()).toEqual(UPSTREAM); + expect(records()?.[MODELS_RECORD_KEY]).toBe(modelsRecordHash(UPSTREAM)); + expect(modelsNote()).toContain( + "Your config had no models block — wrote pharn-oss's.", + ); + }); + + it('moves a block still at its recorded hash to the current one', async () => { + const previous = { + stages: { default: { model: 'opus', effort: 'low' } }, + }; + await installedWith(previous, previous); + + await runUpdate(); + + expect(models()).toEqual(UPSTREAM); + expect(modelsNote()).toContain( + "Moved your models block to pharn-oss's current one: you had", + ); + }); + + it('keeps a block the user changed, in place, with its record carried', async () => { + const edited = { + stages: { + default: { model: 'haiku', effort: 'low' }, + review: { model: 'fable', effort: 'max' }, + }, + }; + await installedWith(edited, { stages: {} }); + const keysBefore = Object.keys( + JSON.parse(readFileSync(join(proj, 'pharn.config.json'), 'utf8')), + ); + + await runUpdate(); + + expect(models()).toEqual(edited); + expect(records()?.[MODELS_RECORD_KEY]).toBe( + modelsRecordHash({ stages: {} }), + ); + expect(modelsNote()).toContain( + 'Kept your models block — you changed it since pharn wrote it.', + ); + // The key keeps its place in the file. + const keysAfter = Object.keys( + JSON.parse(readFileSync(join(proj, 'pharn.config.json'), 'utf8')), + ); + expect(keysAfter.indexOf('models')).toBe(keysBefore.indexOf('models')); + }); + + it('--force replaces a block the user changed, after backing up the config', async () => { + const edited = { + stages: { default: { model: 'haiku', effort: 'low' } }, + }; + await installedWith(edited, { stages: {} }); + + await runUpdate({ force: true }); + + expect(models()).toEqual(UPSTREAM); + const dirs = backupDirs(); + expect(dirs).toHaveLength(1); + const saved = JSON.parse( + readFileSync( + join(proj, BACKUP_DIR, dirs[0]!, 'pharn.config.json'), + 'utf8', + ), + ) as { models: unknown }; + expect(saved.models).toEqual(edited); + expect(modelsNote()).toContain( + "Replaced your models block with pharn-oss's (--force).", + ); + expect(modelsNote()).toContain(`${BACKUP_DIR}/${dirs[0]!}`); + }); + + it("says nothing about a block that is already pharn-oss's", async () => { + await installedWith(structuredClone(UPSTREAM)); + + await runUpdate(); + + expect(modelsNote()).toBe(''); + expect(records()?.[MODELS_RECORD_KEY]).toBe(modelsRecordHash(UPSTREAM)); + }); + + it('with no usable records file, keeps a block it cannot prove is pharn’s', async () => { + const edited = { stages: { default: { model: 'haiku', effort: 'low' } } }; + await installedWith(edited); + rmSync(join(proj, RECORDS_FILE)); + + await runUpdate(); + + expect(models()).toEqual(edited); + expect(modelsNote()).toContain( + `Kept your models block — no usable ${RECORDS_FILE} to check it.`, + ); + }); + + it('when pharn-oss ships no block, still converts yours from the old format', async () => { + rmSync(join(repo, 'pharn.config.json')); + await installedWith(OLD_DEFAULT); + + await runUpdate(); + + expect(models()).toEqual({ + stages: { + default: { model: 'sonnet', effort: 'high' }, + plan: { model: 'opus', effort: 'max' }, + review: { model: 'opus', effort: 'high' }, + }, + }); + const note = modelsNote(); + expect(note).toContain( + 'pharn-oss ships no models block, so yours is kept.', + ); + expect(note).toContain('Converted your models block'); + }); + + it('when pharn-oss ships no block and yours needs nothing, says nothing', async () => { + rmSync(join(repo, 'pharn.config.json')); + await installedWith(structuredClone(UPSTREAM)); + + await runUpdate(); + + expect(models()).toEqual(UPSTREAM); + expect(modelsNote()).toBe(''); + }); + + it("does not apply a block pharn-oss's rules reject, and names why", async () => { + write( + join(repo, 'pharn.config.json'), + JSON.stringify({ + models: { stages: { default: UPSTREAM.stages.default, triage: {} } }, + }), + ); + await installedWith(OLD_DEFAULT); + + await runUpdate(); + + // Yours stays — converted, since the old format is rejected regardless. + expect(models()).toEqual({ + stages: { + default: { model: 'sonnet', effort: 'high' }, + plan: { model: 'opus', effort: 'max' }, + review: { model: 'opus', effort: 'high' }, + }, + }); + const note = modelsNote(); + expect(note).toContain( + "pharn-oss's models block was not applied; this pharn rejects it:", + ); + expect(note).toContain('stage "triage" is not a product stage'); + expect(note).toContain('upgrade pharn'); + }); + }); + describe('layout migration (the (d) fix)', () => { // A project recorded `flat` meeting a `pharn`-layout clone: the copy has // always landed at the clone's paths, but the config used to keep saying diff --git a/tests/upstream-models.test.ts b/tests/upstream-models.test.ts new file mode 100644 index 00000000..0fa01a53 --- /dev/null +++ b/tests/upstream-models.test.ts @@ -0,0 +1,142 @@ +import { mkdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { + MAX_UPSTREAM_CONFIG_BYTES, + readUpstreamModels, +} from '../src/lib/upstream-models.js'; +import { useTmpDir } from './helpers.js'; + +// pharn-oss's own models block, read out of a fetched clone's ROOT +// pharn.config.json. Its verdicts against pharn-oss's checker are pinned in +// tests/model-config-parity.test.ts; this file pins what the reader returns. + +describe('readUpstreamModels', () => { + const tmp = useTmpDir(); + const clone = (text?: string): string => { + const repo = join(tmp.path(), 'clone'); + mkdirSync(repo, { recursive: true }); + if (text !== undefined) + writeFileSync(join(repo, 'pharn.config.json'), text); + return repo; + }; + const block = { + stages: { + default: { model: 'sonnet', effort: 'high' }, + plan: { model: 'opus', effort: 'high', note: 'kept verbatim' }, + }, + comment: 'a key beside stages, kept verbatim', + }; + + it('is absent when the clone has no root config', () => { + expect(readUpstreamModels(clone())).toEqual({ kind: 'absent' }); + }); + + it('is absent when the config has no models block, or a null one', () => { + expect(readUpstreamModels(clone('{"ship":{}}'))).toEqual({ + kind: 'absent', + }); + expect(readUpstreamModels(clone('{"models":null}'))).toEqual({ + kind: 'absent', + }); + }); + + it('returns the block VERBATIM — other keys included — and nothing beside it', () => { + const read = readUpstreamModels( + clone( + JSON.stringify({ + _models_stages_note: 'pharn-oss explains the block here', + models: block, + ship: { requireAttestation: false }, + }), + ), + ); + expect(read).toEqual({ kind: 'ok', block }); + }); + + it('copies a block that declares no stages, as pharn-oss ships it', () => { + expect(readUpstreamModels(clone('{"models":{}}'))).toEqual({ + kind: 'ok', + block: {}, + }); + }); + + it('refuses a block pharn-oss’s rules reject, with every reason', () => { + const read = readUpstreamModels( + clone( + '{"models":{"stages":{"plan":{"model":"opus-4-8","effort":"max"}}}}', + ), + ); + expect(read).toEqual({ + kind: 'invalid', + reasons: [ + 'missing required `default` stage entry (the resolution fallback)', + 'stage "plan" model "opus-4-8" is not an alias {sonnet, opus, haiku, fable, inherit} nor a claude-* id', + ], + }); + }); + + it.each([ + [ + 'not JSON', + '{"models":', + "pharn-oss's pharn.config.json is not valid JSON", + ], + [ + 'a JSON array', + '[]', + "pharn-oss's pharn.config.json is valid JSON but is not an object", + ], + [ + 'JSON null', + 'null', + "pharn-oss's pharn.config.json is valid JSON but is not an object", + ], + ])('refuses a root config that is %s', (_label, text, reason) => { + expect(readUpstreamModels(clone(text))).toEqual({ + kind: 'invalid', + reasons: [reason], + }); + }); + + it('never echoes the parser’s message, which can quote raw bytes', () => { + const read = readUpstreamModels(clone('\u001b[31mRED')); + expect(read).toEqual({ + kind: 'invalid', + reasons: ["pharn-oss's pharn.config.json is not valid JSON"], + }); + }); + + // Review finding (REVIEW.md, P2): the checker ignores keys it does not + // read, so a block can pass while nesting deeper than JSON.stringify can go + // — which used to throw during an install, after the files were copied. + it('refuses a block nested too deeply to serialize — before anything copies it', () => { + const depth = 100_000; + const read = readUpstreamModels( + clone( + `{"models":{"stages":{"default":{"model":"opus","effort":"high"}},"deep":${'['.repeat(depth)}${']'.repeat(depth)}}}`, + ), + ); + expect(read).toEqual({ + kind: 'invalid', + reasons: ["pharn-oss's models block is nested too deeply to copy"], + }); + }); + + it('refuses a directory at the path', () => { + const repo = join(tmp.path(), 'clone'); + mkdirSync(join(repo, 'pharn.config.json'), { recursive: true }); + expect(readUpstreamModels(repo)).toEqual({ + kind: 'invalid', + reasons: ["pharn-oss's pharn.config.json is not a regular file"], + }); + }); + + it('refuses a root config larger than its cap, unread', () => { + const big = `{"models":${JSON.stringify(block)},"pad":"${'x'.repeat(MAX_UPSTREAM_CONFIG_BYTES)}"}`; + expect(readUpstreamModels(clone(big))).toEqual({ + kind: 'invalid', + reasons: ["pharn-oss's pharn.config.json is larger than 1 MiB"], + }); + }); +}); From 01e818fc3366f2f75fdbad416dd9d4e9b291560a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Galarowicz?= Date: Sat, 26 Sep 2026 00:41:28 +0200 Subject: [PATCH 2/2] =?UTF-8?q?test(models):=20measure=20note=20width=20in?= =?UTF-8?q?=20visible=20columns=20=E2=80=94=20CI=20turns=20color=20on?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit picocolors enables color when CI is set, so pc.dim's escape codes counted toward the 70-column bound and a 62-column line read as 71. Co-Authored-By: Claude Opus 5.5 --- .dev/features/models-pharn-oss-owned/SHIP.md | 7 +++++++ .pharn/writes-scope.json | 2 +- tests/status.test.ts | 4 +++- tests/update.test.ts | 6 ++++-- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/.dev/features/models-pharn-oss-owned/SHIP.md b/.dev/features/models-pharn-oss-owned/SHIP.md index f6c22ce5..a57cfa2d 100644 --- a/.dev/features/models-pharn-oss-owned/SHIP.md +++ b/.dev/features/models-pharn-oss-owned/SHIP.md @@ -41,6 +41,13 @@ under that delegation, never as human approvals"). Every decision below was made `.regressions[]`, `.pre_existing[]` and `.failing_gates[]` are empty; `verifiers.registered` is `0`. `/pharn-dev-review` has no structural verdict and none was invented; it is read at GATE 2. +## After the PR opened + +CI's `Test` job failed once: two new tests measured a note's line width with the color codes CI turns +on (picocolors reads `CI`), so a 62-column line counted as 71. The product was unaffected; the tests +now measure visible columns (`stripVTControlCharacters`). The whole suite was re-run locally under +`CI=1` (1818 passed) with `npm run check` and the coverage ratchet green, then pushed. + ## Stage models The maintainer asked that each stage run on the model this repo's `pharn.config.json` `models` block diff --git a/.pharn/writes-scope.json b/.pharn/writes-scope.json index fceff001..8d83bba4 100644 --- a/.pharn/writes-scope.json +++ b/.pharn/writes-scope.json @@ -3,5 +3,5 @@ ".dev/features/models-pharn-oss-owned/SHIP.md" ], "set_by": ".claude/commands/pharn-dev-ship.md", - "set_at": "2026-09-25T22:37:12.910Z" + "set_at": "2026-09-25T22:41:27.905Z" } diff --git a/tests/status.test.ts b/tests/status.test.ts index 275f0ef3..4c6b53ab 100644 --- a/tests/status.test.ts +++ b/tests/status.test.ts @@ -1,5 +1,6 @@ import { mkdirSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; +import { stripVTControlCharacters } from 'node:util'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { ProcessExit, stubProcessExit, useTmpDir } from './helpers.js'; import type { CapabilityIndex, PharnConfig } from '../src/types.js'; @@ -576,7 +577,8 @@ describe('runStatus (archetype)', () => { const models = noteBody('MODELS'); expect(models).toContain('In the format pharn wrote before 0.7.0'); expect(models).toContain('`pharn update` converts it'); - for (const line of models.split('\n')) { + // Visible columns: CI turns color codes on (picocolors reads CI). + for (const line of stripVTControlCharacters(models).split('\n')) { expect(line.length).toBeLessThanOrEqual(70); } expect(models).not.toContain('sonnet-5 · high'); diff --git a/tests/update.test.ts b/tests/update.test.ts index b63edaec..98cbb243 100644 --- a/tests/update.test.ts +++ b/tests/update.test.ts @@ -11,6 +11,7 @@ import { import { hostname } from 'node:os'; import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { stripVTControlCharacters } from 'node:util'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { CANCEL, @@ -2368,8 +2369,9 @@ describe('runUpdate (drift-safe)', () => { expect(note).toContain('stage "review" model "fable-5" → "fable"'); expect(note).toContain("--force replaces it with pharn-oss's"); // The note's own lines (not the quoted, indented details) stay within - // 70 columns: an 80-column note box never wraps one mid-sentence. - for (const line of note.split('\n')) { + // 70 VISIBLE columns: an 80-column note box never wraps one mid-sentence. + // Measured without color codes, which CI turns on (picocolors reads CI). + for (const line of stripVTControlCharacters(note).split('\n')) { if (!line.startsWith(' ')) expect(line.length).toBeLessThanOrEqual(70); } // A kept block is configuration, not unfinished work.