chore(controller): clusterconfig-schema-split task 5.3 — single-node/peers-override/EINVAL live-verified

Live-verified all three cluster_open_listener code paths not exercised by
the 3-node soak: no --peers binds hardcoded default 9990; a single
self-referencing --peers entry binds its custom port instead; --peers
given but local name absent still refuses to start with -EINVAL, unchanged.
Independently reproduced by the reviewer. This closes the actual
regression risk this spec's design was built around.

Review also found (bug-1457, logged, not fixed here) that pre-spec
single-node startup was already broken with -EINVAL before this spec
began, due to an unrelated myapp_run listener-open/config-load ordering
issue -- task 2.1's rewrite incidentally repaired it. research.md updated
to record this against the spec's original preservation premise.
This commit is contained in:
Rongsong Shen 2026-07-21 05:56:05 +00:00
parent 3e01fb1bcb
commit edd79562a4
3 changed files with 117 additions and 4 deletions

View file

@ -88,3 +88,9 @@ The task 1.1 implementer deliberately left `HashZArrayConfig` and `ResmgrConfig`
- **Root cause**: this is the same class of gap Requirement 1.6 already closed for `MasterCandidate` (used only by the removed `candidates` field) — just missed for the tunables side during the original requirements pass, since `MasterCandidate` was called out explicitly by name while the tunables' supporting struct types were not.
- **Decision**: extend the same treatment — remove `HashZArrayConfig`/`ResmgrConfig` from `controller.capnp`. Verified via grep (same method as the original `MasterCandidate` verification) that neither struct is referenced anywhere else in the schema. New Requirement 2.5 added (mirrors 1.6's phrasing); design.md's File Structure Plan and Requirements Traceability updated; `tasks.md` gained task 1.3 (Schema Component, same boundary as 1.1) to close this before task 4's full build.
- **Why this wasn't caught in the requirements-phase discovery pass**: the original `cluster-config-split-analysis.md` and this spec's requirements-phase grep swept for the FIELDS being removed, not for supporting/nested struct TYPES those fields' capnp types depended on. A useful pattern for future field-removal specs: after identifying fields to remove, also check whether any of their types are struct types used nowhere else in the schema.
## Correction: Pre-Spec Single-Node Startup Was Already Broken (discovered during task 5.3's review, 2026-07-20)
Task 5.3's reviewer, verifying the live single-node scenarios, built and ran the actual pre-spec baseline (commit `94f50ce`, HEAD before this spec's first commit) for comparison — and found single-node startup (`xce_controller` with no `--peers`) was **already exiting with `-EINVAL`**, unconditionally, before this spec began. Root cause: `myapp_run()` (`src/controller/myapp.c`) calls `cluster_open_listener()` before the primary config (`cluster_config`) is loaded from disk, so the pre-spec fallback branch (`cluster_config->port`, only reachable when `cluster_config != NULL`) always hit its `else` branch. Logged as `bug-1457` (unrelated to this spec, not fixed as its own change — task 2.1's rewrite incidentally repaired it, verified live in task 5.3).
**This invalidates part of this spec's own premise, stated in the Introduction and Requirement 1.7's original correction**: "`cluster_open_listener`... has a live fallback to `cluster_config->port`... the sole port-sourcing mechanism for single-node deployments" and "single-node controller startup SHALL continue to work exactly as it does today." In reality, it did not work today (pre-spec) — for a reason entirely unrelated to `ClusterConfig`'s schema. The spec's actual behavior (task 2.1's hardcoded-default fallback, independent of `cluster_config` load state) is still correct and was still the right thing to build — it just fixed a real bug as a side effect, rather than merely "preserving" already-working behavior as originally framed. Not correcting the historical Introduction/Requirement 1.7 text itself (it accurately reflects what was believed and reviewed at design time); this note is the record of what live verification actually found.

View file

@ -84,7 +84,7 @@
- _Requirements: 3.3, 5.4_
- _Depends: 4, 5.1_
- [ ] 5.3 Single-node startup, self-referencing-peers port override, and preserved error-path verification
- [x] 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