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/fetch-hard-deadline/GRILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# GRILL — fetch-hard-deadline

Plan: `.dev/features/fetch-hard-deadline/PLAN.md` · spec-hash `bca940a5…d729d3c4e` matches live
`ARCHITECTURE.md`. Registered grillers: `{"registered":0,"grillers":[]}` → inline axes only.

## Findings

```yaml
- type: FINDING
rule_id: 'P1'
severity: important
file: '.dev/features/fetch-hard-deadline/PLAN.md:44'
problem: "The real trigger (undici 6 + forced GC on Node 20/22) is not reproduced in-suite; the tests model it as 'the body ignores the abort signal'. That is the right invariant to pin, but the GC reproduction should be re-run out-of-suite on Node 22 as evidence."
evidence: 'a download whose body stream IGNORES the abort signal (the post-GC undici shape)'
- type: FINDING
rule_id: 'P5'
severity: minor
file: '.dev/features/fetch-hard-deadline/PLAN.md:25'
problem: "Existing error messages/tests for the 8 s abort (`/abort/i`, 'Could not reach') must keep matching, or users lose the host name in the timeout error."
evidence: 'the timeout error keeps the existing "Could not reach <url>" shape'
```

ADVISORY VERDICT: 2 concerns raised (0 blocking-severity, 2 advisory) — for the human to weigh before /pharn-dev-build.
67 changes: 67 additions & 0 deletions .dev/features/fetch-hard-deadline/PLAN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# PLAN — fetch-hard-deadline (PHARN-09: the network timeouts must hold even when undici drops the abort)

- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4
- increment: a small shared `withDeadline(ms, message, work)` in `src/lib/deadline.ts` races the whole
fetch-and-read against a timer that (a) aborts the request's controller and (b) REJECTS on its own, so
the caller is released at the deadline whether or not the abort reaches the body stream; the three
fetch sites (`downloadArchive`, `fetchCommitSha`, `fetchRemoteSkillsVersion`) use it, and the body
readers cancel their stream on abort so the socket is released too.
- layer(s): the CLI itself (`src/lib/deadline.ts` new, `src/lib/repo.ts`, `src/lib/skills-version.ts`)
- constitution_refs: [P0, P1, P2, P3]

## Discovery — verified this run (P6)

Reproduced in the review: minimal `downloadArchive` equivalent (`for await` over `res.body`,
`AbortController`, 3 s timer, server dripping 1 byte / 250 ms): no GC → `AbortError` at 3.0 s; one
forced `gc()` → "NOT ABORTED after 10 s" on Node 22.22.2 and 20.20.2 (undici 6.x holds the caller's
signal via a `WeakRef`); Node 24 unaffected. The helper agent reproduced it on the real `fetchRepo`
(still downloading at 75 s with a 60 s cap) and `fetchCommitSha` (no answer after 20 s). Timing-
dependent in the wild → Low. `SECURITY.md` / `THREAT-MODEL.md` promise the timeout covers a streamed
body. The engines floor is `>=20.12.0`, so Node 20/22 are supported targets.

## Files

- `src/lib/deadline.ts` — `withDeadline<T>(ms, onTimeout: () => Error, work: (signal) => Promise<T>)`:
one `AbortController`, one timer that aborts AND rejects, `Promise.race`, timer cleared in `finally`,
the losing `work` promise's rejection swallowed (no unhandled rejection) — layer CLI/lib
- `src/lib/repo.ts` — `downloadArchive` and `fetchCommitSha` run under `withDeadline`; the archive body
is read through an explicit reader that is cancelled on abort; `fetchCommitSha` still returns `null`
on timeout (best-effort) — layer CLI/lib
- `src/lib/skills-version.ts` — `fetchRemoteSkillsVersion` under `withDeadline`; the timeout error keeps
the existing "Could not reach <url>" shape — layer CLI/lib
- `tests/deadline.test.ts` — work that never settles is rejected at the deadline with the given error
and the signal is aborted; work that settles first wins and the timer is cleared; a late rejection of
the losing work is not unhandled
- `tests/repo.test.ts` — a download whose body stream IGNORES the abort signal (the post-GC undici
shape) still rejects at `CLONE_TIMEOUT_MS`; `fetchCommitSha` with a never-settling body → `null`
- `tests/repo-signals.test.ts` — its fake download bodies become real web `ReadableStream`s (the shape
`fetch` returns; the download now reads through `getReader()`)
- `tests/skills-version.test.ts` — same "abort not wired to the body" case → rejects at 8 s naming the URL

## Contracts satisfied

- `SECURITY.md` / `THREAT-MODEL.md` (8 s / 60 s timeouts cover the streamed body) and CLAUDE.md "Remote
fetches use `redirect: 'error'`, an 8s timeout, and a 256KB body cap" — now independent of whether
the runtime delivers the abort to the stream.

## Evals to write (P1)

- listed above; the "abort ignored by the body" cases hang on the base source (vitest timeout = fail).

## Guarantee audit (P0)

- "a fetch returns control to pharn within its timeout" → floor: the timer's own `reject` in a
`Promise.race` — it does not depend on undici honouring the signal.
- "the socket is released at the deadline" → best effort (reader.cancel + controller.abort); advisory.

## Trust audit (P2)

- No change to what remote bytes are accepted; only when a read is abandoned.

## Determinism audit (P5)

- Timer-based; tests use fake timers.

## Open questions (HALT)

- none
30 changes: 30 additions & 0 deletions .dev/features/fetch-hard-deadline/REGRESSION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# REGRESSION — fetch-hard-deadline

The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment.

## Base and partition

