Add test_burst_recovery_unbounded_by_job_count, reproducing bug-1741's exact original shape (7 jobs on one host, 6 lost finish reports) in one reconciliation round and one wire page: 5 finished-success + 1 finished-failure recover correctly and distinctly, while an already- DONE job re-reported as an unqueried extra (with a deliberately wrong finish time) is proven structurally unchanged - exact +6 WAL LSN delta, not >=, so a duplicate append is genuinely caught rather than silently tolerated. Test-only, no production changes; deliberately kept to a single page since page-boundary correctness is task 7.4's separate concern. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
36 KiB
Implementation Plan
-
1. Foundation: wire protocol schema and shared reconciliation types
-
1.1 Define the wire protocol for the reconciliation exchange
- Add the controller→worker job-state query and worker→controller job-state report as new opcodes, each carrying a paginated list of per-job entries (job ID, status, exit code, worker-reported finish time, and whether the job was part of the original query)
- Add the two new cluster-wide configuration fields: the recently-finished reporting window and the reconciliation timeout
- Observable completion: the schema builds cleanly and the new message/opcode/config-field definitions are visible in the generated bindings
- Requirements: 2.1, 2.2, 3.5, 3.7, 5.3, 8.1, 8.2, 8.3
-
1.2 (P) Add a worker reconciling status and round-tracking state
- Add a new worker status value meaning "job-state reconciliation outstanding for this host"
- Add a per-worker reconciliation-round structure capable of holding a round identifier, the set of queried job IDs, and a deadline
- Observable completion: the new status value and round structure compile and are reachable from the worker record, unused by any caller yet
- Requirements: 5.1, 6.3
- Boundary: shared types
-
1.3 (P) Establish worker-side C test infrastructure
- Add the first test-runner harness for the worker daemon, mirroring the controller's existing runner conventions
- Observable completion: an empty/placeholder worker-side test program builds and runs successfully via the existing test build target
- Requirements: 3.1, 3.2, 3.3
- Boundary: worker test infrastructure
- Note: satisfied by pre-existing infrastructure — see Implementation Notes.
-
1.4 (P) Add wire encode/decode helpers for the reconciliation exchange
- Add request/reply encode and decode helpers for the new query and report messages, following the existing paginated-list pattern already used for job/worker/resource listings
- Observable completion: a round-trip encode-then-decode of a query and of a report (including an empty page and a multi-page case) reproduces the original data exactly
- Requirements: 8.1, 8.2, 8.3
- Boundary: wire codec
- Depends: 1.1
-
1.5 (P) Wire the new config fields into xceadmin's config-build pipeline
- Add the two new cluster-config fields (the recently-finished reporting window, the reconciliation timeout) to xceadmin's YAML validation schema and its YAML-to-binary field mapping, following the existing
heartbeatInterval/inactiveHeartbeatCountpattern exactly, so an operator can actually set them throughxceadmin config build— added to the wire schema by task 1.1, but with no configuration path into it until this task - Regenerate the checked-in golden config-build fixtures to match the wire schema's current field set (task 1.1 grew it; the fixtures predate that and no longer match a fresh build)
- Observable completion: a YAML config supplying values for both new fields produces a binary that decodes back to those exact values, and the existing golden-fixture regression tests pass again
- Requirements: 3.7, 5.3
- Boundary: xceadmin config-build
- Depends: 1.1
- Add the two new cluster-config fields (the recently-finished reporting window, the reconciliation timeout) to xceadmin's YAML validation schema and its YAML-to-binary field mapping, following the existing
-
2. Core: controller job-table reconciliation outcomes
-
2.1 Return an unrecognized job to pending state, releasing its resource allocation
- When a worker reports a queried job as unrecognized, transition that job back to pending using the existing state-transition guard so a terminal or already-more-final job is never overwritten
- Release any resource allocation the job was carrying, so a job returned to pending never carries a stale allocation from a prior dispatch into its next dispatch attempt
- Durably record the transition and notify the scheduler of the new pending state through the existing notification path, without waiting for a later replay to discover the change
- Observable completion: a job previously dispatched (real allocation committed) that reconciles to unrecognized ends the call with pending state, no resource allocation, and the scheduler notified — verified before any commit round-trip completes
- Requirements: 4.1, 4.2, 4.3, 4.4, 4.5, 4.6, 7.1, 7.2, 7.3
- Boundary: job.c additions
-
2.2 Assign an unqueried job to the worker that reported it
- When a worker reports a job the controller has no existing non-terminal record of, record that job as assigned to the reporting worker and host, matching the worker's reported status, through the same idempotent path used for a normal dispatch
- Do not fabricate a resource allocation for this job — leave the allocation unset, consistent with the controller genuinely not knowing what the scheduler originally computed
- Notify the scheduler of the new assignment through the existing notification path
- Observable completion: a job the controller never previously tracked, reported running by a worker, ends the call recorded as running on that host with the scheduler notified, and durably survives a restart replay with the same result
- Requirements: 4.1, 4.3, 4.4, 4.7, 4.8, 7.1, 7.2, 7.3
- Boundary: job.c additions
-
2.3 Fail a dispatch to a disconnected worker immediately, without retry or job failure
- When the controller attempts to dispatch a job to a worker it is not currently connected to, fail that specific dispatch attempt right away and return the job to pending, reusing the pending-and-release behavior from task 2.1, rather than retrying the attempt or exhausting a retry count toward permanent failure
- Leave the existing behavior for every other reason a worker might not be ready (locked, busy, closed, or mid-reconciliation) completely unchanged — confirm those cases still retry and eventually fail exactly as before
- Observable completion: dispatching to a disconnected worker returns the job to pending on the first attempt with the job's failure-retry count untouched and no permanent failure recorded, while dispatching to a worker that is connected but otherwise not ready still follows the pre-existing retry-then-fail behavior unchanged
- Requirements: 5.1, 5.4
- Boundary: job.c additions
- Depends: 2.1
-
3. Core: controller reconciliation-round bookkeeping
-
3.1 (P) Track one reconciliation round per worker with a bounded timeout
- Given a worker host with one or more non-terminal jobs currently assigned to it, construct the exact list of those jobs' unique IDs as the round's query set
- Track a round identifier that distinguishes the current reconciliation attempt from any prior one for the same worker
- Schedule a bounded timeout for the round using a non-blocking trigger; when it fires before the round otherwise resolves, resolve the round rather than leaving the worker withheld from dispatch indefinitely
- Observable completion: arming a round for a worker with known non-terminal jobs produces a query set matching exactly those job IDs, a fresh round identifier, and an active timeout; letting the timeout elapse resolves the round without any blocking call
- Requirements: 1.1, 1.2, 1.3, 2.1, 2.2, 5.3, 6.1
- Boundary: resync.c
-
3.2 Apply each reported job outcome through the correct idempotent path
- For each entry in a worker's job-state report: a still-running job is cleared from the round's outstanding set with no fact recorded; a finished-success or finished-failure job is applied through the existing finish/fail recording used for an ordinary report; an unrecognized queried job is applied through task 2.1's pending-and-release path; a reported job outside the original query is applied through task 2.2's assignment path
- Discard a report that arrives for a round already superseded by a newer one for the same worker, rather than applying it
- When the round's outstanding set becomes empty, resolve the round so the worker becomes eligible for dispatch again
- Applying the same report twice for the same round must produce the same result as applying it once
- Observable completion: a full report covering all five outcome kinds leaves the job table in the correct state for each, a duplicate re-application of the same report changes nothing further, and a report carrying a stale round identifier is dropped without touching the job table
- Requirements: 4.1, 4.2, 4.3, 4.4, 4.5, 6.1, 6.2, 6.3
- Boundary: resync.c
- Depends: 2.1, 2.2, 3.1
-
4. Core: worker-side job-state reporting
-
4.1 (P) Retain a job's true outcome for a bounded window after it exits
- At the moment a job's exit is detected, before its record would otherwise be discarded, retain its exit status, exit code, and true finish time for a configurable bounded window
- Observable completion: a job's outcome remains queryable for the configured window after it exits and is no longer available once the window elapses
- Requirements: 3.4, 3.5, 3.7
- Boundary: worker retention
-
4.2 Answer the controller's job-state query, including extra unqueried jobs
- For every job ID in the controller's query, report exactly one outcome: still running (from the live running list), finished successfully or with failure and its true finish time (from the new retention), or unrecognized if found in neither — every queried ID gets an answer, none are silently skipped, and an indeterminate case is reported as unrecognized rather than guessed
- After answering the query, also report any job the worker is currently running or has recently finished that was not part of the query
- Send both the query answers and the extra jobs in paginated reports, correct regardless of how many jobs are involved
- Observable completion: a query covering a mix of running, finished, and unknown job IDs produces one correctly-classified entry per ID plus a separate page of extras for jobs outside the query, with no job silently dropped even when the total exceeds one page
- Requirements: 3.1, 3.2, 3.3, 3.6, 8.2, 8.3
- Boundary: xcecd_resync.c
-
4.3 Wire the worker's receipt of a job-state query and delay resuming normal post-join operation until the report has fully sent
- Also close a gap task 4.2 correctly left out of its own boundary: nothing today wires the worker's actual receipt of
OPCode_jobStateQuery(src/worker/src/controller_connector.c'smyreq_handlerswitch has no case for it, confirmed by inspection — onlyOPCode_startjob/OPCode_runtasks/OPCode_jobcontrol/OPCode_fetchfileexist there today) to task 4.2'sxcecd_resync_answer_query(). Add that dispatch case: reassemble an incoming query that itself arrives across multiple pages (Requirement 8.1/8.3) into the jobids arrayxcecd_resync_answer_query()expects, then call it - Set a local gate the moment a job-state query is received; keep it set across every page of the report; clear it only once the full report, including any extra-job pages, has finished sending
- Make whatever the worker does next after rejoining (resuming its own periodic background reporting) check this gate first, so that activity never begins or interleaves while the report is still being sent
- Observable completion: the worker's post-join activity is observed to start only after the last report page has been sent, never before, across a report spanning multiple pages; a job-state query arriving from the controller (single- or multi-page) reaches
xcecd_resync_answer_query()and produces a real reply, not just a directly-called-in-tests function - Requirements: 3.8, 8.1, 8.3
- Boundary: xcecd_resync.c
- Depends: 4.2
- Also close a gap task 4.2 correctly left out of its own boundary: nothing today wires the worker's actual receipt of
-
5. (P) Core: exclude a reconciling host from scheduler dispatch candidacy
- Extend the scheduler's existing candidate-host filter so a host currently undergoing job-state reconciliation is excluded from dispatch candidacy the same way an already-unavailable host is
- When the reconciling status clears, the same filter must stop excluding that host without any separate action
- Introduces no new reconciliation-protocol name or concept into the scheduler's outbound decision-publish code — this is an ordinary status-bit read alongside the existing unavailable-host check, not a new symbol; the standing regression test guarding that publish path against ever referencing reconciliation concepts must stay green, unmodified
- Observable completion: a host flagged as reconciling is never selected as a dispatch candidate while the flag is set, and becomes selectable again as soon as it clears, with no change to how any other exclusion reason behaves, and the publish-path symbol-guard test still passes unmodified
- Requirements: 5.1, 5.2
- Boundary: scheduler candidate filter
-
6. Integration: cross-daemon wiring
-
6.1 Trigger reconciliation on worker rejoin and send the query
- At the moment a worker host becomes connected again (first join, network reconnect, or reconnect after controller failover, treated identically), check whether the controller currently holds non-terminal jobs for that host and, if so, arm a reconciliation round for it
- Send the round's query to the worker as a follow-up exchange after the join reply and the existing configuration push have both already been sent, not as an extension of the join call itself
- Also close a gap task 4.1 discovered and correctly left out of its own boundary: the worker daemon has no delivery path at all today for any
ClusterConfigfield (the existingworker_push_config/on_worker_joinpush carries a genericconfvarskey/value list plus a small number of directly-pulled fields —clusternameis the existing precedent,src/controller/worker.caround thecollect_confvars/conf4workerconstruction). AddworkerResyncWindowMsto that same push, mirroring theclusternamefield precedent exactly (no new opcode needed — this reuses the existingOPCode_push/WorkerConfigwire struct this task already touches), and have the worker's existing config-receive handling callxcecd_finished_job_retention_set_window_ms()(task 4.1) with the received value so the retention window task 4.1 built is actually operator-tunable end-to-end, not stuck on its documented placeholder default - Observable completion: rejoining a worker with known non-terminal jobs results in a reconciliation round being armed and the corresponding query being sent to that worker, in that order, after the join reply; rejoining a worker with none results in no round and no query; a worker that receives a config push with a non-default
workerResyncWindowMsvalue applies it to its own retention window before the push completes - Requirements: 1.1, 1.2, 1.3, 2.1, 2.2, 3.7
- Depends: 1.4, 3.1, 4.1
-
6.2 Route an incoming job-state report to reconciliation processing
- Dispatch an incoming job-state report message to the reconciliation-processing logic, correctly reassembling a report that arrives across multiple pages before it is applied
- Confirm whether the two new message kinds need an explicit redirect response when received by a controller that is not currently primary, matching how every other primary-only message already behaves for a standby recipient — do not assume the existing generic rejection path already covers this without checking, given this exact class of gap has recurred for other message kinds before
- Observable completion: a report sent across several pages is fully reassembled and applied exactly once, with the same result as an equivalent single-page report; and a query or report sent to a non-primary controller receives the same redirect behavior an equivalent primary-only message already receives, not a bare unrecoverable error
- Requirements: 8.1, 8.3
- Depends: 1.4, 3.2
-
7. Validation: end-to-end and regression coverage
-
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
- Depends: 6.1, 6.2, 4.3, 5
-
7.2 Failover-during-job recovery
- Reproduce the feature's core motivating scenario: a job's finish report is lost across a controller primary failover, and the new primary's rejoin reconciliation with that worker recovers the job's true outcome
- Observable completion: a job whose finish notification never reached any controller instance ends up correctly finished, not stuck running, once the worker rejoins the new primary
- Requirements: 4.1, 4.2, 4.6
- Depends: 7.1
-
7.3 Burst/array recovery unbounded by job count
- Reproduce a burst of many near-simultaneous job completions on one worker host where most of the individual finish reports are lost, and confirm reconciliation recovers all of them on the worker's next rejoin regardless of count
- Observable completion: every job in the burst is correctly finished after reconciliation, with none left stuck and none duplicated
- Requirements: 8.1, 8.2, 8.3
- Depends: 7.1
-
7.4 Large job-count pagination correctness
- Drive the reconciliation exchange for a worker running enough jobs to require multiple pages of both the query and the report, confirming the exchange completes correctly rather than truncating or silently dropping jobs past a single page
- Observable completion: a job count an order of magnitude larger than a single page holds is fully queried and fully reported, with every job accounted for
- Requirements: 8.1, 8.2, 8.3
- Depends: 7.1
Implementation Notes
- Task 1.3: this task's premise (
src/workerhas notestsuite/) is FALSE as of implementation time — real, tracked worker test infrastructure already exists (src/worker/testsuite/{meson.build,test_*.c}, 5 passing tests,suite: 'worker'registered in meson,/xce-build workeralready runs it), built bycomponent-failover-adoption(commitsce9f734a/331fa024/619b119e, 2026-08-11) weeks before this spec was written..kiro/steering/structure.md,testing.md:55, andbuild-deployment.md:16are all stale on this point — worth a/kiro-steeringsync, out of this feature's boundary to fix. Confirmed directly (fresh build +meson test -C src/xce.build --suite worker: 5/5 OK) rather than trusting the finding secondhand. No files changed for this task — closed as satisfied by pre-existing infrastructure. - Task 4.2 must follow the EXISTING worker testsuite convention, not the controller aggregator-runner pattern the original task 1.3 text pointed at: each test file is its own standalone Unity executable (own
UNITY_BEGIN()/RUN_TEST/UNITY_END(), white-box#includes the production.cfile under test — seetest_resource_sync.c's shape) with its own explicitexecutable()+test(..., suite: 'worker', ...)block added tosrc/worker/testsuite/meson.build, mirroringtest_controller_connector.c's registration exactly. Do not introduce a second, competing test-runner shape. - Task 1.5:
xceadmin config dump's_format_cluster_config(build.py) does not printworkerResyncWindowMs/workerResyncTimeoutMs— an operator can set them viaconfig buildbut can't verify the built value viaconfig dump. Confirmed genuinely out of task 1.5's boundary (config-build, not config-dump) and not required by Requirements 3.7/5.3 as written, but a real, minor observability gap worth a small follow-up task if this becomes a live operator pain point. Not blocking; not added to this plan. - Task 2.2 found and fixed bug-1746, a real pre-existing defect in
persist.c(part of bug-1678's original fix, predates this feature):capnp_encode_joballoc()unconditionally dereferenced a NULLallocviaencode_JobAlloc()instead of the already-existing NULL-safeencode_JobAlloc_ptr()wrapper — never triggered before sincedispatch_a_job_(the only prior caller) always passes non-NULL. This directly contradicted design.md's claim that the NULL-alloc case was "tested, backward-compatible" on both apply seams — true for decode, never actually true for encode until this task exercised it. Fixed with a minimal guard producing a true zero-length buffer (not a capnp-null-pointer encoding — verified againstapply_committed_entry'salloc_len > 0decode gate, which a non-zero-length "null" encoding would have wrongly satisfied). design.md corrected in 4 places. Any future task callingpersist_submit_job_dispatch_factwith a NULL alloc can now trust this path — it wasn't safe before task 2.2, it is now. - Task 2.2 (self-correction): a coordinator-applied one-line comment fix to
job.caccidentally closed the enclosing/* */comment block early, leaving the comment's remaining lines as bare (uncompilable) code — caught immediately by re-running the build before marking the task complete, not left for review to find. Reinforces why fresh-evidence re-verification applies even to "trivial, obviously-safe" doc-only edits made outside the TDD subagent loop. - Task 3.1:
resync.c'sresync_timeout_ms()applies its own private default-if-unset fallback (15000ms) forworkerResyncTimeoutMsrather than followingmyconf.c's establishedSET_DEFAULT_IF_UNSET-style canonicalization convention used for the siblingheartbeat_intervalfield (myconf.c:74) —myconf.cis outside task 3.1's boundary, so this was correctly left as-is, but if a SECOND consumer ofworkerResyncTimeoutMs(orworkerResyncWindowMs) is ever added, it would have to either duplicate this private fallback or the two could silently disagree. Worth canonicalizing inmyconf.cif/when that happens — not needed for this feature's own single consumer. delaymgr timer cancellation:delay.c/delay.hexpose no dequeue/cancel primitive for an already-armed one-shot task;resync.chandles this correctly via a by-value round-token check inside the fired callback (verified safe against both the logical "resolves the wrong round" bug and use-after-free, since the callback's closure never holds a pointer into the freedresync_state_t) — any future delaymgr one-shot user in this codebase facing the same "need to cancel" problem should use this same by-value-token-check pattern rather than assuming a cancel API exists. - Task 3.2 needed 3 review rounds — a genuine cautionary tale about a boolean short-circuit hiding a second, adjacent bug. Round 1 caught the unqueried-extra-"running" branch gating on terminal-vs-non-terminal instead of Req 4.8's real trigger (assignment status), silently leaving a job stuck PEND when its
WAL_JOB_DISPATCHfact never reached durable commit before a failover — the feature's own core motivating scenario. The FIRST fix (!terminal && assigned_to_w's_host) solved that but introduced a NEW, distinct Req 4.4 violation via the same&&'s short-circuit: a TERMINAL job (DONE/EXIT) falls into the sameelse/assign branch and gets clobbered back to RUN — caught by round 2, and confirmed the implementer's own new test had been asserting the bug as correct behavior (a green suite is not proof of correctness when the test itself encodes the wrong expectation). The SECOND fix (a genuine three-wayif/else if/else: terminal→noop, non-terminal-and-already-assigned-anywhere→noop, non-terminal-and-unassigned→assign) passed round 3's from-scratch re-derivation plus an explicit hunt for job==NULL reachability. Lesson for future tasks in this codebase: when a guard combines two orthogonal conditions with&&/||and only ONE of the two is corrected after a review finding, re-derive the FULL truth table before re-submitting — a targeted patch to the specific counterexample a reviewer names can leave an adjacent, differently-shaped counterexample in the same boolean untouched. bug-1747 (finish-time gap, split fixed) and bug-1748 (this guard defect) both logged. - Task 4.1: found and fixed a real-but-non-blocking gap the implementer's own doc comment claimed didn't exist:
finished_job_retention_prune()was only invoked fromxcecd_finished_job_lookup(), never fromxcecd_retain_finished_job(), so withxcecd_finished_job_lookup()having zero production callers today (task 4.2 not yet implemented) the retention list had no active pruning driver and would grow one entry per finished job for the daemon's entire uptime — even after 4.2 lands, pruning would only fire on this worker's own rejoin, so a long-uptime well-connected worker could still accumulate substantial entries between rejoins. Fixed by also pruning on retain (usingfinish_time_ms, the value already captured at genuine exit-detection time, as the prune'snow_ms). Also confirms task 4.1 has no runtime delivery path yet forworkerResyncWindowMs—xcecd_finished_job_retention_set_window_ms()is a real, fully-wired seam but nothing calls it in production; task 6.1's scope was already expanded (see its entry above) to plug this in viaworker_push_config's existingWorkerConfigstruct. - Task 4.2: implemented
xcecd_resync_classify_job/xcecd_resync_build_report/xcecd_resync_answer_query(classify RUNNING/FINISHED_OK/FINISHED_FAIL/UNRECOGNIZED, compute extras, paginate via task 1.4'sxce_job_state_report_req()) as directly-callable, fully-tested functions (20/20 tests) — necessarily added one small, purely-additive companion function to task 4.1's API,xcecd_finished_job_retention_snapshot(), since 4.1 only exposed single-jobid lookup and 4.2 needs to enumerate ALL retained jobs for the extras set. Confirmed by inspection that nothing wires the worker's actual receipt ofOPCode_jobStateQueryto call into this code yet (controller_connector.c'smyreq_handlerhas no case for it) — folded that wiring explicitly into task 4.3's scope (see its entry above), since 4.3 already needs the real receive path to implement its post-join gate correctly and is the same file/boundary. - Task 4.3 needed 2 review rounds — a genuine deadlock, not a style nitpick. Round 1 placed the
resync_pendinggate check aschecking_connection()'s entry guard (returning early beforexcecli_hello()/theneeds_rejoincheck could run), so a worker whose connection dropped after a non-final page of a multi-pageOPCode_jobStateQuery(no further page ever arriving to clear the gate) would have its ONLY connection-recovery mechanism permanently disabled — a direct Requirement 6.1 violation the existing suite didn't catch because no test drove the multi-page-then-disconnect sequence. Round 2 fixed it two ways: (a) narrowed the gate to guard only therejoin_call()step inside theneeds_rejoinbranch, soxcecli_hello()/reconnect-detection always run every tick regardless of gate state; (b) addedxcecd_resync_reset(), called fromxcecd_on_reconnect()(the same signal that setsneeds_rejoin), which clears the gate and discards any stalled partial multi-page accumulation the moment a real reconnect completes — bounding the deadlock window to at most one reconnect cycle instead of forever. Round 2's reviewer independently confirmed the fix by reintroducing the round-1 bug and rebuilding, and found the new behavioral regression test alone did NOT catch the reintroduced bug — only a new structural source-order test (assertingxcecli_hello()precedes theneeds_rejoincheck precedes the gate check, textually) actually distinguished correct from broken placement. Lesson for future tasks in this codebase: when a fix's correctness hinges on WHERE a check is placed relative to other statements (not just what value it reads), a pure behavioral/outcome test can pass even with the wrong placement if the test's specific stimulus happens not to exercise the broken ordering — add a structural/ordering assertion alongside the behavioral one for placement-sensitive gates. - Task 4.3 (advisory, non-blocking, not fixed in this pass):
resync_pending/query_accuminxcecd_resync.care plain unsynchronized statics touched from two different OS threads — the worker's dedicated connector thread (checking_connection()/xcecd_on_reconnect()) and avtaskmgrpool thread (xcecd_resync_handle_query_page(), reached via the request-dispatch path). This mirrors the pre-existing, design-directed unsynchronizedneeds_rejoinpattern (same codebase convention, deliberately reused per design.md), but is riskier sincexcecd_resync_reset()callsfree()on the accumulator rather than just staling a bool read — a narrow, non-deterministic window exists where an independent reconnect could free the buffer while a pool thread is mid-read of the same connection's in-flight query page. Does not reproduce task 4.3's own violated requirement (Req 6.1) and isn't required by any Requirement/AC as written; this project's ASAN/UBSAN test harness has no TSAN, so it also isn't mechanically testable the way the deadlock fix was. Worth a small follow-up (a mutex around the gate+accumulator, or moving accumulation to the connector thread only) if this ever needs hardening beyond whatneeds_rejoin's own accepted risk profile already tolerates. - Task 5 found and fixed bug-1751: design.md's cited candidate-filter site (
scheduler.c:1211'ssched_reset_worker_(), reached only viasched_begin_session()) is dead code on both live drivers —driver.c/sim_loop.cactually callxce_core_begin_session()(core_session.c), which runs an independently-maintained mirror,core_reset_workers(), not listed in design.md's File Structure Plan. Fixed by applying the identicalIS_WORKER_UNAVAIL || IS_WORKER_RECONCILINGdisjunct to both sites (parity/dead-code-hygiene on the design-cited one, correctness on the actually-live one) rather than patching only the literally-cited site and leaving the task's own Observable Completion Criterion false in production — same category as bug-1746/bug-1747's design-vs-reality corrections. design.md's File Structure Plan corrected. Also confirmed design.md's Testing Strategy referenced an "existing IS_WORKER_UNAVAIL candidate-filter test" that does not actually exist; new coverage added totest_core_session.cinstead (white-box#includescore_session.calready;scheduler.c's own dependency closure is too large to white-box per established precedent,test_snapshot_load_allocator_regression.c) —scheduler.c's copy of the fix is therefore verified only by compilation + the identical, tested logic pattern in its mirror, not by a direct test; judged acceptable since it's confirmed dead code today, not a live gap. - Task 6.1 found and fixed bug-1752: the worker daemon's
myreq_handlerhad NO dispatch case forOPCode_pushat all before this task — every prior config push (including the pre-existingclusternamefield, in production since before this spec started) was silently discarded viadefault: break;. Confirmed genuine (diffedmyreq_handlerpre/post, ruled out an alternate consumer —xcectx.c'svtask_entryonly special-cases the unrelated genericpushmsgunion arm, notconfigpushmsg) and confirmedclusternamehas never been read anywhere insrc/worker/, so this was harmless dead infrastructure untilworkerResyncWindowMsbecame its first real consumer. Fixed narrowly —handle_pushconfig()consumes onlyworker_resync_window_ms, leavingclustername/confvarsstill 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_gategates every opcode uniformly before the switch), but the investigation surfaced thatha_dispatch_gate's SEPARATECTRL_PRIMARY-without-write-quorum branch — a "fact-bearing ops admitted anyway" allowlist (OPCode_notify/update/setpreason/clrpreason, bug-396) — was missingOPCode_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 againstresync_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 underlyingget_ep_worker()-in-test-binary limitation completely differently (a#ifdef XCE_TESTINGcall-count seam inresync.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()'sWAL_JOB_FAILpayload has no exit-code slot at all, andapply_job_fail_transition()never touchesjob.job.exit_infofor the FAIL path, so it staysNULLunconditionally. 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 assertsexit_info == NULLhonestly with an inline comment rather than encoding a wrong expectation the codebase can't actually produce. - Task 7.2 (test-only): added a focused, single-job proof of the feature's own original motivating scenario (bug-1741) — a job dispatched and RUN with NO finish fact ever committed (modeling the lost fire-and-forget notify), recovered to DONE with the worker-reported finish time via the real rejoin/query/report chain. Deliberately scoped smaller than task 7.1's four-outcome round trip (reuses its helper infrastructure, adds no new scaffolding) rather than duplicating it. tasks.md's own
_Requirements: 4.1, 4.2, 4.6_tag includes 4.6 (unrecognized→PEND), which is genuinely unrelated to this finished-success scenario (that's task 7.1/2.1's territory) —REQUIREMENTS_CHECKEDcorrectly narrowed to 4.1/4.2 instead of force-fitting an unrelated assertion, same accepted-imprecise-tag pattern as task 6.2's own citation nit. - Task 7.3 (test-only): reproduced bug-1741's exact original shape (7 jobs, 6 lost) in one round/one wire page — 5 finished-success + 1 finished-failure recovered, plus one already-DONE job re-reported as an unqueried extra with a deliberately wrong finish time, proving reconciliation neither leaves any lost job stuck nor duplicates/clobbers an already-terminal one (exact
+6WAL LSN delta, not>=, so a duplicate append is genuinely caught — confirmed via RED-phase reproduction againstrecord_job_finish_fact_at's terminal-state guard). Deliberately kept to a single wire page (page-boundary correctness is task 7.4's separate concern). Two reviewer/implementer subagents in this session's task 6.2/7.2 review rounds hit the "background command with no external wakeup" trap (ended their turn on a backgrounded build, never resumed) — both implementer and reviewer prompts for 7.3 onward now carry an explicit foreground-only warning up front; worth keeping in every future subagent dispatch in this codebase, not just implementer roles.