test(scheduler-core): guard core scheduling loop against ordering-logic regressions (task 2.5)

Task 2.4 already made sched_job_picker() delegate entirely to the picker
interface with zero direct queue/job traversal, satisfying Req 1 AC 1/AC 7
as a byproduct. This adds a source-text regression test locking that
property in place: asserts sched_job_picker()'s body contains none of the
forbidden direct-traversal symbols and does contain the expected
picker-interface call sites.

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

@@ -60,7 +60,7 @@
   - _Requirements: 1.3, 1.4, 1.6, 2.5, 3.1, 3.5_
   - _Depends: 2.3, 1.3_
 
-- [ ] 2.5 Remove ordering logic from the core scheduling loop
+- [x] 2.5 Remove ordering logic from the core scheduling loop
   - The core loop calls only `picker->next_job()` to obtain the next schedulable job — no project/queue/job iteration or ordering logic remains in the loop itself
   - Observable: a code-level check confirms the scheduling loop function contains no direct queue/job traversal — all selection reads happen through the picker interface
   - _Requirements: 1.1, 1.7_

src/scheduler/testsuite/meson.build

@@ -134,6 +134,27 @@ sched_picker_wiring_test = executable('sched_picker_wiring_test',
                                       dependencies: [scheduler_test_deps],
                                       install: false)
 
