docs: guard the asymmetric cache keys (#200); document the ltBoot fault lever (#197)

Two knowledge-preservation fixes. No behavior change: server.mjs gains comments
only, verified by `git diff --stat` (comment lines) and an unchanged suite.

#200 — structuredHash and dedupKey compute the same value and must NOT be
collapsed. They take identical arguments and read as obvious duplicate work,
which is how someone "optimises" them into a bug. Their GUARDS differ, verified
line by line rather than asserted:

  structuredHash: if (CACHE_TTL > 0 && !conversationId && !hasCacheControl(...))
  dedupKey:       (!conversationId && !hasCacheControl(...))          <- no CACHE_TTL

CLAUDE_CACHE_TTL defaults to 0, so in the DEFAULT configuration structuredHash
is null while dedupKey must still be computed — it drives #153's single-flight
stampede protection, which is deliberately independent of response caching.
`dedupKey = structuredHash` would silently disable stampede protection by
default, in exactly the concurrent-AI-Task case it exists to bound. The comment
records that, and says what a safe deduplication would look like if anyone
still wants one (compute under the weaker guard, derive under the stronger,
add a stampede test first).

This was first raised in the #194 review as a cosmetic nit and then RETRACTED
by that reviewer after reading the guards — the retraction is the valuable
part, so it goes in the code rather than staying in a PR thread.

#197 — new "Testing: reaching faults inside server.mjs" section in AGENTS.md.
The premise the issue was originally filed on was wrong twice over, and both
errors were mine: first that no live-server harness existed (ltBoot has been
there all along, and I had used it myself in #191), then that a synchronous
fault deep inside spawnClaudeProcess needed a production test hook (it needs
--stack-size, which ltBoot already forwards).

The section documents the fixture, the --stack-size fault lever with the
concrete numbers (~124k element spread threshold at the default stack vs ~24k
at --stack-size=200, which is what fits Linux's 131072-byte MAX_ARG_STRLEN so
the test RUNS in CI instead of skipping), and the three rules that made it hold:
discover the threshold in a child under the same flags, assert the fault
actually fired, and wait for the thing you are about to assert rather than a
proxy for it. Each rule cites the issue that taught it (#193/#199/#203).

Written to be read BEFORE someone concludes a bug class is untestable, since
that conclusion has now been reached wrongly twice in this repo.

cli.js citation: NOT APPLICABLE — comments only, no wire behavior, no code
path, no argv change. Authorized by ADR 0006 as a behaviour-preserving edit;
the field set and every request/response contract are byte-identical.

Tests: 457 passed, 0 failed (unchanged, as expected for a comments-only diff).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017gbqUZ8HfBZpjjbzQ85oH8
This commit is contained in:
2026-07-27 08:53:30 +10:00
co-authored by Claude Opus 5
parent c998370b8a
commit 868a1953f4
2 changed files with 28 additions and 0 deletions
+16
View File
@@ -52,6 +52,22 @@ Runtime: Node.js (ESM, `.mjs` throughout). No build step. No bundler. `server.mj
---
## Testing: reaching faults inside `server.mjs`
`test-features.mjs` cannot `import` `server.mjs` (it calls `server.listen()` at top level), and that has twice led to the wrong conclusion that a class of bug is untestable. It isn't. Read this before writing "no regression test is possible here".
**There is a real live-server fixture.** `ltBoot(env, dir, nodeArgs)` (around `test-features.mjs:990`) spawns the actual `server.mjs` as a child with a **fake `claude` binary**, so integration tests cost no quota. `ltPost` / `ltWait` / `ltFreePort` / `ltDiag` round it out. It already covers boot gates, cache-epoch invalidation across two boots sharing one SQLite store, and system-prompt capture.
**`--stack-size` is a fault lever.** `ltBoot`'s third argument passes V8 flags to the child, which puts recursion- and argument-count-limited failures in reach at a much smaller input. `#193` needed a *synchronous throw* deep inside `spawnClaudeProcess`; `buildCliArgs` does `args.push("--allowedTools", ...ALLOWED_TOOLS)`, and under `--stack-size=200` that spread throws at ~24k elements instead of ~124k — which is what brings the trigger under Linux's `MAX_ARG_STRLEN` (131072 bytes for a single env string) so the test runs in CI rather than skipping. **No production fault hook was needed.**
Three rules that made it hold up, all learned the hard way:
- **Discover the threshold in a child under the same flags**, never in the test process — the parent's stack is not the one that matters.
- **Assert that the fault actually fired**, not just that the outcome looks right. `#193` asserts HTTP 500 *and* that the body carries `call stack size exceeded`; a control mutation (trigger neutered, bug still present) proves the test fails rather than passing vacuously.
- **Wait for the thing you are about to assert**, not a proxy for it. Waiting on `listening on` and then asserting a different line is a race (`#199`); waiting on `exit` and then reading `stderr` is another (`#203` — use `closed`, which guarantees the pipes drained).
Allocate ports with `ltFreePort()`. Fixed ports have caused at least one flake here.
## Release protocol
OCP follows the machine-readable `release_kit:` overlay in `CLAUDE.md` (Iron Rule 5.5). Before any version bump or tag push, re-read that YAML block and walk every item in `new_feature_doc_expectations` and `bootstrap_quirk_policy`. Tag push triggers `.github/workflows/release.yml`, which creates the GitHub Release automatically — do not create the release manually.