feat(scheduler-core): pin jbuf capacity + record kvmgr determinism audit (task 4.2)

Hardcodes jbuf's cbuf capacity to the literal 1 in xce_sched_core_init()
(no parameter path, no local variable to drift), backed by a regression
test. Audits every kvmgr iteration reachable from the scheduling loop
(jobs/queues/users/projects): only jobs is ever iterated, via xzarray's
deterministic ascending-slot walk with no process-random seeding —
findings recorded durably as a comment above xce_sched_core_init().
core->workers iteration-order stability is flagged as an open follow-up
(scheduler-allocation's territory), not this task's scope.

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

src/scheduler/src/core.c

@@ -32,10 +32,56 @@
  * pointers themselves) lives in xce_sched_core_t.
  */
 
+/*
+ * Req 5 AC 7 (scheduler-core spec, task 4.2) -- kvmgr iteration-order audit,
+ * scope: jobs/queues/users/projects (design.md's "bounded scope" Risks note;
+ * core->workers is explicitly out of this audit's scope -- it belongs to
+ * scheduler-allocation's host-selection path, not this spec's scheduling
+ * loop). Recorded here, durably, rather than only in a task status report:
+ *
+ *   - queues_params/users_params/projects_params (below) back kvmgrs that are
+ *     NEVER iterated anywhere in src/scheduler -- only point key_add/key_find
+ *     lookups exist on core->queues/core->users/core->projects. There is
+ *     nothing to stabilize for these three: no iteration site, no order to
+ *     destabilize.
+ *   - jobs_params (below) backs the one kvmgr of the four that IS iterated,
+ *     at two call sites reachable from the scheduling loop: scheduler.c's
+ *     sched_account_jobs() (every session, from sched_begin_session()) and
+ *     sched_end_session()'s GC live-marking pass. Both go through
+ *     xkvmgr_type's .iter interface (src/lib/kvmgr.c, xkvmgr_make_iter/
+ *     xkvmgr_iter_next), which does NOT walk this struct's hash-bucket chain
+ *     at all -- it delegates to the underlying xzarray_type instance
+ *     (src/lib/zarray.c, xzarray_iter_next), a flat array walked in ascending
+ *     slot-index order, itself a deterministic function of insert/delete
+ *     history via a freed-slot reuse hint (xzarray_clear/xzarray_store) --
+ *     no randomness anywhere in that path.
+ *   - The hash-bucket path (src/lib/kv.c) is used only for O(1) key->index
+ *     lookup (key_add/key_find), never for iteration. It is independently
+ *     deterministic too: nslots is fixed at kvmgr creation with no
+ *     resize/rehash code path; hashing is djb2 (hash_string, fixed seed
+ *     5381) or a fixed Bob-Jenkins-style bit-mixer (hash_int_key), neither
+ *     taking any random/ASLR/pointer/time input; insertion is head-of-list,
+ *     deletion unlinks in place -- both pure functions of insert/delete
+ *     order. A grep of kv.c/kvmgr.c/zarray.c/utils.c for rand(/srand(/
+ *     arc4random turned up nothing in this path (the sole getpid() hit is an
+ *     unrelated tlog log-format helper).
+ *   - The picker's actual scheduling-OUTPUT order doesn't come from kvmgr
+ *     iteration at all: builtin_fcfs/fcfs.c maintain their own private ordq
+ *     structures with explicit comparators (fcfs_queue_cmp/fcfs_job_cmp --
+ *     priority desc, then queue_time asc), a fully specified deterministic
+ *     total order independent of any kvmgr/hash mechanism.
+ *
+ * Conclusion: no unstable site found among jobs/queues/users/projects; no
+ * fix required. (core->workers iteration IS order-sensitive for
+ * scheduler-allocation's first-fit host selection -- seqjob_allocator.c's
+ * sched_collect_workers()/sched_try_alloc_on_hosts() -- and remains an open
+ * item to close before Phase 3 simulation work depends on AC 7's full
+ * guarantee, per design.md's own scoping note; tracked as a follow-up, not
+ * this task's responsibility.)
+ */
 int xce_sched_core_init(xce_sched_core_t* core)
 {
     int ndecisions = 5; /* FIXME: make this configurable */
-    int njobs      = 1; /* FIXME: make this configurable */
 
     kvmgr_params_t plugins_params = { .key_type   = KVMGR_KEY_STRING,
                                       .nslots     = 16,
@@ -85,7 +131,22 @@ int xce_sched_core_init(xce_sched_core_t* core)
 
     mylock_init(&(core->lock));
     cbuf_init(&(core->wbuf), 1, sizeof(int));
-    cbuf_init(&(core->jbuf), njobs, sizeof(uint64_t));
+
+    /* Req 5 AC 7 (scheduler-core spec, task 4.2): jbuf capacity is a
+     * determinism invariant, not a tuning knob -- it is the rendezvous
+     * between the picker and allocator threads (the picker blocks on
+     * cbuf_put() until the allocator drains the current jobid), which is
+     * exactly what makes jobs flow through the pipeline one at a time in
+     * fully determined order. Raising this above 1 turns the rendezvous
+     * into a buffer, letting the picker enqueue faster than the allocator
+     * drains, which would make allocator-observed job order depend on
+     * scheduling/timing rather than on the event sequence alone.
+     * Hardcoded to the literal 1 here per requirements.md's explicit
+     * prohibition on exposing this as a driver- or config-supplied
+     * parameter -- do not reintroduce a local variable or any other
+     * indirection for this value without first re-deriving the
+     * determinism argument in AC 7. */
+    cbuf_init(&(core->jbuf), 1, sizeof(uint64_t));
     cbuf_init(&(core->decisions), ndecisions, sizeof(xce_decision_t*));
 
     core->resmgr = makeInstance(&xkvmgr_type, &resmgr_params);

src/scheduler/testsuite/meson.build

@@ -203,6 +203,28 @@ sched_core_isolation_test = executable(
              + '"'],
     install: false)
 
+# Regression guard for task 4.2's jbuf-capacity half (scheduler-core spec,
+# Req 5 AC 7): "cbuf capacity=1 serialization ... this capacity is a
+# determinism invariant, not a tuning knob." Same white-box #include
+# pattern as test_picker_config_fields.c (pulls the real
+# xce_sched_core_init()/xce_sched_core_fini() from ../src/core.c into this
+# TU) PLUS the source-text-scan genre used by test_picker_core_loop_boundary_test/
+# test_core_isolation_test above -- a real runtime call proving
+# core.jbuf.ncap == 1, and a scan of core.c's actual on-disk text proving
+# the cbuf_init() capacity argument is the literal 1 (never a variable),
+# that the former `njobs` local is gone, and that the separate/unrelated
+# `ndecisions` FIXME was left untouched. XCE_CORE_C_PATH reuses the same
+# absolute-path constant as sched_core_isolation_test above.
+sched_core_jbuf_capacity_pin_test = executable(
+    'sched_core_jbuf_capacity_pin_test',
+    ['test_core_jbuf_capacity_pin.c'],
+    include_directories:[incdir],
+    dependencies: [scheduler_test_deps],
+    c_args: ['-DXCE_CORE_C_PATH="'
+             + (meson.current_source_dir() / '..' / 'src' / 'core.c')
+             + '"'],
+    install: false)
+
 # Register as meson tests so `meson test --suite scheduler` runs them all.
 # detect_leaks=0 matches the suite convention (fixtures are not torn down).
 test('scheduler test', scheduler_test, suite: 'scheduler',
@@ -236,3 +258,6 @@ test('scheduler pendreason factory audit test',
 test('scheduler core isolation test',
      sched_core_isolation_test, suite: 'scheduler',
      env: ['ASAN_OPTIONS=detect_leaks=0'])
+test('scheduler core jbuf capacity pin test',
+     sched_core_jbuf_capacity_pin_test, suite: 'scheduler',
+     env: ['ASAN_OPTIONS=detect_leaks=0'])

src/scheduler/testsuite/test_core_jbuf_capacity_pin.c

@@ -0,0 +1,275 @@
+/*
+ * test_core_jbuf_capacity_pin.c - regression guard for the first half of
+ * task 4.2 (scheduler-core spec, Req 5 AC 7, "cbuf capacity=1
+ * serialization"): "Phase 2 shall either hardcode this capacity to 1
+ * inside xce_sched_core_init() or leave the njobs FIXME untouched; it
+ * shall not expose jbuf capacity as a driver- or config-supplied
+ * parameter."
+ *
+ * Before this task, core.c's xce_sched_core_init() read:
+ *     int njobs = 1; -- FIXME: make this configurable
+ *     ...
+ *     cbuf_init(&(core->jbuf), njobs, sizeof(uint64_t));
+ * -- a local variable, not a hardcoded literal. The value was already 1,
+ * so a purely behavioral check (does core.jbuf end up with capacity 1?)
+ * would already pass before this task and prove nothing about whether the
+ * capacity is *structurally* pinned or just happens to default to 1
+ * today. This task's Observable is about the latter: the value must be a
+ * literal that cannot be fed by any parameter, config field, or variable
+ * without editing this exact line and re-deriving the determinism
+ * argument in AC 7.
+ *
+ * Three checks, two genres (same combination test_core_isolation.c uses):
+ *
+ *   1. test_jbuf_capacity_is_runtime_one - the literal behavioral proof:
+ *      xce_sched_core_init() is pulled into this TU for real (white-box
+ *      #include, same convention as test_picker_config_fields.c) and
+ *      actually invoked; core.jbuf.ncap (mylock.h's cbuf_t.ncap field) must
+ *      read back as 1. This already passed before this task (the value was
+ *      already 1) -- included so a future change to the actual runtime
+ *      capacity is caught even if it doesn't touch this exact source line
+ *      (e.g. a change to cbuf_init() itself, or to mylock.h's cbuf_t
+ *      layout).
+ *
+ *   2. test_jbuf_capacity_is_hardcoded_literal_not_a_variable - RED before
+ *      this task (core.c's real on-disk text had `cbuf_init(&(core->jbuf),
+ *      njobs, sizeof(uint64_t))`, never the literal-1 call text asserted
+ *      here), GREEN after: reads core.c's actual on-disk source at test run
+ *      time (XCE_CORE_C_PATH, baked in by meson.build, same technique as
+ *      test_core_isolation.c / test_picker_core_loop_boundary.c) and
+ *      requires the exact hardcoded call text to appear verbatim in
+ *      xce_sched_core_init()'s body.
+ *
+ *   3. test_njobs_variable_removed_from_core_init - the mirror-image RED/
+ *      GREEN check: before this task, "njobs" appeared in
+ *      xce_sched_core_init()'s body (the local variable); this test asserts
+ *      it no longer appears at all, catching any future reintroduction of
+ *      an indirection (variable, parameter, config field) between the
+ *      literal 1 and the cbuf_init() call -- exactly the regression this
+ *      task's Observable rules out ("no code path to accept a
+ *      caller-supplied handoff-channel capacity").
+ *
+ * A fourth check guards scope discipline: this task explicitly must not
+ * touch the separate, unrelated `ndecisions` FIXME (a different tuning
+ * knob, out of Req 5 AC 7's boundary) -- test_ndecisions_fixme_left_untouched
+ * asserts that line is byte-for-byte unchanged.
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <unity/unity.h>
+
+#include "kvmgr.h"
+#include "scheduler.h"
+#include "xstring.h"
+
+#ifndef XCE_CORE_C_PATH
+#error                                                                         \
+    "XCE_CORE_C_PATH must be defined by meson.build to core.c's absolute path"
+#endif
+
+/* Pull the real xce_sched_core_init()/xce_sched_core_fini() under test into
+ * this TU (white-box #include, same convention as
+ * test_picker_config_fields.c's `#include "../src/core.c"`). */
+#include "../src/core.c"
+
+/* Stubs for scheduler-internal externs core.c references (as kvmgr/zarray
+ * datakiller callback addresses) but which are defined in schedobj.c, not
+ * linked into this TU -- identical rationale to test_picker_config_fields.c's
+ * plugin_unregister stub: xce_sched_core_init() only ever takes these
+ * functions' addresses (kvmgr_params_t.datakiller / zarray_item_killer_t),
+ * it never calls them in the path this test exercises (nothing is ever
+ * added to core.jobs/queues/workers/users/projects/plugins here), so a
+ * resolvable no-op is all the linker needs. */
+void kill_sched_job(void* arg) { (void)arg; }
+void kill_sched_queue(void* arg) { (void)arg; }
+void kill_sched_worker(void* arg) { (void)arg; }
+void kill_sched_user(void* arg) { (void)arg; }
+void kill_sched_project(void* arg) { (void)arg; }
+void kill_sched_resval(xce_val_t* val) { (void)val; }
+void plugin_unregister(void* arg) { (void)arg; }
+
+void setUp(void) { return; }
+void tearDown(void) { return; }
+
+/* ---------------------------------------------------------------------- */
+/* Behavioral check: real xce_sched_core_init() produces ncap == 1         */
+/* ---------------------------------------------------------------------- */
+
+static void test_jbuf_capacity_is_runtime_one(void)
+{
+    xce_sched_core_t core;
+    int              rc;
+
+    rc = xce_sched_core_init(&core);
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    TEST_ASSERT_EQUAL_INT(1, core.jbuf.ncap);
+
+    /* Deliberately no xce_sched_core_fini(&core) call here: fini()
+     * unconditionally dataq_fini()s core->pend_updates[0]/[1], which are
+     * populated by scheduler.c's sched_init() (calloc + dataq_init), never
+     * by xce_sched_core_init() itself (see core.c's file header comment) --
+     * calling fini() on a core built by this fixture alone would
+     * dereference the NULL those fields are left at by init()'s memset().
+     * Matches this directory's established "fixtures aren't torn down"
+     * convention (see test_picker_core_loop_boundary.c's header comment). */
+}
+
+/* ---------------------------------------------------------------------- */
+/* Source-text checks: the value is a literal, not fed by a variable      */
+/* ---------------------------------------------------------------------- */
+
+static char* read_whole_file(const char* path)
+{
+    FILE* f = fopen(path, "rb");
+    if (f == NULL) {
+        return NULL;
+    }
+
+    if (fseek(f, 0, SEEK_END) != 0) {
+        fclose(f);
+        return NULL;
+    }
+
+    long len = ftell(f);
+    if (len < 0) {
+        fclose(f);
+        return NULL;
+    }
+    rewind(f);
+
+    char* buf = (char*)malloc((size_t)len + 1);
+    if (buf == NULL) {
+        fclose(f);
+        return NULL;
+    }
+
+    size_t nread = fread(buf, 1, (size_t)len, f);
+    fclose(f);
+    buf[nread] = '\0';
+
+    return buf;
+}
+
+/* Anchor is the literal function signature text, unique to the real
+ * definition (same technique as test_picker_core_loop_boundary.c's
+ * FUNC_ANCHOR). */
+static const char* const FUNC_ANCHOR
+    = "int xce_sched_core_init(xce_sched_core_t* core)";
+
+/*
+ * extract_function_body - same brace-depth-counting extraction as
+ * test_picker_core_loop_boundary.c's helper of the same name.
+ */
+static char* extract_function_body(const char* text, const char* anchor)
+{
+    const char* sig = strstr(text, anchor);
+    if (sig == NULL) {
+        return NULL;
+    }
+
+    const char* brace = strchr(sig, '{');
+    if (brace == NULL) {
+        return NULL;
+    }
+
+    int         depth = 0;
+    const char* p     = brace;
+    do {
+        if (*p == '{') {
+            depth++;
+        } else if (*p == '}') {
+            depth--;
+        }
+        p++;
+    } while ((depth > 0) && (*p != '\0'));
+
+    if (depth != 0) {
+        return NULL; /* unbalanced -- EOF reached mid-function */
+    }
+
+    size_t len  = (size_t)(p - brace);
+    char*  body = (char*)malloc(len + 1);
+    if (body == NULL) {
+        return NULL;
+    }
+    memcpy(body, brace, len);
+    body[len] = '\0';
+
+    return body;
+}
+
+static char* load_core_init_body(void)
+{
+    char* file_text = read_whole_file(XCE_CORE_C_PATH);
+    TEST_ASSERT_NOT_NULL_MESSAGE(file_text,
+                                 "could not read core.c at XCE_CORE_C_PATH");
+
+    char* body = extract_function_body(file_text, FUNC_ANCHOR);
+    free(file_text);
+
+    TEST_ASSERT_NOT_NULL_MESSAGE(
+        body, "could not locate/extract xce_sched_core_init()'s body -- has "
+              "its signature changed? update FUNC_ANCHOR in this test if so");
+
+    return body;
+}
+
+/* RED before task 4.2 (real text was `cbuf_init(&(core->jbuf), njobs,
+ * sizeof(uint64_t))`), GREEN after. */
+static void test_jbuf_capacity_is_hardcoded_literal_not_a_variable(void)
+{
+    char* body = load_core_init_body();
+
+    TEST_ASSERT_NOT_NULL_MESSAGE(
+        strstr(body, "cbuf_init(&(core->jbuf), 1, sizeof(uint64_t))"),
+        "xce_sched_core_init() must hardcode jbuf's cbuf_init() capacity "
+        "argument to the literal 1 -- Req 5 AC 7 forbids feeding it from a "
+        "variable, parameter, or config field");
+
+    free(body);
+}
+
+/* RED before task 4.2 (the `njobs` local was present), GREEN after --
+ * catches any future reintroduction of an indirection between the literal
+ * 1 and the cbuf_init() call. */
+static void test_njobs_variable_removed_from_core_init(void)
+{
+    char* body = load_core_init_body();
+
+    TEST_ASSERT_NULL_MESSAGE(
+        strstr(body, "njobs"),
+        "xce_sched_core_init() must not declare/reference an `njobs` (or "
+        "similarly named) variable -- jbuf capacity is a determinism "
+        "invariant (Req 5 AC 7), pinned to a literal 1, never a variable");
+
+    free(body);
+}
+
+/* Scope guard: this task must not touch the separate, unrelated
+ * `ndecisions` FIXME (a different cbuf's capacity, out of Req 5 AC 7's
+ * boundary). */
+static void test_ndecisions_fixme_left_untouched(void)
+{
+    char* body = load_core_init_body();
+
+    TEST_ASSERT_NOT_NULL_MESSAGE(
+        strstr(body, "int ndecisions = 5; /* FIXME: make this configurable */"),
+        "the unrelated `ndecisions` FIXME (decisions cbuf capacity) must be "
+        "left exactly as-is -- it is a separate tuning knob, not covered by "
+        "Req 5 AC 7's jbuf pin");
+
+    free(body);
+}
+
+int main(void)
+{
+    UNITY_BEGIN();
+    RUN_TEST(test_jbuf_capacity_is_runtime_one);
+    RUN_TEST(test_jbuf_capacity_is_hardcoded_literal_not_a_variable);
+    RUN_TEST(test_njobs_variable_removed_from_core_init);
+    RUN_TEST(test_ndecisions_fixme_left_untouched);
+    return UNITY_END();
+}