From 7af87c0bd8b38c3e63000ad28b67a80100fa569c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 08:22:50 +0000 Subject: [PATCH] fix(add): accept the version a user-edit-withheld update applied (PHARN-05) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One kept local edit dead-ended `pharn add`: `update` withholds the skillsVersion bump while any file is skipped, so a user keeping an edit could never finish the upgrade, and `add`'s version gate kept sending them back to `pharn update` in a loop. When `update` withholds the bump SOLELY because of `modified`/`unrecorded` skips (the user's own edits — every other file is at the new version), it now records `pendingSkillsVersion` (additive, VERSION_RE-validated, cleared by the next complete run). `add` accepts a clone at that version while keeping the config's own (skillsVersion, commit) pair, so it never claims an upgrade the kept edits did not get and the records stamp stays consistent. Other skip kinds still refuse, and the refusal now names the real ways out. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TvcuVhk8hTeDskp5pAJhnc --- .../add-after-withheld-update/GRILL.md | 33 ++++++++ .../add-after-withheld-update/PLAN.md | 75 +++++++++++++++++++ .../add-after-withheld-update/REGRESSION.md | 30 ++++++++ .../add-after-withheld-update/REVIEW.md | 46 ++++++++++++ .../add-after-withheld-update/SHIP.md | 17 +++++ .../add-after-withheld-update/VERIFY.md | 27 +++++++ .../regression-report.json | 28 +++++++ .../verify-report.json | 17 +++++ .pharn/writes-scope.json | 4 +- CLAUDE.md | 2 +- docs/commands/add.md | 7 ++ docs/reference/pharn-config.md | 4 + src/commands/add.ts | 55 +++++++++++--- src/commands/update.ts | 20 ++++- src/lib/pharn-config.ts | 11 +++ src/types.ts | 5 ++ tests/add.test.ts | 39 ++++++++++ tests/pharn-config.test.ts | 37 +++++++++ tests/update.test.ts | 36 +++++++++ 19 files changed, 479 insertions(+), 14 deletions(-) create mode 100644 .dev/features/add-after-withheld-update/GRILL.md create mode 100644 .dev/features/add-after-withheld-update/PLAN.md create mode 100644 .dev/features/add-after-withheld-update/REGRESSION.md create mode 100644 .dev/features/add-after-withheld-update/REVIEW.md create mode 100644 .dev/features/add-after-withheld-update/SHIP.md create mode 100644 .dev/features/add-after-withheld-update/VERIFY.md create mode 100644 .dev/features/add-after-withheld-update/regression-report.json create mode 100644 .dev/features/add-after-withheld-update/verify-report.json diff --git a/.dev/features/add-after-withheld-update/GRILL.md b/.dev/features/add-after-withheld-update/GRILL.md new file mode 100644 index 0000000..f3e186c --- /dev/null +++ b/.dev/features/add-after-withheld-update/GRILL.md @@ -0,0 +1,33 @@ +# GRILL — add-after-withheld-update + +Plan: `.dev/features/add-after-withheld-update/PLAN.md` · spec-hash `bca940a5…d729d3c4e` matches live +`ARCHITECTURE.md`. Registered grillers: `{"registered":0,"grillers":[]}` → inline axes only. + +## Findings + +```yaml +- type: FINDING + rule_id: 'P0' + severity: important + file: '.dev/features/add-after-withheld-update/PLAN.md:12' + problem: "`add` also merges records stamped with the config pair and refreshes `commit`; at a pending version the clone's commit differs from the recorded one. The plan must confirm that `add` writing the clone's commit does not invalidate the records stamp the withheld update left (stamp = config's old skillsVersion/commit)." + evidence: "`add`'s version gate accepts a clone at `skillsVersion` OR `pendingSkillsVersion`" +- type: FINDING + rule_id: 'P5' + severity: minor + file: '.dev/features/add-after-withheld-update/PLAN.md:33' + problem: "The `unreadable` label is not in FORCEABLE_SKIPS but is a skip label; the set test must use exactly {modified, unrecorded}, not 'forceable'." + evidence: 'whose skip labels are all in `{modified, unrecorded}`' +- type: FINDING + rule_id: 'P7' + severity: minor + file: '.dev/features/add-after-withheld-update/PLAN.md:31' + problem: "A new config field must round-trip through every writer that spreads `...config` (add/remove) and be dropped by init's fresh config; confirm no writer re-serializes a stale pending value after a complete update." + evidence: '`PharnConfig.pendingSkillsVersion?: string` (additive, P7)' +``` + +## Summary + +The key risk is the `commit`/records-stamp interaction in `add` at a pending version — verify in build. + +ADVISORY VERDICT: 3 concerns raised (0 blocking-severity, 3 advisory) — for the human to weigh before /pharn-dev-build. diff --git a/.dev/features/add-after-withheld-update/PLAN.md b/.dev/features/add-after-withheld-update/PLAN.md new file mode 100644 index 0000000..8ac7fd8 --- /dev/null +++ b/.dev/features/add-after-withheld-update/PLAN.md @@ -0,0 +1,75 @@ +# PLAN — add-after-withheld-update (PHARN-05: one kept local edit must not dead-end `pharn add`) + +- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4 +- increment: when `update` withholds the version bump ONLY because of `modified` / `unrecorded` skips + (the user's own kept edits), it records the version it DID apply as an additive + `pendingSkillsVersion` in `pharn.config.json` (cleared by the next complete run); `add`'s version + gate accepts a clone at `skillsVersion` OR `pendingSkillsVersion`. When the gate still refuses, the + message names the real ways out instead of looping on "run `pharn update`". +- layer(s): the CLI itself (`src/types.ts`, `src/lib/pharn-config.ts`, `src/commands/{update,add}.ts`) +- constitution_refs: [P0, P1, P4, P5, P7] + +## Discovery — verified this run (P6) + +Reproduced (`repro/add-deadlock`): `init` 6.11.1 → one local edit in `.claude/commands/pharn-plan.md` → +`update --yes` to 6.17.1 twice, each "skipped 1", config stays 6.11.1 → `pharn add a11y` exit 1 "Skills +version mismatch … run `pharn update` first". Loop: `update` can never finish while the edit is kept. +Code: `update.ts` `versionWithheld = plan.counts.skipped > 0` keeps `skillsVersion`/`commit`; +`add.ts` `versionGate` compares `readSkillsVersion(clone) !== config.skillsVersion`; +`docs/commands/add.md` "`pharn update` is the only resolution". + +Why restricting to `modified`/`unrecorded`: those are files the user owns and chose to keep — everything +else was upgraded, so the tree IS at the new version apart from them. `unverifiable` (no usable +records baseline → every present differing file skipped) and `unreadable` can leave most of the tree at +the old version, so they do NOT set `pendingSkillsVersion` and the gate keeps refusing. + +## Files + +- `src/types.ts` — `PharnConfig.pendingSkillsVersion?: string` (additive, P7) — layer CLI +- `src/lib/pharn-config.ts` — ingest: a present `pendingSkillsVersion` failing `VERSION_RE` is dropped + (like a garbage `layout`), so a hand-edit can only fail closed — layer CLI/lib +- `src/commands/update.ts` — on a withheld run whose skip labels are all in `{modified, unrecorded}`, + write `pendingSkillsVersion: installedVersion`; on a complete run, delete the field; any other + withheld run leaves it absent — layer CLI/command +- `src/commands/add.ts` — `versionGate` passes when the clone's version equals `skillsVersion` or + `pendingSkillsVersion`; its refusal names `pharn update --force` (backs up edits) and reverting the + edits as the ways out when a pending version exists but upstream moved again — layer CLI/command +- `tests/update.test.ts` — withheld-by-`modified` → `pendingSkillsVersion` set; withheld-by- + `unverifiable` → not set; complete run → cleared +- `tests/add.test.ts` — clone at pending version → add proceeds and never touches `skillsVersion`; + clone at a third version → refused with the new message +- `tests/pharn-config.test.ts` — valid pending round-trips; garbage pending dropped; absent is legal +- `docs/commands/add.md` — "Version mismatch" section: the kept-edits case now works (P4) +- `docs/reference/pharn-config.md` — document `pendingSkillsVersion` (P4) +- `CLAUDE.md` — add/update paragraphs (P4) + +## Contracts satisfied + +- CLAUDE.md "`add` must never stamp a newer `skillsVersion` over unchanged old bytes" — preserved: `add` + still never writes `skillsVersion`, and `pendingSkillsVersion` exists only when every non-user file + was upgraded. + +## Evals to write (P1) + +- per test file above; the add-at-pending and update-sets-pending cases fail on the base source. + +## Guarantee audit (P0) + +- "`add` installs only at a version the project's non-user-owned files are at" → floor: exact version + string membership in `{skillsVersion, pendingSkillsVersion}`, where `pendingSkillsVersion` is written + only when every skip label ∈ `{modified, unrecorded}` (enum membership). +- A hand-edited `pendingSkillsVersion` is validated by `VERSION_RE`; a well-formed forged one could let + `add` install at that version — the same trust level as a hand-edited `skillsVersion` today (advisory, + named). + +## Trust audit (P2) + +- No new remote input; the new config field is local, hand-editable, regex-validated at ingest. + +## Determinism audit (P5) + +- Enum/string membership only; terminal = the existing named refusal. + +## Open questions (HALT) + +- none diff --git a/.dev/features/add-after-withheld-update/REGRESSION.md b/.dev/features/add-after-withheld-update/REGRESSION.md new file mode 100644 index 0000000..2289e2c --- /dev/null +++ b/.dev/features/add-after-withheld-update/REGRESSION.md @@ -0,0 +1,30 @@ +# REGRESSION — add-after-withheld-update + +The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment. + +## Base and partition + +- **base:** `a6c55bc7400903236b1f1ee36432a9d74b15e1d0` (`origin/main` at build time; the build is an uncommitted working tree on top of it). +- **inside** (each declared in `PLAN.md` `## Files`): `CLAUDE.md`, `docs/commands/add.md`, `docs/reference/pharn-config.md`, `src/commands/add.ts`, `src/commands/update.ts`, `src/lib/pharn-config.ts`, `src/types.ts`, `tests/add.test.ts`, `tests/pharn-config.test.ts`, `tests/update.test.ts`. +- **scope partition:** `check-regress.mjs scope` exited **0**, `escaped: []`. `.pharn/` (hook scratch) and this + feature's own stage artifacts are not build output. +- **outside gates:** the stdlib `*.test.mjs` / `*.test.cjs` files + whole-repo `validate`; 0 committed eval pairs. +- **style-gate skip:** `inside` touches no shared style config, so `lint` / `format:check` / `lint:md` are absent from both maps. + +## Per-gate exit codes + +| gate | base | head | flipped? | +| ---------- | ---- | ---- | -------- | +| `tests` | 0 | 0 | no | +| `validate` | 0 | 0 | no | + +- `regressions[]`: **empty** +- `pre_existing[]`: **empty** + +## Verdict + +**REGRESSIONS: none — no deterministically-detectable breakage outside the feature.** +(`regression-report.json` `.verdict` = `no-regressions`.) + +Residual (P0/P7): this catches exactly what its suite catches. The vitest suite exercising `src/**` is +owned by `/pharn-dev-build`'s floor and `/pharn-dev-verify`. This certifies the comparison, never the increment. diff --git a/.dev/features/add-after-withheld-update/REVIEW.md b/.dev/features/add-after-withheld-update/REVIEW.md new file mode 100644 index 0000000..c14278a --- /dev/null +++ b/.dev/features/add-after-withheld-update/REVIEW.md @@ -0,0 +1,46 @@ +# REVIEW — add-after-withheld-update + +Floor first: `node .dev/floor/validate.mjs .` → exit 0 (GREEN). Everything below is **advisory**. + +## Floor-gate findings (blocking) + +None. + +- **L-floor (P0):** the gate passes on exact membership in `{skillsVersion, pendingSkillsVersion}`; + `pendingSkillsVersion` is written only when every skip label ∈ `{modified, unrecorded}` (enum set) and + is `VERSION_RE`-validated at ingest. Grill concern #1 was real and is fixed in build: at a pending + version `add` keeps the config's `(skillsVersion, commit)` pair (`addStamp`), so it never stamps a + version the kept edits did not receive and the records stamp stays consistent (tested). +- **L-eval (P1):** 8 new cases fail on the base source: 3 in `update` (set by user-edit skips, not set by + `unverifiable`, cleared by a complete run), 2 in `add` (proceeds at pending without advancing the pair; + third version refused with the new message), 3 ingest cases. +- **L-trust (P2):** no new remote input; the field is local and regex-validated. +- **L-axis (P3):** type in `types.ts`, ingest in `pharn-config.ts`, writer in `update.ts`, reader in + `add.ts` — each file keeps its own axis. + +## Advisory findings + +```yaml +- type: FINDING + rule_id: 'P4' + severity: minor + file: 'docs/reference/pharn-config.md:15' + problem: 'The field table at the top of the reference does not list `pendingSkillsVersion`; it is described in the prose note below it.' + evidence: "| `skillsVersion` | string | The repo's `SKILLS_VERSION` at the installed commit |" +- type: FINDING + rule_id: 'P5' + severity: minor + file: 'src/commands/status.ts' + problem: '`status` still reports the install as outdated (skillsVersion is honest) without mentioning that a pending version exists; a user may not realize `add` works now.' + evidence: 'printArchetypeVersion(config, latest)' +- type: FINDING + rule_id: 'P4' + severity: minor + file: 'CHANGELOG.md:8' + problem: "No CHANGELOG `[Unreleased]` entry (not in the plan's `## Files`)." + evidence: '## [Unreleased]' +``` + +## Verdict + +**GREEN** — 0 floor-gate findings, 3 advisory findings (minor). No lesson proposed for canon. diff --git a/.dev/features/add-after-withheld-update/SHIP.md b/.dev/features/add-after-withheld-update/SHIP.md new file mode 100644 index 0000000..e14faef --- /dev/null +++ b/.dev/features/add-after-withheld-update/SHIP.md @@ -0,0 +1,17 @@ +# SHIP — add-after-withheld-update + +Stages run, in order: `/pharn-dev-plan` → GATE 1 (human: **Approve as written**) → `/pharn-dev-grill` → +`/pharn-dev-build` → `/pharn-dev-regress` → `/pharn-dev-verify` → `/pharn-dev-review` → GATE 2. + +| stage | structural verdict (verbatim) | +| -------------------- | ------------------------------------------------------ | +| `/pharn-dev-build` | `node .dev/floor/validate.mjs .` exit `0` | +| `/pharn-dev-regress` | `regression-report.json` `.verdict` = `no-regressions` | +| `/pharn-dev-verify` | `verify-report.json` `.verdict` = `PASS` | + +- Review: [`REVIEW.md`](REVIEW.md) · Grill (advisory): [`GRILL.md`](GRILL.md) +- Run ended at **GATE 2**. The human's standing instruction for this batch: open a PR and merge it only if + its CI checks are green. + +chain ran; the named floor verdicts are as shown — this is NOT a judgment that the increment is good or +wise; that is the human's call at the post-review gate. diff --git a/.dev/features/add-after-withheld-update/VERIFY.md b/.dev/features/add-after-withheld-update/VERIFY.md new file mode 100644 index 0000000..32744f7 --- /dev/null +++ b/.dev/features/add-after-withheld-update/VERIFY.md @@ -0,0 +1,27 @@ +# VERIFY — add-after-withheld-update + +## FLOOR layer (owns the verdict) + +Gates run over the whole repo with the feature present, as a non-root user on node 22 with the session +proxy variables unset (as root with the proxy set, 5 pre-existing tests in `init.test.ts` / +`update.test.ts` fail for environmental reasons, identically at the baseline). + +| gate | exit | +| -------------- | ---- | +| `format:check` | 0 | +| `lint` | 0 | +| `lint:md` | 0 | +| `test` | 0 | +| `typecheck` | 0 | +| `validate` | 0 | + +No `structural:*` gate — the increment ships no eval-actual pair. + +**VERDICT: PASS** (`.dev/floor/check-verify.mjs`, `failing_gates: []`). + +## ADVISORY layer + +`node .dev/floor/count-verifiers.mjs .` → `{"registered":0,"verifiers":[]}` — floor gates only. + +Residual (P0/P7): "verified" means the named gates passed — not that the feature is correct in any sense +the suite does not encode. diff --git a/.dev/features/add-after-withheld-update/regression-report.json b/.dev/features/add-after-withheld-update/regression-report.json new file mode 100644 index 0000000..2937788 --- /dev/null +++ b/.dev/features/add-after-withheld-update/regression-report.json @@ -0,0 +1,28 @@ +{ + "base": "a6c55bc7400903236b1f1ee36432a9d74b15e1d0", + "inside": [ + "CLAUDE.md", + "docs/commands/add.md", + "docs/reference/pharn-config.md", + "src/commands/add.ts", + "src/commands/update.ts", + "src/lib/pharn-config.ts", + "src/types.ts", + "tests/add.test.ts", + "tests/pharn-config.test.ts", + "tests/update.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/add-after-withheld-update/verify-report.json b/.dev/features/add-after-withheld-update/verify-report.json new file mode 100644 index 0000000..ae852f9 --- /dev/null +++ b/.dev/features/add-after-withheld-update/verify-report.json @@ -0,0 +1,17 @@ +{ + "feature": "add-after-withheld-update", + "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 c7ad7c9..c8c1e59 100644 --- a/.pharn/writes-scope.json +++ b/.pharn/writes-scope.json @@ -1,7 +1,7 @@ { "scope": [ - ".dev/features/hook-wiring-drift/SHIP.md" + ".dev/features/add-after-withheld-update/SHIP.md" ], "set_by": ".claude/commands/pharn-dev-ship.md", - "set_at": "2026-09-24T08:17:20.630Z" + "set_at": "2026-09-24T08:22:49.900Z" } diff --git a/CLAUDE.md b/CLAUDE.md index 2cdedaf..36deef9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,7 +59,7 @@ ESM-only (`"type": "module"`, NodeNext). **Relative imports must use `.js` exten **`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. -**`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), 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, `collectDestDrift` (`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. +**`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, `collectDestDrift` (`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. **`pharn remove` addressing** (`commands/remove.ts`) is the inverse of `add`. `remove ` / `remove :` (no arg → an interactive picker over the installed capabilities) deletes that one isolated capability dir — addressed at the project's recorded `layout` (flat `pharn-review` / `pharn-pipeline/grillers/`, OR the same under `pharn/`, via `configLayout` + `layoutPaths`) — and drops its `capabilities` entry. **No clone, no network** — everything is derivable from `config.capabilities` + the filesystem, so `remove.ts` imports no repo module at all; `archetypes` is never touched; `CONSTITUTION.md`/`memory-bank/` are **never** touched (they are not capability dirs). Removing an entry whose stored `source` is **literally** `'auto'` warns that the next `update` will reinstall it; an **absent** `source` warns NOTHING (absence means provenance-unknown, and a false warning on a legacy manual add is worse than silence) — derived from the stored field only, so `remove` stays zero-network. Not-installed → benign no-op listing the removable capabilities; a name installed in both roles → hard-fail (ambiguous). `remove` has no `--yes`, and the reason is per-path: the NAMED remove deletes without asking (no prompt to skip), while the bare picker's ONE confirm — listing the picks, default No — IS the destructive gate, so nothing may skip that either. **`remove`'s `ALLOWED_FLAGS` row is empty, so `pharn remove --yes` now exits 1** naming the flag. This SUPERSEDES the previous contract, which read: _"`--yes` stays a declared minimist boolean only because it is `update`'s flag; that is what keeps `pharn remove --yes` a harmless parse rather than an unknown-option refusal, and turning it into a refusal belongs to a per-command allowlist, not to the dispatch."_ That allowlist now exists (see the dispatch paragraph above), so the deferral is spent — but its reasoning still holds and is why the row is empty rather than `['yes']`: there is no confirm on the named path to skip, and the picker's one confirm may not be skipped. `--yes` is still declared globally, because the declaration list is DERIVED from the table and `update`'s row grants it. Before any delete (the picker: for the whole selection, before its confirm) `remove` refuses with exit 1 when a target dir's path crosses a symlinked component (`findSymlinkComponent`), since `rmSync` resolves ancestors. Every delete is a **strict child** of its role subtree — `safeChildJoin(safeJoin(cwd, subtree), name)` (`lib/validate.ts`), which, unlike `safeJoin`, never returns the base itself nor a deeper path — a second floor under the ingest-time name validation. `remove` also **prunes that capability's entries from `pharn.records.json`** (`pruneCapabilityRecords`), so it is no longer the one write path leaving records that describe bytes that are gone — the converse of the invariant `update` is pinned to. The prune is a **key-prefix filter over the store**, never an fs enumeration: a walk of the DEST dir finds nothing after the delete AND nothing before it on the "already gone" path — which is exactly where the stale keys are the only thing left to clean, so the store is the only place the answer still exists. It mirrors `add`'s `mergeCapabilityRecords` guard verbatim (`recordsBaseline(...) === null → return`): absent/corrupt/stale-stamped → the file is left byte-identical, **never** minted and never blessed (the baseline `note` is deliberately not surfaced — `update` is where an unusable store changes an outcome, and it is the command that names the reason). The trailing slash in `relDir + '/'` is load-bearing (`lenses/a11y` must not eat `lenses/a11y-extended`); nothing matched → the write is skipped entirely; and the `skillsVersion`/`commit` stamp is re-written from the CONFIG pair unchanged, since `remove` advances neither. Order is delete → prune → config (mirroring `add`'s records-before-config) — a benignity argument, not atomicity: each write is individually atomic (`lib/atomic-write.ts` — temp-then-`rename`), but the PAIR is not a transaction, so a failed prune leaves the old status quo and a failed config write leaves an entry the next `update` restores. The role→dir ternary is single-sourced in `capabilityRelDir`, shared with `deleteCapabilityDir`, so the delete and the prune address the same directory by construction. Both call sites prune — the named path with `[target]`, the picker ONCE with the full selection (one store write matching the one config write). diff --git a/docs/commands/add.md b/docs/commands/add.md index 931df80..dbb3d4c 100644 --- a/docs/commands/add.md +++ b/docs/commands/add.md @@ -86,6 +86,13 @@ itself has already been fetched by then — that is where the version being comp Run [`pharn update`](update.md) to bring your install to the current version, then re-run `pharn add`. +**Kept local edits.** If `pharn update` skipped files **only because you edited them** +(`modified` / `unrecorded`), it leaves `skillsVersion` at the old value but records the version it +applied as `pendingSkillsVersion` in `pharn.config.json` — every other file is already at that version. +`add` accepts a clone at that version too, and still does not advance `skillsVersion` or `commit`. If +upstream has moved on again, the refusal names the ways out: run `pharn update`, and if it keeps +skipping your edits, revert them or run `pharn update --force` (which backs them up first). + **Known limit.** There is no way to add a capability to a deliberately-pinned older install — `add` has no `--force`, and `pharn update` is the only resolution. Matching versions is the condition under which `add` can promise anything about the tree it is adding to. diff --git a/docs/reference/pharn-config.md b/docs/reference/pharn-config.md index 7647182..cd8bdb4 100644 --- a/docs/reference/pharn-config.md +++ b/docs/reference/pharn-config.md @@ -69,6 +69,10 @@ The hash map lives there rather than here so this file stays small and hand-edit Note that `skillsVersion` / `commit` describe the last **complete** install: a `pharn update` that skipped any file deliberately leaves them at their previous values (see [update](../commands/update.md)). +When the only skipped files were ones you edited (`modified` / `unrecorded`), it also writes +`pendingSkillsVersion` — the version it applied to everything else — which lets +[`pharn add`](../commands/add.md) run at that version; the next complete `pharn update` removes it. A +value that is not a `x.y.z` version is ignored. ## Example diff --git a/src/commands/add.ts b/src/commands/add.ts index 78220f5..9dccfb6 100644 --- a/src/commands/add.ts +++ b/src/commands/add.ts @@ -90,10 +90,35 @@ export async function runAdd(capabilityArg: string | undefined): Promise { // INSIDE each path's existing try — not after fetchRepo — because readSkillsVersion // throws on a missing/invalid SKILLS_VERSION, and only inside the try does that // throw still reach the finally that cleans the clone up (P0: cleanup before exit). +// +// A `pendingSkillsVersion` (types.ts) also passes: `update` records it only when +// the bump was withheld SOLELY by the user's kept edits, so every other file is +// at that version. Without it, one kept edit dead-ended `add` for good — `update` +// could never finish while the edit stayed, and this message sent the user back +// to `update` in a loop. function versionGate(repoDir: string, config: PharnConfig): string | null { const fetched = readSkillsVersion(repoDir); if (fetched === config.skillsVersion) return null; - return `Skills version mismatch: pharn.config.json records v${config.skillsVersion}, but the fetched ${REPO_URL} is at v${fetched}. \`pharn add\` installs only at the version your project is already on — run \`pharn update\` first, then re-run \`pharn add\`.`; + if (fetched === config.pendingSkillsVersion) return null; + const base = `Skills version mismatch: pharn.config.json records v${config.skillsVersion}, but the fetched ${REPO_URL} is at v${fetched}. \`pharn add\` installs only at the version your project is already on`; + return config.pendingSkillsVersion === undefined + ? `${base} — run \`pharn update\` first, then re-run \`pharn add\`.` + : `${base} (v${config.pendingSkillsVersion} apart from files you edited). Run \`pharn update\` first; if it keeps skipping your edited files, either revert those edits or run \`pharn update --force\` (it backs them up to .pharn-backup/ first), then re-run \`pharn add\`.`; +} + +// The (skillsVersion, commit) pair `add` writes to the config and stamps the +// records with. Normally the clone's — equal to the recorded version by the gate. +// At a PENDING version the pair must stay the config's own: the records store is +// stamped with it, and writing the newer version would claim a complete upgrade +// the kept edits never received (`update` would early-return "up to date"). +function addStamp( + config: PharnConfig, + version: string, + sha: string | null, +): { skillsVersion: string; commit: string | null } { + return version === config.skillsVersion + ? { skillsVersion: version, commit: sha } + : { skillsVersion: config.skillsVersion, commit: config.commit }; } // THE LAYOUT GATE — the sibling of versionGate, and the same shape for the same @@ -440,8 +465,7 @@ async function resolveAddPicker( // the next pick's stamp check fail and silently drop its records.) cfg = { ...cfg, - skillsVersion: result.version, - commit: sha, + ...result.stamp, // Mirrors the entry resolveArchetypeAdd just persisted — INCLUDING its // `source: 'manual'`. This is the second entry-construction site, and it // must not diverge: the next pick spreads THIS array into its own config @@ -471,7 +495,12 @@ function plural(n: number): string { } type AddResult = - | { kind: 'added'; name: string; version: string } + | { + kind: 'added'; + name: string; + version: string; + stamp: { skillsVersion: string; commit: string | null }; + } | { kind: 'noop'; name: string } // See PickerAddOutcome: `cause` present ⇔ this came from an exception. | { kind: 'error'; message: string; cause?: FatalCause }; @@ -588,8 +617,9 @@ async function resolveArchetypeAdd( installCapabilityDirs(repoDir, cwd, [{ name: cap.name, role: cap.role }]); const version = readSkillsVersion(repoDir); // The SHA the tree was pinned to (recorded == fetched, or null when the branch - // was floated — LIMITS.md §3b); threaded from fetchRepo, no separate fetch. - const commit = sha; + // was floated — LIMITS.md §3b); threaded from fetchRepo, no separate fetch — + // unless the clone is at a pending version (see addStamp). + const stamp = addStamp(config, version, sha); // `source: 'manual'` — the user asked for this capability BY NAME, so it is // theirs, not archetype resolution's. `pharn update` reads that tag and // PRESERVES the entry instead of replacing it with the re-resolved auto set @@ -611,15 +641,20 @@ async function resolveArchetypeAdd( // shipped a file at that path, record==dest would make `update` read the user's // file as cleanly upgradeable instead of `modified`. The hashes are still taken // at the DEST (buildRecords), so a record can never disagree with what landed. - await mergeCapabilityRecords(cwd, config, cloneRels, version, commit); + await mergeCapabilityRecords( + cwd, + config, + cloneRels, + stamp.skillsVersion, + stamp.commit, + ); await writePharnConfig(cwd, { ...config, - skillsVersion: version, - commit, + ...stamp, capabilities, installedAt: new Date().toISOString(), }); - return { kind: 'added', name: cap.name, version }; + return { kind: 'added', name: cap.name, version, stamp }; } // Extend `pharn.records.json` with one just-installed capability's files. The diff --git a/src/commands/update.ts b/src/commands/update.ts index a491264..7b330a9 100644 --- a/src/commands/update.ts +++ b/src/commands/update.ts @@ -500,6 +500,15 @@ async function applyUpdate( ? config.skillsVersion : installedVersion; const nextCommit = versionWithheld ? config.commit : sha; + // Withheld ONLY by the user's own kept edits → every other file IS at + // `installedVersion`, so record that as pending (types.ts) and let `add` + // install at it. Any other skip (`unverifiable`, `unreadable`) can leave most + // of the tree at the old version, so nothing is recorded; a complete run + // clears it. Exact label membership, not "forceable" (P5). + const pendingSkillsVersion = + versionWithheld && plan.skipped.every((g) => USER_EDIT_SKIPS.has(g.label)) + ? installedVersion + : undefined; let written: string[]; try { @@ -564,10 +573,12 @@ async function applyUpdate( ...buildRecords(cwd, written), }, }); + const { pendingSkillsVersion: _previousPending, ...rest } = config; await writePharnConfig(cwd, { - ...config, + ...rest, skillsVersion: nextSkillsVersion, commit: nextCommit, + ...(pendingSkillsVersion !== undefined ? { pendingSkillsVersion } : {}), capabilities: configCapabilities, layout, installedAt: new Date().toISOString(), @@ -602,6 +613,13 @@ async function applyUpdate( // check (there is no shared enum and no test pinning the two lists): a new skip // label has to be added HERE as well as there, or the heading will say UNREADABLE // while the advice offers `--force`. +// The skips that are the user's own kept edits — the only ones that may leave +// a `pendingSkillsVersion` behind (applyUpdate). +const USER_EDIT_SKIPS = new Set([ + 'modified', + 'unrecorded', +]); + const FORCEABLE_SKIPS = new Set([ 'modified', 'unrecorded', diff --git a/src/lib/pharn-config.ts b/src/lib/pharn-config.ts index b7ccd1e..2c956a6 100644 --- a/src/lib/pharn-config.ts +++ b/src/lib/pharn-config.ts @@ -7,6 +7,7 @@ import { CAPABILITY_NAME_RE, isPlainObject, ROLE_VALUES, + VERSION_RE, } from './validate.js'; import { ProjectChangedError } from './project-lock.js'; import { validateModelRouting, ModelRoutingError } from './model-routing.js'; @@ -280,6 +281,16 @@ export function readPharnConfig(cwd: string): PharnConfig | null { if (config.layout !== 'pharn' && config.layout !== 'flat') { delete config.layout; } + // Additive `pendingSkillsVersion` (types.ts): only a VERSION_RE-shaped string + // survives; anything else is dropped, so a garbage hand-edit fails closed — + // `add`'s version gate then compares against `skillsVersion` alone. + if ( + config.pendingSkillsVersion !== undefined && + (typeof config.pendingSkillsVersion !== 'string' || + !VERSION_RE.test(config.pendingSkillsVersion)) + ) { + delete config.pendingSkillsVersion; + } return config; } diff --git a/src/types.ts b/src/types.ts index ea4d4c5..e97d035 100644 --- a/src/types.ts +++ b/src/types.ts @@ -116,6 +116,11 @@ export interface SeamConfig { export interface PharnConfig { pharnVersion: string; skillsVersion: string; + // Additive (P7). The upstream version a `pharn update` APPLIED while + // withholding the `skillsVersion` bump solely because of the user's own kept + // edits (`modified` / `unrecorded` skips): every other file is at this + // version. `add` accepts a clone at it; the next complete update clears it. + pendingSkillsVersion?: string; repo: string; commit: string | null; // Legacy (module/wizard) installs record the chosen constitution variant. diff --git a/tests/add.test.ts b/tests/add.test.ts index 0258d56..93f4d85 100644 --- a/tests/add.test.ts +++ b/tests/add.test.ts @@ -822,6 +822,45 @@ describe('runAdd — pharn.records.json', () => { }); }); + // PHARN-05: after an update withheld ONLY by the user's kept edits, the clone + // is at `pendingSkillsVersion`. `add` proceeds — and keeps the config's own + // (skillsVersion, commit) pair, so the store stamp stays consistent and the + // withheld state is not papered over with a version the edits never got. + it('adds at a pendingSkillsVersion without advancing skillsVersion or commit', async () => { + await seedStore(); + loadArchetypeConfigOrExit.mockReturnValue({ + ...config(), + pendingSkillsVersion: '1.1.0', + }); + readSkillsVersion.mockReturnValue('1.1.0'); + + await runAdd('a11y'); + + const [, written] = writePharnConfig.mock.calls.at(-1)!; + expect((written as PharnConfig).skillsVersion).toBe('1.0.0'); + expect((written as PharnConfig).commit).toBeNull(); + expect((written as PharnConfig).pendingSkillsVersion).toBe('1.1.0'); + expect(store()!.skillsVersion).toBe('1.0.0'); + expect(store()!.commit).toBeNull(); + expect(store()!.files[CAP_FILE]).toBe(sha256File(join(proj, CAP_FILE))); + }); + + it('still refuses a clone at a THIRD version, naming the real ways out', async () => { + loadArchetypeConfigOrExit.mockReturnValue({ + ...config(), + pendingSkillsVersion: '1.1.0', + }); + readSkillsVersion.mockReturnValue('1.2.0'); + + await expect(runAdd('a11y')).rejects.toMatchObject(new ProcessExit(1)); + + const msg = vi.mocked(prompts.log.error).mock.calls.at(-1)![0] as string; + expect(msg).toContain('v1.2.0'); + expect(msg).toContain('pharn update --force'); + expect(msg).toContain('revert'); + expect(writePharnConfig).not.toHaveBeenCalled(); + }); + it('re-stamps the store to match the config written beside it', async () => { await seedStore(); await runAdd('a11y'); diff --git a/tests/pharn-config.test.ts b/tests/pharn-config.test.ts index 3378ea6..f26f590 100644 --- a/tests/pharn-config.test.ts +++ b/tests/pharn-config.test.ts @@ -787,3 +787,40 @@ describe('assertConfigUnchanged', () => { ); }); }); + +// PHARN-05: the additive pendingSkillsVersion round-trips when well-formed and is +// dropped otherwise, so a hand-edit can only fail closed. +describe('pendingSkillsVersion ingest', () => { + const tmp = useTmpDir(); + const base = { + pharnVersion: '0.5.0', + skillsVersion: '1.0.0', + repo: 'pharn-dev/pharn-oss', + commit: null, + modules: [], + installedAt: '2026-09-24T00:00:00.000Z', + archetypes: ['ssr'], + capabilities: [], + }; + const put = (v: unknown): void => + writeFileSync(join(tmp.path(), 'pharn.config.json'), JSON.stringify(v)); + + it('round-trips a VERSION_RE-shaped value', () => { + put({ ...base, pendingSkillsVersion: '1.1.0' }); + expect(readPharnConfig(tmp.path())!.pendingSkillsVersion).toBe('1.1.0'); + }); + + it.each([['latest'], [''], [7], ['1.1']])('drops %j', (v) => { + put({ ...base, pendingSkillsVersion: v }); + expect(readPharnConfig(tmp.path())).not.toHaveProperty( + 'pendingSkillsVersion', + ); + }); + + it('absent is legal (P7)', () => { + put(base); + expect(readPharnConfig(tmp.path())).not.toHaveProperty( + 'pendingSkillsVersion', + ); + }); +}); diff --git a/tests/update.test.ts b/tests/update.test.ts index 0ba30c1..1add33c 100644 --- a/tests/update.test.ts +++ b/tests/update.test.ts @@ -872,6 +872,42 @@ describe('runUpdate (drift-safe)', () => { ).toBe(false); }); + // PHARN-05: a bump withheld ONLY by the user's own kept edits records the + // version it did apply, so `pharn add` is not dead-ended by one kept edit. + it("records pendingSkillsVersion when only the user's edits were skipped", async () => { + await installed(); + write(join(proj, DOC), 'MY LOCAL EDIT'); + + await runUpdate(); + + const config = readPharnConfig(proj)!; + expect(config.skillsVersion).toBe('1.0.0'); + expect(config.pendingSkillsVersion).toBe('1.1.0'); + expect(body(CAP_FILE)).toBe('a11y v2'); + }); + + it('records NO pendingSkillsVersion when a skip is not a user edit (no usable records)', async () => { + await installed(); + rmSync(join(proj, RECORDS_FILE)); + write(join(proj, DOC), 'MY LOCAL EDIT'); + + await runUpdate(); + + const config = readPharnConfig(proj)!; + expect(config.skillsVersion).toBe('1.0.0'); + expect(config.pendingSkillsVersion).toBeUndefined(); + }); + + it('a complete run clears a previous pendingSkillsVersion', async () => { + await installed({ pendingSkillsVersion: '1.0.5' }); + + await runUpdate(); + + const config = readPharnConfig(proj)!; + expect(config.skillsVersion).toBe('1.1.0'); + expect(config.pendingSkillsVersion).toBeUndefined(); + }); + it('re-resolves the RECORDED archetypes against the fresh index, and UNIONS the result with the manual entries', async () => { // This test used to pin only the re-resolve call — which was true of the // wholesale-replace bug too. The re-resolve still happens; what it now also