myxce/.wolf/bugs/bug-1750.md
Rongsong Shen d26ff8870d
feat(buglog): one Markdown record per bug, checked against a record standard
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>
2026-09-28 03:40:11 +00:00

4.8 KiB

id status status_updated status_note component severity timestamp last_seen occurrences files tags related legacy
bug-1750 fixed 2026-09-24 worker unknown 2026-09-01T09:00:00Z 2026-09-19T09:00:00Z 1
src/worker/src/controller_connector.c
worker-rejoin-resync
task-4.3
xcecd_resync-c
controller_connector-c
requirement-6.1
requirement-3.8
deadlock
gate-placement
checking_connection
needs_rejoin
review-caught
bug-1749
bug-1748
true

bug-1750: worker-rejoin-resync task 4.3, round-1 review: the resync_pending gate was checked as…

Description

worker-rejoin-resync task 4.3, round-1 review: the resync_pending gate was checked as checking_connection()'s very first statement (an entry guard), returning early before xcecli_hello() or the needs_rejoin-triggered rejoin_call() could run. xcecd_resync_handle_query_page() sets resync_pending = true on receiving a job-state query's first page and only clears it when called again with a later page. If the worker's connection to the controller drops after a non-final page of a multi-page OPCode_jobStateQuery arrives (a query spanning more non-terminal jobs than fit in one page -- explicitly in scope per Requirement 8.1/8.3), no further page ever arrives, so xcecd_resync_handle_query_page() is never invoked again and resync_pending is stuck true forever -- permanently disabling checking_connection()'s entire body, including the worker's ONLY connection-recovery mechanism (xcecli_hello() / needs_rejoin-triggered rejoin). Directly violated Requirement 6.1 (the worker must be able to disconnect-and-rejoin during an incomplete reconciliation round) and regressed the intent of the pre-existing invariant test test_checking_connection_never_permanently_disables_worker, though none of round 1's own new tests drove the multi-page-then-disconnect sequence so it passed the full suite undetected.

Environment

Not recorded (migrated from buglog.json).

Reproduction

Not recorded (migrated from buglog.json).

Root cause

design.md directed mirroring the existing needs_rejoin boolean pattern for the new resync_pending gate, but needs_rejoin is self-healing by construction -- it gates one step INSIDE checking_connection() (the rejoin action itself), so the function's body including xcecli_hello() still runs every tick regardless of its value, giving the worker a fresh chance to detect and recover every cycle. Round 1 placed resync_pending's check as the function's entry guard instead, which is not actually the same pattern in the load-bearing respect that matters: it makes the gate's only exit condition (a later query page arriving) depend on the very connection health the gate is supposed to be resilient to, with no independent path back to a working state.

Fix

FIXED (2026-09-01, round-2 review, task 4.3). Two changes, defense-in-depth: (a) narrowed the gate so xcecd_resync_is_pending() is checked only inside the needs_rejoin branch, guarding solely rejoin_call() -- xcecli_hello() and the needs_rejoin check itself now run unconditionally on every checking_connection() tick, restoring the self-healing property. (b) added xcecd_resync_reset(), called from xcecd_on_reconnect() (the same signal that sets needs_rejoin, confirmed to fire only after xce_failover_rotate()/xcecli_invoke_rejoin() has already repaired a real, previously-detected transport failure -- never spuriously on a healthy connection), which clears resync_pending and discards any stalled partial multi-page accumulation (frees and zeroes the accumulator, not just the boolean), bounding the worst case to at most one reconnect cycle instead of forever. Verified: round-2 reviewer independently reintroduced the round-1 entry-guard bug, rebuilt, and confirmed a new structural regression test (test_checking_connection_hello_and_needs_rejoin_precede_resync_gate_in_source, asserting strict source-order) fails while the companion behavioral test (test_checking_connection_survives_permanently_stuck_resync_pending) does NOT catch the reintroduced bug alone -- establishing that placement-sensitive gates need a structural/ordering assertion, not just a behavioral one. Full worker suite green after the fix (xcecd_resync_test 28/28, controller_connector_test 22/22 including the pre-existing test_checking_connection_never_permanently_disables_worker invariant). One advisory, non-blocking follow-up noted but not fixed: resync_pending/query_accum are unsynchronized statics touched from two different OS threads (connector thread vs. a vtaskmgr pool thread) -- a narrow theoretical race on xcecd_resync_reset()'s free() vs. a concurrent read, mirroring needs_rejoin's own accepted unsynchronized-static risk profile but riskier since it frees a heap pointer.

Verification

Not recorded (migrated from buglog.json).

References

  • Related: bug-1749, bug-1748