chore(wolf): sync anatomy/cerebrum/memory/hooks/token-ledger bookkeeping
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
This commit is contained in:
parent
0c736c795b
commit
5573ae22c1
5 changed files with 80291 additions and 738 deletions
|
|
@ -62,6 +62,8 @@
|
|||
|
||||
- **Sandbox capability update (2026-08-19): `sudo` now works passwordless here** (`sudo -n true` succeeds) — this unblocks anything previously gated on "this session/skill cannot sudo" (e.g. real `xcecd`, `worker/src/main.c`'s `check_user()`). Still true, and load-bearing for anything assuming a real production host: this sandbox has **no systemd, no dbus, no `/run` at all** (`ls /run` → No such file or directory; PID 1 is `s6-svscan`; no `dbus-daemon`/`systemctl` even under `nix develop`). Root access and a real init/dbus stack are independent axes — don't assume sudo working means systemd-dependent things (job dispatch via `org.freedesktop.systemd1.Manager.StartTransientUnit`, anything needing `/run/dbus/system_bus_socket`) will also work. `/run/systemd/system` existing is the standard `sd_booted(3)`-style signal for "a real systemd instance is running as this host's manager" — used in `src/tools/deploy-ha.sh` to auto-pick `xcecd` (real) vs `xcecd_virtual` per host (`XCE_REAL_WORKER=0|1` overrides).
|
||||
- **A root-owned file created by a daemon under `sudo` is not automatically readable by the invoking non-root user** — confirmed live (bug-1592): real `xcecd`'s main log file comes out `root:root` mode `640` even though the SAME binary explicitly `chmod(..., 0666)`s its domain socket and its `xcecd_worker.log` sibling. Don't assume "the daemon already chmod's some of its output files permissive" generalizes to all of them — check each file's actual mode after a live run before writing a verification command that reads it as the invoking user; prefix with `sudo` where it doesn't.
|
||||
- **The "rule out a concurrent competing build/soak" caution (line 16 above) includes SIBLING AGENTS in your own Claude Code session, not just unrelated external processes** (2026-08-23, scheduler-dispatch-commit task 3.1 GREEN verification). A peer subagent within the same session independently launched its own `xce-build.sh controller asan xce.build.green31 --incremental` + `meson test` against the exact build dir this session was told to use exclusively, overwriting the live `meson-logs/testlog.txt` in place mid-investigation. Identify this via `ps -o pid,ppid,cmd` to find the process tree, then `tr '\0' '\n' < /proc/<pid>/environ | grep CLAUDE_CODE_SESSION_ID` — a match against your own session's ID (visible in your own scratchpad path) confirms it's a sibling agent, not an unrelated build. Mitigation: copy/preserve any log file you need as evidence (e.g. `sed`/`python3`-clean it into your scratchpad) the MOMENT you capture it, before doing further slow diagnostic work that gives a peer time to overwrite the live file in place.
|
||||
- **`controller_unit_test` TIMEOUT (300s SIGTERM, `Ok:4 Fail:0 Timeout:1`) is bug-1429, a pre-existing, NOT-yet-root-caused, systemic hang** — now reproduced 3 times across 3 completely unrelated diffs (delaymgr/persist.c changes; a pure version-string change; scheduler-dispatch-commit's job.c change) and 3 different hang locations each time (test_demotion_replay_yields_committed_only; the test_configack_* group; test_jobsubmit_repeated_truncation_forced_failures_no_phantom_jobs). Before suspecting your own diff caused a hang here: (1) check whether the hanging test is even in a file your diff touched — if not, that's strong evidence against causation; (2) `/proc/<pid>/task/*/wchan` on the hung process (gdb attach is blocked by sandbox ptrace restrictions) — both/all threads in `futex_do_wait` confirms deadlock-consistent, not CPU-bound; (3) re-run the already-built binary directly (bypassing meson's 300s cap) with a generous `timeout` — if it reproduces in total isolation with zero concurrent activity, that rules out resource contention as the cause too. Update bug-1429's `occurrences`/`last_seen`/`fix` narrative rather than filing a new bug — do not conclude "regression in my diff" from this signature alone.
|
||||
|
||||
- **xce_resource_t (xce/types.h) has a `flags` bitmask (XCE_RESF_BUILTIN/DYNAMIC/GLOBAL/SHARED/EXTERNAL/RELEASE/DEFAULT_HOST/CONSUMABLE/LIMIT) distinguishing a genuinely cluster-wide resource (GLOBAL/SHARED, stored ONCE in `core->sharedInst`, a zarray_t indexed by the resource's `idx` -- the same pool RESVALS_FULL/RESVALS_DELTA event handling and snapshot save/load already treat as authoritative) from a host-based/per-worker-distributed resource (stored per-worker in `w->resvals`/`w->resInUse`).** Before this session, `repl_info_resource_one()` (scheduler REPL) conflated the two -- aggregated total/available across every worker's resvals for EVERY resource regardless of kind, a fabricated number for a host-based resource (cpus/mem) and redundant with `info worker`'s own correct per-host display. When adding any new resource-value display/log code, check `r->flags & (XCE_RESF_GLOBAL|XCE_RESF_SHARED)` first to decide which pool (`core->sharedInst` vs per-worker `resvals`) is the correct source of truth -- see bug-1593.
|
||||
- **`repl_info.c` (scheduler REPL state-inspection dumps) was already OVER this project's 800-line cap (883 lines) before this session even started** -- accumulated from three prior sessions' bug fixes (bug-1587/1590/1591) each adding a little without ever triggering a new sibling-file split. A task-12.5 rewrite of the resource-display functions brought it down to 856, still over. Flag this to the user rather than silently ignoring it or unilaterally doing a 4th-generation file split (repl.c -> repl_break.c/repl_info.c -> repl_params.c precedent) without being asked -- a split touches meson.build, test_sim_isolation.c's per-file scan macros, AND every white-box test TU's #include list (real, mechanical work, not worth taking on speculatively).
|
||||
|
|
@ -84,11 +86,36 @@
|
|||
- **Live smoke-testing apiserver's `-version`/startup-log output (build-time-versioning task 4.4) needs a real topdir, but a full `xce-deploy-ha` deployment is overkill.** Recipe: `mkdir -p $TOPDIR/config $TOPDIR/logs` then copy the four shipped templates `src/frontend/apiserver/config/{app,logger,auth,rbac}.yaml` into `$TOPDIR/config/` verbatim, then `timeout 10 xce_apiserver -d $TOPDIR`. The shipped `app.yaml` template has a live `backend: {address: localhost, port: 9901}` block with no controller actually running there, so the process logs `Attempting backend connection` → `WARN All configured backend candidates failed` → `ERROR Failed to connect to backend` and exits 1 within ~10ms — this is fine and expected for verifying only the startup log's FIRST lines (logger init, before backend connect is even attempted); no live controller needed. Log lands at `$TOPDIR/logs/apiserver_out.log` (from `logger.yaml`'s `outputPath: apiserver_out.log`, joined with `$TOPDIR/logs/` by `InitLogger()`). Separately, `xce_apiserver -version` (no `-d` at all) exits 0 with zero filesystem side effects (no `logs/` dir, no config read) — confirms the `-version` early-exit genuinely runs before `PromptForTopDir()`/`InitLogger()`, not just before the visible log line.
|
||||
- **This codebase's apiserver Go code has ONE idiomatic zap-logging style: structured fields via `zap.String(...)`/`zap.Error(...)` appended to the `logger.Info/Error/Debug/Warn(msg, fields...)` call, never Sprintf-style interpolated messages** — confirmed by grepping every `logger.Info/Error/Debug/Warn` call site across `controllers/`, `middleware/`, `router/` (files.go, auth.go, rbac_check.go, validatehandler.go, routers.go, permissions.go all exclusively use this pattern). Even though `encoding: console` is configured in `logger.yaml`, the fields still render as trailing JSON on the log line (already documented above) — that's fine, it's still the right convention to follow for any new log call site in this package, don't reach for `fmt.Sprintf` into the message string instead.
|
||||
- **`gofmt -l`/`gofmt -d` on `apiserver/cmd/main.go` reports a PRE-EXISTING import-ordering violation (`_ "apiserver/docs"` sorted before `"apiserver/backend"` instead of after, alphabetically) that predates build-time-versioning task 4.4 and is unrelated to it** — confirmed via `git show HEAD:.../main.go | gofmt -l -` on the pre-edit blob, same violation. Don't "fix" it as a drive-by inside a task whose boundary is a single file with no mandate to touch unrelated pre-existing formatting — flag it, don't silently reformat, since an unrelated reformat in the same diff makes the real change harder to review (mirrors the existing Do-Not-Repeat entry about not running `clang-format -i` on whole existing C files).
|
||||
- **scheduler-sim-decision-parity task 1.2 (2026-08-22): white-box-including `resource.c` alongside the FULL driver+sim+recording stack (not just `core.c`, unlike test_resource_accounting.c's own deliberately-narrower pairing) works and needs exactly one NEW stub beyond what the driver/sim stack already required: `sched_ctx_get_uncomit_host()`** (resource.c's `sched_try_alloc_on_host_()`'s own link-time dependency, never actually called when the test only drives the public `sched_check_host_satisfy()`/`sched_charge_account()`/`sched_commit_job_alloc()` entry points directly). Two stubs test_resource_accounting.c's OWN narrower resource.c-only pairing needs (`plugin_prepare_allocation`, `sched_fill_limit_requests`) must NOT be redefined once `plugin.c` is also in the TU (test_sim_equivalence.c's set already includes it) — `plugin.c` provides a REAL `plugin_prepare_allocation`, and `resource.c` itself defines the REAL `sched_fill_limit_requests` (not test_resource_accounting.c's stub version) — redefining either is a duplicate-symbol link error, not a harmless no-op. Also: `sched_worker_release_host_ctx` must NOT be stubbed once resource.c is included (every OTHER file in testsuite/ stubs it because none of them pull in resource.c, which defines it for real).
|
||||
- **A genuine, non-fabricated "capacity denied" fixture is achievable without driving scheduler.c's real `sched_allocator()`/resreq-DSL/plugin machinery at all**: a test-owned "stand-in allocator" thread (the SAME established pattern test_sim_equivalence.c already uses to replace `sched_allocator()`, there for the different reason of avoiding real allocation entirely) can instead call the REAL `sched_check_host_satisfy()`/`sched_charge_account()`/`sched_commit_job_alloc()` (resource.c) directly against a real `xce_sched_worker_t` with a hand-set finite `resvals` capacity (added via `sched_add_worker()`, `put_resvals()` — same residx-direct-bypass-of-resmgr convention `test_resource_accounting.c`'s `make_test_account()` already established) and a fixed per-job "wanted" lookup table standing in for resreq-DSL compilation. This produces genuinely-arithmetic-derived DISPATCH/PEND decisions (not fabricated) while staying at the same "don't pull in the untested resreq-DSL/plugin layer" scope every other white-box test in this directory already commits to. `sched_check_host_satisfy(xce_scheduler_t*, ...)` itself was NOT relocated by task 1.1 (only the accounting/charge functions were) — it still needs a real `xce_scheduler_t*`, not just a core, because `unsatisfy_()`'s TRACE line reads `sched->core.resmgr` (harmless when resmgr has no matching entry: the output pointer is pre-`NULL`'d and the TRACE format string degrades to `"?"`, no crash).
|
||||
- **scheduler-sim-decision-parity task 2.2 (2026-08-22): a new `static` function added to `sim_loop.c` but not yet wired into any call site (by design -- the wiring is a LATER task) triggers `-Wunused-function` at production-object compile time (`ninja scheduler/xce_scheduler.p/src_sim_loop.c.o`), but this project has no `-Werror`/`warning_level` override in `src/meson.build` (default meson `warning_level=1`, no werror), so it's a harmless warning, not a build failure -- confirmed by building that exact object directly. Don't mistake this for a real problem needing an early premature call site or a `(void)` no-op reference; just note it as an expected, temporary CONCERNS item until the wiring task lands. Also confirmed the standard "RED phase for a brand-new function with no prior runtime behavior" technique: `cp` the modified source aside, `git checkout -- <file>` to revert to HEAD (pre-implementation), rebuild+run the new test to capture a genuine `implicit declaration of function` compile-time RED failure, then restore the saved copy -- cleaner than hand-editing the diff back out and back in, and git-safe (never touches the actual uncommitted work beyond a temporary revert/restore pair on one file).
|
||||
- **scheduler-sim-decision-parity task 2.3 (2026-08-22): to prove an ASAN "no leak" claim for a white-box test in `src/scheduler/testsuite/`, do NOT trust the meson-registered test run alone** — every test target in this directory is registered with `env: ['ASAN_OPTIONS=detect_leaks=0']` (meson.build) because this whole test family deliberately never tears down its `xce_sched_core_t`/fixture allocations (documented file-header convention). Re-running the standalone binary with default `ASAN_OPTIONS` (leak detection ON) is the only way to actually see leak provenance, but doing so SWALLOWS Unity's own PASS/FAIL summary from stdout entirely (LeakSanitizer's report prints, then the process exits via a path that never flushes the earlier buffered Unity output) — so run it TWICE: once as `ASAN_OPTIONS=detect_leaks=0 ./binary` for Unity's real pass/fail summary (matches the project's own convention/exit code), once as a bare `./binary` (or `ASAN_OPTIONS=detect_leaks=1`) purely to inspect leak stack traces. When triaging those traces, filter for ones whose stack passes through the code you actually changed (`grep -B3 '<your_function>' `) — the other 100+ leak reports are the SAME pre-existing "fixture not torn down" noise on every test in the file (kvmgr/groupmgr/select-parser allocations from `xce_sched_core_init()`), not signal.
|
||||
- **`free_host_account()` (`src/lib/misc.c`) never frees `acct->host`** (the `strdup(halloc->host)` `make_host_account()` sets, `xcealloc.c:119`) — confirmed via the above ASAN leak-provenance technique while verifying task 2.3's reconcile-then-release round trip. Pre-existing, NOT scheduler-sim-decision-parity-specific: equally leaks on the LIVE `scheduler.c` dispatch path (`sched_commit_decision()` → `make_job_account()` → same `free_job_account()`/`free_host_account()` release chain), previously masked only by this test suite's `detect_leaks=0` convention. Logged as bug-1621, not fixed (out of boundary for task 2.3 — `misc.c` wasn't in that task's file set). Correct fix if picked up later: add `FREEUP(acct->host);` to `free_host_account()`, mirroring `free_res_account()`'s adjacent `FREEUP(acct->resname);`.
|
||||
- **scheduler-sim-decision-parity task 3 (2026-08-22): `resource.c`'s `get_resvals(zarray_t*, int nres, int*, float*)` returns `nres` on success (not 0)** — confirmed by reading its body (`return nres;` at the very end). Bit a new end-to-end test (`test_sim_round_boundary_reconciliation.c`) that wrongly asserted `TEST_ASSERT_EQUAL_INT(0, rc)` after a 1-resource query; briefly looked like the task-3 wiring hadn't worked (GREEN rebuild still failed) before the real cause (a test-authoring mistake, not a reconciliation bug) was found. Logged as bug-1622. Lesson: don't assume this codebase's near-universal "0/negative" return convention applies uniformly -- `get_resvals()` is a documented exception, check the actual body.
|
||||
- **scheduler-sim-decision-parity task 3 (2026-08-22): to exercise Recorded-Decision Reconciliation's REAL `make_job_account()`/`make_res_account()` (xcealloc.c) path end-to-end — as opposed to every existing resource-contention-style test in this directory, which hand-builds `xce_job_account_t` with `residx` already resolved, deliberately bypassing this — you must handle that `make_res_account()` (xcealloc.c) always sets `residx = -1`, forcing `sched_update_res_accounts()` (resource.c) to LAZILY resolve `resname` -> `residx` via `key_find(core->resmgr, resname, ...)` at charge time (falling back to auto-registering the name via `add_vres()` if not found).** Don't gamble on which index that lazy auto-registration would land on (e.g. assuming a fresh resmgr's first-ever entry gets index 0) — instead, pre-register the resource explicitly via the real `makeResource(core->resmgr, &xce_resource_t{...})` (resobj.c) BEFORE adding worker capacity, and read back the returned object's own `->idx` to use consistently for both the worker's `resvals`/`resInUse` setup (`put_resvals()`) and every later `sched_check_host_satisfy()` `wanted`/`unmeet` vector. This is the only test file in `src/scheduler/testsuite/` (as of this task) that exercises this lazy-resolution path at all — every sibling test that touches `xce_res_account_t.residx` sets it directly, sidestepping the question.
|
||||
- **In any script running under `set -euo pipefail` (e.g. `xce-build.sh`), a bare assignment whose RHS is solely a command substitution (`fp="$(cmd)"`) is NOT exempt from `-e`** — if `cmd` fails, that assignment's own exit status equals `cmd`'s, and since a standalone assignment statement isn't a context `-e` treats as "tested" (unlike being the condition of `if`/`while`/`&&`/`||`/`!`), a failure there aborts the WHOLE script, not just that value's computation. Only `if var="$(cmd)"; then ... fi` (or `cmd || true`) is actually exempt. This bit dirty-content-fingerprint task 2.1's `dirty_fingerprint` computation (sha256 over `git diff HEAD` + sorted untracked file content) in `xce-build.sh`: every intermediate command (mktemp, `git diff`, the `git ls-files | sort` pipe, each `cat` in the untracked-file loop, and the final `sha256sum | cut`) had to be wrapped in its own `if`/`elif ... else fp_ok=0` check rather than trusted to propagate failure naturally, specifically so a failure (e.g. `sha256sum` missing) degrades to an empty fingerprint (Requirement 5.1's "never fabricate") instead of crashing the entire build invocation. Verified live by temporarily hiding `sha256sum` from `PATH` in an isolated scratch repo — script printed `dirty_fingerprint=` and exited 0, not aborted.
|
||||
- **scheduler-sim-decision-parity task 4.2 (2026-08-22): a job that stays genuinely PEND (never dispatched, never state-changed) is re-offered to the live picker and re-PENDed EVERY subsequent round it remains unresolved, not just the round it first arrives in** — `builtin_fcfs.c`'s `pendJobs` discipline only removes a job on dispatch/state-change (its own comment: "A non-pending job is left removed and not reinserted"), so a still-PEND job simply stays in its queue's `pendJobs` and `next_job()` keeps offering it every round. Confirmed independently by `test_repl_equivalence.c`'s own existing (untouched) fixture output: job 501 (pushed before session 1, never dispatched) recurs as a fresh PEND decision in sessions 2 AND 3 too. Bit a new multi-round `--sim-verify` regression test (`test_sim_verify.c`'s `test_verify_with_multiple_already_resolved_jobs_across_rounds_reports_no_mismatch`): the round-2 fixture initially recorded only the round's one FRESH job's PEND, producing a real MISMATCH (expected vs actual position mismatch) because the still-PEND job from round 1 also reappeared, unrecorded, in the live-drained stream. Fix: any multi-round sim fixture with a job that stays PEND across round boundaries must include that job's OWN recurring PEND in every subsequent round's recorded decisions list, ordered by `queue_time` ascending (same-priority ties break by arrival order) to match `next_job()`'s real dequeue order. General lesson: don't assume a job "handled" in round N (PEND, not DISPATCH) is done being decided — only DISPATCH/state-change removes picker candidacy, so a genuinely-unresolved job is a recurring cost across every fixture round.
|
||||
|
||||
- **scheduler-sim-decision-parity task 4.3 (2026-08-22, FINAL task): Recorded-Decision Reconciliation's SUCCESSFUL-MATCH path is structurally unreachable from ANY continuously, organically live-driven recording (real normal-mode OR simulated) — this is a hard mathematical fact about the design, not a limitation of test-authoring creativity, and it is worth knowing before attempting a "real, long, driven recording" fixture for this feature again.** `sim_reconcile_unaccounted_jobs()` (sim_loop.c) matches a RUN-state, `job->accounts==NULL` job against a DISPATCH decision recorded in THAT SAME replayed round's own decoded ReplaySession only (each round's `decoded_session` is decoded fresh and freed immediately after that round's own processing — no historical carry-forward across rounds exists anywhere in the loop). But a job cannot be BOTH "freshly PEND when this round's live picker session starts" (the ONLY way any live picker/allocator, real or simulated, can ever produce a DISPATCH decision for it) AND "already RUN before this round's live picker session starts" (reconciliation's own trigger precondition) in the SAME round — `builtin_fcfs_on_dispatch()`/`on_job_state_change()` make these two states mutually exclusive by construction (dispatch permanently removes a job from `pendJobs`; a state-change to RUN removes it too, via the SAME `on_job_state_change` guard, whichever fires first in a batch). Confirmed BOTH by direct source read (sim_loop.c/driver.c/builtin_fcfs.c, before writing any test code) AND empirically, live: a test built with 11 "organic" jobs (arrive PEND, get denied/recur, get genuinely dispatched by a real capacity-aware allocator thread in their OWN arrival round, get confirmed RUN via a JOB_STATE_CHANGE event in a LATER round) showed EVERY ONE of those jobs' real dispatches permanently invisible during replay (`job->accounts` stays NULL forever, per Requirement 4's own documented graceful-degradation path) — only ONE job, deliberately engineered so its arrival+RUN-confirmation events AND its own "originating decision" all land in the exact same round (built via real constructors — `make_charge_account`/`sched_commit_job_alloc`/`make_dispatch_decision` — fed through the real `core->decisions` cbuf, NOT hand-crafted capnp bytes), ever showed nonzero resInUse during replay. Every EXISTING test in this feature that exercises reconciliation's success path (`test_sim_round_boundary_reconciliation.c` task 3, `test_sim_contention_parity.c`'s "Part B" task 4.1) independently arrives at the SAME resolution (hand-construct the one round where the trigger and its matching decision coincide) for exactly this reason — it is the ONLY way to reach that code path at all. Lesson: if a future task needs "a job whose historical dispatch becomes visible via reconciliation," do NOT assume dispatch-now-confirm-a-few-rounds-later will work for ANY organically-arriving job — it structurally cannot, regardless of round count or job count; the trigger and its match must be engineered into the identical round, unavoidably requiring at least a minimally test-authored decision object for that one job/round. See buglog.json bug-1623 for the live test-authoring mistake this caused (initial checkpoint values assumed churn jobs' dispatches WOULD be reconciled; they never were).
|
||||
|
||||
- **(2026-08-22) Normal-mode resource accounting does NOT survive a round boundary for a dispatched-but-not-yet-worker-confirmed job — a real over-commit/double-booking risk, distinct from and NOT fixed by `scheduler-sim-eager-reconciliation`.** `sched_commit_job_alloc()` (`scheduler.c:1446`, called from `sched_commit_decision()`'s DISPATCH branch) synchronously charges `job->accounts` into `core->sharedInUse`/`worker->resInUse` at dispatch-decision time, regardless of `job->state` (still PEND — RUN only sets later via the separate `schedStartJob` notify path). But EVERY round, `xce_core_begin_session()` (`core_session.c:66-108`) unconditionally zeroes those same totals, and `sched_account_jobs()` (`scheduler.c:1258-1292`) only re-derives them from jobs where `JOB_IS_RUN(job->state)` is true (`scheduler.c:1276`) — silently skipping a still-PEND-but-dispatched job. Normal-mode rounds run on a 1-second cadence (`main.c:453-460`, `while(1){ ...round...; sleep(1); }`), so any dispatch-to-confirmation latency over ~1s (ordinary for real worker RPC+fork/exec) wipes that job's charge at the very next round and doesn't restore it until confirmation lands. The allocator's admission check (`sched_check_host_satisfy_`/`sched_check_shared_satisfy_`, `resource.c:252-370`, called from `seqjob_allocator.c:130`) reads exactly this same `resInUse`/`sharedInUse` — no separate reserved/committed pool exists (`xce_sched_job_t` only has `state`/`allocs`/`accounts`, `sched/types.h:96-116`) — so the resource looks free again and can be double-granted to a different job. Logged as bug-1624, NOT YET FIXED. `scheduler-sim-eager-reconciliation`'s boundary explicitly excludes normal-mode behavior changes, so this needs its own spec.
|
||||
|
||||
- **(2026-08-23) `scheduler.c`'s `sched_update()`/`sched_job_state_change()` (the notify-driven job-state-apply path) is DEAD CODE in the live production driver — do not reuse or trace through it when designing scheduler changes.** `main.c`'s production loop calls only `xce_sched_driver_run_normal_mode_round()`, which drains controller-originated events via `xce_sched_driver_translate_pending_events()` (`driver.c`) → `xce_core_push_events()` → `core_events.c`'s typed-event switch (e.g. `XCE_CORE_EVENT_JOB_STATE_CHANGE`) — a separate, independently-maintained mirror of `scheduler.c`'s logic. `handler.c`'s notify handlers still enqueue onto the same underlying `core->pend_updates` dataq both drain paths read from, so the ENQUEUE side is shared/live, but only `core_events.c`'s DRAIN/APPLY side is ever actually invoked. Confirmed via direct trace of `main.c`'s `while(1)` loop body (found during `scheduler-dispatch-commit` design). Any future scheduler design work touching state-change application must target `core_events.c`, not `scheduler.c`'s `sched_update()`.
|
||||
|
||||
- **(2026-08-23) Iterative /kiro-validate-design loops (fix → re-review → repeat) keep finding NEW, escalating-severity issues well past round 1 — don't stop at the first clean-looking pass.** For `scheduler-dispatch-commit`'s design, round 1 found 3 issues (a locking claim that didn't match the code, a dead field, a requirements-wording mismatch); fixing those and re-reading surfaced a genuine double-notify race in round 2 (one function unconditionally sent the same notify type on both success and failure); fixing THAT surfaced an rc-value classification ambiguity + a missed 6th failure site in round 3-4; fixing THAT surfaced a 4-site reference-count leak (bug-1627) and an unconditional pre-failure-check commit in the same function; a final full re-read caught the WAL apply logic only covering one of two apply functions (`apply_wal_entry()` recovery path vs `apply_committed_entry()` live path). Each fix changed the code enough to expose a DIFFERENT class of bug the previous read hadn't looked for. Lesson: budget for 4-5+ rounds on a design touching genuinely concurrent/durable code, not 1-2 — and re-read the FULL file after each fix, not just the section just edited, since fixes ripple (count/terminology drift across sections was itself a repeat source of new-looking "issues" that were really just staleness from earlier edits).
|
||||
|
||||
- **(2026-08-23) Weekly status reports are an established, continuous series in `reports/`, and their windows are defined by CONTINUITY, not by the calendar.** Filenames are `mmmDD-mmmDD-YYYY.md` (lowercase month, zero-padded day, end date's year) but the ranges deliberately drift — `jun29-jul04` (Mon–Sat), `jul19-jul26` (8 days), `aug08-aug15` (Sat–Sat) — because each report starts the day after the previous one ended and runs to the last completed day. **Always `ls reports/` and read the latest report's end date BEFORE picking a window**; the naive "last complete ISO week" is usually wrong. Concrete case: on 2026-08-23 the ISO default resolved to Aug 10–15, but `aug08-aug15-2026.md` already covered it — the genuine gap was Aug 16–22, so the default would have written a duplicate while leaving a whole week unreported. The current stable format (last 3 reports) is: `# XCE Project Status — <range>` → `## TL;DR` → `## Commits by day` (per-day `###` subsections each with a *theme*, not a commit dump) → `## Bugs opened/fixed this window` → `## Spec status changes` → `## Stats` → `## Open threads / to-do heading into next session` (numbered, carried forward and explicitly closed out in the next report). Two metric traps: quote **non-`.wolf`** line counts as the headline (`.wolf/token-ledger.json` churn made one week read +288,983 raw vs +28,657 real), and split `chore(wolf)` checkpoint commits out of the commit count (89 of 192 in one window).
|
||||
|
||||
- **(2026-08-23) `.wolf/buglog.json` currently has 39 DUPLICATE IDs from a counter restart on Aug 15–17, 2026 — do not trust `bug-NNN` as a unique key, and do not infer "old bug re-touched" from a low ID with a recent timestamp.** New entries were appended reusing the already-taken `bug-875`–`bug-949` range; e.g. `bug-908` is *both* a 2026-06-21 `raft_sim.c` guard-clause bug and a 2026-08-16 `config-upgrade-snapshot/spec.json` refactor note. When counting bugs for a report or analysis, split IDs at the prior window's max (genuinely-new vs collision) rather than trusting min/max. The Stop hook flags duplicates every turn until this is repaired, and repair needs a git-history-informed merge (identify the newcomer of each pair, renumber above the current max). This is the exact failure the OpenWolf bug-logging rule calls "unrecoverable without a git-history merge" — when appending, always compute the next ID as `max(existing numeric IDs) + 1`, never from a local or remembered counter.
|
||||
- **(2026-08-23, scheduler-dispatch-commit task 5.2) A design.md state-model claim of "no reversal needed on failure" can be silently scoped to only the FIRST occurrence of an event, and a later requirement in the SAME design can anticipate a repeat occurrence the claim doesn't cover — don't trust such a claim without checking it against every scenario the design's OWN other requirements construct.** `dispatch_a_job_()`'s (`job.c`) four retryable-failure branches never write `job.state`/`job.allocs`, justified by design.md's "Controller Dispatch Commit" text: "on RPC failure, state is untouched (still PEND — never entered RUN, so no reversal is needed, per the Research Log finding)" — true ONLY for a job's first-ever dispatch attempt. Requirement 4.3's own text ("consecutive... failures with no intervening successful dispatch") and task 3.1's own observable-completion criterion ("a job that fails, succeeds, then fails again is evaluated against the bound as if it had only failed once") explicitly anticipate a job that has ALREADY succeeded once (RUN, allocs committed) failing again later — a scenario where "never entered RUN" is false, so "no reversal is needed" doesn't hold. Building a real, end-to-end fail→succeed→fail test (not a manual-field-poke unit test — those all vacuously start-and-stay-PEND, which is exactly why 4 EXISTING passing tests never caught this) exposed that `job.state`/`job.allocs` genuinely never revert on this path — logged as bug-1631 (unfixed; fix requires a job.c change, out of a test-only task's boundary). The counter-reset itself (Req 4.3, task 1.3/3.1) IS correctly implemented and IS proven end-to-end — this is a narrower, separate gap (Req 4.1/4.4/4.5) than the one the counter test was written to prove. **General lesson: when a design document asserts an invariant with a specific justifying scenario ("never entered RUN"), check whether ANY other requirement in the same design describes a scenario that would falsify the justifying premise — don't assume the invariant holds universally just because the design states it as if it does.**
|
||||
- **(2026-08-23) Technique: driving a real fail→succeed→fail sequence through a black-box test of a static dispatch function that never gates on the target object's own state** — `dispatch_a_job_()` re-resolves the worker via a fresh `key_find()` on every call and never checks `job.state` itself, so toggling the SAME `worker_t*`'s `->ep` field between a "broken" endpoint (task 3.3's `dfr_setup_broken_ep()` — real socket, unregistered in iomgr's epoll, `xce_startjob_req()` fails deterministically and repeatably) and a "working" one (task 3.1's zeroed/no-iomgr `endpoint_t` — enqueue always succeeds) between consecutive calls against the identical job/worker pair is sufficient to construct a real fail→succeed→fail cycle with no second fixture and no manual field pokes. Reusable pattern for any test needing multiple, alternating real outcomes from the same call site.
|
||||
- **(2026-08-23, scheduler-dispatch-commit task 5.4) `controller_test.c`'s `-t <group>` filter operates at the GROUP-function level only (`TEST(test_commit_barrier_reply)` etc.), never at the individual `RUN_TEST()` case level inside a group's own entry function** — if a hang (e.g. bug-1429) sits at an earlier `RUN_TEST()` call than the specific test you need to isolate, WITHIN THE SAME group function, `-t <that-group>` still hits the hang first and never reaches your test; there is no finer-grained filter in this harness. Confirmed live: `test_jobsubmit_repeated_truncation_forced_failures_no_phantom_jobs` (bug-1429's usual hang site) is registered at `test_commit_barrier_reply.c:3639`, BEFORE `test_jobdispatch_receipt_ack_no_wal_append` at line 3649, inside the same `test_commit_barrier_reply()` function — so `-t test_commit_barrier_reply` cannot isolate the latter if the former hangs. In practice this session bug-1429 did NOT reproduce in 2 consecutive full unfiltered `meson test --suite controller` runs (both completed clean in ~38s, 1133 tests, only the 2 known bug-1561 failures) — reinforces that it's genuinely non-deterministic/timing-dependent, not something to route around by default; try a full unfiltered run FIRST (fast when it doesn't hang) before reaching for `-t` group-exclusion workarounds.
|
||||
- **(2026-08-23, scheduler-dispatch-commit task 5.4) bug-1598's "never-joined background thread + stack-local test fixture" ASAN stack-use-after-return pattern (`sim_picker_thread_fn` in `sim_loop.c`, reading a stack-local `xce_sched_sim_driver_t` after its owning test frame returns/is reused) has an UNFIXED sibling instance in `test_sim_verify.c`'s own separately-defined `run_sim_and_capture_stdout()` helper** — bug-1598's fix only converted the 3 stack-local fixtures in `test_sim_event_loop.c`; any OTHER file with its own stack-local `xce_sched_sim_driver_t sim;` calling `sched_sim_driver_run()`/`sched_sim_driver_start()` still carries the identical latent bug, non-deterministically triggered by whatever stack-layout/`MALLOC_PERTURB_` randomization a given test invocation happens to produce (fired once in a `/xce-build scheduler` run this session, then didn't reproduce across 3 standalone re-runs + 1 more full-suite re-run). Logged as bug-1632. Before trusting "the scheduler suite is at its known N/52 baseline," grep for other stack-local `xce_sched_sim_driver_t` fixtures beyond the ones bug-1598 already fixed — this class of bug is per-file, not fixed globally by one file's remediation.
|
||||
|
||||
## Do-Not-Repeat
|
||||
- **When translating a `fp_ok`-tracked, per-step-gated failure-detection pattern (like `xce-build.sh`'s dirty-fingerprint computation) into a Make `$(shell sh -c '...')` one-liner, do NOT collapse two commands into `{ cmd1; cmd2; } >"$$fp_tmp"` and gate the whole group with one `if`** — a POSIX sequential group's exit status reflects only its LAST command, so `cmd1`'s failure is silently discarded whenever `cmd2` still succeeds (2026-08-21, bug-1612: `git diff HEAD` failing was swallowed because the trailing `git ls-files | sort | xargs cat` pipe still exited 0, producing a fabricated fingerprint instead of degrading to empty). Give each step its own explicit `if ! step; then fp_ok=0; fi` (or `&&`-chain), exactly mirroring the reference script's discipline, even inside a cramped one-liner. Caught by adversarial code review with a targeted failure-injection test (wrapper `git` on `PATH` that fails only the targeted subcommand) — that test pattern is the reusable way to verify this class of fix.
|
||||
- Don't run `clang-format -i` on whole existing files here — the repo isn't clang-format-clean at baseline, so a full reformat buries the real diff; recovering via `git checkout HEAD -- <file>` on a file with other uncommitted work destroys that work irrecoverably.
|
||||
- **(2026-08-22, scheduler-sim-decision-parity task 2.3) Actually ran `clang-format -i` on two files mid-task despite this rule already being logged above — caught it via `git diff` before finishing, not before running it.** If this happens again: do NOT `git checkout` the file (destroys real work). Instead: `git diff -- <file>` and read every hunk; any hunk whose ONLY change is whitespace/line-wrapping on code you did NOT intentionally write/edit this turn is collateral reformatting — manually `Edit` just that hunk back to its pre-format text (find each such hunk, revert by hand). Verify success with `git diff -- <file> | grep -c '^-[^-]'` on regions you didn't touch dropping to 0, then rebuild+retest to confirm nothing broke. Cheap enough to be a non-event once caught, but the real fix is: check this Do-Not-Repeat list (or just never run `clang-format -i` without `--dry-run`/`-output-replacements-xml` first to preview scope) BEFORE running it, not after.
|
||||
- Before citing a bug as "still open" or a spec as "blocked," cross-check `.wolf/buglog.json`'s actual fix status and the spec's own tasks.md/commit history — blocked_on notes and prior narrative claims (including this file's own past entries) can be stale. **Self-caught recurrence (2026-08-19)**: while reconciling OTHER stale doc references, wrote a new tasks.md status line asserting "tasks 2.1-2.6 still carry no `_Verified:` annotations" by trusting a 2026-08-07 validation finding's own wording verbatim, without re-checking the file — they'd actually been backfilled the very next day (commit `d7b49690`, 2026-08-08). A dispatched verification agent caught it via `git log`/direct read. Lesson holds even for a claim you are ABOUT to write, not just one you're citing from memory: grep/read the actual current state of the specific lines before asserting their status, every time, no exceptions for "this one's probably still true."
|
||||
- Before trusting an "all tests pass, GO" claim on a stale/uncommitted tree, re-run the suite yourself — a buglog "FIXED" note can describe the *intended* fix while the code on disk is mid-revert from RED-phase construction.
|
||||
- A generic retry/failover classifier that judges retriable-vs-terminal by a raw return value's magnitude (comparing against known errno values) is unsafe unless every wrapped call site shares the same success convention — a call that legitimately returns a variable count on success can numerically collide with an error code.
|
||||
|
|
@ -114,6 +141,12 @@
|
|||
- **This sandbox's `mktemp` is BusyBox's implementation, not GNU coreutils — it requires the TEMPLATE to literally END in `XXXXXX` with nothing after it (no `--suffix` flag, no trailing literal suffix like `.XXXXXX.log`).** (2026-08-21, dirty-content-fingerprint task 6.3): `mktemp "path/name.XXXXXX.log"` fails with a bare `mktemp: Invalid argument` (no other explanation) under BusyBox; `mktemp --help` confirms ("TEMPLATE must end with XXXXXX"). Always end mktemp templates in this repo's scripts with `XXXXXX` and nothing else — append any desired suffix to the returned path afterward if truly needed, don't rely on GNU's suffix support.
|
||||
- **A self-check that greps a script's OWN SOURCE for a literal flag string (e.g. confirming `--impure` is never used) will false-positive on the script's own comments/log messages if those comments discuss the flag by name (which good documentation often does).** (2026-08-21, dirty-content-fingerprint task 6.3): a blind `grep -q -- '--impure' "$0"` matched this script's own header prose explaining why `--impure` is deliberately avoided. Narrow such self-checks to lines that begin an actual invocation of the relevant command (e.g. `grep -A2 -E '^[[:space:]]*nix (eval|build|develop|log)\b' "$0" | grep -q -- '--impure'`), not a whole-file grep — this generalizes to any "assert we never do X" self-check in a script whose own documentation legitimately mentions X.
|
||||
- **Reinforcement of the bug-1615 same-statement `local a=... b="$a..."` gotcha**: recurred independently in a brand-new file (`extract_flag_from_json()`, dirty-content-fingerprint task 6.3, logged as bug-1616) less than a day after being first documented — this pattern is easy to reach for out of habit when writing a "compute one derived value from a parameter" helper. When writing ANY new bash function with `local x="$1" y="...$x..."`, default to splitting the dependent assignment onto its own `local` statement, don't rely on remembering the gotcha in the moment.
|
||||
- **scheduler-sim-decision-parity task 1.1 (2026-08-22)**: relocating an `xce_scheduler_t*`-signature function to `xce_sched_core_t*` (the `sched_resolve_job_picker()` precedent, now repeated for `sched_account_jobs()`/`sched_charge_account()`/`sched_uncharge_account()`/`sched_update_account()`/`sched_update_res_accounts()`/`sched_commit_job_alloc()`) has a hidden fan-out cost: EVERY testsuite file that white-box `#include`s the driver/scheduler source and provides its OWN link-satisfying spy/stub for that function (grep the whole `testsuite/` dir for the function name, not just the file(s) named in the task boundary) must have its stub's signature updated too, or the build fails with a type mismatch/duplicate-declaration error — 6 separate test files needed this for `sched_account_jobs()` alone (test_driver_decision_cb.c, test_resval_ownership.c, test_admin_request.c, test_sim_equivalence.c, test_record_lifecycle.c, test_driver_translate.c), none of them named in the task's own boundary list but all mechanically required by it. This project's codebase convention on the relocated parameter's name is to rename it from `sched` to `core` (confirmed via `core.c`'s own `sched_get_job`/`sched_add_worker`/etc., not just keeping the old name with a retyped pointer) — match that, don't just retype in place.
|
||||
- **Testing "does function X behave identically given a core from container A vs container B" (Req 1.4-style cross-container proof) does NOT require white-box `#include`-ing the heavyweight driver file that defines the thin iterate-and-call wrapper** (here, `scheduler.c`'s ~20-line `sched_account_jobs()`) if the real accounting logic lives one level down in a lighter, already-provenincludable file (`resource.c`, per `test.c`'s own existing `#include "../src/resource.c"`) — no test in this project's `testsuite/` white-box-includes `scheduler.c` at all (confirmed via grep before writing), because its link-time dependency surface (controller-join, capnp publish, HA candidates) is much larger than `resource.c`'s. Test the real lower-level functions directly instead (`sched_charge_account()`/`sched_uncharge_account()`/`sched_commit_job_alloc()` here) and document in the new test's header comment exactly why the thin wrapper itself isn't independently exercised (already covered elsewhere, e.g. by an existing ordering-proof spy test) — this is honest, still exercises 100% real relocated code, and avoids a huge stub-writing burden.
|
||||
- **`xce_sched_core_init()` is safe to call twice in the same test process/function to build two independent, simultaneously-alive `xce_sched_core_t` instances for a side-by-side comparison** — established precedent `test_snapshot_save_load.c` (`core_a`/`core_b`) already relies on this; it does re-init a couple of process-global singletons internally (`xstring_table_init()`) but that's a harmless re-init, not a crash/corruption risk, confirmed by that pre-existing test passing. White-box-including `../src/core.c` + `../src/resource.c` (NOT `core_events.c`/`core_session.c`/`decision.c`/`plugin.c`, unneeded if the test only calls `xce_sched_core_init()`/`sched_add_worker()`/`sched_charge_account()`-family functions directly, bypassing the event-push/session-begin/decision-drain APIs entirely) needs exactly 7 link-satisfying no-op stubs: the `kill_sched_job/queue/worker/user/project` family + `kill_sched_resval` (core.c's kvmgr/zarray datakillers) + `plugin_unregister` (core.c's plugins-kvmgr datakiller) + `plugin_prepare_allocation`/`sched_ctx_get_uncomit_host` (resource.c's `sched_try_alloc_on_host_()`'s own callees, needed only because that whole file is pulled in, even though this test path never calls it) — the last two are the exact same stub pair `test.c`'s own resource.c-only white-box test already establishes needing.
|
||||
- **Never use `date -d` with a RELATIVE expression in this environment (`-7 days`, `+5 days`, `last monday`) — `date` first on PATH is BusyBox v1.36.1, and its failure is silent-by-default rather than loud.** (2026-08-23, `/weekly_report` skill authoring, bug-1628): BusyBox date *does* parse absolute dates (`date -d 2026-06-29 +%A` → `Monday`), which masks the gap during casual probing, but rejects all relative arithmetic with `invalid date` **on stderr** — so `START=$(date -d "-7 days" +%F)` captures only stdout and yields an EMPTY string. The empty value then flows into `git log --since="" --until=""`, which git treats as *no time filter at all* (unbounded history) instead of erroring, so a broken date computation silently produces a plausible-looking-but-wrong result. Note `/bin/date` IS GNU coreutils 9.8 — the nix profile bin dir simply precedes `/bin` on PATH. **Do date arithmetic in `python3` (`/bin/python3`, always present) via `datetime`/`timedelta`, emitting `VAR=value` lines consumed by `eval "$(python3 - <<'PY' ... PY\n)"`;** call `/bin/date` by absolute path only if GNU date is genuinely required. Corollary that generalizes beyond dates: whenever a computed shell variable feeds a `git log --since/--until`-style range filter, assert it is non-empty first — git's "empty arg means no filter" behavior turns an upstream failure into silent wrong output rather than a crash. Same BusyBox-shadows-GNU family as the `grep -P` and `mktemp` entries above.
|
||||
|
||||
- **A scheduler resource-accounting test fixture that lets a job's `xce_res_account_t.residx` resolve LAZILY (left at -1, relying on `sched_update_res_accounts()`'s own first-use `key_find()`/`add_vres()` fallback) can SEGV inside `zarray_get_ex()` instead of failing loudly.** (2026-08-23, scheduler-dispatch-commit task 2.1, bug-1629): a minimal test fixture that never pre-registers a resource name in `core->resmgr` (no real resource-config load, unlike production) hit a silent failure path — `sched_update_account()` discards `sched_update_res_accounts()`'s return value entirely, so if that inner call's `key_find()`/`add_vres()` resolution doesn't complete, `residx` silently stays `-1` with no error surfaced anywhere up the call chain (`sched_commit_decision()` still returns success). `zarray_get_ex()` (`lib/zarray.c:332`) only guards `idx >= array->caps`, NOT `idx < 0`, so passing that leftover `-1` to `get_resvals()`/`put_resvals()` reads `array->data[-1]` — an underflow read ASAN reports as a high-address SEGV, not a clean bounds error. Fix: in any test fixture exercising real charge/accounting code without loading real resource config, pre-register the resource explicitly first via `add_vres(&vres, core.resmgr, name, 0)` and use its returned (non-negative) index for `get_resvals()`/`put_resvals()`, rather than trusting the production lazy-resolution path to run to completion inside a minimal fixture.
|
||||
|
||||
## Decision Log
|
||||
- HA/consistency model follows Raft + CP strictly: majority ACK is the hard commit barrier for all state-changing writes (reject on timeout, never local-apply-and-degrade); the own-term commit rule (not a 4th "COMMITTING" state) is what gives a new leader safety.
|
||||
|
|
@ -126,4 +159,13 @@
|
|||
- `build-time-versioning` task 4.2 (2026-08-20): `apiserver/meson.build`/`xcecli/meson.build`'s existing `CURRENT_SOURCE_DIR="@CURRENT_SOURCE_DIR@"`/`BUILD_ROOT="@BUILD_ROOT@"` command-array entries (passed to `env_program = find_program('env')`, no shell involved) put literal double-quote characters INTO the env var value seen by the child process — meson's `command:` list is exec'd directly, so shell-style quoting around a placeholder is not stripped by anything; `go_build`'s own `fix_input_envs()` explicitly `sed -e 's@"@@g'`s those two vars to compensate, proving this empirically. When adding new KEY=VALUE entries to this SAME mechanism (`XCE_GIT_COMMIT`/`XCE_BUILD_TIME`, sourced from `get_option('commit_id')`/`get_option('build_time')`, task 1's options — no meson placeholder substitution needed since they're already resolved strings), deliberately did NOT wrap them in quotes (`'XCE_GIT_COMMIT=' + get_option('commit_id')`, not `'XCE_GIT_COMMIT="..."'`) — `go_build`'s task-4.1 code reads `$XCE_GIT_COMMIT`/`$XCE_BUILD_TIME` directly with no quote-stripping, so quoting these would have embedded literal `"` characters into the linked `-X main.GitCommit=...` value. Live-verified via `DEBUG=1` (`go_build`'s own `set -x` trace mode) meson-driven build: clean values (`18ae0cc3`, ISO8601 timestamp) reached `go build -ldflags`, and `xce-build.sh apiserver`/`xcecli` end-to-end produced binaries with `-X main.GitCommit=<hash>-dirty -X main.BuildTime=<ts>` in their `go version -m` build info, no stray quotes. General lesson: "mirror the existing mechanism" doesn't mean copy-paste every byte of an existing established pattern — check whether the DOWNSTREAM consumer (here, `go_build`) actually expects/strips the same decoration before reusing it verbatim.
|
||||
- `resource-value-verification` spec (2026-08-17): verify scheduler/apiserver resource-value propagation via a one-line trace/debug log at each layer's point of application (`core_events.c`'s RESVALS_DELTA case; apiserver's `OnResourceLocationUpdated`), grepped live, instead of record+replay+REPL or a REST/CLI query — matches how bug-1568/1569 were actually root-caused, and is a stronger read of "the scheduler's own resource state" than a replayed reconstruction. Trade-off accepted: doesn't exercise the real query/REST path, only storage correctness. Load-bearing gotcha: `TRACE`=`tlog_debug3` needs `-vvv`; apiserver's `logger.Debug` needs `logger.yaml`'s `logging.level: debug` — both silently no-op otherwise, producing false timeouts. Also: extend `xce-deploy-ha`'s `deploy-ha.sh` (opt-in, conditional-on-binary-present, mirroring its existing worker/scheduler pattern) plus a generated `start-all.sh`, rather than building a parallel standalone deploy script — avoids re-implementing the same deploy conventions twice.
|
||||
- **This project's existing home for a standalone, CLI-runnable regression/integration check that needs a REAL build or live deployment (not a meson `test()`) is `src/tools/test-<topic>.sh`** — `test-commit-durability.sh` and `test-heartbeat-tolerance.sh` are the established precedent (shared header-comment style documenting the invariant under test + its spec, `green()`/`red()`/`yellow()` colour helpers, a `pass()`/`fail()` counter convention). Followed this for `build-time-versioning` task 6.1's `test-build-version-regression.sh` rather than inventing a meson test() (two full `rm -rf`+`meson setup`+compile cycles per profile is far too expensive for `meson test`) or a new top-level test-framework directory. When a task explicitly says "use judgment / follow existing convention if the design doc doesn't pin down an exact path," check `src/tools/` first for this class of check before reaching for `testsuite/` or inventing something new.
|
||||
- **`xce-build.sh` has `set -euo pipefail`, and every profile's `RUN_TESTS` block (asan only, currently) runs the FULL component meson test suite unconditionally as the LAST step of the invocation — so ANY single failing test in that suite (even a pre-existing, unrelated one, e.g. bug-1560's scheduler symlink-escape flake) makes the WHOLE `xce-build.sh <component> <profile> ...` invocation exit non-zero, even though `meson compile` already fully succeeded and the real binary exists on disk.** A caller/wrapper script that treats a non-zero `xce-build.sh` exit as "the build failed, binary unusable" will incorrectly skip/abort real post-build work (e.g. checking `--version` output, as `test-build-version-regression.sh` needs to). Fix pattern: never bare-call it under your own `set -e` — capture the exit code via `cmd || rc=$?` (NOT `if cmd; then ...; fi` with no else, which loses the real nonzero code — POSIX defines an `if` with a false condition and no `else` as exiting 0, discovered live re-deriving this), print a WARNING distinguishing "compile succeeded, only the bundled test run failed" from a genuine failure, and let a SEPARATE binary-existence check be the actual pass/fail signal for "did the build produce something real."
|
||||
- **`xce-build.sh` has `set -euo pipefail`, and every profile's `RUN_TESTS` block (asan only, currently) runs the FULL component meson test suite unconditionally as the LAST step of the invocation — so ANY single failing test in that suite (even a pre-existing, unrelated one, e.g. bug-1560's scheduler symlink-escape flake) makes the WHOLE `xce-build.sh <component> <profile> ...` invocation exit non-zero, even though `meson compile` already fully succeeded and the real binary exists on disk.** A caller/wrapper script that treats a non-zero `xce-build.sh` exit as "the build failed, binary unusable" will incorrectly skip/abort real post-build work (e.g. checking `--version` output, as `test-build-version-regression.sh` needs to). Fix pattern: never bare-call it under your own `set -e` — capture the exit code via `cmd || rc=$?` (NOT `if cmd; then ...; fi` with no else, which loses the real nonzero code — POSIX defines an `if` with a false condition and no `else` as exiting 0, discovered live re-deriving this), print a WARNING distinguishing "compile succeeded, only the bundled test run failed" from a genuine failure, and let a SEPARATE binary-existence check be the actual pass/fail signal for "did the build produce something real."- **A `sched_account_jobs()`-shaped no-op stub already present in SOME white-box test files that `#include "../src/sim_loop.c"` directly does NOT mean it's present in ALL of them — adding a genuinely NEW production call inside a white-box-#include'd file breaks every sibling TU that lacks the matching stub, and the only reliable way to find them all is to build and read the linker errors, not to grep-and-guess.** scheduler-sim-decision-parity task 2.1 (2026-08-22): `sim_loop.c`'s round boundary gained a real call to `sched_account_jobs(&(sim->core))` (previously never called there at all, bug-1613). `test_sim_equivalence.c`/`test_sim_resource_contention.c` (task 1.2-era files) already carried a no-op stub anticipating this; 8 OTHER sibling files that also `#include "../src/sim_loop.c"` (`test_sim_event_loop.c`, `test_sim_verify.c`, `test_sim_alt_events.c`, `test_sim_record_output.c`, `test_repl_breakpoints_inspection.c`, `test_repl_snapshot_params.c`, `test_repl_execution_control.c`, `test_repl_equivalence.c`) did NOT, since none of them ever needed the symbol before. `grep -rl '#include "../src/sim_loop.c"' testsuite/` first, then just build — the linker's `undefined reference to sched_account_jobs` errors enumerated the exact fix list precisely, faster and more reliably than reasoning about it from source alone.
|
||||
- **`xce_request()`/every `xce_*_req()` wrapper (`src/lib/request.c`) is a genuinely non-blocking, fire-and-forget send — it capnp-encodes the request and `endpoint_enq()`s it onto `ep->outq`, then returns; it never waits for a reply itself** (the synchronous `xceapi.c` wrappers like `xce_hello`/`xce_jobsubmit` layer a separate `cbuf_timedget()` wait on top, but the raw `xce_*_req()` call a controller-side handler like `dispatch_a_job_()` makes does not). `endpoint_enq()` (`netio.c`) succeeds unconditionally against a **zeroed `endpoint_t` with `ep->iomgr == NULL`**: `dataq_enq()` has no failure path at all, and `endpoint_update_epoll()` is a pure no-op whenever `ep->iomgr == NULL` (skips the whole `iomgr_update_poll` branch). Net: to make a controller-side `xce_*_req()` call "succeed" in a unit test, you do NOT need a live peer, a synthetic reply thread, or a socketpair — a `static endpoint_t` (or embedded field) zeroed with `.fd = -1` and left with no `iomgr` is sufficient, and packets just accumulate on `ep->outq` for the test to `endpoint_deq()`/`free_packet()` afterward. This exact pattern is already established/proven live by `test_commit_barrier_reply.c`'s `g_wkr_disp_ep`/`cbrp_arrange_jobdispatch()` fixture (its own file header even documents it: "a zeroed static endpoint_t with no iomgr... packets stay queued") — reused directly for `scheduler-dispatch-commit` task 3.1's `test_dispatch_commit.c` success-path tests, no mocking/round-trip machinery needed.
|
||||
- **`controller_unit_test` (and presumably every other `argp`-based Unity runner built the same way in this project) supports a repeatable `-t <group-name>` (or `--run <group-name>`) CLI filter** (`controller_test.c`'s `TEST(x)` macro checks `check_test(&arguments, #x)` before calling `x()` — with zero `-t` args every group runs, with one or more only the named `TEST(...)` group(s) run, matched against the literal `TEST(test_foo)` identifier as a string). Invaluable for isolating ONE new test group from a suite-wide hang/flake without touching any source: run the already-built binary directly, e.g. `./controller_unit_test -t test_dispatch_commit`, or build a `-t` list for "everything except the one broken/hanging group" via `grep -oP '(?<=^extern void )test_\w+(?=\(void\);)' controller_test.c | grep -v '^test_broken_group$'` and pass each as its own `-t name`. This bypassed a pre-existing, already-tracked hang (bug-1429, `test_commit_barrier_reply.c`'s `test_jobsubmit_repeated_truncation_forced_failures_no_phantom_jobs`, unrelated to the diff under test) cleanly — confirmed a new test group's 3 tests all passed AND that every other group (91 of 92) still matched the pristine-tree failure count exactly, without needing to touch/skip/comment out any test source.
|
||||
- **bug-1429 (`controller_unit_test` TIMEOUT 300s/SIGTERM, NOT root-caused) has now recurred a 3rd time (2026-08-23, `scheduler-dispatch-commit` task 3.1), each time with a COMPLETELY unrelated diff and a DIFFERENT hang location** (`test_demotion_replay_yields_committed_only` → `test_configack_*` group → `test_jobsubmit_repeated_truncation_forced_failures_no_phantom_jobs`, all in `test_commit_barrier_reply.c`/`test_commit_barrier_rollout.c`). Before concluding a suite-wide TIMEOUT is caused by your own change, check `.wolf/buglog.json` for bug-1429 first, then get an independent data point on a genuinely pristine tree (`git stash -u` your own diff, rebuild+retest once) — a pristine run completing cleanly while your-diff runs hang 2-for-2 does NOT by itself prove causation when the hang location has zero code-path relationship to your diff (traced directly: the hanging test never calls the function you changed) and the SAME symptom is already logged against two prior, unrelated diffs. The `-t` filter (above) is the fastest way to get a real, uncontaminated pass/fail answer for your OWN new tests when a pre-existing hang blocks the full suite.
|
||||
- **In the current (pre scheduler-sim-decision-parity tasks 2.2-2.4) codebase, a RUN-state job's `job->accounts`/`job->allocs` is ALWAYS NULL in simulation mode — there is no live path that ever sets it.** Two independent, compounding reasons, both confirmed by reading the real code: (1) `sim_picker_thread_fn()`'s (`sim_loop.c`) own picker thread unconditionally PENDs every candidate (`picker->picker_iface->on_pend()` + `make_pend_decision()`) — it never emits a live DISPATCH (documented "Known scope boundary": real allocation needs a live `xce_scheduler_t*` the sim driver deliberately doesn't have, belongs to the not-yet-designed scheduler-allocation spec). (2) Even if it did dispatch, `sched_commit_decision()` (`scheduler.c`) — the ONLY function that ever does `job->allocs = decision->u.alloc; job->accounts = make_job_account(...)` — is only ever called from `sched_allocator()` (scheduler.c's own real allocator thread), which sim mode never runs; sim's own decision-drain loop (`sim_loop.c`) reads `core->decisions` directly and just records/prints/REPL-checks each one, with no `sched_commit_decision()` call anywhere in that path. Consequence for task 2.1: `sched_account_jobs()`'s new round-boundary call (which only charges a job whose `job->accounts != NULL`) has genuinely nothing to charge yet in isolation — it only becomes behaviorally meaningful once Recorded-Decision Reconciliation (tasks 2.2-2.4) populates `job->accounts` from `decoded_session->decisions`. Task 2.1's own regression test therefore has to prove the WIRING (is `sched_account_jobs()` called, once per round, inside the session-active window) via a spy, not an end-to-end "resInUse becomes nonzero" assertion — that end-to-end behavior only becomes true after 2.2-2.4 land.
|
||||
- **`sched_sim_driver_run()` (`sim_loop.c`) is a single monolithic entry point — unlike `driver.c`'s decoupled, single-round `xce_sched_driver_run_normal_mode_round()` — that opens the replay file, spawns its OWN picker thread (`sim_picker_thread_fn`) and bootstraps the real `builtin_fcfs` plugin (`sim_bootstrap_picker()`, `sim_picker.c`) internally, all inside one call.** This means `test_driver_session_ordering.c`'s cross-thread wbuf-rendezvous technique (substitute a test-controlled picker plugin, spawn it in a test-owned thread, call the driver's one-round function directly) is NOT reproducible against the sim driver without either modifying `sim_picker.c` (routinely outside a narrowly-scoped task's boundary) or restructuring `sched_sim_driver_run()` into smaller testable pieces (not what design.md's File Structure Plan asks for — it's described as an inline round-boundary edit, not a new decomposed interface). Pragmatic substitute used for task 2.1's `test_sim_session_ordering.c`: since `xce_core_begin_session()`/`sched_account_jobs()`/`xce_core_signal_go()` are three unconditional, sequential, same-thread calls in `sim_loop.c`'s own source (no branch can reorder them), the only genuinely runtime-observable fact is "does it actually get called, and does it see `core->session_active==1`" (proven via a spy) — NOT a full cross-thread ordering race, which doesn't exist to prove in the first place given `cbuf`'s already-tested blocking rendezvous semantics.
|
||||
- **`controller_unit_test`'s notify() capture (`test_commit_barrier_reply.c`'s `g_notify_*` arrays) is `static`/file-local, so no OTHER test file in the same binary can read it directly — even though that file is the sole `notify()` definition linked into the binary (`dataex.c`'s real one is excluded).** scheduler-dispatch-commit task 3.3: to let a new test file (`test_dispatch_fail_retry.c`) observe `notify_dispatch()` (job.c) actually firing, added a handful of tiny non-static `cbrp_notify_capture_*_ext()` accessor functions to `test_commit_barrier_reply.c` (count/event/jobid/code by index, plus a reset wrapper) rather than duplicating a second `notify()` stub (impossible — duplicate-symbol link error across a single binary) or skipping the observation entirely. Read-only, additive, does not touch that file's own capture/reset logic or any of its existing tests — a low-risk way to extend an existing test's shared mock surface to a sibling file in the same binary.
|
||||
- **`dispatch_a_job_()`'s `key_find(c->workermgr, ...)` for a genuinely UNREGISTERED worker key returns `rc>=0` with `w` left NULL (hits `XCE_DISPATCH_FAIL_SITE_WORKER_NULL`), not `rc<0` (`XCE_DISPATCH_FAIL_SITE_WORKER_LOOKUP`)** — confirmed empirically (task 3.3), matching `test_dispatch_commit.c`'s own prior comment. A genuine `key_find()` internal failure (rc<0, the WORKER_LOOKUP site) has no known black-box trigger from a test without reaching into librtti's own (external, opaque) internals — `test_dispatch_fail_retry.c`'s `test_worker_notfound_resolves_retryable` deliberately documents that it covers "whichever of WORKER_LOOKUP/WORKER_NULL the runtime takes" rather than claiming to isolate one, since both sites share the exact same classify/resolve code shape and the test's assertions hold for either.
|
||||
- **A real, unregistered-with-its-iomgr socket fd (`socket(AF_INET,...)` + a live `iomgr_t` via `iomgr_init()`, `ssl_handshake_done=1`, `flags=0`, never `EPOLL_CTL_ADD`'d) is a reusable, deterministic technique for forcing an `xce_*_req()` call to fail** — `EPOLL_CTL_MOD` on the unregistered fd fails ENOENT every time (`endpoint_update_epoll()`), and since `ep->flags` never advances past 0 on failure, the SAME broken endpoint can be dispatched against repeatedly in a loop for a bounded-retry-escalation test, no per-attempt endpoint reconstruction needed. First established by `src/lib/testsuite/test_netio_arm_failure.c`/`test_request_send_propagation.c` (bug-646/650); reused directly at the controller layer for `test_dispatch_fail_retry.c`'s `XCE_DISPATCH_FAIL_SITE_RPC_FAILURE` tests (task 3.3) by pointing a `worker_t.ep` at the broken endpoint instead of the established "zeroed, no-iomgr ep always succeeds" fixture task 3.1 uses for the success path.
|
||||
- **A prior task's own test file can pre-emptively flag a specific assertion as "this will need to change once task N.M lands" in its own comment/message string — grep for the LATER task's own number in sibling test files before assuming "task boundary == only my new file changes."** Task 3.1's `test_dispatch_commit.c::test_dc_worker_not_found_leaves_job_pend_and_alloc_uncommitted` asserted `dispatch_fail_count` stayed unchanged with the message "incrementing it is task 3.3's job, not 3.1's" — a deliberate, anticipated staleness, not an accidental regression. Confirmed via the mandated RED (git-stash job.c only, rebuild, run new tests → fail as expected) → GREEN (restore, rebuild) cycle: the GREEN pass surfaced this ONE pre-existing sibling-test failure (seeded counter 2 → now correctly 3), fixed by updating that one assertion + its message, not by touching my own new file. This is explicitly within "test infrastructure under testsuite/ may change" boundary language even when the task brief's boundary section only names the NEW file — a task boundary permitting "test infrastructure" changes covers fixing a sibling test's own foreseen staleness, not just adding new tests.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue