Rongsong Shen
d26ff8870d
The buglog was one 27K-line JSON file whose fields covered only symptom,
root cause and fix, as free text: 2% of entries named the code version a
bug was found on, 12% the build profile, 36% a way to reproduce, and 58% of
the file was OpenWolf hook auto-detections ("Missing guard clause",
"Function not marked async" on .md files).
- .claude/skills/buglog/bug-record.md: the record standard. Eight
sections (Summary as the title, Description, Environment, Reproduction;
Root cause, Fix, Verification, References once fixed), each with the
content it must contain -- e.g. Environment needs the OS, the libc the
failing binary loads (from its ELF interpreter: glibc 2.42 in the nix
shell vs 2.40 on the host), toolchain, build profile and commit.
- .wolf/bugs/<id>.md: 500 records migrated from buglog.json; every field
verified to round-trip. They carry legacy: true and an explicit "not
recorded" where the old log had no such field; component derived from
file paths, severity from keywords (else unknown), fix_commits from
hashes in status_note/fix that git resolves (91 of 463 fixed).
- .wolf/buglog-archive.json: the 710 hook auto-detections, verbatim.
- .wolf/buglog.json is now an index generated from the records, keeping
the fields OpenWolf's hooks and `openwolf bug search` read.
- scripts/buglog.py rewritten: new (template + Environment collected from
the machine and --binary), check, status (refuses a status the record
doesn't support; fixed needs Root cause, Fix, Verification and
--commit hashes git resolves), note, recur, set, show, list, triage,
env, index. 29 unittest cases in scripts/tests/test_buglog.py.
- scripts/git-hooks/pre-commit runs `check --staged`: incomplete staged
records, a legacy record hand-changed to fixed without the fix-time
sections, a new record claiming legacy: true, or a stale index refuse
the commit.
- scripts/gitea-sync.py reads the records: issue body = the record's
sections plus linked fix commits (origin remote, GITEA_COMMIT_REPO to
override), and a body hash so an issue's text is refreshed when the
record changes.
- bug-1866 rewritten to the full standard as the worked example.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
5.1 KiB
| id | status | status_updated | status_note | component | severity | timestamp | last_seen | occurrences | files | tags | related | legacy | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| bug-1408 | fixed | 2026-09-24 | controller | security | 2026-07-03T21:00:00.000Z | 2026-09-19T09:00:00Z | 1 |
|
|
true |
bug-1408: raft-apply-decoupling task 3.1 round-1 handle_jobsubmit(): when an array job submission has >1…
Description
raft-apply-decoupling task 3.1 round-1 handle_jobsubmit(): when an array job submission has >1 element and ONE element fails post-commit construction (create_job()/makeJob_() returns NULL, e.g. a jobid collision that slipped past jobid_range_exists() pre-check TOCTOU, or a plain calloc failure), the function returned the construction error IMMEDIATELY after the per-element construct loop, before ever building the AFTER_EVENT notify jlist or firing HOOK_DETAIL_JOB_NEW -- even for SIBLING array elements that constructed successfully and are already durably committed on a majority.
Environment
Not recorded (migrated from buglog.json).
Reproduction
Not recorded (migrated from buglog.json).
Root cause
handle_jobsubmit()'s post-commit construction loop set construct_rc<0 on any failed create_job() call but used continue (not break) to keep constructing remaining siblings correctly -- however, immediately AFTER the loop, "if (construct_rc < 0) { reply error; return; }" fired BEFORE the jlist-build + xce_run_hooks(HOOK_AFTER_EVENT, HOOK_DETAIL_JOB_NEW) block, so a single failed element suppressed notification for every successfully constructed sibling in the same array submission. Both the scheduler and apiserver are purely event-driven off this hook with no periodic reconciliation, so the stranded siblings (real, durable, present in c->jobmgr) were invisible until the next controller restart replayed the WAL.
Fix
Moved the construct_rc<0 early-return in handle_jobsubmit() (src/controller/job.c) to AFTER the jlist-build + xce_run_hooks(HOOK_AFTER_EVENT, HOOK_DETAIL_JOB_NEW, ...) block instead of before it. The jlist-build loop already only finds jobs that key_find() locates in c->jobmgr, so a failed-to-construct element is naturally and silently absent from jlist with no extra bookkeeping -- the hook now fires unconditionally for every successfully constructed element regardless of a sibling's construct_rc, and the client-facing reply still reports the construct_rc error afterward. Added regression test test_jobsubmit_array_partial_construct_failure_notifies_survivors in src/controller/testsuite/test_commit_barrier_reply.c: drives handle_jobsubmit with a 2-element array on a worker pthread, ACKs element 0's WAL entry, then deterministically reproduces the TOCTOU outcome by inserting a stand-in job directly into c->jobmgr for element 1's jobid (via the existing cbrp_make_job helper) strictly after jobid_range_exists() has already run and strictly before the construction loop can run (element 1's commit is still pending on a withheld ACK) -- makeJob_()'s own key_find() then legitimately returns EEXIST for element 1. A new test hook capture (cbrp_jobnew_capture_fn, registered via xce_add_hook) records every jobid delivered through HOOK_AFTER_EVENT/HOOK_DETAIL_JOB_NEW so the test can assert element 0 was both constructed AND notified despite element 1 failing. RED-phase confirmed against the pre-fix code (job 9201 construction failed with errno=17/EEXIST as designed, and the notification assertion for job 9200 failed as predicted). CORRECTION (round-2 review): under the standard verification invocation (--run test_commit_barrier_reply, the full 25-test group), the failing TEST_ASSERT_TRUE_MESSAGE at test_commit_barrier_reply.c longjmps past this test's own inline cbrp_teardown() call (Unity TEST_ABORT semantics) -- the SAME longjmp-past-teardown pathology already documented for round 1's withheld-commit test, not a clean assertion FAIL. It leaks live persist/commit-engine background threads into the next test's setup, producing a deterministic HANG under the full-group command (reproduced 3/3 times, both threads blocked in futex_do_wait). A clean, fast FAIL was only observed when this test was isolated from the other 24 tests in the group -- that isolated form is what the original write-up described. GREEN after the fix (reordering removes the failure entirely, so this hang mode never triggers in the fixed code): both pre-existing single-element jobsubmit tests and the full 25-test test_commit_barrier_reply group plus the full controller_unit_test binary pass (0 failures) via the standard --run invocation. Hardened afterward: the test now captures its assertion-relevant booleans (cbrp_job_exists/cbrp_jobnew_notified) into locals and calls cbrp_teardown() unconditionally before asserting on the captured locals, so a future regression in this exact test can no longer skip cleanup and hang sibling tests.
Verification
Not recorded (migrated from buglog.json).
References
Not recorded (migrated from buglog.json).