test(raft-apply-decoupling): idempotent and ordered re-application of apply seam (task 2.4)

Add test_apply_reapply_idempotency.c (3 tests, no production changes):
triple-reapplication of an identical job submit with hook-count and
pointer-identity assertions across every call, the NOOP_DUPLICATE
guard branch (untested by task 2.2), and the task's named scenario --
real SUBMIT then real FINISH then a stale out-of-order START, all
through the real apply_committed_entry() dispatch -- confirming the
job stays terminal with no spurious notification.

Closes out task-group 2 (shared apply seam for committed entries).

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

@@ -38,7 +38,7 @@
   - _Requirements: 9.1, 9.2, 9.3, 9.4_
   - _Depends: 1.4_
 
-- [ ] 2. Shared apply seam for committed entries
+- [x] 2. Shared apply seam for committed entries
 - [x] 2.1 (P) Extract the guarded state-transition logic used by fact handling into a form usable outside the receipt-time call site
   - Keep the existing monotonic-rank checks (reject backward transitions, no-op duplicate terminal transitions) intact and unchanged in behavior
   - Decouple the transition logic from firing its own hook inline, so hook-firing can be relocated by a later task without touching this logic again
@@ -61,7 +61,7 @@
   - _Requirements: 3.1, 3.2_
   - _Depends: 2.2_
 
-- [ ] 2.4 Unit test idempotent and ordered re-application of the dispatch function
+- [x] 2.4 Unit test idempotent and ordered re-application of the dispatch function
   - Cover applying the same committed entry twice is a no-op with respect to observable state
   - Cover that a duplicate or out-of-order fact entry does not regress a job's lifecycle state
   - Observable: a test applies a FINISH entry followed by a stale, previously-unapplied START entry for the same job and confirms the job remains in its terminal state
@@ -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**.
+- **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.
 - **2.3**: rewired `replay_wal()` (persist.c) to call `apply_committed_entry()` (task 2.2's seam) instead of `apply_wal_entry()` for restart replay's per-entry apply. `apply_wal_entry`/`apply_committed_entry` are siblings, not wrapper/wrappee — `apply_wal_entry` is NOT retired, it remains ha.c's live standby catch-up/replicated-apply dispatch (untouched, different call sites, out of this task's boundary). **Confirmed hazard, investigated to resolution by the reviewer (not left open)**: `replay_wal(p, p->checkpoint_lsn, ...)` replays the FULL WAL range since the last snapshot on every restart (`last_applied_lsn` is deliberately not persisted — recomputed at replay time per design.md), not just a narrow crash-gap tail. Since `apply_committed_entry` fires `AFTER_EVENT` hooks (unlike the old `apply_wal_entry`, which never did), this means every restart re-fires notification hooks for entries already applied+notified in a prior process life — a duplicate-notification hazard in principle. **Independently traced and confirmed inert today** for both existing hook consumers: scheduler hooks aren't registered until `on_scheduler_join()` (sched.c), which happens post-boot via network JOIN handshake, strictly after `persist_recover()` runs; apiserver hooks ARE registered before `persist_recover()` (`apiserver_init()` in `cluster_init_phase2()`), but every apiserver hook body gates its actual network notify on `apiservers[i].ep != NULL`, and the apiserver-join table starts empty each process life — a no-op except via a narrow reconnect race. **MUST be re-verified before tasks 4.1/6.1/6.2/6.3** make this path unambiguously live and broad (e.g. if a future task registers apiserver connections earlier, or scheduler hooks get registered pre-replay) — at that point this either needs a replay-mode flag suppressing hook-firing, or an explicit accepted-idempotent-duplicate-notification decision recorded in design.md. A loud in-line comment cross-referencing this note now sits at the `apply_committed_entry()` call site in `replay_wal()` (persist.c). Also confirmed: job-state facts now replay through 2.1's guarded `apply_job_update_transition()` instead of the previously-unguarded `restore_job_state()` — intentional (Req 3/7 online-replay divergence fix), verified no existing test depended on the old unguarded semantics. `apply_committed_entry`'s missing oversized-payload guard (vs. `apply_wal_entry`'s `MAX_WAL_PAYLOAD_BYTES`/`-EMSGSIZE` check) confirmed inert — `persist_wal_write_replicated()` already rejects oversized payloads at write time, before anything reaches disk.
 - **2.2**: implemented `apply_committed_entry(persist_t*, uint32_t wal_type, uint64_t lsn, const void* payload, uint32_t len)` in persist.c/persist.h, matching design.md's signature. Routes `WAL_JOB_SUBMIT` through `restore_job()` (idempotent on duplicate jobid — no-op, Req 7.1), `WAL_QUEUE_CTRL`/`WAL_WORKER_CTRL` through their existing restore primitives, and fires each type's post-mutation hook exactly once, after mutation. **Confirmed permanent, non-cosmetic deviation from design.md's Apply Seam wording**: ALL nine job-state fact WAL types (`WAL_JOB_{START,FINISH,FAIL,STOP,RESUME,UPDATE,RUSAGE,CKPT,PREASON}`) route through the single guard-only `apply_job_update_transition()` core from 2.1 — NOT the type-specific `apply_job_{start,finish,fail}_transition` cores — and fire one generic `HOOK_DETAIL_JOB_UPDATE` hook, not the type-specific `HOOK_DETAIL_JOB_START`/`_EXIT`/`_FAIL`. Root cause, independently verified by two review rounds: the on-disk wire format for every one of these types (`wal_jobstate_t {jobid, state}`) genuinely only carries the resulting state, not the richer `job_pidinfo_t`/`xce_jobfinish_t`/`job_failinfo_t` payload the type-specific cores/hooks require — this is a pre-existing wire-format reality (`apply_wal_entry` already collapsed these the same way before this task), not a shortcut invented here. **design.md's Apply Seam section and Fact Ingestion → Event Contract section currently describe per-type hook dispatch that this wire format cannot support as written** — this is a real, standing design/reality conflict, not yet reconciled in design.md itself. No task in the current plan (2.3, 4.1, 4.2, 6.1, 6.2, 6.3) enriches the wire format to close this gap — confirmed by reading all of them; task 4.1 is exclusively about removing receipt-time mutation/hook-firing, not payload enrichment. **Explicit blocking-style call-out for whoever implements 4.1/6.2/6.3**: wiring this dispatch function into a live fact-apply path will permanently change every worker-reported job start/finish/fail notification to a generic UPDATE detail instead of the type-specific one, unless the wire format is enriched first or design.md is corrected to formally accept this substitution — verify this is still the accepted behavior before wiring it live, don't assume it'll be fixed by then. Took 3 review rounds to land: round 1 rejected two doc-comment defects (a citation blaming the wrong crash site, and a dangling pointer to task 4.1 as the fix location); round 2's fix corrected the crash site (real one is `sched.c`'s `sched_job_start_fun`/`_exit_fun`/`_fail_fun`, which unconditionally dereference `info->jobid` before any NULL check) but fabricated a nonexistent function name (`encode_JobPIDInfo_ptr`) as supporting evidence; round 3 replaced it with a narrower, fully source-verified claim (apiserver.c's equivalent functions store the raw pointer into a notify union without dereferencing it at that call site — real, checked line-by-line). **Lesson**: when a reviewer catches a citation error, the safest fix is the narrowest claim you can personally verify by reading the actual code, not a broader-sounding replacement — round 2's attempt to explain "why the alternative is fine" reached for an unverified downstream mechanism and got caught fabricating it.
 - **2.1**: split `mark_job_start`/`mark_job_finish`/`mark_job_fail`/`mark_job_update` (job.c) into four hook-free cores (`apply_job_{start,finish,fail,update}_transition`, new `job_transition_result_t` enum in cluster.h) containing only the monotonic-rank guard + mutation, verified an unmodified lift line-by-line against pre-extraction code. Wrappers peek the same guard predicate to decide hook-firing, then delegate to the core — no unlock/relock gap between peek and delegated call, so no new race from the double-evaluation. `mark_job_fail`'s pre-existing lack of a guard is preserved, not added or removed. New `test_apply_seam.c` (8 tests) calls the cores directly, bypassing `on_job_*` receipt handlers entirely — this is the form task 2.2 (dispatch function) and task 6.1 (apply pump) will call into. **Non-blocking follow-up flagged by reviewer**: the new doc-comments on `apply_job_fail_transition`/`apply_job_update_transition` overclaim a universal "caller holds RTTI_LOCK_WRITE" precondition — `job_ctrl` (job.c:396, pre-existing, untouched) calls `mark_job_update`/`mark_job_fail` under only RTTI_LOCK_READ. No new race (extraction added no unlock/relock), but whoever writes task 2.2/6.1 should not trust that comment's blanket claim — verify the actual lock held at each call site instead.

src/controller/meson.build

@@ -160,6 +160,7 @@ controller_unit_test_src = [
   'testsuite/test_apply_seam.c',
   'testsuite/test_apply_committed_entry.c',
   'testsuite/test_replay_apply_seam.c',
+  'testsuite/test_apply_reapply_idempotency.c',
   'ctrlstatus.c',
   'commit.c',
   'job.c',

src/controller/testsuite/controller_test.c

@@ -80,6 +80,7 @@ extern void test_persist_snapshot_entities(void);
 extern void test_apply_seam(void);
 extern void test_apply_committed_entry(void);
 extern void test_replay_apply_seam(void);
+extern void test_apply_reapply_idempotency(void);
 
 void setUp(void) { return; }
 
@@ -229,6 +230,7 @@ int main(int argc, char* argv[])
     TEST(test_apply_seam);
     TEST(test_apply_committed_entry);
     TEST(test_replay_apply_seam);
+    TEST(test_apply_reapply_idempotency);
 
     return UNITY_END();
 }

src/controller/testsuite/test_apply_reapply_idempotency.c

@@ -0,0 +1,380 @@
+/* test_apply_reapply_idempotency.c — raft-apply-decoupling task 2.4
+ *
+ * Extends task 2.2's coverage of apply_committed_entry() (persist.c) with the
+ * idempotent/ordered re-application guarantees Req 3.4/7.1/7.2/7.3 require,
+ * driven end-to-end through the real dispatch function (never by calling the
+ * task-2.1 guard cores directly — test_apply_seam.c already covers those).
+ *
+ * Does NOT duplicate test_apply_committed_entry.c's existing coverage
+ * (one duplicate-job-submit case, one directly-seeded-terminal backward-
+ * rejected case). This file adds:
+ *
+ *   1. A command-family (WAL_JOB_SUBMIT) entry re-applied THREE times,
+ *      proving the mutation and notification stay pinned at "happened once"
+ *      across repeated redelivery, and that the job object's identity
+ *      (pointer) never changes — no reconstruction on re-apply.
+ *   2. A fact-family (WAL_JOB_START) entry re-applied with the IDENTICAL
+ *      state twice — the JOB_TRANSITION_NOOP_DUPLICATE path inside
+ *      apply_job_update_transition() — which 2.2's tests never exercised
+ *      (2.2 covered forward-once and backward-from-a-directly-seeded-
+ *      terminal job, not a same-state duplicate).
+ *   3. The task's named scenario: SUBMIT -> FINISH -> stale, previously-
+ *      unapplied START, all three driven through apply_committed_entry(),
+ *      confirming the job stays in its FINISH-terminal state after the
+ *      stale START and that the stale START fires NO notification. The
+ *      guard here is state-based (job's current lifecycle stage vs. the
+ *      requested next stage), not LSN-position-based — LSN ordering is the
+ *      apply pump's job (task 6.1, out of this task's boundary); this test
+ *      proves the state guard alone is sufficient to stop a regression even
+ *      if a stale entry ever reaches the dispatch seam out of order.
+ */
+
+#include <rtti.h>
+
+#include <capnp_c.h>
+#include <errno.h>
+#include <stdarg.h>
+#include <stdbool.h>
+#include <string.h>
+
+#include <unity/unity.h>
+
+#include "cluster.h"
+#include "controller.capnp.h"
+#include "hook.h"
+#include "jobj.h"
+#include "persist.h"
+#include "wal_types.h"
+#include "xce/cluster_def.h"
+#include "xce/types.h"
+#include "xstring.h"
+
+/* jobj.c's jobsub table and the process-global xstring table are shared
+ * across every test file in the controller_unit_test binary — guard
+ * idempotently, mirroring test_apply_committed_entry.c/test_apply_seam.c. */
+static int g_tari_jobobj_ready;
+
+static cluster_t g_tari_c;
+static persist_t g_tari_p;
+
+/* ── Spy hook: counts AFTER_EVENT firings, ignores all payload args ─────── */
+static int g_tari_hook_fires;
+
+static int tari_spy_hook(int type, int situation, int detail, void* data,
+                         va_list ap)
+{
+    (void)type; (void)detail; (void)data; (void)ap;
+    if (situation == HOOK_AFTER_EVENT)
+        g_tari_hook_fires++;
+    return 0;
+}
+
+static void tari_setup(void)
+{
+    kvmgr_params_t int_params = { .key_type = KVMGR_KEY_INT,
+                                  .nslots   = 32,
+                                  .delta    = 16 };
+    kvmgr_params_t str_params = { .key_type = KVMGR_KEY_STRING,
+                                  .nslots   = 32,
+                                  .delta    = 16 };
+
+    if (makeString((char*)"tari-probe") == NULL)
+        xstring_table_init(64);
+
+    if (!g_tari_jobobj_ready) {
+        TEST_ASSERT_EQUAL_INT(0, job_obj_init(32, 32));
+        g_tari_jobobj_ready = 1;
+    }
+
+    memset(&g_tari_c, 0, sizeof(g_tari_c));
+    g_tari_c.myhost           = &g_tari_c.myhost_info;
+    g_tari_c.myhost_info.name = (char*)"ctrl-test";
+    g_tari_c.myhost_info.key  = (char*)"";
+
+    TEST_ASSERT_EQUAL_INT(0,
+        xce_hook_init(&g_tari_c.hooksys, XCE_CONTROLLER_HOOK_MAX));
+
+    g_tari_c.jobmgr    = makeInstance(&xkvmgr_type, &int_params);
+    g_tari_c.qmgr      = makeInstance(&xkvmgr_type, &str_params);
+    g_tari_c.workermgr = makeInstance(&xkvmgr_type, &str_params);
+    g_tari_c.resmgr    = makeInstance(&xkvmgr_type, &str_params);
+    TEST_ASSERT_NOT_NULL(g_tari_c.jobmgr);
+    TEST_ASSERT_NOT_NULL(g_tari_c.qmgr);
+    TEST_ASSERT_NOT_NULL(g_tari_c.workermgr);
+    TEST_ASSERT_NOT_NULL(g_tari_c.resmgr);
+    zarray_init(&g_tari_c.sharedinst, 16, NULL);
+
+    memset(&g_tari_p, 0, sizeof(g_tari_p));
+    g_tari_p.cluster = &g_tari_c;
+
+    g_tari_hook_fires = 0;
+}
+
+static void tari_teardown(void)
+{
+    zarray_fini(&g_tari_c.sharedinst);
+    if (g_tari_c.resmgr)    dereference(g_tari_c.resmgr);
+    if (g_tari_c.workermgr) dereference(g_tari_c.workermgr);
+    if (g_tari_c.qmgr)      dereference(g_tari_c.qmgr);
+    if (g_tari_c.jobmgr)    dereference(g_tari_c.jobmgr);
+    xce_hook_fini(&g_tari_c.hooksys);
+}
+
+/* Mirrors the file-static capnp_encode_job in persist.c (file-static, so
+ * every test file that needs it keeps a local copy — same pattern as
+ * test_apply_committed_entry.c's tace_encode_job / test_egress_filter.c's
+ * ef_encode_job). */
+static int tari_encode_job(const xce_job_t* j, uint8_t** out, size_t* out_len)
+{
+    struct capn          c;
+    struct capn_segment* cs;
+    struct Job           cj;
+    Job_ptr              jp;
+
+    capn_init_malloc(&c);
+    cs = capn_root(&c).seg;
+    encode_Job(cs, &cj, (xce_job_t*)j);
+    jp = new_Job(cs);
+    write_Job(&cj, jp);
+    capn_setp(capn_root(&c), 0, jp.p);
+
+    *out_len = capn_size(&c);
+    *out     = malloc(*out_len);
+    if (!*out) { capn_free(&c); return -ENOMEM; }
+    capn_write_mem(&c, *out, *out_len, 0);
+    capn_free(&c);
+    return 0;
+}
+
+/* Look up a job_t wrapper by jobid (NULL if absent). */
+static job_t* tari_find_job(uint64_t jobid)
+{
+    job_t* j = NULL;
+    lock(g_tari_c.jobmgr, RTTI_LOCK_READ);
+    int found = key_find(g_tari_c.jobmgr, (void*)jobid, (void**)&j);
+    unlock(g_tari_c.jobmgr);
+    return (found > 0) ? j : NULL;
+}
+
+/* Creates a job directly in the fixture's jobmgr, bypassing
+ * handle_jobsubmit/restore_job entirely — for seeding fact-transition
+ * tests that don't need a real SUBMIT-through-dispatch step. */
+static job_submit_data_t g_tari_seed_submit;
+
+static job_t* tari_seed_job(uint64_t jobid, uint32_t state)
+{
+    xce_job_t jdef;
+
+    memset(&g_tari_seed_submit, 0, sizeof(g_tari_seed_submit));
+    g_tari_seed_submit.queue = (char*)"q";
+    g_tari_seed_submit.user  = (char*)"tester";
+
+    memset(&jdef, 0, sizeof(jdef));
+    jdef.jobid  = jobid;
+    jdef.state  = state;
+    jdef.submit = &g_tari_seed_submit;
+
+    return makeJob(g_tari_c.jobmgr, &jdef);
+}
+
+/* ── 1. Command-family (WAL_JOB_SUBMIT) idempotent re-application ──────── */
+
+static void test_tari_job_submit_reapplied_thrice_is_idempotent(void)
+{
+    tari_setup();
+
+    job_submit_data_t submit;
+    memset(&submit, 0, sizeof(submit));
+    submit.queue = (char*)"q";
+    submit.user  = (char*)"tester";
+
+    xce_job_t jdef;
+    memset(&jdef, 0, sizeof(jdef));
+    jdef.jobid  = 701;
+    jdef.state  = JOB_STAT_PEND;
+    jdef.submit = &submit;
+
+    uint8_t* payload = NULL;
+    size_t   plen    = 0;
+    TEST_ASSERT_EQUAL_INT(0, tari_encode_job(&jdef, &payload, &plen));
+
+    TEST_ASSERT_EQUAL_INT(0,
+        xce_add_hook(&g_tari_c.hooksys, XCE_CONTROLLER_HOOK_JOB,
+                     tari_spy_hook, NULL));
+
+    /* First application: real mutation + one notification. */
+    TEST_ASSERT_EQUAL_INT(0, apply_committed_entry(&g_tari_p, WAL_JOB_SUBMIT,
+                                                   100, payload,
+                                                   (uint32_t)plen));
+    TEST_ASSERT_EQUAL_INT_MESSAGE(1, g_tari_hook_fires,
+        "the first application must fire exactly one notification");
+
+    job_t* first = tari_find_job(701);
+    TEST_ASSERT_NOT_NULL(first);
+    TEST_ASSERT_EQUAL_UINT32(JOB_STAT_PEND, first->job.state);
+
+    /* Re-apply the SAME (wal_type, lsn, payload) twice more — simulating
+     * replay/resync redelivering the identical committed entry. Req 3.4/7.1:
+     * observable state must not change again, and the notification must not
+     * fire again, on EITHER repeat. */
+    for (int i = 0; i < 2; i++) {
+        TEST_ASSERT_EQUAL_INT(0,
+            apply_committed_entry(&g_tari_p, WAL_JOB_SUBMIT, 100, payload,
+                                  (uint32_t)plen));
+        TEST_ASSERT_EQUAL_INT_MESSAGE(1, g_tari_hook_fires,
+            "re-applying an identical committed SUBMIT must never fire a "
+            "second notification, no matter how many times it repeats");
+
+        job_t* again = tari_find_job(701);
+        TEST_ASSERT_EQUAL_PTR_MESSAGE(first, again,
+            "re-application must not reconstruct the job — the same object "
+            "identity must persist across every redelivery");
+        TEST_ASSERT_EQUAL_UINT32(JOB_STAT_PEND, again->job.state);
+    }
+
+    free(payload);
+    tari_teardown();
+    printf("test_tari_job_submit_reapplied_thrice_is_idempotent: PASS\n");
+}
+
+/* ── 2. Fact-family (WAL_JOB_START) identical-duplicate re-application ─── */
+
+static void test_tari_job_state_transition_reapplied_identical_is_idempotent(
+    void)
+{
+    tari_setup();
+
+    job_t* j = tari_seed_job(702, JOB_STAT_PEND);
+    TEST_ASSERT_NOT_NULL(j);
+
+    wal_jobstate_t js = { .jobid = 702, .state = JOB_STAT_RUN };
+
+    xce_add_hook(&g_tari_c.hooksys, XCE_CONTROLLER_HOOK_JOB, tari_spy_hook,
+                NULL);
+
+    /* First application: forward PEND -> RUN, one notification. */
+    TEST_ASSERT_EQUAL_INT(0, apply_committed_entry(&g_tari_p, WAL_JOB_START,
+                                                   200, &js, sizeof(js)));
+    TEST_ASSERT_EQUAL_UINT32(JOB_STAT_RUN, j->job.state);
+    TEST_ASSERT_EQUAL_INT_MESSAGE(1, g_tari_hook_fires,
+        "the first forward transition must fire exactly one notification");
+
+    /* Re-apply the IDENTICAL entry (same jobid, same requested state RUN,
+     * same LSN) — this hits apply_job_update_transition()'s state==cur
+     * NOOP_DUPLICATE guard specifically (Req 3.4/7.1), a path 2.2's own
+     * tests never drove through apply_committed_entry (2.2 only covered a
+     * forward-once apply and a backward-from-directly-seeded-terminal
+     * apply, never a same-state duplicate). Do it twice to prove the no-op
+     * is stable, not a one-shot fluke. */
+    for (int i = 0; i < 2; i++) {
+        int rc = apply_committed_entry(&g_tari_p, WAL_JOB_START, 200, &js,
+                                       sizeof(js));
+        TEST_ASSERT_EQUAL_INT_MESSAGE(0, rc,
+            "a duplicate identical fact re-application is a success no-op, "
+            "not an apply failure");
+        TEST_ASSERT_EQUAL_UINT32_MESSAGE(JOB_STAT_RUN, j->job.state,
+            "a duplicate identical fact must leave the job's state "
+            "unchanged");
+        TEST_ASSERT_EQUAL_INT_MESSAGE(1, g_tari_hook_fires,
+            "a duplicate identical fact re-application must fire NO "
+            "additional notification");
+    }
+
+    tari_teardown();
+    printf("test_tari_job_state_transition_reapplied_identical_is_"
+          "idempotent: PASS\n");
+}
+
+/* ── 3. Named scenario: SUBMIT -> FINISH -> stale, previously-unapplied
+ *      START, entirely through apply_committed_entry() ────────────────── */
+
+static void
+test_tari_finish_then_stale_start_preserves_terminal_state_via_dispatch(void)
+{
+    tari_setup();
+
+    xce_add_hook(&g_tari_c.hooksys, XCE_CONTROLLER_HOOK_JOB, tari_spy_hook,
+                NULL);
+
+    /* Step 1: SUBMIT the job for real, through the dispatch seam (not
+     * seeded directly) — jobid 703 starts PEND. */
+    job_submit_data_t submit;
+    memset(&submit, 0, sizeof(submit));
+    submit.queue = (char*)"q";
+    submit.user  = (char*)"tester";
+
+    xce_job_t jdef;
+    memset(&jdef, 0, sizeof(jdef));
+    jdef.jobid  = 703;
+    jdef.state  = JOB_STAT_PEND;
+    jdef.submit = &submit;
+
+    uint8_t* payload = NULL;
+    size_t   plen    = 0;
+    TEST_ASSERT_EQUAL_INT(0, tari_encode_job(&jdef, &payload, &plen));
+
+    TEST_ASSERT_EQUAL_INT(0, apply_committed_entry(&g_tari_p, WAL_JOB_SUBMIT,
+                                                   300, payload,
+                                                   (uint32_t)plen));
+    free(payload);
+
+    job_t* j = tari_find_job(703);
+    TEST_ASSERT_NOT_NULL(j);
+    TEST_ASSERT_EQUAL_UINT32(JOB_STAT_PEND, j->job.state);
+    TEST_ASSERT_EQUAL_INT_MESSAGE(1, g_tari_hook_fires,
+        "SUBMIT must fire exactly one notification before FINISH is ever "
+        "applied");
+
+    /* Step 2: apply a FINISH entry (job-state-family wire shape, same as
+     * START/UPDATE per 2.2's documented wal_jobstate_t routing deviation) —
+     * drives the job to its terminal state. */
+    wal_jobstate_t finish = { .jobid = 703, .state = JOB_STAT_DONE };
+
+    TEST_ASSERT_EQUAL_INT(0, apply_committed_entry(&g_tari_p, WAL_JOB_FINISH,
+                                                   301, &finish,
+                                                   sizeof(finish)));
+    TEST_ASSERT_EQUAL_UINT32_MESSAGE(JOB_STAT_DONE, j->job.state,
+        "FINISH must drive the job to its terminal state");
+    TEST_ASSERT_EQUAL_INT_MESSAGE(2, g_tari_hook_fires,
+        "FINISH must fire exactly one notification (SUBMIT's + FINISH's = "
+        "2 total so far)");
+
+    /* Step 3: apply a STALE, previously-unapplied START for the SAME job —
+     * simulating a duplicate/out-of-order fact arriving after the job
+     * already reached its terminal state (Req 7.1/7.2). The guard inside
+     * apply_job_update_transition() is state-based (current lifecycle stage
+     * vs. requested next stage) — it does not consult the LSN at all, so
+     * this proves the state guard alone stops the regression regardless of
+     * what LSN the stale entry carries (LSN-order enforcement is the apply
+     * pump's job, task 6.1, out of this task's boundary). Use an LSN lower
+     * than FINISH's to make the "out-of-order" framing literal. */
+    wal_jobstate_t stale_start = { .jobid = 703, .state = JOB_STAT_RUN };
+
+    int rc = apply_committed_entry(&g_tari_p, WAL_JOB_START, 250,
+                                   &stale_start, sizeof(stale_start));
+    TEST_ASSERT_EQUAL_INT_MESSAGE(0, rc,
+        "a guard-rejected stale START is a success no-op, not an apply "
+        "failure");
+    TEST_ASSERT_EQUAL_UINT32_MESSAGE(JOB_STAT_DONE, j->job.state,
+        "the job must remain in its FINISH-terminal state after the stale "
+        "START is applied");
+    TEST_ASSERT_EQUAL_INT_MESSAGE(2, g_tari_hook_fires,
+        "the stale, guard-rejected START must fire NO notification — the "
+        "count must stay exactly where FINISH left it");
+
+    tari_teardown();
+    printf("test_tari_finish_then_stale_start_preserves_terminal_state_via_"
+          "dispatch: PASS\n");
+}
+
+/* ── Test group entry point ───────────────────────────────────────────── */
+
+void test_apply_reapply_idempotency(void)
+{
+    UNITY_BEGIN();
+    RUN_TEST(test_tari_job_submit_reapplied_thrice_is_idempotent);
+    RUN_TEST(test_tari_job_state_transition_reapplied_identical_is_idempotent);
+    RUN_TEST(
+        test_tari_finish_then_stale_start_preserves_terminal_state_via_dispatch);
+    UNITY_END();
+}