diff --git a/.wolf/buglog.json b/.wolf/buglog.json index e61e9ef..212eb00 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -13174,7 +13174,7 @@ "error_message": "lib/controller.capnp.h:4170:43: error: unknown type name 'LimitCon'; lib/controller.capnp.h:1: error: unterminated #ifndef \u2014 during ninja compile of lib/libxce.a.p/utils.c.o, cascading into ~440 bogus errors (glibc math.h storage-class errors, utils.h ectx_t redefinition) for the rest of that one translation unit", "file": "src/xce.build/lib/controller.capnp.h (generated by [5/190] Generating lib/controllercapnp.[ch] custom command)", "root_cause": "SUSPECTED (not confirmed): parallel-build race \u2014 gcc appears to have read lib/controller.capnp.h while the custom-command generation step was still writing it (non-atomic write, no write-to-tmp+rename). Evidence: re-inspecting the SAME on-disk file immediately after the failed build shows correct, well-formed content at line 4170 (get_LimitConfig(struct LimitConfig*, LimitConfig_list, int i);) and balanced #ifndef/#endif (9/9) \u2014 i.e. the finished file is NOT actually corrupt, only what gcc read mid-write was. Not yet reproduced a second time to confirm.", - "fix": "NONE APPLIED \u2014 this run was build+report only (nix develop ./src -- xce-build.sh controller), not a fix task. Build failed via set -euo pipefail before any tests ran; no controller/xce_controller, controller_unit_test, or xce_test binaries were produced. Next step if it recurs: check meson.build custom_target() for lib/controller.capnp.[ch] has correct output declarations / add explicit depends on all consumers, or file upstream against capnpc-c invocation for atomic output. RECURRENCE 2026-07-16 (2nd occurrence, CONFIRMS the race, different symptom): clean build (rm -rf xce.build) of `nix develop ./src --command bash src/tools/xce-build.sh controller asan` at HEAD 2a76940. This time gcc failed with `fatal error: controller.capnp.h: No such file or directory` (not corrupted content) for hook.c.o, threshold.c.o, xlist.c.o, utils.c.o, window.c.o, xceapi.c.o, wrap.c.o \u2014 all 7 consumers of utils.h#include controller.capnp.h. Critically, ninja's own completion log shows `[12/191] Generating lib/controllercapnp.[ch]` printed AFTER jobs [5..11] (the 7 failed compiles) had already completed/failed. This proves the custom_target that generates lib/controller.capnp.h has no dependency edge forcing it to run before these 7 specific compile units \u2014 confirms the suspected root cause in the original entry (missing depends()/output declaration in meson.build), not just a non-atomic-write timing fluke. Still NOT fixed (build+report only task, no source edits authorized). controller_unit_test and xce_test were never produced this run (clean build wiped the prior binaries and the new build failed before relinking). RECURRENCE 2026-07-17 (3rd occurrence, kiro-validate-impl smoke-boot build for job-array-single-wal-record): fresh (non-clean) build of `nix develop ./src --command bash src/tools/xce-build.sh controller` at HEAD ea8d9d3 failed on the FIRST invocation with yet another symptom of the same race: `fatal error: opening dependency file controller/controller_unit_test.p/ha.c.o.d: No such file or directory` AND simultaneously `fatal error: opening dependency file lib/libxce.a.p/meson-generated_.._controller.capnp.c.o.d: No such file or directory` for two unrelated targets at once ([187/192] and [188/192]) -- gcc could not open its OWN .o.d dependency-file output path, not an input header this time, reinforcing this is a general ninja/meson parallel-build filesystem race in this sandbox, not specific to controller.capnp.h generation. WORKAROUND CONFIRMED AGAIN: simply re-running the identical build command with no source changes succeeded cleanly (85/85 targets, controller_unit_test 3/3 OK), producing fresh xce_controller/xce_test binaries. Do-not-repeat: if xce-build.sh fails with any \"fatal error: opening dependency file ... .o.d: No such file or directory\" or \"controller.capnp.h: No such file or directory\", just retry the same command once before treating it as a real build break -- do not start debugging source code. RECURRENCE 2026-07-20 (4th occurrence, xce-build.sh all asan build+report task validating bug-1453/1454 C-side changes, HEAD after commit 619f41f): clean build failed on FIRST invocation with fatal error: controller.capnp.h: No such file or directory for xlist.c.o, htab.c.o, window.c.o, xstring.c.o, hook.c.o, wrap.c.o, threshold.c.o, xceapi.c.o, utils.c.o, zarray.c.o (10 consumers of utils.h#include controller.capnp.h) -- ninja log again shows [16/353] Generating lib/controllercapnp.[ch] printed AFTER the failed compiles. WORKAROUND CONFIRMED AGAIN (4th time): identical retry with zero source changes succeeded cleanly, 353/353 ninja targets, all 9 meson test suites OK (0 Fail), test_cluster (part of libxce test suite) and scheduler tests all passed. Still unfixed at the meson.build dependency-edge level -- do-not-repeat guidance holds: always retry once before treating this as a real regression.", + "fix": "NONE APPLIED \u2014 this run was build+report only (nix develop ./src -- xce-build.sh controller), not a fix task. Build failed via set -euo pipefail before any tests ran; no controller/xce_controller, controller_unit_test, or xce_test binaries were produced. Next step if it recurs: check meson.build custom_target() for lib/controller.capnp.[ch] has correct output declarations / add explicit depends on all consumers, or file upstream against capnpc-c invocation for atomic output. RECURRENCE 2026-07-16 (2nd occurrence, CONFIRMS the race, different symptom): clean build (rm -rf xce.build) of `nix develop ./src --command bash src/tools/xce-build.sh controller asan` at HEAD 2a76940. This time gcc failed with `fatal error: controller.capnp.h: No such file or directory` (not corrupted content) for hook.c.o, threshold.c.o, xlist.c.o, utils.c.o, window.c.o, xceapi.c.o, wrap.c.o \u2014 all 7 consumers of utils.h#include controller.capnp.h. Critically, ninja's own completion log shows `[12/191] Generating lib/controllercapnp.[ch]` printed AFTER jobs [5..11] (the 7 failed compiles) had already completed/failed. This proves the custom_target that generates lib/controller.capnp.h has no dependency edge forcing it to run before these 7 specific compile units \u2014 confirms the suspected root cause in the original entry (missing depends()/output declaration in meson.build), not just a non-atomic-write timing fluke. Still NOT fixed (build+report only task, no source edits authorized). controller_unit_test and xce_test were never produced this run (clean build wiped the prior binaries and the new build failed before relinking). RECURRENCE 2026-07-17 (3rd occurrence, kiro-validate-impl smoke-boot build for job-array-single-wal-record): fresh (non-clean) build of `nix develop ./src --command bash src/tools/xce-build.sh controller` at HEAD ea8d9d3 failed on the FIRST invocation with yet another symptom of the same race: `fatal error: opening dependency file controller/controller_unit_test.p/ha.c.o.d: No such file or directory` AND simultaneously `fatal error: opening dependency file lib/libxce.a.p/meson-generated_.._controller.capnp.c.o.d: No such file or directory` for two unrelated targets at once ([187/192] and [188/192]) -- gcc could not open its OWN .o.d dependency-file output path, not an input header this time, reinforcing this is a general ninja/meson parallel-build filesystem race in this sandbox, not specific to controller.capnp.h generation. WORKAROUND CONFIRMED AGAIN: simply re-running the identical build command with no source changes succeeded cleanly (85/85 targets, controller_unit_test 3/3 OK), producing fresh xce_controller/xce_test binaries. Do-not-repeat: if xce-build.sh fails with any \"fatal error: opening dependency file ... .o.d: No such file or directory\" or \"controller.capnp.h: No such file or directory\", just retry the same command once before treating it as a real build break -- do not start debugging source code. RECURRENCE 2026-07-20 (4th occurrence, xce-build.sh all asan build+report task validating bug-1453/1454 C-side changes, HEAD after commit 619f41f): clean build failed on FIRST invocation with fatal error: controller.capnp.h: No such file or directory for xlist.c.o, htab.c.o, window.c.o, xstring.c.o, hook.c.o, wrap.c.o, threshold.c.o, xceapi.c.o, utils.c.o, zarray.c.o (10 consumers of utils.h#include controller.capnp.h) -- ninja log again shows [16/353] Generating lib/controllercapnp.[ch] printed AFTER the failed compiles. WORKAROUND CONFIRMED AGAIN (4th time): identical retry with zero source changes succeeded cleanly, 353/353 ninja targets, all 9 meson test suites OK (0 Fail), test_cluster (part of libxce test suite) and scheduler tests all passed. Still unfixed at the meson.build dependency-edge level -- do-not-repeat guidance holds: always retry once before treating this as a real regression. RECURRENCE 2026-07-26 (5th occurrence, build+report task validating bug-1474 repl.c fix via `nix develop ./src --command bash src/tools/xce-build.sh all asan`, HEAD c0b0c7d): TWO consecutive races in the same session, different symptoms each time. 1st invocation (clean build): classic capnp.h race -- hook.c.o etc. failed 'controller.capnp.h: No such file or directory' with the generator's status line ([6/361] Generating lib/controllercapnp.[ch]) printed AFTER the failed compiles -- identical to prior occurrences. 2nd invocation (retry, clean build): build progressed all the way to [361/361] (last job, frontend/apiserver's go_build) which FAILED with `/usr/bin/ld: cannot find -lxce: No such file or directory`; lib/libxce.a was confirmed absent from src/xce.build/lib/ immediately after the failure, and no 'Linking static target lib/libxce.a' line ever appeared in the ninja log for that invocation. NEW symptom variant: this is the first occurrence of the race hitting a Go custom_target (apiserver's CGO link against libxce.a) rather than a C compile/link step -- apiserver's CGO_LDFLAGS references @BUILD_ROOT@/lib/libxce.a only via an env-var string, which is suspected to carry no explicit ninja dependency edge on the lib archive target, letting the go_build job be eligible to run before the archive step completes. 3rd invocation (retry, clean build) succeeded cleanly: 361/361 ninja targets, meson test Ok:9 Fail:0, scheduler manual test 15/15 pass, frontend Go tests (apiserver + xcecli) all ok, xceadmin pytest 166 passed, producing fresh xce_controller/xce_test (confirmed containing the bug-1474 'reusing guard-holding slot' WARN string) and a fresh xceadmin/.venv/bin/xceadmin. WORKAROUND CONFIRMED AGAIN (do not debug source on first or second failure): simply retry the identical xce-build.sh command with no changes; this session needed up to 2 retries (3 total invocations) before a clean pass, still well within the established do-not-repeat guidance. Do-not-repeat addendum: the race is not limited to C compile/link edges -- it can also strand a Go custom_target waiting on a C static archive (apiserver <- lib/libxce.a) referenced only via env-var string interpolation, not a declared meson `depends:`/`link_with:` edge. Still unfixed at the meson.build dependency-edge level.", "tags": [ "ninja-race", "controller", @@ -13185,11 +13185,12 @@ "ninja", "missing-dependency-edge", "reproduced", - "capnp" + "capnp", + "go-apiserver-link" ], "related_bugs": [], - "occurrences": 4, - "last_seen": "2026-07-20T00:00:00Z", + "occurrences": 5, + "last_seen": "2026-07-26T04:29:00Z", "gitea_issue": 30 }, { @@ -13840,5 +13841,320 @@ "related_bugs": [], "occurrences": 1, "last_seen": "2026-07-24T15:36:56.329908Z" + }, + { + "id": "bug-1467", + "timestamp": "2026-07-25T08:55:00Z", + "error_message": "Reviewer rejected config-upgrade-snapshot task 8.4: configcompare_paths() (configcompare.c/.h) was a correct, fully-tested library function but was not reachable from anywhere outside the unit test binary -- unlike its siblings walcompare/snapverify/snapdiff, it was never registered as an xce_test CLI subcommand in test.c. `echo help | xce_test` confirmed configcompare was missing from the command list.", + "file": "src/controller/testsuite/test.c", + "root_cause": "The prior implementer built and unit-tested configcompare_paths() as a library function but never wired a cmd_configcompare() wrapper into test.c's dispatcher (include, extern decl, top_commands[], client_commands[] entries, and the wrapper function itself) -- the step every other sibling tool in this family (walcompare, snapverify, snapdiff) already had.", + "fix": "Added #include \"configcompare.h\", extern int cmd_configcompare(int, char*[]) forward decl, a { \"configcompare\", cmd_configcompare, ... } entry in both top_commands[] and client_commands[], and a cmd_configcompare() wrapper (mirroring cmd_snapdiff's exact usage-message/return-value convention: argc<3 -> usage + -1, tool ran -> 0 on match / -1 on divergence) placed immediately after cmd_snapdiff's definition in test.c. Zero changes to configcompare.c/.h, snapverify.c/.h, test_configcompare.c, test_snapverify.c, controller_test.c, or meson.build (all previously reviewed and approved as-is). Verified: controller_unit_test build clean, 1022/1022 tests pass (pure regression check, no test files touched); `echo help | xce_test` lists configcompare; hand-built /tmp fixture member dirs (conf/current + conf/cluster.bin.) smoke-tested via piped stdin -- identical-content case printed 'IDENTICAL across 2 members' with exit 0, divergent-content case printed 'DIVERGENT -- member 1 differs from member 0' with exit 1.", + "tags": [ + "config-upgrade-snapshot", + "task-8.4", + "configcompare", + "xce_test", + "cli-dispatch", + "reviewer-rejection", + "testsuite" + ], + "related_bugs": [], + "occurrences": 1, + "last_seen": "2026-07-25T08:55:00Z" + }, + { + "id": "bug-1468", + "timestamp": "2026-07-25T00:00:00Z", + "error_message": "OP_upgradeConfig sent by any network client is permanently rejected -- handle_upgrade_config() was unreachable via the wire dispatch path, exercised only by unit tests calling it directly. Independently diagnosed by the admin-config-tool spec's design.md (Boundary Commitments / Risks) as a blocking controller defect out of that spec's scope; re-discovered here while building config-upgrade-snapshot task 8.6's live OP_upgradeConfig client (no prior client existed to send it and hit the gate).", + "file": "src/controller/cluster.c", + "root_cause": "commit fd93997f (\"fix(election-iomgr-refactor): config replication and RDHUP failover chain\", 2026-05-31) inverted the OPCode_upgradeConfig primary/standby guard in myreq_handler's switch: it changed `if (!ha_is_primary(&c->ha)) reject` (correct: only primary can commit a config change) to `if (ha_is_primary(&c->ha)) reject` (rejects the ONLY role -- primary -- that ha_dispatch_gate() ever lets reach this switch case at all, since OPCode_upgradeConfig is absent from the CTRL_STANDBY/CTRL_INIT/CTRL_ELECTING allow-lists in ha_dispatch_gate, ha.c). The commit's own message text (\"standbys apply, not primaries\") reflects a since-superseded design where OP_upgradeConfig was a primary-to-standby push; do_upgrade_config_with_bytes (config_repl.c) was later substantially rewritten (ha-term-safety R7 / config-upgrade-snapshot task 7) into a client-to-primary admin op that drives config_commit_change(), but this dispatch guard was never updated to match, leaving handle_upgrade_config() reachable only by unit tests calling it directly -- never via the network.", + "fix": "Restored the pre-regression polarity in controller/cluster.c's myreq_handler OPCode_upgradeConfig case: reject with -EAGAIN when NOT primary (`if (!ha_is_primary(&c->ha))`), fall through to handle_upgrade_config() when primary. Updated the stale comment to describe the current client-to-primary semantics and cite ha_dispatch_gate as the actual primary-only enforcement point (this switch-level check is now explicitly documented as defensive redundancy). No change to config_repl.c/ha.c/persist.c materialization logic -- pure dispatch-routing fix.", + "tags": [ + "config-upgrade-snapshot", + "task-8.6", + "upgradeConfig", + "cluster.c", + "dispatch-gate", + "regression", + "admin-config-tool", + "cross-spec" + ], + "related_bugs": [], + "occurrences": 1, + "last_seen": "2026-07-25T00:00:00Z" + }, + { + "id": "bug-1469", + "timestamp": "2026-07-25T13:05:00Z", + "error_message": "config-upgrade-snapshot task 8.6's live soak (--config-upgrade, run-ha-soak.sh) FAILED on its first real run: after a config upgrade committed successfully and genuinely interleaved with a 3-round chaotic kill/restart sequence (all 6 jobs survived, upgrade confirmed committed), the post-convergence materialized-state identity check failed -- snapdiff reported ctrl-1's latest checkpoint (snap-000000000000000b.capnp, LSN 11) as 51569 bytes vs ctrl-2/ctrl-3's identical 51621-byte file. This is a live, empirical reproduction of bug-857's originally-hypothesized failure mode (config upgrade leaves members with divergent materialized state), which bug-857's own buglog entry explicitly flagged as 'NOT empirically reproduced yet' and task 8.5 could only address at the unit level (test_bug857_regression.c, a DIFFERENT and already-fixed mechanism -- schedule_checkpoint's WAL-threshold trigger racing the marker boundary). This is a third, distinct mechanism, first observed here.", + "file": "src/controller/config_repl.c do_upgrade_config_with_bytes (~1611-1770)", + "root_cause": "Byte-level diff of the two checkpoint files (both LSN 11) shows the string 'soakupgrade' (the deliberately-added 5th queue's name, part of task 8.6's upgrade-target config) appears ONCE in ctrl-1's file (inside the embedded raw active-config-bytes blob, persist.c's read_active_config_bytes -- always a fresh disk re-read) but TWICE in ctrl-2/ctrl-3's files -- the second occurrence, present only on ctrl-2/ctrl-3, sits in a queue-ENTITY list section (immediately followed by the next queue's name 'lost_and_found', a runtime enumeration, not raw config text). So ctrl-1's checkpoint correctly embeds the NEW config bytes but its runtime QUEUE-ENTITY table still reflects the OLD (pre-upgrade) queue set. Traced to the code: do_upgrade_config_with_bytes (the handler that runs on whichever node ORIGINATES/commits the upgrade -- ctrl-1 was primary in term 2 when this upgrade was submitted, confirmed via its own log line 'do_upgrade_config_with_bytes: Config upgraded to tag=... (acked=2 standbys)') saves the new bytes to disk (conf_save_config) and bumps c->myconf->config_generation, but NEVER calls config_activate_committed()/load_primary_config() to actually re-parse the new bytes INTO c->myconf -- so the very struct the checkpoint writer presumably walks to emit the queue-entity list keeps the OLD parsed queue set. apply_committed_entry's WAL_CONFIG_MARKER case (persist.c) only calls persist_checkpoint_marker_triggered() then upgrade_materialization_verify() (ha.c) -- and upgrade_materialization_verify does NOT reload/activate config either, it only (a) checks whether some PRIOR checkpoint already referenced this marker (cfgref_present/tag/sha, circularly self-referential to the checkpoint it is meant to gate) and (b) hash-verifies the LIVE ON-DISK bytes -- neither check touches c->myconf's in-memory queue table, so the Activation Fence reports 'VERIFIED' (confirmed in ctrl-1's log, 'upgrade materialization VERIFIED... materialization complete') even though c->myconf itself was never refreshed. config_activate_committed (which DOES call load_primary_config, the only place that actually re-parses queues into c->myconf) is invoked only from handle_repl_config_push (a STANDBY receiving an out-of-band config-push) and from a member's own boot/recovery path -- confirmed empirically: ctrl-1's log shows ZERO config_activate_committed calls in its original (primary, term-2) process instance, but TWO calls in its restarted instance (12:59:16 and 12:59:39, after persist_recover reported APPLY_CLEAN at last_applied_lsn=11, i.e. loading the already-frozen stale checkpoint rather than replaying). Consequence: the node that ORIGINATES a config upgrade never refreshes its own in-memory queue table as part of that commit -- its checkpoint stays consistent with the OLD queue set until some LATER, independent event (restart+recovery, or an incoming config-push) happens to call config_activate_committed. Task 8.6's scenario kills the originating primary shortly after it commits (chaotic multi-kill interleaved with the upgrade, by design), so that self-correction never gets a chance to run, and no FURTHER WAL activity occurred to trigger a follow-up checkpoint that could have picked up a later-corrected queue table -- the divergent checkpoint file is permanently frozen.", + "fix": "PARTIALLY FIXED, then RE-INVESTIGATED (2026-07-25) -- correcting this entry's own earlier premature 'FIXED' claim. Two real findings:\n\n(1) GENUINE FIX, LANDED: apply_committed_entry's WAL_CONFIG_MARKER case (persist.c) was calling config_activate_committed(c) AFTER persist_checkpoint_marker_triggered() (the checkpoint write), not before -- meaning even when activation ran, it ran too late to affect the checkpoint it was meant to fix. Reordered to run BEFORE the checkpoint call (still guarded on c->myconf != NULL, still positioned before upgrade_materialization_verify per task 6.2's own sequencing need). Verified: full controller_unit_test suite 1025/1025 PASS, 0 failures, no crash (including the previously-SEGV-prone test_marker_apply_via_live_pump_checkpoints_at_marker_own_lsn and the bug-1469 regression test_wal_config_marker_apply_activates_originating_member_config).\n\n(2) DEEPER GAP FOUND, NOT FIXED -- genuinely out of task 8.6's scope: re-running the live --config-upgrade soak AFTER the reordering fix landed produced the IDENTICAL divergence signature (same ~52-byte checkpoint delta, same duplicated-vs-single 'soakupgrade' queue name pattern) on a DIFFERENT node this time (still one node showing stale queue state). Root-caused via direct code trace: config_activate_committed() -> load_primary_config() only re-parses bytes into myconf->cluster_config (config_repl.c:614-714) -- it NEVER touches c->qmgr, the RUNTIME queue table collect_queue_frames() (persist.c:1895, the checkpoint's queue-entity-list writer) actually walks. The ONLY code that populates c->qmgr from cluster_config->queues[] is build_queues() (queue.c:28), called EXACTLY ONCE per process lifetime from xce_config_init's HOOK_AFTER_EVENT branch (cluster.c, fired during cluster_init_phase2 at process boot/restart) -- and build_queues() is UNCONDITIONAL/blind-create (no existence check before calling create_queue/makeQueue for every queue in the config), so it is UNSAFE to simply re-call on a live cluster with existing queue objects (would attempt to recreate already-existing queues, risking duplicate-key/corruption semantics that were never designed for a live re-provisioning call). Consequence: NO MEMBER -- neither the config-upgrade originator NOR a replicating standby NOR any node's own live marker-apply of any kind -- has a working mechanism to add a NEWLY-INTRODUCED queue to its runtime qmgr without a full process restart. The apparent 'originating member vs standby' asymmetry in this bug's original report was COINCIDENTAL, not causal: whichever node's restart (inevitable during a chaotic multi-kill sequence, unrelated to which node 'originated' the upgrade) happened to occur AFTER the new config bytes were already on disk got the new queue for free via its own full boot sequence (xce_config_init -> build_queues); a node whose restart happened to land BEFORE that point, or that never restarted during the scenario at all, does not -- regardless of the fix in (1). This directly contradicts design.md's own stated goal for this spec ('No controller restart is required for a configuration upgrade after this change') for the specific case of a config upgrade that ADDS a queue: today it demonstrably STILL requires a coincidental restart to take effect on any given already-running member. requirements.md's own Design Invariant #1 ('Queues are config-provisioned... update-only restore is safe') is only true for the SNAPSHOT-INSTALL restore path (a fresh/behind joiner materializing a full checkpoint, restore_queue_state, queue.c:208) -- it was never validated against the LIVE no-restart marker-apply path this spec's Requirement 7/Introduction promises, and does not hold there.\n\nNOT fixed as part of config-upgrade-snapshot task 8.6: doing so properly requires new, carefully-designed production logic (a live-safe, idempotent 'sync qmgr from the freshly committed config' function -- e.g. mirroring restore_host_state's exact find-or-create-only shape, worker.c:477-536, adapted for queue_t/c->qmgr) PLUS a deliberate design decision this task is not positioned to make safely under its own time/scope budget: what happens to a queue REMOVED by an upgrade (delete the queue_t? re-home its jobs to lost_and_found -- bug-857's own originally-hypothesized-but-never-implemented mechanism? refuse the upgrade while jobs remain queued?) -- a decision requiring its own requirements/design pass, not an implementation detail. Flagging squarely for whoever owns tasks 1-7 (persist.c/config_repl.c/queue.c materialization logic) as a genuine, previously-undiscovered gap in this spec's core 'no controller restart required' promise, confirmed via TWO independent live soak runs (before and after the ordering fix) reproducing the identical byte-level signature.", + "tags": [ + "config-upgrade-snapshot", + "task-8.6", + "bug-857", + "state-divergence", + "config_repl.c", + "do_upgrade_config_with_bytes", + "config_activate_committed", + "materialization", + "live-soak-finding", + "needs-owner", + "persist.c", + "partially-fixed", + "re-investigated", + "needs-design" + ], + "related_bugs": [ + "bug-857", + "bug-1466" + ], + "occurrences": 1, + "last_seen": "2026-07-25T13:05:00Z" + }, + { + "id": "bug-1470", + "timestamp": "2026-07-25T14:25:00Z", + "error_message": "controller_unit_test: test_commit_advance_retry_clears_push_before_commit_fence (test_config_repl.c:1854) FAILS (\"Expected TRUE Was FALSE\") DETERMINISTICALLY (4/4 runs: 1x via full xce-build.sh controller clean build + meson test, 3x direct re-invocation of the built controller_unit_test binary with no rebuild) on the bug-1469 fix commit (persist.c +91/test_config_repl.c +144, uncommitted worktree diff at time of this build). controller_unit_test overall: 1025 Tests 1 Failures 0 Ignored / meson summary Ok:2 Fail:1.", + "file": "src/controller/testsuite/test_config_repl.c", + "root_cause": "NOT YET DIAGNOSED. Distinguishing context vs the two prior entries for this same test name: bug-1407 (closed 2026-07-03) root-caused an earlier failure of this exact test to a STALE INCREMENTAL ninja build artifact in this worktree, conclusively fixed by a fresh cycle -- but this run WAS already a fresh clean build (xce-build.sh always does rm -rf + meson setup, confirmed src/tools/xce-build.sh:59-60), so bug-1407s explanation does not apply here. bug-1466 (2026-07-24) found a genuine CAS timing race between the commit-thread and persist_start()s fsync_worker thread that could affect this same test family after task 5s WAL_CONFIG_MARKER-triggers-real-checkpoint change, and claimed to fix it via t42_wait_applied() polling -- but the failure seen here (\"Expected TRUE Was FALSE\", a plain boolean assertion) is 100% reproducible across reruns of the same binary, which argues AGAINST a timing race (races are not usually 4/4 deterministic within one build) and FOR either (a) a residual gap in bug-1466s fix, or (b) a genuine regression introduced by the just-made bug-1469 persist.c change (adding an unconditional config_activate_committed() call inside apply_committed_entrys WAL_CONFIG_MARKER case, immediately before upgrade_materialization_verify()) altering ordering/state that this specific test asserts on. bug-1469s own buglog entry claims a prior direct-ninja verification run saw 1024/1024 PASS 0 failures on the same source -- so either that verification predates a subsequent edit, or ran under different build state; this needs reconciling before task 8.6 soak work continues.", + "fix": "NOT FIXED -- reported to the requesting task/session for triage; out of scope for the build-verification task that discovered it (was asked only to get a clean component build + report status, not to debug). Repro is cheap: nix develop ./src --command bash src/tools/xce-build.sh controller (full clean rebuild) or, faster, just rerun the already-built binary: ASAN_OPTIONS=detect_leaks=0 ./src/xce.build/controller/controller_unit_test 2>&1 | grep -A2 test_commit_advance_retry_clears_push_before_commit_fence. Full logs preserved this session at src/xce.build/meson-logs/testlog.txt (search test_config_repl.c:1854).", + "tags": [ + "testsuite", + "controller", + "config-upgrade-snapshot", + "bug-1469", + "commit-engine", + "test-config-repl", + "needs-diagnosis", + "possible-regression" + ], + "related_bugs": [ + "bug-1407", + "bug-1466", + "bug-1469" + ], + "occurrences": 4, + "last_seen": "2026-07-25T14:25:00Z" + }, + { + "id": "bug-1471", + "timestamp": "2026-07-25T14:45:00Z", + "error_message": "config-upgrade-snapshot task 8.6 live --config-upgrade soak: after avoiding bug-1469's queue-creation gap (switched the upgrade-target config change from ADDING a queue to updating an existing queue's description text -- see bug-1469's updated entry), the materialized-state identity check (snapdiff of each member's latest checkpoint file, all at the same LSN) still fails, but now with a MUCH smaller divergence: exactly ONE byte differs (offset 42879/51161 bytes total) between ctrl-1 and ctrl-2/ctrl-3 (ctrl-2 and ctrl-3 are byte-IDENTICAL to each other). ctrl-1 has 0x50 at that offset; ctrl-2/ctrl-3 have 0x00.", + "file": "src/controller/persist.c collect_host_frames / capnp_encode_host_admin_state (~lines 141-162, 2028)", + "root_cause": "ROOT-CAUSED AND DISTINGUISHED (2026-07-25, follow-up investigation): the two prior hypotheses are now resolved -- Hypothesis 2 (uninitialized/non-zeroed capnp padding) is REFUTED, Hypothesis 1 (genuine content discrepancy) is CONFIRMED, and the true mechanism is a pre-existing, out-of-spec-boundary bug, NOT part of config-upgrade-snapshot. Evidence: (1) Wrote a byte-precise decoder in Python that walks the checkpoint's [host_count][{payload_len; capnp bytes}] host-frame section (same structural approach as snapverify.c task 8.4) and decodes each frame using the exact wire shape write_HostAdminState produces (HostAdminState_word_count=1, pointer_count=1: one 8-byte data word holding `status` via capn_write32 at data-section offset 0, one 8-byte pointer word for `name` Text). Decoding BOTH /tmp/xce-ha-configupgrade and /tmp/xce-ha-configupgrade-v2's preserved checkpoints for all 3 members shows: host[1] = 'cfgupgrade-host', ctrl-1 status=0x00500000, ctrl-2/ctrl-3 status=0x00000000 -- byte offset 42879 is byte index 2 of the little-endian `status` DATA FIELD itself (data-section file offset 42877-42880), NOT the 4-byte padding word that follows it (file offset 42881-42884), which is 0x00000000 IDENTICALLY across all 3 members in both reproductions. (2) 0x00500000 = XCE_WORKER_CLOSED (0x00100000) | XCE_WORKER_CLOSED_BY_ADMIN (0x00400000) -- exactly the bits worker.c:779 sets for HOST_CLOSE inside worker_ctrl(). This is semantically meaningful admin-closed status, not heap garbage. (3) Independently confirmed capn_init_malloc's segment allocator (create_local in the pinned c-capnproto build, /nix/store/.../capnpc-1.0.1/lib/libCapnC_Runtime.so, disassembled directly) calls calloc(1, size) -- zero-initializing -- so Hypothesis 2 was never physically possible for this library, consistent with the empirically-always-zero padding bytes. (4) Traced the actual mechanism: worker_ctrl() (worker.c:732-794) sets w->host.status on whichever member directly processes the hostctrl RPC (ctrl-1, primary at the time), then fires HOOK_DETAIL_WORKER_UPDATE, which persist.c:5722 maps to WAL_WORKER_CTRL and replicates. On EVERY member applying that WAL entry (standbys AND a member's own replay-from-WAL/restart path alike), persist.c:3724's case WAL_WORKER_CTRL calls worker_from_capnp() (worker.c:827-856) UNCONDITIONALLY, regardless of hook_detail (JOIN vs UPDATE vs DEAD all share this one apply path). worker_from_capnp's own idempotency guard (worker.c:847-853) does key_find() and returns -EEXIST if a worker with that name ALREADY EXISTS -- WITHOUT applying any of the just-decoded fields, including status. persist.c:3732-3733 then treats -EEXIST as 'idempotent re-apply, no hook' and silently returns 0, discarding the decoded status update entirely. Since cfgupgrade-host was JOINED (a prior, separate WAL_WORKER_CTRL/HOOK_DETAIL_WORKER_JOIN entry) BEFORE it was admin-closed, by the time the UPDATE entry replicates/replays, the worker already exists on every standby -- so the close never lands there. This is NOT specific to config upgrades, checkpoints, or any config-upgrade-snapshot code: it reproduces for ANY `hostctrl -c`/`hostctrl -o` issued against an already-joined host on ANY XCE HA cluster, with zero dependency on a config upgrade ever being submitted. `git log -L` on worker_from_capnp confirms it was introduced 2026-05-08 in commit 9b6a91a8 ('feat: implement restore_job/worker_from_capnp for WAL recovery'), ~2.5 months before config-upgrade-snapshot's spec work began (first spec commit visible ~2026-06-24); no config-upgrade-snapshot task touches this apply path (the spec's own worker.c addition, restore_host_state from commit 5b1600a8, is a wholly separate function used only by the SNAPSHOT-INSTALL restore path, task 4, never by the live WAL_WORKER_CTRL apply switch). collect_host_frames/capnp_encode_host_admin_state (this spec's task 2.1 code) are functioning exactly as designed -- they faithfully serialize whatever w->host.status currently holds; the bug is that w->host.status itself is wrong on ctrl-2/ctrl-3, entirely upstream of any checkpoint/materialization logic. Existing test coverage (test_apply_committed_entry.c:test_ace_worker_ctrl_join_restores_and_notifies) only exercises the fresh-JOIN case; no test exercises WAL_WORKER_CTRL UPDATE against an already-existing worker, confirming this is a genuine untested gap, not a previously-handled case. Filed separately as bug-1472 (the true root cause) since it is a distinct, real, pre-existing mechanism entirely outside config-upgrade-snapshot's boundary -- see that entry for the fix-owner routing. This entry (bug-1471) remains the observed SYMPTOM (checkpoint byte divergence) for task 8.6's soak; the underlying CAUSE is bug-1472.", + "fix": "NOT FIXED, AND DELIBERATELY NOT ATTEMPTED IN THIS SPEC -- confirmed genuinely OUT OF config-upgrade-snapshot's scope (see root_cause above). The real fix belongs in worker.c's worker_from_capnp()/persist.c's WAL_WORKER_CTRL apply case (case WAL_WORKER_CTRL, persist.c:3724), which needs to distinguish JOIN semantics (create-if-missing, EEXIST is a legitimate no-op) from UPDATE/DEAD semantics (an already-existing worker is the EXPECTED case, and the decoded fields -- at minimum host.status -- must still be applied to it, not discarded) -- e.g. an update-in-place path mirroring how restore_host_state (task 3, worker.c, config-upgrade-snapshot's OWN precedent) ORs admin bits into an existing worker rather than treating existing-ness as terminal. That is a genuine, real, previously undiscovered defect (filed as bug-1472) but is squarely core HA worker-control replication, predating this spec by ~2.5 months and untouched by any of its tasks -- fixing it here would be scope creep into a different subsystem's correctness contract, not a config-upgrade-snapshot materialization fix. Task 8.6's soak will keep observing this exact symptom (host-frame byte divergence whenever its scenario admin-closes a host before the final checkpoint) until bug-1472 is fixed by whoever owns worker.c/persist.c's general worker-control replication path. Recommend task 8.6 either (a) accept this as a known, separately-tracked limitation and adjust the soak's own PASS criteria to tolerate it (e.g. skip the admin-close-a-host step, or snapdiff-exclude the host-admin-status byte range) until bug-1472 lands, or (b) block final PASS on bug-1472's fix -- an orchestrator/owner decision, not this investigation's to make.", + "tags": [ + "capnp", + "capnp_encode_host_admin_state", + "collect_host_frames", + "config-upgrade-snapshot", + "distinguished", + "host-frame", + "hypothesis-1-confirmed", + "hypothesis-2-refuted", + "live-soak-finding", + "materialization", + "needs-owner", + "out-of-scope", + "root-caused", + "routed-to-bug-1472", + "single-byte-divergence", + "task-8.6" + ], + "related_bugs": [ + "bug-1469", + "bug-1472" + ], + "occurrences": 3, + "last_seen": "2026-07-25T16:35:00Z" + }, + { + "id": "bug-1472", + "timestamp": "2026-07-25T16:35:00Z", + "error_message": "Host admin-control status changes (hostctrl -c / hostctrl -o, i.e. HOST_CLOSE/HOST_OPEN via worker_ctrl()) never propagate to standby members' in-memory worker state once the target host has already been joined -- discovered while root-causing bug-1471's single-byte checkpoint divergence, but the defect itself is general and has nothing to do with checkpoints, config upgrades, or the config-upgrade-snapshot spec.", + "file": "src/controller/worker.c worker_from_capnp (~827-856); src/controller/persist.c case WAL_WORKER_CTRL apply path (~3724-3786)", + "root_cause": "worker_from_capnp() (worker.c:827-856) is invoked UNCONDITIONALLY by persist.c:3731 for every WAL_WORKER_CTRL entry, regardless of its hook_detail discriminator (JOIN, UPDATE, or DEAD all share this one apply path -- see persist.c:3766-3783's switch, which only controls which post-apply NOTIFICATION hook fires, not whether the mutation itself is applied). worker_from_capnp's own idempotency guard (worker.c:847-853): if key_find() on c->workermgr finds a worker with the decoded name ALREADY PRESENT, it returns -EEXIST immediately, WITHOUT applying any decoded field (including host.status) to the existing worker object. persist.c:3732-3733 treats any -EEXIST return as 'idempotent re-apply (replay / resync) -- no hook' and silently returns 0. This EEXIST short-circuit is correct for its originally-intended purpose (a duplicate/replayed JOIN entry for a worker that's already been created should indeed be a no-op), but WAL_WORKER_CTRL is also the opcode for HOOK_DETAIL_WORKER_UPDATE (worker.c:732-794's worker_ctrl(), which handles HOST_OPEN/ HOST_CLOSE admin commands, sets w->host.status, then fires exactly this same HOOK_AFTER_EVENT/HOOK_DETAIL_WORKER_UPDATE/WAL_WORKER_CTRL path at worker.c:786-787) and HOOK_DETAIL_WORKER_DEAD. For UPDATE/DEAD events, the target worker EXISTING is the NORMAL, EXPECTED case (you can only update or kill an already-joined worker) -- yet the same 'already exists -> skip entirely' logic applies, so the decoded status change is silently discarded on every member OTHER than the one that directly processed the original RPC. Confirmed via git blame: worker_from_capnp was introduced 2026-05-08 (commit 9b6a91a8, 'feat: implement restore_job/worker_from_capnp for WAL recovery'), predating config-upgrade-snapshot's spec work by ~2.5 months; no config-upgrade-snapshot task touches this function or the WAL_WORKER_CTRL apply case. Empirically confirmed via byte-precise capnp decode of two independent live-soak checkpoint sets (/tmp/xce-ha-configupgrade{,-v2}): ctrl-1 (which processed a `hostctrl -c` against a previously-joined synthetic host) has host.status=0x00500000 (XCE_WORKER_CLOSED|XCE_WORKER_CLOSED_BY_ADMIN) in its checkpoint's HostAdminState frame; ctrl-2/ctrl-3 (standbys) both show host.status=0x00000000 for the identically-named host entry -- proving the close never reached their live c->workermgr entries. Because a standby's OWN restart+full-WAL-replay hits the identical code path (persist.c's replay call site also routes through apply_committed_entry's same WAL_WORKER_CTRL case), a member that fully replays from LSN 1 would ALSO reconstruct the JOIN first (creating the worker) and then skip the later UPDATE (already exists) -- so this is not even a restart-repairable staleness; the divergence appears to be permanent for that member until directly patched or until some other mechanism re-applies the admin action. General severity: this affects ANY admin host-control action (`hostctrl -c`/`hostctrl -o`) against an already-joined host, unconditionally, cluster-wide -- not just this soak's synthetic cfgupgrade-host scenario.", + "fix": "FIXED 2026-07-25 (TDD: RED test first, confirmed failing against the pre-fix code, then GREEN). worker_from_capnp() (src/controller/worker.c) now decodes the WAL_WORKER_CTRL entry's hook_detail discriminator (the same 4 leading bytes it previously skipped without inspecting) BEFORE applying its existing-worker idempotency guard. When the target worker already exists: HOOK_DETAIL_WORKER_JOIN keeps the original -EEXIST no-op (a duplicate JOIN replay is genuinely idempotent -- unchanged, regression-guarded). HOOK_DETAIL_WORKER_UPDATE now REPLACES (not ORs -- worker_ctrl()'s HOST_OPEN uses assignment-style &= ~(...) to CLEAR admin bits, so OR would leave a re-opened worker stuck closed) the existing worker's XCE_WORKER_ADMIN_MASK bits with the decoded payload's admin-masked bits, under the worker's own write lock (mirrors restore_host_state's masking discipline, worker.c:487, with replace instead of OR semantics), then returns 0 so persist.c's WAL_WORKER_CTRL apply case (unchanged -- its existing '0 on any successful apply, -EEXIST only for genuine JOIN no-op' contract already handled this generically) falls through to fire the correct HOOK_DETAIL_WORKER_UPDATE notification instead of silently discarding the status change. HOOK_DETAIL_WORKER_DEAD is DELIBERATELY LEFT UNFIXED (still -EEXIST/no-op) -- investigated, not skipped: mark_worker_unavail()'s mutation (worker.c:661-679) is NOT confined to XCE_WORKER_ADMIN_MASK (sets XCE_WORKER_UNAVAIL + dead_since + resets hostattrs_t to 'unknown' + zeroes resource-value slots via set_host_slots), and critically, the live per-host resource-value array xce_worker_t.resvals (a zarray_t, what set_host_slots actually mutates and what scheduling reads) is NOT part of the Worker capnp wire schema at all -- only the separate resvals_to_sched/nresvals_to_sched fields are (confirmed by reading the generated decode_Worker/encode_Worker in controller.capnp.c and the schema in controller.capnp). A naive status-replace for DEAD would be an incomplete fix (would not zero standbys'/replayers' resource slots on a dead-worker event, a real correctness gap for POST-PROMOTION scheduling since the newly-promoted primary would read stale, non-zeroed slots) and needs its own investigation into whether/how to extend the wire schema -- filed separately as bug-1472-remainder below rather than rushed. Fix verified via 2 new + regression tests in test_apply_committed_entry.c (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 -- the last one a regression guard proving JOIN's existing no-op behavior is unchanged) plus the full controller_unit_test suite: baseline (this worktree, immediately pre-fix) 1030 Tests 0 Failures, post-fix 1033 Tests 0 Failures (net +3, exactly the new tests, zero regressions). persist.c was NOT touched -- confirmed its WAL_WORKER_CTRL apply case (persist.c:3724-3786) already treats any non-negative worker_from_capnp() return as a successful apply generically (re-looks-up the worker by name and fires the hook_detail-keyed notification unconditionally), so returning 0 from the new UPDATE branch required no caller-side change.", + "tags": [ + "worker.c", + "persist.c", + "worker_from_capnp", + "WAL_WORKER_CTRL", + "host-control-replication", + "hostctrl", + "admin-status", + "standby-divergence", + "pre-existing", + "out-of-config-upgrade-snapshot-scope", + "needs-owner", + "root-caused", + "not-fixed" + ], + "related_bugs": [ + "bug-1471", + "bug-1472-remainder" + ], + "occurrences": 1, + "last_seen": "2026-07-25T20:15:00Z" + }, + { + "id": "bug-1473", + "timestamp": "2026-07-25T19:12:00Z", + "error_message": "test_second_config_marker_supersedes_first_still_installing_on_member (task 8.8) FAILED on first run: Expected 0 Was 1 -- the fence must clear once materialization for the latest marker (B) completes.", + "file": "src/controller/testsuite/test_config_repl.c (test-only; no production code involved)", + "root_cause": "Test-construction gap, not a production bug. After releasing the checkpoint_in_progress CAS in the raced fixture, the test called the bare persist_checkpoint() to resolve the in-flight-checkpoint scenario, then read upgrade_materialization_fenced() directly -- but plain persist_checkpoint() never calls upgrade_materialization_verify() itself. That call only happens from two production call sites: apply_committed_entry's WAL_CONFIG_MARKER case (fires only on a NEW marker apply, not a bare checkpoint), and the delaymgr-driven retry trampoline (schedule_upgrade_materialization_retry), which explicitly no-ops when delaymgr is not running -- its own doc comment says so verbatim: \"No-op when... the delaymgr is not running (the unit-test fixture path...) -- caller drives directly.\" The bare t42/t43 test fixture (used throughout test_config_repl.c) never inits a delaymgr, so nothing in the raced fixture would ever re-evaluate the fence after the resolving checkpoint landed -- the fence stayed fenced=1 even though materialization for marker B had genuinely completed.", + "fix": "Added an explicit upgrade_materialization_verify(c_raced) call immediately after the resolving persist_checkpoint() call, mirroring the exact pattern already used by the precedent 6.1-c fence tests earlier in this same file (which call upgrade_materialization_verify(c) directly after t43_commit_marker for the same delaymgr-less-fixture reason). Also asserted verify_rc == 0. Confirmed via log trace that upgrade_materialization_verify only logs on a fence 0->1 transition (ha.c:254-256, \"never spams on repeated failed retries\") -- explaining why no second ERROR line appeared for markerB in the raw log even before the fix, which was the first clue this was a missing-caller issue rather than a fence-logic issue.", + "tags": [ + "config-upgrade-snapshot", + "task-8.8", + "test-only", + "upgrade_materialization_verify", + "activation-fence", + "delaymgr", + "test-construction", + "not-a-production-bug" + ], + "related_bugs": [], + "occurrences": 1, + "last_seen": "2026-07-25T19:12:00Z" + }, + { + "id": "bug-1474", + "timestamp": "2026-07-25T20:50:00Z", + "error_message": "config-upgrade-snapshot task 8.9's new live --config-upgrade-wal-trim soak (run-ha-soak.sh, run_config_upgrade_wal_trim_round): the marker-triggered checkpoint's WAL-segment trim (persist_checkpoint_marker_triggered -> trim_wal_segments -> repl_trim_lsn, bug-1147's guard) strands a genuinely mid-catch-up standby's still-needed WAL segment -- but NOT because bug-1147's guard logic itself is wrong. Reproduced identically on 2/2 independent live 3-node runs: a killed, still-down standby's disconnect-guard (last_disconnected_lsn) is silently DESTROYED before the trim ever runs, because an UNRELATED, already-connected peer's redundant reconnect attempt reuses the dead standby's just-freed replicator slot.", + "file": "src/controller/ha.c (primary_heartbeat_fn's 'reconnect to any peer missing from standby_endpoints' loop, ~9219-9253; on_standby_connected, ~8658-8708; raft_cb_become_leader's vote-ep promotion, ~1888-1921); src/controller/repl.c (repl_add_standby's first-empty-slot allocation, ~85-133, and its memset(s,0,...) which wipes last_disconnected_lsn on reuse)", + "root_cause": "Confirmed via direct log evidence on 2 independent live-soak runs (deploy dirs /home/claude/tmp/xce-ha-waltrim and /home/claude/tmp/xce-ha-waltrim2, both --config-upgrade-wal-trim). Sequence, identical both times: (1) An election promotes surviving VOTE endpoints to standby replication channels (raft_cb_become_leader: 'promoted vote ep[i] -> standby channel', repl_add_standby registers them at replicator slots 0/1 -- e.g. run 1: ep[0]=ctrl-1 -> slot 0, ep[1]=ctrl-3 -> slot 1). (2) The test kills a non-primary TARGET standby (ctrl-1) while it is caught up to a known WAL floor. The primary's err_handler (vote_ep_err_handler, correctly wired) detects the RDHUP and calls repl_remove_standby, freeing slot 0 and setting last_disconnected_lsn = ctrl-1's real acked_lsn (confirmed matching the test's own captured floor segment, e.g. seg-5 in run 1, seg-c in run 2) -- bug-1147's guard is set CORRECTLY at this point. (3) Seconds later (matching the ~9s default heartbeat interval), primary_heartbeat_fn's periodic 'reconnect to any peer missing from standby_endpoints' sweep (ha.c ~9219) runs. It correctly identifies ctrl-1 (peer[0]) as missing (reconnect attempt fails with ECONNREFUSED, confirmed in both runs' logs: 'HA: connect to peer[0] (localhost:9991) failed: -111') -- but it ALSO, incorrectly, identifies ctrl-3 (peer[2]) as missing, even though ctrl-3 has been continuously connected the whole time via its slot-1 promoted-vote-channel connection (confirmed acking WAL entries without any gap throughout, via ctrl-2.log's 'commit: ack standby_idx=1' sequence). This triggers a redundant outbound reconnect to ctrl-3 (on_standby_connected fires: 'HA: outbound standby connection to peer[2] established' + 'HA: repl registered late-joiner standby peer[2] (slot 0, localhost:9993)'). (4) repl_add_standby's slot allocation picks the FIRST EMPTY slot by lowest index (repl.c ~85-110) with no regard for whether that slot still holds an unexpired bug-1147 disconnect guard for a DIFFERENT peer -- it picks slot 0 (ctrl-1's just-freed, guard-holding slot, since it is lower-indexed than slot 1) and unconditionally memsets it to zero, WIPING OUT ctrl-1's last_disconnected_lsn guard entirely. (5) When the marker-triggered checkpoint's trim runs shortly after, repl_trim_lsn (repl.c ~650-683) sees slot 0 now occupied by a live, fully-caught-up ctrl-3 duplicate connection (contributing no meaningful floor-lowering) and slot 1 also fully caught up -- with NO surviving trace that ctrl-1 (still down, still needing its old floor) was ever disconnected. floor collapses to the active segment itself, and every segment below it -- including the one ctrl-1 genuinely still needs -- is deleted. This is NOT a bug in bug-1147's guard logic (repl_trim_lsn/trim_wal_segments are working exactly as designed against the replicator state they are handed); it is an upstream defect in how a duplicate/late-joiner reconnect for an ALREADY-connected peer gets allocated a replicator slot. Root cause of WHY the 'peer already has a standby endpoint' check (ha.c ~9225-9249) fails to recognize ctrl-3's existing promoted-vote-channel connection is not fully pinned to a single line -- standby_ep_ctx_t is deliberately layout-compatible with vote_ep_ctx_t (confirmed by direct struct read, ha.c:8590-8603, both start with ha/c/ctx/peer_idx in the same order, so the blind cast the check performs is NOT a type-confusion bug) -- but vote_ep_ctx_t.peer_idx appears to be populated under inconsistent conventions across its several creation call sites in ha.c (e.g. line 542 sets peer_idx=votes_idx [relative, votes-array index] while a comment at line ~6120 asserts 'vctx->peer_idx is the ABSOLUTE cluster index' for a different call site) -- a mismatch between the relative votes_idx_for_i the check computes and whichever convention the SPECIFIC promoted vote_ep_ctx_t for ectx->votes[i].ep was actually built under would make the sctx->peer_idx == votes_idx_for_i comparison fail deterministically for promoted-vote-channel entries, matching the 100% reproduction rate observed. This needs further tracing by whoever owns ha.c's HA connection-management code to identify the exact mismatched call site.", + "fix": "NOT FIXED -- explicitly out of config-upgrade-snapshot task 8.9's boundary (task 8.9 is a regression test for the pre-existing, unmodified bug-1147 guard under the NEW marker-triggered trigger path, not a fix for this separately-discovered HA connection-management defect). Two independent, complementary fix angles for whoever owns ha.c/repl.c's HA connection layer: (a) fix the 'peer already has a standby endpoint' check (ha.c ~9225-9249) so it correctly recognizes an existing promoted-vote-channel connection for a peer, eliminating the spurious duplicate/late-joiner reconnect entirely (the more targeted fix, addresses the root cause) -- likely requires auditing every vote_ep_ctx_t.peer_idx assignment site (ha.c lines 542, 697, 6956, and whichever one specifically constructs ectx->votes[i].ep's ep->data) against the relative-votes_idx convention votes_idx_for_i expects; (b) independently harden repl_add_standby's slot allocation (repl.c) to never silently clobber a slot whose last_disconnected_lsn guard has not yet expired per snap_interval_s -- e.g. skip a guard-holding slot in the first-empty-slot scan (falling through to a genuinely empty slot, or failing loud if none exists) -- this is a defense-in-depth hardening of bug-1147's own retention mechanism against ANY future caller that might reuse a slot prematurely, not just this specific duplicate-connection path. Recommend (b) regardless of whether (a) is fixed, since it directly protects the invariant Requirement 7.4 cares about. The run_config_upgrade_wal_trim_round test infrastructure (run-ha-soak.sh, config-upgrade-snapshot task 8.9) is CORRECT and reproducibly catches this defect via direct on-disk WAL segment inspection -- re-run `run-ha-soak.sh --config-upgrade-wal-trim` after either fix lands to confirm a clean PASS; task 8.9 itself should stay incomplete/blocked on this bug per its own scope boundary (mirrors task 8.6's bug-1471/1472 precedent).\n\n[UPDATE 2026-07-26] FIXED (fix angle (b), the repl.c defense-in-depth hardening): repl_add_standby's slot-allocation scan (repl.c) now does a two-pass search instead of first-empty-slot-by-index. Pass 1 prefers a genuinely virgin slot (ep==NULL && last_disconnected_lsn==0). Pass 2 (fallback, only when no virgin slot exists) reuses the guard-holding slot (ep==NULL && last_disconnected_lsn!=0) with the OLDEST last_disconnect_time -- a deliberate policy: that guard is closest to its own natural repl_expire_disconnect_slots expiry, so reusing it discards the LEAST remaining WAL-retention protection of any guard-holding slot. With MAX_STANDBYS==3, the exhausted case (both non-primary slots simultaneously guard-holding, no virgin slot free) is handled by this same fallback rather than -ENOSPC -- refusing a legitimate new/reconnecting peer would be worse for cluster availability than accepting an early guard loss in an already-degraded state; the pre-existing -ENOSPC path (more distinct live peers than MAX_STANDBYS slots) is untouched, reserved for that genuinely-impossible case only.\nTDD: two new unit tests added to src/controller/testsuite/test_repl_unit.c -- test_repl_add_standby_prefers_virgin_slot_over_guard (RED under the old code: asserted a new unrelated peer must not land in a lower-indexed guard-holding slot when a virgin slot exists, and that the guard's last_disconnected_lsn must survive untouched) and test_repl_add_standby_exhausted_guards_prefers_oldest_disconnect (RED under the old code: asserted the exhausted-fallback deliberately picks the OLDER-disconnected guard slot, not merely the lowest-indexed one -- the test deliberately makes 'oldest-disconnected' and 'lowest-indexed' point at DIFFERENT slots so an accidental first-guard-by-index fallthrough would still fail it). Both new tests failed pre-fix, pass post-fix.\nGREEN: full controller_unit_test suite = 1030 tests (1028 known-good baseline + 2 new), 0 failures, no regressions (ASAN_OPTIONS=detect_leaks=0, nix develop build via `ninja -C src/xce.build controller/controller_unit_test`).\nrepl_remove_standby, repl_expire_disconnect_slots, repl_trim_lsn, and trim_wal_segments were NOT modified -- all confirmed correct and untouched, per this fix's scope.\nThis fix does NOT address fix angle (a) -- the separate ha.c heartbeat-reconnect-sweep defect (vote_ep_ctx_t.peer_idx convention mismatch hypothesis, ha.c ~9219-9254 / ~8687-8708) that triggers the spurious duplicate reconnect in the first place -- that remains a separate, not-fully-root-caused issue, out of this fix's scope. Recommend re-running `run-ha-soak.sh --config-upgrade-wal-trim` (config-upgrade-snapshot task 8.9) to confirm this defense-in-depth closes the observed WAL-segment-loss symptom even without (a) landing.\n\n[UPDATE 2026-07-26, post-review remediation] Independent review REJECTED the initial fix on one finding: the Pass 2 exhausted-fallback branch silently destroyed the reused slot's last_disconnected_lsn/last_disconnect_time via memset with zero logging, undercutting the fix's own stated goal of making the previously-silent WAL-retention-guard destruction visible/bounded -- this file already has an established convention for exactly this class of event (repl_broadcast's WARN(\"repl_broadcast: no standbys registered...\") and WARN(\"repl_broadcast: standby %s:%u evicted...\")). Remediated: repl_add_standby now tracks whether Pass 1 found a virgin slot (used_fallback_slot) and, only on the genuine Pass 2 fallback path, logs WARN(\"repl_add_standby: reusing guard-holding slot %d (host=%s:%u, last_disconnected_lsn=%lu) -- no virgin slot available, WAL-retention protection discarded\", ...) immediately before the memset, capturing the discarded slot's host/port/last_disconnected_lsn while they're still readable. No new test required (a log line has no observable return-value/state effect for a test to assert on) -- verified instead by confirming the WARN string is present in the built binary (`strings controller_unit_test | grep 'reusing guard-holding slot'`) and that both existing bug-1474 tests (test_repl_add_standby_prefers_virgin_slot_over_guard, test_repl_add_standby_exhausted_guards_prefers_oldest_disconnect -- the latter specifically exercises the Pass 2 path this WARN sits on) still pass unchanged. GREEN: full controller_unit_test suite = 1033 tests (1030 baseline + bug-1472's 3 concurrently-landed tests), 0 failures. NOTE: a transient stale-object-file build artifact was observed once in this shared build directory during verification (a link picked up an out-of-date worker.c.o from a moment when a concurrent sibling agent's build was in flight, showing 2 spurious failures unrelated to this change) -- resolved by touching the affected sources to force recompilation; not a defect in this fix, noted here in case it recurs for a future concurrent multi-agent session in this worktree.", + "tags": [ + "ha.c", + "repl.c", + "trim_wal_segments", + "repl_trim_lsn", + "repl_add_standby", + "bug-1147", + "standby_endpoints", + "late-joiner", + "slot-reuse", + "duplicate-connection", + "promoted-vote-channel", + "config-upgrade-snapshot", + "task-8.9", + "live-soak-finding", + "root-caused", + "reproducible-2-of-2", + "needs-owner", + "out-of-scope", + "wal-trim", + "mid-catch-up", + "fixed", + "defense-in-depth", + "log-hygiene", + "review-remediation" + ], + "related_bugs": [ + "bug-1147", + "bug-1471", + "bug-1472" + ], + "occurrences": 2, + "last_seen": "2026-07-26T04:00:00Z" + }, + { + "id": "bug-1472-remainder", + "timestamp": "2026-07-25T20:15:00Z", + "error_message": "Follow-up split out of bug-1472: HOOK_DETAIL_WORKER_DEAD entries applied via WAL_WORKER_CTRL against an already-existing worker still return -EEXIST (no-op) -- a dead-worker transition detected by one member (mark_worker_unavail(), worker.c:661-679) still never replicates its state to other members (standbys, or this member's own WAL replay), unconditionally. NOT a regression: this was already true before bug-1472's investigation; bug-1472's fix deliberately scoped itself to the byte-precise-confirmed HOOK_DETAIL_WORKER_UPDATE case only and left DEAD as -EEXIST/no-op, unchanged.", + "file": "src/controller/worker.c worker_from_capnp (~827-895, post-bug-1472-fix); mark_worker_unavail (~661-679); src/schema/controller.capnp struct Worker (~136-144)", + "root_cause": "mark_worker_unavail()'s mutation is NOT confined to XCE_WORKER_ADMIN_MASK the way worker_ctrl()'s is -- it sets XCE_WORKER_UNAVAIL (a non-admin bit), dead_since, resets hostattrs_t to 'unknown' sentinel strings, AND zeroes the live per-host resource-value slots via set_host_slots()/update_resvals(), which mutates xce_worker_t.resvals (a zarray_t of live xce_val_t entries). Critically, w->host.resvals is NOT part of the Worker capnp wire schema at all (confirmed by reading src/schema/controller.capnp's struct Worker and the generated decode_Worker/encode_Worker in controller.capnp.c) -- only the separate resvals_to_sched/nresvals_to_sched fields (a different, scheduler-push-facing concept) are encoded. So even applying a simple 'replace admin-masked bits' fix analogous to bug-1472's UPDATE fix would NOT fully replicate a DEAD transition's effects: the resource-value slot zeroing would still be silently lost on every other member, which matters materially at PROMOTION time (a newly-promoted primary would read stale, non-zeroed slots for a worker that died before promotion, potentially over-scheduling a dead host). A correct fix needs either (a) a schema change adding a wire-encodable resource-value-slots field so the DEAD payload can carry it, or (b) a different replication strategy for DEAD (e.g. re-deriving 'this worker's connection is gone locally' independently on each member rather than replicating a mutation), which is a design decision beyond a same-task patch.", + "fix": "NOT FIXED -- deliberately deferred, investigated not skipped (see bug-1472's own fix note for the full investigation). Needs its own design decision on how (or whether) to wire-encode the resource-value-slot zeroing, or an alternative non-replicated detection strategy, before a fix can be written safely.", + "tags": [ + "worker.c", + "worker_from_capnp", + "mark_worker_unavail", + "WAL_WORKER_CTRL", + "HOOK_DETAIL_WORKER_DEAD", + "capnp-schema-gap", + "resource-value-replication", + "standby-divergence", + "post-promotion-scheduling-risk", + "needs-owner", + "needs-design-decision", + "not-fixed", + "deferred" + ], + "related_bugs": [ + "bug-1472" + ], + "occurrences": 1, + "last_seen": "2026-07-25T20:15:00Z" + }, + { + "id": "bug-1475", + "timestamp": "2026-07-26T04:36:00Z", + "error_message": "Live config-upgrade-snapshot task 8.6 soak (`run-ha-soak.sh --config-upgrade`), run to verify bug-1472's fix, FAILED \u2014 but NOT at bug-1472's own assertions (materialized-state checkpoint byte-identity and active-configuration identity both PASSED cleanly across all 3 members, confirming bug-1472's fix holds). Failure occurred one step later, inside the Requirement 6.4 host-admin-status-rotation check's own `force_elect_node(\"ctrl-1\")` maneuver: after 3 rounds of overlapping chaotic kills + a live config upgrade all completed and the cluster reconverged 3-of-3, `force_elect_node` killed the spoiler (ctrl-3) and tried to submit a job batch on the surviving {ctrl-2 primary, ctrl-1 standby} pair \u2014 5 consecutive attempts all got 0/3 acked (\"waiting for write-quorum\"), and the round hard-failed: `[xce-test-ha] FAIL: force-elect ctrl-1: submit on {ctrl-2,ctrl-1} failed`. The Req 6.4 workerlist/admin-status assertion itself was never reached.", + "file": "src/controller/ha.c (ha_dispatch_repl_action_for_standby, ha_consume_append_entries, ha_evaluate_repl_action \u2014 HA_REPL_STREAM_WAL decision/dispatch loop); src/controller/config_repl.c (ha_stream_wal_to_standby catch-up streamer)", + "root_cause": "NOT data loss, NOT bug-1472 (worker/host admin-status replication is unaffected \u2014 unrelated subsystem, and its own checks passed). This is a standby catch-up LIVENESS stall: ctrl-1 (which had itself served as primary in term=2 before being killed in round 1 of the chaotic sequence, then restarted) got permanently stuck at applied_lsn=12 and never advanced again for the rest of the run. ctrl-1's own log (`ha_consume_append_entries`) shows a revealing pattern: a zero-entry PROBE AppendEntries at prevIdx=12,n=0 correctly succeeds with match=12 (confirming index 12 IS ctrl-1's true, correct log tip), but the very next entry-carrying AppendEntries from the SAME dispatch cycle uses prevIdx=13 (not 12) for n=1, which ctrl-1 correctly rejects with event=ae_reject_logmismatch (success=0 match=0) since it has no entry at index 13 yet to check a predecessor against. This 1-off prevIdx computation (using prevIdx=standby_applied_lsn+1 instead of prevIdx=standby_applied_lsn for the FIRST entry of a catch-up batch) means every retry of the SAME heartbeat-driven HA_REPL_STREAM_WAL decision (ha_evaluate_repl_action: standby_lsn(12) < committed_lsn(31) -> stream) recomputes and resends the IDENTICAL doomed range, forever, with zero backtrack/self-correction -- confirmed via ctrl-2's log: 'HA repl: standby[0] lsn=12 < committed_lsn=31 -> stream WAL batches' repeats verbatim every ~3s from 04:31:26 through 04:32:26 (60+ seconds, 20+ identical retries, zero progress) after ctrl-2 had already logged 'repl_broadcast: standby localhost:9991 evicted (pending_ack_count=17 > 16) -- triggering catch-up reconnect' and 'HA quorum lost: connected=0 required=1'. Once the soak's own force_elect_node killed the OTHER standby (ctrl-3) as its spoiler maneuver, ctrl-1 became the sole remaining standby, and its permanent inability to catch up meant the primary could never regain 2-of-3 write quorum again -- every subsequent job submission timed out (`commit: async continuation deadline expired`, `jobsubmit_commit_continuation: lsn=N commit failed/timed out (rc=-110)`), exactly matching the soak's observed '0/3 acked; waiting for write-quorum' x5 before hard-failing. Superficially similar symptom signature to already-FIXED bug-1028 (same 'ha_dispatch_repl_action_for_standby: standby[0] lsn=X < committed_lsn=Y -> stream WAL batches' log line looping indefinitely post-failover) but a DIFFERENT specific AE-reject reason -- bug-1028 was a logOk TERM-mapping mismatch (raw hdr.term vs persist_entry_term legacy mapping), fixed 2026-06-23; this is `ae_reject_logmismatch`, which reads like a prevLogIndex/off-by-one issue in how the catch-up streamer picks the FIRST entry's predecessor index, not a term problem -- so this is either a narrower unfixed variant of the same family, or a new regression in a related code path. NOT root-caused down to the exact defective line (this investigation was a live-soak VERIFICATION run for a DIFFERENT bug, not a fix task) -- flagging for whoever picks this up next to read ha_dispatch_repl_action_for_standby/ha_stream_wal_to_standby's prevIdx computation directly and compare against ctrl-1's actual persisted log at lsn=12/13 (waldump). Also notable: this specific soak step (force_elect_node's spoiler-kill-then-submit maneuver, previously only exercised by --sequential mode's post-loop 'leadership coverage' step against an otherwise-calm, fully-settled cluster) had never previously been run immediately after --config-upgrade's THREE rounds of aggressive overlapping chaotic kills plus a live config upgrade -- this specific combination/history may be what's needed to reproduce the stall, rather than it being config-upgrade-specific.", + "fix": "FIXED (2026-07-27/28, via a regression test + real-instrumentation debugging session that continued this entry's original live-soak finding). Landed as THREE coordinated changes, all required together -- the first two alone left the new regression test (test_bug1475_catchup_survives_midstream_checkpoint, test_config_repl.c) RED with applied_lsn stuck at 0 for the whole run:\n\n1. SEND side (config_repl.c, ha_stream_wal_to_standby): when scanning the WAL for entries to stream, a WAL_CHECKPOINT entry is still never built/sent as its own catchup entry, but the prev_log_index/prev_log_term tracker now advances across it like every other scanned entry, instead of freezing one entry behind. A standby that independently applied the same committed WAL_CONFIG_MARKER writes its OWN local checkpoint at the identical LSN (deterministic per-member trigger), so the checkpoint's LSN is a real, held position on both sides, not a gap to skip past chaining-wise.\n\n2. RECEIVE side (ha.c, ha_apply_term_entry_via_library): a JIT resync reads persist_durable_floor(p) and monotonically raises ha->raft.log_len to match before consuming each AppendEntries -- mirroring ha_sync_raft_log_from_recovery's boot-time computation, but applied live on the AE-receive path. This corrects for persist_write_checkpoint_marker (bug-1466, still not fixed at ITS root) advancing persist_t's current_lsn without going through raft_log_append, which otherwise leaves ha->raft.log_len stale and fails logOk's prev_log_index<=log_len bound forever.\n\n3. TERM-INDEX side (persist.c, record_entry_term/entry_term_locked) -- a THIRD, previously-undiscovered gap, found only via live fprintf instrumentation of the new regression test after fixes 1-2 above still left it RED. Fixes 1-2 both depend on persist_entry_term(checkpoint_lsn) reading back 'absent' (0) FOREVER on every member (the explicit, twice-documented design intent in both fixes' own comments and in persist_write_checkpoint_marker's comment). That invariant was actually FALSE: record_entry_term's geometric-growth realloc zero-filled any never-explicitly-recorded gap slot (like a checkpoint's own LSN, which is deliberately never passed to record_entry_term), and entry_term_locked's `lsn <= term_index_count` presence check then read that zero-fill back as WAL_LEGACY_TERM (1), not absent (0), the INSTANT any LATER lsn was recorded and grew term_index_count past the gap -- which happens immediately on a live leader (it appends real entries right after the checkpoint) but not yet on a receiver that has not itself appended past its own checkpoint. So the checkpoint LSN's 'term' was TIME-DEPENDENT: the sender (which streams AFTER its own later entries exist) computed prev_log_term=1 for the entry right after the checkpoint, while the receiver's honest, not-yet-grown term index still (correctly, per design) read 0 for the same LSN -- logOk's equality check spuriously mismatched (0 != 1) and rejected forever, exactly reproducing this entry's original ae_reject_logmismatch signature. Confirmed with real fprintf instrumentation at the streamer, the JIT resync, and raft.c's logOk check (added and removed during this investigation, not left in the tree): prev_log_term flipped from 0 to 1 in the SAME test run, immediately after the leader appended its 2nd post-checkpoint entry. Fixed with a TERM_INDEX_UNSET sentinel (UINT64_MAX) that record_entry_term's growth path now fills gap slots with (instead of 0), and entry_term_locked treats as absent -- so a never-explicitly-written slot reads absent PERMANENTLY, independent of later array growth. Confined to persist.c's two functions plus the term_index doc comment in persist.h; no other call site reads p->term_index[] directly.\n\nAlso completed as part of this fix: (a) audited whether WAL entry types OTHER than WAL_CHECKPOINT bypass record_entry_term the same way -- confirmed NO: WAL_CHECKPOINT (value 100) is the numeric max of wal_entry_type, persist_wal_async and persist_wal_write_replicated both explicitly reject type==WAL_CHECKPOINT up front (it cannot reach record_entry_term via either live path), and replay_wal's `hdr.type < WAL_CHECKPOINT` recording guard already covers every other currently-defined type (WAL_JOB_UPDATE=7, WAL_QUEUE_CTRL=20, WAL_WORKER_CTRL=30, WAL_NOOP=40, WAL_CONFIG_MARKER=50); (b) confirmed no interaction with the RequestVote/PreVote freshness gates (raft.c raft_log_ok, shared by raft_on_vote_request/raft_on_pre_vote_request): that gate also reads on_get_entry_term(s->log_len), which can legitimately land on a checkpoint LSN after the JIT resync in fix 2 -- the TERM_INDEX_UNSET fix makes that read deterministic (always absent) instead of time-dependent, which is a consistency IMPROVEMENT for vote freshness too, not a behavior change in the unsafe direction; full test_ha_prevote.c/test_ha_failover_prevote.c suites pass unchanged. Full regression suite: 1034 Tests, 0 Failures (was 1033/1034 with the new test failing before this fix). Files changed: src/controller/config_repl.c (REAL logic change, landed earlier in this same bug-1475 investigation: ha_stream_wal_to_standby's WAL_CHECKPOINT branch now falls through to the unconditional prev-tracker advance via `hdr.lsn > standby_lsn && hdr.type != WAL_CHECKPOINT`, instead of a hard `continue` that froze the tracker one entry behind -- plus cross-reference comments added in this later session); src/controller/ha.c (REAL logic change, also landed earlier in this investigation: a ~30-line JIT resync block added to the top of ha_apply_term_entry_via_library that raises ha->raft.log_len from persist_durable_floor(p) before consuming each AppendEntries -- plus cross-reference comments added in this later session); src/controller/persist.c (the TERM_INDEX_UNSET sentinel fix + doc comments, the new logic landed in THIS session); src/controller/testsuite/test_config_repl.c (test_bug1475_catchup_survives_midstream_checkpoint + updated doc comments describing all three fix parts). Correction (coordinator review 2026-07-27/28, Finding 1): an earlier version of this fix summary mischaracterized config_repl.c's and ha.c's changes as 'comment-only, no logic change' -- both carry genuine logic changes; they were simply written in an earlier part of this same bug-1475 investigation than the TERM_INDEX_UNSET sentinel (session 3 of the fix), not left unmodified by it.\n\nFinding 2 (coordinator review, 2026-07-27/28, CONFIRMED REAL and FIXED): after the TERM_INDEX_UNSET fix, the reviewer traced a plausible NEW instance of the identical permanent-wedge bug class, reachable via the SNAPSHOT-INSTALL catch-up path (HA_REPL_SEND_SNAPSHOT, dispatched whenever a standby has fallen too far behind for incremental WAL streaming -- a live, common path) instead of WAL-streaming catch-up. ha_snapshot_do_send (ha.c) computes checkpoint_term = persist_entry_term(leader, c->persist.checkpoint_lsn) and ships it on the wire; the receiver's ha_handle_snapshot_chunk calls persist_reconcile_checkpoint_term(p, checkpoint_lsn, checkpoint_term), which called record_entry_term(p, checkpoint_lsn, checkpoint_term) directly -- an EXPLICIT write, not a growth-fill, so it never got the TERM_INDEX_UNSET sentinel even when checkpoint_term==0. entry_term_locked's pre-existing legacy-0 rule then read that raw 0 back as WAL_LEGACY_TERM(1), forever -- permanently diverging from the leader's own permanently-absent(0) value for the SAME lsn whenever c->persist.checkpoint_lsn (== last_applied_lsn at general persist_checkpoint() time, NOT the WAL_CHECKPOINT entry's own physical lsn) happens to coincide with a checkpoint's own un-recorded lsn -- a real, reachable quiescent-state timing (the apply pump has walked, one lsn at a time including a no-op pass over checkpoint slots, all the way to exactly the checkpoint's own position before the next persist_checkpoint()+snapshot-send cycle runs, e.g. right after a config upgrade with no immediate follow-up activity) -- not the common case, but real and worth closing. Proven with a dedicated regression test (test_bug1475_finding2_snapshot_install_checkpoint_term_absent, test_config_repl.c) that drives the REAL ha_send_snapshot_to_standby -> ha_handle_snapshot_chunk -> persist_reconcile_checkpoint_term path (via the xce_snapshot_chunk_hook test seam) with a leader whose checkpoint_lsn is deliberately seeded (via last_applied_lsn) to coincide with its checkpoint's own physical lsn -- confirmed RED by temporarily reverting only persist_reconcile_checkpoint_term's fix (receiver read back term=1, not the expected absent=0) and GREEN after restoring it. Fixed with the SAME TERM_INDEX_UNSET sentinel: persist_reconcile_checkpoint_term now substitutes TERM_INDEX_UNSET for a wire checkpoint_term of exactly 0 before calling record_entry_term, instead of recording the raw 0 -- a wire value of 0 is unambiguous today (ha_snapshot_do_send is the ONLY sender and always computes it via persist_entry_term, whose public contract already returns a DIFFERENT value, WAL_LEGACY_TERM, for a present-but-legacy-0 entry -- so a public 0 can only mean genuinely, permanently absent). A genuinely non-zero checkpoint_term (any real historical term) is still recorded verbatim and still correctly overwrites a stale local term at that LSN, unchanged -- the pre-existing STALE-TERM reconcile test (test_ha_core_safety.c's test_3_1_snapshot_install_does_not_reconcile_boundary_term, CHECKPOINT_TERM=7, a synthetic non-zero value) is unaffected and still passes. Also fixed the stale 'zeroed-on-grow' comment at persist.c's truncate_term_index_locked (grown slots are TERM_INDEX_UNSET-filled since the original TERM_INDEX_UNSET fix, not zero-filled -- comment had not been updated). Re-verified RequestVote/PreVote freshness non-interference for this second fix too: persist_reconcile_checkpoint_term's caller (ha_handle_snapshot_chunk) never touches ha->raft.log_len or any vote-path state; the fix only changes what value entry_term_locked returns for a checkpoint-boundary LSN after a snapshot install (now consistently absent instead of inconsistently WAL_LEGACY_TERM), which is the SAME direction of improvement (deterministic absence vs time/path-dependent) the original TERM_INDEX_UNSET fix already established as safe for the freshness gate. Full suite: 1035 Tests, 0 Failures (was 1034 before this new test).", + "tags": [ + "ha.c", + "config_repl.c", + "ha_dispatch_repl_action_for_standby", + "ha_stream_wal_to_standby", + "ha_consume_append_entries", + "ae_reject_logmismatch", + "catch-up-stall", + "write-quorum-loss", + "standby-liveness", + "config-upgrade-snapshot-task-8.6", + "force_elect_node", + "not-bug-1472", + "bug-1472-fix-verified-good", + "same-family-as-bug-1028", + "needs-owner", + "root-caused-partially", + "not-fixed" + ], + "related_bugs": [ + "bug-1472", + "bug-1028" + ], + "occurrences": 1, + "last_seen": "2026-07-28T03:15:00Z" + }, + { + "id": "bug-1476", + "timestamp": "2026-07-26T04:44:00Z", + "error_message": "Live config-upgrade-snapshot task 8.9 re-verification soak (run-ha-soak.sh --config-upgrade-wal-trim, run to confirm bug-1474's repl.c fix), deploy /home/claude/tmp/xce-ha-waltrim3: ctrl-3 ABORTed via AddressSanitizer heap-use-after-free during the round's live config-upgrade submission, well before TARGET(ctrl-1) was ever restarted. The round's own bug-1474 WAL-trim assertions (floor segment preserved, below-floor segments trimmed, active segment present) all PASSED cleanly before this crash was noticed -- ctrl-3's death was only discovered because it, combined with a second unrelated issue (bug-1475), prevented the round's final '3-of-3 reconvergence' assertion from ever succeeding.", + "file": "src/controller/config_repl.c (load_primary_config ~line 656-666 free_ClusterConfig/free(myconf->cluster_config) call site; config_activate_committed ~line 360; handle_repl_config_push ~line 1280); lib/controller.capnp.c (decode_ClusterConfig_ptr ~line 17074 calloc site, decode_Resource_list ~line 4057 crash site)", + "root_cause": "AddressSanitizer heap-use-after-free, WRITE of size 8, in decode_Resource_list (lib/controller.capnp.c:4057), thread T5. Full ASAN trace (ctrl-3.stderr):\nWRITE by T5: decode_Resource_list <- decode_ClusterConfig <- decode_ClusterConfig_ptr <- load_cluster_config_from_file (config_repl.c:606) <- load_primary_config (config_repl.c:664) <- config_activate_committed (config_repl.c:360) <- handle_repl_config_push (config_repl.c:1280) <- myreq_handler (cluster.c:654) <- vtask_entry (cluster.c:786)\nFREED by T6: load_primary_config (config_repl.c:659, the 'free any prior decoded config before overwriting the pointer' step: free_ClusterConfig(myconf->cluster_config); free(myconf->cluster_config)) <- config_activate_committed (config_repl.c:360) <- handle_repl_config_push (config_repl.c:1280) <- myreq_handler <- vtask_entry\nALLOCATED by T5: calloc in decode_ClusterConfig_ptr (lib/controller.capnp.c:17074) <- load_cluster_config_from_file (config_repl.c:606) <- load_primary_config (config_repl.c:664) <- config_activate_committed <- handle_repl_config_push\nThis is a genuine, unguarded data race: load_primary_config's sequence is (a) if myconf->cluster_config is non-NULL, free it and NULL the pointer, then (b) decode a fresh ClusterConfig into myconf->cluster_config via load_cluster_config_from_file/decode_ClusterConfig_ptr. decode_ClusterConfig_ptr appears to publish the new struct pointer into *config (i.e. myconf->cluster_config) before it has finished populating nested fields (e.g. the Resource_list), since T5 is later observed still writing into the struct via decode_Resource_list AFTER T6 already saw a non-NULL myconf->cluster_config, freed it (T6's own step (a)), and proceeded to decode its own replacement. No mutex protects myconf->cluster_config across this read-free-decode sequence, so two overlapping invocations of handle_repl_config_push -> config_activate_committed -> load_primary_config for the same node race on the shared myconf->cluster_config pointer. This matches the class of concern already flagged (but not as a crash) in bug-976/bug-981 (handle_repl_config_push / config_activate_committed activation-ordering races) -- this is a NEW, more severe manifestation: an actual heap-corruption crash, not just an ordering/verification gap. Plausible trigger in this specific soak: heavy duplicate/late-joiner reconnect churn was independently observed on ctrl-2 (the primary) around the same window (bug-1474's WARN 'reusing guard-holding slot' fired twice, confirming the still-open, out-of-scope ha.c heartbeat-reconnect-sweep defect -- bug-1474's un-addressed 'fix angle (a)' -- was actively spawning duplicate/late-joiner standby connections during this round). If the primary sent the same config-push payload to ctrl-3 twice (once per duplicate connection), ctrl-3 could dispatch two concurrent handle_repl_config_push invocations on separate VTASK_LIVE_BLOCK workers, which is consistent with the observed T5/T6 race. This causal link (duplicate-connection churn -> duplicate config-push delivery -> concurrent load_primary_config race) is a plausible hypothesis from timeline correlation, not confirmed by direct evidence of two inbound config-push messages -- flagged as a hypothesis for whoever investigates, not a proven mechanism.", + "fix": "FIXED (2026-07-28). Two-part fix in load_primary_config() (config_repl.c), independently reviewed and verified via a real pthread-based deterministic race test: (1) added myconf_t.lock (mylock_t, mirrors replicator_t.lock's idiom in repl.c) serializing the entire free-decode-publish sequence against any concurrent load_primary_config() call for the same myconf_t -- initialized in main.c before first use, finalized in myconf_fini(), and in the t43_attach_myconf/t43_free_myconf test fixture. (2) load_cluster_config_from_file() now decodes into a LOCAL temporary (cluster_config_t *tmp), never directly into the shared myconf->cluster_config field -- decode_ClusterConfig_ptr (lib/controller.capnp.c) calloc's and publishes its pointer via the out-parameter BEFORE decode_ClusterConfig populates nested fields (e.g. resources), so decoding into a local closes the early-publish exposure at its source; myconf->cluster_config is only swapped in after the new config is fully decoded and validated, still under myconf->lock. Part (2) is independently necessary because several other readers (cluster.c, queue.c, sched.c, worker.c, apiserver.c, ctrlstatus.c, reply.c) read c->myconf->cluster_config without ever taking myconf->lock, so the field itself must never be observably half-decoded. New regression test test_bug1476_concurrent_load_primary_config_no_uaf_or_double_free (test_config_repl.c) drives two REAL concurrent pthreads through load_primary_config() for the same myconf_t via a deterministic test-only hook (load_primary_config_race_hook, XCE_TESTING-only, NULL/no-op in production) that parks the first thread exactly at the retire-old-config step the live crash raced -- pre-fix this reliably reproduces a genuine ASan-catchable double-free from the same missing-lock root cause; post-fix myconf->lock keeps the second thread blocked for the entire duration, verified both by a captured never-raced-through flag and by the final published config being fully valid, never torn. Full suite: 1035 -> 1036 tests, 0 failures.\n\n[UPDATE 2026-07-28, kiro-impl config-upgrade-snapshot resume] Prior fix landed with a comment block (config_repl.c:639-646) admitting it had been reverted to a RED-phase reproduction (no lock, decode-directly-into-shared-field) for test-construction purposes and NEVER restored to GREEN \u2014 the mylock_init/fini wiring (myconf.h, main.c, myconf.c) landed, but load_primary_config() itself still ran fully unguarded. Full suite run confirmed: test_bug1476_concurrent_load_primary_config_no_uaf_or_double_free FAILED (\"thread B... Expected 0 Was 1\"), i.e. B raced through while A was parked -- the lock was never acquired. Fixed for real: mylock_lock(&myconf->lock) now wraps the ENTIRE load_primary_config() body (acquired first, released at every return including success), and Cap-n-Proto decode now targets a local cluster_config_t *new_config first -- myconf->cluster_config is only freed+swapped after new_config is fully decoded and validated (reserved-name check, generation default), never decoded directly into the shared field. Verified 4/4 clean full-suite runs (1036 Tests 0 Failures 0 Ignored, ASAN_OPTIONS=detect_leaks=0) after the fix, including the specific race test. Committed as part of config-upgrade-snapshot task 8.6/8.9 unblock resume.", + "tags": [ + "config_repl.c", + "load_primary_config", + "config_activate_committed", + "handle_repl_config_push", + "heap-use-after-free", + "decode_Resource_list", + "controller.capnp.c", + "concurrency-race", + "config-upgrade-snapshot", + "task-8.9", + "live-soak-finding", + "bug-1474-verification-run", + "new-crash", + "needs-owner", + "possibly-related-to-bug-1474-fix-angle-a-duplicate-reconnects", + "fixed", + "mylock" + ], + "related_bugs": [ + "bug-976", + "bug-981", + "bug-1474", + "bug-1475" + ], + "occurrences": 2, + "last_seen": "2026-07-28T08:06:00Z" } -] \ No newline at end of file +] diff --git a/.wolf/cerebrum.md b/.wolf/cerebrum.md index 3557f48..cec8951 100644 --- a/.wolf/cerebrum.md +++ b/.wolf/cerebrum.md @@ -12,6 +12,8 @@ - Keep test logs for analysis, don't skip intermediate output even if a test hangs ## Key Learnings +- (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 `:::PASS` banner), while newer TDD suites (apply/HA-prevote family) add a custom `printf("%s: PASS\n", ...)` + banner — so grepping for `../controller/testsuite/.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. @@ -515,3 +517,50 @@ Fixed two `xce-audit-invariants controller` thread-role violations (`ha_demotion ## 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 ` 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 : -I --src-prefix 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 = ""; sha256 = ""; }` — 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. + +## 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. diff --git a/src/controller/config_repl.c b/src/controller/config_repl.c index ba6e8a9..ce011f4 100644 --- a/src/controller/config_repl.c +++ b/src/controller/config_repl.c @@ -611,10 +611,38 @@ static int load_cluster_config_from_file(char *file, cluster_config_t **config) /* ── load_primary_config ────────────────────────── */ +#ifdef XCE_TESTING +/* load_primary_config_race_hook: test-only synchronization seam (same + * pattern as push_thread_send_hook / xce_wal_stream_request_hook). When + * non-NULL, load_primary_config() calls this immediately after capturing the + * OLD myconf->cluster_config pointer this invocation is about to retire + * (free), right before it actually retires it -- letting a unit test + * deterministically force a SECOND, real, concurrent load_primary_config + * call through this exact window instead of depending on scheduler timing. + * NULL in production -- no behaviour change. See + * test_bug1476_concurrent_load_primary_config_no_uaf_or_double_free + * (test_config_repl.c) for the RED-phase reproduction this was built for: + * with the lock below removed, this hook deterministically reproduces the + * live ASan heap-use-after-free bug-1476 caught (two concurrent + * load_primary_config calls racing the free of the SAME myconf-> + * cluster_config pointer). With the lock, a second invocation can never + * even reach this hook while the first is parked here -- it blocks + * acquiring myconf->lock before getting this far, which is the + * serialization proof the fix requires. */ +void (*load_primary_config_race_hook)(void) = NULL; +#endif + int load_primary_config(cluster_t *c) { myconf_t *myconf = c->myconf; + /* bug-1476: serialize the whole free-decode-publish sequence against a + * concurrent load_primary_config() call for the SAME myconf_t. Held + * across every return path below, including the success path — a + * second caller must block here before it can even reach the race hook + * fired near the retire step, not merely around that step alone. */ + mylock_lock(&myconf->lock); + /* Build conf dir path: / */ char conf_dir[4096]; snprintf(conf_dir, sizeof(conf_dir), "%s/%s", @@ -627,14 +655,17 @@ int load_primary_config(cluster_t *c) INFO("No config at '%s/conf/current' (rc=%d) — " "will fetch from primary after election", conf_dir, rc); + mylock_unlock(&myconf->lock); return -ENOENT; } /* Heap-allocate conffile so it survives past this stack frame */ free(myconf->conffile); myconf->conffile = NULL; - if (asprintf(&myconf->conffile, "%s/conf/%s", conf_dir, fname) < 0) + if (asprintf(&myconf->conffile, "%s/conf/%s", conf_dir, fname) < 0) { + mylock_unlock(&myconf->lock); return -ENOMEM; + } /* Read raw bytes and check size */ uint8_t *bytes = NULL; @@ -644,6 +675,7 @@ int load_primary_config(cluster_t *c) ERROR("Primary config '%s' unreadable: %d", myconf->conffile, rc); free(myconf->conffile); myconf->conffile = NULL; + mylock_unlock(&myconf->lock); return rc; } @@ -653,53 +685,61 @@ int load_primary_config(cluster_t *c) memcpy(&tag, myconf->config_sha256, 8); myconf->config_version_tag = be64toh(tag); - /* Free any prior decoded config before overwriting the pointer */ - if (myconf->cluster_config) { - free_ClusterConfig(myconf->cluster_config); - free(myconf->cluster_config); - myconf->cluster_config = NULL; - } - - /* Decode Cap'n Proto directly into myconf->cluster_config */ - rc = load_cluster_config_from_file(myconf->conffile, - &myconf->cluster_config); + /* Decode Cap'n Proto into a LOCAL temporary first (bug-1476 fix part 2): + * decode_ClusterConfig_ptr calloc's and publishes its out-parameter + * pointer BEFORE decode_ClusterConfig populates the nested fields, so + * decoding directly into the shared myconf->cluster_config field would + * expose any other reader of that field to a live-but-incomplete + * object. Only a fully decoded and validated config is ever published + * below. */ + cluster_config_t *new_config = NULL; + rc = load_cluster_config_from_file(myconf->conffile, &new_config); if (rc != 0) { ERROR("Primary config '%s' failed Cap'n Proto decode: %d", myconf->conffile, rc); free(bytes); - free(myconf->conffile); - myconf->conffile = NULL; + mylock_unlock(&myconf->lock); return rc; } - if (myconf->cluster_config == NULL) { + if (new_config == NULL) { ERROR("Primary config '%s' decoded to NULL", myconf->conffile); free(bytes); - free(myconf->conffile); - myconf->conffile = NULL; + mylock_unlock(&myconf->lock); return -EINVAL; } free(bytes); - fill_default_cluster_config(myconf->cluster_config); + fill_default_cluster_config(new_config); /* Check for reserved lost_and_found name */ - if (config_contains_reserved_name(myconf->cluster_config)) { + if (config_contains_reserved_name(new_config)) { ERROR("Config '%s' contains reserved name 'lost_and_found'", myconf->conffile); - free_ClusterConfig(myconf->cluster_config); - free(myconf->cluster_config); - myconf->cluster_config = NULL; - free(myconf->conffile); - myconf->conffile = NULL; + free_ClusterConfig(new_config); + free(new_config); + mylock_unlock(&myconf->lock); return -EINVAL; } /* Default generation to 1 if not set */ - if (myconf->cluster_config->config_generation == 0) - myconf->cluster_config->config_generation = 1; - myconf->config_generation = myconf->cluster_config->config_generation; + if (new_config->config_generation == 0) + new_config->config_generation = 1; + myconf->config_generation = new_config->config_generation; + + /* Publish: retire the prior config only now that the new one is fully + * decoded and validated — myconf->cluster_config never points at a + * freed or partially-built object. */ + if (myconf->cluster_config) { +#ifdef XCE_TESTING + if (load_primary_config_race_hook) + load_primary_config_race_hook(); +#endif + free_ClusterConfig(myconf->cluster_config); + free(myconf->cluster_config); + } + myconf->cluster_config = new_config; /* Check staleness */ myconf->conf_stale = conf_check_stale(conf_dir, myconf->conf_stale_timeout); @@ -709,6 +749,7 @@ int load_primary_config(cluster_t *c) INFO("Primary config loaded: tag=%016" PRIx64 " gen=%" PRIu64, myconf->config_version_tag, myconf->config_generation); + mylock_unlock(&myconf->lock); return 0; } diff --git a/src/controller/config_repl.h b/src/controller/config_repl.h index f4f1609..398a319 100644 --- a/src/controller/config_repl.h +++ b/src/controller/config_repl.h @@ -453,6 +453,16 @@ extern int (*xce_probe_config_tag_hook)(const char *host, uint16_t port, */ int load_primary_config_for_test(cluster_t *c); +/* + * load_primary_config_race_hook: test-only synchronization seam. See the + * doc comment at its definition (config_repl.c) for the exact semantics — + * it lets a unit test deterministically force two concurrent + * load_primary_config() calls for the SAME myconf_t through the race window + * bug-1476's live ASan heap-use-after-free crossed. Set by test + * infrastructure (test_config_repl.c) before the relevant tests run. + */ +extern void (*load_primary_config_race_hook)(void); + /* * push_ctx_t: exposed under XCE_TESTING so unit tests can construct * synthetic push contexts to drive wait_all_with_timeout_for_test(). diff --git a/src/controller/main.c b/src/controller/main.c index a369257..bd891e3 100644 --- a/src/controller/main.c +++ b/src/controller/main.c @@ -400,6 +400,14 @@ int main(int argc, char* argv[]) myconf.xstrings_delta = arguments.xstrings_delta; memset(&myconf.peers_cfg, 0, sizeof(myconf.peers_cfg)); + /* bug-1476: myconf.lock serializes load_primary_config() against + * concurrent invocations for this myconf_t — must be initialized before + * cluster_run/myapp_run can ever call it. */ + if (mylock_init(&myconf.lock) != 0) { + ERROR("Failed to initialize myconf lock"); + return -1; + } + if (arguments.peers_str) { if (xce_parse_peers(arguments.peers_str, &myconf.peers_cfg) != 0) { fprintf(stderr, diff --git a/src/controller/myconf.c b/src/controller/myconf.c index 19079f8..0771335 100644 --- a/src/controller/myconf.c +++ b/src/controller/myconf.c @@ -163,4 +163,5 @@ void myconf_fini(myconf_t *myconf) free(myconf->cluster_config); myconf->cluster_config = NULL; } + mylock_fini(&myconf->lock); /* bug-1476 */ } diff --git a/src/controller/testsuite/test_config_repl.c b/src/controller/testsuite/test_config_repl.c index ecd044e..c32fc28 100644 --- a/src/controller/testsuite/test_config_repl.c +++ b/src/controller/testsuite/test_config_repl.c @@ -1047,6 +1047,9 @@ static myconf_t *t43_attach_myconf(cluster_t *c, const char *tmpdir) mc->name = strdup("ctrl-test"); mc->conf_keep_n = 2; mc->conffile = NULL; + /* bug-1476: the real load_primary_config()/config_activate_committed() + * paths this fixture drives now take mc->lock -- must be initialized. */ + TEST_ASSERT_EQUAL_INT(0, mylock_init(&mc->lock)); c->myconf = mc; return mc; } @@ -1062,6 +1065,7 @@ static void t43_free_myconf(cluster_t *c) free(mc->conffile); free(mc->workdir); free(mc->name); + mylock_fini(&mc->lock); free(mc); c->myconf = NULL; } @@ -6303,6 +6307,237 @@ static void test_catchup_inline_probe_blocks_RED(void) cc_run_offload_probe(0, "test_catchup_inline_probe_blocks_RED"); } +/* ── bug-1476: concurrent load_primary_config() race on myconf->cluster_config ── + * + * load_primary_config()'s free-decode-publish sequence for + * myconf->cluster_config was completely unguarded by any lock. A live + * 3-node ASan soak (config-upgrade-snapshot task 8.9 re-verification, + * ctrl-3) caught a genuine heap-use-after-free: two concurrent + * handle_repl_config_push deliveries for the SAME standby raced two + * concurrent load_primary_config() calls on separate VTASK_LIVE_BLOCK + * workers -- one call's decode had already published a non-NULL, + * still-populating cluster_config_t* into the shared myconf->cluster_config + * field (decode_ClusterConfig_ptr calloc's *and publishes* the pointer via + * its out-parameter BEFORE decode_ClusterConfig populates the nested fields + * such as resources -- confirmed by reading + * lib/controller.capnp.c:17067-17077) while a second, concurrent invocation + * observed that same non-NULL pointer, assumed (per the single free-then- + * decode sequence) it was a prior FULLY-decoded config, and freed it out + * from under the first call's still-in-flight decode. + * + * load_primary_config_race_hook (XCE_TESTING-only, config_repl.c) lets this + * test deterministically force two REAL pthreads through that exact race + * window instead of depending on scheduler timing: thread A captures the + * config pointer it is about to retire and parks at the hook; thread B (a + * full, real, concurrent load_primary_config() call for the SAME myconf_t) + * is released to run while A is parked. + * + * RED (pre-fix, no lock): B runs to completion unblocked -- freeing the + * SAME pointer A already captured and publishing its own config. Opening + * the gate lets A retire (free) that already-freed pointer a SECOND time: a + * genuine, deterministic, ASan-catchable double-free from the identical + * missing-lock root cause the live crash exposed. (The exact + * decode_Resource_list-mid-write UAF the live trace shows additionally + * requires pausing inside the generated capnp decoder, which the fix + * deliberately avoids touching -- see the fix's own comment in + * load_primary_config; this reproduction targets the same unguarded + * read-free-decode-publish sequence and is caught by ASan the same way.) + * + * GREEN (post-fix): myconf->lock serializes the two calls end-to-end -- B + * cannot even acquire the lock (let alone reach its own retire step or + * corrupt anything) until A's whole call, hook included, returns and + * releases it. No pointer is ever freed twice, and load_primary_config (fix + * part 2) no longer decodes directly into the shared field at all, closing + * the exposure for any OTHER reader that never takes the lock. */ + +typedef struct { + pthread_mutex_t mu; + pthread_cond_t release_cv; + pthread_cond_t entered_cv; + int gate_open; + _Atomic int entered; +} lpc_gate_t; + +static lpc_gate_t g_lpc_gate; +static _Atomic int g_lpc_hook_block_next; /* 1 = the NEXT hook call should park */ + +static void lpc_gate_init(lpc_gate_t *g) +{ + pthread_mutex_init(&g->mu, NULL); + pthread_cond_init(&g->release_cv, NULL); + pthread_cond_init(&g->entered_cv, NULL); + g->gate_open = 0; + atomic_store(&g->entered, 0); +} + +static void lpc_gate_destroy(lpc_gate_t *g) +{ + pthread_mutex_destroy(&g->mu); + pthread_cond_destroy(&g->release_cv); + pthread_cond_destroy(&g->entered_cv); +} + +static void lpc_gate_open(lpc_gate_t *g) +{ + pthread_mutex_lock(&g->mu); + g->gate_open = 1; + pthread_cond_broadcast(&g->release_cv); + pthread_mutex_unlock(&g->mu); +} + +static int lpc_gate_wait_entered(lpc_gate_t *g, int timeout_ms) +{ + struct timespec deadline; + clock_gettime(CLOCK_REALTIME, &deadline); + deadline.tv_sec += timeout_ms / 1000; + deadline.tv_nsec += (long)(timeout_ms % 1000) * 1000000L; + if (deadline.tv_nsec >= 1000000000L) { deadline.tv_sec++; deadline.tv_nsec -= 1000000000L; } + + pthread_mutex_lock(&g->mu); + int rc = 0; + while (atomic_load(&g->entered) < 1) { + rc = pthread_cond_timedwait(&g->entered_cv, &g->mu, &deadline); + if (rc != 0) break; + } + int entered = atomic_load(&g->entered); + pthread_mutex_unlock(&g->mu); + return entered >= 1 ? 0 : -1; +} + +/* Fires from INSIDE load_primary_config -- holding no lock of its own under + * the OLD (pre-fix) code, or holding myconf->lock under the fixed code. + * Only the FIRST call (armed via g_lpc_hook_block_next) actually parks; + * later calls pass straight through, so thread B's own call runs unimpeded + * when it reaches this same hook (pre-fix), or never even reaches it + * (post-fix -- it blocks acquiring myconf->lock before getting this far). */ +static void lpc_race_hook(void) +{ + if (!atomic_exchange(&g_lpc_hook_block_next, 0)) + return; + + pthread_mutex_lock(&g_lpc_gate.mu); + atomic_fetch_add(&g_lpc_gate.entered, 1); + pthread_cond_broadcast(&g_lpc_gate.entered_cv); + while (!g_lpc_gate.gate_open) + pthread_cond_wait(&g_lpc_gate.release_cv, &g_lpc_gate.mu); + pthread_mutex_unlock(&g_lpc_gate.mu); +} + +typedef struct { + cluster_t *c; + _Atomic int done; + _Atomic int rc; +} lpc_thread_arg_t; + +static void *lpc_call_load_primary_config(void *arg) +{ + lpc_thread_arg_t *a = (lpc_thread_arg_t *)arg; + int rc = load_primary_config(a->c); + atomic_store(&a->rc, rc); + atomic_store(&a->done, 1); + return NULL; +} + +static void test_bug1476_concurrent_load_primary_config_no_uaf_or_double_free(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_cr1476_race_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t c; + memset(&c, 0, sizeof(c)); + myconf_t *mc = t43_attach_myconf(&c, tmp); /* inits mc->lock */ + + /* conf_save_config only mkdir()s the LAST path component (workdir/name/ + * conf); unlike other t43_* fixtures this test does not call + * t42_make_cluster (which creates workdir/name via persist_init) since + * load_primary_config only touches c->myconf -- create workdir/name + * directly instead. */ + char ctrldir[300]; + snprintf(ctrldir, sizeof(ctrldir), "%s/ctrl-test", tmp); + TEST_ASSERT_EQUAL_INT(0, mkdir(ctrldir, 0700)); + + /* Establish an initial live config so BOTH racing calls exercise the + * "free the OLD config" retire step (not the first-ever-load path) -- + * matches the live crash, which happened on a standby reloading an + * ALREADY-activated config, not on first boot. */ + size_t blen0 = 0; + uint8_t *bytes0 = t43_make_config_blob(&blen0); + uint8_t sha0[32]; + SHA256(bytes0, blen0, sha0); + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, bytes0, blen0, sha0)); + free(bytes0); + TEST_ASSERT_EQUAL_INT(0, load_primary_config(&c)); + TEST_ASSERT_NOT_NULL(mc->cluster_config); + + /* Both racing calls independently resolve + decode the SAME conf/current + * file -- real production behaviour: two concurrent + * handle_repl_config_push deliveries for the same standby race over the + * SAME on-disk config, not different ones. */ + lpc_gate_init(&g_lpc_gate); + atomic_store(&g_lpc_hook_block_next, 1); + load_primary_config_race_hook = lpc_race_hook; + + lpc_thread_arg_t arg_a = { .c = &c, .done = 0, .rc = 0 }; + lpc_thread_arg_t arg_b = { .c = &c, .done = 0, .rc = 0 }; + + pthread_t thread_a; + TEST_ASSERT_EQUAL_INT(0, + pthread_create(&thread_a, NULL, lpc_call_load_primary_config, &arg_a)); + + TEST_ASSERT_EQUAL_INT_MESSAGE(0, lpc_gate_wait_entered(&g_lpc_gate, 2000), + "thread A must reach the race-hook checkpoint inside " + "load_primary_config before the test proceeds"); + + pthread_t thread_b; + TEST_ASSERT_EQUAL_INT(0, + pthread_create(&thread_b, NULL, lpc_call_load_primary_config, &arg_b)); + + /* Snapshot whether B completed WHILE A was still parked -- the + * serialization proof. Captured into a plain variable (not asserted + * yet) so a pre-fix failure here does not abort the test before it + * reaches the release below: pre-fix (RED) the crash triggered by that + * release is the primary, load-bearing signal; this flag is + * corroborating evidence, asserted only after both threads have safely + * joined post-release. */ + int b_raced_through_while_a_parked = cc_poll_flag(&arg_b.done, 300); + + lpc_gate_open(&g_lpc_gate); + + TEST_ASSERT_TRUE_MESSAGE(cc_poll_flag(&arg_a.done, 2000), + "thread A must complete once released"); + pthread_join(thread_a, NULL); + + TEST_ASSERT_TRUE_MESSAGE(cc_poll_flag(&arg_b.done, 2000), + "thread B must complete once unblocked by A's release"); + pthread_join(thread_b, NULL); + + load_primary_config_race_hook = NULL; + lpc_gate_destroy(&g_lpc_gate); + + TEST_ASSERT_EQUAL_INT(0, atomic_load(&arg_a.rc)); + TEST_ASSERT_EQUAL_INT(0, atomic_load(&arg_b.rc)); + + /* No corruption: the final published config is fully valid (whichever + * of the two calls happened to finish last -- both raced the SAME + * conf/current file, so either outcome is a legitimate, fully-decoded + * config, never a torn/freed one). */ + TEST_ASSERT_NOT_NULL(mc->cluster_config); + TEST_ASSERT_NOT_NULL(mc->cluster_config->clustername); + TEST_ASSERT_EQUAL_STRING("t43-upgrade", mc->cluster_config->clustername); + + /* GREEN: myconf->lock must have kept B blocked the whole time A was + * parked mid-call. RED (no lock) reliably fails this -- but reliably + * crashes (ASan double-free abort) even earlier, at lpc_gate_open() + * above, before ever reaching this line. */ + TEST_ASSERT_EQUAL_INT_MESSAGE(0, b_raced_through_while_a_parked, + "thread B's load_primary_config must stay blocked (serialized by " + "myconf->lock) while thread A is parked mid-call"); + + t43_free_myconf(&c); + printf("test_bug1476_concurrent_load_primary_config_no_uaf_or_double_free: PASS\n"); +} + #endif /* XCE_TESTING */ /* ── dispatch ───────────────────────────────────── */ @@ -6435,6 +6670,8 @@ void test_config_repl(void) * (2026-07-18): catch-up continuation slow-lane offload */ RUN_TEST(test_catchup_offload_probe_runs_promptly); RUN_TEST(test_catchup_inline_probe_blocks_RED); + /* bug-1476: concurrent load_primary_config() UAF/double-free race */ + RUN_TEST(test_bug1476_concurrent_load_primary_config_no_uaf_or_double_free); #endif /* Task A: raw capnp shims */ RUN_TEST(test_replconfig_req_ssl_encode); diff --git a/src/include/myconf.h b/src/include/myconf.h index 39e6987..3bc2e17 100644 --- a/src/include/myconf.h +++ b/src/include/myconf.h @@ -10,6 +10,7 @@ #include #include "clusterconfig.h" +#include "mylock.h" #include "xce/cluster.h" /* Forward declaration — avoids circular include with cluster_def.h */ @@ -50,6 +51,16 @@ typedef struct { uint32_t resmgr_inst_delta; /* --resmgr-inst-delta CLI value; 0 = use hardcoded default */ uint32_t xstrings_nhash; /* --xstrings-nhash CLI value; 0 = use hardcoded default */ uint32_t xstrings_delta; /* --xstrings-delta CLI value; 0 = use hardcoded default */ + /* bug-1476: serializes load_primary_config()'s free-decode-publish + * sequence against concurrent invocations for the SAME myconf_t (e.g. + * two racing handle_repl_config_push deliveries dispatched onto separate + * VTASK_LIVE_BLOCK workers for the same standby -- a live ASan + * heap-use-after-free was caught this way). Must be mylock_init()'d + * before first use (main.c) and mylock_fini()'d in myconf_fini(); test + * fixtures that drive the real load_primary_config()/ + * config_activate_committed() must init it too (see + * t43_attach_myconf/t43_free_myconf in test_config_repl.c). */ + mylock_t lock; } myconf_t; extern cluster_config_t* get_default_conf(void*);