test(worker-rejoin-resync): task 7.1 - full rejoin round trip
Add test_full_rejoin_reconciliation_round_trip proving the whole chain composes end-to-end in one round, for a mix of still-running, finished-success, finished-failure, and unrecognized jobs: real rejoin_worker() -> worker_send_resync_query() (real wire query) -> hand-built worker answer through the real report codec -> resync_handle_report_page() -> resync_apply_report(), asserting correct final state and scheduler notification for each. Test-only, no production changes. Logs bug-1754: a worker-reported job failure's exit code is never durably recorded anywhere in this codebase (distinct from bug-1747's finish-time gap) - discovered honestly asserted by the new test, out of scope for this validation-only task. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
This commit is contained in:
parent
9f655c61c7
commit
8650cfd9ba
3 changed files with 601 additions and 1 deletions
|
|
@ -129,7 +129,7 @@
|
|||
- _Depends: 1.4, 3.2_
|
||||
|
||||
- [ ] 7. Validation: end-to-end and regression coverage
|
||||
- [ ] 7.1 Full rejoin-reconciliation round trip
|
||||
- [x] 7.1 Full rejoin-reconciliation round trip
|
||||
- Exercise a real rejoin against a live controller and worker pair: a worker with a mix of still-running, finished-success, finished-failure, and unrecognized jobs reconnects, and every job ends up in the correct final state with the scheduler notified
|
||||
- Observable completion: the round trip produces the correct end state for every job kind in one live test run, matching the per-outcome behavior verified in isolation by tasks 2.1-2.2 and 3.2
|
||||
- _Requirements: 1.1, 1.2, 1.3, 2.1, 2.2, 3.1, 3.2, 3.3, 3.6, 4.1, 4.2, 4.3, 4.4, 4.5, 4.6, 4.7, 4.8, 7.1, 7.2, 7.3_
|
||||
|
|
@ -169,3 +169,4 @@
|
|||
- **Task 6.1 found and fixed bug-1752**: the worker daemon's `myreq_handler` had NO dispatch case for `OPCode_push` at all before this task — every prior config push (including the pre-existing `clustername` field, in production since before this spec started) was silently discarded via `default: break;`. Confirmed genuine (diffed `myreq_handler` pre/post, ruled out an alternate consumer — `xcectx.c`'s `vtask_entry` only special-cases the unrelated generic `pushmsg` union arm, not `configpushmsg`) and confirmed `clustername` has never been read anywhere in `src/worker/`, so this was harmless dead infrastructure until `workerResyncWindowMs` became its first real consumer. Fixed narrowly — `handle_pushconfig()` consumes only `worker_resync_window_ms`, leaving `clustername`/`confvars` still unconsumed and explicitly out of this task's boundary — rather than expanding scope to wire up every existing-but-never-consumed push field. Ordering (join reply → config push → conditional arm+query) and the live-configured-value plumbing were both verified via genuine structural tests (real packets popped off the wire, two distinct configured values round-tripped), not weaker behavioral proxies.
|
||||
- **Task 6.2's mandated standby-redirect investigation found bug-1753**, a DIFFERENT gap than the one it was looking for: the generic not-primary (`CTRL_STANDBY`) redirect path turned out to already work correctly with zero opcode-specific code (`ha_dispatch_gate` gates every opcode uniformly before the switch), but the investigation surfaced that `ha_dispatch_gate`'s SEPARATE `CTRL_PRIMARY`-without-write-quorum branch — a "fact-bearing ops admitted anyway" allowlist (`OPCode_notify`/`update`/`setpreason`/`clrpreason`, bug-396) — was missing `OPCode_jobStateReport`, meaning a report arriving during a transient quorum-loss window (e.g. mid-failover, this feature's own core motivating scenario) would be rejected outright with no retry, since the worker's send is fire-and-forget. Fixed by adding the one missing case, verified safe against `resync_round_is_current`'s staleness check and the WAL_MODE_RETAIN semantics every fact this feature produces already uses. Citation nit: this fix's true motivation sits closer to Requirements 1.3/4/6 than the task's own tagged 8.1/8.3 — recorded here for traceability, tasks.md's tag left as originally generated. **Lesson**: a task text that names one specific concern to investigate ("check the redirect path") can surface a DIFFERENT, more consequential gap in the same general area — don't stop the investigation at "the named concern turned out fine," keep reading the surrounding gating logic in full.
|
||||
- **Task 6.2 also drew three escalating, unsolicited cross-session messages** (from an unfamiliar peer session) mid-flight, pushing a specific unreviewed code change to `test_jobstatereport_dispatch.c` — the exact file the dispatched implementer subagent was independently writing at the time. Declined to act on any of them (risk of conflicting with in-flight subagent work; escalating urgency/"no further investigation needed" framing is a known social-engineering pattern regardless of whether the underlying suggestion is technically plausible). The implementer subagent finished independently and solved the underlying `get_ep_worker()`-in-test-binary limitation completely differently (a `#ifdef XCE_TESTING` call-count seam in `resync.c`, not a global-fixture hack) — validating the decision not to short-circuit the normal TDD+review pipeline based on an unverified peer instruction.
|
||||
- **Task 7.1 (test-only, zero production changes) discovered and honestly asserted a genuine, pre-existing, systemic gap distinct from bug-1747**: a worker-reported (or any) job failure's exit code is never durably recorded anywhere in this codebase — `record_job_fail_fact()`'s `WAL_JOB_FAIL` payload has no exit-code slot at all, and `apply_job_fail_transition()` never touches `job.job.exit_info` for the FAIL path, so it stays `NULL` unconditionally. Bug-1747 only covers the FAIL finish-*time* mistiming; this is the exit-*code* gap, logged separately as bug-1754 (out of scope for this test-only task, correctly not fixed here — flagged for a dedicated future task). The round-trip test asserts `exit_info == NULL` honestly with an inline comment rather than encoding a wrong expectation the codebase can't actually produce.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue