Skip to content

Commit 4ec3551

Browse files
committed
fix(scripts): teach check:durability-log-level a declared failure-propagation vocabulary (#5241)
The gate had two legal answers for a catch guarding a durability-critical callee — rethrow, or log at `error` — and no way to express the third: the failure was handed to the CALLER. AGENTS.md's own judgment question ("does the system still look normal from the outside?") answers NO for that shape, so it is not a degradation at all. #4754 hit this the moment `saveMetaItem` entered the vocabulary: three of its four hits were propagation, not degradation, and had to be parked in `durability-degradation.baseline.json` — entries for CORRECT code, in a shrink-only ledger whose header calls every line debt. Worse, the cheapest way out at `meta.ts` was to bolt on a `logger.error` on a path whose common case is a rejected request: one durability `error` per bad keystroke, the mirror-image failure AGENTS.md names. The vocabulary is DECLARED, never guessed, and the declaration only supplies the NAME — the checker still proves the STRUCTURE (every path out of the catch delivers the failure), so a declaration cannot become a baseline entry wearing a friendlier name: - FAILURE_PROPAGATION_CALLEES — repo-wide names (`errorFromThrown`), split by how they deliver: `via: 'return'` (the value IS the answer, so it only counts inside a `return`) vs `via: 'effect'` (the call IS the delivery). - FAILURE_PROPAGATION_SITES — function-scoped names for the second shape: a batch whose contract is a per-item outcome report, delivering through `record(...)` / `failed.push(...)`. Keyed by file plus enclosing FUNCTION, not by file: `protocol.ts` is nine thousand lines and a file-wide licence for `saveMetaItem` would hide the next #4669. Entries are staleness-checked. `catchDeliversFailure()` is conservative by construction: it can carry a delivery forward but never invent one, and any exit it cannot prove delivered counts as an escape — "cannot prove" reads as "does not deliver", which judges the seam instead of excusing it. Both #5241 baseline entries went stale and are deleted; the ledger is now empty, which is its intended steady state. No production code changed — the three sites were correct all along. Self-test grows 19 -> 35 cases, pinning both halves of the rule: partial propagation, an envelope built but never returned, a delivery in a later callback, a report written on one branch only, an undeclared report shape, and a site declaration keyed to a different function all still flag. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V7WetGmnfoXNn8cLieKKmx
1 parent e0b2ea7 commit 4ec3551

3 files changed

Lines changed: 669 additions & 55 deletions

File tree

AGENTS.md

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,22 @@ mirror-image failure: it trains everyone to skim `error`, which is what made the
608608
composition branch is usually functional and usually belongs at `warn`; a `catch`
609609
around a write, a DDL call, or a store initialization is where this rule bites.
610610

611+
**A failure handed to the CALLER is not a degradation at all** — this is the
612+
third legal answer, and forgetting it is how the mirror-image failure gets
613+
written. Ask the judgment question honestly: a `/meta` PUT whose `catch` answers
614+
`errorFromThrown(e, 400)`, or a batch whose contract IS a per-item outcome report
615+
and whose `catch` writes the failure into it, does **not** look normal from the
616+
outside — the requester was told, per item, that the write did not land. That is
617+
already louder than a log line. Do **not** bolt a `logger.error` onto such a
618+
site: the common case on a validation path is an author submitting an off-spec
619+
body, so you would emit one durability `error` per rejected keystroke, which is
620+
exactly what makes `error` unreadable. Declare **how it delivers** instead —
621+
`FAILURE_PROPAGATION_CALLEES` (repo-wide names like `errorFromThrown`) or the
622+
function-scoped `FAILURE_PROPAGATION_SITES` (local report sinks, whose names mean
623+
nothing repo-wide) in the checker, which then proves structurally that *every*
624+
path out of the `catch` delivers. #5241 added it after #4754 was forced to park
625+
three correct sites in the baseline for want of a way to say this.
626+
611627
**It has teeth** (a rule this repo only writes down is the very "declared ≠
612628
enforced" shape it keeps paying to fix): `pnpm check:durability-log-level` walks
613629
the AST for `catch` blocks guarding a declared vocabulary of durability-critical
@@ -617,7 +633,11 @@ known ones from regressing. Found a new one? Add it to
617633
`DURABILITY_CRITICAL_CALLEES` in `scripts/check-durability-degradation-log-level.mjs`
618634
in the same PR that fixes it. Accepted exceptions live in
619635
`scripts/durability-degradation.baseline.json`, hand-edited with a reason and
620-
shrink-only.
636+
shrink-only — and that file is currently **empty**, which is its intended steady
637+
state: an entry there means a real degradation nobody has fixed yet, never a site
638+
the gate merely cannot classify. If a red seam turns out to hand its failure to
639+
the caller, declare the propagation vocabulary above; do not baseline correct
640+
code.
621641

622642
---
623643

0 commit comments

Comments
 (0)