feat(controller): clusterconfig-schema-split task 1.1 — remove dead fields from ClusterConfig schema

Removes port, candidates, vcpus, jobmgr, workermgr, qmgr, resmgr, xstrings
from the ClusterConfig capnp struct plus the now-unused MasterCandidate
struct, per the config-design.md shared-vs-local classification rule.

Cap'n Proto forbids ordinal gaps, so the 13 retained fields are renumbered
to a contiguous @0..@12 (accepted pre-1.0 wire-compat break — see
research.md's "Design Revision 2"). requirements.md/design.md updated to
match: Req 3.1 drops field-number invariance, Req 3.2/4.1 state a pre-spec
cluster.bin is not expected to decode correctly, and a new Req 5.3 covers
regenerating the checked-in ha/data/cluster.bin fixture in a later task.

Also folds in this session's live design revision: Requirement 1.7's port
replacement mechanism is the existing --peers self-reference + a hardcoded
default (no new --port CLI flag), confirmed via interactive review with
the spec owner.
This commit is contained in:
Rongsong Shen 2026-07-21 03:01:26 +00:00
parent 94f50ce6e1
commit 515248200a
6 changed files with 589 additions and 26 deletions

View file

@ -0,0 +1,271 @@
> Sections may be reordered when it improves clarity; this is a backend/systems (C daemon + capnp schema) change, not a web app — "Service Interface"/API/Event contract subsections are adapted to C function-signature contracts where used, and omitted where not applicable.
## Overview
**Purpose**: Removes eight fields from the `ClusterConfig` capnp struct (`port`, `candidates`, `vcpus`, `jobmgr`, `workermgr`, `qmgr`, `resmgr`, `xstrings`) that are either already dead at runtime or (in `port`'s case) load-bearing for single-node startup but conceptually misplaced in cluster-shared config. `port`'s replacement source is the already-existing `--peers` candidate-match mechanism plus a hardcoded default — no new CLI flag.
**Users**: XCE maintainers (schema/controller code owners) and, indirectly, XCE operators running single-node or `--peers`-mismatched controllers, whose startup behavior must not change.
**Impact**: `ClusterConfig` shrinks from 21 to 13 fields. `cluster_config_t` (the generated C struct) loses the matching members. `cluster_open_listener`'s port-resolution fallback moves from a capnp-blob field to a two-tier shape: `--peers` candidate match (unchanged, including the already-supported single self-referencing `--peers <name>=<host>:<port>` entry for single-node mode) → hardcoded constant when `--peers` was never passed at all.
### Goals
- Remove the 8 fields and the `MasterCandidate` struct from `controller.capnp`, and their mirrors in `cluster_config_t`.
- Replace `port`'s ClusterConfig-sourced fallback with the existing `--peers` candidate-match mechanism (unchanged) plus a hardcoded default for the true zero-`--peers` case, preserving single-node startup behavior exactly with no new CLI flag.
- Prove behavior-preservation live (single-node startup, 3-node soak against a regenerated `ha/data/cluster.bin` fixture) — not just by unit tests, given the touched files (`cluster.c`, `ha.c`, `main.c`) are HA-critical.
- (Nice-to-have) Add CLI flags for the hash-tuning tunables, closing the existing `cluster.c` TODOs.
### Non-Goals
- `heartbeatInterval`/`inactiveHeartbeatCount` (controller→worker liveness) — unchanged, stay in `ClusterConfig`.
- `fetchJobInterval`/`netioTimeout` — unresolved, left untouched, a candidate for a future pass.
- A real HA/Raft election-heartbeat CLI parameter — separate roadmap item, not this spec.
- Peer list replication protocol / joint consensus — future, unrelated to a field deletion.
- Any change to `entity-bearing-snapshots` / `config-upgrade-snapshot`.
## Boundary Commitments
### This Spec Owns
- `ClusterConfig` and `MasterCandidate` struct shape in `controller.capnp`.
- `cluster_config_t`'s field list in `clusterconfig.h`.
- The port-resolution fallback branch in `cluster_open_listener` (`cluster.c`): `--peers` candidate match (unchanged) → hardcoded default constant.
- The dead-code removals that don't need a replacement: `candidates` KV-mirror loop (`cluster_build_config`), `SET_DEFAULT_IF_UNSET(c->port, ...)` call sites, and the (already-inert) `jobmgr`/`workermgr`/`qmgr`/`resmgr`/`xstrings`/`vcpus` blob reads.
- The two known test-fixture updates (`test_commit_barrier_rollout.c`, `test_config_repl.c`) and any others a full-repo grep surfaces during implementation.
- Regenerating capnp bindings.
- (Optional, Requirement 6) CLI-configurable perf tunables — five new `OPT_*` flags mirroring the existing `--vcpus`, deferrable without blocking the rest of this spec.
### Out of Boundary
- `heartbeatInterval`/`inactiveHeartbeatCount`, `fetchJobInterval`/`netioTimeout` — no field or code change.
- A `ClusterConfig` field or CLI flag for the HA/Raft election heartbeat — structurally disallowed (`config-design.md` Rule 0); tracked as its own roadmap item.
- Peer list replication / joint-consensus membership — unrelated, `candidates` is deleted, not replaced with a distributed mechanism.
- `config-upgrade-snapshot`, `entity-bearing-snapshots` — no dependency either direction.
- `admin-config-tool`'s `xceadmin` implementation — this spec closes the risk that tool depends on (per its requirements.md), but does not implement or block on any of that tool's own code.
### Allowed Dependencies
- Existing build tooling: capnp compiler + `c-capnproto` codegen (schema → C bindings), already wired into the meson build.
- `argp` CLI parsing (already used by every other controller flag, including `--vcpus`).
- No new external dependencies, no new libraries.
### Revalidation Triggers
- Any future proposal to add a field to `ClusterConfig` must be checked against `.kiro/steering/config-design.md`'s inclusion rules before landing.
- `admin-config-tool`'s `xceadmin config build`/`push` implementation, when it lands, should target the post-split (13-field) schema — if it's implemented against the pre-split schema instead, that's a signal this spec's sequencing assumption was violated.
- If the controller's invocation contract changes elsewhere (e.g., `--peers` becomes mandatory for another reason), revisit whether the hardcoded-default fallback added here is still needed.
## Architecture
### Existing Architecture Analysis
`ClusterConfig` today mixes three concerns in one capnp struct: cluster-wide business config (queues, resources, groups — genuinely shared), HA peer membership (`candidates`, already superseded in practice by the `--peers` CLI flag into a separate `xce_cluster_config_t`/`peers_cfg` struct), and per-controller performance tunables (`jobmgr`/`workermgr`/`qmgr`/`resmgr`/`xstrings`/`vcpus`, already hardcoded in `cluster.c`'s `init_controller_tunables()` with TODOs acknowledging the blob values are dead). `port` sits in the same struct but, unlike the others, still has one live reader: `cluster_open_listener`'s fallback branch, reached whenever the local controller's name isn't matched in `--peers` (including single-node mode, which passes no `--peers` at all).
### Architecture Pattern & Boundary Map
```mermaid
graph TB
Schema[ClusterConfig capnp schema]
Codegen[capnp codegen]
ConfigStruct[cluster_config_t]
Blob[cluster.bin blob]
PeersFlag[peers CLI flag]
PeersCfg[peers_cfg struct]
Listener[cluster_open_listener]
Tunables[init_controller_tunables]
Business[cluster_init_phase2 business config]
Schema --> Codegen --> ConfigStruct
Blob --> ConfigStruct
ConfigStruct --> Business
PeersFlag --> PeersCfg --> Listener
Listener --> DefaultConst[hardcoded default constant]
Tunables --> HardcodedTunables[hardcoded tunable constants]
```
**Architecture Integration**:
- Selected pattern: no new CLI surface — `port` resolution reuses the existing `--peers` candidate-match loop (a single self-referencing `--peers <name>=<host>:<port>` entry already gives single-node mode a custom port today) plus a hardcoded fallback constant for the true zero-`--peers` case.
- Domain/feature boundaries: `ClusterConfig` (schema) retains only fields that are genuinely cluster-shared (per `config-design.md`); `peers_cfg` (CLI-sourced, per-process, already existing) owns port resolution for everything this spec touches.
- Existing patterns preserved: `--peers` remains the sole port/membership source; `init_controller_tunables()`'s hardcoded constants are untouched (Requirement 2.2 — no behavior change).
- New components: one new hardcoded constant (`cluster.c` or `clusterconfig.h`) — the minimum needed to satisfy Requirement 1.7 without any invocation-contract or CLI-surface change.
- Steering compliance: enforces `.kiro/steering/config-design.md`'s independence/frequency/Rule-0 classification; follows `src/controller/CLAUDE.md`'s HA protocol verification checklist for the touched files.
### Technology Stack
| Layer | Choice / Version | Role in Feature | Notes |
|-------|------------------|------------------|-------|
| Schema | Cap'n Proto (existing, `controller.capnp`) | Defines `ClusterConfig`/`MasterCandidate` shape | No version change |
| Codegen | `c-capnproto` (existing) | Schema → C struct/reader/builder | Regenerated, not upgraded |
| Backend | C (gnu17, existing) | `cluster.c`/`myconf.c`/`config_repl.c` changes | No new C dependency |
| CLI | `argp` (existing) | No new flag for Req 1.7; reused unchanged for optional Req 6 tunables, same pattern as `--vcpus` | No new dependency |
## File Structure Plan
### Modified Files
- `src/schema/controller.capnp` — remove `port` (@1), `candidates` (@2), `vcpus` (@7), `jobmgr` (@13), `workermgr` (@14), `qmgr` (@15), `resmgr` (@16), `xstrings` (@17) from `ClusterConfig`; remove the `MasterCandidate` struct entirely. Cap'n Proto requires gapless ordinals, so the 13 retained fields are renumbered to a contiguous `0..12` range as a direct consequence — see `research.md`'s ordinal-gap finding (Req 1.1, 1.2, 1.6, 2.1, 3.1).
- `ha/data/cluster.bin` (binary fixture, not source) — regenerated in place against the renumbered schema, preserving the retained fields' semantic values from the pre-spec blob (Req 3.2, 5.3).
- `src/include/clusterconfig.h` — remove the matching `cluster_config_t` members: `port`, `ncandidates`, `candidates`/`master_candidate_t**`, `vcpus`, `jobmgr`, `workermgr`, `qmgr`, `resmgr`, `xstrings` (Req 1.5, 2.4).
- `src/controller/cluster.c` — (a) remove the `candidates`/`ncandidates` KV-mirror loop in `cluster_build_config` (Req 1.4); (b) change `cluster_open_listener`'s fallback branch from reading `cluster_config->port` to a hardcoded default constant, reached only when `--peers` was never passed at all — the `--peers` candidate-match branch itself is unchanged (Req 1.3, 1.7); `init_controller_tunables()` itself is unchanged (Req 2.2).
- `src/controller/myconf.c` — remove the `SET_DEFAULT_IF_UNSET(c->port, 0, 9990)` call (the field it defaults no longer exists) (part of Req 1.7's cleanup).
- `src/controller/config_repl.c` — remove the equivalent `SET_DEFAULT_IF_UNSET(c->port, 0, 9990)` call (same reason).
- `src/controller/testsuite/test_commit_barrier_rollout.c` — drop the `cfg.port = 9990;` literal at line 1359 (Req 5.1).
- `src/controller/testsuite/test_config_repl.c` — drop the `cfg.port = 9991;` literal at line 983 (Req 5.1).
- Capnp-generated bindings (build output, not hand-edited) — regenerated as part of the normal build after the schema change (Req 1.1-1.2, 1.6, 2.1).
### Verification Infrastructure (existing, no new files)
- `controller_unit_test` (meson target, existing) — runs the full existing suite including the two updated test files above (Req 5.2).
- A small throwaway capnp-encoding tool (written and discarded during implementation, not part of the shipped codebase) — decodes `ha/data/cluster.bin`'s retained-field values under the pre-spec schema and re-encodes them under the post-spec (renumbered) schema, overwriting the fixture (Req 3.2, 5.3).
- `/xce-deploy-ha` skill (existing, `src/tools/deploy-ha.sh`) — deploys the 3-node live cluster for the config-push cycle, using the regenerated `ha/data/cluster.bin` (Req 5.4).
- Direct `xce_controller` invocation with no `--peers` flag (existing supported mode, `main.c:293-295`) — the single-node startup verification (Req 5.5).
- Direct `xce_controller` invocation with a single self-referencing `--peers <name>=<host>:<port>` entry (existing supported mode) — verifies the custom-port replacement mechanism itself (Req 5.5).
### Optional (Requirement 6, nice-to-have)
- `src/controller/main.c` — five new `OPT_*` flags (`--jobmgr-nhash`, `--workermgr-nhash`, `--qmgr-nhash`, `--resmgr-nhash`, `--xstrings-nhash`, plus `-delta`/`-inst` variants where the tunable has more than one dimension), same pattern as the existing `--vcpus`.
- `src/include/myconf.h` — matching `myconf_t` fields.
- `src/controller/cluster.c` — `init_controller_tunables()` reads the new `myconf_t` fields with the existing hardcoded values as fallback (0 = unset), rather than always using the hardcoded constant.
No new files are created by this spec — every change is to an existing file.
## System Flows
Port resolution at controller startup — corrected during design review (see `research.md` Design Decisions for the two fixes below):
```mermaid
flowchart TD
Start[cluster_open_listener starts]
HasPeers{peers_cfg candidates_count greater than 0}
SearchName[search peers_cfg for local name]
NameFound{local name found}
PortValid{matched candidate port greater than 0}
UseMatchedPort[use matched candidate port]
UseFallback[use hardcoded default port]
RefuseStart[log error and refuse to start]
Bind[bind listener]
Start --> HasPeers
HasPeers -->|no, --peers never passed| UseFallback --> Bind
HasPeers -->|yes| SearchName --> NameFound
NameFound -->|no| RefuseStart
NameFound -->|yes| PortValid
PortValid -->|yes| UseMatchedPort --> Bind
PortValid -->|no, malformed entry| UseFallback
```
**Key decisions** (corrected from the initial draft during `/kiro-validate-design`; the `--port` CLI flag was further revised away in favor of the mechanism below during `/kiro-impl` kickoff — see `research.md`'s "Design Revision" section):
- **Fallback trigger is port value, not just match presence.** A matched candidate whose port field is `0` (a malformed `--peers` entry — already WARNed about at `main.c:289-291`) falls through to the hardcoded-default tier exactly like today's `if (my_port == 0)` check, rather than binding to literal port 0. This preserves Requirement 1.3's "exactly as it does today" for the already-tolerated malformed-entry case.
- **Error path is preserved, but only for one specific case.** `candidates_count == 0` can only happen when `--peers` was never passed at all (`validate_peer_count` already rejects even counts including 0 for a passed-but-empty `--peers`) — that's genuine single-node mode, and gets the hardcoded-default fallback per Requirement 1.7. An operator who wants a non-default port in this mode passes a single self-referencing `--peers <name>=<host>:<port>` entry instead — an already-existing, already-supported invocation (this resolves via the unchanged `NameFound`/`PortValid` branches above, not a new mechanism). But when `--peers` WAS passed and the local name simply isn't in it, that's a likely operator misconfiguration (own name typo'd or omitted) — this spec keeps today's `-EINVAL`/refuse-to-start behavior for that case rather than silently defaulting, per the project's fail-loud convention (`src/controller/CLAUDE.md` process-lifecycle invariant). Decided interactively during design review rather than defaulting to "simplest single code path."
## Requirements Traceability
| Requirement | Summary | Components | Files |
|-------------|---------|-------------|-------|
| 1.1 | Remove `port` from schema | Schema Component | `controller.capnp` |
| 1.2 | Remove `candidates` from schema | Schema Component | `controller.capnp` |
| 1.3 | Listener uses `--peers` match when available | Listener/Port-Resolution Component | `cluster.c` |
| 1.4 | Debug dump stops reading `candidates` | Debug-Dump Component | `cluster.c` |
| 1.5 | `cluster_config_t` drops `port`/`candidates` members | C Struct Component | `clusterconfig.h` |
| 1.6 | Remove `MasterCandidate` struct | Schema Component | `controller.capnp` |
| 1.7 | Replacement port-sourcing mechanism (existing `--peers` self-reference + hardcoded default) | Listener/Port-Resolution Component | `cluster.c`, `myconf.c`, `config_repl.c` |
| 2.1 | Remove 6 perf-tuning fields from schema | Schema Component | `controller.capnp` |
| 2.2 | Hardcoded tunables unchanged | Tunables Component | `cluster.c` (no change) |
| 2.3 | Controller stops attempting to read removed tunable fields | Tunables Component | `cluster.c` (implicit — field no longer exists) |
| 2.4 | `cluster_config_t` drops tunable members | C Struct Component | `clusterconfig.h` |
| 3.1 | Retained fields unchanged in type/number | Schema Component | `controller.capnp` (no change to retained fields) |
| 3.2 | Renumbering breaks pre-spec `cluster.bin` binary-compat (accepted); fixture rebuilt | Schema Component, Verification | `controller.capnp`, `ha/data/cluster.bin` regeneration |
| 3.3 | Worker-liveness behavior unaffected | Tunables/Business Component | `cluster.c` (no change) |
| 4.1 | Discard removed-field values from legacy blobs silently | Schema Component | `controller.capnp` (capnp default behavior) |
| 4.2 | No deprecate-in-place transition | Schema Component | `controller.capnp` |
| 5.1 | Build + test-fixture compile cleanly | Test Fixture Updates | `test_commit_barrier_rollout.c`, `test_config_repl.c` |
| 5.2 | No new unit test failures | Verification | controller unit test suite |
| 5.3 | Regenerate `ha/data/cluster.bin` fixture against the new schema | Verification | `ha/data/cluster.bin` |
| 5.4 | 3-node live soak (config-push, peer/listener/worker-liveness) | Verification | live HA cluster |
| 5.5 | Single-node + bare-peers port-override + `-EINVAL` live verification | Verification | live single-node controller |
| 6.1 | (Nice-to-have) CLI tunable flags | CLI Tunables Component (optional) | `main.c`, `myconf.h` |
| 6.2 | (Nice-to-have) Fallback to hardcoded default | Tunables Component (optional) | `cluster.c` |
## Components and Interfaces
| Component | Domain | Intent | Req Coverage | Key Dependencies | Contracts |
|-----------|--------|--------|---------------|-------------------|-----------|
| Schema Component | capnp | Defines `ClusterConfig`/`MasterCandidate` shape | 1.1, 1.2, 1.6, 2.1, 3.1, 4.1, 4.2 | capnp compiler (P0) | State |
| C Struct Component | C headers | `cluster_config_t` field list matches schema | 1.5, 2.4 | Schema Component (P0) | State |
| Listener/Port-Resolution Component | C, `cluster.c` | Resolves listen port via `--peers` match (unchanged) with a hardcoded-default fallback | 1.3, 1.7 | C Struct Component (P0) | Service |
| Debug-Dump Component | C, `cluster.c` | Internal status/debug KV dump, no longer mirrors `candidates` | 1.4 | C Struct Component (P0) | State |
| Tunables Component | C, `cluster.c` | Hardcoded (or optionally CLI-sourced) perf tunables | 2.2, 2.3, 3.3, 6.2 | CLI Tunables Component (P1, optional path) | State |
| CLI Tunables Component (optional) | C, `main.c`/`myconf.h` | Requirement 6 only: parses tunable flags, populates `myconf_t`, same pattern as the existing `--vcpus` | 6.1 | argp (P0) | Service |
| Test Fixture Updates | C tests | Two known struct-literal sites updated to compile | 5.1 | C Struct Component (P0) | — |
| Verification | Build + live cluster | Proves behavior preservation | 5.1, 5.2, 5.3, 5.4, 5.5, 3.2, 4.1 | All components above (P0) | — |
Only the Listener/Port-Resolution Component introduces new logic (the two-tier fallback); every other component is either schema/struct shrinkage or already-hardcoded behavior left untouched, so only that one gets a full contract block below.
### Controller
#### Listener/Port-Resolution Component
| Field | Detail |
|-------|--------|
| Intent | Determine the controller's own listen port without depending on a `ClusterConfig` field |
| Requirements | 1.3, 1.7 |
**Responsibilities & Constraints**
- Primary responsibility: resolve exactly one port value before `cluster_open_listener` binds.
- Precedence is strict and non-configurable: `--peers` match (if local name found) > hardcoded default constant. No tier is skipped or reordered by configuration. No CLI flag mediates this — an operator who needs a non-default port in the no-other-peers case supplies it via a single self-referencing `--peers <name>=<host>:<port>` entry, which resolves through the same match branch as any other `--peers` candidate.
- Must not depend on `cluster_config_t` in any form (the field this component replaces no longer exists after this spec).
**Dependencies**
- Inbound: `main.c` startup sequence — invokes this resolution before/during `cluster_open_listener` (P0)
- Outbound: `myconf->peers_cfg` (existing, unchanged parsing) (P0)
- Outbound: hardcoded default constant (same file or `clusterconfig.h`) — no external dependency (P0)
**Contracts**: Service [x]
##### Service Interface
```c
/* cluster_open_listener — port resolution, updated contract */
/* Precondition: myconf->peers_cfg is populated (possibly empty, iff
* --peers was never passed). */
/* Postcondition, --peers never passed (peers_cfg.candidates_count == 0):
* returns 0 with the listener bound to the hardcoded default (9990).
* Always succeeds. An operator wanting a non-default port here passes
* a single self-referencing --peers entry instead (see below).
* Postcondition, --peers given: if the local name is matched with
* a nonzero port, binds to it (today's unchanged happy path — this
* is also how a single-node operator gets a custom port). If
* matched with a zero port (malformed entry), falls through to the
* hardcoded-default resolution. If the local name is NOT matched at
* all, returns -EINVAL and refuses to start — unchanged from today,
* preserved deliberately (see System Flows "Error path is preserved"
* decision). */
int cluster_open_listener(cluster_t *c);
```
- Preconditions: `myconf->peers_cfg` populated (empty iff `--peers` never passed).
- Postconditions: see the branches above — no-`--peers` and matched-with-zero-port always resolve to a bound listener; `--peers`-given-but-unmatched still returns `-EINVAL`.
- Invariants: precedence order (`--peers` match > hardcoded default) never changes based on runtime state; the `-EINVAL` case is determined solely by "was `--peers` passed AND local name absent," never by port value.
**Implementation Notes**
- Integration: the match/no-match branch structure is unchanged from today's `cluster_open_listener`; only the final fallback's source changed (from `cluster_config->port` to a hardcoded `XCE_CONTROLLER_DEFAULT_PORT` constant). No new CLI flag, no new `myconf_t` field — see `research.md`'s "Design Revision" section for why this replaced the originally-designed `--port` flag.
- Validation: no new validation needed — the hardcoded constant requires no runtime validation, and `--peers` candidate validation is unchanged.
- Risks: none identified beyond what Requirement 5.5's live single-node test is designed to catch — this is the one path that removes a previously-live capability, and this component IS the requirement's specified replacement. Requirement 5.5's live verification must cover BOTH the true zero-`--peers` case AND the self-referencing single-candidate `--peers` case (the latter is now the sole way to get a non-default single-node port, so it's load-bearing, not incidental). The preserved `-EINVAL` path for "peers given, no match" should also get its own live/unit test coverage during tasks (not separately required by requirements.md, but a natural extension of Requirement 5.5's spirit — flag during task generation).
## Error Handling
### Error Strategy
No new error categories are introduced, and the existing `-EINVAL` error path (`cluster_open_listener`'s "no port could be determined," `cluster.c:1226` today) is **preserved**, not removed — corrected during design review, since single-code-path simplicity would have traded a clear startup error for a silent wrong-default in a likely-misconfiguration scenario (`--peers` given, local name absent). It now applies to exactly one case: `--peers` was passed and the local name isn't in it. The genuinely-new case (`--peers` never passed at all, i.e. single-node) and the pre-existing-tolerated case (name matched, that entry's port is `0`) both resolve via the hardcoded-default fallback and never error — unchanged from today's tolerance for the latter, newly-defined for the former. All other error handling (schema decode failures, capnp field-absence for legacy blobs) is capnp's existing default behavior: an old `cluster.bin` with data in now-unused field slots decodes without error, and those values are simply never read (Requirement 4.1) — no new error branch needed for that case either.
### Monitoring
No new logging beyond what already exists for `--peers` parsing (`main.c:284-291`) and controller startup (`INFO`/`WARN`/`ERROR` via the existing `mylog` macros). No new metrics.
## Testing Strategy
- **Unit**: build-time enforcement is the primary "test" for Requirements 1.1-1.2, 1.5-1.6, 2.1, 2.4 — a removed field/struct member that's still referenced anywhere is a compile error, not a silent gap (this is why Requirement 5.1's test-fixture fix is explicit rather than assumed).
- **Unit**: `controller_unit_test` full suite run (Requirement 5.2) — existing coverage for `cluster_build_config`, `init_controller_tunables`, and the two touched test files, now compiling against the shrunk struct.
- **Integration**: regenerate the `ha/data/cluster.bin` fixture (Requirement 3.2, 5.3) — since retained-field renumbering breaks binary compatibility with any pre-spec blob, decode the current fixture's semantic field values under the pre-spec schema and re-encode them under the new schema with a small throwaway tool; confirm the regenerated blob decodes correctly under the post-spec schema and `deploy-ha.sh`'s verbatim-copy usage keeps working unmodified.
- **Live/E2E**: 3-node HA cluster deploy + config-push cycle (Requirement 5.4) — using the regenerated fixture; peer discovery via `--peers`, listener binding, worker-liveness tracking all behave identically to pre-spec.
- **Live/E2E**: single-node controller startup with no `--peers` flag, AND with a single self-referencing `--peers` entry for a custom port (Requirement 5.5) — the actual regression risk this spec's design resolves; must be verified live, not inferred from the 3-node soak (single-node never exercises the `--peers`-match branch at all).
- **Live/E2E (optional, Requirement 6)**: if the nice-to-have CLI tunable flags are implemented, verify each flag overrides its corresponding hardcoded default and that omitting all of them reproduces today's exact hardcoded values.
## Migration Strategy
No data migration is required for the fields this spec targets — Requirement 4.2 explicitly rules out a deprecate-in-place transition, and Requirement 4.1 confirms retained-field renumbering is an accepted, unguarded compatibility break for this pre-1.0 product: a `cluster.bin` built before this spec is not expected to be usable as-is against a controller built after it, and no backward-read path is supported or verified for such an artifact. The one thing this spec DOES migrate is the checked-in `ha/data/cluster.bin` test fixture (Requirement 3.2, 5.3): its retained-field values are decoded under the pre-spec schema and re-encoded under the new schema with a small throwaway tool, so `/xce-deploy-ha` and dependent live-verification tooling keep working. There is no forward path either (a post-spec `cluster.bin` is not expected to be readable by a pre-spec controller, but that direction isn't a supported downgrade path in this project regardless of this spec).
## Supporting References
- `src/docs/cluster-config-split-analysis.md` — prior team analysis; primary design input for the dead-field classification.
- `.kiro/specs/clusterconfig-schema-split/research.md` — port-replacement-mechanism investigation, alternatives considered, and the decision rationale in full.
- `.kiro/steering/config-design.md` — the general shared-vs-local config rule this spec enforces.

View file

@ -0,0 +1,103 @@
# Requirements Document
## Introduction
`ClusterConfig` (`src/schema/controller.capnp:711-733`, generated as C struct `cluster_config_t`) is the capnp struct pushed cluster-wide via `OP_upgradeConfig`. It currently carries several fields that were bundled in without a principled reason: `port`, `candidates` (peer/standby list), `vcpus`, and the `jobmgr`/`workermgr`/`qmgr`/`resmgr`/`xstrings` performance-tuning fields. This spec removes those fields from the schema and the C code that reads them, so `ClusterConfig` contains only fields that are genuinely cluster-shared, per the classification in `.kiro/steering/config-design.md`.
**Correction (2026-07-20, second review pass)**: `candidates`, `vcpus`, and the perf-tuning fields ARE already dead weight at runtime (verified — see Adjacent Expectations). `port` is NOT — `cluster_open_listener` (`cluster.c:1216-1258`) has a live fallback to `cluster_config->port` that is the **sole port-sourcing mechanism for single-node deployments** (no `--peers` flag is passed at all in single-node mode — `main.c:293-294`). Removing the `port` field without adding a replacement mechanism would break single-node controller startup. This spec now requires that replacement as part of the `port` removal (Requirement 1.7), not as an assumed-safe deletion.
This closes an assumption flagged in `.kiro/specs/admin-config-tool/requirements.md` (the "per-instance field preservation" risk) — not by verifying controller behavior, but by making the scenario structurally impossible: fields that don't exist in the schema can't be silently zeroed on a config push.
## Boundary Context
- **In scope**:
- Remove `port` (@1) and `candidates` (@2, `List(MasterCandidate)`) from `ClusterConfig`.
- Remove `vcpus` (@7) from `ClusterConfig` — already marked deprecated in the schema comment ("use --vcpus CLI flag; kept for config blob backward compatibility"); this is the schema revision that comment anticipated.
- Remove `jobmgr` (@13), `workermgr` (@14), `qmgr` (@15), `resmgr` (@16), `xstrings` (@17) from `ClusterConfig`.
- Remove the C code that reads the above fields from `cluster_config_t`: the `candidates`/`ncandidates` KV-mirror loop in `cluster_build_config` (`cluster.c`) is dead-code removal (no behavior change); the port-fallback branch in `cluster_open_listener` (`cluster.c:1216-1258`) is NOT dead code (see Introduction correction) and its removal requires a replacement mechanism per Requirement 1.7, not a bare deletion. Also remove the corresponding fields in `cluster_config_t` (`clusterconfig.h`) and the `SET_DEFAULT_IF_UNSET(c->port, ...)` calls in `myconf.c` and `config_repl.c` (only after the replacement mechanism lands).
- Regenerate capnp bindings after the schema change.
- Direct field removal (no deprecate-in-place transition, no field-number reservation) — the product has not reached 1.0, so capnp wire/on-disk compatibility for these fields is not a constraint (user decision, 2026-07-20).
- **Out of scope**:
- `heartbeatInterval` (@3) and `inactiveHeartbeatCount` (@4) — these govern controller→**worker** liveness, not the HA/Raft election heartbeat (see Adjacent Expectations). Investigated and deliberately kept in `ClusterConfig` (operational-uniformity override — see `.kiro/steering/config-design.md`); this spec does not touch them.
- `fetchJobInterval` (@5) and `netioTimeout` (@6) — flagged by the prior `src/docs/cluster-config-split-analysis.md` as candidates for the same treatment, but not yet decided in this pass. Left untouched; a candidate for a future iteration of this spec or a follow-up.
- `clusterName` (@0), `resources`, `queues`, `ugroups`, `hgroups`, `hostConfigs`, `hproviders`, `limits` — cluster-shared business config, unaffected.
- `configGeneration` (@20) — controller-assigned protocol state, not operator config; unaffected, not part of this spec's field-removal scope.
- Adding a real HA/Raft election-heartbeat CLI parameter (`--heartbeat-interval-ms`/`--heartbeat-timeout-ms`) to replace the currently-hardcoded `HA_DEFAULT_HEARTBEAT_INTERVAL_MS`/`HA_DEFAULT_HEARTBEAT_TIMEOUT_MS` — tracked as a separate roadmap item (`.kiro/steering/roadmap.md`, "Raft/HA heartbeat interval+timeout as CLI parameters"), not part of this spec. This spec does not add any new `ClusterConfig` field for HA heartbeat timing, and per Rule 0 in `config-design.md` never should.
- Peer list replication protocol (joint-consensus membership changes) — a much larger future item noted in the prior analysis doc's Phase 5; not needed here since `candidates` is simply being deleted, not replaced with a new distributed mechanism (the live peer source, `--peers` CLI, already exists and is unaffected).
- `entity-bearing-snapshots` / `config-upgrade-snapshot` — unrelated concern (snapshot consistency at config-change boundaries); this spec has no dependency on either.
- **Adjacent expectations**:
- **Blast-radius finding (corrected 2026-07-20)**: `candidates` is fully dead in `ClusterConfig` at runtime — the live peer list comes exclusively from the `--peers` CLI flag (parsed into a separate struct, `myconf.peers_cfg`), and the blob's `candidates` is read only into a debug-only KV mirror (`cluster_build_config`, `cluster.c:851-885`) with no other readers. `jobmgr`/`workermgr`/`qmgr`/`resmgr`/`xstrings`/`vcpus` are likewise dead — hardcoded constants in `cluster.c`'s `init_controller_tunables()` (`cluster.c:1093-1108`), with existing TODOs acknowledging the blob values are ignored; `vcpus` is sourced only from the separate CLI-set `myconf->vcpus`, never from the blob. **`port` is the exception**: `cluster_open_listener` (`cluster.c:1216-1258`) falls back to `cluster_config->port` whenever the local controller's name isn't matched in `--peers` — and single-node mode passes no `--peers` at all (`main.c:293-294`, "No --peers flag; running as single-node primary"), making this the sole port-sourcing path for that mode today. Also: `xce_parse_peers` only WARNs (not rejects) on a `--peers` candidate with `port == 0` (`main.c:289-291`), so a malformed `--peers` entry for the local node hits the same live fallback. Full analysis: `src/docs/cluster-config-split-analysis.md` (note: that doc's own effort/risk table calls this "Low risk... no fallback needed," which this spec's investigation found does not hold for single-node mode — treat the doc's file:line citations as reliable, its risk conclusion for `port` specifically as superseded by this correction).
- **Two distinct "heartbeat" concepts** (do not conflate when implementing): (1) the HA/Raft controller-to-controller election heartbeat (`ha->config.heartbeat_interval_ms`, governs failover timing, currently hardcoded, no `ClusterConfig` field, out of scope here per Rule 0 in `config-design.md`); (2) the controller→worker liveness heartbeat (`ClusterConfig.heartbeatInterval`/`inactiveHeartbeatCount`, governs when a controller marks its own worker unavailable, stays in `ClusterConfig`, out of scope here because it's unchanged, not because it's unrelated).
- **`admin-config-tool` dependency**: `xceadmin config push` is soft-blocked on this spec landing (see `admin-config-tool` requirements.md Adjacent Expectations) — not because `xceadmin`'s code depends on it, but because until these fields are removed, nobody has verified the controller preserves them correctly when a pushed config leaves them zero-valued.
- **`MasterCandidate` capnp struct**: verified (grep, 2026-07-20) as referenced only by `ClusterConfig.candidates` and nowhere else in `controller.capnp` — removal is required by Requirement 1.6, not an open design question.
- **Stale citation, verified and dropped**: the prior analysis doc cites `testsuite/test.c:124,266` for an unrelated `get_setting("port")` (scheduler/worker connect-to-primary port) as a "don't touch this" landmine. That call no longer exists in `test.c` as of this spec's discovery pass — the citation has drifted. Two other test files DO reference a removed field directly: `test_commit_barrier_rollout.c:1359` and `test_config_repl.c:983` (see Requirement 5.1).
- **Prior analysis document**: `src/docs/cluster-config-split-analysis.md` (2026-06-08) already scoped this exact change (its "Phase 1"), verified the zero-blast-radius claim, and listed the exact file:line change sites — treat it as a primary design input, not something to re-derive from scratch. Some line numbers in it have drifted; current locations are in this spec's discovery notes.
## Requirements
### Requirement 1: Remove Dead Peer/Listener Fields
**Objective:** As an XCE maintainer, I want `port` and `candidates` removed from `ClusterConfig`, so that the schema no longer implies these are cluster-shared values when the live peer list is already sourced from `--peers`.
#### Acceptance Criteria
1. The `ClusterConfig` capnp struct shall not contain a `port` field.
2. The `ClusterConfig` capnp struct shall not contain a `candidates` field.
3. When the controller opens its listener and its name is found in the `--peers` CLI-parsed candidate list, the system shall determine the bind port from that list, exactly as it does today.
4. When the controller builds its internal debug/status config dump, the system shall not attempt to read `candidates` or `ncandidates` from `cluster_config_t` (the field no longer exists).
5. The `cluster_config_t` C struct (`src/include/clusterconfig.h`) shall not contain `port`, `ncandidates`, or a `candidates`/`master_candidate_t**` member, matching the capnp struct.
6. The `MasterCandidate` capnp struct shall be removed from `controller.capnp` (verified: referenced only by `ClusterConfig.candidates`, nowhere else in the schema — its removal is not a design-phase open question, it's a direct consequence of Requirement 1.2's `candidates` removal).
7. Where the controller's name is NOT found in `--peers` (including single-node mode, where `--peers` is not passed at all), the system shall provide a way to determine the controller's listen port that does not depend on a `ClusterConfig` field. This spec does not mandate the specific mechanism (e.g., a new `--port` CLI flag, or requiring single-node mode to pass a self-referencing `--peers` entry) — selecting it is a design-phase decision — but Requirement 1.1's removal of the `port` field SHALL NOT ship without this replacement in place, and single-node controller startup SHALL continue to work exactly as it does today once the replacement lands.
### Requirement 2: Remove Dead Performance-Tuning Fields
**Objective:** As an XCE maintainer, I want the already-hardcoded performance-tuning fields removed from `ClusterConfig`, so that the schema doesn't advertise configurability that doesn't exist.
#### Acceptance Criteria
1. The `ClusterConfig` capnp struct shall not contain `vcpus`, `jobmgr`, `workermgr`, `qmgr`, `resmgr`, or `xstrings` fields.
2. While these fields are absent from the schema, the controller shall continue to use its existing hardcoded tunable values (`c->tunables.*` defaults in `cluster.c`) with no behavior change from before this spec.
3. When the controller starts, the system shall not attempt to read `vcpus`, `jobmgr`, `workermgr`, `qmgr`, `resmgr`, or `xstrings` from `cluster_config_t` (the fields no longer exist).
4. The `cluster_config_t` C struct (`src/include/clusterconfig.h`) shall not contain `vcpus`, `jobmgr`, `workermgr`, `qmgr`, `resmgr`, or `xstrings` members, matching the capnp struct.
### Requirement 3: Preserve Cluster-Shared Fields Unchanged
**Objective:** As an XCE operator, I want the fields that are genuinely cluster-shared to remain exactly as they are today, so that this schema cleanup doesn't regress any existing config-push behavior.
#### Acceptance Criteria
1. The `ClusterConfig` capnp struct shall retain `clusterName`, `heartbeatInterval`, `inactiveHeartbeatCount`, `fetchJobInterval`, `netioTimeout`, `resources`, `queues`, `ugroups`, `hgroups`, `hostConfigs`, `hproviders`, `limits`, and `configGeneration` unchanged in type and semantics. Field numbers (`@N`) for these fields ARE expected to change (compacted to a contiguous `0..12` range): Cap'n Proto requires struct ordinals to be a gapless sequence, so removing 8 fields via direct deletion (Requirement 4.2 — not in-place reservation) necessarily renumbers every retained field declared after a removed one. This was verified empirically during `/kiro-impl` (2026-07-20): a schema with an ordinal gap fails to compile (`error: Skipped ordinal @N. Ordinals must be sequential with no holes.`).
2. Because retained fields are renumbered per 3.1, a `cluster.bin` built before this spec is NOT expected to decode its retained fields correctly when pushed to a controller built after this spec — Cap'n Proto's field layout is ordinal-position-based, so renumbering shifts wire byte-offsets for every field after the first removed one, not just the removed fields' own bytes. This is an accepted compatibility break for this pre-1.0 product (see Requirement 4.1). Any `cluster.bin` artifact that predates this spec — including the checked-in `ha/data/cluster.bin` used by `/xce-deploy-ha` and `/xce-test-ha` — must be rebuilt against the new schema before being used with a post-spec controller (Requirement 5.3).
3. The controller→worker liveness behavior driven by `heartbeatInterval`/`inactiveHeartbeatCount` shall be unaffected by this spec.
### Requirement 4: Compatibility Posture
**Objective:** As an XCE maintainer, I want the compatibility expectations for this change stated explicitly, so implementation and review don't over- or under-build compatibility handling.
#### Acceptance Criteria
1. Because this spec performs direct field removal via ordinal renumbering rather than in-place reservation (4.2), a `cluster.bin` built before this spec is not binary-compatible with a controller built after this spec — not only are the removed fields' values (`port`, `candidates`, `vcpus`, `jobmgr`, `workermgr`, `qmgr`, `resmgr`, `xstrings`) discarded without error, warning, or migration, but retained-field values will not decode correctly if such a pre-spec blob is pushed as-is, since their wire byte-offsets shifted (see 3.1/3.2). This is accepted given the product has not reached 1.0; no backward-read guarantee is provided for any pre-spec `cluster.bin`, and any such artifact — including the repo's own fixture — must be rebuilt against the new schema (Requirement 5.3).
2. The system shall not implement a deprecate-in-place transition (field-number reservation, dual-read-old-and-new) for the fields removed by this spec — direct removal (with the resulting renumbering of retained fields) is the chosen approach.
### Requirement 5: Build and Test Verification
**Objective:** As an XCE maintainer, I want this schema change verified as behavior-preserving, so the "zero blast radius" claim from the prior analysis is proven, not assumed.
#### Acceptance Criteria
1. When the controller AND its test suite are built after this spec's changes, the system shall compile without errors using the regenerated capnp bindings. This includes test code that directly constructs a `cluster_config_t` literal and sets a removed field — confirmed present at `test_commit_barrier_rollout.c:1359` (`cfg.port = 9990;`) and `test_config_repl.c:983` (`cfg.port = 9991;`); both shall be updated to drop the now-nonexistent assignment. Any other direct struct-literal reference to a removed field found during implementation shall be updated the same way.
2. When the existing controller unit test suite is run after this spec's changes, the system shall show no new failures attributable to the removed fields.
3. Because field-number renumbering breaks binary compatibility with any pre-spec `cluster.bin` (Requirement 3.2/4.1), the system shall regenerate the checked-in `ha/data/cluster.bin` fixture against the new (post-renumbering) schema, preserving the semantic values of all retained fields from the pre-spec blob, so `/xce-deploy-ha` and `/xce-test-ha` continue to work for this and every future spec that relies on that fixture.
4. When a 3-node HA cluster is deployed and run through a config-push cycle after this spec's changes — using the regenerated `ha/data/cluster.bin` from Criterion 3 — the system shall behave identically to before this spec (peer discovery via `--peers`, listener binding, worker liveness tracking) — verified live, not just by unit tests, given this touches `cluster.c`/`ha.c`/`main.c`.
5. When a single-node controller (no `--peers` flag) is started after this spec's changes, the system shall successfully bind its listener and operate correctly using Requirement 1.7's replacement port-sourcing mechanism — this is the scenario Finding #2 of this spec's review identified as the actual regression risk, and it shall be verified live, not assumed from the multi-node soak in Criterion 4.
### Requirement 6 (Nice-to-Have): CLI-Configurable Performance Tunables
**Objective:** As an XCE operator running a high-memory or high-latency controller node, I want the previously-hardcoded hash-table-size tunables to become CLI-configurable, so I can tune an individual controller's performance without needing a schema field that was never wired up anyway.
#### Acceptance Criteria
1. Where this nice-to-have is implemented, the system shall accept `--jobmgr-nhash`, `--workermgr-nhash`, `--qmgr-nhash`, `--resmgr-nhash`, `--xstrings-nhash` (and corresponding `-delta`/`-inst` variants as applicable) CLI flags, following the same `OPT_*` + `struct arguments` + `myconf_t` field pattern already used for `--vcpus`.
2. Where a tunable CLI flag is not specified (or is 0), the system shall fall back to the existing hardcoded default for that tunable, with no behavior change from Requirement 2.

View file

@ -0,0 +1,82 @@
# Research & Design Decisions
## Summary
- **Feature**: `clusterconfig-schema-split`
- **Discovery Scope**: Extension (existing system) — light discovery per `design-discovery-light.md`
- **Key Findings**:
- `candidates`, `vcpus`, and the five perf-tuning fields are genuinely dead in `ClusterConfig` at runtime (verified across every `.c` file that touches `cluster_config_t`).
- `port` is NOT dead — its removal (Requirement 1.1) needs a replacement mechanism (Requirement 1.7), which this design resolves as a new `--port` CLI flag, matching the existing `--vcpus` pattern exactly.
- No tooling in this repo (`deploy-ha.sh`, test scripts) currently starts a controller with zero peer-related flags in a way that would be affected either way — but the codebase's own `main.c:294` message ("No --peers flag; running as single-node primary") confirms the zero-`--peers` invocation is an intentional, first-class supported mode, not incidental. The design preserves it exactly rather than forcing a CLI-contract change.
## Research Log
### Requirement 1.7's port-sourcing replacement mechanism
- **Context**: `requirements.md` Requirement 1.7 deliberately left the replacement mechanism as an open design question after the requirements-phase review found `cluster_open_listener` (`cluster.c:1216-1258`) depends on `cluster_config->port` as the sole port source for single-node mode (no `--peers` flag passed at all, `main.c:293-294`).
- **Sources Consulted**: `src/controller/main.c` (CLI parsing, `--vcpus` precedent at lines 65-66/114-118/252, single-node branch at 293-295), `src/controller/cluster.c:1216-1258` (port resolution), `src/include/myconf.h` (existing CLI-derived field conventions), `src/controller/ha.h` (hardcoded-default naming convention, `HA_DEFAULT_*`), prior memory of this codebase (`--peers` with a single self-referencing candidate is a confirmed, already-used alternative single-node invocation — but no repo tooling actually relies on the zero-flag form being removed).
- **Findings**:
- No standalone `--port` CLI flag exists today — confirmed via grep (zero hits for `OPT_PORT`/`"port"` in `main.c`).
- `--vcpus` establishes the exact pattern to follow: `OPT_VCPUS` argp option → `struct arguments.vcpus` → validated → copied into `myconf.vcpus` (0 = unset sentinel, documented inline in `myconf.h:37`) → consumed at the point of use with a hardcoded fallback (`cluster.c:1320`, `myapp.c:129`: `(myconf->vcpus > 0) ? myconf->vcpus : 8`).
- `deploy-ha.sh` (the only repo-internal controller launcher found) always passes `--peers`; no repo script exercises the true zero-flag single-node path. This means changing that path's invocation contract would carry no *repo-internal* tooling risk — but it's still a documented, INFO-logged, intentionally-supported mode (`main.c:294`), not dead code, so removing it outright is a real behavior change to a supported feature, not cleanup.
- `xce_parse_peers`'s side effects (CA cert path population, identity string, `main.c:278-292`) only run inside the `if (arguments.peers_str)` branch — the zero-flag path currently skips them. Preserving the zero-flag path (rather than forcing a synthetic self-referencing `--peers` entry) avoids introducing any change to that unrelated cert/identity setup.
- **Implications**: Adding `--port` as a new CLI flag (rather than mandating `--peers` for all invocations) requires slightly more new code but changes zero existing invocation behavior and directly reuses a pattern already reviewed and shipped in this exact file. Given this spec's own history of an overconfident "zero blast radius" claim being wrong once already, the lower-risk option is preferred even at the cost of a few more lines.
## Architecture Pattern Evaluation
| Option | Description | Strengths | Risks / Limitations | Notes |
|--------|-------------|-----------|---------------------|-------|
| A: `--port` CLI flag (chosen) | New `OPT_PORT` flag, `myconf.port` field, hardcoded fallback constant when unset and not found in `--peers` | Zero invocation-UX change; exact precedent (`--vcpus`) to copy; isolated, low-risk diff | Slightly more new code than B | Selected — see Decision below |
| B: Mandate `--peers` everywhere (self-referencing entry for single-node) | Remove the zero-flag path; single-node must pass `--peers ctrl-1=host:port` | Single source of truth for port; reuses an already-proven 1-peer self-quorum path; less code | Removes a documented, intentionally-supported invocation mode; unverifiable whether anything outside this repo (docs, operator muscle memory) depends on the zero-flag form | Rejected — the removed mode is intentional/documented, not incidental, and this spec's mandate is schema cleanup, not an invocation-contract change |
## Design Decisions
### Decision: Add `--port` CLI flag as the replacement for `ClusterConfig.port`
- **Context**: Requirement 1.7 requires a non-`ClusterConfig` port-sourcing mechanism before `port` can be removed from the schema, without breaking single-node controller startup.
- **Alternatives Considered**: (A) new `--port` flag, hardcoded fallback default; (B) mandate `--peers` for all invocations including single-node.
- **Selected Approach**: (A). New `OPT_PORT` argp option → `struct arguments.port` (uint32_t, default 0) → copied to a new `myconf_t.port` field → `cluster_open_listener()`'s fallback branch reads `myconf->port` instead of `cluster_config->port`; if still 0 (flag not passed), falls back to a new hardcoded constant `XCE_CONTROLLER_DEFAULT_PORT = 9990` (matching today's exact `SET_DEFAULT_IF_UNSET(c->port, 0, 9990)` value byte-for-byte, so the default behavior is unchanged for anyone relying on it).
- **Rationale**: Exact structural precedent already exists and was already reviewed/shipped for `--vcpus`; zero changes to any existing invocation (scripted or manual); avoids introducing a new port-precedence edge case between two different flags (`--port` only matters when `--peers` doesn't supply a match, so precedence is unambiguous: `--peers` match > `--port` > hardcoded default).
- **Trade-offs**: A few more lines of new code than mandating `--peers` everywhere; two flags (`--peers`, `--port`) both nominally relate to "port" now, though only one (`--peers`) is meaningful once any peer is configured.
- **Follow-up**: None — this is fully specified for the tasks phase.
## Risks & Mitigations
- **Risk**: A future contributor reintroduces a `ClusterConfig`-sourced default for `port`/`candidates`/hash-tuning "for convenience" during an unrelated change, recreating the original problem. **Mitigation**: `.kiro/steering/config-design.md` now documents the general rule; this spec's removal is the concrete enforcement.
- **Risk**: The checked-in `ha/data/cluster.bin` might carry non-default values for the removed fields, silently masking a real backward-compat issue if the live-soak test (Requirement 5.3) doesn't actually inspect them first. **Mitigation**: Requirement 5.3 (and Testing Strategy below) mandates inspecting or synthesizing non-default legacy field values before relying on that test as proof.
- **Risk**: `test_commit_barrier_rollout.c:1359` / `test_config_repl.c:983` are the only *known* struct-literal hits, but a broader grep pattern might surface more during implementation. **Mitigation**: Requirement 5.1's catch-all clause; the build itself will fail loudly on any missed site (C struct field removal is a compile error, not a silent runtime gap).
## References
- `src/docs/cluster-config-split-analysis.md` — prior team analysis (2026-06-08), primary design input for the dead-field classification.
- `.kiro/steering/config-design.md` — general shared-vs-local config classification rule this spec enforces.
- `.kiro/specs/admin-config-tool/requirements.md` — downstream consumer whose "per-instance field preservation" risk this spec closes structurally.
## Design Review (`/kiro-validate-design`, 2026-07-20)
Initial GO/NO-GO: NO-GO, 3 issues found on a fresh critical pass of the first design draft. All fixed in `design.md` directly:
1. **Flow diagram dropped the zero-value fallback trigger.** Original diagram branched on "name found" (boolean); actual pre-spec code branches on `my_port == 0`, which also covers "found but that candidate's port field is malformed/zero" (`main.c:289-291` already WARNs on this input). As drafted, a matched-but-zero-port entry would have bound to literal port 0. Fixed: diagram and Service Interface contract now check the port value, not just match presence.
2. **Removing the `-EINVAL` path conflated two different scenarios.** Original design treated "`--peers` never passed" (true single-node) and "`--peers` passed but local name absent" (likely operator typo) identically — both silently falling to `--port`/hardcoded-default. User decision (interactive, during review): keep today's hard error for the second case; only the first gets the new fallback. Rationale: the project's fail-loud convention (`src/controller/CLAUDE.md` process-lifecycle invariant) — a silent wrong-default here would surface later as a confusing "peer unreachable" symptom instead of a clear startup error now. Fixed: System Flows, Service Interface, and Error Handling sections all updated to reflect the split.
3. **Requirement 6 missing from Boundary Commitments.** Present in File Structure Plan/Components/Traceability but not in "This Spec Owns" — mechanical inconsistency. Fixed: one bullet added.
Re-affirmed GO after fixes — no further review pass required (fixes were direct, not requiring a requirements-phase return).
## Design Revision (`/kiro-impl` kickoff, 2026-07-20)
**Reversed the Architecture Pattern Evaluation decision** (Option A → Option B) before implementation started, per direct user instruction at the start of `/kiro-impl`.
- **Original choice**: Option A, a new `--port` CLI flag + `myconf_t.port` field, mirroring `--vcpus`.
- **Revised choice**: Option B (previously rejected) — no new CLI flag. `cluster_open_listener`'s fallback becomes two-tier: `--peers` candidate match (unchanged) → hardcoded `XCE_CONTROLLER_DEFAULT_PORT` (9990) when `--peers` was never passed at all. Operators who want a non-default single-node port pass a single self-referencing `--peers <name>=<host>:<port>` entry (matching `-n`/`--myname`) — an **already-existing, already-supported** invocation mode (confirmed working: single-node primary via `--peers` omission OR a 1-peer `--peers` self-quorum entry are both live paths today, verified by grepping `cluster_open_listener`'s existing candidate-match loop against `c->myconf->name`).
- **Why the reversal holds up**: the original rejection of Option B was "removes a documented, intentionally-supported invocation mode" — but Option B as now scoped does NOT remove the zero-`--peers` mode; it only asks operators who need a *non-default port* in that mode to use the already-supported 1-peer `--peers` form instead of a brand-new flag. Zero new surface area, zero new plumbing (`main.c` argp wiring, `myconf_t.port` field, `myconf.vcpus`-style wiring all become unnecessary), and it reuses code that already exists and is already exercised.
- **Requirements impact**: none — Requirement 1.7 already listed "requiring single-node mode to pass a self-referencing `--peers` entry" as an explicit example alternative and left mechanism selection to the design phase.
- **Design impact**: `design.md`'s Overview, Goals, Boundary Commitments, Architecture diagram, File Structure Plan, Requirements Traceability, Components and Interfaces, Service Interface contract, and System Flows are all updated in place to reflect the two-tier fallback with no CLI Component for Requirement 1.7 (the CLI Component now exists only for the optional Requirement 6 tunables, reusing the pre-existing `--vcpus` pattern directly rather than a `--port` precedent).
- **Tasks impact**: `tasks.md`'s former Task 2 (`--port` CLI flag) is removed; all subsequent major tasks renumbered down by one.
- **Note**: `xce_parse_peers` also accepts a bare `host:port` candidate (no `name=` prefix), matched via `hostname` when no name is given — but `design.md`/`tasks.md` document the explicit `name=host:port` form as the canonical example for this replacement mechanism (clearer about intent than relying on hostname matching implicitly).
## Design Revision 2: Ordinal-Gap Discovery (`/kiro-impl` task 1.1, 2026-07-20)
The task 1.1 implementer, before touching the schema, found and empirically proved a contradiction between Requirement 3.1 (retained fields keep their literal original `@N` field number) and Requirement 4.2 (direct removal, no field-number reservation): Cap'n Proto requires struct ordinals to be a gapless `0..N-1` sequence — verified independently (`capnp compile` on a schema with a gap fails: `error: Skipped ordinal @1. Ordinals must be sequential with no holes.`). Deleting 8 fields from the middle of `ClusterConfig`'s 21 while freezing the 13 survivors' literal ordinals is not something any compiling schema can satisfy — you either renumber the survivors (breaks their wire byte-offsets for anything reading an old blob) or leave inert renamed placeholders at the old ordinals (which is field-number reservation, exactly what 4.2 forbids).
Neither `requirements.md` nor `design.md` had anticipated this; both were written assuming Cap'n Proto allows field deletion independent of ordinal freezing, which is not the case.
- **Decision (user, interactive)**: renumber the 13 survivors to a contiguous `0..12` range — literal direct removal, matching Requirement 4.2's exact text and the project's "still pre-release, no wire-compat obligation" posture. Requirement 3.1 amended to drop "field number" from its invariance clause (type/semantics only); Requirement 3.2 and 4.1 amended to state plainly that a `cluster.bin` built before this spec is not expected to decode correctly (not just the removed fields — every retained field after the first removed one shifts byte-offset).
- **Consequence discovered next**: the checked-in `ha/data/cluster.bin` (`deploy-ha.sh:33-36` copies it verbatim as every cluster's seed config) is itself a pre-spec artifact. Post-renumbering, its retained fields (`resources`, `queues`, `hostConfigs`, `hproviders`, `limits`, etc.) would silently decode to wrong bytes — breaking `/xce-deploy-ha` and `/xce-test-ha` for every future spec that depends on that fixture, not just this one's verification task. No existing tool rebuilds this blob (`xceadmin config build` isn't implemented yet — separate, unstarted spec).
- **Decision (user, interactive)**: add a task to regenerate `ha/data/cluster.bin` — decode its current retained-field values under the pre-spec schema, re-encode under the new schema with a small throwaway tool (written and discarded, not shipped), overwrite the fixture in place. New Requirement 5.3; old 5.3 (3-node soak) → 5.4, old 5.4 (single-node) → 5.5. `tasks.md` gained a new task 5.1 (regenerate fixture) ahead of the renumbered 3-node-soak task.
- **Rationale**: given the renumbering choice, the fixture WILL be silently wrong if left untouched — this is not optional cleanup, it's a repo-wide landmine for every HA spec's live verification, self-inflicted by this spec's own schema edit. Regenerating it once, in-scope, is far cheaper than every future spec independently rediscovering "why is deploy-ha producing garbage config."
- **Scope boundary held**: the throwaway encoding tool is explicitly NOT a permanent addition — building a real `cluster.bin` authoring tool is `admin-config-tool`'s job (`xceadmin config build`), not this spec's. This task only needs to reproduce ONE known blob's values under the new layout, not build general tooling.

View file

@ -0,0 +1,22 @@
{
"feature_name": "clusterconfig-schema-split",
"created_at": "2026-07-20T00:29:00Z",
"updated_at": "2026-07-20T06:15:00Z",
"language": "en",
"phase": "tasks-generated",
"approvals": {
"requirements": {
"generated": true,
"approved": true
},
"design": {
"generated": true,
"approved": true
},
"tasks": {
"generated": true,
"approved": true
}
},
"ready_for_implementation": true
}

View file

@ -0,0 +1,99 @@
# Implementation Plan
- [ ] 1. Schema & Struct Foundation
- [x] 1.1 Remove obsolete fields and the `MasterCandidate` struct from the `ClusterConfig` schema, regenerate bindings
- Remove `port` (@1), `candidates` (@2), `vcpus` (@7), `jobmgr` (@13), `workermgr` (@14), `qmgr` (@15), `resmgr` (@16), `xstrings` (@17) from `ClusterConfig` in `controller.capnp`
- Remove the `MasterCandidate` struct entirely
- Confirm `clusterName`, `heartbeatInterval`, `inactiveHeartbeatCount`, `fetchJobInterval`, `netioTimeout`, `resources`, `queues`, `ugroups`, `hgroups`, `hostConfigs`, `hproviders`, `limits`, `configGeneration` remain unchanged in type and semantics; renumber them to a contiguous `@0..@12` range (Cap'n Proto forbids ordinal gaps — verified empirically, see `research.md`'s "Design Revision 2" — so direct removal of the 8 fields necessarily renumbers every retained field declared after a removed one)
- Regenerate the capnp C bindings via the existing meson codegen step; do not hand-edit generated output
- Observable: capnp compiler succeeds against the edited schema; the generated `ClusterConfig` reader/builder exposes exactly 13 fields, ordinals `@0..@12`, and no `MasterCandidate` type
- _Requirements: 1.1, 1.2, 1.6, 2.1, 3.1, 4.2_
- _Boundary: Schema Component_
- [ ] 1.2 Remove the matching members from `cluster_config_t`
- Delete `port`, `ncandidates`, `candidates`/`master_candidate_t**`, `vcpus`, `jobmgr`, `workermgr`, `qmgr`, `resmgr`, `xstrings` members from `cluster_config_t` in `clusterconfig.h`
- Leave all retained members (matching the schema's surviving 13 fields) untouched
- Observable: `clusterconfig.h`'s field list matches the regenerated schema field-for-field; any remaining reference to a removed member elsewhere in the tree is now a compile error, not a silent gap
- _Requirements: 1.5, 2.3, 2.4_
- _Depends: 1.1_
- _Boundary: C Struct Component_
- [ ] 2. Core: cluster.c Port Resolution, Debug-Dump & Port-Defaulting Cleanup
- [ ] 2.1 Update `cluster_open_listener`'s port-resolution fallback
- Change the fallback branch from reading `cluster_config->port` to a new hardcoded `XCE_CONTROLLER_DEFAULT_PORT` constant (9990), reached only when `--peers` was never passed at all (`peers_cfg.candidates_count == 0`)
- Do NOT add a new CLI flag or `myconf_t` field — the `--peers` candidate-match branch (including its existing single self-referencing `--peers <name>=<host>:<port>` usage for single-node mode) is left completely unchanged; this is the only mechanism an operator uses to get a non-default single-node port
- Preserve the exact precedence and branch structure: a `--peers` match with nonzero port binds to it unchanged; a matched candidate with port == 0 falls through to the hardcoded-default tier exactly as today's tolerated case; `--peers` passed but local name absent still returns `-EINVAL` and refuses to start, unchanged
- Observable: a controller started with no `--peers` binds to 9990; a controller started with a single self-referencing `--peers` entry binds to that entry's port; a controller started with `--peers` not containing the local name still exits with the existing `-EINVAL` startup error
- _Requirements: 1.3, 1.7_
- _Depends: 1.2_
- _Boundary: Listener/Port-Resolution Component (cluster.c)_
- [ ] 2.2 Remove the `candidates` debug-dump mirror
- Delete the `candidates`/`ncandidates` KV-mirror loop from `cluster_build_config`
- Confirm `init_controller_tunables()` itself is left unchanged — the hardcoded tunable constants and their consumption are untouched by this spec
- Observable: `cluster_build_config` compiles without referencing `candidates` or `ncandidates`; the controller's internal debug/status config dump no longer includes those keys
- _Requirements: 1.4, 2.2_
- _Depends: 1.2_
- _Boundary: Debug-Dump Component (cluster.c)_
- [ ] 2.3 Remove the now-dead `ClusterConfig` port defaulting in `myconf.c` and `config_repl.c`
- Delete the `SET_DEFAULT_IF_UNSET(c->port, 0, 9990)` call in `myconf.c`
- Delete the equivalent `SET_DEFAULT_IF_UNSET(c->port, 0, 9990)` call in `config_repl.c`
- Observable: `myconf.c` and `config_repl.c` compile without referencing `cluster_config_t`'s `port` member
- _Requirements: 1.7_
- _Depends: 2.1_
- _Boundary: myconf.c, config_repl.c_
- [ ] 3. (P) Update struct-literal test fixtures for the shrunk `cluster_config_t`
- Drop the `cfg.port = 9990;` literal at `test_commit_barrier_rollout.c:1359`
- Drop the `cfg.port = 9991;` literal at `test_config_repl.c:983`
- Grep the full tree for any other direct `cluster_config_t` struct-literal reference to a removed field and update each the same way
- Observable: both known test files compile against the shrunk struct with no remaining reference to a removed field; a repo-wide grep for the removed field names against struct-literal assignments returns no further hits
- _Requirements: 5.1_
- _Depends: 1.2_
- _Boundary: Test Fixture Updates_
- [ ] 4. Build the controller and run the full unit test suite
- Build `controller_unit_test` (meson target) with the regenerated capnp bindings and all updated source/test files
- Run the full existing suite and compare against the pre-spec baseline
- Observable: build completes with zero compile errors; `controller_unit_test` passes with no new failures attributable to the removed fields
- _Requirements: 5.1, 5.2_
- _Depends: 1.1, 1.2, 2.1, 2.2, 2.3, 3_
- [ ] 5. Validation: Live HA Verification
- [ ] 5.1 Regenerate the `ha/data/cluster.bin` fixture against the renumbered schema
- Decode the current (pre-spec) `ha/data/cluster.bin`'s retained-field values under the OLD (pre-renumbering) schema — build against a pre-1.1 checkout or otherwise capture the pre-spec field values before they become unreadable
- Write and run a small throwaway capnp-encoding tool (not part of the shipped codebase — discard it after use) that constructs a `ClusterConfig` with those same retained-field values and serializes it under the NEW (post-task-1.1) schema
- Overwrite `ha/data/cluster.bin` with the newly-encoded blob
- Observable: the regenerated `ha/data/cluster.bin` decodes correctly under the post-spec schema with every retained field matching the pre-spec blob's semantic values; `deploy-ha.sh`'s existing verbatim-copy usage (`deploy-ha.sh:33-36`) requires no changes and keeps working
- _Requirements: 3.2, 5.3_
- _Depends: 4_
- [ ] 5.2 3-node HA cluster config-push verification
- Deploy a 3-node HA cluster using the existing deploy tooling (which seeds ctrl-1 from the regenerated `ha/data/cluster.bin`) and confirm peer discovery via `--peers`, listener binding, and worker-liveness tracking behave identically to pre-spec
- Observable: the 3-node cluster reaches steady state with unchanged peer/listener/worker-liveness behavior, using the regenerated fixture with no error or warning
- _Requirements: 3.3, 5.4_
- _Depends: 4, 5.1_
- [ ] 5.3 Single-node startup, self-referencing-peers port override, and preserved error-path verification
- Start a controller with no `--peers` flag and confirm it binds its listener to the hardcoded default port (9990) from task 2.1
- Start a controller with a single self-referencing `--peers <name>=<host>:<port>` entry (own `-n`/`--myname` matching, custom port) and confirm it binds to that port instead of the default — this is the sole replacement mechanism for a non-default single-node port under this spec
- Start a controller with a `--peers` list that does not contain the controller's own name and confirm it still refuses to start with the existing `-EINVAL` error, unchanged from pre-spec behavior
- Observable: the no-peers controller reaches a running state bound to port 9990; the self-referencing-peers controller reaches a running state bound to its specified custom port; the name-absent-from-peers controller exits with the same startup error it produced before this spec
- _Requirements: 5.5_
- _Depends: 4_
- [ ] 6. (Optional — Requirement 6, nice-to-have, deferrable without blocking the rest of this spec) CLI-Configurable Performance Tunables
- [ ] 6.1 Add CLI flags for the hash-table tunables
- Add `--jobmgr-nhash`, `--workermgr-nhash`, `--qmgr-nhash`, `--resmgr-nhash`, `--xstrings-nhash` (and corresponding `-delta`/`-inst` variants where the tunable has more than one dimension) following the same `OPT_*` + `struct arguments` + `myconf_t` pattern already used for `--vcpus`
- Observable: `xce_controller --help` lists all new flags; passing one populates the corresponding `myconf_t` field
- _Requirements: 6.1_
- _Depends: 1.2_
- _Boundary: CLI Tunables Component (main.c, myconf.h)_
- [ ] 6.2 Wire the tunables into `init_controller_tunables()`
- Read the new `myconf_t` fields in `init_controller_tunables()`, falling back to the existing hardcoded default for any field left at 0 (unset)
- Observable: passing a tunable flag changes the corresponding controller tunable at startup; omitting all of them reproduces today's exact hardcoded values
- _Requirements: 6.2_
- _Depends: 6.1_
- _Boundary: Tunables Component (cluster.c)_

View file

@ -671,12 +671,6 @@ struct ResmgrConfig $C.mapname("resmgr_config_t") {
instDelta @2: UInt32 $C.mapname("inst_delta");
}
struct MasterCandidate $C.mapname("master_candidate_t") {
name @0: Text $C.mapname("name");
host @1: Text $C.mapname("host");
port @2: UInt32 $C.mapname("port");
}
struct HostConfig $C.mapname("xce_hostconfig_t") {
name @0: Text $C.mapname("name");
model @1: Text $C.mapname("hostmodel");
@ -710,26 +704,18 @@ struct LimitConfig $C.mapname("xce_limit_config_t") {
struct ClusterConfig $C.mapname("cluster_config_t") {
clusterName @0: Text $C.mapname("clustername");
port @1: UInt32 $C.mapname("port");
candidates @2: List(MasterCandidate) $C.mapname("candidates") $C.maplistcount("ncandidates");
heartbeatInterval @3: UInt32 $C.mapname("heartbeat_interval");
inactiveHeartbeatCount @4: UInt32 $C.mapname("inactive_heartbeat_count");
fetchJobInterval @5: UInt32 $C.mapname("fetch_job_interval");
netioTimeout @6: UInt32 $C.mapname("netio_timeout");
vcpus @7: UInt32 $C.mapname("vcpus"); # deprecated: use --vcpus CLI flag; kept for config blob backward compatibility
resources @8: List(Resource) $C.mapname("resources") $C.maplistcount("nres");
queues @9: List(QueueConf) $C.mapname("queues") $C.maplistcount("nqueues");
ugroups @10: List(GroupConfig) $C.mapname("ugroups") $C.maplistcount("nugroups");
hgroups @11: List(GroupConfig) $C.mapname("hgroups") $C.maplistcount("nhgroups");
hostConfigs @12: List(HostConfig) $C.mapname("hconfigs") $C.maplistcount("nhconfigs");
jobmgr @13: HashZArrayConfig;
workermgr @14: HashZArrayConfig;
qmgr @15: HashZArrayConfig;
resmgr @16: ResmgrConfig;
xstrings @17: HashZArrayConfig;
hproviders @18: List(ScalerConfig) $C.mapname("hproviders") $C.maplistcount("nhproviders");
limits @19: List(LimitConfig) $C.mapname("limits") $C.maplistcount("nlimits");
configGeneration @20: UInt64 $C.mapname("config_generation");
heartbeatInterval @1: UInt32 $C.mapname("heartbeat_interval");
inactiveHeartbeatCount @2: UInt32 $C.mapname("inactive_heartbeat_count");
fetchJobInterval @3: UInt32 $C.mapname("fetch_job_interval");
netioTimeout @4: UInt32 $C.mapname("netio_timeout");
resources @5: List(Resource) $C.mapname("resources") $C.maplistcount("nres");
queues @6: List(QueueConf) $C.mapname("queues") $C.maplistcount("nqueues");
ugroups @7: List(GroupConfig) $C.mapname("ugroups") $C.maplistcount("nugroups");
hgroups @8: List(GroupConfig) $C.mapname("hgroups") $C.maplistcount("nhgroups");
hostConfigs @9: List(HostConfig) $C.mapname("hconfigs") $C.maplistcount("nhconfigs");
hproviders @10: List(ScalerConfig) $C.mapname("hproviders") $C.maplistcount("nhproviders");
limits @11: List(LimitConfig) $C.mapname("limits") $C.maplistcount("nlimits");
configGeneration @12: UInt64 $C.mapname("config_generation");
}
# ── Persistence configuration (Part 2.6) ────────────────────────────