test(scheduler-snapshot-job-allocation): close originally-reported symptom end to end

Task 4.2: extend test_e2e_original_motivation_long_replay with PREEXIST_ID
(job 70013, dedicated worker worker-e2e-pre) -- a job submitted,
live-dispatched, and confirmed RUN entirely before sched_record_start(),
so its dispatch decision can never appear in the replay stream. Proves
its "used=9.00" resource contribution is correct starting from the very
first replayed round, recoverable only via snapshot-load allocation
restoration (tasks 2.1/2.2), not replay-based reconciliation -- the
complementary case to the file's existing ANCHOR_ID scenario.

Completes and fixes a pre-existing uncommitted WIP addendum to this file:
removed a redundant enqueue_queue_update() call that was masking a
genuine, unrelated latent bug in core_upsert_queue() (rtti makeString()
pointer freed via plain free(), same class as bug-1503/bug-1600) --
noted in tasks.md's Implementation Notes as a follow-up, not fixed here
(test-only task boundary).

Verified via fault injection: gating snapshot_load.c's allocation
restore reproduces the exact pre-fix failure at this test's new
assertion, then cleanly reverts.

Requirements: 1.1, 1.2, 1.3

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

.kiro/specs/scheduler-snapshot-job-allocation/tasks.md

@@ -61,7 +61,7 @@
   - _Depends: 1.2_
   - _Boundary: snapshot load (scheduling-representation construction), test_snapshot_save_load.c_
 
-- [ ] 4.2 (P) Reproduce and close the originally-reported resource-accounting symptom end to end
+- [x] 4.2 (P) Reproduce and close the originally-reported resource-accounting symptom end to end
   - Extend the existing simulation-mode scenario that already reproduces the reported symptom (a saved state with many running jobs, replayed forward) to assert that every running job's resource contribution is correctly reflected immediately after loading, not only the jobs whose dispatch also happens to appear later in the replay.
   - Observable completion: the extended end-to-end scenario passes, showing 100% of the saved running jobs' resource contribution accounted for immediately on load.
   - _Requirements: 1.1, 1.2, 1.3_
@@ -89,3 +89,6 @@
   - Observable completion: the audit dimension reports clean, with this feature's fields correctly classified as fixed or documented.
   - _Requirements: 3.2, 3.3, 3.4_
   - _Depends: 4.1, 4.2, 4.3, 4.4_
+
+## Implementation Notes
+- Task 4.2 side-finding (not fixed here, out of boundary): `core_upsert_queue()`'s existing-queue branch (`core_events.c:335-336`) calls `FREEUP(q->resreq)` (plain `free()`) on a `resreq` that was set via the rtti-backed `makeString()` (mismatched allocator/free, same bug class as already-fixed bug-1503/bug-1600). Reproducible via any second `QUEUE_UPDATE` for a queue whose `resreq` was ever populated via `makeString()` — not specific to snapshot restoration; the near-identical pattern already exists in the legacy `sched_update_existing_queue()` (`schedobj.c:573-575`). Confirmed real by two independent readings (implementer + reviewer). Needs its own buglog.json entry and likely a separate follow-up task — tracked here so it isn't lost.

src/scheduler/testsuite/test_sim_e2e_original_motivation.c

@@ -120,6 +120,43 @@
  * check/accounting primitives). Links a REAL (not no-op) sched_account_jobs()
  * reimplementation, copied verbatim from scheduler.c (task 1.1's relocated
  * signature) -- same rationale those two files' own header comments give.
+ *
+ * ============================================================================
+ * ADDENDUM (scheduler-snapshot-job-allocation spec, task 4.2): PREEXIST_ID
+ * ============================================================================
+ * Everything above this addendum predates and is unchanged by this task.
+ * Task 4.2's own Observable Completion ("100% of the saved running jobs'
+ * resource contribution accounted for immediately on load") is NOT proven
+ * by ANCHOR_ID above: ANCHOR_ID's accounting is recovered by replay-based
+ * reconciliation (sim_reconcile_unaccounted_jobs(), a DIFFERENT, already-
+ * shipped mechanism from a prior spec) matching its DISPATCH decision
+ * against round 1's own decoded ReplaySession -- i.e. its correctness
+ * depends on a dispatch decision that DOES appear somewhere in the replay
+ * stream. This spec (scheduler-snapshot-job-allocation) closes the
+ * COMPLEMENTARY case: a job whose dispatch predates the snapshot entirely
+ * and can NEVER appear in the subsequent replay stream, recoverable only
+ * via xce_sched_core_load_snapshot()'s own allocation restoration (task
+ * 2.1's build_wire_job() save-side wiring + task 2.2's dup_job_alloc()/
+ * make_job_account() load-side wiring).
+ *
+ * PREEXIST_ID (job 70013, worker "worker-e2e-pre", a dedicated worker
+ * isolated from worker-e2e's carefully-derived R2-R24 capacity arithmetic
+ * above) is constructed to be exactly that case: submitted, live-dispatched
+ * (a completely ordinary, unrigged sched_check_host_satisfy()-gated
+ * dispatch -- see preexist_prelude_allocator_thread_fn below), and
+ * confirmed RUN, ALL strictly before sched_record_start() is ever called.
+ * Its dispatch decision is therefore never written to the .replay file at
+ * all -- sched_record_start()'s own initial SnapshotEntry (recording.c's
+ * write_snapshot_entry()) is the ONLY place its allocation is ever
+ * captured, and xce_sched_core_load_snapshot() (sim.c's
+ * sim_locate_and_load_snapshot(), called before round 1 ever runs) is the
+ * ONLY place it is ever restored. The test asserts PREEXIST_ID's "used"
+ * value is already correct after just ONE replayed round (the earliest
+ * possible observation point, strictly before ANCHOR_ID's own first
+ * checkpoint) and stays correct through all 24, while its own DISPATCH
+ * decision is proven to NEVER appear anywhere in the captured replay
+ * output -- the concrete, falsifiable difference from ANCHOR_ID's
+ * reconciliation-based mechanism.
  */
 #include <errno.h>
 #include <limits.h>
@@ -311,6 +348,15 @@ void tearDown(void) { return; }
 #define WORKER_NAME     "worker-e2e"
 #define WORKER_CAPACITY 20.0f
 
+/* scheduler-snapshot-job-allocation task 4.2: PREEXIST_ID's own dedicated
+ * worker, isolated from worker-e2e's R2-R24 capacity arithmetic above --
+ * see this file's ADDENDUM header comment. */
+#define WORKER2_NAME     "worker-e2e-pre"
+#define WORKER2_CAPACITY 15.0f
+
+#define PREEXIST_ID   70013ULL
+#define PREEXIST_WANT 9.0f
+
 #define ANCHOR_ID   70001ULL
 #define ANCHOR_WANT 12.0f
 #define J2_ID       70002ULL
@@ -433,6 +479,31 @@ static void fixture_init(xce_scheduler_t* sched, char* record_dir)
 
     rc = sched_add_worker(&(sched->core), w);
     TEST_ASSERT_TRUE(rc >= 0);
+
+    /* scheduler-snapshot-job-allocation task 4.2: PREEXIST_ID's own
+     * dedicated worker (see this file's ADDENDUM header comment) --
+     * isolated from worker-e2e's own carefully-derived R2-R24 capacity
+     * arithmetic above so this task's addition cannot perturb it. */
+    {
+        xce_sched_worker_t* w2;
+
+        w2 = (xce_sched_worker_t*)calloc(1, sizeof(*w2));
+        TEST_ASSERT_NOT_NULL(w2);
+        w2->name = strdup(WORKER2_NAME);
+        TEST_ASSERT_NOT_NULL(w2->name);
+
+        rc = zarray_init(&(w2->resvals), 8, NULL);
+        TEST_ASSERT_EQUAL_INT(0, rc);
+        rc = zarray_init(&(w2->resInUse), 8, NULL);
+        TEST_ASSERT_EQUAL_INT(0, rc);
+
+        val[0] = WORKER2_CAPACITY;
+        rc     = put_resvals(&(w2->resvals), 1, &g_slots_residx, val);
+        TEST_ASSERT_TRUE(rc >= 0);
+
+        rc = sched_add_worker(&(sched->core), w2);
+        TEST_ASSERT_TRUE(rc >= 0);
+    }
 }
 
 static xce_queue_t* make_queue(const char* name, int32_t priority)
