diff --git a/.dev/features/gitignore-run-state/BUILD.md b/.dev/features/gitignore-run-state/BUILD.md new file mode 100644 index 0000000..d8f0fe9 --- /dev/null +++ b/.dev/features/gitignore-run-state/BUILD.md @@ -0,0 +1,17 @@ +# BUILD — gitignore-run-state + +Landed `src/lib/gitignore.ts` (the plan, append, covered test and wording) and `writeFileAtomic` (`writeJsonAtomic` delegates to it). They are wired into `init` (`runInstallArchetype`, after the config write) and `update` (`applyPlannedUpdate`, after the config write, plus the same-version re-open). Both dry runs (text PLAN line, additive JSON `gitignore` / `plan.gitignore`) and the conditional backup-notice advice are in. Tests: `tests/gitignore.test.ts` (59, including a real `git check-ignore`) plus the cases folded into the atomic-write, backup, init/update dry-run (text + JSON), update and integration suites. Docs, CHANGELOG 0.16.0 and the version bump are in too. Floor: `npm run check` GREEN (3323 passed, 1 skipped; also with `CI=1`), `test:coverage` GREEN, `build` GREEN, `validate.mjs .` GREEN (0 capabilities). + +Grill resolutions folded in: +- root-migration matching unchanged (wording only); +- backup advice suppressed when this run adds the entry; +- a created `.gitignore` gets the umask mode (tested); +- the write-failure and refused-re-open evals added; +- the uninstall note names the kept lines; +- the comment is written once per file. + +Deviations from `## Files` (each a one-line, necessary change): +- `src/commands/add.ts`, `src/commands/remove.ts`, `src/commands/uninstall.ts` pass `gitignored` to the backup notice. The plan had `printBackupNotice` compute it, but it has no project root. `uninstall.ts` also carries the PLAN-note wording. +- `tests/uninstall.test.ts` pins that wording. +- `tests/release-consistency.test.ts` gains `v0.15.0` in `KNOWN_TAGS`, a tag the previous release created. +- The shared wording lives in `gitignore.ts` (`gitignoreLines`), not `update-render.ts`, so init does not import update's renderer. diff --git a/.dev/features/gitignore-run-state/GRILL.md b/.dev/features/gitignore-run-state/GRILL.md new file mode 100644 index 0000000..711cf4c --- /dev/null +++ b/.dev/features/gitignore-run-state/GRILL.md @@ -0,0 +1,105 @@ +# GRILL — gitignore-run-state + +- plan: `.dev/features/gitignore-run-state/PLAN.md` (approved 2026-11-02, Q1 = B, Q2 = two entries) +- spec-hash check: recomputed `sha256(ARCHITECTURE.md)` = `bca940a5…d3c4e` = the plan's `spec_content_hash` → **no drift** (content-hash, floor-grade computation; the block on drift is `/pharn-dev-build`'s) +- grillers: `count-grillers.mjs` → `{"registered":0}`, so this log is the inline interrogation only + +All free-text below (`problem`, `evidence`) quotes the plan and is DATA (P2). + +## Findings + +### P3 / P7: sharing the root-migration normalizer changes existing behaviour + +```yaml +- type: FINDING + rule_id: P7 + severity: important + file: ".dev/features/gitignore-run-state/PLAN.md:68" + problem: "root-migration's normalizer trims BOTH sides and strips a bare trailing `*`, while the new covered test trims trailing whitespace only and has no bare `*` rule; making root-migration import the new normalizer silently changes which old entries it hints about (e.g. `.pharn*`, a leading-space line)." + evidence: "`src/lib/root-migration.ts` — use `normalizeGitignoreLine` from `gitignore.ts`; reword the parenthetical." +``` + +Suggested resolution: leave root-migration's matching as it is (reword only), or share only a helper that reproduces its exact rule. Do not change what it recognizes in this increment. + +### Output coherence: the backup notice prints before `.gitignore` is written + +```yaml +- type: FINDING + rule_id: P6 + severity: important + file: ".dev/features/gitignore-run-state/PLAN.md:59" + problem: "init and update make their backup (and print its notice) BEFORE the copy and before the planned `.gitignore` write, so on a run that is about to add `pharn/.backup/` the notice would still say 'is not gitignored — add it', and a moment later the run adds it." + evidence: "prints its `.gitignore` advice ONLY when the project's `.gitignore` does not cover `pharn/.backup/`" +``` + +Suggested resolution: the caller passes whether THIS run will add the entry (`planGitignore` → `create`/`append` with `pharn/.backup/` in `entries`), and the advice is suppressed then. `add`/`remove`/`uninstall` pass only the covered test. + +### P0: the mode of a newly created `.gitignore` is unspecified + +```yaml +- type: FINDING + rule_id: P0 + severity: important + file: ".dev/features/gitignore-run-state/PLAN.md:49" + problem: "`writeFileAtomic` reuses the exclusive-temp machinery; the SYNC temp is created 0600, so a newly CREATED `.gitignore` could land owner-only, which breaks a shared checkout. The plan pins mode preservation for an existing file only." + evidence: "with the same exclusive temp, mode preservation and rename as `writeJsonAtomic`" +``` + +Suggested resolution: use the async path (`open(tmp,'wx')`, default 0o666 & umask), and add an eval that a created `.gitignore` gets the umask-default mode (not 0600). + +### P1: the eval list misses two paths the plan specifies + +```yaml +- type: FINDING + rule_id: P1 + severity: minor + file: ".dev/features/gitignore-run-state/PLAN.md:43" + problem: "Two specified paths have no eval: the write-failure path ('the write fails (EACCES, …)' → warn, run still exits 0) and the re-open NOT counting a refused `.gitignore` (symlink/dir) — the plan's bound on Q1 depends on that exclusion." + evidence: "the write fails (EACCES, …)." +``` + +### P4: the trailing-newline wording and the uninstall PLAN note + +```yaml +- type: FINDING + rule_id: P4 + severity: minor + file: ".dev/features/gitignore-run-state/PLAN.md:37" + problem: "The brief says 'preserve the file's trailing newline'; the plan instead adds a separator when the file lacks one, so the result always ends in a line ending. That is the right call (an append needs a line break), but the docs must say so rather than claim the ending is preserved." + evidence: "When the old file is non-empty and does not end in `\\n`, one line ending is added first, as a separator." +``` + +```yaml +- type: FINDING + rule_id: P4 + severity: minor + file: ".dev/features/gitignore-run-state/PLAN.md:61" + problem: "uninstall's PLAN note already lists what it leaves untouched (settings files, pharn/.backup/); the plan keeps the `.gitignore` lines but does not say the note will name them, so the doc and the note could disagree." + evidence: "**uninstall** leaves the lines in place" +``` + +### P5: the repeated comment line + +```yaml +- type: FINDING + rule_id: P5 + severity: minor + file: ".dev/features/gitignore-run-state/PLAN.md:39" + problem: "The comment header is appended on EVERY non-empty addition, so a user who deletes one entry gets a second identical comment line when it is re-added. This is harmless and deterministic, but a duplicate the brief's 'never duplicate' spirit may object to." + evidence: "Then come a comment line `# PHARN run state and update backups (added by pharn)` and each missing entry" +``` + +Suggested resolution: append the comment only when the file does not already contain that exact comment line (a membership test). + +## Summary + +The plan's core is sound and floor-reduced: an append-only Buffer prefix, membership-tested spellings, an atomic replace, the dry-run snapshot, and a stated advisory label on "never committed". The concerns are at the edges: +- one change to existing behaviour (the shared normalizer); +- one piece of contradictory user output (the backup advice printed before the write); +- one unspecified file mode on create, which matters for shared checkouts; +- two untested paths; +- small doc/wording gaps. + +None needs a re-plan. Each folds into the build within the plan's `## Files`. + +ADVISORY VERDICT: 7 concerns raised (0 blocking-severity, 3 important, 4 minor) — for the human to weigh before /pharn-dev-build. This is not a pass; it gates nothing. diff --git a/.dev/features/gitignore-run-state/PLAN.md b/.dev/features/gitignore-run-state/PLAN.md new file mode 100644 index 0000000..1cef1fe --- /dev/null +++ b/.dev/features/gitignore-run-state/PLAN.md @@ -0,0 +1,147 @@ +# PLAN — `init` / `update` keep `pharn/.run/` and `pharn/.backup/` out of git + +- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4 +- increment: `pharn init` and `pharn update` make sure the project's root `.gitignore` covers `pharn/.run/` and `pharn/.backup/`, appending only the missing lines. The dry runs preview the change and the real runs report it. +- layer(s): `src/lib` (a new `.gitignore` axis + atomic text write), `src/steps` (init apply), `src/commands` (update apply), the report/render libs, tests, docs. Release 0.16.0. +- constitution_refs: [P0, P1, P2, P3, P4, P5, P6, P7] +- base: origin/main fe16608 (branch `feat/gitignore-run-state`) + +## Live state (read this run, P6) + +- **Trigger.** pharn-oss 7.2.0 fixed issue #408: reconcile counted PHARN's run state (`pharn/.run/`) as build escapes because installed projects do not ignore it. pharn-oss ≥ 7 writes run state at a FIXED `pharn/.run/` for every layout (`build-gate.mjs` `STATE_ROOT = "pharn/.run"`). pharn-oss's own `.gitignore` lists `pharn/.run/`, `pharn/.backup/`, `pharn/.lock*`. +- **This CLI never writes `.gitignore` today, and says so in several places:** + - `src/lib/root-migration.ts` `gitignoreHints` reads it bounded and prints `… (pharn never edits .gitignore).` + - `src/lib/backup.ts` `backupNoticeLines` always prints `pharn/.backup/ is not gitignored — add it to .gitignore …`. + - `README.md` §172-174, `docs/commands/update.md` (522-524, 851), `docs/commands/add.md` (221-223), plus the notice examples in `remove.md` and `uninstall.md`. +- `root-migration.ts` already normalizes `.gitignore` lines (trim, one leading `/`, one trailing `/`, `/*`, `/**`, `*`). That is the same membership test this increment needs, so it moves to the new lib and both callers share it (P3). +- **Write sites.** `init`: `runInstallArchetype` (`src/steps/install-archetype.ts`, under init's `pharn/.lock`). `update`: `applyPlannedUpdate` (`src/commands/update.ts`, under `withProjectLock`). `add`/`remove`/`uninstall` stay out of scope. +- `init --yes` refuses when ANY of its write targets exists (`planWriteTargets`). `.gitignore` must NOT join that target set, or `init --yes` would refuse in nearly every project. It gets its own channel. +- Atomic writes: `writeJsonAtomic` / `copyFileAtomicSync` (`src/lib/atomic-write.ts`). There is no atomic text/Buffer writer yet. +- Bounded read: `readBoundedFile` (`src/lib/bounded-read.ts`). It returns `absent` / `unusable` / `ok {bytes}`, never throws, and FIFO/dir/oversize come back `unusable`. +- Tests that pin today's wording: `tests/backup.test.ts`, `tests/init-archetype.test.ts:808`, `tests/update.test.ts:2962/3069`, and `tests/commands-integration.test.ts:590`, which asserts `.gitignore` is byte-identical after a root migration. +- Release flow (CLAUDE.md "Releasing"): the bump travels in the PR and merging it IS the release (`publish.yml`). 0.15.0 → **0.16.0**: new behaviour, minor. +- Note: the floor hook runs from the MAIN checkout's cwd and reads its stale `.pharn/writes-scope.json`, so Write/Edit into this worktree is blocked whatever scope is set here. The worktree scope is set per stage (as the commands require), and writes go through Bash (the saved memory note on this hook). + +## Behaviour (the spec this plan builds) + +- **Entries:** `PHARN_GITIGNORE_ENTRIES = ['pharn/.run/', 'pharn/.backup/']` (see Q2). +- **Covered test (P5, membership on fixed literals).** A line is normalized by: + 1. dropping one trailing CR; + 2. trimming trailing whitespace only (git ignores unescaped trailing spaces, while a leading space is part of the pattern); + 3. dropping one leading `/` or `**/`; + 4. dropping one trailing `/`, `/*` or `/**`. + + An entry counts as **present** when a normalized line equals `pharn/.run` (or `pharn/.backup`). A bare `.run` / `.backup` also counts, because a slash-less pattern matches at any depth. A NEGATION (`!pharn/.run/` etc., the same normalization after the `!`) counts as the user's decision, so pharn never adds the entry. Comments (`#…`) never count. + + Broader patterns that also cover the path (`pharn/`, `pharn/.*`, `*`) are NOT recognized. pharn then appends its line, which is redundant but harmless. This limit is documented. +- **Append-only.** The new file's bytes START WITH the old file's exact bytes (Buffer concat, never a decode/re-encode). No existing line is reordered, rewritten or removed. + - When the old file is non-empty and does not end in `\n`, one line ending is added first, as a separator. + - Then come a comment line `# PHARN run state and update backups (added by pharn)` and each missing entry, each ending in a line ending. +- **Line endings:** CRLF when the old file's FIRST line ending is `\r\n`; otherwise LF. The rule is deterministic, and a missing file gets LF. +- **Missing file** → created with the comment and both entries (LF). +- **Idempotent:** both entries are present after a run, so a second run computes an empty addition and writes nothing (byte- and mtime-identical). +- **Refusals (never fatal, never written through), each one a `log.warn` naming the lines to add by hand:** + - `.gitignore` is a symlink, or crosses a symlinked component (`findSymlinkComponent`); + - it is not a regular file (dir/FIFO/device), or it is unreadable or oversize (`readBoundedFile` → `unusable`); + - the write fails (EACCES, …). + + The install's own files have already landed by then, so a `.gitignore` problem warns instead of failing the run. +- **Write** = new `writeFileAtomic(target, data)` in `atomic-write.ts`, with the same exclusive temp, mode preservation and rename as `writeJsonAtomic`, which now delegates to it. So a hard-linked `.gitignore` is replaced, never written into. +- **Where:** + - `init`: in `runInstallArchetype`, after `writePharnConfig` and before the root-migration moves, under init's lock. The outro gets a line `✔ .gitignore updated → pharn/.run/, pharn/.backup/` (or `created`). Nothing prints when nothing was added. + - `update`: in `applyPlannedUpdate`, after the config write, under the lock. It prints `log.info` `Added pharn/.run/, pharn/.backup/ to .gitignore.` (or `Created .gitignore with …`). +- **Same-version `update` (see Q1, recommended B).** A missing entry RE-OPENS the same-version early return, the established bounded re-open pattern (`pendingRootMigration`, `findMissingInstalled`). The header note gets one line saying `.gitignore` is missing pharn's entries and this run adds them. + + The re-open counts only a `.gitignore` the run CAN write: an absent file, or a regular, non-symlinked, bounded-readable one. The opened run adds the lines and the next run early-returns. Residual: a regular file whose write fails (EACCES) re-opens every run. That is stated, the same shape as `pendingRootMigration`'s residual. +- **Dry runs** (they write nothing and share the planner): + - text: `init --dry-run` and `update --dry-run` add one `PLAN` line, `.gitignore: would add pharn/.run/, pharn/.backup/ (would create it)`; + - JSON: additive `gitignore: {action: 'create' | 'append', entries: string[]}` (`init --dry-run --json`, top level) and `plan.gitignore` (`update --dry-run --json`). The key is ABSENT when nothing would be added, the `absentUpstream` convention, so the existing byte pins hold and `schemaVersion` stays 1. A refusal shows as `{action: 'refused', reason}` (enum reason: `symlink` | `not-a-file` | `unreadable`). +- **The backup notice** (`backupNoticeLines`) prints its `.gitignore` advice ONLY when the project's `.gitignore` does not cover `pharn/.backup/`. It uses the same covered test, so `add`/`remove`/`uninstall` stop advising what is already done. Because `backupNoticeLines` is pure, it takes a `gitignored: boolean` input; `printBackupNotice` computes it. +- **root-migration** keeps its hints for old entries (pharn never rewrites a user's line) and imports the shared normalizer. Its parenthetical becomes `(pharn only appends its own entries; it never rewrites yours)`. +- **uninstall** leaves the lines in place: they are harmless, and removing user-file lines is a rewrite. This is documented. + +## Files + +- `src/lib/gitignore.ts` (NEW) — `PHARN_GITIGNORE_ENTRIES`, `normalizeGitignoreLine`, `coveredEntries(bytes)`, `gitignoreAddition(bytes | null)` (pure: the Buffer to append + the entries), `planGitignore(projectRoot)` (read-only: `{action: 'none' | 'create' | 'append', entries} | {action: 'refused', reason}`), `applyGitignore(projectRoot)` (re-plans, then writes atomically; returns the outcome; never throws a write error, it returns `{action: 'failed', reason}`). One axis: pharn's lines in the project `.gitignore`. — layer: lib +- `src/lib/atomic-write.ts` — add `writeFileAtomic(target, data: string | Buffer)`; `writeJsonAtomic` delegates to it, with byte-identical output. — layer: lib +- `src/lib/root-migration.ts` — use `normalizeGitignoreLine` from `gitignore.ts`; reword the parenthetical. — layer: lib +- `src/lib/backup.ts` — `backupNoticeLines(notice, {aborted, gitignored})`; `printBackupNotice` computes `gitignored` from `coveredEntries`. — layer: lib +- `src/steps/install-archetype.ts` — `applyGitignore` after the config write; the warning and outro line. — layer: steps +- `src/commands/update.ts` — `applyGitignore` in `applyPlannedUpdate`; the re-open signal plus its header line. — layer: commands +- `src/lib/update-plan.ts` — `report.gitignore` from `planGitignore`. — layer: lib +- `src/lib/update-report.ts` — additive `plan.gitignore` (absent when `none`). — layer: lib +- `src/lib/update-render.ts` — the dry-run PLAN line + a shared `gitignoreLines(outcome, tense)` renderer used by init too. Init and update share it through lib, never command→command (P3). — layer: lib +- `src/lib/init-plan.ts` — `InitPlanReport.gitignore`. — layer: lib +- `src/lib/init-report.ts` — additive top-level `gitignore`. — layer: lib +- `src/commands/init.ts` — the text dry run prints the line. — layer: commands +- `tests/gitignore.test.ts` (NEW) — the unit evals below. — layer: tests +- `tests/atomic-write.test.ts` — `writeFileAtomic` (Buffer + string, mode kept, hard link not written through). — layer: tests +- `tests/backup.test.ts`, `tests/init-archetype.test.ts`, `tests/update.test.ts` — adjust the advice assertions (shown both ways: covered → no advice; uncovered → advice). — layer: tests +- `tests/init-dry-run.test.ts`, `tests/update-dry-run.test.ts` — the preview line + the JSON key present/absent; the whole-tree snapshot proves no write. — layer: tests +- `tests/commands-integration.test.ts` — real `init --yes` creates/appends `.gitignore`; a real `update --yes` at the same version with the entries present early-returns, and with one removed re-opens and re-adds it. Line 590's expectation becomes "old lines kept byte-for-byte as the prefix, pharn's lines appended". — layer: tests +- `tests/root-migration.test.ts` — the parenthetical wording, if pinned. — layer: tests +- `README.md`, `docs/commands/init.md`, `docs/commands/update.md`, `docs/commands/add.md`, `docs/commands/remove.md`, `docs/commands/uninstall.md`, `docs/troubleshooting.md` (only if it repeats the rule) — the "never edits `.gitignore`" sentences become the append-only rule, plus the JSON key rows. — layer: docs +- `CLAUDE.md` — one paragraph on the `.gitignore` axis (it is the architecture map). — layer: docs +- `CHANGELOG.md` — `## [0.16.0] — 2026-11-02` with an `### Added` entry + compare links. — layer: docs +- `package.json`, `package-lock.json` — `npm version --no-git-tag-version 0.16.0`. — layer: release + +### Files added by amendment (human-approved 2026-11-02, after the regress scope check flagged them) + +- `src/commands/add.ts`, `src/commands/remove.ts` — pass `gitignored: backupDirIgnored(cwd)` to the backup notice — layer: commands +- `src/commands/uninstall.ts` — the same, plus the PLAN note names the `.gitignore` lines it leaves — layer: commands +- `tests/uninstall.test.ts` — pins that wording — layer: tests +- `tests/init-report.test.ts`, `tests/update-report.test.ts` — fixtures gain `gitignore: {action: 'none'}` — layer: tests +- `tests/init-dry-run-json.test.ts`, `tests/update-dry-run-json.test.ts` — the JSON `gitignore` / `plan.gitignore` evals — layer: tests +- `tests/release-consistency.test.ts` — `v0.15.0` joins `KNOWN_TAGS` (it exists on origin) — layer: tests +- `.dev/features/gitignore-run-state/PLAN.md`, `.dev/features/gitignore-run-state/GRILL.md`, `.dev/features/gitignore-run-state/BUILD.md`, `.dev/features/gitignore-run-state/REGRESSION.md`, `.dev/features/gitignore-run-state/regression-report.json`, `.dev/features/gitignore-run-state/VERIFY.md`, `.dev/features/gitignore-run-state/verify-report.json`, `.dev/features/gitignore-run-state/REVIEW.md`, `.dev/features/gitignore-run-state/SHIP.md` — the pipeline's own artifacts — layer: dev-loop +- `.pharn/writes-scope.json` — the per-stage scope file (local state; restored before push) — layer: dev-loop + +## Contracts satisfied + +- CLAUDE.md "Releasing": the bump + the CHANGELOG section travel in the PR; `tests/release-consistency.test.ts` checks them. +- CLAUDE.md JSON-report convention: additive keys absent when empty, `schemaVersion` 1 kept (`absentUpstream`/`plan.orphans` precedent). +- CLAUDE.md "the dry run reaches no write function" (`snapshotTree` pins), extended to `applyGitignore`/`writeFileAtomic`. +- `init --yes` never-overwrite contract: `.gitignore` stays outside `planWriteTargets`, so `--yes` behaviour is unchanged. + +## Evals to write (P1) + +- missing `.gitignore` → created, exactly `# PHARN …\npharn/.run/\npharn/.backup/\n` +- existing LF file without entries (`node_modules\n`) → old bytes are the prefix; comment + both entries appended +- existing file without a trailing newline (`node_modules`) → one `\n` separator, then the block +- each equivalent spelling present (`pharn/.run/`, `/pharn/.run/`, `pharn/.run`, `pharn/.run/**`, `pharn/.run/*`, `**/pharn/.run/`, `.run/`, `pharn/.run/ ` with trailing spaces) → that entry is not added (the other one still is) +- ` pharn/.run/` (LEADING space) → NOT counted (git reads it as a different pattern), so the entry is added +- comment `# pharn/.run/` → not counted +- negation `!pharn/.run/` → counted as decided, so the entry is not added +- both present → no write (bytes and mtime unchanged) +- CRLF file (`a\r\nb\r\n`) → appended lines use `\r\n`, old bytes are the prefix, no bare `\n` introduced +- second `applyGitignore` → `none`, file unchanged +- `.gitignore` is a symlink → refused, link and target untouched; a directory → refused; a FIFO → refused without hanging +- hard-linked `.gitignore` → replaced, and the other link keeps the old bytes +- `writeJsonAtomic` output byte-identical after delegating (the existing tests stay green) +- backup notice: advice shown when uncovered, omitted when covered +- `init --dry-run` / `update --dry-run` (text + JSON): line / key present when needed, absent when not; tree unchanged +- integration: `init --yes` fresh → `.gitignore` has both; `update --yes` same version, entries present → `up-to-date`; one entry deleted → re-opens, re-adds, next run `up-to-date` + +## Guarantee audit (P0) + +- "existing `.gitignore` bytes are never rewritten, reordered or removed" → floor: Buffer prefix (`Buffer.concat([old, addition])`), asserted byte-for-byte by tests +- "no entry is duplicated across the recognized spellings" → floor: regex/literal membership over normalized lines (P5) +- "idempotent" → floor: the addition is a pure function of the bytes; it is empty once both entries are covered, and an empty addition does not write +- "never written through a symlink or a hard link" → floor: `findSymlinkComponent` + lstat regular-file check + exclusive-temp-then-rename +- "the dry run writes nothing" → floor: `snapshotTree` + zero-call spies (existing pattern) +- "run state will never be committed" → **advisory.** A `.gitignore` line does not untrack files already committed, and a user can delete it or force-add. Broader covering patterns are not recognized (redundant append only). This is stated in the docs. +- "the same-version re-open is bounded" → floor for the counted cases (only a writable-shaped file counts). The EACCES residual is named. + +## Trust audit (P2) + +- `.gitignore` is project content (user-owned, possibly hostile checkout). It is read bounded (`readBoundedFile`, `O_NONBLOCK`, regular-file `fstat`, 16 MiB cap) and only membership-tested against fixed literals. Its text is never echoed to the terminal, never executed, and never drives a path join. The only write target is the fixed `safeJoin(root, '.gitignore')`. + +## Determinism audit (P5) + +- Every branch is a membership test (normalized-line ∈ fixed set; first line ending is `\r\n`; lstat type). Anything unrecognized falls to "append" (harmless) or "refused + warn naming the lines" — never a guess that removes or rewrites. + +## Open questions (resolved by the human, 2026-11-02) + +- Q1 same-version `update` with entries missing → **B: re-open the early return** (bounded, like `pendingRootMigration`). +- Q2 entries → **the two asked for** (`pharn/.run/`, `pharn/.backup/`). +- Plan → **approved as written**. diff --git a/.dev/features/gitignore-run-state/REGRESSION.md b/.dev/features/gitignore-run-state/REGRESSION.md new file mode 100644 index 0000000..07ed8e4 --- /dev/null +++ b/.dev/features/gitignore-run-state/REGRESSION.md @@ -0,0 +1,20 @@ +# REGRESSION — gitignore-run-state + +- base: `fe16608` (origin/main, the branch's fork point); head: `ce375a0` +- inside (the changed scope): the 42 files in `regression-report.json` `.inside`. The regress scope check found nothing outside the declared `## Files`, after the human-approved amendment. +- outside gates: the 46 `.mjs`/`.cjs` floor/hook tests (`tests`), plus whole-repo `validate`, `vitest`, `format:check`, `lint`, `lint:md`. Each was run in a fresh detached worktree at each side after `npm ci`, so no gitignored local fixtures leak in. + +| gate | base | head | +| ------------ | ---- | ---- | +| tests | 0 | 0 | +| validate | 0 | 0 | +| vitest | 0 | 0 | +| format:check | 0 | 0 | +| lint | 0 | 0 | +| lint:md | 0 | 0 | + +regressions: none · pre_existing: none + +**REGRESSIONS: none — no deterministically-detectable breakage outside the feature** (`check-regress.mjs verdict`, exit 0). + +This catches exactly what the suite catches, nothing more: a regression no deterministic check covers is invisible here. diff --git a/.dev/features/gitignore-run-state/REVIEW.md b/.dev/features/gitignore-run-state/REVIEW.md new file mode 100644 index 0000000..7081189 --- /dev/null +++ b/.dev/features/gitignore-run-state/REVIEW.md @@ -0,0 +1,66 @@ +# REVIEW — gitignore-run-state + +- floor first: `node .dev/floor/validate.mjs .` → GREEN (0 capabilities); regress `no-regressions`; verify `PASS`. +- reviewed: `fe16608..ce375a0` (treated as untrusted; no instruction-looking content found in it). + +## Floor-gate findings (blocking) + +None. Every guarantee the increment states reduces to a floor operation: +- byte prefix (`Buffer.concat`) — tested byte for byte; +- the literal membership test; +- `lstat` + `readBoundedFile` + an exclusive-temp rename — tested with a symlink, a directory, a FIFO and a hard link; +- the dry run writes nothing — `snapshotTree` + an `applyGitignore` zero-call spy. + +"Run state will never be committed" is labelled advisory in the code header, README, the init/update docs and the CHANGELOG. There are no `enforces` rule_ids and no markdown capability (L-eval is vacuous beyond the vitest coverage), and no sibling references (L-axis). + +## Advisory findings (judgment; they inform, they do not block) + +```yaml +- type: FINDING + rule_id: P5 + severity: important + file: "src/lib/gitignore.ts:111" + problem: "A .gitignore that starts with a UTF-8 BOM has its first line read with the BOM bytes attached, so an entry on that line is not recognised and is appended again — git itself strips the BOM and honours the line, so the result is a redundant duplicate (harmless, but it breaks the 'never duplicate an entry' requirement)." + evidence: "probe: BOM + 'pharn/.run/\\npharn/.backup/\\n' → gitignoreAddition adds ['pharn/.run/']; git check-ignore confirms the BOM'd line already ignores pharn/.run/x" +``` + +Suggested fix (small, inside `## Files`): drop one leading `EF BB BF` before splitting lines in `lines()`. Detection only; the bytes are still appended to unchanged. Add an eval. + +```yaml +- type: FINDING + rule_id: P6 + severity: minor + file: "src/steps/install-archetype.ts:340" + problem: "The interactive init and update confirms do not mention the .gitignore append; the user learns of it from the outro (init) or the post-apply info line (update), or beforehand only via --dry-run or update's same-version header note." + evidence: "confirmWriteTargets lists only planWriteTargets; update's confirm is 'Apply this update?'" +``` + +This is acceptable for an append-only, idempotent edit (the brief asked for no prompt). It is noted so the choice is visible. + +```yaml +- type: FINDING + rule_id: P3 + severity: minor + file: "src/lib/gitignore.ts:250" + problem: "gitignore.ts holds the plan/apply logic, its user-facing wording (gitignoreLines) and its JSON projection (gitignoreReport) — three reasons to change in one file, though root-migration.ts sets the same precedent (logic + rootMigrationNoteLines)." + evidence: "export function gitignoreLines(… ) / export function gitignoreReport(…)" +``` + +```yaml +- type: FINDING + rule_id: P2 + severity: minor + file: "src/lib/gitignore.ts:170" + problem: "Between the lstat (not a symlink) and readBoundedFile (which follows links) a swap could make pharn READ a link target; the write still replaces the name by rename and never writes through, so the residual is a read of a bounded regular file, never an outside write." + evidence: "isLink = lstatSync(target).isSymbolicLink(); … const read = readBoundedFile(target);" +``` + +## Resolution (after GATE 2) + +The human chose "fix BOM, then ship". `lines()` now drops one leading UTF-8 BOM before matching (detection only; the bytes are still appended to unchanged). Two evals cover it: `decidedEntries` reads past a BOM, and a BOM file that already covers both is left byte-identical. + +## Verdict + +**GREEN — 0 floor findings, 4 advisory (1 important: the BOM duplicate).** The decision is the human's. + +No lesson proposed for canon: the BOM case is a one-off edge, not a recurring failure (P7). diff --git a/.dev/features/gitignore-run-state/SHIP.md b/.dev/features/gitignore-run-state/SHIP.md new file mode 100644 index 0000000..f03789a --- /dev/null +++ b/.dev/features/gitignore-run-state/SHIP.md @@ -0,0 +1,18 @@ +# SHIP — gitignore-run-state + +Advisory roll-up of a gated `/pharn-dev-ship` run. + +## Stages, in order + +1. `/pharn-dev-plan` → `PLAN.md`, approved at GATE 1 (Q1 = re-open the same-version update, Q2 = two entries). +2. `/pharn-dev-grill` → `GRILL.md` (advisory: 7 concerns, 0 blocking; resolutions folded into the build). +3. `/pharn-dev-build` → `BUILD.md`; floor `validate.mjs .` exit **0**; `npm run check` GREEN. +4. `/pharn-dev-regress` → first run STOPPED at the scope check (exit 1: the build touched files outside `## Files`). The human approved amending `## Files` (recorded in `PLAN.md`); the re-run's `regression-report.json` `.verdict` = **`"no-regressions"`**. +5. `/pharn-dev-verify` → `verify-report.json` `.verdict` = **`"PASS"`** (`failing_gates: []`; verifiers registered: 0). +6. `/pharn-dev-review` → `REVIEW.md` (0 floor findings, 4 advisory — see the file; not restated here). + +## Where the run ended + +GATE 2 — the post-review decision (merge / fix / abandon) is the human's. Nothing is pushed, merged or released. + +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/gitignore-run-state/VERIFY.md b/.dev/features/gitignore-run-state/VERIFY.md new file mode 100644 index 0000000..86e1ed3 --- /dev/null +++ b/.dev/features/gitignore-run-state/VERIFY.md @@ -0,0 +1,20 @@ +# VERIFY — gitignore-run-state + +All gates ran in a fresh detached worktree at HEAD (`ce375a0`) after `npm ci`. + +| gate | exit | +| ------------ | ---- | +| test | 0 | +| validate | 0 | +| lint | 0 | +| format:check | 0 | +| lint:md | 0 | +| typecheck | 0 | + +No `structural:*` gate: the increment ships no committed eval pair (pharn is TypeScript; its evals are the vitest tests counted in `test`). + +**VERIFIED: floor gates PASS** (`check-verify.mjs`, exit 0). + +Verifiers: no verifiers registered — floor gates only (`count-verifiers.mjs` → `registered: 0`). + +Verified = the named gates passed. This is NOT a guarantee of correctness beyond what those gates check; verifier concerns are advisory help, not assurance. diff --git a/.dev/features/gitignore-run-state/regression-report.json b/.dev/features/gitignore-run-state/regression-report.json new file mode 100644 index 0000000..e46899c --- /dev/null +++ b/.dev/features/gitignore-run-state/regression-report.json @@ -0,0 +1,75 @@ +{ + "base": "fe16608", + "inside": [ + ".dev/features/gitignore-run-state/BUILD.md", + ".dev/features/gitignore-run-state/GRILL.md", + ".dev/features/gitignore-run-state/PLAN.md", + "CHANGELOG.md", + "CLAUDE.md", + "README.md", + "docs/commands/add.md", + "docs/commands/init.md", + "docs/commands/uninstall.md", + "docs/commands/update.md", + "package-lock.json", + "package.json", + "src/commands/add.ts", + "src/commands/init.ts", + "src/commands/remove.ts", + "src/commands/uninstall.ts", + "src/commands/update.ts", + "src/lib/atomic-write.ts", + "src/lib/backup.ts", + "src/lib/gitignore.ts", + "src/lib/init-plan.ts", + "src/lib/init-report.ts", + "src/lib/root-migration.ts", + "src/lib/update-plan.ts", + "src/lib/update-render.ts", + "src/lib/update-report.ts", + "src/steps/install-archetype.ts", + "tests/atomic-write.test.ts", + "tests/backup.test.ts", + "tests/commands-integration.test.ts", + "tests/gitignore.test.ts", + "tests/init-archetype.test.ts", + "tests/init-dry-run-json.test.ts", + "tests/init-dry-run.test.ts", + "tests/init-report.test.ts", + "tests/release-consistency.test.ts", + "tests/uninstall.test.ts", + "tests/update-dry-run-json.test.ts", + "tests/update-dry-run.test.ts", + "tests/update-report.test.ts", + "tests/update.test.ts" + ], + "outside_gates": { + "format:check": { + "base": 0, + "head": 0 + }, + "lint": { + "base": 0, + "head": 0 + }, + "lint:md": { + "base": 0, + "head": 0 + }, + "tests": { + "base": 0, + "head": 0 + }, + "validate": { + "base": 0, + "head": 0 + }, + "vitest": { + "base": 0, + "head": 0 + } + }, + "regressions": [], + "pre_existing": [], + "verdict": "no-regressions" +} diff --git a/.dev/features/gitignore-run-state/verify-report.json b/.dev/features/gitignore-run-state/verify-report.json new file mode 100644 index 0000000..f43bcfe --- /dev/null +++ b/.dev/features/gitignore-run-state/verify-report.json @@ -0,0 +1,17 @@ +{ + "feature": "gitignore-run-state", + "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/CHANGELOG.md b/CHANGELOG.md index e5b0c3b..6e29b92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,17 @@ Releases 0.4.0 and older are in [CHANGELOG-ARCHIVE.md](CHANGELOG-ARCHIVE.md). ## [Unreleased] +## [0.16.0] — 2026-11-02 + +### Added + +- **`pharn init` and `pharn update` keep PHARN's run state and backups out of git.** They append `pharn/.run/` (pharn-oss's runtime scratch — stage logs, gate outputs, cost markers) and `pharn/.backup/` (pharn's backups) to the project's root `.gitignore`, creating it when absent. An installed project used to track `pharn/.run/`, so pharn-oss's reconcile counted PHARN's own run state as build escapes and failed a correct build (pharn-oss #408, fixed upstream in 7.2.0). Append-only: the file's bytes stay exactly as they were, with only the missing line(s) and one comment added after them, in the file's own line endings (CRLF stays CRLF); a line already covering an entry in any common spelling (`/pharn/.run`, `pharn/.run/**`, a bare `.run/`, …), or a negation you wrote (`!pharn/.run/`), is respected; a second run changes nothing. A symlinked, non-regular or unreadable `.gitignore` is never written — the run warns, naming the lines to add — and a failed write is a warning, never a failed install. `init`'s outro says `.gitignore created`/`updated`; `update` prints `Added … to .gitignore`. At the same skills version a `.gitignore` missing either line re-opens `update`'s "Already up to date" return, so existing installs get the lines on their next `update`. `--dry-run` previews the change, and `init --dry-run --json` / `update --dry-run --json` carry an additive `gitignore` / `plan.gitignore` key, present only when `.gitignore` would change (`schemaVersion` stays 1). `.gitignore` is not an `init --yes` write target, so `--yes` never refuses because of it. See [`.gitignore`](docs/commands/init.md#gitignore). +- **The "pharn never edits `.gitignore`" rule is reversed — for these two lines only.** Earlier releases documented that pharn never touches `.gitignore` and left adding `pharn/.run/` and `pharn/.backup/` to the user. In practice installed projects did not, and pharn-oss's issue #408 showed the cost: 55 run-state files under `pharn/.run/` became tracked candidates and failed a correct build's verify. pharn still never rewrites, reorders or removes a line of yours; it only appends these two entries. `add`, `remove` and `uninstall` still never write `.gitignore`. + +### Changed + +- **The backup notice's `.gitignore` advice is printed only when it applies.** `pharn/.backup/ is not gitignored — add it …` no longer follows every backup: it is dropped when `.gitignore` already covers `pharn/.backup/`, or when the same `init`/`update` run is about to add it. `uninstall`'s `PLAN` note now also says it leaves the lines pharn added to `.gitignore`, and `update`'s root-migration hint says pharn only appends its own entries and never rewrites yours. + ## [0.15.0] — 2026-10-26 ### Changed @@ -288,7 +299,8 @@ _Never released: 0.9.0 followed on the same day, so `0.8.0` was neither tagged n matcher that silently stopped matching, or a comparator read in the wrong direction, would otherwise leave every assertion green. -[Unreleased]: https://github.com/pharn-dev/pharn-cli/compare/v0.15.0...HEAD +[Unreleased]: https://github.com/pharn-dev/pharn-cli/compare/v0.16.0...HEAD +[0.16.0]: https://github.com/pharn-dev/pharn-cli/compare/v0.15.0...v0.16.0 [0.15.0]: https://github.com/pharn-dev/pharn-cli/compare/v0.14.1...v0.15.0 [0.14.1]: https://github.com/pharn-dev/pharn-cli/compare/v0.14.0...v0.14.1 [0.14.0]: https://github.com/pharn-dev/pharn-cli/compare/v0.13.0...v0.14.0 diff --git a/CLAUDE.md b/CLAUDE.md index 4e7b87b..d613168 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -79,6 +79,8 @@ ESM-only (`"type": "module"`, NodeNext). **Relative imports must use `.js` exten **The root footprint lives under `pharn/` (0.14.0).** The lock is `pharn/.lock` and backups go to `pharn/.backup//` for EVERY layout (`LOCK_DIR`/`LOCK_FILE` in `lib/project-lock.ts`, `BACKUP_DIR` in `lib/backup.ts`); `withProjectLock` creates `pharn/` when absent (non-recursive `mkdirSync`, EEXIST = success), refuses a `pharn` that is a symlink (`findSymlinkComponent`) or not a directory, and on release `rmdir`s a `pharn/` it CREATED when still empty; `createBackup` lstat-refuses a symlinked or non-directory `pharn` or `pharn/.backup`. An old root `.pharn-backup/` (≤ 0.13.0) is never moved or deleted; `legacyBackupNotice` (`lib/backup.ts`, lstat — only a real directory) prints one line about it from `update` and `init`, real run and text dry run (0.14.1). For ONE release the lock also checks the legacy root `.pharn.lock` (`LEGACY_LOCK_FILE`, `checkLegacyLock`, run while holding `pharn/.lock`): live → refused with the old wording (ours released), stale → broken by the same rename→verify path but never re-created, root corpses swept with `LEGACY_CORPSE_RE`; a directory there is ignored. Residual: a 0.13.0 CLI does not see `pharn/.lock`. `uninstall` makes one non-recursive `rmdir pharn` after the lock is released (ENOENT/ENOTEMPTY = success) and names what is left — always at least `.backup` (the config's backup). The pharn layout's trusted docs are all under `pharn/` (`PHARN_TRUSTED_DOCS`); `resolveTrustedDocs(repoDir, layout)` (`lib/layout.ts`, `resolveFeaturesReadme`'s shape) is the ordered probe — `pharn/` if the clone has it, else the root path from `PHARN_ROOT_FALLBACK_DOCS` (THREAT-MODEL/LIMITS, a pre-7.0 clone) — used by `collectInstallManifest` and `installCapabilities`; an absent doc is named at the layout path. Root `THREAT-MODEL.md`/`LIMITS.md` stay `isInstallRootPath` through the FLAT list. **`lib/root-migration.ts`** retires the old root footprint of a pharn-layout install, for `update` (`buildUpdatePlan` → `report.rootMigration`, `apply.rootMoveBackups`; executed in `applyPlannedUpdate`) and a RE-RUN `init` (`initRootMigration` in `lib/init-plan.ts` — null on a first install; executed in `runInstallArchetype`): `planRootMigration` (null unless BOTH the clone and the recorded install are the pharn layout — `installedLayout`) → `docs` (exactly the two fallback docs, only when the clone ships the `pharn/` copy; classified by `planUninstall` — `delete`/`keep-edited`/`absent`/`unverifiable`, no record = `unverifiable`), and, only when the clone's SKILLS major ≥ 7, `moves` (`memory-bank/` → `pharn/memory-bank/`, `pharn.spec-template.md` → `pharn/spec-template.md`: `move`/`conflict`/`refused-symlink`/`unreadable`/`absent` — `unreadable` = wrong type or an unlistable dir, never a crash of the plan), `scratchNotice` (`.pharn/` holds more than `ci.yml`; lstat, never followed) and `gitignoreHints` (`.gitignore` read bounded, never written). Order in a real run: the ONE `createBackup` includes the move sources' regular files → removals → writes → `deleteRootDocs` (only if `pharn/` LANDED at the clone's `contentHash`; a failed unlink keeps the file and record) → records (retired keys filtered out of the carried orphan records; kept docs stay orphans) → config → `applyRootMoves` LAST (re-checked under the lock; `renameSync`). `conflict`/`refused-symlink`/`unreadable`/a failed rename → `process.exitCode = 1` (`rootMigrationFailed`) after the report. A deleted root doc is filtered out of `report.orphans`. The real `update` prints the future-tense note before its confirm, and `membershipKeys` includes the migration's deletes and moves, so the locked re-plan refuses if they changed. `report.rootMoveBackups` (= `apply.rootMoveBackups`) feeds the dry run's backup list and `--json` `plan.backups`. `pendingRootMigration` re-opens update's same-version early return (pharn layout, config SKILLS major ≥ 7, and either a record-equal root doc whose `pharn/` copy is neither recorded nor on disk, or a movable user-data path); nothing the opened run cannot finish counts, so it is bounded — residual: a ≥ 7 pharn-oss still shipping a doc only at the root would re-open every run. The `ROOT MIGRATION` note wording is `rootMigrationNoteLines` (shared by update, init and both dry runs); `update --dry-run --json` carries `plan.rootMigration` (ALWAYS present, `null` when nothing applies — the 0.10.0 byte pin is now "those bytes plus that one key"). `init --dry-run --json` does not carry it. +**`lib/gitignore.ts` keeps PHARN's run state and backups out of git (0.16.0).** `init` (`runInstallArchetype`, after `writePharnConfig`) and `update` (`applyPlannedUpdate`, after the config write) call `applyGitignore` under the project lock: it re-plans (`planGitignore` — `create` / `append` / `none` / `refused {symlink|not-a-file|unreadable}`, read through `readBoundedFile`, a symlinked `.gitignore` lstat-refused) and writes the OLD bytes + the addition through `writeFileAtomic` (`lib/atomic-write.ts`, which `writeJsonAtomic` now delegates to; a new file gets the umask mode). APPEND-ONLY: `gitignoreAddition` returns only the missing `PHARN_GITIGNORE_ENTRIES` (`pharn/.run/`, `pharn/.backup/`), one `GITIGNORE_COMMENT` (once per file), in the file's first line ending, after a separator when the file lacks a final newline — the old bytes are always the prefix (latin1, never re-encoded). The covered test (`decidedEntries`) is a membership test on fixed literals: trailing whitespace/CR stripped (never leading), one leading `/` or `**/`, one trailing `/`, `/*`, `/**`; the exact path always counts, a bare basename (`.run`) only while it floats; a negation counts as decided; broader patterns are not recognised (redundant append, documented). A refusal or a failed write (`{action:'failed', code}`) is returned, never thrown, and printed as a warning naming the lines (`gitignoreLines`, ONE wording for both commands and both dry runs). `.gitignore` is NOT a `planWriteTargets` target, so `init --yes` never refuses over it. A pending addition (`gitignorePending`, never a refused file) re-opens `update`'s same-version early return, bounded like `pendingRootMigration` (residual: a write that keeps failing). The dry runs carry it as `InitPlanReport.gitignore` / `UpdatePlanReport.gitignore` and the additive JSON keys `gitignore` / `plan.gitignore` (`gitignoreReport`), absent when nothing changes. The backup notice's advice is skipped when `backupDirIgnored(cwd, plan?)` — `pharn/.backup/` covered, or about to be added by this run. `uninstall` leaves pharn's lines. `root-migration.ts`'s old-entry hints keep their own normalizer. + **The `models` block is pharn-oss's** (schema and defaults: pharn-oss's `pharn/floor/check-model-config.mjs` and the block in its root `pharn.config.json`). Two things use it. Since pharn-oss 6.27.0, `/pharn-ship` and `/pharn-loop` READ `models.stages` at run time to pick the MODEL requested for the stage agents their routing policy routes (`ROUTE_POLICY` in pharn-oss's `stage-agent-core.mjs`; everything else, and `inherit` / `claude-*` ids, runs inline on the session's model); EFFORT is never routed. A person's direct `/pharn-` uses the command's static `model:`/`effort:` frontmatter, which the block is the SOURCE OF TRUTH held to (the checker's `agreement` mode) — and nothing in an installed project runs that check automatically. A routed model is a request, never proof of what ran. Editing a `/pharn-*` command's frontmatter makes that file `modified`, so `update` SKIPS it and withholds the `skillsVersion` bump; editing only `models.stages` causes no file drift (`update` keeps the block whole; a re-run `init` resets it). Every surface that prints the block says this (`modelsLabelLines`, `lib/model-config-format.ts`). The CLI owns no model/effort/stage rule of its own. **`lib/model-config.ts` is a pinned COPY of the checker's `validate`/`resolve` rules** (`PRODUCT_STAGES`, `MODEL_ALIASES`, `MODEL_ID_RE`, `EFFORT_LEVELS`, `checkModelsBlock`, `resolveStageModel`) — a copy, not a call, because the CLI never executes a file it installs (THREAT-MODEL §1; `status --strict` runs in CI on PRs, and at `init` the only checker is inside the untrusted clone). `tests/model-config-parity.test.ts` makes pharn-oss the one owner: it runs the vendored checker (`tests/fixtures/pharn-oss/check-model-config.mjs`, byte-for-byte, sha256-pinned with its upstream commit) over a corpus and compares every verdict, RED line and resolution with the copy's, reads the four sets out of the checker's source, and fails if the corpus stops reaching one of the checker's validate-path RED kinds. Never add a rule the checker does not have; the refresh procedure is in `docs/contributing.md`. When upstream widens its sets before the copy follows, a block using the new member is NOT applied and the rejection is named — never fatal (LIMITS §3e); upstream's `MIN_CLI` is the lever. `lib/upstream-models.ts` (`readUpstreamModels`) is the fetch-boundary read of the clone's root config (bounded to `MAX_UPSTREAM_CONFIG_BYTES`, never throwing: `absent` / `ok` / `invalid` with reasons). **`lib/models-update.ts`** holds the CLI's knowledge of its OWN pre-0.7.0 format (the 4-id alias map `opus-4-8→opus`, `sonnet-5→sonnet`, `fable-5→fable`, `haiku-4-5→haiku`; the two defaults it ever wrote, compared byte-for-byte as `JSON.stringify`), `convertLegacyModels` (top-level `default` → `stages.default` unless that is set or `stages` is not an object — then named as a leftover; old ids → aliases; everything else verbatim; own-property rebuilds so a `__proto__` key stays data), `needsModelsConversion`, and `decideModelsUpdate`, which CALLS `decideFileAction` with the block's hash (`modelsRecordHash`, stored in `pharn.records.json` under the non-path key `MODELS_RECORD_KEY` = `pharn.config.json#/models`, defined in `lib/install-records.ts`) as the file hash and a historical default fed in as proof of authorship — so the rows ARE the per-file rows: absent → restored; equal to pharn-oss's → ok; still pharn's → updated; else KEPT (`modified`/`unrecorded`/`unverifiable`), converted if old, its previous record carried; `--force` → `pharn.config.json` backed up, then replaced. Upstream absent/invalid → the user's block stays (converted if old). The block's outcome never withholds the `skillsVersion` bump (configuration, not bytes of a version), and `needsModelsConversion` re-opens `update`'s same-version early return so a current install is migrated too. `update` prints a `MODELS` note for every outcome but `ok`. **`pharn status` (`commands/status.ts`) is strictly read-only** — it never writes, deletes, or overwrites (fixing is `update`/`add`). Two sections: a **version** check (installed `skillsVersion` vs the upstream `SKILLS_VERSION` file, plus an archetype + capability-count summary) and a **drift** check. Default clones the source the install follows (the newest verified release for `ref: "latest"`, or the tip of `@main` when no `ref` is recorded) once and reuses it for both; `--no-drift` skips the clone and uses `fetchRemoteSkillsVersion` for the version section only; `--strict` exits 1 on any outdated/modified/missing/unreadable/not-shipped-upstream finding or an unwired hook (CI gate, default exit 0). The DRIFT note's `NOT SHIPPED UPSTREAM` section is `absentInstallSurfaces` at the CLONE's layout (`detectLayout(repo.dir)`), not the recorded one; a recorded layout that differs from the clone's prints a separate advisory `LAYOUT` note (`pharn update` moves the install) and is deliberately NOT a `--strict` input. `--json` carries both as additive keys: `drift.absentUpstream[{surface, path}]` and top-level `layout: {recorded, upstream}` (`upstream` null under `--no-drift`). Cleanup runs in a `finally`, and every `process.exit` happens _after_ it. The pure (no I/O) engine is **`lib/diff.ts` → `diffInstalledCapabilities`**: it mirrors `installCapabilities` to derive the **expected** file set — the selected capability dirs + the fixed product surfaces, at the recorded `layout` — then `contentHash`-compares each against the project root, returning `{modified, missing, okCount}`. Every read is `safeJoin`-guarded (from `lib/validate.ts`). `.claude/settings.json` is user-owned (preserved at install) and excluded from the file diff — but its `hooks` block is compared by `lib/hook-wiring.ts` (`diffHookWiring`: exact-string set difference over `Event · matcher · command · args`, against the UNION of the project's `.claude/settings.json` and `.claude/settings.local.json` — what Claude Code merges; user-level settings are never read. Every read is size-capped and opened `O_NONBLOCK` (a FIFO is refused, not waited on); upstream's file is symlink-refused at every component, while a project file may be a symlinked FILE (read through, as Claude Code does) but never sit under a symlinked `.claude/`. Only upstream entries are displayed, each as the hook's own JSON — exec vs shell form visible, paste-ready — with every `terminalSafe`-class character and U+2028/2029 turned into a `\u` JSON escape and each line capped at 300 chars); upstream hooks wired in neither file print a `HOOKS` note in `status` (and fail `--strict`, via `hookWiringFails`, as does an unreadable project file) and in `update` (report only — `update` still never writes the file); hooks wired only in `settings.local.json` are named as `local-only` and do NOT fail `--strict`; the copied-verbatim trusted docs (including `CONSTITUTION.md` / `pharn/CONSTITUTION.md`), hooks, contracts, and floor checkers ARE compared. Drift is derived live from the clone, always against that source, never the pinned `commit`. `status`'s DRIFT section does NOT use `pharn.records.json`, so it is a report, not a preview: a file it lists as differing may be cleanly upgraded OR skipped — only `update` can tell those apart. `status` reads the store for ONE thing — orphans (`statusOrphans` in `commands/status.ts` → `findOrphans`, against the same expected set as drift, frozen excluded): an `ORPHANED` note after DRIFT, and `--json` `orphans` (`null` under `--no-drift`, without a usable baseline, or when `detectLayout(clone) !== configLayout(config)`, where every recorded path would read as orphaned). Orphans are NOT a `--strict` input (`strictFails` is unchanged): no pharn command can clear one, since `update` deletes only the directories of capabilities it drops — never an orphan. Orphans and removals are disjoint by construction: `findOrphans` skips every key under a capability outside the next `capabilities`, which is exactly the set `removals` deletes. A `MODELS` note (both paths, no clone needed) shows the `models` block resolved for every product stage (`(default)` marks a fallback) under the truthful label, says `pharn update` converts an old-format block, or lists the pinned copy's REDs through `terminalSafe`; it is NOT a `--strict` input — the block's verdict is pharn-oss's checker's. **`status --json`** emits ONE object (`schemaVersion: 1`) on stdout and nothing else: `runStatus` branches to `runArchetypeStatusJson`, which calls no clack function that writes to stdout, builds the whole report first (`lib/status-report.ts` → `buildStatusReport`, pure; `drift`/`hooks`/`unknownCapabilities`/`orphans` are `null` under `--no-drift`), and prints once through `serializeStatusReport` (`JSON.stringify`, then every `terminalSafe`-class character plus U+2028/2029 as a `\u` escape — `escapeUnsafe`, single-sourced in `lib/terminal-safe.ts`, lossless: it parses back to the exact string). Config errors use `loadArchetypeConfigOrExit(cwd, { json: true })` and fatal failures `reportFatalPlain` (plain stderr, stdout empty, exit 1). A `--strict` finding still prints the report and sets `process.exitCode = 1` (no `process.exit`, so a large piped object is flushed); `strictFails` is the ONE predicate both modes use, and `versionStatus` (`lib/version-status.ts`) the one ahead/outdated computation. The proxy notice goes to stderr under `--json`. diff --git a/README.md b/README.md index 4a2fc78..b1d315c 100644 --- a/README.md +++ b/README.md @@ -168,10 +168,17 @@ As you run the workflow, PHARN writes one directory per increment — `RUN-REPORT.md`, and the machine-readable records beside them (`AC-TESTS.lock.json`, `cost.json`, the `*-report.json` files). Those are the durable record; commit them. `pharn/.run/` (runtime scratch; `.pharn/` before -pharn-oss 7.0.0), `pharn/.backup/` (see [Safety model](#safety-model)) and -`pharn/.lock*` are not — add them to your `.gitignore`. PHARN never edits -`.gitignore` for you; `pharn update` prints the replacement lines when yours still -names the old `.pharn/`, `.pharn-backup` or `.pharn.lock`. +pharn-oss 7.0.0) and `pharn/.backup/` (see [Safety model](#safety-model)) are not: +`pharn init` and `pharn update` append them to your `.gitignore` (creating it if +needed) unless it already covers them — in any common spelling, such as +`/pharn/.run`, `pharn/.run/**` or a negation `!pharn/.run/` you wrote on purpose. +They only ever append: your lines are never reordered or rewritten, CRLF stays +CRLF, and a second run changes nothing. A symlinked `.gitignore` is never written +through (you get a warning naming the lines). Lines added to `.gitignore` do not +untrack files you already committed (`git rm -r --cached pharn/.run` does). +`pharn/.lock*` is a transient file — add it yourself if you like. When your +`.gitignore` still names the old `.pharn/`, `.pharn-backup` or `.pharn.lock`, +`pharn update` prints the replacement lines; it never rewrites yours. Everything PHARN keeps in your project lives under `.claude/` and `pharn/`, plus the two JSON files at the root. An install made from a pharn-oss older than 7.0.0 diff --git a/docs/commands/add.md b/docs/commands/add.md index 63c62a5..25cb454 100644 --- a/docs/commands/add.md +++ b/docs/commands/add.md @@ -218,9 +218,10 @@ against today. > as soon as it is written, so it stays visible even if a later step fails. **Retention is yours**, exactly as for [`update --force`](update.md#--force-and-pharnbackup): `pharn` -never prunes `pharn/.backup/` and never edits your `.gitignore`, so the directories accumulate and are -committable by accident — and a picker run that overwrites edits in several capabilities writes **one -per capability**. Delete them once you are happy, or add `pharn/.backup/` to your `.gitignore`. +never prunes `pharn/.backup/`, so the directories accumulate — and a picker run that overwrites edits in +several capabilities writes **one per capability**. Delete them once you are happy. `add` itself never +edits `.gitignore`; `init` and `update` append `pharn/.backup/` to it, and the notice's advice to add it +is printed only while your `.gitignore` does not cover it. ## The capability argument diff --git a/docs/commands/init.md b/docs/commands/init.md index d630d10..c7a914e 100644 --- a/docs/commands/init.md +++ b/docs/commands/init.md @@ -196,9 +196,15 @@ losslessly. `--json` is accepted only together with `--dry-run`: `pharn init --j }, // null unless preserved "models": { "outcome": "copy", "reasons": [] }, // "copy" | "none" | "rejected" "config": { "carriedKeys": ["testResults"] }, // your own keys, by name only + "gitignore": { "action": "create", "entries": ["pharn/.run/", "pharn/.backup/"] }, // present only when .gitignore would change } ``` +`gitignore` is present **only** when the install would change [`.gitignore`](#gitignore): `action` is +`create` or `append` with the lines it adds, or `refused` with a `reason` (`symlink`, `not-a-file`, +`unreadable`) and the lines to add yourself. With nothing to add the key is absent, so the report is +byte-identical to earlier releases. Added in 0.16.0. + `archetypeSource`, `detectedArchetypes` and `archetypeEvidence` say where `archetypes` came from. `archetypeEvidence` is always detection's — one entry per archetype detection found, with the dependency name (`"source": "dependency"`, or `"devDependency"` when only `devDependencies` lists it), the first @@ -502,6 +508,7 @@ cannot find them; PHARN still runs them. Delete them by hand. | Write `pharn.config.json` | `pharnVersion`, `skillsVersion` (from `SKILLS_VERSION`), `repo`, `commit`, `installedAt`, `archetypes`, `capabilities` (`source: "auto"`, or `"manual"` for a kept `pharn add`), `layout`, `models`, `seam`, `modules: []`, `frozenCapabilities` (see above) | | Copy the `models` block | pharn-oss's own, verbatim, from its root `pharn.config.json`; none when pharn-oss ships none, and none — with the reason — when it fails pharn-oss's rules as this pharn knows them ([Models](../reference/pharn-config.md#models)) | | Write `pharn.records.json` | A sha256 of every file the install wrote, so [`pharn update`](update.md) can keep your later edits ([reference](../reference/pharn-records.md)) | +| Update `.gitignore` | Appends `pharn/.run/` and `pharn/.backup/` when it does not already cover them, creating the file if needed ([below](#gitignore)) | The install also copies pharn-oss's Apache-2.0 `LICENSE` — to `pharn/LICENSE`, or `PHARN-LICENSE` at the root in the legacy flat layout. The destination is deliberately **not** a plain root `LICENSE`: @@ -517,6 +524,32 @@ The four trusted docs keep the paths pharn-oss keeps them at, and those are not to `pharn/.backup//` before the copy; `init --yes` refuses instead. Rename yours first if you want to keep it where it is. +#### `.gitignore` + +PHARN's runtime scratch (`pharn/.run/`, where pharn-oss's floor keeps stage logs, gate outputs and cost +markers) and pharn's backups (`pharn/.backup/`) must never be committed: a tracked run directory makes +pharn-oss's reconcile count PHARN's own files as build escapes. So, after everything else is written, +`init` makes sure your root `.gitignore` covers both: + +- **Created** with a comment and both lines when it does not exist. +- **Appended to** with only the missing line(s) otherwise. Your bytes are kept exactly — nothing is + reordered, rewritten or removed — and the new lines use the file's line endings (CRLF when its first + line ends in CRLF). A file that does not end in a newline gets one before pharn's lines. +- **Left alone** when it already covers both, in any of the spellings git treats the same way: + `pharn/.run/`, `/pharn/.run`, `pharn/.run`, `pharn/.run/*`, `pharn/.run/**`, `**/pharn/.run/`, or a + bare `.run/`. A negation you wrote on purpose (`!pharn/.run/`) counts as your decision and is never + overridden. Broader patterns (`pharn/`, `*`) are not recognised; pharn then appends its own line, + which is redundant but harmless. +- **Never written through a symlink**, nor when it is not a regular file or cannot be read. The install + still succeeds, with a warning naming the lines to add yourself; a failed write is reported the same + way. + +A second run changes nothing. The outro says `.gitignore created` or `.gitignore updated`. `--dry-run` +prints the line it would add, and `--dry-run --json` carries it as `gitignore`. `.gitignore` is **not** +one of the [write targets](#install-without-prompts---yes), so `init --yes` never refuses because of it. +A `.gitignore` line does not untrack files you already committed: `git rm -r --cached pharn/.run` +does that. + #### A surface pharn-oss does not ship Every product surface `init` copies is optional on the way in: a clone that lacks one still installs. diff --git a/docs/commands/uninstall.md b/docs/commands/uninstall.md index d2c4c5d..d9a4caa 100644 --- a/docs/commands/uninstall.md +++ b/docs/commands/uninstall.md @@ -67,7 +67,8 @@ The backup keeps whatever only the config held: your `models` and `seam` blocks Before anything is written, `uninstall` prints a `PLAN` note: how many files it will delete, the files it keeps because you changed them, paths it cannot verify, recorded paths outside the install paths, and how many recorded files are already gone. It always ends with the reminder that `.claude/settings.json`, -`.claude/settings.local.json`, and every `pharn/.backup/` directory are left untouched. +`.claude/settings.local.json`, every `pharn/.backup/` directory and the lines pharn added to `.gitignore` +are left untouched (they are harmless without PHARN; delete them by hand if you like). In a terminal it then asks one question, defaulting to **No**: diff --git a/docs/commands/update.md b/docs/commands/update.md index 43a4840..62cbb93 100644 --- a/docs/commands/update.md +++ b/docs/commands/update.md @@ -225,6 +225,7 @@ key. Treat a `status` you do not recognise as "do not apply". | `plan.rootMigration` | Always present: what a real run would do with the [old root footprint](#the-root-footprint-migration), as `{docs, moves, scratchNotice, gitignoreHints}`, or `null` when nothing applies (a flat clone, or one below SKILLS 7 that still keeps both docs at the root). `docs[]` is `{rel, action}` for root `THREAT-MODEL.md` / `LIMITS.md` (`delete`, `keep-edited`, `absent`, `unverifiable`); `moves[]` is `{from, to, action}` for `memory-bank/` and `pharn.spec-template.md` (`move`, `conflict`, `refused-symlink`, `unreadable`, `absent`; empty below SKILLS 7); `scratchNotice` says the old `.pharn/` holds more than `ci.yml`; `gitignoreHints[]` are the replacement `.gitignore` lines. The files the moves would back up are in `plan.backups`. Added in 0.14.0 within `1`. | | `frozenCapabilities`, `unknownCapabilities` | Capabilities upstream ships that this CLI cannot read, kept as they are: `role:name` keys, and `{role, name, subtree, reason}`. | | `plan.recordsNote` | Why `pharn.records.json` could not be used (then every difference is `unverifiable`), else `null`. | +| `plan.gitignore` | Present only when a real run would change [`.gitignore`](#gitignore): `{action, entries}` with `action` `create` or `append` and `entries` the lines it adds, or `{action: "refused", reason, entries}` (`reason` `symlink`, `not-a-file` or `unreadable`) when it would have to leave the file alone. Absent otherwise, so such a report is byte-identical to earlier releases. Added in 0.16.0 within `1`. | | `plan.orphans` | Present only when there is at least one: every [orphaned file](#orphaned-files) as `{path, state}` — `state` is `unchanged`, `edited` or `unreadable` (then with a `reason`). With none, the key is **absent** (not `[]`), so such a report is byte-identical to the one 0.10.0 printed; read it as `plan.orphans ?? []`. Never filtered by `--compact`. | #### `--compact`: leave out the unchanged files @@ -519,9 +520,9 @@ To restore a file, copy it back: cp pharn/.backup/20260807-091500/pharn/CONSTITUTION.md pharn/CONSTITUTION.md ``` -**Retention is yours.** `pharn` never prunes `pharn/.backup/` and never edits your `.gitignore` — so -backups accumulate and are committable by accident. Delete them once you are happy, or add -`pharn/.backup/` to your `.gitignore`. +**Retention is yours.** `pharn` never prunes `pharn/.backup/`, so backups accumulate. They are kept out +of git by the `pharn/.backup/` line `update` (and `init`) append to your `.gitignore` — see +[`.gitignore`](#gitignore). Delete them once you are happy. ## Release channel @@ -848,8 +849,9 @@ it as `plan.rootMigration`): - **`.pharn/`** (pharn-oss's runtime scratch) is never moved or deleted. When it holds anything but pharn-ci's `ci.yml`, the note says the scratch now lives in `pharn/.run/` and the old contents can be deleted. -- **`.gitignore`** is never edited. When it names `.pharn/`, `.pharn-backup` or `.pharn.lock`, the note - prints the replacements: `pharn/.run/`, `pharn/.backup/`, `pharn/.lock*`. +- **`.gitignore`**: your lines are never rewritten. When it names `.pharn/`, `.pharn-backup` or + `.pharn.lock`, the note prints the replacements: `pharn/.run/`, `pharn/.backup/`, `pharn/.lock*`. (The + first two are appended for you anyway — see [`.gitignore`](#gitignore).) A real `update` prints the note in the future tense before it asks `Apply this update?` (and under `--yes`), and, after taking the lock, refuses with exit 1 if what it would delete or move is no longer @@ -875,6 +877,23 @@ on every run and fails `--strict` while it lasts. Hooks are installed as `.cjs` `*.test.cjs` / `*.test.mjs`); an `.mjs` hook's name is held to the same filename allowlist as a `.cjs` one, so a name outside it refuses the run. +## `.gitignore` + +Like [`init`](init.md#gitignore), every applied `update` makes sure your root `.gitignore` covers +`pharn/.run/` (PHARN's runtime scratch) and `pharn/.backup/` (these backups), appending only the +missing lines — created if absent, your bytes kept exactly, CRLF kept, equivalent spellings and your +own negations respected, never written through a symlink. It runs after the config write, under the +lock, and prints `Added … to .gitignore` (or `Created .gitignore with …`); a refusal or a failed write +is a warning naming the lines, never a failed update. + +**At the same skills version**, a `.gitignore` missing either line re-opens the +"Already up to date" return (the header note says so), so an install made before 0.16.0 gets the lines +on its next `update`; the following run returns early again. A `.gitignore` pharn cannot write (a +symlink, a directory, unreadable) does not re-open it. One residual: a regular file whose write keeps +failing (no permission) re-opens every run, each with its warning. + +`--dry-run` prints `.gitignore: would add …`, and `--dry-run --json` carries `plan.gitignore`. + ## What is protected by default - `.claude/settings.json` — your Claude Code configuration. `init` writes it only when absent; `update` diff --git a/package-lock.json b/package-lock.json index d18f174..7f0f180 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@pharn-dev/pharn", - "version": "0.15.0", + "version": "0.16.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@pharn-dev/pharn", - "version": "0.15.0", + "version": "0.16.0", "license": "Apache-2.0", "dependencies": { "@clack/prompts": "^1.8.1", diff --git a/package.json b/package.json index db75780..a410d2f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@pharn-dev/pharn", - "version": "0.15.0", + "version": "0.16.0", "description": "Install and update PHARN in any git project. PHARN gets features shipped right with Claude Code, with spec, plan and checks kept in Git.", "keywords": [ "claude-code", diff --git a/src/commands/add.ts b/src/commands/add.ts index 6c98f09..1b3961b 100644 --- a/src/commands/add.ts +++ b/src/commands/add.ts @@ -15,6 +15,7 @@ import { } from '../steps/add-picker.js'; import { interactiveAllowed } from '../lib/capability-picker.js'; import { createBackup, printBackupNotice } from '../lib/backup.js'; +import { backupDirIgnored } from '../lib/gitignore.js'; import { scanDest } from '../lib/dest-drift.js'; import { installCapabilityDirs } from '../lib/install-capabilities.js'; import { capabilityCloneFiles } from '../lib/install-manifest.js'; @@ -525,6 +526,7 @@ async function resolveArchetypeAdd( dir: backupDir, saved: scan.drifted.length, action: 'overwriting', + gitignored: backupDirIgnored(cwd), }); } diff --git a/src/commands/init.ts b/src/commands/init.ts index 62762b0..8d595c2 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -1,3 +1,4 @@ +import { gitignoreLines } from '../lib/gitignore.js'; import { rootMigrationNoteLines } from '../lib/root-migration.js'; import { intro, log, note, outro, spinner } from '@clack/prompts'; import { showBanner } from '../lib/banner.js'; @@ -663,6 +664,10 @@ function reportInitDryRun(r: InitPlanReport): void { ` Would preserve the existing ${CLAUDE_SETTINGS_FILE} — pharn never overwrites it.`, ); } + // What the install would append to `.gitignore` (lib/gitignore.ts owns the + // wording); a refusal is a warning, printed after the note. + const gitignore = gitignoreLines(r.gitignore, { preview: true }); + if (gitignore !== null && !gitignore.warn) lines.push(` ${gitignore.text}`); lines.push( '', deletes.length > 0 @@ -672,6 +677,7 @@ function reportInitDryRun(r: InitPlanReport): void { : ` \`pharn init --yes\` would refuse: ${r.counts.overwrite} existing file(s) would be overwritten.`, ); note(lines.join('\n'), 'PLAN'); + if (gitignore?.warn) log.warn(gitignore.text); // The ROOT MIGRATION note a real re-run prints after the install, as a // preview (lib/root-migration.ts owns the wording). diff --git a/src/commands/remove.ts b/src/commands/remove.ts index 8b3c37b..176c713 100644 --- a/src/commands/remove.ts +++ b/src/commands/remove.ts @@ -18,6 +18,7 @@ import { import { parseCapabilityArg } from '../lib/capability-address.js'; import { errorMessage, logError } from '../lib/report-error.js'; import { createBackup, printBackupNotice } from '../lib/backup.js'; +import { backupDirIgnored } from '../lib/gitignore.js'; import { removalBackupSet } from '../lib/removal-backup.js'; import { buildRemoveSelection, @@ -171,6 +172,7 @@ function backupBeforeDelete( dir: backupDir, saved: rels.length, action: 'removing', + gitignored: backupDirIgnored(cwd), }); } catch (err) { throw new RemoveBackupError( diff --git a/src/commands/uninstall.ts b/src/commands/uninstall.ts index 974f0e4..0157d4f 100644 --- a/src/commands/uninstall.ts +++ b/src/commands/uninstall.ts @@ -3,6 +3,7 @@ import { posix } from 'node:path'; import { confirm, intro, isCancel, note, outro } from '@clack/prompts'; import pc from '../lib/color.js'; import { BACKUP_DIR, createBackup, printBackupNotice } from '../lib/backup.js'; +import { backupDirIgnored } from '../lib/gitignore.js'; import { interactiveAllowed } from '../lib/capability-picker.js'; import { cancelAndExit } from '../lib/confirm.js'; import { displayHook, hooksReferencing } from '../lib/hook-wiring.js'; @@ -194,7 +195,11 @@ async function applyUninstall( `Could not back up ${CONFIG_FILENAME} before uninstalling: ${errorMessage(err).replace(/\.$/, '')}. Nothing was removed.`, ); } - printBackupNotice({ dir: backupDir, saved: CONFIG_FILENAME }); + printBackupNotice({ + dir: backupDir, + saved: CONFIG_FILENAME, + gitignored: backupDirIgnored(cwd), + }); const deleted: string[] = []; for (const rel of plan.delete) { @@ -274,7 +279,7 @@ function listed(rels: readonly string[]): string[] { return more > 0 ? [...shown, ` …and ${more} more`] : shown; } -const UNTOUCHED = `${CLAUDE_SETTINGS_FILE}, .claude/settings.local.json and every ${BACKUP_DIR}/ directory are left untouched.`; +const UNTOUCHED = `${CLAUDE_SETTINGS_FILE}, .claude/settings.local.json, every ${BACKUP_DIR}/ directory and the lines pharn added to .gitignore are left untouched.`; function planLines(plan: UninstallPlan): string[] { const lines = [ diff --git a/src/commands/update.ts b/src/commands/update.ts index 22cb9ea..2fd2baa 100644 --- a/src/commands/update.ts +++ b/src/commands/update.ts @@ -45,6 +45,13 @@ import { minCliGate } from '../lib/min-cli-gate.js'; import { PHARN_VERSION } from '../version.js'; import { applyWrites, ApplyError } from '../lib/apply-update.js'; import { BACKUP_DIR, createBackup, legacyBackupNotice } from '../lib/backup.js'; +import { + applyGitignore, + backupDirIgnored, + gitignoreLines, + gitignorePending, + planGitignore, +} from '../lib/gitignore.js'; import { buildRecords, MODELS_RECORD_KEY, @@ -373,6 +380,15 @@ async function runArchetypeUpdate( current && !force ? pendingRootMigration(cwd, config, baselineRecords) : null; + // A project `.gitignore` that does not yet cover PHARN's run state and + // backups re-opens the gate too (lib/gitignore.ts) — the install predates + // 0.16.0, or someone deleted a line. Local and read-only, and bounded like + // `missing`: only a `.gitignore` the run can write counts (absent, or a + // regular file — never a symlink or an unreadable one), the opened run + // appends the lines, and the next run early-returns. Residual: a regular + // file whose write fails (EACCES) re-opens every run, named in its warning. + const gitignoreMissing = + current && !force && gitignorePending(planGitignore(cwd)); if ( current && !force && @@ -381,7 +397,8 @@ async function runArchetypeUpdate( !switchChannel && missing === null && !staleRecords && - rootDoc === null + rootDoc === null && + !gitignoreMissing ) { if (json) { emitEarlyReturn('up-to-date'); @@ -441,6 +458,13 @@ async function runArchetypeUpdate( ), ] : []), + ...(gitignoreMissing + ? [ + pc.dim( + " .gitignore does not yet keep PHARN's run state and backups out of git — this run re-applies the install and adds them.", + ), + ] + : []), ...(staleRecords ? [ pc.dim( @@ -635,6 +659,10 @@ async function runArchetypeUpdate( // Before the report, whose outro is the run's last line. const legacyBackups = legacyBackupNotice(cwd); if (legacyBackups) log.info(legacyBackups); + const gitignore = + outcome.gitignore === undefined ? null : gitignoreLines(outcome.gitignore); + if (gitignore?.warn) log.warn(gitignore.text); + else if (gitignore !== null) log.info(gitignore.text); reportOutcome(outcome, force); // A user-data move that could not be made (a conflict, a symlink, a failed // rename) fails the run, after everything else it did has been reported and @@ -838,6 +866,9 @@ async function applyPlannedUpdate( deletes, apply.rootMoveBackups.length > 0, ), + // This run adds `pharn/.backup/` to `.gitignore` when it is missing + // (after the config write, below), so the advice would be stale. + gitignored: backupDirIgnored(cwd, report.gitignore), } : null; if (backup) onBackup(backup); @@ -952,6 +983,12 @@ async function applyPlannedUpdate( models: models.next, }); + // Keep PHARN's run state and backups out of git (lib/gitignore.ts): planned + // again here, under the lock, and appended after every pharn-owned write. A + // refusal or a failed write is reported, never thrown — the update above has + // already landed. + const gitignore = await applyGitignore(cwd); + // The user-data moves, LAST: everything pharn-owned is in place, so a move // that fails (or was refused at plan time) costs nothing but itself, and the // caller turns it into exit 1 after the report. @@ -977,6 +1014,7 @@ async function applyPlannedUpdate( models, modelsFloor: report.modelsFloor, orphans: report.orphans, + gitignore, rootMigration: migration, rootMigrationResult: migration === null ? null : { ...docResult, ...moveResult }, diff --git a/src/lib/atomic-write.ts b/src/lib/atomic-write.ts index 4427630..a059786 100644 --- a/src/lib/atomic-write.ts +++ b/src/lib/atomic-write.ts @@ -220,11 +220,31 @@ export function copyFileAtomicSync(from: string, target: string): void { export async function writeJsonAtomic( target: string, value: unknown, +): Promise { + await writeFileAtomic(target, `${JSON.stringify(value, null, 2)}\n`); +} + +/** + * `writeJsonAtomic`'s replacement for arbitrary content: a string is written as + * utf8, a Buffer byte for byte (lib/gitignore.ts appends to the user's + * `.gitignore` this way, so its bytes are never re-encoded). Every property + * documented on `writeJsonAtomic` is this function's: the exclusive temp, the + * copied mode of an existing regular file, a symlinked target replaced rather + * than followed, a hard-linked target's other names left with the old bytes. + * + * A NEW target gets the mode an ordinary `writeFile` gives it (0o666 masked by + * the umask): the async temp is opened with the default mode, unlike + * `copyFileAtomicSync`'s 0o600 temp, which always takes the source's mode. + */ +export async function writeFileAtomic( + target: string, + data: string | Buffer, ): Promise { const { tmp, handle } = await createExclusiveTemp(target); try { try { - await handle.writeFile(`${JSON.stringify(value, null, 2)}\n`, 'utf8'); + if (typeof data === 'string') await handle.writeFile(data, 'utf8'); + else await handle.writeFile(data); } catch (err) { // The write's error is the cause; a close error here must not mask it. await handle.close().catch(() => {}); diff --git a/src/lib/backup.ts b/src/lib/backup.ts index e2374ac..e8fb1ae 100644 --- a/src/lib/backup.ts +++ b/src/lib/backup.ts @@ -202,6 +202,11 @@ export interface BackupNotice { saved: number | string; /** What the copies were made ahead of; omitted, the sentence names nothing. */ action?: BackupAction; + /** + * `pharn/.backup/` is already out of git, or this run adds it to `.gitignore` + * (lib/gitignore.ts → `backupDirIgnored`). True drops the advice to add it. + */ + gitignored?: boolean; } /** @@ -210,7 +215,8 @@ export interface BackupNotice { * `update`'s carried the `.gitignore` advice). * * The pointer comes first; the advice follows it, once, when the backup is - * made. `aborted` is `update`'s failure path, which repeats the pointer on + * made — and only while `pharn/.backup/` is not gitignored (`init` and `update` + * add it to `.gitignore`, so after them the advice would be noise). `aborted` is `update`'s failure path, which repeats the pointer on * stderr (what `2> err.log` keeps) — advice, not a pointer, so it is not * repeated there. */ @@ -222,7 +228,7 @@ export function backupNoticeLines( typeof notice.saved === 'number' ? `${notice.saved} file(s)` : notice.saved; const before = notice.action === undefined ? '' : ` before ${notice.action}`; const lines = [`Backed up ${what} to ${notice.dir}${before}.`]; - if (opts.aborted !== true) { + if (opts.aborted !== true && notice.gitignored !== true) { lines.push( `${BACKUP_DIR}/ is not gitignored — add it to .gitignore or delete it once you are happy.`, ); diff --git a/src/lib/gitignore.ts b/src/lib/gitignore.ts new file mode 100644 index 0000000..e898d83 --- /dev/null +++ b/src/lib/gitignore.ts @@ -0,0 +1,337 @@ +import { lstatSync } from 'node:fs'; +import { readBoundedFile } from './bounded-read.js'; +import { writeFileAtomic } from './atomic-write.js'; +import { safeJoin } from './validate.js'; + +// --------------------------------------------------------------------------- +// pharn's lines in the project's root `.gitignore`. One axis (P3): make sure +// git never tracks PHARN's runtime scratch (`pharn/.run/`, written by +// pharn-oss's floor at a fixed path for every layout) or this CLI's backups +// (`pharn/.backup/`). A tracked run directory turned a correct build into 55 +// "escapes" in pharn-oss's reconcile (pharn-oss #408), and a backup holds the +// user's pre-overwrite bytes, which do not belong in a commit. +// +// APPEND-ONLY. The new file is the old file's exact bytes followed by pharn's +// block — never a decode/re-encode, never a reorder, never a removed line. An +// entry already covered (in any recognised spelling, or deliberately negated) +// is not added, so a second run writes nothing. +// +// What this does NOT promise (advisory, P0): a `.gitignore` line does not +// untrack a file already committed, and a broader pattern that also covers the +// path (`pharn/`, `pharn/.*`, `*`) is not recognised — pharn then appends its +// own line, which is redundant and harmless. +// +// Trust (P2): `.gitignore` is project content and may be hostile. It is read +// bounded (lib/bounded-read.ts) and only compared against fixed literals; its +// text never reaches a terminal and never drives a path. The only write target +// is the fixed root `.gitignore`, replaced atomically — never written through a +// symlink or into a hard-linked inode (lib/atomic-write.ts). +// --------------------------------------------------------------------------- + +export const GITIGNORE_FILE = '.gitignore'; + +/** The lines pharn makes sure the project's `.gitignore` covers, in order. */ +export const PHARN_GITIGNORE_ENTRIES = [ + 'pharn/.run/', + 'pharn/.backup/', +] as const; + +export type GitignoreEntry = (typeof PHARN_GITIGNORE_ENTRIES)[number]; + +/** Written above pharn's entries, once per file. */ +export const GITIGNORE_COMMENT = + '# PHARN run state and update backups (added by pharn)'; + +/** Why `.gitignore` was left alone. Fixed values, never file content. */ +export type GitignoreRefusal = 'symlink' | 'not-a-file' | 'unreadable'; + +/** What a run would do (or did) to `.gitignore`. */ +export type GitignorePlan = + | { action: 'none' } + | { action: 'create' | 'append'; entries: GitignoreEntry[] } + | { action: 'refused'; reason: GitignoreRefusal; entries: GitignoreEntry[] }; + +/** A plan, plus what applying it can add: a write that failed. */ +export type GitignoreOutcome = + GitignorePlan | { action: 'failed'; code: string; entries: GitignoreEntry[] }; + +// A gitignore pattern reduced to what decides whether it covers one of our +// directories. `floating` = it matches at any depth (no slash before its end, +// or a leading `**/`). +interface Pattern { + negated: boolean; + core: string; + floating: boolean; +} + +// One line → its pattern, or null for a blank line or a comment. Git strips +// unescaped TRAILING spaces but keeps leading ones (they are part of the +// pattern), so only the end is trimmed; one trailing CR is a CRLF file's line +// ending. Then one leading `/` (anchored) or `**/` (any depth), and one +// trailing `/`, `/*` or `/**` (the directory itself, or everything in it). +function parsePattern(raw: string): Pattern | null { + const line = (raw.endsWith('\r') ? raw.slice(0, -1) : raw).replace( + /[ \t]+$/, + '', + ); + if (line === '' || line.startsWith('#')) return null; + let p = line; + const negated = p.startsWith('!'); + if (negated) p = p.slice(1); + let anyDepth = false; + let anchored = false; + if (p.startsWith('**/')) { + anyDepth = true; + p = p.slice(3); + } else if (p.startsWith('/')) { + anchored = true; + p = p.slice(1); + } + // `/*` and `/**` put a slash before the end, which anchors the pattern; a + // lone trailing `/` only limits it to directories. + const suffix = /(\/\*\*|\/\*|\/)$/.exec(p)?.[0] ?? ''; + p = p.slice(0, p.length - suffix.length); + const floating = + anyDepth || + (!anchored && suffix !== '/*' && suffix !== '/**' && !p.includes('/')); + return { negated, core: p, floating }; +} + +// Does `pattern` decide `entry`? Its exact path does (anchored or not, both +// include the root one); a bare basename (`.run`) does only while it floats. +function decides(pattern: Pattern, entry: GitignoreEntry): boolean { + const path = entry.slice(0, -1); // 'pharn/.run' + const base = path.slice(path.lastIndexOf('/') + 1); // '.run' + return pattern.core === path || (pattern.core === base && pattern.floating); +} + +// A UTF-8 byte-order mark, as latin1 decodes it. Git strips one at the start +// of a `.gitignore`, so the first line is read without it too — detection +// only: the file's bytes are never changed. +const UTF8_BOM = '\u00ef\u00bb\u00bf'; + +function lines(bytes: Buffer): string[] { + // latin1 is a 1:1 byte decode: our literals are ASCII, and no byte is lost + // or merged (the bytes are never re-encoded — only appended to). + const text = bytes.toString('latin1'); + return (text.startsWith(UTF8_BOM) ? text.slice(UTF8_BOM.length) : text).split( + '\n', + ); +} + +/** + * The entries `bytes` (a `.gitignore`'s content) already decides — covered by a + * pattern, or deliberately negated (`!pharn/.run/`), which is the user's call + * and never overridden. + */ +export function decidedEntries(bytes: Buffer): Set { + const patterns = lines(bytes) + .map(parsePattern) + .filter((p): p is Pattern => p !== null); + return new Set( + PHARN_GITIGNORE_ENTRIES.filter((entry) => + patterns.some((p) => decides(p, entry)), + ), + ); +} + +/** + * The bytes to APPEND to `bytes` (null = no file yet) so that every entry is + * decided, and which entries they add. Empty when nothing is missing. + * + * Line endings follow the file: CRLF when its first line ending is `\r\n`, + * else LF (and LF for a new file). A non-empty file that does not end in a line + * ending gets one first, so pharn's block starts on a line of its own. The + * comment is written only when the file does not already hold it. + */ +export function gitignoreAddition(bytes: Buffer | null): { + entries: GitignoreEntry[]; + append: Buffer; +} { + const old = bytes ?? Buffer.alloc(0); + const decided = decidedEntries(old); + const entries = PHARN_GITIGNORE_ENTRIES.filter((e) => !decided.has(e)); + if (entries.length === 0) return { entries, append: Buffer.alloc(0) }; + const text = old.toString('latin1'); + const firstLf = text.indexOf('\n'); + const eol = firstLf > 0 && text[firstLf - 1] === '\r' ? '\r\n' : '\n'; + const hasComment = lines(old).some( + (l) => l.replace(/[ \t\r]+$/, '') === GITIGNORE_COMMENT, + ); + const parts: string[] = []; + if (text.length > 0 && !text.endsWith('\n')) parts.push(eol); + if (!hasComment) parts.push(GITIGNORE_COMMENT + eol); + for (const entry of entries) parts.push(entry + eol); + return { entries, append: Buffer.from(parts.join(''), 'latin1') }; +} + +// The plan plus the bytes it was made from, so apply writes exactly what it +// planned over. +function readPlan(projectRoot: string): { + plan: GitignorePlan; + old: Buffer | null; + append: Buffer; +} { + const all = [...PHARN_GITIGNORE_ENTRIES]; + const target = safeJoin(projectRoot, GITIGNORE_FILE); + const refused = (reason: GitignoreRefusal) => ({ + plan: { action: 'refused' as const, reason, entries: all }, + old: null, + append: Buffer.alloc(0), + }); + let isLink: boolean; + try { + isLink = lstatSync(target).isSymbolicLink(); + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== 'ENOENT') + return refused('unreadable'); + const { entries, append } = gitignoreAddition(null); + return { plan: { action: 'create', entries }, old: null, append }; + } + // A link is never written through, nor replaced (it is the user's). + if (isLink) return refused('symlink'); + const read = readBoundedFile(target); + if (read.kind === 'absent') return refused('unreadable'); // gone since lstat + if (read.kind === 'unusable') + return refused( + read.code === 'not-a-file' || read.code === 'EISDIR' + ? 'not-a-file' + : 'unreadable', + ); + const { entries, append } = gitignoreAddition(read.bytes); + if (entries.length === 0) + return { plan: { action: 'none' }, old: read.bytes, append }; + return { plan: { action: 'append', entries }, old: read.bytes, append }; +} + +/** What a run would do to `.gitignore`. Read-only; never throws. */ +export function planGitignore(projectRoot: string): GitignorePlan { + return readPlan(projectRoot).plan; +} + +/** + * Make `.gitignore` cover pharn's entries: plan again (callers run this under + * the project lock, after the install wrote everything else), then write the + * old bytes plus the addition atomically. A refusal writes nothing; a failed + * write is returned, never thrown — the install it follows has already landed. + */ +export async function applyGitignore( + projectRoot: string, +): Promise { + const { plan, old, append } = readPlan(projectRoot); + if (plan.action !== 'create' && plan.action !== 'append') return plan; + try { + await writeFileAtomic( + safeJoin(projectRoot, GITIGNORE_FILE), + Buffer.concat([old ?? Buffer.alloc(0), append]), + ); + return plan; + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + return { + action: 'failed', + code: + typeof code === 'string' && /^[A-Z0-9_]{1,32}$/.test(code) + ? code + : 'EUNKNOWN', + entries: plan.entries, + }; + } +} + +/** Would a run add an entry (a plan pharn can carry out)? */ +export function gitignorePending(plan: GitignorePlan): boolean { + return plan.action === 'create' || plan.action === 'append'; +} + +/** + * Is `pharn/.backup/` already out of git, or about to be (`plan` adds it)? The + * backup notice's advice is printed only when neither holds. + */ +export function backupDirIgnored( + projectRoot: string, + plan: GitignorePlan | null = null, +): boolean { + if ( + plan !== null && + gitignorePending(plan) && + 'entries' in plan && + plan.entries.includes('pharn/.backup/') + ) + return true; + const read = readBoundedFile(safeJoin(projectRoot, GITIGNORE_FILE)); + return read.kind === 'ok' && decidedEntries(read.bytes).has('pharn/.backup/'); +} + +const REFUSAL_TEXT: Record = { + symlink: 'it is a symlink, and pharn never writes through one', + 'not-a-file': 'it is not a regular file', + unreadable: 'it could not be read', +}; + +/** + * The lines that report `outcome` — ONE wording for `init`, `update` and both + * dry runs. `preview` = future tense (a dry run). Null when there is nothing to + * say. `warn` marks lines a caller prints as a warning. + */ +export function gitignoreLines( + outcome: GitignoreOutcome, + opts: { preview?: boolean } = {}, +): { text: string; warn: boolean } | null { + const list = (entries: readonly string[]) => entries.join(', '); + switch (outcome.action) { + case 'none': + return null; + case 'create': + return { + text: opts.preview + ? `.gitignore: would create it with ${list(outcome.entries)} (PHARN's run state and update backups).` + : `Created .gitignore with ${list(outcome.entries)} (PHARN's run state and update backups).`, + warn: false, + }; + case 'append': + return { + text: opts.preview + ? `.gitignore: would add ${list(outcome.entries)} (PHARN's run state and update backups).` + : `Added ${list(outcome.entries)} to .gitignore (PHARN's run state and update backups).`, + warn: false, + }; + case 'refused': + case 'failed': { + const why = + outcome.action === 'refused' + ? REFUSAL_TEXT[outcome.reason] + : `writing it failed (${outcome.code})`; + return { + text: `.gitignore ${opts.preview ? 'would not be' : 'was not'} updated: ${why}. Add these lines yourself so PHARN's run state is never committed: ${list(outcome.entries)}`, + warn: true, + }; + } + } +} + +/** The `--dry-run --json` projection of a plan (init and update share it). */ +export type GitignoreReport = + | { action: 'create' | 'append'; entries: string[] } + | { action: 'refused'; reason: GitignoreRefusal; entries: string[] }; + +/** + * The JSON form of `plan`, or null when nothing would change — the caller then + * omits the key, so a report with nothing to add keeps its earlier bytes. + * Built field by field: every value is a fixed literal. + */ +export function gitignoreReport(plan: GitignorePlan): GitignoreReport | null { + switch (plan.action) { + case 'none': + return null; + case 'create': + case 'append': + return { action: plan.action, entries: [...plan.entries] }; + case 'refused': + return { + action: 'refused', + reason: plan.reason, + entries: [...plan.entries], + }; + } +} diff --git a/src/lib/init-plan.ts b/src/lib/init-plan.ts index 2b2942a..7255da7 100644 --- a/src/lib/init-plan.ts +++ b/src/lib/init-plan.ts @@ -1,3 +1,4 @@ +import { planGitignore, type GitignorePlan } from './gitignore.js'; import { planRootMigration, type RootMigration, @@ -407,6 +408,9 @@ export interface InitPlanReport { // null — always null for a first install, which has nothing of pharn's at // the root to retire and must never move the user's data unasked. rootMigration: RootMigration | null; + // What the install would append to the project's `.gitignore` + // (lib/gitignore.ts); `none` when it already covers pharn's entries. + gitignore: GitignorePlan; } /** @@ -596,5 +600,6 @@ export function buildInitPlan(params: { absentUpstream: absentInstallSurfaces({ repoDir, layout }), carriedConfigKeys: Object.keys(readCarriedEntries(cwd)), rootMigration: rootMigration?.report ?? null, + gitignore: planGitignore(cwd), }; } diff --git a/src/lib/init-report.ts b/src/lib/init-report.ts index c77eb12..19a51c5 100644 --- a/src/lib/init-report.ts +++ b/src/lib/init-report.ts @@ -1,3 +1,4 @@ +import { gitignoreReport, type GitignoreReport } from './gitignore.js'; import type { ArchetypeEvidence } from './archetype.js'; import type { DriftLabel } from './dest-drift.js'; import type { HookEntry, HookWiringDiff } from './hook-wiring.js'; @@ -126,6 +127,10 @@ export interface InitReport { absentUpstream?: { surface: InstallSurface; path: string }[]; models: { outcome: InitPlanReport['models']['outcome']; reasons: string[] }; config: { carriedKeys: string[] }; + // Present ONLY when the install would add to (or create) the project's + // `.gitignore`, or would have to leave it alone (lib/gitignore.ts) — so a + // report with nothing to add keeps its earlier bytes. Added in 0.16.0. + gitignore?: GitignoreReport; } const key = (c: { role: string; name: string }): string => @@ -238,5 +243,14 @@ export function buildInitReport(p: InitPlanReport): InitReport { config: { carriedKeys: p.carriedConfigKeys.map((k) => cap(k, MAX_UNKNOWN_FIELD)), }, + ...gitignoreKey(p.gitignore), }; } + +// `{gitignore}` when a real run would touch `.gitignore`, else nothing. +function gitignoreKey(plan: InitPlanReport['gitignore']): { + gitignore?: GitignoreReport; +} { + const report = gitignoreReport(plan); + return report === null ? {} : { gitignore: report }; +} diff --git a/src/lib/root-migration.ts b/src/lib/root-migration.ts index 001da13..b6e97f1 100644 --- a/src/lib/root-migration.ts +++ b/src/lib/root-migration.ts @@ -86,7 +86,8 @@ const USER_DATA_MOVES: readonly { from: string; to: string; dir: boolean }[] = [ ]; // Old `.gitignore` entries and the line that replaces each. Matched as whole -// lines after normalizing (see gitignoreHints); never written. The old names +// lines after normalizing (see gitignoreHints); never rewritten (pharn only +// appends its own entries — lib/gitignore.ts). The old names // stay in src/ only here (to recognise them) and in project-lock.ts (the // one-release legacy lock): this is the migration step that retires them. const GITIGNORE_REPLACEMENTS: readonly { old: string; hint: string }[] = [ @@ -539,7 +540,7 @@ export function rootMigrationNoteLines( ); if (m.gitignoreHints.length > 0) lines.push( - ` .gitignore names pharn's old paths. Replace them with: ${m.gitignoreHints.join(', ')} (pharn never edits .gitignore).`, + ` .gitignore names pharn's old paths. Replace them with: ${m.gitignoreHints.join(', ')} (pharn only appends its own entries; it never rewrites yours).`, ); return lines.length > 0 ? lines : null; } diff --git a/src/lib/update-plan.ts b/src/lib/update-plan.ts index 0d2b9d0..4dd9904 100644 --- a/src/lib/update-plan.ts +++ b/src/lib/update-plan.ts @@ -37,6 +37,7 @@ import { import { decideModelsUpdate, type ModelsUpdate } from './models-update.js'; import { readUpstreamModels } from './upstream-models.js'; import { findOrphans, type Orphan } from './orphans.js'; +import { planGitignore, type GitignorePlan } from './gitignore.js'; import { planRootMigration, type RootMigration } from './root-migration.js'; import { planUpdate, @@ -136,6 +137,10 @@ export interface UpdatePlanReport { // list `apply.rootMoveBackups` hands the writer, so the preview cannot // under-report the real run's backup. rootMoveBackups: string[]; + // What this run adds to the project's `.gitignore` (lib/gitignore.ts): + // `none` when it already covers pharn's entries. Re-planned under the lock + // by the writer, like every other read-only answer here. + gitignore: GitignorePlan; } export interface UpdateApplyInputs { @@ -397,6 +402,7 @@ export function buildUpdatePlan(input: { orphans, rootMigration: rootMigration?.report ?? null, rootMoveBackups: rootMigration?.backups ?? [], + gitignore: planGitignore(cwd), }, apply: { expected, diff --git a/src/lib/update-render.ts b/src/lib/update-render.ts index 48e9187..c7e8a9f 100644 --- a/src/lib/update-render.ts +++ b/src/lib/update-render.ts @@ -1,3 +1,8 @@ +import { + gitignoreLines, + gitignorePending, + type GitignoreOutcome, +} from './gitignore.js'; import { log, note, outro } from '@clack/prompts'; import { hookWiringLines, type HookWiringDiff } from './hook-wiring.js'; import { absentSurfaceLines } from './absent-surfaces.js'; @@ -57,6 +62,9 @@ export interface Backup { // What the copies were made ahead of — the notice says it, and the abort // line says what may already be gone. action: BackupAction; + // `pharn/.backup/` is gitignored, or this run adds it (lib/gitignore.ts): + // the notice then drops its advice to add it. + gitignored?: boolean; } // The outcome of the fetch+apply phase, assembled inside the try so cleanup can @@ -92,6 +100,9 @@ export interface UpdateOutcome { modelsFloor: string; // Recorded files upstream no longer ships, still on disk (lib/orphans.ts). orphans: Orphan[]; + // What this run did to the project's `.gitignore` (lib/gitignore.ts). + // Absent on paths that never applied. + gitignore?: GitignoreOutcome; // The old root footprint this run retired (lib/root-migration.ts), and what // the writer actually did. Both null when nothing applied. rootMigration: RootMigration | null; @@ -336,7 +347,17 @@ export function reportDryRun( for (const rel of backupRels) lines.push(` ${rel}`); lines.push(''); } - if (writes.length === 0 && deletes.length === 0 && !migrates) + // What a real run would append to `.gitignore` (lib/gitignore.ts owns the + // wording); a refusal is a warning, printed after the note. + const gitignore = gitignoreLines(r.gitignore, { preview: true }); + if (gitignore !== null && !gitignore.warn) + lines.push(` ${gitignore.text}`, ''); + if ( + writes.length === 0 && + deletes.length === 0 && + !migrates && + !gitignorePending(r.gitignore) + ) lines.push(' Nothing would be written.', ''); lines.push( ` UNCHANGED ${r.counts.ok} file(s) · SKIPPED ${r.counts.skipped} file(s)`, @@ -345,6 +366,7 @@ export function reportDryRun( lines.push('', ` Would record channel: ${channel}`); } note(lines.join('\n'), 'PLAN'); + if (gitignore?.warn) log.warn(gitignore.text); reportCapabilityChanges(r.capabilityChanges, r.removals, true); reportModels(r.models, r.modelsFloor, null, true); @@ -404,7 +426,12 @@ export function reportBackup(backup: Backup, opts: { aborted: boolean }): void { // The pointer (and, on success, the .gitignore advice) — lib/backup.ts's one // wording, shared with every other command that makes a backup. printBackupNotice( - { dir: backup.dir, saved: backup.count, action: backup.action }, + { + dir: backup.dir, + saved: backup.count, + action: backup.action, + gitignored: backup.gitignored, + }, opts, ); } diff --git a/src/lib/update-report.ts b/src/lib/update-report.ts index 7955dd8..87578d6 100644 --- a/src/lib/update-report.ts +++ b/src/lib/update-report.ts @@ -11,6 +11,11 @@ import type { } from './update-decision.js'; import type { CapabilityRemoval, UpdatePlanReport } from './update-plan.js'; import type { OrphanState } from './orphans.js'; +import { + gitignoreReport, + type GitignorePlan, + type GitignoreReport, +} from './gitignore.js'; import type { InstallSurface } from './install-manifest.js'; import type { Layout } from '../types.js'; @@ -154,6 +159,11 @@ export interface UpdateReport { // bytes 0.10.0 shipped; additive within schemaVersion 1; never filtered by // `--compact`. orphans?: { path: string; state: OrphanState; reason?: string }[]; + // What a real run would add to the project's `.gitignore` + // (lib/gitignore.ts). Present ONLY when it would add something (or is + // refused), so a report with nothing to add keeps its earlier bytes; + // additive within schemaVersion 1. Added in 0.16.0. + gitignore?: GitignoreReport; } | null; } @@ -302,9 +312,16 @@ function planReport( })), } : {}), + ...gitignoreKey(p.gitignore), }; } +// `{gitignore}` when a real run would touch `.gitignore`, else nothing. +function gitignoreKey(plan: GitignorePlan): { gitignore?: GitignoreReport } { + const report = gitignoreReport(plan); + return report === null ? {} : { gitignore: report }; +} + export function buildUpdateReport(input: UpdateReportInput): UpdateReport { const { plan } = input; const compact = input.compact ?? false; diff --git a/src/steps/install-archetype.ts b/src/steps/install-archetype.ts index 69a596d..06ecdda 100644 --- a/src/steps/install-archetype.ts +++ b/src/steps/install-archetype.ts @@ -60,6 +60,12 @@ import { readUpstreamModels } from '../lib/upstream-models.js'; import { terminalSafe } from '../lib/terminal-safe.js'; import { DEFAULT_SEAM_CONFIG } from '../lib/seam-config.js'; import { writePharnConfig } from '../lib/pharn-config.js'; +import { + applyGitignore, + backupDirIgnored, + gitignoreLines, + planGitignore, +} from '../lib/gitignore.js'; import { readSkillsVersion } from '../lib/skills-version.js'; import { PHARN_VERSION } from '../version.js'; import type { @@ -209,6 +215,9 @@ export async function runInstallArchetype( dir: backupDir, saved: toBackUp.length, action, + // This install adds `pharn/.backup/` to `.gitignore` when it is missing + // (below), so the advice to add it would be stale before the run ends. + gitignored: backupDirIgnored(cwd, planGitignore(cwd)), }); } // Deleted BEFORE the copy, after the backup: a delete that fails stops the @@ -412,6 +421,15 @@ export async function runInstallArchetype( // actually changed. await writePharnConfig(cwd, { ...config, ...readCarriedEntries(cwd) }); + // Keep PHARN's run state and backups out of git (lib/gitignore.ts): append + // whichever of `pharn/.run/` and `pharn/.backup/` the project's `.gitignore` + // does not already cover, after every pharn-owned write and under the same + // lock. Append-only; a symlinked or unusable `.gitignore`, or a failed write, + // is a warning naming the lines — the install above has already landed. + const gitignore = await applyGitignore(cwd); + const gitignoreReport = gitignoreLines(gitignore); + if (gitignoreReport?.warn) log.warn(gitignoreReport.text); + // The user-data moves, LAST — after every pharn-owned write — and the note // that says what happened (the wording `update` prints too). A move that // could not be made fails the run with exit 1 once the outro is out. @@ -463,6 +481,11 @@ export async function runInstallArchetype( : `${pc.yellow('!')} no PHARN commands or hooks written ${pc.dim('(the fetched repo shipped none)')}`, docsLine, `${check} pharn.config.json written ${pc.dim(`(skills v${skillsVersion}, archetypes: ${archetypes.join(', ')})`)}`, + ...(gitignore.action === 'create' || gitignore.action === 'append' + ? [ + `${check} .gitignore ${gitignore.action === 'create' ? 'created' : 'updated'} → ${pc.dim(gitignore.entries.join(', '))}`, + ] + : []), `${pc.dim(`Done in ${elapsed}s`)}`, ...modelsSection, '', diff --git a/tests/atomic-write.test.ts b/tests/atomic-write.test.ts index 5983ca2..ac9d200 100644 --- a/tests/atomic-write.test.ts +++ b/tests/atomic-write.test.ts @@ -20,6 +20,7 @@ import { TMP_NONCE_BYTES, copyFileAtomicSync, tmpPathFor, + writeFileAtomic, writeJsonAtomic, } from '../src/lib/atomic-write.js'; @@ -260,6 +261,43 @@ describe('writeJsonAtomic', () => { }); }); +// The arbitrary-content form writeJsonAtomic delegates to (lib/gitignore.ts +// appends to the user's `.gitignore` through it). +describe('writeFileAtomic', () => { + const tmp = useTmpDir(); + + it('writes a Buffer byte for byte (no re-encoding) and a string as utf8', async () => { + const bin = join(tmp.path(), 'bin'); + const bytes = Buffer.from([0x61, 0xe9, 0xff, 0x0a]); + await writeFileAtomic(bin, bytes); + expect(readFileSync(bin).equals(bytes)).toBe(true); + const txt = join(tmp.path(), 'txt'); + await writeFileAtomic(txt, 'café\n'); + expect(readFileSync(txt, 'utf8')).toBe('café\n'); + expect(readdirSync(tmp.path()).sort()).toEqual(['bin', 'txt']); + }); + + it('gives a NEW file the ordinary umask mode, not the 0600 of a sync temp', async () => { + const target = join(tmp.path(), 'new'); + await writeFileAtomic(target, 'x'); + const probe = join(tmp.path(), 'probe'); + writeFileSync(probe, ''); + expect(statSync(target).mode & 0o777).toBe(statSync(probe).mode & 0o777); + }); + + it('keeps an existing file’s mode and replaces a hard link by name', async () => { + const target = join(tmp.path(), 'target'); + const other = join(tmp.path(), 'other'); + writeFileSync(target, 'old'); + chmodSync(target, 0o640); + linkSync(target, other); + await writeFileAtomic(target, Buffer.from('new')); + expect(readFileSync(target, 'utf8')).toBe('new'); + expect(statSync(target).mode & 0o777).toBe(0o640); + expect(readFileSync(other, 'utf8')).toBe('old'); + }); +}); + describe('copyFileAtomicSync (F-19)', () => { const tmp = useTmpDir(); const source = () => join(tmp.path(), 'src', 'upstream.md'); diff --git a/tests/backup.test.ts b/tests/backup.test.ts index 9045386..465b98d 100644 --- a/tests/backup.test.ts +++ b/tests/backup.test.ts @@ -175,6 +175,28 @@ describe('backupNoticeLines', () => { ]); }); + it('drops the .gitignore advice once pharn/.backup/ is gitignored (or the run adds it)', () => { + expect( + backupNoticeLines({ + dir: 'pharn/.backup/x', + saved: 1, + action: 'removing', + gitignored: true, + }), + ).toEqual(['Backed up 1 file(s) to pharn/.backup/x before removing.']); + expect( + backupNoticeLines({ + dir: 'pharn/.backup/x', + saved: 1, + action: 'removing', + gitignored: false, + }), + ).toEqual([ + 'Backed up 1 file(s) to pharn/.backup/x before removing.', + HINT, + ]); + }); + it('drops the .gitignore advice on an aborted run (the pointer alone is repeated)', () => { expect( backupNoticeLines( diff --git a/tests/commands-integration.test.ts b/tests/commands-integration.test.ts index bce574a..5b19f1f 100644 --- a/tests/commands-integration.test.ts +++ b/tests/commands-integration.test.ts @@ -299,7 +299,11 @@ const INSTALLED = [ 'pharn/pharn-core/seam-resolver/SKILL.md', SECURITY, ].sort(); -const STATE_FILES = ['pharn.config.json', 'pharn.records.json']; +// `.gitignore` is the user's, but a fresh install creates it with PHARN's +// entries (lib/gitignore.ts). +const STATE_FILES = ['.gitignore', 'pharn.config.json', 'pharn.records.json']; +const PHARN_GITIGNORE_BLOCK = + '# PHARN run state and update backups (added by pharn)\npharn/.run/\npharn/.backup/\n'; describe('commands end to end, against a real project (no src/ mocks)', () => { it('init --yes installs the archetype selection into an empty project', async () => { @@ -356,6 +360,34 @@ describe('commands end to end, against a real project (no src/ mocks)', () => { expect(report.drift?.missing).toEqual([]); }); + // 0.16.0 (lib/gitignore.ts): init appends PHARN's run state and backups to + // the user's `.gitignore` (CRLF kept, their bytes the prefix); a same-version + // update with both lines present changes nothing; a deleted line re-opens it + // and comes back, once. + it('init and update keep pharn/.run/ and pharn/.backup/ in .gitignore, idempotently', async () => { + writeFileSync(at('.gitignore'), 'node_modules\r\ndist\r\n'); + await init(); + const full = + 'node_modules\r\ndist\r\n# PHARN run state and update backups (added by pharn)\r\npharn/.run/\r\npharn/.backup/\r\n'; + expect(read('.gitignore')).toBe(full); + + const installedAt = JSON.parse(read('pharn.config.json')).installedAt; + expect(await run(() => runUpdate({ yes: true }))).toBeNull(); + expect(read('.gitignore')).toBe(full); + // The early return: nothing re-applied, so the config was not rewritten. + expect(JSON.parse(read('pharn.config.json')).installedAt).toBe(installedAt); + + writeFileSync(at('.gitignore'), full.replace('pharn/.backup/\r\n', '')); + expect(await run(() => runUpdate({ yes: true }))).toBeNull(); + expect(read('.gitignore')).toBe( + full.replace('pharn/.backup/\r\n', '') + 'pharn/.backup/\r\n', + ); + expect(await run(() => runUpdate({ yes: true }))).toBeNull(); + expect(read('.gitignore')).toBe( + full.replace('pharn/.backup/\r\n', '') + 'pharn/.backup/\r\n', + ); + }); + it('update --yes upgrades a clean file, skips an edited one, and withholds the bump', async () => { await init(); const v1Constitution = records().files[CONSTITUTION]; @@ -441,10 +473,14 @@ describe('commands end to end, against a real project (no src/ mocks)', () => { const backups = left.filter((f) => f.startsWith('pharn/.backup/')); expect(backups).toHaveLength(1); expect(backups[0]).toMatch(/^pharn\/\.backup\/[^/]+\/pharn\.config\.json$/); + // `.gitignore` too: pharn's lines in it are harmless and stay (removing + // a user-file line would be a rewrite). expect(left.filter((f) => !f.startsWith('pharn/.backup/'))).toEqual([ '.claude/settings.json', + '.gitignore', CONSTITUTION, ]); + expect(read('.gitignore')).toBe(PHARN_GITIGNORE_BLOCK); expect(read(CONSTITUTION)).toBe(`my own constitution\n`); expect(existsSync(at('pharn/pharn-pipeline'))).toBe(false); expect(existsSync(at('.claude/commands'))).toBe(false); @@ -585,9 +621,13 @@ describe('root-footprint migration, end to end', () => { expect( backed.map((f) => f.replace(/^pharn\/\.backup\/[^/]+\//, '')), ).toEqual(['memory-bank/lessons-learned.md', 'pharn.spec-template.md']); - // The scratch dir and the user's .gitignore are only noticed. + // The scratch dir is only noticed. The user's .gitignore lines are kept + // byte for byte (the old names are hinted at, never rewritten) and pharn's + // block is appended after them (lib/gitignore.ts). expect(read('.pharn/writes-scope.json')).toBe('{}\n'); - expect(read('.gitignore')).toBe('node_modules\n.pharn/\n.pharn-backup\n'); + expect(read('.gitignore')).toBe( + `node_modules\n.pharn/\n.pharn-backup\n${PHARN_GITIGNORE_BLOCK}`, + ); // Nothing of pharn's at the root but .claude/, pharn/ and the JSON files; // `.gitignore` and `.pharn/` are the user's and pharn-oss's. @@ -713,7 +753,7 @@ describe('root-footprint migration, end to end', () => { await init(); expect(await run(() => runUninstall({ yes: true }))).toBeNull(); expect(readdirSync(at('pharn'))).toEqual(['.backup']); - expect(rootEntries()).toEqual(['.claude', 'pharn']); + expect(rootEntries()).toEqual(['.claude', '.gitignore', 'pharn']); }); }); diff --git a/tests/gitignore.test.ts b/tests/gitignore.test.ts new file mode 100644 index 0000000..52755a9 --- /dev/null +++ b/tests/gitignore.test.ts @@ -0,0 +1,385 @@ +import { execFileSync } from 'node:child_process'; +import { + chmodSync, + linkSync, + mkdirSync, + readFileSync, + statSync, + symlinkSync, + utimesSync, + writeFileSync, +} from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { + applyGitignore, + backupDirIgnored, + decidedEntries, + GITIGNORE_COMMENT, + gitignoreAddition, + gitignoreLines, + gitignorePending, + gitignoreReport, + planGitignore, + PHARN_GITIGNORE_ENTRIES, +} from '../src/lib/gitignore.js'; +import { useTmpDir } from './helpers.js'; + +// pharn's lines in the project's `.gitignore` (lib/gitignore.ts): append-only, +// spelling-aware, idempotent, never through a symlink. + +const BLOCK = `${GITIGNORE_COMMENT}\npharn/.run/\npharn/.backup/\n`; + +const decided = (text: string) => [...decidedEntries(Buffer.from(text))]; + +describe('decidedEntries — the covered test (a membership test on fixed literals)', () => { + it.each([ + 'pharn/.run/', + '/pharn/.run/', + 'pharn/.run', + '/pharn/.run', + 'pharn/.run/**', + 'pharn/.run/*', + '**/pharn/.run/', + '**/pharn/.run', + '.run/', + '.run', + '**/.run/', + 'pharn/.run/ ', + 'pharn/.run/\t', + 'pharn/.run/\r', + ])('%j covers pharn/.run/', (line) => { + expect(decided(`node_modules\n${line}\n`)).toEqual(['pharn/.run/']); + }); + + it.each(['/pharn/.backup/', 'pharn/.backup', 'pharn/.backup/**', '.backup/'])( + '%j covers pharn/.backup/', + (line) => { + expect(decided(`${line}\n`)).toEqual(['pharn/.backup/']); + }, + ); + + it.each([ + ['a leading space (git reads it as part of the pattern)', ' pharn/.run/'], + ['a comment', '# pharn/.run/'], + ['a root-anchored basename', '/.run/'], + ['an anchored basename via /*', '.run/*'], + ['another directory', 'pharn/.runs/'], + ['a deeper path', 'src/pharn/.run/'], + [ + 'a broader pattern (not recognised; pharn appends its own line)', + 'pharn/', + ], + ])('%s does not count', (_label, line) => { + expect(decided(`${line}\n`)).toEqual([]); + }); + + it('reads past a leading UTF-8 BOM, as git does', () => { + const bom = Buffer.from([0xef, 0xbb, 0xbf]); + expect([ + ...decidedEntries( + Buffer.concat([bom, Buffer.from('pharn/.run/\npharn/.backup/\n')]), + ), + ]).toEqual(['pharn/.run/', 'pharn/.backup/']); + }); + + it('a negation is the user’s decision: the entry counts as decided', () => { + expect(decided('!pharn/.run/\n!/pharn/.backup\n')).toEqual([ + 'pharn/.run/', + 'pharn/.backup/', + ]); + }); +}); + +describe('gitignoreAddition — what is appended', () => { + it('no file: the comment and both entries, LF', () => { + const { entries, append } = gitignoreAddition(null); + expect(entries).toEqual([...PHARN_GITIGNORE_ENTRIES]); + expect(append.toString()).toBe(BLOCK); + }); + + it('an empty file: the same block, no separator', () => { + expect(gitignoreAddition(Buffer.alloc(0)).append.toString()).toBe(BLOCK); + }); + + it('an existing LF file without the entries: the block after it', () => { + const { append } = gitignoreAddition(Buffer.from('node_modules\n')); + expect(append.toString()).toBe(BLOCK); + }); + + it('a file without a trailing newline: one line ending first', () => { + const { append } = gitignoreAddition(Buffer.from('node_modules')); + expect(append.toString()).toBe(`\n${BLOCK}`); + }); + + it('only the missing entry is added', () => { + const { entries, append } = gitignoreAddition(Buffer.from('/pharn/.run\n')); + expect(entries).toEqual(['pharn/.backup/']); + expect(append.toString()).toBe(`${GITIGNORE_COMMENT}\npharn/.backup/\n`); + }); + + it('the comment is written once: a file that holds it gets the entry alone', () => { + const { append } = gitignoreAddition( + Buffer.from(`${GITIGNORE_COMMENT}\npharn/.run/\n`), + ); + expect(append.toString()).toBe('pharn/.backup/\n'); + }); + + it('a CRLF file gets CRLF lines, and no bare LF', () => { + const { append } = gitignoreAddition(Buffer.from('a\r\nb\r\n')); + expect(append.toString()).toBe( + `${GITIGNORE_COMMENT}\r\npharn/.run/\r\npharn/.backup/\r\n`, + ); + expect(append.toString().replace(/\r\n/g, '')).not.toContain('\n'); + }); + + it('a CRLF file without a trailing newline gets a CRLF separator', () => { + const { append } = gitignoreAddition(Buffer.from('a\r\nb')); + expect(append.toString().startsWith('\r\n# PHARN')).toBe(true); + }); + + it('both entries decided: nothing', () => { + const { entries, append } = gitignoreAddition( + Buffer.from('pharn/.run/**\n.backup\n'), + ); + expect(entries).toEqual([]); + expect(append.length).toBe(0); + }); +}); + +describe('applyGitignore — the write (append-only, idempotent)', () => { + const tmp = useTmpDir(); + const file = () => join(tmp.path(), '.gitignore'); + + it('creates a missing .gitignore with the block', async () => { + expect(planGitignore(tmp.path())).toEqual({ + action: 'create', + entries: ['pharn/.run/', 'pharn/.backup/'], + }); + expect(await applyGitignore(tmp.path())).toEqual({ + action: 'create', + entries: ['pharn/.run/', 'pharn/.backup/'], + }); + expect(readFileSync(file(), 'utf8')).toBe(BLOCK); + }); + + it('a created .gitignore gets the ordinary umask mode, not owner-only', async () => { + await applyGitignore(tmp.path()); + const probe = join(tmp.path(), 'probe'); + writeFileSync(probe, ''); + expect(statSync(file()).mode & 0o777).toBe(statSync(probe).mode & 0o777); + }); + + it('appends to an existing file: its exact bytes stay the prefix', async () => { + // Non-UTF-8 bytes too: the file is never decoded and re-encoded. + const old = Buffer.concat([ + Buffer.from('node_modules\n# caf'), + Buffer.from([0xe9, 0xff]), + Buffer.from('\ndist/\n'), + ]); + writeFileSync(file(), old); + expect(await applyGitignore(tmp.path())).toEqual({ + action: 'append', + entries: ['pharn/.run/', 'pharn/.backup/'], + }); + const now = readFileSync(file()); + expect(now.subarray(0, old.length).equals(old)).toBe(true); + expect(now.subarray(old.length).toString()).toBe(BLOCK); + }); + + it('a BOM file that already covers both is left byte-identical', async () => { + const old = Buffer.concat([ + Buffer.from([0xef, 0xbb, 0xbf]), + Buffer.from('pharn/.run/\npharn/.backup/\n'), + ]); + writeFileSync(file(), old); + expect(await applyGitignore(tmp.path())).toEqual({ action: 'none' }); + expect(readFileSync(file()).equals(old)).toBe(true); + }); + + it('keeps a CRLF file CRLF', async () => { + writeFileSync(file(), 'node_modules\r\ndist/\r\n'); + await applyGitignore(tmp.path()); + expect(readFileSync(file(), 'utf8')).toBe( + `node_modules\r\ndist/\r\n${GITIGNORE_COMMENT}\r\npharn/.run/\r\npharn/.backup/\r\n`, + ); + }); + + it.each([ + ['pharn/.run/\npharn/.backup/\n'], + ['/pharn/.run/\n/pharn/.backup/\n'], + ['pharn/.run\npharn/.backup\n'], + ['pharn/.run/**\npharn/.backup/**\n'], + ['!pharn/.run/\n.backup/\n'], + ])('entries already present (%j): nothing is written', async (text) => { + writeFileSync(file(), text); + const past = new Date('2020-01-01T00:00:00Z'); + utimesSync(file(), past, past); + expect(planGitignore(tmp.path())).toEqual({ action: 'none' }); + expect(await applyGitignore(tmp.path())).toEqual({ action: 'none' }); + expect(readFileSync(file(), 'utf8')).toBe(text); + expect(statSync(file()).mtime.getTime()).toBe(past.getTime()); + }); + + it('a second run is a no-op (bytes and mtime unchanged)', async () => { + writeFileSync(file(), 'node_modules\n'); + await applyGitignore(tmp.path()); + const once = readFileSync(file()); + const past = new Date('2020-01-01T00:00:00Z'); + utimesSync(file(), past, past); + expect(await applyGitignore(tmp.path())).toEqual({ action: 'none' }); + expect(readFileSync(file()).equals(once)).toBe(true); + expect(statSync(file()).mtime.getTime()).toBe(past.getTime()); + }); + + it('git itself ignores both directories afterwards', async () => { + writeFileSync(file(), 'node_modules\n'); + await applyGitignore(tmp.path()); + execFileSync('git', ['init', '-q'], { cwd: tmp.path() }); + for (const rel of ['pharn/.run/x/stage.json', 'pharn/.backup/20260101/a']) { + // check-ignore exits 0 when the path is ignored, and throws otherwise. + execFileSync('git', ['check-ignore', '-q', '--no-index', rel], { + cwd: tmp.path(), + }); + } + }); + + it('a symlinked .gitignore is refused: neither the link nor its target changes', async () => { + const outside = join(tmp.path(), 'outside'); + writeFileSync(outside, 'node_modules\n'); + symlinkSync(outside, file()); + const plan = planGitignore(tmp.path()); + expect(plan).toEqual({ + action: 'refused', + reason: 'symlink', + entries: ['pharn/.run/', 'pharn/.backup/'], + }); + expect(gitignorePending(plan)).toBe(false); + expect(await applyGitignore(tmp.path())).toEqual(plan); + expect(readFileSync(outside, 'utf8')).toBe('node_modules\n'); + }); + + it('a directory at .gitignore is refused', async () => { + mkdirSync(file()); + expect(await applyGitignore(tmp.path())).toMatchObject({ + action: 'refused', + reason: 'not-a-file', + }); + }); + + it('a FIFO at .gitignore is refused without hanging', async () => { + execFileSync('mkfifo', [file()]); + expect(await applyGitignore(tmp.path())).toMatchObject({ + action: 'refused', + reason: 'not-a-file', + }); + }); + + it('a hard-linked .gitignore is replaced: the other name keeps the old bytes', async () => { + const other = join(tmp.path(), 'other-name'); + writeFileSync(file(), 'node_modules\n'); + linkSync(file(), other); + await applyGitignore(tmp.path()); + expect(readFileSync(file(), 'utf8')).toBe(`node_modules\n${BLOCK}`); + expect(readFileSync(other, 'utf8')).toBe('node_modules\n'); + }); + + it('keeps an existing file’s mode', async () => { + writeFileSync(file(), 'node_modules\n'); + chmodSync(file(), 0o640); + await applyGitignore(tmp.path()); + expect(statSync(file()).mode & 0o777).toBe(0o640); + }); + + it('a failed write is returned, never thrown, and leaves the file as it was', async () => { + writeFileSync(file(), 'node_modules\n'); + // The temp sibling cannot be created in a read-only directory. (Inert for + // uid 0 — this suite must not run as root.) + chmodSync(tmp.path(), 0o555); + try { + expect(await applyGitignore(tmp.path())).toEqual({ + action: 'failed', + code: 'EACCES', + entries: ['pharn/.run/', 'pharn/.backup/'], + }); + } finally { + chmodSync(tmp.path(), 0o755); + } + expect(readFileSync(file(), 'utf8')).toBe('node_modules\n'); + }); +}); + +describe('backupDirIgnored — whether the backup notice may skip its advice', () => { + const tmp = useTmpDir(); + + it('false without a .gitignore, true once it covers pharn/.backup/', () => { + expect(backupDirIgnored(tmp.path())).toBe(false); + writeFileSync(join(tmp.path(), '.gitignore'), '/pharn/.backup\n'); + expect(backupDirIgnored(tmp.path())).toBe(true); + }); + + it('true when the run’s plan adds it', () => { + expect(backupDirIgnored(tmp.path(), planGitignore(tmp.path()))).toBe(true); + }); + + it('false when the plan is a refusal', () => { + mkdirSync(join(tmp.path(), '.gitignore')); + expect(backupDirIgnored(tmp.path(), planGitignore(tmp.path()))).toBe(false); + }); +}); + +describe('gitignoreLines / gitignoreReport — the one wording and the JSON form', () => { + const both = ['pharn/.run/', 'pharn/.backup/'] as const; + + it('says nothing, and has no JSON key, when nothing changes', () => { + expect(gitignoreLines({ action: 'none' })).toBeNull(); + expect(gitignoreReport({ action: 'none' })).toBeNull(); + }); + + it('past and future tense for create and append', () => { + expect(gitignoreLines({ action: 'create', entries: [...both] })).toEqual({ + text: "Created .gitignore with pharn/.run/, pharn/.backup/ (PHARN's run state and update backups).", + warn: false, + }); + expect( + gitignoreLines( + { action: 'append', entries: ['pharn/.backup/'] }, + { preview: true }, + ), + ).toEqual({ + text: ".gitignore: would add pharn/.backup/ (PHARN's run state and update backups).", + warn: false, + }); + }); + + it('a refusal or a failed write is a warning naming the lines to add by hand', () => { + expect( + gitignoreLines({ + action: 'refused', + reason: 'symlink', + entries: [...both], + }), + ).toEqual({ + text: ".gitignore was not updated: it is a symlink, and pharn never writes through one. Add these lines yourself so PHARN's run state is never committed: pharn/.run/, pharn/.backup/", + warn: true, + }); + expect( + gitignoreLines({ action: 'failed', code: 'EACCES', entries: [...both] }) + ?.text, + ).toContain('writing it failed (EACCES)'); + }); + + it('projects a plan field by field', () => { + expect(gitignoreReport({ action: 'append', entries: [...both] })).toEqual({ + action: 'append', + entries: [...both], + }); + expect( + gitignoreReport({ + action: 'refused', + reason: 'unreadable', + entries: [...both], + }), + ).toEqual({ action: 'refused', reason: 'unreadable', entries: [...both] }); + }); +}); diff --git a/tests/init-archetype.test.ts b/tests/init-archetype.test.ts index 45665ab..e411fe9 100644 --- a/tests/init-archetype.test.ts +++ b/tests/init-archetype.test.ts @@ -805,9 +805,12 @@ describe('re-running init over an existing install (PHARN-11)', () => { .mock.calls.map((c) => String(c[0])) .join('\n'); expect(info).toContain(`Backed up 1 file(s) to pharn/.backup/${dir!}`); - // F-40: the same .gitignore advice `update`'s backup notice carries. - expect(info).toContain( - 'pharn/.backup/ is not gitignored — add it to .gitignore or delete it once you are happy.', + // F-40's .gitignore advice is dropped here: this install adds + // `pharn/.backup/` to `.gitignore` itself (lib/gitignore.ts), so telling + // the user to add it would be stale before the run ends. + expect(info).not.toContain('not gitignored'); + expect(readFileSync(join(proj, '.gitignore'), 'utf8')).toContain( + 'pharn/.backup/\n', ); }); diff --git a/tests/init-dry-run-json.test.ts b/tests/init-dry-run-json.test.ts index 7b281fd..64228e3 100644 --- a/tests/init-dry-run-json.test.ts +++ b/tests/init-dry-run-json.test.ts @@ -339,6 +339,18 @@ describe('runInit --dry-run --json', () => { expect(existsSync(join(proj, 'pharn-review/stale/NOTES.md'))).toBe(true); }); + it('carries gitignore when the install would create or append to .gitignore, and omits it otherwise', async () => { + expect((await run()).gitignore).toEqual({ + action: 'create', + entries: ['pharn/.run/', 'pharn/.backup/'], + }); + expect(existsSync(join(proj, '.gitignore'))).toBe(false); + + logSpy.mockClear(); + writeFileSync(join(proj, '.gitignore'), 'pharn/.run/\npharn/.backup/\n'); + expect(await run()).not.toHaveProperty('gitignore'); + }); + it('--ref main reports the main channel and no tag', async () => { const r = await run({ ref: 'main' }); expect(r.channel).toBe('main'); diff --git a/tests/init-dry-run.test.ts b/tests/init-dry-run.test.ts index e864aff..167a355 100644 --- a/tests/init-dry-run.test.ts +++ b/tests/init-dry-run.test.ts @@ -93,6 +93,15 @@ vi.mock('../src/lib/project-lock.js', async () => { return { ...actual, withProjectLock }; }); +const applyGitignore = vi.fn(); +vi.mock('../src/lib/gitignore.js', async () => { + const actual = await vi.importActual< + typeof import('../src/lib/gitignore.js') + >('../src/lib/gitignore.js'); + applyGitignore.mockImplementation(actual.applyGitignore); + return { ...actual, applyGitignore }; +}); + const { runInit } = await import('../src/commands/init.js'); const prompts = await import('@clack/prompts'); const { BACKUP_DIR } = await import('../src/lib/backup.js'); @@ -191,6 +200,7 @@ function noWriteReached(): void { expect(writeRecords).not.toHaveBeenCalled(); expect(writePharnConfig).not.toHaveBeenCalled(); expect(withProjectLock).not.toHaveBeenCalled(); + expect(applyGitignore).not.toHaveBeenCalled(); expect(prompts.select).not.toHaveBeenCalled(); expect(prompts.confirm).not.toHaveBeenCalled(); } @@ -451,6 +461,71 @@ describe('pharn init --dry-run', () => { }); }); + // 0.16.0: the install appends PHARN's run state and backups to the project's + // `.gitignore` (lib/gitignore.ts); the preview says so and writes nothing. + describe('the project .gitignore', () => { + beforeEach(() => vi.clearAllMocks()); + + it('previews creating it, writes nothing, and the real run writes exactly that', async () => { + const { repo, proj } = fixture(tmp.path()); + const before = snapshotTree(proj); + + await init(repo, proj, { dryRun: true }); + + expect(snapshotTree(proj)).toEqual(before); + noWriteReached(); + expect(notes('PLAN')[0]).toContain( + ".gitignore: would create it with pharn/.run/, pharn/.backup/ (PHARN's run state and update backups).", + ); + + await init(repo, proj, { yes: true }); + expect(readFileSync(join(proj, '.gitignore'), 'utf8')).toBe( + '# PHARN run state and update backups (added by pharn)\npharn/.run/\npharn/.backup/\n', + ); + expect(outro()).toContain( + '.gitignore created → pharn/.run/, pharn/.backup/', + ); + }); + + it('previews appending only the missing entry', async () => { + const { repo, proj } = fixture(tmp.path()); + writeFileSync(join(proj, '.gitignore'), 'node_modules\n/pharn/.run\n'); + + await init(repo, proj, { dryRun: true }); + + expect(notes('PLAN')[0]).toContain( + ".gitignore: would add pharn/.backup/ (PHARN's run state and update backups).", + ); + expect(readFileSync(join(proj, '.gitignore'), 'utf8')).toBe( + 'node_modules\n/pharn/.run\n', + ); + }); + + it('says nothing when it already covers both', async () => { + const { repo, proj } = fixture(tmp.path()); + writeFileSync(join(proj, '.gitignore'), 'pharn/.run/**\npharn/.backup\n'); + + await init(repo, proj, { dryRun: true }); + + expect(notes('PLAN')[0]).not.toContain('.gitignore'); + expect(logged('warn')).not.toContain('.gitignore'); + }); + + it('warns, naming the lines, when .gitignore is a symlink', async () => { + const { repo, proj } = fixture(tmp.path()); + const outside = join(tmp.path(), 'outside'); + writeFileSync(outside, 'x\n'); + symlinkSync(outside, join(proj, '.gitignore')); + + await init(repo, proj, { dryRun: true }); + + expect(logged('warn')).toContain( + ".gitignore would not be updated: it is a symlink, and pharn never writes through one. Add these lines yourself so PHARN's run state is never committed: pharn/.run/, pharn/.backup/", + ); + expect(readFileSync(outside, 'utf8')).toBe('x\n'); + }); + }); + describe('warnings in the text preview', () => { beforeEach(() => setTTY(undefined, undefined)); diff --git a/tests/init-report.test.ts b/tests/init-report.test.ts index 80c1f59..0d4961f 100644 --- a/tests/init-report.test.ts +++ b/tests/init-report.test.ts @@ -14,6 +14,7 @@ const BIDI = String.fromCharCode(0x202e); function plan(over: Partial = {}): InitPlanReport { return { + gitignore: { action: 'none' }, archetypes: ['ssr'], archetypeOrigin: { kind: 'detected', diff --git a/tests/release-consistency.test.ts b/tests/release-consistency.test.ts index 3abcb37..7c954d2 100644 --- a/tests/release-consistency.test.ts +++ b/tests/release-consistency.test.ts @@ -77,6 +77,7 @@ const KNOWN_TAGS: readonly string[] = [ 'v0.13.0', 'v0.14.0', 'v0.14.1', + 'v0.15.0', ]; // Sections with no tag at all, and so no link: 0.8.0 was bumped and superseded // by 0.9.0 on the same day, never tagged or published; 0.2.0 / 0.1.0 predate diff --git a/tests/uninstall.test.ts b/tests/uninstall.test.ts index 272fbbd..2ea32b1 100644 --- a/tests/uninstall.test.ts +++ b/tests/uninstall.test.ts @@ -232,7 +232,7 @@ describe('runUninstall', () => { `Deleted ${FLAT_FILES.length} file(s) and 11 empty directories`, ); expect(done).toContain( - '.claude/settings.json, .claude/settings.local.json and every pharn/.backup/ directory are left untouched.', + '.claude/settings.json, .claude/settings.local.json, every pharn/.backup/ directory and the lines pharn added to .gitignore are left untouched.', ); expect(done).toContain('Left 1 directory that still hold other files:'); expect(done).toContain(' • .claude'); @@ -370,7 +370,7 @@ describe('runUninstall', () => { ); expect(plan).toContain(' • src/app.ts'); expect(plan).toContain( - '.claude/settings.json, .claude/settings.local.json and every pharn/.backup/ directory are left untouched.', + '.claude/settings.json, .claude/settings.local.json, every pharn/.backup/ directory and the lines pharn added to .gitignore are left untouched.', ); }); diff --git a/tests/update-dry-run-json.test.ts b/tests/update-dry-run-json.test.ts index 931913c..5859eca 100644 --- a/tests/update-dry-run-json.test.ts +++ b/tests/update-dry-run-json.test.ts @@ -269,6 +269,10 @@ describe('runUpdate --dry-run --json', () => { proj = join(tmp.path(), 'proj'); repo = join(tmp.path(), 'repo'); mkdirSync(proj, { recursive: true }); + // A project that already keeps PHARN's run state and backups out of git + // (0.16.0): without it every same-version run re-opens to add the lines + // (lib/gitignore.ts). The tests that exercise that path remove it. + writeFileSync(join(proj, '.gitignore'), 'pharn/.run/\npharn/.backup/\n'); vi.spyOn(process, 'cwd').mockReturnValue(proj); logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); errSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); @@ -658,6 +662,19 @@ describe('runUpdate --dry-run --json', () => { ).toMatchObject({ action: 'write', label: 'restored' }); }); + it("is planned when .gitignore lacks pharn's entries at the current version, and plan.gitignore says what it adds", async () => { + await installed({ skillsVersion: '1.1.0' }); + writeFileSync(join(proj, '.gitignore'), 'pharn/.run/\n'); + const before = snapshotTree(proj); + const { json } = await run(); + expect(json.status).toBe('planned'); + expect(json.plan.gitignore).toEqual({ + action: 'append', + entries: ['pharn/.backup/'], + }); + expect(snapshotTree(proj)).toEqual(before); + }); + it('is planned when --ref switches the channel at the current version', async () => { await installed({ skillsVersion: '1.1.0' }); const { json } = await run({ ref: 'latest' }); @@ -729,6 +746,24 @@ describe('runUpdate --dry-run --json', () => { ); }); + it('omits plan.gitignore when .gitignore already covers both entries', async () => { + await installed(); + const { json } = await run(); + expect(json.plan).not.toHaveProperty('gitignore'); + }); + + it('reports a .gitignore it would have to leave alone as refused', async () => { + await installed(); + rmSync(join(proj, '.gitignore')); + mkdirSync(join(proj, '.gitignore')); + const { json } = await run(); + expect(json.plan.gitignore).toEqual({ + action: 'refused', + reason: 'not-a-file', + entries: ['pharn/.run/', 'pharn/.backup/'], + }); + }); + it('omits plan.orphans when there is none', async () => { await installed(); const { json } = await run(); diff --git a/tests/update-dry-run.test.ts b/tests/update-dry-run.test.ts index 6ed5562..678811d 100644 --- a/tests/update-dry-run.test.ts +++ b/tests/update-dry-run.test.ts @@ -214,6 +214,10 @@ describe('runUpdate --dry-run', () => { proj = join(tmp.path(), 'proj'); repo = join(tmp.path(), 'repo'); mkdirSync(proj, { recursive: true }); + // A project that already keeps PHARN's run state and backups out of git + // (0.16.0): without it every same-version run re-opens to add the lines + // (lib/gitignore.ts). The tests that exercise that path remove it. + writeFileSync(join(proj, '.gitignore'), 'pharn/.run/\npharn/.backup/\n'); vi.spyOn(process, 'cwd').mockReturnValue(proj); cleanup = vi.fn(); scaffoldClone(repo, '1.1.0', 'v2'); @@ -545,6 +549,22 @@ describe('runUpdate --dry-run', () => { expect(outroText()).toContain('Run `pharn update --force` to apply'); }); + it('previews the .gitignore lines a same-version run would add, writing nothing', async () => { + await installed({ skillsVersion: '1.1.0' }); + for (const rel of [CAP_FILE, DOC, HOOK, CMD]) { + write(join(proj, rel), readFileSync(join(repo, rel), 'utf8')); + } + rmSync(join(proj, '.gitignore')); + const before = snapshotTree(proj); + await runUpdate({ dryRun: true }); + expect(snapshotTree(proj)).toEqual(before); + expect(noteBody('PLAN')).toContain( + ".gitignore: would create it with pharn/.run/, pharn/.backup/ (PHARN's run state and update backups).", + ); + // It WOULD write something: .gitignore. + expect(noteBody('PLAN')).not.toContain('Nothing would be written'); + }); + it('lists skip buckets and withholds the version, with the pending record', async () => { await installed(); write(join(proj, DOC), 'MY EDIT'); diff --git a/tests/update-report.test.ts b/tests/update-report.test.ts index 491365b..c9e3cc6 100644 --- a/tests/update-report.test.ts +++ b/tests/update-report.test.ts @@ -15,6 +15,7 @@ const LS = String.fromCodePoint(0x2028); function planReport(over: Partial = {}): UpdatePlanReport { return { + gitignore: { action: 'none' }, currentVersion: '1.0.0', installedVersion: '1.1.0', recordedVersion: '1.0.0', diff --git a/tests/update.test.ts b/tests/update.test.ts index cd1282b..5358f79 100644 --- a/tests/update.test.ts +++ b/tests/update.test.ts @@ -205,6 +205,10 @@ describe('runUpdate (drift-safe)', () => { proj = join(tmp.path(), 'proj'); repo = join(tmp.path(), 'repo'); mkdirSync(proj, { recursive: true }); + // A project that already keeps PHARN's run state and backups out of git + // (0.16.0): without it every same-version run re-opens to add the lines + // (lib/gitignore.ts). The tests that exercise that path remove it. + writeFileSync(join(proj, '.gitignore'), 'pharn/.run/\npharn/.backup/\n'); vi.spyOn(process, 'cwd').mockReturnValue(proj); cleanup = vi.fn(); @@ -732,6 +736,68 @@ describe('runUpdate (drift-safe)', () => { expect(prompts.confirm).not.toHaveBeenCalled(); }); + // 0.16.0: a `.gitignore` that does not cover PHARN's run state and backups + // re-opens the gate too (lib/gitignore.ts) — bounded: the opened run + // appends the lines, so the next run early-returns; and a `.gitignore` the + // run could not write never counts. + it("a .gitignore missing pharn's entries re-opens it; the run appends them, the next run early-returns", async () => { + await installed({ skillsVersion: '1.1.0' }); + writeFileSync(join(proj, '.gitignore'), 'node_modules\n'); + + await runUpdate(); + + expect(prompts.outro).not.toHaveBeenCalledWith( + 'Already up to date (skills v1.1.0).', + ); + expect(fetchRepo).toHaveBeenCalledTimes(1); + expect(noteText()).toContain( + " .gitignore does not yet keep PHARN's run state and backups out of git — this run re-applies the install and adds them.", + ); + expect(readFileSync(join(proj, '.gitignore'), 'utf8')).toBe( + 'node_modules\n# PHARN run state and update backups (added by pharn)\npharn/.run/\npharn/.backup/\n', + ); + expect(vi.mocked(prompts.log.info).mock.calls.map((c) => c[0])).toContain( + "Added pharn/.run/, pharn/.backup/ to .gitignore (PHARN's run state and update backups).", + ); + + vi.clearAllMocks(); + loadArchetypeConfigOrExit.mockReturnValue(readPharnConfig(proj)!); + await runUpdate(); + expect(prompts.outro).toHaveBeenCalledWith( + 'Already up to date (skills v1.1.0).', + ); + expect(fetchRepo).not.toHaveBeenCalled(); + }); + + it('a symlinked .gitignore does not re-open it (the run could not write it)', async () => { + await installed({ skillsVersion: '1.1.0' }); + const outside = join(tmp.path(), 'outside-gitignore'); + writeFileSync(outside, 'node_modules\n'); + rmSync(join(proj, '.gitignore')); + symlinkSync(outside, join(proj, '.gitignore')); + + await runUpdate(); + + expect(prompts.outro).toHaveBeenCalledWith( + 'Already up to date (skills v1.1.0).', + ); + expect(fetchRepo).not.toHaveBeenCalled(); + expect(readFileSync(outside, 'utf8')).toBe('node_modules\n'); + }); + + it('a .gitignore that is a directory does not re-open it', async () => { + await installed({ skillsVersion: '1.1.0' }); + rmSync(join(proj, '.gitignore')); + mkdirSync(join(proj, '.gitignore')); + + await runUpdate(); + + expect(prompts.outro).toHaveBeenCalledWith( + 'Already up to date (skills v1.1.0).', + ); + expect(fetchRepo).not.toHaveBeenCalled(); + }); + // A symlinked component is not "missing": the real run refuses to write // through it, so re-opening could not repair it and would loop forever. it('a recorded path behind a symlinked dir does not re-open it', async () => { @@ -2947,9 +3013,46 @@ describe('runUpdate (drift-safe)', () => { expect(readPharnConfig(proj)!.skillsVersion).toBe('1.1.0'); }); + // Replace the seeded `.gitignore` with a DIRECTORY: pharn refuses to touch + // it (lib/gitignore.ts), so `pharn/.backup/` is neither covered nor added. + function ungitignorable(): void { + rmSync(join(proj, '.gitignore')); + mkdirSync(join(proj, '.gitignore')); + } + + it('drops the .gitignore advice when .gitignore already covers pharn/.backup/', async () => { + await installed(); + write(join(proj, DOC), 'MY LOCAL EDIT'); + + await runUpdate({ force: true }); + + expect(infoLines()).toContain('Backed up 1 file(s)'); + expect(infoLines()).not.toContain('not gitignored'); + }); + + it('drops the .gitignore advice when this run adds pharn/.backup/ to .gitignore', async () => { + await installed(); + write(join(proj, DOC), 'MY LOCAL EDIT'); + rmSync(join(proj, '.gitignore')); + + await runUpdate({ force: true }); + + expect(infoLines()).toContain('Backed up 1 file(s)'); + expect(infoLines()).not.toContain('not gitignored'); + expect(infoLines()).toContain( + "Created .gitignore with pharn/.run/, pharn/.backup/ (PHARN's run state and update backups).", + ); + expect(readFileSync(join(proj, '.gitignore'), 'utf8')).toBe( + '# PHARN run state and update backups (added by pharn)\npharn/.run/\npharn/.backup/\n', + ); + }); + it("prints the backup directory — the user's only pointer to their bytes", async () => { await installed(); write(join(proj, DOC), 'MY LOCAL EDIT'); + // A `.gitignore` pharn has to leave alone (a directory), so the backup + // dir is neither covered nor about to be: the advice is printed. + ungitignorable(); await runUpdate({ force: true }); @@ -3067,6 +3170,7 @@ describe('runUpdate (drift-safe)', () => { // once per stream (the stderr copy is what `2> err.log` keeps), and the // .gitignore hint only once — it is advice, not a pointer. it('prints the .gitignore hint once, and the backup dir once per stream', async () => { + ungitignorable(); await abortedForcedRun(); const infos = vi.mocked(prompts.log.info).mock.calls;