test(ha-commit-barrier-rollout): RED tests for mark_job_* guards (task 6.1)

3 RED carriers (duplicate FINISH on DONE appends a 2nd WAL entry + overwrites state; START on terminal clobbers it; replay non-equivalent) + 2 preservation pins (forward postdone PDONE-merge, fail-after-finish PERR-merge) that 6.2 must not break. Reviewer-confirmed: the UPDATE-clears-terminal arm is production-unreachable (pinned, not driven). 22 tests / 3 RED; census 508 distinct triples, only those 3 fail.
2026-06-13 04:20:05 +00:00 · 2026-06-13 04:20:05 +00:00 · 2308389eb1
commit 2308389eb1
parent 5a2fae1d37
4 changed files with 365 additions and 1 deletions
--- a/.kiro/specs/ha-commit-barrier-rollout/tasks.md
+++ b/.kiro/specs/ha-commit-barrier-rollout/tasks.md
@ -79,7 +79,7 @@
  - _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
+- [x] 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_
@ -112,6 +112,7 @@
 - (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).
+- (6.1) For 6.2 (reviewer-confirmed): the "UPDATE clears terminal bits" guard arm (Req 5.2) is UNREACHABLE in production — mark_job_update's 3 callers (job.c:400 SIGCONT clears only PSUSP|USUSP, :410 SIGSTOP ORs PSUSP, :876 postdone ORs PDONE/EXIT) never produce terminal→non-terminal. 6.2 will most naturally share ONE backward-guard predicate across START+UPDATE; the START-on-terminal RED test exercises it, and the forward-update PIN protects the one reachable terminal UPDATE (postdone PDONE-merge) from over-rejection. on_job_* wrappers return 0 unconditionally — 6.2's caller audit must keep them returning 0 (success) on guarded-out transitions; the pins lock this. Guard placement: top of each mark_job_*, under the caller-held WRITE lock, before BEFORE_EVENT. Live anchors drifted: mark_job_start:592, finish:647, fail:705 (PERR-merge :711-721), update:735.
 - (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.
--- a/.wolf/cerebrum.md
+++ b/.wolf/cerebrum.md
@ -51,6 +51,7 @@
 - key_find (xkvmgr_find) returns 1 when found, NOT 0 — queue_ctrl/worker_ctrl/job_ctrl leak this into rc: `if (rc == 0)` hook gates never fire on the found path (bug-447); any code reusing key_find's rc for success checks must normalise it.
 - Controller test binary reply observation: no xce_reply hook exists in src/lib; observe replies on a zeroed no-iomgr endpoint_t — xce_reply endpoint_enq()s onto ep->outq; drain with endpoint_deq + raw capnp read_Request/read_RequestReply (test_commit_barrier_reply.c).
 - makeJob needs job_obj_init() once per process AND xce_job_t.submit != NULL; makeString needs xstring_table_init (test_worker_join.c inits it unguarded at group entry — probe makeString for NULL before initialising to stay -t-selection safe).
+- (2026-06-13, barrier-rollout 6.1) mark_job_start/finish/fail/update are STATIC in job.c — testsuite reaches them ONLY via the non-static wrappers on_job_start/on_job_finish/on_job_fail (and on_job_postdone → mark_job_update). Those wrappers `return 0` unconditionally regardless of mark_job_* rc, so the guard's "success-no-op rc" observable is asserted on the wrapper rc (== 0). No public caller produces a terminal→non-terminal UPDATE (on_job_postdone OR's PDONE = forward; job_ctrl SIGCONT only clears suspend bits) — mark_job_start (state := JOB_STAT_RUN) is the faithful publicly-reachable terminal-clearing witness for the shared backward rule. current_lsn is the "did a hook fire / WAL append" oracle: a guarded-out no-op runs no hooks → no persist_wal_async → current_lsn frozen; combine with cbrp_count_jobstate_entries (on-disk scan) + a key_find live-state read. Controller suite baseline 2026-06-13: 502 distinct PASS triples pre-6.1; +3 with 6.1's 2 preservation pins landing green (forward-update, PERR-merge) = 505, 3 FAIL = the 3 new RED guard tests.

 ## Do-Not-Repeat

--- a/.wolf/memory.md
+++ b/.wolf/memory.md
@ -1011,3 +1011,10 @@
 |------|--------|---------|---------|--------|
 | 20:45 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | inline fix | ~37 |
 | 20:46 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | 1→2 lines | ~383 |
