myxce/.kiro/specs/component-failover-adoption/tasks.md

Implementation Plan

• 1. Foundation: candidate-count constant reconciliation and shared conversion helper

• 1.1 Reconcile the candidate-count limit constant across libxce

  • Derive client.h's N_MAX_MASTERS from config.h's MAX_HA_CANDIDATES instead of an independent literal, so the two constants can never silently diverge again
  • Fix the two hardcoded > 3 candidate-count checks (worker's and scheduler's startup argument validation) to reference the constant symbolically instead of a magic number, so they track the raised value automatically
  • Update the client-retry test fixture that currently hardcodes exactly 3 literal host/port entries so it doesn't silently exercise an uninitialized 4th slot once the constant is raised
  • Observable completion: the project builds clean with the candidate-master array now sized to 4; the two updated startup checks and the updated test fixture all reference the same symbolic constant, not a literal
  • Requirements: 1.7

• 1.2 Implement the candidate-list-to-client-config conversion helper in libxce

  • Add a pure, socket-free helper that converts a validated candidate list into a fully-populated client config, copying candidates in order and leaving identity/auth fields for the caller to set
  • A one-candidate input is handled through the identical code path as any other count — no special-cased branch for "single controller"
  • The truncate-with-warning safety net for an over-count input stays in place as a defensive guard, even though it can no longer be reached via a properly validated input now that the two limits match
  • Observable completion: given a validated candidate list with 1, 3, and 4 candidates, the helper returns a correctly populated client config in each case, exercising the defensive truncation path only when called with an artificially invalid over-count input
  • Requirements: 1.1, 1.2, 1.3, 1.5, 1.6, 1.7
  • Depends: 1.1

• 1.3 Unit test the conversion helper and the constant reconciliation

  • Cover: candidate order preserved; one-candidate input produces a one-master output with no branch taken; truncation-with-warning exercised directly against the defensive path; NULL/empty-candidate-list rejection
  • Add a compile-time or unit-level assertion that the two candidate-count constants stay equal, guarding against reintroducing the divergence
  • Observable completion: new test cases pass under the existing libxce unit test run, including the constant-equality assertion
  • Requirements: 1.5, 1.7
  • Depends: 1.2

• 2. Worker: unified candidate-list connect and transparent reconnect

• 2.1 (P) Replace worker's dual connect mechanisms with the single unified pipeline

  • Delete the standalone client-config construction used for worker's default connection and the separate single-host branch used when no candidate list is configured — both bypass the shared candidate-list validation and conversion helper entirely today
  • Build exactly one validated candidate list before connecting, whether it came from an operator-supplied peer list or was synthesized as a single candidate from the existing default when none was given, then run it through the shared helper and connect once
  • Register a rejoin callback that only sets a flag and returns immediately — it must not call back into the client library itself, since the library invokes it while still holding its own lock
  • Reconcile the periodic liveness probe's own bounded give-up behavior (today it tears the worker's connection down for good after a handful of failed probes) with the library's own indefinite retry, so an outage longer than that old bound no longer causes the worker to give up permanently — the library now owns connection recovery
  • Observable completion: starting the worker with no candidate list configured and starting it with a single-entry candidate list produce the identical connect log sequence; a three-candidate config where the first is a live standby joins correctly, showing the standby redirect and the successful candidate; a simulated outage longer than the old give-up bound no longer tears the connection down for good; invoking the registered rejoin callback while the client's lock is held returns immediately without blocking and performs no I/O
  • Requirements: 1.1, 1.2, 1.3, 1.5, 2.1, 2.2, 2.3, 5.1, 5.2
  • Boundary: WorkerControllerConnector
  • Depends: 1.2

• 2.2 Fix worker's candidate-list startup ordering and add rejoin retry-on-failure

  • Move the candidate-list assignment to before the connector starts, so the very first connection attempt already sees the full configured list instead of only a synthesized single candidate
  • After each periodic liveness probe returns, check whether a rejoin is pending; if so, issue the rejoin call as a separate step and clear the pending indicator only if that call succeeds — on its own failure, leave the indicator set so the next liveness probe retries the rejoin rather than leaving the worker connected but never actually re-registered
  • Observable completion: starting the worker with a peer list where the first listed candidate is unreachable succeeds on the very first connect attempt, not only on a later reconnect; a rejoin call that fails once before succeeding is retried on the next liveness-probe cycle rather than abandoned
  • Requirements: 1.4, 2.5
  • Depends: 2.1

