chore: sync harness/session state and OpenWolf bookkeeping
- .claude/settings.json: worktree.baseRef = "head"
- .kiro/specs/scheduler-core/.solomd: session tracking state
- .wolf/{anatomy,cerebrum,memory,token-ledger}.json, hooks/_session.json:
accumulated OpenWolf bookkeeping (includes this session's fmt_jobid work)
- .gitignore: ignore Python __pycache__/*.pyc (scripts/gitea-sync.py) and
.agent-shell/ (session transcripts can capture raw env dumps/secrets --
one already had a live-looking ANTHROPIC_API_KEY in a captured `env`
output; excluded from this commit entirely, never checked in)
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
979cd24f6c
commit
94fd1504e9
8 changed files with 97789 additions and 602 deletions
|
|
@ -10,6 +10,7 @@
|
|||
- Keep test logs for analysis, don't skip intermediate output even if a test hangs.
|
||||
|
||||
## Key Learnings
|
||||
- **xcecli Cobra rootCmd location**: `src/frontend/xcecli/cmd/main.go` imports `cmd "xcecli/router"` (the import alias is `cmd`, but the actual package lives at `xcecli/router/router.go`, package-named `cmd`). Cobra's `rootCmd` is declared there as an **unexported** package-level var (`var rootCmd = &cobra.Command{...}`, router.go:21). ~~The only exported symbol is `Execute(cm *config.ConfigManager) error`, which takes no version param... boundary conflict~~ — **SUPERSEDED, resolved by build-time-versioning task 4.5 (commit fb09afe) and confirmed still true as of dirty-content-fingerprint task 4.5 (2026-08-21)**: `Execute` was extended to `Execute(cm *config.ConfigManager, version string) error`, which itself sets `rootCmd.Version = version` (router.go:29-34) before `rootCmd.Execute()`. main.go now composes the full version string (`GitCommit`/`BuildTime`, plus task 4.5's `composeVersionDisplay(GitCommit, DirtyFingerprint)` strip-and-append) and passes it into `cmd.Execute(configManager, version)` — no further router.go changes needed for version-string composition; only main.go is the boundary now.
|
||||
- **Build entry point**: `flake.nix` lives in `src/`, not the repo root. Always `nix develop /abs/path/to/src --command bash -c '...'` with an explicit path — Bash cwd isn't stable across calls, and a bare `cd src && nix develop` can silently short-circuit.
|
||||
- **Test coverage gaps**: documented full-suite commands (`xce-build.sh <component>`, `make test`) each cover only a subset — e.g. a component's default meson run misses newly-added standalone test binaries unless wired into its `RUN_TESTS` case, and the CGO/apiserver backend suite needs `make test-backend` with CGO_CFLAGS/LDFLAGS reconstructed via pkg-config (package is `c-capnproto`, not `capnp_c`). A green default build is not proof of full coverage — verify every new test binary actually runs.
|
||||
- **Unity harness**: use `UnitySetTestFile(__FILE__)` (not nested `UNITY_BEGIN()/UNITY_END()`) when multiple test-group files share one binary — nested BEGIN/END resets global counters and can mask most of the real suite exit code. Authoritative green = grep raw testlog for `:FAIL:` / word-boundary `\bTIMEOUT\b`, never trust an aggregate `Fail:0` alone. Also rule out a concurrent competing build/soak against the same build dir before treating a timeout/crash as a regression.
|
||||
|
|
@ -22,18 +23,23 @@
|
|||
- **Live-soak validation discipline**: a unit-test-green fix can still be unreachable/dead code in production — always re-run the exact live scenario end-to-end, not just the mechanism fixed. A later, differently-shaped soak failure after a fix isn't automatically a regression from that fix — check whether that failure point was ever exercised before.
|
||||
- **Sandbox quirks**: `/tmp` can be overlayfs with intermittent fdatasync EIO — durability-sensitive tests need a real fs (e.g. `~/tmp` on ext4). This sandbox's `date` may resolve to BusyBox (no `%N`, silently passes it through instead of erroring) — use bash's `$EPOCHREALTIME`/`SECONDS` builtins for timing instead.
|
||||
- **HA election timer discipline (bug-1562)**: every early-return branch in a function driven by a one-shot timer (`ha_on_primary_ctrl_fd_rdhup`'s liveness-watchdog callback) must either re-arm the next timer itself or verify some OTHER live timer is actually still scheduled before skipping the re-arm — checking only elapsed-time-since-last-event (the 5.10b debounce branch) is not the same as checking an election is still in flight (the 5.6 guard's actual role/pre_vote_in_progress check). A silently-dropped re-arm freezes the node with zero CPU/log activity forever (looks identical to a clean idle state, not a crash) — diagnose via `/proc/<pid>/task/*/wchan` (all threads in `futex_do_wait`/`do_epoll_wait` with no fds beyond listen+epoll = dropped timer, not deadlock) before reaching for gdb (ptrace is blocked in this sandbox, `yama ptrace_scope=1` + no ancestor relationship to the target).
|
||||
- **kiro-spec-tasks' independent task-graph sanity review is worth actually dispatching, not skipping as formality**: writing `scheduler-sim-repl-verbosity`'s design, the round-boundary "collapse active output range" logic was placed entirely in repl.c (where the round-boundary hook lives) — but a fresh review subagent (no drafting context) caught that the OTHER two triggers for ending that range (a breakpoint hit, a structural event arriving) are detected in repl_break.c, which had no interface to reach into repl.c's state. This is exactly the class of bug a same-context self-review reliably misses (the drafting mind already "knows" the round-boundary path handles it and doesn't re-derive from the other call sites) — worth the two review rounds it took (RETURN_TO_DESIGN, then NEEDS_FIXES catching a second smaller gap in the same fix) rather than rubber-stamping the first draft.
|
||||
- **Reproducing HA bootstrap bugs**: match the user's actual inter-node start stagger exactly — a tight ~0.5s stagger (the `/xce-deploy-ha` skill's default) can mask a bug that only triggers when a node sits solo for tens of seconds (real peers answering fast enough prevents the trap window from ever closing). Always ask/check the real timestamps before concluding "could not reproduce."
|
||||
- **The sim/replay driver (sim_loop.c) does not fully replicate the normal-mode driver's (driver.c) per-session obligations, and this has now caused two separate real bugs**: `xce_core_begin_session()`/`xce_core_signal_go()` (core_session.c) were deliberately split from the older monolithic `xce_core_go()` (scheduler-core spec task 5.2) specifically so a driver can interleave `plugin_begin_session()`/`sched_account_jobs()` between the resval reset and the picker signal — `driver.c:739-749` does this correctly; `sim_loop.c:680` still calls the old monolithic `xce_core_go()` with zero interleaving slot and never calls `sched_account_jobs()` anywhere (bug-1613: every worker's/shared resource's "used" accounting stays permanently zero in `--sim`/`--interactive`, since the per-session resval reset is never followed by re-accumulation). Before trusting any sim-driver behavior that has a normal-mode counterpart, grep the normal-mode driver (driver.c) for the equivalent call and confirm sim_loop.c actually has one too — don't assume parity.
|
||||
- **Scheduler sim REPL has TWO independent, un-gated stdout print paths, not one**: `xce_repl_on_event()` (repl_break.c, `sim->interactive`-only) unconditionally echoes every decoded event regardless of `step` vs `continue` (bug-1595's fix over-applied — it was requested for `step` only), and `sim_print_decision()` (sim_loop.c, fires independent of the REPL/breakpoint machinery) reprints an identical `PEND`/`DISPATCH`/`PREEMPT` line every round a decision recurs, with no dedup and no session/round context on the line. Neither path consults `xce_repl_on_decision()`'s existing per-round tally (`last_n_dispatch`/`last_n_pend`/`last_n_preempt`, surfaced only via `info session`). Logged as bug-1609/bug-1610 (not yet fixed) — before touching REPL output volume, check both files, not just repl_break.c.
|
||||
- **A soak's own "excluded"/PASS verdict is not a license to stop reading (bug-1563)**: `/xce-test-ha --chaotic`'s verdict deliberately excludes `bug924_fdatasync` (overlayfs EIO) from pass/fail — but a huge excluded count (300K+ in one run) or a different errno than the documented one (EINVAL, not EIO) is worth a manual look anyway. Found: `fsync_worker`'s retry loop only backs off when NOT pending, and a sustained fdatasync failure makes "pending" permanently true (durable_lsn never advances on failure) — a real busy-spin/livelock, invisible to the soak's own job-loss/divergence checks since the other 2-of-3 standbys still satisfy majority. Lesson: skim excluded/noise counts for magnitude and errno before trusting a documented exclusion covers them.
|
||||
- **Fixing a busy-spin/livelock can unmask a latent notification gap (bug-1564)**: throttling an unbounded retry loop (bug-1563's fix) removed an *accidental* fast-repoll that had been quietly covering for `persist_flush()` never firing `on_durable_advance` (only the async `fsync_worker` did). A deterministic (3/3) new test failure appeared immediately after — always suspect "the old code accidentally covered for a real gap via sheer retry speed" when a correctness fix (removing a spin/race) makes a previously-green test start failing 100% of the time; that's a signal to look for a missing notification/wakeup, not to revert the fix.
|
||||
- **nix devShell Python packages are `PYTHONPATH` entries, not merged into `python3`'s own site-packages** (found building a standalone-installable xceadmin wheel): `python3 -m venv --system-site-packages` only inherits the base interpreter's OWN site-packages dir, which is empty in this project's nix shell (`python3Packages.click`/`pycapnp`/etc. are each a separate `/nix/store/...` path added via `$PYTHONPATH`, confirmed via `nix develop … --command sh -c 'echo $PYTHONPATH'`). A `--system-site-packages` venv therefore silently CANNOT see them (`ModuleNotFoundError` at runtime, discovered live). Fix: skip venv/pip entirely — extract the wheel directly (`python3 -m zipfile -e`) and bake the *resolved* `$PYTHONPATH` string into a wrapper script's `export PYTHONPATH=...` at generation time (expand now, not `${PYTHONPATH:-}` at runtime) — mirrors `src/xceadmin/meson-install.sh`'s existing nix-packaging wrapper, which already solved this correctly; I should have grepped for that pattern before reaching for venv the first time.
|
||||
- **`src/tools/xce-build.sh`'s `setup_build()` does `rm -rf "$BUILD_DIR"` for every single-component invocation** (bug-1565, FIXED): `/xce-build xceadmin` after `/xce-build controller` in the same dir used to silently delete `xce_controller`/`xce_test`/etc. Fixed by an opt-in `--incremental` flag (`meson setup --reconfigure` in place instead of wiping) — deliberately NOT the default, since this project's capnp-generation `custom_target` has known missing-dependency-edge races that a full wipe papers over as a loud, retry-fixable failure (skipping the wipe could turn the same gap into silent stale-generated-code instead). `/xce-build all` (build everything together) remains the simplest option when you don't specifically need incremental speed.
|
||||
- **A never-joined background thread reading a stack-local test fixture is a latent ASAN stack-use-after-return, exposed only by test ORDERING, not by the thread's own logic** (2026-08-19, scheduler-core design-review follow-up). `sched_sim_driver_run()` (sim_loop.c) spawns `sim_picker_thread_fn` on a real pthread that is, by this codebase's own documented design, "never joined or cancelled... no sim-driver teardown/fini function exists yet" — it loops forever on `cbuf_get(&sim->core.wbuf, ...)`. Every existing white-box test in `test_sim_event_loop.c` already called this with a stack-local `xce_sched_sim_driver_t sim;`, which was ALWAYS a latent stack-use-after-return once the test function returned — it just hadn't been caught, because no test's stack frame had yet been reused by a later frame in a way ASAN's shadow-memory detector caught. Simply INSERTING one new test between two existing ones (no logic change to the existing tests) shifted the stack-reuse pattern enough to make it fire deterministically (reproduced twice). Fix: change the fixture from stack-local to `static` storage — safe here specifically because (a) `sched_sim_driver_start()` unconditionally `memset()`s the whole struct as its first action so no stale-data leak across runs, and (b) each affected test function is called exactly once per process (verified via grep, no re-entrancy) so a second call's memset can never stomp a still-live zombie thread's cbuf mutex/condvar state from a first call — `static` would be UNSAFE to apply to a helper called more than once while its first zombie thread might still be alive. General lesson: when a background thread spawned by code-under-test is documented as "never joined," any test giving that code a stack-local fixture is a ticking ASAN time bomb regardless of whether it has fired yet — check for exactly this "never joined/cancelled" phrasing in the target function's own comments before adding a new test that changes stack layout/ordering in the same file.
|
||||
- **A plain (non-`--system-site-packages`) venv's `pip install` DOES work for xceadmin's deps, including pycapnp, as long as venv creation + `pip install` + running the installed entry point all happen inside `nix develop … --command`** (2026-08-20, build-time-versioning task 6.3, extends the PYTHONPATH entry above rather than contradicting it): unlike the `--system-site-packages` case, a plain venv's `pip install pycapnp` resolves the prebuilt PyPI wheel fresh over the network (confirmed reachable: `pypi.org`/`files.pythonhosted.org` both resolve in this sandbox) — no PYTHONPATH-visibility problem, since pip is installing its own separate copy into the venv rather than trying to see the nix-injected one. The catch: that PyPI pycapnp wheel dynamically links `libstdc++.so.6`, which the bare host has NO system-wide copy of at all (`import capnp` → `ImportError: libstdc++.so.6: cannot open shared object file` outside nix) — confirmed the SAME venv/pip-install/`import capnp` sequence works cleanly when the whole thing (venv creation, pip install, AND the later `python`/entry-point invocation) runs inside `nix develop $SRC_DIR --command bash -c '...'`, which resolves it via the devshell's own environment. Net: use a plain venv + real `pip install` (matching `.claude/skills/xce-build/SKILL.md`'s own documented "Installing the xceadmin wheel elsewhere" section) for throwaway/test installs where network is available and everything runs under `nix develop`; reach for the PYTHONPATH-entry's zipfile-extraction wrapper trick instead only for a permanent, no-nix, no-network deployment target (e.g. `/xce-deploy-ha`'s `scripts/xceadmin`).
|
||||
- **A pipeline into `head -N` can SIGPIPE-kill the upstream script early** under `set -o pipefail` (bit twice this session: once with `timeout N cmd | head -N` leaving a stale background process running for hours after the harness reported the task "complete", once with `xce-build.sh ... | head -3` silently truncating a build before it finished writing its wheel). If you need to sanity-check just the first few lines of a long-running command's output, redirect to a file and `head` the file afterward instead of piping directly into `head`.
|
||||
- **`sim_loop.c` (and its `*_isolation`-audited siblings) genuinely has zero line-budget headroom** — its own file header says so explicitly. When a fix needs new logic there, split a new sibling file (bug-1567's `sim_picker.c`, following the exact precedent chain `sim_loop.c`→`sim_record.c`→`repl.c`→`repl_break.c`/`repl_info.c`→`repl_params.c`) rather than growing it even slightly. This requires wiring the new file into: `scheduler_srcs` AND `sched_sim_isolation_lib` (both `src/scheduler/meson.build`), a new `XCE_SIM_*_PATH` macro (`testsuite/meson.build`) consumed by `test_sim_isolation.c`'s 3 per-file text-scan functions, AND every white-box test TU that `#include`s `sim_loop.c` directly (9 files this session — grep `#include "../src/sim_loop.c"` in `testsuite/` to find them all) needs the new file `#include`d too or the link fails on the new extern. Mechanical but real work — budget for it.
|
||||
- **`controller_unit_test`'s intermittent SIGSEGV (bug-1553, ~1-in-3 full runs) is a known pre-existing flake**, not a new regression — reproduce standalone (`./src/build.dbg/controller/controller_unit_test`, no `meson test` wrapper) a few times before suspecting your own change; `meson test`'s `MALLOC_PERTURB_` randomization per invocation is what makes it flaky, not deterministic per-build.
|
||||
- **bug-1568 (FIXED) had TWO independent, unrelated root causes stacked on the same symptom** ("info worker" showing 0.00 for real resource values) — worth remembering the general pattern: (1) `xce_core_push_events()`'s `WORKERS_PUSH` case called `sched_add_worker()` (a plain `key_add()`, add-only) unconditionally on every push; a worker connecting sends multiple pushes in quick succession, so any push after the first for an already-registered name silently -EEXIST-discarded its resource values — fixed via `core_upsert_worker()`, mirroring the existing `core_upsert_queue()` add-or-update precedent (`core_events.c`). (2) Separately, `repl_info.c`'s display code compared `xce_val_t.which` against `XCE_VAL_NUMERIC` (`xce/types.h` `#define`, value 1 — a *resource-definition* value-type constant, `xce_resource_t.valtype`'s own enum) instead of `Value_u_fvalue` (`controller.capnp.h`-generated, value 5 — the real tag every producer in this codebase actually sets on `xce_val_t.which`) — always false, so it displayed 0.00 regardless of whether (1) was fixed. **Lesson**: when value-level TRACE at every pipeline hop (push → apply → snapshot save → snapshot load/decode) all show the correct number but the final displayed/returned value is still wrong, the bug is one hop further than "the pipeline" — trace the DISPLAY/formatting code too, don't stop at the data layer. And don't assume two enum families with parallel-looking names (`XCE_VAL_*` vs `Value_u_*`) are interchangeable just because both appear near `xce_val_t`/`.which`/`.valtype` — check which one a given producer actually sets before trusting a comparison against it.
|
||||
- **`IFS=$'\t' read` (or any single-tab IFS) silently COLLAPSES adjacent/empty fields, just like the default whitespace IFS does** -- bash classifies tab (along with space/newline) as "IFS whitespace" for splitting purposes regardless of what IFS is actually set to, so `a\tb\t\tc\td` (an empty 3rd field) reads back as 3 fields (`a`,`b`,`c\td` collapsed) not 4, silently shifting every field after the gap. Found live building resource-value-verification task 7.1's `aggregate_verdict()` (`run-resource-verify.sh`) -- its own required synthetic self-test caught it immediately (an UNREACHABLE-layer record's empty `*_observed` field shifted every later field by one, corrupting the apiserver classification). Every OTHER tab-delimited TSV already in that file happens to never carry a genuinely-empty field, which is why this never surfaced before. Fix: when a delimited record format can legitimately have an empty field, use a non-whitespace-class separator (e.g. ASCII Unit Separator `$'\x1f'`) instead of tab -- `IFS=$'\x1f' read` does NOT collapse/strip, tab-like or space-like IFS always does.
|
||||
- **This Bash tool aborts a multi-line script at the first command returning nonzero**, even with `; true`/`|| true` on that exact line — observed repeatedly with `pkill` matching zero processes (a completely normal, expected outcome) silently killing every following line in the same call. Don't chain a possibly-zero-match `pkill`/similar with follow-on commands in one Bash call — verify via `pgrep` first (it's just as prone to this, so treat ITS nonzero "no match" the same way) or issue the pkill as its own standalone call and check the result before proceeding.
|
||||
- **`nix build` in this sandbox dispatches to a remote builder (`ssh://rshen@172.31.109.18`) that is NOT reachable from inside the sandbox** (2026-08-21, dirty-content-fingerprint task 6.3 review). `nix show-config`/`/etc/nix/nix.conf` inside the sandbox show `builders = ` (empty) — the remote dispatch is configured on the nix-daemon side (invisible to a `show-config` run inside the sandbox's mount/network namespace), not discoverable by inspecting client-side config. A plain `nix build` therefore either hangs/times out on SSH connect or falls back to the known bcctool/goproxy.cn failure without ever completing locally. **Fix: pass `--builders ''` to force a fully local build** (this sandbox has `max-jobs = 22`, plenty of local capacity) whenever running a real `nix build`/`nix develop`-triggered build for verification in this sandbox — don't assume remote-dispatch failure/unreachability is unavoidable or treat it as an automatic excuse to fall back to eval-only/log-only verification; try the local-builder override first, since it can produce a genuinely complete, runnable binary instead of just a build log.
|
||||
|
||||
- **apiserver's `main.go` only calls `RegisterToBackend` when `BackendYamlConfig.Address != ""` — checked BEFORE `Peers` is ever consulted** (`cmd/main.go:58`). Setting `app.yaml`'s `backend.peers` to a fully-populated HA candidate list while leaving `backend.address`/`backend.port` blank (reasoning "Peers takes priority inside RegisterToBackend anyway, per `backend/config.go`'s own doc comment") is NOT sufficient — the apiserver silently boots in permanent "stub mode" (`logger.Info("Backend not configured")`) and never even attempts a connection, `peers` or not. Caught live while building `xce-deploy-ha`'s new apiserver step (resource-value-verification task 3.1): always populate `address`/`port` too (e.g. the first `--peers` candidate's host:port) even when also setting `peers` — belt-and-suspenders, not redundant, because the two fields gate two different things (whether to connect at all, vs. which candidate(s) to try).
|
||||
- **resource-value-verification task 4.1's scheduler trace-log exact format** (`xce_core_push_events()`'s `XCE_CORE_EVENT_RESVALS_DELTA` case, `src/scheduler/src/core_events.c`, right after `zarray_put(&(w->resvals), res->idx, rv->val)`): `TRACE("resval applied host=%s resource=%s value=%f", ...)` for numeric (`Value_u_fvalue`) resources, or `value=%s` for `Value_u_svalue` ones — live-confirmed via `xce-deploy-ha` + `xce_test` worker-context `join`/`resupdate -n r15m -v 12345.5` producing exactly `resval applied host=task41-worker resource=r15m value=12345.500000` in `scheduler.log` at `-vvv`. Task 6.3 (scheduler-side log-tail check) MUST grep for `resval applied host=<H> resource=<R> value=<V>` — labeled `key=value` fields, no other fixed text to depend on. `%f` on a normal resource value produces trailing zeros (e.g. `12345.500000`), not the input's original decimal precision — task 6.3's comparison should parse-and-compare as float, not do a literal string match against the injected value string.
|
||||
|
|
@ -67,7 +73,21 @@
|
|||
- **`apply_committed_entry()`'s shared WAL_JOB_START/FINISH/STOP/RESUME/RUSAGE/CKPT/PREASON dispatch case (`persist.c`) never stamped `start_time`/`finish_time`/`run_time`** — it only ever called the generic, state-only `apply_job_update_transition(j, state)` (2026-08-19, `xcecli joblist` showed `StartTime`=0 for every RUN/DONE/EXIT job, `FinishTime`=0 for DONE jobs). Fixed additively (user's explicit steering: "don't change the logic, just add extra information") by keying a `time(NULL)` stamp on `wal_type` right after `apply_job_update_transition()` succeeds, still under the same `lock(j, RTTI_LOCK_WRITE)` — `WAL_JOB_START` → `start_time`, `WAL_JOB_FINISH` → `finish_time` + `run_time = finish_time - start_time`. No change to `apply_job_update_transition()`'s signature or the dispatch structure. Mirrors what the richer (currently-unreached-on-this-path) `apply_job_start_transition()`/`apply_job_finish_transition()` cores in `job.c` already do for their own callers.
|
||||
- **`test_config_repl.c`'s `test_conf_trim_keeps_only_n_versions`/`test_conf_get_saved_tag_no_config` failing with the exact same Expected/Was values plus a burst of `persist: flusher fdatasync FAILED (errno=5 Input/output error)` lines is bug-1561, a confirmed overlayfs-sandbox `fdatasync` artifact — 6 occurrences now across unrelated sessions/diffs, never once traced to a real code change.** If a `/xce-build controller` run shows exactly these 2 failures with that exact error pattern, it's environment noise, not something your change caused — don't spend time re-investigating, just note it and move on (though still bump bug-1561's `occurrences`/`last_seen` per buglog protocol).
|
||||
|
||||
- **Testing an "on a clean tree" scenario against a shared repo that's under continuous concurrent background editing (other agent sessions, OpenWolf hooks, etc.) cannot rely on the ambient working tree's `git status` ever actually being clean.** dirty-content-fingerprint task 6.1 (`src/tools/test-dirty-fingerprint-distinguishability.sh`, 2026-08-21) solved this by creating its own throwaway, fully-isolated `git worktree add --detach <tmp-dir> HEAD` before doing anything else — a fresh linked worktree checkout is clean by construction (zero uncommitted changes) and is a completely separate directory tree, so concurrent activity in the original shared worktree can never touch it. Bonus: `src/tools/xce-build.sh` resolves its own `SRC_DIR` from `$0`'s location and `cd`s there itself before running any `git` command — so simply invoking the ISOLATED worktree's own copy of `xce-build.sh` (not the shared tree's copy, and regardless of the caller's own cwd) automatically scopes every git command inside it (rev-parse/status/diff/ls-files) to the isolated worktree, with zero extra `-C`/`cd` bookkeeping needed by the caller. Clean up with `git worktree remove --force <dir>` (plain `rm -rf` alone leaves stale worktree metadata — needs a `git worktree prune` fallback too). General pattern: reach for an isolated `git worktree` (not `git stash`, not hoping the ambient tree happens to be clean) any time a test genuinely needs to observe a controlled clean/dirty state transition in a repo that isn't exclusively yours for the run.
|
||||
- **A `local a="$1" b="$a/something"` — two assignments in ONE `local` statement where the second references the first — is a real bug under `set -u`, not just bad style (bug-1615, found live writing the script above).** Bash expands every word of a single `local` command before ANY of that command's assignments take effect, so `$a` in the second word's value expands against the OLD (not-yet-declared) `a`, producing `iso_src: unbound variable` (or silent empty-string under plain `set -e` without `-u`). Fix: split into two separate `local` statements on their own lines. Same family as the earlier-logged `fp="$(cmd)"` under `set -e` gotcha (cerebrum, `xce-build.sh` dirty_fingerprint note) — another case where a single-line-looking assignment has a non-obvious evaluation-order trap under bash's strict-mode flags.
|
||||
|
||||
- **`builtins.currentTime` is entirely unavailable ("attribute 'currentTime' missing") during a flake's default pure evaluation — not merely fixed to a dummy value, the attribute is removed from `builtins` outright.** Confirmed via `nix eval` both ways: a bare `nix eval .#pkg.someAttr` (or `nix build`) referencing `builtins.currentTime` anywhere it gets forced fails at eval time; `nix eval --impure .#pkg.someAttr` succeeds and returns the real epoch. This is unrelated to the derivation *build* sandbox (`sandbox = false` in this machine's nix.conf) — it's Nix's flake-level pure-eval restriction on impure builtins, and it applies regardless of where in the file the reference sits (laziness only delays the error until the value is actually forced, e.g. by string-concatenating it into `mesonFlags`). `self.shortRev`/`self.dirtyShortRev` (the existing `commit_id` mechanism) are NOT affected — those come from flake input/git introspection that Nix computes specially even in pure mode, not from an impure builtin. **RESOLVED (bug-1603, 2026-08-20): task 2.2's first attempt used `builtins.currentTime` in `src/flake.nix` per research.md's original decision, and was REJECTED by review because `nix build .#xcecore_debug` (no flag, the documented canonical command) failed at evaluation exactly as predicted above.** Fixed by moving the computation entirely out of Nix evaluation and into each derivation's own build phase instead — see the `preConfigure`/`mesonFlagsArray` entry below. Nix-eval-time impure builtins should be treated as generally unusable for anything that must survive a plain `nix build`/`nix eval` (no `--impure`), full stop — don't reach for `builtins.currentTime` (or similar) in a flake output again; use a build-phase shell hook.
|
||||
- **nixpkgs' `meson.hook`/setup-hook.sh (`mesonConfigurePhase`, confirmed by reading the actual hook source in the locked nixpkgs tree at `pkgs/by-name/me/meson/setup-hook.sh`) runs `runHook preConfigure` FIRST, then builds its `meson setup` invocation via `concatTo flagsArray mesonFlags mesonFlagsArray`** — so a Nix derivation's `preConfigure` shell-hook attribute can compute a value at BUILD time (inside the sandbox, via stdenv's own bundled coreutils — no extra `nativeBuildInputs` needed) and `mesonFlagsArray+=("-Dfoo=$val")` it, and that flag reaches the real `meson setup` command line, all within the same shell process (stdenv's generic builder runs every phase/hook in one continuous bash script, no subshells between them) — confirmed `foo+=(...)` on a variable never declared as a Nix attribute (so genuinely unset, not just empty) still works fine even under stdenv's `set -eu` (`bash -c 'set -eu; foo+=("x"); echo "${foo[@]}"'` succeeds, auto-vivifies as an indexed array). This is the general pattern for "a nix package needs a value only knowable at build time, not eval time" (e.g. build-time-versioning task 2.2's `build_time`, fixed via this exact mechanism in `xce.nix`/`xcecli.nix` after `builtins.currentTime` broke pure-eval — see the entry above). Live-verified: `nix build .#xcecore_debug`'s build log (`nix log <drv>`) showed `mesonConfigurePhase flags: ... -Dbuild_time=2026-08-20T09:13:29Z` and meson's own summary printing `build_time : 2026-08-20T09:13:29Z`.
|
||||
- **nixpkgs.lib has no epoch-seconds→ISO8601 formatter** (checked `lib/*.nix` in the locked nixpkgs source tree directly — no `strings.nix`/`trivial.nix`/etc. function does this). This is now moot for `build_time` specifically (see above — it's computed via shell `date -u` in a build-phase hook, not in Nix expression code at all), but still true in general for any future nix-side ISO8601 need.
|
||||
- **`xce-build.sh` defaults `BUILD_DIR` to the same path (`xce.build`) regardless of component, and its non-`--incremental` `setup_build()` does `rm -rf "$BUILD_DIR"` + fresh `meson setup` on every invocation — two concurrent `xce-build.sh` runs for DIFFERENT components in the SAME working directory race on that shared path.** (bug-1604, 2026-08-20). Live-confirmed twice in one session: (1) a `scheduler asan` run's initial `rm -rf` hit `rm: cannot remove '.../controller_unit_test.p': Directory not empty` because a second, independently-running `xce-build.sh controller asan` process (same cwd, different session) was actively writing into `xce.build/` at that instant — simply retrying once the other process's own clean pass finished resolved it. (2) minutes later, the freshly-verified `xce_scheduler` binary (checked present + fresh mtime, `--version` run successfully) had VANISHED from `src/xce.build/scheduler/` — the same concurrent controller build's own `rm -rf` had wiped and was regenerating `xce.build/` for its component, silently invalidating the just-verified scheduler binary out from under the caller. **Before trusting any non-incremental `xce-build.sh` binary/mtime to still be valid, or before starting a clean build in a worktree another session might also be using, check `ps aux | grep xce-build.sh` for a concurrent run — different components sharing one worktree's default `xce.build` is a real, reproducible race, not environment flakiness (this was initially misdiagnosed as an overlayfs artifact before the concurrent process was found).**
|
||||
|
||||
- **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).
|
||||
- **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.
|
||||
|
||||
## 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.
|
||||
- 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.
|
||||
|
|
@ -81,9 +101,20 @@
|
|||
- **`xce_test`'s `!<shell-command>` escape (`preprocess_and_run()`, single call site in `main()`'s top-level REPL loop, `test.c` ~line 950-998, backed by a plain blocking `system()`) works IDENTICALLY in every REPL context, including `worker` — no `leave_ctx()`/disconnect happens, unlike moving between contexts via `..`/`cmd_up`.** This is the general pattern for "run an out-of-band check while a stateful REPL session must stay connected": bake a `!`-prefixed line into the script fed to the session (before its `quit`) that shells out to ANOTHER invocation of your own driver script in a hidden internal mode (e.g. `! bash "$0" --internal-<mode> <args>`), intercepted via a dispatch check placed BEFORE the normal arg-parsing entry point at the very bottom of the file. Used to fix bug-1579 (resource-value-verification): the controller-side resval check now runs nested inside the injecting worker session's own `!` escape instead of as a separate xce_test process launched after `quit` already disconnected the worker (which reset host-attrs resources to baseline first, an always-false MISMATCH). Gotchas: (1) the nested process must generally return exit 0 even on partial/expected failure conditions — a nonzero exit becomes the outer session's `g_last_status`/`$?`, and in non-interactive/batch mode `preprocess_and_run()` ABORTS the REPL loop before any later command (including `quit`) runs; surface real failures via a sentinel/marker file the OUTER script checks AFTER the whole session completes, not via the nested process's own exit code. (2) `system()`'s child inherits the invoking process's environment but NOT its parent bash's local (non-exported) shell variables — bake every value the nested invocation needs (paths, host lists, names) as literal, `printf '%q'`-quoted arguments directly into the `!` line's command string, don't rely on env-var passthrough through the `nix develop`/ASAN-wrapped intermediate hops. (3) since accept/reject-style bookkeeping computed by the REPL session itself is only visible as TEXT PRINTED to that session's own stdout — not accessible to a freshly-spawned nested process — have the nested step collect RAW, unfiltered data for every candidate (not just ones later known to be "accepted") and defer accept/reject filtering to the outer script, after it has parsed the whole session's captured output post-`quit`. (4) live-verified no orphan/zombie processes result even when a SIGTERM interrupts the outer driver script WHILE the nested `!`-escape's own sub-session is actively running — bash defers a pending trap until the current foreground command (the whole piped xce_test invocation, nested check included) returns, so the child process tree either completes and exits cleanly on its own or the trap fires right after; confirmed via a full `/proc/[0-9]*/cmdline` scan, not just `pgrep`.
|
||||
- **Piping scripted commands into `xce_test` (the controller REPL/testsuite binary) with default `ASAN_OPTIONS` loses ALL stdout output when redirected to a file/pipe.** `xce_test` is an ASAN build; `detect_leaks=1` is the default, and this binary has small pre-existing intentional leaks (e.g. `set_resval`'s `calloc`, `cmd_resupdate`'s `strdup` for `resname` — never freed by design, REPL-lifetime scratch data). LeakSanitizer's exit-time report appears to bypass/precede the normal stdio flush when stdout is non-interactive (fully block-buffered), so a redirected-output smoke test showed ONLY the ASAN leak report and none of the actual REPL command output (join/resupdate/resval results), even though the commands genuinely executed (confirmed via the leak stack trace itself). Fix: always run `xce_test` with `ASAN_OPTIONS=detect_leaks=0` (matching the meson test harness's own invocation, which already sets this) whenever piping/redirecting its stdout for a live smoke test — e.g. `nix develop ./src --command env ASAN_OPTIONS=detect_leaks=0 xce_test < cmds.txt`.
|
||||
- **A fork/subagent dispatched in a prior session that claimed "build/tests in progress, will report back" is NOT reachable in a new session — `ListAgents` will not show it, and its self-report (if logged to memory before the session ended) describes an *intent*, not a completed result.** 2026-08-19: a fork agent from the previous session was recorded as "actively writing tests... then building via xce-build skill and reporting final metrics," but the session ended before it reported. On resuming ("continue"), the scheduler test binaries in `src/xce.build` were still dated Aug 18 14:27 while the source edits were Aug 19 09:18-09:27 — proof the claimed build never actually happened in this environment. Always verify via binary/source `stat` mtime comparison (or just re-run the build) before trusting a cross-session "in progress"/"dispatched" claim, same discipline as the existing "stale/uncommitted tree GO claim" entry above but for cross-session fork continuity specifically, not just tree staleness.
|
||||
- **Go's `-ldflags -X importpath.name=value` is silently a no-op (not a build error, not even a printed warning) when the target `main` package has no such package-level string var declared** — confirmed live (build-time-versioning task 4.1, 2026-08-20): built `xcecli/cmd/main.go` (which does NOT yet declare `GitCommit`/`BuildTime`, those land in task 4.4/4.5) with `-ldflags "-X main.GitCommit=abc123 -X main.BuildTime=..."` via both a direct `go_build` invocation and the full `xce-build.sh xcecli` meson path — exit 0 both times, working ELF binary produced, `grep -i "warning\|no such"` on the full captured output found nothing beyond an unrelated nix git-dirty-tree line. This means task 4.1 (the `-ldflags` injection in `go_build`) has NO real ordering dependency on tasks 4.4/4.5 (the consuming vars) — safe to land and validate standalone, contrary to what a cautious reading of Go linker docs might suggest.
|
||||
- **A REPL/replay test fixture that reuses the SAME jobid for both a "real" event (job push) and a "cheap padding" event (job state change, added purely to exercise a second event type) accidentally mutates the real job's state, not just decorates the log.** bug-1595 test regression (task 12.5, 2026-08-19): `test_step_echoes_events_before_session_decisions` pushed job 6161 then sent a JOBSTATECHANGE event for the SAME jobid 6161 (PEND->RUN) purely to get a second event type echoed — this genuinely transitioned the real job to RUNNING before the session ran, so the picker's `next_job()` never offered it and the session's own PEND decision (which the test also asserted on) was never produced. The sibling test `test_event_breakpoint_pauses_repl_at_matching_event` already documents the correct pattern in its own comment (`append_jobstatechange_event() establishes: core_events.c WARN+skips an unknown jobid rather than fail, so ... below needs no real job to exist`) — it deliberately used unrelated jobids (111/222) precisely to keep the padding event inert. When adding a "just need any event of type X to be logged" filler event to a fixture that also has a real job in flight, always target an unrelated/nonexistent jobid unless the state transition itself is the thing under test.
|
||||
- **`[ -n "${arr[key]+x}" ]` (associative-array key-EXISTENCE) is NOT a safe proxy for "the value is meaningful/reachable" when the producer unconditionally sets that key even on failure.** bug-1582 (resource-value-verification task 7.1, round-1 REJECTED, round-2 fixed): `main()`'s record-building loop derived `sched_ok`/`api_ok` from whether `SCHEDULER_OBSERVED[$vname]`/`APISERVER_OBSERVED[$vname]` existed — but `query_scheduler_resvals()`/`query_apiserver_resvals()` (already-approved tasks 6.3/6.4) always set that key, setting it to `""` on a genuine UNREACHABLE/timeout rather than leaving it unset. Key-existence was therefore always true, silently masking every real UNREACHABLE as a MISMATCH (comparing the injected value against an empty string). Fix: derive the "is this layer reachable" signal from the layer's own pre-classified status string (`SCHEDULER_CHECK_RESULTS[$vname]`/`APISERVER_CHECK_RESULTS[$vname]`, which DOES distinguish `UNREACHABLE:*` from `MATCH`/`MISMATCH:*`) instead of from a sibling data array's key presence. General lesson: before using `${arr[k]+x}`/`-v arr[k]` as a "was this checked / did this succeed" signal, verify the producer actually leaves the key UNSET on failure — if it sets it to an empty/placeholder value instead (a common pattern for "always populate every requested key so downstream code doesn't need existence-checks"), key-existence collapses to "was this key in the requested set" a wholly different, weaker signal than "did this succeed". Also: a synthetic test that only calls the pure aggregation function directly with hand-built records (task 8.2's original approach) cannot catch a bug living in the data-marshaling code that BUILDS those records — this class of bug needs the marshaling code itself extracted into a directly-callable function (`build_verdict_records_from_tsv()`) and exercised by its own test.
|
||||
|
||||
- **This coding-agent harness's interactive shell aliases plain `grep` to a PCRE-capable (ugrep-backed) wrapper via a `.bashrc`-sourced shell FUNCTION — but that function is interactive-shell-only and is NOT inherited by a script invoked as `bash script.sh`, which falls back to whatever the real `grep` binary on PATH is (in this environment: BusyBox, no `-P` support at all).** (2026-08-20, build-time-versioning task 6.2): `echo x | grep -oP '(?<=x)y'` succeeded fine when typed directly as a Bash-tool command, but the identical `grep -oP` call inside a standalone `.sh` file, run via `bash thefile.sh`, failed with `grep: invalid option -- 'P'` (BusyBox usage dump). Confirmed via `type grep` in an interactive call (shows a `_cc_a`-prefixed shell function shelling out to the Claude Code binary itself acting as ugrep) vs. `which -a grep`/`/sbin/grep --version` showing BusyBox as the real underlying binary. **Any shell script file meant to be run standalone (not just typed inline as a Bash-tool command) must assume POSIX-only grep/sed (BRE, `-E`, `-F`, `-o`) — no `-P`/PCRE, no lookbehind/`\K`.** Use `sed -n 's/.../\1/p'` (POSIX BRE, works under BusyBox) instead of `grep -oP '...\K...'` for field extraction inside any committed script.
|
||||
- **A direct `go build` (even a real, working one) without `-mod=readonly` can silently rewrite a COMMITTED `go.sum`, adding unrelated indirect-dependency checksum lines as a side effect of resolving the full module graph — even though `go env GOFLAGS` was empty and go.mod itself was never touched.** (2026-08-20, build-time-versioning task 6.2): building `apiserver/cmd/main.go` directly (bypassing the network-hung `go_build` wrapper, see below) added ~65 new `/go.mod`-hash-only lines to `src/frontend/apiserver/go.sum` on the first attempt — caught only via a post-hoc `git status --porcelain` sweep, not by the build itself (exit 0, no warning). Go 1.16+'s `-mod=readonly` default blocks go.mod EDITS but does NOT by itself block go.sum EXTENSION for modules already implied by go.mod's requirements. Fix: pass `-mod=readonly` EXPLICITLY on every ad-hoc `go build` invocation that isn't supposed to touch repo state — confirmed live that the same build still succeeds under explicit `-mod=readonly` with zero go.sum diff, so nothing was genuinely missing; the extra entries were pure side effect. **Any script/agent that runs `go build` directly against a checked-out repo (not via the project's own wrapped build path) must always pass `-mod=readonly` and verify `git status --porcelain` on go.sum/go.mod afterward, every time, no exceptions.**
|
||||
- **`src/tools/go_build`'s hardcoded `GOPROXY=https://goproxy.cn` + a fresh-per-invocation `GOCACHE`/`GOPATH` under meson's own `$TMPDIR` sandbox can hang indefinitely in this environment — not fail fast, HANG: an established (not refused/reset) TCP connection to goproxy.cn:443 sits in `futex_do_wait` with near-zero CPU time for 8-10+ minutes across repeated attempts.** (2026-08-20, build-time-versioning task 6.2): `xce-build.sh apiserver <profile> <dir>` (which builds apiserver AND xcecli together via this wrapper) reliably hung on xcecli's fresh module downloads; confirmed via `ss -tnp` showing `ESTAB` to `8.45.176.179:443` (=goproxy.cn, independently reachable fine via plain `curl`/`/dev/tcp` from the SAME shell) with the `go` process's CPU time frozen — a genuinely stuck data path, not slowness, since it never progressed no matter how long left running. Root cause isolated to `go_build`'s own env overrides (`export GOCACHE=$TMPDIR/go-cache; export GOPATH=$TMPDIR/go; export GOPROXY=https://goproxy.cn`), which bypass the ALREADY-POPULATED default `$GOPATH/pkg/mod` cache (`go list -m all` from an ordinary `nix develop` shell, no wrapper, resolved apiserver's full module graph in under a second, zero downloads). **Workaround (do NOT modify go_build/Makefile — this is this specific sandbox's networking quirk, not a real bug in those files): invoke `go build` directly with the default module cache (`GOPROXY=off` is fine since everything needed is already cached) plus the same CGO_CFLAGS/CGO_LDFLAGS pkg-config derivation `test-backend`/apiserver's own meson.build already uses (see bug-1597's documented fix) whenever a real apiserver/xcecli build is needed standalone in this sandbox and `xce-build.sh apiserver`/`make build-server` hang or fail.** Do not blindly increase the timeout and retry — a truly stuck TCP data path does not resolve itself no matter how long you wait; diagnose via `ss -tnp`/`/proc/<pid>/wchan` first.
|
||||
- **Refinement to the above (dirty-content-fingerprint task 4.4, 2026-08-21)**: when the Go-side change under test lives ONLY in `apiserver/cmd/main.go` (or another `cmd`-level file) and doesn't need the real cgo backend, skip bug-1597's full CGO_CFLAGS/CGO_LDFLAGS pkg-config reconstruction entirely — `apiserver/backend/` already ships a `//go:build !cgo` stub (`stub.go`) alongside every `//go:build cgo` file, so `CGO_ENABLED=0 GOPATH=/home/claude/go GOPROXY=off go build ./cmd/main.go` (matching the Makefile's own `test` target convention, `GOFLAGS := CGO_ENABLED=0`) compiles cleanly with zero C dependencies and produces a real, runnable binary — confirmed live across `go vet`, `go test`, and 5 distinct `-ldflags`-injected `-version` scenarios plus a real (if CGO-stub) daemon-start log capture. Only reach for the full CGO reconstruction when the task actually needs the real backend/libxce integration exercised (e.g. bug-1597's own `build-server` scenario). `-mod=readonly` was NOT needed this time (checked `git status --porcelain` on go.sum/go.mod after every build — zero diff both times), likely because the module graph was already fully resolved in `/home/claude/go/pkg/mod` from prior sessions; still worth checking every time per the entry above, since whether it triggers depends on cache state, not code.
|
||||
|
||||
- **This sandbox's `nix build` (not `nix develop`) dispatches to a REMOTE builder over SSH (`ssh://rshen@172.31.109.18`), which is NOT reachable directly via SSH from inside this sandbox (connection times out — same network restriction family as the goproxy.cn DNS failures).** (2026-08-21, dirty-content-fingerprint task 6.3): confirmed via `nix build`'s own stderr ("The failed build directory was kept on the remote builder due to `--keep-failed`... re-run with `--builders ''` to disable remote building"). Practical consequence: `--keep-failed`'s leftover build directory (containing real, successfully-linked binaries from a partially-failed multi-target derivation) is NOT locally inspectable/runnable without either `--builders ''` (forces a full local rebuild from scratch — expensive, full ASAN tree) or SSH access to that remote host (unavailable here). When a task needs evidence from a `nix build` that can't fully succeed (e.g. the known bcctool/goproxy.cn limitation), prefer `nix eval` (fast, eval-time, no builder involved) cross-checked against the real build LOG's `mesonConfigurePhase flags:` line (via `-L`) rather than trying to extract-and-run an actual compiled artifact — this is also literally what task 2.2 already did.
|
||||
- **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.
|
||||
|
||||
## 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.
|
||||
- A controller process must NEVER self-exit/self-abort as a designed response to an operational condition (only a genuine crash or admin/supervisor kill) — an unrecoverable/fenced member logs loudly, stays permanently non-promotable, stays alive, and keeps retrying.
|
||||
|
|
@ -92,4 +123,7 @@
|
|||
- Product is pre-release: no backward/mixed-version wire compatibility required; wire format changes may be made freely.
|
||||
- `resource-value-verification` task 7.1 (2026-08-18): `aggregate_verdict()`/`build_verdict_record()` (`run-resource-verify.sh`) re-derive MATCH/MISMATCH/UNREACHABLE from raw `*_OBSERVED` values via `controller_values_match()` at aggregation time, rather than trusting each layer's already-rendered `CONTROLLER_CHECK_RESULTS`/`SCHEDULER_CHECK_RESULTS`/`APISERVER_CHECK_RESULTS` strings from tasks 6.2-6.4 -- deliberate, so the SAME classification code path is exercised by both the live-run wiring and task 8.2's synthetic-record tests (a synthetic test that only fed pre-classified strings would just be testing string formatting, not the comparison logic). A rejected resource name is modeled as its own failure category (`REJECTED`, layer=`injection`) distinct from a per-layer `MISMATCH`/`UNREACHABLE`, and always forces overall FAIL, per design.md's Error Handling ("unrecognized resource name is a run-level failure") -- this is NOT implied by Requirement 6.2/6.3 alone (those only speak to "checked layers"), it's a deliberate design-level extension. The script's bottom dispatch block is now guarded (`[[ "${BASH_SOURCE[0]}" == "${0}" ]]`) so it can be `source`d to get `aggregate_verdict()` et al. in scope with zero side effects (no `main()`/deployment triggered) -- this is the intended integration point for task 8.2's dedicated test harness.
|
||||
- `resource-value-verification` task 7.1 round 2 (2026-08-18, bug-1582, round-1 REJECTED -- see Do-Not-Repeat entry for the mechanism): `main()`'s OWN record-building loop (feeding `aggregate_verdict()`, distinct from `aggregate_verdict()`/`build_verdict_record()` themselves, which were and remain correct) had the actual bug -- `sched_ok`/`api_ok` derived from `*_OBSERVED` array-key existence instead of the `*_CHECK_RESULTS` classification strings, masking every genuine scheduler/apiserver UNREACHABLE as MISMATCH. Fixed by switching to a leading-`UNREACHABLE:` check on `SCHEDULER_CHECK_RESULTS`/`APISERVER_CHECK_RESULTS` (the controller layer's `ctrl_ok` derivation from `CONTROLLER_OBSERVED` array-key existence was independently confirmed fine and left untouched -- that layer has no separate UNREACHABLE concept). Also extracted the loop into a standalone `build_verdict_records_from_tsv()` function (populates global `VERDICT_RECORDS`) specifically so a synthetic test can exercise `main()`'s actual data-marshaling code, not just `aggregate_verdict()` called with hand-built records -- the round-1 gap was that no test exercised the code that BUILDS records from the `*_OBSERVED`/`*_CHECK_RESULTS` globals, only the pure aggregation function itself. Live-reproduced by killing the scheduler process right after `confirm_deployment_up()` returns (before `inject_resources()`/`query_scheduler_resvals()` ever run) -- deterministic, no race window, and confirmed the controller/apiserver layers are unaffected by an isolated scheduler kill (each layer's check is independent).
|
||||
- `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.
|
||||
- `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."
|
||||
Loading…
Add table
Add a link
Reference in a new issue