fix(gitea-sync): survive live Gitea replies; point project memory at /buglog
gitea-sync.py fixes found on the first live run (bug-1855 follow-up): - 204/empty-body replies (label DELETE) no longer crash in json.loads - every request has a 60s timeout, so a stalled call fails instead of hanging - spec.json sync state is inserted as a trailing "gitea" block instead of re-serializing the file (15 hand-formatted spec.json files would reflow) - implementation issue body labels generated vs approved correctly; the validation body uses the same done-phase set as the close rules Project memory now routes all buglog writes through the buglog skill: CLAUDE.md key rule, a cerebrum Do-Not-Repeat entry (with the older duplicate-id / json.dump / tags-vs-fix lessons tagged as superseded), and xce-audit-invariants step 5. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
19104f1940
commit
e915f8def4
GPG key ID:
80533536B0EB651C
4 changed files with 53 additions and 20 deletions
|
|
@ -268,9 +268,12 @@ several minutes per mode.
|
|||
|
||||
## Step 5 — Record and report
|
||||
|
||||
- Append every genuinely new root-caused bug to `.wolf/buglog.json` (id,
|
||||
error_message, file, root_cause, fix, tags, related_bugs) — check the
|
||||
existing entry count first so the new `bug-NNNN` id doesn't collide.
|
||||
- Log every genuinely new root-caused bug with the `buglog` skill:
|
||||
`python3 scripts/buglog.py add --error ... --root-cause ... --file ...
|
||||
--tags <component>,... --fix "<what changed>" --status fixed` (or `open` /
|
||||
`partial` with `--note` for what remains). It assigns the id; don't pick one
|
||||
by hand. For a violation that an existing entry already covers, update that
|
||||
entry instead: `scripts/buglog.py status <id> ...` / `note <id> ...`.
|
||||
- Append durable, non-obvious learnings to `.wolf/cerebrum.md`'s Key
|
||||
Learnings section (anchor the edit near an existing dated entry to avoid
|
||||
the "## Key Learnings" heading collision — the file has one per dated
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
|
||||
> OpenWolf's learning memory. General, reusable knowledge only — bug-specific history lives in .wolf/buglog.json, not here.
|
||||
> Do not edit manually unless correcting an error.
|
||||
> Last updated: 2026-08-16 (condensed from a ~40k-token file; see .wolf/buglog.json for full bug history)
|
||||
> Last updated: 2026-09-24 (condensed from a ~40k-token file; see .wolf/buglog.json for full bug history)
|
||||
|
||||
## User Preferences
|
||||
- Run soak tests with specific round counts and deploy directories for precise regression tracking.
|
||||
|
|
@ -107,7 +107,7 @@
|
|||
|
||||
- **(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-09-24: `scripts/buglog.py add` now assigns max id + 1, so new entries can't reuse a taken id; the historical duplicates noted here remain.]** **(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.
|
||||
|
|
@ -117,10 +117,10 @@
|
|||
|
||||
- **(2026-08-25, bug-1686) A lock that correctly protects a data structure DURING a single critical section does NOT protect a raw pointer CACHED across SEPARATE lock acquisitions from going stale if freed by another properly-locked call in between — mutual exclusion prevents concurrent corruption, not cross-call pointer lifetime bugs.** `builtin_fcfs.c`'s `fcfs_plugin_data_t.lock` (the bug-1611-era concurrency fix) makes every individual hook call — `on_queue_update()`, `next_job()`, etc — atomic with respect to each other; that part works. But `priv->cursor`/`priv->job_cursor` are raw `ordq_item_t*` values that PERSIST in `fcfs_plugin_data_t` across separate `next_job()` calls, each taking and releasing the lock independently. A later, individually well-locked `on_queue_update()` call (removing-then-reinserting a queue's node to re-sort it — `ordq_get` + `FREEUP(it)` + `ordq_put`) can legitimately free the exact node a cursor from an EARLIER, already-completed `next_job()` call is still resting on; the very next `next_job()` call then dereferences that freed node — a genuine heap-use-after-free, no concurrency/threading required to trigger it (single-threaded, sequential calls reproduce it deterministically, confirmed live under ASAN). Fixed by NULLing whichever cursor points at a node right before that node is freed, at every site that frees/removes a node the cursor might reference (`on_queue_update`'s reuse branch, `on_job_state_change`'s `!JOB_IS_PEND` removal, `fcfs_remove_job_everywhere`) — safe because `next_job()` already treats a NULL cursor as "exhausted" and `begin_session()` resets both fresh every round, so invalidation costs at most a one-round delay, never a lost job or a crash. **Rule: any struct field that is a raw pointer INTO a lock-protected structure, and that OUTLIVES the single critical section that produced it (i.e., read under one lock acquisition, dereferenced under a LATER, separate one), is a latent stale-pointer bug regardless of how correctly the lock itself is used — audit for "cache a pointer, release the lock, reacquire the lock later, dereference the cached pointer" as its own category, distinct from and not fixed by the mutex being correctly held everywhere.** Same file as bug-1611's original concurrency fix (`builtin_fcfs.c`) — worth re-checking any other "cursor"/"cached node"-shaped field in this codebase for the identical cross-call pattern.
|
||||
|
||||
- **(2026-08-25, bug-1687) This worktree can have MULTIPLE concurrent agent sessions actively editing it at once — always re-check `.wolf/buglog.json`'s max id and `git status` for files you never touched immediately before writing, not just once at session start.** Mid-session, `.wolf/buglog.json`'s max id moved from 1685 to 1686 (a different session's `builtin_fcfs.c` SIGSEGV fix, bug-1686) between my first check and my write — required a bulk `sed -i 's/bug-1686/bug-1687/g'` across 6 already-edited files (job.c, persist.c, persist.h, wal_types.h, the new test file, and a test-fixture fix) to renumber every in-code comment reference before writing the buglog entry. Also: `git status` mid-session showed `src/scheduler/src/plugins/builtin_fcfs.c`/`.wolf/cerebrum.md`/`.wolf/buglog.json` as modified by that OTHER session, and a full top-level `ninja`/meson build can fail on THEIR uncommitted, unrelated changes even when your own target component builds fine — that other session's own bug-1686 entry documents hitting the mirror image of this same collision (my job.c/persist.c changes broke ITS full-tree `ninja` build). **Never assume `.wolf/*` files or the working tree are exclusively yours in this environment; re-verify immediately before any write that depends on "current state" (next bug id, clean baseline, etc), and scope builds/tests to your own component when a full-tree build fails on files you didn't touch.**
|
||||
- **[2026-09-24: use `/buglog` / `scripts/buglog.py add`, which reads the max id at write time. Still don't pre-bake a guessed id into code comments before the entry exists: run `add` first, then use the id it prints.]** **(2026-08-25, bug-1687) This worktree can have MULTIPLE concurrent agent sessions actively editing it at once — always re-check `.wolf/buglog.json`'s max id and `git status` for files you never touched immediately before writing, not just once at session start.** Mid-session, `.wolf/buglog.json`'s max id moved from 1685 to 1686 (a different session's `builtin_fcfs.c` SIGSEGV fix, bug-1686) between my first check and my write — required a bulk `sed -i 's/bug-1686/bug-1687/g'` across 6 already-edited files (job.c, persist.c, persist.h, wal_types.h, the new test file, and a test-fixture fix) to renumber every in-code comment reference before writing the buglog entry. Also: `git status` mid-session showed `src/scheduler/src/plugins/builtin_fcfs.c`/`.wolf/cerebrum.md`/`.wolf/buglog.json` as modified by that OTHER session, and a full top-level `ninja`/meson build can fail on THEIR uncommitted, unrelated changes even when your own target component builds fine — that other session's own bug-1686 entry documents hitting the mirror image of this same collision (my job.c/persist.c changes broke ITS full-tree `ninja` build). **Never assume `.wolf/*` files or the working tree are exclusively yours in this environment; re-verify immediately before any write that depends on "current state" (next bug id, clean baseline, etc), and scope builds/tests to your own component when a full-tree build fails on files you didn't touch.**
|
||||
- **(2026-08-25, bug-1687) When a WAL/wire fact type's payload grows past a previously-fixed size (e.g. adding trailing bytes to what used to be a bare fixed-size struct — the bug-1678/bug-1687 pattern), grep the WHOLE testsuite for any test helper that manually parses the raw WAL segment file and matches entries by an EXACT payload-length equality (`hdr.payload_len == sizeof(fixed_struct)`) — such a helper will silently start UNDER-COUNTING (returning 0 instead of the real count) the instant the payload legitimately grows, and if any OTHER test polls/waits on that count reaching a value, the undercounting can manifest as a genuine, reproducible DEADLOCK far downstream (all threads futex-blocked, near-zero CPU accrual) rather than a clean assertion failure at the actual call site. Found via `cbrp_count_jobstate_entries_ex()` in `test_commit_barrier_reply.c` (`hdr.payload_len == sizeof(wal_jobstate_t)` — fixed to `>=`, since jobid/state always live at the same fixed leading offset regardless of trailing bytes). This is a DIFFERENT, more dangerous failure mode than bug-1678 ever hit (that fix's own tests never counted WAL_JOB_DISPATCH via this exact helper, so it went unnoticed) — always grep for `sizeof(wal_jobstate_t)`-style exact-match parsers project-wide whenever changing a WAL fact's payload shape, don't just trust the direct callers/apply seams to be the whole blast radius.
|
||||
- **(2026-08-25, bug-1689) The bug-1687 apply-seam pattern (split a shared-payload WAL fact type out of `apply_wal_entry()`'s shared `case` group into its own case, extend `apply_committed_entry()`'s existing inline `else if (wal_type == X)` branch, add a `restore_job_<type>_state()` sibling in job.c, add a `persist_submit_job_<type>_fact()` sibling in persist.c/persist.h) generalizes cleanly to a THIRD job-fact type (WAL_JOB_FINISH) with one twist: not every such fact needs Cap'n Proto.** `job_exitinfo_t` (`xce/types.h`) is a single `{int exitcode;}` with no existing capnp mapping (confirmed via grep — no `JobExitInfo` in `controller.capnp`), so the trailing payload was a small NATIVE (raw memcpy) fixed-size trailer instead of a capnp-encoded blob — no malloc needed either, since the whole payload is fixed-size (unlike WAL_JOB_START's variable-length JobPIDInfo or WAL_JOB_DISPATCH's JobAlloc). General rule going forward: before reaching for capnp on a new WAL trailing payload, check whether the struct being carried already has a capnp mapping AND has any variable-length/nested-pointer fields — if it's a small fixed POD with neither, a raw `memcpy`'d native trailer (mirroring `wal_jobstate_t`'s own style) is simpler and correct, matching this codebase's own "small native trailing struct" precedent for `wal_jobstate_t` itself. Also discovered mid-fix: `apply_job_finish_transition()` (the "richer" core that DOES set exit_info/finish_time/run_time correctly) is effectively DEAD CODE on the real production apply path today — raft-apply-decoupling's redesign routes all real WAL_JOB_FINISH mutation through the leaner `apply_job_update_transition()` (state-only) instead, so `exit_info` was never populated through the live commit-apply pipeline at all, restart or not, prior to this fix (worse than "only breaks on restart"). When auditing a "richer transition function exists but the apply seam uses a leaner one" pattern (already documented for WAL_JOB_START in bug-1687's persist.h comment), always check whether the richer function has ANY live caller at all, not just whether it's correct.
|
||||
- **(2026-08-25, bug-1689) Reconfirms the bug-1687 "multiple concurrent sessions write `.wolf/buglog.json`" lesson with a fresh, independent collision — re-check the max id immediately before EVERY write, every time, not just once per session.** Checked max id at session start (1687, free), implemented the full fix using "bug-1688" throughout code comments/test file, then re-checked right before writing the buglog entry (per this project's own mandated protocol) and found bug-1688 had been claimed by a sibling session's unrelated scheduler REPL bug in the interim — required a `sed -i 's/bug-1688/bug-1689/g'` across all 5 touched files before writing the entry as bug-1689. Zero cost when caught by re-checking as instructed; would have been a duplicate-id buglog corruption if skipped.
|
||||
- **[2026-09-24: same remedy as above: `scripts/buglog.py add` first, then use its id.]** **(2026-08-25, bug-1689) Reconfirms the bug-1687 "multiple concurrent sessions write `.wolf/buglog.json`" lesson with a fresh, independent collision — re-check the max id immediately before EVERY write, every time, not just once per session.** Checked max id at session start (1687, free), implemented the full fix using "bug-1688" throughout code comments/test file, then re-checked right before writing the buglog entry (per this project's own mandated protocol) and found bug-1688 had been claimed by a sibling session's unrelated scheduler REPL bug in the interim — required a `sed -i 's/bug-1688/bug-1689/g'` across all 5 touched files before writing the entry as bug-1689. Zero cost when caught by re-checking as instructed; would have been a duplicate-id buglog corruption if skipped.
|
||||
- **(2026-08-25, bug-1687) `ptrace` is fully unavailable in this sandbox — `gdb -p <pid>` fails with "Operation not permitted" even with `dangerouslyDisableSandbox: true`, and `/proc/<pid>/task/*/stack` is also unreadable (no permission).** The only process-introspection signal available without ptrace is `/proc/<pid>/task/*/{wchan,status,comm}` (shows which kernel function a thread is blocked in, e.g. `futex_do_wait`, and its state, e.g. `S (sleeping)`) plus `ps -o pid,etime,time` (wall-clock ELAPSED vs. CPU TIME — a process with large ELAPSED but near-zero TIME is genuinely blocked/deadlocked, not slow-computing; this distinction is what first proved the bug-1687 hang was a real deadlock and not just a slow test run). To actually isolate WHICH test in a monolithic Unity binary is hanging when `-t <group>` only filters at group-function granularity (see the existing entry on this): temporarily insert an early `return;` inside the group's own driver function (e.g. `test_commit_barrier_reply()`) right after a candidate `RUN_TEST()` call, rebuild+run with a bounded `timeout N`, and binary-search by moving the `return;` — each step is a full, clean process exit (no ptrace needed) so the log's last line IS trustworthy for that step (unlike a genuinely-hung run, where stdio is fully-buffered when redirected to a file/pipe and the last visible log line can be UP TO ~4KB of buffered-but-unflushed output stale relative to where the hang actually occurred — do not trust "last line before a real hang" as the hanging test's identity without this bisection). **After finishing this kind of diagnostic bisection, grep the group's own driver function for accidentally-DUPLICATED `RUN_TEST()` calls before considering the file clean** — inserting new `RUN_TEST()` lines ahead of a temporary `return;` marker and then later deleting only the marker (not the inserted lines) leaves the original, further-down calls to the same tests still present, silently double-running them (caught here via `git diff` showing a nonzero `+RUN_TEST`/`-RUN_TEST` net and a suspiciously-inflated total test count).
|
||||
- **(2026-08-25, bug-1691) `/tmp` in this shared sandbox can silently fill to 0 bytes free across many unrelated sessions, because `mkdtemp()`-based test fixtures across this whole codebase only `rm -rf` their scratch dir on the NORMAL end-of-test path — any run aborted mid-way by ENOSPC (or a crash, or a killed process) leaves its 64-128MB scratch dir behind forever, compounding: fuller disk -> more runs abort early -> more orphans -> fuller disk still.** Found live: a build+test run failed with `OSError: [Errno 28] No space left on device`, both from meson's own JSON/XML report writer AND, worse, from genuine mid-test `mkdtemp failed`/spurious `Expected Non-NULL` failures in UNRELATED test files once disk hit 0 during actual test execution — that second failure mode produces a CORRUPTED, untrustworthy pass/fail signal, not just a build-tool crash, so a run showing many unrelated failures right after an ENOSPC condition should be re-run after fixing disk, never trusted as real regression evidence. Root cause found via `ls -1 /tmp | grep -c '^xce_'`: 15,437 orphaned `/tmp/xce_*` scratch directories (~658GB) had accumulated from prior sessions. Verified none were held open by any live process first (`for p in /proc/[0-9]*; do ls -la "$p/fd" | grep <pattern>; done` — empty output = safe to delete; a live `xce-deploy-*` HA cluster's OWN workdir, e.g. `/tmp/xce-deploy-1679`, DOES show up in a live process's cmdline/cwd and must NOT be deleted). Fix: `rm -rf /tmp/xce_*` recovered 658GB instantly (359MB free -> 659GB free). **Whenever a build/test run in this sandbox fails with ENOSPC or `df -h /` shows near-0 available, check for this exact pattern before assuming it's a real disk-capacity problem or someone else's live process** — it is very likely safe, accumulated test-fixture debris, not a resource genuinely in use.
|
||||
- **(2026-08-25, bug-1691) A shared-fixture footgun found copy-pasting a sibling WAL-fact test file's "drive the real production entry point through a real restart cycle" test: `persist_init(&c.persist, &cfg, NULL)` (the exact line every prior sibling test — bug-1687/1689's START/FINISH suites — uses) is WRONG for a hook-driven fact type, even though it is byte-identical to what those working sibling tests do.** `persist_start()` only registers `persist_job_hook_fun`/`persist_queue_hook_fun`/`persist_worker_hook_fun` on `p->cluster->hooksys` when `p->cluster != NULL` (persist.c). DISPATCH/START/FINISH's production functions call `persist_submit_job_*_fact()` DIRECTLY, bypassing the hook system entirely, so those sibling tests' `NULL` 3rd-arg is harmless. But CKPT (like bug-1690's RUSAGE) is HOOK-DRIVEN: `on_job_ckpt()` only ever calls `xce_run_hooks(..., HOOK_DETAIL_JOB_CKPT, ...)` and relies ENTIRELY on `persist_job_hook_fun` being a registered subscriber to reach the WAL at all. With `NULL` passed, the real-call test silently appends NOTHING (no crash — `xce_run_hooks` on an unregistered detail is simply a no-op), so the restart-replay assertion fails with "ckptinfo NULL" even though the fix code itself was already fully correct — a genuine RED-signal red herring that looks exactly like a fix defect. Diagnosed by noticing the 6 hand-built-payload tests all passed (proving the apply-seam decode/adopt logic itself was right) while ONLY the real-production-call test failed, then re-reading `persist_start()`'s actual gating condition rather than assuming the copied fixture was correct because it was copied from a passing sibling. **General rule: before copying a sibling WAL-fact test file's "drive the real production function" fixture, check whether YOUR fact type's producer calls `persist_submit_job_*_fact()` directly (safe to pass NULL) or is purely hook-driven via `xce_run_hooks()` (MUST pass a real `cluster_t*` to `persist_init()`'s 3rd arg so `persist_start()` registers the hook) — grep the producer function's own body, don't assume parity from a structurally-similar sibling test.**
|
||||
|
|
@ -150,14 +150,15 @@
|
|||
- **(2026-08-26) To produce a clean "before/after" pair of binaries for live E2E verification when OTHER unrelated files have uncommitted in-flight changes you must not touch or rebuild against**, use `git stash push -- <only the files for this fix>` (explicit pathspec, NOT a bare `git stash`) to isolate exactly the fix's own diff — rebuild for the "before" binary, `git stash pop` to restore the fix, rebuild again for the "after" binary. This leaves every other uncommitted file (the unrelated in-flight `ha.c`/`job.c`/`persist.c` work in this session) completely undisturbed throughout, unlike a bare `git stash` which would have stashed everything. For a PYTHON-only fix specifically, this same before/after swap works without any rebuild at all — just `cp` the two scheduler.py variants in and out of `src/xceadmin/src/xceadmin/` and re-run the SAME editable-install venv's `xceadmin` entry point; a `pip install -e` venv reads live from the source tree, no reinstall needed between swaps.
|
||||
- **(2026-08-26, bug-1713) In this sandbox's local-loopback 3-node deploy, the FIRST-BOOT election-settling window varies wildly run to run (sometimes near-instant, sometimes ~10-12s via a term-1 election-timeout-then-PreVote-failover fallback path) — a fixed "wait for start-all.sh's health-check, then fire N requests" tight loop is NOT a reliable way to catch a transient `E_XCE_NOT_READY`/`E_XCE_NOMASTER` window: by the time start-all.sh's own 1s-granularity health-check first observes `CTRL_PRIMARY` in a log and returns, `ha_has_write_quorum()`'s `standby_endpoints` (populated synchronously inside `raft_cb_become_leader` from the raft library's own connected-voter set) is typically ALREADY satisfied, so the gate is already open. A REAL FAILOVER (kill -9 the current primary, then hammer the survivors with NO sleep, no health-check wait) reproduced the exact live symptom reliably instead — got a genuine bare `-10013` at ~t+8.6s post-kill on an unfixed client, and confirmed zero such failures surfaced across the equivalent window on the fixed client. The bug's own root-cause writeup already named this as an equally-valid trigger ("...or any time a real failover is in flight") — prefer the failover trigger over racing first-boot timing when a live repro is needed and cold-boot timing proves flaky.
|
||||
- **(2026-08-26) A hand-rolled multi-connection mock TCP/TLS server in a test file needs a genuinely FINITE accept loop (`for handler in handlers:`), not `while self._handlers:` guarding a list that's never mutated — the latter is an always-true condition that leaves the server thread blocked in `accept()` indefinitely once the test's real client stops connecting, which does not fail the test but silently adds `thread.join(timeout=N)`'s FULL timeout to every such test's teardown (measured: 3 tests each added a flat 5s of pure teardown wait with zero effect on pass/fail).** Match the mock's handler-list length exactly to the number of connections the test actually expects (assert on `connection_count` for the "stops on its own budget" property instead of over-provisioning handlers "just in case") and additionally set a short `socket.settimeout()` on the listener as a safety net so a hypothetical future regression that causes MORE connections than expected still fails within one timeout window instead of silently reintroducing the same multi-second stall.
|
||||
- **(2026-09-19, `/xce-audit-invariants scheduler` after `scheduler-sim-live-allocation`) A buglog entry's `tags` array can say `"fixed"` while its own `fix` text says `"Not yet fixed"` — a self-contradiction that survives silently unless something actually re-reads the entry, because most tooling in this project checks tags, not the narrative field.** Found on bug-1682 (and its unnoticed duplicate, bug-1492 — same defect, `plugin_end_session()` guarding on `begin_session_fn` instead of `end_session_fn`, independently filed twice 7 weeks apart because bug-1681's investigation never cross-referenced the earlier entry) via an audit subagent that read BOTH the code and the buglog text, not just the tags — it caught that `plugin.c:441` was still visibly broken despite the tags claiming otherwise. **Lesson 1**: when closing out a large feature and bulk-stamping buglog entries as fixed/superseded, verify each stamped entry's `fix` narrative actually matches the tag before writing it — a tag-only update is a claim with no evidence behind it. **Lesson 2**: before filing a new bug for a defect discovered "while fixing X," grep buglog.json for the exact symptom text first (e.g. `grep -i "begin_session_fn.*end_session_fn"`) — this exact one-line guard/call mismatch was independently re-discovered and re-filed as a "new" bug once already in this project's history.
|
||||
- **[2026-09-24: open/closed now lives in each entry's `status` field (`.claude/rules/buglog-status.md`), not in `tags` or `fix` text.]** **(2026-09-19, `/xce-audit-invariants scheduler` after `scheduler-sim-live-allocation`) A buglog entry's `tags` array can say `"fixed"` while its own `fix` text says `"Not yet fixed"` — a self-contradiction that survives silently unless something actually re-reads the entry, because most tooling in this project checks tags, not the narrative field.** Found on bug-1682 (and its unnoticed duplicate, bug-1492 — same defect, `plugin_end_session()` guarding on `begin_session_fn` instead of `end_session_fn`, independently filed twice 7 weeks apart because bug-1681's investigation never cross-referenced the earlier entry) via an audit subagent that read BOTH the code and the buglog text, not just the tags — it caught that `plugin.c:441` was still visibly broken despite the tags claiming otherwise. **Lesson 1**: when closing out a large feature and bulk-stamping buglog entries as fixed/superseded, verify each stamped entry's `fix` narrative actually matches the tag before writing it — a tag-only update is a claim with no evidence behind it. **Lesson 2**: before filing a new bug for a defect discovered "while fixing X," grep buglog.json for the exact symptom text first (e.g. `grep -i "begin_session_fn.*end_session_fn"`) — this exact one-line guard/call mismatch was independently re-discovered and re-filed as a "new" bug once already in this project's history.
|
||||
|
||||
## Do-Not-Repeat
|
||||
- **(2026-09-24) Never hand-edit or re-serialize `.wolf/buglog.json` to log a bug or change its status; use the `buglog` skill (`scripts/buglog.py add | status | note | recur | list | show | triage`).** Each entry's `status` (open / partial / investigating / fixed / cannot-reproduce / wontfix / duplicate, defined in `.claude/rules/buglog-status.md`) is the only source of truth for open/closed. Never infer it from `fix` text or `tags`; both drifted (bug-1772, bug-1682), and gitea-sync closed unfixed bugs by trusting a non-empty `fix` (bug-1855). `add` picks max id + 1. OpenWolf's hook uses entry count + 1, which collides, so its auto-detection is off (bug-1856). After a fix commit: `scripts/buglog.py status <id> fixed --note <sha>`.
|
||||
- **A TDD-implementer SUBAGENT (spawned by a kiro-impl coordinator) has NO external wakeup mechanism — arming a `Monitor` tool call or a `Bash run_in_background` call and then ending the turn to "wait for a notification" just stops the subagent forever; the coordinator has to manually resume it.** (2026-08-26, xce-test-rc-file task 1.2, recurred across multiple tasks in this same spec before being called out explicitly.) Only the top-level session genuinely receives async task-notification events between turns; a subagent's turn ending IS the end of its execution for that dispatch. When a validation command (e.g. `controller_unit_test`) risks exceeding the default 120-300s Bash timeout, pass an explicit large `timeout` (e.g. 300000-600000ms) to a PLAIN FOREGROUND `Bash` call and let the tool call itself block until real output returns — never `run_in_background`, never `Monitor`, never end a turn on an unfinished command in this role. If a command is auto-moved to background because the timeout already elapsed, immediately re-run it in the foreground with a longer timeout rather than trying to "wait" for the backgrounded one.
|
||||
- **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."
|
||||
- **[2026-09-24: start from the entry's `status` (`scripts/buglog.py show <id>`), then verify against code/commits as below.]** 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.
|
||||
- When two independent "is it ready" signals exist server-side, a wait/convergence helper gating on only one does not imply the other is satisfied — check what the actual raced precondition depends on.
|
||||
|
|
@ -185,7 +186,7 @@
|
|||
- **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.
|
||||
- **Editing `.wolf/buglog.json` (or any large existing JSON file with unicode em-dashes/arrows in prose fields) via Python's `json.load()`/`json.dump()` reformats the ENTIRE file, not just the new entry — `json.dump`'s default `ensure_ascii=True` rewrites every existing `—`/`→` character anywhere in the file into a `\uXXXX` escape, producing a diff hundreds of lines larger than the actual change.** (2026-09-01, worker-rejoin-resync task 4.1 self-caught, then recurred anyway in a dispatched implementer's own task-5 edit before being caught and fixed forward as its own commit): appending one ~30-line bug entry this way produced a 400+-line diff touching ~400 unrelated pre-existing entries. Always pass `ensure_ascii=False` to `json.dump()` when writing back a file that may contain non-ASCII prose — or better, avoid `json.load`/`json.dump` round-tripping entirely for a pure append (insert the new entry as raw text right before the file's closing `]\n}`) so untouched entries are never re-serialized at all. If this recurs a third time, brief every implementer prompt that touches `buglog.json` with this exact gotcha up front rather than relying on self-catching it after the fact.
|
||||
- **[2026-09-24: `scripts/buglog.py` edits only the touched entry's lines; use it instead of json.load/json.dump.]** **Editing `.wolf/buglog.json` (or any large existing JSON file with unicode em-dashes/arrows in prose fields) via Python's `json.load()`/`json.dump()` reformats the ENTIRE file, not just the new entry — `json.dump`'s default `ensure_ascii=True` rewrites every existing `—`/`→` character anywhere in the file into a `\uXXXX` escape, producing a diff hundreds of lines larger than the actual change.** (2026-09-01, worker-rejoin-resync task 4.1 self-caught, then recurred anyway in a dispatched implementer's own task-5 edit before being caught and fixed forward as its own commit): appending one ~30-line bug entry this way produced a 400+-line diff touching ~400 unrelated pre-existing entries. Always pass `ensure_ascii=False` to `json.dump()` when writing back a file that may contain non-ASCII prose — or better, avoid `json.load`/`json.dump` round-tripping entirely for a pure append (insert the new entry as raw text right before the file's closing `]\n}`) so untouched entries are never re-serialized at all. If this recurs a third time, brief every implementer prompt that touches `buglog.json` with this exact gotcha up front rather than relying on self-catching it after the fact.
|
||||
- **(2026-08-26) Before treating a fresh HA deploy's anomalous behavior (term jumping impossibly fast, a standby permanently failing to load config, a node never converging with peers) as a real product bug, check for and kill ANY leftover controller/scheduler/worker processes from EARLIER in the SAME session — a same-session soak/investigation left running and forgotten (`ps aux | grep xce_controller`) can bind the exact same default ports (`9991`/`9992`/`9993`, this project's `/xce-test-ha` skill default) a brand-new deploy also uses.** Concretely: an abandoned `xce-ha-chaotic2` 3-node cluster from an earlier bug-1696 investigation, still running hours later, was still bound to 9991-9993 when a fresh `run-ha-soak.sh --sequential` deploy was launched at the same default ports; the new cluster's vote traffic cross-talked with the ancient zombie (which had organically reached a high term over its own long runtime), producing an apparently-impossible term jump (term=1→23 within 50ms of a genuinely fresh first boot) and a standby that could never load cluster_config for the whole test — both fully explained once the zombies were found (`ss -tlnp | grep -E "999[123]"` showing FOUR processes across two unrelated deploys all bound to the same three ports) and killed; a clean re-run on the exact same code, same binaries, same script passed cleanly (`PASS: 2 rounds, 12 jobs, zero job loss; all 3 nodes led`). **Always run `ps aux | grep -i "xce_controller\|xce_scheduler\|xcecd"` (and `ss -tlnp` for the ports you're about to use) before launching a "fresh" HA deploy in this sandbox, especially after any earlier soak/investigation in the same session — a `/xce-test-ha` skill soak that "completed" or whose agent died without explicit cleanup does not guarantee its processes actually exited.** This directly extends the earlier bug-1487-era lesson ("a chaotic-soak verdict is not self-verifying, cross-check the raw WAL") one level further back: an anomalous LIVE symptom from a fresh-looking deploy is not self-verifying either until same-session process/port contamination is ruled out first.
|
||||
- **`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.
|
||||
|
|
@ -246,5 +247,5 @@
|
|||
- **When a fix needs to peek/seed then undo a resource-accounting baseline (e.g. "temporarily charge every already-running job so a capacity check sees real usage, then undo it before returning"), do NOT reach for the SAME by-name accounting entry point (`sched_account_jobs()`) an existing test elsewhere specifically instruments and call-count-asserts as a protected invariant ("must be called exactly once per round").** Doubling that exact symbol's call count broke `test_sim_contention_parity.c`'s `g_round_idx` mechanism — and the failure was initially confusing because a coincidental fixed-size counter array (`g_round_resinuse[8]`) CAPPED the observed count, making a 2-round fixture's true delta (+2, correctly proportional) look identical to a 5-round fixture's true delta (which should have been +5 but was capped at the array's own ceiling, also showing "+2" after the cap) — always check whether an assertion's input value could itself be silently clamped by an array bound before concluding two different-sized reproductions "coincidentally" show the same delta. Fix: build a small `static` local helper in the SAME file doing the identical `sched_charge_account()`/`sched_uncharge_account()` iteration, deliberately NOT calling the protected symbol by name, so the call-count invariant elsewhere stays intact while the underlying accounting effect is identical.
|
||||
- **Adding a genuine admission/capacity check to test-owning code can turn dozens of PRE-EXISTING tests red for a completely mechanical, expected reason: their fixtures name a resource/host in a mock allocation without ever registering a real object of that name, because nothing previously validated it.** 12 tests across 2 files broke this way when bug-1836's fix added a `sched_get_worker(hostname) == NULL` check — each one just needed a real (or, in a file that stubs the checker entirely and never even reaches the worker lookup meaningfully, a bare/minimal) worker registered for whichever hostname its fixture already referenced, with generous capacity (these tests were about ownership/dedup/timing mechanics, not capacity limits). This class of breakage is a sign the new check is doing its job, not a sign the check is wrong — verify each failure's root cause individually before assuming a batch of new failures means the production fix itself is flawed.
|
||||
- **(2026-09-23, bug-1843/1844/1845/1772, `/xce-audit-invariants scheduler`) When a prior session's buglog entry already proposes a root-cause hypothesis for a `--sim-verify` mismatch, EMPIRICALLY RE-VERIFY it against a real captured `.replay` file before extending the fix — a plausible-sounding hypothesis inherited from a prior investigation can be flatly wrong, and trusting it wastes effort fixing the wrong layer.** bug-1843's prior session concluded the mismatch was `sim_loop.c`'s event-batching-granularity (a job's first appearance landing one round early in replay). Adding temporary per-round diagnostic `WARN()` logging (reverted after use, never committed) and replaying the ORIGINAL capture the bug was filed against instead showed the true mechanism was the opposite layer entirely: `sched_record_session_add_decision()` (recording_session.c) silently truncated a round's decisions on a `realloc()` failure, with nothing marking the round incomplete — a live-recorder data-loss bug, not a replay-side timing artifact. The same instrument-first discipline then found THREE more distinct `--sim-verify` false-positive classes in one session, each with a different, narrow, well-precedented fix: `instid` (bug-1844, a bare per-process monotonic counter with no reason to match between two separate process runs — same treatment as `from`, bug-1842), PEND reason host/resource DETAIL (bug-1845, sim mode's documented "simulation isolation" from the unsatisfiable-cache, `core.h`'s own comment, means live can record a cache-hit's sparse reason while replay always computes full detail — relax the comparator to `main_reason` only, drop the deep list-compare), and a genuinely separate, still-OPEN resource-accounting divergence (bug-1846) that instrumentation ruled OUT the first two mechanisms for but hasn't yet root-caused. **General pattern for this specific comparator (`sim_verify_alloc_eq`/`sim_verify_job_preason_eq`, sim_verify.c): before comparing ANY field between a live-produced and a recorded-then-replayed value, ask whether that field is semantic content (job/host/resource identity, requested amount) or per-process/per-run BOOKKEEPING (a monotonic counter, a wall-clock stamp, a cache-driven diagnostic-detail level) — only the former belongs in the comparison.** Every fix in this family followed strict TDD with a REAL red→green cycle (temporarily revert just the production fix, rebuild, confirm the new test fails, restore, rebuild, confirm the full suite is green) rather than trusting a passing test written after the fix.
|
||||
- **A buglog entry's own `tags` field can silently claim "fixed"/"closed-by-..." while its `fix` text still says "NOT FIXED" — the two fields can drift independently, and either can be wrong.** bug-1772 (snapshot-restored jobs never computing `resreq_fp`) carried both `"fixed"` and `"closed-by-scheduler-sim-live-allocation"` tags from an earlier session, but its `fix` field still read "NOT FIXED -- logged for its own follow-up" and direct code inspection confirmed the gap was still live (`snapshot_build_primary_schedobj()` had no `resreq_fp` assignment at all). Independently re-found by both a dedicated dimension-4 audit subagent and direct manual investigation into bug-1845's cache-key mechanism, before either checked the buglog for a pre-existing entry. **When investigating a suspected-new bug in an area with buglog history, grep the buglog for the SAME symbol/function BEFORE writing a fresh entry — a "new" finding may be a previously-deferred bug whose stale tags say "fixed" while its own fix text says otherwise; trust the `fix` text and current code over `tags` alone, and correct the stale tags once genuinely fixed rather than leaving the contradiction for the next session to rediscover.**
|
||||
- **[2026-09-24: superseded by the `status` field, the single source of truth for open/closed.]** **A buglog entry's own `tags` field can silently claim "fixed"/"closed-by-..." while its `fix` text still says "NOT FIXED" — the two fields can drift independently, and either can be wrong.** bug-1772 (snapshot-restored jobs never computing `resreq_fp`) carried both `"fixed"` and `"closed-by-scheduler-sim-live-allocation"` tags from an earlier session, but its `fix` field still read "NOT FIXED -- logged for its own follow-up" and direct code inspection confirmed the gap was still live (`snapshot_build_primary_schedobj()` had no `resreq_fp` assignment at all). Independently re-found by both a dedicated dimension-4 audit subagent and direct manual investigation into bug-1845's cache-key mechanism, before either checked the buglog for a pre-existing entry. **When investigating a suspected-new bug in an area with buglog history, grep the buglog for the SAME symbol/function BEFORE writing a fresh entry — a "new" finding may be a previously-deferred bug whose stale tags say "fixed" while its own fix text says otherwise; trust the `fix` text and current code over `tags` alone, and correct the stale tags once genuinely fixed rather than leaving the contradiction for the next session to rediscover.**
|
||||
- **(2026-09-21, bug-1838) When a design revision removes a secondary decision-producing shortcut so more work routes through "the real path," don't assume every test file's "real path" IS real — some test files deliberately stub the exact mechanism you're now relying on, for reasons unrelated to what you're testing.** Predicted two failing tests' root cause as "fixture never registered a real worker" (correct pattern from the bug-1836 experience immediately above) — actually wrong for both: `test_sim_verify.c` and `test_sim_contention_parity.c` each stub `sched_job_picker()`/`sched_allocator()` to a deliberately PEND-only pair, used to keep those files' OTHER tests decoupled from real dispatch logic. Narrowing `sim_reconcile_unaccounted_jobs()`'s trigger to stop force-applying a PEND job's recorded decision meant those fixtures' jobs now correctly stay PEND forever (nothing in that file can ever dispatch them) — the fix wasn't adding a missing worker, it was inverting the test's own assertion to lock in the new, correct behavior ("still PEND, `--sim-verify` correctly reports the resulting MISMATCH, since this minimal harness has no real capacity-tracked path to reproduce a historical dispatch"). **Lesson: before assuming a test failure needs a "make the fixture more realistic" fix, grep the failing test's OWN file for whether the specific mechanism your change now depends on (a picker, an allocator, a resource check) is the REAL production implementation or a same-named local override/stub — a same-file `grep -n "^void\* sched_job_picker\|^int sched_check_host_satisfy"` etc. before touching the test.** A prior, adjacent, and still-correct architectural point (a design doc's own "Live/Recorded Authority Rule" can be stated but never actually enforced by the code's real ordering) is a separate, valid finding from this same investigation — don't conflate "the design doc says X should happen" with "therefore the live mechanism in THIS test file is capable of making X happen."
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ For details on any topic below, read the corresponding steering file in `.kiro/s
|
|||
- **Middleware chain** (non-negotiable): WhitelistURL → WhitelistIP → RateLimit → JWT → CORS → RBAC
|
||||
- **Error pattern**: `ReturnError(code, msg, nil); return` — early return on every error
|
||||
- **Build**: `/xce-build [component]` — debug + ASAN + tests. Components: `all` (default), `lib`, `controller`, `harness`, `scheduler`, `worker`, `apiserver`, `xcecli`, `xceadmin`. See `build-deployment.md` for full commands
|
||||
- **Buglog**: log and update bugs only through the `buglog` skill (`scripts/buglog.py add | status | note | recur | list --open | triage`), never by hand-editing or re-serializing `.wolf/buglog.json`. Each entry's `status` is the only source of truth for open/closed (values in `.claude/rules/buglog-status.md`); after a fix commit run `scripts/buglog.py status <id> fixed --note <sha>`
|
||||
- **Save/restore field-completeness**: any persistence mechanism (WAL fact, scheduler snapshot, checkpoint) must capture every field of the entity it claims to persist — a state that "recovers correctly" while silently missing a field it depends on (e.g. `job.state` correct but `job.allocs` lost) is a bug, not an acceptable simplification, unless the omission is explicitly documented as a scope decision (see `persistence.md`'s "Snapshot Save/Load Field-Completeness" + `APPLY_CLEAN`/bug-1678 sections). When adding or extending save/restore logic, verify the field round-trips through BOTH the write side and every read/apply seam, and run `/xce-audit-invariants` (controller dimension 7 / scheduler dimension 4) rather than assuming a passing test suite already proves it — `APPLY_CLEAN`-style verdicts check row counts and cursor position, not per-field completeness
|
||||
|
||||
## Key Files
|
||||
|
|
|
|||
|
|
@ -223,8 +223,10 @@ def request(method, path, body=None):
|
|||
req.add_header("Accept", "application/json")
|
||||
|
||||
try:
|
||||
with urllib.request.urlopen(req) as resp:
|
||||
return resp.status, json.loads(resp.read().decode("utf-8"))
|
||||
with urllib.request.urlopen(req, timeout=60) as resp:
|
||||
raw = resp.read().decode("utf-8")
|
||||
# DELETE (and some PUT/POST) answer 204 with an empty body.
|
||||
return resp.status, (json.loads(raw) if raw.strip() else None)
|
||||
except urllib.error.HTTPError as e:
|
||||
if e.code == 404:
|
||||
return None, None
|
||||
|
|
@ -240,6 +242,8 @@ def request(method, path, body=None):
|
|||
raise RuntimeError(
|
||||
f"Gitea API unreachable: {method} {path}\n{e.reason}"
|
||||
) from e
|
||||
except TimeoutError as e:
|
||||
raise RuntimeError(f"Gitea API timed out: {method} {path}") from e
|
||||
|
||||
|
||||
# ── GiteaAPI ─────────────────────────────────────────────────────────────────
|
||||
|
|
@ -493,9 +497,11 @@ def spec_implementation_body(spec_name, spec_data, tasks_path):
|
|||
parts.append("")
|
||||
|
||||
# Phase status
|
||||
tasks_approved = spec_data.get("approvals", {}).get("tasks", {}).get("approved", False)
|
||||
tasks = spec_data.get("approvals", {}).get("tasks", {})
|
||||
ready = spec_data.get("ready_for_implementation", False)
|
||||
parts.append(f"**Tasks generated**: {tasks_approved}")
|
||||
parts.append(f"**Phase**: {spec_data.get('phase', 'unknown')}")
|
||||
parts.append(f"**Tasks generated**: {tasks.get('generated', False)}")
|
||||
parts.append(f"**Tasks approved**: {tasks.get('approved', False)}")
|
||||
parts.append(f"**Ready for implementation**: {ready}")
|
||||
parts.append("")
|
||||
|
||||
|
|
@ -539,10 +545,9 @@ def spec_validation_body(spec_name, spec_data):
|
|||
if v.get("generated") and not v.get("approved")
|
||||
]
|
||||
|
||||
if "complete" in phase or (
|
||||
approvals.get("implementation") and
|
||||
approvals["implementation"].get("approved")
|
||||
):
|
||||
if phase == "superseded":
|
||||
parts.append(f"**Status**: Superseded by {spec_data.get('superseded_by', '?')}")
|
||||
elif phase in DONE_PHASES:
|
||||
parts.append("**Status**: ✅ Complete")
|
||||
elif open_phases:
|
||||
parts.append(f"**Status**: ⏳ Pending — awaiting: {', '.join(open_phases)}")
|
||||
|
|
@ -715,7 +720,7 @@ def sync_spec(api, spec_name, dry_run=False, quiet=False):
|
|||
"milestone_id": milestone_id,
|
||||
"issues": phase_issues,
|
||||
}
|
||||
_write_json(spec_json_path, spec_data)
|
||||
_append_top_level_key(spec_json_path, "gitea", spec_data["gitea"])
|
||||
|
||||
|
||||
def _sync_spec_phase_states(api, spec_name, spec_data, gitea_state,
|
||||
|
|
@ -919,6 +924,29 @@ def sync_bugs(api, dry_run=False, quiet=False, since=None):
|
|||
f".claude/rules/buglog-status.md: {' '.join(needs_status)}")
|
||||
|
||||
|
||||
def _append_top_level_key(path, key, value):
|
||||
"""Add `key` as the last member of a JSON file's top-level object without
|
||||
re-serializing the rest (spec.json files use hand-formatted inline arrays
|
||||
that a json.dumps rewrite would reflow). Falls back to a full rewrite when
|
||||
the key already exists or the file doesn't end in a plain '}' line."""
|
||||
text = path.read_text()
|
||||
data = json.loads(text)
|
||||
lines = text.rstrip("\n").split("\n")
|
||||
if key in data or lines[-1] != "}":
|
||||
data[key] = value
|
||||
_write_json(path, data)
|
||||
return
|
||||
body = json.dumps(value, indent=2, ensure_ascii=False).split("\n")
|
||||
last = len(lines) - 2
|
||||
if lines[last].strip() and not lines[last].rstrip().endswith(("{", ",")):
|
||||
lines[last] = lines[last].rstrip() + ","
|
||||
lines[-1:] = [f" {json.dumps(key)}: {body[0]}"] + [" " + b for b in body[1:]] + ["}"]
|
||||
out = "\n".join(lines) + "\n"
|
||||
if json.loads(out) != {**data, key: value}:
|
||||
raise RuntimeError(f"{path}: surgical write did not round-trip")
|
||||
path.write_text(out)
|
||||
|
||||
|
||||
# ── JSON Writing ─────────────────────────────────────────────────────────────
|
||||
|
||||
def _write_json(path, data):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue