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

The /mcp transport has no test: 57 endpoint tests all enter through REST, the door no agent uses #460

New Issue
Open
opened 2026-09-10 13:08:33 +02:00 by ltms · 1 comment
ltms commented 2026-09-10 13:08:33 +02:00
Owner
Copy Link
Copy Source

What I measured

Run in fleetd/ at main = 3c5873d.

Total test files, as a control that the searches below reached the tree:

$ find src/test/java -name '*Test.java' | wc -l
114

Tests that actually send an HTTP request (build an HttpClient and call .send):

$ grep -rl '\.send(\|sendAsync' src/test/java --include='*Test.java' | xargs grep -l 'HttpClient'
src/test/java/dev/ltms/fleet/rest/FleetAppAuthTest.java
src/test/java/dev/ltms/fleet/rest/FleetAppTest.java
src/test/java/dev/ltms/fleet/rest/FleetAppTwoDaemonTest.java

@Test counts in those three: 11, 37, 9. So 57 tests cross a real socket into a real
FleetApp. That is the only place in the whole suite where anything does.

The /mcp path:

$ grep -rn '"/mcp' src/main/java --include='*.java'
src/main/java/dev/ltms/fleet/rest/FleetApp.java   (1 site)
src/main/java/dev/ltms/fleet/mcp/FleetMcp.java    (1 site)

$ grep -rn '/mcp"' src/test/java --include='*.java'
only OpenCodeLauncherTest — every match asserts a config URL string, none sends a request

And the MCP surface's own test:

$ grep -cE '^\s*@Test\s*$' src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java
85

All 85 build a FleetMcp in memory and call its Java methods directly.

The gap

   production traffic                    what the tests enter through
   ------------------                    ---------------------------
   lead / worker  --MCP--> /mcp  ......  nothing. 0 tests over the wire.
                                         85 in-process calls instead.
   fallback       --REST--> /sessions .  57 tests, real socket, FakeHerdr

Every lead and every member in this fleet talks to the daemon over /mcp. REST is the documented
fallback for when that mount drops. The tests are the other way round: the fallback has all the
transport coverage and the live channel has none.

This is not a claim that the endpoint tests are wasted. They pin HTTP status codes, JSON shape and
the auth behaviour of the REST surface, and 57 of them is real work. The claim is narrower: they
cannot see the MCP surface, and three things I hit this week live there.

  1. The tool-to-permission map is in FleetMcp.java:864
    (case "fleet_status", "fleet_list", ... -> Authz.Action.READ), not in Authz.
    FleetAppAuthTest has no route to it, so the mapping from a tool name to a permission is
    pinned only by in-process tests.
  2. #439: coordinatorView(CoordinationSource) takes no caller argument, so it structurally
    cannot vary by who asked. I checked the REST side and no handler calls coordinatorView or
    heldView, so no endpoint test could ever have caught that disclosure.
  3. FleetAppTest runs against FakeHerdr and FakeWorktrees. In #449 the fake and the one
    real-herdr test disagreed on the herdr protocol number for weeks. The fake happened to be
    right and the contract test had rotted to 14, but nothing in the suite could tell you which
    one was wrong — and the one test that touches reality was excluded from CI by name.

What I am asking for, and what I am not

I am not asking for 85 MCP tests over HTTP. Most of FleetMcpTest is logic that an
in-process call tests just as well, and moving it to a socket would be slower for no gain.

What is missing is a small number of tests that can only pass if the real transport works. The
question to answer first, before writing any: which claims about /mcp are currently believed
and never measured? My candidates, for the implementer to confirm or reject:

  • A tool call arriving over /mcp reaches the same Authz decision that FleetMcpTest asserts
    in process. If the transport layer resolves the caller differently, every in-process authz test
    is testing a caller that never exists.
  • ConnectionIdentity resolves a role from the connection. Over a real socket that resolution
    runs for real (peer pid, lsof). In process it does not run at all, or runs on a stub. This is
    the one where in-process and over-the-wire are most likely to disagree, and it is the one that
    decides whether a worker can act as a lead.
  • The registered tool list on the wire matches the tool list the server thinks it registered.
    McpContractDocTest already checks the docs against the registry; nothing checks the registry
    against what a client can actually call.

Scope

  1. Answer the question above first and write the answer in this ticket, before any code. If the
    honest answer is "the transport cannot disagree, here is why", then close this ticket with
    that reasoning. That is a good outcome, not a failure.
  2. If it can disagree, add the smallest set of tests that speak MCP over HTTP to a started
    FleetApp and cover the claims that survive step 1. Tag them so a selector can pick them by
    tag and never by class name (fleetd #449: a hardcoded class list decides coverage on the day
    it is written).
  3. Do not touch the 57 REST tests. They are not the problem.

