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

fleetd #469: one canonical FleetTool set backs registration, authz and charter checks #472

Closed
agent wants to merge 0 commits from worker/469-canonical-tool-names-2a472a-16 into main
pull from: worker/469-canonical-tool-names-2a472a-16
merge into: fleet:main
Conversation 0 Commits - Files Changed -
agent commented 2026-09-10 14:55:47 +02:00
Member
Copy Link
Copy Source

fleetd #469: one canonical FleetTool set backs registration, authz and charter checks.

What was missing

FleetConfig.validateCharters() only checked that a charter key is a role wire name and its text is non-blank -- it never looked at what the text names. #464's CharterToolSurfaceTest compared charter text against the registered tool surface, but wrote its own charter into a @TempDir fixture, so nothing anyone wrote into the live fleetd.yaml could ever fail it.

What changed

  • New dev.ltms.fleet.mcp.FleetTool enum: the one canonical set of registered tool wire names (fleet_send, fleet_reply, ... 11 total).
  • FleetMcp's tool schemas now derive their names from FleetTool.X.wireName() instead of a literal string.
  • FleetMcp's constructor asserts at startup that what it actually registers with the MCP SDK equals FleetTool.wireNames() exactly -- a mismatch in either direction (a canonical entry never registered, or a registration with no canonical entry) fails the daemon's own boot.
  • FleetMcp.toolAction/authzAction: the wire string is resolved against FleetTool first (still a run-time check by necessity -- a raw wire string can be anything), then authzAction switches on the resolved FleetTool enum itself with no default -- adding a tool to FleetTool without pinning its Authz.Action is now a compile error, not only a test gap. The outer default -> throw behaviour for a genuinely unregistered/garbage tool name is unchanged.
  • New dev.ltms.fleet.mcp.CharterToolSurface.assertChartersNameOnlyRegisteredTools(Map<String,String>): checks a charter's text for fleet_*/bridge_* tokens against FleetTool.wireNames(), throwing IllegalStateException naming both the charter key and the unknown tool. It lives in mcp, not config -- config loads before the MCP server exists and must not gain a dependency on it.
  • Fleetd.main calls it right after cfg.validateAll(), the one seam that already holds both a loaded FleetConfig and the mcp package.
  • CharterToolSurfaceTest, FleetMcpAuthzTest and McpContractDocTest each kept an independent regex scrape of FleetMcp.java's source for the registered side of their own comparison -- three more copies of the same list, nothing tying them together (this predates #469 and was not mentioned in the ticket). All three now read FleetTool.wireNames() instead; the dead scrape helpers/fields were removed.