+# Regression guard for the core scheduling loop boundary (scheduler-core
+# spec, task 2.5, Req 1 AC 1 / AC 7): "the core loop calls only
+# picker->next_job() ... no project/queue/job iteration or ordering logic
+# remains in the loop itself." Does NOT #include scheduler.c (that file's
+# external dependency surface is why test_picker_wiring.c's own header
+# comment already ruled out driving sched_job_picker() directly) -- instead
+# it reads scheduler.c's real on-disk text at test run time and mechanically
+# checks sched_job_picker()'s body for required picker-interface call sites
+# and absent direct-traversal symbols. XCE_SCHEDULER_C_PATH bakes in the
+# absolute path via meson.current_source_dir() so the check has no
+# dependency on the test's cwd at run time.
+sched_picker_core_loop_boundary_test = executable(
+    'sched_picker_core_loop_boundary_test',
+    ['test_picker_core_loop_boundary.c'],
+    include_directories:[incdir],
+    dependencies: [scheduler_test_deps],
+    c_args: ['-DXCE_SCHEDULER_C_PATH="'
+             + (meson.current_source_dir() / '..' / 'src' / 'scheduler.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',
@@ -158,3 +179,6 @@ test('scheduler picker fcfs session test', sched_picker_fcfs_session_test, suite
      env: ['ASAN_OPTIONS=detect_leaks=0'])
 test('scheduler picker wiring test', sched_picker_wiring_test, suite: 'scheduler',
      env: ['ASAN_OPTIONS=detect_leaks=0'])
+test('scheduler picker core loop boundary test',
+     sched_picker_core_loop_boundary_test, suite: 'scheduler',
+     env: ['ASAN_OPTIONS=detect_leaks=0'])

src/scheduler/testsuite/test_picker_core_loop_boundary.c

@@ -0,0 +1,240 @@
+/*
+ * test_picker_core_loop_boundary.c - regression guard for the core
+ * scheduling loop boundary (scheduler-core spec, task 2.5, Req 1 AC 1 /
+ * AC 7): "The Scheduler core loop shall call picker->next_job(picker) ...
+ * and shall not contain any logic about project iteration, queue
+ * iteration, or job ordering -- those concerns are entirely encapsulated
+ * inside the picker plugin."
+ *
+ * Task 2.5's own verification pass (independently re-reading scheduler.c
+ * after task 2.4's rewrite) found sched_job_picker() already satisfies
+ * this property as a byproduct of task 2.4's work: it resolves the
+ * configured picker via sched_resolve_job_picker(), then either delegates
+ * wholesale to a legacy job_elect_fn plugin (self-contained, unchanged) or
+ * runs a generic loop that reads job order exclusively through
+ * picker_iface->begin_session()/next_job()/end_session() -- zero direct
+ * sched->queues/sched->jobs traversal. This task's Observable is therefore
+ * the *regression guard* on an already-true property, not a behavior
+ * change: a code-level check that keeps this true if a future change
+ * reintroduces ordering logic into the core loop.
+ *
+ * Why a source-text scan and not a linked white-box test: test_picker_wiring.c
+ * (task 2.4)'s own header comment already established that sched_job_picker()
+ * cannot be tractably driven directly -- it is `static` in scheduler.c, whose
+ * external dependency surface (xcecli_ctx_*, sel_register_alloc_thread,
+ * resource.c's allocation path, decision.c's decision constructors, ...) is
+ * far larger than any other white-box test in this directory stubs, and
+ * spinning the real picker/allocator threads to exercise it live is exactly
+ * what that file's task chose not to do. This file instead reads the
+ * actual, current on-disk source of sched_job_picker() at test run time (an
+ * absolute path baked in by meson.build via XCE_SCHEDULER_C_PATH, so no cwd
+ * assumption is needed -- see meson.build's comment on this test target)
+ * and mechanically checks the function body: it must contain the
+ * picker-interface call sites the Observable requires, and must NOT
+ * contain any of the direct-traversal symbols that would indicate
+ * project/queue/job ordering logic crept back into the loop. This is a
+ * grep-based assertion, same genre as test_upgrade_snapshot_apply.c's
+ * popen+grep structural check (controller testsuite) -- here done via
+ * fopen+strstr instead of popen since no shell/glob is needed.
+ *
+ * RED demonstration performed manually during development (not committed):
+ * temporarily inserting a `for (job = sched->jobs.head; ...)`-style
+ * fragment into sched_job_picker()'s real body made
+ * test_core_loop_has_no_forbidden_traversal_symbols fail immediately with
+ * the exact forbidden-symbol message, confirming the check exercises live
+ * source text rather than a frozen snapshot -- then reverted.
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <unity/unity.h>
+
+#ifndef XCE_SCHEDULER_C_PATH
+#error                                                                         \
+    "XCE_SCHEDULER_C_PATH must be defined by meson.build to the absolute path of scheduler.c"
+#endif
+
+/* Anchor is the literal function signature text, unique to the real
+ * definition -- every comment reference to sched_job_picker elsewhere in
+ * this codebase omits the "(void* arg)" argument (verified via `grep -rn
+ * sched_job_picker src/scheduler`: only the definition itself carries this
+ * exact substring). If this signature is ever reformatted, this test fails
+ * loudly (extraction returns NULL) rather than silently passing on stale
+ * text. */
+static const char* const FUNC_ANCHOR
+    = "static void* sched_job_picker(void* arg)";
+
+/* Symbols whose presence in sched_job_picker()'s body would mean project/
+ * queue/job iteration or ordering logic leaked back into the core loop
+ * (Req 1 AC 1). None of these are legitimately needed by a function whose
+ * only jobs are: resolve the picker, delegate to job_elect_fn OR drive
+ * picker_iface's session hooks, and shuttle jobids across cbuf. */
+static const char* const FORBIDDEN_SYMBOLS[] = {
+    "sched->queues", "sched->jobs", "iter_make(", "iter_next(",
+    "ordq_get(",     "ordq_put(",   "ordq_peek(", "key_find(",
+};
+#define NUM_FORBIDDEN (sizeof(FORBIDDEN_SYMBOLS) / sizeof(FORBIDDEN_SYMBOLS[0]))
+
+/* Picker-interface call sites the Observable requires: "all selection
+ * reads happen through the picker interface." Both the legacy-delegation
+ * branch and the native session-loop branch must be present -- task 2.5
+ * did not collapse them (that would be a behavior change, out of scope;
+ * see this task's own "likely non-behavioral" framing). */
+static const char* const REQUIRED_SYMBOLS[] = {
+    "job_elect_fn(job_picker)",
+    "picker_iface->begin_session(job_picker)",
+    "picker_iface->next_job(job_picker)",
+    "picker_iface->end_session(job_picker)",
+};
+#define NUM_REQUIRED (sizeof(REQUIRED_SYMBOLS) / sizeof(REQUIRED_SYMBOLS[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;
+}
+
+/*
+ * extract_function_body - returns a newly malloc'd, NUL-terminated copy of
+ * the {...} body of the function whose signature text is `anchor` within
+ * `text` (brace-depth counting from the first '{' following the anchor).
+ * Returns NULL if the anchor isn't found or braces never balance before
+ * EOF -- the caller must treat NULL as a hard test failure, not "skip".
+ */
+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;
+}
+
+/* Loads scheduler.c and extracts sched_job_picker()'s body fresh for each
+ * test (no shared/torn-down fixture state -- matches this directory's
+ * "fixtures aren't torn down" convention, but here each test cleans up its
+ * own two allocations at the end anyway since there's no ASAN leak check
+ * reason not to). */
+static char* load_core_loop_body(void)
+{
+    char* file_text = read_whole_file(XCE_SCHEDULER_C_PATH);
+    TEST_ASSERT_NOT_NULL_MESSAGE(
+        file_text, "could not read scheduler.c at XCE_SCHEDULER_C_PATH");
+
+    char* body = extract_function_body(file_text, FUNC_ANCHOR);
+    free(file_text);
+
+    TEST_ASSERT_NOT_NULL_MESSAGE(
+        body, "could not locate/extract sched_job_picker()'s body -- has its "
+              "signature changed? update FUNC_ANCHOR in this test if so");
+
+    return body;
+}
+
+/* Req 1 AC 1: "shall not contain any logic about project iteration, queue
+ * iteration, or job ordering". */
+static void test_core_loop_has_no_forbidden_traversal_symbols(void)
+{
+    char* body = load_core_loop_body();
+
+    for (size_t i = 0; i < NUM_FORBIDDEN; i++) {
+        char msg[256];
+        snprintf(msg, sizeof(msg),
+                 "sched_job_picker() body contains forbidden direct-"
+                 "traversal symbol '%s' -- ordering logic has leaked back "
+                 "into the core scheduling loop (Req 1 AC 1)",
+                 FORBIDDEN_SYMBOLS[i]);
+        TEST_ASSERT_NULL_MESSAGE(strstr(body, FORBIDDEN_SYMBOLS[i]), msg);
+    }
+
+    free(body);
+}
+
+/* Req 1 AC 1 / Req 1 AC 4: "the core scheduling loop shall call
+ * picker->next_job(picker) ... all selection reads happen through the
+ * picker interface" -- the positive-side check, so a change that silently
+ * dropped a picker-interface call site (e.g. hardcoding a job again, or
+ * skipping begin_session/end_session) fails loud even though it wouldn't
+ * trip the forbidden-symbol list above. */
+static void test_core_loop_calls_only_through_picker_interface(void)
+{
+    char* body = load_core_loop_body();
+
+    for (size_t i = 0; i < NUM_REQUIRED; i++) {
+        char msg[256];
+        snprintf(msg, sizeof(msg),
+                 "sched_job_picker() body is missing required picker-"
+                 "interface call site '%s'",
+                 REQUIRED_SYMBOLS[i]);
+        TEST_ASSERT_NOT_NULL_MESSAGE(strstr(body, REQUIRED_SYMBOLS[i]), msg);
+    }
+
+    free(body);
+}
+
+void setUp(void) { return; }
+void tearDown(void) { return; }
+
+int main(void)
+{
+    UNITY_BEGIN();
+    RUN_TEST(test_core_loop_has_no_forbidden_traversal_symbols);
+    RUN_TEST(test_core_loop_calls_only_through_picker_interface);
+    return UNITY_END();
+}