Acceptance criteria

  • The ticket says which claims about /mcp were unmeasured, and which of them a new test now
    measures.
  • Every new test is proven live: break the behaviour it claims to pin, paste the failure, restore,
    show green. A new test that passes against both the fixed and the broken code pins nothing.
  • New tests are selected by tag. State the tag and show the command that runs the group.
  • Say whether ConnectionIdentity's real resolution runs in the new tests or is still stubbed. If
    it is still stubbed, say so plainly — the gap is then narrowed, not closed.

Found while answering "are the e2e endpoint tests less relevant now". The answer is no: they are
relevant and they are pointed at the wrong door.

## What I measured Run in `fleetd/` at `main` = `3c5873d`. Total test files, as a control that the searches below reached the tree: ``` $ find src/test/java -name '*Test.java' | wc -l 114 ``` Tests that actually send an HTTP request (build an `HttpClient` **and** call `.send`): ``` $ grep -rl '\.send(\|sendAsync' src/test/java --include='*Test.java' | xargs grep -l 'HttpClient' src/test/java/dev/ltms/fleet/rest/FleetAppAuthTest.java src/test/java/dev/ltms/fleet/rest/FleetAppTest.java src/test/java/dev/ltms/fleet/rest/FleetAppTwoDaemonTest.java ``` `@Test` counts in those three: 11, 37, 9. So 57 tests cross a real socket into a real `FleetApp`. That is the only place in the whole suite where anything does. The `/mcp` path: ``` $ grep -rn '"/mcp' src/main/java --include='*.java' src/main/java/dev/ltms/fleet/rest/FleetApp.java (1 site) src/main/java/dev/ltms/fleet/mcp/FleetMcp.java (1 site) $ grep -rn '/mcp"' src/test/java --include='*.java' only OpenCodeLauncherTest — every match asserts a config URL string, none sends a request ``` And the MCP surface's own test: ``` $ grep -cE '^\s*@Test\s*$' src/test/java/dev/ltms/fleet/mcp/FleetMcpTest.java 85 ``` All 85 build a `FleetMcp` in memory and call its Java methods directly. ## The gap ``` production traffic what the tests enter through ------------------ --------------------------- lead / worker --MCP--> /mcp ...... nothing. 0 tests over the wire. 85 in-process calls instead. fallback --REST--> /sessions . 57 tests, real socket, FakeHerdr ``` Every lead and every member in this fleet talks to the daemon over `/mcp`. REST is the documented fallback for when that mount drops. The tests are the other way round: the fallback has all the transport coverage and the live channel has none. This is not a claim that the endpoint tests are wasted. They pin HTTP status codes, JSON shape and the auth behaviour of the REST surface, and 57 of them is real work. The claim is narrower: **they cannot see the MCP surface, and three things I hit this week live there.** 1. The tool-to-permission map is in `FleetMcp.java:864` (`case "fleet_status", "fleet_list", ... -> Authz.Action.READ`), not in `Authz`. `FleetAppAuthTest` has no route to it, so the mapping from a tool name to a permission is pinned only by in-process tests. 2. #439: `coordinatorView(CoordinationSource)` takes no caller argument, so it structurally cannot vary by who asked. I checked the REST side and no handler calls `coordinatorView` or `heldView`, so no endpoint test could ever have caught that disclosure. 3. `FleetAppTest` runs against `FakeHerdr` and `FakeWorktrees`. In #449 the fake and the one real-herdr test disagreed on the herdr protocol number for weeks. The fake happened to be right and the contract test had rotted to 14, but nothing in the suite could tell you which one was wrong — and the one test that touches reality was excluded from CI by name. ## What I am asking for, and what I am not I am **not** asking for 85 MCP tests over HTTP. Most of `FleetMcpTest` is logic that an in-process call tests just as well, and moving it to a socket would be slower for no gain. What is missing is a small number of tests that can only pass if the real transport works. The question to answer first, before writing any: **which claims about `/mcp` are currently believed and never measured?** My candidates, for the implementer to confirm or reject: - A tool call arriving over `/mcp` reaches the same `Authz` decision that `FleetMcpTest` asserts in process. If the transport layer resolves the caller differently, every in-process authz test is testing a caller that never exists. - `ConnectionIdentity` resolves a role from the connection. Over a real socket that resolution runs for real (peer pid, `lsof`). In process it does not run at all, or runs on a stub. This is the one where in-process and over-the-wire are most likely to disagree, and it is the one that decides whether a worker can act as a lead. - The registered tool list on the wire matches the tool list the server thinks it registered. `McpContractDocTest` already checks the docs against the registry; nothing checks the registry against what a client can actually call. ## Scope 1. Answer the question above first and write the answer in this ticket, before any code. If the honest answer is "the transport cannot disagree, here is why", then close this ticket with that reasoning. That is a good outcome, not a failure. 2. If it can disagree, add the smallest set of tests that speak MCP over HTTP to a started `FleetApp` and cover the claims that survive step 1. Tag them so a selector can pick them by tag and never by class name (fleetd #449: a hardcoded class list decides coverage on the day it is written). 3. Do not touch the 57 REST tests. They are not the problem. ## Acceptance criteria - The ticket says which claims about `/mcp` were unmeasured, and which of them a new test now measures. - Every new test is proven live: break the behaviour it claims to pin, paste the failure, restore, show green. A new test that passes against both the fixed and the broken code pins nothing. - New tests are selected by tag. State the tag and show the command that runs the group. - Say whether `ConnectionIdentity`'s real resolution runs in the new tests or is still stubbed. If it is still stubbed, say so plainly — the gap is then narrowed, not closed. Found while answering "are the e2e endpoint tests less relevant now". The answer is no: they are relevant and they are pointed at the wrong door.
ltms commented 2026-09-10 14:17:04 +02:00
Author
Owner
Copy Link
Copy Source