• 2.3 Validate worker thread isolation and add worker candidate/reconnect test coverage

  • Confirm the connector thread is isolated from the worker's other recurring liveness duties, and that a blocked connector thread during retry/backoff does not stall them
  • Add end-to-end coverage: worker starts and joins across a multi-candidate config; killing the joined primary results in reconnect and re-join to a surviving candidate without a worker restart
  • Add coverage for the case where every configured candidate is unreachable or rejects the join at startup: assert the worker reports a clear failure identifying every candidate it attempted, not a generic error
  • Confirm the worker's periodic liveness probe to a connected controller keeps its regular cadence and keeps succeeding even while a separate reconnect is in progress elsewhere, so a stalled reconnect attempt cannot itself look like a missed heartbeat and trigger an unwanted failover
  • Observable completion: the new test passes, and the worker's other liveness duties continue observably updating while a connect/reconnect retry is in progress on the connector thread; the all-candidates-unreachable case produces a failure message naming each attempted candidate; the liveness probe's cadence to the connected controller is unaffected by a concurrent reconnect elsewhere
  • Requirements: 1.6, 3.1, 3.3
  • Depends: 2.1, 2.2

• 3. Scheduler: unified candidate-list connect, decision-publish redelivery, and runtime reconnect

• 3.1 (P) Replace scheduler's dual connect mechanisms with the single unified pipeline

  • Delete the scheduler's inline per-candidate loop and its separate single-host branch used when no candidate list is configured — both bypass the shared candidate-list validation and conversion helper entirely today
  • Build exactly one validated candidate list before connecting, whether from an operator-supplied peer list or synthesized as a single candidate from the existing default when none was given, then run it through the shared helper and connect once
  • Register a rejoin callback that only sets a flag and returns immediately, for the same self-deadlock reason as worker's
  • Observable completion: starting the scheduler with no candidate list and with a single-entry candidate list produce the identical connect log sequence; a three-candidate config where the first is a live standby joins correctly; invoking the registered rejoin callback while the client's lock is held returns immediately without blocking and performs no I/O
  • Requirements: 1.1, 1.2, 1.3, 1.5, 1.7, 2.1, 2.2, 2.3, 5.1, 5.2
  • Boundary: SchedulerControllerClient
  • Depends: 1.2

• 3.2 Add decision-publish retry-until-success-or-shutdown, composed with rejoin retry-on-failure

  • Wrap the scheduler's per-decision publish call so that a retry-budget-exhausted outcome is retried rather than treated as final and the decision discarded — checking whether the scheduler is shutting down before each attempt, not only after, so decisions still queued behind the one actively retrying do not each burn a full retry cycle just to discover shutdown was requested
  • After a publish finally succeeds, check whether a rejoin is pending; if so, issue it as a separate step and clear the pending indicator only on that call's success
  • Log an extended-retry success distinctly from an ordinary first-attempt success
  • Observable completion: a publish call that fails repeatedly before succeeding results in the same decision retried — never dropped, never skipped to the next queued one; setting the shutdown flag mid-retry breaks the loop within one retry cycle
  • Requirements: 7.1, 7.2, 7.3, 7.4, 2.5
  • Depends: 3.1