Acceptance criteria (fleetd #469)

  1. Break-and-restore, live config shape. FleetdStartupValidationTest.mainRefusesACharterNamingAnUnregisteredTool writes a @TempDir config with fleet.charters.dev naming bridge_send (CB-634's own removed name) and asserts Fleetd.main itself throws IllegalStateException containing bridge_send, before opening any socket -- same pattern as the file's five existing mainRefuses* tests.
  2. Proved canonical by removal (see "Mutation proof" below).
  3. Compile-error form: yes, for the authz mapping -- authzAction's switch over FleetTool has no default, so a new FleetTool constant with no case fails mvn compile. The outer wire-name resolution (FleetTool.byWireName) stays a run-time Optional/throw by necessity: it consumes an arbitrary string off the wire, which cannot be exhaustive over any closed set.
  4. Tool-name site count. Measured on origin/main before this change (not just the ticket's stated "2 in FleetMcp plus 1 fixture" -- that undercounted): FleetMcp.java's 11 tool("fleet_x", ...) registrations, its 11-case authz switch, and three independent source-text scrapes of FleetMcp.java (CharterToolSurfaceTest, FleetMcpAuthzTest, McpContractDocTest each had their own toolsTheServerRegisters()) = 5 independent inventories that could drift from each other. After: 1 -- the FleetTool enum. Every other site (FleetMcp's registration + authz switch, CharterToolSurface, and all three test files) now reads/derives from that one enum instead of maintaining its own list.
  5. CharterToolSurfaceTest's two original helpers were the starting point: toolsNamedIn (charter-text extraction) is kept as-is -- charters are free-text config, so finding a fleet_*/bridge_* token in one has no source but a scrape, that isn't the duplicated-list problem. toolsTheServerRegisters (the FleetMcp.java source scrape) is replaced by FleetTool.wireNames(). Both original vacuity guards (!named.isEmpty(), !registered.isEmpty()) survive, reworded for the new source.
  6. mvn -B clean test result below.

Mutation proof (criterion 2)

  • Removed FleetTool.ACK while ackTool() still referenced FleetTool.ACK.wireName(): mvn -o compile failed with two cannot find symbol errors (FleetMcp.java:940 in authzAction's switch, FleetMcp.java:1898 in ackTool()). Reverted.
  • The reverse -- registered a schema under a literal not backed by FleetTool (ackTool() temporarily returned tool("fleet_ack_v2", ...) instead of deriving from the enum): FleetMcp's new startup assertion threw IllegalStateException: fleetd #469: registered MCP tools [..., fleet_ack_v2, ...] do not match the canonical tool set [..., fleet_ack, ...] -- FleetTool is the single source of truth for what this server registers at FleetMcp.<init>, breaking every test in FleetMcpAuthzTest that constructs a real FleetMcp (12 errors). Reverted.

Build

mvn -B clean test
[INFO] Tests run: 1612, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS

Exit code: 0 (run unpiped, redirected to a file).

Out of scope (noted, not investigated further)

  • Authz.Action itself has no equivalent canonical-set/compile-checked tie between its own enum constants and the role-permission table FleetMcpAuthzTest exercises -- same "list written twice" shape, different pair of lists. Not touched here.
  • MemberRole (ARCHITECT/DEV/REVIEWER) has its own wireName() pattern already (used by FleetConfig.validateCharters()); it is a second, independently-canonical enum of the same shape as FleetTool, not a duplicate of it -- flagging only because the shape recurs, not because it needs a fix.

Hazards observed / respected

  • fleetd/fleetd.yaml is gitignored and was not read; all fixtures use @TempDir and reproduce the live YAML shape rather than reasoning about the real file.
  • Did not run the wiki-sync check (wiki/ is uninitialized in this worktree) -- not attempted, not claimed as passed.
  • No .mcp.json/opencode.json/.autoenv/wiki/ files touched or committed.
fleetd #469: one canonical FleetTool set backs registration, authz and charter checks. ## What was missing FleetConfig.validateCharters() only checked that a charter key is a role wire name and its text is non-blank -- it never looked at what the text names. #464's CharterToolSurfaceTest compared charter text against the registered tool surface, but wrote its own charter into a @TempDir fixture, so nothing anyone wrote into the live fleetd.yaml could ever fail it. ## What changed - New `dev.ltms.fleet.mcp.FleetTool` enum: the one canonical set of registered tool wire names (`fleet_send`, `fleet_reply`, ... 11 total). - `FleetMcp`'s tool schemas now derive their names from `FleetTool.X.wireName()` instead of a literal string. - `FleetMcp`'s constructor asserts at startup that what it actually registers with the MCP SDK equals `FleetTool.wireNames()` exactly -- a mismatch in either direction (a canonical entry never registered, or a registration with no canonical entry) fails the daemon's own boot. - `FleetMcp.toolAction`/`authzAction`: the wire string is resolved against `FleetTool` first (still a run-time check by necessity -- a raw wire string can be anything), then `authzAction` switches on the resolved `FleetTool` enum itself with **no `default`** -- adding a tool to `FleetTool` without pinning its `Authz.Action` is now a compile error, not only a test gap. The outer `default -> throw` behaviour for a genuinely unregistered/garbage tool name is unchanged. - New `dev.ltms.fleet.mcp.CharterToolSurface.assertChartersNameOnlyRegisteredTools(Map<String,String>)`: checks a charter's text for `fleet_*`/`bridge_*` tokens against `FleetTool.wireNames()`, throwing `IllegalStateException` naming both the charter key and the unknown tool. It lives in `mcp`, not `config` -- config loads before the MCP server exists and must not gain a dependency on it. - `Fleetd.main` calls it right after `cfg.validateAll()`, the one seam that already holds both a loaded `FleetConfig` and the `mcp` package. - `CharterToolSurfaceTest`, `FleetMcpAuthzTest` and `McpContractDocTest` each kept an **independent** regex scrape of `FleetMcp.java`'s source for the registered side of their own comparison -- three more copies of the same list, nothing tying them together (this predates #469 and was not mentioned in the ticket). All three now read `FleetTool.wireNames()` instead; the dead scrape helpers/fields were removed. ## Acceptance criteria (fleetd #469) 1. **Break-and-restore, live config shape.** `FleetdStartupValidationTest.mainRefusesACharterNamingAnUnregisteredTool` writes a `@TempDir` config with `fleet.charters.dev` naming `bridge_send` (CB-634's own removed name) and asserts `Fleetd.main` itself throws `IllegalStateException` containing `bridge_send`, before opening any socket -- same pattern as the file's five existing `mainRefuses*` tests. 2. **Proved canonical by removal** (see "Mutation proof" below). 3. **Compile-error form:** yes, for the authz mapping -- `authzAction`'s switch over `FleetTool` has no `default`, so a new `FleetTool` constant with no case fails `mvn compile`. The outer wire-name resolution (`FleetTool.byWireName`) stays a run-time `Optional`/throw by necessity: it consumes an arbitrary string off the wire, which cannot be exhaustive over any closed set. 4. **Tool-name site count.** Measured on `origin/main` before this change (not just the ticket's stated "2 in FleetMcp plus 1 fixture" -- that undercounted): `FleetMcp.java`'s 11 `tool("fleet_x", ...)` registrations, its 11-case authz switch, **and three independent source-text scrapes** of `FleetMcp.java` (`CharterToolSurfaceTest`, `FleetMcpAuthzTest`, `McpContractDocTest` each had their own `toolsTheServerRegisters()`) = 5 independent inventories that could drift from each other. After: **1** -- the `FleetTool` enum. Every other site (`FleetMcp`'s registration + authz switch, `CharterToolSurface`, and all three test files) now reads/derives from that one enum instead of maintaining its own list. 5. `CharterToolSurfaceTest`'s two original helpers were the starting point: `toolsNamedIn` (charter-text extraction) is kept as-is -- charters are free-text config, so finding a `fleet_*`/`bridge_*` token in one has no source but a scrape, that isn't the duplicated-list problem. `toolsTheServerRegisters` (the FleetMcp.java source scrape) is replaced by `FleetTool.wireNames()`. Both original vacuity guards (`!named.isEmpty()`, `!registered.isEmpty()`) survive, reworded for the new source. 6. `mvn -B clean test` result below. ## Mutation proof (criterion 2) - **Removed `FleetTool.ACK` while `ackTool()` still referenced `FleetTool.ACK.wireName()`:** `mvn -o compile` failed with two `cannot find symbol` errors (`FleetMcp.java:940` in `authzAction`'s switch, `FleetMcp.java:1898` in `ackTool()`). Reverted. - **The reverse -- registered a schema under a literal not backed by `FleetTool`** (`ackTool()` temporarily returned `tool("fleet_ack_v2", ...)` instead of deriving from the enum): `FleetMcp`'s new startup assertion threw `IllegalStateException: fleetd #469: registered MCP tools [..., fleet_ack_v2, ...] do not match the canonical tool set [..., fleet_ack, ...] -- FleetTool is the single source of truth for what this server registers` at `FleetMcp.<init>`, breaking every test in `FleetMcpAuthzTest` that constructs a real `FleetMcp` (12 errors). Reverted. ## Build ``` mvn -B clean test [INFO] Tests run: 1612, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ``` Exit code: 0 (run unpiped, redirected to a file). ## Out of scope (noted, not investigated further) - `Authz.Action` itself has no equivalent canonical-set/compile-checked tie between its own enum constants and the role-permission table `FleetMcpAuthzTest` exercises -- same "list written twice" shape, different pair of lists. Not touched here. - `MemberRole` (`ARCHITECT`/`DEV`/`REVIEWER`) has its own `wireName()` pattern already (used by `FleetConfig.validateCharters()`); it is a second, independently-canonical enum of the same *shape* as `FleetTool`, not a duplicate of it -- flagging only because the shape recurs, not because it needs a fix. ## Hazards observed / respected - `fleetd/fleetd.yaml` is gitignored and was not read; all fixtures use `@TempDir` and reproduce the live YAML shape rather than reasoning about the real file. - Did not run the wiki-sync check (`wiki/` is uninitialized in this worktree) -- not attempted, not claimed as passed. - No `.mcp.json`/`opencode.json`/`.autoenv`/`wiki/` files touched or committed.
agent added 2 commits 2026-09-10 14:55:47 +02:00
Merge #470: the exhaustion quarantine escalates instead of retrying flat (fleetd #466) 8b4ff78546 
A flat 30-minute cooldown suits a backend that is out of capacity for the
hour. It does not suit a weekly subscription limit: that keeps reporting
exhausted until the window resets, so the daemon retried it roughly 336
times across a week and learned nothing each time.

BackendQuarantine now tracks, per credential, how many consecutive
exhaustion reports it has seen with no quiet gap between them, and doubles
the cooldown each time, capped at 12x the base (about 6 hours at the 1800s
default). That is about a dozen attempts a week instead of ~336.

Three things I want on the record because they are judgement calls, not
facts:

- The reset is a TIME PROXY, not a success signal. Nothing in this
  codebase reports a spawn success back to this class, so "it started
  working again" cannot be observed here. A base cooldown of quiet is the
  best available evidence. The class doc says this plainly rather than
  implying the stronger thing.
- No automatic probing. That was the operator's design constraint and the
  implementation respects it: the daemon warns and waits, it never pokes a
  limited backend to see whether the limit lifted.
- The multiplier (2.0) and ceiling (12x) are constants, not config surface,
  so no new ConfigRef hot/cold/deferred classification question arises.
  quarantineCooldownSeconds stays Deferred and is now the BASE of the
  backoff; fleetd.example.yaml and FleetConfig's javadoc say so.

Escalation fires on the exhaustion signal alone. Cooling-off
(BackendOutagePolicy, a flat 60s after repeated non-exhaustion errors) is a
separate mechanism and is deliberately NOT escalated: doing so would turn a
transient 5xx storm into a multi-hour outage.

The old two-argument constructor is behaviourally unchanged - it is the
same formula with multiplier 1.0 and a ceiling equal to the base, which
collapses to the original flat "now + cooldown". Every existing call site
keeps its shape.

Build number and my own mutation results are reported in the PR and on the
ticket, measured on this merge commit rather than on the branch.
fleetd #469: one canonical FleetTool set backs registration, authz and charter checks
CI / contract (pull_request) Successful in 56s
Details
CI / build (pull_request) Successful in 1m37s
Details
6c2d6e93cb 
FleetConfig.validateCharters() only checked that a charter key is a role
wire name and its text is non-blank; #464's CharterToolSurfaceTest compared
charter text against the registered tool surface, but wrote its own charter
into a @TempDir fixture, so nothing anyone wrote into the live fleetd.yaml
could ever fail it.

Add FleetTool, an enum in dev.ltms.fleet.mcp holding the one canonical set
of registered tool wire names. FleetMcp's tool schemas now derive their
names from it, its constructor asserts at startup that what it actually
registers with the SDK equals FleetTool.wireNames() exactly, and its authz
dispatch (toolAction/authzAction) resolves the wire string against FleetTool
before switching on the enum itself with no default -- adding a tool without
pinning its Authz.Action is now a compile error, not just a test gap.

Add CharterToolSurface (mcp package, not config -- config loads before the
MCP server exists) and call it from Fleetd.main right after
cfg.validateAll(), so a charter naming a tool the server does not register
refuses the daemon's startup, naming both the charter key and the unknown
tool. FleetdStartupValidationTest proves this through Fleetd.main itself
against a live-shaped config fixture (bridge_send, CB-634's own removed
name).

CharterToolSurfaceTest, FleetMcpAuthzTest and McpContractDocTest each kept
an independent regex scrape of FleetMcp.java's source for the registered
side of their own comparison -- three more copies of the same list nothing
tied together. All three now read FleetTool.wireNames() instead.

Proved canonical by removal: deleting FleetTool.ACK while ackTool() still
referenced it broke mvn compile in two places (FleetMcp.java:940,:1898);
registering a schema under a literal not backed by FleetTool
("fleet_ack_v2") failed FleetMcp's new startup assertion in every test that
constructs it (12 errors, IllegalStateException at FleetMcp.<init>). Both
reverted before this commit.
ltms closed this pull request 2026-09-10 15:11:30 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 56s
Details
CI / build (pull_request) Successful in 1m37s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
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#472
Delete Branch "worker/469-canonical-tool-names-2a472a-16"

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?