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

#297: REST gaps — HerdrException envelope on /agents,/members; /profiles quarantine+coolingOff #300

Closed
agent wants to merge 0 commits from worker/fix-297-66bd11-7 into main
pull from: worker/fix-297-66bd11-7
merge into: fleet:main
Conversation 0 Commits - Files Changed -
agent commented 2026-09-04 07:14:29 +02:00
Member
Copy Link
Copy Source

Fixes fleetd #297 — three items from the ticket, plus a required report-only paragraph and a shape check.

What changed

Gap 1 — HerdrException now maps to the JSON error envelope on the two read-only endpoints
GET /agents and GET /members wrapped workers.list() (and, for /members, the roster build) in try { ... } catch (HerdrException e) { herdrError(ctx, e); }, reusing the existing herdrError helper — no new error shape. GET /members is the endpoint the file's own comment names as "the out-of-band path a lead falls back to when its MCP mount drops."

Gap 2 — GET /profiles now reports quarantined and coolingOff
FleetApp takes two new constructor parameters, FleetMcp.QuarantineSource and FleetMcp.OutageSource (both are already-public nested records in FleetMcp — no change to FleetMcp itself). In Fleetd.java, the two source objects that used to be built inline as constructor arguments to FleetMcp are now named local variables (quarantineSource, outageSource) and passed to both FleetMcp and the new FleetApp constructor — the same object instances, not two independently-built copies of the same rule (that drift is exactly what fleetd #284 was). profiles()'s loop shape matches FleetMcp.profiles()'s field names/nesting for parity. FleetMcp was not modified.

Item 3 — capacity block on GET /members: NOT added, as instructed.
One paragraph, as the ticket asked: adding it would take (a) an injected FleetMcp.CapacitySource-equivalent (liveCount, maxLoad, configuredProfiles, a clock) — FleetApp has none of these today and would need them threaded in from Fleetd.java the same way quarantineSource/outageSource now are; (b) a LeadSeatSource for the leadSeats key; (c) messages (already a field) for the reclaimable count per FleetMcp.reclaimable; and (d) a decision on shape — fleet_list's capacity rows are keyed by profile, while GET /members today is a session roster, so this is a new top-level array/key, not a field added to each existing row. That shape decision is exactly what the ticket says the lead wants to make before it becomes a compatibility surface, so it was left alone.

Tests (new — first coverage of this area)

FleetAppTest had no test that makes workers.list() fail and hits GET /members/GET /agents, and no quarantine/coolingOff coverage at all (confirmed by grep, matching the ticket's claim). Added:

  • agentsMapsAHerdrFailureToTheJsonErrorEnvelope
  • membersMapsAHerdrFailureToTheJsonErrorEnvelope
  • profilesReportsQuarantineAndCoolingOffFromTheSameSharedSources (puts one profile in both states at once, matching FleetMcpTest's own overlap coverage)
  • an added assertion in the existing profilesEndpointListsConfiguredProfilesAndDefault that the two keys are absent when nothing is wired

Both new failure-injection tests use FakeHerdr's existing .healthy(false) — no changes to FakeHerdr.java were needed, so I did not touch that file (another worker is editing it per the brief).

Mutation proof (each fix reverted in isolation, test re-run, then restored)

Gap 1 — agents(): reverting the try/catch back to a bare call:

org.opentest4j.AssertionFailedError: Server Error ==> expected: <502> but was: <500>
	at dev.ltms.fleet.rest.FleetAppTest.agentsMapsAHerdrFailureToTheJsonErrorEnvelope(FleetAppTest.java:196)
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0

(Uncaught HerdrException reaches Javalin's default handler → bare 500, not the {error, detail} envelope.)

Gap 1 — listMembers(): same mutation, same failure shape:

org.opentest4j.AssertionFailedError: Server Error ==> expected: <502> but was: <500>
	at dev.ltms.fleet.rest.FleetAppTest.membersMapsAHerdrFailureToTheJsonErrorEnvelope(FleetAppTest.java:324)
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0

Gap 2 — profiles(): reverting to the original two-key Map.of(...) body:

org.opentest4j.AssertionFailedError: {"default":"ltms-local","profiles":["ltms-local"]} ==> expected: <true> but was: <false>
	at dev.ltms.fleet.rest.FleetAppTest.profilesReportsQuarantineAndCoolingOffFromTheSameSharedSources(FleetAppTest.java:268)
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0

All three fixes were then restored and the full suite re-run green (see Build below).

Shape check (found, NOT fixed, per the ticket)

Same shape — a rule FleetMcp enforces that FleetApp skips — found in three more places in FleetApp.java (verified by reading both files; independently corroborated by a second read-only pass):

  • POST /members (spawnMember) — no catch (HerdrException e) around sessions.acquire(...); FleetMcp.spawn catches it and returns "herdr error spawning worker: ...", so a herdr failure during a REST spawn escapes as an unhandled exception instead of a JSON error.
  • DELETE /members/{paneId} (stopMember) — no try/catch at all around sessions.release(paneId); FleetMcp.stop catches HerdrException and returns "herdr error stopping ...: ...", so a herdr failure during a REST stop escapes uncaught.
  • POST /sessions/{id}/reply (replyMessage) — reads content with .asText(""), so a request with no content field silently becomes an empty-string reply; FleetMcp.reply rejects null content with "content is required".

Already ruled out per the ticket and not re-reported: sendMessage's missing profileTargetError check, and PrimaryRegistry not being wired into FleetApp.

Build

cd fleetd && mvn clean install

Full suite, after all three fixes were restored:

[INFO] Tests run: 1306, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS

FleetAppTest alone:

[INFO] Tests run: 35, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.778 s -- in dev.ltms.fleet.rest.FleetAppTest

Files changed

  • fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java
  • fleetd/src/main/java/dev/ltms/fleet/Fleetd.java
  • fleetd/src/test/java/dev/ltms/fleet/rest/FleetAppTest.java

FleetMcp.java and FakeHerdr.java are both untouched.

Fixes fleetd #297 — three items from the ticket, plus a required report-only paragraph and a shape check. ## What changed **Gap 1 — `HerdrException` now maps to the JSON error envelope on the two read-only endpoints** `GET /agents` and `GET /members` wrapped `workers.list()` (and, for `/members`, the roster build) in `try { ... } catch (HerdrException e) { herdrError(ctx, e); }`, reusing the existing `herdrError` helper — no new error shape. `GET /members` is the endpoint the file's own comment names as "the out-of-band path a lead falls back to when its MCP mount drops." **Gap 2 — `GET /profiles` now reports `quarantined` and `coolingOff`** `FleetApp` takes two new constructor parameters, `FleetMcp.QuarantineSource` and `FleetMcp.OutageSource` (both are already-public nested records in `FleetMcp` — no change to `FleetMcp` itself). In `Fleetd.java`, the two source objects that used to be built inline as constructor arguments to `FleetMcp` are now named local variables (`quarantineSource`, `outageSource`) and passed to **both** `FleetMcp` and the new `FleetApp` constructor — the same object instances, not two independently-built copies of the same rule (that drift is exactly what fleetd #284 was). `profiles()`'s loop shape matches `FleetMcp.profiles()`'s field names/nesting for parity. `FleetMcp` was not modified. **Item 3 — capacity block on `GET /members`: NOT added, as instructed.** One paragraph, as the ticket asked: adding it would take (a) an injected `FleetMcp.CapacitySource`-equivalent (`liveCount`, `maxLoad`, `configuredProfiles`, a clock) — `FleetApp` has none of these today and would need them threaded in from `Fleetd.java` the same way `quarantineSource`/`outageSource` now are; (b) a `LeadSeatSource` for the `leadSeats` key; (c) `messages` (already a field) for the `reclaimable` count per `FleetMcp.reclaimable`; and (d) a decision on shape — `fleet_list`'s capacity rows are keyed by *profile*, while `GET /members` today is a *session* roster, so this is a new top-level array/key, not a field added to each existing row. That shape decision is exactly what the ticket says the lead wants to make before it becomes a compatibility surface, so it was left alone. ## Tests (new — first coverage of this area) `FleetAppTest` had no test that makes `workers.list()` fail and hits `GET /members`/`GET /agents`, and no quarantine/coolingOff coverage at all (confirmed by grep, matching the ticket's claim). Added: - `agentsMapsAHerdrFailureToTheJsonErrorEnvelope` - `membersMapsAHerdrFailureToTheJsonErrorEnvelope` - `profilesReportsQuarantineAndCoolingOffFromTheSameSharedSources` (puts one profile in both states at once, matching `FleetMcpTest`'s own overlap coverage) - an added assertion in the existing `profilesEndpointListsConfiguredProfilesAndDefault` that the two keys are absent when nothing is wired Both new failure-injection tests use `FakeHerdr`'s existing `.healthy(false)` — no changes to `FakeHerdr.java` were needed, so I did not touch that file (another worker is editing it per the brief). ## Mutation proof (each fix reverted in isolation, test re-run, then restored) **Gap 1 — agents():** reverting the try/catch back to a bare call: ``` org.opentest4j.AssertionFailedError: Server Error ==> expected: <502> but was: <500> at dev.ltms.fleet.rest.FleetAppTest.agentsMapsAHerdrFailureToTheJsonErrorEnvelope(FleetAppTest.java:196) [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 ``` (Uncaught `HerdrException` reaches Javalin's default handler → bare 500, not the `{error, detail}` envelope.) **Gap 1 — listMembers():** same mutation, same failure shape: ``` org.opentest4j.AssertionFailedError: Server Error ==> expected: <502> but was: <500> at dev.ltms.fleet.rest.FleetAppTest.membersMapsAHerdrFailureToTheJsonErrorEnvelope(FleetAppTest.java:324) [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 ``` **Gap 2 — profiles():** reverting to the original two-key `Map.of(...)` body: ``` org.opentest4j.AssertionFailedError: {"default":"ltms-local","profiles":["ltms-local"]} ==> expected: <true> but was: <false> at dev.ltms.fleet.rest.FleetAppTest.profilesReportsQuarantineAndCoolingOffFromTheSameSharedSources(FleetAppTest.java:268) [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 ``` All three fixes were then restored and the full suite re-run green (see Build below). ## Shape check (found, NOT fixed, per the ticket) Same shape — a rule `FleetMcp` enforces that `FleetApp` skips — found in three more places in `FleetApp.java` (verified by reading both files; independently corroborated by a second read-only pass): - `POST /members` (`spawnMember`) — no `catch (HerdrException e)` around `sessions.acquire(...)`; `FleetMcp.spawn` catches it and returns `"herdr error spawning worker: ..."`, so a herdr failure during a REST spawn escapes as an unhandled exception instead of a JSON error. - `DELETE /members/{paneId}` (`stopMember`) — no try/catch at all around `sessions.release(paneId)`; `FleetMcp.stop` catches `HerdrException` and returns `"herdr error stopping ...: ..."`, so a herdr failure during a REST stop escapes uncaught. - `POST /sessions/{id}/reply` (`replyMessage`) — reads `content` with `.asText("")`, so a request with no `content` field silently becomes an empty-string reply; `FleetMcp.reply` rejects `null` content with `"content is required"`. Already ruled out per the ticket and not re-reported: `sendMessage`'s missing `profileTargetError` check, and `PrimaryRegistry` not being wired into `FleetApp`. ## Build ``` cd fleetd && mvn clean install ``` Full suite, after all three fixes were restored: ``` [INFO] Tests run: 1306, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ``` `FleetAppTest` alone: ``` [INFO] Tests run: 35, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.778 s -- in dev.ltms.fleet.rest.FleetAppTest ``` ## Files changed - `fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java` - `fleetd/src/main/java/dev/ltms/fleet/Fleetd.java` - `fleetd/src/test/java/dev/ltms/fleet/rest/FleetAppTest.java` `FleetMcp.java` and `FakeHerdr.java` are both untouched.
agent added 1 commit 2026-09-04 07:14:29 +02:00
#297: map HerdrException on GET /agents and /members; GET /profiles reports quarantine + cool-off
CI / contract (pull_request) Successful in 45s
Details
CI / build (pull_request) Successful in 1m33s
Details
85c90d440a 
Two REST-only visibility gaps, both against the same shared instances FleetMcp
reads (BackendQuarantine/BackendOutagePolicy), never recomputed:

- GET /agents and GET /members let a HerdrException escape uncaught, outside
  the {error, detail} envelope every other failure path in FleetApp uses.
  Both now route through the existing herdrError() helper, matching healthz/
  sessionStatus. GET /members is the endpoint's own comment names as the
  out-of-band path a lead falls back to when its MCP mount drops.
- GET /profiles omitted the two outage states fleet_profiles already reports:
  quarantined (CB-578 stage B) and coolingOff (fleetd #201 Unit 5). FleetApp
  now takes the SAME FleetMcp.QuarantineSource/OutageSource instances Fleetd
  wires into FleetMcp (extracted to local vars in Fleetd.java so both doors
  share one object, not two independently-built copies of the same rule).

FleetMcp itself is unchanged. Item 3 of the ticket (a capacity block on
GET /members) is explicitly out of scope and was not added.
ltms closed this pull request 2026-09-04 07:20:17 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 45s
Details
CI / build (pull_request) Successful in 1m33s
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#300
Delete Branch "worker/fix-297-66bd11-7"

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?