@@ -710,6 +781,99 @@ static xce_decision_t* decide_one_candidate(xce_scheduler_t*    sched,
     return decision;
 }
 
+/* ---------------------------------------------------------------------- */
+/* Prelude allocator (scheduler-snapshot-job-allocation task 4.2, see this  */
+/* file's ADDENDUM header comment): dispatches PREEXIST_ID onto its own    */
+/* dedicated worker (WORKER2_NAME) via the exact same real                 */
+/* sched_check_host_satisfy()/sched_charge_account()/sched_commit_job_     */
+/* alloc()/make_dispatch_decision() round trip decide_one_candidate()      */
+/* above already uses -- a completely ordinary, unrigged live dispatch, no */
+/* special-casing at all (unlike ANCHOR_ID's necessarily-rigged injection  */
+/* in round1_allocator_thread_fn below, which exists only because ANCHOR_  */
+/* ID must be simultaneously RUN and carry a same-round DISPATCH decision, */
+/* a shape no live picker/allocator can ever itself produce). This is a    */
+/* deliberately SEPARATE helper from decide_one_candidate() (which         */
+/* hardcodes WORKER_NAME) rather than a generalization of it, so this      */
+/* task's addition cannot perturb that already-validated function's        */
+/* behavior for ANCHOR_ID/J2-J12. Runs strictly BEFORE sched_record_start  */
+/* () (see the Prelude section of the test body below), so this dispatch   */
+/* decision is never written to the .replay file at all.                  */
+/* ---------------------------------------------------------------------- */
+
+static void* preexist_prelude_allocator_thread_fn(void* arg)
+{
+    xce_scheduler_t*    sched  = (xce_scheduler_t*)arg;
+    xce_sched_core_t*   core   = &(sched->core);
+    xce_sched_plugin_t* picker = NULL;
+    uint64_t            jobid;
+
+    if (sched_resolve_job_picker(core, &picker) < 0) {
+        xce_decision_t* decision = NULL;
+
+        cbuf_get(&(core->jbuf), &jobid);
+        cbuf_put(&(core->decisions), &decision);
+        return NULL;
+    }
+
+    for (;;) {
+        xce_decision_t*     decision;
+        xce_sched_job_t*    job;
+        xce_sched_worker_t* w2;
+        xvec_t              uncomitted;
+        xvec_idval_t        wanted;
+        xvec_idval_t        unmeet;
+        int                 rc;
+
+        cbuf_get(&(core->jbuf), &jobid);
+        if (jobid == 0) {
+            decision = NULL;
+            cbuf_put(&(core->decisions), &decision);
+            break;
+        }
+
+        TEST_ASSERT_EQUAL_UINT64_MESSAGE(
+            PREEXIST_ID, jobid,
+            "the prelude round must only ever present PREEXIST_ID as a "
+            "candidate -- nothing else has been pushed to the core yet at "
+            "this point");
+
+        job = sched_get_job(core, jobid);
+        w2  = sched_get_worker(core, (char*)WORKER2_NAME);
+        TEST_ASSERT_NOT_NULL(job);
+        TEST_ASSERT_NOT_NULL(w2);
+
+        rc = xvec_init(&uncomitted, 1, sizeof(float));
+        TEST_ASSERT_TRUE(rc >= 0);
+        rc = xvec_idval_init(&wanted, 1);
+        TEST_ASSERT_EQUAL_INT(0, rc);
+        rc = xvec_idval_store(&wanted, g_slots_residx, PREEXIST_WANT);
+        TEST_ASSERT_TRUE(rc >= 0);
+
+        rc = sched_check_host_satisfy(sched, w2, &uncomitted, &wanted, &unmeet);
+
+        xvec_fini(&uncomitted);
+        xvec_idval_fini(&wanted);
+        xvec_idval_fini(&unmeet);
+
+        TEST_ASSERT_EQUAL_INT_MESSAGE(
+            1, rc,
+            "PREEXIST_ID's own dedicated worker must have ample capacity -- "
+            "this is a genuine, unrigged live dispatch, not an engineered "
+            "coincidence like ANCHOR_ID's");
+
+        job->accounts
+            = make_charge_account(WORKER2_NAME, PREEXIST_WANT, g_slots_residx);
+        sched_commit_job_alloc(core, job);
+        job->allocs = make_dispatch_alloc(jobid, WORKER2_NAME, PREEXIST_WANT);
+
+        picker->picker_iface->on_dispatch(picker, jobid);
+        decision = make_dispatch_decision(job->allocs);
+        cbuf_put(&(core->decisions), &decision);
+    }
+
+    return NULL;
+}
+
 /* ---------------------------------------------------------------------- */
 /* Round 1's allocator: injects ANCHOR_ID's own already-resolved DISPATCH  */
 /* (see this file's header comment for the full reasoning), THEN          */
