diff --git a/.dev/features/lock-read-modify-write/GRILL.md b/.dev/features/lock-read-modify-write/GRILL.md new file mode 100644 index 00000000..187ee143 --- /dev/null +++ b/.dev/features/lock-read-modify-write/GRILL.md @@ -0,0 +1,34 @@ +# GRILL — lock-read-modify-write + +Plan: `.dev/features/lock-read-modify-write/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/lock-read-modify-write/PLAN.md:67' + problem: 'The stress test can show the race is absent in N rounds but cannot prove it; the floor claim must rest on the mtime-grace unit test, with the stress test labeled advisory evidence (the plan does so).' + evidence: '"zero overlapping holders" → demonstrated by the stress test (advisory evidence, not a proof).' +- type: FINDING + rule_id: 'P5' + severity: minor + file: '.dev/features/lock-read-modify-write/PLAN.md:33' + problem: 'Comparing JSON.stringify of two parsed configs is exact only because both sides go through the same readPharnConfig; a snapshot mutated in memory before the re-check would false-positive. The check must run before any in-memory mutation of `config`.' + evidence: '`JSON.stringify` of two `readPharnConfig` results over the same schema' +- type: FINDING + rule_id: 'P1' + severity: minor + file: '.dev/features/lock-read-modify-write/PLAN.md:41' + problem: 'A multi-process stress test inside vitest adds wall-clock time and potential CI flakiness; keep rounds bounded and assert only overlap = 0.' + evidence: 'a multi-process stress test (N child processes × rounds' +``` + +## Summary + +Both halves reduce to deterministic compares under the existing O_EXCL lock. The residuals (a >10 s +stalled `tryCreate`, non-pharn writers) are named in the plan. + +ADVISORY VERDICT: 3 concerns raised (0 blocking-severity, 3 advisory) — for the human to weigh before /pharn-dev-build. diff --git a/.dev/features/lock-read-modify-write/PLAN.md b/.dev/features/lock-read-modify-write/PLAN.md new file mode 100644 index 00000000..be956889 --- /dev/null +++ b/.dev/features/lock-read-modify-write/PLAN.md @@ -0,0 +1,78 @@ +# PLAN — lock-read-modify-write (PHARN-03: the project lock must cover the config snapshot and admit one holder) + +- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4 +- increment: (a) `add` / `update` / `remove` re-read `pharn.config.json` as the first step INSIDE the + project lock and refuse (exit 1, nothing written) when it differs from the snapshot they planned + against; (b) a lock file that exists but is not a parseable payload and is younger than a short + grace window is treated as LIVE (a holder mid-`tryCreate`), not stale. +- layer(s): the CLI itself (`src/lib/project-lock.ts`, `src/lib/pharn-config.ts`, `src/commands/{add,update,remove}.ts`) +- constitution_refs: [P0, P1, P3, P5, P6] + +## Discovery — verified this run (P6) + +Base = `main` after PHARN-02. Reproduced in the review: + +- (a) `repro/stale-config`: `remove` picker waits at its confirm; a concurrent `add a11y` succeeds; the + confirmed `remove` writes its pre-prompt snapshot → `a11y` vanishes from the config, its files and 13 + records stay, both exit 0. `update` has the same shape (config read `update.ts:81`, confirm, lock + `:292`); `remove` named/picker read `:50`, lock `:275`/`:380`; `add` read `:65`, lock `:174`/`:297`. +- (b) `repro/lock-race`: `tryCreate` (`project-lock.ts`) opens with `O_EXCL` then writes the payload, + so the file is briefly EMPTY; `parsePayload('') → null`, `isStale(null) → true`, and a concurrent + process breaks the live lock. 8 processes × 150 rounds → 15 rounds with 2–5 simultaneous holders. + +## Files + +- `src/lib/project-lock.ts` — `ProjectChangedError extends ProjectLockedError` (so every existing + catch site reports it as a named refusal + exit 1 with cleanup); `isStale` takes the lock file's + mtime and returns `false` for an unparseable payload younger than `MALFORMED_GRACE_MS` (10 s) — + an older or unreadable-and-unstattable one is still broken (never wedge) — layer CLI/lib +- `src/lib/pharn-config.ts` — `assertConfigUnchanged(cwd, snapshot, command)`: re-reads the config and + throws `ProjectChangedError` when it is absent or its parsed form differs (`JSON.stringify` of two + `readPharnConfig` results over the same schema) — layer CLI/lib +- `src/commands/add.ts` — call `assertConfigUnchanged` first inside both `withProjectLock` callbacks — layer CLI/command +- `src/commands/update.ts` — same, first inside its `withProjectLock` callback — layer CLI/command +- `src/commands/remove.ts` — same, in both lock callbacks — layer CLI/command +- `tests/project-lock.test.ts` — empty lock with fresh mtime → refused, fn not run, lock untouched; + empty lock with mtime past the grace → broken and acquired (the multi-process race is measured + out-of-suite with the review's `repro/lock-race` harness, 150 rounds × 8 processes, before/after) +- `tests/pharn-config.test.ts` — `assertConfigUnchanged`: identical → no throw; changed / deleted → + `ProjectChangedError` naming the file and the command +- `tests/remove.test.ts` — picker: config rewritten (a capability added) between load and confirm → + exit 1, nothing deleted, the concurrently-added entry preserved +- `tests/update.test.ts` — config changed between load and lock → exit 1, nothing written +- `tests/add.test.ts` — config changed between load and lock → exit 1, nothing written +- `tests/project-lock-commands.test.ts` — its `pharn-config.js` mock gains a no-op `assertConfigUnchanged` stub +- `README.md` — "One writer at a time" paragraph: a run whose config changed underneath it refuses (P4) +- `CLAUDE.md` — project-lock / command paragraphs (P4) + +## Contracts satisfied + +- README "One writer at a time… a second run refuses" and the `project-lock.ts` header's stated threat + (planning against a stale snapshot) — now true for the prompt window too. + +## Evals to write (P1) + +- listed per test file above; each must fail on the base source. + +## Guarantee audit (P0) + +- "a command never persists a config planned against a stale snapshot" → floor: exact comparison of + the re-read config against the snapshot, under the O_EXCL lock. Residual (named): an edit landing + AFTER the re-check but while the lock is held by a non-pharn writer (a human editor) is not covered. +- "a live lock is never broken while its payload is being written" → floor: mtime-age comparison + against `MALFORMED_GRACE_MS`. Residual (advisory): a holder stalled > 10 s between `open` and + `write` (practically impossible for a two-syscall window) could still be broken. +- "zero overlapping holders" → demonstrated by the stress test (advisory evidence, not a proof). + +## Trust audit (P2) + +- The lock file and config are local, user-editable; mtime is read via `statSync` only. No new + untrusted remote input. + +## Determinism audit (P5) + +- Branches: string equality of two serialized configs; integer age compare. Terminal = named refusal. + +## Open questions (HALT) + +- none diff --git a/.dev/features/lock-read-modify-write/REGRESSION.md b/.dev/features/lock-read-modify-write/REGRESSION.md new file mode 100644 index 00000000..d4395e95 --- /dev/null +++ b/.dev/features/lock-read-modify-write/REGRESSION.md @@ -0,0 +1,30 @@ +# REGRESSION — lock-read-modify-write + +The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment. + +## Base and partition + +- **base:** `31f15cedcad80465396e2532f617f8afb60b3012` (`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`, `README.md`, `src/commands/add.ts`, `src/commands/remove.ts`, `src/commands/update.ts`, `src/lib/pharn-config.ts`, `src/lib/project-lock.ts`, `tests/add.test.ts`, `tests/pharn-config.test.ts`, `tests/project-lock-commands.test.ts`, `tests/project-lock.test.ts`, `tests/remove.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/lock-read-modify-write/REVIEW.md b/.dev/features/lock-read-modify-write/REVIEW.md new file mode 100644 index 00000000..eb2848ce --- /dev/null +++ b/.dev/features/lock-read-modify-write/REVIEW.md @@ -0,0 +1,50 @@ +# REVIEW — lock-read-modify-write + +Floor first: `node .dev/floor/validate.mjs .` → exit 0 (GREEN). Everything below is **advisory**. + +## Floor-gate findings (blocking) + +None. + +- **L-floor (P0):** (a) reduces to an exact string compare of two `readPharnConfig` parses under the + O_EXCL lock; (b) to an integer mtime-age compare against `MALFORMED_GRACE_MS`. Residuals named in code: + a non-pharn writer editing after the re-check, and a holder stalled > 10 s inside `tryCreate`. +- **L-eval (P1):** 8 new cases fail on the base source: 2 fresh-unparseable-lock refusals, 3 + `assertConfigUnchanged` units, and the `remove` picker / `remove` named / `update` / `add` + interleave cases in `project-lock-commands.test.ts` (plus a control). Nine existing lock tests now + backdate their planted malformed lock past the grace window — that is their intent (a leftover), not a + weakening. +- **Out-of-suite evidence (advisory):** the review's `repro/lock-race` harness (8 processes started in the + same millisecond, 150 rounds) measured **26/150** rounds with >1 simultaneous holder on the base lock and + **0/150** on this build. An in-suite version at CI-affordable size (6×12) did not reproduce the race on + the base either, so it was dropped rather than shipped as a test that proves nothing. +- **L-trust (P2):** no new untrusted input; the lock/config are local files read defensively. +- **L-axis (P3):** `pharn-config.ts` imports `ProjectChangedError` from `project-lock.ts` (lib→lib, no + cycle: `project-lock.ts` imports only `validate.ts`). The lock module stays config-agnostic. + +## Advisory findings + +```yaml +- type: FINDING + rule_id: 'P5' + severity: minor + file: 'src/lib/pharn-config.ts' + problem: 'The comparison covers pharn.config.json only; a concurrent writer that changed pharn.records.json but not the config would not be detected. Every pharn writer that touches records also rewrites the config, so this is reachable only by a non-pharn edit.' + evidence: 'assertConfigUnchanged(cwd, snapshot, command)' +- type: FINDING + rule_id: 'P7' + severity: minor + file: 'src/commands/update.ts' + problem: "A user who hand-edits pharn.config.json while `update`'s confirm is open is now refused (exit 1) instead of having the edit silently overwritten — intended, but user-visible." + evidence: "assertConfigUnchanged(cwd, config, 'update');" +- 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/lock-read-modify-write/SHIP.md b/.dev/features/lock-read-modify-write/SHIP.md new file mode 100644 index 00000000..a1f0e159 --- /dev/null +++ b/.dev/features/lock-read-modify-write/SHIP.md @@ -0,0 +1,17 @@ +# SHIP — lock-read-modify-write + +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/lock-read-modify-write/VERIFY.md b/.dev/features/lock-read-modify-write/VERIFY.md new file mode 100644 index 00000000..f74e5f11 --- /dev/null +++ b/.dev/features/lock-read-modify-write/VERIFY.md @@ -0,0 +1,27 @@ +# VERIFY — lock-read-modify-write + +## 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/lock-read-modify-write/regression-report.json b/.dev/features/lock-read-modify-write/regression-report.json new file mode 100644 index 00000000..40b4c95e --- /dev/null +++ b/.dev/features/lock-read-modify-write/regression-report.json @@ -0,0 +1,31 @@ +{ + "base": "31f15cedcad80465396e2532f617f8afb60b3012", + "inside": [ + "CLAUDE.md", + "README.md", + "src/commands/add.ts", + "src/commands/remove.ts", + "src/commands/update.ts", + "src/lib/pharn-config.ts", + "src/lib/project-lock.ts", + "tests/add.test.ts", + "tests/pharn-config.test.ts", + "tests/project-lock-commands.test.ts", + "tests/project-lock.test.ts", + "tests/remove.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/lock-read-modify-write/verify-report.json b/.dev/features/lock-read-modify-write/verify-report.json new file mode 100644 index 00000000..49997a37 --- /dev/null +++ b/.dev/features/lock-read-modify-write/verify-report.json @@ -0,0 +1,17 @@ +{ + "feature": "lock-read-modify-write", + "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 4d7e4964..c774326e 100644 --- a/.pharn/writes-scope.json +++ b/.pharn/writes-scope.json @@ -1,12 +1,7 @@ { "scope": [ - "src/lib/install-capabilities.ts", - "src/commands/remove.ts", - "tests/install-capabilities.test.ts", - "tests/remove.test.ts", - "README.md", - "CLAUDE.md" + ".dev/features/lock-read-modify-write/SHIP.md" ], - "set_by": ".dev/features/dest-symlink-guard-init-remove/PLAN.md", - "set_at": "2026-09-24T07:58:14.028Z" + "set_by": ".claude/commands/pharn-dev-ship.md", + "set_at": "2026-09-24T08:06:32.625Z" } diff --git a/CLAUDE.md b/CLAUDE.md index aa3a5a6b..a7efbfd9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,7 +57,7 @@ ESM-only (`"type": "module"`, NodeNext). **Relative imports must use `.js` exten **`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. +**`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. diff --git a/README.md b/README.md index 4915aebf..344046d8 100644 --- a/README.md +++ b/README.md @@ -242,6 +242,9 @@ Three behaviours matter if you script the CLI: destructive overwrite confirmation. - **One writer at a time.** `init`, `add`, `remove` and `update` take an advisory lock at `.pharn.lock`; a second run refuses rather than queueing. + `add`, `remove` and `update` also re-read `pharn.config.json` once they hold + the lock, and refuse — writing nothing — if another run changed it after they + read it (for example while a confirm prompt was open). `list` and `status` never take it and are never blocked by one, so `pharn status --strict` stays runnable in CI while an update is in flight. diff --git a/src/commands/add.ts b/src/commands/add.ts index bbbf459c..78220f5d 100644 --- a/src/commands/add.ts +++ b/src/commands/add.ts @@ -42,6 +42,7 @@ import { proxyNoticeMessage } from '../lib/proxy-env-format.js'; import { readSkillsVersion } from '../lib/skills-version.js'; import { ProjectLockedError, withProjectLock } from '../lib/project-lock.js'; import { + assertConfigUnchanged, loadArchetypeConfigOrExit, writePharnConfig, } from '../lib/pharn-config.js'; @@ -172,6 +173,7 @@ async function runArchetypeAdd( // resolve against. Every error decidable WITHOUT the clone (a legacy config, // a malformed address) still wins. result = await withProjectLock(cwd, 'add', async () => { + assertConfigUnchanged(cwd, config, 'add'); // What a configured proxy means here (nothing: fetch never uses one) — // emitted before the spinner so it survives the frame and precedes a // proxy-caused failure (see src/commands/init.ts for the full rationale). @@ -295,6 +297,7 @@ async function runAddPicker(config: PharnConfig, cwd: string): Promise { // already unbounded — a change of degree, not of kind. `init` is the case where // it WOULD be a change of kind, which is why init's lock stays where it is. outcome = await withProjectLock(cwd, 'add', async () => { + assertConfigUnchanged(cwd, config, 'add'); // What a configured proxy means here (nothing: fetch never uses one) — // emitted before the spinner so it survives the frame and precedes a // proxy-caused failure (see src/commands/init.ts for the full rationale). diff --git a/src/commands/remove.ts b/src/commands/remove.ts index 6bc59f4d..4ee38689 100644 --- a/src/commands/remove.ts +++ b/src/commands/remove.ts @@ -20,6 +20,7 @@ import { safeChildJoin, safeJoin } from '../lib/validate.js'; import { findSymlinkComponent } from '../lib/symlink-guard.js'; import { ProjectLockedError, withProjectLock } from '../lib/project-lock.js'; import { + assertConfigUnchanged, loadArchetypeConfigOrExit, writePharnConfig, } from '../lib/pharn-config.js'; @@ -273,6 +274,7 @@ async function removeNamed( // land between them. `remove` has no prompt on this path and no network, so // the held window is the write itself. const note = await withProjectLock(cwd, 'remove', async () => { + assertConfigUnchanged(cwd, config, 'remove'); const existed = deleteCapabilityDir(cwd, paths, target); // Order: delete → prune records → write config (mirroring `add`'s @@ -378,6 +380,7 @@ async function runRemovePicker( // prompt would block an agent hook for as long as a human takes to answer — // and spanning the whole selection, not one lock per capability. await withProjectLock(cwd, 'remove', async () => { + assertConfigUnchanged(cwd, config, 'remove'); for (const target of targets) deleteCapabilityDir(cwd, paths, target); // One prune for the whole selection — the same delete → prune → config diff --git a/src/commands/update.ts b/src/commands/update.ts index 6a7c2da2..29ecb6ed 100644 --- a/src/commands/update.ts +++ b/src/commands/update.ts @@ -59,6 +59,7 @@ import { } from '../lib/skills-version.js'; import { row } from '../lib/format.js'; import { + assertConfigUnchanged, loadArchetypeConfigOrExit, writePharnConfig, } from '../lib/pharn-config.js'; @@ -290,6 +291,9 @@ async function runArchetypeUpdate( // would have been stranded in the project root on every offline / // rate-limited / DNS failure, the most common failure this command has. outcome = await withProjectLock(cwd, 'update', async () => { + // First, before the spinner: the config this run planned (and confirmed) + // against was read before the lock; refuse if another run rewrote it. + assertConfigUnchanged(cwd, config, 'update'); const s2 = spinner(); spinnerRef.current = s2; s2.start(`Updating from ${REPO_URL}`); diff --git a/src/lib/pharn-config.ts b/src/lib/pharn-config.ts index 8baf6f28..b7ccd1e1 100644 --- a/src/lib/pharn-config.ts +++ b/src/lib/pharn-config.ts @@ -8,6 +8,7 @@ import { isPlainObject, ROLE_VALUES, } 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'; @@ -313,6 +314,40 @@ export function loadConfigOrExit(cwd: string): PharnConfig { process.exit(1); } +/** + * Refuse, under the project lock, when `pharn.config.json` no longer matches + * the `snapshot` this command loaded — and planned against — before it took the + * lock. Between that load and the lock sit a confirm prompt (`update`, the + * `remove` picker) or a fetch, and another pharn run can land in that window: + * measured, a `remove` picker confirmed after a concurrent `add` rewrote its + * stale snapshot, silently dropping the added capability while its files and + * records stayed. The lock alone cannot see this — it only serializes writers + * that are already inside it. + * + * Exact comparison of two parses through the same `readPharnConfig`, so it is + * deterministic (P5); it must run before `snapshot` is mutated. A config that + * is now absent, unparseable or invalid also counts as changed. Throws + * `ProjectChangedError` (a `ProjectLockedError`), which every lock caller + * already reports as a named refusal + exit 1, with nothing written. + */ +export function assertConfigUnchanged( + cwd: string, + snapshot: PharnConfig, + command: string, +): void { + let current: PharnConfig | null; + try { + current = readPharnConfig(cwd); + } catch { + current = null; + } + if (current !== null && JSON.stringify(current) === JSON.stringify(snapshot)) + return; + throw new ProjectChangedError( + `${CONFIG_FILENAME} changed while \`pharn ${command}\` was running — another pharn process (or an edit) wrote it after this command read it. Nothing was written. Re-run \`pharn ${command}\` to act on the current state.`, + ); +} + /** * 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 diff --git a/src/lib/project-lock.ts b/src/lib/project-lock.ts index 236a668e..3e506c33 100644 --- a/src/lib/project-lock.ts +++ b/src/lib/project-lock.ts @@ -83,6 +83,18 @@ const CORPSE_RE = /^\.pharn\.lock\.[0-9]+\.[0-9a-f]{8}$/; */ export const STALE_MS = 6 * 60 * 60 * 1000; // 6 hours +/** + * How long a lock file that exists but does not parse is presumed LIVE. + * + * `tryCreate` creates the file (O_EXCL) and writes its payload in a second + * syscall, so for an instant a live lock is EMPTY. Reading that as stale let a + * concurrent acquire break a lock whose owner was mid-write, and 8 processes + * started together measured 2–5 simultaneous holders in 15 of 150 rounds. An + * unparseable lock is therefore stale only once its mtime is older than this; + * a genuinely corrupt one still cannot wedge the project for longer. + */ +export const MALFORMED_GRACE_MS = 10 * 1000; + /** Raised when another pharn process holds the lock. Exit 1, with a named reason. */ export class ProjectLockedError extends Error { constructor(message: string) { @@ -91,6 +103,19 @@ export class ProjectLockedError extends Error { } } +/** + * Raised under the lock when the project state a command planned against has + * changed since it was read — another writer landed while this command was + * prompting or fetching. A subclass so every existing `ProjectLockedError` + * catch site reports it the same way: a named refusal, exit 1, nothing written. + */ +export class ProjectChangedError extends ProjectLockedError { + constructor(message: string) { + super(message); + this.name = 'ProjectChangedError'; + } +} + interface LockPayload { pid: number; host: string; @@ -177,8 +202,19 @@ function isDead(held: LockPayload): boolean { } } -function isStale(held: LockPayload | null): boolean { - if (held === null) return true; // malformed or unreadable — never wedge on it +/** The lock file's age in ms, or `null` when it cannot be stat'ed. */ +function lockAgeMs(cwd: string): number | null { + try { + return Date.now() - statSync(lockPath(cwd)).mtimeMs; + } catch { + return null; + } +} + +function isStale(held: LockPayload | null, ageMs: number | null): boolean { + // Malformed or unreadable. Young → presumed a holder mid-`tryCreate` (see + // MALFORMED_GRACE_MS); old, or not stat-able → stale, so it never wedges. + if (held === null) return ageMs === null || ageMs >= MALFORMED_GRACE_MS; if (Date.now() - Date.parse(held.startedAt) > STALE_MS) return true; return isDead(held); } @@ -448,7 +484,8 @@ export async function withProjectLock( // is exactly the unchecked delete it replaced. const observedRaw = readRawAt(lockPath(cwd)); const held = observedRaw === null ? null : parsePayload(observedRaw); - if (!isStale(held)) throw new ProjectLockedError(refusal(held)); + if (!isStale(held, lockAgeMs(cwd))) + throw new ProjectLockedError(refusal(held)); breakStaleLock(cwd, command, observedRaw); } try { diff --git a/tests/add.test.ts b/tests/add.test.ts index 2ec7c7b7..0258d567 100644 --- a/tests/add.test.ts +++ b/tests/add.test.ts @@ -74,7 +74,9 @@ vi.mock('../src/lib/skills-version.js', () => ({ const loadArchetypeConfigOrExit = vi.fn(); const writePharnConfig = vi.fn(); +const assertConfigUnchanged = vi.fn(); vi.mock('../src/lib/pharn-config.js', () => ({ + assertConfigUnchanged, loadArchetypeConfigOrExit, writePharnConfig, })); diff --git a/tests/pharn-config.test.ts b/tests/pharn-config.test.ts index 3c23804a..3378ea60 100644 --- a/tests/pharn-config.test.ts +++ b/tests/pharn-config.test.ts @@ -1,9 +1,17 @@ -import { mkdirSync, readFileSync, readdirSync, writeFileSync } from 'node:fs'; +import { + mkdirSync, + readFileSync, + readdirSync, + rmSync, + writeFileSync, +} from 'node:fs'; import { join } from 'node:path'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { log } from '@clack/prompts'; import { ProcessExit, stubProcessExit, useTmpDir } from './helpers.js'; +import { ProjectChangedError } from '../src/lib/project-lock.js'; import { + assertConfigUnchanged, readPharnConfig, loadConfigOrExit, loadArchetypeConfigOrExit, @@ -723,3 +731,59 @@ describe('writePharnConfig — atomic replacement', () => { expect(readPharnConfig(tmp.path())?.skillsVersion).toBe('0.68.0'); }); }); + +// PHARN-03 (a): a command re-reads the config under the lock and refuses when +// it no longer matches what it planned against. +describe('assertConfigUnchanged', () => { + const tmp = useTmpDir(); + const cfg = (caps: { name: string; role: 'griller' | 'lens' }[]) => ({ + 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: caps, + }); + const put = (v: unknown): void => + writeFileSync(join(tmp.path(), 'pharn.config.json'), JSON.stringify(v)); + + it('passes when the file still parses to the snapshot', () => { + put(cfg([{ name: 'a11y', role: 'lens' }])); + const snapshot = readPharnConfig(tmp.path())!; + expect(() => + assertConfigUnchanged(tmp.path(), snapshot, 'remove'), + ).not.toThrow(); + }); + + it('refuses with ProjectChangedError when another run rewrote it', () => { + put(cfg([{ name: 'a11y', role: 'lens' }])); + const snapshot = readPharnConfig(tmp.path())!; + put( + cfg([ + { name: 'a11y', role: 'lens' }, + { name: 'n-plus-one', role: 'lens' }, + ]), + ); + expect(() => assertConfigUnchanged(tmp.path(), snapshot, 'remove')).toThrow( + ProjectChangedError, + ); + expect(() => assertConfigUnchanged(tmp.path(), snapshot, 'remove')).toThrow( + /pharn\.config\.json changed while `pharn remove` was running/, + ); + }); + + it('refuses when the file is gone or no longer valid', () => { + put(cfg([{ name: 'a11y', role: 'lens' }])); + const snapshot = readPharnConfig(tmp.path())!; + put('{ not json'); + expect(() => assertConfigUnchanged(tmp.path(), snapshot, 'add')).toThrow( + ProjectChangedError, + ); + rmSync(join(tmp.path(), 'pharn.config.json')); + expect(() => assertConfigUnchanged(tmp.path(), snapshot, 'add')).toThrow( + ProjectChangedError, + ); + }); +}); diff --git a/tests/project-lock-commands.test.ts b/tests/project-lock-commands.test.ts index e537f21c..4b54e612 100644 --- a/tests/project-lock-commands.test.ts +++ b/tests/project-lock-commands.test.ts @@ -34,7 +34,9 @@ vi.mock('@clack/prompts', () => ({ const loadArchetypeConfigOrExit = vi.fn(); const writePharnConfig = vi.fn(); +const assertConfigUnchanged = vi.fn(); vi.mock('../src/lib/pharn-config.js', () => ({ + assertConfigUnchanged, loadArchetypeConfigOrExit, writePharnConfig, })); @@ -323,3 +325,94 @@ describe('a failed download under the lock strands nothing', () => { expect(msg).toContain('offline'); }); }); + +// PHARN-03 (a): each writer re-reads pharn.config.json as the first step under +// the lock. Here the real check runs against a real file that "another run" +// rewrote after this command loaded its snapshot — the measured case was a +// `remove` picker confirmed after a concurrent `add`, which dropped the added +// capability from the config while its files and records stayed. +describe('a config rewritten before the lock refuses a writing command', () => { + const tmp = useTmpDir(); + stubProcessExit(); + let proj = ''; + + const withAdd = () => ({ + ...archetypeConfig(), + capabilities: [ + { name: 'a11y', role: 'griller' }, + { name: 'n-plus-one', role: 'lens', source: 'manual' }, + ], + }); + const configFile = () => join(proj, 'pharn.config.json'); + + beforeEach(async () => { + proj = tmp.path(); + vi.spyOn(process, 'cwd').mockReturnValue(proj); + writePharnConfig.mockReset(); + fetchRepo.mockReset(); + fetchRemoteSkillsVersion.mockReset(); + fetchRemoteSkillsVersion.mockResolvedValue('2.0.0'); + const actual = await vi.importActual< + typeof import('../src/lib/pharn-config.js') + >('../src/lib/pharn-config.js'); + assertConfigUnchanged.mockImplementation(actual.assertConfigUnchanged); + // What this command loaded and planned against… + loadArchetypeConfigOrExit.mockReturnValue(archetypeConfig()); + // …and what is on disk by the time it takes the lock. + writeFileSync(configFile(), JSON.stringify(withAdd())); + }); + afterEach(() => { + assertConfigUnchanged.mockReset(); + restoreTTY(); + }); + + it('`remove` (picker, after its confirm) refuses: nothing deleted, the concurrent add survives', async () => { + setTTY(true, true); + mkdirSync(join(proj, 'pharn-pipeline/grillers/a11y'), { recursive: true }); + writeFileSync(join(proj, 'pharn-pipeline/grillers/a11y/a11y.md'), 'CAP'); + vi.mocked(prompts.groupMultiselect).mockResolvedValueOnce(['griller:a11y']); + + await expect(runRemove(undefined)).rejects.toMatchObject( + new ProcessExit(1), + ); + + expect( + readFileSync(join(proj, 'pharn-pipeline/grillers/a11y/a11y.md'), 'utf8'), + ).toBe('CAP'); + expect(writePharnConfig).not.toHaveBeenCalled(); + expect(JSON.parse(readFileSync(configFile(), 'utf8'))).toEqual(withAdd()); + expect(vi.mocked(prompts.log.error).mock.calls.map(String).join()).toMatch( + /pharn\.config\.json changed while `pharn remove` was running/, + ); + expect(existsSync(join(proj, LOCK_FILE))).toBe(false); + }); + + it('`remove` (named) refuses too', async () => { + await expect(runRemove('a11y')).rejects.toMatchObject(new ProcessExit(1)); + expect(writePharnConfig).not.toHaveBeenCalled(); + }); + + it('`update` refuses before fetching or writing', async () => { + await expect(runUpdate({ yes: true })).rejects.toMatchObject( + new ProcessExit(1), + ); + expect(fetchRepo).not.toHaveBeenCalled(); + expect(writePharnConfig).not.toHaveBeenCalled(); + expect(existsSync(join(proj, LOCK_FILE))).toBe(false); + }); + + it('`add` refuses before fetching or writing', async () => { + await expect(runAdd('n-plus-one')).rejects.toMatchObject( + new ProcessExit(1), + ); + expect(fetchRepo).not.toHaveBeenCalled(); + expect(writePharnConfig).not.toHaveBeenCalled(); + }); + + it('an unchanged config lets the command through (control)', async () => { + writeFileSync(configFile(), JSON.stringify(archetypeConfig())); + mkdirSync(join(proj, 'pharn-pipeline/grillers/a11y'), { recursive: true }); + await runRemove('a11y'); + expect(writePharnConfig).toHaveBeenCalledTimes(1); + }); +}); diff --git a/tests/project-lock.test.ts b/tests/project-lock.test.ts index 27f9ddea..00ea37da 100644 --- a/tests/project-lock.test.ts +++ b/tests/project-lock.test.ts @@ -14,6 +14,7 @@ import { describe, expect, it } from 'vitest'; import { useTmpDir } from './helpers.js'; import { LOCK_FILE, + MALFORMED_GRACE_MS, ProjectLockedError, withProjectLock, } from '../src/lib/project-lock.js'; @@ -63,13 +64,24 @@ function plantCorpse(dir: string, name: string, ageMs: number): void { utimesSync(p, when, when); } -/** A lock file with an arbitrary payload, as a hand-edit or a crashed run leaves. */ -function plantLock(dir: string, data: unknown): void { +/** + * A lock file with an arbitrary payload, as a hand-edit or a crashed run leaves. + * Backdated past MALFORMED_GRACE_MS by default, because that is what a leftover + * is: a malformed lock with a FRESH mtime is presumed to be a live holder + * mid-write (see the PHARN-03 cases below), which is not what these tests plant. + */ +function plantLock( + dir: string, + data: unknown, + ageMs = MALFORMED_GRACE_MS + 60_000, +): void { writeFileSync( lockPath(dir), typeof data === 'string' ? data : JSON.stringify(data), 'utf8', ); + const when = new Date(Date.now() - ageMs); + utimesSync(lockPath(dir), when, when); } describe('withProjectLock', () => { @@ -327,6 +339,8 @@ describe('withProjectLock', () => { // imply recursive, so the delete threw ERR_FS_EISDIR on every run and the // project refused forever — hazard #1 (wedging) in this file's own header. mkdirSync(lockPath(dir)); + const when = new Date(Date.now() - MALFORMED_GRACE_MS - 60_000); + utimesSync(lockPath(dir), when, when); let ran = false; await withProjectLock(dir, 'add', () => { @@ -341,3 +355,39 @@ describe('withProjectLock', () => { expect(statSync(join(dir, left[0]!)).isDirectory()).toBe(true); }); }); + +// PHARN-03 (b): `tryCreate` opens with O_EXCL and writes the payload in a +// second syscall, so a LIVE lock is briefly empty. Reading that as stale let a +// concurrent acquire break it — measured: 2–5 simultaneous holders in 15 of 150 +// rounds with 8 processes started together. +describe('withProjectLock — a lock being written is live (PHARN-03)', () => { + const tmp = useTmpDir(); + + it.each([[''], ['{"pid":']])( + 'refuses on a FRESH unparseable lock %j, without running fn or touching the file', + async (raw) => { + const dir = tmp.path(); + plantLock(dir, raw, 0); + let ran = false; + await expect( + withProjectLock(dir, 'update', () => { + ran = true; + }), + ).rejects.toBeInstanceOf(ProjectLockedError); + expect(ran).toBe(false); + expect(readFileSync(lockPath(dir), 'utf8')).toBe(raw); + expect(lockSiblings(dir)).toEqual([]); + }, + ); + + it('still breaks an unparseable lock once it is older than the grace window', async () => { + const dir = tmp.path(); + plantLock(dir, '', MALFORMED_GRACE_MS + 1_000); + let ran = false; + await withProjectLock(dir, 'update', () => { + ran = true; + }); + expect(ran).toBe(true); + expect(existsSync(lockPath(dir))).toBe(false); + }); +}); diff --git a/tests/remove.test.ts b/tests/remove.test.ts index 68ceadfe..4f721cc8 100644 --- a/tests/remove.test.ts +++ b/tests/remove.test.ts @@ -30,7 +30,9 @@ vi.mock('@clack/prompts', () => ({ const loadArchetypeConfigOrExit = vi.fn(); const writePharnConfig = vi.fn(); +const assertConfigUnchanged = vi.fn(); vi.mock('../src/lib/pharn-config.js', () => ({ + assertConfigUnchanged, loadArchetypeConfigOrExit, writePharnConfig, })); diff --git a/tests/update.test.ts b/tests/update.test.ts index ef5bf491..384f26a6 100644 --- a/tests/update.test.ts +++ b/tests/update.test.ts @@ -51,11 +51,12 @@ const resolveCapabilities = vi.fn(); vi.mock('../src/lib/resolve-capabilities.js', () => ({ resolveCapabilities })); const loadArchetypeConfigOrExit = vi.fn(); +const assertConfigUnchanged = vi.fn(); vi.mock('../src/lib/pharn-config.js', async () => { const actual = await vi.importActual< typeof import('../src/lib/pharn-config.js') >('../src/lib/pharn-config.js'); - return { ...actual, loadArchetypeConfigOrExit }; + return { ...actual, assertConfigUnchanged, loadArchetypeConfigOrExit }; }); const { runUpdate } = await import('../src/commands/update.js');