- **base:** `dd165c837e536d8321d4013b5006e95a7de7dbf9` (`origin/main` at build time; the build is an uncommitted working tree on top of it).
- **inside** (each declared in `PLAN.md` `## Files`): `src/lib/deadline.ts`, `src/lib/repo.ts`, `src/lib/skills-version.ts`, `tests/deadline.test.ts`, `tests/repo-signals.test.ts`, `tests/repo.test.ts`, `tests/skills-version.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.
40 changes: 40 additions & 0 deletions .dev/features/fetch-hard-deadline/REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# REVIEW — fetch-hard-deadline

Floor first: `node .dev/floor/validate.mjs .` → exit 0 (GREEN). Everything below is **advisory**.

## Floor-gate findings (blocking)

None.

- **L-floor (P0):** "a fetch returns control within its timeout" reduces to the timer's own `reject` in
a `Promise.race` — independent of whether the runtime delivers the abort to the body stream.
- **L-eval (P1):** `tests/deadline.test.ts` (4 cases) + one "abort never reaches the body" case per
call site (download, commit-SHA resolve, SKILLS_VERSION) — all three HANG on the base source (vitest
5 s timeout). `repo.test.ts` / `repo-signals.test.ts` fakes now return real web `ReadableStream`s.
- **Out-of-suite evidence (grill #1):** the review's original repro (chunked server dripping every
250 ms, 3 s cap, `gc()` at 1 s): old shape "NOT ABORTED after 10 s" on Node 22.22.2 and 20.20.2;
the `withDeadline` shape rejects at 3.0 s on both.
- **L-trust (P2):** no change to what remote bytes are accepted; the byte caps are unchanged.
- **L-axis (P3):** `lib/deadline.ts` has one axis (bounding a network operation's duration); the two
fetch modules consume it.

## Advisory findings

```yaml
- type: FINDING
rule_id: 'P5'
severity: minor
file: 'src/lib/deadline.ts'
problem: "After the deadline the losing work may keep a socket open until undici's own timeouts if the cancel does not propagate; the CLI's callers exit or continue regardless, so this is a resource note, not a hang."
evidence: 'running.catch(() => undefined);'
- 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/fetch-hard-deadline/SHIP.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# SHIP — fetch-hard-deadline

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/fetch-hard-deadline/VERIFY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# VERIFY — fetch-hard-deadline

## 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.
25 changes: 25 additions & 0 deletions .dev/features/fetch-hard-deadline/regression-report.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"base": "dd165c837e536d8321d4013b5006e95a7de7dbf9",
"inside": [
"src/lib/deadline.ts",
"src/lib/repo.ts",
"src/lib/skills-version.ts",
"tests/deadline.test.ts",
"tests/repo-signals.test.ts",
"tests/repo.test.ts",
"tests/skills-version.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/fetch-hard-deadline/verify-report.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"feature": "fetch-hard-deadline",
"gates": {
"format:check": 0,
"lint": 0,
"lint:md": 0,
"test": 0,
"typecheck": 0,
"validate": 0
},
"verdict": "PASS",
"failing_gates": [],
"verifiers": {
"registered": 0,
"findings": []
}
}
4 changes: 2 additions & 2 deletions .pharn/writes-scope.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"scope": [
".dev/features/capability-index-nonfile-md/SHIP.md"
".dev/features/fetch-hard-deadline/SHIP.md"
],
"set_by": ".claude/commands/pharn-dev-ship.md",
"set_at": "2026-09-24T08:43:14.331Z"
"set_at": "2026-09-24T08:51:54.189Z"
}
50 changes: 50 additions & 0 deletions src/lib/deadline.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// ---------------------------------------------------------------------------
// A hard deadline for a network operation — one that does not depend on the
// runtime delivering an abort.
//
// Every fetch here used the same shape: an AbortController, a setTimeout that
// calls `abort()`, and a `finally` that clears it. That shape relies on undici
// forwarding the abort into the RESPONSE BODY stream. On Node 20/22 (undici 6)
// it holds the caller's signal through a WeakRef; measured, after one full GC a
// server dripping one byte every 250 ms kept a 3 s-capped read alive past 10 s
// (Node 24 is unaffected). undici's own bodyTimeout (300 s) measures the gap
// BETWEEN chunks, so a slow drip never trips it: `init`/`add`/`update`/`status`
// could hang indefinitely — `add`/`update` while holding the project lock.
//
// So the timer here does two things: it aborts the controller (the polite path,
// which releases the socket whenever undici honours it) AND rejects on its own
// through a Promise.race, which returns control to pharn at the deadline
// regardless. Callers additionally cancel their body reader on abort (their
// own listener on their own signal — no WeakRef involved).
//
// One axis (P3): bounding how long a network operation may take.
// ---------------------------------------------------------------------------

/**
* Run `work` with a signal that is aborted after `ms`, and settle no later than
* that: at the deadline the returned promise rejects with `onTimeout()` even if
* `work` never settles. A `work` rejection after the deadline is swallowed — the
* caller has already been answered.
*/
export async function withDeadline<T>(
ms: number,
onTimeout: () => Error,
work: (signal: AbortSignal) => Promise<T>,
): Promise<T> {
const controller = new AbortController();
let timer: ReturnType<typeof setTimeout> | undefined;
const expired = new Promise<never>((_, reject) => {
timer = setTimeout(() => {
controller.abort();
reject(onTimeout());
}, ms);
});
const running = work(controller.signal);
// The loser of the race must never surface as an unhandled rejection.
running.catch(() => undefined);
try {
return await Promise.race([running, expired]);
} finally {
clearTimeout(timer);
}
}
Loading
Loading