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

Nothing checks charter text against the tool surface the server registers #464

New Issue
Closed
opened 2026-09-10 13:43:26 +02:00 by ltms · 3 comments
ltms commented 2026-09-10 13:43:26 +02:00
Owner
Copy Link
Copy Source

The gap

fleet.charters.* is instruction text sent to a member at spawn. It names fleet_* tools. Nothing checks those names against the tools the server actually registers.

Measured on 92a96fc:

  • FleetConfig.validateCharters() (FleetConfig.java:2552) checks exactly two things: that the charter key is a MemberRole wire name, and that the text is not blank. It never reads the text.
  • The repo already has this check for a document. McpContractDocTest reads docs/MCP-Contract.md plus FleetMcp.java's tool registrations and fails if the page names a fleet_* tool the server does not register.
  • There is no equivalent for charter text.

So CB-634's bridge_* -> fleet_* rename could have left a dead tool name inside a live charter with no test going red. A member that follows its charter literally then calls a tool that is not there.

Why this is not hypothetical

A wiki design page (CB-548-Lead-Quorum-Design) reported exactly this defect: that fleet.charters.architect told an architect to call bridge_send.

I could not reproduce it. Measured today in fleetd/fleetd.yaml: bridge_send appears 0 times, any bridge_[a-z] name appears 0 times, fleet_send appears twice. Control: architects appears twice, so the grep was reading the right file. So either it was fixed after the page was drafted, or the page measured wrongly.

That is the argument for the ticket rather than against it. The claim went stale between two drafts of one page and nothing in the build could tell anyone which version was true. The page has been corrected with today's measurement.

The work

A test in the McpContractDocTest shape: read the charter text from config, pull every fleet_[a-z_]+ and bridge_[a-z_]+ token out of it, and fail on any token the server does not register as a tool.

Two things it needs to get right, and both have bitten source-reading tests here before:

  1. A control inside the test. Assert the charter text it read is non-empty and that the registered-tool set is non-empty, so a config the test cannot find fails loudly instead of passing on nothing.
  2. bridge_* must be in the pattern, not just fleet_*. The whole point is catching a name from the old surface, and no bridge_* tool is registered, so any hit is a defect.

Note the config seam: fleetd/fleetd.yaml is gitignored and is the live file, so the test must drive FleetConfig with its own fixture text, not read the operator's file. See the addendum note in CLAUDE.md about config-dependent changes shipping green and inert.

Acceptance criteria

  1. The test fails when a charter names a tool the server does not register. Prove it: put bridge_send into the fixture charter, paste the failure, restore, show green.
  2. The test fails loudly when it can find no charter text and when it can find no registered tools. Prove both the same way.
  3. Whole suite green, totals and exit code checked separately from the output.
## The gap `fleet.charters.*` is instruction text sent to a member at spawn. It names `fleet_*` tools. Nothing checks those names against the tools the server actually registers. Measured on `92a96fc`: - `FleetConfig.validateCharters()` (`FleetConfig.java:2552`) checks exactly two things: that the charter key is a `MemberRole` wire name, and that the text is not blank. It never reads the text. - The repo already has this check for a document. `McpContractDocTest` reads `docs/MCP-Contract.md` plus `FleetMcp.java`'s tool registrations and fails if the page names a `fleet_*` tool the server does not register. - There is no equivalent for charter text. So CB-634's `bridge_*` -> `fleet_*` rename could have left a dead tool name inside a live charter with no test going red. A member that follows its charter literally then calls a tool that is not there. ## Why this is not hypothetical A wiki design page (`CB-548-Lead-Quorum-Design`) reported exactly this defect: that `fleet.charters.architect` told an architect to call `bridge_send`. **I could not reproduce it.** Measured today in `fleetd/fleetd.yaml`: `bridge_send` appears 0 times, any `bridge_[a-z]` name appears 0 times, `fleet_send` appears twice. Control: `architects` appears twice, so the grep was reading the right file. So either it was fixed after the page was drafted, or the page measured wrongly. That is the argument for the ticket rather than against it. The claim went stale between two drafts of one page and nothing in the build could tell anyone which version was true. The page has been corrected with today's measurement. ## The work A test in the `McpContractDocTest` shape: read the charter text from config, pull every `fleet_[a-z_]+` and `bridge_[a-z_]+` token out of it, and fail on any token the server does not register as a tool. Two things it needs to get right, and both have bitten source-reading tests here before: 1. **A control inside the test.** Assert the charter text it read is non-empty and that the registered-tool set is non-empty, so a config the test cannot find fails loudly instead of passing on nothing. 2. **`bridge_*` must be in the pattern, not just `fleet_*`.** The whole point is catching a name from the old surface, and no `bridge_*` tool is registered, so any hit is a defect. Note the config seam: `fleetd/fleetd.yaml` is gitignored and is the live file, so the test must drive `FleetConfig` with its own fixture text, not read the operator's file. See the addendum note in `CLAUDE.md` about config-dependent changes shipping green and inert. ## Acceptance criteria 1. The test fails when a charter names a tool the server does not register. Prove it: put `bridge_send` into the fixture charter, paste the failure, restore, show green. 2. The test fails loudly when it can find no charter text and when it can find no registered tools. Prove both the same way. 3. Whole suite green, totals and exit code checked separately from the output.
ltms commented 2026-09-10 14:11:25 +02:00
Author
Owner
Copy Link
Copy Source

Merged as 49df792. One new test file, 72 lines, no production change.

CharterToolSurfaceTest pulls every fleet_* / bridge_* token out of the configured charters and every tool("fleet_…") FleetMcp registers, then asserts the first set is a subset of the second.

The three criteria I set are met

All cells run on the merge commit, tree c444d4b.

Cell What it changes Result
harness proof selector only, no mutation Tests run: 1, no surefire config error
M1 fixture charter names bridge_send, the tool CB-634 renamed away KILLED
M2 the tool("…") scrape broken so it matches nothing KILLED by the registered.isEmpty() guard
M3 fixture stripped of every tool name KILLED by the named.isEmpty() guard
M4 the server stops registering fleet_reply, which the fixture names KILLED

M2 and M3 are the two vacuity guards, and they fail loudly rather than passing on an empty set. M4 is my own addition and it is the one that mattered: it proves the registered half is read from real production source and is not a second fixture.

The scrape finds 11 registered tools — ack, ask, list, poll, profiles, reply, send, spawn, status, stop, whoami. An independent count of every "fleet_x" literal in FleetMcp.java is also 11, so the scrape is not missing a registration idiom.

What this does not close — and it is this ticket's actual gap

The charter half is a @TempDir fixture the test writes itself. No charter text anyone writes can make this test fail. Measured on the merge:

  • the test mentions fleetd.yaml 0 times;
  • the commit changes 0 production files (against its own merge-base 92a96fc);
  • FleetConfig.validateCharters() still never reads charter text — 0 lines of its body mention a tool name.

So what shipped pins the comparison logic, and works as a rename tripwire for the two tools the fixture happens to name. It does not check the live config, which is the thing this ticket was filed about.

That residue is my fault, not the worker's. The three acceptance criteria I wrote are exactly the three it met. The criteria described a test, when what the gap needs is a production-side check that runs when the daemon loads the real config. Filing that as a follow-up with the layering question named, because it is a design decision rather than a defect: FleetConfig lives in config and the tool surface lives in mcp, so the check needs a seam that does not make the config package depend on the MCP server.

Leaving this ticket open until the follow-up is filed and linked.

A note on my own battery, because it nearly published four false kills

My first run reported rc=1 on all four mutation cells. Read at face value that is four kills. It was zsh: unquoted parameters are not word-split, so mvn -B $scope test handed surefire -Dtest=CharterToolSurfaceTest -DfailIfNoTests=false as a single argument, and the build died with No tests matching pattern "CharterToolSurfaceTest -DfailIfNoTests=false". Zero tests ran in every cell.

The tell was a missing Tests run: line. The rerun proves the harness before any cell is believed — the selector alone must report Tests run: 1 — and every cell now prints its surefire summary line count, so a void cell cannot be mistaken for a kill. This is the same failure as "a control fails together with the mutations when the harness is broken", except here the control passed, because the control cell used an empty scope and so never hit the bug.

Merged as `49df792`. One new test file, 72 lines, no production change. `CharterToolSurfaceTest` pulls every `fleet_*` / `bridge_*` token out of the configured charters and every `tool("fleet_…")` `FleetMcp` registers, then asserts the first set is a subset of the second. ## The three criteria I set are met All cells run on the merge commit, tree `c444d4b`. | Cell | What it changes | Result | |---|---|---| | harness proof | selector only, no mutation | `Tests run: 1`, no surefire config error | | **M1** | fixture charter names `bridge_send`, the tool CB-634 renamed away | **KILLED** | | **M2** | the `tool("…")` scrape broken so it matches nothing | **KILLED** by the `registered.isEmpty()` guard | | **M3** | fixture stripped of every tool name | **KILLED** by the `named.isEmpty()` guard | | **M4** | the server stops registering `fleet_reply`, which the fixture names | **KILLED** | M2 and M3 are the two vacuity guards, and they fail loudly rather than passing on an empty set. **M4 is my own addition and it is the one that mattered**: it proves the *registered* half is read from real production source and is not a second fixture. The scrape finds **11** registered tools — `ack`, `ask`, `list`, `poll`, `profiles`, `reply`, `send`, `spawn`, `status`, `stop`, `whoami`. An independent count of every `"fleet_x"` literal in `FleetMcp.java` is also 11, so the scrape is not missing a registration idiom. ## What this does not close — and it is this ticket's actual gap The charter half is a `@TempDir` fixture the test writes itself. **No charter text anyone writes can make this test fail.** Measured on the merge: - the test mentions `fleetd.yaml` **0** times; - the commit changes **0** production files (against its own merge-base `92a96fc`); - `FleetConfig.validateCharters()` still never reads charter text — **0** lines of its body mention a tool name. So what shipped pins the comparison logic, and works as a rename tripwire for the two tools the fixture happens to name. It does not check the live config, which is the thing this ticket was filed about. **That residue is my fault, not the worker's.** The three acceptance criteria I wrote are exactly the three it met. The criteria described a test, when what the gap needs is a production-side check that runs when the daemon loads the real config. Filing that as a follow-up with the layering question named, because it is a design decision rather than a defect: `FleetConfig` lives in `config` and the tool surface lives in `mcp`, so the check needs a seam that does not make the config package depend on the MCP server. Leaving this ticket open until the follow-up is filed and linked. ## A note on my own battery, because it nearly published four false kills My first run reported `rc=1` on all four mutation cells. Read at face value that is four kills. It was **zsh**: unquoted parameters are not word-split, so `mvn -B $scope test` handed surefire `-Dtest=CharterToolSurfaceTest -DfailIfNoTests=false` as a **single argument**, and the build died with `No tests matching pattern "CharterToolSurfaceTest -DfailIfNoTests=false"`. Zero tests ran in every cell. The tell was a missing `Tests run:` line. The rerun proves the harness before any cell is believed — the selector alone must report `Tests run: 1` — and every cell now prints its surefire summary line count, so a void cell cannot be mistaken for a kill. This is the same failure as "a control fails together with the mutations when the harness is broken", except here the control passed, because the control cell used an empty scope and so never hit the bug.
ltms commented 2026-09-10 14:12:30 +02:00
Author
Owner
Copy Link
Copy Source

Correcting one number in the comment above

I wrote that FleetConfig.validateCharters() has "0 lines of its body mention a tool name". The conclusion is right; the measurement behind it was void. My command was:

sed -n '/static void validateCharters/,/^    }/p' FleetConfig.java | grep -cE 'fleet_|bridge_|tool'

The method is public void validateCharters(), not static. The sed range never matched, so grep -c counted an empty stream and printed 0. A zero from a pattern that matched nothing is not a finding — and I had put a control in that battery, but the control checked a different question.

Re-measured against the real method (FleetConfig.java:2552), with a control that must be non-zero:

body lines total:                  19
body lines naming a tool:           0
CONTROL, lines naming 'charter':    5   (non-zero, so the search ran)

So the answer is genuinely 0, and reading the body confirms it. validateCharters() does exactly two things: it checks the key is a MemberRole wire name, and it checks the text is not blank. It never looks inside the text.