@@ -1017,6 +1181,7 @@ static void test_e2e_original_motivation_long_replay(void)
     char            capture_path[PATH_MAX];
     xce_job_t**     anchor_jobs;
     xce_job_t**     j2_jobs;
+    xce_job_t**     preexist_jobs;
     uint64_t        sub_time;
     int             rc;
     int             i;
@@ -1029,20 +1194,88 @@ static void test_e2e_original_motivation_long_replay(void)
     char*           p2;
     char*           p3;
     char*           p4;
+    char            preexist_jobid_str[64];
+    char            preexist_marker[96];
+    char*           q1;
+    char*           q2;
+    char*           q3;
+    char*           q4;
+    char*           q5;
 
     /* ==== Build the recording: a real, driven 24-round session ==== */
 
     make_tmp_dir(record_dir, sizeof(record_dir), "_rec");
     fixture_init(&sched, record_dir);
 
+    /* ---- Prelude (scheduler-snapshot-job-allocation task 4.2, see this
+     * file's ADDENDUM header comment): PREEXIST_ID is submitted,
+     * genuinely live-dispatched, and confirmed RUN entirely BEFORE
+     * recording starts -- so it "predates the snapshot" exactly as
+     * Requirement 1 describes, and its dispatch decision is never written
+     * to the .replay file at all (nothing is recorded yet). ---- */
+    enqueue_queue_update(&sched, "normal", 1);
+
+    preexist_jobs    = (xce_job_t**)calloc(1, sizeof(xce_job_t*));
+    preexist_jobs[0] = make_job(PREEXIST_ID, 100ULL, "normal", "preexist-user");
+    enqueue_jobs_push(&sched, preexist_jobs, 1);
+
+    run_round(&sched, preexist_prelude_allocator_thread_fn);
+
+    {
+        xce_sched_job_t* preexist = sched_get_job(&(sched.core), PREEXIST_ID);
+
+        TEST_ASSERT_NOT_NULL(preexist);
+        TEST_ASSERT_FALSE_MESSAGE(
+            JOB_IS_RUN(preexist->state),
+            "PREEXIST_ID must still be PEND immediately after its dispatch "
+            "decision -- RUN confirmation is a separate step, exactly as it "
+            "is for every other job in this file (J2-J12)");
+        TEST_ASSERT_NOT_NULL_MESSAGE(
+            preexist->accounts,
+            "the prelude's genuine live dispatch must have already set "
+            "PREEXIST_ID's accounts");
+    }
+
+    enqueue_jstate_change(&sched, PREEXIST_ID, JOB_STAT_RUN);
+    run_round(&sched, generic_allocator_thread_fn);
+
+    {
+        xce_sched_job_t* preexist = sched_get_job(&(sched.core), PREEXIST_ID);
+
+        TEST_ASSERT_NOT_NULL(preexist);
+        TEST_ASSERT_TRUE_MESSAGE(
+            JOB_IS_RUN(preexist->state),
+            "PREEXIST_ID must now be RUN -- fully 'already running' before "
+            "recording ever starts, matching Requirement 1's exact "
+            "scenario");
+    }
+
     rc = sched_record_start(&sched, "scheduler-e2e-original-motivation.replay");
     TEST_ASSERT_EQUAL_INT(0, rc);
     TEST_ASSERT_TRUE(strlen(sched.record_file_path) < sizeof(r_path));
     strcpy(r_path, sched.record_file_path);
 
     /* ---- Round 1: ANCHOR_ID's genesis (arrival + RUN confirmation, both
-     * via the real dataq/translate path) + J2's genuinely live dispatch. */
+     * via the real dataq/translate path) + J2's genuinely live dispatch.
-    enqueue_queue_update(&sched, "normal", 1);
+     * No enqueue_queue_update() here (unlike before task 4.2's prelude was
+     * added): the "normal" queue already exists as of the prelude above,
+     * and is therefore already part of the initial SnapshotEntry
+     * sched_record_start() just captured. Re-issuing an identical
+     * QUEUE_UPDATE here would additionally get RECORDED into round 1 and
+     * REPLAYED against that same snapshot-restored queue -- exercising
+     * core_upsert_queue()'s "queue already exists -> update in place"
+     * branch for a queue whose xce_sched_queue_t was built by
+     * xce_sched_core_load_snapshot()'s own restore path rather than
+     * core_build_queue() -- a genuine, pre-existing (latent, previously
+     * uncovered by any test) defect in that combination, confirmed by
+     * empirically triggering it (ASAN bad-free in core_upsert_queue,
+     * core_events.c:336, freeing q->resreq) while first writing this
+     * addendum. Fixing that defect is production code, out of this
+     * test-only task's boundary (Requirement 1/2/3 do not depend on it);
+     * simply not re-declaring an already-existing queue is both correct
+     * (the queue's priority/status are unchanged either way) and sufficient
+     * to keep this task's own addition from exercising unrelated,
+     * out-of-boundary production code. */
 
     register_want(ANCHOR_ID, ANCHOR_WANT);
     anchor_jobs    = (xce_job_t**)calloc(1, sizeof(xce_job_t*));
@@ -1100,17 +1333,28 @@ static void test_e2e_original_motivation_long_replay(void)
     snprintf(script_path, sizeof(script_path), "%s/script.txt", aux_dir);
     snprintf(capture_path, sizeof(capture_path), "%s/capture.out", aux_dir);
 
-    /* 3 + 10 + 8 = 21 stepped rounds, then continue drains the final 3
+    /* 1 + 2 + 10 + 8 = 21 stepped rounds, then continue drains the final 3
-     * (22, 23, 24) to end of stream. */
+     * (22, 23, 24) to end of stream. The lone "step 1" first (splitting
+     * off what used to be the first "step 3") gives PREEXIST_ID (task
+     * 4.2) a checkpoint after just ONE round -- the earliest possible
+     * observation point, strictly before ANCHOR_ID's own first checkpoint
+     * at round 3 -- while the cumulative round counts at every other
+     * checkpoint (3, 13, 21, 24) are unchanged from before this task. */
     write_text_file(script_path,
-                    "step 3\n"
+                    "step 1\n"
+                    "info worker worker-e2e-pre\n"
+                    "step 2\n"
                     "info worker worker-e2e\n"
+                    "info worker worker-e2e-pre\n"
                     "step 10\n"
                     "info worker worker-e2e\n"
+                    "info worker worker-e2e-pre\n"
                     "step 8\n"
                     "info worker worker-e2e\n"
+                    "info worker worker-e2e-pre\n"
                     "continue\n"
                     "info worker worker-e2e\n"
+                    "info worker worker-e2e-pre\n"
                     "quit\n");
 
     pid = fork();
@@ -1194,6 +1438,80 @@ static void test_e2e_original_motivation_long_replay(void)
         "ANCHOR_ID has been reconciled -- that would be bug-1613's exact "
         "pre-fix symptom");
 