• 3.3 Validate scheduler allocation/decision backpressure behavior and add connect/reconnect test coverage

  • Confirm the bounded, occupancy-dependent buffering behavior: the scheduler's allocation/decision processing continues briefly past the point a retry begins, then blocks until connectivity is restored and resumes automatically — assert the worst-case (least-slack) occupancy scenario explicitly, not only the most-generous one, since that is the guarantee actually being made
  • Add end-to-end coverage: scheduler starts and joins across a multi-candidate config; killing the joined primary results in reconnect and re-join without a scheduler restart
  • Add coverage for the case where every configured candidate is unreachable or rejects the join at startup: assert the scheduler reports a clear failure identifying every candidate it attempted
  • Confirm by code inspection that the scheduler makes no periodic liveness/heartbeat call of its own to a connected controller — the reconnect-retries-don't-delay-heartbeat-responses guarantee is satisfied by construction here, not by a dedicated runtime assertion, since there is no such response to delay
  • Observable completion: the new test passes, showing allocation/decision processing continuing briefly during a simulated reconnect delay, then stalling for the remainder of the outage, then resuming automatically once the connection recovers; the all-candidates-unreachable case produces a failure message naming each attempted candidate
  • Requirements: 1.6, 3.2, 3.3
  • Depends: 3.1

• 3.4 Add scheduler decision-redelivery test coverage

  • Assert decisions already queued behind a stuck one publish afterward in their original order, not out of order and not dropped
  • With several decisions queued when scheduler shutdown fires mid-retry on the first, assert total shutdown latency stays bounded to roughly one retry cycle rather than accumulating one cycle per queued decision
  • Confirm this fix is scoped to the scheduler's own decision-publishing direction only — it does not touch worker job-state resync and adds no persistence for queued-but-unpublished decisions, so a scheduler process restart mid-outage still loses whatever was queued, same as before this work
  • Observable completion: all decision-redelivery acceptance criteria are exercised by passing tests, including the bounded-shutdown-latency case and a restart-loses-in-flight-state confirmation
  • Requirements: 7.1, 7.2, 7.3, 7.4, 7.5, 7.6, 2.5
  • Depends: 3.2

• 4. apiserver: unified candidate-list connect via CGO, config parity, rejoin, and fail-fast

• 4.1 (P) Route apiserver's candidate-list parsing through the real C parser

  • Replace the hand-written Go candidate-list parser with the shared C parser used by worker and scheduler, retiring the Go parser and its call sites entirely
  • Map validation errors to clear Go error messages naming the malformed entry
  • Observable completion: the same malformed-candidate-string cases worker/scheduler already reject are rejected identically when passed through apiserver's config path
  • Requirements: 6.1, 6.2
  • Boundary: ApiserverBackendBridge
  • Depends: 1.2

