myxce/.kiro/specs/raft-adapter-conformance/tasks.md

Implementation Plan

• 1. Foundation: extend the configuration-push reply schema with a term field

  • Add term @2: UInt64 to ReplConfigPushReply in src/schema/controller.capnp, alongside the existing accepted/versionTag fields
  • Rebuild the controller so the Cap'n Proto codegen custom_target regenerates controller.capnp.c/.h with the new field
  • Observable: repl_config_push_reply_t exposes a term member usable from config_repl.c and cluster.c; existing config-push tests still build and pass unmodified
  • Requirements: 7.4

• 2. Core: configuration-push reply integrity

• 2.1 (P) Send an explicit accept/reject reply carrying term on every configuration push outcome

  • In handle_repl_config_push, construct and send ReplConfigPushReply{accepted=0, term=current_term} on each of the four reject early-returns (malformed request, missing bytes/sha256, sha256 mismatch, conf_save_config failure) instead of returning without a reply
  • Populate term on the existing success reply path too
  • Preserve the existing -EAGAIN-from-config_activate_committed behavior unchanged: that path still sends accepted=1 (activation-deferred is not a rejection)
  • Add an XCE_TESTING-only send-hook (mirroring ha_wal_ack_send_hook) so a unit test can observe the constructed reply payload without a live endpoint
  • Observable: every code path through handle_repl_config_push now sends exactly one ReplConfigPushReply, with no path returning silently
  • Requirements: 2.2, 2.3, 2.5, 7.4
  • Boundary: config_repl.c reply construction

• 2.2 (P) Wire term-adoption into the primary-side configuration-push reply handler

  • In the existing ReplConfigPushReply branch of myreq_handler (cluster.c, the code that already checks cack->accepted and calls repl_on_config_ack or warns-and-ignores), add a term comparison: when cack->term exceeds the primary's current term, call raft_on_higher_term_observed — the same adoption entry point the heartbeat-reply and WAL-NACK vectors already use
  • Do not alter the existing accept/reject branching logic itself, only add the term check alongside it
  • Observable: a config-push reply carrying a higher term causes the primary to adopt that term and step down from leadership held at the lower term
  • Requirements: 7.4, 7.5
  • Boundary: cluster.c reply handling

• 2.3 Test configuration-push reply integrity end-to-end

  • Unit test: each of the four reject conditions in handle_repl_config_push produces accepted=0 with the current term, via the new send-hook
  • Unit test: the -EAGAIN activation-defer path still produces accepted=1
  • Unit test: an inbound config-push reply carrying a higher term triggers adoption via the cluster.c wiring from 2.2
  • Observable: new tests in test_config_repl.c fail against pre-fix code and pass against the code from 2.1/2.2
  • Requirements: 2.2, 2.3, 2.4, 2.5, 7.4, 7.5
  • Depends: 2.1, 2.2

• 3. Core: vote and pre-vote endpoint failure routing

• 3.1 (P) Split the shared vote-endpoint error handler and route post-connect failures to the production tally

  • Split vote_ep_err_handler into a real-vote handler (registered at its existing connection-setup call site) and a pre_vote_ep_err_handler (registered at its existing, separate PreVote connection-setup call site)
  • Extract the identical CTRL_PRIMARY-role cleanup branch (standby-endpoint deregistration, repl_remove_standby, quorum-transition logging) into one shared helper called by both, unchanged in behavior
  • Change each handler's election-phase branch to route a post-connect failure through the same production tally path already used for connect-time failures (vote_reply_dispatch for the real-vote handler, pre_vote_reply_dispatch for the pre-vote handler) instead of the legacy handle_request_vote_reply
  • Observable: a failure arriving after a vote or pre-vote endpoint has already connected now reaches the same tally counter a connect-time failure or a delivered denial would
  • Requirements: 4.2, 4.3, 4.4
  • Boundary: ha.c vote endpoint handlers

• 3.2 Test post-connect vote and pre-vote failure routing

  • Extend the existing test-construction helper (vote_ep_ctx_build_for_test) to synthesize a post-connect failure on a real-vote endpoint and assert it reaches vote_reply_dispatch's tally
  • Do the same for a PreVote endpoint against pre_vote_reply_dispatch
  • Confirm a round where every peer's outcome is now counted (including this failure mode) reaches a quorum decision without waiting for the full round timeout
  • Observable: new tests in test_ha_vote_send.c fail against pre-fix code (tally never updates) and pass against the code from 3.1
  • Requirements: 4.2, 4.3, 4.5
  • Depends: 3.1

• 4. Core: document the dormant AppendEntries sender and guard it against silent regression

• 4.1 Record the production-authoritative send mechanism and add a non-regression guard

  • Extend the on_send_append_entries doc-comment (ha.c, the ha_cb_send_append_entries_stub callback) to state plainly that this callback is reachable only via raft_send_append_entries, which has no production caller, and that repl_broadcast/ha_stream_wal_to_standby are the actual production AppendEntries-equivalent send/retry/backoff mechanism
  • Add an XCE_TESTING-only call counter to ha_cb_send_append_entries_stub
  • Observable: the counter exists and is queryable from a test; the doc-comment names both the dormant path and the live substitute explicitly
  • Requirements: 3.3

• 4.2 Test that the dormant sender stays uncalled in production

  • Drive the production heartbeat/replication path end-to-end in a test and assert the new call counter stays at zero
  • In the same test, confirm repl_broadcast/ha_stream_wal_to_standby did fire for the same scenario, proving the assertion isn't vacuous
  • Observable: the test fails if a future change accidentally gives raft_send_append_entries a production caller
  • Requirements: 3.3
  • Depends: 4.1
  • Boundary: test_ha_core_safety.c

• 5. Core: regression-lock tests for already-shipped reply and append behavior

• 5.1 (P) Test that a WAL catch-up rejection actually sends the reply on the wire

  • Add a test asserting ha_send_wal_ack_reply_ex(success=0) is invoked (via the existing ha_wal_ack_send_hook) when a WAL catch-up entry is rejected, closing the reply-construction-boundary coverage gap that existed alongside the already-shipped bug-1479 fix
  • Observable: new test in test_ha_core_safety.c fails if the reject branch is ever changed back to a silent drop
  • Requirements: 2.1
  • Boundary: test_ha_core_safety.c

• 5.2 (P) Test that one failing payload in a submitted batch propagates to the whole batch

  • Add a test asserting persist_submit_job_payloads_async resolves every payload in a batch with the first failure's outcome when one payload fails to persist, rather than completing a subset
  • Observable: new test in test_commit_barrier_reply.c fails if a future change lets a batch partially succeed
  • Requirements: 5.3
  • Boundary: test_commit_barrier_reply.c

• 6. Integration: extend the controller's durable conformance record with this feature's findings

  • Update src/controller/CLAUDE.md's "TLA+ Action-to-Code Mapping" table and "Protocol Verification Checklist" with corrected/added rows covering: AppendEntries reply integrity (WAL catch-up and configuration push), vote/PreVote failure routing across all failure timings, and the dormant-sender resolution from task 4
  • Append a one-line pointer to the existing known_limitations entry (index 30) in .kiro/specs/ha-term-safety/spec.json, referencing this spec's resolution of the dormant on_send_append_entries send-path drift
  • Observable: the CLAUDE.md table cites the current, post-fix code paths for every action this feature touched, with no reference to the pre-fix silent-drop behavior as if it were still current; a reader of ha-term-safety's known_limitations lands on this spec's resolution instead of re-discovering the drift
  • Requirements: 1.1, 1.2, 1.3, 1.4, 1.5, 1.6
  • Depends: 2.1, 2.2, 3.1, 4.1, 4.2

• 7. Validation: regression confirmation and live verification

