docs: log JSON-reformat gotcha for buglog.json edits in cerebrum.md
Recorded as a Do-Not-Repeat: editing buglog.json via json.load/dump without ensure_ascii=False silently reformats every existing entry's unicode punctuation into \uXXXX escapes, ballooning the diff. Caught and fixed forward twice already this session (once by me, once by a dispatched task-5 implementer) - documenting so future task subagents that touch buglog.json don't repeat it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
This commit is contained in:
parent
f4c91babba
commit
48c6cfd58a
1 changed files with 1 additions and 0 deletions
|
|
@ -184,6 +184,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.
|
- **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.
|
- **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.
|
- **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-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.
|
- **(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.
|
- **`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.
|
- **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.
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue