From 6e50c589582f0d0ef20ba01c3fe0cdb4f283d440 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 08:55:23 +0000 Subject: [PATCH 1/2] fix(lock,update): an interrupted write exits 130, releases the lock, names the backup (PHARN-10) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While a clack spinner is up, @clack/core turns Ctrl-C into process.exit(0), which runs no `finally`. Reproduced: `update --force` interrupted during its records write printed "Canceled", exited 0, left .pharn.lock behind, and never named the .pharn-backup// it had already created — with originals already overwritten. - withProjectLock registers an `exit` listener while the lock is held: it releases the lock synchronously and turns an exit code of 0 into 130 with one stderr line saying the run was interrupted while writing. - update prints the backup pointer the moment the backup is created (as `add` does and docs/commands/update.md promises), not only at the end. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TvcuVhk8hTeDskp5pAJhnc --- .dev/features/interrupt-exit-code/GRILL.md | 23 +++++++ .dev/features/interrupt-exit-code/PLAN.md | 63 +++++++++++++++++++ .../interrupt-exit-code/REGRESSION.md | 30 +++++++++ .dev/features/interrupt-exit-code/REVIEW.md | 38 +++++++++++ .dev/features/interrupt-exit-code/SHIP.md | 17 +++++ .dev/features/interrupt-exit-code/VERIFY.md | 27 ++++++++ .../regression-report.json | 22 +++++++ .../interrupt-exit-code/verify-report.json | 17 +++++ .pharn/writes-scope.json | 4 +- src/commands/update.ts | 8 ++- src/lib/project-lock.ts | 26 ++++++++ tests/project-lock.test.ts | 34 ++++++++++ tests/update.test.ts | 36 +++++++++++ 13 files changed, 341 insertions(+), 4 deletions(-) create mode 100644 .dev/features/interrupt-exit-code/GRILL.md create mode 100644 .dev/features/interrupt-exit-code/PLAN.md create mode 100644 .dev/features/interrupt-exit-code/REGRESSION.md create mode 100644 .dev/features/interrupt-exit-code/REVIEW.md create mode 100644 .dev/features/interrupt-exit-code/SHIP.md create mode 100644 .dev/features/interrupt-exit-code/VERIFY.md create mode 100644 .dev/features/interrupt-exit-code/regression-report.json create mode 100644 .dev/features/interrupt-exit-code/verify-report.json diff --git a/.dev/features/interrupt-exit-code/GRILL.md b/.dev/features/interrupt-exit-code/GRILL.md new file mode 100644 index 0000000..e0cc0b0 --- /dev/null +++ b/.dev/features/interrupt-exit-code/GRILL.md @@ -0,0 +1,23 @@ +# GRILL — interrupt-exit-code + +Plan: `.dev/features/interrupt-exit-code/PLAN.md` · spec-hash `bca940a5…d729d3c4e` matches live +`ARCHITECTURE.md`. Registered grillers: `{"registered":0,"grillers":[]}` → inline axes only. + +## Findings + +```yaml +- type: FINDING + rule_id: 'P5' + severity: important + file: '.dev/features/interrupt-exit-code/PLAN.md:23' + problem: "The listener's premise — no legitimate exit(0) happens while a lock is held — is true of today's call sites but not enforced; a future prompt inside a lock would have its graceful cancel reported as 130. Name the premise in the code comment so the next edit sees it." + evidence: 'Inside every lock callback there is no prompt that can legitimately `exit(0)`' +- type: FINDING + rule_id: 'P1' + severity: minor + file: '.dev/features/interrupt-exit-code/PLAN.md:35' + problem: 'A child-process test needs tsx to import TS; keep it bounded (one spawn) so it stays cheap in CI.' + evidence: 'a child process that calls `process.exit(0)` while holding the lock exits 130' +``` + +ADVISORY VERDICT: 2 concerns raised (0 blocking-severity, 2 advisory) — for the human to weigh before /pharn-dev-build. diff --git a/.dev/features/interrupt-exit-code/PLAN.md b/.dev/features/interrupt-exit-code/PLAN.md new file mode 100644 index 0000000..888de8c --- /dev/null +++ b/.dev/features/interrupt-exit-code/PLAN.md @@ -0,0 +1,63 @@ +# PLAN — interrupt-exit-code (PHARN-10: Ctrl-C mid-write must not exit 0, strand the lock, or hide the backup) + +- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4 +- increment: (1) while `withProjectLock` holds the lock it registers a process `exit` listener that + releases the lock synchronously and, when the process is exiting with code 0 (clack's `block()` turns + Ctrl-C during a spinner into `process.exit(0)`), sets `process.exitCode = 130` and prints one stderr + line saying the run was interrupted while writing; the listener is removed when `fn` settles. + (2) `update` prints the backup pointer the moment `.pharn-backup//` is created (as `add` does and + `docs/commands/update.md` already claims), instead of only at the end. +- layer(s): the CLI itself (`src/lib/project-lock.ts`, `src/commands/update.ts`) +- constitution_refs: [P0, P1, P4, P5] + +## Discovery — verified this run (P6) + +Reproduced (`repro/ctrlc`): `update --yes --force` in a TTY, records write stalled by an injected fault, +Ctrl-C → "Canceled", **exit 0**, `.pharn.lock` left behind, `.pharn-backup//` created but never +named, `pharn/CONSTITUTION.md` already overwritten. `src/lib/repo.ts:23-40` documents the mechanism +(`@clack/core` `block()` → `process.exit(0)`, no `finally` runs) and only fixes the clone leak. +Inside every lock callback there is no prompt that can legitimately `exit(0)`: `update`'s confirm and +the `remove` picker's confirm are before the lock, and the `add` picker returns `cancelled` and exits +after the lock. `update.ts` prints the backup via `onBackup` only at the end / on failure; +`docs/commands/update.md` says "The directory is printed when it is created". + +## Files + +- `src/lib/project-lock.ts` — the held-lock `exit` listener (release + 130 + one stderr line), added after + acquisition and removed in the existing `finally` — layer CLI/lib +- `src/commands/update.ts` — `onBackup` prints the pointer immediately (stdout, the existing + `printBackupNotice` text); the end-of-run success notice is not printed a second time; the aborted + path keeps its stderr "stopped part-way" warning — layer CLI/command +- `tests/project-lock.test.ts` — a child process that calls `process.exit(0)` while holding the lock + exits 130, prints the interruption line, and leaves no `.pharn.lock`; a normal release leaves no + listener behind +- `tests/update.test.ts` — with `--force`, the backup pointer is printed before the records/config + writes (ordering against a write that fails), and exactly once on success + +## Contracts satisfied + +- `docs/commands/update.md` "The directory is printed when it is created" — now true. +- CLAUDE.md / `project-lock.ts` "fail fast … never wedge" — an interrupted holder no longer strands its + lock. + +## Evals to write (P1) + +- listed above; the exit-130 and print-at-creation cases fail on the base source. + +## Guarantee audit (P0) + +- "an interrupted write never exits 0" → floor for the `process.exit` path: the `exit` listener runs on + every `process.exit`; it is not reached by SIGKILL (named — the next run already breaks a dead-pid lock). +- "the lock is released on `process.exit` while held" → same listener, synchronous `rmSync`. + +## Trust audit (P2) + +- No input change. + +## Determinism audit (P5) + +- Branch on the exit code (`=== 0`) and on "lock still held" (listener registered only while held). + +## Open questions (HALT) + +- none diff --git a/.dev/features/interrupt-exit-code/REGRESSION.md b/.dev/features/interrupt-exit-code/REGRESSION.md new file mode 100644 index 0000000..fd83f44 --- /dev/null +++ b/.dev/features/interrupt-exit-code/REGRESSION.md @@ -0,0 +1,30 @@ +# REGRESSION — interrupt-exit-code + +The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment. + +## Base and partition + +- **base:** `c3e347337b6867af8726cfbd6f5f9dcb57692526` (`origin/main` at build time; the build is an uncommitted working tree on top of it). +- **inside** (each declared in `PLAN.md` `## Files`): `src/commands/update.ts`, `src/lib/project-lock.ts`, `tests/project-lock.test.ts`, `tests/update.test.ts`. +- **scope partition:** `check-regress.mjs scope` exited **0**, `escaped: []`. `.pharn/` (hook scratch) and this + feature's own stage artifacts are not build output. +- **outside gates:** the stdlib `*.test.mjs` / `*.test.cjs` files + whole-repo `validate`; 0 committed eval pairs. +- **style-gate skip:** `inside` touches no shared style config, so `lint` / `format:check` / `lint:md` are absent from both maps. + +## Per-gate exit codes + +| gate | base | head | flipped? | +| ---------- | ---- | ---- | -------- | +| `tests` | 0 | 0 | no | +| `validate` | 0 | 0 | no | + +- `regressions[]`: **empty** +- `pre_existing[]`: **empty** + +## Verdict + +**REGRESSIONS: none — no deterministically-detectable breakage outside the feature.** +(`regression-report.json` `.verdict` = `no-regressions`.) + +Residual (P0/P7): this catches exactly what its suite catches. The vitest suite exercising `src/**` is +owned by `/pharn-dev-build`'s floor and `/pharn-dev-verify`. This certifies the comparison, never the increment. diff --git a/.dev/features/interrupt-exit-code/REVIEW.md b/.dev/features/interrupt-exit-code/REVIEW.md new file mode 100644 index 0000000..85e1f6d --- /dev/null +++ b/.dev/features/interrupt-exit-code/REVIEW.md @@ -0,0 +1,38 @@ +# REVIEW — interrupt-exit-code + +Floor first: `node .dev/floor/validate.mjs .` → exit 0 (GREEN). Everything below is **advisory**. + +## Floor-gate findings (blocking) + +None. + +- **L-floor (P0):** an `exit` listener runs on every `process.exit`; while the lock is held it releases + the lock synchronously and maps exit code 0 → 130. SIGKILL is out of reach (named; the next run + already breaks a dead-pid lock). The premise (no graceful exit(0) inside a lock) is stated in the code, + per grill #1. +- **L-eval (P1):** a real child process calling `process.exit(0)` under the lock exits 130, prints the + interruption line and leaves no `.pharn.lock` (fails on base); the listener is removed after a normal + release; `update` prints the backup pointer at creation (fails on base) and exactly once on success. +- **L-trust (P2):** no input change. +- **L-axis (P3):** the listener lives with the lock it releases; `update` only moves an existing print. + +## Advisory findings + +```yaml +- type: FINDING + rule_id: 'P5' + severity: minor + file: 'src/commands/update.ts' + problem: "On an aborted --force run the pointer now prints twice: once on stdout at creation and once on stderr with the 'stopped part-way' warning — deliberate (stderr logs must carry it), but visible." + evidence: 'printBackupNotice(backup, { aborted: false });' +- type: FINDING + rule_id: 'P4' + severity: minor + file: 'CHANGELOG.md:8' + problem: "No CHANGELOG `[Unreleased]` entry (not in the plan's `## Files`)." + evidence: '## [Unreleased]' +``` + +## Verdict + +**GREEN** — 0 floor-gate findings, 2 advisory findings. No lesson proposed for canon. diff --git a/.dev/features/interrupt-exit-code/SHIP.md b/.dev/features/interrupt-exit-code/SHIP.md new file mode 100644 index 0000000..9a89030 --- /dev/null +++ b/.dev/features/interrupt-exit-code/SHIP.md @@ -0,0 +1,17 @@ +# SHIP — interrupt-exit-code + +Stages run, in order: `/pharn-dev-plan` → GATE 1 (human: **Approve as written**) → `/pharn-dev-grill` → +`/pharn-dev-build` → `/pharn-dev-regress` → `/pharn-dev-verify` → `/pharn-dev-review` → GATE 2. + +| stage | structural verdict (verbatim) | +| -------------------- | ------------------------------------------------------ | +| `/pharn-dev-build` | `node .dev/floor/validate.mjs .` exit `0` | +| `/pharn-dev-regress` | `regression-report.json` `.verdict` = `no-regressions` | +| `/pharn-dev-verify` | `verify-report.json` `.verdict` = `PASS` | + +- Review: [`REVIEW.md`](REVIEW.md) · Grill (advisory): [`GRILL.md`](GRILL.md) +- Run ended at **GATE 2**. The human's standing instruction for this batch: open a PR and merge it only if + its CI checks are green. + +chain ran; the named floor verdicts are as shown — this is NOT a judgment that the increment is good or +wise; that is the human's call at the post-review gate. diff --git a/.dev/features/interrupt-exit-code/VERIFY.md b/.dev/features/interrupt-exit-code/VERIFY.md new file mode 100644 index 0000000..69d8294 --- /dev/null +++ b/.dev/features/interrupt-exit-code/VERIFY.md @@ -0,0 +1,27 @@ +# VERIFY — interrupt-exit-code + +## FLOOR layer (owns the verdict) + +Gates run over the whole repo with the feature present, as a non-root user on node 22 with the session +proxy variables unset (as root with the proxy set, 5 pre-existing tests in `init.test.ts` / +`update.test.ts` fail for environmental reasons, identically at the baseline). + +| gate | exit | +| -------------- | ---- | +| `format:check` | 0 | +| `lint` | 0 | +| `lint:md` | 0 | +| `test` | 0 | +| `typecheck` | 0 | +| `validate` | 0 | + +No `structural:*` gate — the increment ships no eval-actual pair. + +**VERDICT: PASS** (`.dev/floor/check-verify.mjs`, `failing_gates: []`). + +## ADVISORY layer + +`node .dev/floor/count-verifiers.mjs .` → `{"registered":0,"verifiers":[]}` — floor gates only. + +Residual (P0/P7): "verified" means the named gates passed — not that the feature is correct in any sense +the suite does not encode. diff --git a/.dev/features/interrupt-exit-code/regression-report.json b/.dev/features/interrupt-exit-code/regression-report.json new file mode 100644 index 0000000..f3024d9 --- /dev/null +++ b/.dev/features/interrupt-exit-code/regression-report.json @@ -0,0 +1,22 @@ +{ + "base": "c3e347337b6867af8726cfbd6f5f9dcb57692526", + "inside": [ + "src/commands/update.ts", + "src/lib/project-lock.ts", + "tests/project-lock.test.ts", + "tests/update.test.ts" + ], + "outside_gates": { + "tests": { + "base": 0, + "head": 0 + }, + "validate": { + "base": 0, + "head": 0 + } + }, + "regressions": [], + "pre_existing": [], + "verdict": "no-regressions" +} diff --git a/.dev/features/interrupt-exit-code/verify-report.json b/.dev/features/interrupt-exit-code/verify-report.json new file mode 100644 index 0000000..f04b770 --- /dev/null +++ b/.dev/features/interrupt-exit-code/verify-report.json @@ -0,0 +1,17 @@ +{ + "feature": "interrupt-exit-code", + "gates": { + "format:check": 0, + "lint": 0, + "lint:md": 0, + "test": 0, + "typecheck": 0, + "validate": 0 + }, + "verdict": "PASS", + "failing_gates": [], + "verifiers": { + "registered": 0, + "findings": [] + } +} diff --git a/.pharn/writes-scope.json b/.pharn/writes-scope.json index 623222d..f0349b6 100644 --- a/.pharn/writes-scope.json +++ b/.pharn/writes-scope.json @@ -1,7 +1,7 @@ { "scope": [ - ".dev/features/fetch-hard-deadline/SHIP.md" + ".dev/features/interrupt-exit-code/SHIP.md" ], "set_by": ".claude/commands/pharn-dev-ship.md", - "set_at": "2026-09-24T08:51:54.189Z" + "set_at": "2026-09-24T08:55:23.446Z" } diff --git a/src/commands/update.ts b/src/commands/update.ts index 7b330a9..afe230d 100644 --- a/src/commands/update.ts +++ b/src/commands/update.ts @@ -333,6 +333,10 @@ async function runArchetypeUpdate( force, (backup) => { backupRef.current = backup; + // Named the moment it exists (as `add` does): everything after the + // backup can throw or be interrupted, and this pointer is the + // user's only route back to the pre-overwrite bytes. + printBackupNotice(backup, { aborted: false }); }, ); s2.stop( @@ -630,7 +634,7 @@ const FORCEABLE_SKIPS = new Set([ // failure — but they are never silent: each bucket is listed with the one action // that resolves it. function reportOutcome(outcome: UpdateOutcome, force: boolean): void { - const { plan, backup, recordsNote, versionWithheld } = outcome; + const { plan, recordsNote, versionWithheld } = outcome; const { counts } = plan; if (recordsNote) log.warn(`⚠ ${recordsNote}`); @@ -681,7 +685,7 @@ function reportOutcome(outcome: UpdateOutcome, force: boolean): void { note(lines.join('\n'), 'SKIPPED'); } - if (backup) printBackupNotice(backup, { aborted: false }); + // The backup pointer was printed when the backup was created (onBackup). // Both directions are named. `abandonedLayout` has always been computed // direction-agnostically (see its assignment above), but only 'flat' used to be diff --git a/src/lib/project-lock.ts b/src/lib/project-lock.ts index 3e506c3..9afde96 100644 --- a/src/lib/project-lock.ts +++ b/src/lib/project-lock.ts @@ -488,9 +488,35 @@ export async function withProjectLock( throw new ProjectLockedError(refusal(held)); breakStaleLock(cwd, command, observedRaw); } + // While the lock is held, an `exit` listener is the only code that runs on a + // `process.exit` — `finally` does not. The one exit(0) that reaches here is + // @clack/core's block(): during a spinner, stdin is raw and Ctrl-C arrives as a + // KEYPRESS, on which clack calls process.exit(0) (see lib/repo.ts). That used + // to report success over a half-written project and strand this lock. So: + // release synchronously, and turn a 0 into 130 with one honest line. + // + // PREMISE, stated so the next edit sees it: no lock callback shows a prompt + // whose graceful cancel exits 0 — `update`'s and the `remove` picker's + // confirms run before the lock, and the `add` picker returns `cancelled` and + // exits after it. A new prompt inside a lock must keep that shape. + const onExit = (code: number): void => { + release(cwd); + if (code === 0) { + process.exitCode = 130; + try { + process.stderr.write( + `pharn ${command} was interrupted while writing to this project — it may be partially updated. Re-run \`pharn ${command}\`.\n`, + ); + } catch { + /* the exit code already tells the truth */ + } + } + }; + process.on('exit', onExit); try { return await fn(); } finally { + process.off('exit', onExit); release(cwd); } } diff --git a/tests/project-lock.test.ts b/tests/project-lock.test.ts index 00ea37d..6adba40 100644 --- a/tests/project-lock.test.ts +++ b/tests/project-lock.test.ts @@ -391,3 +391,37 @@ describe('withProjectLock — a lock being written is live (PHARN-03)', () => { expect(existsSync(lockPath(dir))).toBe(false); }); }); + +// PHARN-10: while a spinner is up, @clack/core turns Ctrl-C into +// process.exit(0), which runs no `finally`. Holding the lock at that moment used +// to report success over a half-written project and strand .pharn.lock. +describe('withProjectLock — process.exit while held (PHARN-10)', () => { + const tmp = useTmpDir(); + + it('exits 130, says so on stderr, and releases the lock', async () => { + const { spawnSync } = await import('node:child_process'); + const { fileURLToPath } = await import('node:url'); + const lockModule = fileURLToPath( + new URL('../src/lib/project-lock.ts', import.meta.url), + ); + const dir = tmp.path(); + const child = ` + const { withProjectLock } = await import(${JSON.stringify(lockModule)}); + await withProjectLock(process.argv[1], 'update', () => { process.exit(0); }); + `; + const r = spawnSync( + process.execPath, + ['--import', 'tsx', '--input-type=module', '-e', child, dir], + { encoding: 'utf8' }, + ); + expect(r.status).toBe(130); + expect(r.stderr).toContain('pharn update was interrupted while writing'); + expect(existsSync(lockPath(dir))).toBe(false); + }, 30_000); + + it('removes its exit listener once the lock is released normally', async () => { + const before = process.listenerCount('exit'); + await withProjectLock(tmp.path(), 'add', () => undefined); + expect(process.listenerCount('exit')).toBe(before); + }); +}); diff --git a/tests/update.test.ts b/tests/update.test.ts index 1add33c..69b42b3 100644 --- a/tests/update.test.ts +++ b/tests/update.test.ts @@ -1648,6 +1648,42 @@ describe('runUpdate (drift-safe)', () => { expect(printedLines()).toContain(`${BACKUP_DIR}/${dirs[0]!}`); }); + // PHARN-10: the pointer is printed the moment the backup EXISTS (as `add` + // does, and as docs/commands/update.md promises) — an interrupt after it + // (clack's Ctrl-C → process.exit, which runs no catch) must not hide it. + it('prints the backup pointer at creation, before a later write fails', async () => { + await installed(); + write(join(proj, DOC), 'MY LOCAL EDIT'); + rmSync(join(proj, 'pharn.config.json'), { force: true }); + mkdirSync(join(proj, 'pharn.config.json'), { recursive: true }); + + await expect(runUpdate({ force: true })).rejects.toMatchObject( + new ProcessExit(1), + ); + + const atCreation = vi + .mocked(prompts.log.info) + .mock.calls.filter( + (c) => + String(c[0]).startsWith('Backed up') && + (c[1] as { output?: unknown } | undefined)?.output === + process.stdout, + ); + expect(atCreation).toHaveLength(1); + }); + + it('names the backup exactly once on a successful --force run', async () => { + await installed(); + write(join(proj, DOC), 'MY LOCAL EDIT'); + + await runUpdate({ force: true }); + + const named = vi + .mocked(prompts.log.info) + .mock.calls.filter((c) => String(c[0]).startsWith('Backed up')); + expect(named).toHaveLength(1); + }); + it('aborts without touching any original when the backup cannot be written', async () => { await installed(); write(join(proj, DOC), 'MY LOCAL EDIT'); From 953704ff94520b8967c61e22e1d2f99485fe33b2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 08:58:56 +0000 Subject: [PATCH 2/2] test(lock,hooks): cover the held-lock exit listener in-process; restore the 97% gate CI's coverage gate failed at 96.79% statements: the exit listener only ran in a child process (invisible to coverage), on top of uncovered hook-wiring error branches. Adds in-process listener tests (0 -> 130, non-zero untouched) and two hook-wiring error-branch tests: 97.10%. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TvcuVhk8hTeDskp5pAJhnc --- .dev/features/interrupt-exit-code/PLAN.md | 5 +++ .dev/features/interrupt-exit-code/REVIEW.md | 8 +++++ .pharn/writes-scope.json | 6 ++-- tests/hook-wiring.test.ts | 22 ++++++++++++ tests/project-lock.test.ts | 39 ++++++++++++++++++++- 5 files changed, 76 insertions(+), 4 deletions(-) diff --git a/.dev/features/interrupt-exit-code/PLAN.md b/.dev/features/interrupt-exit-code/PLAN.md index 888de8c..0458f92 100644 --- a/.dev/features/interrupt-exit-code/PLAN.md +++ b/.dev/features/interrupt-exit-code/PLAN.md @@ -34,6 +34,11 @@ after the lock. `update.ts` prints the backup via `onBackup` only at the end / o - `tests/update.test.ts` — with `--force`, the backup pointer is printed before the records/config writes (ordering against a write that fails), and exactly once on success +- `tests/hook-wiring.test.ts` — two error-branch cases (`.claude` is a regular file; `settings.json` is a + directory). Not this finding's behavior: CI's repo-wide 97% statement-coverage gate failed on this PR + (96.79%) because the exit listener only runs in a child process, and these uncovered branches from + PHARN-04 are the cheapest honest way back over the line + ## Contracts satisfied - `docs/commands/update.md` "The directory is printed when it is created" — now true. diff --git a/.dev/features/interrupt-exit-code/REVIEW.md b/.dev/features/interrupt-exit-code/REVIEW.md index 85e1f6d..8dfbd68 100644 --- a/.dev/features/interrupt-exit-code/REVIEW.md +++ b/.dev/features/interrupt-exit-code/REVIEW.md @@ -16,6 +16,14 @@ None. - **L-trust (P2):** no input change. - **L-axis (P3):** the listener lives with the lock it releases; `update` only moves an existing print. +## CI follow-up (floor, acted on) + +CI's `Test` job (`test:coverage`) failed the repo-wide 97% statement gate at **96.79%**: the new exit +listener only ran in a child process, which coverage does not see, on top of uncovered error branches +left by PHARN-04. Fixed with two in-process listener tests (0 → 130; non-zero untouched) and two +`hook-wiring` error-branch tests → **97.10%** locally (non-root, node 22). The local gate script now runs +`test:coverage`, not `npm test`, so this class of miss is caught before a push. + ## Advisory findings ```yaml diff --git a/.pharn/writes-scope.json b/.pharn/writes-scope.json index f0349b6..164c0a3 100644 --- a/.pharn/writes-scope.json +++ b/.pharn/writes-scope.json @@ -1,7 +1,7 @@ { "scope": [ - ".dev/features/interrupt-exit-code/SHIP.md" + ".dev/features/interrupt-exit-code/REVIEW.md" ], - "set_by": ".claude/commands/pharn-dev-ship.md", - "set_at": "2026-09-24T08:55:23.446Z" + "set_by": ".claude/commands/pharn-dev-review.md", + "set_at": "2026-09-24T08:58:56.214Z" } diff --git a/tests/hook-wiring.test.ts b/tests/hook-wiring.test.ts index 9567898..b4eb094 100644 --- a/tests/hook-wiring.test.ts +++ b/tests/hook-wiring.test.ts @@ -147,6 +147,28 @@ describe('diffHookWiring', () => { }, ); + it('a project whose .claude is a regular file → unreadable, not a throw', () => { + const repo = join(tmp.path(), 'repo'); + const proj = join(tmp.path(), 'proj'); + mkdirSync(join(repo, '.claude'), { recursive: true }); + writeFileSync(join(repo, '.claude/settings.json'), JSON.stringify(NEW)); + mkdirSync(proj, { recursive: true }); + writeFileSync(join(proj, '.claude'), 'not a directory'); + expect(diffHookWiring(repo, proj)).toMatchObject({ + status: 'unreadable', + reason: 'a path component is not a directory', + }); + }); + + it('a project settings.json that is a directory → unreadable', () => { + const { repo, proj } = setup(NEW, undefined); + mkdirSync(join(proj, '.claude/settings.json')); + expect(diffHookWiring(repo, proj)).toMatchObject({ + status: 'unreadable', + reason: 'it is not a regular file', + }); + }); + it('refuses to follow a symlinked project settings.json', () => { const { repo, proj } = setup(NEW, undefined); const outside = join(tmp.path(), 'outside.json'); diff --git a/tests/project-lock.test.ts b/tests/project-lock.test.ts index 6adba40..f57a0ed 100644 --- a/tests/project-lock.test.ts +++ b/tests/project-lock.test.ts @@ -10,7 +10,7 @@ import { } from 'node:fs'; import { hostname } from 'node:os'; import { join } from 'node:path'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { useTmpDir } from './helpers.js'; import { LOCK_FILE, @@ -419,6 +419,43 @@ describe('withProjectLock — process.exit while held (PHARN-10)', () => { expect(existsSync(lockPath(dir))).toBe(false); }, 30_000); + // The same listener, exercised IN-PROCESS (coverage cannot see the child + // above): emitting `exit` while the lock is held is what process.exit does. + it('in-process: the held-lock exit listener releases the lock and maps 0 → 130', async () => { + const dir = tmp.path(); + const write = vi.spyOn(process.stderr, 'write').mockReturnValue(true); + const before = process.exitCode; + try { + await withProjectLock(dir, 'remove', () => { + expect(existsSync(lockPath(dir))).toBe(true); + process.emit('exit', 0); + expect(existsSync(lockPath(dir))).toBe(false); + expect(process.exitCode).toBe(130); + }); + expect(String(write.mock.calls[0]?.[0])).toContain( + 'pharn remove was interrupted while writing', + ); + } finally { + process.exitCode = before; + write.mockRestore(); + } + }); + + it('in-process: a non-zero exit code is left alone', async () => { + const dir = tmp.path(); + const before = process.exitCode; + try { + await withProjectLock(dir, 'add', () => { + process.exitCode = undefined; + process.emit('exit', 1); + expect(process.exitCode).toBeUndefined(); + expect(existsSync(lockPath(dir))).toBe(false); + }); + } finally { + process.exitCode = before; + } + }); + it('removes its exit listener once the lock is released normally', async () => { const before = process.listenerCount('exit'); await withProjectLock(tmp.path(), 'add', () => undefined);