fix(raft-apply-decoupling): bug-1419 layer 2 — checkpoint_lsn mislabeled from WAL tip instead of applied cursor

persist_checkpoint() stamped checkpoint_lsn from p->current_lsn (physical
WAL append tip) instead of p->last_applied_lsn (the true "jobmgr reflects
up through here" cursor), uncoordinated with the async apply pump
introduced by raft-apply-decoupling. write_snapshot() serializes live
jobmgr state, so a checkpoint taken while apply lags the WAL tip mislabels
the snapshot: on restart last_applied_lsn is re-seeded from checkpoint_lsn
and replay resumes strictly above it, silently skipping committed-but-not-
yet-applied entries jobmgr never received. This is the actual mechanism
behind kill-mid-apply producing a permanent APPLY_DIVERGENT fence —
apply_committed_entry() is deterministic/idempotent and write_snapshot()
is crash-atomic, so the bug was a mislabeled watermark, not a structural
divergence requiring a safety/liveness tradeoff.

Fix stamps checkpoint_lsn from last_applied_lsn and holds apply_pump_lock
across the label capture + write_snapshot() so the label and the jobmgr
state it describes are captured atomically w.r.t. the live apply pump.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VgT3n4vTv4ZjCuCTmzed1F

src/controller/persist.c

@@ -2258,10 +2258,27 @@ int persist_checkpoint(persist_t* p)
     if (rc != 0)
         goto done;
 
-    /* 2. Record checkpoint_lsn = current position after the marker */
-    pthread_mutex_lock(&p->lock);
-    p->checkpoint_lsn = p->current_lsn;
-    pthread_mutex_unlock(&p->lock);
+    /* 2. Record checkpoint_lsn = the APPLIED cursor (last_applied_lsn), not
+     *    the physical WAL append tip (current_lsn). write_snapshot() below
+     *    serializes live jobmgr state, which only reflects entries the
+     *    apply pump has actually dispatched -- NOT every entry durably
+     *    appended/committed, since apply is decoupled from commit
+     *    (raft-apply-decoupling) and can lag behind current_lsn under load.
+     *    Stamping current_lsn here mislabels the snapshot: on restart,
+     *    last_applied_lsn is re-seeded FROM checkpoint_lsn and replay
+     *    resumes strictly above it, so any committed-but-not-yet-applied
+     *    entries between the true apply cursor and current_lsn at
+     *    checkpoint time are silently skipped forever, even though jobmgr
+     *    never actually contained them (persist.h last_applied_lsn doc).
+     *    Held under apply_pump_lock together with 3a/3b so the label and
+     *    the jobmgr walk it describes are captured atomically w.r.t. the
+     *    live apply pump -- the pump can only ever move last_applied_lsn
+     *    and jobmgr forward, never backward, so freezing it here rules out
+     *    the label ever claiming more than jobmgr actually holds. */
+    pthread_mutex_lock(&p->apply_pump_lock);
+
+    p->checkpoint_lsn = atomic_load_explicit(&p->last_applied_lsn,
+                                              memory_order_acquire);
 
     /* 3a. Capture the latest committed config-marker reference BEFORE trimming
      *     (ha-term-safety task 4.2 r1): the trim below can unlink the segment
@@ -2272,6 +2289,9 @@ int persist_checkpoint(persist_t* p)
     /* 3b. Serialize in-memory state to a snapshot file
      *    (P0: no live kvmgrs — writes sentinel header + cached config ref) */
     rc = write_snapshot(p);
+
+    pthread_mutex_unlock(&p->apply_pump_lock);
+
     if (rc != 0)
         goto done;
 

src/controller/testsuite/test_persist_checkpoint.c

@@ -108,6 +108,15 @@ static void test_checkpoint_sets_checkpoint_lsn(void)
         persist_wal_async(p, WAL_JOB_UPDATE, "x", 1);
 
     uint64_t lsn_before = p->current_lsn;
+
+    /* No live apply pump runs in this cluster-less unit-test fixture, so
+     * last_applied_lsn stays 0 unless a test drives it directly — simulate
+     * apply having fully caught up to what was appended (persist_checkpoint
+     * now stamps checkpoint_lsn from last_applied_lsn, not current_lsn;
+     * see the checkpoint_lsn-vs-last_applied_lsn fix). */
+    atomic_store_explicit(&p->last_applied_lsn, lsn_before,
+                           memory_order_relaxed);
+
     TEST_ASSERT_EQUAL_INT(0, persist_checkpoint(p));
 
     /* checkpoint_lsn must have advanced to at least lsn_before */
@@ -117,6 +126,50 @@ static void test_checkpoint_sets_checkpoint_lsn(void)
     printf("test_checkpoint_sets_checkpoint_lsn: PASS\n");
 }
 
