wdgzyjx/myxce
1
0
Fork 0
forked from lei.chen/myxce
Code Pull requests Activity

docs(raft-adapter-conformance): commit final design.md/research.md and requirements/spec.json state

Browse source 
Spec reached GO with 15/15 tasks complete (see prior task-confirmation
commits); these doc artifacts were generated during the design/research
phases but never separately staged.
This commit is contained in:
Rongsong Shen 2026-07-31 09:29:35 +00:00
parent a86de77f91
commit d4b80010b1
4 changed files with 737 additions and 85 deletions
Download patch file Download diff file Expand all files Collapse all files

385
.kiro/specs/raft-adapter-conformance/design.md Normal file
View file

@ -0,0 +1,385 @@
# Design Document: raft-adapter-conformance
## Overview
This feature closes the wire-level reply-integrity gaps between the XCE controller's Raft adapter (`src/controller/ha.c`, `repl.c`, `config_repl.c`) and the protocol guarantees `raft/raft.tla` assumes always hold, and produces a durable, action-by-action conformance record so the next gap is found by inspection rather than by a live incident. It extends the work already landed for bug-1479 (the WAL catch-up reject-reply fix and the catch-up streaming-ceiling fix) to the two other places the same class of defect was found live during this feature's discovery pass: the configuration-push reply channel and the vote/PreVote endpoint failure-routing path.
**Users**: HA cluster operators (who experience the failure mode as a stalled or slow-to-converge cluster) and XCE maintainers (who consume the conformance record when reasoning about future HA changes).
**Impact**: Two adapter functions gain previously-missing reply/routing behavior (`handle_repl_config_push`, `vote_ep_err_handler`); one Cap'n Proto struct gains an additive field (`ReplConfigPushReply.term`); `src/controller/CLAUDE.md`'s existing TLA+ mapping tables are extended and reconciled against production behavior; no other subsystem changes.
### Goals
- Every reject of a replication-class message (WAL catch-up entry, configuration push) produces an explicit reply distinguishable from a lost message.
- Every vote/PreVote request outcome, including a failure arriving after connect, reaches the production tally.
- Every reply channel that can carry a peer's observed term can trigger term adoption.
- A single, explicit, written decision resolves which `AppendEntries` send mechanism is production-authoritative.
- A durable, precise mapping exists from each `raft.tla` action to the adapter code that realizes it, with a conformance verdict.
- The four already-conformant guarantee areas (`ClientRequest`, `AdvanceCommitIndex`/commit-source-ownership, restart/recovery, the two already-fixed term-adoption vectors) are protected by regression coverage so a future change cannot silently reopen them.
### Non-Goals
- No change to `src/lib/raft/raft.c`'s protocol logic (election, `logOk`, `(term,index)` truncation, `UpdateTerm`) — validated by `raft-log-replication`/`raft-commit-safety`/`raft-sim-conformance`.
- No configuration-*upgrade* materialized-state work (`config-upgrade-snapshot`'s scope).
- No controller voting-set/membership change support (`ha-config-bootstrap`'s scope).
- No change to `committed_lsn` advancement semantics beyond confirming the existing durable-majority-authoritative behavior is preserved.
- No wiring-in of `raft_send_append_entries` as the production send path (see Decision in Architecture below) — it remains dormant, now formally documented as such.
## Boundary Commitments
### This Spec Owns
- Reply-integrity behavior on the standby → primary direction for WAL catch-up rejects (regression-lock only, already shipped) and configuration-push rejects (new fix).
- Failure-routing behavior for vote/PreVote endpoints, across all failure timings (connect-time — regression-lock, already shipped; post-connect — new fix).
- The `term` field addition to `ReplConfigPushReply` and its use for term-adoption.
- The written conformance record (`CLAUDE.md` extension) mapping `raft.tla` actions to adapter code, including the explicit "documented dormant" resolution for the unused nextIndex-driven `AppendEntries` sender.
- Regression tests that lock in both the already-shipped guarantees and the guarantees this feature adds.
### Out of Boundary
- The Raft protocol library itself (`src/lib/raft/raft.c`/`raft.h`) — this feature never modifies it, only verifies the adapter drives it faithfully.
- Configuration-upgrade materialized-state consistency (`config-upgrade-snapshot`).
- Controller membership/quorum-set changes (`ha-config-bootstrap`).
- Client-side primary-discovery (bug-854) and multi-segment catch-up payload handling (bug-847).
- Any change to `committed_lsn` advancement authority (`ha-commit-engine`/`ha-term-safety` own that decision; this feature only confirms it is unchanged).
### Allowed Dependencies
- Raft library callback contract and entry points (`on_persist_entry`, `on_get_entry_term`, `on_send_append_entries`, `on_truncate_from`, `on_commit_advanced`, `raft_log_append`, `raft_on_append_entries_request`/`reply`, `raft_advance_commit`, vote/pre-vote handling) — consumed as-is, never modified.
- The existing out-of-band configuration-push transport (`ha-term-safety`) — this feature adds reply integrity to it without changing what it carries.
- The existing Cap'n Proto codegen build step (`src/lib/meson.build`) — consumed as-is for the additive schema field.
- Constraint: no dependency direction change — the adapter continues to depend on the library, never the reverse.
### Revalidation Triggers
- Any change to `vote_ep_ctx_t`'s layout or to how vote/PreVote endpoints are registered.
- Any change to `ReplConfigPushReply`'s wire shape (this feature is itself one such change; a future one must re-check `config_activate_committed`'s `-EAGAIN` handling described below).
- Any change that reintroduces a hard coupling between `raft_commit_index` and `committed_lsn` advancement (would violate a guarantee this feature regression-locks).
- Any change that gives `raft_send_append_entries` a production caller (would invalidate the "documented dormant" resolution and require revisiting Requirement 3's decision).
## Architecture
### Existing Architecture Analysis
The controller is a thin adapter over the Raft library (`ha-term-safety`'s framing, confirmed still accurate): the library owns election/commit/truncate decisions; the adapter's job is to persist what the library decides (`on_persist_entry`), source terms for it (`on_get_entry_term`), transmit its decisions on the wire, and turn wire replies back into library calls (`raft_on_append_entries_reply`, `raft_on_vote_reply`, `raft_on_pre_vote_reply`). This feature's defects are all instances of one pattern: an adapter code path receives or is meant to send a wire message, but a specific branch of that path (a reject, a late failure) returns without ever calling the corresponding "turn this into a library-visible outcome" step. The fix pattern established by bug-1479 (`ha_send_wal_ack_reply_ex`, plus its `XCE_TESTING` send-hook for testability) is reused verbatim for the two new fixes rather than inventing a new mechanism.
Integration points that must be maintained: the `ha_dispatch_gate` opcode allowlist (unaffected — no new opcode), the five-callback contract (unaffected — no callback signature changes), the two-tier replication model (unaffected — this feature only makes existing tiers report failure explicitly).
Technical debt addressed: three silent-drop-on-reject sites (WAL catch-up — done pre-feature; config-push — this feature) and one silent-drop-on-late-failure site (vote/PreVote post-connect). Technical debt formally reconciled rather than removed: the dormant nextIndex-driven `AppendEntries` sender (documented, not deleted — see Decision below).
### Architecture Pattern & Boundary Map
```mermaid
graph TB
subgraph Library
RaftLib[Raft library raft.c]
end
subgraph Adapter
HaC[ha.c callbacks and dispatch]
ReplC[repl.c WAL replication]
ConfigReplC[config_repl.c config push]
end
subgraph Wire
WalAck[WalAckReply term lsn success]
ConfigPushReply[ReplConfigPushReply accepted versionTag term]
VoteReply[RequestVoteReply and PreVoteReply]
end
subgraph Peer
Standby[Standby controller]
Primary[Primary controller]
end
RaftLib --> HaC
HaC --> ReplC
HaC --> ConfigReplC
ReplC --> WalAck
ConfigReplC --> ConfigPushReply
HaC --> VoteReply
WalAck --> Primary
ConfigPushReply --> Primary
VoteReply --> Primary
Standby --> WalAck
Standby --> ConfigPushReply
Standby --> VoteReply
Primary --> RaftLib
```
**Architecture Integration**:
- Selected pattern: extend-in-place (see `research.md` Architecture Pattern Evaluation) — reply-integrity fixes land inside the existing functions that already own each reply, no new cross-cutting abstraction introduced.
- Domain boundaries: `ha.c` owns election/vote wiring and top-level dispatch; `repl.c` owns WAL replication send/ack; `config_repl.c` owns out-of-band configuration push/ack — unchanged by this feature.
- Existing patterns preserved: the `XCE_TESTING` send-hook pattern (`ha_wal_ack_send_hook`), the `raft_on_higher_term_observed` single term-adoption entry point, the `vote_reply_dispatch`/`pre_vote_reply_dispatch` production tally paths.
- New components: none introduce a new boundary; all four code changes (config-push reply, config-push term field, vote endpoint handler split, CLAUDE.md conformance record) extend existing ownership.
- Steering compliance: preserves the async-I/O thread-role contract (all changes are in already-non-blocking callback/handler code, no new blocking calls introduced) and the "controller never self-terminates" invariant (no new terminal paths added).
### Decision: dormant `AppendEntries` sender resolution (Requirement 3)
No architecture change. `raft_send_append_entries`/`ha_cb_send_append_entries_stub` remains in the codebase, unused in production, and is now explicitly documented as such (see `research.md` Design Decisions for the full rationale). `repl_broadcast` + `ha_stream_wal_to_standby` remain the sole production `AppendEntries`-equivalent send/retry/backoff mechanism. Per Requirement 3.3 (amended after design review to permit this third resolution alongside retire/wire-in), this documentation-only resolution must additionally be backed by a non-regression guard: an `XCE_TESTING`-only call-counter on `ha_cb_send_append_entries_stub` (mirroring the `ha_wal_ack_send_hook` idiom) exercised by a test that drives the production heartbeat/replication path end-to-end and asserts the counter stays at zero while confirming `repl_broadcast`/`ha_stream_wal_to_standby` did fire — so a future change that accidentally starts calling this path is caught, not silently accepted. This adds one small `ha.c`/test diff beyond the pure documentation change described above (see File Structure Plan and Testing Strategy).
### Technology Stack
| Layer | Choice / Version | Role in Feature | Notes |
|-------|------------------|-----------------|-------|
| Wire serialization | Cap'n Proto (existing `c-capnproto`) | `ReplConfigPushReply` gains an additive `term @2: UInt64` field | Regenerated automatically by the existing `capnp compile` Meson `custom_target` (`src/lib/meson.build:1-10`); no build/Nix change needed (see `research.md`) |
| Controller adapter | C (gnu17), existing `src/controller/` | All behavior changes | No new dependency |
No other stack layer is touched.
## File Structure Plan
This is a small, surgical extension of an existing subsystem — files are listed individually rather than by directory pattern.
### Modified Files
- `src/schema/controller.capnp` — add `term @2: UInt64 $C.mapname("term")` to `struct ReplConfigPushReply` (Requirement 7.4).
- `src/controller/config_repl.c` — `handle_repl_config_push`: construct and send `ReplConfigPushReply{accepted=0, term}` on each of its four reject early-returns (malformed request, missing bytes/sha256, sha256 mismatch, `conf_save_config` failure); populate `term` on the existing success reply too; preserve the existing `-EAGAIN`-from-`config_activate_committed` behavior unchanged (still sends `accepted=1` — activation-deferred is not a rejection). Add an `XCE_TESTING`-only send-hook (e.g. `ha_config_push_reply_send_hook`), mirroring `ha_wal_ack_send_hook` (`ha.c:3085-3086`), so tests can observe the reply payload without a live endpoint. (Requirements 2.2, 2.3, 2.5, 7.4, 7.5)
- `src/controller/cluster.c` — the existing primary-side `ReplConfigPushReply` handler (`myreq_handler`, `cluster.c:745-762`) already branches on `cack->accepted` (ACKs via `repl_on_config_ack` on true, WARN-and-ignores on false — the false branch has simply never fired in production before this feature's fix). Add a term check alongside the existing `accepted` check: when `cack->term` exceeds the primary's current term, call `raft_on_higher_term_observed`, the same single adoption entry point the heartbeat-reply and WAL-NACK vectors already use. (Requirement 7.4, 7.5)
- `src/controller/ha.c`:
- Split `vote_ep_err_handler` into `vote_ep_err_handler` (real-vote endpoints, registered at the existing `ha.c:688-689` call site) and `pre_vote_ep_err_handler` (PreVote endpoints, registered at the existing `ha.c:827-828` call site), each calling `vote_reply_dispatch` or `pre_vote_reply_dispatch` respectively for the election-phase branch instead of the legacy `handle_request_vote_reply`. Extract the identical `CTRL_PRIMARY` cleanup branch (standby-endpoint removal, `repl_remove_standby`, quorum-transition logging) into a shared static helper called by both. (Requirements 4.2, 4.3, 4.4)
- Extend the `on_send_append_entries` doc-comment at `ha.c:2520-2532` to state plainly that this callback is reachable only via `raft_send_append_entries`, which has no production caller, and point to the CLAUDE.md conformance record for the authoritative production mechanism. Add an `XCE_TESTING`-only call counter to `ha_cb_send_append_entries_stub` (e.g. `ha_send_append_entries_stub_call_count`) as the non-regression guard the amended Requirement 3.3 requires. (Requirement 3.3)
- `src/controller/CLAUDE.md` — extend the "TLA+ Action-to-Code Mapping" table with corrected/added rows for `AppendEntries` reply integrity (WAL catch-up and config-push), vote/PreVote failure routing, and the dormant-sender resolution; extend the "Protocol Verification Checklist" with a checklist item for "every reject path sends an explicit reply." (Requirement 1.1-1.6)
- `.kiro/specs/ha-term-safety/spec.json` — append a one-line pointer on the existing `known_limitations` entry (index 30, the dormant `on_send_append_entries` note) referencing this spec's resolution, so a future reader lands on the authoritative decision. (Requirement 1.2, 1.6)
### New Test Additions (existing files extended, no new test files needed)
- `src/controller/testsuite/test_config_repl.c` — new tests: each of the four config-push reject paths sends `accepted=0` with the current term; the `-EAGAIN` activation-defer path still sends `accepted=1`; a higher term in an inbound config-push reply triggers adoption. (Requirements 2.2, 2.3, 2.5, 7.4, 7.5)
- `src/controller/testsuite/test_ha_vote_send.c` — new tests: a post-connect endpoint failure on a real-vote endpoint reaches `vote_reply_dispatch`'s tally; the same on a PreVote endpoint reaches `pre_vote_reply_dispatch`'s tally; reuses/extends `vote_ep_ctx_build_for_test` (`ha.c:7337`) as the test-construction seam. (Requirements 4.2, 4.3, 4.5)
- `src/controller/testsuite/test_ha_core_safety.c` — new test asserting `ha_send_wal_ack_reply_ex(success=0)` is actually invoked (via `ha_wal_ack_send_hook`) on a WAL catch-up reject, closing the reply-construction-boundary coverage gap found during design discovery. (Requirement 2.1, regression-lock)
- `src/controller/testsuite/test_commit_barrier_reply.c` or `test_commit_barrier_rollout.c` — new test: one failed payload in a submitted batch propagates failure to every payload in that batch. (Requirement 5.3)
### Unmodified, Regression-Locked Files
These files back the four already-conformant components (`ClientLogAppendGuarantee`, `CommitSourceGuarantee`, `TermDiscoveryCompleteness`, `RestartRecoveryGuarantee`). No source changes; cited here so each component in the table above resolves to a concrete file, and so a future diff touching them is recognized as revalidation-worthy rather than incidental.
- `src/controller/persist.c` — `ha_primary_log_append`, `persist_submit_job_payloads_async` (Requirement 5); confirmed conformant, existing tests in `test_commit_barrier_rollout.c`/`test_commit_barrier_reply.c`/`test_term_safety_adapter.c` cover it (plus the one new batch-propagation test above).
- `src/controller/commit.c` — `majority_candidate_locked`, `commit_set_raft_commit_index` (Requirement 6); confirmed conformant, `test_term_safety_adapter.c:646-780` already pins both directions of the advisory-vs-authoritative split, including the `XCE_TESTING` assert at `commit.c:222`.
- `src/controller/ha.c` — heartbeat-reply and WAL-NACK term-adoption call sites (Requirement 7.1, 7.2), `ha_sync_applied_lsn_from_recovery`/`ha_sync_raft_log_from_recovery`/phase2 retry helper/JIT log-length resync (Requirement 8); confirmed conformant, existing coverage in `test_persist_recovery.c`/`test_config_repl.c`.
No files are created; no files outside `src/controller/` (and its one schema dependency) are touched. `src/lib/`, `src/frontend/`, `src/worker/`, `src/scheduler/` are untouched.
## System Flows
### Configuration-push reject reply (Requirement 2, 7)
```mermaid
sequenceDiagram
participant Primary
participant Standby
Primary->>Standby: ReplConfigPushRequest bytes sha256 versionTag
alt Malformed or missing fields
Standby-->>Primary: ReplConfigPushReply accepted false term
else SHA-256 mismatch
Standby-->>Primary: ReplConfigPushReply accepted false term
else conf_save_config fails
Standby-->>Primary: ReplConfigPushReply accepted false term
else Save succeeds, activation deferred EAGAIN
Standby-->>Primary: ReplConfigPushReply accepted true term
Note over Standby: Activation retried later on commit advance
else Save and activation succeed
Standby-->>Primary: ReplConfigPushReply accepted true term
end
Primary->>Primary: distinguish reject from timeout, adopt term if higher
```
Key decision not obvious from the diagram: `accepted` means "bytes saved out-of-band," not "activated" — the activation-deferred branch and the fully-succeeded branch both reply `accepted=true` by design, since activation is gated separately on the committed marker (`ha-term-safety`) and is expected to retry.
### Vote/PreVote endpoint failure routing (Requirement 4)
```mermaid
sequenceDiagram
participant Candidate
participant Peer
Candidate->>Peer: connect and monitor
alt Connect or monitor fails
Candidate->>Candidate: vote_reply_dispatch or pre_vote_reply_dispatch denied
else Connect succeeds, later RDHUP or SSL error before reply
Candidate->>Candidate: vote_ep_err_handler or pre_vote_ep_err_handler
Candidate->>Candidate: vote_reply_dispatch or pre_vote_reply_dispatch denied
else Peer replies before any failure
Peer-->>Candidate: RequestVoteReply or PreVoteReply
Candidate->>Candidate: vote_reply_dispatch or pre_vote_reply_dispatch
end
Candidate->>Candidate: fast quorum decision once every peer outcome is counted
```
Key decision not obvious from the diagram: today only the "connect or monitor fails" and "peer replies" branches reach the production tally; this feature adds the "later RDHUP" branch to the same tally instead of the legacy no-op-counting path.
## Requirements Traceability
| Requirement | Summary | Components | Interfaces | Flows |
|---|---|---|---|---|
| 1.1 | Action-by-action mapping produced | ConformanceRecord | CLAUDE.md table | — |
| 1.2 | Live-vs-dormant path recorded on drift | ConformanceRecord | CLAUDE.md table, ha-term-safety spec.json pointer | Dormant sender decision |
| 1.3 | Gap detail sufficient to re-verify | ConformanceRecord | design.md + research.md | — |
| 1.4 | Fix vs documentation-drift classification | ConformanceRecord | design.md Decisions | — |
| 1.5 | Code-fix findings delivered under Req 2-8 | ConformanceRecord | (cross-reference) | — |
| 1.6 | Documentation-drift findings recorded for reconciliation | ConformanceRecord | ha-term-safety spec.json pointer | Dormant sender decision |
| 2.1 | WAL catch-up reject reply (regression-lock) | WalCatchupReplyIntegrity (existing) | `ha_send_wal_ack_reply_ex`, `ha_wal_ack_send_hook` | — |
| 2.2 | Config-push reject reply (new) | ConfigPushReplyIntegrity | `ReplConfigPushReply{accepted=0}` | Config-push reject flow |
| 2.3 | Reject distinguishable from transport loss | ConfigPushReplyIntegrity | `ReplConfigPushReply` vs no reply/timeout | Config-push reject flow |
| 2.4 | Unknown outcome not inferred as success | ConfigPushReplyIntegrity, WalCatchupReplyIntegrity | timeout handling (existing) | — |
| 2.5 | No regression to success-reply behavior | ConfigPushReplyIntegrity, WalCatchupReplyIntegrity | existing `accepted=1` / `success=1` paths | Config-push reject flow |
| 3.1 | Retry on rejection | Existing (`ha_raft_on_repl_nack`, `repl_start_catchup`) | — | — |
| 3.2 | Multi-round convergence | Existing (catch-up subsystem) | — | — |
| 3.3 | Single authoritative send mechanism | ConformanceRecord (documentation decision) | CLAUDE.md table, `ha.c:2520` doc-comment | Dormant sender decision |
| 3.4 | Transport-level send failure not treated as caught-up | Existing (`repl_broadcast`, heartbeat re-check) | — | — |
| 4.1 | Connect-time failure counted (regression-lock) | VoteEndpointFailureRouter (existing behavior) | `vote_reply_dispatch`, `pre_vote_reply_dispatch` | Vote failure routing |
| 4.2 | Post-connect failure counted (new) | VoteEndpointFailureRouter | `vote_ep_err_handler`, `pre_vote_ep_err_handler` | Vote failure routing |
| 4.3 | Same behavior for PreVote | VoteEndpointFailureRouter | `pre_vote_ep_err_handler` | Vote failure routing |
| 4.4 | No regression to connect/monitor routing | VoteEndpointFailureRouter | shared `CTRL_PRIMARY` cleanup helper | Vote failure routing |
| 4.5 | Fast quorum decision reachable | VoteEndpointFailureRouter | tally functions (existing) | Vote failure routing |
| 5.1 | Append persists or fails explicitly | ClientLogAppendGuarantee (existing) | `raft_log_append`, `ha_primary_log_append` | — |
| 5.2 | No log-length advance on failed persist | ClientLogAppendGuarantee | `raft_log_append` | — |
| 5.3 | Batch failure propagates to whole batch | ClientLogAppendGuarantee | `persist_submit_job_payloads_async` | — |
| 5.4 | No regression to failure propagation | ClientLogAppendGuarantee | existing tests | — |
| 6.1 | Durable-majority sole authority | CommitSourceGuarantee (existing) | `majority_candidate_locked` | — |
| 6.2 | Advisory index informational only | CommitSourceGuarantee | `commit_set_raft_commit_index` | — |
| 6.3 | No hard-ceiling reintroduction | CommitSourceGuarantee | `commit.c:139-155` invariant | — |
| 6.4 | Advance proceeds despite lagging advisory index | CommitSourceGuarantee | existing `XCE_TESTING` assert | — |
| 7.1 | Heartbeat-reply term adoption (regression-lock) | TermDiscoveryCompleteness (existing) | `raft_on_higher_term_observed` | — |
| 7.2 | WAL-NACK term adoption (regression-lock) | TermDiscoveryCompleteness (existing) | `raft_on_higher_term_observed` | — |
| 7.3 | Vote/PreVote reply term adoption (regression-lock) | TermDiscoveryCompleteness (existing) | `raft_on_vote_reply` Step 1 | — |
| 7.4 | Config-push reply carries term (new) | ConfigPushReplyIntegrity | `ReplConfigPushReply.term` | Config-push reject flow |
| 7.5 | No regression to existing term-adoption vectors | TermDiscoveryCompleteness, ConfigPushReplyIntegrity | — | Config-push reject flow |
| 8.1 | Recovery raises applied LSN/log length/term | RestartRecoveryGuarantee (existing) | `ha_sync_applied_lsn_from_recovery`, `ha_sync_raft_log_from_recovery` | — |
| 8.2 | Failed recovery step retries, no false-latch | RestartRecoveryGuarantee | phase2 retry helper (`ha.c:415-501`) | — |
| 8.3 | JIT log-length resync beyond boot | RestartRecoveryGuarantee | `ha_apply_term_entry_via_library` resync | — |
| 8.4 | No regression to clean-failover convergence | RestartRecoveryGuarantee | existing tests | — |
## Components and Interfaces
| Component | Domain/Layer | Intent | Req Coverage | Key Dependencies (P0/P1) | Contracts |
|---|---|---|---|---|---|
| ConfigPushReplyIntegrity | Controller/Replication | Send an explicit accept/reject reply, carrying term, for every configuration push | 2.2, 2.3, 2.5, 7.4, 7.5 | `ReplConfigPushReply` schema (P0) | Event |
| WalCatchupReplyIntegrity | Controller/Replication | Existing bug-1479 fix; regression-locked, not modified | 2.1, 2.4, 2.5 | `WalAckReply` schema (P0, existing) | Event |
| VoteEndpointFailureRouter | Controller/Election | Route every vote/PreVote endpoint failure, at any timing, to the production tally | 4.1-4.5 | `vote_reply_dispatch` (P0), `pre_vote_reply_dispatch` (P0), `vote_ep_ctx_build_for_test` (P1, test-only) | Service |
| ConformanceRecord | Documentation | Durable action-by-action mapping and dormant-path resolution | 1.1-1.6, 3.3 | `CLAUDE.md`, `ha-term-safety/spec.json` | State |
| ClientLogAppendGuarantee | Controller/Persistence | Existing conformant append-or-explicit-failure behavior; regression-locked | 5.1-5.4 | `persist.c` (existing) | State |
| CommitSourceGuarantee | Controller/Commit | Existing conformant durable-majority-authoritative behavior; regression-locked | 6.1-6.4 | `commit.c` (existing) | State |
| TermDiscoveryCompleteness | Controller/Election | Existing conformant term-adoption vectors (heartbeat, WAL-NACK, vote reply); regression-locked | 7.1-7.3, 7.5 | `raft_on_higher_term_observed` (existing) | Event |
| RestartRecoveryGuarantee | Controller/Recovery | Existing conformant restart/recovery reconcile; regression-locked | 8.1-8.4 | `ha.c` recovery functions (existing) | State |
Only the two components introducing new code (`ConfigPushReplyIntegrity`, `VoteEndpointFailureRouter`) warrant full detail blocks below; the regression-lock components need no new interface — their existing interfaces are cited in the table above and exercised by the tests in File Structure Plan.
### Controller / Replication
#### ConfigPushReplyIntegrity
| Field | Detail |
|-------|--------|
| Intent | Every configuration-push outcome (accept or reject) produces an explicit `ReplConfigPushReply` carrying the replying member's current term |
| Requirements | 2.2, 2.3, 2.5, 7.4, 7.5 |
**Responsibilities & Constraints**
- Owns the reply-construction boundary inside `handle_repl_config_push`; does not own configuration activation (`config_activate_committed` remains the sole activation gate, unchanged).
- Must not conflate an activation-deferred outcome (`-EAGAIN`) with a rejection — that path continues to reply `accepted=1`.
- Must include the current term on every reply, success or failure.
**Dependencies**
- Outbound: `ReplConfigPushReply` schema field (P0) — the additive `term` field this component populates.
- Outbound: `raft_on_higher_term_observed` (P0) — invoked by the primary's inbound-reply handler when this component's reply carries a higher term (existing entry point, no change to it).
- External: none.
**Contracts**: Event [x]
##### Event Contract
- Published: `ReplConfigPushReply {accepted: bool, versionTag: uint64, term: uint64}` — one per `ReplConfigPushRequest` received, always sent (never silently dropped).
- Subscribed: none (this component only replies).
- Ordering/delivery: request/reply pair over the existing outbound control channel (`ha-async-io.md` connection ownership); no new ordering guarantee introduced.
**Implementation Notes**
- Integration: reuses the existing `xce_reply`/`init_reply_by_req` pattern already present in `handle_repl_config_push`'s success path; the four reject branches gain the same call shape.
- Validation: `XCE_TESTING`-only send-hook (mirroring `ha_wal_ack_send_hook`) lets unit tests assert the reply payload without a live endpoint.
- Risks: none material — narrow, single-function change with a directly analogous shipped precedent (bug-1479).
### Controller / Election
#### VoteEndpointFailureRouter
| Field | Detail |
|-------|--------|
| Intent | Every vote/PreVote endpoint failure, regardless of when in the round it occurs, is counted through the same production tally as a delivered denial |
| Requirements | 4.1-4.5 |
**Responsibilities & Constraints**
- Owns endpoint-failure handling for outgoing vote and PreVote connections (`on_err` callbacks); does not own the tally logic itself (`vote_reply_dispatch`/`pre_vote_reply_dispatch` are consumed, not modified).
- Must preserve the existing `CTRL_PRIMARY` cleanup behavior (standby-endpoint deregistration, quorum-transition logging) identically for both endpoint kinds.
- Must not introduce a new discriminant field on `vote_ep_ctx_t`; the split-handler registration already distinguishes endpoint kind by call site.
**Dependencies**
- Outbound: `vote_reply_dispatch` (P0) — real-vote tally, already correct for connect/monitor failures.
- Outbound: `pre_vote_reply_dispatch` (P0) — PreVote tally, already correct for connect/monitor failures.
- Outbound: `vote_ep_ctx_build_for_test` (P1, test-only) — reused/extended as the unit-test construction seam.
- Inbound: iomgr `on_err` callback invocation (existing, unchanged trigger).
**Contracts**: Service [x]
##### Service Interface
```c
/* vote_ep_err_handler: on_err callback for outgoing real-vote endpoints.
* Election-phase branch now counts a post-connect failure through
* vote_reply_dispatch instead of handle_request_vote_reply. */
static int vote_ep_err_handler(void *ep_raw);
/* pre_vote_ep_err_handler: on_err callback for outgoing PreVote endpoints.
* Election-phase branch counts a post-connect failure through
* pre_vote_reply_dispatch. */
static int pre_vote_ep_err_handler(void *ep_raw);
/* shared helper: identical CTRL_PRIMARY cleanup extracted from both. */
static void vote_ep_handle_promoted_disconnect(ha_state_t *ha, endpoint_t *ep,
vote_ep_ctx_t *vctx);
```
- Preconditions: `ep->data` is a populated `vote_ep_ctx_t` for the endpoint kind the handler is registered against.
- Postconditions: exactly one outcome (denied/counted) reaches the applicable tally function per invocation; `CTRL_PRIMARY`-role invocations perform standby cleanup identically to today.
- Invariants: a candidate's own term is never mistaken for an observed peer term through this path (the synthesized outcome carries the candidate's own term, `granted=0` — unchanged from today's behavior).
**Implementation Notes**
- Integration: registration sites (`ha.c:688-689`, `827-828`) each point at the correspondingly-named handler; no change to endpoint setup beyond the callback pointer.
- Validation: new unit tests per File Structure Plan, using the reused test-construction helper to simulate a post-connect failure without a live peer.
- Risks: touches live election-path code — mitigated by keeping the extracted `CTRL_PRIMARY` branch byte-for-byte identical and requiring the new tests before this task is considered done (see `research.md` Risks & Mitigations).
## Data Models
### Data Contracts & Integration
**`ReplConfigPushReply` (Cap'n Proto, `src/schema/controller.capnp`)**
| Field | Ordinal | Type | Change |
|---|---|---|---|
| `accepted` | 0 | Bool | unchanged |
| `versionTag` | 1 | UInt64 | unchanged |
| `term` | 2 | UInt64 | **new, additive** |
Backward/forward compatibility: Cap'n Proto additive fields are safe under mixed-version rolling upgrade — an old-code standby's reply (no `term` field) decodes as `term=0` on a new-code primary. This is safe by construction: `0` can never present as a term higher than the primary's own current term (terms start at 1 and only increase), so no spurious adoption occurs; the primary simply doesn't get early term-discovery from an old-code peer's config-push replies, which is exactly today's (pre-feature) behavior for that peer. No explicit migration phasing is required beyond the existing rolling-upgrade tolerance the cluster already relies on.
`WalAckReply` is unchanged (already carries `term`, `lsn`, `success`); listed here only to confirm no modification is needed.
## Error Handling
### Error Strategy
This feature's domain is internal cluster wire-protocol reply integrity, not user-facing request handling — the applicable categories are protocol-rejection, transport-failure, and validation-failure, each with a distinct, already-established response pattern that this feature extends consistently.
### Error Categories and Responses
- **Validation/protocol rejection** (malformed config-push request, SHA-256 mismatch, `conf_save_config` failure): explicit reject reply (`accepted=0`) carrying term, so the primary can distinguish "actively rejected" from "lost" and react (retry, log, or escalate) — this is the category this feature's fixes target.
- **Transport failure** (connection drop, send failure, RDHUP): counted as a failure outcome through the same production tally/reply path used for a delivered rejection wherever the protocol model requires an outcome (vote/PreVote tally); where no reply is architecturally expected (snapshot chunk transfer, by explicit design — see requirements.md Boundary Context), recovery remains via the next heartbeat cycle, unchanged by this feature.
- **Activation-deferred, not an error** (`config_activate_committed` returning `-EAGAIN`): explicitly not classified as a rejection; the existing loud-fence-and-retry pattern (steering `ha-async-io.md`, "controller never self-terminates") already governs it and is preserved unchanged.
### Monitoring
No new metrics/logging channels introduced; existing `WARN`/`INFO` logging in `handle_repl_config_push` and `vote_ep_err_handler` is preserved and extended with the same log level conventions already used in each function.
## Testing Strategy
- **Unit Tests**:
1. `handle_repl_config_push` sends `ReplConfigPushReply{accepted=0, term}` for each of the four reject conditions (malformed request, missing bytes/sha256, sha256 mismatch, save failure) — via the new `XCE_TESTING` send-hook. (2.2, 2.3, 2.5)
2. `handle_repl_config_push` still sends `accepted=1` when `config_activate_committed` returns `-EAGAIN` (activation-deferred is not a rejection). (2.5, 7.5)
3. `ha_send_wal_ack_reply_ex(success=0)` is actually invoked on a WAL catch-up reject (closing the reply-construction-boundary coverage gap identified during design discovery). (2.1)
4. `vote_ep_err_handler` routes a synthetic post-connect failure through `vote_reply_dispatch`'s tally, using the reused test-construction helper. (4.2, 4.4)
5. `pre_vote_ep_err_handler` routes the same failure shape through `pre_vote_reply_dispatch`'s tally. (4.3)
6. A submitted batch with one failing payload propagates that failure to every payload in the batch. (5.3)
7. Driving the production heartbeat/replication path end-to-end never increments `ha_send_append_entries_stub_call_count`, while `repl_broadcast`/`ha_stream_wal_to_standby` do fire for the same scenario — the Requirement 3.3 non-regression guard. (3.3)
- **Integration Tests**:
1. A three-node cluster with one standby configured to reject a pushed configuration (bad hash) observes the primary correctly distinguishing the rejection from a timeout and not treating the push as silently lost.
2. A three-node election round where one peer's vote endpoint fails after connect (simulated) still reaches a quorum decision without waiting for the full election timeout.
3. Existing `test_term_safety_adapter.c` advisory-vs-authoritative commit tests re-run unmodified as the Requirement 6 regression lock (no new test needed, confirmed sufficient during design discovery).
4. Existing `test_persist_recovery.c`/`test_config_repl.c` recovery tests re-run unmodified as the Requirement 8 regression lock.
- **Live/Soak Tests** (per project convention for HA changes, `xce-test-ha` skill):
1. A rolling chaotic-failover soak where the test driver issues a configuration push whose payload deliberately does not match the sha256 it advertises (no special fault-injection hook needed — this exercises `handle_repl_config_push`'s real SHA-256-mismatch reject branch), confirming the primary observes the explicit reject rather than degrading to the timeout path.
2. A soak round exercising the post-connect vote/PreVote endpoint failure path end-to-end. **Open question for the tasks phase**: unlike item 1, this needs a way to sever an already-established outbound vote connection before a reply arrives (e.g. a firewall-rule toggle or proxy in the soak harness) — no existing `xce-test-ha` mechanism for this was confirmed during design. If no such mechanism exists cheaply, this item may be deferred; the unit tests (Requirement 4.2/4.3, via `vote_ep_ctx_build_for_test`) already give direct, deterministic coverage of the fix itself.
## Supporting References
- `research.md` — full gap analysis (asset map, options, effort/risk), design-phase discovery log, and the two recorded design decisions (dormant sender resolution, vote-handler split) with full rationale.
- `src/controller/CLAUDE.md` — "TLA+ Action-to-Code Mapping" and "Protocol Verification Checklist," extended by this feature as the durable conformance record (Requirement 1).

184
.kiro/specs/raft-adapter-conformance/requirements.md
View file

@ -1,84 +1,120 @@
# Requirements Document
## Project Description (Input)
## Introduction
Systematic conformance gap-validation of the XCE controller's Raft replication ADAPTER
(`src/controller/ha.c`, `repl.c`, `config_repl.c` — the real wire-level send/receive/reply
code) against `raft.tla`'s action model, action by action (AppendEntries request/response,
RequestVote, ClientRequest, AdvanceCommitIndex, term rules). This is distinct from and
complements the existing `raft-sim-conformance` spec, whose boundary is explicitly scoped to
`src/lib/raft/raft_sim.c` only (an in-process, in-memory harness validating the LIBRARY's
protocol semantics) and explicitly does not touch the controller adapter code.
This feature performs a systematic, action-by-action conformance audit of the XCE controller's Raft replication **adapter** — the wire-level send/receive/reply code in `src/controller/ha.c`, `repl.c`, and `config_repl.c` — against the actions of the canonical Raft protocol model (`raft/raft.tla`: `AppendEntries` request/response, `RequestVote`/`PreVote` request/response, `ClientRequest`/log append, `AdvanceCommitIndex`, term/`UpdateTerm` handling, and restart/recovery reconcile). It is distinct from and complements `raft-sim-conformance`, whose boundary is the in-process, in-memory library harness (`src/lib/raft/raft_sim.c`) validating the library's own protocol semantics; this feature validates that the controller adapter faithfully carries the library's decisions onto the wire without silently dropping a response, retry, or backoff step the protocol model assumes always happens.
**Motivating finding (already confirmed live — seed this spec with it as a known first
defect rather than rediscovering it):** during a fresh live-soak re-verification of
bug-1479's fix (`config-upgrade-snapshot` task 8.6's `--config-upgrade` scenario,
`run-ha-soak.sh`, deploy preserved at `~/tmp/xce_ha_bug1479_verify`), the fix's new
primary-side NACK-routing code (`ha_raft_on_repl_nack` + `repl_start_catchup` in
`repl_ack_decode_and_dispatch`'s WAL-ACK-reply-not-success branch, `ha.c` ~6680-6720) was
confirmed CORRECT but UNREACHABLE in production: `ha.c`'s `ha_apply_wal_catchup_entry` has
three reject paths (~3230-3232 for the term-carrying single/batch entry via
`ha_apply_term_entry_via_library`, ~3320-3321 for a batch sub-entry apply failure, ~3373-3374
for the legacy single-entry path) that all explicitly send NO reply at all on a logOk reject
/ forward-gap (not even a NACK) — and `ha_send_wal_ack_reply` (`ha.c:3084-3106`), the ONLY
function anywhere in the controller that ever constructs a `WalAckReply`, hardcodes
`success=1` unconditionally. There is no production code path that ever sends a failing
`WalAckReply`. In `raft.tla`, AppendEntries always yields a response (success true or false)
that drives nextIndex backoff; this codebase's silent-drop-on-reject deviates from that and
reproduces bug-1479's original permanent-quorum-loss symptom via a different mechanism
(confirmed live: `ctrl-2` standby stuck rejecting `ae_reject_logmismatch` at growing
`prevIdx` forever, `ctrl-1` primary logs zero WAL-NACK messages the entire time,
`committed_lsn` frozen, job submissions permanently stuck at 0/3 acked).
**Motivating finding.** A fresh live-soak re-verification of bug-1479's fix surfaced that the controller's only `WalAckReply` constructor hardcoded success, so no production code path ever sent a failing reply — a standby's WAL catch-up rejection was silently dropped rather than reaching the primary as a NACK, reproducing bug-1479's permanent-quorum-loss symptom by a different mechanism than the original defect. That gap, plus a second, independently-discovered adapter gap it exposed (a standby's catch-up streaming range was capped at the leader's committed point rather than its real log tail, creating a self-referential deadlock), are already fixed and live-verified as of this spec's seeding (see bug-1479 in `.wolf/buglog.json` and `config-upgrade-snapshot` task 8.6). This feature's job is the broader pass that finding was a symptom of: walk every Raft protocol action against the adapter code that is supposed to realize it, and treat each specific type of gap consistently rather than fixing instances one at a time as they surface via live incidents.
**Scope for this spec:**
1. Fix the concrete missing-NACK-reply defect at the three reject sites so a real
`WalAckReply{success=0}` reaches the primary and the bug-1479 NACK-routing code can
actually engage.
2. Beyond that one confirmed defect, perform the broader systematic pass — walk `raft.tla`'s
action list (AppendEntries send+response, RequestVote/PreVote send+response,
ClientRequest/log append, AdvanceCommitIndex, term/UpdateTerm handling,
Restart/recovery) against the corresponding controller adapter code paths in
`ha.c`/`repl.c`/`config_repl.c` and flag every other place a response, retry, or backoff
the spec model assumes is silently missing or short-circuited — the same class of gap
this first finding represents.
**Audit results seeded into this requirements pass.** A fresh action-by-action pass performed for this spec, cross-referenced against prior `raft-log-replication`/`raft-commit-safety`/`ha-term-safety`/`raft-sim-conformance` validation notes, confirmed:
- Two areas already conformant and worth protecting with regression coverage: `ClientRequest`/log append integrity, and the durable-majority-authoritative / advisory-raft-index split for `AdvanceCommitIndex`.
- Two live, unfixed gaps of the same class as the motivating finding: a standby's configuration-push rejection (malformed message, missing fields, checksum mismatch, or save failure) never sends an explicit reject reply, so the primary cannot distinguish an actively-rejected push from a lost packet; and a vote/PreVote endpoint failure arriving after connect (not at connect time) is counted through a legacy path instead of the production vote-tally path, reopening — for that one failure window — the same silent-drop defect class bug-1423 already fixed for connect-time failures.
- One architecture question requiring a design decision rather than a behavioral fix: a fully protocol-conformant, per-peer index-driven `AppendEntries` sender exists in the codebase but has zero production callers; the mechanism actually driving replication in production today is a different, coarser substitute (broadcast-plus-catch-up-marking) that converges correctly post-fix but is not the mechanism the design documentation and the protocol model depict.
**Depends on / builds on:** `raft-log-replication`, `raft-commit-safety`, `ha-term-safety`,
`raft-sim-conformance` (consumes their library entry points and prior `validate_impl`
`known_limitations`, several of which already flagged "documentation drift" around this
exact send/receive boundary — cross-reference those notes rather than re-deriving them).
Per this feature's scope decision, the confirmed live gaps found during this pass are fixed within this feature rather than deferred to separate specs, following the same precedent as the motivating finding.
**Related bugs:** bug-1479 (this session's re-verification, `buglog.json`, occurrence 2 —
still not fully fixed), bug-1466/1471/1472/1474/1475/1477/1478 (same HA-replication defect
family).
## Boundary Context
## Status Update (2026-07-29)
**Scope item 1 (the concrete missing-NACK-reply defect) is FIXED**, as part of closing out
`config-upgrade-snapshot` task 8.6 / bug-1479. `ha_apply_term_entry_via_library`'s logOk-reject
path (the one live-reachable reject site of the three named above — the other two are
documented dormant/unreachable with real senders, see `ha_apply_wal_catchup_entry`'s own
comments) now sends a real `WalAckReply{success=0}` via a new `ha_send_wal_ack_reply_ex`
helper, confirmed live: the primary now genuinely receives and routes the NACK
(`ha_raft_on_repl_nack`, previously-correct-but-unreachable code, now actually fires).
Closing that gap alone was still not sufficient — it exposed a SECOND, independent adapter
gap (also now fixed, also a `raft.tla`-deviation of exactly the kind this spec's scope item 2
is meant to find systematically): `ha_stream_wal_to_standby`'s catch-up range was hard-bounded
at `committed_lsn` rather than the leader's real log tail, which is a `raft.tla` deviation
(`leaderCommit` is advisory to the follower in the real spec, never a send-cap) and caused a
self-referential deadlock whenever the streamed-to standby's own ack was required to advance
`committed_lsn` in the first place. Fixed via a new `ha_compute_repl_stream_ceiling_lsn` pure
function. Full live re-verification (`run-ha-soak.sh --config-upgrade`, 3/3 `force_elect_node`
rounds) now PASSES end-to-end. See `.wolf/buglog.json` bug-1479 and
`config-upgrade-snapshot/tasks.md` task 8.6's Implementation Notes for the complete fix
narrative and evidence.
**Scope item 2 (the broader systematic `raft.tla` action-by-action pass) remains open and
valid** — the fact that fixing item 1 alone immediately surfaced a second, independent
sibling gap (the committed_lsn-vs-log-tail conflation) is itself evidence this class of
adapter/spec-model divergence is not a one-off, and a systematic pass is still worth doing
to find any remaining siblings before they're each independently rediscovered via a live
soak the way these two were.
- **In scope**:
- A documented, action-by-action mapping between each `raft.tla` protocol action and the controller adapter code path(s) that realize it, with an explicit conformance verdict per action and citations precise enough to be independently re-verified.
- Reconciling documentation-vs-code drift the audit finds at this boundary (a design document describing a code path that is not the one actually exercised in production) by recording which path is live and which is dormant or superseded.
- Fixing every live gap the audit confirms, where the adapter fails to produce a response, retry, or backoff step that the protocol model or an already-shipped sibling fix assumes exists — including the configuration-push rejection reply and the post-connect vote/PreVote failure routing named above, and any other such gap the systematic pass confirms.
- Regression coverage that locks in both the gaps already fixed before this spec (WAL catch-up rejection replies, catch-up streaming ceiling, vote/PreVote connect-time failure routing, restart/recovery log resync) and the gaps this spec fixes, so a future change cannot silently reopen either.
- A single, unambiguous decision (recorded as a requirement) about which `AppendEntries` send/retry/backoff mechanism is production-authoritative — the existing per-peer index-driven sender, or the broadcast-plus-catch-up substitute — so the codebase does not carry two divergent implementations of the same guarantee.
- **Out of scope**:
- Any change to the Raft protocol logic itself (election, `logOk` freshness, `(term,index)` truncation, current-term commit, `UpdateTerm`) inside `src/lib/raft/raft.c` — that logic is validated by `raft-log-replication`, `raft-commit-safety`, and `raft-sim-conformance`; this feature validates only that the controller adapter carries the library's decisions onto the wire.
- Configuration-*upgrade* materialized-state consistency (full entity-bearing state snapshot at a configuration change boundary) — remains the independent `config-upgrade-snapshot` spec's scope.
- Changes to the controller voting set (adding/removing a controller peer) — remains `ha-config-bootstrap`'s scope; this feature does not introduce quorum/membership transitions.
- Client-side primary-discovery behavior (bug-854) and catch-up payload handling across multiple WAL segments (bug-847) — tracked separately, not part of the adapter's reply-integrity contract this feature audits.
- Any change to when or how the primary decides to advance `committed_lsn` beyond preserving the existing durable-majority-authoritative behavior — that decision boundary belongs to `ha-commit-engine`/`ha-term-safety`.
- **Adjacent expectations**:
- Relies on the raft library's callback contract (`on_persist_entry`, `on_get_entry_term`, `on_send_append_entries`, `on_truncate_from`, `on_commit_advanced`) and entry points (`raft_log_append`, `raft_on_append_entries_request`/`reply`, `raft_advance_commit`, vote/pre-vote handling) exactly as delivered by the Raft TLA+ Conformance series; this feature does not change that contract, only how faithfully the adapter drives it.
- Relies on the out-of-band configuration-push transport (`ha-term-safety`) continuing to carry configuration bytes hash-verified against the committed `(term, lsn)` marker; this feature adds reply integrity to that transport's rejection path without changing what it carries.
- Any gap this feature's audit finds that is genuinely new protocol-model territory (not covered by an existing sibling spec's boundary) is fixed here; any gap that falls within an existing sibling spec's stated scope is cross-referenced to that spec rather than duplicated.
## Requirements
<!-- Will be generated in /kiro-spec-requirements phase -->
### Requirement 1: Systematic Raft adapter conformance audit
**Objective:** As a maintainer, I want a documented, action-by-action mapping between the Raft protocol model's actions and the controller's adapter code, so that gaps between the model's assumptions and production behavior are found deliberately and once, instead of being rediscovered independently through separate live incidents.
#### Acceptance Criteria
1. The conformance audit shall produce a written mapping that lists, for each of `AppendEntries` request, `AppendEntries` response, `RequestVote`/`PreVote` request, `RequestVote`/`PreVote` response, `ClientRequest`/log append, `AdvanceCommitIndex`, term/`UpdateTerm` handling, and restart/recovery reconcile, the controller adapter code path(s) that realize it and a conformance verdict.
2. When the audit finds that a sibling spec's design documentation describes an adapter code path that differs from the code path actually exercised in production, the audit shall record which path is live and which is dormant or superseded, rather than leaving the discrepancy implicit.
3. When the audit finds a gap where the protocol model assumes a response, retry, or backoff step that the adapter does not perform, the audit shall record the affected code path, the triggering condition, and the observed effect in enough detail to be independently re-verified without re-deriving the finding from scratch.
4. The audit shall classify each finding as either requiring a code fix (a live path that violates the model's guarantee) or documentation-only drift (the guarantee holds in production, but design documentation describes a different mechanism than what runs).
5. Where a finding is classified as requiring a code fix, the fix shall be delivered under this feature's Requirements 2 through 8, not deferred to a separate spec.
6. Where a finding is classified as documentation-only drift, the audit shall record the correction needed so the affected design documentation can be reconciled without re-investigating the code.
### Requirement 2: AppendEntries-equivalent rejection reply integrity
**Objective:** As an operator of an HA controller cluster, I want every replication message a standby rejects to produce an explicit reply back to the primary, so that the primary always learns of a rejection and can retry or re-route rather than the cluster stalling on a silently dropped reject.
#### Acceptance Criteria
1. When a standby rejects a WAL catch-up entry for any reason (log mismatch, forward gap, or apply failure), the Controller shall send an explicit reply indicating rejection back to the primary.
2. When a standby rejects a configuration push for any reason (malformed message, missing required fields, checksum mismatch, or a local save failure), the Controller shall send an explicit reply indicating rejection back to the primary, mirroring the acceptance reply already sent on a successful push.
3. The rejection reply for a configuration push shall be distinguishable by the primary from a lost or undelivered push, so that the primary's push-and-wait logic never treats an active rejection and a transport loss as the same outcome.
4. While no explicit rejection or acceptance reply has arrived within the applicable transport wait bound, the Controller shall continue to treat the outcome as unknown rather than inferring success.
5. The Controller shall not regress the acceptance-reply behavior for a successful WAL catch-up entry or a successful configuration push.
### Requirement 3: AppendEntries send retry and backoff guarantee
**Objective:** As an operator, I want exactly one production-authoritative mechanism driving `AppendEntries`-equivalent replication retries and per-peer backoff, so that a rejected or lost replication attempt is corrected without relying on two divergent, only-one-of-which-is-live implementations of the same guarantee.
#### Acceptance Criteria
1. When the primary receives an explicit rejection reply for a replicated entry, the Controller shall retry replication to that standby from an earlier point rather than repeating the same rejected attempt.
2. While a standby remains behind after a rejection, the Controller shall continue advancing that standby's catch-up point across successive rounds until it converges, rather than requiring convergence in a single round.
3. The Controller shall have exactly one send/retry/backoff mechanism that is authoritative in production for `AppendEntries`-equivalent replication; where more than one fully-implemented mechanism exists in the codebase, the non-authoritative one shall be resolved by one of: retiring it, making it the production path, or formally documenting it as an intentionally-dormant reference path guarded against silent regression (e.g. an explicit non-production marker plus a test asserting it stays uncalled in production) — so the codebase never carries two divergent, unreconciled implementations of the same guarantee.
4. If a replication send fails at the transport level (not a rejection), the Controller shall not treat the standby as caught up, and shall re-attempt replication to it within a bounded number of subsequent replication cycles.
### Requirement 4: RequestVote/PreVote reply integrity across all failure modes
**Objective:** As an operator, I want every vote or pre-vote request outcome — granted, denied, or a failure that prevents a reply — to be counted through the same production tally, regardless of when in the round the failure occurs, so that an election or pre-vote round can reach its fast "all replied" decision instead of always falling back to the full round timeout.
#### Acceptance Criteria
1. When a vote request cannot be delivered because the connection could not be established, the Controller shall count that outcome through the same production vote-tally path used for a delivered denial.
2. When a vote request connection is established but then fails before a reply arrives (a post-connect failure), the Controller shall count that outcome through the same production vote-tally path used for a connect-time failure and for a delivered denial, rather than through a separate legacy path that does not update the tally.
3. The Controller shall apply the same failure-counting behavior to a pre-vote request as to a real vote request.
4. The Controller shall not regress the existing connect-time and monitor-time failure routing already verified for vote and pre-vote requests.
5. When every peer in an election or pre-vote round has replied or had its outcome counted, the Controller shall be able to reach a quorum decision without waiting for the round's full timeout.
### Requirement 5: ClientRequest / log append integrity
**Objective:** As a maintainer, I want a client write to always resolve to either an acknowledged append or an explicit, propagated failure, so that a write is never silently lost between submission and its outcome.
#### Acceptance Criteria
1. When the primary appends a client-submitted entry, the Controller shall either persist the entry and make it eligible for replication, or return an explicit failure to the submitter.
2. The Controller shall not advance the log length for an entry whose persistence failed.
3. When a batch of submitted entries includes one that fails to persist, the Controller shall propagate that failure to every entry in the same batch rather than silently completing a subset.
4. The Controller shall not regress the existing behavior where a persistence failure is returned to the caller rather than swallowed.
### Requirement 6: Commit advancement source-of-truth preserved
**Objective:** As an operator, I want the durable-majority barrier to remain the sole authority for advancing the committed point, so that the advisory Raft commit index can never gate, delay, or override a safe commit advance.
#### Acceptance Criteria
1. The Controller shall advance the committed point using the durable-majority barrier as the sole authoritative source.
2. The Controller shall treat the Raft library's advisory commit index as informational only; it shall neither block nor be required for the durable-majority barrier to advance the committed point.
3. The Controller shall not reintroduce a hard ceiling that requires both the durable-majority barrier and the advisory Raft commit index to agree before advancing the committed point.
4. When the advisory Raft commit index lags the durable-majority watermark, the Controller shall still advance the committed point according to the durable-majority barrier.
### Requirement 7: Term discovery completeness
**Objective:** As an operator, I want every reply channel capable of carrying a peer's observed election term to trigger term adoption when that peer's term is higher, so that a stale-term node cannot remain stuck retrying against a peer that has already moved to a higher term.
#### Acceptance Criteria
1. When a heartbeat reply carries a term higher than the primary's current term, the Controller shall adopt the higher term and step down from any leadership held at the lower term.
2. When a WAL rejection reply carries a term higher than the sender's current term, the Controller shall adopt the higher term.
3. When a vote or pre-vote reply carries a term higher than the candidate's current term, the Controller shall adopt the higher term.
4. When a configuration-push reply is sent, the Controller shall include the replying member's current term, so a higher term can be discovered through that reply channel as it already is through heartbeat and WAL-rejection replies.
5. The Controller shall not regress the existing heartbeat-reply and WAL-rejection term-adoption behavior already verified for this cluster.
### Requirement 8: Restart / recovery reconciliation conformance
**Objective:** As an operator, I want a restarted or rejoining controller to reconcile its applied state, log length, and term from its durable floor every time recovery runs, so that a boot-ordering or partial-failure hazard cannot leave a node permanently out of sync with what it has actually persisted.
#### Acceptance Criteria
1. When a controller completes startup recovery, the Controller shall raise its applied LSN, log length, and term from the recovered durable floor, never leaving them below what recovery found.
2. If a recovery step fails partway through cluster initialization, the Controller shall retry that step rather than permanently latching a completed state without having finished it.
3. While a term-safe append-entries receive occurs after startup, the Controller shall re-check and, if needed, re-raise its log length from the durable floor, not only at boot time.
4. The Controller shall not regress the existing byte-identical convergence behavior for a single clean failover and restart.

231
.kiro/specs/raft-adapter-conformance/research.md Normal file
View file

@ -0,0 +1,231 @@
# Gap Analysis: raft-adapter-conformance
Analysis date: 2026-07-29. Requirements phase: generated (not yet approved). Per skill fallback rule, proceeding — gap analysis findings can also inform requirement refinement before approval.
## Method
Codebase investigation used direct `Read`/`grep` against `src/controller/ha.c`, `repl.c`, `config_repl.c`, `cluster.c`, `src/lib/raft/raft.c`/`raft.h`, `src/schema/controller.capnp`, and `src/controller/testsuite/`, plus the two Explore-agent passes already run during requirements generation (their findings are the primary input to Requirement-to-Asset mapping below; this pass adds the concrete integration-point evidence — schema fields, dispatcher code, struct layouts — needed to select an implementation approach). No external dependency research was needed; this is entirely an internal-codebase conformance question.
## Requirement-to-Asset Map
### Requirement 1 — Systematic conformance audit deliverable
| Asset | Status |
|---|---|
| `raft/raft.tla` (canonical action list: `Restart`, `Timeout`, `RequestVote`, `BecomeLeader`, `ClientRequest`, `AdvanceCommitIndex`, `AppendEntries`, `Receive`→`UpdateTerm`/`HandleRequestVoteRequest`/`HandleRequestVoteResponse`/`HandleAppendEntriesRequest`/`HandleAppendEntriesResponse`) | Present, unmodified upstream copy |
| `src/controller/CLAUDE.md` → "TLA+ Action-to-Code Mapping" + "Protocol Verification Checklist" tables | **Present and already the natural home for this deliverable** — but incomplete/partially stale (its own checklist entry admits a 2026-07-18 citation correction; it does not yet list the two already-fixed reply-integrity gaps or the two newly-found ones) |
| `ha-term-safety/spec.json` `known_limitations` (3 entries) + `tasks.md:195` | Present — already contain two of the three documentation-drift notes this audit would otherwise re-derive (dormant `on_send_append_entries`/`raft_send_append_entries` send path; advisory-vs-authoritative commit index) |
| `raft-sim-conformance/spec.json` `known_limitations` (3 entries) | Present — contains the follower-commit-index documentation-drift note |
| A standalone, dedicated conformance-map document | **Missing** (no such file exists yet under `.kiro/specs/raft-adapter-conformance/` or `docs/`) |
**Gap tag**: Constraint — CLAUDE.md's table is intentionally terse (a living quick-reference, not an evidentiary audit trail); the systematic pass's detailed findings (file:line citations, live-vs-dormant verdicts, reasoning) don't fit there without bloating it.
### Requirement 2 — AppendEntries-equivalent rejection reply integrity
| Sub-area | Asset | Status |
|---|---|---|
| WAL catch-up reject reply | `ha_send_wal_ack_reply_ex` (bug-1479, `ha.c` ~6680+), `WalAckReply` schema (`controller.capnp:876-880`, already has `term`, `lsn`, `success`) | **Fixed and live.** Gap tag: Unknown — need to confirm a regression test explicitly asserts a reject reply is sent (not just that the NACK routes once received); `test_ha_core_safety.c` has adjacent pin tests (`test_2_3_termblind_route_payload_guarded`, `test_2_3_unrepairable_catchup_raises_nonpromotable_fence`) but their scope wasn't fully re-verified this pass. |
| Config-push reject reply | `handle_repl_config_push` (`config_repl.c:1261-1337`); dispatch site `cluster.c:652-655` (`rc = handle_repl_config_push(...)`, no reply built on any `rc`); contrast `cluster.c:767-772` `default:` case, which *does* send a generic error reply on unknown opcode | **Missing — confirmed live gap.** Read in full this pass: 4 early returns (`-EINVAL` malformed body ~1266, `-EINVAL` missing bytes/sha256 ~1286, `-EBADMSG` sha mismatch ~1294, `rc` from `conf_save_config` failure ~1309) all `return` before constructing any `repl_config_push_reply_t`. Only the success tail (~1326-1335) builds and sends `{accepted=1, version_tag}`. The outer dispatcher never synthesizes a reply from a nonzero `rc` for this opcode — logged bug-1480. |
| `ReplConfigPushReply` schema | `controller.capnp:805-808`: `struct ReplConfigPushReply { accepted @0: Bool; versionTag @1: UInt64; }` | **Constraint, not itself missing for the reject-reply fix** — adding `accepted=0` needs no schema change (the field already exists); only Requirement 7's term field is a schema addition (see below). |
| Distinguishing reject from activation-deferred | `config_activate_committed` returning `-EAGAIN` (activation gated on committed marker, by design) does **not** early-return in `handle_repl_config_push` — the function still falls through to send `accepted=1` in that case | **Important design nuance for the fix**: `accepted` currently means "bytes saved out-of-band," not "activated." The reject-reply fix must not conflate the two — an `-EAGAIN` activation defer is expected/retriable and must keep sending `accepted=1`; only the four genuine save/validation failures above should send `accepted=0`. |
### Requirement 3 — AppendEntries send retry/backoff, single authoritative mechanism
| Asset | Status |
|---|---|
| `raft_send_append_entries` (`raft.c:595`) — protocol-conformant, nextIndex-driven | **Confirmed zero production callers** (only `raft_sim.c` and an `XCE_TESTING` helper `ha_cb_send_append_entries_for_test`) — re-verified this pass via full-repo grep. |
| `ha_cb_send_append_entries_stub` (`ha.c:2533-2574`) — the registered `on_send_append_entries` callback, delegates to `repl_send_append_entries` | Registered (`s_real_cbs`) but **only reachable via `raft_send_append_entries`**, which nothing in the live controller calls — so this callback is wired but dormant, confirmed by its own call graph, not just the sibling-spec note. |
| `repl_broadcast` (`repl.c:295-380`) + `ha_stream_wal_to_standby` (`config_repl.c`, catch-up streamer) | **This is the actual production send/retry mechanism** — unconditional newest-entry broadcast plus heartbeat-driven catch-up range streaming, not nextIndex-indexed. |
| **A prior, already-recorded design decision** — `test_ha_core_safety.c:2474`: *"DECISION: DOCUMENT SAFE-BY-GUARD, do not structurally retire."* Reasoning given: the term-blind branches this dead path would exercise are (a) unreachable with real data today, and (b) even if reached, already guarded elsewhere (`persist_wal_write_replicated`'s (term,lsn,payload-crc) guard + sticky non-promotable fence). | **This materially changes the decision space** for Requirement 3 AC3. A prior investigation already considered and rejected structural retirement, for reasons that still hold. This is not new information the design phase needs to re-derive — it's an existing, reasoned precedent. |
**Gap tag**: Constraint (design precedent already exists and argues against literal retirement) — see Options below.
### Requirement 4 — RequestVote/PreVote reply integrity, all failure modes
| Asset | Status |
|---|---|
| `vote_ep_err_handler` (`ha.c:7564-7608`, read in full this pass) | Confirmed: election-phase branch calls `handle_request_vote_reply(ha, vctx->ctx, vctx->peer_idx, vctx->my_term, 0, 0, 0)` directly — the legacy path, bypassing `vote_reply_dispatch`/`pre_vote_reply_dispatch`. |
| `vote_ep_ctx_t` (`ha.c:46-61`, full struct read this pass) | **Missing a discriminant.** The struct has no field distinguishing a real-vote endpoint from a PreVote endpoint — `peer_idx`, `my_term`, `my_tag`, `my_lsn`, `my_last_log_term/index`, `my_name`, but nothing like `is_prevote`. `vote_ep_err_handler` is registered identically at both call sites (`ha.c:688-689` real-vote, `ha.c:827-828` PreVote) and currently has no way to route correctly even if it tried. |
| `vote_reply_dispatch` (~`ha.c:6333-6394`), `pre_vote_reply_dispatch` (~`ha.c:7982-8000`) | Both exist and are already the correct production tally paths for the connect-time/monitor-time failures bug-1423/1424 fixed. |
**Gap tag**: Missing (discriminant field + routing branch) — concrete, bounded fix; not an unknown.
### Requirement 5 — ClientRequest / log append integrity
| Asset | Status |
|---|---|
| `raft_log_append` (`raft.c:562-593`), `ha_primary_log_append` (`ha.c:4777-4823`), `append_job_wal_entry_local`/`persist_submit_job_payloads_async` (`persist.c:6081-6199`) | Traced and confirmed conformant this pass (persist-before-act, no phantom log-length advance, failures propagated per-payload, first-failure rc propagated across a batch). |
**Gap tag**: Unknown (test-coverage only) — no code gap found; need to confirm which existing test IDs already assert "failed persist ⇒ no log-length advance ⇒ caller sees the failure" as a locked invariant, or whether this needs a new regression test. Not investigated further this pass (out of gap-analysis scope per the framework's "defer deep research to design phase").
### Requirement 6 — AdvanceCommitIndex / commit-source-ownership
| Asset | Status |
|---|---|
| `raft_advance_commit` (`raft.c:769-821`), `ha_cb_commit_advanced_stub` (`ha.c:2616-2667`) → `commit_set_raft_commit_index` (`commit.c:1082-1095`, advisory-only, monotonic) | Confirmed conformant. |
| `majority_candidate_locked` (`commit.c:121-198`) — the actual authoritative ceiling, capped at `persist_durable_lsn` | Confirmed authoritative; `commit.c:139-155` comment documents *why* a prior `min(durable-majority, raft_commit_index)` coupling was tried and removed (starvation). |
| `XCE_TESTING` assert at `commit.c:222` (`committed_lsn` never exceeds `persist_durable_lsn`) | **Existing regression guard already present** — a strong asset; likely already catches any future accidental re-coupling. |
**Gap tag**: Low — mostly already regression-guarded by the existing `XCE_TESTING` assert; confirm during design whether an additional explicit test naming `raft_commit_index` as advisory-only is warranted, or whether the existing assert suffices.
### Requirement 7 — Term discovery completeness
| Asset | Status |
|---|---|
| Heartbeat-reply (`ha.c:6636-6651`) and WAL-NACK (`ha.c:6746-6784`) term-adoption vectors, both feeding `raft_on_higher_term_observed` (`raft.c:855-874`) | Confirmed fixed and live (bug-1421). |
| `WalAckReply` schema already carries `term @0` | Present — no schema work needed for the WAL-NACK vector. |
| `ReplConfigPushReply` schema (`controller.capnp:805-808`) | **Missing a `term` field** — confirmed by direct read this pass: only `accepted`/`versionTag`. Adding a reply-carried term here is an additive `.capnp` schema change (new field number, e.g. `term @2: UInt64`), not a breaking one. |
| Cap'n Proto codegen build step | **Unknown this pass** — need to confirm which Meson target regenerates the `.capnp.c`/`.capnp.h` from `controller.capnp` (likely `capnpc` invoked from the top-level `meson.build`), and whether the historical `nixSupport/xce.nix` capnpc-pin issue (bug-918, already fixed) has any bearing on a schema *field addition* (as opposed to a `c-capnproto` version bump) — almost certainly not, but worth a one-line confirmation in design rather than assuming. |
**Gap tag**: Missing (schema field) + Unknown (codegen build-step confirmation, low-risk).
### Requirement 8 — Restart/recovery reconciliation
| Asset | Status |
|---|---|
| `ha_sync_applied_lsn_from_recovery`/`ha_sync_raft_log_from_recovery` (`ha.c:5296-5351`), phase2 retry helper (`ha.c:415-501`), promotion call sites (`ha.c:1867-1897`, `2112-2131`), JIT log_len resync (`ha.c:3200-3204`) | Confirmed conformant and already fixed (bug-1042, bug-1477, bug-1475). |
**Gap tag**: Unknown (test-coverage only, same caveat as Requirement 5/6) — no code gap found.
## Implementation Approach Options
### Requirement 1 (audit deliverable)
- **Option A — Extend `CLAUDE.md`'s existing tables.** Update the "TLA+ Action-to-Code Mapping" and "Protocol Verification Checklist" in place; add rows/checklist items for the reply-integrity guarantees this feature establishes. ✅ single source of truth, no new doc to keep in sync. ❌ CLAUDE.md is meant to stay terse — doesn't have room for full per-finding evidence (file:line trails, live-vs-dormant reasoning) without bloating a document every session reads.
- **Option B — New standalone conformance-map document** (e.g. `.kiro/specs/raft-adapter-conformance/design.md`'s own dedicated section, or a `docs/raft-adapter-conformance.md`). ✅ room for full evidentiary detail. ❌ a second place that can drift from CLAUDE.md's summary table, recreating exactly the "documentation drift" class of problem this feature exists to fix.
- **Option C — Hybrid (recommended direction for design phase).** CLAUDE.md's table stays the terse, always-current pointer (updated as part of this feature's fixes); the full audit trail with citations and verdicts lives in this spec's own `design.md`, referenced *from* CLAUDE.md by name so there is one canonical detailed source and one canonical terse pointer, not two competing summaries.
### Requirement 2 (rejection reply integrity)
- **Option A — Extend `handle_repl_config_push` in place**, mirroring the `ha_send_wal_ack_reply_ex` pattern already established for the WAL catch-up fix: build and send a `ReplConfigPushReply{accepted=0}` at each of the four reject early-returns, taking care not to touch the `-EAGAIN` activation-defer path (which correctly stays `accepted=1`). ✅ minimal, uses an established in-codebase pattern, touches one function. ❌ none significant — this is a narrow, well-bounded fix.
- No Option B/C considered necessary — the fix is a direct, small extension of an existing function using a precedent already proven in production (bug-1479). Effort/risk assessment below reflects this.
### Requirement 3 (single authoritative send/retry mechanism)
- **Option A — Document as intentionally dormant (recommended, matches existing precedent).** Formalize the `test_ha_core_safety.c:2474` decision into the design doc and CLAUDE.md: `raft_send_append_entries`/`ha_cb_send_append_entries_stub` remain in the codebase as a library-conformant reference implementation exercised only by `raft_sim`/tests, explicitly marked as such; `repl_broadcast` + catch-up streaming remains the sole production mechanism. No behavior change. ✅ lowest risk, respects a reasoned prior decision, zero regression surface. ❌ leaves two implementations in the codebase long-term (a documentation cost, not a correctness one).
- **Option B — Structurally retire the dead path** (delete `raft_send_append_entries`'s production wiring, keep it library-internal/test-only via `#ifdef XCE_TESTING` or move it fully into `raft_sim.c`). ✅ removes the "two implementations" documentation burden entirely. ❌ contradicts the specific reasoning already recorded against this (loses a working, protocol-exact reference implementation `raft_sim` and future specs may want); touches library-public API (`raft.h:378`) that other tests reference directly.
- **Option C — Wire in as the real production path**, replacing `repl_broadcast`/catch-up-streaming with nextIndex-driven sends. ✅ closest to `raft.tla` literally. ❌ highest risk by far — replaces a converging, live-verified (post bug-1479) core replication mechanism with an unexercised-in-production one; would need its own multi-round chaotic-soak verification before trust; effort XL, not justified by any live defect (the current substitute already converges correctly).
### Requirement 4 (vote/PreVote failure routing)
- **Option A — Add a discriminant field to `vote_ep_ctx_t`** (e.g. `bool is_prevote`), set at allocation (`ha.c:629` real-vote alloc → `false`, `ha.c:784` PreVote alloc → `true`), then branch inside `vote_ep_err_handler`'s election-phase case to call `vote_reply_dispatch` or `pre_vote_reply_dispatch` accordingly. ✅ single handler retained, minimal struct change, mirrors how `my_term`/`peer_idx` are already threaded per-context. ❌ handler gains a branch, slightly more complex than today.
- **Option B — Split into two handlers** (`vote_ep_err_handler` / `pre_vote_ep_err_handler`), registered at their respective existing call sites (`ha.c:688-689` vs `827-828`, which are already separate registration points today). ✅ no runtime branch, each handler stays single-purpose, matches the existing pattern where connect/monitor failures are *already* handled at separate call sites for the two endpoint kinds. ❌ near-duplicate logic in two functions (the `CTRL_PRIMARY` branch — standby-endpoint cleanup — is identical in both; would need a shared helper to avoid copy-paste).
- Recommendation for design phase: Option B composed with a small shared helper for the `CTRL_PRIMARY` cleanup branch is likely cleanest, but this is a genuine, low-stakes design choice — either is a bounded, low-risk change.
### Requirements 5, 6, 8 (already-conformant areas)
- **Option A only, no alternatives needed** — these are regression-lock requirements. The task is to confirm or add test coverage that pins the already-correct behavior (existing `XCE_TESTING` assert at `commit.c:222` for Requirement 6 is a strong existing asset; Requirements 5 and 8 need a design-phase check of which existing test IDs already exercise the traced code paths before deciding whether any new test is needed at all).
### Requirement 7 (term discovery / config-push term field)
- **Option A — Add `term @2: UInt64` to `ReplConfigPushReply`, populate it in `handle_repl_config_push`'s reply construction** (both the existing success path and the new Requirement 2 reject paths), and route it through the same `raft_on_higher_term_observed` adoption call already used for the heartbeat and WAL-NACK vectors. ✅ additive schema change (safe, no wire-format break), reuses the established adoption entry point, and — since Requirement 2 already touches every reply-construction site in this function for the reject-reply fix — can be implemented in the same edit pass with no extra integration surface. This pairing should be called out explicitly in the design/tasks phase so it isn't accidentally split into two separate change sets touching the same lines twice.
## Effort & Risk
| Requirement | Effort | Risk | Justification |
|---|---|---|---|
| 1 — Audit deliverable | S (1-3 days) | Low | Documentation + citation verification; no code risk. |
| 2 — Rejection reply integrity | S (extends one existing function with a proven pattern) | Low | Bounded to `handle_repl_config_push`; must respect the `-EAGAIN` activation-defer nuance found this pass, otherwise trivial. |
| 3 — Single authoritative send mechanism | S if Option A (document only) / XL if Option C (wire in) | Low (Option A) to High (Option C) | A prior in-codebase decision already argues for Option A; Option C would require full chaotic-soak reverification of a currently-working guarantee for no confirmed live defect. |
| 4 — Vote/PreVote failure routing | S-M | Low-Medium | Small struct/handler change, but touches election-path code; needs a live/soak test that can actually inject a post-connect (not connect-time) endpoint failure — that injection capability's current existence is unconfirmed (see Research Needed). |
| 5 — ClientRequest integrity | S (test-only, likely) | Low | No code gap found; effort is test-coverage confirmation. |
| 6 — Commit source-of-truth | S (test-only, likely) | Low | Existing `XCE_TESTING` assert already guards this. |
| 7 — Term discovery completeness | S-M (schema change + wiring, shares an edit pass with Requirement 2) | Low-Medium | Additive schema field is low-risk; the only real unknown is confirming the capnp codegen build step, not the change itself. |
| 8 — Restart/recovery reconciliation | S (test-only, likely) | Low | No code gap found; already-fixed by bug-1042/1477/1475. |
## Research Needed (carried forward to design phase)
1. **Test-coverage confirmation for Requirements 5, 6, 8** — identify the specific existing test IDs (in `test_ha_core_safety.c`, `test_apply_pump.c`, `test_commit_durability.c`, or elsewhere) that already lock each already-conformant guarantee, versus needing a net-new regression test. Not investigated deeply this pass per the gap-analysis framework's "defer deep research to design."
2. **Post-connect endpoint failure injection** — confirm whether the existing test harness (`test_ha_vote_send.c` or similar) can already simulate a failure arriving *after* a successful connect+monitor (as opposed to a connect-time failure, which is already tested per bug-1423/1424's regression coverage) — needed to properly test the Requirement 4 fix.
3. **Cap'n Proto codegen build step** — confirm the exact Meson target/invocation that regenerates `controller.capnp.c/.h` from the schema, for the Requirement 7 field addition. Low risk expected (additive field, not a `c-capnproto` version change like bug-918), but unconfirmed this pass.
4. **`handle_repl_config_push`'s WAL catch-up sibling regression test scope** — re-confirm (rather than assume from prior-session notes) exactly which `test_ha_core_safety.c` test IDs assert the bug-1479 WAL-ACK reject-reply fix, so the new config-push reject-reply test can be modeled on the same pattern.
5. **Requirement 3 wording tension** — Requirement 3 AC3 currently reads "...shall be either retired or made the production path." The asset found this pass (`test_ha_core_safety.c:2474`'s prior "document, do not structurally retire" decision) suggests a third real option — formally document as intentionally dormant — that isn't literally "retired." Flag for the design phase to either treat "retired" broadly enough to cover a documented-dormant-with-guard outcome, or revisit the requirement's wording before design finalizes.
## Recommendations Summary
- Requirements 2 and 7 should be planned as a single combined change (same function, same schema, same edit pass) rather than sequenced separately.
- Requirement 3's design decision should start from the existing `test_ha_core_safety.c:2474` precedent (Option A, document-only) rather than re-litigating retire-vs-wire-in from scratch; only revisit if the design phase surfaces a concrete reason the prior reasoning no longer holds.
- Requirements 5, 6, and 8 are low-effort, test-coverage-confirmation items — worth batching into one design-phase task rather than three separate ones.
- Requirement 4's Option A vs B (discriminant field vs split handler) is a genuine, low-stakes design choice; either is acceptable, defer to design phase author preference.
---
# Design-Phase Discovery & Decisions
This section extends the gap analysis above with the design-phase discovery pass and the decisions it resolved. Discovery type: **Extension** (existing controller subsystem, no new external dependency) — light discovery process applied.
## Summary
- **Feature**: `raft-adapter-conformance`
- **Discovery Scope**: Extension
- **Key Findings**:
- The Cap'n Proto codegen path for the schema change (Req 2/7) is a pre-existing, unconditional Meson `custom_target` (`src/lib/meson.build:1-10`, `capnp compile -o c:lib`) — an additive field needs no build-system change, closing Research Needed item 3 from the gap analysis with high confidence.
- `ha_send_wal_ack_reply_ex` (`ha.c:3095-3116`) already establishes the exact pattern to mirror for the config-push reject-reply fix: an `XCE_TESTING`-only send-hook (`ha_wal_ack_send_hook`) that lets a unit test observe the reply payload without a live endpoint. This resolves the "how do we test this" question for Req 2 by precedent rather than invention.
- Existing regression coverage was confirmed strong for three of the four "already-conformant" requirements: Req 6 (`test_term_safety_adapter.c:646-780`, explicit bidirectional advisory-vs-authoritative pins) and Req 8 (`test_persist_recovery.c`, `test_config_repl.c`) already lock the guarantees. Req 5 has partial coverage (`test_commit_barrier_rollout.c`, `test_commit_barrier_reply.c`) but no test was found asserting batch-wide failure propagation specifically — flagged as a small net-new test item, not a code gap.
- No existing test exercises `vote_ep_err_handler` at all (`grep` across `test_ha_vote_send.c` for the handler name, `RDHUP`, or `on_err` returned zero matches) — Req 4 needs net-new test infrastructure, not just a net-new test case. This is a real (small) infrastructure gap, not a research unknown anymore.
- No existing test was found asserting that a WAL catch-up reject actually *sends* `WalAckReply{success=0}` on the wire (as opposed to testing what happens downstream once a NACK is routed) — the bug-1479 fix itself is under-tested at the reply-construction boundary. Folded into this feature's Testing Strategy as a regression-lock item for Req 2.1.
## Research Log
### Cap'n Proto schema regeneration path
- **Context**: Requirements 2 and 7 both need `ReplConfigPushReply` to gain a `term` field; needed to confirm this doesn't require a Nix/build change given the historical `nixSupport/xce.nix` capnpc-pin issue (bug-918, memory-recorded from a prior session).
- **Sources Consulted**: `src/lib/meson.build` (read directly), `src/schema/controller.capnp` (read directly).
- **Findings**: `src/lib/meson.build:1-10` unconditionally regenerates `controller.capnp.c/.h` from `src/schema/controller.capnp` on every build via `capnp compile -o c:lib` using the `capnpc-c` plugin and the `c-capnproto` dependency. Bug-918 was about the Nix derivation's *own* pin for the `c-capnproto` toolchain source (a mutable upstream `main` ref), not about schema content — an additive field is orthogonal to that concern.
- **Implications**: No meson/Nix change needed for the schema addition; a normal `/xce-build controller` (or `all`) picks it up. No new Technology Stack row needed beyond noting the existing Cap'n Proto toolchain already covers this.
### Existing reply-hook test pattern
- **Context**: Needed a concrete, low-risk way to unit-test the new config-push reject-reply construction without standing up a live endpoint pair.
- **Sources Consulted**: `ha.c:3085-3131` (`ha_wal_ack_send_hook`, `ha_send_wal_ack_reply_ex`, `ha_send_wal_ack_reply`), read directly.
- **Findings**: The WAL-ACK path already has exactly this shape: an `XCE_TESTING`-guarded global function pointer that, when set, receives the constructed reply payload instead of the function sending over the wire. This is bug-1479's own test-support mechanism.
- **Implications**: The config-push reject-reply fix should add an analogous `XCE_TESTING`-only hook (e.g. `ha_config_push_reply_send_hook`) rather than inventing a different test-injection mechanism — keeps the two reply-integrity fixes symmetric and reviewable by the same pattern.
### Regression-coverage confirmation for Requirements 5, 6, 8
- **Context**: Gap analysis flagged these as "Unknown — test-coverage only"; needed to resolve before Testing Strategy could be written concretely.
- **Sources Consulted**: `grep` across `src/controller/testsuite/*.c` for the traced functions/concepts, read matched line ranges directly.
- **Findings**: Requirement 6 has strong existing dedicated coverage (`test_term_safety_adapter.c`, tests explicitly pin both directions: a lagging advisory index does not cap `committed_lsn`, and a well-ahead advisory index does not raise it past the durable-majority floor). Requirement 8 has existing coverage in `test_persist_recovery.c` and `test_config_repl.c`. Requirement 5 has partial coverage; no test specifically named or scoped to "one failed payload in a batch propagates to all" was found.
- **Implications**: Requirements 6 and 8 need no new tests — the design's Testing Strategy references the existing test files as the regression lock. Requirement 5 gets one small net-new test item (batch failure propagation) rather than a full new suite.
### Vote endpoint failure-path test infrastructure
- **Context**: Needed to confirm whether `vote_ep_err_handler`'s failure path (the Requirement 4 fix target) has any existing test scaffolding.
- **Sources Consulted**: `grep -n "vote_ep_err_handler\|RDHUP\|on_err" src/controller/testsuite/test_ha_vote_send.c` — zero matches.
- **Findings**: No existing test drives `vote_ep_err_handler` at all, for either the already-fixed connect-time paths or the not-yet-fixed post-connect path. This is a genuine test-infrastructure gap (need a way to invoke the handler directly with a synthetic `vote_ep_ctx_t`/endpoint, e.g. a small `XCE_TESTING`-only constructor mirroring `vote_ep_ctx_build_for_test` at `ha.c:7337`, which already exists for a different purpose and can likely be reused/extended).
- **Implications**: Requirement 4's task will need a small test-support addition (reusing `vote_ep_ctx_build_for_test`), not just a new test case in isolation.
## Architecture Pattern Evaluation
| Option | Description | Strengths | Risks / Limitations | Notes |
|--------|-------------|-----------|---------------------|-------|
| Extend in place (chosen for all code fixes) | Add the missing reply/routing logic directly inside the existing adapter functions (`handle_repl_config_push`, `vote_ep_err_handler`) | Matches the bug-1479 precedent exactly; no new abstraction; minimal review surface | None material — these are narrow, well-bounded functions already | Consistent with `ha_send_wal_ack_reply_ex`'s own extension of `ha_send_wal_ack_reply` |
| New shared "reply integrity" abstraction (rejected) | A generic cross-cutting helper/interface for "send an explicit reply on every reject path" spanning WAL-ACK, config-push, and vote replies | Would prevent the class of bug from recurring elsewhere by construction | The three reply shapes (`WalAckReply`, `ReplConfigPushReply`, `RequestVoteReply`) are distinct Cap'n Proto structs with different fields and different dispatch semantics (one is a direct reply, one is a tally-feed); forcing a shared interface over them adds indirection with only three call sites — fails the Simplification lens (no second concrete beneficiary today) | Documented here so a future spec doesn't have to re-derive why this was rejected |
## Design Decisions
### Decision: Documentation-only resolution for the dead nextIndex-driven AppendEntries sender (Requirement 3)
- **Context**: `raft_send_append_entries`/`ha_cb_send_append_entries_stub` is fully protocol-conformant but has zero production callers; `repl_broadcast` + catch-up streaming is the actual production mechanism.
- **Alternatives Considered**:
1. Retire the dead path (remove production wiring, keep library-only/test-only).
2. Wire it in as the real production path, replacing `repl_broadcast`.
3. Formally document it as intentionally dormant, matching the reasoning already recorded at `test_ha_core_safety.c:2474`.
- **Selected Approach**: Option 3. No source code changes. Update `src/controller/CLAUDE.md`'s TLA+ Action-to-Code Mapping to note `AppendEntries(i,j)` production behavior is `repl_broadcast`/`ha_stream_wal_to_standby`, with `raft_send_append_entries` explicitly marked as a dormant, protocol-exact reference path exercised only by `raft_sim`/tests. Add a one-line cross-reference note to `ha-term-safety/spec.json`'s existing `known_limitations` entries (index 30) pointing at this spec's resolution, so a future reader lands on the authoritative decision instead of re-discovering the drift.
- **Rationale**: A prior investigation already weighed retire-vs-keep and rejected retirement because the term-blind branches this path would otherwise expose are unreachable with real data today and independently guarded by `persist_wal_write_replicated`'s conflict guard even if reached. Re-litigating that decision without new information would violate the Simplification lens in reverse (removing working, guarded code for documentation tidiness alone). Wiring it in (Option 2) is rejected as disproportionate risk (XL effort, full chaotic-soak reverification) for zero confirmed live defect in the current mechanism.
- **Trade-offs**: Keeps two implementations of the same guarantee in the codebase long-term (a documentation cost); avoids all correctness/regression risk.
- **Follow-up**: Requirement 3's task should be scoped as a documentation task with zero `src/controller/*.c` diff; reviewers should treat any code-touching PR for this requirement as scope creep unless a new live defect is found.
### Decision: Split `vote_ep_err_handler` into two handlers sharing a helper (Requirement 4)
- **Context**: `vote_ep_ctx_t` has no field distinguishing a real-vote endpoint from a PreVote endpoint, and the shared handler's election-phase branch calls the legacy `handle_request_vote_reply` instead of the production tally path for post-connect failures.
- **Alternatives Considered**:
1. Add a boolean discriminant field to `vote_ep_ctx_t` and branch inside the one handler.
2. Split into `vote_ep_err_handler` / `pre_vote_ep_err_handler`, each registered at its existing distinct call site, sharing a small helper for the identical `CTRL_PRIMARY` cleanup branch.
- **Selected Approach**: Option 2.
- **Rationale**: The two endpoint kinds are *already* registered at distinct call sites today (`ha.c:688-689` vs `827-828`) even though they share the `vote_ep_ctx_t` type — the split mirrors an architectural seam that already exists, rather than introducing a new one. It avoids adding a field that only exists to support a runtime branch in one function.
- **Trade-offs**: Two small handler functions instead of one slightly larger one; a shared static helper for the `CTRL_PRIMARY` branch avoids duplicating that logic.
- **Follow-up**: The task should reuse `vote_ep_ctx_build_for_test` (`ha.c:7337`) as the test-construction seam for both new handlers' unit tests, per the test-infrastructure finding above.
## Risks & Mitigations
- Adding `term` to `ReplConfigPushReply` changes a wire struct consumed during rolling upgrades — mitigated because Cap'n Proto field addition is backward/forward compatible by construction (old readers ignore the new field; an old sender's reply decodes with `term == 0` on a new-code primary, which is naturally safe since `0` can never look like a *higher* term than the primary's own and so triggers no adoption — see Migration Strategy in `design.md`).
- Splitting `vote_ep_err_handler` touches live election-path code — mitigated by keeping the `CTRL_PRIMARY` branch behavior byte-for-byte identical (extracted, not rewritten) and requiring the new dedicated unit tests (via the reused test-construction helper) before this task is considered done.
- No test currently exists for the exact reply-construction boundary of the already-shipped bug-1479 fix — mitigated by adding it as an explicit Requirement 2.1 regression-lock test in this feature rather than assuming the existing catch-up/fence tests already cover it.
## References
- `src/controller/testsuite/test_ha_core_safety.c:2440-2480` — the prior "DOCUMENT SAFE-BY-GUARD, do not structurally retire" decision informing Requirement 3.
- `src/lib/meson.build:1-10` — Cap'n Proto codegen custom_target.
- `src/controller/ha.c:3085-3131` — `ha_send_wal_ack_reply_ex`/`ha_wal_ack_send_hook`, the precedent pattern for Requirement 2's test hook.

22
.kiro/specs/raft-adapter-conformance/spec.json
View file

@ -1,25 +1,25 @@
{
"feature_name": "raft-adapter-conformance",
"created_at": "2026-07-29T03:10:00Z",
"updated_at": "2026-07-29T03:10:00Z",
"updated_at": "2026-07-29T09:20:00Z",
"language": "en",
"phase": "initialized",
"phase": "tasks-generated",
"approvals": {
"requirements": {
"generated": false,
"approved": false
"generated": true,
"approved": true
},
"design": {
"generated": false,
"approved": false
"generated": true,
"approved": true
},
"tasks": {
"generated": false,
"approved": false
"generated": true,
"approved": true
}
},
"ready_for_implementation": false,
"ready_for_implementation": true,
"blocked_on": [],
"related_bugs": ["bug-1479", "bug-1466", "bug-1471", "bug-1472", "bug-1474", "bug-1475", "bug-1477", "bug-1478"],
"related_specs": ["raft-log-replication", "raft-commit-safety", "ha-term-safety", "raft-sim-conformance", "config-upgrade-snapshot"]
"related_bugs": ["bug-1479", "bug-1480", "bug-1481", "bug-1466", "bug-1471", "bug-1472", "bug-1474", "bug-1475", "bug-1477", "bug-1478", "bug-1423"],
"related_specs": ["raft-log-replication", "raft-commit-safety", "ha-term-safety", "raft-sim-conformance", "config-upgrade-snapshot", "ha-config-bootstrap", "ha-commit-engine"]
}