Two more facts the follow-up needs

  • FleetMcp does not import dev.ltms.fleet.config at all, and FleetConfig does not import dev.ltms.fleet.mcp. There is no edge between those two packages in either direction today, so a charter check placed in FleetConfig would create the first one.
  • Tool names are already written in two places inside FleetMcp: 11 tool("fleet_…") registrations, and the authz action switch at :879 with 11 case labels. That switch does have default -> throw new IllegalArgumentException("unregistered tool: " + toolName), so it fails loudly rather than silently defaulting. A charter check would be the third place naming the same set, which is the argument for one canonical set rather than a third scrape.
## Correcting one number in the comment above I wrote that `FleetConfig.validateCharters()` has "0 lines of its body mention a tool name". **The conclusion is right; the measurement behind it was void.** My command was: ```bash sed -n '/static void validateCharters/,/^ }/p' FleetConfig.java | grep -cE 'fleet_|bridge_|tool' ``` The method is `public void validateCharters()`, not `static`. The `sed` range never matched, so `grep -c` counted an empty stream and printed `0`. A zero from a pattern that matched nothing is not a finding — and I had put a control in that battery, but the control checked a different question. Re-measured against the real method (`FleetConfig.java:2552`), with a control that must be non-zero: ``` body lines total: 19 body lines naming a tool: 0 CONTROL, lines naming 'charter': 5 (non-zero, so the search ran) ``` So the answer is genuinely 0, and reading the body confirms it. `validateCharters()` does exactly two things: it checks the key is a `MemberRole` wire name, and it checks the text is not blank. It never looks inside the text. ## Two more facts the follow-up needs - **`FleetMcp` does not import `dev.ltms.fleet.config` at all**, and `FleetConfig` does not import `dev.ltms.fleet.mcp`. There is no edge between those two packages in either direction today, so a charter check placed in `FleetConfig` would create the first one. - **Tool names are already written in two places inside `FleetMcp`**: 11 `tool("fleet_…")` registrations, and the authz action switch at `:879` with 11 case labels. That switch does have `default -> throw new IllegalArgumentException("unregistered tool: " + toolName)`, so it fails loudly rather than silently defaulting. A charter check would be the **third** place naming the same set, which is the argument for one canonical set rather than a third scrape.
ltms commented 2026-09-10 15:36:19 +02:00
Author
Owner
Copy Link
Copy Source

Done, and then hardened twice. Closing.