+| 20:56 | Edited src/controller/testsuite/test_commit_barrier_reply.c | added 2 condition(s) | ~4126 |
+| 20:56 | Edited src/controller/testsuite/test_commit_barrier_reply.c | expanded (+9 lines) | ~216 |
+| 20:58 | Edited src/controller/testsuite/test_commit_barrier_reply.c | TEST_ASSERT_EQUAL_INT() → WAL_JOB_START() | ~111 |
+| 04:07 | task 6.1 RED: 5 mark_job_* guard tests (3 red, 2 preservation pins) | testsuite/test_commit_barrier_reply.c | full suite 3 FAIL=new RED, 505 PASS | ~14k |
+| 04:18 | task 6.1 review: RED tests for mark_job_* guards verified | test_commit_barrier_reply.c | APPROVED — 22/3 group, 508/3 census, boundary clean | ~38k |
+| 21:19 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | inline fix | ~13 |
+| 21:19 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | modified 2() | ~243 |
--- a/src/controller/testsuite/test_commit_barrier_reply.c
+++ b/src/controller/testsuite/test_commit_barrier_reply.c
@ -1498,6 +1498,352 @@ static void test_fact_retention_across_quorum_loss(void)
    printf("test_fact_retention_across_quorum_loss: PASS\n");
 }

