chore(openwolf): sync anatomy index, memory log, and cerebrum notes
Hook-maintained OpenWolf bookkeeping accumulated across this session's work (controller-status Req 13 delta, deploy-ha.sh sudo-orphan fix, xceadmin --controller failover-list fix). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RLSNJhNoULvVcUhXEFafQC
This commit is contained in:
parent
723fdb637b
commit
bfb0db4af9
4 changed files with 1368 additions and 78 deletions
|
|
@ -237,3 +237,5 @@
|
|||
- **(2026-09-11, bug-1804) A test that calls a "re-derive from scratch" function (`sched_account_jobs()`) a SECOND time to prove a later event didn't change anything must reset the state it re-derives FIRST (`core_reset_workers()`), exactly mirroring the real per-round `xce_core_begin_session()`-then-`sched_account_jobs()` sequence — calling it twice back-to-back with no reset in between is a test-methodology artifact (an artificial double-charge, e.g. `resInUse` going 4.0→8.0) that can never happen in real production flow, not a real bug.** Caught immediately by RED/GREEN: the first draft's "redundant confirmation event must not double-charge" assertion failed with `Expected 4 Was 8` purely because the test itself skipped the reset step production always performs — fixed by calling `core_reset_workers()` (white-box-includable from `core_session.c`, already `#include`d in this test file) before the second `sched_account_jobs()` call, not by touching any production code. General pattern: before treating a `sched_account_jobs()`-adjacent test failure as evidence of a real double-charge bug, check whether the test itself reproduces the real reset-then-resum round-boundary sequence, or just calls the resum half twice.
|
||||
- **(2026-09-14, bug-1807) `optind = 1;` alone does NOT fully reset glibc `getopt_long()` state between independent parses sharing the same process — use `optind = 0;` instead.** glibc only calls its internal `_getopt_initialize()` (which clears the static `nextchar`/`first_nonopt`/`last_nonopt` scan state) when `optind == 0`, or on the process's very first-ever `getopt_long()` call (gated by an internal `__getopt_initialized` flag that latches true forever after). `xce_test`'s REPL dispatches many independent `cmd_*()` handlers per process lifetime, each with its own differently-sized `argv`; every one of them reset with `optind = 1; opterr = 0;` only (20 call sites in `test.c` + 1 in `join_worker_args.c`, ALL of them, no exceptions — this was systemic, not a one-off typo). Confirmed via a 100%-reproducible isolated pty repro, bypassing Unity/gdb entirely: a command whose parse ends by consuming a short no-argument flag as its LAST token (`qctrl -q normal -a`, `hostctrl -h node01 -o`) leaves stale internal state that corrupts the very next unrelated `getopt_long()` call so badly the next command's own name (`argv[0]`, e.g. `"upgradeconfig"`) gets misread as an unrecognized flag. This surfaced as 4 of `harness_unit_test`'s 132 tests deterministically (not flakily — 20/20 identical repeat runs) failing, each burning its full 5s assertion-wait timeout (~28s total suite runtime vs ~9s once fixed) — and WHICH of several structurally-similar assertions failed shifted between two otherwise-unrelated builds of the same source, which is the tell for "residual interpreter/parser state carried across unrelated calls in one process," not a logic bug in the specific command being blamed. **General rule: any C code that resets `getopt`/`getopt_long` state for a fresh, independent parse (as opposed to genuinely continuing scan on the SAME `argv` across multiple calls) must use `optind = 0`, never `optind = 1` — `1` is only correct for the very first parse of a process, and coincidentally "worked" in ad-hoc single-command manual testing here precisely because that's always a fresh process.** Fixed by a blanket `optind = 1;` → `optind = 0;` replace across both files; verified via 3 targeted isolated repros passing, `harness_unit_test` 10x clean (132/132, 0 Failures every run), and a full ASAN `/xce-build harness` run (5/5 OK, no sanitizer findings).
|
||||
- **(2026-09-14, bug-1808) Unity's `TEST_ASSERT_*` macros abort a failing test via `longjmp()` (`TEST_PROTECT()`), which skips every line of code AFTER the failing assertion in that same function — including cleanup code placed only at the bottom of the function body (e.g. `clitest_kill_and_reap()`+`clitest_close()` in `test_test_cli.c`, tearing down a spawned pty-driven child process).** A single assertion failure anywhere mid-test therefore leaks that test's spawned child (left running forever, blocked on its own prompt) plus its pty/pipe fds — confirmed directly by finding 4 live orphaned `xce_test --norc` processes under one in-progress `harness_unit_test` run. Unity DOES guarantee `tearDown()` runs after every `RUN_TEST()` regardless of pass/fail/abort, so that (not each test's own inline "happy path" cleanup) is the only reliable place to catch this — but `harness_test.c`'s shared `tearDown()` (used by all 13 Unity test groups in that binary) was a no-op beyond a cosmetic print. **General rule: in any Unity (or other longjmp-based C test framework) suite, resource cleanup for a value that only exists on ONE test function's own stack must not rely on inline code after the last assertion — either wire it through the framework's guaranteed-to-run teardown hook, or track it in a process-wide static/global (not a stack pointer, which dangles once the longjmp unwinds past it) that the teardown hook can safely inspect and reap.** Fixed via a static `clitest_session_t` mirror updated by spawn/kill_and_reap/close, reaped from `harness_test.c`'s `tearDown()`; ASAN build found no sanitizer issues in the new code.
|
||||
- **(2026-09-15, controller-status Req 12 discovery/requirements) Any new `ctrlstatus`-response field that describes a property of the current PRIMARY (not just the responding node itself) forces an explicit standby-fidelity design fork, and it is worth asking the user about it up front rather than assuming.** Options are: (a) propagate the value to standbys (learned via heartbeat, matching the precedent Req 10 already set for committedLsn/committedConfigVersion) so any reachable node answers accurately, or (b) report "unknown" on a standby even when it already knows the primary's name/term. Asked explicitly for the new "primary elected in term X at time Y" fields; user chose (a), full learned fidelity — consistent with how `primaryName` (Req 2.3) and committed position (Req 10) already work off-primary. Default to proposing (a) as the recommended option when this fork recurs, since it's the established pattern for this spec, but still confirm rather than assume, since it materially changes design/task scope (heartbeat payload changes) vs. a pure read of local state.
|
||||
- **(2026-09-15) `controller-status`'s spec.json tracks its delta/reopen history in a single top-level `delta` object, not an array** — when starting a new delta cycle on top of one that already reached `validate_impl.decision: "GO"`, preserve the just-closed cycle by nesting it under `delta.prior_delta` (reopened_at/reason/new_requirements/impl_status) rather than overwriting it outright, so the requirement-number provenance ("which delta added Req N") stays recoverable from spec.json alone without needing git blame.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue