fix(core): an accepted non-2xx no longer becomes a cached absence (#704)

[rejifald]

The bug

verdict: { accept: [404] } makes a declared non-2xx a successful outcome. The cache's store
gate was out.ok and nothing else, so the absence behind that 404 was written to the store.

packages/core/src/engine.ts:1668-1670
(on origin/main):

const store = async (out: RunOutcome) => {
    if (out.ok) await op.set(out.value, out.status, out.vary);
};

Two consequences, both verified live on origin/main:

1. A record created after the 404 is masked for the whole TTL. The absence is cached, so the
stitch keeps answering "not found" long after the thing exists.

2. The cached 404 crosses into a stitch that never accepted it.
engine.ts:1658-1662
replays a hit as resultEvt(found.data, found.status, 0) — without running the reader's
interpret/verdict at all:

if (!stale) {
    yield cacheEvt(ctl.revalidateOnHit ? 'hit (revalidated)' : 'hit');
    yield resultEvt(found.data, found.status, 0);
    yield doneEvt(true, t0, 0);
    return;
}

So an entry's accept list ends up belonging to its writer rather than its reader. Two
stitches reach one key easily: cacheStitchId falls back to cfg.name ?? cfg.path ?? 'stitch'
(cache.ts:294-296),
so two unnamed stitches over one URL already share a namespace. That collision is #704 §3 and
stays open — this fix makes the replay unreachable through an accepted non-2xx regardless of it.

The fix

The gate now reads the status, accept-blind:

if (out.ok && out.status < 400)
    await op.set(out.value, out.status, out.vary);

< 400 is classifyStatus's own threshold minus the accept term
(surface.ts:147) —
exactly the set every reader counts as a success on its own merits. Nothing a verdict.accept
had to rescue becomes durable, so consequence 2 is closed by construction: there is no stored
non-2xx left to replay.

It is the narrowest gate that does the job. 2xx/3xx still cache exactly as before (a 301 under
redirect: 'manual' is canonically cacheable and keeps working); only the accepted-≥400 set, which
is precisely the buggy set, stops being written.

Coalescing is deliberately untouched. claim.settle stays ungated: the coalescer belongs to the
stitch that owns it, so every follower shares the leader's verdict, and sharing one concurrent
accepted 404 among identical in-flight callers is the live behaviour each would have got anyway.
Only the durable write — which outlives the burst and can be read back by someone else — is held
back.

Why inline, and not a predicate plus a trace event

This started as ctl.storableStatus(status) on the cache controller with a
cacheEvt('bypass: non-storable status 404') alongside it, mirroring the nearby
bypass: non-storable responseType. It did not fit the bundle budget. runCached is in the core
entry, and #676 left it 14 B of headroom:

variant	whole entry	vs budget 24.10 KB
main	24664 B	✓ 14 B left
predicate + bypass: event	24694 B (+30 B)	✗ over by 16 B
bare predicate, no event	~24685 B (+21 B)	✗ over by 7 B
inline out.status < 400	24670 B (+6 B)	✓ 8 B left

That file says sizing a budget step is the maintainer's call, not a bug fix's, so no budget is
raised here — the fix fits under the existing ceiling instead. import { stitch } is +7 B
(22026 → 22033 B) against a 22067 B budget, 34 B left. The advertised 24 / 22 kB figures do not
move, verified via bundle-size.mjs --json against the bundle-advertised-size tether.

The cost is that the skipped write is silent. The reasoning lives in a comment at the gate,
which costs nothing at runtime.

Semver: a fix, not a breaking change

Filed under ### Fixed.

A caller who today deliberately caches an accepted 404 does lose that caching and pays a live call
per call — the one visible consequence, and the CHANGELOG entry says so. It is still a fix, not a
break:

• The entry it loses was never sound. ADR 0003 decision 2 has it that a stored entry is always a
  known-good value; this one could be handed to a reader whose verdict rejects it.
• The success value a caller sees is unchanged. Only staleness and the origin-call count move,
  and both move toward correct: where the old behaviour returned a stale absence, the new one
  returns the record that now exists.
• The one case where a result changes — a reader being handed the writer's accepted 404 as a
  success — is the bug itself. Restoring the reader's declared verdict is a correction, not a
  removal.

What I tested

Four tests in packages/core/test/cache.spec.ts, matching the existing counting-adapter style:

1. The absence does not mask the record created after it — an adapter that 404s once then
   serves the record; the second call reaches the origin and returns it.
2. The cache is still consulted — the 404 run still misses and runs the chain, and the 200
   that follows caches and replays normally (so the skip is on the write only, not a full bypass).
3. Cross-stitch replay (consequence 2) — two stitches over one memoryStore landing on one
   derived key; the writer accepts 404, the reader does not. The reader reaches the origin and fails
   on its own verdict instead of inheriting the writer's entry.
4. A plain 200 caches and replays exactly as before — same accept: [404] list, which simply
   never fires.

Tests 1-3 fail on main (verified by reverting the guard to if (out.ok)); test 4 passes both
ways, which is the point of it.

Gates

All green from the repo root: prettier --write, check:lint, check:types, test
(139 files, 1495 passed), check-changelog.mjs, check-contract.mjs, check-unknown-keys.mjs, and
check:size (added after noticing CI runs it as its own job).

check-contract.mjs is worth a note: the original cacheableStatus spelling tripped R8/P24 —
cacheableMethod + cacheableStatus share a leading-word prefix and would have had to fold into an
envelope. Moot now that the check is inline, but it is a second, independent reason the controller
predicate was the wrong shape here.

Scope

§2 (no working invalidate spelling), §3 (cacheStitchId collision) and §4 are untouched — hence
Refs, not Fixes.

Refs #704

🤖 Generated with Claude Code