myxce/.wolf/cerebrum.md
Rongsong Shen 0206b9e166 fix(xce-test-ha): retry chaotic-mode verify_classify before counting job loss (bug-1487)
30-round chaotic soak reported committed_job_loss=1 at round 25, but the
flagged job was confirmed present on every later verify and the final
sweep -- a real loss never comes back. Root-caused from ctrl-1/ctrl-2/ctrl-3
logs, no controller change needed: a newly-elected primary cannot fold a
prior-term entry into its own committed_lsn merely because it holds it
durably (Raft Figure-8 safety via ha_arm_own_term_gate) -- it needs a fresh
majority ack for its own-term WAL_NOOP first. That normally closes in
<200ms, but chaotic mode's overlapping-restart design let it stretch to
6.232s this run (the peer whose ack was needed was itself mid-catch-up from
its own recent restart). verify_classify ran its expect-job check
immediately on primary detection with no settle/retry, unlike verify_no_loss
(bug-1422) or wait_for_full_convergence, so it landed inside that window.

Added a bounded retry (5 attempts, 2s settle) around the LOSS classification
only, mirroring verify_no_loss's precedent -- a job still missing after that
still counts as a genuine loss. Verified with a fresh 30-round chaotic
re-soak: committed_divergence=0 committed_job_loss=0 result=PASS.
2026-07-31 05:16:14 +00:00

607 lines
330 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Cerebrum
> OpenWolf's learning memory. Updated automatically as the AI learns from interactions.
> Do not edit manually unless correcting an error.
> Last updated: 2026-05-24
## User Preferences
<!-- How the user likes things done. Code style, tools, patterns, communication. -->
- Run soak tests with specific round counts and deploy directories for precise regression tracking
- Prefer parallel agent runs for independent analysis work; sequential for dependent tasks
- Keep test logs for analysis, don't skip intermediate output even if a test hangs
## Key Learnings
- (2026-07-28, bug-1479 ROOT-CAUSED not fixed) A permanent quorum-loss family member unrelated to admission control: when repl.c's `repl_broadcast` (live single-entry WAL tail-follow) gets an `ae_reject_logmismatch` NACK back from a standby (a genuinely NORMAL Raft outcome after rapid leader-election churn leaves a conflicting uncommitted tail entry at the same LSN with a different term — confirmed byte-precise via `xce_test waldump`: ctrl-2's own lsn=13 was its own deposed term=3 noop, while the canonical/surviving log has a DIFFERENT term=4 noop at the same lsn), NOTHING in ha.c's `repl_ack_decode_and_dispatch` NACK branch (`!success`) does anything but check for a higher observed term and then literally `WARN("...success=0 — ignoring")` — no `repl_on_ack` call (the ONLY site that decrements `pending_ack_count`), no nextIndex backoff-and-retry, no catch-up trigger. Consequence: `pending_ack_count` climbs 1-per-broadcast-attempt with zero decrements until `repl_broadcast`'s own `MAX_PENDING_ACKS=16` defensive eviction fires and marks the standby `in_catch_up=1` — which correctly excludes it from `repl_active_standby_count`/quorum, but NOTHING then drives an actual catch-up stream/snapshot-install for it (the only other `repl_complete_catchup` call site, config_repl.c's ACK-driven `catchup_lane_fn`, can never engage a standby that never sends a genuine ACK in the first place). Net effect: a standby with ONE conflicting log entry from election churn (routine, expected) becomes PERMANENTLY excluded from quorum with no self-healing path — in a 3-node cluster this is one unlucky election sequence away from total write-quorum loss. **General lesson**: when a symptom looks like "resource exhaustion" (admission cap, connection pool, whatever), always verify the actual counter's INCREMENT and DECREMENT sites are symmetric across EVERY reply outcome (success AND failure/reject), not just the happy path — a counter that only decrements on success but increments unconditionally will silently drift to its ceiling under ANY sustained failure condition, which looks identical to a genuine leak from the outside (repeated timeouts / repeated rejects) but has a completely different fix (handle the reject/failure path, not "find the missing release call"). Confirmed the ORIGINAL admission-slot-leak hypothesis for this bug was WRONG by checking `commit_resolve()`/`wait_release_locked()` — those DO release correctly on every timeout, 1:1 (17 timeouts, 17 releases) — before chasing the real mechanism.
- (2026-07-28, waldump REPL usage) `xce_test` (built at `src/xce.build/controller/xce_test` or `src/build.dbg/controller/xce_test`) has no `waldump` CLI flag — `--help` only shows generic argp options. It's a REPL: pipe commands via stdin, e.g. `printf 'waldump <data_dir_or_wal_seg>\nquit\n' | ASAN_OPTIONS=detect_leaks=0 timeout 60 <path-to-xce_test>` (mirrors the `xtest()` shell helper in both `clean_failover_preservation.sh` and `run-ha-soak.sh`). Output columns are `LSN | TERM | TYPE | PLEN | PCRC | INFO` — the PLEN column is payload length, NOT a second type code (a WAL_CONFIG_MARKER shows as `UNKNOWN | 40 |` because waldump's type-name table has no case for type 50 yet — 40 there is the marker's 40-byte payload length, not its wire type number). This is a fast, byte-precise way to independently confirm/refute term or index divergence between two members' WALs without instrumenting production code.
- (2026-07-27/28, bug-1475 FIXED) A term/version index that grows its backing array geometrically and zero-fills the newly-grown region to mean "absent" has a subtle, TIME-DEPENDENT bug: zero-filling with the same sentinel value used for a legitimate "recorded as literal 0" entry makes a deliberately-never-recorded slot (here: persist.c's per-LSN term_index, where a WAL_CHECKPOINT's own LSN is BY DESIGN never passed to record_entry_term) silently and RETROACTIVELY flip from "absent" to "present, value 0" the instant ANY later index is recorded and the array-growth watermark (`term_index_count`) crosses it — even though nothing ever explicitly wrote to that slot. Two readers that check the SAME never-recorded slot at DIFFERENT times (e.g. a sender that reads it AFTER appending later entries, vs. a receiver that reads it BEFORE appending anything past it) get DIFFERENT answers for what should be a stable, position-only fact — this produced a permanent AppendEntries logOk rejection (bug-1475: sender computed prev_log_term=1 (WAL_LEGACY_TERM), receiver computed prev_log_term=0 (absent), for the identical checkpoint LSN). **General fix pattern**: reserve a DIFFERENT sentinel (here `TERM_INDEX_UNSET = UINT64_MAX`, filled via `memset(...,0xFF,...)` on array growth) distinct from every legitimate stored value, so a never-explicitly-written slot reads "absent" PERMANENTLY, independent of later growth — never let "grown but unwritten" collide with a real, meaningful value (here, literal term 0 / WAL_LEGACY_TERM) in the same sentinel. **Do-not-repeat**: when hand-tracing (not instrumenting) a bug involving an in-memory index that has a documented "grows on demand, gaps read as absent" contract, don't trust that the gap actually STAYS absent across the FULL lifetime of the process/test — verify with real instrumentation whether "absent" is a stable property of the position or merely a snapshot of "not yet grown past." This was found only after two prior, plausible-looking production fixes (config_repl.c's tracker-advance + ha.c's JIT log_len resync, both already landed) still left the regression test RED — real fprintf instrumentation at the streamer, the JIT resync, and raft.c's logOk check (all added and fully removed afterward) pinned the exact moment prev_log_term flipped from 0 to 1. See `.wolf/buglog.json` bug-1475 for the full three-part fix and `test_bug1475_catchup_survives_midstream_checkpoint` (test_config_repl.c) for the regression proof.
- (2026-07-11, controller_unit_test isolated-group + leak-detection runs) `controller_unit_test` supports `--run STR` (repeatable, arg key `-t`) to select one or more TEST()-registered groups by exact function name (matches `argp`-parsed `-t`/`--run`, dispatch via `check_test()`/`TEST()` macro in `controller_test.c`) — use it to isolate a single suite instead of parsing the full-suite log. Full-suite direct runs interleave two PASS-print conventions: older suites only print via Unity's own per-assertion output (no `<file>:<line>:<name>:PASS` banner), while newer TDD suites (apply/HA-prevote family) add a custom `printf("%s: PASS\n", ...)` + banner — so grepping for `../controller/testsuite/<file>.c` banners under-counts groups that don't use the newer convention; don't infer suite pass/fail or crash-truncation from banner presence alone, always check the process exit code. Confirmed via meson (`Ok:1/Fail:0`, one lumped "controller test") AND a clean direct exit(0) with zero grep hits for AddressSanitizer/heap-buffer-overflow/UAF/double-free/FAIL across a 15.7k-line full-suite log — the log's abrupt-looking tail (last group's own internal UNITY_BEGIN/END subtotal printed twice) is normal, not truncation; verify with `echo $?` instead. The historical "test_ha_prevote 18/18" figure from an earlier session was the COMBINED total of `test_ha_prevote` (12 tests, confirmed via `grep -c RUN_TEST` in the .c file) + `test_ha_failover_prevote` (6 tests) — the two are separate TEST() groups in `controller_test.c`, not one suite of 18; report them separately (12/12 + 6/6) to avoid a false "regression" reading.
- (2026-07-08, HA soak test sequential vs chaotic modes) Sequential mode soak validates clean failover lifecycle: 10 rounds, 36 jobs, zero loss, all 3 nodes led primary — deterministic validation of promotion + catch-up + reapply paths working as designed. Chaotic mode deliberately overlaps kills across election/catch-up windows to surface edge cases: rounds 1-4 pass verification, round 5 hits primary-discovery lag (bug-854 root-caused earlier, excluded from verdict), rounds 6-7 stall from election convergence lag + duplicate-node-guard (bug-1418, pre-existing guard). The election stall in chaotic round 6 is likely compound: (a) tight overlapping kill cadence outpaces election+restart timing (heartbeat-timeout bounds ~500ms but nix shell spinup adds latency), (b) a node restarted but not fully converged rejoins before the prior kill round's primary finishes its term tenure. Sequential mode PASS verdict is solid baseline. Chaotic mode needs either slower kill spacing OR a real-fs host deploy (current sandbox has overlayfs fdatasync EIO bug-924) to distinguish fault-tolerance (still holds) from environment-induced timing collapse.
- (2026-07-04, raft-apply-decoupling task 6.1 round 2 — job-state merge-staleness fix) Round 1's `apply_committed_entry` wired the live pump but left a real corruption: `record_job_fail_fact`/`record_job_update_fact` (job.c) precomputed a FINAL absolute state at RECEIPT time (a scratch-copy merge for FAIL, an inline backstop+PDONE merge for postdone) and baked it into `wal_jobstate_t.state`; under deferred apply, a second fact for the same job (e.g. FINISH) could apply BETWEEN receipt and this fact's own apply, making the baked snapshot stale — a POST_EXEC-stage FAIL received while a job is still `RUN` (FINISH not yet applied) bakes `RUN|EXIT|PERR`, and when FINISH applies first (`DONE`) then FAIL's blind absolute overwrite clobbers it back to `RUN|EXIT|PERR` — DONE silently lost. FIX PATTERN: `wal_jobstate_t._pad` (previously always-zero, unused) is repurposed as a discriminator — `_pad==0` still means "legacy absolute state, safe" (job_ctrl's mark_job_fail/mark_job_update paths, which mutate+capture state synchronously under one lock, no staleness window); a nonzero `_pad` (`WAL_JOB_FAIL_STAGE_FLAG|stage` for FAIL, `WAL_JOB_UPDATE_POSTDONE_MARK` for postdone) marks "recompute at apply time against ACTUAL current state" — `apply_committed_entry` (persist.c) dispatches on this bit per WAL type, calling the REAL `apply_job_fail_transition`/new `apply_job_postdone_transition` (job.c, mirrors the existing guard-core family) instead of `apply_job_update_transition`'s blind overwrite. New low-level `persist_submit_job_fact_ex(p, type, jobid, state, pad)` added as the primitive; `persist_submit_job_fact` becomes a pad=0 thin wrapper — ZERO existing call sites (including ~20 in test_commit_barrier_reply.c) needed touching. GENERAL PATTERN for "wire format has 2 producers, one safe one not": don't change the safe producer's shape — add a discriminator field/flag so the consumer can tell them apart, and make "0"/"absent" mean exactly the old/safe behavior so every existing writer keeps working unchanged.
- (2026-07-04, same task) Fixing a wire-format bug INSIDE a task's own scope can break pre-existing tests that assert the wire format directly (not just behavior) — 2 tests in test_commit_barrier_reply.c (`test_guard_forward_update_on_terminal_preserved`, `test_guard_fail_after_finish_preserves_perr_merge`) read the raw on-disk `wal_jobstate_t.state` via a helper (`cbrp_count_jobstate_entries`) and asserted DONE/PDONE/PERR bits directly on it — exactly the field this fix stops populating for FAIL/postdone. These are NOT the "old apply-before-commit ordering" tests task 7.3 owns (a different axis — ordering vs wire-format) — they are a DIRECT, in-scope, mechanical consequence of the wire-format fix itself, so I fixed them here rather than deferring: extended the helper to a `_ex` variant returning `_pad` too (existing helper becomes a thin wrapper, zero other call sites touched — same pattern as persist_submit_job_fact_ex), changed the wire-level assertions to check the new `_pad` marker, and added a direct call to the guard core (`apply_job_fail_transition`/`apply_job_postdone_transition`) on a scratch `job_t` to keep pinning the actual merge semantics the test was written to protect. A 3rd test (`test_fact_state_unchanged_throughout_quorum_loss_outage`) had the identical `st_upd` bit-check for postdone; fixed the same way (already had a `(void)st_fail` precedent in the same file for exactly this "order-dependent, can't check at receipt time" reasoning — extended it to `st_upd`/`pad_upd`). Lesson: when a task's fix legitimately changes what a durable record contains, grep for every DIRECT byte/field-level test assertion on that record (not just behavioral tests) before declaring the regression sweep clean — `grep cbrp_count_jobstate_entries` found them, `--run test_apply_pump` alone would not have.
- (2026-07-04, same task) RED-phase verification for a bugfix already merged into the same commit as its own regression test (no separate "before" commit to check out) — reverted the exact 2 production hunks in-place (record_job_fail_fact's scratch-copy computation + apply_committed_entry's WAL_JOB_FAIL case ignoring `_pad`) using Edit, confirmed the new test fails with the EXACT predicted corrupted value (`Expected 264 Was 262` = DONE|PERR vs RUN|EXIT|PERR, byte-for-byte matching the hand-derived bug trace), then restored from a pre-saved `cp` snapshot of the fixed files and `diff`-verified byte-identical restoration before rebuilding GREEN. Combined with the existing longjmp-past-teardown lesson (isolate the new test alone in RUN_TEST via `#if 0` during the revert-and-check step, restore full list after) — both techniques compose cleanly.
- (2026-07-04, same task) **Do-not-repeat**: this repo has TWO independently-built `controller_unit_test` binaries — `src/build/controller/controller_unit_test` (correct, incremental, what `ninja -C build` from `src/` updates) and `../build/controller/controller_unit_test` i.e. repo-root `build/` (a STALE separate build tree from an earlier/different session — NOT updated by the `src/`-relative ninja command). Running `../build/controller/controller_unit_test` after rebuilding via `ninja -C build` silently executes stale, unrelated test binaries — symptom observed: a `--run test_apply_pump` invocation printed exactly 3 tests (the pre-round-2 test count) with `PASS` for all, at line numbers that didn't match the current file, right after a successful rebuild that should have produced 4 tests. Always verify with `strings build/controller/controller_unit_test | grep <new_test_name>` (or check `ls -la --full-time` / mtimes) when a test run's output looks suspiciously like an older version of the suite — don't trust "the build succeeded" alone to mean "the binary I'm about to run reflects it." ALWAYS run `./build/controller/controller_unit_test` (relative, from `src/`), never `../build/...`.
- (2026-07-04, same task) Observed a rare (~1/50 runs) flake in `test_apply_pump_fail_racing_finish_merges_against_current_state` where the final job state read back as the untouched SEEDED value (job never mutated at all) despite every prior assertion (including `committed_lsn == watermark`) passing — i.e. the covering ACK genuinely advanced `committed_lsn` but the inline (NULL-pool fallback) apply-pump walk apparently didn't mutate the job that pass. Reproduced 1 time in ~55 total runs across 3 batches; the identical ACK+inline-pump trigger mechanism is shared with round-1's already-reviewer-verified `test_apply_pump_kick_applies_committed_burst_in_ascending_lsn_order`, which never flaked in the same sampling — inconclusive whether this is environmental (system load during rapid repeated `nix develop` shell spin-up) or a genuine rare race in `persist_apply_pump_run`/`commit_resolve_pending`'s kick path (both explicitly out of this task's remit, verified-correct by the round-1 reviewer). Flagged in the status report as a CONCERN, not fixed — the fix logic itself (`apply_job_fail_transition`/`apply_job_postdone_transition`/the `_pad` dispatch) is pure and time-independent, so this does not implicate the round-2 correctness fix specifically.
- (2026-07-03, raft-apply-decoupling task 1.5) Wrote the concurrency unit tests closing task-group 1: `test_commit_resolve.c` gained 3 new tests (a genuine multi-threaded admission-cap burst using a sampling monitor thread; a mixed majority-ack+timeout burst using a `bounded_pool_setup` helper that DECOUPLES the continuation registry's capacity from the vtaskmgr pool's real worker-thread count so "bounded worker usage" is a meaningful, non-trivial assertion via `pthread_equal`-based distinct-thread counting; and a nice-to-have genuine two-real-thread majority-wins-over-timeout race relying on `commit_advance_locked`'s `pthread_cond_broadcast` to wake the sweep thread early). The demotion- and truncation-triggered burst cases were placed in `test_demotion_convergence.c` and `test_persist_truncate.c` INSTEAD of `test_commit_resolve.c`, reusing those files' existing `dc_setup`/`make_fresh_persist` fixtures rather than duplicating a full `cluster_t` fixture — keeps each file's fixture cohesive; design.md's file-plan wording ("test_commit_resolve.c covers demotion/truncation admission behavior") is better read as describing task-group 1's aggregate coverage, not a literal single-file mandate. IMPORTANT DISCOVERY: bug-1407 (`test_commit_advance_retry_clears_push_before_commit_fence`, test_config_repl.c), which task 1.4's notes closed as "70/70 passing, 3/3 runs", is REOPENED — it now fails 100% of the time (10/10), confirmed on BOTH the task-1.5 working tree AND a `git stash`-clean checkout of the task-1.4 commit with zero task-1.5 changes applied (i.e. genuinely pre-existing on the current tree, not caused by this task's test-only diff). Do not trust a prior task's "CLOSED, N/N runs" bug verdict at face value when validating a later task in the same task-group — always re-run the specific regression yourself before citing it as settled, especially for a concurrency-adjacent flake with no isolated root cause.
- (2026-07-03, raft-apply-decoupling task 1.4) Wired demotion + log-truncation to resolve pending commit continuations as FAILED. TWO call sites, TWO different idioms, deliberately: (1) **demotion** — `raft_cb_become_follower`'s existing `if (prior_role == CTRL_PRIMARY && c != NULL)` hook-disable block (ha.c) now also calls the new `commit_resolve_pending_above(&c->commit, 0, -XCE_ERR_STANDBY)` directly — ha.c is already a direct consumer of commit.c's public API elsewhere (commit_set_writable_lsn etc.), so a direct call is idiomatic there; lsn_floor=0 means "all pending" (a demoted node can never commit ANYTHING it had pending — there is no partial-floor case, unlike truncation). (2) **truncation** — used the SAME inversion-of-control pattern as the existing durable-advance wake (`persist_set_durable_cb`/`on_durable_advance`) rather than calling commit.c from ha.c/persist.c's 3 real truncate call sites (`raft_cb_become_leader`'s post-election truncate, `ha_cb_truncate_from_stub`'s conflict-reconcile, `ha_demotion_convergence_fn`'s post-demotion truncate) individually: added `persist_set_truncate_cb`/`on_truncate`/`truncate_cb_ctx` to persist_t, registered by `commit_engine_init` (`commit_on_truncate` → `commit_resolve_pending_above(ce, cutoff_lsn, -ENOENT)`), fired ONCE from the single shared `persist_truncate_wal_above` wrapper (never `truncate_wal_above_locked`, which runs under `p->lock` — the callback must fire AFTER `pthread_mutex_unlock(&p->lock)`, checked via `after < before` on `current_lsn`, mirroring the flusher's `after > before` durable-advance guard EXACTLY) — this is DRY (one wiring point catches all 3 real+future truncate callers) and preserves the documented "persist.c never depends on commit.c" direction. `-XCE_ERR_STANDBY` (demotion) and `-ENOENT` (truncation) reuse the exact rc vocabulary `test_commit_resolve.c`'s task-1.2 author had already picked in `test_commit_resolve_delivers_failed_outcome_for_{demotion,truncation}` — do not invent new codes when an existing test file has already established the vocabulary for a not-yet-wired scenario. New commit.c/h function `commit_resolve_pending_above(ce, lsn_floor, rc)`: snapshot-then-resolve under `ce->lock` (mirrors `commit_resolve_pending`'s shape exactly), resolves every `lsn > lsn_floor`, exactly-once via `commit_resolve`'s own idempotent slot-clear.
- (2026-07-03, RED-verification hazard with Unity + threaded fixtures — reusable gotcha) Proving a genuinely-RED test in THIS harness by temporarily reverting production code and running the WHOLE test group (`--run <group>`) can HANG the process, even though the identical group passes cleanly once GREEN. Root cause: Unity's `TEST_ASSERT_*` macros `longjmp` out of the failing test function on the FIRST failed assertion, which SKIPS any teardown code written after that assertion in the same function (e.g. `dc_teardown()` at the end of a test in test_demotion_convergence.c) — so a thread-owning fixture (a `commit_engine_t` with its task-1.3 sweep thread, a persist flusher thread) is never joined/freed. The NEXT `RUN_TEST` in the same process then calls the SAME group's `setup()` which does `memset(&fixture, 0, sizeof(fixture))` on a struct a live orphaned thread still references — corrupting a mutex/cond mid-use and hanging (observed: 0% CPU, process alive, no output, indefinitely). FIX for RED verification (not a product bug — never manifests in GREEN, since teardown runs normally when assertions pass): temporarily edit the suite's `RUN_TEST` list down to JUST the one new test (wrap the rest in `#if 0`/`#endif`), rebuild, confirm the single isolated failure, then revert the RUN_TEST list back before restoring the production fix. Reusable rule: whenever a fixture under test owns a background thread AND you need to prove RED via a real (not calling-the-helper-directly) test, isolate that ONE test in the group during the revert-and-check step — do not run the full group against reverted production code.
- (2026-07-03, raft-apply-decoupling task 1.3 REVIEW-FIX) Reviewer caught a real lost-wakeup bug in the FIRST `commit_sweep_worker` cut (see the task 1.3 entry below): `commit_sweep_once()` computed `have_next`/`next_deadline` UNDER `ce->lock`, then UNLOCKED before dispatching (`commit_resolve()` must run unlocked, lock-ordering rule) — a `commit_submit_async()` registration broadcasting `ce->cond` during that unlocked dispatch window found NO thread parked in `cond_wait`/`cond_timedwait` to receive it (POSIX condvars do not latch a signal sent to no waiter), so the worker then trusted the STALE pre-dispatch `have_next`/`next_deadline` and could commit to an unbounded `pthread_cond_wait()` — a fresh registration landing in that narrow window would not be noticed until some UNRELATED future broadcast, unboundedly violating the DEFAULT_SYNC_TIMEOUT_MS bound in a low-traffic cluster. FIX: applied the textbook "recheck the wait condition in the SAME lock hold that leads directly into the wait call" idiom — split the old `commit_sweep_once` into `commit_sweep_scan_locked()` (pure classification, caller holds the lock, never unlocks, never dispatches) and rewrote `commit_sweep_worker` as ONE loop that holds `ce->lock` continuously across iterations, only releasing it for the dispatch step itself, then immediately re-acquiring and re-scanning BEFORE ever reaching the `cond_wait`/`cond_timedwait` call — so no wait is ever sized from a pre-dispatch snapshot. DETERMINISTIC test technique for this class of bug (microsecond races are unreliable to hit black-box): added a `sleep_us` field to the shared `continuation_recorder_t` (test_commit_resolve.c) that makes `recorder_fn` sleep before recording; combined with the NO-POOL inline-dispatch fallback (`dispatch_continuation` calls `fn()` synchronously ON THE CALLING THREAD when `ce->pool == NULL`), a continuation registered for timeout resolution runs its slow recorder INLINE ON THE SWEEP THREAD ITSELF, deterministically widening the unlocked-dispatch window from microseconds to a fully-controlled ~200ms so a second registration can land inside it with certainty — `test_commit_submit_async_registration_during_dispatch_not_lost`. VERIFIED the test is a genuine regression guard, not a tautology: temporarily reconstructed the old buggy loop under an `#if 1 /* TEMP_BUGGY_REPRO */` guard, confirmed this specific test (only this one) FAILed, then reverted. Reusable pattern: whenever a bug hinges on "thread A doing slow/unlocked work while thread B tries to signal it," give the test's callback a controllable `sleep_us`-style knob rather than fighting the real timing with `usleep`-and-hope.
- (2026-07-03, raft-apply-decoupling task 1.3) Armed a timeout for registered async continuations by giving `commit_engine_t` its OWN dedicated pthread (`sweep_thread`, started in `commit_engine_init`, stopped+joined in `commit_engine_fini` before `lock`/`cond`/`continuations` are torn down) rather than routing through the shared controller `delaymgr` (src/lib/delay.c). REASON: task boundary is commit.c/commit.h only (no cluster.c/ha.c wiring), AND the production `delaymgr` polling precision is 200ms (cluster.c `delaymgr_init(&c->delaymgr, 200, ...)`) vs `DEFAULT_SYNC_TIMEOUT_MS`=500ms (config.h) — routing through it would add up to +200ms jitter to async-timeout firing, arguably breaking the "identical failure timing" no-regression bar (Req 11.1). The sweep thread instead reuses `ce->lock`/`ce->cond` (already broadcast on every commit-advance/membership/gate event) and sleeps via `pthread_cond_timedwait` to the EARLIEST pending continuation's deadline — computed via the SAME `wait_deadline()` helper + SAME `DEFAULT_SYNC_TIMEOUT_MS` constant `commit_submit(COMMIT_MODE_WAIT)` already uses — giving byte-identical precision to the blocking path it replaces, with ONE thread regardless of how many continuations are pending (unlike the old one-parked-thread-per-waiter model). Added a `struct timespec deadline` field to `commit_continuation_t` (commit.h), set in `commit_submit_async` right after slot claim, plus a `pthread_cond_broadcast` there so the sweep thread re-evaluates a freshly-armed nearer deadline instead of oversleeping toward a stale one. RACE DECISION (documented + test-proven): majority-satisfied ALWAYS wins over an already-expired deadline when a single sweep pass (`commit_sweep_once`) observes both true for the same lsn in the SAME `ce->lock` hold — checks `wait_done_locked` before the deadline compare, mirroring `commit_submit`'s existing last-look-before-timeout tie-break (its final `wait_done_locked` recheck after `pthread_cond_timedwait` returns, still under the same lock). Proof test `test_commit_submit_async_majority_wins_race_at_expired_deadline` forces this deterministically by white-box-mutating a slot's `.deadline` to epoch-0 AND `g_p.committed_lsn` directly (bypassing `commit_on_ack`/`commit_resolve_pending` entirely) so ONLY the sweep thread's own scan can be what resolves it — asserts COMMITTED, not FAILED. `commit_engine_fini` now blocks briefly on `pthread_join` every teardown (negligible; every existing commit-engine test fixture already exercises this path with zero regressions across the 316-test commit-adjacent regression sweep — test_commit_engine/admission_pool/repl_majority/commit_barrier_reply+rollout/commit_durability/ha_role_lifecycle/term_safety_adapter/config_repl/p2_integration/coldstart_lsn0/convergence_verification/demotion_convergence/egress_filter, ONLY pre-existing bug-1407 (`test_commit_advance_retry_clears_push_before_commit_fence`) still fails, unchanged).
- (2026-07-03, raft-apply-decoupling task 1.1) Added `commit_submit_async(ce, lsn, fn, ctx)` to commit.c/commit.h — the non-blocking, per-LSN continuation-registration sibling of `commit_submit(..., COMMIT_MODE_WAIT, ...)`, per the keystone-conversion design in `.kiro/specs/raft-apply-decoupling/design.md` §Commit Engine Resolve. Registration ONLY (task 1.1 boundary) — it stores `{lsn, fn, ctx}` in a new `ce->continuations[]` array (calloc'd to `n_vcpus` slots in `commit_engine_init`, freed in `commit_engine_fini`; `lsn==0` is the free-slot sentinel, safe because LSN 0 is the reserved empty/none value repo-wide since reserve-lsn-0). CRITICAL DESIGN DECISION for whoever implements task 1.2 (`commit_resolve`, wiring `commit_advance_locked`'s majority path + the timeout/demotion/truncation failure paths to invoke the continuation via `vtask_enq_classified`): async registration calls the SAME `wait_admit_locked`/`wait_release_locked`/`inflight_waits` admission-cap discipline that `COMMIT_MODE_WAIT` uses — the two calling conventions share ONE budget. This means `commit_resolve()` MUST call `wait_release_locked(ce)` (mirroring `commit_submit`'s WAIT-mode release) after invoking/dispatching a continuation, or the shared cap permanently leaks a slot per resolved async entry. The array is sized exactly `n_vcpus` because `wait_cap(ce) <= n_vcpus` always holds (both the pool-folded cap `n_vcpus - VTASK_LIVENESS_RESERVED` and the local fallback `n_vcpus - COMMIT_WAIT_RESERVED` are `<= n_vcpus`), so an admitted registration is always guaranteed a free slot — do not shrink this without re-deriving that invariant. New test file `test_commit_resolve.c` (5 tests, RUN_TEST-only, no own UNITY_BEGIN/END) covers: immediate non-blocking return, invalid-arg rejection, cap enforcement, shared-budget-with-WAIT, and distinct-slot tracking for concurrent LSNs.
- (2026-07-03, build-dir path gotcha) The Bash tool's cwd for this project is ALREADY `src/` (matches env `Working directory`), not the repo root. So "from src/, run `ninja -C ../build`" (as worded in some skill/task instructions) is WRONG in this shell — `../build` from an already-`src/`-rooted cwd resolves to the repo root's `build/`, which does not contain the controller build. The correct incremental command is `ninja -C build controller/controller_unit_test` (relative to `src/`). Always `pwd` once at the start of a build task to confirm which directory a relative `../build` is actually anchored to before trusting copy-pasted instructions.
- (2026-07-03, Unity harness census across mixed sub-groups) In `controller_unit_test`, only a MINORITY of `test_xxx(void)` group functions (14 of 72 `TEST()` entries in `controller_test.c`, e.g. `test_ha_core_safety`, `test_ha_partition`, the snapent fixture group) call their own nested `UNITY_BEGIN()`/`UNITY_END()` and print a standalone "N Tests M Failures K Ignored / OK" block; the rest (e.g. `test_commit_resolve`, `test_commit_admission_pool`) just call `RUN_TEST()` and feed the ONE global Unity run `controller_test.c`'s `main()` opens, whose cumulative tally only appears via the per-test `file:line:test:PASS/FAIL` lines, not a second summary block. So "grep for N Tests M Failures blocks" undercounts total tests — the authoritative zero-regression check is: ANSI-strip the log (GNU `sed -r 's/\x1b\[[0-9;]*m//g'` did NOT reliably strip the escape byte in this environment — use `python3` with `re.sub(r'\x1b\[[0-9;]*m', '', text)` instead) then grep for the regex `:\d+:[A-Za-z0-9_]+:FAIL` (zero matches required) and count distinct `:\d+:[A-Za-z0-9_]+:PASS` matches. A naive `grep -c FAIL` massively overcounts (7000+) because log lines like `persist: flusher fdatasync FAILED (errno=5 ...)` are EXPECTED fault-injection output from durability tests, not Unity failures.
- (2026-06-30, bug-1306 FIXED — hook-rc clobber) `xce_run_hooks` (src/lib/hook.c:134-157) returns ONLY the LAST-registered hook's rc — `rc` is overwritten every loop iteration and the `break` is `BEFORE_EVENT`-only. So a protocol/durability step must NEVER carry its verdict through a hook return: multiple JOB `AFTER_EVENT` subscribers are registered (persist + sched + apiserver + a promotion re-add in ha.c), and a later one returning 0 CLOBBERS an earlier `-ETIMEDOUT`. This silently defeated the job-submit majority-commit gate — `handle_jobsubmit` saw `hook_rc==0` and replied `jobsubmit ok` for an entry that never reached majority → a new-term leader legally overwrote it (committed-job-loss, "reply-success ⇒ durable" violated). FIX (raft-apply-decoupling item #1): extracted the WAL append + commit barrier into an explicit `persist_submit_wal_and_commit(p, jlist)` that `handle_jobsubmit` calls DIRECTLY and reads the rc from; the JOB_NEW hook is now notification-only (regression pin `test_job_hook_new_is_notify_only`). CORRECTION 2026-06-30: the clobber does NOT extend to the other two `WAL_MODE_WAIT` ops (WAL_QUEUE_CTRL, WAL_WORKER_CTRL). JOB's hook was UNIQUE in running the full commit barrier (`persist_wal_sync_append`) INSIDE the hook; queue/worker hooks call `wal_hook_append_classified`, which for `WAL_MODE_WAIT` returns after the LOCAL append only (persist.c:2764 `if (rc != 0 || mode != WAL_MODE_RETAIN) return rc;`) — so they carry NO durability verdict through the hook return, nothing to clobber. Their majority-commit barrier (if any) lives outside the hook (ha-commit-barrier-rollout "handler barrier", tasks 2-3) — a SEPARATE question from the bug-1306 clobber; verify it independently, do NOT mechanically extend the JOB extraction. Diagnosis method that nailed it: `xce_test waldump <data_dir>` (REPL cmd) proved all 3 WALs gapless/CRC-intact/full-payload with the lost job only on the old leader at a term boundary (term-9 JOB vs term-10 NOOP at the same LSN) → divergence-from-premature-ack, NOT the catch-up prefix-gap; then the controller log's `commit: WAIT deadline expired` + absence of any `rejecting job` line proved the gate fired but its verdict was dropped.
- (2026-06-23, bug-1028 FIXED) There MUST be exactly ONE authoritative source for a WAL entry's Raft term. The leader's library send path (`ha_read_wal_entry_at`, ha.c:1472-1476) and the standby's `logOk`/`on_get_entry_term` both source terms from `persist_entry_term` (in-memory index, legacy-0→WAL_LEGACY_TERM). The heartbeat catch-up streamer (`ha_stream_wal_to_standby`, config_repl.c) — the third consumer — was sourcing terms from raw on-disk `hdr.term` (0 for legacy entries). A standby's `prev_log_term=1` (normalised) ≠ streamer's `prev_log_term=0` (raw) → `logOk` rejects the new-term NOOP forever → `applied_lsn` stuck → primary re-streams every heartbeat cycle. Fix: all three `hdr.term` reads in the streamer replaced with `persist_entry_term(&persist, hdr.lsn)`; function gained a `const persist_t*` parameter. This is the pattern: every source of the per-LSN (term,index) tuple MUST go through `persist_entry_term`; NEVER read `hdr.term` raw from disk — it is not normalised.
- (2026-06-23, bug-854 FIXED) The `XCECLI_FAILOVER_CALL` retry loop classifies a status-0 reply from ANY reachable node as SUCCESS and returns — it only rotates on failure. But ctrlstatus is answerable in every role, so a standby always answers 0, making `xcecli_ctrlstatus` pin to the first reachable node forever. The outer `xcecli_ctrlstatus_primary` loop captures a redirect hint from the non-authoritative reply but never forces rotation to the next master — so the configured master list is never traversed. Fix: `xcecli_ctrlstatus_primary` now calls `xce_failover_rotate(ctx)` + `xcecli_ctx_recover(ctx)` under `failover.lock` on every non-authoritative reply; the floor of `max_attempts` was raised to `nmasters+1` to cover the full ring. General pattern: any outer retry loop that queries a role-agnostic status must **force endpoint rotation** on a non-authoritative reply — do not rely on the inner failover wrapper, because status-0 preempts it.
- (2026-06-22, ha-term-safety task 3.1) The single primary-append seam is `ha_primary_log_append(cluster_t*,type,payload,len,&out_lsn)` (ha.c, decl ha.h ~333): packs the `raft_entry_blob_t` envelope and calls `raft_log_append(&c->ha.raft,...)` so the LIBRARY stamps `s->term` + assigns LSN + fires `on_persist_entry`. CRITICAL: `raft_log_append` returns `-EPERM` unless `s->role==RAFT_LEADER` — and `ha_do_promote`/the non-raft `ha_promote_to_primary` road sets `ha->role=CTRL_PRIMARY` but NOT `ha->raft.role` (only the raft election road via on_become_leader sets RAFT_LEADER). So `ha_primary_log_append` MUST have a `-EPERM`→legacy `persist_wal_async` fallback (stamping `ha->term` via persist_set_term first) or init-time/non-raft/engine-less promotion breaks. Retired the hand-rolled bypass by routing persist.c `wal_hook_append_classified` (RETAIN job/queue/worker hooks) + `persist_wal_sync_append` (WAL_JOB_SUBMIT WAIT) through `ha_primary_log_append` GUARDED on `p->cluster != NULL` — standalone persist (lib-level tests, `persist_init(p,&cfg,NULL)`) keeps the direct persist_wal_async path. `p->cluster` is the back-ref (persist.h, set in persist_init); ha.h is reachable in persist.c via `xce/cluster_def.h`. Co-tracking invariant `p->current_lsn == s->log_len + 1` holds because raft_init (2.5) seeds log_len from durable floor and every primary append advances both by 1. To make a controller TEST drive raft_log_append: `ha_init_raft` then set `c->ha.raft.role=RAFT_LEADER` + `c->ha.raft.term=T` directly (the raft-sim convention). on_persist_entry calls persist_wal_async (NOT the hooks) so no recursion. `ha_arm_own_term_gate` is static → test seam `ha_arm_own_term_gate_for_test`. Validation: pre-existing `test_wal_catchup_*` (7, bug-924 overlayfs) fail IDENTICALLY on baseline (git stash + rebuild proves it); every other group 0 fail.
- (2026-06-20, waldump tooling) The `xce_test` binary (`src/controller/meson.build`) builds from `testsuite/test.c` ONLY + `libxce_dep` — it does NOT link `persist.c`/`job.c` (those are in the `controller_unit_test` target). The investigation doc's claim that xce_test "links persist.c/job.c" was WRONG. BUT capnp `read_Job` is generated into `controller.capnp.c` which IS compiled into `libxce`, so it's reachable transitively. To USE capnp/zlib symbols from test.c you must add `c_capnproto_dep` + `zlib_dep` to `xce_test_deps` (static-lib deps don't auto-propagate through `declare_dependency(link_with: libxce)`). For a zero-alloc jobid decode read `struct Job.jobid` directly after `read_Job` — skip the allocating `decode_Job`. WAL CRC reader template = `persist_test_durable_lsn_on_disk` (persist.c ~888): seg files `seg-%016llx.wal` under `<node>/data/wal`, 40B `wal_entry_hdr_t`, crc32 over `WAL_HDR_CRC_SIZE`(=32) header bytes + payload, prev_crc chain, all-zero hdr = clean EOF (pre-alloc). Offline `waldump` lives in `testsuite/waldump.{c,h}` + `cmd_waldump` in test.c top_commands.
- (2026-06-18, ha-commit-durability task 5.2 VALIDATION) Deterministic E2E on the FIXED build = GREEN, stable 3/3 runs (9/9 probes PASS), in-window primary fdatasync=2 on EVERY probe (vs 0/RED on the unfixed 1.3 build) — the RED→GREEN flip is the deterministic acceptance and is confirmed. Run via `nix develop ./src --command bash -c 'bash src/tools/test-commit-durability.sh --deterministic <deploy>'` background+poll worktree log; cold election ~15s, 3 probes, exit 0. The ROLLING SIGKILL SOAK is host-verification-needed in-sandbox: rounds 1-2 PASS (jobs survive failover), then round 3 onward "no new primary elected within 40s" and every "DATA LOSS" report is a query against a cluster with NO live primary (same 5 jobs frozen, never re-acked). ROOT CAUSE of the collapse is the durability fix's OWN promotion fence working AS DESIGNED: ctrl-1 won PreVote for term=5, computed safe_committed_lsn=13, then `persist_recover_fatal_if_unrecoverable` FATAL-terminated ("recovery-truncated log ... no healthy peer reachable ... durable_floor_lsn=13 < hwm=13 ... terminating rather than serving divergent state, Req 2.4") — refusing to let a structurally-truncated node lead and overwrite the good timeline (exactly the bug-661 failure mode the fix prevents). Under aggressive rolling-kill+immediate-restart the fence repeatedly self-terminates recovering nodes → cluster drops below quorum → liveness collapse, NOT durable loss. PROOF it's not loss: committed entries still on disk after the run (ctrl-3 WAL has seg-1 + seg-15; ctrl-2 seg-1) and 0 live controllers at end. So the soak's exit=1 / "DATA LOSS" verdict is a HARNESS ARTIFACT (it equates empty-joblist-with-no-primary to loss) — the soak harness conflates election-stall liveness failure with durability loss. The deterministic flip is the must-have and passed; the live multi-round soak needs a real host (slower election timing, no sandbox reaping) to distinguish fence-driven safety self-termination from a true regression. Did NOT patch production (5.2 is validation-only).
- (2026-06-18, ha-commit-durability task 5.1) End-to-end durable-majority commit verified via 3 integration tests in test_commit_durability.c (NO production wiring gap found — the 2.x/4.x wiring already connects append→flusher fdatasync→durable_lsn→durable-advance wake→majority_candidate_locked self-cap→committed_lsn→commit barrier). (1) test_durable_majority_commit_end_to_end: REAL persist + REAL flusher + commit_engine(n_peers=3) + 2 registered standbys; with persist_test_set_force_fsync_fail(1) a FULL standby-ACK majority does NOT advance committed_lsn (durable precondition gate, Req 1.1/1.4) and a WAIT submit on its own thread does NOT release; after clearing the fault + persist_flush, durable_lsn advances, the durable-advance cb re-derives majority and the WAIT releases rc=0. GOTCHA: persist_wal_async fans out via repl_broadcast to registered standby eps — dummy int eps cause an ASAN global-buffer-overflow in dataq_enq; FIX = install the XCE_TESTING `repl_broadcast_send_hook` (drop hook, same as test_commit_engine.c) before appending, NULL it after. (2) test_sole_durable_contributor_loss_survives: models the N=3 durable-majority property (committed ⇒ ≥2 durable copies) — write the same op durably to two independent persist dirs (one via persist_wal_async+flush, one via persist_wal_write_replicated which fdatasyncs inline), rm -rf one, prove the survivor still recovers it CLEAN; live multi-host failover proof deferred to 5.2 (Req 1.5/2.5). (3) test_durability_work_off_iomgr_preserves_liveness (Req 1.6): new XCE_TESTING seam persist_test_last_fdatasync_thread() records pthread_self() at the successful flusher fdatasync site (flusher_flush_once, under p->lock); test asserts it != caller thread, and that heartbeat-style atomic reads on the caller thread complete <50ms while flushes are in flight. RED PROVEN by injecting the pre-2.2 bug (cap=highest_appended_lsn): the commit.c:162 `assert(cand <= persist_durable_lsn)` aborts when the ACK-majority tries to advance committed over a non-durable entry. 4.3's bounded-window catch-up TIMER left as 5.3 follow-up (immediate fail-loud is safety-conservative). Suite Fail:0 ASAN x2; seam nm-excluded from xce_controller. 34 durability tests (was 31).
- (2026-06-18, ha-commit-durability task 4.3) Pre-promotion eligibility gate + fence release. Added TWO functions in ha.c (decls in ha.h): `ha_promotion_gate_decision(bool truncated, uint64_t durable_floor, uint64_t safe_hwm, bool peer_reachable, bool catch_up_succeeded) -> recover_decision_t` (PURE — builds recover_policy_inputs_t with `durable_below_hwm = durable_floor < safe_hwm` and delegates to `persist_recover_decision`, so the 3.2/3.3 decision table is the SINGLE source of truth, DRY) and `ha_evaluate_promotion_fence(ha, ...)` (LIVE — ELIGIBLE clears `ha->in_catch_up`, FENCE sets it, FATAL routes through `persist_recover_fatal_if_unrecoverable`). WIRED into `raft_cb_become_leader` (ha.c ~703) AFTER `persist_truncate_wal_above(safe_lsn)` (so `persist_durable_floor` reflects the 2.2 clamp) and BEFORE the CTRL_PRIMARY store; on `!= ELIGIBLE` it logs WARN and `return`s WITHOUT becoming primary (so a test-hooked FATAL never falls through to leadership). `safe_lsn` lifted to fn-scope var `promotion_safe_lsn` (the cluster HWM from voter replies — `ha_compute_safe_committed_lsn` arithmetic UNTOUCHED). peer_reachable derived from surviving granted vote eps (`ectx->votes[i].vote_granted && .ep != NULL`). GATE SKIPPED when `c==NULL` (the `raft_cb_become_leader_for_test` shim has no persist) — that is WHY existing election/role/convergence suites (all driven via the c==NULL shim or `ha_promote_to_primary`, a SEPARATE function with no gate) pass unchanged with ZERO fixture edits. Turned 3.2 rows 1/2/3/5/6 GREEN (0 Ignored) by driving the live gate (rows include a fence-set then catch-up→release sequence proving Req 2.6); added `test_promotion_gate_hollow_refused_at_minimal_quorum` (Req 2.5). test_commit_durability.c needed `#include "ha.h"` (ha.h forward-declares cluster_t, self-contained). LIVE-TIMER FATAL DEFERRED to 5.1 (allowed): the gate evaluates `catch_up_succeeded=false` at the synchronous promotion instant, so a truncated/behind node with NO peer FATALs immediately (correct terminal) — the bounded-window TIMER that gives DEFAULT_RECOVERY_CATCHUP_WINDOW_MS to find a peer before FATAL is the live-timer aspect 5.1 owns; the decision + fence + release are all live HERE. Suite: 31/0 durability (0 Ignored), full controller direct-binary = only the pre-existing barrier baseline fail (overlay-/tmp EIO), meson controller test OK Fail:0.
- (2026-06-18, ha-commit-durability task 3.3) Boot-policy FATAL on bounded-unrecoverable recovery. SPLIT into a PURE decision + a SIDE-EFFECTING terminate so the decision is unit-testable without aborting: `recover_decision_t {ELIGIBLE=0,FENCE=1,FATAL=2}` + `recover_policy_inputs_t {verdict, durable_below_hwm, healthy_peer_reachable, catch_up_succeeded, durable_floor_lsn, cluster_hwm_lsn}` in persist.h; `persist_recover_decision(const inputs*)` (pure, no side effects) + `persist_recover_fatal_if_unrecoverable(const inputs*)` (logs truncation boundary at ERROR then terminates) in persist.c. FATAL ONLY when (truncated OR below-HWM) AND no healthy peer AND !catch_up_succeeded (rows 4,4b). A reachable healthy peer is NEVER FATAL (rows 3,5 → FENCE) — this is the invariant 3.3 owns; the full FENCE→ELIGIBLE eligibility is 4.3. TERMINATE SEAM: added `persist_fatal_hook` (extern void(*)(const char*), declared in persist.h, DEFINED =NULL in persist.c — NOT under XCE_TESTING so production links it as a NULL global → production path is `tlog_exit(); abort()`). Mirrors the existing `ha_fatal_hook` convention (ha_convergence.c:58 / ha.h:1045). Tests install the hook to capture FATAL without dying (test_commit_durability.c dur_fatal_hook + g_fatal_hook_calls counter). Bounded window = `DEFAULT_RECOVERY_CATCHUP_WINDOW_MS 30000` (persist.h) — FINITE, never indefinite. Production nm-verified: persist_recover_decision/persist_recover_fatal_if_unrecoverable are real T symbols, XCE_TESTING seams excluded. Rows 4/4b + a focused peer-never-fatal test PASS; rows 1/2/3/5/6 stay TEST_IGNORE for 4.3 (suite GREEN). `tlog_exit()` is reachable in persist.c transitively (no extra include needed — ERROR/WARN already pull tlog via xcelog.h chain).
- (2026-06-18, ha-commit-durability task 3.1) Recovery-integrity verdict: `recover_verdict_t {RECOVER_CLEAN=0, RECOVER_TRUNCATED=1}` in persist.h (enum declared BEFORE struct persist_s since the struct field uses it); `recovery_verdict` field on persist_t (NOT ha_state_t — keeps the boundary in persistence, avoids pulling in ha.c); `replay_wal` gained a `recover_verdict_t* verdict_out` out-param (single caller: persist_recover at persist.c). Classification: clean stops = `hn==0` EOF, all-zero pre-alloc sentinel, end-of-segments → CLEAN; structural corruption = CRC chain break (persist.c ~1463/1471), CRC mismatch (~1500), short payload read (~1490), AND `0<hn<sizeof(hdr)` partial-header / `hn<0` read-error → TRUNCATED. Verdict init CLEAN once at fn entry, only ever SET to TRUNCATED (a later clean segment never resets it). Snapshot-sentinel + clean replay is CLEAN (load_snapshot sets checkpoint_lsn from the 16-byte sentinel; replay only adds entries ABOVE it, no sub-sentinel entries = normal). 3.1 ONLY classifies + sets the flag (accessors persist_recovery_verdict / persist_recovery_truncated) — does NOT FATAL (task 3.3) and does NOT fence promotion (task 4.3 consumes it). Verdict is about LOCAL log structure, NOT being behind the cluster (a clean log ending below high-water is CLEAN; freshness/promotion fence catches "behind"). Test crafts corruption by append+flush then pwrite-flipping the on-disk first entry's `crc` field; WAL seg path is `<tmpdir>/<name>/data/wal/seg-%016llx.wal` (name="ctrl-test" in make_durability_persist — first entry is LSN 1 ⇒ seg-...0001.wal, LSN 0 reserved).
- (2026-06-18, controller test ordering) PRE-EXISTING FAILURE (NOT a regression): `test_fact_hook_append_is_retained_and_nonblocking` (test_commit_barrier_rollout.c:446 "a covering ACK must advance committed_lsn over the retained fact") FAILS even on a clean baseline (git-stash-confirmed) and FAILS in isolation (`controller_unit_test -t test_commit_barrier_rollout`) — a timing-sensitive commit-barrier test, unrelated to recovery/persist changes. Also: leftover `/tmp/xce_*` dirs cause a cascade (errno=5 EIO fdatasync noise + extra spurious FAILs like test_demotion_replay_yields_committed_only); `rm -rf /tmp/xce_*` before each suite run clears it and the run is stable at exactly that one pre-existing fail. When validating a persistence task, confirm a failing barrier test is the baseline one before treating it as your regression.
- (2026-06-18, ha-commit-durability task 1.1) Controller CLI flags follow a fixed 6-point argp pattern in `src/controller/main.c`: (1) `#define OPT_*` key above ASCII, (2) field in `struct arguments`, (3) entry in `options[]`, (4) `case` in `parse_opt` (validate, e.g. `atoi`<0 → `argp_error`), (5) default-zero init in `main()` (0 = "not passed"), (6) assign into `myconf` (struct in `src/include/myconf.h`, NOT `src/controller/`). `persist_config_t` overrides live in `src/include/persist.h` as `static inline` helpers next to `persist_config_defaults()`; there are TWO `persist_config_defaults()` callers to keep in sync — `cluster.c` phase2 (~L1267) and `myapp.c` step 6 (~L164). GOTCHA: capnp generates a same-named `PersistConfig_set_walSyncMs` (wire serialization) — the compiler's "did you mean" suggests it; it is NOT the runtime override helper. Test home: `testsuite/test_cluster_persist.c` (already covers `persist_config_defaults` + `wal_sync_ms`).
- (2026-06-17, bug-660 slow-election convergence) `ha->last_activity` is DUAL-PURPOSE and that conflation is a trap: it's the watchdog grace clock (reset to now on every `ha_arm_heartbeat_timeout_vtask`, incl. `ha_prevote_rearm_liveness` after each lost PreVote) AND the source for `ha_heartbeat_is_fresh` (the `reason=fresh_heartbeat` PreVote/vote deny). So a node with NO live primary keeps refreshing its own "fresh heartbeat" every failover cycle and denies every legitimate higher-term candidate → elections take 11+ rounds / minutes to converge (a standby denied for 2.5 min after its primary was killed). FIX: the eligibility gate must also require a LIVE primary connection. `raft_cb_vote_eligible` (ha.c:311) checked `is_fresh` ALONE; `handle_request_vote` (ha.c:2205) already required `primary_ctrl_fd >= 0 && is_fresh`. Adding `ha->primary_ctrl_fd >= 0 &&` to ha.c:311 (+ cluster.c:526 reason logging) makes a dead/absent primary (fd=-1 on init / RDHUP / watchdog-fire; real fd only set at config_repl.c:1451 on first ctrlhello) GRANT → converge in ~1 round. The existing fresh-deny tests already set `primary_ctrl_fd=999` and documented "the deny gate requires a live connection AND a fresh heartbeat" — production just didn't enforce it. Diagnostic signature: `ag "PreVote DENIED" logs/` shows `reason=fresh_heartbeat` long after the primary's last log line. Test: test_pre_vote_dead_primary_grants_despite_fresh_activity (controller_unit_test -t test_ha_vote_gate).
- (2026-06-15, ha-rejoin bug-340) A killed+restarted controller loops elections forever because the PRIMARY never evicts the dead/wedged outbound standby endpoint. primary_heartbeat_fn (ha.c) has two steps: (1) NOP each ep in ha->standby_endpoints, (2) reconnect to any peer NOT present in standby_endpoints. A wedged ep (peer died → handle_error did EPOLL_CTL_DEL but left it in the array; epoll MOD then returns ENOENT → the `WARN endpoint_update_epoll rc1=-1` spam) makes step 2 think the peer is still connected → reconnect SUPPRESSED → restarted node never gets a ctrlhello → re-elects every heartbeat_timeout (term climbs unbounded). xce_nop_req returns >=0 on enqueue even when the epoll re-arm fails, so the heartbeat layer can't see the wedge. FIX: evict in step 1 using endpoint_t.ts_last_in (existing wall-clock field, stamped by handle_input) — if ts_last_in>0 && now-ts_last_in>heartbeat_timeout, zarray_del from standby_endpoints + repl_remove_standby; reconnect then re-establishes the channel and ctrl-1 settles via ha_discover_primary (NO term gate — accepts any primary msg while ELECTING regardless of its own high term). Gotchas: (a) ha_arm_heartbeat_timeout_vtask ALREADY resets last_activity=now, so 'become_follower doesn't reset liveness' is NOT a bug — the ~timeout loop period proves the watchdog is correct; the sole cause is the broken channel. (b) standby_endpoints holds TWO ep types (promoted vote_ep_ctx_t with ctx!=NULL, and heartbeat standby_ep_ctx_t with ctx==NULL) sharing first 4 fields; don't free vote-ep data (UAF — live vctx pointers). (c) Run one suite: controller_unit_test -t test_ha_liveness, ASAN_OPTIONS=detect_leaks=0.
- (2026-06-17, bug-646) The `WARN endpoint_update_epoll rc1=-1` spam (same one called out in the bug-340 learning above) was a SILENT-SWALLOW: `endpoint_update_epoll` (netio.c) logged the `iomgr_update_poll` failure but returned 0, leaving the poll interest unarmed. `iomgr_update_poll` uses EPOLL_CTL_MOD → ENOENT when the fd isn't registered (e.g. after a failed reconnect, or a wedged ep that handle_error EPOLL_CTL_DEL'd but left in the array). Consequence on the CLIENT: a `jobsub` after failover hangs forever in `cbuf_get` (xceapi.c, no timeout) while `io_worker` sleeps in `epoll_wait` — because the request bytes were never flushed (EPOLLOUT never armed) and no socket-drop fired `handle_disconnect`. FIXED: endpoint_update_epoll now returns -errno; `endpoint_enq` runs `handle_error(ep)` on arm-failure (wakes the waiter via handle_disconnect, failover rotates); `handle_output` propagates too. Non-obvious facts that ruled out other causes: (1) server commit barrier is bounded — DEFAULT_SYNC_TIMEOUT_MS=500 (config.h:93); (2) `ha_dispatch_gate` (ha.c:1628, called at cluster.c:251) ALREADY replies E_XCE_STANDBY / -XCE_ERR_NOT_READY for writes to a non-primary / quorum-less-primary — so "writes aren't role-gated" is NOT a gap; (3) client connection-break IS handled (handle_disconnect injects -ECONNRESET into wbuf). Test: test_netio_arm_failure.c.
- (2026-06-17, bug-648 + VERIFIED SSL close semantics) OpenSSL 3.x peer-close behavior, confirmed against official docs (SSL_get_error/SSL_read man pages) AND a live probe on this build (OpenSSL 3.6.0): a peer close makes **SSL_read() return 0** (never <0 for a clean FIN, never WANT_READ) and the meaning is carried by **SSL_get_error(), NOT the return value or errno**. Mapping: clean close_notify → SSL_ERROR_ZERO_RETURN (errno 0); **unexpected EOF (FIN, no close_notify) → SSL_ERROR_SSL since OpenSSL 3.0** (pre-3.0 it was SSL_ERROR_SYSCALL + errno 0 — documented in the SSL_get_error NOTES); RST → SSL_ERROR_SYSCALL errno=ECONNRESET(104). **errno is authoritative ONLY for SSL_ERROR_SYSCALL**; for SSL_ERROR_SSL the error QUEUE is authoritative and any errno is incidental — we measured EPIPE(32), which is the leaked side effect of OpenSSL's internal alert-WRITE (raw write() on a broken conn → EPIPE; raw read() → EOF/0 or ECONNRESET, never EPIPE). Consequence: the old `rc = -errno` in io_fsm_do swallowed every documented EOF case (errno 0 → 0). FIX: io_classify_io_error() (pure, non-static, unit-tested in test_netio_close_propagation.c) returns 0 only for WANT_READ/WANT_WRITE + EAGAIN syscall, else negative ((ecode!=0)?-ecode:-ECONNRESET). DESIGN CONCLUSION (user, reaffirmed [[feedback-fail-loud-over-timeout]]): **no client reply-wait timeout** — close detection is made deterministic at the source instead. CAVEAT: on THIS 3.6 build a process-death FIN gave SSL_ERROR_SSL/errno=32 which the OLD code already propagated, so bug-648 is likely NOT the sole cause of the controller-kill permanent hang — that still implicates the reconnect/SSL-handshake-stall path (client TCP-connected, state RUN, handshake never completes → request queued, never flushed, io_worker idle in epoll_wait). Verify live before closing that out.
- (2026-06-15, ha-election bug-339) Candidate-advertised RequestVote LSN MUST equal the voter-side freshness floor. Voter floor = ha_vote_self_freshness(ha, config_tag, applied_lsn) = max(applied_lsn, last_known_committed_lsn) (cluster.c OPCode_requestVote). On a STANDBY, persist.committed_lsn NEVER advances (only the primary commit engine writes it at persist.c:1192); standbys advance applied_lsn (ha_apply_wal_catchup_entry) + last_known_committed_lsn (heartbeat replies). So a candidate that advertises persist.committed_lsn under-advertises vs its own floor → after a primary dies, every surviving standby advertises stale-low LSN while each peer's floor is high → freshness gate (cand_lsn<my_lsn) denies on BOTH sides → no quorum → infinite RDHUP→ELECTING→STANDBY loop, terms climb in lockstep, nobody ever become_leader. Boot path (myapp.c:217) already uses applied_lsn; only the RDHUP failover path (ha_on_primary_ctrl_fd_rdhup) was inconsistent. Fix: compute cand_tag/cand_lsn via ha_vote_self_freshness once and feed BOTH the elect_ctx (votes[0]+self_*) AND the ha_rdhup_election_hook args. Symmetric advertised==floor guarantees the freshest node is granted by all peers. Diagnostic signature in logs: both nodes only raft_cb_become_follower (never become_leader) + RDHUP every ~heartbeat_timeout.
- (2026-06-14, election-safety) ha_on_primary_ctrl_fd_rdhup MUST mirror the ha_start_election_async no-config voter-only guard: a config-less node (myconf->config_version_tag==0) with candidates_count>1 must NOT start an RDHUP re-election (it would tie-grant with another tag=0 node — freshness gate only denies a LOWER tag — and elect a config-less primary that pushes an empty config). Guard sits AFTER the WAL-truncate block, BEFORE elect_ctx alloc/hook/raft_on_leader_lost; sets role=CTRL_STANDBY and returns. RDHUP integration/failover tests that assert election-after-RDHUP use make_cluster/memset which leaves config_version_tag=0 + candidates_count=2 — they were incidentally testing the buggy condition; give them a nonzero tag (0x1234) so they exercise the legitimate CONFIGURED-node path. RDHUP handlers called with c==NULL early-return before the guard (unit-mode tests assert only the CTRL_ELECTING transition). CTRL_PRIMARY enum==0, CTRL_STANDBY==1, CTRL_ELECTING==3 (test printf "role=0==CTRL_PRIMARY" is expected). Full controller suite: meson driver SIGTERMs at 300s — run ./xce.build/controller/controller_unit_test directly with ASAN_OPTIONS=detect_leaks=0.
- (2026-06-13, barrier-rollout 5.2) push_config_to_standbys now RETURNS the configAck barrier result (0/-ETIMEDOUT/-EINVAL), push count via acked_out; the configAck branch of repl_wait_majority_ack reads entry_lsn as the push tag and r->ce->n_peers (set-once, lock-free) as the denominator, ce==NULL -> -EINVAL fail-loud. ha.c/join.c streaming pushes discard the return BY DESIGN (log-only); only do_upgrade_config_with_bytes treats it as authoritative.
- [2026-06-12] Census parsing caveat: controller_unit_test logs (tinylog) and Unity result lines BOTH go to stdout and can splice mid-line under the multi-threaded config-push tests — a distinct-Unity-line count silently drops spliced results. Robust census = union of Unity result names AND the `test_x: PASS` printf lines, cross-checked against the per-group `N Tests M Failures` summary blocks (which are splice-immune).
- [2026-06-12] Config-barrier test trick: when a test needs a covering config ACK for a tag only known AFTER conf_save_config hashes the re-encoded blob (upgradeConfig path), pre-ack with acked_config_lsn = UINT64_MAX — covers any push tag under both legacy (>0) and per-push (>= tag) counting.
- [2026-06-12] A zeroed stack cbuf_t DEADLOCKS on cbuf_put (ncap==0 → len==ncap → cond_wait forever); any fixture driving do_upgrade_config_with_bytes' restart path must cbuf_init(&c->wbuf, …) first.
- [2026-06-13, single-leader-role 1.1] A WAL type with value 31..99 (below WAL_ENTRY_TYPE_MAX=100) is observationally IDENTICAL to a generic entry in the current tree for append (persist_wal_async writes any type), apply_wal_entry (common-path LSN advance via `default:` arm), and replay_wal (its OWN type filter only skips `type > WAL_CHECKPOINT(100)`). So a behavioral RED test for "WAL_NOOP append/apply works" PASSES even before WAL_NOOP exists. The ONLY genuinely-broken behavior pre-impl is wal_classify (totality `assert(0)` aborts on the unclassified value). RED carrier for append/apply cases = couple them to `wal_classify(WAL_NOOP)→RETAIN` via the fork helper (the enum value + classify row land in the SAME task 1.2 change per design).
- [2026-06-13] persist_t starts current_lsn==0 (calloc-zeroed; persist_init does NOT reset it). The FIRST persist_wal_async gets lsn==0; in replay_wal the `hdr.lsn <= from_lsn` (0<=0) skip means current_lsn never advances for a lone first entry. Replay fixtures asserting an LSN advance must write a preceding entry (or otherwise land the entry-under-test at lsn>0). replay_wal (persist.c:783) is SEPARATE from apply_wal_entry and has its own zero-region detector (`lsn==0 && type==0 && payload_len==0 → break`) and type filter.
- Census EXACT method (2026-06-12): count DISTINCT Unity `file:line:test:PASS` triples (`grep -aoE` + sort -u), not raw `2mPASS` occurrences — tinylog interleaving garbles ~9 verdict lines per run and duplicates others; distinct-triple count matched the 494-baseline arithmetic exactly (495 after task 4.3's +1).
- repl registry reconnect semantics: repl_remove_standby vacates the slot (ep=NULL, keeps last_disconnected_lsn guard); repl_add_standby reuses the first empty slot and memsets it (acked_lsn=0). With both standbys removed, majority_candidate_locked zero-fills to n_peers-1 → candidate 0 → committed_lsn frozen (RETAIN entries safely pending); after re-add, ONE covering ack = 2-of-3 majority.
- repl_wait_majority_ack walAck track is engine-only since barrier-rollout 2.2: entry_lsn==0 returns 0 immediately (documented once-per-lifetime LSN-0 first entry), everything else delegates to commit_submit unconditionally — ce==NULL surfaces as -EINVAL from commit_submit (unreachable in production; cluster_init_phase2 always wires the engine). The counting loop that remains in the function is configAck-ONLY (no self-count, n_active==0 shortcut is Req 8.5 config behavior). persist_wal_sync_append's residual non-engine branch can only store committed_lsn for the LSN-0 no-op case. (2026-06-12, task 2.2)
- `commit_submit(COMMIT_MODE_RETAIN)` is fully lock-free: it returns 0 BEFORE `pthread_mutex_lock(&ce->lock)` (commit.c:233-239), so it is safe to call from hook functions even while entity write locks are held — only WAIT mode takes ce->lock. (barrier-rollout task 1.2, 2026-06-12)
- ha-commit-engine: once repl.c references commit_* symbols (task 5.2), commit.c MUST be in controller_src (meson.build) or the production xce_controller link fails — the "linkage lands with 5.3" note in tasks.md only held while repl.c had no engine references. Lock order ce->lock → r->lock: repl.c calls engine hooks only AFTER releasing r->lock.
- Use `clang-format` for C/C++ formatting; shared config lives at `xce/.clang-format`.
- Logging macros in utils.h: TRACE (= tlog_debug3, verbose), INFO, WARN. There is NO DEBUG3 macro — use TRACE for debug-level logging in repl.c and other controller files. (2026-06-10)
- `msg_handler_t` in netio.h is `int (*)(void*)` — NOT `int (*)(endpoint_t*)`. Functions assigned to `ep->msg_handler` must use `void *ep` parameter. (Task 2.2, 2026-05-30)
- `xce_request()` is defined in request.c but not declared in request.h; files that need it must add `extern int xce_request(endpoint_t *ep, xce_request_t *req);` locally (pattern from config_repl.c).
- Test seam pattern for ha.c XCE_TESTING: declare `int (*hook)(ep, req) = NULL` under `#ifdef XCE_TESTING`, expose as extern in test file, test sets it before calling function under test. Mirrors `xce_retry_send_hook` in request.c.
- capnp UAF fix pattern (Task 2.3): INVOKE_MSG_HANDLER fires ep->msg_handler(ep) directly from iomgr thread; packet is in ep->fsm_in.pkt. Do raw capn_init_mem decode, read_RequestVoteReply into local struct BEFORE capn_free, then capn_free + free_packet + free(pkt). Use local scalars (not capnp pointers) after free. (2026-05-30)
- capnp serialization for tests: use new_RequestVoteReply(seg) / new_RequestReply(seg) / new_Request(seg) — NOT raw capn_new_struct. Follow pattern from xce_encode / xce_replconfigpush_req_raw in request.c.
- To avoid C99 "goto jumps over declaration" error: declare all local vars at top of function (or in a sub-block), not after the first goto target label.
- hook seam forward-declaration order: declare the hook variable BEFORE the function that references it (even within the same XCE_TESTING block). Use a dispatch helper static function to decouple seam from production path.
- term fencing must live in the handler, not the dispatch path (2026-05-31): vote_reply_msg_handler delegates to vote_reply_dispatch which calls either xce_vote_reply_hook (XCE_TESTING) or raft_on_vote_reply (production). If term fencing only happens inside raft_cb_become_follower (production), it is silently skipped in the test hook path. Pattern: do explicit CAS loop (atomic_compare_exchange_weak) on ha->term before calling vote_reply_dispatch, so the update applies regardless of which dispatch path runs.
- ha_start_election_async ownership rule (Task 2.4, 2026-05-30): in the production path (c != NULL), ha_elect_ctx_t is owned by election_timeout_fn which frees it on fire. In the test-mode path (c == NULL, peers == NULL), no delaymgr task owns ctx — must free ctx inline after ha_elect_evaluate_and_signal fires. ASAN will catch the leak if forgotten.
- ha_start_election_async single-node shortcircuit (Task 2.4): n_peers==1 means n_other=0 — allocate ha_elect_ctx_t with 0 flexible entries (no votes[] slots needed), set votes_granted=1, call evaluate directly, then free ctx immediately. No iomgr or delaymgr involvement for single-node.
- election_timeout_fn must lock ctx->lock before calling ha_elect_evaluate_and_signal (Task 2.4): the evaluate function has a done_posted guard but its callers in the reply path hold ctx->lock; timeout fn must also hold the lock to avoid races with concurrent reply processing.
- single-node shortcircuit enters CTRL_PRIMARY_COMMITTING, NOT CTRL_PRIMARY (Task 2.4 review fix): task 4.4 committing timeout promotes to PRIMARY for degraded/single-node mode (Req 8.4). Tests must assert CTRL_PRIMARY_COMMITTING.
- ha_elect_evaluate_and_signal out-of-bounds guard (Task 2.4 review fix): ctx->votes[] is a C99 flexible array with n_peers entries. When n_peers==0 (single-node), votes[0] is out-of-bounds. Must check n_peers==0 FIRST and short-circuit to PRIMARY (quorum=1, self-vote=1 always wins). Guard added before any votes[0] access.
- client_ssl_ctx for outgoing vote connections (Task 2.4 review fix): endpoint_connect() creates SSL from iomgr->ssl_ctx (server context). After connect returns, replace ep->ssl with SSL_new(client_ssl_ctx), SSL_set_fd to the same fd, SSL_set_connect_state. If SSL_new fails, fall through (handshake fails → vote denied via err_handler).
- vote_ep_ctx_t lifetime (Task 2.4 review fix): do NOT free vctx in vote_ep_err_handler. ep->data (vctx) is also used by vote_reply_msg_handler. Cleanup deferred to ha_election_cleanup() (task 5.1). Freeing in err_handler causes UAF.
- timeout_ctx field in ha_elect_ctx_t (Task 2.4 review fix): added void *timeout_ctx field pointing to election_timeout_ctx_t. Set after delay_task_ms succeeds. Allows task 4.1 (quorum-reached path) to cancel the watchdog by setting timeout_ctx->cancelled=1 atomically.
- repl_add_standby idempotency (2026-06-10): scan all MAX_STANDBYS slots for ep match BEFORE the nstandbys guard and empty-slot search. Return the existing slot index immediately on match (no state change). This prevents double-registration when raft_cb_become_leader promotes vote eps and on_standby_connected fires for a reconnecting late-joiner. Covered by test_repl_add_standby_idempotent.
- Heartbeat freshness source of truth (2026-06-09): `ha_heartbeat_is_fresh` derives from `ha->last_activity` (whole CLOCK_MONOTONIC seconds, written by ha_on_heartbeat_received, lock-free time_t read). The old `last_heartbeat` timespec field was REMOVED from ha_state_t — do not reintroduce it. The standby vote-deny gate is `role==CTRL_STANDBY && primary_ctrl_fd >= 0 && ha_heartbeat_is_fresh(ha)`; RDHUP resets primary_ctrl_fd to -1 so a standby whose primary died can grant votes immediately even if last_activity is still fresh.
- raft_init restore contract (2026-06-09): persistence writes (term, voted_for) as a pair, so `raft_init` must set `voted_for_term = initial_term` when `initial_voted_for != -1`. Leaving it 0 lets a restarted node double-vote in the same term (Election Safety violation). Covered by test_proto_H6.
- repl_add_standby returns the SLOT INDEX (>=0) on success, not 0 (2026-06-11, ha-commit-engine task 1.1): asserting `== 0` breaks on the second standby. Assert `>= 0` or the expected index.
- Unity harness quirks in controller_unit_test (2026-06-11): (a) failure lines misattribute the FILE — Unity.TestFile comes from whichever group's UNITY_BEGIN ran, so a failure printed as test_repl_unit.c:294 may live in test_p2_integration.c; locate the actual test by grepping for the test NAME. (b) Each group's UNITY_BEGIN resets counters, so the process exit code only reflects the LAST group — failures in earlier groups are masked (exit=0); meson reports green despite real failures. Grep output for FAIL with `grep FAIL` on the raw log (ANSI codes sit between ':' and 'FAIL', so `grep ":FAIL"` misses them).
- 3 pre-existing controller suite failures on persistent_ha baseline (2026-06-11, verified by stash/rebuild without ha-commit-engine changes, see bug-411): test_committed_lsn_does_not_advance_on_timeout (rc -110 vs expected 0 — persist_wal_sync_append now returns -ETIMEDOUT, test expects old rc==0+sync_rc contract), test_repl_ack_after_promote_gate_allows_jobsub and test_rdhup_primary_dispatch_gate_opens (both -10013: ha_has_write_quorum false, standby_endpoints empty with n_peers=2).
- ha-commit-engine: task 1.2 shipped commit_on_membership_change as a FULL re-evaluation (commit_advance_locked + cond broadcast), and majority_candidate_locked already excludes in_catch_up + ep==NULL slots. Task 3.1 membership tests are green-on-arrival; task 3.2 is a no-op (verified 2026-06-11, tests 17-20 in test_commit_engine.c).
- key_find (xkvmgr_find) returns 1 when found, NOT 0 — queue_ctrl/worker_ctrl/job_ctrl leak this into rc: `if (rc == 0)` hook gates never fire on the found path (bug-447); any code reusing key_find's rc for success checks must normalise it.
- Controller test binary reply observation: no xce_reply hook exists in src/lib; observe replies on a zeroed no-iomgr endpoint_t — xce_reply endpoint_enq()s onto ep->outq; drain with endpoint_deq + raw capnp read_Request/read_RequestReply (test_commit_barrier_reply.c).
- makeJob needs job_obj_init() once per process AND xce_job_t.submit != NULL; makeString needs xstring_table_init (test_worker_join.c inits it unguarded at group entry — probe makeString for NULL before initialising to stay -t-selection safe).
- (2026-06-13, barrier-rollout 6.1) mark_job_start/finish/fail/update are STATIC in job.c — testsuite reaches them ONLY via the non-static wrappers on_job_start/on_job_finish/on_job_fail (and on_job_postdone → mark_job_update). Those wrappers `return 0` unconditionally regardless of mark_job_* rc, so the guard's "success-no-op rc" observable is asserted on the wrapper rc (== 0). No public caller produces a terminal→non-terminal UPDATE (on_job_postdone OR's PDONE = forward; job_ctrl SIGCONT only clears suspend bits) — mark_job_start (state := JOB_STAT_RUN) is the faithful publicly-reachable terminal-clearing witness for the shared backward rule. current_lsn is the "did a hook fire / WAL append" oracle: a guarded-out no-op runs no hooks → no persist_wal_async → current_lsn frozen; combine with cbrp_count_jobstate_entries (on-disk scan) + a key_find live-state read. Controller suite baseline 2026-06-13: 502 distinct PASS triples pre-6.1; +3 with 6.1's 2 preservation pins landing green (forward-update, PERR-merge) = 505, 3 FAIL = the 3 new RED guard tests.
- (2026-06-13, single-leader-role 2.2) The own-term promotion noop uses `persist_wal_async(&c->persist, WAL_NOOP, NULL, 0)` which ALWAYS calls repl_broadcast → xce_wal_batch_req_raw(s->ep) for every registered, non-catch-up standby. Any test fixture that registers fake `int` endpoints (the `g_*_eps[]` pattern) via repl_add_standby MUST install `repl_broadcast_send_hook` (drop hook) before driving a real append, or ASAN aborts with global-buffer-overflow reading ep->peer (offset 36 past the 4-byte int). Precedent: test_commit_engine.c ce_drop_send_hook, test_p2_integration.c p2_drop_send_hook. The task-2.1 fixture lifecycle_cluster_setup (test_ha_role_lifecycle.c) OMITS this hook AND off-by-ones the current_lsn prime (sets appended_lsn+1; persist_wal_async takes current_lsn then ++, so to land the noop at `appended_lsn` the prime must be `appended_lsn`). Both are fixture bugs (bug-477/478), out of ha.c boundary. Production noop_lsn = current_lsn-1 read under p->lock AFTER the append is correct.
- (2026-06-13, single-leader-role 2.2) raft_cb_become_leader does NOT call ha_do_promote — the raft FSM already synced ha->term at entry (~ha.c:463 atomic_store of the raft-reported term); calling ha_do_promote would double-increment (research §4 finding 2). The CTRL_PRIMARY store sits OUTSIDE the `if(c)` block so the c==NULL test shim reaches it. The non-raft road (ha_promote_to_primary) DOES call ha_do_promote (it owns its term). Both roads share ha_register_persist_hooks (BEFORE noop append) + ha_arm_own_term_gate (noop + commit_set_writable_lsn).
- (2026-06-13, single-leader-role COMPLETE 11/11, validate-impl GO) `CTRL_PRIMARY_COMMITTING` REMOVED — `ctrl_role_t` now 4 values (CTRL_PRIMARY=0, CTRL_STANDBY=1, CTRL_INIT=2, CTRL_ELECTING=3). Deleted without replacement: `committing_timeout_fn` (60s watchdog), `ha_check_majority_convergence`, `ha_repl_ack_received`/`repl_ack_count`, `committing_timeout_ms`/`HA_DEFAULT_COMMITTING_TIMEOUT_MS`. `OPCode_replCatchUpDone` is now a gate-allowed INFO no-op. Dispatch-gate PRIMARY branch + barrier-rollout fact allowlist preserved byte-for-byte; commit.c consumed with ZERO diff. Repo-wide completion grep (`CTRL_PRIMARY_COMMITTING|PRIMARY_COMMITTING|committing_timeout|repl_ack_count|ha_repl_ack_received|ha_check_majority_convergence`, excl .git/.kiro/specs) = ZERO.
- (2026-06-13) OPERATIONAL: controller meson test wrapper ROUTINELY hits its 300s timeout under load (barrier commit-deadline waits) — `Timeout: 1` is NOT a failure. Authoritative green = direct binary `src/xce.build/controller/controller_unit_test` (EXIT 0, every group "N Tests 0 Failures"). A pre-existing ~153KB suite-wide LSan leak is SUPPRESSED by `ASAN_OPTIONS=detect_leaks=0` at controller/meson.build:163 — LSan exit=1 only appears on a RAW direct-binary run without that env; baseline, reproduces at pre-spec commits, not a per-change regression.
- (2026-06-14, reserve-lsn-0 / bug-543 FIX) LSN 0 is now RESERVED as the none/empty sentinel — `persist_init()` sets `current_lsn = 1` on a fresh store (persist.c, the ONLY fresh-init site; recovery overwrites from snapshot/WAL tip). First-ever WAL entry (promotion NOOP) is lsn 1. This was a ONE-LINE production change + a comment block; everything downstream ALREADY treated 0 as "none/empty" and lsn>=1 as real, so NO change was needed in commit.c (highest_appended_lsn `current_lsn>0?-1:0`, commit_submit rejects lsn 0, wait_done writable_lsn==0=gate-off), the streamer (`hdr.lsn>standby_lsn`: 1>0=true now streams the first entry → no hole), ha.c own-term gate (noop_lsn=current_lsn-1=1, writable_lsn=1 effective), or the wire/capnp format. The pre-alloc zero-region sentinel `hdr.lsn==0 && type==0 && payload_len==0` (persist.c:874, config_repl.c:1580, truncate:1666) is STRENGTHENED — no real entry is ever lsn 0. Segment naming follows current_lsn at open time, so the first segment is `seg-...001.wal` (not 000); recovery/truncate scan by sscanf, no hard-coded seg-000 assumption.
- (2026-06-14, reserve-lsn-0 TEST FALLOUT) Base-shift broke tests that did a "priming append so the entry lands at LSN >= 1" — that priming was ONLY to escape the old lsn-0 special-case and is now redundant. Fix = REMOVE the priming append so the real entry lands at lsn 1 (test_commit_engine tests 32/34, test_commit_barrier_rollout fact-hook, test_commit_barrier_reply cbrp_setup, test_p2_integration single_node) OR bump a hard-coded `current_lsn==1` assertion to `==2` if the priming must stay (test_egress_filter ef_setup). The ENGINE-LESS walAck path only "worked" via `repl_wait_majority_ack`'s `entry_lsn==0 → return 0` hatch (repl.c:437); with the first entry now lsn 1 it routes to `commit_submit(ce=NULL)` → -EINVAL, so engine-less commit tests MUST wire a commit engine (test_p2_integration test 9: added commit_engine_init(n_peers=3) + repl_set_commit_engine + commit_engine_fini). Tests asserting RELATIVE lsn (lsn_before+1, current_lsn>0, h1.lsn>h0.lsn) or byte-offset truncation math are base-shift-immune.
- (2026-07-03, raft-apply-decoupling task 3.3) `test_commit_barrier_reply.c`'s `cbrp_setup` fixture wires job submission's commit continuations through a SEPARATE standalone `commit_engine_t g_hce` (via `commit_engine_init(&g_hce, &g_hc.persist, ...)` + `repl_set_commit_engine`), NOT through `cluster_t`'s own embedded `c->commit` field. This matters because `raft_cb_become_follower`'s demotion branch (ha.c:1395) calls `commit_resolve_pending_above(&c->commit, 0, -XCE_ERR_STANDBY)` — i.e. it operates on `c->commit`, the field `g_hce` deliberately bypasses. So `raft_cb_become_follower_with_cluster_for_test(&g_hc, term)` CANNOT be used to force a job-submission continuation FAILED in this file's harness — it would touch the wrong (never-initialized, all-zero) engine. `persist_truncate_wal_above(&g_hc.persist, cutoff)` has no such problem: `persist_set_truncate_cb` is wired directly by `commit_engine_init` onto whichever engine you pass it (here, `g_hce`), so truncation is the ONLY viable way to force a deterministic, explicit non-timeout FAILED outcome (`-ENOENT`) for a job-submit continuation in this specific fixture. Added 4 tests exercising this (single + repeated truncation-forced FAILED, repeated timeout-forced FAILED, array mixed-outcome) — all GREEN on first run (no bug found; task 3.2's failure-path guarantee holds). Generic demotion-resolve-continuations-FAILED behavior already has its own dedicated coverage in `test_demotion_convergence.c` (task 1.4/1.5), which uses `ha_promote_to_primary` + the cluster-aware `_for_test` seam against a REAL `c->commit`-shaped fixture — reuse THAT file, not this one, for anything demotion-specific.
- (2026-06-13, kiro-impl process) A RED-only test pin is only valid if its OWN impl task can turn it green via the SAME seam the test drives. Misfires caught in review: boot-replay pin RED on an lsn-0 `replay_wal` skip artifact; hooks pin via the `c==NULL` raft shim where `ha_register_persist_hooks(&c->hooksys)` can never run (re-point to the real-cluster `ha_promote_to_primary` seam). Deleting machinery → DELETE the machinery-subject tests (design "no shims, rewritten or deleted not stubbed"), never add file-local no-op stubs. Unity runner prints the PREVIOUS passing test's file:line as a stale prefix on FAIL lines → attribute failures by test NAME.
- (2026-07-10, persist_apply_pump_run_sync build+test validation) A bare `grep -i TIMEOUT testlog.txt` is NOT a reliable single-shot hang detector in this suite: it returns 100+ matches every clean run because "timeout" appears in test names (`test_wait_ack_timeout`, `test_heartbeat_timeout_fires_failover`, ...) and in expected WARN-level log lines from tests that deliberately exercise timeout paths (`commit: WAIT deadline expired without commit ... timeout_ms=250`), every one of which is immediately followed by `PASS`. Same false-positive shape as the already-documented `grep -c FAIL` overcount from expected "FAILED" fault-injection log lines (see the `:FAIL:`-anchor entry above) — a plain substring match on an English word that's also load-bearing test vocabulary in an HA/timeout-heavy codebase will always overcount. The AUTHORITATIVE signal for a real meson-level hang is the per-test `result:` field in testlog.txt (`result: exit status 0` = clean; a real hang shows `result: TIMEOUT` or "killed by signal") plus a case-SENSITIVE word-boundary check `grep -noE '\bTIMEOUT\b'` (zero hits = no real timeout verdict anywhere). This is a DIFFERENT axis from the pre-existing 2026-06-13 learning two entries up ("meson wrapper's OWN 300s `Timeout: 1` summary field is not fatal, use the direct binary") — that one is about meson's outer test-runner timing out the whole suite; this one is about naively grepping the word "timeout" inside a clean log. Both must be checked via their authoritative field, never via a bare case-insensitive substring grep.
- (2026-07-10, bug-1431/1432) `delay_task_ms(mgr, interval_ms, -1, fn, arg)` fires its FIRST invocation only after `interval_ms` elapses — it is a periodic timer, not "run now then repeat." A comment claiming a delaymgr-scheduled task starts "immediately" is not proof it does; verify against this semantics before trusting it. `ha_start_primary_heartbeat` had exactly this stale claim, costing a full heartbeat interval (3000ms default) of promotion latency before a reconnecting standby's true log position was learned via the heartbeat-reply stateReport. Fix pattern: call the task function synchronously once before arming the periodic `delay_task_ms`, but ONLY if that function does purely non-blocking work (verified here — `primary_heartbeat_fn` only issues async `xce_ha_heartbeat_req` sends).
- (2026-07-10, bug-1431) `persist_truncate_wal_above`/`truncate_wal_above_locked` only sets `p->committed_lsn` on branches that perform a REAL truncation. Its common no-op early-return (nothing above cutoff_lsn — the case when a newly-promoted leader's own log already covers the just-computed `safe_committed_lsn`) leaves `committed_lsn` untouched. Any caller that derives a new commit floor and expects `persist_truncate_wal_above` to have applied it (a comment in `raft_cb_become_leader` claimed exactly this) must NOT assume that — use a dedicated advance function instead (`persist_promotion_advance_committed`, durable-capped, monotonic via a CAS loop). **UPDATED 2026-07-10 (bug-1433, controller-invariant-audit):** an earlier version of this fix also force-flushed (`flusher_flush_once`, a blocking `fdatasync`) when durable_lsn lagged — REMOVED, because the real multi-node winning-vote-reply path reaches this function on a `VTASK_LIVENESS`-classified vtask (never expected to block — same bug class as bug-1198). The visibility gap is closed non-blockingly by bug-1432's immediate-heartbeat fix alone; no force-flush needed.
- (2026-07-10) A chaotic-HA-soak "COMMITTED JOB LOSS" verdict with `committed_divergence=0` and all jobs present at final settlement is a VISIBILITY-LATENCY gap, not a durability violation: the job was legitimately committed by the old primary before it died, but the new leader's post-election `safe_committed_lsn` (computed only from THIS election's granted-voter sample, which can be conservative when a second voter is mid-restart) is lower until fresh quorum reconfirmation — correct Raft Figure-8 safety, not a bug by itself. Per this project's test philosophy (skill xce-test-ha: "a committed job an elected primary cannot list IS a real loss regardless of eventual reappearance") this is still a hard soak FAIL and must be fixed — but ONLY by shrinking the reconfirmation window (e.g. sending the first post-promotion heartbeat immediately), never by unsafely exposing unconfirmed old-term entries early just to make the check pass.
- (2026-07-10, bug-1433, controller-invariant-audit) `peer_ctrl_classify` (ha.c ~5935) hardcodes `VTASK_LIVENESS` for EVERY inbound peer-control packet (vote replies, PreVote replies, WAL/config ACKs) — a task class documented as "never admission-gated," i.e. never expected to block. `raft_cb_become_leader` (the raft library's `.on_become_leader` callback) is invoked synchronously from THIS dispatch path whenever a real multi-node winning vote reply arrives — NOT from the separate `VTASK_LIVE_BLOCK`-dispatched election-*start* path (`ha_dispatch_election_start`, which only covers RDHUP-failover/single-node self-grant). Any blocking call added inside `raft_cb_become_leader` (or anything it calls) is reachable on a worker class the design guarantees never blocks — same bug class as bug-1198 (closed by moving `persist_checkpoint`'s snapshot-send fdatasync off VTASK_LIVENESS entirely). Before adding ANY blocking call to the promotion path, trace peer_ctrl_classify's actual dispatch, don't trust an existing nearby comment claiming "already on VTASK_LIVE_BLOCK" — one was WRONG (see bug-1433 in buglog.json for the full trace). `persist_apply_pump_run_sync` still has this residual (bounded `pread`, lower risk than `fdatasync`, not fixed this session — see buglog.json bug-1433 for why it can't simply be removed).
- (2026-07-10, bug-1434, controller-invariant-audit) `persist_promotion_advance_committed` and `ha_standby_advance_committed_and_apply` both did an unlocked `load; if(target<=cur) return; store` on `p->committed_lsn` — safe ONLY if truly single-writer, but `raft_cb_become_leader` calls the former BEFORE setting `role = CTRL_PRIMARY` (~170 lines later), so a still-in-flight WAL catch-up entry on another thread can call the latter concurrently during that window — classic lost-update race that can regress `committed_lsn` (monotonicity violation). General lesson: when two independent call sites do "monotonic-max store to a shared atomic, no lock" and you can't prove they're strictly serialized by role/state gating (gating checked AT CALL TIME can still race with a WINDOW where the gate value itself is about to change), use a CAS retry loop (`persist_committed_lsn_advance_atomic`), not a bare load-then-store — it's safe under ANY interleaving with zero lock-ordering risk.
- (2026-07-28, bug-1477) `cluster_init_phase2()` (cluster.c) is the SOLE place that runs `commit_engine_init()` -- which is itself the ONLY place that registers `persist_t`'s `on_durable_advance` wake hook (persist.c:796-799 says so explicitly: "starts unregistered ... the commit engine registers it"). Without that hook, `persist_apply_pump_run`/`apply_committed_entry` NEVER runs on that node, for ANY committed LSN, for its whole process lifetime -- a node can still fully participate in WAL replication/ACK (that path is independent, in ha.c) while never materializing ANY committed state. `cluster_init_phase2` short-circuits with `-EINVAL` (cluster.c:1349-1358) whenever `c->myconf->cluster_config == NULL` (config not loaded yet) -- normal during cold boot -- and its THREE raft-driven callers (`raft_cb_become_follower` ha.c:2000-2012, `raft_cb_become_leader` ha.c:1772-1783, `ha_elect_evaluate_and_signal`'s two branches ha.c:5609-5629/5699-5710) all set `ha->phase2_done=1` UNCONDITIONALLY after calling it, with NO check of its return value -- unlike `myapp.c`'s own `skip_election` fast-path caller (~line 415-425, bug-849 lineage) which correctly gates the latch on success. Worse: a low-priority node (highest `my_priority`, longest startup stagger) that finds "leader already known" on its very first check takes `goto skip_election` and NEVER runs `ha_start_election_async` at all -- so it never reaches ANY of the three raft-callback retry opportunities, ever, for its whole life, if it never later contests its own election. If its one-shot `myapp.c` attempt races ahead of config load, `cluster_init_phase2`/`commit_engine_init` never runs again, PERIOD -- a real, permanent HA-bootstrap gap, predates config-upgrade-snapshot by ~1 month (myapp.c skip_election = commit 20e59d0b 2026-06-21; ha.c raft-callback structure = commit 8f0d2d55 task 6.2), NOT introduced by it. It stayed invisible until config-upgrade-snapshot's task 5 (`persist_checkpoint_marker_triggered`) created a NEW cross-member invariant (every member independently writes a local checkpoint at a synchronized marker-boundary LSN) that a node with a never-wired apply pump silently violates -- surfacing as a PERMANENT (not transient/racy) `ae_reject_logmismatch` catch-up wedge that superficially resembles bug-1475 but is a categorically different, unrecoverable failure (the missing entry is genuinely, permanently absent -- not merely stale bookkeeping) since a WAL_CHECKPOINT entry is local-only/non-replicated by design and can never arrive over the wire to fill the gap. Diagnostic tell: grep the suspect node's log for "commit engine wired" -- if it NEVER appears (vs. multiple times on healthy peers, once per boot/restart/election), the commit engine/apply pump never wired on that node at all.
## Do-Not-Repeat
- (2026-07-29, bug-1482) When a code-review finding says "X is never freed, add a free() call" citing a `_for_test` wrapper's own doc comment as the ownership contract, VERIFY the doc comment against the actual callee before complying literally — don't just trust it. `myreq_handler_for_test`'s comment claimed "the caller still owns/frees `pkt` itself", but `myreq_handler` unconditionally calls `xce_decode(pkt, ...)` as its first step, and `xce_decode()` (src/lib/request.c:274-275) itself unconditionally does `free_packet(pkt); FREEUP(pkt);` before returning — pkt is fully consumed/freed internally, no leak exists. Blindly adding the suggested `free_packet(pkt); free(pkt);` after the call produced an immediate ASAN heap-use-after-free/double-free (independent of `detect_leaks`, since UAF detection isn't gated by leak detection) — caught only by actually rebuilding and running the authoritative test command, not by code inspection alone. Fix was to correct the misleading doc comment instead of adding the free. **Lesson: for any wrapper that decodes/consumes a buffer (xce_decode-style), trace the actual free call sites transitively before trusting an ownership comment — a stale/wrong doc comment is itself a bug, and "add the missing free" is not always the right fix for a reported leak.**
- (2026-07-19, bug-1452) `commit_engine_fini(&ce)` MUST run AFTER `persist_stop(p)`, never before, whenever the `persist_t` was `persist_start`'d (i.e. has a live `fsync_worker` thread). `commit_engine_fini`'s own header comment says this explicitly ("Callers are expected to have stopped the flusher (persist_stop) first") but `test_persist_truncate.c`'s three continuation tests called it backwards (`commit_engine_fini` then `teardown_persist`→`persist_stop`). `persist_set_durable_cb(p, NULL, NULL)` inside `commit_engine_fini` is an unsynchronized plain-pointer unhook — while the flusher thread is still alive it can already be mid-flight in `commit_on_durable_advance`/`commit_advance_locked` (no lock held, per persist.c:398-399's documented lock-ordering) and race the teardown: the guard `ce->p == NULL` check passes on a still-valid read, then a SEPARATE later re-read of `ce->p` inside `commit_advance_locked` (commit.c:208) observes the value `commit_engine_fini` just nulled — SEGV at exactly `offsetof(persist_t, committed_lsn)` (0x38). This was misdiagnosed for a day as caused by the g_drive_async_demotion/catchup_offload flags (bug-1450/1451) because flipping them changed thread-scheduling timing enough to trigger the always-latent race more reliably — the SAME crash signature was independently seen on 2026-07-07, before those flags existed, proving the flags were never causal. Fix: call `persist_stop(p)` immediately before `commit_engine_fini(&ce)`; the teardown helper's own internal `persist_stop` becomes a safe no-op second call. **Same backwards ordering also exists in `test_demotion_convergence.c` (line 233 `commit_engine_fini` before line 235 `persist_stop`) against a real `persist_start`'d fixture — not yet fixed, flag for a follow-up sweep of every `commit_engine_fini` call site in the controller test suite.**
- (2026-07-03, raft-apply-decoupling task 1.5) `ninja -C build`'s INCREMENTAL rebuild in this worktree's shared `build/` dir can silently produce a stale/wrong `controller_unit_test` binary that reports a build success but gives WRONG test results for the actual HEAD source — distinct from bug-924 (which is the `meson test` driver hanging on overlayfs fdatasync). Symptom that tipped this off: `test_config_repl`'s `test_commit_advance_retry_clears_push_before_commit_fence` was independently measured as 70/70-passing by one task's reviewer and 10/10-failing by the next task's implementer, on what both believed was the same commit. Root-caused by isolating each measurement in its own throwaway `git worktree add <tmp-path> <commit>` + fresh `meson setup --buildtype=debug -Db_sanitize=address` + `ninja -C <tmp-build>` — proved the SAME commit passes cleanly with a fresh build and fails deterministically only in the main worktree's incrementally-rebuilt `build/`. Fix was `rm -rf build && meson setup build ... && ninja -C build` in the main worktree. **Lesson: whenever a test result looks surprising or contradicts a very recent measurement on the same commit, do NOT immediately conclude a source regression — first rule out a stale incremental build by either a full `rm -rf build` rebuild or an isolated `git worktree add` + fresh setup.** Never mutate the current worktree in place to "check an old commit" (e.g. `git checkout <sha> -- .`) — it silently reverts working-tree files against current HEAD; use `git worktree add` instead.
- (2026-06-24, raft-sim-conformance task 1.2) A CONFORMANCE cluster (`raft_sim_conformance_init`) wires `on_send_vote_request`/`on_send_append_entries` to the unified `c->bag[]` (sim_conf_send_*), NOT to the legacy `c->pending[]`/`c->ae_pending[]`. So `raft_sim_deliver_pending` / `raft_sim_deliver_append_entries` (which drain pending[]/ae_pending[]) DO NOTHING on a conformance cluster — a 3-node conformance election will leave node 0 stuck in CANDIDATE (assert "Expected 2 Was 1" = RAFT_LEADER vs RAFT_CANDIDATE). Bag delivery is task 2.1. To force a leader in a task-1.2 test WITHOUT bag delivery, use a 1-node conformance cluster: `raft_start_election` short-circuits node 0 straight to RAFT_LEADER (no peers to canvass). The conformance `applied[]` decode reads the committed entry's `node->log[k-1].bytes` (the legacy `raft_sim_entry_t` byte WAL, reused) as a little-endian uint64 (memcpy), NOT the `raft_sim_log_entry_t {term,value}` struct (that type is only used by the bag message `entry_value` field).
- (2026-06-21, raft-sim tasks 9.1-9.4) In raft_sim.c, leader per-peer `next_index[peer]` is initialized to `Len(log)+1` at become_leader and ONLY advances via `raft_on_append_entries_reply` success (= matchIndex+1). It does NOT change when a send to a partitioned peer is dropped (no reply comes back). So to exercise nextIndex BACK-OFF in a sim test you cannot just partition+drop — you must create a real `(prev_log_index)` mismatch: replicate so next_index runs ahead, then shrink the follower's actual log (call `c.nodes[k].cbs.on_truncate_from(&c.nodes[k], idx)`) so the leader's stale next_index points past the follower's true tail; the next AE is then rejected and walks back. To bump a sim leader's term for "higher-term leader" tests, set `c.nodes[0].state.term = N` directly AFTER a clean single election (raft_log_append stamps entries from s->term; next_index/match_index are term-independent) — do NOT chain `raft_on_election_timeout`+`raft_start_election` on a node that is already LEADER (election_timeout is CANDIDATE→FOLLOWER, a no-op on a leader, so the term never climbs). For a duplicate-AE idempotency assertion, rewind `next_index[peer]` to re-send an ALREADY-PRESENT entry (already-have branch = no log growth); re-sending at the live next_index sends the NEXT entry, which is not a duplicate.
- (2026-06-21, bug-850) Do NOT fix "a rejoining standby races live replication ahead of catch-up" by defaulting `repl_add_standby` to `in_catch_up=1`. It has a huge blast radius: a fresh standby then does NOT count toward `repl_active_standby_count` write quorum until it converges, so EVERY commit-WAIT path stalls — the full controller suite TIMED OUT at 300s (commit.c "WAIT deadline expired ... committed_lsn=0"), because unit tests register standbys + commit directly without running the heartbeat dispatch that would clear the flag. The correct, surgical fix is a CONTIGUITY GUARD at the persist layer (Raft log-matching): `persist_wal_write_replicated` rejects a forward gap (`lsn > current_lsn`) with `-EAGAIN` (no write, no current_lsn jump); the apply path treats `-EAGAIN` as a quiet skip (no ack, no ERROR spam) so the ACK-driven catch-up continuation re-streams from `current_lsn` and the log fills in order. Zero quorum-semantics change → zero suite regression (129/0/0). Relies on bug-848 (advertise `durable_floor == current_lsn-1`) so catch-up always starts AT `current_lsn` and is never spuriously rejected.
- (2026-06-18) When a task changes commit semantics to be ASYNC-durable (self caps at durable_lsn, advanced by the demand-driven group-commit flusher), every test that SYNCHRONOUSLY asserts "covering ACK ⇒ committed_lsn advances" must first force durability (persist_flush) or bounded-poll. ha-commit-durability 2.2 primed durable_lsn in the in-memory barrier_setup_vcpus stubs but MISSED the real-persist test_fact_hook_append_is_retained_and_nonblocking → 6/6 deterministic FAIL. Do NOT trust an impl-note that blames a test failure on a /tmp-overlay fdatasync-EIO without re-verifying: fdatasync returned rc=0 on /tmp here and the test passed pre-spec — it was a real durable-gating regression. Also: meson 'Fail:0' masks Unity sub-group failures — always gate on ANSI-stripped ':FAIL:' count from the test binary's stdout, not the meson summary.
- (2026-06-22, ha-term-safety task 3.3 remediation) Do NOT run `clang-format -i` on whole XCE controller source files (ha.c/ha.h/test files): these files are NOT clang-format-clean against the repo's config, so a full reformat rewrites ~1700 lines and BURIES the targeted change (and violates a "preserve the existing diff" instruction). I then made it worse with `git checkout HEAD -- <files>` to undo the format — but the uncommitted task-3.3 work lived ONLY in the working tree, so that wiped it (no stash, unrecoverable from git). Recovery worked only because the full original content was already in my context. Lesson: (1) only format the exact changed regions or skip formatting; (2) NEVER `git checkout HEAD -- <file>` to discard formatting noise when the file also holds uncommitted feature work — instead revert just the format hunks, or stash first. The reflog showed branch HEAD was already at task-3.2 (754e586), and the initial-context "recent commit 2c5d298" was a stale snapshot — the uncommitted 3.3 diff sat on top of 3.2.
- (2026-06-22, ACK peer-index, bug-960) The walAck branch of `repl_ack_decode_and_dispatch` (ha.c) fires for TWO endpoint types whose stored `peer_idx` means DIFFERENT things: promoted-vote eps (`vote_ep_ctx_t`, `ctx!=NULL`) carry a self-EXCLUDED *votes* index (relative); heartbeat standby eps (`standby_ep_ctx_t`, `ctx==NULL`) carry an ALREADY-ABSOLUTE candidate index. The authoritative branch is ha.c:6187-6189 (`sctx->ctx != NULL ? votes_idx : i`). The pre-existing ACK consumer `ha_walack_on_ack(c, vctx->peer_idx, …)` uses `peer_idx` DIRECTLY (no remap) — proof the ACK path's index is already absolute. Any code consuming the ACK `peer_idx` for the library (match_index[]/next_index[], which raft.c writes UNCHECKED) MUST branch on `vctx->ctx != NULL` before remapping, or it double-maps the heartbeat path → wrong-peer false quorum / OOB write. `self_idx == ha->my_priority` (raft_cfg.self_idx = my_priority everywhere).
- (2026-06-24, bug-1060 — RECURRENCE of the 2026-06-22 entry above) Ran `clang-format -i` on whole ha.c/ha.h/test_ha_election.c AGAIN → spurious whole-file reformat (3268/969/635 lines) that buried a ~180-line fix. Confirmed spurious by checking the repo's HEAD ha.c is NOT idempotent under the nix `clang-format` (a `.clang-format` exists at src/.clang-format but the pinned formatter version differs from whatever produced HEAD). Recovery THIS time succeeded because: (a) I had already printed the full pre-format `git diff ha.c` (5 hunks: bug-1028/1042/1043 pre-existing + my bug-1060) into context; (b) ha.h's only genuine new symbol vs HEAD was `ha_sync_raft_log_from_recovery` (found via `comm` on `grep -oE '\bha_[a-z_]+\b'` of HEAD-vs-current — everything else was pointer-style/comment reflow); (c) test_ha_election.c was NOT in the session-start `M` list so it held only my changes. Then `git checkout HEAD -- <3 files>` + re-applied the logical hunks by Edit. LESSON STANDS: do NOT clang-format whole files here; format only changed regions or skip. The `git checkout HEAD --` rescue is ONLY safe after you have positively accounted for EVERY uncommitted change in those files (capture the pre-format `git diff` FIRST, and confirm which files had pre-existing work via the session-start `git status`).
<!-- Mistakes made and corrected. Each entry prevents the same mistake recurring. -->
<!-- Format: [YYYY-MM-DD] Description of what went wrong and what to do instead. -->
- [2026-06-17] Do NOT invoke handle_error()/an endpoint err_handler from the SYNCHRONOUS send path (endpoint_enq / request.c senders). The err_handler is an io_worker-thread callback that may take ha->lock (vote_ep_err_handler, ha.c:3478) or ctx-level locks; the send path runs on the caller's thread, which often already holds that lock (controller primary_heartbeat_fn holds ha->lock while sending heartbeat NOPs) → self-deadlock on a non-recursive mutex (bug-652, a regression from bug-646). Correct pattern: endpoint_enq RETURNS a retriable error (-ECONNRESET) and the caller propagates it (request.c senders return it — bug-650); the real teardown stays on the io_worker thread (io_handler→handle_error), which holds no caller locks. General rule: a libxce send must never run endpoint teardown/callbacks inline — only surface the error.
- [2026-06-09] Do NOT make the duplicate-vote deny candidate-agnostic ("term-only deny"). Raft §5.2 grants iff votedFor ∈ {Nil, candidateId} — same-candidate re-grant must stay idempotent (network retransmission path). An uncommitted ha.c change did this and broke test_vote_granted_to_same_candidate_same_term; ha.c must match raft.c's Step 4 semantics (deny only a DIFFERENT candidate in the same term). See bug-338.
- [2026-06-09] The "self" voted_for_name placeholder only exists in test mode; production raft_cb_persist records the node's REAL name for a self-vote. Any check for "did I vote for myself" must compare against BOTH "self" and c->myconf->name (see myapp.c leader_known).
- [2026-06-15] SOLVED (bug-563): "jobsub ok but joblist/workerlist empty on the HA primary; reslist works". ROOT CAUSE: `cluster_start()` is the ONLY setter of `c->running = 1` and it is ORPHANED — zero callers (grep `cluster_start(`). The HA `myapp_run` path starts iomgr/vtaskmgr/delaymgr/persist itself but never calls `cluster_start`, so `c->running` stays 0 on the primary. `check_controller()` (the streaming gate `xce_reply_get` uses for joblist/workerlist) returns `-EINVAL` when `!c->running`; `xce_reply_get` then sets the chunk count `nc = 0`, shipping an EMPTY list even though `collect_jobs()` found the rows. `reslist` passes `checkf=NULL` so it bypasses the gate — that asymmetry is the tell. Single-node "works" because the legacy `cluster_init` path called `cluster_start`. FIX: `c.running = 1;` in `myapp_run()` after `persist_start()` (src/controller/myapp.c). RULE: when only SOME list ops are empty but the data is present server-side, check the `xce_reply_get` `check_*` predicate — a `<0` return silently zeroes the chunk (`nc=0`), not an error. To prove it: log `nc`/`checkstatus` in the `xce_reply_get` send loop. Reproduce HA bugs locally with `/xce-build` + a 3-node deploy-ha under ASAN rather than theorizing. (My earlier UAF/stale-binary guess here was WRONG — disproven by "crash gone, binary current".) Still-latent: `cluster_start` also armed `check_workers_timeout` (delaymgr) — that side-effect remains unrestored on the HA path.
- [2026-06-16] When adding a NEW opcode to the HA protocol, it MUST be wired into `ha_dispatch_gate()` (src/controller/ha.c) — the role-based admission gate runs in `myreq_handler` (cluster.c:251) BEFORE the per-opcode handler, and an unlisted opcode is rejected (`-XCE_ERR_STANDBY`/`-XCE_ERR_NOT_READY`) so the handler never runs. bug-605: ha-prevote added `OPCode_preVoteRequest` (opcode 38) + handler (cluster.c:440) but not the gate; STANDBY voters silently dropped every incoming PreVote, the candidate counted the failed exchange as a denial, and a cold cluster looped PreVote forever with NO primary. INVARIANT: a PreVote is strictly non-mutating, so it must be admitted in EXACTLY the roles a real `requestVote` is (CTRL_INIT/CTRL_ELECTING/CTRL_STANDBY). Diagnostic tell: voters log zero "PreVote DENIED" yet the candidate keeps "losing" — means the request never reached the handler (gate drop), not a policy denial. Confirm with gdb breakpoint on `ha_dispatch_gate` rc.
- [2026-06-16] HA election timeouts must be RANDOMIZED/STAGGERED on the FAILOVER path, not just at startup. bug-606: when the primary dies, the surviving standbys lose it at the same instant and (before the fix) campaigned in perfect lock-step — both win PreVote (neither sees a primary), both vote self at the same term, split the vote (quorum unreachable with 2 survivors), time out on a FIXED watchdog (raft.c:105) and retry on the FIXED heartbeat_timeout liveness cadence → term climbs unbounded, no leader. The jitter machinery (election_priority_step_ms=500, election_jitter_max_ms=200, xorshift64 on jitter_seed) existed but was wired ONLY into the startup heartbeat-wait (ha_wait_for_heartbeat_or_timeout / myapp.c stagger); the PreVote failover path (liveness_poll_fn→ha_on_primary_ctrl_fd_rdhup→ha_begin_failover→ha_start_prevote_async) bypassed it, and raft's own staggered-start (raft_on_leader_lost→on_start_election_after_delay, raft.c:421) is also bypassed by the PreVote path. FIX: ha_election_stagger_ms() + a delaymgr-scheduled, guarded PreVote start in ha_begin_failover (skip if a primary appears during the stagger). RULE: any new re-election/failover trigger must apply per-node stagger (priority*step + fresh random jitter, re-randomized each attempt) AND a "primary alive?" guard before campaigning. Diagnostic tell: identical ms-aligned election timestamps across nodes + term climbing by 1 each cycle = lock-step split vote, not a vote-policy bug. step MUST exceed jitter_max so adjacent priorities never overlap.
- [2026-06-14] A partially-initialized `xce_reply_t` is a UAF hazard. The Cap'n-C generated `encode_RequestReply` (controller.capnp.c) reads the COMMON TRAILER fields (`redirectHost`, `redirectPort`) on EVERY reply regardless of `data_which` — `redirect_host` is `strlen`'d whenever `!= NULL`. Handlers declare `xce_reply_t reply;` on the stack and only set status/data, so those trailer fields were stack garbage; a garbage `redirect_host` aliasing a freed capnp segment = heap-use-after-free (seen on the OPCode_nop/state-report path, cluster.c). FIX (bug-562): `init_reply_by_req()` now `memset`s the whole struct first. RULE: any xce_reply_t built WITHOUT init_reply_by_req (e.g. ha_send_wal_ack_reply) MUST memset it. Generated union encoders are not safe to feed a partially-init struct — common/trailer fields are always serialized.
- [2026-06-04] capnpc-c 1.0.1 has a bug with `:group` inside a `union` when `$C.codecgen` is active: the encode/decode generator emits `d->FIELDNAME` (flat) instead of `d->data.GROUPNAME.FIELDNAME`, AND places the case as dead code after the prior `break`. Do NOT use inline `:group` in a union that has `$C.codecgen`. Instead, define a named outer struct (`struct Foo $C.mapname("foo_t")`) and reference it as a pointer union member. The named struct generates correct `encode_Foo_ptr`/`decode_Foo_ptr` calls. The `read_RequestReply`/`write_RequestReply` wire layer works correctly with groups but the codec layer does not.
- ha-consistency-invariants decomposed into 4 sub-specs (2026-06-11, user decision): umbrella spec keeps requirements/design/research as the authoritative contract (generates NO tasks); implementation via ha-commit-engine → ha-commit-barrier-rollout → ha-single-leader-role (HARD ordering: barrier before COMMITTING removal), with ha-convergence-recovery parallel after engine. Briefs reference umbrella docs with per-spec requirement ID subsets. Dependency order lives in steering/roadmap.md "## Specs (dependency order)" (kiro-spec-batch parseable). Rationale: ~30-38 tasks in one spec exceeds the ~20-task decomposition threshold; phases are independently shippable (each leaves the system no less consistent).
- Commit-first-then-reply contract (2026-06-11, user, ha-consistency-invariants): job/queue/host(worker) control operations MUST follow validate → mutate(provisional) → WAL append → commit_submit(WAIT) → reply. No success reply before commit. Known violators to restructure: do_jobcontrol_op (job.c:921-934, replies with only async WAL), handle_jobdispatch (job.c:459-471, replies before processing — classification pending). handle_jobsubmit is the reference implementation.
- Four design decisions locked (2026-06-11, user, ha-consistency-invariants): (1) fact entries follow pure Raft — log-is-the-queue, append to WAL + retry replication forever, NO commit timeout for facts; (2) crash-loss of uncommitted facts accepted until worker-rejoin-resync lands (strictly better than today); (3) demotion eviction = process SELF-RESTART after truncation (rebuild-in-place only if restart proves disruptive); (4) own-term commit via explicit NO-OP WAL entry at promotion. Rationale: "follow Raft as it has been well verified."
- Fact-vs-request commit semantics (2026-06-11, user decision, ha-consistency-invariants Req 2.2/2.8): job state transitions are REPORTED FACTS, not requests. Request-shaped ops (submit, queue ctrl, config) reject to requester on commit timeout; fact-shaped ops (worker-reported transitions) are retained and retried until committed — including across quorum loss; primary never discards an uncommitted fact while running+leading. Backstop for facts never received / lost across failover: worker-rejoin-resync (roadmap backlog, designated official backstop).
- vcpus is operator tuning, not a spec constant (2026-06-11, user): `--vcpus` should be sized per deployment by host capability + cluster size + workload; 8 is only the fallback default. Specs/designs must express thread-pool-relative mechanisms as functions of n_vcpus, never absolute counts, and put sizing advice in operator docs.
- vtask pool sizing facts (verified 2026-06-11): default is 8 (NOT 4 — ha-async-io.md was stale, now fixed) via `--vcpus` CLI override, cluster.c:1082. WAL ACK processing runs on the IOMGR thread (`peer_ctrl_msg_handler` → `repl_ack_decode_and_dispatch` → `repl_on_ack`, ha.c:2770-2792) — so blocking commit-waits on vtask threads can never deadlock their own wakeups; all-workers-blocked is a throughput problem only. Pool size should scale with write concurrency/cores, NOT with n_peers (ACKs gather in parallel; Little's law). Mitigation pattern: commit-wait admission cap (n_vcpus - K), async continuation as durable fix.
- xzarray COW shadow is DORMANT infrastructure (discovered 2026-06-11): `xzarray_freeze`/`xzarray_thaw` (zarray.c:695-757) are built, in the varray vtable, unit-tested (test_zarray_cow.c) — but have ZERO production callers (kvmgr.c:297 `vfreeze = NULL /* wired in P0-2 */`). It is slot-level STRUCTURAL COW only: slots hold item pointers, so in-place field mutations are visible through both shadow and active views; freeze is O(1) pointer swap, reads merge active-over-shadow, thaw merges back with a `should_retain` GC filter, single generation (-EALREADY on double freeze). Intended consumer: the never-implemented entity-bearing checkpoint (Task 11 / sentinel-only snapshots).
## Decision Log
<!-- Significant technical decisions with rationale. Why X was chosen over Y. -->
- Recovery-oriented reframing supersedes ARIES/undo direction (2026-06-10, user insight, ha-consistency-invariants): replication here serves FAILOVER/RECOVERY, not transactions. Structural fact: a single-writer leader never truncates its own log while leader, so provisional in-memory state on a live primary either commits later or the node is demoted — discard coincides exactly with role/lifecycle events. Therefore NO per-entry undo log; discard handled by recovery mechanics (truncate divergent suffix + coarse rebuild via existing persist_recover/replay_wal at demotion/rejoin). Only the watermark egress filter survives (entity WAL-LSN tag, committed-only export to dataex/scheduler) — justified as failover correctness (prevent orphan dispatch), not isolation. Also: Raft commitIndex is volatile — prefer leader-driven log reconciliation at rejoin over persisting committed_lsn.
- STM evaluated and rejected for provisional-state mechanism (2026-06-10, ha-consistency-invariants): user asked if STM could help; analysis concluded full STM is a mismatch (commit point = majority network ACK up to seconds — STM transactions can't span syscalls; libitm/TSX dead in C; single-writer + RTTI per-object locks already cover the concurrency). Adopted the useful subset instead: "committed-view + undo log" (ARIES/MVCC-lite) — entities tagged with WAL LSN, committed-only egress filter for dataex/scheduler, per-in-flight undo records for prompt rollback on abort. Dual-zarray COW checkpoint mechanism is the in-house precedent for maintaining a second consistent view.
- Provisional-state model for Req 2.4 (2026-06-10, user decision, ha-consistency-invariants): in-memory controller state is a PROVISIONAL materialized view; the persisted majority-committed WAL (+snapshot) is the authoritative cluster state. Chosen over hook inversion (apply-after-commit). Mutation order stays mutate-first; instead, commit gates (1) requester confirmation and (2) irrevocable-action egress — dataex/scheduler export must filter uncommitted entities (closes the dispatch-of-uncommitted-job race). Status queries MAY observe provisional state (documented deviation; read consistency out of scope). Consequence: §C convergence/reconciliation work (truncate-on-demotion, phantom eviction, persisted committed_lsn) is mandatory core, not optional — "provisional" is only true if something discards it.
- Raft + CP adopted as governing HA model (2026-06-10, user decision, supersedes "No Raft" in product.md): consistency requirements are DEDUCED from the Raft paper + CP, not chosen per-feature. Concretely: (1) no CTRL_PRIMARY_COMMITTING state — Raft has only Follower/Candidate/Leader; new-leader safety comes from the own-term commit rule (§5.4.2), not a fourth state; (2) majority ACK is the hard commit barrier for ALL state-changing writes — reject on timeout/quorum loss, never local-apply-and-degrade (committed_lsn ≈ commitIndex); (3) replication fans out to all standbys with leader-driven continuous catch-up; majority is the commit threshold, not the fan-out target. Single-writer log retained as XCE's simplification. Spec: .kiro/specs/ha-consistency-invariants/brief.md. Implementation ordering constraint: universal commit barrier MUST land before removing COMMITTING.
- Replication payload wire format (2026-06-09, ha-repl-hardening 7.1): chose the established `_raw` sender convention (manual capnp build via capn_new_list8/capn_setv8 + xce_decode per-opcode raw extraction into raw_payload/raw_payload_len struct fields) over extending the typemap codegen to map capnp Data ↔ raw buffers. Rationale: replConfigPush/upgradeConfig already prove the pattern in production; the codegen route risks regressions across all opcodes and fights the known capnpc-c 1.0.1 codecgen bugs. Schema change: Request.body union gained walcatchupentrymsg @28 / snapshottransfermsg @29 — these messages previously had NO union arm at all (sent as body_which=nomsg, i.e. nothing crossed the wire).
- Convergence threshold (2026-06-09, ha-repl-hardening 7.5): chose requirements.md Req 2.3 formula — ceil(n_peers/2)−1 converged standbys (integer form n_peers/2) — over the code's previous all-standbys rule. Rationale: primary + converged standbys form a cluster majority, matching Raft quorum; committed entries are already on a majority per-entry, and safe_committed_lsn derives from a majority of voters, so promotion with one dead minority standby is safe and avoids a 60s degraded-mode timeout. Lagging standbys keep catching up via the heartbeat decision loop after promotion.
- Election liveness fix (2026-06-09): `skip_election` path in myapp.c had two bugs: (1) `voted_for_name != ""` triggered even for self-votes and stale terms — fixed by adding `strcmp(name,"self")!=0 && voted_for_term==ha->term`; (2) `ha_arm_heartbeat_timeout_vtask` was never called in skip_election path — fixed by adding arm call after role=CTRL_STANDBY. raft.c Step 4 simplified to term-only deny (same-candidate idempotent re-grant preserved). ha.c `handle_request_vote_req` also simplified to term-only deny.
- CORRECTION (2026-06-09 validation pass): the ha.c "term-only deny" part was WRONG — it denied same-candidate re-grants, diverging from raft.c and failing tests. Reverted to deny-different-candidate-only (see Do-Not-Repeat). The myapp.c "self" check was also incomplete — production self-votes are persisted under the node's real name, so the check now also compares myconf->name.
- Task 4.1 (ha_elect_evaluate_and_signal COMMITTING path): Added cluster_t *c field to ha_elect_ctx_t so evaluate can call cluster_init_phase2 without signature change. election_timeout_ctx_t and committing_timeout_ctx_t must be defined BEFORE ha_elect_evaluate_and_signal. cluster_init_phase2 wrapped in #ifndef XCE_TESTING to avoid linker error (cluster.c not in unit-test binary). ha_phase2_hook seam used in XCE_TESTING builds. On PRIMARY: cancel election timeout watchdog, call phase2 if first_boot, set CTRL_PRIMARY_COMMITTING, reset repl_ack_count=0, arm committing_timeout_fn. On STANDBY: call phase2 if first_boot, set CTRL_STANDBY.
- (2026-06-11, ha-convergence-recovery spec) `repl_complete_catchup` (repl.c:521) has NO production caller — `in_catch_up` is set by broadcast-overflow eviction (repl.c:240) and never cleared; re-inclusion trigger is wired by the ACK-driven catch-up continuation.
- (2026-06-11) Scheduler-bound job egress has TWO seams, not one: full push `xcesched_push_jobs` (sched.c:169, on scheduler join) AND incremental JOB_NEW push `sched_job_new_fun`→`xcesched_push_joblist` (sched.c:236-248). Status reads (apiserver.c:83, opget.c:173) share `collect_jobs` (dataex.c:294) and must stay unfiltered per umbrella deviation.
- (2026-06-11) No saved-argv/execv machinery exists anywhere in src/controller or src/lib — demotion self-restart is net-new; `job_t` is a thin wrapper `{ xce_job_t job; }` at src/include/xce/types.h:483-485 (controller-local fields go on the wrapper, wire type untouched).
- (2026-06-16, ha-prevote cleanup) The HA election carrier `ha_elect_ctx_t` (`ha->elect_ctx`) is deliberately leaked one-per-round on BOTH the real-election and PreVote paths — vote endpoints hold a bare `vctx->ctx` pointer freed async by endpoint_fini, so freeing the carrier risks UAF; there is NO real `ha_election_cleanup()`. Pre-existing + bounded (rare failover events). Deferred by user decision to follow-up spec `.kiro/specs/ha-elect-ctx-lifecycle/` (refcount-the-carrier or teardown-ordered free, both paths). Do NOT attempt a drive-by free — UAF on failover is worse than the bounded leak. The ha-prevote 3.1 log-reason nit is RESOLVED (commit 47f7338): `ha_heartbeat_is_fresh` un-static'd + declared in ha.h; cluster.c denial-reason now mirrors `raft_cb_vote_eligible` order exactly.
- (2026-06-17, bug-646 client-hang fix) User rejected adding a client reply-wait timeout (`cbuf_timedget`) as a fix for the failover hang: "a timeout may hide potential issues and make things more complex." Principle adopted: **fail loud at the source, never mask with a backstop timeout.** The hang was a netio silent-swallow (`endpoint_update_epoll` returned 0 on `iomgr_update_poll`/EPOLL_CTL_MOD failure), so the fix surfaces the error and runs the error path (wake the waiter via handle_disconnect → failover rotates), rather than papering over the missing reply with a deadline. When choosing between "add a timeout" and "propagate the real error," prefer propagation on this codebase.
- (2026-06-16, ha-prevote impl) The controller `"controller test"` 300s TIMEOUT under `meson test` is a WALL-CLOCK WRAPPER ARTIFACT, not a hang/failure: running the binary directly (`ASAN_OPTIONS=detect_leaks=0 ./xce.build/controller/controller_unit_test`) completes all 564 tests incl. the slow `test_commit_barrier_reply` (61 tests, real 500ms commit barriers) with 0 failures. For a true controller pass/fail, run the binary directly (no cap) or filter with `-t <suite>`; do NOT treat the meson timeout as a regression. ha-prevote feature COMPLETE: 11 tasks, raft suite 123/0, controller 564/0, real-vote path byte-for-byte unchanged (0 raft.c deletions), commits 0d66026..e6b234a.
- (2026-06-15, ha-prevote design review) PRODUCTION vote-reply tally path = `raft_on_vote_reply` in the raft library, NOT `ha_elect_ctx_t`. `vote_reply_dispatch` (ha.c ~2630-2650) calls `raft_on_vote_reply` directly in production; the `ha_elect_ctx_t` / `handle_request_vote_reply` / `ha_elect_evaluate_and_signal` path runs ONLY behind `#ifdef XCE_TESTING` via `xce_vote_reply_hook` (plus the startup/first-boot `ha_elect_ctx_t.done` semaphore). The raft library owns quorum tracking and fires `on_become_leader`/`on_become_follower`. Do-Not-Repeat: when mirroring the election reply path, mirror `raft_on_vote_reply` (raft lib), not `ha_elect_ctx_t`. (An earlier exploration mislabeled ha_elect_ctx_t as the operative tally; caught in design validation.)
- (2026-06-15, user decision, ha-prevote RN-2) Leader-stickiness window for PreVote = reuse `ha_heartbeat_is_fresh()` / `heartbeat_timeout_ms` (10s), gated on role==CTRL_STANDBY. Rationale: the stickiness window MUST equal XCE's failure-detection window or you get either a disruption gap (window too short) or a liveness stall (window too long). XCE splits textbook Raft's overloaded "election timeout" — `HEARTBEAT_TIMEOUT_MS` (10s) is the leader-failure detector; `ELECTION_TIMEOUT_MS` (5s) is only the candidate vote-collection deadline. Raft's "minimum election timeout" disruption rule maps to HEARTBEAT_TIMEOUT_MS, NOT ELECTION_TIMEOUT_MS. Reusing it keeps stickiness == failure-detect == existing real-vote gate (`raft_cb_vote_eligible`), one predicate; role-gate handles RDHUP/connection-loss for free. A shorter window buys no failover speedup (nobody campaigns before failure-detect anyway; real crashes RDHUP immediately) and only opens a disruption gap on flaky links. Rejected a separate `prevote_stickiness_ms` constant (lets the two timeouts drift). Full comparison table in .kiro/specs/ha-prevote/research.md §6.
- (2026-06-15, user decision, ha-prevote) Product is PRE-RELEASE: no backward/rolling-upgrade compatibility is required for any HA/wire change. All controllers in a cluster deploy from the same build, so there is no mixed-version (old-binary ↔ new-binary) cluster. Wire-format changes (new capnp opcodes/fields, vote-RPC changes) may be made freely without negotiation/fallback paths. Do NOT raise inter-version compat as a requirement or research item. (Dropped RN-5 from ha-prevote/research.md on this basis.)
- commit_submit(COMMIT_MODE_RETAIN) is REGISTRATION-FREE (commit.c:233-239, verified 2026-06-11): returns 0 immediately, records NOTHING per entry — commit state derives entirely from registry watermarks at ACK time. RETAIN routing at the hook seam therefore has NO engine-side observable distinct from the append itself; tests of RETAIN routing must assert classification + non-blocking return + lsn > committed_lsn pending + ACK-driven advancement, never a fake "registered" counter. (ha-commit-barrier-rollout 1.1)
- XCE_TESTING fail-fast (assert/abort) paths are testable via fork(): child silences stderr, calls the guarded fn, _exit(0); parent asserts WIFSIGNALED && WTERMSIG == SIGABRT. Precedent: lib testsuite test.c:1783; ASAN-compatible with detect_leaks=0. (ha-commit-barrier-rollout 1.1)
- ha_has_write_quorum (ha.c:1507) is STATIC — testsuite reaches it only through ha_dispatch_gate(ha, opcode) (extern-declared in test files, pattern: test_ha_repl_ack.c). Gate fixtures: ha_init + n_peers + atomic role store + heap zarray standby_endpoints (test_ha_connection_reuse.c pattern). Catch-up exclusion at the gate is untestable until the ha_state_t.repl back-pointer lands (ha-commit-engine 4.2) — assert via repl_active_standby_count instead. (2026-06-11, task 4.1)
- persist_commit_barrier watermark = current_lsn - 1 (matching persist.c:1021 persist_wal_sync_append read); covered ⇔ current_lsn == 0 OR committed_lsn >= current_lsn - 1. Design §Commit Barrier Helper "lsn = p->current_lsn" is shorthand — the parenthetical persist.c:1019-1021 reference pins it to current_lsn - 1 (engine clamps committed to highest appended, so waiting on current_lsn itself would never complete). (2026-06-12, task 2.1)
- (2026-06-13, single-leader-role 4.2) ha_dispatch_gate's OUTER `default: return 0` ADMITS — so after deleting the `case CTRL_PRIMARY_COMMITTING:` block, a role manually set to COMMITTING falls through to admit (not NOT_READY). That is exactly why test_ha_startup.c:test_dispatch_gate_committing_denies_job_ops flips from PASS→FAIL (Expected -10013 Was 0) — category-(a), 5.1-owned. The 6 PRIMARY||COMMITTING compound sites in ha.c: raft_cb_vote_eligible(~215), handle_request_vote term-stepdown(~1984)+role-deny(~2004), peer_ctrl_msg_handler case label(~2820), vote_ep_err_handler(~3088), primary_heartbeat_fn re-arm(~3883). Collapsing all to PRIMARY-only makes a manually-COMMITTING test role a NON-leader. Census after 4.2: exactly 5 FAIL, ALL COMMITTING-state category-(a) (test_ha_startup ×2, test_ha_rdhup_integration, test_ha_repl_push, test_ha_connection_reuse), zero category-(b). enum value CTRL_PRIMARY_COMMITTING=4 stays in ha.h (5.2 deletes it); no ha.c CODE branches on it post-4.2 (only comments in 2.2/3.1-owned functions remain). Unity attributes vote-gate cases to test_ha_failover.c by group UNITY_BEGIN — locate by NAME. FAIL bytes: `name:\x1b[41mFAIL\x1b[0m` (color BETWEEN ':' and FAIL) — grep raw `FAIL`, count was exactly 5.
### libxce async failover (2026-06-16)
- `xce_request(ep,req)` is fire-and-forget enqueue; reply status is only available at the `cbuf_get` round trip in xceapi.c. `xce_request_with_retry()` production branch is a no-op stub (retries only under the test hook). Any client-side retry/failover MUST wrap the send+wait round trip at the `api_ctx_t` level, not `xce_request`. Controller redirect (`E_XCE_REDIRECT`, join.c) sends status-only; redirect_host/port schema fields exist (controller.capnp:568) but are never populated. See spec libxce-failover, bug-608.
- xceapi.c per-op guard rejects ctx->state != XCE_CTX_RUN with -EINVAL; the transport-error path (xcectx.c:148) drives ctx to XCE_CTX_ERR -> this is the exact source of the failover "Invalid argument" symptom. Any failover must restore XCE_CTX_RUN after reconnect.
- ctx->wbuf is a single-slot status-only channel (cbuf_init ...,1,sizeof(int), xcectx.c:185): no seq correlation, so client is single-in-flight per ctx. Retries must drain/seq-match to avoid consuming a stale reply.
- Job submit uses a CLIENT-generated jobid (job.c:56) + server EEXIST/E_XCE_DUP_JOB replay guard (job.c:95) -> writes are idempotent on re-send; DUP on a retried submit means success.
- libxce layering: xcecli_* (client.c) is the UNIVERSAL client surface — worker (controller_connector.c), scheduler (scheduler.c), apiserver cgo (clib.go), xce_test all call xcecli_*, never xceapi xce_* directly. Only api_ctx_t (client.c) holds the master list; xce_ctx_t (xceapi/xcectx.h) does not. => client-side cross-cutting concerns (failover/retry) belong in client.c wrapping xce_*, keeping xce_* one-shot/fail-fast and xceapi unchanged.
### Decision: full-state rejoin on controller failover (2026-06-16)
- On failover, stateful daemons re-join the new primary and apiserver/scheduler receive FULL state (full job list), not a delta. Accepted because failover is rare, some handshake is unavoidable (new primary has no live connection to the daemon until rejoin), and full-state-on-rejoin is simpler/correct and still better than IBM LSF (full-history replace, no snapshot). Lightweight re-attach / delta transfer deferred (YAGNI) — roadmap backlog. Revisit only if measured as a scale problem.
## Session 2026-06-17 (bug-661 reproduction) — learnings
### Do-Not-Repeat
- `pkill -9 -f xce_controller` inside an INLINE Bash one-liner kills its OWN shell — the inline command's cmdline contains the literal "xce_controller". Use `pkill -9 -x xce_controller` (exact comm match). Scripts run as a file are safe (cmdline = `bash script.sh ...`).
- Daemons spawned with `start.sh ... &` inside a Bash TOOL call get reaped when the call returns. Run multi-process HA E2E via the self-managing script with `run_in_background: true`; write logs to the WORKTREE (e.g. ./dur-run.log), NOT /tmp (the sandbox tears /tmp files down between calls).
- The Bash tool has a ~120s wall limit; long foreground HA runs get SIGKILLed and buffered stdout is lost. Use discrete `>>` file appends (each echo flushes) or background+poll.
### Key Learnings
- bug-661 (committed-job loss on failover) is a RACE: `wal_sync_ms` is hardcoded `DEFAULT_WAL_SYNC_MS=10` (config.h:86), NO CLI/env override. Loss needs the ~10ms un-fsync'd window hit AND a killed+restarted node re-leading before replication catch-up repairs its WAL. Single-node kill+restart soak passed 8 rounds clean; reproduced loss once (4 jobs). Deterministic repro needs a configurable fsync interval (test seam for ha-commit-durability).
- Cold-start 3-node election on /tmp/xce-ha-deploy takes ~15s to first CTRL_PRIMARY.
- xce_test reads piped stdin via readline and exits on EOF — drive it with `printf 'set masters ..\nclient\njobsub ..\n' | xce_test`.
- (2026-06-18, task 1.3 FINAL) bug-661 deterministic repro = `bash src/tools/test-commit-durability.sh --deterministic` (strace-based GAP proof, NOT joblist data-loss). WHY NOT data-loss: single-host SIGKILL is physically NON-deterministic — `persist_wal_async` pwrites INLINE (page cache survives process death; kernel still flushes), and standbys fdatasync-before-WalAck (persist.c:515) so a committed job is always durable off the primary. A rolling-primary-kill data-loss run flips GREEN/RED/RED across identical invocations (REJECTED by review). The DETERMINISTIC slice: with `--wal-sync-ms 30000` the primary's fsync_worker never fires in-window, so the primary replies "jobsubmit ok" with ZERO primary fdatasync between submit and ack. The mode straces each controller (`strace -f -tt -e trace=fdatasync,fsync`, exec-wrap — PTRACE_ATTACH `-p` is blocked by yama, exec-wrap works) and asserts in-window primary syncs==0 ⇒ RED. CONFIRMED 15/15 RED across 5 fresh-cluster runs; asymmetry primary=0 vs standby=2 in-window. Valid RED→GREEN gate: the fsync-before-ack fix (task 2.x) makes the count ≥1 ⇒ GREEN. The exact defect: commit.c:79 `majority_candidate_locked` caps the primary self-contribution at `highest_appended_lsn` (page cache), not a durable_lsn. `--wal-sync-ms` plumbed via deploy-ha.sh start.sh `$*` passthrough.
- (2026-06-18, GOTCHA) A SINGLE double-kill (primary + 1 standby) does NOT reproduce bug-661: this codebase enforces STRICT majority — a lone survivor will NOT self-elect (harness log: "no single-voter primary within 40s"). The documented `required=1` loss came from ROLLING failover across terms (incident: ctrl-3 became PRIMARY term=4). Also, committed jobs are durable on the STANDBYS (persist_wal_write_replicated fdatasyncs before WalAck, persist.c:515) — only the PRIMARY's copy is un-fsync'd. So the loss needs rolling hollow re-leads faster than catch-up repair, not just one kill. The deterministic harness drives rolling primary kills to hit this.
- (2026-06-18, BUILD) `meson`/`ninja` are NOT on PATH in a fresh sandbox shell — they live in the nix store. Build a single target into the EXISTING `src/xce.build` (no clean) with `cd src && nix develop --command bash -c 'meson compile -C xce.build <target>'`. `xce-build.sh`'s setup_build does `rm -rf` (clean) — avoid it for incremental. The controller/xce_test binaries also need the nix runtime libs (openssl etc.), so run the durability harness itself under `nix develop ./src --command bash -c '...'`.
## Decision Log — 2026-06-17: ha-commit-durability mechanism
- INVARIANT (user, authoritative): reply "ok" to client ⇒ WAL entry fdatasync'd on a fault-tolerant majority (self + ⌊N/2⌋ standbys).
- DEFECT: commit.c:majority_candidate_locked counts self as highest_appended_lsn (page-cache), not fsync'd. Standby already persists-before-ACK (persist.c:515); primary does not (relies on fsync_worker timer) → "committed" durable on only 1 node = zero durability fault-tolerance = bug-661.
- DECISION: synchronous persistence — eliminate fsync_worker as the durability mechanism. Primary fdatasyncs before its self-contribution covers an LSN; commit.c self uses durable_lsn not highest_appended_lsn; reply stays gated on commit barrier. Liveness-safe because the submit path already blocks OFF the iomgr thread waiting for majority replication (job.c:568-569).
- THROUGHPUT: per-op fdatasync; array amplification removed by roadmap item job-array-single-wal-record (array → 1 WAL record → 1 fsync). Group-commit (batched fsync) is a deferred strict optimization (same contract), add only if high-rate independent submits measured as a bottleneck.
## Decision Log — 2026-06-18: ha-commit-durability design-review reversal (group-commit)
- /kiro-validate-design surfaced: highest-volume WAL traffic is WAL_MODE_RETAIN facts (job start/finish/fail/rusage; persist.c wal_classify) — high-frequency, NOT array-collapsible, NOT covered by job-array-single-wal-record, and not client-latency-bound. A pure per-entry synchronous design (delete fsync_worker) would penalize the dominant write path.
- REVERSAL (with user): do group-commit NOW. Repurpose fsync_worker timer→demand-driven flusher: on append-signal, one fdatasync over pending appends, advance durable_lsn, wake commit-eval. commit self-count uses durable_lsn. Roadmap "WAL group-commit batching" item ABSORBED into ha-commit-durability.
- Lesson: when deciding sync-vs-batched durability, classify ALL write modes first (wal_classify), not just the client-acked submit path. The RETAIN fact stream dominates volume.
- Gating items pinned for tasks: (1) freshness-floor formula + raft.tla liveness+safety re-check; (2) FATAL-vs-fence boot decision point; (3) flusher lost-wakeup proof (level-triggered on appended-vs-synced gap).
## Key Learning — 2026-06-18: commit-ordering rule documented
- Authoritative commit/durability invariants now live in src/controller/CLAUDE.md (Safety Invariants + Protocol Verification Checklist):
- Commit Ordering (RULE, true today): committed_lsn advances only as a contiguous LSN-ordered prefix; single-writer p->lock is the serialization point; durability batching flushes a contiguous prefix only (durable_lsn is a prefix watermark, never per-entry); log order must preserve causal deps (submit before start); out-of-order/parallel commit NOT supported (single-log architecture).
- Commit Durability (TARGET RULE) carries a KNOWN GAP note for bug-661/ha-commit-durability: "committed" does NOT yet imply durable-on-majority until the spec lands. Task 5.3 flips the note to a guarantee when the code lands.
- When a safety invariant changes, document it in src/controller/CLAUDE.md (not just the spec) — but mark not-yet-true guarantees as KNOWN GAP so fresh sessions aren't misled into assuming durability that doesn't exist.
## Key Learning — 2026-06-18: build invocation in worktrees + impl preflight
- The flake is at src/flake.nix, NOT the repo root. The xce-build skill's `nix develop --command ...` only works after `cd src`. Correct build: `cd src && nix develop --command bash tools/xce-build.sh controller` (~2-3min, debug+ASAN+tests; nix env is cached, no network). Running from the worktree root fails: "not part of a flake".
- meson/ninja are ONLY inside the nix shell (not on PATH); cc/gcc are on PATH but won't configure the meson tree.
- ha-commit-durability impl preflight (2026-06-18): tasks approved; cleaned baseline by committing 2 pre-existing WIP units (ha-election stagger + libxce netio error propagation); baseline build+controller tests GREEN. Sandbox caveats stand: 120s Bash cap (background+poll builds), daemon spawns reaped (E2E task 5.2 needs host), no confirmed TLA+ toolchain (tasks 4.1/5.3 need host).
## Key Learning — 2026-06-18: raft.tla does NOT model durability
- raft/raft.tla (Ongaro's spec) restores `log` intact from "stable storage" on restart (raft.tla:165-174) — it ASSUMES durable logs, i.e. assumes exactly what bug-661 violates. Running TLC on the unmodified spec PASSES while the durability bug is still present. raft.tla validates election/freshness logic (mlastLogIndex/LastTerm), NOT commit durability.
- The repo has only raft.tla — no .cfg/model/tla2tools.jar/.toolbox/runner; java not on PATH. Project practice = manual conceptual alignment + the CLAUDE.md Protocol Verification Checklist, not automated TLC. So tasks 4.1/5.3 are reasoning tasks (sandbox-doable), not "run TLC" tasks.
- TLA+ Toolbox (bundles TLC) — or tla2tools.jar CLI + Java — is sufficient TOOLING for any model-checking, but proving the DURABILITY invariant would require EXTENDING raft.tla (durableLog vs log; Restart truncates to durableLog). That spec-extension is the real work, not the tool. Tasks 4.1/5.3 now carry this caveat so an implementer won't conclude "TLC passed ⇒ durability proven".
## Key Learning — 2026-06-18: /tmp overlay fdatasync EIO breaks durability tests
- This sandbox's `/tmp` can be an OVERLAY filesystem where `fdatasync` returns EIO (errno=5), intermittently under load (3000+/run observed). Controller unit tests `mkdtemp` their WAL dirs into `/tmp/xce_*`, so their fdatasync hits this.
- CONSEQUENCE after ha-commit-durability 2.1/2.2/2.4: `committed_lsn` is gated on `durable_lsn` which advances ONLY on fdatasync rc==0 (fail-loud). On EIO, `durable_lsn` correctly HOLDS, so `committed_lsn` cannot advance — this is the gate WORKING AS DESIGNED, but it makes `test_fact_hook_append_is_retained_and_nonblocking` (test_commit_barrier_rollout.c) FAIL on overlay /tmp ("covering ACK must advance committed_lsn, Expected 1 Was 0"). The SAME test FAILS on the clean pre-spec baseline too on overlay /tmp — NOT a regression from any single task.
- Prior task reviews reported "0 Failures" because they ran on a tmpfs /tmp or a quiet-IO window (EIO is intermittent). Don't trust a single "0 Failures" run on this fs.
- ACTION for final validation (tasks 5.x / /kiro-validate-impl): run the controller suite on a REAL fsync-able filesystem (tmpfs `/dev/shm`, or point test temp dirs at the worktree) so durability tests aren't masked by spurious EIO. The worktree `xce.build` is on a real fs (build is fine); it's the TEST WAL dirs in /tmp that are the problem. Always `rm -rf /tmp/xce_*` before a run (stale dirs also cause an "Expected Non-NULL" mkdtemp cascade).
## Key Learning — 2026-06-18: meson "Fail:0" masks Unity sub-group failures
- The controller suite is ONE meson test ("controller test") running the aggregated `controller_unit_test` binary. A Unity `TEST_ASSERT_*` failure inside a sub-group (e.g. test_fact_hook_append) is recorded + printed `:FAIL:` in testlog.txt but is NOT always propagated to the process exit code, so `meson test` can report `Ok:1 Fail:0` while a test actually FAILED. A C `assert()` (SIGABRT) DOES propagate (so RED-phase asserts are reliable).
- RELIABLE GREEN GATE: `grep -c ':FAIL:' src/xce.build/meson-logs/testlog.txt` must be 0 — do NOT trust the meson `Fail:0` summary alone.
- Combined with the /tmp-overlay-fdatasync-EIO issue, the documented pre-existing `test_fact_hook_append_is_retained_and_nonblocking` (commit_barrier_rollout) intermittently `:FAIL:`s on overlay /tmp (EIO holds durable_lsn ⇒ committed can't advance over the retained fact — the durability gate working). Final validation (5.3 / /kiro-validate-impl) MUST run on a real fsync-able fs (worktree/tmpfs) AND grep testlog for `:FAIL:`.
## Decision Log — 2026-06-18: a controller MUST NEVER self-exit (user invariant)
- AUTHORITATIVE (user, 2026-06-18): a controller process exits ONLY on (a) a genuine crash (segfault/OOM) or (b) an admin/supervisor kill. It must NEVER self-`exit()`/`abort()` as a *designed response to an operational condition*. Plain `assert()` for genuine programming-invariant bugs is still OK (that's a "crash").
- CONSEQUENCE: the ha-commit-durability "FATAL → tlog_exit(); abort()" terminal (tasks 3.3/4.3/4.4, `persist_recover_fatal_if_unrecoverable` + `persist_fatal_hook`) was WRONG — it self-terminated on an operational condition (recovery-truncated WAL after failover). This was the root cause of the 5.2 soak quorum collapse (self-aborting nodes), NOT a sandbox artifact (correcting the earlier cerebrum framing).
- CORRECT behavior (user-chosen): an unrecoverable member (truncated, can't catch up) logs LOUDLY (operator-visible ERROR, with the truncation boundary) + stays PERMANENTLY non-promotable/fenced + STAYS ALIVE + KEEPS RETRYING catch-up indefinitely. "Don't serve divergent state" (Req 2.4) = refuse to lead, NEVER kill the process. No bounded-window-to-abort deadline. If a peer returns it catches up and clears the fence; otherwise it waits (loud) for an operator.
- REWORK (in progress): remove abort/exit/`persist_fatal_hook` from 3.3; the decision's FATAL terminal becomes a non-terminating "loud permanent fence"; 4.3/4.4 route to it (stay alive); update Req 2.4 / design §3.2 table / §3.3 wording from "FATAL/terminate" to "loud non-promotable fence, stays alive"; update tests.
- REWORK DONE (2026-06-18, rework-no-self-exit). Concrete shape: (1) enum `RECOVER_DECISION_FATAL`→`RECOVER_DECISION_FENCE_UNRECOVERABLE` (persist.h:61). (2) `persist_recover_fatal_if_unrecoverable(const inputs*)`→`persist_recover_loud_fence_if_unrecoverable(persist_t* p, const inputs*)` (persist.c:1788) — logs LOUD ERROR (boundary), sets sticky flag, RETURNS; NO tlog_exit/abort. `persist_fatal_hook` global DELETED. (3) sticky `_Atomic bool recovery_unrecoverable` on persist_t (persist.h, after recovery_verdict) + `persist_recovery_unrecoverable(p)` accessor + `persist_clear_recovery_unrecoverable(p)` (clears on a genuine transition + logs "fence RELEASED"); init false in persist_init (persist.c:451). (4) ha.c `ha_evaluate_promotion_fence` FENCE_UNRECOVERABLE arm: calls loud-fence with `p = ha->raft_c ? &ha->raft_c->persist : NULL` (pure-decision callers pass NULL) + sets in_catch_up. (5) ha.c `promotion_fence_timeout_fn` no longer aborts: on still-unrecoverable it RE-EMITS loud alert (sets sticky flag) + RE-ARMS the timer (periodic operator re-alert, indefinite); on caught-up clears in_catch_up + sticky flag; UAF gotcha — capture `t->target_hwm` to a local BEFORE `free(t)` then re-arm (the -Wuse-after-free caught it). `ha_arm_promotion_fence_timer` fail path emits loud-fence once (stays alive), no abort. (6) become_leader gate: also refuses while `persist_recovery_unrecoverable(&c->persist)` (belt-and-suspenders), clears flag on ELIGIBLE. (7) Tests: dropped `persist_fatal_hook`/`dur_fatal_hook`/`g_fatal_hook_calls`; rows 4/4b assert decision==FENCE_UNRECOVERABLE + loud-fence helper RETURNS (process stayed alive) + sticky flag SET + clears on `persist_clear_recovery_unrecoverable`; timer tests use `dur_wait_unrecoverable(c,ms)` polling the sticky flag; renamed *_no_fatal/_prevents_fatal/_bounded_fatal/_defers_fatal. VERIFIED: controller suite Fail:0 / `:FAIL:`==0 ASAN ×2; `grep abort(/exit(/tlog_exit(` ha.c=0 everywhere, persist.c only in comments; production xce_controller `nm` has the 4 new T symbols and NO persist_recover_fatal/persist_fatal_hook. NOTE: `ha_self_restart` (Req 1.3/1.4 demotion execv-restart, ha.h:305) is a SEPARATE primitive — its abort-on-exec-fail is OUT OF SCOPE for this rework (it's the convergence/restart path, not the recovery-unrecoverable operational fence) and was left untouched.
### xce_test scripting primitives (xce-test-e2e-primitives)
- `$jobid`/`$jobcount`/`$?` auto-vars substitute ONLY as a WHOLE token (preceded by line-start/whitespace). `MARK=$jobid` is NOT expanded; use a space: `echo MARK $jobid`. (test.c substitute_autovars, ~line 462)
- jobsub on a clean ack sets `$jobid` to the `fmt_jobid` form (non-array `N-N-N-N`, array `N-N-N-N[i]`). To capture it in a shell driver, `echo MARK $jobid` then parse the marker line — do NOT re-impose a numeric id regex (drops array ids).
- In a shell driver, `script+=$(printf '...\n')` STRIPS the trailing newline (command-subst), concatenating lines into one bad token. Accumulate with explicit $'\n' instead.
- xce_test exit code is the verdict; run with ASAN_OPTIONS=detect_leaks=0 (known client leak) or the exit code is untrustworthy. stderr (errors/sanitizer) does not corrupt stdout capture.
## Key Learning (2026-06-21) — WAL entry term is dead (bug-851 keystone)
`wal_entry_hdr_t.term` (wal_types.h:30) is real but the writer does `hdr.term = p->term` (persist.c:588/726/1040) and **`p->term` is NEVER assigned from `ha->term`** anywhere in the controller — so every WAL entry persists `term=0`. waldump confirms term=0 cluster-wide. This makes Raft (term,lsn) log-matching impossible, so reconcile can't truncate divergent same-LSN tails. Under CHAOTIC failover (kill-the-primary repeatedly), a restarted node recovers a stale term and re-elects at the SAME term (PreVote does not persist/propagate the won term to a majority before the leader acts) → two leaders per term → silent committed-prefix divergence. A CLEAN single failover converges fine (848/849/850 validated). Fixing this is HA-core + persistence = full spec workflow, not a quick patch.
## Decision Log (2026-06-21) — ha-term-safety: unify config + job state in one log, config bytes out-of-band
For the ha-term-safety spec (bug-851 fix), the user chose a SIMPLE, UNIFORM consistency model: configuration changes and job state share ONE replicated (term,lsn) log. A config change is a LIGHTWEIGHT in-log MARKER carrying only a reference (version_tag + sha256), never the bytes. The real config bytes stay OUT of the WAL records and OUT of the cluster (state) snapshot — transferred/stored out-of-band. Design direction: model the config artifact LIKE A SNAPSHOT for replication (chunked, (term,lsn)-anchored, hash-verified, install-before-activate; reuse snapshot_transfer_s pattern), but it is an admin-supplied INPUT, not compacted log state. The marker (not pure snapshot) is mandatory because it gives config the shared (term,lsn) reconcile path; a snapshot anchored at an lsn with no log entry would recreate a separate config reconcile domain (= today's latent config divergence). committed_config_tag becomes DERIVED from the latest committed marker (single commit index). Controller voting-set (peer add/remove) changes are out of scope here (quorum transition → ha-config-bootstrap). Captured as Requirement 7 in .kiro/specs/ha-term-safety/requirements.md.
## Key Learning (2026-06-21) — raft.c is ELECTION-ONLY; log layer is separate & not TLA+-conformant
src/lib/raft/raft.c implements ONLY the election half of raft/raft.tla (Ongaro canonical): currentTerm, votedFor, RequestVote, BecomeLeader, PreVote, UpdateTerm-on-vote-RPCs. It has NO log, NO AppendEntries, NO AdvanceCommitIndex, NO commitIndex. The replicated log = the WAL (persist.c); "AppendEntries" = catch-up streaming/repl.c; "AdvanceCommitIndex" = the commit barrier; conflict reconcile = ha_convergence.c — and NONE of them follow the TLA+ log protocol. Gaps (see .kiro/specs/ha-term-safety/TLA-gap-analysis.md): G1 entry term always 0; G2 replication matches LSN contiguity not prevLogTerm; G3 reconcile truncates by committed-boundary not (term,lsn) conflict; G4 commit is term-blind (no Figure-8 current-term check; own-term noop is partial); G5 standby never adopts leader term from heartbeat/replication (only vote RPCs) → term reuse across restart → two leaders/term; G6 vote freshness is LSN-only not (lastLogTerm,lastLogIndex). G5 confirmed live: rejoined ctrl-1 reported term:2 under a term-3 primary. These compose into bug-851. Config is a THIRD domain entirely outside both (committed_config_tag, no term).
## Decision Log (2026-06-21) — make src/lib/raft strictly implement raft.tla (library owns the log)
User chose to EXTEND the raft library to strictly follow raft/raft.tla, using the verified spec as a conformance oracle, instead of patching the controller's hand-rolled log protocol. Scaffolded via roadmap + /kiro-spec-batch as the "Raft TLA+ Conformance" series (3 specs, all generated+auto-approved 2026-06-21, cross-spec review PASSED):
- raft-log-replication (8 reqs/24 tasks): library owns log; per-entry term (G1), AppendEntries prevLogTerm match (G2), (term,index) conflict truncation (G3), UpdateTerm on EVERY RPC (G5). New callbacks: on_persist_entry, on_get_entry_term, on_send_append_entries, on_truncate_from. Entry pts: raft_log_append, raft_on_append_entries_request/reply, raft_send_append_entries. State: log_len, next_index[], match_index[]. Index 1-based = WAL LSN.
- raft-commit-safety (6 reqs/18 tasks): AdvanceCommitIndex Figure-8 current-term rule (G4); (lastLogTerm,lastLogIndex) freshness (G6). New: raft_advance_commit, on_commit_advanced(commit_index) [TRIGGERS the existing barrier, never replaces], commit_index state. raft_on_vote_request/raft_on_pre_vote_request gain (cand_last_log_term, cand_last_log_index); library now owns logOk (locus moved from caller/dispatcher → library; supersedes raft.h:246-248 doc).
- raft-sim-conformance (8 reqs/23 tasks): extend raft_sim.c into in-process oracle driving raft.tla Next-actions, asserting the 4 safety invariants; negative test via compile-time RAFT_SIM_BREAK_* switches (test build only). Would have caught bug-851.
Pure library core (no I/O/threads) + controller callback adapters keeps it simulator-verifiable. Preserve as policy: PreVote, priority leadership, sync commit-barrier + persist-before-ACK durability, controller-never-self-terminates. INTENTIONAL safe deviation from raft.tla: conflict branch does truncate-THEN-append in one handler (raft.tla is truncate-only) — documented. ha-term-safety reframed = controller wiring + config R7/R8 + live regression; its R1-R6 realized by the library specs. Implement order: raft-library(done) → log-replication → commit-safety → sim-conformance, then ha-term-safety.
## Key Learnings (raft-log-replication, 2026-06-21)
- Library build/test: `cd src && nix develop --command bash tools/xce-build.sh lib` does a CLEAN ASAN build (rm -rf + meson setup). For fast TDD inner loop reuse the dir: `nix develop --command bash -c 'meson compile -C xce.build xcelib_test && meson test -C xce.build "libxce test" --print-errorlogs'`. test binary: `./xce.build/lib/xcelib_test` (Unity, runs all). raft tests live in testsuite/test_raft.c + raft_sim.c, registered in lib/meson.build.
- `src/lib/raft` is a PURE library: stores no entry bytes, no commit index, no transport. All side effects via callbacks. Index is 1-based == WAL LSN.
- Simulator gotcha: buffered AE RPCs must capture the leader's term at SEND time (raft_sim_ae_t.leader_term), NOT re-read live state.term at delivery — else reorder/stale-churn tests inject the wrong term and defeat UpdateTerm/stale-drop (G5).
- Adapter invariant: never deliver entry_term==0 with entries_len==1 (already-have branch would false-match; correct leader stamps term>=1).
## Key Learnings (ha-term-safety task 2.4, 2026-06-22)
- The controller commit barrier (commit.c) is SELF-DERIVING: `majority_candidate_locked` computes committed_lsn autonomously from registry watermarks (median) capped at `persist_durable_lsn`. The own-term `writable_lsn` gate is ONLY a WAIT-completion predicate, NOT a cap on committed_lsn (so it can't deadlock a commit source).
- Task 2.4 makes the raft library's current-term-gated `commit_index` the COMMIT SOURCE via a NEW `commit_engine_t.raft_commit_index` ceiling + `commit_set_raft_commit_index()` setter; `on_commit_advanced` callback (ha.c) feeds it into `&ha->raft_c->commit`. committed_lsn = min(durable-majority, raft_commit_index). Own-term gate RETAINED (design lines 193/201/228) as complementary durability gate.
- FEATURE GATE: raft_commit_index==0 means INERT (library source not yet wired by tasks 3.x) → barrier keeps autonomous durable-majority behavior. This is what stops 2.4 from regressing the whole test_commit_engine suite (which sets no raft index). The ceiling only engages at commit_index>=1.
- Unit-test runner: `./xce.build/controller/controller_unit_test -t <group> [-t <group2>]` runs only named groups (argp `--run`/`-t`). Far faster than the full binary which times out (bug-924). Full clean build still `nix develop --command bash tools/xce-build.sh controller`.
## Key Learning (2026-06-24, bug-1060) — a new leader MUST NEVER truncate its own log (Raft §5.4 Leader Completeness)
- The post-election `safe_committed_lsn` (median of granted-voter LSNs) is a valid commit LOWER-BOUND and the promotion-gate HWM, but it is NOT a safe WAL-truncation cutoff: it can sit BELOW entries the new leader itself holds durably. Those entries were acked by this node as a standby ⇒ committed by the prior leader. `raft_cb_become_leader` calling `persist_truncate_wal_above(safe_lsn)` destroyed them (granted `{self=47, peer=44}` → median 44 cut the leader's own committed 45..47), and `persist_truncate_wal_above` ALSO sets `committed_lsn=cutoff` (a commitIndex DECREASE) + clamps durable down. The leadership NOP then landed at LSN 45 (the rewound tip) under term+1, diverging vs peers still holding the old job at 45 → contradictory freshness denials (one node fresher by index, the other by term) → permanent prevote deadlock + committed-job loss.
- FIX shape: clamp the truncation cutoff to `max(safe_lsn, persist_durable_floor)` via pure helper `ha_promotion_truncate_floor` so truncate is a no-op on the leader's durable log. Do NOT clamp `safe_lsn` itself inside `ha_compute_safe_committed_lsn` — `promotion_safe_lsn` is also the behind-node fence HWM; inflating it to self's tip would make every node trivially pass its own gate. The deadlock was a SYMPTOM; the freshness gate is correct (it refused to elect a leader that would lose committed data). Same class as bug-795/bug-828 ("new leader truncates own committed entries"); bug-828 fixed grant-RECORDING so the median reflects the majority, bug-1060 fixes the remaining case where the median is below the leader's OWN tip.
- `persist_truncate_wal_above(cutoff)` is a NO-OP when `cutoff+1 >= current_lsn` (returns early, does NOT touch committed_lsn). Only when it actually cuts does it set committed_lsn=cutoff + clamp durable. So clamping the cutoff to >= durable tip makes it inert on a recovered leader (current_lsn == durable_tip+1).
## Key Learning (2026-06-24, bug-1067) — AppendEntries has TWO terms; never conflate them
- Raft's AppendEntries carries (a) the HEADER/leader term — `term` arg of raft_on_append_entries_request, used for UpdateTerm + the stale-reject `if term < s->term → ae_reject_stale` — and (b) each ENTRY's STORED term (`entry_term` arg), used for logOk + on_persist_entry. The XCE library `raft_on_append_entries_request` already takes them as SEPARATE args; the controller adapter `ha_consume_append_entries` was setting `ae_term = entry_term` (the stored term). Correct for a LIVE append (leader appends at its current term ⇒ equal), but for CATCH-UP of a HISTORICAL entry written by a PRIOR leader, entry_term < current leader term ⇒ stale-reject of every historical entry ⇒ cross-term catch-up wedges forever (standby stuck, streamer retransmits). FIX: carry the leader's current term out-of-band per entry (`wal_catchup_entry_t.leader_current_term`, wire `WalCatchupEntry.leaderCurrentTerm @8`), stamped by `ha_stream_wal_to_standby` = ha->term, used as ae_term with fallback to entry_term when 0.
- DO NOT try to fix this receiver-side by using `s->term` as the AE term: a genuinely STALE leader (leader term < s->term) could then push divergent entries through (the stale-check is the ONLY thing rejecting it). The leader's current term MUST travel on the wire and be checked. Test both: cross-term ACCEPT + stale-leader REJECT.
- `wal_catchup_entry_t` is HAND-DEFINED in src/include/typemap.h, NOT generated — but `capnp compile` GENERATES encode/decode (controller.capnp.c) AGAINST it via `$C.mapname`. So adding a schema field requires editing BOTH controller.capnp (wire) AND typemap.h (the C struct) or the build fails "no member". All scalar mapname fields auto-decode (request.c xce_decode only hand-extracts the Data payload); the leader term rides on each sub-entry and reaches ha_consume via ae.entries[0]=e by reference (no extra plumbing through ha_apply_term_entry_via_library).
- `ha_stream_wal_to_standby` has MANY callers incl. several in test_config_repl.c (2589/2630/2664/2740) and test_coldstart_lsn0.c (339/387) — grep ALL of them before changing the signature, not just the production caller.
## Do-Not-Repeat (2026-06-21)
- Don't trust a green "baseline" reading blindly: the lib suite was actually red on HEAD via a stale test_config.c assertion (DEFAULT_SYNC_TIMEOUT_MS 50 vs committed 500 from commit ca1d9ae). Verify with the exact meson test command, not assumptions.
## Do-Not-Repeat (2026-06-22, task 2.4)
- When the commit source is a feature-gated ceiling that is INERT at 0, a test that ACKs a durable majority BEFORE feeding the ceiling will commit autonomously to the durable floor, and the ceiling (monotonic) can never regress committed below it. Establish the raft_commit_index/on_commit_advanced ceiling FIRST (production order: the ACK path drives raft_advance_commit which fires on_commit_advanced), THEN drive the ACK. Initial ACK-then-ceiling ordering cost 3 test failures.
- Do NOT make `raft_commit_index` an unconditional ceiling: capping at 0 when unset breaks every autonomous-barrier test (test_commit_engine: 25/38 failed). Gate it with `!= 0`.
## Key Learnings (appended 2026-06-22, ha-term-safety 4.3 r2)
- Config MARKER (WAL stream) and config BYTES (out-of-band OPCode_replConfigPush) arrive on INDEPENDENT transports. config_committed_marker only returns a marker once it is in the committed prefix (LSN <= committed_lsn); an uncommitted marker is ignored. This creates a push-before-commit window where saved-but-unverified bytes mismatch the older committed marker.
- conf_save_config bumps myconf->config_version_tag (the advertised tag) WITHOUT decoding/activating (cluster_config unchanged). Only load_primary_config decodes into mc->cluster_config. So the activation witness in tests is the cluster_config POINTER (load_primary_config frees+reallocates it), NOT config_version_tag.
- In the single-member commit fixture (commit_engine_init n_peers=1), majority_candidate_locked returns persist_durable_lsn => committed_lsn auto-advances to the durable WAL tip on any commit re-eval (incl. conf_save_config). To hold a flushed marker "uncommitted" in a test, SAVE first then atomic_store committed_lsn back below the marker LSN (models a standby whose committed prefix lags its local WAL tip).
- Off-iomgr scheduling pattern: ha_convergence.c is the canonical example — delay_task_ms(&c->delaymgr, 1, 1, entry, ctx) enqueues (ordq_put only, non-blocking) and the delaymgr worker runs the body off the iomgr ACK thread. Tests drive the queued task by calling its entry fn directly (the fixture leaves delaymgr unstarted). Guard scheduling on c->delaymgr.running to avoid leaking tasks into an undrained queue in the unit-test fixture.
## Key Learnings (2026-06-23, /xce-test-ha HA soak — 3 failover bugs)
- (bug-854 dead-first residual) The client `client`/`xcecli_start` initial connect was hardcoded to `endpoint_connect(masters[0])`; if masters[0] is a KILLED node (connection-refused), the whole session fails BEFORE the failover loop (which only runs on an already-connected ctx) can rotate. FIX: loop `xcecli_start` over ALL masters, re-init the endpoint between attempts (endpoint_fini + endpoint_init + endpoint_set_peer + rebind ctx.iomgr — same as xce_ctx_reconnect), set failover.cur_master to the connected index. Verified: dead-first masters list discovers primary in 46ms (was: hang). Distinct from the earlier bug-854 fix (xcecli_ctrlstatus_primary force-rotation) which only covered the all-live standby-first case.
- (bug-1042 rejoin never converges) ha_init_raft (myapp.c, BEFORE election) sources the raft library `initial_log_len` from persist_durable_floor — but on a standby-REJOIN boot persist_recover runs LATER (cluster_init_phase2). So a restarted node's library comes up log_len=0 and its logOk rejects EVERY AppendEntries (prevLogIndex>=1 vs empty log) → permanently stuck non-converging standby; primary logs "standby[i] lsn=N < committed_lsn=M -> stream WAL batches" forever. FIX: ha_sync_raft_log_from_recovery(ha,p) after persist_recover (cluster.c), raising raft.log_len/commit_index/term to recovered values (monotonic, clamp commit<=log_len). Mirrors bug-848's applied_lsn sync. Diagnostic tell: `ae_reject_logmismatch` at prevIdx==recovered_floor on the rejoined node.
- (bug-1043 catch-up apply gap) The term-safe library catch-up route (ha_apply_term_entry_via_library → ha_consume_append_entries → on_persist_entry → persist_wal_write_replicated) writes the WAL but NEVER calls apply_wal_entry/restore_job — so a member that catches up the missing suffix by REPLICATION holds entries on disk but NOT in the queryable job table. On promotion joblist/expect-job miss them → looks like committed-job loss but walcompare is byte-identical (it's an apply gap, not data loss). The LEGACY ha_apply_single_entry path called apply_wal_entry after persist; the term-safe route (ha-term-safety 3.4) dropped it. FIX: re-add apply_wal_entry on the accepted path in ha_apply_term_entry_via_library.
- (HA soak harness design) /xce-test-ha convergence gate must NOT key on "standby[i] converged" log (only re-emitted for a standby that was NOT-converged under the current primary term — a standby restarted already-caught-up never logs it). Use "no NEW 'stream WAL batches' line on the primary for ~6s" == all standbys converged (HA_REPL_CONVERGED is a silent no-op). Detect new-primary election via the INFO line "HA FSM (raft): → CTRL_PRIMARY (term=N)" with term STRICTLY > pre-kill term (user guidance). Only SINGLE-failover is guaranteed — the soak MUST wait full 3-of-3 reconvergence before the next kill, else it tests an unsupported double-fault window and reports false loss.
## Do-Not-Repeat (2026-06-23, shell portability)
- This deploy env's `realpath` is BUSYBOX (`/sbin/realpath`): it does NOT support `-m` (treats it as a filename, errors to stderr, and on a relative path whose parent exists still prints a resolved line to stdout → with `|| echo "$TOP"` you get MULTI-LINE garbage in the var). `readlink -f` also fails on nonexistent paths here. Don't rely on `realpath -m`/`readlink -f` for path normalization in scripts that run under nix/this host. Use pure-bash absolutization instead: `case "$P" in /*) : ;; *) P="$PWD/$P" ;; esac`. (Caught while adding the [dir] arg to run-ha-soak.sh.)
## Key Learnings (2026-06-23, /xce-test-ha [dir] arg)
- run-ha-soak.sh deploy dir now resolves: positional `$1` > `$XCE_HA_TOP` env > default `/home/claude/tmp/xce-ha-deploy`. The skill maps `/xce-test-ha [rounds] [dir]` → rounds to `XCE_HA_ROUNDS` env, dir to the positional arg. Because the deploy is `rm -rf`-destructive and the dir is now caller-chosen, a sentinel `.xce-ha-deploy` marker is written at deploy time and a guard refuses to wipe a non-empty dir lacking it (fail-loud, protects a typo'd home/source path). Empty/nonexistent dir is always safe.
## Key Learnings (2026-06-23, bug-845 WAL double-append fixed)
- The hook system (src/lib/hook.c) had NO dedup: xce_add_hook appended a new slot for a repeat (fn,data); xce_run_hooks fires ALL slots; xce_disable_hook NULLs only ONE. So registering the same hook twice = fires twice + survives one disable. Every WAL_JOB_SUBMIT was double-appended (byte-identical, adjacent LSN) because persist_start() (boot, unconditional) AND ha_register_persist_hooks() (PRIMARY promotion) both register persist_job_hook_fun with the same persist_t. The hooks_registered guard only stops ha_register from re-adding ITSELF — it's blind to persist_start's registration.
- FIX = make xce_add_hook idempotent on exact (fn,data): re-adding an active (fn,data) is a no-op; same fn + DISTINCT data still creates a 2nd slot (legitimate multi-closure, preserved by test). Chosen over "remove persist_start's registration" because MANY controller unit tests (test_e2e_checkpoint, test_ha_role_lifecycle, test_commit_barrier_reply:387 "persist_start registers the production hooks") DEPEND on persist_start registering the hooks and do NOT call ha_register — removing it would break that contract.
- ack gating: handle_jobsubmit (job.c) acks the client iff xce_run_hooks(AFTER_EVENT JOB_NEW) returns 0, and xce_run_hooks returns the LAST hook's rc. With the double-register the ack was (accidentally) gated on the 2nd append's repl_wait_majority_ack (LSN+1) — an extra majority round-trip that MASKED a latent failover durability race. Single-append (correct) removed that margin: one post-fix soak then hit the KNOWN intermittent committed-job-loss (bug-795/828 class — LSN-N cross-node divergence: killed primary had the entry, the 2 survivors did not). Confirmed pre-existing/timing (baseline also passes-or-fails on timing; 3/3 post-fix soaks then passed), NOT caused by the dedup. Validation recipe for HA soak: rebuild controller/xce_controller+xce_test, then run .claude/skills/xce-test-ha/run-ha-soak.sh; judge by waldump single-vs-double entries, not the leaky ASAN exit code.
## Do-Not-Repeat (2026-06-23, isolate before blaming your own change)
- When a soak regresses right after your edit, DO NOT assume your edit caused it: the working tree here had pre-existing UNCOMMITTED HA changes (cluster.c/ha.c/persist.c/config_repl.c) that the stale prebuilt binaries (dated 15:01) had NEVER compiled. The first rebuild pulls BOTH your change AND those deltas. Isolate by `git stash push -- <your files>`, rebuild, re-run. Here it proved the WAL-dup fix was clean and the job-loss was the known intermittent durability race.
## Key Learnings (2026-06-23, /xce-test-ha leadership coverage — ctrl-3 never primary)
- WHY ctrl-3 never led: HA election timeout is PRIORITY-STAGGERED. my_priority = index in --peers list (ctrl-1=0, ctrl-2=1, ctrl-3=2; lower = higher priority), and the election delay = my_priority * election_priority_step_ms (ha.h:138, default 500ms → p0 immediate / p1 +500 / p2 +1000). The soak only ever kills ONE node (single-failover), so the surviving pair always contains a lower-index node that out-races ctrl-3. Confirmed: ctrl-3 had 0 CTRL_PRIMARY announcements but DID start 6 elections (it tries, always loses the race). No leadership-transfer/stepdown command exists in xce_test or ha.c, so leadership can't be handed over directly.
- HOW to force a specific (lowest-priority) node T to lead, DETERMINISTICALLY and loss-free (run-ha-soak.sh force_elect_node): exploit the RequestVote freshness gate (a behind node cannot win). P=current primary, S=third node. (1) kill S so it stops receiving entries; (2) submit a batch → commits on {P,T}, S now provably behind; (3) kill P → only T alive briefly (T holds ALL commits — they were on {P,T} — so nothing is lost); (4) restart the STILL-BEHIND S → quorum {T fresh, S stale}; S cannot catch up while there is no primary, so it stays freshness-fenced for the whole election → T wins deterministically (NOT by timing); (5) verify no loss, restart P, reconverge. KEY INSIGHT: a behind node can't catch up without a primary, and there's no primary during the election, so the spoiler is guaranteed fenced — this removes all timing-race flakiness.
- The soak now tracks SEEN_PRIMARY per node and the verdict ASSERTS all 3 led (PASS line: "all 3 nodes led (ctrl-1,ctrl-2,ctrl-3)"); a node that never leads is a FAIL. force_elect_node is generic (works for any never-led node), run post-loop for each missing node after restoring 3-of-3.
## Key Learning (2026-06-24, bug-1077) — ctx->reply.u is a SHARED, UNTAGGED union
`xce_ctx_t.reply.u` (src/include/xcectx.h) overlays debugmsg/get(dataset_t*)/jobid/clusterconf/ctrlstatus in the SAME 8 bytes. A non-NULL slot does NOT tell you its type. NEVER free or dereference it by type in `xce_handle_reply` (xcectx.c) — `MOVE_PTR` overwrite only (it never touches the old value). The reply slot is owned by the *consumer* (xceapi.c), which MUST clear `u.get = NULL` on EVERY exit path (the get/resval consumers leaked the caller's dead stack `&getreply` on the failover error paths → a later ctrlstatus reply freed it as a CtrlStatus* → stack-use-after-return).
## Do-Not-Repeat (2026-06-24, bug-1077)
- Do NOT add "free the prior occupant" guards on an untagged union slot (this is what commit 61e1391 did incidentally, bundled into an unrelated "fix bug-1060 bug-1067" — it caused a CRITICAL ASan crash). If you want leak-safety on such a slot you need a type tag, not a blind free.
- When a request consumer stashes a STACK pointer into a shared ctx field (e.g. `ctx->reply.u.get = &local`), it MUST clear it on ALL exit paths, not just success. Use a single `goto done` cleanup so a new error path can't reintroduce the leak.
- Keep speculative "while I'm here" hardening OUT of bugfix commits — it hid a memory-safety regression behind an unrelated commit title.
## Do-Not-Repeat (2026-06-24) — rebuild controller BEFORE /xce-test-ha
The /xce-test-ha soak deploys whatever `src/xce.build/controller/{xce_controller,xce_test}` currently are. After editing libxce/controller source, those binaries can be STALE (older than your fix) — the soak would then validate the wrong code (or hit the bug you just fixed). ALWAYS `ninja -C src/xce.build controller/xce_controller controller/xce_test` (or /xce-build controller) and confirm binary mtime > your last source edit BEFORE running the soak. (Caught here: deployed binaries were 09:24, bug-1077 fix was 09:32.)
## Key Learning (2026-06-24) — ha-convergence-recovery CLOSED
Task 8 (live 3-node E2E) passed: `/xce-test-ha` 8 rounds, 30 jobs, zero loss, all 3 nodes led (ctrl-3 forced via RequestVote-freshness fence maneuver). bug-1028 (post-failover availability) already fixed 2026-06-23. The soak is ROLLING single-failover only — it does NOT cover cold-restart-all (bug-795/828 gap, separate) nor isolate/heal quorum-WARN (unit-covered in test_convergence_verification). All 8 tasks [x].
## Key Learnings (appended 2026-06-25)
- XCE thread-role contract (ha-async-io.md, corrected 2026-06-25): iomgr = non-blocking IO + dispatch ONLY (never process inline); delaymgr = trigger-only, NEVER block (it drives heartbeats/election-timeouts — blocking there stalls cluster liveness → spurious failover); vtask pool = bounded blocking ONLY under COMMIT_WAIT_RESERVED admission cap; long blocking IO (reconnect/snapshot/config-push) → dedicated slow lane. This SUPERSEDES the old "delaymgr for blocking ops = Permitted" rule, which was the root cause of the bug-851 chaotic-failover family (delaymgr overloaded as both heartbeat-driver and blocking-sink).
- `endpoint_block_tasks(ep)`/`endpoint_unblock_tasks(ep)` (src/lib/netio.c:896-911) set a per-endpoint `ep->busy` flag enforcing SEQUENTIAL processing of messages from the same endpoint. Auto-applied on the VTASK_IO dispatch path (vtaskmgr.c:249 block on start, :66 unblock on completion; vtask_eligible :203 skips a busy endpoint). Use this when strict per-connection message-processing order is required. Caveat: the block is held for the whole task duration — never run long I/O inline in a blocked handler (it queues that peer's later messages, incl. heartbeat ACKs); trigger long I/O to a slow lane and return.
- epoll registration in libxce is thread-safe: iomgr_monitor (EPOLL_CTL_ADD), iomgr_unmonitor (DEL), iomgr_update_poll (MOD) all wrap epoll_ctl in mylock_lock(&iomgr->lock) (src/lib/netio.c:1046-1082). So an endpoint's fd can be registered into the iomgr epoll set from ANY thread (connect→register already runs from delaymgr/main, e.g. on_vote_connected). No eventfd/self-pipe "post to iomgr" primitive exists or is needed for fd registration — prepare the endpoint fully before iomgr_monitor (unmonitored fd yields no events → no race).
## Key Learning (2026-07-02, bug-1381) — the bug-1298 endpoint refcount is the ONLY safe way to hand a borrowed endpoint_t* to another thread
Any time code snapshots a raw `endpoint_t*` under `c->ha.lock` (or another owner lock) and then touches it from a DIFFERENT thread after releasing that lock — a spawned `pthread_create` worker, a slow-lane task, anything — it MUST `endpoint_ref()` the ep in the same critical section the pointer is read, with an unconditional matching `endpoint_unref()` on every path that thread can exit (success, send-failure, thread-never-started, alloc-failure before the thread even runs). Without this, a concurrent RDHUP teardown (`handle_endpoint_error` → `zarray_del` → `endpoint_killer` → `endpoint_unref`) can free the ep mid-use. This is the THIRD instance of this exact hazard shape (bug-1197/snapshot-lane, bug-1376/cold-start dispatch, bug-1381/config-push transport fan-out) — when reviewing any NEW code that borrows a standby/vote endpoint across a thread hop, check for this bracket by default, don't wait for a bug report.
## Do-Not-Repeat (2026-07-02, bug-1381 test approach)
When a production send function has no test seam (e.g. `xce_replconfigpush_req_raw` calls real capnp+SSL machinery), don't try to unit-test the real network path — add a small function-pointer hook (`push_thread_send_hook`, same shape as the existing `xce_wal_stream_request_hook`) so a test can park/gate the send and probe invariants (like `endpoint_ref` counts) deterministically. Confirmed via temporary revert (`Expected 2 Was 1`) that the new regression test actually fails without the fix before trusting it as the RED→GREEN gate.
## Do-Not-Repeat (2026-07-02, re-triggered) — verify xce.build mtime EVERY soak run, not just once
Nearly repeated the exact mistake already logged above (2026-06-24 entry): ran two `/xce-test-ha`-style soaks (chaotic PASS, force-snapshot FAIL x2) against `src/xce.build/controller/xce_controller` that was BUILT BEFORE the `g_drive_async_configpush_offload` flag flip — the binary's mtime (04:05) predated the source edit (07:43). The chaotic "PASS" therefore validated old code, not the new flip; the force-snapshot failures are inconclusive noise, not evidence against the change. Root cause: I rebuilt `build.release` (a different meson output dir used for other purposes) and assumed that was what the soak script deploys — it is NOT. `run-ha-soak.sh` / `/xce-test-ha` always deploys from `src/xce.build/controller/{xce_controller,xce_test}` specifically. RULE: before EVERY soak invocation (not just once per session), run `stat -c '%Y %n' src/xce.build/controller/xce_controller src/controller/*.c` (or equivalent) and confirm the binary is newer than every source file it depends on — `ninja -C xce.build controller/xce_controller controller/xce_test` unconditionally right before the soak costs nothing when already up to date (ninja says "no work to do") and is cheap insurance against silently testing stale code.
## Key Learning (2026-07-02, bug-1386) — a soak PASS/FAIL always needs a flag-OFF control run before blaming the flag
Flipped `g_drive_async_configpush_offload` ON after closing its correctness blocker (bug-1381) and got 0/3 force-snapshot soak passes — looked damning. Before reverting-and-declaring-regression, ran the SAME scenario with the flag OFF on the identical freshly-built binary: 1/2 pass, SAME failure signature ("no snapshot install" / "did not reconverge"). That proved the defect was pre-existing (root cause: `ha_send_snapshot_to_standby` in config_repl.c ignores `xce_snapshot_chunk_req_raw`'s return in its chunk loop, silently dropping a chunk on `-ECONNRESET` and still logging "complete") and unrelated to the flag being tested. Lesson: when a soak fails after a flag flip, ALWAYS run an A/B (flag-ON vs flag-OFF) control on the same binary/environment before attributing the failure to the change — a single-arm test can't distinguish "my change broke this" from "this was already flaky." [[project_bug1381_configpush_uaf]]
## Do-Not-Repeat (2026-07-02) — small soak sample sizes overstate confidence either direction
0/3 vs 1/2 is NOT strong evidence of a flag-specific regression (P(0/3 fails | true 50% pass rate) ≈ 12%, well within noise) — but it's also not strong enough to declare "definitely safe, flip it." When a flip's soak sample is this small and the underlying bug is independently reproducible at baseline, the correct call is to REVERT and defer (don't ship on ambiguous evidence) rather than accept either "probably a coincidence" or "probably a regression" without more data. Bar to re-flip: fix the underlying bug (bug-1386) first, THEN re-run enough rounds (the existing 2/3-pass precedent from the 3.1/3.2 flip session) to get a real signal.
## Key Learning (2026-07-02, bug-1386 FIXED) — a fire-and-forget send-loop must check EVERY iteration's return, not just fread's
`ha_send_snapshot_to_standby`'s chunk loop already checked `fread`'s return (`rc = -EIO; break;`) but not the network send call two lines later — an inconsistent half-checked loop is an easy trap: it *looks* like errors are handled because the pattern exists right there, but the actual transport call was left bare. Fix: capture into a per-iteration `send_rc`, `if (send_rc != 0) { rc = send_rc; break; }`, mirroring the existing fread check exactly. When reviewing/writing any chunked-transfer loop, verify *every* fallible call inside the loop body is checked, not just the first one — grep for the pattern once isn't enough, read the whole loop body. Also confirmed: no receive-side timeout/re-request was needed to fully close this — `ha->snapshot_inflight[peer_idx]` already unconditionally clears when the send thread exits (`snapshot_lane_fn`, success or failure), so the next heartbeat's SEND_SNAPSHOT decision naturally retries a peer whose `applied_lsn` never advanced. A silently-lying "success" return was the actual bug, not a missing retry mechanism — fixing the lie let the existing retry cadence do its job. [[project_bug1381_configpush_uaf]]
## Do-Not-Repeat (2026-07-02) — "the fix is verified" and "the failure mode is gone" are different claims; re-test the FULL scenario, not just the mechanism you fixed
Fixed bug-1386 (send-side chunk-drop), confirmed via unit test AND via re-running the exact `--force-snapshot` soak scenario that originally surfaced it. The soak STILL failed — but with a genuinely different signature (sender now correctly reports "complete" for a truly-complete transfer; receiver's `persist_load_snapshot` then rejects the file as corrupted, `rc=-5`). Byte-diffing sender-vs-receiver files proved chunk CONTENT corruption at 64KB boundaries (a capnp encode issue, likely same family as bug-847) — a THIRD, previously-invisible bug that bug-1386's silent-drop bug had been masking/co-occurring with. Logged as bug-1389 (open). Lesson: when a user asks "did the fix solve the problem," don't stop at "the code path I changed now behaves correctly" — re-run the ORIGINAL failing scenario end-to-end. A fixed bug can still leave the outer symptom (soak FAIL) unchanged if a second, previously-undetected bug was co-resident in the same failure path. Report both facts distinctly: "your specific fix works" vs "the scenario now passes" are not the same sentence.
## Key Learning (2026-07-02, bug-1389 FIXED — corrects the capnp hypothesis below) — snapshot chunk transfer had TWO independent failure classes in the same code path
`ha_send_snapshot_to_standby`/`ha_handle_snapshot_chunk`'s OPCode_snapshotTransfer pipeline had two distinct bugs stacked in the same force-snapshot scenario: bug-1386 = chunk DROP (send loop didn't check return, silently truncated); bug-1389 = chunk MISORDERING (full byte count arrives, every chunk's content is byte-for-byte correct, but chunks land at the WRONG FILE POSITION). Both produce a receiver-side failure but via different mechanisms (bug-1386: is_last never arrives, transfer hangs forever; bug-1389: is_last arrives, size check passes, `persist_load_snapshot` rejects the misassembled file with -EIO). **The initial "capnp encode/segment-boundary" hypothesis (below, and in the first version of this entry) was WRONG** — ruled out two ways: (1) an exhaustive trace of the wire framing (8-byte length-prefix, all uint64_t, clean) and the capnp far-pointer resolution (walked byte-exact, correct); (2) a deterministic in-process encode→decode round-trip test at the REAL 65536-byte chunk size (test_wal_wire.c) passed cleanly — proof, not just inspection, that capnp/transport was never the bug. **What actually happened**: the vtask pool dispatches one transfer's chunks to DIFFERENT workers (cpu#0-3 in the log) with no ordering guarantee — a live crc32 log at both send and receive (correlated by snapshot_lsn+chunk_index) proved chunks arrived as `0,1,3,4,2,5,...` with every individual chunk's crc32 matching exactly, while `ha_handle_snapshot_chunk` blindly appended bytes in ARRIVAL order instead of at `chunk->chunk_offset` (a field already on the wire, just unused) — reordered chunks landed at the wrong byte range. Fixed with `pwrite`-at-offset (order-independent) + a chunk-count-vs-expected-total completion tracker (not "this call has is_last") to avoid finalizing before a straggler chunk arrives. **The decisive technique**: live per-chunk crc32 correlation logging at BOTH ends of a transport pipeline, re-run against the real failing scenario — this is what separated "content genuinely corrupted in transit" from "content correct but reassembled in the wrong order," which byte-diffing the final files alone could NOT distinguish (a diff just shows "different at offset X"; only a per-hop hash trail shows WHERE the divergence enters). When a "corruption" bug's symptom is a byte-shift within an otherwise-correct total, always separate "wrong bytes" from "right bytes, wrong position" before hypothesizing a specific encode/transport mechanism — the two have completely different root-cause spaces (protocol/library bug vs. ordering/concurrency bug). `g_drive_async_configpush_offload` re-flipped to DEFAULT ON same day after this fix — soak-validated (6-round chaotic + 3/3 force-snapshot PASS) — raft-drive-async task 3.3 fully landed. [[project_bug1386_snapshot_chunk_drop]]
## Key Learning (2026-07-02, ha-commit-durability 6.1 closed) — bug-795's cold-restart-all loss was overlayfs, not real; 0/5 on genuine ext4
Ran `test-commit-durability.sh` (8 rolling rounds + cold-restart-all) 5 times against a live 3-node cluster deployed to `~/tmp` (confirmed via `mount`/`df -T` to be real ext4 on `/dev/nvme0n1p2`, distinct from the sandbox's overlay `/`) — zero committed-job loss across all 5 cold-restart-all cycles. This closes the long-standing "pending confirmation under simultaneous total outage" caveat on the Commit Durability guarantee (`src/controller/CLAUDE.md`) — it was genuinely an overlayfs `fdatasync` artifact (as suspected since 2026-06-19), not a real durability gap; no production code changed. Confirms [[reference_nonoverlay_fs_soak]]'s `~/tmp` recommendation generalizes beyond HA failover soaks to durability soaks too. One SEPARATE anomaly surfaced (run 5/5): cluster showed 20 committed jobs, harness's own ACKED_FILE tracked only 19 — an EXTRA job, not a missing one, opposite of the bug-795 symptom. Deliberately did NOT chase this to ground (it doesn't match what task 6.1 was investigating, and full root-cause needs a live `joblist` capture at the failure point, which the cluster had already been torn down before I thought to check) — logged as bug-1396 and left open rather than either ignoring it silently or burning more budget on a tangent. When a soak run turns up an anomaly that ISN'T the one you're hunting, the right move is: log it precisely (so it's not lost), don't silently drop it, but also don't let it hijack the current investigation's scope.
## Key Learning (2026-07-02) — proving a fs-blocker resolved unblocks EVERY task gated on it, not just the one you were investigating
Closing ha-commit-durability 6.1 (proving `~/tmp` is real ext4, not overlay) turned out to unblock TWO OTHER unrelated specs' live-validation tasks in one shot: controller-status 6.4 and ha-heartbeat-committed-propagation 4.1 both carried the IDENTICAL `_Blocked: ... this sandbox is overlayfs ... cannot run here_` annotation, referencing the exact same bug-795/overlayfs root cause. Once 6.1 resolved that root cause, both were immediately runnable with zero further investigation — just deploy to `~/tmp` and execute the hand-off recipes already written in their Implementation Notes. Lesson: when closing an environment/infrastructure blocker (not just a code bug), grep ALL specs' tasks.md for the SAME blocking annotation/bug-ID before moving on — a single fix can cascade-unblock several stale "blocked" checkboxes across unrelated specs that all cited the same underlying constraint. Found via `grep -oP '^\s*-\s\[[ ~]\]\s+\d+\.\d+' .kiro/specs/*/tasks.md` to enumerate genuinely-open leaf tasks (excludes cosmetic phase-header lines) across the whole `.kiro/specs/` tree — a fast way to get the REAL remaining-work picture instead of trusting `spec.json`'s `phase` field (which only tracks spec-generation phase, not implementation completion, and was stale/misleading for several fully-done specs in this project).
## Key Learning (2026-07-02) — job-facing client ops are primary-only by design; verify standby replication via ctrlstatus, not joblist
Attempted to validate ha-wal-replication 5.4 ("job appears in joblist on primary AND all standbys") by pointing `xce_test`'s `masters` setting at a single standby only — got `xce_joblist: Unknown error 10014` (`E_XCE_STANDBY`). This is NOT a bug: `ha.c`'s `CTRL_STANDBY` dispatch switch (~line 3501) only allows replication/election/`OPCode_ctrlstatus` opcodes; every job-facing op (`joblist`, `jobsub`, etc.) falls through to `-XCE_ERR_STANDBY` by design — standbys never serve job reads/writes directly, only the client-side failover loop transparently rotates to the primary. Correct way to verify a standby *has* replicated data without going through the primary-only ops: query `ctrlstatus` (explicitly allowed in every role) and compare `committedLsn`/`appliedLsn` across nodes — a match proves the WAL entry replicated and applied, even though you can't `joblist` the standby directly. Confirmed this is intentional, not an oversight, by reading the full `ha_dispatch_gate`-style switch statement rather than assuming a gap. [[project_xce_system_map]]
## Key Learning (2026-07-02, bug-1401 FIXED) — a generated codec's `free_X(struct*)` vs `free_X_ptr(struct**)` split is easy to get wrong in a hand-written client wrapper
`decode_X_ptr()` (capnp-generated) `calloc`s the top-level struct THEN fills its fields; the matching free is `free_X_ptr(&ptr)` (frees fields + the struct + nulls the caller's pointer), NOT `free_X(ptr)` (fields only — the codegen intentionally splits these because `free_X` is also reused on stack-allocated structs, e.g. this project's own `test_ctrlstatus_roundtrip_all_fields` test calls `free_CtrlStatus(&out)` on a stack var, which is correct there but wrong for a heap ptr). `src/include/client.h`'s `xce_free_ctrlstatus(s)` macro used the fields-only variant, leaking `sizeof(xce_ctrlstatus_t)` (120 bytes) on every real `ctrlstatus` call. **Root cause of it going undetected**: every existing ctrlstatus unit test built its `xce_ctrlstatus_t` via the stack-decode helper (`decode_CtrlStatus`) and freed it with the correct raw `free_CtrlStatus(&out)` call directly — none of them ever exercised the actual client-facing `xce_free_ctrlstatus()` macro that real code paths use. **Rule going forward**: whenever a generated codec exposes both a bare `free_X` and a `free_X_ptr`, any hand-written `xce_free_*`/`xcecli_free_*` wrapper macro over it MUST use `free_X_ptr(&(s))` if the value being freed came from a `decode_X_ptr()` heap allocation (check what the corresponding `xce_X(...)` API call's `MOVE_PTR` source was decoded with, not just what looks intuitively "free-shaped"). And: a roundtrip test that exercises only the raw encode/decode/free primitives is NOT a substitute for a test that goes through the actual client macro/wrapper — add at least one regression per free-wrapper that uses the real heap-allocating decode path + the real macro. RED confirmed the exact 120-byte/1-alloc LeakSanitizer signature from the bug report before applying the fix.
## Key Learning (2026-07-03) — buglog.json's biggest lie is silence: 6 of 7 swept "open" bugs were already fixed and just never marked
Swept 7 old "not yet fixed" buglog entries (846, 847, 855, 1096, 1147, 1198, 1307) by checking the ACTUAL current code/tests instead of trusting the buglog text — 6 were already fixed (some for over a week), only bug-847 needed real action (and even that action was "un-ignore the regression test," not a production code change — its underlying condition was already gone). Common thread: every one of these had a purpose-built regression test or an explicit in-code citation that would have revealed the fix immediately, but nobody closed the loop after landing the fix under a DIFFERENT bug number or spec task. **Rule going forward**: before spending real investigation budget on an "open" bug, first check (a) does it have a named regression test — run it directly; (b) does its bug ID appear in a code comment — grep for it; (c) does its described symptom show up in the relevant spec's tasks.md as a since-closed task. All three are cheap (single tool call) and would have caught 6/7 of these immediately. **Second rule**: when you land a fix, grep buglog.json for the bug ID(s) it plausibly closes (including "family" bugs like bug-851's many sub-numbers) BEFORE moving to the next task — don't rely on remembering to circle back later, because "later" doesn't reliably happen and staleness compounds silently. [[project_ha_bootstrap_progress]]
## Key Learning (2026-07-03, bug-847 investigation) — trace the actual library mechanism before trusting a plausible-sounding root-cause hypothesis
A prior investigation (and a fresh subagent re-investigation) both converged on "the encoder captures a capnp segment pointer once outside a loop and reuses it stale across iterations" as bug-847's root cause — plausible-sounding, cites real code (`cs = capn_root(&c).seg;` before a `for` loop in `xce_wal_batch_encode_raw`, src/lib/request.c). Tracing the ACTUAL c-capnproto library (`new_object`/`new_data`/`write_ptr`/`capn_setp` in the vendored `capn.c`) showed this is a non-issue by design: `new_object` always re-resolves and re-tags `.seg` on segment overflow regardless of what stale segment pointer was passed in, and `write_ptr`/`capn_setp` correctly emit far/double-far pointers whenever the source and target objects land in different segments. The theory was never tested against the library source, only against the observed symptom — and the observed symptom (RED test failure) turned out to no longer reproduce at all once actually run. Lesson: a hypothesis that "sounds mechanistically right" and cites real line numbers is not the same as a verified root cause — for a data-loss bug specifically, always run the existing/write a NEW regression test before accepting a hypothesis, even one two independent investigations agreed on.
## Key Learning (2026-07-04, raft-apply-decoupling task 5.3) — `recovery_unrecoverable`'s clear path is LIVE-catch-up-only, not recovery-driven; apply_divergent has no live self-heal yet
`persist_clear_recovery_unrecoverable()` (persist.c) has ZERO call sites inside persist.c itself — its only two callers are in ha.c (`promotion_fence_timeout_fn` ~ha.c:867, and the promotion-eligibility gate ~ha.c:1115), both gated on STRUCTURAL getters (`persist_recovery_truncated`/`persist_durable_floor` vs cluster HWM) that can genuinely change while a process stays up (a fenced standby keeps receiving replicated WAL from a healthy peer). `persist_recover()`'s own APPLY_CLEAN branch never calls the clear function — a second, later `persist_recover()` call reaching APPLY_CLEAN does NOT auto-clear a sticky flag set by an earlier APPLY_DIVERGENT verdict on the same object. This is correct by design (task 5.2's decision table fences apply_divergent unconditionally specifically because more WAL bytes/catch-up can't prove a local apply-dispatch defect is fixed) but means: today, the apply_divergent-triggered fence has no LIVE self-heal analogue to the structural fence's catch-up-timer release — it only "clears" via a full process restart into a fresh `persist_t` (persist_init defaults `recovery_unrecoverable=false`) that then replays cleanly. Task 6 (ordered apply pump) is where a live re-verification analogue would need to be built if ever wanted. Don't assume "the fence auto-clears like the structural one" for this trigger without checking which mechanism (live re-check vs restart) actually applies.
## Key Learning (2026-07-04, raft-apply-decoupling task 5.3 round-2, CLOSED) — the trigger-agnostic sticky-fence bug: fix the guard on BOTH live-clear call sites, not just one
Round-1 review found a CRITICAL gap: `persist_recovery_unrecoverable` is a bare boolean with no provenance about WHY it was set — task 5.2 added `apply_divergent` as a new trigger reusing the existing sticky flag, but the flag's two pre-existing LIVE clear call sites (`promotion_fence_timeout_fn` ha.c:~867, and `raft_cb_become_leader`'s promotion-eligibility gate ha.c:~1109) both only ever checked STRUCTURAL conditions (`persist_recovery_truncated`/`persist_durable_floor` vs HWM) — never the apply verdict. Since a WAL can be structurally perfectly healthy (not truncated, durable_floor>=HWM) while the local apply-dispatch path is still defective (APPLY_DIVERGENT — the two are orthogonal by construction), the structural timer would eventually observe "healthy" and silently clear an apply-divergence fence, letting a genuinely divergent node get promoted on its NEXT election. FIX (additive, ~30 lines, both call sites in ha.c): added `bool apply_divergent = persist_apply_verdict(&c->persist) == APPLY_DIVERGENT;` at each site and ANDed `!apply_divergent` into the existing enclosing `if` condition that gates the clear (`if (!truncated && !below_hwm && !apply_divergent)` in the timer; `if (gate==ELIGIBLE && !unrecoverable && !apply_divergent)` in the promotion gate). **Key design insight**: gating just the bare `persist_clear_recovery_unrecoverable(...)` call-statement itself is NOT sufficient at the become_leader site — the safety-critical action there is falling through to `CTRL_PRIMARY` promotion, not the clear call (which in production can only be reached when `unrecoverable` already reads false); you must extend the ENCLOSING branch condition that also gates promotion, or the fix is a no-op that still promotes a divergent node while merely skipping a redundant flag-clear. Added to `promotion_fence_timeout_fn`'s guard anyway (timer site) since it DOES flip true→false there and is the actually-exploitable half. Since `apply_verdict` is only ever recomputed by `persist_recover()` (boot-time-once, single call site `cluster.c:1451`), this guard means an apply-divergent fence can now ONLY clear via a full process restart that replays cleanly — never a live timer/gate. Regression test `test_promotion_fence_apply_divergent_survives_structural_catchup` (test_commit_durability.c) mirrors `test_promotion_fence_released_on_catchup`'s exact harness/timing, differing only in also forcing `c->persist.apply_verdict = APPLY_DIVERGENT` + `recovery_unrecoverable=true` up front — confirmed RED (fence wrongly cleared) against the pre-fix code, GREEN after. All 3 required suites pass with 0 regressions: test_commit_durability 39/39, test_apply_correctness_verification 11/11, test_ha_core_safety 28/28. [[project_bug1060_leader_completeness]]
## Do-Not-Repeat (2026-07-04, sticky-flag-with-multiple-triggers pattern)
When a shared sticky/latching flag gains a NEW trigger reason (reusing an existing mechanism, as instructed by a design doc's "don't build a new mechanism" guidance), always audit EVERY existing call site that CLEARS or CHECKS that flag for whether it silently assumes the OLD trigger's semantics (e.g. "clearable once structural conditions are healthy"). A flag with no provenance field is only as safe as the least-restrictive clear site — adding a new, stricter-to-clear trigger without updating every clearer creates exactly this class of bug. Grep for all callers of the clear function FIRST (`grep -n "persist_clear_recovery_unrecoverable" src/controller/ha.c` found exactly 2, matching the reviewer's claim precisely) rather than trusting a blast-radius/paraphrased caller count.
## Key Learning (2026-07-04, raft-apply-decoupling task 5.3 round-3, CLOSED) — a logically-correct guard with a green suite can still have zero coverage; prove it by reverting the guard, not by reading the code
Round-2's `!apply_divergent` conjunct at the `raft_cb_become_leader` promotion-eligibility gate (ha.c:1130) was independently verified LOGICALLY correct twice, yet reverting ONLY that conjunct left all 298 pre-existing tests green — no test ever drove a node that was simultaneously (a) structurally ELIGIBLE (`ha_promotion_gate_decision` genuinely returns `RECOVER_DECISION_ELIGIBLE`), (b) NOT sticky-fenced (`persist_recovery_unrecoverable()==false`), AND (c) `apply_verdict==APPLY_DIVERGENT`. Closed by adding `test_promotion_refused_when_apply_divergent_even_if_eligible` to `test_convergence_verification.c`, reusing `test_promotion_registers_standbys_when_peer_set`'s exact `cv_setup` fixture (n_peers=2, n_standbys=0, appended_lsn=0 — this fixture is PROVEN to reach genuine ELIGIBLE because that sibling test's own assertion, `repl_active_standby_count==2` after promotion, is unreachable unless the gate let promotion through) and forcing only `c->persist.apply_verdict = APPLY_DIVERGENT` before calling `raft_cb_become_leader_with_cluster_for_test`. RED→GREEN confirmed by temporarily deleting the `&& !apply_divergent` conjunct in-place, rebuilding, observing exactly 1 failure (the new test, on the promotion-refused assertion), then restoring and reconfirming 0 failures. Full 13-group / 299-test regression (all files touching `raft_cb_become_leader`) stayed green throughout. **Lesson**: "the reviewer traced the boolean logic and it's correct" and "a regression test exists that would catch a revert of this exact line" are different claims — always close a coverage gap by demonstrating the RED failure, not by re-reading the code once more. **Environment note**: contrary to a round-2 flag about a possible openssl/flake.nix issue in this worktree, `nix develop --command bash -c "ninja -C ../build ..."` from `src/` built cleanly with no header-path problems — that concern did not reproduce here.
## Key Learning (2026-07-04, raft-apply-decoupling task 6.1, CLOSED) — the "detail-filter" fix for a dual-origin hook detail is a trap; use call-provenance instead
Hazard 1 (task 4.1's double-append warning) named `HOOK_DETAIL_JOB_UPDATE` as the risky detail once `apply_committed_entry()` goes live, and the obvious-sounding fix is "early-return `persist_job_hook_fun` when `detail == HOOK_DETAIL_JOB_UPDATE`". That fix is WRONG: `job_ctrl`'s SIGCONT/SIGSTOP path (`mark_job_update`, job.c:498/508) fires the IDENTICAL detail for its own still-legitimate live append, and a detail-based filter would silently break job-suspend/resume durability with zero test failure to catch it (no existing test exercises job_ctrl's WAL append in isolation). The correct fix is a call-PROVENANCE guard: a `_Thread_local bool t_apply_seam_notifying` set only around `apply_committed_entry()`'s own `xce_run_hooks()` calls, checked by `persist_job_hook_fun`/`persist_queue_hook_fun`/`persist_worker_hook_fun`. Thread-local (not a shared `persist_t` field) because a shared flag races across the apply-pump thread and any concurrent live command thread touching a DIFFERENT job. Lesson: when a hook detail is dual-origin (one AFTER_EVENT firing point that both a legacy live-command path and a new apply-seam path share), filtering on the detail value is never safe — filter on which CALL STACK is asking.
## Key Learning (2026-07-04, raft-apply-decoupling task 6.1) — commit_resolve_pending is the ONE choke point for ALL SIX committed_lsn-advance paths; wiring a new trigger only into ha.c's on_commit_advanced silently starves single-node/self-quorum clusters
design.md's File Structure Plan says to wire the live apply pump from ha.c's `on_commit_advanced` callback. That callback is fed by exactly ONE of the six commit.c-internal paths that can move `committed_lsn` (durable-advance, self-quorum `commit_submit`, ack-driven `commit_on_ack`, the writable-lsn gate, the raft-library commit-index feed, membership change) — task 1.2 already discovered and consolidated all six into `commit_resolve_pending()`. Wiring the apply-pump kick ONLY into ha.c would leave 5 of 6 paths never triggering it — concretely, most unit-test fixtures (n_peers<=1 self-quorum, no raft library wired) would NEVER see the pump fire. Wired the kick into `commit_resolve_pending()` itself instead (one new line, `persist_apply_pump_kick(ce->p, ce->pool)`, after its existing resolve loop) — a documented, deliberate deviation from design.md's literal text, justified by this discovery. Lesson: when a design doc names ONE integration point for "whenever X changes," check whether X actually has multiple internal advance paths before trusting that single named point is complete — grep for every writer of the field.
## Do-Not-Repeat (2026-07-04, raft-apply-decoupling task 6.1) — a retry-until-caught-up drain loop MUST distinguish "no more work" from "the same failed work is still there"
`apply_pump_drain()`'s first version cleared the scheduled flag, rechecked `last_applied_lsn+1 <= committed_lsn`, and reclaimed+looped if true — without checking whether the PRECEDING `persist_apply_pump_run()` call actually made progress. When a WAL read genuinely fails (e.g. a test fixture advances `committed_lsn` without real WAL bytes on disk for that LSN), `last_applied_lsn` never advances, so the recheck ALWAYS sees identical unresolved work and reclaims the flag forever — an unbounded tight busy-loop (18.5M+ log lines before a 90s timeout killed the process in `test_demotion_convergence`). Fixed by checking the walk's return code immediately: a negative rc returns without rechecking or reclaiming, so a genuinely new future kick (a distinct commit-advance event) is what retries, never the drain loop's own tail. Lesson: any "loop until caught up, recheck before exiting to avoid a lost wakeup" idiom (task 1.3's precedent) MUST also handle "the loop body failed and made zero progress" as a THIRD outcome, distinct from both "caught up" and "genuinely more work" — otherwise the recheck-for-lost-wakeup safety mechanism itself becomes an infinite-retry hazard the moment the underlying operation can fail.
## Key Learning (2026-07-04, raft-apply-decoupling task 6.1) — Hazard 2 (merge-staleness) is real, immediately exploitable under ordinary traffic, and requires a wire-format change; it was escalated as BLOCKED, not silently shipped
Traced `apply_job_start_transition` (`job->job.state = JOB_STAT_RUN`, a full overwrite to a FIXED constant, discarding any other bits) and confirmed different job-state fact types need genuinely different apply semantics: START overwrites-to-fixed-value, FINISH/FAIL/UPDATE are guard-then-set, RESUME needs to CLEAR a bit (not just OR one in). A naive fix (change the shared `apply_job_update_transition` core from overwrite to OR-merge) would silently break RESUME's clear semantics on both replay and any live re-apply — trading one correctness bug for another. Since the wire format (`wal_jobstate_t{jobid,state}`) carries ONLY the final computed value, not which bits a given fact intends to set vs. clear vs. leave alone, no in-boundary fix exists without either (a) a wire-format change (delta/mask encoding) or (b) a receipt-time per-job serialization gate (a much bigger architectural change, arguably violating Req 11.4's "no per-fact synchronous barrier"). Escalated as a BLOCKED-severity, carried-forward finding in task 6.1's status report rather than either ignoring it or shipping a wrong compensating fix — mirrors task 4.1's own precedent of carrying forward a documented, unfixed hazard to the next task rather than guessing at scope it doesn't own.
## Key Learning (2026-07-04, raft-apply-decoupling task 6.2 round 2, bug-1415 FIXED) — self-heal a late registration by checking the already-applied watermark; reproduce the multi-vcpu race in a test by forcing the interleaving with a poll-barrier, not a sleep
Fixed via the self-healing option this same entry's original write-up proposed: `persist_register_pending_apply()` (persist.c) now checks `lsn <= persist_last_applied_lsn(p)` BEFORE linking onto `pending_applies`; if the walk already passed `lsn`, it fires `on_applied(ctx, lsn)` immediately, synchronously, on the CALLER's thread instead of orphaning a registration nothing will ever revisit. Proving this deterministically required a genuinely new test-fixture pattern for this codebase: `vtaskmgr_init()` alone starts NOTHING (no dispatcher, no vcpu threads) — only `vtaskmgr_start()` spawns them, so `test_commit_admission_pool.c`'s existing pool fixture (admission-accounting only) is NOT sufficient for a test that needs real concurrent vtask execution; you must call both. To force the specific bad interleaving (apply-pump wins) deterministically rather than relying on a timing-guessed `usleep()`, the test's stand-in commit-continuation callback blocks in a poll loop on `persist_last_applied_lsn(p) < lsn` BEFORE calling `persist_register_pending_apply()` — this is a genuine barrier tied to the actual race predicate (not a sleep guess), so it reproduces the exact interleaving on every run once a real multi-vcpu pool is wired via `commit_set_pool`. New test `test_apply_pump_late_pending_apply_registration_self_heals` (test_apply_pump.c) is the ONE test in this entire spec that wires a real pool; every other fixture deliberately keeps `ce->pool == NULL` and must stay that way (verified via full regression sweep: zero new failures, only the 2 pre-existing task-7.3-owned `test_commit_barrier_reply.c` failures persist unchanged). [[project_raft_commit_safety_done]]
## Key Learning (2026-07-04, raft-apply-decoupling task 6.2, historical — original bug-1415 finding, now fixed above) — two vtask dispatches enqueued back-to-back from the same function have NO completion-order guarantee once a real (n_vcpus>1) pool is wired
Wired `persist_apply_pump_run()` to consume task 3.2's hand-off (`persist_take_pending_apply()` immediately after each entry's `apply_committed_entry()` + `last_applied_lsn` advance, then invoke the returned continuation) and made `jobsubmit_apply_complete` actually send the batch's success reply (`RequestReply_data_jobid`, gated on `actx->replied` the same way `jobsubmit_reply_failure_once` already gates the failure reply — added a `jobsubmit_reply_success_once` sibling and a `jobid` field on `jobsubmit_async_ctx_t`). While tracing correctness end-to-end, found (not fixed — out of this task's boundary, which is scoped to `persist_apply_pump_run`'s loop only) that `commit_resolve_pending()` (commit.c) enqueues TWO independent `VTASK_LIVE_BLOCK` tasks back-to-back onto the SAME `vtaskmgr_t` pool: first each ready continuation's dispatch (which, for a job submit, calls `persist_register_pending_apply()`), then `persist_apply_pump_kick()`'s own walk task. `vtaskmgr.c`'s dispatcher (`vdispatcher_entry`/`pick_idle_vcpu`) hands queued tasks to whichever of `n_vcpus` worker threads is next idle — with more than one idle vcpu, two tasks enqueued in that order can run on two DIFFERENT threads with no guarantee the first-enqueued one finishes first. If the apply-pump task wins, it applies the entry and advances `last_applied_lsn` past it BEFORE the registration exists; the registration then lands for an lsn the walk will never revisit (it only ever walks `[last_applied_lsn+1..]` forward) — a silently, permanently orphaned reply. **This is invisible in every test in this entire spec** because every fixture (`tap_setup`, `cbrp_setup`, etc.) deliberately never calls `commit_set_pool`, so `ce->pool` stays NULL and `dispatch_continuation`/`persist_apply_pump_kick` both fall back to running INLINE, synchronously, in enqueue order — a single-threaded test can never observe a multi-worker race that only exists when a pool is actually wired. Logged as bug-1415 (not fixed) with two candidate fixes (make the pump-kick wait for its batch's continuation dispatches to finish first, or make `persist_register_pending_apply()` self-heal by checking `lsn <= persist_last_applied_lsn()` and firing inline immediately if so) — flag for whoever builds a real multi-vcpu regression test (task 6.3/6.4 or a fast-follow) to close it. **Lesson**: "the test fixture never wires a pool" is a load-bearing fact worth stating explicitly whenever reasoning about ordering between two `vtask_enq_classified` calls — pool-less inline fallback silently converts an async multi-threaded dispatch into a same-thread synchronous one, which can hide a real race for an entire task's worth of otherwise-thorough tests. [[project_raft_commit_safety_done]]
## Key Learning (2026-07-05, raft-apply-decoupling task 6.3) — a test spy for a hook-fan-out reaction must mirror the REAL routing table, not the idealized one
`apply_committed_entry()` fires every job-state fact via the generic `HOOK_DETAIL_JOB_UPDATE` only (never the type-specific `HOOK_DETAIL_JOB_START/_EXIT/_FAIL/_RUSAGE_UPDATE` — a documented wire-format deviation since task 2.2, persist.h's doc comment). Consequence for test-writing: `sched_job_start_fun`/`sched_job_exit_fun`/`sched_job_fail_fun` (sched.c) and their apiserver.c equivalents are structurally UNREACHABLE for any live-applied fact. A reaction spy registered for the type-specific details would silently test a code path that can never fire in production — passing vacuously, catching nothing. The correct spy listens ONLY for `HOOK_DETAIL_JOB_NEW` (submit) and `HOOK_DETAIL_JOB_UPDATE` (every fact type), since that is the only pair actually reachable. Verified this file's tests are NOT vacuous by temporarily reintroducing the pre-task-4.1 eager receipt-time mutation+notify bug in `on_job_start` (job.c) — both new integration tests failed immediately (double-append from the reintroduced live-notify racing `persist_job_hook_fun`'s durable-append guard), confirmed clean revert (`git diff` empty), then GREEN again, 30/30 stable reruns.
## Do-Not-Repeat (2026-07-05) — `TEST_ASSERT_EQUAL_INT_MESSAGE` requires 3 args; a bare 2-arg call is a compile error, not a silent no-message pass
Caught immediately by the build (gcc: "macro requires 3 arguments, but only 2 given"), not by review — worth remembering `TEST_ASSERT_EQUAL_INT` (no `_MESSAGE` suffix) is the correct macro when no custom message is needed, in this Unity-based C test harness.
## Key Learning (2026-07-05, raft-apply-decoupling task 7.2) — xce_test's `worker` REPL context was missing jstart/jfail; libxce already had the full wire API
`src/lib/client.c` already implements the ENTIRE worker-fact wire API (`xcecli_notify_jobstart/_jobfail/_jobstop/_jobresume/_jobchkpt/_jobpostdone`, alongside the pre-existing `_jobfinish`) — but `testsuite/test.c`'s `worker_commands[]` table only ever exposed `jfinish` as a REPL command. Added `jstart`/`jfail` (mirroring `jfinish`'s exact getopt/parse_jobid/SET_ERROR shape) to unlock a synthetic-but-real-wire-protocol worker-fact stream for the new `--factstream` HA soak scenario, without needing a real `xcecd_worker` + scheduler dispatch handshake — `on_job_start`/`on_job_fail` (job.c) only require the job to already exist in jobmgr (i.e. be submitted), not to have been dispatched, so this is genuinely the SAME production receipt→commit→apply path a real worker would drive, just skipping actual OS-process execution (irrelevant to raft-apply-decoupling's apply-ordering concern). **Do-not-repeat**: `cmd_jobfinish` never reset `optind = 1` — harmless for every EXISTING caller (always invoked at most once per xce_test process) but a REAL bug once a script calls `jfinish` repeatedly in one session (as the factstream scenario does): getopt's static `optind` carries over from whatever position the PRIOR command's parse left it at, silently causing the 2nd+ call to skip its own `-j`/`-c` flags. Fixed by adding `optind = 1` to `cmd_jobfinish` (matching the majority convention most other `cmd_` handlers already follow) — any new REPL command that might be invoked repeatedly in one script MUST reset `optind` itself; don't assume the previous command's handler already did it for you.
## Key Learning (2026-07-05, raft-apply-decoupling task 7.2) — waldump only decoded jobid for WAL_JOB_SUBMIT; extended it to the flat wal_jobstate_t fact types
`testsuite/waldump.c` only ever decoded `jobid`/`state` for `WAL_JOB_SUBMIT` (capnp Job schema) — every fact type (`WAL_JOB_START/FINISH/FAIL/STOP/RESUME/UPDATE/RUSAGE/CKPT/PREASON`, all sharing the flat native-endian `wal_jobstate_t{jobid,state,_pad}` struct, wal_types.h) printed as `jobid=?`. Added `wal_type_is_jobstate()`/`decode_jobstate_info()` (a bounds-checked `memcpy`, not a parser — same triviality as the struct itself) so these types now print `jobid=<fmt> state=0x%x pad=0x%x` too, letting a caller grep waldump output per-jobid per-fact-type across nodes. This is what makes the factstream scenario's phantom-fact cross-check possible (grep -F "JOB_START" + "jobid=$jid " per node). Generically useful beyond this task: any future "did fact X actually commit for job Y" question can now be answered offline via waldump alone.
## Key Learning (2026-07-05, raft-apply-decoupling task 7.2) — `job_state_str`'s if/else-if priority ordering (RUN checked before EXIT) HIDES bits from the pretty-printed state string
`apply_job_fail_transition` (job.c) merges `JOB_STAT_EXIT|JOB_STAT_PERR` onto the job's CURRENT state via `|=` — it does NOT clear `JOB_STAT_RUN`. So a job that receives a POST_EXEC-stage FAIL WITHOUT a prior FINISH (a legitimate worker-report ordering — "died without ever reporting finish") ends up with `state_raw = RUN(0x2)|EXIT(0x4)|PERR(0x100) = 0x106`, genuinely correct per the merge semantics — but `job_state_str()`'s `if (RUN) return "RUN"` check fires FIRST and never reaches the `EXIT` branch, so `jobinfo`/`joblist` print "RUN", silently hiding that the fail was actually applied. Confirmed via a live standalone probe (3-node cluster, `jstart`+`jfail` with no `jfinish`): `state: RUN` / raw `0x106`. This is a pre-existing, minor, likely-harmless-until-now display quirk (not introduced by raft-apply-decoupling — `apply_job_fail_transition` is an unmodified lift from the original `mark_job_fail`), NOT fixed (out of task 7.2's boundary), but it DOES matter for any script-level check that infers "did a terminal fact apply" from the pretty string — the factstream phantom-check was written against `state:` first and produced a false negative (silently failed to check for a committed JOB_FAIL backing a RUN-displayed job) until switched to a new `state_raw: 0x%x` field added to `print_jobinfo()` (test.c) and bitwise-AND'd against the real `JOB_STAT_*` constants. **Do-not-repeat**: never infer "which facts applied" from a job's pretty-printed state string in this codebase — always use `state_raw` (or the raw bits directly) since the string's priority-ordering can mask co-set bits.
## Key Learning (2026-07-05, bug-1416, raft-apply-decoupling task 7.2) — cluster_init_phase2's persist_recover() apply-correctness verification can false-positive APPLY_DIVERGENT when a returning node's recovery vtask races a compressed catch-up replication burst — FIXED round 2
Found live, reproduced 10/10 times, building the `--factstream` kill-mid-fact-stream HA soak scenario. `cluster_init_phase2()` (cluster.c) calls `cluster_open_listener()` (accepts network I/O, including replicated/catch-up WAL writes) BEFORE `persist_recover()` (the task-5.1 boot-time apply-correctness verification, Req 8.1/8.2) — and `persist_recover()` itself fires from a vtask, not synchronously. `persist_verify_apply_correctness()`'s LIVE reads of `p->last_applied_lsn` and `count_jobmgr_entries()` can race two live mutation paths (the apply pump, `persist_apply_pump_run`/`persist_apply_pump_kick`; and the standby live-replication apply path, `ha_apply_single_entry`/`ha_apply_term_entry_via_library` in ha.c) that write those SAME values concurrently, producing a DIVERGENT verdict on a node that has NOT actually diverged. **FIXED (round 2, review remediation)**: new `_Atomic bool persist_t.recovery_complete` gate (persist.h/persist.c). `persist_apply_pump_kick()` no-ops while closed; `ha_apply_single_entry`/`ha_apply_term_entry_via_library` skip ONLY the in-memory `apply_wal_entry()` call while closed (durability write + ACK + applied_lsn advance stay ungated — mirrors bug-1043's durability/materialization decoupling). `persist_recover()` closes it at entry, reopens it + issues one deliberate catch-up kick before returning. **Round-2-of-the-fix gap, found via live verification, not just code review**: this alone was NOT enough — a node stuck retrying its own election as a losing/timing-out candidate can receive and durably APPLY live-replicated entries via the ungated (`persist_init` default=true) path for tens of seconds BEFORE `cluster_init_phase2()` ever calls `persist_recover()` for the first time, reproducing the identical false-DIVERGENT symptom through an earlier entry point `persist_recover()`'s own bracket can't see. Closed with `persist_mark_recovery_pending(&c.persist)` in `myapp_run()` (myapp.c), called right after `persist_init()`/`persist_start()` succeed and BEFORE `ha_start_election_async()`/`cluster_run()` can process any network traffic — closing the gate from the true earliest safe moment. `persist_init()`'s default stays TRUE so none of this spec's ~20 unit-test fixtures that construct a `persist_t` directly (bypassing `myapp_run`) are affected. Verified: live 3-node `--factstream` soak (2 rounds, kill+restart both) now PASSes clean, zero `APPLY_DIVERGENT` across 5 `persist_recover()` calls; broader `controller_unit_test` sweep (23 groups) shows zero new failures; real-divergence detection re-confirmed intact via the 3 dedicated `test_apply_correctness_verification.c` divergence tests. **Do-not-repeat lesson**: when gating a live path behind a "recovery in progress" flag, the flag's OWN bracket around the recovery function itself is not sufficient if OTHER code paths (election retry, live replication) can mutate the same state BEFORE that function is ever first called — the gate must close at the EARLIEST point in the real process lifecycle traffic could possibly begin, not just around the specific function whose output it protects.
## Key Learning (2026-07-07, raft-apply-decoupling task 7.4 O(n^2) apply-pump fix, CLOSED) — segment-lookup cache on `apply_pump_read_entry` fixes the regression; validated live A/B in one environment
Root cause: `apply_pump_read_entry` (persist.c) redid a full `opendir`/`readdir`/segment-sort + a `pread` scan from byte 0 on EVERY single committed-LSN lookup; its only caller (`persist_apply_pump_run`) calls it once per LSN in strict ascending order with no gaps in the common case, making N contiguous applies O(N^2). Fix: 4 new `persist_t` fields (`apply_pump_cache_fd/seg_lsn/off/next_lsn`, guarded ENTIRELY by `apply_pump_lock`, never `p->lock`) cache the open segment fd + byte offset positioned for the next expected LSN; a fast path does a direct `pread` at the cached offset (hit → O(1), advance cache; miss → close+invalidate, fall through to the UNCHANGED slow path, which also now populates the cache on its own success branch before `fd` gets closed). Invalidated in `persist_init`/`persist_fini`/`persist_recover` (stale pre-recovery cursor) and `persist_truncate_wal_above` (segment could be ftruncate'd/unlinked away) — correctness never depends on the cache being right, only performance does (every miss path is the pre-existing, already-correct slow path). Live A/B in the SAME environment (single-node, release build, `git stash`/`pop` on persist.c/.h to swap fixed vs pre-fix): pre-fix N=2000→1.8s (1111/s), N=4000→13.3s (301/s) = **7.4x elapsed for 2x volume** (confirms superlinear); fixed N=2000→0.52s (3831/s), N=4000→0.76s (5249/s) = **1.46x elapsed for 2x volume**, throughput actually IMPROVING with N (fixed-cost amortization) — clean confirmation of the fix with zero superlinear scaling.
## Do-Not-Repeat (2026-07-07) — the synchronous `jobsub` (COMMIT_MODE_WAIT) request/reply path has a real, reproducible intermittent hang; NOT this task's bug, has its own out-of-scope status
While benchmarking task 7.4's throughput fix, a single-connection burst of sequential `jobsub` REPL calls hung after a handful to a few hundred calls (variable: 7, 150, 778, 1579 — never a fixed threshold), with the SERVER-side commit completing near-instantly (visible in the controller log) but the CLIENT's `cbuf_get` on that connection never unblocking; a BRAND NEW connection to the same server succeeds immediately, so the server itself is not deadlocked. Ruled out REQ_BLOCK admission-cap exhaustion as the cause (raising `--vcpus` 4→32, i.e. cap 3→31, did not help — still hung after 13 calls). This is `jobsub`-specific (COMMIT_MODE_WAIT, synchronous reply-wait); `jfinish`/`jstart` (COMMIT_MODE_RETAIN, fire-and-forget `xce_notify`, no reply wait) never exhibited it even at N=4000 in one burst. Confirmed pre-existing/environmental, NOT caused by the apply-pump fix (reproduces identically on stashed pre-fix binaries). **Workaround, not a fix**: submit jobs in small waves (~20) over FRESH connections with generous per-wave retry — a hung wave doesn't poison later ones, and patient retrying eventually gets 100% through. **Do-not-repeat**: don't waste time hypothesis-testing admission-pool/vcpus tuning for this class of hang without first confirming server-side liveness via an independent fresh-connection probe — that single check (does a NEW connection succeed right now?) instantly distinguishes "server deadlocked" from "this one connection's reply got lost," which have completely different fixes/workarounds.
## Do-Not-Repeat (2026-07-07) — `$EPOCHREALTIME` read inside a polling loop with per-iteration subprocess calls can yield a wildly-wrong before/after diff; capture it as a single simple pair, not inline in deadline arithmetic
A benchmark script computed elapsed time via `$EPOCHREALTIME` timestamps read inside a `while` polling loop that also computed its own timeout deadline via `awk` on every iteration — it printed a "commit-poll timeout after 180s" message in the SAME run whose reported `commit_elapsed` was 0.176s, an impossible combination. `$EPOCHREALTIME` itself is fine in isolation (verified: bash 5.3, two consecutive reads show a proper sub-ms-resolution increment) — the bug was in the surrounding script structure (mixing a `done_count`-vs-target break condition, a separate deadline-exceeded break condition, and per-iteration subprocess/awk calls all touching timing state), not the variable. **Fix that worked**: use bash's plain integer `SECONDS` builtin (`SECONDS=0` before, read `$SECONDS` after) for the loop's OWN deadline/elapsed bookkeeping, and take a SEPARATE single `t_start`/`t_end` `$EPOCHREALTIME` pair (captured once, diffed once via one final `awk` call) purely for sub-second-precision reporting. **Do-not-repeat**: never trust a timing result that contradicts an adjacent diagnostic message (e.g. a "timeout after Xs" alongside an elapsed value ≪X) — that contradiction itself is the tell that the measurement code (not the system under test) is broken; re-derive elapsed time via the simplest possible mechanism before trusting the number.
## Do-Not-Repeat (2026-07-16) — plain `date +%s%N`/`date +%s%3N` is UNSAFE in this environment: BusyBox `date` silently passes an unrecognized `%N` through as a literal string (rc=0, no error) instead of failing
Fixing task 6.1's array-soak replay-time measurement (whole-second `date +%s` collapsed both sub-rounds to "0s", making the ratio math report `n/a`), the natural fix looked like switching to `date +%s%3N` for millisecond resolution. Verified this is UNSAFE here: this environment's plain `date` resolves to `/sbin/date` (BusyBox, ahead of `/bin/date`→GNU-coreutils on `$PATH`, since `/sbin` precedes `/bin`), and BusyBox's `date` does not support `%N` — it passes it through as a literal substring (`date +%s%3N` => `1784175723%3N`, exit code 0, no error) instead of failing loudly. Any downstream `$((...))` arithmetic on that string would break or silently misbehave. **Fix that worked**: use bash's own `$EPOCHREALTIME` builtin (no subprocess, no `$PATH`/coreutils-vs-BusyBox dependency at all) for the sub-second timestamp, split on `.` and use `10#${frac:0:3}` to force base-10 on the (possibly leading-zero) millisecond slice, combined with the existing `SECONDS`-builtin pattern for the loop's own timeout/budget bookkeeping (see the adjacent 2026-07-07 EPOCHREALTIME entry for why those two must stay separate). **Do-not-repeat**: never add `date +%N`/`%3N`/`%s%N` to a script that must run in this sandbox — check `command -v date` / `type date` first, or just use `$EPOCHREALTIME` instead; the failure mode (silent garbage string, not a loud error) makes this easy to miss until arithmetic breaks downstream.
## Do-Not-Repeat (2026-07-07) — a joblist/committed-count poll loop must scope its match to THIS run's own job IDs, never a cluster-wide state count
A benchmark script counted `done_count` as "every joblist row with state DONE" — on a controller that had accumulated DONE jobs from a PRIOR run in the same process lifetime, this produced an instant false-positive "already done" (0s elapsed) the moment the pre-existing DONE count alone exceeded the new batch's N, even though none of the NEW batch's facts had been sent yet. Fixed by writing the batch's own job IDs to a file and using `awk`'s `getline` to build a lookup set, counting only `($1 in ids) && $4=="DONE"`. **Do-not-repeat**: any poll-for-completion loop against a stateful, non-reset-between-runs server must scope its match to the specific IDs/records THIS run created — a bare aggregate count silently double-counts history from earlier runs against the same live process.
## bug-1436 FIXED (2026-07-12) — a test-harness process-collision bug, not a product defect: unscoped `pgrep`/`pkill` in run-ha-soak.sh let one soak invocation silently SIGKILL a DIFFERENT invocation's live cluster
Investigated as a suspected controller/libxce ECONNRESET bug (`wait-primary` succeeds against the freshly-elected primary, then the very next `expect-job`/joblist call gets a raw "Connection reset by peer" — same round-36 signature that bug-1435 had, but with no `E_XCE_STANDBY` anywhere and the queried node "confirmed alive" in its own log). Extensive static analysis of the client/server request path (io_classify_io_error's deliberate ECONNRESET-on-clean-close sentinel from bug-648, `xce_reply_get`'s chunked-get wake-on-last-chunk protocol via `extract_data`, `ha_dispatch_gate`'s opcode whitelist, `myreq_handler`'s bug-1286 NULL-owner guard, `vtask_handle_msg`/`run_task`'s discarding of the handler's return code, the accept path in `handle_connection`, the LivenessAdmission REQ_BLOCK gate) found EVERY mechanism behaving correctly — none implicated. The actual break came from archaeology on the deploy directory's OWN controller logs: the queried node's log showed normal request processing (a jobsub ack) followed immediately by a connection-teardown trace line and then TOTAL SILENCE for ~20 seconds (no heartbeat/RAFT log lines at all — a live process emits one every ~3s regardless of client traffic; no shutdown/self-terminate log, since controllers never self-exit here) until a BRAND NEW PID cold-started from disk (`restored term=N from ha-term.bin`) — the unambiguous signature of an EXTERNAL SIGKILL, not a crash (zero ASAN/SIGSEGV/abort) and not a graceful stop. Root cause: `run-ha-soak.sh`'s `cleanup()` did `pkill -9 -x xce_controller` — matched on BINARY NAME ONLY, called unconditionally at the START of every deploy, on every EXIT/INT/TERM trap, and inside `fail()` — and every other process lookup (`kill_node`, `start_node`'s bug-1418 dup-launch guard, `wait_for_full_convergence`'s liveness `pgrep -c`, both round-loop primary-kill sites) matched only `--myname $node`, likewise unscoped by deploy directory. Two `run-ha-soak.sh` invocations (confirmed directly: this deploy dir's logs and a scratchpad transcript showed a SECOND, unrelated chaotic-mode soak's own deploy sequence launching against the SAME hardcoded node names ctrl-1/ctrl-2/ctrl-3) are therefore indistinguishable to pgrep/pkill by process name alone — the second run's own startup `cleanup()` silently killed the FIRST run's entire live cluster, including the primary the first run's `wait-primary` had legitimately just confirmed serving 46ms earlier; the client's very next request then correctly, faithfully reported a genuine ECONNRESET (the process really was gone). The 20-attempt failover retry budget couldn't ride this out because detecting a dead primary + electing a new one needs the survivors' full liveness timeout (~10s, seen elsewhere as `no primary activity for 10s (timeout=10s)`), far longer than `verify_no_loss`'s ~4-6s total retry window — explaining the "sustained ~4-6s unavailability" that earlier looked like a real server-side stall. The "round-36 recurrence across two different bugs" was coincidental wall-clock cadence between two independently-dispatched 40-round runs of similar per-round duration, NOT a resource-accumulation effect (that hypothesis was checked and found unsupported — no small fixed-size pool/counter anywhere in the connection path). **FIX**: added `node_pattern()`/`run_pattern()` helpers scoping every pgrep/pkill to THIS run's own `--workdir "$TOP/controller"` (unique per deploy dir, already present in every node's argv) instead of binary-name/`--myname`-only matches — verified directly (grep -E against synthetic argv) that a same-named node in a DIFFERENT workdir no longer matches. Two runs sharing the SAME deploy dir (and hence the same hardcoded ports 9991-9993) still can't usefully coexist, but now fail LOUDLY with a bind error instead of one silently killing the other's cluster. **Do-not-repeat**: while investigating a "confirmed alive" server-side log, always check the LOGGED PID stays constant across the window you're relying on, not just that the log file keeps receiving new lines — a silently-restarted process (new PID, e.g. from an external SIGKILL) looks identical to "still alive" if you only tail the file; and when a test harness kills processes by NAME (`pgrep -f`/`pkill -x`) rather than by a handle it created itself (a saved PID, a PID file, a directory-scoped argv match), any concurrent second invocation of that same harness — even against a different target directory — can and will collide. This is a whole class of test-infra risk distinct from the product-code raft-drive-async class of bugs (bug-1197/1198/1417/1435): always rule out "a different process is interfering with mine" before concluding a client/server code path is broken when the observed symptom is transport-level (ECONNRESET/ECONNREFUSED/hang), especially in a shared, long-lived sandbox where many prior soak deploy directories accumulate (`/home/claude/tmp/xce-test-ha-*`) and can still have live or stale processes.
## bug-1436 RE-OPENED then REALLY FIXED (2026-07-12, later same day) — the process-collision theory above was DISPROVEN by direct re-test; actual root cause is a status-code/row-count magnitude collision in the client failover classifier
The FIXED entry immediately above (process-collision, `run-ha-soak.sh` unscoped pkill) is **not wrong as a fix** — the workdir-scoping change is real and stays — but it was **not the (sole) root cause**: a fresh, fully-isolated 40-round soak (no concurrent soak running, verified via `ps aux` before/during/after) FAILED AGAIN at round 36/40 with the byte-identical signature, and the target primary's own log proved via constant PID + unbroken RAFT/heartbeat stream that it was NEVER killed. Per the user's explicit instruction this time ("go live, don't re-do log archaeology"), reproduced it a SECOND time live (deploy dir `xce-test-ha-40-bug1436-live`), with a parallel 1s-resolution process/connection forensic poller running throughout to independently confirm process continuity. `tcpdump`/raw-packet capture was UNAVAILABLE in this sandbox (no CAP_NET_RAW, confirmed via `tcpdump: You don't have permission to perform this capture on that device` even after fetching the binary via nix) — substituted with tight, fast synthetic repro harnesses (hammer2.sh/hammer3.sh, ~7000 combined get-class calls with periodic real primary kills) to try to shrink the reproduction window, THEN fell back to the real `/xce-test-ha` 40-round soak per the sanctioned fallback when the synthetic harnesses came back clean. **The synthetic harnesses' clean result was itself the key clue in hindsight**: they held the tracked job count FIXED (60 and 495 respectively) for their whole run, and the real bug requires the job count to pass through one EXACT value — a fixed-count harness, no matter how much connection-churn volume it generates, structurally cannot hit it. Direct `-vvv` log correlation on the live reproduction (no debugger/strace needed once the right angle was found) showed the smoking gun for the WHOLE 'verify' client session: `cluster.c:786 server sock 4 accepted 10` (a brand-new TCP connection) immediately before EVERY SINGLE `opcode get` request, followed by `cluster.c:64` (`handle_endpoint_error`'s TRACE line, i.e. connection teardown) immediately after each reply — ~1799 accept/serve/close cycles on ONE short-lived primary PID's ~90s lifetime. This is not a misreading; it is the client reconnecting on (nearly) every single, mostly-successful `get` call. **Real mechanism**: `xcecli_joblist`/`xcecli_reslist`/`xcecli_workerlist`/`xcecli_resval`/`xcecli_jobinfo` are all built on `xce_get_with_condition` (xceapi.c), which returns a non-negative ROW COUNT on success (`rc = getreply.nrows`) — not the uniform "0 = success" status-code convention every other `xcecli_*` wrapper follows. All five were routed through the generic `XCECLI_FAILOVER_CALL` macro, which unconditionally feeds the raw EXPR return value into `xce_failover_classify()` — a function that recognizes retriable/terminal outcomes purely by MAGNITUDE, deliberately accepting POSIX errno values in either sign via `fo_abs()` (ECONNRESET=104, ETIMEDOUT=110, ECONNREFUSED=111 on Linux). A joblist reply whose row count happens to numerically equal one of these is silently misclassified as a transport failure on EVERY call with that exact count — forcing a spurious `xce_failover_rotate()` even though the read fully succeeded; landing on a standby (get-class ops are primary-only) correctly-but-pointlessly fails with `E_XCE_STANDBY` and rotates again, repeating for every subsequent same-sized call and burning the finite 20-attempt retry budget on self-inflicted churn until it coincides with a genuine transient (e.g. a peer mid-restart right after the round's own real election) and exhausts the budget while mid-rotation, surfacing a REAL `ECONNRESET`/`ECONNREFUSED` — matching "xce_joblist: Connection reset by peer" exactly. `run-ha-soak.sh` submits `NJOBS=3`/round, so the tracked count follows `3+3*round`; of the three dangerous magnitudes only 111 is a multiple of 3, and it is hit for the FIRST time at EXACTLY round 36 (`3+3*36=111`) — explaining both the rarity (needs an exact numeric collision, not a generic race) and the suspiciously-exact round-36 recurrence across unrelated runs. **FIX**: added `XCECLI_FAILOVER_CALL_COUNT` (client_internal.h) — only enters the retry/classify loop when the raw EXPR result is NEGATIVE; any non-negative value (0 or a row count of ANY size) is unconditional success, never fed to the magnitude classifier. Switched the 5 count-returning call sites in client.c to it. Left `xce_failover_classify` and the other 14 `XCECLI_FAILOVER_CALL` call sites (which genuinely follow the 0/status convention) untouched — confirmed via `test_libxce_retry.c`'s existing StatusClassifier taxonomy tests, still green. controller_unit_test 926/0/0, xcelib_test 293/0/0. Verification: fresh 40-round soak (`xce-test-ha-40-v4`) — see below for outcome once confirmed.
## Do-Not-Repeat (2026-07-12, bug-1436) — a retry/failover classifier that inspects a raw return value's MAGNITUDE is only safe if EVERY call site's success convention is uniform; a count-returning success value is a landmine
When a generic retry/failover wrapper (here `XCECLI_FAILOVER_CALL` → `xce_failover_classify`) decides retriable-vs-terminal by comparing the wrapped call's raw return value against known error-code magnitudes, it is implicitly assuming EVERY caller follows the SAME "0/negative = status" convention. A single outlier call site whose SUCCESS value is a variable, unbounded quantity (a row/byte/item COUNT, a sequence number, anything that legitimately grows without bound) is a landmine: it will almost always pass through by luck, and then fail in a way that looks like a rare, mysterious transport race exactly when the count happens to numerically collide with a recognized error magnitude — which for small POSIX errno values (ECONNRESET=104, ETIMEDOUT=110, ECONNREFUSED=111) is entirely plausible for ordinary list/count sizes seen in production. **Do-not-repeat**: before wrapping ANY call in a shared retry/classify macro, verify its ACTUAL success-value convention (grep the real implementation, not just the header doc comment) — "returns 0 on success" and "returns >=0 on success (a count)" look identical at a single call site with a small test fixture but diverge catastrophically once the count crosses into three-digit territory. Also: **a synthetic repro harness that holds a size/count parameter FIXED for its whole run cannot reproduce a bug that requires that value to pass through one exact number** — when two well-designed high-volume repro attempts (7000+ calls combined) come back clean against a bug the real system hits reliably, treat "the repro parameter never varies through the danger zone" as a live hypothesis, not just "try more volume."
## bug-1435 FIXED (2026-07-12) — raft_state_t is unprotected against concurrent pool-dispatched vote-reply processing; a promoted PRIMARY can silently self-revert to STANDBY mid-election with no further trigger
Post-failover `expect-job`/`joblist` calls got a genuine, persistent `-XCE_ERR_STANDBY` (10014) from the client's OWN just-confirmed primary (`wait-primary` had verified it moments earlier). Ruled out EVERY client-side theory first (this took most of the investigation and is the reusable lesson): `ha_dispatch_gate`'s `CTRL_PRIMARY` branch structurally cannot return STANDBY for `OPCode_get` (read the whole switch — confirmed only `CTRL_STANDBY: default:` returns it), so the answering node's `ha->role` was GENUINELY `CTRL_STANDBY` — this was never a libxce failover/retry-budget/stale-reply/cur_master-cursor bug (`XCECLI_FAILOVER_CALL`'s RETRY_NOW classification for `E_XCE_STANDBY` with `max_retries=20` and zero backoff working exactly as designed; final rc staying `-E_XCE_STANDBY` on exhaustion is DOCUMENTED behavior, not evidence of a broken loop). The missing "xcecli failover: attempt N" INFO logs (zero matches across a 40-round run) is a RED HERRING explained by `wait-primary`'s own `xcecli_ctrlstatus_primary` doing a SEPARATE, silent redirect-follow loop that never touches the shared `xcecli_retry_next` logging path — don't chase an absent log line as a clue without first confirming which code path is actually supposed to emit it. REAL mechanism: raft-drive-async's `peer_ctrl_dispatch_to_pool` (ha.c, `VTASK_LIVENESS`, default ON via `g_drive_async_peerctrl_dispatch_enabled=1`) dispatches each peer's message as an INDEPENDENT vtask onto the shared pool; its per-endpoint serialization (`bind_vtask`) only orders messages on the SAME endpoint — it does NOT serialize DIFFERENT peers' endpoints against each other. So two `RequestVoteReply`s from two different voters CAN run `raft_on_vote_reply(&ha->raft, ...)` CONCURRENTLY on two different pool workers, racing on `raft_state_t`'s PLAIN (non-atomic) `votes_granted`/`replies_received`/`election_decided` — one worker reaches quorum and fires `on_become_leader` (role→PRIMARY), the OTHER, racing on a pre-increment read, can independently satisfy the library's "all replied, no quorum" branch and fire `on_become_follower` (role→STANDBY) for the SAME term an instant later, with NOTHING ever correcting it back — every subsequent request then correctly, faithfully gets rejected as STANDBY forever. Needed ~36 rounds to surface (narrow, load-dependent overlap window). **FIX**: added `ha->raft_lock` (new dedicated `pthread_mutex_t`, ha.h/ha.c) serializing every mutating/reading raft_state_t entry point on the ELECTION/VOTE path (`raft_on_vote_reply`, `raft_on_vote_request`, `raft_on_pre_vote_request`, `raft_on_pre_vote_reply`, `raft_begin_pre_vote`, `raft_start_election`, `raft_check_and_start_election` x3, `raft_on_election_timeout`, `raft_on_higher_term_observed` x2 — 12 call sites across ha.c + cluster.c). **MUST be PTHREAD_MUTEX_RECURSIVE, not plain**: `raft_on_pre_vote_reply`'s WON branch synchronously calls `ha_enter_real_election → ha_start_election_async → raft_start_election` on the SAME thread within one call chain — a plain mutex self-deadlocks on that legitimate nesting (same class as the precedent bug-652 ha->lock self-deadlock-on-synchronous-re-entry). Deliberately did NOT touch the AE/commit-replication path (`raft_log_append`/`raft_on_append_entries_reply`/`raft_advance_commit`) — `ha_primary_log_append→raft_log_append` is ALSO reachable from concurrent client-write vtasks with no visible lock (persist.c's `persist_wal_sync_append` calls it before acquiring `p->lock`), a plausible SEPARATE, analogous concurrency gap, but out of scope for this bug's demonstrated mechanism and higher regression risk to touch under time pressure — flagged as a follow-up, not silently dropped. Verified no ABBA: grepped+read every existing `ha->lock`-held critical section (9 sites) and confirmed none call into any `raft_*` function, so lock order is always `raft_lock`-then-`ha->lock`, never reversed. **Do-not-repeat**: when a callback-invoking library assumes single-threaded/serialized calling (the Raft "single event-loop" model is a classic example) and a later "drive it async on a thread pool" feature is bolted on (raft-drive-async here), the CONVERSION must include a lock around the library's own internal state, not just per-endpoint delivery-ordering (`bind_vtask`) — per-endpoint serialization silently does NOT imply serialization of the shared struct across different endpoints/peers. This is a whole CLASS of bug (bug-1197 UAF, bug-1198 admission-gating, bug-1417 index-mapping, and now bug-1435 unsynchronized state) that all trace back to the same raft-drive-async conversion — audit any OTHER shared struct the async-dispatched callbacks touch before declaring that conversion complete. CONFIDENCE: high on mechanism (every original-investigation fact is fully explained, zero residual unexplained facts), but NOT live-reproduced under a debugger in this session (timing-rare race) — a well-evidenced static diagnosis, not a repro-confirmed one; said so explicitly rather than overclaiming certainty.
## Do-Not-Repeat (2026-07-15, bug-1440) — a `_test.go` file can NEVER directly `import "C"`, regardless of CGO_ENABLED/nix env/build tags; this is a hard Go toolchain rule, not a build-config problem
`src/frontend/apiserver/backend/wire_dispatch_test.go` (new, `//go:build cgo`) hand-builds real C structs (`xce_job_submit_t`, `xce_data_t`, `xce_request_t`) via `import "C"` directly inside the `_test.go` file itself, to drive the real `C.xce_encode`/`C.xce_decode`/`C.extract_data`/`go_push_handler()` chain. This fails immediately with `use of cgo in test wire_dispatch_test.go not supported` under both `make test-backend` and a manual `CGO_ENABLED=1 go test ./backend/... -tags cgo`. Confirmed by reading the actual installed Go 1.25.5 source in the nix shell (`$(go env GOROOT)/src/cmd/go/internal/modindex/read.go` ~line 589): the module indexer explicitly special-cases `imp.path == "C"` when `isTest` is true and calls `badGoFile(...)`, unconditionally — this is golang/go#13938, a permanent Go toolchain restriction with NO environment/flag workaround. CGO directives (`/* #include ... */` comments) in a test file are NOT what triggers it — only the literal `import "C"` statement is. Since 4 of this package's pre-existing test files carry no build tag at all (`handler_test.go`, `convert_test.go`, `peers_test.go`, `heartbeat_test.go` — they build under both cgo and !cgo), one bad cgo-in-test file poisons the WHOLE `apiserver/backend` package build under CGO_ENABLED=1 (`[setup failed]`), silently taking those otherwise-unrelated tests down with it; the `!cgo`-tagged files (`integration_test.go`, `control_host_test.go`, `stub_funcs_test.go`) and the plain `make test` (CGO_ENABLED=0) path are unaffected since the poisoned file is excluded by its own `//go:build cgo` tag in that mode. **Do-not-repeat**: any new CGO-touching Go test in this codebase must put the `import "C"` and all `C.xxx` calls in a plain (non-`_test.go`-suffixed) `//go:build cgo` helper file in the same package, and have the actual `_test.go` file call into pure-Go-signature wrapper functions only — this is the same `!cgo`/`cgo`-split pattern already established for clib.go/convert.go/convert_pure.go, just also required WITHIN the cgo side to keep the literal `import "C"` out of any `_test.go` filename. Because the toolchain rejects the file before type-checking, a fix along these lines may still surface FURTHER errors (undefined C symbols, type mismatches) that were never reached in this run.
## Do-Not-Repeat (2026-07-17) — `xce-build.sh all` / plain `meson test` can report a suite as TIMEOUT purely from meson's default parallel test scheduling, not a real hang/regression
During bug-1445's post-fix regression pass, `nix develop ./src --command bash src/tools/xce-build.sh all` reported `controller test` as a 300s TIMEOUT under meson's default *parallel* scheduling (multiple ASAN-instrumented binaries — lib/controller/scheduler suites — contending for CPU simultaneously on the sandbox's shared cores). Also independently observed during the controller-status validation pass: a genuine ASAN SEGV in an unrelated test file (`test_persist_truncate.c`) traced to a SECOND, concurrent session running its own build+HA-soak against the SAME worktree/build directory at the same time (confirmed live via `ps aux`). **Do-not-repeat**: before concluding a code regression from an "all"-scope build+test run that times out or crashes in a way that doesn't reproduce per-file, (1) check for a competing process against the same build dir (`ps aux` / `pgrep -af meson\|ninja\|xce_controller`), and (2) rerun the specific suite in isolation, and if still ambiguous rerun the full suite serially (`meson test --num-processes 1` from the build dir) — a serial rerun matching the isolated timing exactly (both incidents here: clean 9/9, controller test ~34s, matching its solo-run time) confirms resource contention, not a real hang or crash, before spending more time chasing a phantom regression.
## Key Learning (2026-07-17) — `/tmp/xce_*` WAL test-fixture dirs accumulate fast across repeated ASAN cycles; same root pattern as bug-924
Found 2089 leftover `/tmp/xce_*` directories (~89GB) during the bug-1445 regression pass, left behind by C testsuite fixtures that `mkdtemp("/tmp/xce_*_XXXXXX")` a workdir and don't always clean it up on every exit path (crash, ASAN abort, or a test killed mid-run). This is disk-pressure buildup of the same shape as bug-924 (overlayfs fdatasync wedge under accumulated pressure), not a new bug — just a maintenance reminder. **Key learning**: periodically `rm -rf /tmp/xce_*` between heavy test cycles in a long-lived sandbox session, especially after any run that included ASAN aborts/kills (which skip the test's own cleanup code) — cheap to do, and left unchecked it can eventually degrade or destabilize unrelated test runs sharing `/tmp`.
## Fixed (2026-07-17) — `controller_unit_test`'s nested `UNITY_BEGIN()`/`UNITY_END()` calls masked most of the suite's exit code; fixed suite-wide via `UnitySetTestFile`
Root cause of the 2026-06-11 Unity-quirk note (file misattribution) turned out to be worse than cosmetic: only 21 of 86 `test_*.c` group-function files called their own nested `UNITY_BEGIN()`/`UNITY_END()` (each call resets `Unity.NumberOfTests`/`TestFailures` globally); the other 65 correctly relied on the single outer `UNITY_BEGIN()`/`return UNITY_END();` pair in `controller_test.c:main()`. Net effect: `controller_test.c`'s final `return UNITY_END();` — the value `meson test` actually gates on — only ever reflected whichever small window of tests ran since the LAST nested reset (empirically 16 of 972 total tests in one observed run), silently discarding pass/fail evidence for the other ~956. **Fix**: replaced every nested `UNITY_BEGIN();` with `UnitySetTestFile(__FILE__);` (sets the file-attribution pointer only, no counter reset — `unity_internals.h:546`, already visible via the existing `#include <unity/unity.h>`) and deleted the corresponding `UNITY_END();`; added the same `UnitySetTestFile(__FILE__);` as the first statement to the other 65 group functions that had neither. Left the 2 genuinely-standalone binaries (`test_job_array_sched_push.c`, `test_job_array_apiserver_push.c` — each has its own `int main()`, not registered via `controller_test.c`'s `TEST()` macro) untouched. Verified both effects: (1) file attribution — every `PASS`/`FAIL` banner now names its own true source file; (2) exit-code integrity — the single final summary now reads `972 Tests 0 Failures 0 Ignored OK`, exactly matching the ground-truth `grep -c RUN_TEST(` across all 83 registered group files (976 total RUN_TEST call sites minus 4 belonging to the 2 standalone binaries = 972). Full rebuild + `xce-build.sh all` regression pass clean. **Key learning**: `UnitySetTestFile()` (distinct from `UnityBegin()`, which calls it internally AND resets counters) is the correct primitive when a codebase wants per-file test-source attribution inside a single shared Unity counter/exit-code scope spanning multiple group functions in one binary — never call `UNITY_BEGIN()`/`UNITY_END()` from more than one place per test binary.
## Key Learning (2026-07-18) — ha-entity-snapshots validated GO; task 7.1's "pending upstream snapshot-boundary recon" blocker was a REAL, since-resolved cross-spec dependency, not a stale checkbox
Before trusting tasks.md's `[x] 7.1`, traced the blocker's full lifecycle: commit `3fab291` explicitly BLOCKED task 7.1 citing "committed_divergence is upstream (raft-ha-core-safety task 3)"; that upstream task ("Snapshot-boundary reconciliation", raft-ha-core-safety tasks.md task 3, `[x]`) plus its related task 5.10a were later CLOSED with a real investigation chain (buglog + tasks.md 5.10a note, 2026-07-01/02): a `walcompare` false-positive (no per-member checkpoint-floor awareness, fixed `f3c86a6`), a delaymgr seconds/ms deadline-type-confusion starvation bug (fixed `b9ad018`), a hardcoded 64MB WAL-segment-rotation stall, and a netio.h `INVOKE_MSG_HANDLER` UAF during concurrent snapshot-transfer+election — culminating in a live `--force-snapshot` soak (`committed_divergence=0 committed_job_loss=0 result=PASS`, confirmed real snapshot install) plus a clean 3-round `--chaotic` regression (commit `1439e26`). Only after confirming this closure chain (git log grep for the feature name + the blocking-note's own cited upstream task, not just trusting the checkbox) did full validation proceed: build 192/192, controller suite 962 PASS/0 FAIL (ANSI-stripped, incl. all 9 test_persist_snapshot_entities.c cases), cross-task wiring (`out_entities_restored`, `snapshot_inflight_lsn[]`) verified end-to-end by direct code read against design.md's diagrams, File Structure Plan match exact, requirements traceability complete (12/12 line items). GO written to spec.json. **Key learning**: a task marked `[x]` with a documented external "blocked on upstream X" history needs the upstream spec's own tasks.md/buglog evidence checked before trusting the checkbox — don't assume either (a) blind trust in `[x]`, or (b) that any "phase mislabeling" pattern seen elsewhere in the project (ha-consistency-invariants) automatically applies here; each blocker needs its own resolution evidence traced.
## Key Learning (2026-07-18) — `ready_for_implementation: false` with no `blocked_on` is often just a never-flipped flag, not a real blocker; check tasks.md completion + git log before assuming supersession
libxce-failover spec.json showed all 19 tasks [x] (done since 2026-06-18) but `phase: tasks-generated` and `ready_for_implementation: false` with zero explanation. Investigated whether it was superseded by later HA specs (ha-repl-hardening, raft-drive-async, raft-library) touching similar failover-client territory — it was NOT: git log shows 11 dedicated feature commits (foundation through bug-1436 fix) that later specs build ON TOP of, not duplicate. This is the SAME administrative-staleness pattern already seen in ha-term-safety and controller-status spec.json (both also "first post-implementation validation run, all tasks [x], flag never flipped"). **Do-not-repeat pattern for triage**: `ready_for_implementation: false` + all tasks `[x]` + no `blocked_on` field is a strong signal to check (1) git log for feature-tagged commits proving real implementation happened, (2) whether a live/E2E validation entry already exists in .wolf/memory.md for that spec, (3) whether later specs reference it as a dependency rather than a replacement — before concluding it's blocked or dead. Sibling spec.json files (ha-term-safety, controller-status) are the reference shape for the `validate_impl: {decision, validated_at, note, known_limitations}` block used to close this out.
## Key Learning (2026-07-18) — ha-commit-durability validated GO after re-verifying the core invariant survived heavy adjacent churn, NOT by trusting the 2026-06-18/07-02 evidence alone
Same "all tasks [x], phase stuck at tasks-generated, no validate_impl" administrative-staleness pattern as ha-entity-snapshots/libxce-failover, but this one had a real reason for extra scrutiny: persist.c/ha.c/cluster.c/commit.c (the exact files this spec owns) received substantial edits AFTER the spec's own live validation (2026-06-18 durability fix, 2026-07-02 cold-restart-all real-fs revalidation) — notably bug-1433/1434 (2026-07-11, added `persist_committed_lsn_advance_atomic` CAS-safe committed_lsn advance + removed a force-flush call from the promotion path) and bug-1445 (2026-07-17, a first-boot promotion-check race in the SAME `raft_cb_become_leader` function this spec's promotion-eligibility gate lives in). Resolved by (1) code-level re-verification: `majority_candidate_locked` still caps self at `persist_durable_lsn`, and the newer CAS helper's own code comments explicitly state it preserves "committed_lsn never advances past durable_lsn" — later fixes were written TO respect this spec's invariant, not bypass it; (2) a FRESH live re-run this session (not reused from the stale record) on genuine ext4 (`/home/claude` confirmed backed by `/dev/nvme0n1p2 ext4`, NOT the container's overlay `/`): a 4-round rolling-SIGKILL soak + cold-restart-all, 8/8 PASS, 0 job loss. **Do-not-repeat pattern**: before accepting old live evidence as sufficient for a validate_impl GO, check `git log --oneline -- <owned files>` since the evidence's date — if there's a density of unrelated-feature commits touching the SAME files (not just the same subsystem), don't just infer safety from commit-message claims; re-run at least a short version of the spec's own live check on the CURRENT build.
## Do-Not-Repeat (2026-07-18, bug-1447) — SSL* objects aren't thread-safe; multi-threaded epoll dispatch touching the same endpoint from two io_worker threads crashes deep inside OpenSSL, not at an obvious null check
raft-drive-async sequential 10-round HA soak crashed round 9/10: ctrl-3 (standby) died to an ASAN SEGV (addr 0x40) in ssl3_read_bytes <- tls_get_message_header <- state_machine <- handle_ssl_handshake (src/lib/netio.c:548) <- io_handler <- io_worker, 50ms after ctrl-2's promotion mid reconnect-offload burst (Task 2.1/2.2 ha_connect_prep_peer/ha_connect_finish_peer/reconnect_lane_fn). Ruled out simpler causes by reading code directly, not guessing: ssl_handshake_done resets to 0 on every connect (no stale flag), SSL_new(client_ctx) is fresh on every prep (no cross-fd reuse). Leading hypothesis: unsynchronized concurrent SSL_do_handshake/SSL_read on the same ep->ssl from two io_worker threads (classic multithreaded-epoll double-dispatch) — the crash is a deep, plausible-pointer-chain SEGV inside OpenSSL's state machine, not an immediate null-ssl crash at the top of SSL_do_handshake, which is the fingerprint of a race, not a simple logic bug. Same defect family as fixed bug-1282 (ABBA deadlock netio.c), bug-1286 (NULL ep->owner), bug-1298 (endpoint UAF refcount) — newly exposed by raft-drive-async's own faster reconnect cadence. **Do-not-repeat**: when a crash lands deep inside a non-thread-safe C library's internal state machine (OpenSSL SSL*, similar libs) under a multi-threaded epoll/io_worker dispatch loop, suspect concurrent same-object access before anything else — verify no two threads can be handed the same fd/endpoint's readiness event simultaneously (EPOLLONESHOT/EPOLLEXCLUSIVE, or a per-endpoint mutex around the handshake+read+write path). Logged bug-1447 (NOT-FIXED, blocking); NO-GO written to raft-drive-async spec.json validate_impl with 5-step remediation (reproduce, instrument thread-id+ep-pointer logging, serialize per-endpoint, add regression test, re-run soak to clean 10/10).
## CORRECTION + FIX (2026-07-18, bug-1447 CLOSED) — the "two io_worker threads" hypothesis above was WRONG; the real bug is a SAME-THREAD, SAME-EPOLL-BATCH use-after-free, and it needed no thread race at all to reproduce
Read `iomgr_start` (netio.c) closely before instrumenting anything: it does exactly ONE `pthread_create(&iomgr->worker, ..., io_worker, iomgr)`. A controller process has exactly one iomgr/one io_worker thread for all peer connections — there is no second io_worker thread to race against, so the multithreaded-double-dispatch hypothesis in the entry above is retracted. The real mechanism: `io_worker`'s epoll loop dispatches every fd from ONE `epoll_wait()` batch in a plain for-loop over `io_handler()`, holding no reference on any of them. `ha.c`'s `on_standby_connected` (this spec's own Task 2.1/2.2 reconnect-completion callback) evicts a stale duplicate outbound peer channel via `iomgr_unmonitor()`+`endpoint_unref()` when a fresher reconnect lands — but `epoll_ctl(EPOLL_CTL_DEL)` only stops a FUTURE `epoll_wait()` from returning events for that fd; it does NOT invalidate an event for that SAME stale endpoint already sitting in the CURRENT batch's stack-local `events[]` array (very plausible during a reconnect burst — several peer fds become ready close together right after a new-primary election). `endpoint_unref()` hitting refcnt 0 frees the endpoint (and `SSL_free`s its SSL object) immediately; the loop's NEXT iteration then reads that freed memory in `io_handler`, matching the observed deep OpenSSL SEGV exactly (a freed-but-plausible pointer chain, not an immediate null-ssl crash). Proved this INSTEAD OF the thread-race theory with a deterministic unit test (no live soak, no timing luck needed): `test_netio_batch_dispatch_uaf.c` drives `io_worker`'s real batch-dispatch loop (new seam `io_worker_dispatch_batch_for_test`) with a synthetic 2-event batch where event 0's handler `endpoint_unref()`s event 1's endpoint — confirmed RED (ASAN heap-use-after-free at netio.c:587, freed at netio.c:843) before the fix, GREEN after. **Fix**: extracted `io_worker_dispatch_batch()` (netio.c ~line 639) which now takes `endpoint_ref()` on every endpoint in the batch BEFORE dispatching any of them and `endpoint_unref()`s each one only AFTER the whole batch is processed — same lock-free bracket idiom already used for the vtask dispatch path (bug-1298) and `INVOKE_MSG_HANDLER`, just never applied to io_worker's OWN dispatch loop before. Verified: full lib suite 294/0/0, controller suite 972/0/0 (zero ASAN hits), two independent clean sequential 10-round soaks (33 and 36 jobs, zero loss, all 3 nodes led each run) with an explicit post-hoc grep for the crash signature finding zero matches. **Do-not-repeat**: before chasing a "concurrent access from two threads" hypothesis for a crash inside a single-threaded-by-design event loop, verify the actual thread topology first (`grep -n pthread_create` on the suspected loop's `_start`/`_init` function) — a same-thread, same-batch UAF (an earlier event's handler freeing a struct a LATER event in the identical `epoll_wait()` batch still points to) produces an IDENTICAL-looking deep-library-internals SEGV to a genuine cross-thread race, but is far more likely whenever the crash trigger is "a burst of several connections churning close together" (reconnect bursts, election transitions) rather than sustained steady-state load, and is trivially reproducible with a synthetic 2-event batch instead of a multi-minute live soak. `INVOKE_MSG_HANDLER`-style "ref myself for the duration of my own handler" brackets do NOT protect a DIFFERENT endpoint that handler's side effects tear down — that gap is invisible unless you specifically ask "what if this handler frees something ELSE the batch already has queued."
## Key Learning (2026-07-18) — ha-wal-replication validated GO right after raft-drive-async's bug-1447 NO-GO; a shared-codebase crash from a sibling spec's soak does NOT automatically block an adjacent spec's own soak
Ran the same canonical sequential 10-round `/xce-test-ha` soak (same session, same fresh ASAN binaries, same codebase state) that had just crashed raft-drive-async at round 9/10 with bug-1447 (SEGV in netio.c's handle_ssl_handshake/io_worker, triggered by that spec's own faster reconnect-offload cadence). ha-wal-replication's run went 10/10 clean — 33 jobs, zero loss, all 3 nodes led, and an explicit grep of every node log for bug-1447's exact signature (AddressSanitizer/DEADLYSIGNAL/SEGV/ssl3_read_bytes/handle_ssl_handshake/io_worker) found zero matches. Reasoned explicitly about what this does and doesn't prove: it does NOT prove bug-1447 is fixed or absent (it's a rare timing-dependent race that took the sibling until round 9/10 to trigger once) — but it DOES establish that ha-wal-replication's own boundary (repl.c/ha.c/cluster.c WAL-transmission+ACK+commit path) doesn't touch raft-drive-async's reconnect-offload trigger path, so its own regression contract (soak passes clean) is independently satisfiable. Wrote GO with bug-1447 listed as a known_limitation (shared-codebase risk, owned elsewhere) rather than either ignoring it or letting it block an unrelated spec. **Pattern**: when validating spec B right after spec A hit a live-soak crash on the shared codebase, always re-run B's own soak and explicitly grep for A's crash signature before writing B's verdict — don't assume "codebase is broken" (over-blocking) or "A's finding doesn't apply here" (under-checking) without evidence either way.
## Key Learning (2026-07-18) — ha-heartbeat-committed-propagation validated GO on a RETRY after a prior validation crashed silently mid-soak with zero diagnosable artifacts
A prior `/kiro-validate-impl` attempt for this spec died mid-live-soak ~4 hours before this retry, leaving only an empty stale deploy-dir marker (`~/tmp/xce-ha-heartbeat-committed-deploy`) — no logs, no core, no error trail of any kind. Rather than guess at a root cause or treat the silence itself as evidence of a product bug, ran the FULL validation from scratch (fresh ASAN build, 972/972 unit tests including the 12 heartbeat-owned tests, then a clean 10/10 sequential soak with explicit bug-1447 signature grep) and found nothing — no crash, no job loss, no unusual log lines anywhere in the fresh run. **Pattern**: when a validation pass vanishes with no artifacts, don't spend time forensically reconstructing an unrecoverable crash — clean the stale directory, retry the full pipeline end-to-end, and let a clean reproduction (or lack thereof) speak for itself. A silent crash with zero logs is far more often an agent/harness/environment loss (OOM kill, session death, disconnect) than a reproducible product defect; write it into `known_limitations` as an unexplained-but-not-reproduced event rather than blocking the verdict on it.
## Do-Not-Repeat (2026-07-18, bug-1446) — a live-soak script's `date +%N` silently degrades to a false verdict on BusyBox `date`; don't trust a single-mode script result without cross-checking an independent timestamp source
`test-commit-durability.sh --deterministic` reported 3/3 FAIL ("0 fdatasync in-window") on a build that, by every other signal (code read, and the very same run's own strace log), was correct. Root cause: this sandbox's `/sbin/date` is BusyBox date, which has NO `%N` (nanoseconds) support — `date +%H:%M:%S.%N` emits the literal unexpanded string `...%N`, breaking the script's sub-second submit/ack window arithmetic (`_hms_to_sod`) so it never lines up with the real fdatasync timestamp even when it fired in the same wall-clock second. Diagnosed (not accepted at face value) by cross-checking strace's own microsecond timestamps (unaffected — strace does its own formatting, not shelling out to `date`) against the controller log's `commit_advance_locked` lines for the same 3 probes: fdatasync consistently preceded the commit-advance/ack by 16-43ms in all 3, i.e. the real invariant held; the script was lying. **Do-not-repeat**: when a bash test script computes sub-second timing via `date +%N` (or any GNU-date-specific format), verify `date +%N` actually expands in the current environment BEFORE trusting a RED verdict from it — a BusyBox/musl/alpine-family environment will silently degrade instead of erroring. When in doubt, cross-check against a tool that timestamps independently of the shell's `date` binary (here, `strace -tt`'s own log) rather than assuming the product regressed. Logged as bug-1446 (test-harness only, no product fix).
## Key Learning (2026-07-18) — ha-prevote validated GO; a spec whose original 2026-06-16 implementation had already been gap-audited in research.md still needed full re-validation because of heavy adjacent churn since
tasks.md showed 18/18 (12 leaf + 6 group-header) checked since 2026-06-16, and research.md itself already contained a full 2026-06-18 gap re-audit (27/27 acceptance criteria MET/preserved, file:line evidence) — strong prior evidence, but NOT a substitute for re-validation, because 40+ commits touched raft.c/ha.c/cluster.c since then (raft-commit-safety's G6 vote-freshness closure, ha-term-safety, raft-drive-async, raft-apply-decoupling, ha-elect-ctx-lifecycle, and bug fixes 605/606/660/1421/1424/1430/1435 — several DIRECTLY inside the PreVote functions themselves, e.g. bug-1430's UAF in `ha_prevote_round_timeout_fn`). Re-verified via codegraph that the three pure raft-library PreVote functions (`raft_begin_pre_vote`/`raft_on_pre_vote_request`/`raft_on_pre_vote_reply`) still hold the zero-mutation invariant byte-for-byte, then ran the full pipeline fresh: 293 raft-lib + 972 controller unit tests (52 PreVote-tagged across 5 files) all green, then a clean 10/10 sequential soak with LIVE PreVote activity confirmed straight from the per-node logs (13 WON events, 1 DENIED-with-reason, 2 mid-round-cancel events) rather than just inferred from the absence of job loss — the mid-round-cancel events incidentally live-exercised the frozen-then-resumed re-attach path (Req 4.2) even though the sequential kill-schedule soak has no dedicated SIGSTOP scenario. Also found one legitimate, non-blocking design-doc drift: `raft_on_pre_vote_request`'s shipped signature grew two params (cand_last_log_term/cand_last_log_index) beyond design.md's original snapshot, because a LATER sibling spec (raft-commit-safety, G6) closed a freshness gap by moving the (lastLogTerm,lastLogIndex) check INTO the function itself — strictly stronger than the original design, correctly attributed to the owning spec rather than treated as this spec's regression. **Pattern**: `research.md`'s own historical gap-audit is excellent supporting evidence but has a staleness date — always check `git log --since=<that date>` on the spec's core files before trusting an old audit as current.
## Key Learning (2026-07-18, bug-1449 fixed) — run-ha-soak.sh had TWO independent convergence signals conflated into one; round 1 was only guarded by the wrong one
`wait_for_full_convergence`/`cluster_committed_converged` (gates every round AFTER round 1) checks role-count (1 primary + 2 standby) + committed-LSN catch-up — it says nothing about `configVersion` (the config "tag" a standby needs before `ha_on_primary_ctrl_fd_rdhup`'s ha-config-bootstrap Req 2.8 voter-only guard will let it self-elect). Round 1 had NO wait for that second, independent signal before its first kill, so an unlucky-but-legitimate slow cold-start election (observed 10-12s across 3 verification runs, vs. the ~1.7s job-submit budget the old code implicitly relied on) could let the kill land before config-push finished, correctly triggering the tag=0 guard and producing a false "no primary elected" round-1 failure. Fix mirrors `wait_for_full_convergence`'s exact polling primitive (alive-count via `pgrep -c -f "$(run_pattern)"`, STABLE_TICKS=2, sleep 2, ~90s bound) but gates on a NEW `cluster_config_converged` (all 3 nodes' `configVersion` via ctrlstatus non-zero, probed by a new `node_config_tag()` mirroring `node_probe`'s single-node client-probe technique) — added right after `wait_for_full_convergence` (~line 556) and called once, right before the round loop (~line 1897), so it applies ONLY to sequential mode's round-1 kill (chaotic mode intentionally tolerates this exact race elsewhere — a stalled election there is a COUNTED outcome, never a hard fail, so it was correctly left untouched). **Do-not-repeat**: when a soak/test driver has more than one "is the cluster ready" signal server-side (committed-prefix vs. config-tag here), a wait function gating on ONE does not imply the other is also satisfied — check what the actual guard/precondition being raced depends on before assuming an existing convergence wait already covers it. Verified 3/3 back-to-back clean `--sequential 6` runs on genuine ext4 (`/home/claude/tmp`, confirmed via `mount`/`df -T`, not overlayfs); cross-checked cold-start timing from controller logs across all 3 runs (election 10.3-11.7s after process start, config-push landing only 40-46ms after election every time) to confirm the new gate's ~4s minimum dwell / ~90s ceiling has ~2 orders of magnitude of margin, not just enough for that day's luck.
## Do-Not-Repeat (2026-07-18, bugs 1450/1451) — a new slow-lane offload's own non-blocking-submit contract is not enough; defaulting the flag ON can STILL hang controller_unit_test via a DIFFERENT mechanism than the vtask-pool deadlock it replaces
Fixed two `xce-audit-invariants controller` thread-role violations (`ha_demotion_convergence_fn` ha_convergence.c, `ha_catchup_continuation_fn` config_repl.c — both ran blocking work directly on the delaymgr thread) by adding a 4th/5th instance of the established slow-lane pattern (`slowlane_submit`/`*_lane_fn`, mirroring `ha_snapshot_send_dispatch`/`ha_config_push_offloadable`/`ha_connect_to_peer` in ha.c) — NOT the VTASK_LIVE_BLOCK vtask-pool approach a prior session tried and reverted (bug-1429) after it deadlocked `controller_unit_test`. Confirmed via `git log`/memory why that attempt failed: `c->ha.pool` was never initialized in the lightweight unit-test fixtures it ran against, so `vtask_enq_classified(..., VTASK_LIVE_BLOCK)` blocked forever waiting for a worker that didn't exist. `slowlane_submit` is architecturally immune to THAT failure mode (it is unconditionally non-blocking — with zero live workers it just returns `-EAGAIN`/`0` and leaves the work queued-but-never-run, never blocks the caller). BUT defaulting the new offload flags to ON (`g_drive_async_demotion_offload`/`g_drive_async_catchup_offload = 1`) reproduced a REAL hang anyway, via a DIFFERENT mechanism: the production callback being offloaded (`raft_cb_become_follower` for demotion, `ha_walack_on_ack`/`ha_catchup_continuation_fn` for catch-up) is exercised by a VERY BROAD set of PRE-EXISTING `controller_unit_test` fixtures across many files (confirmed hit: `test_convergence_verification.c`'s ACK-driven catch-up suite) that never call `slowlane_init` — so the offloaded work is silently queued on an uninitialized lane and NEVER RUNS, and any downstream code (in THOSE pre-existing tests, not the new ones) that waits — even boundedly — for the now-never-happening completion stalls/times out. Reproduced with a direct standalone binary run (`timeout -s QUIT 90 ./controller_unit_test`, bypassing meson) BOTH before (hang, SIGQUIT after 90s) and after (clean, "976 Tests 0 Failures 0 Ignored") the flag-default flip to OFF. **General rule**: when adding a NEW slow-lane offload flag for an existing, widely-exercised callback, default it OFF and add your own dedicated tests that explicitly flip it ON against a fixture you control (with `slowlane_init` called) — do NOT default a new offload flag ON just because the lane mechanism itself is provably non-blocking; the blast radius of "which pre-existing fixtures reach this code path without lane infra" is the real risk, and it can only be ruled out by a full test-suite compatibility audit (exactly how the ORIGINAL three raft-drive-async offloads were rolled out: shipped OFF, flipped ON only in a later dedicated task-3.4-style pass). Always verify a "fixed hang" claim by running the STANDALONE `controller_unit_test` binary directly with an external `timeout -s QUIT`, not just `meson test` (whose `Ok:N/Fail:0` aggregate can hide a TIMEOUT line entirely if you only grep for `Fail:`) — grep for `Timeout` explicitly too.
## Key Learning (2026-07-18) — `test_convergence_verification.c` is the direct, pre-existing test surface for `ha_walack_on_ack`/`ha_catchup_continuation_fn` (the ACK-driven catch-up decision, ha-convergence-recovery task 4.x); it drives them via `ha_walack_continuation_for_test` against a real-but-un-started `delaymgr` and has NO `slowlane_init` — any future change that makes catch-up continuation offload-capable must either default OFF or update this file's fixture.
## Do-Not-Repeat (2026-07-21, admin-config-tool task 1.3, bug-1458) — adding `python3Packages.{pycapnp,pyyaml,jsonschema,click,pytest,ruff}` to a nix `nativeBuildInputs` list does NOT put a `pip` executable on `$PATH`; none of those packages pull one in transitively. You must add `python3Packages.pip` explicitly if any build step needs to run `pip install`. brief.md's own suggested package list for xceadmin omitted it — verified by actually running the resulting `nix develop` shell end-to-end, not by reading the package list.
## Do-Not-Repeat (2026-07-21, admin-config-tool task 1.3, bug-1459) — a nix-provided `python3`'s site-packages lives under the read-only `/nix/store`, so `pip install`/`pip install -e .` against it directly fails with "externally-managed-environment" (PEP 668), even from inside `nix develop`. Fix pattern: `python3 -m venv --system-site-packages "$VENV_DIR"` (inherits the nix-provided heavy binary deps like `pycapnp` for free, so pip never re-downloads/rebuilds them) then run `pip`/`python` via `"$VENV_DIR/bin/..."`, not bare. This is the general pattern for any future nix + `pip install` component in this repo, not xceadmin-specific.
## Do-Not-Repeat (2026-07-21, admin-config-tool task 1.3, bug-1460) — a Python package's non-`.py` data files (here: `xcecommon/schema_files/*.capnp` and `schema_files/json/*.schema.json`) are silently DROPPED from a wheel built by `pip install .` (non-editable) unless declared via `[tool.setuptools.package-data]` in `pyproject.toml`. An editable install (`pip install -e .`) never catches this — it resolves straight into the checked-out source tree, bypassing wheel packaging entirely. Any bundled-non-code-asset pattern in this repo's Python components needs an explicit `package-data` entry, and the ONLY way to verify it works is to actually run the non-editable/`release` install profile and inspect `site-packages`, not just the editable one.
## Do-Not-Repeat (2026-07-23, bug-1464) — every single-component `/xce-build <comp>` invocation does `rm -rf` on the SHARED `xce.build` dir before compiling; all components (controller, xceadmin, etc.) default to the same build dir per profile. Running `/xce-build controller` then `/xce-build xceadmin` separately wipes whichever was built first — only the most recent single-component build survives. When a downstream task (e.g. an HA soak's `deploy-ha.sh`) needs binaries from MULTIPLE components present simultaneously, use `/xce-build all` (one `setup_build()` + one unfiltered `meson compile`), not sequential per-component calls.
## Key Learning (2026-07-23, bug-1418) — a `blocked_on` entry in a spec.json (or a project memory note) claiming a bug is "NOT FIXED" can itself be stale; the 2026-07-21 blocked_on note on config-upgrade-snapshot was ~2 weeks out of date against `.wolf/buglog.json`'s own 2026-07-09 resolution entry for bug-1418. Before trusting/acting on any blocked_on or "still open" claim, cross-check buglog.json's own entries for that bug id first — re-verified fresh here with a 10-round `--chaotic` soak (PASS, zero divergence/loss), then cleared config-upgrade-snapshot's blocked_on to [].
## Key Learning (2026-07-23, config-upgrade-snapshot task 1) — this codebase's `controller.capnp` convention: EVERY struct gets `$C.mapname(...)`, even a brand-new wire-only "frame" concept with no corresponding "live" runtime C struct. For that case, define a small hand-written `typedef struct { ... } capnp_X_t;` in `src/include/typemap.h` (matching field-for-field, `Text`→`char*`, `UIntN`→`uintN_t`) and `$C.mapname` the schema struct onto it — exactly the pattern already used by `SnapshotHeader`/`capnp_snap_hdr_t`, `PersistConfig`/`capnp_persist_cfg_t`, `ReplEnvelope`/`capnp_repl_envelope_t` (all in typemap.h's "Persistence Cap'n Proto mapped types" section). This gives you generated `encode_X`/`decode_X`/`free_X` conversion functions for free (via the file-level `$C.codecgen` annotation) instead of hand-writing capnp field accessors. Do NOT emit a bare `struct X { name @0: Text; ... }` with no mapname even if a design doc's sketch shows it that way literally — cross-check real schema precedent first. Used this for `HostAdminState`/`QueueAdminState` → `capnp_host_admin_state_t`/`capnp_queue_admin_state_t`.
## Do-Not-Repeat (2026-07-23, bug-918 recurrence 3) — when `nix develop`/`/xce-build` fails with a fixed-output-derivation hash mismatch on `capnpc-1.0.1.drv`'s `source.drv`, do NOT assume `nix flake lock --update-input xbuild` (the documented fix from the 2026-06-22 occurrence) will help without checking first — `nix flake metadata` may already show `xbuild` pinned to the LATEST available `shen390s/my-nix-channel@master` commit, in which case re-locking is a verified no-op (confirmed: ran it, `git diff -- src/flake.lock` was empty). Also confirmed via a direct `nix develop` in the canonical non-worktree repo (`/home/rshen/projects/xce/xce/src`) that it fails IDENTICALLY — this is a live, machine-wide upstream break (c-capnproto's mutable `main.tar.gz` drifted again), not a worktree-isolation artifact, and is NOT fixable from inside this repo at all. **Workaround that DOES work for schema-only verification**: `find /nix/store -maxdepth 1 -iname '*capnp*'` to check for already-REALIZED (not just `.drv`) `capnpc-1.0.1`/`capnproto-1.2.0` store paths from a build that completed before the drift — if present, invoke their `bin/capnp`/`bin/capnpc-c` DIRECTLY (`capnp compile -o <full-path-to-capnpc-c>:<outdir> -I <schema_dir> --src-prefix <schema_dir> controller.capnp`), bypassing `nix develop` entirely. This proves a schema change compiles and inspect the generated `.c`/`.h` for correct struct/function generation, but does NOT unblock a full project build — `controller_unit_test` also needs `librtti` (private SSH git flake input), which had NO `.drv` or realized store path on this machine at all (never built here), so full build+link+test remains infeasible until upstream fixes the pin or a machine with a pre-cached librtti toolchain is used. When this recurs, check bug-918's full `fix` field in buglog.json first — it now documents this exact triage sequence.
## Key Learning (2026-07-24, bug-918 RESOLVED, recurrence 4) — the actual durable fix for the c-capnproto fixed-output-derivation hash mismatch (recurrences 1-3 above): a local `overrideAttrs` in `src/nixSupport/xce.nix`'s `capnpc` derivation pinning `src = pkgs.fetchFromGitHub { owner = "shen390s"; repo = "c-capnproto"; rev = "<immutable commit sha>"; sha256 = "<matching hash>"; }` — this makes THIS repo's build independent of upstream xbuild's package.nix, which pins the MUTABLE `main` ref and breaks every time that branch moves. `librtti` was NOT actually a blocker on this machine (recurrence 3's concern that it had "never been built/cached" turned out to be stale/machine-state-dependent) — a full `meson setup --reconfigure && ninja controller/controller_unit_test` built clean and the full suite ran (978 Tests 0 Failures). **Do-not-repeat**: for ANY future fixed-output-derivation hash mismatch traced to an upstream flake pinning a mutable branch/ref (not a tag/commit), the fix is a local `overrideAttrs` pinning `src` to an explicit immutable commit in the CONSUMING repo, not re-locking flake.lock or waiting on upstream — that path is a dead end once the upstream input is already at its latest commit (recurrence 3 already proved this).
## Do-Not-Repeat (2026-07-24, config-upgrade-snapshot task 1, bug-1465) — bumping a shared on-disk format-version constant (here `persist.c`'s `SNAPSHOT_FORMAT_VERSION`) breaks ANY test file that hardcodes its own independent copy of that value to structurally validate bytes it reads directly (bypassing the real read-path on purpose, to test the write side in isolation) — even in a DIFFERENT, already-shipped spec's test file that the current task has no reason to otherwise touch. `test_config_repl.c`'s `fake_fmt` sites were caught and fixed in the same session the bump was made; `test_persist_snapshot_entities.c`'s independent `SNAPENT_TEST_FORMAT_VERSION` constant was NOT (different file, no shared symbol/include tying them together) and only surfaced as 3 failures once the full suite finally ran (blocked for a session by the bug-918 nix issue above). **General rule**: after bumping any shared format/version constant, `grep -rn` the whole test tree for OTHER hardcoded copies of the old literal value (not just call sites that reference the primary constant by name) before declaring the bump done — a stale hardcoded copy in an unrelated spec's test file is a silent regression that only a full-suite run catches.
## Key Learning (2026-07-24, config-upgrade-snapshot task 2.3) — `persist.c` deliberately has ZERO function-call dependency on `config_repl.c` (not even the trivial `#include "config_repl.h"`). Two precedents establish this as an intentional domain boundary, not an oversight: `scan_latest_committed_config_ref` (persist.c) inline-decodes WAL_CONFIG_MARKER payloads instead of calling any config_repl.c decode helper ("decoded inline here so persist stays free of a config_repl layering dependency"); task 2.3's new `read_active_config_bytes` (persist.c, embeds active config bytes into every checkpoint) fully inline-reimplements `conf_read_current_filename`+`conf_read_raw`'s path-resolution-and-read logic (~140 lines) rather than calling those existing, already-public (`config_repl.h`) functions, even though `ha.c` DOES call `conf_read_raw` cross-module via a bare `extern` declaration at 2 sites (config push to standbys) — design.md's Architecture boundary map explains the asymmetry: `ha.c` is the intentional CONSUMER of "config replication (config_repl.c) owns config-bytes sourcing," so a ha.c→config_repl.c call is expected in-boundary integration, while `persist.c` owns materialization/on-disk-format and is kept deliberately free of that domain's dependency. **When touching persist.c again** (e.g. task 5's marker-apply trigger, task 6's Activation Fence — which design.md explicitly says must "verify against LIVE on-disk config bytes each attempt" mirroring `config_activate_committed`'s re-read-every-time pattern, `config_repl.c:275-302`) — check whether the new code lands in persist.c (keep it config_repl.c-dependency-free, duplicate the small read-with-validation routine if needed) or ha.c (fine to call config_repl.c directly, precedent already established). Do not "clean up" persist.c's duplication by refactoring it to call config_repl.c — that would be reversing a deliberate, design.md-documented boundary decision, not a simplification.
## Key Learning (2026-07-24, config-upgrade-snapshot task 5) — a lock-reentrancy or stale-value hazard in a NEW `apply_committed_entry()` case can NEVER be caught by a test that calls `apply_committed_entry()` directly and in isolation. `persist_apply_pump_run()` holds `apply_pump_lock` across its ENTIRE per-LSN dispatch loop (including the call into `apply_committed_entry`); a naive `persist_checkpoint()` call from inside a new case would self-deadlock on that same non-recursive mutex, but only if the test actually drives the call through `persist_apply_pump_run()`/`persist_apply_pump_kick()` — a bare direct call never holds the lock, so it can't catch the deadlock. Same for a stale-`last_applied_lsn` hazard: `persist_apply_pump_run`/`replay_wal` both advance `last_applied_lsn` STRICTLY AFTER `apply_committed_entry` returns, never before — so a test needs a leading unrelated WAL entry before the entry under test to make `last_applied_lsn` genuinely differ from the tested entry's own lsn at dispatch time, or a "reads `last_applied_lsn` instead of the `lsn` parameter" bug is invisible. **General rule for any future `apply_committed_entry` case whose body triggers a SEPARATE persist_t operation that also touches `apply_pump_lock` or LSN-position state**: the test file MUST drive it through the real `persist_apply_pump_run`/`persist_apply_pump_kick` production call site (disk-backed persist_t + cluster_t fixture, mirrors `test_replay_apply_seam.c`/`test_apply_pump.c`, NOT `test_apply_committed_entry.c`'s bare in-memory fixture) with at least one leading entry, not just a bare direct call.
## Do-Not-Repeat (2026-07-24, config-upgrade-snapshot task 5) — in a Unity test file that does real `persist_start()`/`persist_init()` (spins up a live `fsync_worker` background thread holding a `persist_t*` pointer into the test function's own stack frame), NEVER put the fixture's teardown call at the END of the test function after a run of `TEST_ASSERT_*` calls. A failed `TEST_ASSERT_*` does a `longjmp` straight back to `RunTest`, skipping every line after it — including the teardown call — which leaves the background thread alive holding a dangling pointer into a stack frame the harness is about to reuse for the NEXT test in the same binary. This manifests as an ASAN `stack-use-after-return` crash inside `flusher_fsync_forced_fail`/`fsync_worker`, attributed to a LATER, unrelated test's frame, making it look like that later test is broken when the REAL bug is an earlier test's missing unconditional teardown. This exact pathology is already documented in `test_apply_pump.c`'s own file header ("longjmp-past-teardown pathology from task 3.1's round-2 lesson") — re-discovered the hard way while drafting `test_upgrade_snapshot_apply.c`'s RED-phase run. **Required pattern**: gather every assertion-relevant observable into a plain local variable FIRST (no `TEST_ASSERT` between "the SUT ran" and "teardown runs"), call teardown UNCONDITIONALLY, THEN run every `TEST_ASSERT` against the captured locals.
## Key Learning (2026-07-24, config-upgrade-snapshot task 5, bug-1466) — `persist_write_checkpoint_marker()` (called by ANY checkpoint, periodic OR the new task-5 marker-triggered one) appends its local-only, never-replicated `WAL_CHECKPOINT` entry by incrementing `persist_t.current_lsn` DIRECTLY, bypassing `ha_primary_log_append()`/`raft_log_append()` entirely. This silently violates `ha_primary_log_append`'s own documented co-tracking invariant (`ha.c:4057`, "p->current_lsn == s->log_len + 1 always holds") — the raft LIBRARY's `s->log_len` (a separate in-memory counter, `lib/raft/raft.c:574/585`) does NOT advance for a checkpoint marker, so it under-counts the true physical WAL tip from that point on. Consequence: `config_append_marker`'s/`ha_primary_log_append`'s RETURNED lsn (sourced from the library's `idx = s->log_len + 1`) becomes UNRELIABLE as an exact physical-position identifier for any entry appended after an earlier checkpoint has interleaved — this is a PRE-EXISTING gap (the periodic WAL-threshold checkpoint trigger has always had it) that task 5's marker-triggered checkpoint turns from "rare, needs >=100 WAL records to fire" into "the common case for any second config marker committed after an earlier one." **Verified this does NOT corrupt on-disk data or commit/durability correctness**: `commit_advance_locked`'s `majority_candidate_locked` (commit.c) is driven entirely by `persist_t`'s own `durable_lsn`/`current_lsn`, never by `s->log_len` (which the code's own comment calls "advisory only" for commit-ceiling purposes, ha-term-safety commit-ceiling-optA) — and `persist_wal_async` always self-assigns from `current_lsn`, ignoring the library's `index` parameter for a primary's own append (`ha_cb_persist_entry_stub`), so there is no on-disk LSN collision either. The blast radius is confined to any caller trusting the RETURNED lsn for precise identity/ordering once a checkpoint has occurred earlier in the session. **Do NOT attempt to fix the deeper `s->log_len` resync inside a `persist.c`-scoped task** — it requires touching `ha.c`/`lib/raft/raft.c` (owned by `ha-term-safety`/`raft-log-replication`), which is a bigger, deliberate architectural decision (e.g. adding a `raft_advance_log_len`-style API), not a same-task patch. Test-side fix that IS correct and in-scope: read the entry's TRUE physical lsn directly off `persist_t.current_lsn - 1` right after appending, instead of trusting the raft-library-derived return value — exactly how production code that needs precise positioning (`persist.c`'s apply seam, which receives `lsn` from the WAL entry's own on-disk header) already effectively does it. Full finding in `.wolf/buglog.json` bug-1466.
## Key Learning (2026-07-25, config-upgrade-snapshot task 8.6, bug-1469) — task 8.6's live `--config-upgrade` soak EMPIRICALLY REPRODUCED bug-857 on its first real run, via a THIRD distinct mechanism neither the original (disproven lost_and_found) hypothesis nor task 8.5's unit-level fix (`schedule_checkpoint` WAL-threshold-vs-marker-boundary race) covered: `do_upgrade_config_with_bytes` (config_repl.c, the handler for the node that ORIGINATES/commits an upgrade) saves new config bytes to disk and bumps `config_generation`, but never calls `config_activate_committed()`/`load_primary_config()` to re-parse those bytes INTO `c->myconf` — so the originating primary's own in-memory queue table (and whatever the checkpoint writer walks to emit the queue-entity list) stays stale even though its Activation Fence (`upgrade_materialization_verify`, ha.c) reports "VERIFIED" (that function only hash-checks on-disk bytes + a checkpoint's own cfgref stamp, never touches `c->myconf`). `config_activate_committed` is only invoked from `handle_repl_config_push` (a STANDBY's push-receipt path) or a member's own boot/recovery — confirmed via ctrl-1's log: ZERO `config_activate_committed` calls in its original (primary) process instance, TWO in its restarted instance. If the originating primary is killed before some LATER independent event triggers that call (exactly what the chaotic-kill-interleaved-with-upgrade scenario does by design), its last checkpoint permanently freezes a self-contradictory state: correct new config bytes, stale old queue-entity set. **Do-not-repeat**: when investigating ANY "config activation" or "materialization" claim in this codebase, do not trust an Activation-Fence/"VERIFIED" log line as proof that `c->myconf`'s in-memory parsed structures were refreshed — trace whether `config_activate_committed`/`load_primary_config` was ACTUALLY called on that specific code path (grep the live log for the function name itself), since the fence's hash-check and a checkpoint's cfgref stamp can both be satisfied by disk-bytes-only activity. Full finding in `.wolf/buglog.json` bug-1469; NOT fixed (out of task 8.6's scope — flagged for tasks 1-7's owner). Task 8.6's own harness (dispatch wiring, `run_config_upgrade_round`, SKILL.md docs) is otherwise correct and not implicated — confirmed the on-disk-checkpoint-comparison methodology is the design-intended one and waiting longer would not have self-corrected (no further WAL activity to trigger a follow-up checkpoint).
## Key Learning (2026-07-25, config-upgrade-snapshot task 8.6) — a live HA soak against a NEW client RPC can surface bugs the RPC's own author never had a way to trigger; two distinct, real materialization bugs found this way, only one safely fixable in-task
Building task 8.6's `--config-upgrade` live soak required first discovering `OP_upgradeConfig` had NO working client anywhere (`xce_upgradeconfig_req_raw`/`xce_upgradeconfig`/`xcecli_upgradeconfig` built fresh in request.c/xceapi.c/client.c) AND that `cluster.c`'s dispatch gate for it was inverted (rejected primary, accepted standby — backwards; commit `fd93997f`, 2026-05-31, flipped it from a since-superseded primary-push design; already independently diagnosed as a blocking risk by the sibling `admin-config-tool` spec's design.md and explicitly left for whoever needed live `OP_upgradeConfig` first — that was this task). Fixing the dispatch gate (bug-1468) was a small, obviously-correct routing fix, in scope. Once the RPC worked, the FIRST real live run surfaced bug-1469: `apply_committed_entry`'s `WAL_CONFIG_MARKER` case called `config_activate_committed()` (which refreshes `c->myconf`) AFTER the marker-triggered checkpoint write, not before — so the checkpoint captured stale config. Reordering fixed HALF of it (verified: full 1025-test suite clean, checkpoint-content correctness improved). But re-running the SAME live soak after that fix showed the IDENTICAL divergence signature on a different node — root cause turned out DEEPER: `config_activate_committed`→`load_primary_config` only re-parses bytes into `myconf->cluster_config`, it NEVER touches `c->qmgr` (the runtime queue table `collect_queue_frames`/the checkpoint writer actually walks). The ONLY code that ever populates `c->qmgr` from config is `build_queues()` (queue.c), called EXACTLY ONCE per process lifetime at boot (`xce_config_init` hook), and it's unconditional/blind-create — unsafe to just re-call live (would attempt to recreate already-existing queues). This means NO live (non-restart) mechanism exists anywhere in this codebase to add a config-upgrade's NEW queue to an already-running member's runtime state — directly contradicting this whole spec's stated goal ("no controller restart required for a configuration upgrade"). This is genuinely deep, tasks-1-7 territory (needs a new create-if-missing sync function mirroring `restore_host_state`'s established pattern, PLUS an undecided queue-REMOVAL semantics design question) — NOT something to improvise under a test-infra task's time budget. **Resolution used**: changed the soak's own upgrade-target config diff from "add a queue" to "update an existing queue's description" — a legitimate, still fully meaningful config-identity test (Requirements 4.2/4.3/6.1/6.4 don't require adding an entity) that sidesteps the unrelated, unfixed mechanism entirely, rather than either (a) faking a pass by weakening the assertion, or (b) rushing an unreviewed fix into live queue-provisioning code. A third, much smaller (1-byte) checkpoint non-determinism remained even after that (bug-1471, host-frame encoding, root cause not yet isolated) — logged for follow-up rather than guessed at. **General lesson**: when a new regression-test/soak harness for feature X finds a bug in X's own materialization logic, first ask whether the SAME class of gap could apply more broadly than the one field your test happened to touch (here: ANY new queue OR host from ANY live upgrade, not just this soak's specific queue) before writing a fix — and if the honest answer requires new production logic + an undecided design question, stop and document rather than patch under the wrong task's time pressure.
## Do-Not-Repeat (2026-07-25, config-upgrade-snapshot task 8.6) — a long-running/compacted session can leave substantial prior work on disk with no visible conversational memory of it; always `git status`/`git diff --stat` early when picking up a "continue"
Mid-task, discovered via `git status` that `persist.c`/`test_config_repl.c` were already modified with a detailed, real bug fix (bug-1469) I had no memory of writing — from an earlier, compacted portion of the same long-running session (confirmed via claude-mem observation hooks showing timestamps hours earlier than the visible transcript, and via the fix's own content matching exactly what I'd have derived independently). Nearly redid Part A-D from scratch before noticing Part E (the whole soak script + SKILL.md docs) was ALREADY fully written on disk too. Lesson: when resuming any task (especially on a bare "continue"), run `git status --short` + `git diff --stat` against the expected file set FIRST, before writing anything — don't trust a feeling of "starting fresh" just because the visible conversation transcript looks short. The claude-mem observation timestamps (visible in PreToolUse:Read hook context) are a reliable secondary signal for how much wall-clock work actually preceded the current visible slice.
## Key Learning (2026-07-25, config-upgrade-snapshot task 8.6, bug-1471/bug-1472) — a "1-byte checkpoint divergence near a padding-sized gap" is not proof of padding; decode the actual field before guessing
bug-1471's two live hypotheses (uninitialized capnp padding vs. genuine content discrepancy) were distinguished definitively by (1) writing a throwaway byte-precise capnp decoder that reuses the SAME wire shape the real `write_HostAdminState`/`encode_HostAdminState` generated code produces (1 data word holding `status` via `capn_write32` at offset 0 + 4 bytes of trailing padding, 1 pointer word for `name` Text) rather than eyeballing `xxd` output, and (2) directly disassembling the pinned c-capnproto runtime (`/nix/store/*-capnpc-1.0.1/lib/libCapnC_Runtime.so`, since no `.c` source is shipped in the nix store — only the built `.so` — `objdump -d` on `capn_init_malloc`'s `create_local` callback) to confirm its segment allocator calls `calloc(1, size)`, not `malloc`, so it IS zero-initialized. Both lines of evidence agreed: the differing byte (file offset 42879) was byte index 2 of the 4-byte `status` field itself (0x00500000 = `XCE_WORKER_CLOSED|XCE_WORKER_CLOSED_BY_ADMIN` on ctrl-1 vs 0x0 on ctrl-2/ctrl-3), NOT the always-zero padding word right after it — a genuine content bug, not noise. Root cause: `worker_from_capnp` (worker.c:827-856, introduced 2026-05-08, ~2.5 months before this spec) is called for EVERY `WAL_WORKER_CTRL` apply regardless of JOIN/UPDATE/DEAD detail, and its idempotency guard returns `-EEXIST` (silently discarding the decoded fields, including `status`) whenever the named worker already exists — correct for JOIN replay, WRONG for UPDATE/DEAD (where "already exists" is the NORMAL case). This means ANY `hostctrl -c`/`hostctrl -o` against an already-joined host never propagates its admin-status change to standbys, unconditionally, cluster-wide — a real, general, pre-existing bug entirely outside config-upgrade-snapshot's boundary (filed as bug-1472, NOT patched here — scope discipline). **Do-not-repeat**: when a buglog entry frames two hypotheses as "padding vs. content" for a capnp field, always decode the field itself using the codebase's own generated wire-layout constants (`X_word_count`/`X_pointer_count`) before assuming either — "looks like it's near a text/entity boundary" is not evidence of which one it is.
## Key Learning (2026-07-25, config-upgrade-snapshot task 8.9, bug-1474) — a live soak that FAILS is not automatically a test-script bug; forensically distinguish "my assumption was wrong" from "production genuinely did the wrong thing" via direct log evidence, byte-for-byte, before touching either
Building `run_config_upgrade_wal_trim_round` (`--config-upgrade-wal-trim`) to prove bug-1147's WAL-trim retention guard holds under the marker-triggered path required a "baseline capture" step (record the primary's currently-active WAL segment as a genuinely-caught-up standby's floor, right before killing it) — a reasonable design, but its correctness is NOT self-evident from "the round ran without erroring." When the round's own final assertion failed (floor segment trimmed away, only the active segment survived) on BOTH of 2 independent live runs, the instinct to weaken/second-guess the test's own logic was strong — instead, cross-referenced `ctrl-2.log`'s (the primary's) own `commit: ack standby_idx=N` lines, `on_standby_connected`/`repl registered late-joiner standby` lines, and the exact wall-clock kill timestamp (`ctrl-1.log`'s own last line) to reconstruct, byte-for-byte, WHAT the replicator's slot state actually was at trim time. This revealed a REAL, 2-of-2-reproducible production defect: the primary's heartbeat task's "reconnect to any peer missing from standby_endpoints" sweep spuriously treats an ALREADY-connected peer (one connected only via a promoted-vote-channel, not yet a heartbeat-task connection) as missing, and the resulting duplicate "late-joiner" reconnect's `repl_add_standby` call picks the LOWEST-INDEXED free replicator slot with no regard for whether that slot still holds an UNRELATED, unexpired bug-1147 disconnect guard — silently clobbering it via `memset`. Filed as bug-1474 (NOT fixed — out of task 8.9's scope, mirrors the bug-1471/1472 precedent). **Do-not-repeat**: when a live-soak assertion fails, do NOT assume the test's own timing/logic is at fault just because the mechanism is complex — grep the ACTUAL controller logs for the specific slot/connection/ack events around the failure window first; a "the guard should have protected X but didn't" failure is exactly the shape a genuine upstream defect takes, and 2 independent runs showing the IDENTICAL mechanism is strong evidence it's real, not a timing fluke.
## Key Learning (2026-07-26) — bug-1474 fixed: repl_add_standby's first-empty-slot scan must never clobber an unexpired bug-1147 disconnect guard
Prior entry (2026-07-25) noted `repl_remove_standby` correctly sets the `last_disconnected_lsn`/`last_disconnect_time` guard and `repl_expire_disconnect_slots` correctly clears it on expiry — but nothing protected the guard from PREMATURE clobbering by `repl_add_standby`'s slot-allocation scan, which picked the first `ep==NULL` slot by lowest index with zero awareness of whether that slot still held an unexpired guard for a different peer. This let an unrelated peer's `repl_add_standby` call (triggered in the wild by a separate ha.c duplicate-reconnect defect, but the vulnerability exists regardless of the trigger) unconditionally `memset` away a guard `repl_trim_lsn` needed, letting `trim_wal_segments` delete WAL segments a genuinely disconnected-but-reconnecting standby still needed. **Fix pattern**: two-pass slot allocation — Pass 1 prefers a genuinely virgin slot (`ep==NULL && last_disconnected_lsn==0`); Pass 2 (only if no virgin slot exists) falls back to the guard-holding slot with the OLDEST `last_disconnect_time` (closest to its own natural `repl_expire_disconnect_slots` expiry, so reusing it discards the least remaining protection) rather than refusing the connect (`-ENOSPC` would be worse for cluster availability than an early guard loss in an already-degraded multi-disconnect scenario). **Do-Not-Repeat**: when a struct slot has BOTH a "live" state (`ep != NULL`) and a "recently-freed-but-still-meaningful" state (`ep==NULL && guard_field != 0`), a first-empty-slot allocator that only checks the live/dead boundary will silently destroy the recently-freed state's data — always give the allocator a way to distinguish "truly free" from "free but still guarding something," and pick a deliberate, explained tie-break policy (here: oldest-guard-first) for the exhausted-fallback case rather than an accidental index-order fallthrough. TDD: 2 new tests in `test_repl_unit.c` (`test_repl_add_standby_prefers_virgin_slot_over_guard`, `test_repl_add_standby_exhausted_guards_prefers_oldest_disconnect`), RED confirmed against the pre-fix scan, GREEN at 1030/1030 (0 failures, no regressions).
## Key Learning (2026-07-25, bug-1472 fix) — the `Worker` capnp struct's `resvals` field maps to `resvals_to_sched`/`nresvals_to_sched`, NOT to `xce_worker_t.host.resvals` (the live per-host resource-value `zarray_t` that scheduling actually reads and that `set_host_slots()`/`update_resvals()` mutate). Confirmed by reading `src/schema/controller.capnp`'s `struct Worker` (`resvals @5: List(NameValue) $C.mapname("resvals_to_sched") $C.maplistcount("nresvals_to_sched")` — no field maps to `host.resvals` at all) and the generated `decode_Worker`/`encode_Worker` in `controller.capnp.c`. Consequence: the WAL_WORKER_CTRL wire format (`persist_worker_hook_fun` → `capnp_encode_worker(&w->host, ...)`, same single encode call site for JOIN/UPDATE/DEAD) can NEVER replicate a `mark_worker_unavail()`-style resource-slot zeroing to another member, no matter how the apply side is fixed — this is a genuine schema gap, not an apply-logic bug. Any future work on `HOOK_DETAIL_WORKER_DEAD` replication (see bug-1472-remainder in buglog.json) needs a schema change (a new wire-encodable resource-value-slots field) or a non-replication-based strategy, not just an apply-side fix mirroring bug-1472's UPDATE fix. **Also confirmed**: `encode_Worker`/`decode_Worker` DO faithfully round-trip `status`, `dead_since`, and `attrs` (`decode_HostAttrs_ptr` allocates and copies) for all three hook_detail values, since the encode call always captures the worker's CURRENT (post-mutation) `w->host` state uniformly — so a DEAD-case fix is NOT blocked by field-fidelity for those three, only by the missing `resvals` wire mapping.
## Key Learning (2026-07-25, bug-1472 fix) — `worker_from_capnp`'s pre-existing fresh-restore path (`restore_worker(c, host)` on a MISS) leaks the decoded `xce_worker_t`'s `name`, `attrs` (+ its `hosttype`/`hostmodel` strings), and `resvals_to_sched` list on every call — NOT introduced by this fix, already present before it, out of scope to fix here, but worth knowing before touching this code again. Root cause: `copy_worker` (`src/lib/wobj.c:51-81`, called by `makeWorker`/`makeWorker_`/`override_worker`) has THREE different ownership behaviors for the SAME `xce_worker_t*` argument depending on the field: `name` and `attrs->hosttype`/`attrs->hostmodel` are re-created via `makeString()` (a COPY — caller's originals are untouched and must be freed by the caller); `dispatch_window_config` and `thresholds` are transferred via `MOVE_PTR` (caller must NOT free these — ownership passes to the new worker); `resvals_to_sched`/`nresvals_to_sched` are DROPPED entirely (`dst->host.nresvals_to_sched=0; dst->host.resvals_to_sched=NULL` — neither copied nor moved, silently discarded). None of `worker_from_capnp`'s existing callers (`restore_worker`, and this fix's own -EEXIST/JOIN branch) call `free_Worker(&host)` after `makeWorker`/`copy_worker` returns, so `name`/`attrs`/`resvals_to_sched` leak every time. This has never been caught because this repo's controller test config runs with `ASAN_OPTIONS=detect_leaks=0` (required separately for an unrelated reason — see bug-918/controller_unit_test hang note). **Do-not-repeat**: if you ever decode an `xce_worker_t` via `decode_Worker` and DON'T hand it to `makeWorker`/`copy_worker`/`restore_worker` (e.g. bug-1472's own new UPDATE branch, which only reads `host.status`), you MUST call `free_Worker(&host)` yourself — copy_worker's mixed copy/move/drop semantics mean there is no single safe default, and the existing call sites' silence is leaked technical debt, not a pattern to copy.
## Key Learning (2026-07-25) — bug-1472 FIXED: `worker_from_capnp` now decodes `hook_detail` and applies UPDATE in place to an existing worker
`worker_from_capnp` (`src/controller/worker.c`) previously returned `-EEXIST` unconditionally whenever the decoded worker's name was already registered, silently discarding the decoded fields regardless of whether the WAL entry was a JOIN, UPDATE, or DEAD. Fixed by reading the WAL_WORKER_CTRL payload's leading `hook_detail` discriminator (already decoded by the caller at `persist.c:3728-3729` but never passed down or re-read by `worker_from_capnp` itself) and branching: `HOOK_DETAIL_WORKER_JOIN` keeps the original -EEXIST no-op (regression-guarded); `HOOK_DETAIL_WORKER_UPDATE` now REPLACES (bitwise, not OR — see worker_ctrl's assignment-style `&= ~(...)`/`|= (...)` semantics, an OR-based fix would make HOST_OPEN unable to ever clear the closed bits again) `XCE_WORKER_ADMIN_MASK` bits on the existing worker under its own write lock, then returns 0 so `persist.c`'s existing generic "0 ⇒ fire the hook_detail-keyed notification" fallthrough (persist.c:3737-3785, unmodified) fires the correct `HOOK_DETAIL_WORKER_UPDATE` hook; `HOOK_DETAIL_WORKER_DEAD` is deliberately left as -EEXIST/no-op (see the two Key Learning entries above for why — filed as bug-1472-remainder, not silently ignored). `persist.c` needed NO changes — confirmed its WAL_WORKER_CTRL apply case already treats any non-EEXIST, non-negative `worker_from_capnp` return generically. TDD: RED confirmed by temporarily reverting just `worker.c` (via `git stash push -- <2 files>` / `git apply` round-trip, keeping the new tests) and re-running — exactly the 2 new UPDATE tests failed, the JOIN-duplicate regression-guard test already passed (proving it exercises the untouched path). GREEN: full suite 1030→1033 tests (this worktree's immediate pre-fix baseline, not a stale prior-session number), 0 failures, net +3 = exactly the 3 new tests (`test_ace_worker_ctrl_update_applies_to_existing_worker`, `test_ace_worker_ctrl_update_replaces_not_ors_admin_bits`, `test_ace_worker_ctrl_join_duplicate_existing_is_noop`), zero regressions.
## Key Learning (2026-07-28, config-upgrade-snapshot task 8.9, bug-1478 FIXED) — a function's own doc comment can describe a NARROWER behavior than the code actually implements; the gap between the two is where a real bug hides
`ha.c`'s `repl_ack_decode_and_dispatch` computed the primary's replication/convergence target as `target_lsn = max(committed_lsn, last_written)` (`last_written = persist.current_lsn - 1`, the RAW physical WAL tip), with a comment saying this exists "so standbys can catch up even when committed_lsn is stuck at 0" — but the code applied the `max()` UNCONDITIONALLY, not only in that `committed_lsn==0` case. This was harmless until config-upgrade-snapshot task 5's marker-triggered checkpoint (a local-only, never-replicated `WAL_CHECKPOINT` entry that `persist_write_checkpoint_marker` appends by bumping `current_lsn` directly, bypassing `raft_log_append`/the commit engine — the same architectural gap already flagged, unfixed at its root, as bug-1466) started interleaving a real, mid-operation local-only entry: `last_written` then permanently sat one LSN past the REAL, majority-gated `committed_lsn` (confirmed via `commit_advance_locked`'s own log never advancing past 21, while `ha_dispatch_repl_action_for_standby`'s log reported "committed_lsn=22" forever) — and since a checkpoint's own consumed LSN slot is BY DESIGN never streamed/applied by any standby, `HA_REPL_CONVERGED` became permanently unreachable, forever re-streaming an empty range every heartbeat. **Ground-truthed via `xce_test waldump <wal_dir>`** (an existing REPL command in the `xce_test` binary, `src/controller/testsuite/waldump.c` — prints `LSN | TERM | TYPE | PLEN | PCRC | INFO` for every entry in a WAL directory, plus a gap/tip summary) directly on the preserved live-soak deploy dir, which is what turned "I think lsn=22 is the checkpoint" into "confirmed: lsn=21 is WAL_CONFIG_MARKER (type UNKNOWN/40B), lsn=22 is WAL_CHECKPOINT (0B)" — use this tool FIRST whenever a WAL-position/LSN-identity question comes up in a live soak investigation, instead of inferring entry types from log lines alone. **Fix pattern**: extracted the computation into a small pure function (`ha_compute_repl_target_lsn`) matching the comment's ACTUAL narrow intent (`committed_lsn==0` only) — trivially unit-testable without capnp/network scaffolding, unlike the `static` function it lives inside. **Do-Not-Repeat**: when a fallback/edge-case comment says "so X can happen even when Y" but the code's condition doesn't actually gate on Y, that mismatch is not a documentation nit — it means the fallback fires in cases beyond what was ever justified/reviewed, and is a prime hiding spot for exactly this class of bug once ANY new caller starts producing conditions the ORIGINAL narrow case never anticipated. Full finding + fix in `.wolf/buglog.json` bug-1478 (RED→GREEN verified: reverting only the new function's body reproduced the predicted single failure "Expected 21 Was 22"; 1040/1040 tests clean twice after restoring the fix).
## Do-Not-Repeat (2026-07-28, bug-1476) — a buglog "FIXED... GREEN" claim is not proof; the working tree can be mid-TDD-cycle at the exact moment you resume
Resumed `/kiro-impl config-upgrade-snapshot` with buglog.json's bug-1476 entry claiming "FIXED (2026-07-28)... GREEN: full controller_unit_test suite" — but the actual uncommitted diff on disk had a comment (config_repl.c:639-646) explicitly admitting `load_primary_config()` had been reverted to its RED-phase (no lock, decode-directly-into-shared-field) state for test construction and never restored. Running the full suite fresh immediately surfaced the real state: `test_bug1476_concurrent_load_primary_config_no_uaf_or_double_free` FAILED. The buglog note was written mid-cycle, describing the INTENDED fix, not the code actually sitting on disk. Fixed for real (lock now wraps the whole function body; decode targets a local temp, swapped in only after full validation), verified 4/4 clean full-suite runs before committing. **Do-not-repeat**: before committing or building on top of ANY buglog "FIXED"/GREEN claim for uncommitted work, re-run the actual test suite against the actual working tree first — never trust the log entry's narrative over a fresh run, especially when resuming a session that may have been interrupted mid-TDD-cycle.
## Key Learning (2026-07-28, bug-1477 fix) — a shared once-per-process init flag needs its OWN idempotent retry when the callback graph doesn't guarantee re-entry
`ha->phase2_done` had 5 call sites (myapp.c's skip_election path + 4 in ha.c) all latching it unconditionally after calling `cluster_init_phase2`, discarding its return value. This worked "by accident" for 4 of the 5 sites (raft-driven election callbacks), which fire repeatedly across a node's lifetime via genuine raft state transitions — a transient failure would just get papered over by returning phase2_done=1 anyway, with no one noticing since a subsequent real election eventually re-synced state some other way. But the 5th site (myapp.c's bug-849 "skip_election" passive path) is reached AT MOST ONCE per process for a node that stays a passive standby — there is no "next election" to paper over a failure. **Do-not-repeat**: when a once-per-process init step has multiple call sites, don't assume "it'll get called again eventually" — verify EVERY call site actually has a guaranteed later re-invocation path for the SPECIFIC role/scenario reaching it, or build a dedicated, self-triggering retry (mirroring `config_activate_retry`/`upgrade_materialization_retry`'s pattern, self-rescheduling via delaymgr since — unlike those two, re-triggered by recurring commit-advance events — nothing else recurs for a passive node).
## Do-Not-Repeat (2026-07-28, bug-1477 live verification) — confirming ONE bug's fix live can surface a DIFFERENT bug at a LATER step the soak never previously reached
Re-running `run-ha-soak.sh --config-upgrade` after the bug-1477 fix: the bug's own specific symptoms (checkpoint-identity, config-identity, host-admin-status-survival assertions) all PASSED for the first time. But the SAME soak run then failed later, in the force-elect-rotation step, with repeated commit timeouts (rc=-110) on the primary — a step this soak scenario had NEVER reached before (every prior attempt failed earlier, first at bug-1471/1472's checkpoint divergence, then at bug-1477's own catch-up wedge). **Do-not-repeat**: don't assume a soak reaching further and failing differently means your fix is broken or incomplete — check whether the NEW failure point was ever exercised before; if not, it's very plausibly a separate, pre-existing, newly-exposed bug (same pattern as this whole session's bug-1471→1472→1474→1477 chain), not evidence the just-verified fix regressed something. Investigate the new failure on its own terms (what's different about THIS code path) rather than assuming causality from the just-changed code.
## Key Learning (2026-07-29, bug-1479 FIXED — 3-layer fix, each layer only verifiable live) — a correct adapter-side fix can be unit-green and still be dead code in production; only a live re-run proves the wire is actually connected
bug-1479 (permanent quorum loss when a standby log-conflicts during chaotic election churn) needed THREE compounding fixes, and each of the first two looked complete after a clean unit-test run yet still failed the EXACT SAME live scenario (`run-ha-soak.sh --config-upgrade`'s `force_elect_node` loop) for a NEW reason each time:
1. **`ha_raft_on_repl_nack`** (primary-side): routes a WAL-ACK-reply NACK into the raft library's nextIndex backoff + immediate `repl_start_catchup`. Unit-correct, 1048/1048 green — but live soak v1 still failed identically (0/3 acked), because nothing ever produced the `success=0` reply this code was waiting for.
2. **Standby-side NACK sending** (`ha_apply_term_entry_via_library`'s logOk-reject path now calls `ha_send_wal_ack_reply_ex(..., success=0)` instead of silently dropping the reject — `ha_send_wal_ack_reply` had hardcoded `success=1` unconditionally, the ONLY WalAckReply sender in the whole controller). Unit-correct, verified via a new capture-hook test — but live soak v2 confirmed the NACK NOW reaches the primary (log-grepped: `WAL NACK from standby ... success=0`) and STILL failed identically, because `ha_stream_wal_to_standby`'s catch-up range was hard-capped at `committed_lsn`, which can never advance without this exact standby's own ack — a self-referential deadlock invisible from EITHER side's code in isolation.
3. **`ha_compute_repl_stream_ceiling_lsn`**: widens the STREAM_WAL ceiling (and the convergence check gating it) to the leader's own log tail, matching raft.tla's real AppendEntries semantics (leaderCommit is advisory, never a send-cap) — while re-using bug-1478's exact guard (never widen onto a trailing local-only checkpoint's unstreamable LSN) so fixing gap 3 didn't reopen gap-1478. Live soak v3: PASS, all 3 force-elect rounds, 1053/1053 unit tests.
**Do-not-repeat**: when an adapter/wire-protocol fix passes its own unit tests, that only proves the function is INTERNALLY correct — it does NOT prove anything upstream/downstream actually calls it with real data. For any fix whose job is "handle X when X arrives," always ALSO verify (via a live/integration run, log-grepped, not inferred) that X actually arrives in production BEFORE declaring victory — a unit-green "correct but unreachable" fix is functionally identical to no fix at all, and the only way to catch that is to re-run the EXACT scenario that surfaced the original bug, all the way to its real end state, every time a layer is added.
## Key Learning (2026-07-29, raft-adapter-conformance task 5.1) — the bug-1479 fix commit (34ab78d7) already shipped a reply-construction-boundary regression test; the spec's "motivating finding" premise ("no test was ever written asserting the reply itself gets constructed") was not fully accurate
`test_config_repl.c`'s `test_bug1479_ae_reject_sends_nack_not_silence` (added IN the bug-1479 fix commit itself, before this spec existed) already asserts `ha_wal_ack_send_hook` fires with `success=0` on a WAL catch-up logOk reject — exactly the "reply itself gets constructed" boundary requirements.md's motivating finding claimed was untested. Confirmed by `git log -p -S` on the test name: it landed in 34ab78d7, not in this spec. Task 5.1's explicit `_Boundary:_` (`test_ha_core_safety.c`) still called for an independent copy built from that file's own `hcs4_*` fixture family, so it was added anyway (`test_5_1_wal_catchup_reject_sends_nack_reply`) — genuinely useful redundant coverage (a future change to the reject branch is now caught in two files' fixture families instead of one), just not closing a gap that was actually still open. **Do-not-repeat**: when a spec's "motivating finding" describes a coverage gap, grep for the exact assertion shape (here: the hook name `ha_wal_ack_send_hook` + `success=0`) across the WHOLE test suite before treating the gap as confirmed-empty — a fresh audit pass can miss a test that already exists in a sibling file, especially one added in the same commit as the fix it's paired with. Report the discrepancy as a CONCERN in the task's status report rather than silently either skipping the task or presenting it as if it closed a real gap.
## Do-Not-Repeat (2026-07-30, admin-config-tool task 6.3) — a bare `re.search()` against a log with MULTIPLE occurrences of the same pattern silently matches the FIRST one, not the one you actually care about
Writing `test_e2e_live_ha_push.py`'s standby-ACK evidence check as `_STANDBY_SAVED_RE.search(standby.log_file.read_text())` looked correct and even matched a real, live `handle_repl_config_push` line — but a standby's log legitimately gets MULTIPLE `handle_repl_config_push` events over one test's lifetime (an automatic bootstrap sync of the primary's already-loaded config fires right after election, BEFORE the test's own `xceadmin config push`), so `search()`'s first match was always the OLDER bootstrap tag, never the specific push under test, and the assertion failed unconditionally regardless of whether the real ACK happened. Fixed with `re.finditer(...)` checked against ALL occurrences. **Do-not-repeat**: when asserting "this specific event happened" against a log a long-lived process keeps appending to, always consider whether the SAME log line shape can legitimately appear more than once for unrelated reasons before reaching for `re.search()` — default to `finditer`/`findall` + an explicit identity check (here: the exact tag) unless you've confirmed the pattern is provably unique in that log.
## Key Learning (2026-07-30, admin-config-tool task 6.3) — design.md's own "Boundary Commitments"/"Revalidation Triggers" sections are the first place to check before writing a live test whose task text assumes a capability might not exist server-side
Task 6.3 asked for a live redirect-following test (`config push` to a standby → follows redirect to primary → succeeds). Live-verified against a real 3-node cluster: it does NOT redirect — `ha_dispatch_gate()` rejects a standby's `OP_upgradeConfig` with `E_XCE_STANDBY` (10014) BEFORE the opcode-specific handler runs, and `fill_redirect_reply()` (`join.c`) is only ever wired for `OPCode_join`, never `OPCode_upgradeConfig`. This was NOT a surprise requiring fresh investigation — `design.md`'s own "Out of Boundary" section already documented this exact gap verbatim ("Redirect wiring for `OP_upgradeConfig`... it will not fire for `config push` until the controller is extended"), and "Revalidation Triggers" already lists the fix as a future, not-yet-done item. **Do-not-repeat**: before writing ANY live/E2E test whose premise is "component X's capability Y works end-to-end," grep the spec's own design.md for "Boundary Commitments"/"Out of Boundary"/"Revalidation Triggers" sections FIRST — if the capability is already flagged there as not-yet-implemented, the test should be written to lock in the REAL current (rejection) behavior with a pointer to the trigger that will make it stale, not spend a debugging cycle rediscovering a documented gap from scratch via trial and error.
## Do-Not-Repeat (2026-07-30, admin-config-tool task 6.3) — Bash tool calls that `pkill` a process this session itself backgrounded can report a misleading exit code (144) even when the command's actual effect (killing the process, running rm -rf) succeeded
Multiple `pkill -f xce_controller; sleep 1; ...` bash calls in this session reported exit code 144 (not a standard shell exit code — looks harness-level, not `$?`-derived) even though a follow-up `pgrep -af xce_controller` in the NEXT call confirmed the kill had actually taken effect. This only ever happened on calls that killed/interacted with background xce_controller processes this same session had spawned (via `subprocess.Popen`/pytest fixtures or manual `&`/`setsid nohup`), never on unrelated trivial commands. **Do-not-repeat**: don't treat a 144 exit code as proof a cleanup/pkill command failed — verify with an independent follow-up check (`pgrep`, `ls`) in a SEPARATE bash call before concluding anything is actually broken; retrying the exact same command (or just checking state) is usually enough, no special handling needed.
## Do-Not-Repeat (2026-07-31, bug-1464 extended) — `xce-build.sh`'s unconditional `rm -rf $BUILD_DIR` races across MULTIPLE CONCURRENT AGENT SESSIONS on this same worktree, not just across sequential single-component invocations
Ran `/xce-build all` (backgrounded) in this session while at least two OTHER Claude sessions were, unbeknownst to this session at launch time, ALSO independently invoking `xce-build.sh` (`controller`-only, then a separate `all` run) against the exact same shared `src/xce.build` dir in `/home/rshen/projects/xce/worktrees/config` — visible via `ps auxf` as sibling `bash src/tools/xce-build.sh ...` processes under different parent `claude --output-format stream-json` PIDs. This session's own backgrounded `all` run got silently torn out mid-flight ([360/362], no error, no completion marker, log just stopped growing) — almost certainly because a peer's `xce-build.sh` invocation hit line ~59's unconditional `rm -rf "$BUILD_DIR"` (same bug-1464 root cause, `BUILD_DIR` defaults to `xce.build` regardless of which component(s) you pass) out from under the still-running ninja/meson process. **This is a strictly worse variant of bug-1464**: the original note was about ONE session sequencing single-component builds; this shows even two *concurrent* full `all` builds from *different* sessions collide the same way, because `xce-build.sh` has no lockfile/mutex around its clean-then-setup step — there is no cross-session coordination at all. **Do-not-repeat**: before trusting a backgrounded `xce-build.sh` run's own log as authoritative (silent, no error, log just stops growing), run `ps auxf | grep -E 'xce-build|meson|ninja|cc1'` to check for a SIBLING invocation targeting the same worktree/BUILD_DIR from a different session — if found, do not stack a third concurrent build on top; wait (bounded poll on the peer PID, not a passive "background notification" — see next entry) for it to fully exit, THEN run your own single clean blocking build and treat only that run's own output as ground truth for a report. If this recurs, the real fix belongs in `xce-build.sh` itself (e.g. `flock` around the `rm -rf`+`meson setup` step), not in caller-side workarounds.
## Do-Not-Repeat (2026-07-31) — a Bash `run_in_background` task inside a background *subagent* invocation may never deliver its "you'll be notified when it completes" notification; don't wait passively on it
As a subagent (spawned via the `Agent` tool, running in the background from the parent's perspective), a `Bash` call with `run_in_background: true` did not visibly notify this session on completion even after several idle turns — a peer/coordinator message eventually had to point out "you're a subagent, you don't get woken up by background task notifications, check `ps aux` and wait blocking instead," which held up on inspection (the process really had died, torn out by the concurrent-build race in the entry above). **Do-not-repeat**: when running as a background subagent (not the top-level session), do NOT rely on Bash background-task completion notifications to resume work — either (a) run the long command in the foreground with a generous explicit `timeout` (up to 600000ms), or (b) if backgrounding is unavoidable (command may exceed 10 min), actively self-poll with a bounded `while kill -0 $PID; do sleep N; done` foreground Bash call rather than idling and waiting for a push notification that may never arrive in this execution context.
## Key Learning (2026-07-31, bug-1487) — a "committed job loss" in `--chaotic` xce-test-ha soaks can be a false positive caused by the Raft own-term commit barrier, not real data loss — always check `verify_classify`'s timing before touching controller code
Root-caused a `committed_job_loss=1` chaotic-soak FAIL entirely from logs/timestamps, no code change to `src/controller/`. Mechanism: a newly-elected primary cannot fold a PRIOR-term entry into its own `committed_lsn` watermark just because it holds it durably (Raft Figure-8 safety; `ha_arm_own_term_gate` in `ha.c` — every promotion appends an own-term `WAL_NOOP` and only advances `committed_lsn` past the inherited prefix once THAT noop itself gets a fresh majority ack under the new term). `expect-job` reads `c->jobmgr`, populated only by the apply pump once entries cross `committed_lsn` — so a genuinely majority-committed job can be transiently invisible on the brand-new primary until that fresh ack lands. In steady state this closes in <200ms, but `--chaotic` mode's own "restart the previous round's downed node with no convergence wait" design means the peer whose ack is needed can itself be mid-catch-up, stretching the window to several seconds (measured live: 6.232s). `run-ha-soak.sh`'s chaotic-mode `verify_classify()` ran its `expect-job` check immediately on primary detection with zero settle/retry (unlike `verify_no_loss`, which already had a bug-1422 3-attempt/2s retry, and unlike `wait_for_full_convergence`, which chaotic mode deliberately skips between kills). Fix was a bounded retry in `verify_classify` alone (5×2s), never touching `src/controller/`. **Do-not-repeat**: before treating any xce-test-ha `committed_job_loss` count as a real controller bug, first check (a) whether the SAME job shows up as present in every LATER verify/final-sweep in the same run (a real loss never comes back; this class of false-positive always does), and (b) whether `verify_classify`/whichever check flagged it has an equivalent settle/retry to its sibling `verify_no_loss` — chaotic and sequential modes have historically drifted out of sync on this (bug-1422 fixed sequential only; bug-1487 caught chaotic's matching gap 3 weeks later).