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
  - 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
  - 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
  - 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_