+/* ------------------------------------------------------------------ */
+/* Test 2b: checkpoint_lsn tracks the APPLIED cursor, not the WAL tip  */
+/* ------------------------------------------------------------------ */
+
+/* Regression test for the checkpoint_lsn/last_applied_lsn mislabeling bug
+ * (persist.h last_applied_lsn doc, "DISTINCT from current_lsn"): when the
+ * apply pump lags behind the physical WAL append tip -- the normal state
+ * under raft-apply-decoupling once a commit resolves before its entry is
+ * dispatched into jobmgr -- a checkpoint must label the snapshot with the
+ * position jobmgr ACTUALLY reflects (last_applied_lsn), never the tip
+ * (current_lsn). Stamping current_lsn here would seed the next recovery's
+ * last_applied_lsn/replay-resume point past entries jobmgr never received,
+ * permanently skipping them (the root cause behind bug-1419's apply-divergent
+ * fence on kill-mid-apply). */
+static void test_checkpoint_lsn_tracks_last_applied_not_current_lsn(void)
+{
+    char tmpdir[256];
+    persist_t* p = make_ckpt_persist(tmpdir, sizeof(tmpdir));
+    TEST_ASSERT_NOT_NULL(p);
+
+    /* Append 5 entries, but only simulate the apply pump having dispatched
+     * the first 2 of them (jobmgr reflects up through lsn_before + 1). */
+    uint64_t lsn_before = p->current_lsn;
+    for (int i = 0; i < 5; i++)
+        persist_wal_async(p, WAL_JOB_UPDATE, "z", 1);
+
+    TEST_ASSERT_EQUAL_UINT64(lsn_before + 5, p->current_lsn);
+
+    const uint64_t simulated_apply_cursor = lsn_before + 1;
+    atomic_store_explicit(&p->last_applied_lsn, simulated_apply_cursor,
+                           memory_order_relaxed);
+
+    TEST_ASSERT_EQUAL_INT(0, persist_checkpoint(p));
+
+    /* checkpoint_lsn must equal the applied cursor, NOT the WAL tip -- a
+     * checkpoint_lsn of 5 here would mean recovery resumes replay above
+     * LSN 5, silently dropping entries 3-5 that jobmgr never applied. */
+    TEST_ASSERT_EQUAL_UINT64(simulated_apply_cursor, p->checkpoint_lsn);
+    TEST_ASSERT_TRUE(p->checkpoint_lsn < p->current_lsn);
+
+    cleanup_ckpt_persist(p);
+    printf("test_checkpoint_lsn_tracks_last_applied_not_current_lsn: PASS\n");
+}
+
 /* ------------------------------------------------------------------ */
 /* Test 3: checkpoint writes a WAL_CHECKPOINT marker (advances lsn)   */
 /* ------------------------------------------------------------------ */
@@ -192,6 +245,7 @@ void test_persist_checkpoint(void)
 {
     RUN_TEST(test_checkpoint_creates_snapshot_file);
     RUN_TEST(test_checkpoint_sets_checkpoint_lsn);
+    RUN_TEST(test_checkpoint_lsn_tracks_last_applied_not_current_lsn);
     RUN_TEST(test_checkpoint_writes_marker_to_wal);
     RUN_TEST(test_checkpoint_busy_guard);
     RUN_TEST(test_checkpoint_rotation_keeps_snap_keep_n);