Skip to content

fix(init): a re-install backs up only what update would keep; refusals come before the backup - #223

Merged
PrzemekGalarowicz merged 1 commit into
mainfrom
claude/optimistic-heisenberg-8rw8ke
Sep 25, 2026
Merged

PrzemekGalarowicz merged 1 commit into
mainfrom
claude/optimistic-heisenberg-8rw8ke

Conversation

@PrzemekGalarowicz

Copy link
Copy Markdown
Contributor

What this changes

Plan B of the review follow-ups (.dev/features/init-reinstall-safety/). It covers six findings in a re-run pharn init:

  • F9 — "edited" meant "differs from upstream". After any upstream bump, every file pharn itself had written was marked (edited) and copied to .pharn-backup/. The prompt and the backup now use update's own decideFileAction against pharn.records.json (stamp-checked):

    • (edited): the file changed since pharn wrote it.
    • (no pharn record): no record covers the file.
    • (differs from upstream): there is no usable records file, so every difference counts.

    A file still at its recorded hash is a clean upgrade: it is not marked and not backed up.

  • F10 — PHARN-LICENSE / pharn/LICENSE was never compared. The scan looked for the same path in the clone, but the file's source is LICENSE. Each file is now compared with its real source (manifestSources).

  • F20 — a refused install could still create a backup. The destination pre-flight (prepareInstall, now exported) runs before the backup.

  • F19 — a directory at pharn.config.json / pharn.records.json left a half-install. Every file was copied, then the write failed with EISDIR. The type pre-flight now refuses it before the first write.

  • F8 (init half) — .claude/settings.json symlinks.

    • A live link is now accepted, because init never writes an existing settings file.
    • A dangling link is still refused, now with a message about the file.
    • Linked files and linked directories get separate messages.
  • F28 — the manifest was built 5× per run. It is now built once and shared by the prompt, pre-flight, backup scan, copy and records.

Type of change

  • feat — new stack option, wizard step, or command capability
  • fix — bug fix
  • docs — docs-only change
  • chore / refactor — tooling or internal restructure, no behavior change

Area(s) touched

commands/init | steps/install-archetype | steps/overwrite-check | lib/dest-drift | lib/install-capabilities | lib/install-manifest | docs

Checklist

  • Read the existing file(s) before editing; followed the ESM .js-extension import convention.
  • Updated the matching tests/*.test.ts when wizard behavior changed. 35 new or changed cases fail on the base 5b63e31, each for the reason the plan names.
  • Updated the relevant docs/ page (docs/commands/init.md, docs/troubleshooting.md), plus CLAUDE.md and CHANGELOG.
  • Preserved the security invariants. The new sources values are re-safeJoined under the clone, and a test pins an escaping value as a throw.

Quality gates

  • npm run check passes locally (1605 tests). I ran it as root with the DAC-override capabilities dropped (CI-equivalent) and with no proxy variables set.
  • npm run build succeeds.
  • npm run test:coverage passes (97.61 / 92.93 / 98.32 / 98.45).

Notes for the reviewer

  • The plan's reason for refusing a dangling settings.json link was wrong. It said cpSync writes through such a link. Measured on Node 20.13.0, 22.22.2 and 24.21.0, it replaces the link with a regular file. The refusal still stands, for the true reason: the user's link would be lost silently. The comment, docs and test title say that.
  • I corrected a grill premise in this PR. The grill said a flat → pharn/ layout change "backs up everything". It doesn't: the new paths don't exist in a flat project yet. A test now pins that.
  • Pre-existing and not fixed here (REVIEW.md): readRecords blocks on a FIFO at pharn.records.json. update already has this problem; init now reads that file on every re-install too. The fix belongs in lib/install-records.ts.
  • The prompt's labels are advisory. The scan under the lock, just before the copy, decides what is backed up.

🤖 Generated with Claude Code

https://claude.ai/code/session_0199owRmYfskqYQVQrVP679o


Generated by Claude Code

…s come before the backup

- Edited = changed since pharn wrote it. init's prompt and its backup now
  classify each existing file through update's own decideFileAction,
  against pharn.records.json (stamp-checked against the config being
  replaced). A file still at its recorded hash is a clean upgrade: not
  marked, not backed up. Marks: (edited), (no pharn record), and
  (differs from upstream) when there is no usable store.
- Each file is compared with its REAL source, so an edited
  PHARN-LICENSE / pharn/LICENSE (source: upstream LICENSE) is marked and
  backed up instead of silently overwritten.
- The destination pre-flight (prepareInstall, exported) runs before the
  backup: a refused install writes nothing, .pharn-backup/ included.
- The type pre-flight also refuses a directory at pharn.config.json or
  pharn.records.json (was: every file copied, then EISDIR, leaving no
  records and no config).
- A live .claude/settings.json symlink is accepted (init never writes an
  existing settings file); a dangling one is refused with a file-specific
  message. Linked files and linked directories are named apart. Measured
  on Node 20.13 / 22 / 24: cpSync replaces a dangling leaf link, it never
  writes through.
- The install manifest is built once per run and shared by the prompt,
  the pre-flight, the backup scan, the copy and the records (was: 5x).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0199owRmYfskqYQVQrVP679o
@coderabbitai

coderabbitai Bot commented Sep 25, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 32f552e5-2387-4660-8ee2-bf45ff62c40f


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@PrzemekGalarowicz
PrzemekGalarowicz merged commit 8b53ba9 into main Sep 25, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants