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:
Rongsong Shen 2026-08-25 14:13:40 +00:00
parent 3eba7c16bf
commit d1803376d8
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
5 changed files with 1593 additions and 36 deletions

View file

@ -188,3 +188,4 @@
- **A prior task's own test file can pre-emptively flag a specific assertion as "this will need to change once task N.M lands" in its own comment/message string — grep for the LATER task's own number in sibling test files before assuming "task boundary == only my new file changes."** Task 3.1's `test_dispatch_commit.c::test_dc_worker_not_found_leaves_job_pend_and_alloc_uncommitted` asserted `dispatch_fail_count` stayed unchanged with the message "incrementing it is task 3.3's job, not 3.1's" — a deliberate, anticipated staleness, not an accidental regression. Confirmed via the mandated RED (git-stash job.c only, rebuild, run new tests → fail as expected) → GREEN (restore, rebuild) cycle: the GREEN pass surfaced this ONE pre-existing sibling-test failure (seeded counter 2 → now correctly 3), fixed by updating that one assertion + its message, not by touching my own new file. This is explicitly within "test infrastructure under testsuite/ may change" boundary language even when the task brief's boundary section only names the NEW file — a task boundary permitting "test infrastructure" changes covers fixing a sibling test's own foreseen staleness, not just adding new tests.
- **A "mirror the precedent's free-and-replace pattern exactly" instruction (bug-1678/1687/1689's own established shape) can be actively WRONG for a new field if that field's live-mutation path sources any nested pointer from `makeString()` (bug-1690, WAL_JOB_RUSAGE/job.run_usage).** `update_host_runusage()` (misc.c) builds `xce_host_runusage_t.host_name` from `makeString()` — an RTTI-managed, refcounted, HEADER-BEFORE-DATA pointer (confirmed by reading librtti's actual source in the nix store, `RTTI_DATA_TO_OBJ`: `((meta_obj_t*)(((char*)v) - RTTI_OBJ_HEADER_SIZE))`), NOT a plain `malloc()`. `persist_apply_pump_run()` (the LIVE online-apply pump, confirmed via its own doc comment to run continuously on the primary itself, not just boot replay) re-applies every committed fact — including one the SAME process just live-mutated moments earlier via the synchronous "mutate-then-persist" pattern (the SAME pattern bug-1678's `dispatch_a_job_()` uses for `job.allocs`). For DISPATCH this is safe because `job.allocs`'s nested `host` field is genuinely capnp-wire-decoded end-to-end (arrives from the scheduler already deserialized) — `free_JobAlloc_ptr()` on it is always consistent. For RUSAGE it is NOT safe: calling the capnp-generated `free_JobRunusage_ptr()` on the LIVE (xstring-backed) structure would `free()` a non-malloc'd pointer — guaranteed ASAN heap-corruption abort on the very first live report for a new host. **General check before reusing this "free-and-replace at apply time" precedent for any NEW WAL fact type: grep the field's LIVE-mutation call site for `makeString()` (or any other non-malloc allocator) on any pointer the apply seam might later free — if found, replace with a MERGE (overwrite scalars in place, only adopt genuinely-new nested pointers by ownership transfer, never free an existing one) instead of a free-and-replace.** Proved with a real regression test using an actual `makeString()`'d pointer and asserting POINTER IDENTITY survives the merge (not just value equality) — a value-only check would not have caught this class of bug. Also found, but explicitly NOT fixed (separate file/component, out of scope): `job_destroy()` (`src/lib/jobj.c`)'s own existing manual `FREEUP`-based walk over `run_usage` has this exact same latent host_name/xstring mismatch, independent of and pre-dating this fix.
- **This project's Unity build has `TEST_ASSERT_EQUAL_DOUBLE` silently non-functional (fails every call with "Unity Double Precision Disabled") because `unity_dep = dependency('unity')` is a prebuilt nix package with no `-DUNITY_INCLUDE_DOUBLE`** (Unity's own default is double support OFF unless explicitly opted in; float support is ON by default). Grep any existing test file comparing a C `double` field before assuming `TEST_ASSERT_EQUAL_DOUBLE` works — the established codebase convention (e.g. `test_apply_committed_entry.c`, `test_dispatch_commit.c`, `test_worker_join.c`) is `TEST_ASSERT_EQUAL_FLOAT` even for genuinely-`double`-typed fields (implicit narrow to float; fine for clean/round test fixture values). Caught only by actually running the RED build and reading the real failure message, not by reasoning from Unity's public API alone.
- **`controller_unit_test` (the main shared meson.build `controller_unit_test_src` binary) does NOT link the real `sched.c`/`dataex.c` at all** (bug-1693, 2026-08-25) — `sched.c`'s own `on_scheduler_join()`/`collect_confvars()` and `dataex.c`'s `notify()` collide at link time with fake local definitions several OTHER files in that same binary already provide (`test_commit_barrier_reply.c`, `test_worker_join.c`), so `test_config_repl.c` instead ships a trivial no-op `handle_scheduler_leave(cluster_t *c) { (void)c; return 0; }` stub purely to satisfy the linker. **Any new/changed behavior added to a real `sched.c` function is therefore SILENTLY UNTESTED by any `controller_unit_test`-based test that calls that function by name** — the call resolves to the stub, not the real implementation, with no compile/link error to flag the mismatch (confirmed empirically: a new regression test asserting sub-second wake timing after calling `handle_scheduler_leave()` passed compilation and linking fine, then failed by taking the full 5s deadline — the stub had silently swallowed the call). `nm '<obj>.o' | grep ' T <symbol>$'` across every `.o` in the target's build dir is the fast way to confirm which TU actually provides a given symbol in a given binary before trusting a test's result. The two standalone binaries that DO link the real `sched.c` (`sched_push_unit_test`, `sched_worker_change_unit_test` — see their own `meson.build` comments for the collision rationale) don't link `cluster.c`, so neither binary alone can exercise a fix that spans both files for real; bug-1693's fix split verification across both (a direct-call test against `cluster.c`'s real new function in `controller_unit_test`, plus a small counting-spy test in `test_sched_worker_change.c` proving `sched.c`'s real caller invokes it) rather than inventing a third binary that links both (would need to duplicate most of `controller_unit_test_src`'s already-large dependency list to satisfy `cluster.c`'s own huge fan-out). General lesson: when a fix spans a function in `sched.c`/`dataex.c` AND a function in any of the ~15 other files real-linked into `controller_unit_test_src`, check with `nm`/grep for a same-name fake stub in a sibling test file in that binary BEFORE trusting a test that calls the `sched.c`/`dataex.c` side by name — it may be exercising a stub, not your change.