test(scheduler-core): audit data-sharing invariants across the picker rework (task 2.6)

Closes out group 2 (Core: Builtin FCFS Picker). Confirms builtin_fcfs's
ordering matches legacy fcfs_elect_job (byte-identical comparators, same
sched_queue_is_ready/sched_job_is_ready gates, provably equivalent jobid
sequence), confirms tasks 1.1-2.5 left every pre-existing test file
unmodified, and confirms resreq immutability, pend-reason factory-only
allocation, and select_eval_ctx_t memoization all hold across every file
this spec has touched so far. Adds a durable regression lock for the
pend-reason factory invariant (no direct calloc(pendreason_t) outside
make_pendreason()).

.kiro/specs/scheduler-core/tasks.md

@@ -66,7 +66,7 @@
   - _Requirements: 1.1, 1.7_
   - _Depends: 2.4_
 
-- [ ] 2.6 Picker regression tests and data-sharing invariant checks
+- [x] 2.6 Picker regression tests and data-sharing invariant checks
   - Assert `builtin_fcfs`'s `next_job()` ordering matches today's pre-change FCFS ordering exactly, given identical queue/job state
   - Confirm the existing scheduler unit test suite passes unmodified except at the specific assertions that exercised the removed hardcoded string and the decision-publishing path this feature replaces
   - Verify resreq objects are never mutated in place by any touched code path, all pend-reason allocation goes through the existing factory function (no direct `calloc` of the reason struct), comparisons use pointer equality as a fast path before deep comparison, and the existing per-session select-expression memoization is untouched

src/scheduler/testsuite/meson.build

@@ -155,6 +155,26 @@ sched_picker_core_loop_boundary_test = executable(
              + '"'],
     install: false)
 
