Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .dev/features/interrupt-exit-code/GRILL.md
Original file line number Diff line number Diff line change
@@ -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.
68 changes: 68 additions & 0 deletions .dev/features/interrupt-exit-code/PLAN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# 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/<ts>/` 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/<ts>/` 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

- `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.
- 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
30 changes: 30 additions & 0 deletions .dev/features/interrupt-exit-code/REGRESSION.md
Original file line number Diff line number Diff line change
@@ -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.
46 changes: 46 additions & 0 deletions .dev/features/interrupt-exit-code/REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# 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.

## 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
- 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.
17 changes: 17 additions & 0 deletions .dev/features/interrupt-exit-code/SHIP.md
Original file line number Diff line number Diff line change
@@ -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.
27 changes: 27 additions & 0 deletions .dev/features/interrupt-exit-code/VERIFY.md
Original file line number Diff line number Diff line change
@@ -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.
22 changes: 22 additions & 0 deletions .dev/features/interrupt-exit-code/regression-report.json
Original file line number Diff line number Diff line change
@@ -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"
}
17 changes: 17 additions & 0 deletions .dev/features/interrupt-exit-code/verify-report.json
Original file line number Diff line number Diff line change
@@ -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": []
}
}
6 changes: 3 additions & 3 deletions .pharn/writes-scope.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"scope": [
".dev/features/fetch-hard-deadline/SHIP.md"
".dev/features/interrupt-exit-code/REVIEW.md"
],
"set_by": ".claude/commands/pharn-dev-ship.md",
"set_at": "2026-09-24T08:51:54.189Z"
"set_by": ".claude/commands/pharn-dev-review.md",
"set_at": "2026-09-24T08:58:56.214Z"
}
8 changes: 6 additions & 2 deletions src/commands/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -630,7 +634,7 @@ const FORCEABLE_SKIPS = new Set<UpdateLabel | 'unreadable'>([
// 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}`);
Expand Down Expand Up @@ -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
Expand Down
26 changes: 26 additions & 0 deletions src/lib/project-lock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -488,9 +488,35 @@ export async function withProjectLock<T>(
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);
}
}
22 changes: 22 additions & 0 deletions tests/hook-wiring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Loading
Loading