What landed

  • 49df792 "Merge #464: a test that charter text names only registered tools" — this ticket's own scope.
  • 1477e43 (#469) turned it into a real gate rather than only a test: one canonical tool set in FleetTool with wireNames(), CharterToolSurface.assertChartersNameOnlyRegisteredTools, and a startup assertion in FleetMcp's constructor plus a call in Fleetd.main. A charter naming a dead tool now refuses to boot the daemon, not only to pass a test.
  • #474 is the third step: ConfigRef.reload() runs the same gate, so the bad charter cannot slip into a running daemon either. That one is under verification right now and is not merged yet.

Your three acceptance criteria, checked

1. The test fails when a charter names a tool the server does not register. Yes, and by name. In my #469 battery I inverted the filter so it rejected the registered tools and accepted the unknown ones. Three tests failed: CharterToolSurfaceTest.charterToolSurfaceRejectsAnUnregisteredTool, CharterToolSurfaceTest.charterToolSurfaceAcceptsKnownTools, and FleetdStartupValidationTest.mainRefusesACharterNamingAnUnregisteredTool. The negative case alone cannot catch an inversion, so the positive case had to fail too — it did.

2. It fails loudly when it can find no charter text and when it can find no registered tools. Both guards are in the file: CharterToolSurfaceTest.java:72 asserts the names it pulled out are non-empty, and :75 asserts the registered set is non-empty. The second half is also proven at suite level. I made FleetTool.wireNames() return an empty set and got 5 failures and 12 errors across CharterToolSurfaceTest, FleetMcpAuthzTest and McpContractDocTest — including McpContractDocTest.theCheckActuallyHasSomethingToCheck. Before #469 the same fact was scraped from FleetMcp.java in three separate places, and an empty scrape made two of the three silently pass. One canonical source is what makes an empty surface fail in 17 places instead of passing quietly in two.

3. Whole suite green, totals and exit code read separately. 1613 tests, 0 failures, 0 errors, BUILD SUCCESS, exit 0 on 1477e43's own tree. Measured again today at 1633 on the tree carrying #393, #473 and the #474 candidate.

Two things from this ticket worth keeping

Your note about bridge_send in the wiki was right to file even though you could not reproduce it. You wrote that the argument for the ticket was that the claim went stale between two drafts and nothing in the build could say which draft was true. That is now false in the good way: the build can say. FleetMcp's constructor throws if the registered tools and the canonical set disagree, so the two cannot drift apart silently any more.

Your instruction to put bridge_* in the pattern, not only fleet_*, is load-bearing and is still there — CharterToolSurface.java compiles (fleet_[a-z_]+|bridge_[a-z_]+). No bridge_* tool is registered, so any hit is a defect by construction. That is what makes the check catch a leftover from the CB-634 rename rather than only a typo in a current name.

Done, and then hardened twice. Closing. ## What landed - `49df792` "Merge #464: a test that charter text names only registered tools" — this ticket's own scope. - `1477e43` (#469) turned it into a real gate rather than only a test: one canonical tool set in `FleetTool` with `wireNames()`, `CharterToolSurface.assertChartersNameOnlyRegisteredTools`, and a startup assertion in `FleetMcp`'s constructor plus a call in `Fleetd.main`. A charter naming a dead tool now refuses to boot the daemon, not only to pass a test. - #474 is the third step: `ConfigRef.reload()` runs the same gate, so the bad charter cannot slip into a **running** daemon either. That one is under verification right now and is not merged yet. ## Your three acceptance criteria, checked **1. The test fails when a charter names a tool the server does not register.** Yes, and by name. In my #469 battery I inverted the filter so it rejected the registered tools and accepted the unknown ones. Three tests failed: `CharterToolSurfaceTest.charterToolSurfaceRejectsAnUnregisteredTool`, `CharterToolSurfaceTest.charterToolSurfaceAcceptsKnownTools`, and `FleetdStartupValidationTest.mainRefusesACharterNamingAnUnregisteredTool`. The negative case alone cannot catch an inversion, so the positive case had to fail too — it did. **2. It fails loudly when it can find no charter text and when it can find no registered tools.** Both guards are in the file: `CharterToolSurfaceTest.java:72` asserts the names it pulled out are non-empty, and `:75` asserts the registered set is non-empty. The second half is also proven at suite level. I made `FleetTool.wireNames()` return an empty set and got **5 failures and 12 errors** across `CharterToolSurfaceTest`, `FleetMcpAuthzTest` and `McpContractDocTest` — including `McpContractDocTest.theCheckActuallyHasSomethingToCheck`. Before #469 the same fact was scraped from `FleetMcp.java` in three separate places, and an empty scrape made two of the three silently pass. One canonical source is what makes an empty surface fail in 17 places instead of passing quietly in two. **3. Whole suite green, totals and exit code read separately.** 1613 tests, 0 failures, 0 errors, `BUILD SUCCESS`, exit 0 on `1477e43`'s own tree. Measured again today at **1633** on the tree carrying #393, #473 and the #474 candidate. ## Two things from this ticket worth keeping **Your note about `bridge_send` in the wiki was right to file even though you could not reproduce it.** You wrote that the argument for the ticket was that the claim went stale between two drafts and nothing in the build could say which draft was true. That is now false in the good way: the build can say. `FleetMcp`'s constructor throws if the registered tools and the canonical set disagree, so the two cannot drift apart silently any more. **Your instruction to put `bridge_*` in the pattern, not only `fleet_*`, is load-bearing and is still there** — `CharterToolSurface.java` compiles `(fleet_[a-z_]+|bridge_[a-z_]+)`. No `bridge_*` tool is registered, so any hit is a defect by construction. That is what makes the check catch a leftover from the CB-634 rename rather than only a typo in a current name.
ltms closed this issue 2026-09-10 15:36:25 +02:00
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#464
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?