+# Regression guard for Req 6 AC 2 (scheduler-core spec, task 2.6): "All pend
+# reason allocation shall go through make_pendreason() (lib/preason.c).
+# Callers never calloc(sizeof(xce_pendreason_t)) directly." Same genre as
+# sched_picker_core_loop_boundary_test above (reads real on-disk source at
+# test run time rather than linking the huge dependency surface a live
+# scheduler pulls in), but scans every non-testsuite .c/.h file under
+# src/scheduler for the forbidden pattern rather than one function's body.
+# XCE_SCHEDULER_ROOT_PATH bakes in src/scheduler's absolute path;
+# the scanned file list itself lives in the test source (see that file's
+# header comment for why a fixed list, not a directory walk).
+sched_pendreason_factory_audit_test = executable(
+    'sched_pendreason_factory_audit_test',
+    ['test_picker_pendreason_factory_audit.c'],
+    include_directories:[incdir],
+    dependencies: [scheduler_test_deps],
+    c_args: ['-DXCE_SCHEDULER_ROOT_PATH="'
+             + (meson.current_source_dir() / '..')
+             + '"'],
+    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',
@@ -182,3 +202,6 @@ test('scheduler picker wiring test', sched_picker_wiring_test, suite: 'scheduler
 test('scheduler picker core loop boundary test',
      sched_picker_core_loop_boundary_test, suite: 'scheduler',
      env: ['ASAN_OPTIONS=detect_leaks=0'])
+test('scheduler pendreason factory audit test',
+     sched_pendreason_factory_audit_test, suite: 'scheduler',
+     env: ['ASAN_OPTIONS=detect_leaks=0'])

src/scheduler/testsuite/test_picker_pendreason_factory_audit.c

@@ -0,0 +1,239 @@
+/*
+ * test_picker_pendreason_factory_audit.c - regression guard for Req 6 AC 2
+ * (scheduler-core spec, task 2.6): "All pend reason allocation shall go
+ * through make_pendreason() (the single factory in lib/preason.c). Callers
+ * never calloc(sizeof(xce_pendreason_t)) directly."
+ *
+ * Task 2.6 is primarily an audit, not new behavior. A one-off `grep -rn`
+ * during that audit confirmed the invariant currently holds (the only
+ * calloc() constructing an xce_pendreason_t anywhere in src/ is
+ * lib/preason.c:164, inside make_pendreason() itself, which lives outside
+ * src/scheduler entirely). This file turns that one-off finding into a
+ * durable, re-runnable check per the task's Observable ("a grep-based or
+ * code-review check confirms no new calloc(sizeof(...pendreason...)) call
+ * site was introduced") -- a future task that adds a picker, allocator, or
+ * handler path under src/scheduler and forgets make_pendreason() fails this
+ * test immediately instead of relying on someone re-running the grep by
+ * hand.
+ *
+ * Why a same-line check, not whole-file substring co-occurrence: verified
+ * during audit that src/decision.c and src/scheduler.c each contain BOTH
+ * "calloc(" and "pendreason_t" somewhere in the file, but never on the same
+ * line (decision.c:6 declares a `xce_pendreason_t* reason` parameter,
+ * decision.c:10/24 calloc a *different* struct, `xce_decision_t`;
+ * scheduler.c:350 reads an existing `job->schedobj->reason` pointer,
+ * scheduler.c's five calloc() calls all construct other struct types). A
+ * whole-file check would false-positive on both files. Flagging only a
+ * line that contains both substrings together is the same signal a human
+ * reviewer eyeballing `grep -n calloc` output would use, and matches every
+ * real violation shape this Observable cares about: `(xce_pendreason_t*)
+ * calloc(...)` or `calloc(1, sizeof(xce_pendreason_t))` collapse the type
+ * name and the allocation call onto one line by construction.
+ *
+ * Why a fixed file list, not a directory walk: mirrors
+ * test_picker_core_loop_boundary.c's own acknowledged limitation (a single
+ * baked-in function-signature anchor, not a fully generic scan) rather than
+ * adding a recursive-directory-walk dependency (nftw/opendir) for a
+ * 14-file, rarely-changing tree. SCANNED_RELATIVE_PATHS below is every
+ * non-testsuite .c/.h file under src/scheduler as of task 2.6 (verified via
+ * `find src/scheduler -name '*.c' -o -name '*.h' | grep -v testsuite`) --
+ * add new files here as they're created, the same maintenance burden
+ * test_picker_core_loop_boundary.c's FUNC_ANCHOR already carries.
+ *
+ * make_pendreason() itself is never allowlisted here because it lives in
+ * lib/preason.c, outside XCE_SCHEDULER_ROOT_PATH entirely -- the scanned
+ * tree structurally cannot see the one legitimate call site.
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <unity/unity.h>
+
+#ifndef XCE_SCHEDULER_ROOT_PATH
+#error                                                                         \
+    "XCE_SCHEDULER_ROOT_PATH must be defined by meson.build to the absolute path of src/scheduler"
+#endif
+
+static const char* const SCANNED_RELATIVE_PATHS[] = {
+    "include/decision.h",
+    "include/plugin.h",
+    "include/scheduler.h",
+    "include/sched/types.h",
+    "src/decision.c",
+    "src/handler.c",
+    "src/main.c",
+    "src/plugin.c",
+    "src/plugins/builtin_fcfs.c",
+    "src/plugins/fcfs.c",
+    "src/plugins/limit.c",
+    "src/plugins/parjob_allocator.c",
+    "src/plugins/seqjob_allocator.c",
+    "src/resource.c",
+    "src/schedobj.c",
+    "src/scheduler.c",
+};
+#define NUM_SCANNED                                                            \
+    (sizeof(SCANNED_RELATIVE_PATHS) / sizeof(SCANNED_RELATIVE_PATHS[0]))
+
+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;
+}
+
+/* The detector: a single source line that mentions both "calloc(" and
+ * "pendreason_t" is exactly the shape of a caller constructing an
+ * xce_pendreason_t (or a pointer to one) via direct calloc() rather than
+ * make_pendreason(). Operates on one already-NUL-terminated line (no
+ * embedded '\n'), never on whole-file text -- see file header for why. */
+static int line_has_violation(const char* line)
+{
+    return (strstr(line, "calloc(") != NULL)
+        && (strstr(line, "pendreason_t") != NULL);
+}
+
+/* Scans `path` line by line. Returns 1 if some line violates (and copies
+ * that line into violating_line_out for the failure message), 0 if clean,
+ * -1 if the file could not be read at all (caller must treat -1 as a hard
+ * failure, not "skip" -- an unreadable path means this whole test is
+ * silently checking nothing). */
+static int scan_file_for_violation(const char* path, char* violating_line_out,
+                                   size_t out_sz)
+{
+    char* text    = read_whole_file(path);
+    char* saveptr = NULL;
+    char* line;
+    int   found = 0;
+
+    if (text == NULL) {
+        return -1;
+    }
+
+    line = strtok_r(text, "\n", &saveptr);
+    while (line != NULL) {
+        if (line_has_violation(line)) {
+            found = 1;
+            if ((violating_line_out != NULL) && (out_sz > 0)) {
+                snprintf(violating_line_out, out_sz, "%s", line);
+            }
+            break;
+        }
+        line = strtok_r(NULL, "\n", &saveptr);
+    }
+
+    free(text);
+    return found;
+}
+
+void setUp(void) { return; }
+void tearDown(void) { return; }
+
+/* Sanity check for the audit itself: every baked-in path must actually
+ * open. Without this, a stale/typo'd relative path would make
+ * scan_file_for_violation() silently treat a missing file as "clean" (0
+ * lines scanned, 0 violations found) -- the exact false-confidence failure
+ * mode this whole test exists to avoid for the real invariant. */
+static void test_all_scanned_paths_are_readable(void)
+{
+    size_t i;
+
+    for (i = 0; i < NUM_SCANNED; i++) {
+        char  path[4096];
+        char* text;
+
+        snprintf(path, sizeof(path), "%s/%s", XCE_SCHEDULER_ROOT_PATH,
+                 SCANNED_RELATIVE_PATHS[i]);
+
+        text = read_whole_file(path);
+        TEST_ASSERT_NOT_NULL_MESSAGE(text, path);
+        free(text);
+    }
+}
+
+/* Proves the detector can actually fire, on synthetic text -- not just that
+ * it stays quiet on today's real files. Without this, a typo in
+ * line_has_violation() (e.g. a misspelled substring) could make every real
+ * scan vacuously pass forever. Mirrors the spirit of
+ * test_picker_core_loop_boundary.c's manual RED demonstration, but as a
+ * committed, permanently-runnable test of the detector function itself
+ * rather than an uncommitted one-time manual edit. */
+static void test_detector_fires_on_same_line_and_not_on_split_lines(void)
+{
+    TEST_ASSERT_TRUE(line_has_violation(
+        "    r = (xce_pendreason_t*)calloc(1, sizeof(*r));"));
+    TEST_ASSERT_TRUE(
+        line_has_violation("    calloc(1, sizeof(xce_pendreason_t));"));
+
+    /* same two substrings, but never on one line -- must NOT fire (the
+     * exact decision.c/scheduler.c shape found during the audit). */
+    TEST_ASSERT_FALSE(line_has_violation("xce_pendreason_t* reason;"));
+    TEST_ASSERT_FALSE(line_has_violation(
+        "    thing = (xce_decision_t*)calloc(1, sizeof(*thing));"));
+}
+
+/* Req 6 AC 2's actual regression lock: no scanned file contains a line
+ * constructing an xce_pendreason_t via direct calloc(). */
+static void test_no_direct_pendreason_calloc_in_scheduler_tree(void)
+{
+    size_t i;
+
+    for (i = 0; i < NUM_SCANNED; i++) {
+        char path[4096];
+        char violating_line[512];
+        char msg[4096 + 512 + 256];
+        int  rc;
+
+        snprintf(path, sizeof(path), "%s/%s", XCE_SCHEDULER_ROOT_PATH,
+                 SCANNED_RELATIVE_PATHS[i]);
+
+        rc = scan_file_for_violation(path, violating_line,
+                                     sizeof(violating_line));
+
+        TEST_ASSERT_NOT_EQUAL_MESSAGE(-1, rc, path);
+
+        if (rc == 1) {
+            snprintf(msg, sizeof(msg),
+                     "%s contains a direct xce_pendreason_t calloc() -- "
+                     "Req 6 AC 2 requires make_pendreason() (lib/preason.c) "
+                     "as the sole factory. Offending line: %s",
+                     path, violating_line);
+            TEST_FAIL_MESSAGE(msg);
+        }
+    }
+}
+
+int main(void)
+{
+    UNITY_BEGIN();
+    RUN_TEST(test_all_scanned_paths_are_readable);
+    RUN_TEST(test_detector_fires_on_same_line_and_not_on_split_lines);
+    RUN_TEST(test_no_direct_pendreason_calloc_in_scheduler_tree);
+    return UNITY_END();
+}