• 4.2 Replace apiserver's dual connect mechanisms with the single unified pipeline

  • Delete apiserver's per-peer Go connect loop and its separate direct-construction path used when no peer list is configured — both bypass the shared candidate-list validation and conversion helper entirely today
  • Build exactly one validated candidate list before connecting, whether from the configured peer list (via Task 4.1's parser) or synthesized as a single candidate from the legacy address/port fields when no peer list was given, then run it through the shared helper (via CGO) and connect once
  • Observable completion: starting apiserver with no peer list and with a single-entry peer list produce the identical connect log sequence; a three-peer config where the first is a live standby registers against the actual primary in one call
  • Requirements: 1.1, 1.2, 1.3, 1.5, 1.7, 5.1, 5.2
  • Depends: 4.1

• 4.3 Add apiserver's rejoin logic with a correctly-placed cgo registration and retry-on-failure

  • Add a flag-setter registered as the library's rejoin callback that does no I/O, only sets a pending-rejoin flag and returns immediately, and a separate function that performs the actual rejoin call
  • Register the flag-setter's C shim in the same file as its extern declaration (cgo symbol resolution is per-file)
  • Add one shared helper wrapping every backend call: after the call returns, check the pending-rejoin flag; if set, issue the rejoin as a separate step, clearing the flag only if that call succeeds — on its own failure, leave it set so the next backend call's return retries the rejoin
  • Observable completion: after killing and restoring connectivity while a write request is in flight, apiserver resumes receiving backend push notifications without a process restart; a rejoin call that fails once before succeeding is retried on the next backend call rather than abandoned; invoking the registered flag-setter while the client's lock is held returns immediately without blocking and performs no I/O
  • Requirements: 2.1, 2.2, 2.3, 2.4, 2.5
  • Depends: 4.2

• 4.4 Trigger apiserver's rejoin from the idle-period heartbeat path

  • On a transport failure-to-success transition detected by the heartbeat prober, set the same pending-rejoin flag the write path uses (the heartbeat's own probe mechanism never sets it itself)
  • On every subsequent heartbeat tick where the probe succeeds and a rejoin is still pending, retry the rejoin — checked every tick, not only on the transition edge, so a rejoin attempt that itself fails still gets retried on a later tick
  • Do not gate this retry on the separate backend-degraded circuit-breaker state, since that state clears itself on its own timer independent of whether a rejoin actually succeeded
  • Observable completion: killing the primary while apiserver has no in-flight write request still results in apiserver rejoining and resuming push notifications; a rejoin that fails on the first post-recovery tick before succeeding on a later tick confirms the retry, not just the single-attempt case
  • Requirements: 2.4, 2.5
  • Depends: 4.3

• 4.5 Add apiserver's backend-health circuit breaker

  • Add a degraded-until state that lets request handlers fail fast instead of blocking on the full retry budget when the backend is known-degraded, clearing on either a successful rejoin or its own timeout
  • Wire it into the shared backend-call helper: check before every call, mark degraded when a call's outcome indicates retry-budget exhaustion, mark healthy on a successful rejoin from either the write path or the idle-period path
  • Keep this state's own timeout independent from the pending-rejoin flag — the circuit breaker may reset optimistically on its timer, but that must never suppress the pending-rejoin flag's own retry-until-actually-joined behavior
  • Observable completion: with the state forced degraded, a wrapped backend call returns immediately with a clear unavailable error instead of invoking the backend call
  • Requirements: 4.1, 4.2
  • Depends: 4.4

• 4.6 apiserver unit test coverage

  • Cover: degraded-to-healthy transitions via both an explicit successful rejoin and timeout expiry; concurrent-request safety; candidate-list parity cases from Task 4.1
  • Observable completion: the full apiserver test suite passes with the new cases included
  • Requirements: 1.5, 4.1, 4.2, 6.1, 6.2
  • Depends: 4.5

• 5. Integration and cross-component validation

• 5.1 E2E: full-cluster reconnect after primary failover

  • Extend the project's existing HA soak pattern to a scenario with worker, scheduler, and apiserver all configured with the full candidate list; kill the primary mid-session with a write request in flight
  • Assert: worker reconnects and resumes job-state reporting; scheduler reconnects and resumes dispatch; apiserver reconnects, resumes serving API requests, and resumes receiving push notifications — none of the three restarted
  • Observable completion: the soak scenario passes with all three components showing a successful re-join to the new primary in their logs and no manual intervention
  • Requirements: 2.1, 2.2, 2.3, 2.4
  • Depends: 2.3, 3.3, 3.4, 4.6

• 5.2 E2E: apiserver reconnects during an idle period

  • Kill the primary while apiserver has only its heartbeat prober active, no pending write requests; assert apiserver still rejoins and resumes push notifications via the heartbeat-triggered path
  • Observable completion: the scenario passes without requiring any client-triggered write request during the outage window
  • Requirements: 2.4, 2.5
  • Depends: 4.6

• 5.3 E2E: apiserver fails fast while backend is degraded, and keeps serving requests that don't need it

  • With apiserver's backend connection killed and not yet reconnected, issue an API request that requires the backend and assert it returns a clear service-unavailable error within a bounded, sub-second time — not after the full retry budget
  • In the same degraded window, issue a request that does not require the backend connection and assert it still succeeds normally, confirming degradation is scoped to backend-dependent requests only
  • Observable completion: both timed assertions pass against the same live apiserver instance while it remains degraded
  • Requirements: 4.1, 4.2
  • Depends: 4.6

• 5.4 Regression: unified single-candidate path is observably correct across all three components

  • Run the full worker, scheduler, and apiserver test suites, plus the startup path for each component with no candidate list configured
  • Confirm each component still starts and operates normally with no candidate list configured, and that this now takes the identical code path as a validated single-entry candidate list rather than a separately-maintained fallback
  • Observable completion: all existing tests pass unchanged, and each component's no-candidate-list startup log sequence matches its single-entry-candidate-list startup log sequence
  • Requirements: 1.5
  • Depends: 5.1, 5.2, 5.3

Implementation Notes

• Task 1.1: src/lib/cluster.c:99-102's xce_cluster_config_validate() still has a third hardcoded >3 WARN check (message says "<=3 only") of the same magic-number class Req 1.7 targets, and src/include/xce/cluster.h:31's doc comment repeats the stale "3" figure. Task 1.1's text named only "the two" worker/scheduler checks, so this was left untouched there — task 1.2 already touches cluster.c to add the new conversion helper, so fold this fix in there rather than letting it silently persist as the one remaining un-reconciled magic number.
• Task 1.1 follow-up resolved in 1.2: the >3→>MAX_HA_CANDIDATES rename initially proposed for xce_cluster_config_validate()'s WARN check turned out to be dead code — after task 1.1's constant reconciliation, that check tests the identical field against the identical constant as an earlier -EINVAL return in the same function, so it can never fire. Deleted rather than renamed (with the matching stale doc-comment line also removed), caught by task 1.2's review round 1.
• Task 1.2: its 8 new tests in test_cluster.c substantively cover task 1.3's stated checklist already (candidate order, one-candidate-no-branch, truncation-with-warning, NULL/empty rejection) — task 1.3 should scope down to just the constant-equality assertion (N_MAX_MASTERS == MAX_HA_CANDIDATES) rather than duplicating this coverage.
• Task 1.3: closed with zero new code. Verified fresh (source read + a clean xce-build.sh lib asan run) that every item in its checklist was already satisfied by prior tasks: the _Static_assert(N_MAX_MASTERS == MAX_HA_CANDIDATES, ...) landed in task 1.1 (test_libxce_retry.c:44), and the four behavioral cases (order/no-branch/truncation/NULL-empty) landed in task 1.2's 8 tests (test_cluster.c). No implementer/reviewer cycle was dispatched since there was no diff to review — dispatching one would have either duplicated existing test names (build conflict) or been a no-op review of an empty diff.
• Task 2.1: xce_cluster_config_to_client_config() (task 1.2) aliases out->masters[i] directly to cfg->candidates[i].hostname — no strdup. Any caller building the source xce_cluster_config_t on the stack and passing it through the helper then using the resulting client config beyond that stack frame's lifetime has a use-after-scope bug. Worker's fix keeps the source config in a file-scope static for this reason; tasks 3.1 (scheduler) and 4.2 (apiserver) hit the identical hazard and must do the same (or otherwise guarantee the source config outlives the client config's use).
• Task 2.2: fixed the startup-ordering bug by threading cluster_cfg by value through xcecd_config_t → xcecd_context_t → cc_ctx (set in main.c before xcecd_init()/xcecd_start() run, instead of after). Safe as a plain struct copy because xce_candidate_t.hostname is a fixed char[] array, not a pointer — no aliasing hazard here (unlike the xce_cluster_config_to_client_config() output struct noted above). Scheduler (3.1) likely has the same ordering gap (sched_init loops candidates correctly already per design.md, so may not need this fix — verify against current source, don't assume). Rejoin-retry pattern established: checking_connection() checks needs_rejoin after every probe return, calls xcecli_join() directly (not full re-entry into the join-cluster function), clears the flag only on success — same pattern scheduler/apiserver rejoin tasks (3.1, 4.3) should follow.
• Task 2.3 (real E2E caught what unit tests missed — 2 production bugs slipped through tasks 2.1/2.2's approved reviews): (1) xcecli_start() (src/lib/client.c) unconditionally resets ctx->failover.rejoin_cb = NULL as part of its own init — any xcecli_set_reconnect_handler() call before xcecli_start() is silently wiped. Register the reconnect handler AFTER xcecli_start() succeeds, not before. Scheduler/apiserver's own reconnect-handler registration (3.1, 4.3) must follow this same ordering or they'll hit the identical silent-wipe bug. (2) A successful xcecli_join()/worker join returns the assigned slot index as reply.status (any rc >= 0, not always exactly 0) — checking rc == 0 for success is wrong; use rc < 0 as the only failure test (matches xcecd_join_cluster()'s own established convention). Any new code checking a join/rejoin call's success must use this same rc < 0 convention, not rc == 0. (3) Security lesson: an env-var runtime bypass of a security check (XCECD_ALLOW_NONROOT) added for test/E2E convenience was rejected on review — the correct pattern for skipping a production-only check (root privilege, etc.) in a test/virtual build is a compile-time guard against the project's existing VIRTUAL_CLUSTER or XCE_TESTING flags, never a runtime environment variable reachable in the shipped production binary.
• Task 3.2: implemented as a shared sched_publish_retry_loop()/sched_publish_check_rejoin() pair called from both sched_publish_dispatch() and sched_publish_pend_reason(), not literally inlined into sched_publish_dispatch() alone as design.md's prose names it — reviewed and judged compliant (same thread, fires once after the whole retry loop's successful return, not per-attempt, not in sched_allocator). Testing lesson for any future scheduler.c unit test needing to intercept xcecli_* calls: xcecli_jobdispatch/xcecli_set_pend_reason/xcecli_join/xcecli_start/xcecli_set_reconnect_handler all live in the single client.c translation unit inside libxce.a alongside symbols scheduler.c genuinely needs — the "test double linked ahead of the archive" trick (used by test_driver_decision_cb.c one layer up) causes duplicate-definition errors here. Use -Wl,--wrap=<symbol> linker interposition instead (see test_scheduler_decision_publish_retry.c/its meson.build target) when a test needs scheduler.c itself linked live alongside intercepted libxce calls. Also confirmed: sched_admin_request_test's test_recordstart_symlink_escape_replies_error fails independent of this spec's changes (its white-box #include list doesn't pull in scheduler.c at all) — a pre-existing, unrelated, environment-specific (root-in-sandbox symlink-permission) failure; do not re-investigate it as a regression in later tasks.
• Task 3.3: fixed a real production gap found along the way — sched_init()'s two all-candidates-unreachable/join-failure log lines logged only a candidate COUNT, never host:port names (same gap worker's task 2.3 fixed for the worker). Added sched_format_candidate_list(), threading sched->cluster_cfg through as a parameter rather than a new file-scope global (unlike worker's g_cluster_cfg) since test_scheduler_connect.c links scheduler_srcs as separate objects, not #include, so the static helper isn't directly callable from tests — coverage requires a real dup2()-based stdout capture around a live sched_init() call hitting the failure path, not a white-box direct call (see capture_stdout_during_sched_init() in test_scheduler_connect.c — reusable pattern for any future scheduler.c log-content assertion). Round-1 review caught this diagnostic fix shipping with zero test coverage (proven by reverting the 59-line diff and rebuilding: all existing tests still passed) — a reminder that "the fix works, I verified it manually/via live logs" is not the same as a committed regression test; always add the assertion, don't just observe the correct behavior once. The worst-case-occupancy backpressure test (test_scheduler_backpressure.c) only proves Req 3 AC2 (buffering/block/resume-with-no-loss) — it does not exercise task 3.2's -E_XCE_NOT_READY retry path, so don't cite it for Req 7 coverage (that's task 3.4's job). E2E kill-primary/reconnect and all-unreachable-at-startup were validated manually against a real 3-node /xce-deploy-ha cluster rather than committed as an automated test, following worker's task 2.3 precedent — this repo has no automated live-multi-process-deploy E2E harness for any C daemon (the only one, test_e2e_live_ha_push.py, is Python/apiserver-only); reviewer judged this acceptable both rounds. Manual validation confirmed join, primary-death survival, and lazy socket-orphaning-on-idle (matches design), but could not observe a live rejoin firing against real sockets — blocked by bug-010 (xcecd ASAN "already joined thread" abort under unshare -r, needed to bypass the worker daemon's root-only check in this sandbox; unconfirmed whether it's a genuine bug or a userns artifact, never tried under real root). The exact rejoin-after-publish-success code path already has solid white-box mock coverage from task 3.2's test_rejoin_composition_retries_across_publish_calls, so this residual gap is narrowly "proven against mocks, not yet against a live socket" — worth closing opportunistically if bug-010 ever gets resolved, not a blocker for this spec.