chore(wolf): sync anatomy/cerebrum/memory/hooks/token-ledger bookkeeping
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
This commit is contained in:
parent
d0e5f85aa8
commit
c83b388be2
5 changed files with 1330 additions and 37 deletions
|
|
@ -125,6 +125,9 @@
|
|||
- **(2026-08-25, bug-1691) A shared-fixture footgun found copy-pasting a sibling WAL-fact test file's "drive the real production entry point through a real restart cycle" test: `persist_init(&c.persist, &cfg, NULL)` (the exact line every prior sibling test — bug-1687/1689's START/FINISH suites — uses) is WRONG for a hook-driven fact type, even though it is byte-identical to what those working sibling tests do.** `persist_start()` only registers `persist_job_hook_fun`/`persist_queue_hook_fun`/`persist_worker_hook_fun` on `p->cluster->hooksys` when `p->cluster != NULL` (persist.c). DISPATCH/START/FINISH's production functions call `persist_submit_job_*_fact()` DIRECTLY, bypassing the hook system entirely, so those sibling tests' `NULL` 3rd-arg is harmless. But CKPT (like bug-1690's RUSAGE) is HOOK-DRIVEN: `on_job_ckpt()` only ever calls `xce_run_hooks(..., HOOK_DETAIL_JOB_CKPT, ...)` and relies ENTIRELY on `persist_job_hook_fun` being a registered subscriber to reach the WAL at all. With `NULL` passed, the real-call test silently appends NOTHING (no crash — `xce_run_hooks` on an unregistered detail is simply a no-op), so the restart-replay assertion fails with "ckptinfo NULL" even though the fix code itself was already fully correct — a genuine RED-signal red herring that looks exactly like a fix defect. Diagnosed by noticing the 6 hand-built-payload tests all passed (proving the apply-seam decode/adopt logic itself was right) while ONLY the real-production-call test failed, then re-reading `persist_start()`'s actual gating condition rather than assuming the copied fixture was correct because it was copied from a passing sibling. **General rule: before copying a sibling WAL-fact test file's "drive the real production function" fixture, check whether YOUR fact type's producer calls `persist_submit_job_*_fact()` directly (safe to pass NULL) or is purely hook-driven via `xce_run_hooks()` (MUST pass a real `cluster_t*` to `persist_init()`'s 3rd arg so `persist_start()` registers the hook) — grep the producer function's own body, don't assume parity from a structurally-similar sibling test.**
|
||||
- **(2026-08-25, bug-1691) `controller_unit_test` (not just `xce_test`, extending the existing entry on this) also loses ALL buffered stdout (both PASS and FAIL lines) when run directly with output redirected to a file/pipe AND `ASAN_OPTIONS`'s default `detect_leaks=1` finds leaks** — LeakSanitizer's atexit-registered check appears to bypass libc's normal `exit()`-time stdio flush, so a process that ran 7/7 tests successfully to completion produced a log with ONE clean LeakSanitizer summary and ZERO printed PASS/FAIL text anywhere, first reading as "harness produced no output, something is badly wrong" before realizing it was a buffering artifact. `stdbuf -oL` does NOT fix it (worse: it breaks ASAN's LD_PRELOAD-ordering requirement, producing `ASan runtime does not come first in initial library list`) — the correct fix is `ASAN_OPTIONS=detect_leaks=0 ./controller_unit_test -t <group>` when reading stdout directly (matches meson's own test-harness invocation, which already sets this — the SAME binary run through `meson test` shows real PASS/FAIL text fine). The leaks themselves (spy-fixture jobs created via `makeJob()` into a `jobmgr` with no `.datakiller`, never individually torn down) are PRE-EXISTING and IDENTICAL across every sibling `test_wal_job_*_fact.c` suite (confirmed: `test_wal_job_rusage_fact` in isolation shows 78 leaked allocations too) — not a new leak introduced by any one fix, not worth chasing.
|
||||
|
||||
- **WAL field-parity fix family (bug-1678/1687/1689/1690/1691/1692, controller persist.c/job.c)**: when a hook-driven WAL fact (payload built inside `persist_job_hook_fun`, not a direct `persist_submit_job_*_fact()` call site) needs a new field added to survive replay: (1) check whether the field's own release function uses `FREEUP()`/`free()` (plain malloc/strdup/calloc — safe to free-and-replace, e.g. `JobAlloc`/`JobPIDInfo`/`JobPendReason`) vs `dereference()` (xstring/RTTI-backed, e.g. `ckptinfo`) vs "can be the SAME live object the apply pump re-applies moments later" (needs a MERGE, not free-and-replace — only `WAL_JOB_RUSAGE`'s `run_usage` needed this, because `update_host_runusage()` mutates it in place before the WAL fact is even appended); (2) check whether adoption must be gated on `JOB_TRANSITION_APPLIED` or unconditional — if the setter never changes `job.state` (true for CKPT/RUSAGE/PREASON, false for START/FINISH/DISPATCH), gating makes the fix dead code since every real replay hits `JOB_TRANSITION_NOOP_DUPLICATE`; (3) **check whether the field has a genuine CLEAR-to-NULL operation** (only `pend_reasons`/`do_clr_preason` in this family so far) — if so, "trailing bytes present or absent" (bug-1690/1691's convention) cannot distinguish a legitimate clear from an old-format payload; needs an explicit presence byte instead. Always re-verify (2) and (3) from the real setter code, never assume the newest precedent's shape applies unchanged.
|
||||
- **capnp-c's generated decode path does not validate content, only bounds via `capn_getp`/`capn_resolve`**: passing well-formed-length-but-semantically-garbage bytes to a `decode_X()`/`read_X()` pair (e.g. `decode_JobPendReason`) does NOT return NULL or error — it silently produces a zero-valued struct. A "decode failure returns NULL" defensive branch (`decode_X() == NULL`) is only reachable via allocation failure or an upstream length/bounds guard (e.g. `payload_len` too short to even attempt the read) — never via corrupt-but-present bytes. Don't write a regression test assuming garbadge bytes will fail decode; it will silently pass through and the test either false-passes or fails for a confusing, misleading reason (`Expected 4 Was 0` — the "corrupted" value decoded as an all-zero struct, not a NULL).
|
||||
|
||||
## Do-Not-Repeat
|
||||
- **When translating a `fp_ok`-tracked, per-step-gated failure-detection pattern (like `xce-build.sh`'s dirty-fingerprint computation) into a Make `$(shell sh -c '...')` one-liner, do NOT collapse two commands into `{ cmd1; cmd2; } >"$$fp_tmp"` and gate the whole group with one `if`** — a POSIX sequential group's exit status reflects only its LAST command, so `cmd1`'s failure is silently discarded whenever `cmd2` still succeeds (2026-08-21, bug-1612: `git diff HEAD` failing was swallowed because the trailing `git ls-files | sort | xargs cat` pipe still exited 0, producing a fabricated fingerprint instead of degrading to empty). Give each step its own explicit `if ! step; then fp_ok=0; fi` (or `&&`-chain), exactly mirroring the reference script's discipline, even inside a cramped one-liner. Caught by adversarial code review with a targeted failure-injection test (wrapper `git` on `PATH` that fails only the targeted subcommand) — that test pattern is the reusable way to verify this class of fix.
|
||||
- Don't run `clang-format -i` on whole existing files here — the repo isn't clang-format-clean at baseline, so a full reformat buries the real diff; recovering via `git checkout HEAD -- <file>` on a file with other uncommitted work destroys that work irrecoverably.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue