Features: #330 — the split reload class, and the top-level coverage checker

Dai Ha
2026-09-04 15:24:41 +07:00
parent 0aefbae904
commit 3dce3d1889
+66
@@ -4029,3 +4029,69 @@ startup snapshot and live, at different sites, so no single class fits either. A
them still under-claims. That count and both verdicts are now in `ConfigRef`'s class doc so the next
person does not measure it again. Twice now — `worktreeGroup`, then `primary`/`configReload` — the
untriaged kind hid among the correct kind.
---
## A reload can now say "half of this applied"
**What it does.** `ConfigRef` has a fourth reload class, `split`, for a key that is read **both**
off the startup snapshot and live off the config supplier, at different sites. `health:` and
`coordinator:` are both like that. A reload that changes either now names the key and says which
half is already live and which half waits for a restart. `Outcome` carries a new `split` list
beside `deferred`.
**On.** Always on (fleetd #330, merged `7b918c5`). Visible in the reload summary:
```
config reloaded; partially live — coordinator: the LeadMailbox connection (uri, uriEnv, selfId,
prefetch) is opened once and needs a restart; the broker URI env-var name kept out of a member's
environment is read live on every spawn and already applied
```
**Why it exists.** Before this, changing either key reported a bare `config reloaded`. That
under-claims: it tells the operator a change applied when half of it did not. The three existing
classes could not express the truth, and forcing one of them would be wrong in one direction or the
other. The direction matters. Over-claiming a restart costs an unnecessary restart, which the
operator can see and recover from. Under-claiming is what this file's own doc calls "the worst
thing a reload can do to an operator debugging one". `split` is the only option that is simply
true. Making the frozen half live was rejected for `coordinator`: `selfId` names this daemon's own
AMQP inbox queue, so changing it live is a distributed-identity problem, not a reconnect.
**One thing to know for maintenance.** Membership in `SPLIT_KEYS` is **not** proof that any
reporting code exists for that key. `ConfigRefTopLevelCoverageTest` reads a name in the set as
"triaged" and stops there — it cannot see whether `changedSplitKeys` has a branch for it. Measured:
dropping the `coordinator` branch while leaving the name in the set left both the coverage test and
the `assert` silent; only three hand-written behavioural tests caught it. The same one-way shape is
older than this change — `assert COLD_KEYS.containsAll(changed)` catches "reported but not listed"
and never the reverse. `fleet:` is already an instance of the gap: it is split too and it sits in
the checker's hot-exclusion hatch, so a changed `fleet.leaders` still reports nothing. Both are open
in fleetd #333.
---
## Every top-level config key must be triaged before it ships
**What it does.** `ConfigRefTopLevelCoverageTest` enumerates `FleetConfig`'s record components by
reflection and requires each one to sit in exactly one of four buckets: `COLD_KEYS`, the deferred
set, `SPLIT_KEYS`, or a pinned hot-exclusion set. A new key in none of them fails the build by name.
It prints its own denominator every run:
```
FleetConfig top-level coverage — 22 components total: 5 cold [...], 11 deferred [...],
2 split [health, coordinator], 4 hot-excluded [...]
```
**On.** Always on — it is a unit test (fleetd #330).
**Why it exists.** Three times the same key-was-forgotten bug shipped: `worktreeGroup` (#323),
then `primary` and `configReload` (#326), then `fleet` (#333). Each time a reload reported success
for a change the daemon never picked up. "Not mentioned in `ConfigRef`" looks identical for a key
that is correctly hot and a key nobody triaged, so the forgotten kind kept hiding among the correct
kind. This is the top-level twin of the per-profile checker #323 added.
**One thing to know for maintenance.** Read the test's own javadoc before trusting a green run. It
says plainly what it cannot do: it proves the record's *shape* is triaged, and it **cannot** prove a
citation is true. "Compared in `changedDeferredKeys`" and "read live off `config.get()`" are facts
about other files that a reflection test over one record cannot inspect. That disclosure is not
modesty — it is what made the two gaps in #333 findable in minutes. Hold any checker in this repo to
the same standard: say what it does not cover, next to what it does.