A concrete, cheap case for this ticket, measured today

#446 round 3 (merged 1fb6176) extracted the quarantine ExhaustionSink out of Fleetd.main into a static factory, Fleetd.exhaustionSink(...), and pinned what that factory logs with a ListAppender on the real logger. That closed the round-2 gap.

It left one mutation alive, and it is exactly this ticket's subject:

M7 — replace main()'s call to the factory with an inert lambda:

-        ExhaustionSink exhaustionSink = exhaustionSink(sessions, config, quarantine,
-                quarantineReasonByCredential, cfg);
+        ExhaustionSink exhaustionSink = (target, reason, profileHint) -> { };

The factory is untouched and its tests still pass. The daemon now quarantines nothing and logs nothing when a backend reports exhaustion. 1600 tests green.

So nothing anywhere proves main() wires that factory. Same for every other collaborator main assembles — this is one instance of the general gap, not a special case.

Two ways to close it, and the cheap one is worth naming

  1. The real answer, which is this ticket: drive fleetd over its real transport so main's wiring is exercised. Expensive, and the reason this ticket is still open.
  2. A cheap partial, available now: a source-reading assertion of the kind FleetMcpAuthzTest already uses for #439. It scrapes the handler declaration out of FleetMcp.java, bounds the scrape between two anchors, asserts a control inside the scraped text so an empty scrape cannot pass, and then pins the argument at the call site. That idiom would kill M7 without starting the daemon.

Its limits should be stated with it, because they are real. A source-reading test proves the text of a call site, not that the call runs. And it goes stale silently unless it fails loudly when its anchor moves — which is worth verifying by renaming the anchor and checking the test fails. I ran that probe on #439's detector (listHandler → listHandlerX, behaviour identical) and it failed loudly, so that particular one is not vacuous. Any new one should get the same probe.

Option 2 is not a substitute for option 1. It converts "nothing notices" into "something notices if the line changes", which for main's wiring is most of the value at a small fraction of the cost.

## A concrete, cheap case for this ticket, measured today #446 round 3 (merged `1fb6176`) extracted the quarantine `ExhaustionSink` out of `Fleetd.main` into a static factory, `Fleetd.exhaustionSink(...)`, and pinned what that factory logs with a `ListAppender` on the real logger. That closed the round-2 gap. It left one mutation alive, and it is exactly this ticket's subject: **M7** — replace `main()`'s call to the factory with an inert lambda: ```java - ExhaustionSink exhaustionSink = exhaustionSink(sessions, config, quarantine, - quarantineReasonByCredential, cfg); + ExhaustionSink exhaustionSink = (target, reason, profileHint) -> { }; ``` The factory is untouched and its tests still pass. The daemon now quarantines nothing and logs nothing when a backend reports exhaustion. **1600 tests green.** So nothing anywhere proves `main()` wires that factory. Same for every other collaborator `main` assembles — this is one instance of the general gap, not a special case. ## Two ways to close it, and the cheap one is worth naming 1. **The real answer, which is this ticket:** drive fleetd over its real transport so `main`'s wiring is exercised. Expensive, and the reason this ticket is still open. 2. **A cheap partial, available now:** a source-reading assertion of the kind `FleetMcpAuthzTest` already uses for #439. It scrapes the handler declaration out of `FleetMcp.java`, bounds the scrape between two anchors, asserts a control inside the scraped text so an empty scrape cannot pass, and then pins the argument at the call site. That idiom would kill M7 without starting the daemon. Its limits should be stated with it, because they are real. A source-reading test proves the *text* of a call site, not that the call runs. And it goes stale silently unless it fails loudly when its anchor moves — which is worth verifying by **renaming the anchor** and checking the test fails. I ran that probe on #439's detector (`listHandler` → `listHandlerX`, behaviour identical) and it failed loudly, so that particular one is not vacuous. Any new one should get the same probe. Option 2 is not a substitute for option 1. It converts "nothing notices" into "something notices if the line changes", which for `main`'s wiring is most of the value at a small fraction of the cost.
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#460
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?