• 7.1 (P) Confirm append and commit-source regression coverage is unaffected

  • Run test_commit_barrier_rollout.c, test_commit_barrier_reply.c (including the new test from 5.2), and test_term_safety_adapter.c and confirm all pass without modification to the already-conformant code they exercise
  • Observable: test run output confirms client-append-integrity and durable-majority-authoritative behavior are unchanged
  • Requirements: 5.1, 5.2, 5.4, 6.1, 6.2, 6.3, 6.4
  • Depends: 5.2
  • Boundary: test_commit_barrier_rollout.c, test_commit_barrier_reply.c, test_term_safety_adapter.c

• 7.2 (P) Confirm election, replication, and recovery regression coverage is unaffected

  • Run test_ha_core_safety.c (including the new tests from 5.1 and 4.2), test_ha_vote_send.c (including the new tests from 3.2), test_persist_recovery.c, and test_config_repl.c (including the new tests from 2.3; also confirms 2.4's existing timeout/unknown-outcome handling, unchanged) and confirm all pass
  • Run test_repl_catchup.c, test_repl_unit.c, test_repl_majority.c, and test_ha_repl_push.c and confirm all pass, closing the coverage gap for the Requirement 3 retry/backoff/catch-up-streaming-ceiling regression lock (ha_raft_on_repl_nack, repl_start_catchup, repl_broadcast)
  • Observable: test run output confirms the already-fixed retry/backoff, catch-up streaming ceiling, connect-time vote-failure routing, term-adoption vectors, restart/recovery reconcile, and the new dormant-sender guard all remain correct
  • Requirements: 2.4, 3.1, 3.2, 3.3, 3.4, 4.1, 4.4, 7.1, 7.2, 7.3, 7.5, 8.1, 8.2, 8.3, 8.4
  • Depends: 5.1, 2.3, 3.2, 4.2
  • Boundary: test_ha_core_safety.c, test_ha_vote_send.c, test_persist_recovery.c, test_config_repl.c, test_repl_catchup.c, test_repl_unit.c, test_repl_majority.c, test_ha_repl_push.c

• 7.3 Live soak: configuration-push rejection end-to-end

  • Run a rolling chaotic-failover soak where the test driver issues a configuration push whose payload deliberately does not match its advertised sha256, exercising the real handle_repl_config_push SHA-256-mismatch reject branch
  • Observable: the soak log shows the primary receiving and logging the explicit reject rather than a timeout
  • Requirements: 2.2, 2.3, 2.5, 7.4, 7.5
  • Depends: 2.1, 2.2

• * 7.4 (P) Live soak: vote/PreVote post-connect failure end-to-end (optional, defer if no fault-injection mechanism exists)

  • Attempt a soak round that severs an already-established outbound vote connection before a reply arrives (e.g. a firewall-rule toggle or proxy in the soak harness); if no such mechanism exists cheaply in xce-test-ha, defer this task — the unit tests from 3.2 already give deterministic coverage of the fix itself
  • Observable, if run: the soak log shows the affected round reaching a quorum decision within the fast-tally path rather than the full election timeout
  • Requirements: 4.2, 4.3, 4.5
  • Depends: 3.1
  • Boundary: xce-test-ha soak harness

Implementation Notes

• Task 1: adding a $C.mapname-tagged capnp field requires a companion edit to the hand-written C struct in src/include/typemap.h (the codegen writes into that struct by name; it does not synthesize it). ReplConfigPushReply.term required struct repl_config_push_reply_s.term in typemap.h — design.md's File Structure Plan didn't call this out, but it's the established, required pattern for every existing $C.mapname struct in that file, not scope creep. Any future task adding a capnp field should expect the same pairing.
• Authoritative test verification recipe (confirmed working): rm -rf /tmp/xce_* then run ASAN_OPTIONS=detect_leaks=0 src/xce.build/controller/controller_unit_test directly and read its own stdout N Tests M Failures line — meson's aggregate Ok/Fail can show a spurious TIMEOUT/inline FAIL under its own harness even when the direct binary run is clean (1053 Tests 0 Failures confirmed for task 1's baseline).
• Task 2.2 (bug-1482): myreq_handler's first unconditional step is xce_decode(pkt, &request), which itself unconditionally free_packet(pkt) + frees the io_packet_t struct before returning on every path — pkt ownership transfers into xce_decode, the caller must never touch/free it afterward. Any new _for_test wrapper around myreq_handler (or xce_decode callers generally) must document this, not the reverse ("caller owns/frees pkt") — that wrong assumption is what caused the original test-only wrapper's doc comment to mislead a reviewer into requesting a double-free.
• Task 2.2: driving a test through cluster.c's real myreq_handler (rather than testing extracted logic in isolation) requires adding cluster.c to controller_unit_test_src in meson.build plus link-satisfying fakes for symbols its OTHER unrelated switch-cases reference (task.c/opget.c/sched.c/apiserver.c/myconf.c) — this is pre-existing, established convention in this codebase (test_worker_join.c, test_commit_barrier_reply.c already do it for the same reason), not scope creep. Any new test driving myreq_handler end-to-end should expect this same footprint.
• New test-only wrappers exposing a static production dispatch function (the _for_test pattern, e.g. peer_ctrl_msg_handler_for_test, myreq_handler_for_test) MUST be wrapped in #ifdef XCE_TESTING/#endif — the source file compiles into the production binary too, and an unguarded wrapper ships as a new linked symbol in production with zero production purpose.
• Task 3.2 (bug-1483, not fixed): driving a PreVote round to RAFT_PRE_VOTE_LOST in a unit test (c==NULL) without first stubbing ha_prevote_rearm_liveness_hook falls through to the real ha_arm_heartbeat_timeout_vtask, which callocs into the static s_liveness_ctx and never frees it when c==NULL short-circuits the scheduling guard. test_ha_prevote.c's existing LOST-path tests already stub-and-reset this hook — any new PreVote-round test that reaches LOST must do the same.
• pre_vote_reply_dispatch has no XCE_TESTING mock-hook seam (unlike vote_reply_dispatch's xce_vote_reply_hook) — it calls raft_on_pre_vote_reply directly. Tests verifying PreVote routing must assert on raft_state_t's own tally fields (pre_vote_replies[], pre_votes_granted, pre_vote_in_progress) instead, per test_ha_prevote.c's established technique.
• Task 7.2 (bug-1484, residual gap, NOT fixed — outside this spec's task list): no test anywhere in testsuite/ drives the real repl_ack_decode_and_dispatch path and asserts raft_on_higher_term_observed fires for either the heartbeat-reply (ha.c ~6690-6711) or WAL-NACK (ha.c ~6805-6814) term-discovery vector bug-1421 introduced. test_intake_higher_term_heartbeat_adopts_term looks like it covers this but actually drives a different AE-request code path. Requirement 7.5's "already verified" premise currently rests on unverified ground for these two vectors specifically (the third, config-push, vector added by this feature IS tested — task 2.2). A future task should add a dedicated test mirroring test_config_push_reply_higher_term_adopts_and_steps_down's pattern for the other two.
• Task 7.3: the real elected-primary's own outbound config-push has no TOCTOU window to inject a sha256 mismatch without new production/relay code (primary hashes and pushes from the same in-memory buffer) — confirmed by reading config_commit_change/push_config_to_standbys. Live soaks of this specific reject path must act as a direct client/sender against a standby (using xce_replconfigpush_req_raw, the real production wire function) rather than corrupting a real primary's push; decode the ReplConfigPushReply envelope via xce_reply()'s Request{opcode:reply, body.replymsg} wrapper (not a bare RequestReply) to get real accepted/term content instead of only inferring correctness from the receiver's local log line.