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.4 KiB
| id | status | status_updated | status_note | component | severity | timestamp | last_seen | occurrences | files | tags | related | legacy | |||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| bug-1693 | fixed | 2026-09-24 | controller | hang | 2026-08-25T14:05:00Z | 2026-09-19T09:00:00Z | 1 |
|
|
|
true |
bug-1693: Invariant audit (Decision Log fidelity dimension) found two violations of the recorded Decision…
Description
Invariant audit (Decision Log fidelity dimension) found two violations of the recorded Decision Log entry "No client reply-wait timeout as a fix for a hung request -- fail loud at the real transport/source instead of masking with a deadline backstop": (1) OPCode_schedAdmin's forward-and-block-for-reply (cluster.c) blocks up to SCHEDADMIN_TIMEOUT_MS=5000ms via pthread_cond_timedwait, but handle_scheduler_leave() (sched.c) -- the actual scheduler-disconnect handler -- only ever nulled c->sched.ep; it never signalled the pending waiter, so a scheduler disconnect mid-wait could only ever be observed at the full 5s deadline. (2) on_fetchfile's identical proxy pattern (job.c, 10s pthread_cond_timedwait) had the same gap: handle_worker_leave() (worker.c) only ever nulled w->ep, never woke a pending fetchfile waiter proxied to that worker.
Environment
Not recorded (migrated from buglog.json).
Reproduction
Not recorded (migrated from buglog.json).
Root cause
Both sites already had a working signal function used on the SUCCESS path (schedadmin_signal() in cluster.c, fetchfile_signal() in job.c, both wake the pending condvar wait when a real reply arrives) -- but the corresponding DISCONNECT handlers (handle_scheduler_leave/handle_worker_leave) were never wired to call them. cluster.c's own OPCode_schedAdmin comment explicitly documents schedAdmin as mirroring on_fetchfile's proxy pattern, confirming this was one shared, repeated design mistake copied into both sites, not two independent bugs. The timeout backstop was therefore the SOLE failure-detection mechanism for a mid-wait disconnect at both sites, exactly what the Decision Log entry (itself the outcome of bug-646/648/650's original fail-loud fix) forbids.
Fix
Kept the timeout as a defensive backstop (per the Decision Log's exact wording: the rule is about not relying SOLELY on a timeout, not removing timeouts) and added an immediate-wake path on disconnect at both sites, mirroring bug-646/648/650's original fail-loud precedent. schedAdmin site: new int schedadmin_signal_disconnect(void) (cluster.c) -- a thin wrapper calling the existing static schedadmin_signal(-ECONNRESET, NULL) -- declared extern in cluster.h; handle_scheduler_leave() (sched.c) now calls it BEFORE acquiring c->sched.lock (calling it after would deadlock, since the OPCode_schedAdmin dispatch case holds c->sched.lock for its entire forward+wait duration; schedadmin_signal_disconnect() itself never touches c->sched.lock, only g_schedadmin_mu + the pending slot's own mu, so no new lock-ordering hazard). cluster.c's post-wait dispatch gained a new got_status==-ECONNRESET branch relaying a structured SchedAdminReply{error="scheduler disconnected"}, distinguishable from both the immediate "scheduler not connected" rejection and the deadline's "request timeout". fetchfile site: fetchfile_pending_t (job.c) gained a worker_t* worker field (set in on_fetchfile() while still holding the worker's read lock, before fetchfile_register()); new void fetchfile_signal_worker_disconnect(worker_t*) (job.c, declared extern in cluster.h) walks g_fetchfile_list under g_fetchfile_mu and signals every pending entry matching the disconnected worker with status=-ECONNRESET (unlike fetchfile_signal, which is keyed by a single jobid, more than one fetchfile request can be in flight against the same worker); handle_worker_leave() (worker.c) calls it right after nulling w->ep (no lock-ordering constraint there, since on_fetchfile's wait never holds any worker_t lock). Hidden fan-out cost discovered only at link time (matches this project's established pattern for this class of change): controller_unit_test does not link the real sched.c at all (test_config_repl.c already provides its own no-op handle_scheduler_leave() stub, to avoid an unrelated on_scheduler_join()/notify() link collision with other files in that binary) -- so the primary schedAdmin regression test drives schedadmin_signal_disconnect() directly (the real, unmodified, unstubbed production function in cluster.c) rather than through handle_scheduler_leave(), and a SEPARATE small spy-based test was added in test_sched_worker_change.c (which DOES link the real sched.c) proving handle_scheduler_leave() itself calls schedadmin_signal_disconnect() exactly once, before clearing c->sched.ep. Also had to add a one-line link-satisfying schedadmin_signal_disconnect() stub to test_job_array_sched_push.c and test_sched_worker_change.c (the two standalone binaries that link sched.c without cluster.c), mirroring their existing check_worker_() stub precedent.
Verification
Not recorded (migrated from buglog.json).
References
- Related: bug-646, bug-648, bug-650