myxce/.kiro/specs/ha-commit-barrier-rollout/tasks.md

Implementation Plan: ha-commit-barrier-rollout

TDD ordering per project steering (testing.md): every implementation sub-task is preceded by a testsuite sub-task that lands failing (RED) tests first. New tests live in testsuite/test_commit_barrier_rollout.c plus targeted updates to test_repl_majority.c — this shared-file reality, plus job.c being touched by three groups, is why no (P) markers are applied: the plan is sequential. Hard upstream prerequisite: ha-commit-engine must be implemented and green first — every task below consumes commit_submit(WAIT/RETAIN) from include/commit.h and never modifies commit.c internals. Verify exact line numbers with grep before editing (research.md §Verified Code Anchors has the 2026-06-11 verified table; files drift).

• 1. Write-path classification: table, dispatch, RETAIN routing

• 1.1 Write RED tests for classification totality and RETAIN registration

  • Create testsuite/test_commit_barrier_rollout.c (XCE_TESTING pattern, AAA structure) and register it in controller_unit_test_src (meson.build); declare wal_commit_mode_t + wal_classify() in src/include/persist.h so tests compile
  • Tests: every value of wal_entry_type (wal_types.h:44-58) maps to a mode — WAL_JOB_SUBMIT/WAL_QUEUE_CTRL → WAIT, WAL_WORKER_CTRL with detail JOIN/UPDATE → WAIT and detail DEAD → RETAIN, all WAL_JOB_* state types → RETAIN, WAL_CHECKPOINT → LOCAL; an unknown type triggers the XCE_TESTING fail-fast path; a fact-type append through persist_job_hook_fun registers the entry with the engine in RETAIN mode (observable via the engine's retained/registered accounting against a stub engine) and returns without blocking
  • Observable completion: build succeeds with the new header declarations; all new test cases FAIL (red) against the unmodified persist.c
  • Requirements: 2.1, 2.2, 3.1

• 1.2 Implement wal_classify and the hook-seam dispatch to green

  • Implement wal_classify(wal_type, detail) in persist.c per the design's verified classification table; consult it in persist_job_hook_fun / persist_queue_hook_fun / persist_worker_hook_fun (persist.c:1093/1172/1204 anchors): RETAIN types → persist_wal_async + commit_submit(ce, appended_lsn, COMMIT_MODE_RETAIN, 0) registration; WAIT types → WAL_JOB_SUBMIT keeps the existing sync path, queue/worker appends stay async (handler barrier owes completion, task 3); LOCAL → unchanged
  • Unclassified guard: assert under XCE_TESTING with the offending type logged; production ERROR + RETAIN fallback
  • Fact appends never propagate commit-progress errors to hook callers (local append rc only)
  • Observable completion: all task 1.1 tests PASS; grep shows no bare persist_wal_async call in the three hook functions that bypasses classification
  • Requirements: 2.1, 2.2, 3.1

• 2. Commit barrier helper and legacy adapter retirement

• 2.1 Write RED tests for persist_commit_barrier semantics

  • Declare persist_commit_barrier(persist_t *p, uint32_t timeout_ms) in src/include/persist.h
  • Tests (stub engine + replicator, n_peers=3 unless noted): returns 0 immediately when committed_lsn already covers current_lsn; returns -ETIMEDOUT when ACKs are withheld past the timeout and committed_lsn stays unmoved; returns 0 once a covering ACK advances the commit index; timeout_ms = 0 falls back to p->config.sync_timeout_ms; n_peers=1 returns 0 immediately (standalone preserved); -XCE_ERR_NOT_READY from the engine passes through unmodified
  • Observable completion: new cases FAIL (red) — the helper does not exist yet
  • Requirements: 1.1, 1.2, 1.3

• 2.2 Implement the barrier helper and retire the legacy walAck counting

  • Implement persist_commit_barrier in persist.c: snapshot current_lsn under p->lock (mirror persist.c:1019-1021), short-circuit when covered, else commit_submit(ce, lsn, COMMIT_MODE_WAIT, timeout); return engine code unmodified
  • repl.c repl_wait_majority_ack (repl.c:386-462 anchor): remove the legacy walAck counting branch (upstream marked it "retired by ha-commit-barrier-rollout") — the walAck path delegates to the engine unconditionally; the function's remaining body is the configAck track (hardened in task 5)
  • Observable completion: all task 2.1 tests PASS; grep shows no remaining acked_lsn > 0 counting and no n_active == 0 → success short-circuit reachable from any walAck call path
  • Requirements: 1.1, 1.2, 1.3

• 3. Reply-ordering restructure of request-shaped handlers

• 3.1 Write RED integration tests for commit-first-then-reply per opcode

  • Tests drive the four handlers directly (vtask-context test harness, stub standby registry): for each of do_jobcontrol_op (not-started-job branch, producing WAL_JOB_UPDATE/WAL_JOB_FAIL), do_queuecontrol_op, do_hostcontrol_op, handle_worker_join — with ACKs withheld the requester reply carries an error (-ETIMEDOUT) and no success reply is ever observable while the operation's entries exceed committed_lsn; with ACKs supplied the success reply is observable only after committed_lsn covers the entries; local validation failures still reply immediately with their existing error codes
  • Observable completion: new cases FAIL (red) against the current reply-before-commit handlers (anchors: job.c:932, queue.c:130, worker.c:700, join.c:107-109 — re-verify with grep)
  • Requirements: 1.1, 1.2, 1.3, 1.4

• 3.2 Restructure the four handlers to green (incl. bug-447 fix: queue/host control now actually persist+replicate)

  • Apply the uniform pattern: rc = <ctrl fn>; if (rc == 0) rc = persist_commit_barrier(&c->persist, 0); reply.status = rc; xce_reply(...) — in do_jobcontrol_op (job.c:921-934), do_queuecontrol_op (queue.c:119-132), do_hostcontrol_op (worker.c:689-702); in handle_worker_join (join.c:33-119) insert the barrier between the AFTER_EVENT hook and the success reply, keep error replies immediate, keep on_worker_join after the reply
  • Barrier runs after entity locks are released in every handler (the ctrl fns unlock before returning)
  • Observable completion: all task 3.1 tests PASS; handle_jobsubmit untouched (reference implementation)
  • Requirements: 1.1, 1.2, 1.3, 1.4

• 3.3 Pin the receipt-ACK verdicts with tests

  • Tests: handle_jobdispatch (job.c:459-491) replies immediately AND dispatch processing appends no WAL entries (classification guard stays silent through a dispatch cycle; notify_dispatch outcomes unaffected); do_set_preason/do_clr_preason and the do_update_op rusage path remain receipt-style replies whose subsequent fact appends are RETAIN-registered
  • Document the verdict in the test comments with the umbrella Reply-Ordering Contract reference
  • Observable completion: verdict tests PASS without modifying those handlers; any future WAL append from dispatch processing trips the classification guard in CI
  • Requirements: 2.3

• 4. Fact admission during quorum loss and retention proof

• 4.1 Write RED tests for the dispatch-gate fact allowlist

  • Tests against ha_dispatch_gate (ha.c:1527 anchor) with CTRL_PRIMARY and write quorum absent: OPCode_notify, OPCode_update, OPCode_setpreason, OPCode_clrpreason return 0; OPCode_jobsub, OPCode_jobcontrol, OPCode_queuecontrol, OPCode_hostcontrol, OPCode_jobdispatch, OPCode_join still return -XCE_ERR_NOT_READY; with quorum present everything returns 0 as today
  • Observable completion: the four fact-opcode cases FAIL (red) against the current allowlist
  • Requirements: 3.2

• 4.2 Implement the allowlist to green

  • Add the four fact-bearing opcodes to the CTRL_PRIMARY no-write-quorum switch (ha.c:1537-1556 anchor); CTRL_PRIMARY_COMMITTING branch untouched (deleted wholesale by ha-single-leader-role)
  • Observable completion: all task 4.1 tests PASS; existing gate tests for request opcodes unchanged
  • Requirements: 3.2

• 4.3 Write the fact-retention-across-quorum-loss integration test (bug-396 end-to-end evidence complete)

  • Scenario (stub registry, n_peers=3): disconnect standbys (quorum lost) → drive worker-fact paths (job start/finish via do_notify-level calls) → appends succeed with no error, entries pending (lsn > committed_lsn), nothing dropped; reconnect + supply covering ACKs → committed_lsn advances over the retained suffix with no re-delivery of the facts; assert no timeout was ever applied to the fact entries
  • Observable completion: test PASSES against tasks 1.2 + 4.2 output (if it fails, fix the RETAIN/gate path, not the test); retained-depth observability counter visible in test logs
  • Requirements: 3.1, 3.3, 3.4

• 5. Configuration change commit barrier

• 5.1 Write RED tests for configAck arithmetic and the upgrade failure path

  • Unit tests (repl.c configAck branch via repl_wait_majority_ack): a standby's acked_config_lsn from a previous tag does not count toward the current push; n_peers=3 with zero covering acks → -ETIMEDOUT (the n_active == 0 → success short-circuit dies on this branch); self + one covering standby (majority of n_peers=3) → 0; n_peers=1 → immediate 0; update test_repl_majority.c configAck cases to the per-push arithmetic
  • Integration tests (config_repl.c): push_config_to_standbys returns -ETIMEDOUT with acks withheld and 0 with majority acks, push count via out-param; on failure the upgradeConfig requester receives an error reply (send_upgrade_error shape), committed_config_tag is unmoved, and restart_requested stays clear; on success the existing reply + restart behavior is unchanged
  • Observable completion: new cases FAIL (red) against the current logged-only result (config_repl.c:916-929) and unconditional success ACK (config_repl.c:1062-1073)
  • Requirements: 4.1, 4.2, 4.3, 4.4

• 5.2 Implement the config barrier to green (+ remediation r1: wired production config-ACK ingestion, kept barrier off iomgr thread)

  • repl.c: configAck branch takes the in-flight push tag (signature change in src/include/repl.h), counts acked_config_lsn >= push_tag, majority 1 + count > n_peers / 2 over configured members, n_peers <= 1 immediate success
  • config_repl.c: push_config_to_standbys gains int *acked_out and returns the barrier result, passing new_tag to the wait; do_upgrade_config_with_bytes (config_repl.c:1056-1094 anchor) on non-zero return sends send_upgrade_error, skips the success ACK and skips the restart request; committed_config_tag advance stays success-branch-only
  • Observable completion: all task 5.1 tests PASS; a failed upgrade leaves the admin tool with an explicit error and the primary running on the old committed tag
  • Requirements: 4.1, 4.2, 4.3, 4.4

• 6. Idempotency and ordering guards for job transitions

• 6.1 Write RED tests for mark_job_* guards

  • Tests: duplicate FINISH on a DONE job → state unchanged, INFO-level no-op, no AFTER_EVENT hook fired (no WAL entry appended); START on a DONE/EXIT job → rejected with WARN, terminal state preserved; UPDATE attempting to clear terminal bits → rejected; double application of the same transition yields state identical to single application (replay equivalence); fail-after-finish keeps the existing PERR-merge behavior (job.c:702-716 anchor); guard rejections return a code all existing callers treat as success-no-op
  • Observable completion: new cases FAIL (red) against the unguarded mark_job_start/finish/fail/update (job.c:583/638/696/726 anchors)
  • Requirements: 5.1, 5.2, 5.3

• 6.2 Implement the guards to green

  • Add transition guards at the top of each mark_job_* (under the caller-held job WRITE lock, before BEFORE_EVENT fires): duplicate → log + return no-op; terminal → non-terminal → WARN + reject preserving state; forward transitions unchanged
  • Audit all callers (on_job_*, job_ctrl, update_job_rusages) for the new return semantics
  • Observable completion: all task 6.1 tests PASS; existing job lifecycle tests green
  • Requirements: 5.1, 5.2, 5.3

• 7. Exhaustive reply-site sweep and final validation

• 7.1 Run the exhaustive xce_reply audit over all control paths

  • Grep ALL xce_reply call sites under src/controller/ (excluding testsuite/) — research.md lists the design-time population (job.c, queue.c, worker.c, join.c, opupd.c, cluster.c, config_repl.c, ha.c, opget.c, reply.c, task.c); re-run the grep fresh since lines drift
  • Classify every site into exactly one category: post-commit confirmation (restructured in task 3 / job submit / config success), receipt-only ACK for fact-shaped delivery (verdicts from task 3.3), or read-only/replication/error reply (no state-changing entries produced)
  • Fix any site that confirms a request-shaped state change before commit (a missed handler silently violates umbrella 2.1); commit the audit table (site → category → evidence) in the task completion notes
  • Observable completion: zero unclassified or violating reply sites in the recorded audit; any fixes covered by tests added to test_commit_barrier_rollout.c
  • Requirements: 2.4, 1.1, 1.4

• 7.2 Full regression and barrier-completeness validation

  • Build and run the complete controller testsuite with ASAN (/xce-build controller); fix regressions from the reply restructure, gate allowlist, configAck hardening, or guards
  • End-to-end ordering assertion: across all restructured opcodes in one run, no success reply is observable while the operation's entries exceed committed_lsn (integration harness from 3.1 driven as a suite); fact retention test (4.3) and config barrier tests (5.1) green in the same run
  • Confirm downstream readiness: classification guard active, sweep audit recorded — the hard precondition for ha-single-leader-role ("barrier before state removal") is satisfied
  • Observable completion: full testsuite green under ASAN; audit + ordering evidence recorded
  • Requirements: 1.1, 1.2, 1.3, 1.4, 2.1, 2.2, 2.3, 2.4, 3.1, 3.2, 3.3, 3.4, 4.1, 4.2, 4.3, 4.4, 5.1, 5.2, 5.3

Implementation Notes

• (1.1) commit_submit(COMMIT_MODE_RETAIN) is registration-free (commit.c:233-239, reviewer-confirmed): commit state derives purely from registry watermarks at ACK time, so RETAIN routing has NO per-entry engine observable. Task 1.2's RETAIN registration call is therefore contract-documentation, not bookkeeping — the observable contract is: non-blocking return, local-append-rc-only propagation, pending via lsn > committed_lsn, ACK-driven advance. Test 8's behavioral pins execute only at GREEN (Unity longjmp at the wal_classify carrier assert).
• (1.1) The unknown-type fail-fast test forks and asserts WTERMSIG==SIGABRT — task 1.2 must implement the XCE_TESTING guard as assert/abort (or the test needs a matching tweak).
• (1.1) wal_entry_type has 13 values (wal_types.h:44-58), no WAL_NOOP yet (lands in ha-single-leader-role, which must add a classification row + sweep entry — the totality sweep will catch it).
• (1.1) Suite baseline is ZERO failures since ha-commit-engine 6.1. detect_leaks=0 remains the harness norm (6 inventoried pre-existing leak groups).
• (1.2) CENSUS METHOD: only ~9 of 48 groups emit Unity summary blocks; the rest report per-test PASS lines only — count distinct Unity results (~465) and grep FAIL lines, never sum summary blocks. commit_submit(RETAIN) early-returns BEFORE taking ce->lock (commit.c:233-239) — registration is lock-free, safe with entity write locks held. wal_hook_append_classified is the single classified-append seam; appended_lsn read = current_lsn - 1 under p->lock (benign race, commented).
• (5.2) REJECTED then remediated (r1) — two CRITICAL findings the first pass missed, both fixed: (F1) the config-ACK signal was never produced in production (repl_on_config_ack had zero callers; the standby's replconfigpushreplymsg reply was dropped on the primary) so the new authoritative barrier would have BRICKED every multi-member config upgrade — this was the unlanded half of ha-config-bootstrap task 8.1. Fixed by a replconfigpushreplymsg decode branch in repl_ack_decode_and_dispatch (ha.c) → repl_on_config_ack on the repl_add_standby-registered promoted-vote ep (production-normal path), + a defensive cluster.c OPCode_reply ingestion for inbound standbys (no-ops against unregistered slots, forward-looking). Schema unchanged (ReplConfigPushReply already carried versionTag). (F2) the 500ms barrier wait reached the iomgr epoll thread via 5 ha.c push sites. Fixed by exporting push_config_transport_only (non-blocking fan-out) for all best-effort ha.c/join.c streaming sites; push_config_to_standbys (transport+barrier) now has exactly ONE caller — do_upgrade_config_with_bytes in vtask context. LESSON: the design's "handlers run on vtask threads" claim was only true for cluster.c-dispatched opcodes, not the ha.c peer-channel push sites the design never analyzed (ha.c was outside its modified-files list). Wire-path test drives real capnp ACK bytes through the production decode branch.
• (3.3) bug-451 DISCOVERED (reviewer-verified byte-precise, NOT fixed — boundary): update_job_rusages fires HOOK_DETAIL_JOB_RUSAGE_UPDATE with (cluster_t*, xce_job_rusages_t*) (job.c:1014-1015) but persist_job_hook_fun's default arm (persist.c:1355-1359) reads vararg 2 as job_t* and dereferences j->job.state at offset 56 — one past the end of the exactly-56-byte xce_job_rusages_t → 4-byte OOB read + garbage state field in every WAL_JOB_RUSAGE payload (jobid lines up by accident). Production fix candidate for task 7.1's sweep or a follow-up: persist arm needs a RUSAGE-specific va_list signature. Census now 488.
• (3.2) bug-447 FIXED via rc normalization inside the three ctrl fns (real errors/misses preserved; reviewer-verified per-fn). Semantics change shipped intentionally: idempotent no-op control actions now return 0 (was leaked 1); queue/host mutations now append WAL + replicate. Join barrier failure → error reply + on_worker_join SKIPPED (coherent reading of design; worker retries via idempotent rejoin; documented at join.c:108-113). PRE-EXISTING ref leak: job_ctrl takes reference(j) (job.c:317) but normal exit never dereferences (bug-448) — out of scope, future fix.
• (3.1) bug-447 CONFIRMED (reviewer-verified to librtti disassembly): key_find returns 1 on found (unnormalized from xkvmgr_find/kvtab_find); queue_ctrl (queue.c:61, gate :106) and worker_ctrl (worker.c:644, gate :678) leak rc=1 → AFTER_EVENT never fires on the found path → queue/host control silently unpersisted+unreplicated in production today; job_ctrl appends via mark_job_* hooks but still replies status 1. Fix point = 3.2's handler rc normalization (key_find is in binary librtti, out of repo). RED tests pin it.
• (3.1) Test link gained job.c/queue.c/join.c (test-only meson change) + 8 inert link stubs in test_commit_barrier_reply.c (none on tested paths — reviewer-traced). Reply capture = zeroed stack endpoint ep->outq inspection (test_config_repl.c:2210 precedent; endpoint_update_epoll no-ops when iomgr NULL). RED-window orphaned fsync threads self-resolve at GREEN (reviewer finding 5).
• (2.2) Legacy walAck counting RETIRED: walAck track = entry_lsn==0 → 0 (LSN-0 interim) else unconditional commit_submit (ce==NULL → -EINVAL, fail-loud). configAck track byte-equivalent (task 5's surface intact, incl. its n_active==0 shortcut and acked_config_lsn>0 counting — both die at task 5). persist_commit_barrier: ce==NULL → 0 (vacuous, documented). test_repl_unit's two lsn=0 walAck callers are now vacuous for quorum counting (INFO for task 5/7.2). CENSUS = 472 (465 at 1.2 + 7 from 2.1).
• (2.1) BARRIER WATERMARK RESOLVED (reviewer-judged): design's literal "Snapshot lsn = p->current_lsn" is loose phrasing contradicted by its own anchors — the coherent contract is watermark = current_lsn - 1 (last appended; persist.h:71 invariant committed_lsn <= current_lsn - 1 makes the literal reading unsatisfiable). Tests pin current_lsn - 1 with off-by-one killing power in BOTH directions; task 2.2 must implement that. Resolved contract recorded in persist.h doc comment.