fix(init): a re-run init keeps what update would keep, and re-checks the config under its lock - #216
Merged
Conversation
…the config under its lock A re-run `pharn init` carried over hand-added capabilities, but by a narrower rule than `pharn update`'s merge table: - a `manual` entry the archetypes ALSO selected was re-recorded `auto`, so a later archetype change let `update` drop a capability the user asked for by name (merge row 3 keeps it sticky); - an entry upstream still ships but this CLI cannot parse was dropped from the config and its files orphaned, where `update` keeps it (row 0); - a manual entry upstream no longer ships was dropped silently. init now applies the same rules. Manual entries stay manual. Unparseable entries of any source are written back verbatim, listed in `frozenCapabilities`, and keep their records from a stamp-valid store. Dropped manual entries are named. Carried entries show as "added by hand" in the summary instead of also appearing under SKIPPED. init also reads the config before its prompts and takes the lock after them, but never re-checked it (PHARN-03's contract for add/update/remove), so a concurrent `pharn add` during the confirm was lost. It now takes a fingerprint of pharn.config.json (absent / unreadable / sha256 of the bytes) BEFORE the tolerant parse, and refuses under the lock, writing nothing, if it changed. Docs: docs/reference/pharn-config.md and the init.md config table no longer say a re-run init resets every entry to `auto`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199owRmYfskqYQVQrVP679o
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Resolve the overlap with #215 (init keeps the config keys pharn does not own): both helpers are kept, `keptRecords` and `readCarriedEntries`. The three doc passages and the CHANGELOG now describe both carry-overs. Two of #215's comments are updated. One cited the removed carriedManualCapabilities. The other said a key edited while init's prompt is open is carried; init now refuses first when the config changed, so the key is still not lost. Adds one test: kept entries and user-owned keys survive the same re-run. Regress and verify re-run on the merged tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199owRmYfskqYQVQrVP679o
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
A re-run
pharn initcarries over hand-added capabilities (PHARN-11, #205), but it used a narrower rule thanpharn update's merge table (lib/merge-capabilities.ts). It also broke PHARN-03's lock contract (#197). The review of the last 18 commits reproduced all of these:source: "manual"entry that the archetypes also selected was re-recorded asauto. A later archetype change then letupdatereport "REMOVED — no longer selected" and drop a capability the user asked for by name.updatekeeps itmanual(merge row 3).updatekeeps it (merge row 0,kept-frozen).updatenames it (dropped-gone).initreads the config before its prompts and takes the lock only after them, but never re-checked the config. A concurrentpharn addduring the confirm prompt was overwritten.Now:
src/commands/init.ts:unreadable:<code>/ sha256 of the bytes), then the config is parsed.carryOversorts previous entries by update's rules:withProjectLock,assertConfigFingerprintUnchangedruns before any write (including the backup). If the config changed, init refuses with exit 1.src/lib/pharn-config.ts: addsconfigFingerprintandassertConfigFingerprintUnchanged, reusing the existingProjectChangedErrormessage.src/steps/install-archetype.ts: anInstallCarryargument replaces the baremanualKeys. Kept entries are written verbatim and listed infrozenCapabilities. Their records are carried over from a stamp-valid store; an absent or stale store is never minted from and never trusted.src/steps/archetype-summary.ts+src/types.ts: carried entries show as "added by hand". They no longer also appear under SKIPPED.docs/commands/init.mdanddocs/reference/pharn-config.mdno longer say a re-run init resets every entry toauto. CHANGELOG[Unreleased]→ Fixed.Decisions you made during this run:
Second of the four fixes from the 18-commit review. #213 was the first.
Type of change
feat— new stack option, wizard step, or command capabilityfix— bug fixdocs— docs-only changechore/refactor— tooling or internal restructure, no behavior changeArea(s) touched
commands/init | steps/install-archetype, steps/archetype-summary | lib/pharn-config | types | docs
Checklist
.js-extension import convention.tests/*.test.ts. 8 cases intests/init.test.tsfail against the basesrc/(checked by stashing it).docs/pages.index.unknownare used only as set keys.Quality gates
npm run checkpasses locally (format:check+lint+typecheck+test): 1486 tests.npm run buildsucceeds.npm run test:coveragepasses (coverage thresholds met).Floor workflow tests: 754/754 passed.
validate.mjs: GREEN. Pharn-dev verdicts: regressno-regressions, verifyPASS, review GREEN (3 minor advisory findings). I ran all gates with the CI-equivalent setup described in #213 (root with the permission-override capabilities dropped, proxy variables unset).Notes for the reviewer
carryOver(src/commands/init.ts) is where init's rules matchlib/merge-capabilities.tsrows 0, 3, 6 and 7. An entry with nosourceis deliberately not inferred here, because the merge is the only place allowed to resolve a missing source.update. A re-init that also moves the install flat →pharn/carries none; this is noted inkeptRecords.manualKeys.🤖 Generated with Claude Code
https://claude.ai/code/session_0199owRmYfskqYQVQrVP679o
Generated by Claude Code