test(raft-apply-decoupling): command failure path leaves no trace under repetition (task 3.3)

Add 4 tests to test_commit_barrier_reply.c (25->29), no production
changes: explicit-FAILED via truncation (distinct from timeout),
repeated failure loops (5 cycles) for both timeout and truncation,
and an array_size>1 mixed-outcome test proving a committed sibling's
apply hand-off isn't blocked by failing siblings while producing
zero phantom jobs for the failures and exactly one batch-level
failure reply.

Closes out task-group 3 (command apply-after-commit ordering).

.kiro/specs/raft-apply-decoupling/tasks.md

@@ -68,7 +68,7 @@
   - _Requirements: 3.4, 7.1, 7.2, 7.3_
   - _Depends: 2.2_
 
-- [ ] 3. Command apply-after-commit ordering
+- [x] 3. Command apply-after-commit ordering
 - [x] 3.1 (P) Stop constructing the in-memory job object before the commit barrier
   - Change job submission to carry forward only the data needed to build the WAL entry payload, without inserting anything into the in-memory job table until the (still-synchronous, at this point) commit call returns success
   - Remove the now-unreachable code path that previously ran the durability step from inside the notification hook for job submission
@@ -84,7 +84,7 @@
   - _Requirements: 1.1, 1.4, 2.1, 2.3_
   - _Depends: 1.1, 1.2, 1.3, 1.4_
 
-- [ ] 3.3 Unit test the command failure path leaves no observable trace
+- [x] 3.3 Unit test the command failure path leaves no observable trace
   - Force repeated commit failures and timeouts for job submission and confirm no job object is ever created
   - Observable: the test suite demonstrates zero phantom job objects across repeated forced commit failures
   - _Requirements: 2.1, 2.3_
@@ -193,6 +193,7 @@
 - **1.2**: implemented `commit_resolve()` + wired it into all 6 commit.c-internal paths that can advance `committed_lsn`/`writable_lsn` (the COMMITTED/majority-ack outcome). Chose `VTASK_LIVE_BLOCK` for dispatch (never admission-gated, unlike `VTASK_REQ_BLOCK` which can fast-reject — unacceptable for "exactly-once, never unresolved"). **Reviewer caught a real task-graph gap, not a defect in 1.2 itself**: converting `COMMIT_MODE_WAIT` away from a blocking wait also removes the *only* mechanism that ever detected a commit timing out (the caller's own `pthread_cond_timedwait` deadline) — nothing yet arms an equivalent timeout for async-registered continuations, and nothing yet wires demotion/log-truncation detection to call `commit_resolve(FAILED, ...)`. Without this, a pending async command during quorum loss would hang forever instead of failing — worse than today's behavior. Split out as new tasks **1.3** (timeout arming) and **1.4** (demotion/truncation wiring), inserted before the renumbered **1.5** (was 1.3, concurrency test — now exercises the real detection paths from 1.3/1.4 instead of calling `commit_resolve()` directly). Task 3.2 now also depends on 1.3/1.4, since its own "failure/timeout outcome replies" observable is meaningless without them.
 - **1.3**: added a dedicated per-commit-engine sweep thread (not the shared `delaymgr` — would violate boundary + delaymgr's 200ms poll granularity vs. `DEFAULT_SYNC_TIMEOUT_MS`=500ms would add timing jitter). Race rule: majority-satisfied always wins over an already-expired deadline when both are true under the same lock hold. **Round 1 REJECTED**: `commit_sweep_once()` computed the next-deadline snapshot under `ce->lock`, unlocked to dispatch, then slept on the *stale* snapshot — a `commit_submit_async()` registration/broadcast landing during that unlocked dispatch window was lost (no thread parked on the condvar to receive it), so the sweep thread could sleep on an absent/late deadline indefinitely (worst case: unbounded delay in a low-traffic quorum-loss scenario, violating the "identical failure timing" no-regression bar). **Fixed** by holding `ce->lock` continuously across scan→wait decisions (releasing only for the unlocked dispatch step) and always re-scanning under the lock immediately before ever calling `cond_wait`/`cond_timedwait` — the standard recheck-then-wait idiom. Round 2 reviewer independently reproduced the round-1 bug shape to confirm the fix actually closes it, not just narrows it. **Precedent for 1.4**: any future code that needs to interrupt the sweep thread's wait (e.g., a demotion/truncation handler resolving a pending LSN out-of-band) must go through the same lock+broadcast discipline `commit_submit_async()` uses, or it risks the identical lost-wakeup class of bug.
 - **1.4**: added `commit_resolve_pending_above(ce, lsn_floor, rc)` (snapshot-then-`commit_resolve()`-per-entry, no parallel bookkeeping). Truncation: a single new `on_truncate`/`truncate_cb_ctx` callback slot on `persist_t` (same inversion-of-control shape as the existing `on_durable_advance` hook) fires from `persist_truncate_wal_above` after releasing `p->lock`, only when a truncation actually removed entries — this ONE wrapper covers all three real truncation call sites (`raft_cb_become_leader`'s post-election truncate, the library's `on_truncate_from` conflict-reconcile, `ha_demotion_convergence_fn`'s post-demotion truncate), verified by the reviewer reading all three. Demotion: `raft_cb_become_follower` calls `commit_resolve_pending_above(&c->commit, 0, -XCE_ERR_STANDBY)` (lsn_floor=0 → fails everything — a demoted node can never authoritatively confirm anything pending; verified against the CP posture). **Known residual, non-blocking**: a fourth internal truncation path (`persist.c` inside `persist_wal_write_replicated`'s tip-conflict-repair branch, STANDBY replicated-apply only) calls `truncate_wal_above_locked()` directly, bypassing the new wrapper — safe today only because demotion resolves ALL pending continuations before role flips to STANDBY (so nothing can still be registered by the time that path runs), but this is an *implicit* ordering invariant, not an asserted one. Flag for whoever next touches the STANDBY replicated-apply path: add a defensive comment/assertion there, or route it through the wrapper too, before relying on it further. Also: bug-1407 was believed closed at this point (70/70 across 3 runs) — **see 1.5's note below, this was a false reading, not a real fix**.
+- **3.3**: pure test-only close-out of task-group 3 — 4 new tests added to `test_commit_barrier_reply.c` (25→29), approved first round, zero production changes. Closes two gaps 3.2 flagged: (1) an explicit-FAILED test via `persist_truncate_wal_above`/`commit_on_truncate`/`commit_resolve_pending_above(-ENOENT)`, distinct from the existing timeout (`-ETIMEDOUT`) path; (2) the `array_size > 1` mixed-outcome gap — one element ACKed to real majority while two time out, proving the committed sibling's `persist_register_pending_apply` hand-off isn't blocked by failing siblings, zero phantom jobs for the failures, and exactly one batch-level failure reply (via `jobsubmit_reply_failure_once`'s ref-counted first-failure-wins gate). Also added two repeated-failure loop tests (5 independent cycles each, fresh jobid/LSN per iteration) proving the no-phantom-job guarantee isn't a one-shot fluke. **Confirmed correct, non-obvious harness detail**: this file's fixture wires job-submit continuations through a standalone `commit_engine_t g_hce`, NOT `cluster_t`'s embedded `c->commit` field that demotion (`raft_cb_become_follower`) resolves against — so truncation, not demotion, is the only deterministic way to force an explicit FAILED (non-timeout) outcome in this specific test file. Demotion-forces-FAILED already has its own dedicated coverage in `test_demotion_convergence.c` (tasks 1.4/1.5) via a different fixture — not a gap, a file-boundary distinction. **Non-blocking observation carried forward, not fixed here** (pre-existing since 3.2, correctly out of scope for a test-only task): the single batch-level failure reply carries no per-element granularity — a client can't tell "1 of 3 committed" from "all 3 failed" from the reply alone. **Task-group 3 (command apply-after-commit ordering) is now complete**: 3.1 (job construction deferred past commit, bug-1408 caught+fixed) → 3.2 (converted to real async continuation-based commit, majority-ack safety independently verified intact, several documented design deviations) → 3.3 (failure-path guarantee proven under repetition and array mixed-outcomes). **Standing operational blocker carried from 3.2, still true**: no live path applies a successfully-committed job submission until task 6 exists — this branch must not be soak-tested or deployed against live traffic yet.
 - **3.2**: converted `handle_jobsubmit()` from 3.1's still-synchronous commit to a real async registration against task 1's continuation API. New `persist_submit_job_payloads_async()` (persist.c) appends the WAL entries via the same `ha_primary_log_append`/`persist_wal_async` primitive the pre-existing RETAIN-fact path (`wal_hook_append_classified`) already uses, then calls `commit_submit_async()` directly on `p->repl.ce` — bypassing `repl_wait_majority_ack`/`repl_submit_majority_commit` in repl.c, which is what design.md's text actually specifies as the integration site. **Independently verified safe** (reviewer traced the full ACK→`commit_resolve()` path in commit.c): majority-ack detection is driven entirely by `(ce, lsn)` events inside the commit engine, agnostic to which wrapper registered the continuation — no false-commit risk. But it's a real, permanent design deviation, not just an implementation detail: this creates a THIRD parallel "append+track a commit" implementation (alongside the old sync one, now alive only in 3 test files, and the RETAIN-fact one), narrowing the reusable primitive design.md wanted — a future command handler (e.g. a config-change command) gets no shared plumbing from this work and would need its own bespoke path. On FAILED/timeout, replies failure immediately via `jobsubmit_reply_failure_once()`, no job ever constructed. On COMMITTED, calls new `persist_register_pending_apply(p, lsn, on_applied, ctx)` — a hand-off linked list on `persist_t` for task 6.1 to consume — and does NOT apply or reply, per the task's explicit instruction. **Deleted (not preserved-dead) task 3.1's bug-1408 create_job-loop/notify block**: verified correct — `apply_committed_entry()`'s existing `WAL_JOB_SUBMIT` case (task 2.2) already fires `HOOK_DETAIL_JOB_NEW` per-entry independently, structurally immune to bug-1408's batch-suppression class; resurrecting the old loop in task 6 would double-construct/double-notify. Original code fully recoverable from `git show 6f1fa97:src/controller/job.c` if ever needed. **CRITICAL OPERATIONAL CALLOUT — this branch must NOT be soak-tested or deployed against live traffic until task 6 lands**: there is currently NO live code path that applies a successfully-committed job submission (only restart replay, task 2.3, does) — every successful job submission produces a durably-committed WAL entry but ZERO reply to the client, forever, until task 6.1/6.2 exist to consume the pending-apply hand-off and complete the reply. This is exactly what the task text sanctions ("no independently observable behavior until task 6"), not a bug, but it means this commit range is only safe in isolated unit-test form right now. **Hand-off mechanism for task 6.1's briefing**: `persist_register_pending_apply()`/`persist_has_pending_apply()` exist; there is NO "take and remove" accessor yet — task 6.1 must build the consumer side itself. The per-batch `jobsubmit_async_ctx_t` (ref-counted across array elements, frees only once every element has either failed-and-replied or been applied via `jobsubmit_apply_complete`) will not free for any batch containing a committed element until task 6 exists — an accepted, bounded-by-test-scope leak today, but would be unbounded growth under any live/soak run before task 6 lands. Outcome encoding uses a plain `int rc` (not `commit_outcome_t`+rc) to avoid a persist.h→commit.h circular dependency — verified unambiguous today (every real FAILED call site in commit.c passes non-zero rc) but not compiler-enforced; fragile by convention, not by construction. **New regression found by review, not yet fixed, non-blocking**: `array_size == 0` now hangs the client forever and leaks one `jobsubmit_async_ctx_t` per request (3.1's version replied synchronously even for this input; 3.2's async loop never runs so the ctx never releases) — confirmed NOT reachable via any current legitimate client (Go apiserver always sets `arraySize >= 1`; `xce_test`'s `cmd_jobsub` clamps `arsize <= 0` to 1), so flagged as a defense-in-depth hardening gap for a future task rather than fixed here. Also no `array_size > 1` mixed-outcome test exists yet (per-element independent resolution verified correct by code inspection, not by test) — fast-follow recommended.
 - **3.1**: reordered `handle_jobsubmit()` (job.c) so job objects are only constructed via `create_job()` AFTER the (still-synchronous) WAL commit succeeds — extracted `jobid_range_exists()` (fast-fail pre-check, no WAL work), `build_job_submit_payloads()` (WAL-payload construction with no jobmgr insertion), and a new persist.c sibling `persist_submit_job_payloads_and_commit()` (operates on raw `xce_job_t*` instead of pre-existing `job_t*`, single production call site in job.c — legitimately within `handle_jobsubmit`'s call chain, not a boundary spillover; no task-1 continuation API dependency, genuinely still synchronous). Took 3 review rounds. **Round 1 REJECTED — real defect, not documentation**: the post-commit array-construction loop suppressed the `AFTER_EVENT`/`HOOK_DETAIL_JOB_NEW` notification for an ENTIRE array batch if even one sibling element failed to construct (e.g. a jobid collision slipping past `jobid_range_exists()`'s inherent TOCTOU window, or a plain allocation failure — no race needed) — stranding OTHER elements that were durably committed AND successfully constructed, invisible to the purely event-driven scheduler/apiserver until the next restart's replay. Logged as bug-1408. **Round 2 fixed the defect** (moved the `construct_rc < 0` early-return to AFTER the jlist-build + hook-fire block, so every successfully-constructed sibling is always notified regardless of a sibling's failure; client reply still separately reports the batch's `construct_rc`) but was REJECTED again on a narrower, documentation-only ground: bug-1408's RED-phase writeup claimed a clean assertion FAIL, but under the standard full-group `--run test_commit_barrier_reply` invocation the failing assertion actually longjmps past the test's own `cbrp_teardown()` call (Unity `TEST_ABORT` semantics) and HANGS the next test — the same longjmp-past-teardown pathology already known from task 1.3's era. **Round 3 (parent-handled directly, no code regression risk)**: corrected bug-1408's buglog.json description to accurately describe the hang mode, and hardened the new test itself — every assertion-relevant observable is now captured into a local, `cbrp_teardown()` is called unconditionally, and assertions run only after teardown, so a future regression in this exact test can no longer skip cleanup and deadlock sibling tests. No production code changed in round 3; job.c's fix from round 2 was independently verified correct by the round-2 reviewer (traced jlist-build/hook-fire at job.c:717-766 running unconditionally before the `construct_rc` check at line 775) and needed no further changes. **Known low-severity residual, not blocking, flagged for a future task**: `apiserver_push_joblist` (apiserver.c) lacks the `nfiltered > 0` guard `sched.c`'s equivalent has, so a total-batch construction failure (every array element fails, jlist ends up empty) now sends an empty/no-op jobs push to every connected apiserver — informationless, can't corrupt state, but a minor behavioral quirk introduced by firing the hook unconditionally.
 - **2.4**: pure test-only close-out of task-group 2 — new `test_apply_reapply_idempotency.c` (3 tests), no production changes, approved first round. Covers three distinct cases beyond 2.2's existing coverage: (1) triple-reapplication of an identical WAL_JOB_SUBMIT through `apply_committed_entry()` with hook-fire-count AND pointer-identity assertions across all 3 calls (not just final state); (2) the `JOB_TRANSITION_NOOP_DUPLICATE` guard branch (state == current state) — a genuinely untested path, distinct from 2.2's forward-once and backward-rejected cases; (3) the task's named scenario, driven entirely through the real dispatch seam (not seeded/shortcut): real SUBMIT → real FINISH → stale out-of-order START (lower LSN, arriving after), asserting the job stays in its FINISH/terminal state and no third hook fires for the rejected START. Test assertions key on hook-fire COUNT and job STATE, never a specific `HOOK_DETAIL_*` value — so these tests stay valid even if 2.2's documented generic-hook-routing deviation is later corrected. **Task-group 2 (shared apply seam) is now complete**: 2.1 (guard cores extracted) → 2.2 (dispatch function built, generic job-state-family routing is a confirmed permanent deviation from design.md, tracked) → 2.3 (replay wired to the seam, duplicate-notification-on-restart hazard confirmed inert today but must be re-verified before 4.1/6.1/6.2/6.3) → 2.4 (idempotency proven through the real dispatch path). Next: task-group 3 (command apply-after-commit ordering), which is the first consumer of both this seam and task-group 1's commit-engine continuation API.