fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

A config key's hot/deferred class is a claim about every consumer, and nothing checks it that way #427

New Issue
Open
opened 2026-09-10 06:54:03 +02:00 by ltms · 1 comment
ltms commented 2026-09-10 06:54:03 +02:00
Owner
Copy Link
Copy Source

The structural gap behind #404, #416, #400, #424 and #425. Filing it separately because fixing any one of those does nothing about the next one.

The pattern, five times

ticket direction what happened
#404 live read, deferred key exhaustionDetectionArmed read the live config; detection read the startup snapshot
#416 live read, deferred key CapacitySource read the live profile keySet(); the spawn gate reads the frozen set
#400 (receipt, same harm) measured the attempt, not the effect
#424 frozen read, hot key MemberRegistry freezes fleet.architects, so revoking an architect slot does not revoke it
#425 frozen read, hot key fleet_profiles reports a boot-time default while placement reads the live pool

Every one was found by a person reading code. Not one was found by a test.

Why the existing checkers cannot see it

ConfigRefTopLevelCoverageTest, ConfigRefTopLevelReportingCoverageTest and ConfigRefProfileCoverageTest are good tests and they all check the same thing: that ConfigRef's COLD_KEYS / SPLIT_KEYS / DEFERRED_KEYS sets are internally consistent with FleetConfig's record shape, and that each set member has a reporting branch.

None of them looks at a consumer. So they catch a key nobody triaged, and they cannot catch a consumer that disagrees with the triage. But the triage is a claim about consumers — "hot" means every site re-reads it. ConfigRef.java:29-37 states that claim explicitly for role pools, and #424 proves it false for one consumer while true for another.

fleet.architects turns out to be a split key inside the already-split fleet: key: read live for placement, frozen for identity. That was recorded nowhere, and the reload tells the operator the wrong half out loud (ConfigRef.java:543-551).

Why it is easy to write and impossible to see

In Fleetd.main these two sit three lines apart and look equally innocent:

cfg.profiles()              // frozen — the boot snapshot local from Fleetd.java:121
config.get().profiles()     // live   — through the ConfigRef

#416 was exactly one lambda that mixed both: the live keySet() beside the live maxLoad(), where only maxLoad should have been live. Reviewing that line, "it reads the config" is true and useless. Nothing in the type system, the naming, or the tests distinguishes the two.

The frozen read is also invisible to search. Every one of us has hunted this with grep 'config\.get()', which by construction can only find the live half. The frozen half is a constructor argument or a field, and it does not match any idiom you would think to grep for. That is why #424 and #425 sat unfound while three live-read instances were fixed.

The goal

Make a frozen read of a config value an explicit, named, greppable act, so that "this site is frozen" is a decision somebody wrote down rather than the default that happens when you pass a value.

That is the invariant. Everything below is a candidate, not a requirement — pick your own mechanism, and if you find a better one, say so in your report.

Candidate A — forbid holding config by value

An ArchUnit rule: no class outside the config package may declare a field whose type is FleetConfig or a nested FleetConfig.* record. A long-lived collaborator must hold a Supplier / Function instead, so the live read is the path of least resistance.

Frozen is then opt-in: a site that genuinely needs the boot snapshot declares it and appears in an explicit exception list, each entry naming why. coordinator.peers at Fleetd.java:684-688 is the model of a correct frozen read — it reads the same snapshot the mailbox itself was opened from — and it would be the first, well-documented entry.

This is the strongest candidate because it inverts the default. It is also the most invasive, so measure the blast radius before committing: count the fields that would violate it today and report that number before you change anything. If it is large, say so — that is a result, and it may push toward candidate B.

Candidate B — name the snapshot

Rename Fleetd.main's cfg local to something that says what it is (bootSnapshot, say). Then grep -n 'bootSnapshot\.' Fleetd.java enumerates every frozen read in the file in one command, and a reviewer can check each against DEFERRED_KEYS by eye.

Cheap, no new test, no new infrastructure — and it makes the invisible half greppable, which is the specific thing that let #424 and #425 hide. Weaker than A: it is a convention, and nothing enforces it. But a convention that makes a defect findable in one command is worth more than it looks, and it can ship the same day.

A and B are not exclusive. B is the cheap half of A and can land first.

What this ticket does not ask for

Do not build a checker that hand-maintains a second list of "which consumers read which key". That list would drift from the code exactly the way DEFERRED_KEYS' old test-side copy did before #337 promoted it into ConfigRef — and the reason given there applies with full force here: a second, hand-maintained copy of a set is what silently drifts from the thing it describes. A checker that needs a human to keep it truthful has moved the problem, not solved it.

The test must derive what it checks from the code.