+/* ════════════════════════════════════════════════════════════════════════
+ * Idempotency / ordering guards for mark_job_* (task 6.1, Req 5.1/5.2/5.3)
+ * ════════════════════════════════════════════════════════════════════════
+ *
+ * RED-phase tests for the transition guards that task 6.2 adds at the top
+ * of mark_job_start / mark_job_finish / mark_job_fail / mark_job_update
+ * (job.c:583/638/696/726 pre-task anchors). Design §Idempotency Guards:
+ *   - duplicate (already-reflected transition) → INFO log, NO-OP: state
+ *     unchanged, BEFORE/AFTER_EVENT hooks NOT fired → NO WAL entry appended
+ *     (current_lsn unmoved), caller-visible rc treated as success-no-op (5.1)
+ *   - backward (terminal current state, non-terminal requested: START on a
+ *     finished job, UPDATE clearing terminal bits) → WARN log, REJECT,
+ *     terminal state preserved (5.2)
+ *   - replay: re-applying the same transition converges to single-
+ *     application state — no second WAL entry, identical final state (5.3)
+ *   - mark_job_fail's existing fail-after-finish PERR-merge (job.c:702-716)
+ *     is forward-compatible and must be PRESERVED by the guard (pin)
+ *
+ * Drivers: the static mark_job_* are reached through their non-static
+ * callers on_job_start / on_job_finish / on_job_fail (job.c) — exactly the
+ * fact path used by test_fact_retention_across_quorum_loss, minus the
+ * opcode decode. These wrappers each return 0 unconditionally today, so the
+ * "success-no-op rc" observable (every caller treats a guarded-out
+ * transition as success) is asserted directly on the wrapper rc.
+ *
+ * Observability of state: the per-job WAL append count and the LAST
+ * recorded post-transition state come from cbrp_count_jobstate_entries
+ * (direct on-disk scan, defined above); the job's LIVE in-memory state is
+ * read via cbrp_job_live_state (key_find on g_hc.jobmgr). current_lsn is
+ * the "did a hook fire / WAL entry append" oracle (a guarded-out no-op runs
+ * no hooks → no persist_wal_async → current_lsn frozen).
+ *
+ * RED expectation against the UNGUARDED mark_job_* (no guard yet):
+ *   - duplicate FINISH re-fires the AFTER_EVENT hook → a 2nd WAL_JOB_FINISH
+ *     append, current_lsn advances → the "current_lsn unmoved / count==1"
+ *     assertions FAIL.
+ *   - START on a DONE job unconditionally sets state = JOB_STAT_RUN and
+ *     appends WAL_JOB_START → the "state preserved DONE / no new append"
+ *     assertions FAIL.
+ *   - replay FINISH appends a duplicate entry → the "single append"
+ *     assertion FAILS.
+ * The fail-after-finish PERR-merge pin and the success-no-op rc pins lock
+ * behavior that must hold both before and after the guard (preservation
+ * pins, like the task-3.1 validation pins); they ride this group so 6.2
+ * cannot silently regress the merge or the wrapper return contract.
+ *
+ * NOTE on the UPDATE-clears-terminal arm (5.2): mark_job_update is static
+ * with no public caller that produces a terminal→non-terminal state
+ * (on_job_postdone's update is forward — it OR's PDONE, preserving terminal;
+ * job_ctrl SIGCONT only clears suspend bits). mark_job_start IS the
+ * canonical "set state to the non-terminal JOB_STAT_RUN" overwrite, so the
+ * START-on-terminal case below is the faithful, publicly-reachable witness
+ * of the shared backward rule the design lists for START and UPDATE
+ * together. test_guard_forward_update_on_terminal_preserved additionally
+ * pins that a FORWARD update on a terminal job (on_job_postdone) is NOT
+ * over-rejected — the boundary the guard must respect.
+ */
+
+/* Read a job's live in-memory state from g_hc.jobmgr (post-transition).
+ * Returns the state flags, or -1 if the job is absent. */
+static int cbrp_job_live_state(uint64_t jobid)
+{
+    job_t *j  = NULL;
+    int    st = -1;
+
+    lock(g_hc.jobmgr, RTTI_LOCK_READ);
+    key_find(g_hc.jobmgr, (void *)jobid, (void **)&j);
+    if (j != NULL) {
+        reference(j);
+    }
+    unlock(g_hc.jobmgr);
+
+    if (j != NULL) {
+        lock(j, RTTI_LOCK_READ);
+        st = (int)j->job.state;
+        unlock(j);
+        dereference(j);
+    }
+    return st;
+}
+
+/* Drive a job to terminal DONE via start(jobid) → finish(jobid, exit 0).
+ * Returns current_lsn after the two forward facts (the watermark the
+ * duplicate/backward cases must NOT advance). */
+static uint64_t cbrp_drive_to_done(uint64_t jobid)
+{
+    static unsigned int gd_pids[1] = { 9999 };
+    job_pidinfo_t       start;
+    xce_jobfinish_t     fin;
+    uint64_t            lsn;
+
+    memset(&start, 0, sizeof(start));
+    start.jobid = jobid;
+    start.npids = 1;
+    start.pids  = &gd_pids[0];
+    TEST_ASSERT_EQUAL_INT(0, on_job_start(&g_hc, &start));
+
+    memset(&fin, 0, sizeof(fin));
+    fin.jobid     = jobid;
+    fin.exit_code = 0; /* → JOB_STAT_DONE */
+    TEST_ASSERT_EQUAL_INT(0, on_job_finish(&g_hc, &fin));
+
+    TEST_ASSERT_EQUAL_INT_MESSAGE(JOB_STAT_DONE, cbrp_job_live_state(jobid),
+        "precondition: job must be DONE after start→finish(0)");
+
+    pthread_mutex_lock(&g_hc.persist.lock);
+    lsn = g_hc.persist.current_lsn;
+    pthread_mutex_unlock(&g_hc.persist.lock);
+    return lsn;
+}
+
+/* 5.1 — duplicate FINISH on a DONE job is a logged no-op: state unchanged,
+ * NO AFTER_EVENT hook fired (NO new WAL entry → current_lsn frozen), exactly
+ * one FINISH entry on disk, and on_job_finish returns success-no-op (0). */
+static void test_guard_duplicate_finish_on_done_is_noop(void)
+{
+    xce_jobfinish_t fin;
+    uint64_t        lsn_after_done, lsn_after_dup;
+    uint32_t        st_finish = 0;
+    int             rc;
+
+    cbrp_setup(3000);
+    cbrp_make_job(70, &g_job_submit);
+    memset(&g_job_submit, 0, sizeof(g_job_submit));
+
+    lsn_after_done = cbrp_drive_to_done(70);
+    TEST_ASSERT_EQUAL_INT_MESSAGE(1,
+        cbrp_count_jobstate_entries(&g_hc.persist, WAL_JOB_FINISH, 70, NULL),
+        "exactly one FINISH entry must exist after the first finish");
+
+    /* Act: a duplicate FINISH on the already-DONE job (retransmit / replay) */
+    memset(&fin, 0, sizeof(fin));
+    fin.jobid     = 70;
+    fin.exit_code = 0;
+    rc = on_job_finish(&g_hc, &fin);
+
+    pthread_mutex_lock(&g_hc.persist.lock);
+    lsn_after_dup = g_hc.persist.current_lsn;
+    pthread_mutex_unlock(&g_hc.persist.lock);
+
+    /* Assert: success-no-op rc; state unchanged; NO hook fired (no append) */
+    TEST_ASSERT_EQUAL_INT_MESSAGE(0, rc,
+        "a guarded-out duplicate FINISH must return success-no-op (5.1)");
+    TEST_ASSERT_EQUAL_INT_MESSAGE(JOB_STAT_DONE, cbrp_job_live_state(70),
+        "duplicate FINISH must leave the job's state unchanged (5.1)");
+    TEST_ASSERT_EQUAL_UINT64_MESSAGE(lsn_after_done, lsn_after_dup,
+        "duplicate FINISH must fire NO AFTER_EVENT hook → NO WAL entry "
+        "appended → current_lsn unmoved (5.1)");
+    TEST_ASSERT_EQUAL_INT_MESSAGE(1,
+        cbrp_count_jobstate_entries(&g_hc.persist, WAL_JOB_FINISH, 70,
+                                    &st_finish),
+        "duplicate FINISH must not append a second FINISH entry (5.1)");
+    TEST_ASSERT_EQUAL_UINT32(JOB_STAT_DONE, st_finish);
+
+    cbrp_teardown();
+    printf("test_guard_duplicate_finish_on_done_is_noop: PASS\n");
+}
+
+/* 5.2 — START on a finished (DONE) job is rejected: the terminal state is
+ * preserved (never overwritten with JOB_STAT_RUN), NO WAL_JOB_START entry is
+ * appended, and on_job_start returns a success-no-op code that the caller
+ * (do_notify) treats as success. mark_job_start is the canonical
+ * terminal→non-terminal overwrite, so this also witnesses the UPDATE-clears-
+ * terminal arm of the shared backward rule (5.2). */
+static void test_guard_start_on_terminal_rejected_state_preserved(void)
+{
+    static unsigned int rs_pids[1] = { 7001 };
+    job_pidinfo_t       restart;
+    uint64_t            lsn_after_done, lsn_after_start;
+    int                 rc;
+
+    cbrp_setup(3000);
+    cbrp_make_job(71, &g_job_submit);
+    memset(&g_job_submit, 0, sizeof(g_job_submit));
+
+    lsn_after_done = cbrp_drive_to_done(71);
+    /* Precondition: drive_to_done's start→finish appended exactly one
+     * WAL_JOB_START (the forward start). The rejected backward START below
+     * must not add a second one. */
+    TEST_ASSERT_EQUAL_INT_MESSAGE(1,
+        cbrp_count_jobstate_entries(&g_hc.persist, WAL_JOB_START, 71, NULL),
+        "precondition: exactly one START entry from the forward start");
+
+    /* Act: a stale/duplicate START arrives for the already-finished job */
+    memset(&restart, 0, sizeof(restart));
+    restart.jobid = 71;
+    restart.npids = 1;
+    restart.pids  = &rs_pids[0];
+    rc = on_job_start(&g_hc, &restart);
+
+    pthread_mutex_lock(&g_hc.persist.lock);
+    lsn_after_start = g_hc.persist.current_lsn;
+    pthread_mutex_unlock(&g_hc.persist.lock);
+
+    /* Assert: terminal preserved, no append, success-no-op rc */
+    TEST_ASSERT_EQUAL_INT_MESSAGE(0, rc,
+        "a rejected backward START must return success-no-op to the caller "
+        "(5.2)");
+    TEST_ASSERT_EQUAL_INT_MESSAGE(JOB_STAT_DONE, cbrp_job_live_state(71),
+        "START on a DONE job must preserve the terminal state — never "
+        "overwrite it with JOB_STAT_RUN (5.2)");
+    TEST_ASSERT_EQUAL_UINT64_MESSAGE(lsn_after_done, lsn_after_start,
+        "a rejected backward START must append NO WAL entry (5.2)");
+    TEST_ASSERT_EQUAL_INT_MESSAGE(1,
+        cbrp_count_jobstate_entries(&g_hc.persist, WAL_JOB_START, 71, NULL),
+        "a rejected backward START must not append a second START entry "
+        "(5.2)");
+
+    cbrp_teardown();
+    printf("test_guard_start_on_terminal_rejected_state_preserved: PASS\n");
+}
+
+/* 5.3 — replay equivalence: applying the same FINISH transition twice yields
+ * state identical to a single application AND a single WAL entry. The second
+ * application is the guarded-out duplicate; the on-disk FINISH count staying
+ * at 1 with current_lsn frozen is the replay-once invariant. */
+static void test_guard_replay_finish_equals_single_application(void)
+{
+    xce_jobfinish_t fin;
+    uint64_t        lsn_single, lsn_replay;
+    int             state_single, state_replay;
+    uint32_t        st_disk = 0;
+
+    cbrp_setup(3000);
+    cbrp_make_job(72, &g_job_submit);
+    memset(&g_job_submit, 0, sizeof(g_job_submit));
+
+    /* Single application baseline */
+    lsn_single   = cbrp_drive_to_done(72);
+    state_single = cbrp_job_live_state(72);
+    TEST_ASSERT_EQUAL_INT(1,
+        cbrp_count_jobstate_entries(&g_hc.persist, WAL_JOB_FINISH, 72, NULL));
+
+    /* Replay the FINISH (retry / WAL replay re-application) */
+    memset(&fin, 0, sizeof(fin));
+    fin.jobid     = 72;
+    fin.exit_code = 0;
+    TEST_ASSERT_EQUAL_INT_MESSAGE(0, on_job_finish(&g_hc, &fin),
+        "replayed FINISH must return success-no-op (5.3)");
+
+    pthread_mutex_lock(&g_hc.persist.lock);
+    lsn_replay = g_hc.persist.current_lsn;
+    pthread_mutex_unlock(&g_hc.persist.lock);
+    state_replay = cbrp_job_live_state(72);
+
+    /* Assert: state and the WAL log are identical to single application */
+    TEST_ASSERT_EQUAL_INT_MESSAGE(state_single, state_replay,
+        "replayed transition must converge to single-application state "
+        "(5.3)");
+    TEST_ASSERT_EQUAL_UINT64_MESSAGE(lsn_single, lsn_replay,
+        "replaying a committed transition must append no second WAL entry "
+        "(5.3)");
+    TEST_ASSERT_EQUAL_INT_MESSAGE(1,
+        cbrp_count_jobstate_entries(&g_hc.persist, WAL_JOB_FINISH, 72,
+                                    &st_disk),
+        "the FINISH fact must remain on disk exactly once after replay "
+        "(5.3)");
+    TEST_ASSERT_EQUAL_UINT32(JOB_STAT_DONE, st_disk);
+
+    cbrp_teardown();
+    printf("test_guard_replay_finish_equals_single_application: PASS\n");
+}
+
+/* 5.2 boundary pin — a FORWARD update on a terminal job must NOT be
+ * over-rejected by the guard. on_job_postdone is the reachable mark_job_update
+ * caller: on a DONE job it OR's JOB_STAT_PDONE (terminal preserved, strictly
+ * forward). This must still apply (state gains PDONE, an entry appends) after
+ * the guard lands — the guard rejects backward, never a forward merge.
+ * Preservation pin: green before and after task 6.2. */
+static void test_guard_forward_update_on_terminal_preserved(void)
+{
+    uint64_t lsn_after_done, lsn_after_pdone;
+    int      st;
+
+    cbrp_setup(3000);
+    cbrp_make_job(73, &g_job_submit);
+    memset(&g_job_submit, 0, sizeof(g_job_submit));
+
+    lsn_after_done = cbrp_drive_to_done(73);
+
+    /* Act: post-done arrives — forward update merging PDONE onto DONE */
+    TEST_ASSERT_EQUAL_INT(0, on_job_postdone(&g_hc, &g_req_ep, 73));
+
+    pthread_mutex_lock(&g_hc.persist.lock);
+    lsn_after_pdone = g_hc.persist.current_lsn;
+    pthread_mutex_unlock(&g_hc.persist.lock);
+    st = cbrp_job_live_state(73);
+
+    /* Assert: terminal preserved, PDONE merged forward, the update appended */
+    TEST_ASSERT_TRUE_MESSAGE((st & JOB_STAT_DONE) != 0,
+        "a forward update on a terminal job must preserve the terminal bit");
+    TEST_ASSERT_TRUE_MESSAGE((st & JOB_STAT_PDONE) != 0,
+        "the forward update must merge JOB_STAT_PDONE (not be rejected)");
+    TEST_ASSERT_TRUE_MESSAGE(lsn_after_pdone > lsn_after_done,
+        "a forward update on a terminal job must still append its WAL entry "
+        "— the guard rejects backward transitions, never forward merges");
+
+    cbrp_teardown();
+    printf("test_guard_forward_update_on_terminal_preserved: PASS\n");
+}
+
+/* 5.2 pin — mark_job_fail's existing fail-after-finish PERR-merge
+ * (job.c:702-716) must be PRESERVED by the guard: a POST_EXEC-stage fail on a
+ * DONE job keeps DONE and OR's JOB_STAT_PERR (it does NOT reset the job to a
+ * bare JOB_STAT_EXIT). Forward-compatible terminal→terminal merge — the guard
+ * must not reject it. Preservation pin: green before and after task 6.2. */
+static void test_guard_fail_after_finish_preserves_perr_merge(void)
+{
+    job_failinfo_t failinfo;
+    uint64_t       lsn_after_done, lsn_after_fail;
+    int            st;
+
+    cbrp_setup(3000);
+    cbrp_make_job(74, &g_job_submit);
+    memset(&g_job_submit, 0, sizeof(g_job_submit));
+
+    lsn_after_done = cbrp_drive_to_done(74);
+
+    /* Act: a post-exec-stage failure for the already-finished job */
+    memset(&failinfo, 0, sizeof(failinfo));
+    failinfo.jobid  = 74;
+    failinfo.stage  = JOB_EXEC_STAGE_POST_EXEC;
+    failinfo.code   = 1;
+    failinfo.detail = (char *)"post-exec fail after finish";
+    TEST_ASSERT_EQUAL_INT(0, on_job_fail(&g_hc, &failinfo));
+
+    pthread_mutex_lock(&g_hc.persist.lock);
+    lsn_after_fail = g_hc.persist.current_lsn;
+    pthread_mutex_unlock(&g_hc.persist.lock);
+    st = cbrp_job_live_state(74);
+
+    /* Assert: DONE preserved, PERR merged in — not reset to bare EXIT */
+    TEST_ASSERT_TRUE_MESSAGE((st & JOB_STAT_DONE) != 0,
+        "fail-after-finish must keep the DONE terminal bit (PERR-merge "
+        "behavior, job.c:702-716)");
+    TEST_ASSERT_TRUE_MESSAGE((st & JOB_STAT_PERR) != 0,
+        "fail-after-finish must OR JOB_STAT_PERR onto the existing terminal "
+        "state (PERR-merge behavior, job.c:702-716)");
+    TEST_ASSERT_TRUE_MESSAGE(lsn_after_fail > lsn_after_done,
+        "the PERR-merge fail must still append its WAL entry (forward "
+        "terminal→terminal merge — not a guarded-out no-op)");
+
+    cbrp_teardown();
+    printf("test_guard_fail_after_finish_preserves_perr_merge: PASS\n");
+}
+
 /* ── Test group entry point ──────────────────────────────────────────── */

 void test_commit_barrier_reply(void)
@ -1534,5 +1880,14 @@ void test_commit_barrier_reply(void)
    /* Fact retention across quorum loss (task 4.3, Req 3.1, 3.3, 3.4) —
     * green on landing against tasks 1.2 + 4.2 output */
    RUN_TEST(test_fact_retention_across_quorum_loss);
+    /* mark_job_* idempotency / ordering guards (task 6.1, Req 5.1/5.2/5.3)
+     * — the no-op / backward-reject / replay cases FAIL (red) against the
+     * unguarded mark_job_*; the forward-merge and PERR-merge cases are
+     * preservation pins (green before and after task 6.2) */
+    RUN_TEST(test_guard_duplicate_finish_on_done_is_noop);
+    RUN_TEST(test_guard_start_on_terminal_rejected_state_preserved);
+    RUN_TEST(test_guard_replay_finish_equals_single_application);
+    RUN_TEST(test_guard_forward_update_on_terminal_preserved);
+    RUN_TEST(test_guard_fail_after_finish_preserves_perr_merge);
    UNITY_END();
 }