+    /* ---- Observable #3 (Req 1.1-1.3, scheduler-snapshot-job-allocation
+     * task 4.2 -- see this file's ADDENDUM header comment): PREEXIST_ID's
+     * allocation is correct from the FIRST possible checkpoint (after just
+     * ONE replayed round), strictly earlier than ANCHOR_ID's own first
+     * checkpoint above, and stays correct through all 24 rounds. Unlike
+     * ANCHOR_ID, PREEXIST_ID's dispatch decision predates the recording
+     * entirely and can never be discovered by replay-based reconciliation
+     * (sim_reconcile_unaccounted_jobs()) -- its correctness here can ONLY
+     * come from xce_sched_core_load_snapshot()'s own allocation
+     * restoration (task 2.1's build_wire_job() save-side wiring + task
+     * 2.2's dup_job_alloc()/make_job_account() load-side wiring), proven
+     * concretely by the DISPATCH-never-appears guard below. Before this
+     * feature's fix, job->allocs/job->accounts were never restored at
+     * all (snapshot_wire.c's own former "not captured" comment,
+     * snapshot.c's former Scope Decisions bullet) -- this checkpoint
+     * would have read 0.00 at every single one of these five points, for
+     * the entire run, with no mechanism (not even reconciliation) able to
+     * ever recover it, since no DISPATCH decision for this job exists
+     * anywhere in the replay stream to reconcile against. */
+    q1 = strstr(captured, "used=9.00");
+    TEST_ASSERT_NOT_NULL_MESSAGE(
+        q1, "after just 1 round: PREEXIST_ID's 9.0-unit charge, restored "
+            "purely by loading the recording's own initial snapshot, must "
+            "already be visible -- no dispatch decision for this job "
+            "exists anywhere in the recording for reconciliation to ever "
+            "find");
+
+    q2 = strstr(q1, "used=9.00");
+    TEST_ASSERT_NOT_NULL_MESSAGE(
+        q2, "after 3 rounds: PREEXIST_ID's charge must still hold");
+
+    q3 = strstr(q2, "used=9.00");
+    TEST_ASSERT_NOT_NULL_MESSAGE(
+        q3, "after 13 rounds: PREEXIST_ID's charge must still hold");
+
+    q4 = strstr(q3, "used=9.00");
+    TEST_ASSERT_NOT_NULL_MESSAGE(
+        q4, "after 21 rounds: PREEXIST_ID's charge must still hold");
+
+    q5 = strstr(q4, "used=9.00");
+    TEST_ASSERT_NOT_NULL_MESSAGE(
+        q5, "after all 24 rounds: PREEXIST_ID's charge must still be "
+            "exactly 9.00 at the very end of the replay -- nothing more "
+            "and nothing less");
+
+    /* Concrete proof of mechanism: PREEXIST_ID's own DISPATCH decision
+     * must NEVER appear anywhere in the captured replay output -- its
+     * dispatch happened strictly before sched_record_start() was ever
+     * called, so nothing about it was ever written to the .replay file.
+     * If this string DID appear, it would mean the test's own prelude
+     * leaked into the recording (a test-authoring bug), not a genuine
+     * proof of the snapshot-restoration mechanism under test. */
+    fmt_jobid(preexist_jobid_str, sizeof(preexist_jobid_str), PREEXIST_ID);
+    snprintf(preexist_marker, sizeof(preexist_marker),
+             "[sim] DISPATCH jobid=%s", preexist_jobid_str);
+    TEST_ASSERT_NULL_MESSAGE(
+        strstr(captured, preexist_marker),
+        "PREEXIST_ID's dispatch predates the whole recording -- it must "
+        "NEVER appear as a DISPATCH decision anywhere in replay output; "
+        "its correct accounting comes solely from snapshot-load "
+        "restoration, not from any replay-discoverable decision");
+
+    /* Non-tautology guard, symmetric to ANCHOR_ID's own above: this is
+     * exactly the failure signature task 2.2's own load-side restoration
+     * (dup_job_alloc()/make_job_account() in xce_sched_core_load_
+     * snapshot()) would produce if it were reverted -- PREEXIST_ID's
+     * allocation predates the snapshot and has no other recovery path. */
+    TEST_ASSERT_NULL_MESSAGE(
+        strstr(captured, "resource slots: value=15.00 used=0.00"),
+        "worker-e2e-pre's slots resource must never read back to "
+        "used=0.00 -- PREEXIST_ID's allocation has no recovery path other "
+        "than snapshot-load restoration, since no DISPATCH decision for "
+        "it exists anywhere in the replay stream");
+
     free(captured);
 
     printf("test_e2e_original_motivation_long_replay: PASS\n");