Acceptance

  • Introducing a new frozen read of a hot key fails the build, or shows up in a single documented grep. State which of the two you achieved — do not claim the first if you built the second.
  • Prove it by adding one on a scratch branch and showing the failure. A checker nobody has seen fail is not a checker.
  • Every accepted exception names why it is frozen, in the exception entry itself, not in a commit message.
  • Report the count of sites that violate your rule today, before any fix. That number is the finding, whatever you do about it.
  • mvn clean install green.
  • If your answer is "this cannot be checked mechanically without a hand-maintained list", that is an acceptable and useful result. Say it plainly, show what you tried, and propose the best non-mechanical mitigation instead. Do not build a checker with an escape hatch wide enough to make it green — this repo has already filed that as a defect once (#323), and a checker that passes by excusing its own cases is worse than none, because it makes the next person believe the invariant is enforced.

Notes for whoever takes this

  • ArchUnit is not on the classpath yet. #131 adds it, for package cycles. If that has not landed, either take the dependency here and coordinate with #131, or start with candidate B and leave A to follow. Say which you did.
  • Read ConfigRef's class javadoc end to end first. It is long and it is the actual specification of hot/deferred/cold/split. The four definitions are the contract your rule has to encode.
  • Do not change any key's classification in this PR. #424 and #425 do that for the two keys we know about. This ticket is about the checker.
  • Do not weaken or delete the three existing coverage tests. They check something real; they just check it against the record shape rather than the consumers.
The structural gap behind #404, #416, #400, #424 and #425. Filing it separately because fixing any one of those does nothing about the next one. ## The pattern, five times | ticket | direction | what happened | |---|---|---| | #404 | live read, deferred key | `exhaustionDetectionArmed` read the live config; detection read the startup snapshot | | #416 | live read, deferred key | `CapacitySource` read the live profile `keySet()`; the spawn gate reads the frozen set | | #400 | (receipt, same harm) | measured the attempt, not the effect | | #424 | frozen read, hot key | `MemberRegistry` freezes `fleet.architects`, so revoking an architect slot does not revoke it | | #425 | frozen read, hot key | `fleet_profiles` reports a boot-time `default` while placement reads the live pool | Every one was found by a person reading code. Not one was found by a test. ## Why the existing checkers cannot see it `ConfigRefTopLevelCoverageTest`, `ConfigRefTopLevelReportingCoverageTest` and `ConfigRefProfileCoverageTest` are good tests and they all check the same thing: that `ConfigRef`'s `COLD_KEYS` / `SPLIT_KEYS` / `DEFERRED_KEYS` sets are internally consistent with **`FleetConfig`'s record shape**, and that each set member has a reporting branch. None of them looks at a **consumer**. So they catch a key nobody triaged, and they cannot catch a consumer that disagrees with the triage. But the triage *is a claim about consumers* — "hot" means *every* site re-reads it. `ConfigRef.java:29-37` states that claim explicitly for role pools, and #424 proves it false for one consumer while true for another. `fleet.architects` turns out to be a split key **inside** the already-split `fleet:` key: read live for placement, frozen for identity. That was recorded nowhere, and the reload tells the operator the wrong half out loud (`ConfigRef.java:543-551`). ## Why it is easy to write and impossible to see In `Fleetd.main` these two sit three lines apart and look equally innocent: ```java cfg.profiles() // frozen — the boot snapshot local from Fleetd.java:121 config.get().profiles() // live — through the ConfigRef ``` `#416` was exactly one lambda that mixed both: the live `keySet()` beside the live `maxLoad()`, where only `maxLoad` should have been live. Reviewing that line, "it reads the config" is true and useless. Nothing in the type system, the naming, or the tests distinguishes the two. **The frozen read is also invisible to search.** Every one of us has hunted this with `grep 'config\.get()'`, which by construction can only find the live half. The frozen half is a constructor argument or a field, and it does not match any idiom you would think to grep for. That is why #424 and #425 sat unfound while three live-read instances were fixed. ## The goal **Make a frozen read of a config value an explicit, named, greppable act, so that "this site is frozen" is a decision somebody wrote down rather than the default that happens when you pass a value.** That is the invariant. Everything below is a candidate, not a requirement — pick your own mechanism, and if you find a better one, say so in your report. ### Candidate A — forbid holding config by value An ArchUnit rule: no class outside the `config` package may declare a **field** whose type is `FleetConfig` or a nested `FleetConfig.*` record. A long-lived collaborator must hold a `Supplier` / `Function` instead, so the live read is the path of least resistance. Frozen is then opt-in: a site that genuinely needs the boot snapshot declares it and appears in an explicit exception list, each entry naming why. `coordinator.peers` at `Fleetd.java:684-688` is the model of a *correct* frozen read — it reads the same snapshot the mailbox itself was opened from — and it would be the first, well-documented entry. This is the strongest candidate because it inverts the default. It is also the most invasive, so measure the blast radius before committing: count the fields that would violate it today and report that number before you change anything. If it is large, say so — that is a result, and it may push toward candidate B. ### Candidate B — name the snapshot Rename `Fleetd.main`'s `cfg` local to something that says what it is (`bootSnapshot`, say). Then `grep -n 'bootSnapshot\.' Fleetd.java` enumerates every frozen read in the file in one command, and a reviewer can check each against `DEFERRED_KEYS` by eye. Cheap, no new test, no new infrastructure — and it makes the invisible half greppable, which is the specific thing that let #424 and #425 hide. Weaker than A: it is a convention, and nothing enforces it. But a convention that makes a defect *findable in one command* is worth more than it looks, and it can ship the same day. **A and B are not exclusive.** B is the cheap half of A and can land first. ## What this ticket does not ask for Do **not** build a checker that hand-maintains a second list of "which consumers read which key". That list would drift from the code exactly the way `DEFERRED_KEYS`' old test-side copy did before #337 promoted it into `ConfigRef` — and the reason given there applies with full force here: a second, hand-maintained copy of a set is what silently drifts from the thing it describes. A checker that needs a human to keep it truthful has moved the problem, not solved it. The test must derive what it checks from the code. ## Acceptance - Introducing a new frozen read of a hot key fails the build, or shows up in a single documented grep. State which of the two you achieved — do not claim the first if you built the second. - Prove it by adding one on a scratch branch and showing the failure. A checker nobody has seen fail is not a checker. - Every accepted exception names **why** it is frozen, in the exception entry itself, not in a commit message. - Report the count of sites that violate your rule today, before any fix. That number is the finding, whatever you do about it. - `mvn clean install` green. - If your answer is "this cannot be checked mechanically without a hand-maintained list", **that is an acceptable and useful result.** Say it plainly, show what you tried, and propose the best non-mechanical mitigation instead. Do not build a checker with an escape hatch wide enough to make it green — this repo has already filed that as a defect once (#323), and a checker that passes by excusing its own cases is worse than none, because it makes the next person believe the invariant is enforced. ## Notes for whoever takes this - **ArchUnit is not on the classpath yet.** #131 adds it, for package cycles. If that has not landed, either take the dependency here and coordinate with #131, or start with candidate B and leave A to follow. Say which you did. - Read `ConfigRef`'s class javadoc end to end first. It is long and it is the actual specification of hot/deferred/cold/split. The four definitions are the contract your rule has to encode. - Do not change any key's classification in this PR. #424 and #425 do that for the two keys we know about. This ticket is about the checker. - Do not weaken or delete the three existing coverage tests. They check something real; they just check it against the record shape rather than the consumers.
ltms commented 2026-09-10 23:21:31 +02:00
Author
Owner
Copy Link
Copy Source

A live instance of this ticket's shape, found today. It is not in the code — it is in the config file's own comment, which is the surface an operator actually reads.

What I measured

fleetd/fleetd.yaml, in the comment block directly above the models: key:

# DEFERRED, not hot: a reload reports a models: change as needing a restart.
# Edit-then-reload is a half-change; restart with scripts/redeploy-fleetd.sh.

That is false. The authority is the code:

$ grep -n '"models"' fleetd/src/main/java/dev/ltms/fleet/config/*.java
FleetConfig.java:1728:            "idleSleepGuard", "models");

models is in FleetConfig.KNOWN_TOP_LEVEL_KEYS (:1723-1728) and in neither
ConfigRef.COLD_KEYS (:244, five keys: bind, herdrSocket, memberHerdrSocket, broker,
auth) nor ConfigRef.DEFERRED_KEYS (:270-273, thirteen keys, models absent). So it is hot.
ConfigRef.java:169 says so in words as well: "models: was added as deferred, and again for
fleetd #422, which moved models:" into the hot class.

Control, so a zero match cannot read as a clean pass: the same grep pattern over ConfigRef.java
returns 16 quoted key names, and grep -c hot returns 28 lines. The files were read.

The shipped warning text agrees with the code, not with the config comment —
Fleetd.java:943 tells the operator (models: is hot, no restart needed).

Why this belongs on this ticket rather than its own

This ticket says a key's hot/deferred class is a claim about every consumer and nothing checks it
that way. The models: comment is such a claim, made in the one place an operator is most likely
to read it, and it went stale the moment #422 moved the key. Nothing could have caught it, because
the check this ticket asks for does not exist and would not cover a comment anyway.

The direction of the error is what makes it expensive. The comment tells the operator to
restart when a reload is enough. During a subscription outage — the exact moment someone edits
models: to turn a model off — that sends them to scripts/redeploy-fleetd.sh, which drops every
in-flight ticket and rendezvous. The fix is cheaper than the workaround the comment recommends,
and the operator has no way to know.

This is the "false comment is worse than absent" case in its strongest form: a missing comment
makes the reader investigate, while a wrong one makes them stop investigating with a false
conclusion.

What I did not do

I did not fix it. fleetd.yaml is gitignored, it is the live config, and a write to it was
refused by the operator's command classifier in an earlier session. I did not route around that
refusal. Replacement text for the operator to paste, if they want it:

# HOT since fleetd #422: a reload applies a models: change and it takes effect
# on the next spawn, with no restart. (It was deferred when first added; the
# note saying so was left behind.) Authority: `models` is in
# FleetConfig.KNOWN_TOP_LEVEL_KEYS and in neither ConfigRef.COLD_KEYS nor
# ConfigRef.DEFERRED_KEYS.

What this suggests for this ticket's scope

Whatever check this ticket produces, consider whether it can also cover fleetd.example.yaml —
the committed description of the schema, and the only copy a test can read. If the example file
carries the same hot/deferred annotations as the live file, a test can compare each annotation
against COLD_KEYS/DEFERRED_KEYS/SPLIT_KEYS and fail when they drift. That would not have
caught this exact instance, because the stale text is in the gitignored file. It would stop the
next one from shipping, and it would give the operator a correct copy to compare against.

I am not widening the scope by decision — flagging it, because a checker that reads only Java
would have declared this clean.

A live instance of this ticket's shape, found today. It is not in the code — it is in the config file's own comment, which is the surface an operator actually reads. ## What I measured `fleetd/fleetd.yaml`, in the comment block directly above the `models:` key: ``` # DEFERRED, not hot: a reload reports a models: change as needing a restart. # Edit-then-reload is a half-change; restart with scripts/redeploy-fleetd.sh. ``` That is false. The authority is the code: ``` $ grep -n '"models"' fleetd/src/main/java/dev/ltms/fleet/config/*.java FleetConfig.java:1728: "idleSleepGuard", "models"); ``` `models` is in `FleetConfig.KNOWN_TOP_LEVEL_KEYS` (:1723-1728) and in **neither** `ConfigRef.COLD_KEYS` (:244, five keys: `bind`, `herdrSocket`, `memberHerdrSocket`, `broker`, `auth`) nor `ConfigRef.DEFERRED_KEYS` (:270-273, thirteen keys, `models` absent). So it is hot. `ConfigRef.java:169` says so in words as well: *"`models:` was added as deferred, and again for fleetd #422, which moved `models:`"* into the hot class. Control, so a zero match cannot read as a clean pass: the same grep pattern over `ConfigRef.java` returns 16 quoted key names, and `grep -c hot` returns 28 lines. The files were read. The shipped warning text agrees with the code, not with the config comment — `Fleetd.java:943` tells the operator `(models: is hot, no restart needed)`. ## Why this belongs on this ticket rather than its own This ticket says a key's hot/deferred class is a claim about every consumer and nothing checks it that way. The `models:` comment is such a claim, made in the one place an operator is most likely to read it, and it went stale the moment #422 moved the key. Nothing could have caught it, because the check this ticket asks for does not exist and would not cover a comment anyway. The direction of the error is what makes it expensive. The comment tells the operator to **restart** when a reload is enough. During a subscription outage — the exact moment someone edits `models:` to turn a model off — that sends them to `scripts/redeploy-fleetd.sh`, which drops every in-flight ticket and rendezvous. The fix is cheaper than the workaround the comment recommends, and the operator has no way to know. This is the "false comment is worse than absent" case in its strongest form: a missing comment makes the reader investigate, while a wrong one makes them stop investigating with a false conclusion. ## What I did not do I did not fix it. `fleetd.yaml` is gitignored, it is the live config, and a write to it was refused by the operator's command classifier in an earlier session. I did not route around that refusal. Replacement text for the operator to paste, if they want it: ``` # HOT since fleetd #422: a reload applies a models: change and it takes effect # on the next spawn, with no restart. (It was deferred when first added; the # note saying so was left behind.) Authority: `models` is in # FleetConfig.KNOWN_TOP_LEVEL_KEYS and in neither ConfigRef.COLD_KEYS nor # ConfigRef.DEFERRED_KEYS. ``` ## What this suggests for this ticket's scope Whatever check this ticket produces, consider whether it can also cover `fleetd.example.yaml` — the committed description of the schema, and the only copy a test can read. If the example file carries the same hot/deferred annotations as the live file, a test can compare each annotation against `COLD_KEYS`/`DEFERRED_KEYS`/`SPLIT_KEYS` and fail when they drift. That would not have caught this exact instance, because the stale text is in the gitignored file. It would stop the next one from shipping, and it would give the operator a correct copy to compare against. I am not widening the scope by decision — flagging it, because a checker that reads only Java would have declared this clean.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#427
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?