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

CB-599: hitting the fleet capacity cap returns an opaque 500 "Server Error" — the reason exists only in the daemon log #89

New Issue
Closed
opened 2026-08-16 17:25:27 +02:00 by ltms · 1 comment
ltms commented 2026-08-16 17:25:27 +02:00
Owner
Copy Link
Copy Source

Hit live by the lead while orchestrating, 2026-08-16. Not a hypothetical.

What happened

I asked for a member on profile sonnet. The fleet already had three. The daemon refused, correctly:

POST /members {"profile":"sonnet","worktree":true,"ticket":"cb597"}
HTTP/1.1 500 Server Error
Content-Type: text/plain
Content-Length: 12

Server Error

The refusal itself is right — maxLoad did its job. What is wrong is that the caller is told nothing. The actual reason was only in bridged/bridged.out:

dev.ltms.bridged.placement.PlacementException: worker profile 'sonnet' is at maxLoad: 3 live >= 3 cap;
    refusing spawn — no fallback to another profile

That message is excellent. It names the profile, the live count, the cap, and even that no fallback was tried. None of it reaches the caller.

I then guessed local and got the same opaque 500, spending a second round-trip to learn a second thing the daemon already knew. A lead with no shell access to the log file cannot diagnose this at all.

Root cause

PlacementException extends IllegalStateException (placement/PlacementException.java:7).

Both spawn paths catch a fixed list of types, and IllegalStateException is not on either:

  • rest/BridgedApp.java:293-299 catches GuardException (403), IllegalArgumentException (400), PeerUnreachableException (502).
  • mcp/BridgeMcp.java:693-698 catches the same three.

So PlacementException escapes to Javalin's default handler, which renders a bare 500 Server Error with a text/plain body. Every other failure on this endpoint returns a structured {"error": ..., "detail": ...} JSON body; this one alone does not.

Note it is a near miss, not an oversight of the whole class: the IllegalArgumentException catch immediately above maps to unknown_profile. Had PlacementException been declared as an IllegalArgumentException it would have surfaced its message already — with a misleading error code, but at least visibly.

Why this belongs in 1.1

Capacity refusal is not an edge case. It is the expected answer whenever a lead delegates enough work, which is exactly what this project tells leads to do — "delegate by default" is the documented policy. So the most routine limit in the system reports as an internal server error.

Worse, 500 reads as "the daemon is broken". The correct reading is "the fleet is full, wait or pick another profile" — a completely benign condition needing no action. That mismatch will send someone to restart a healthy daemon.

This is single-host and would be broken with exactly one host, so it is 1.1 by the admission rule.

Fix

Catch PlacementException on both paths and return its message in the standard structured shape. Catch it before any broader handler, and keep it distinct from unknown_profile — "this profile does not exist" and "this profile is full" need different reactions from the caller.

Status code: 503 is the better fit than 500 or 400. The request was valid and will likely succeed later, which is exactly what 503 means. 409 is defensible; 400 is not, because the caller did nothing wrong. Whatever is chosen, use the same code on both surfaces.

Also check the other PlacementException throw sites are covered by the same mapping — several carry equally useful messages that are currently lost the same way:

  • quarantined profile, naming the remaining cooldown (member/CompositePeerLauncher.java:354)
  • weight 0 (excluded ...) (placement/FixedPlacementPolicy.java:42)
  • all worker profiles are at maxLoad / all ... quarantined / all ... unreachable (placement/PlacementPolicyUtil.java:72-78)

That last group matters most: those distinguish "wait a moment" from "your backends are gone", and today all three arrive as the same blank 500.

Acceptance criteria

  1. A spawn refused for capacity returns a structured body carrying the exception's message, on both REST and MCP.
  2. The status code is not 500, is the same on both paths, and is documented.
  3. A quarantined-profile refusal and an all-profiles-exhausted refusal also surface their messages.
  4. "Profile does not exist" stays distinguishable from "profile is full".
  5. Tests cover at least the at-cap case on both surfaces, asserting the caller can read the reason.
  6. mvn -f bridged/pom.xml clean install green, run unpiped.

Note for whoever picks this up

The messages are already written and already good. This ticket is almost entirely about delivering them, not composing them. Resist rewording the log messages while you are in there.

Hit live by the lead while orchestrating, 2026-08-16. Not a hypothetical. ## What happened I asked for a member on profile `sonnet`. The fleet already had three. The daemon refused, correctly: ``` POST /members {"profile":"sonnet","worktree":true,"ticket":"cb597"} HTTP/1.1 500 Server Error Content-Type: text/plain Content-Length: 12 Server Error ``` The refusal itself is right — `maxLoad` did its job. What is wrong is that the caller is told nothing. The actual reason was only in `bridged/bridged.out`: ``` dev.ltms.bridged.placement.PlacementException: worker profile 'sonnet' is at maxLoad: 3 live >= 3 cap; refusing spawn — no fallback to another profile ``` That message is excellent. It names the profile, the live count, the cap, and even that no fallback was tried. None of it reaches the caller. I then guessed `local` and got the same opaque 500, spending a second round-trip to learn a second thing the daemon already knew. A lead with no shell access to the log file cannot diagnose this at all. ## Root cause `PlacementException extends IllegalStateException` (`placement/PlacementException.java:7`). Both spawn paths catch a fixed list of types, and `IllegalStateException` is not on either: - `rest/BridgedApp.java:293-299` catches `GuardException` (403), `IllegalArgumentException` (400), `PeerUnreachableException` (502). - `mcp/BridgeMcp.java:693-698` catches the same three. So `PlacementException` escapes to Javalin's default handler, which renders a bare `500 Server Error` with a `text/plain` body. Every other failure on this endpoint returns a structured `{"error": ..., "detail": ...}` JSON body; this one alone does not. Note it is a near miss, not an oversight of the whole class: the `IllegalArgumentException` catch immediately above maps to `unknown_profile`. Had `PlacementException` been declared as an `IllegalArgumentException` it would have surfaced its message already — with a misleading error code, but at least visibly. ## Why this belongs in 1.1 Capacity refusal is not an edge case. It is the *expected* answer whenever a lead delegates enough work, which is exactly what this project tells leads to do — "delegate by default" is the documented policy. So the most routine limit in the system reports as an internal server error. Worse, 500 reads as "the daemon is broken". The correct reading is "the fleet is full, wait or pick another profile" — a completely benign condition needing no action. That mismatch will send someone to restart a healthy daemon. This is single-host and would be broken with exactly one host, so it is 1.1 by the admission rule. ## Fix Catch `PlacementException` on both paths and return its message in the standard structured shape. Catch it **before** any broader handler, and keep it distinct from `unknown_profile` — "this profile does not exist" and "this profile is full" need different reactions from the caller. Status code: **503** is the better fit than 500 or 400. The request was valid and will likely succeed later, which is exactly what 503 means. 409 is defensible; 400 is not, because the caller did nothing wrong. Whatever is chosen, use the same code on both surfaces. Also check the other `PlacementException` throw sites are covered by the same mapping — several carry equally useful messages that are currently lost the same way: - quarantined profile, naming the remaining cooldown (`member/CompositePeerLauncher.java:354`) - `weight 0 (excluded ...)` (`placement/FixedPlacementPolicy.java:42`) - `all worker profiles are at maxLoad` / `all ... quarantined` / `all ... unreachable` (`placement/PlacementPolicyUtil.java:72-78`) That last group matters most: those distinguish "wait a moment" from "your backends are gone", and today all three arrive as the same blank 500. ## Acceptance criteria 1. A spawn refused for capacity returns a structured body carrying the exception's message, on **both** REST and MCP. 2. The status code is not 500, is the same on both paths, and is documented. 3. A quarantined-profile refusal and an all-profiles-exhausted refusal also surface their messages. 4. "Profile does not exist" stays distinguishable from "profile is full". 5. Tests cover at least the at-cap case on both surfaces, asserting the caller can read the reason. 6. `mvn -f bridged/pom.xml clean install` green, run unpiped. ## Note for whoever picks this up The messages are already written and already good. This ticket is almost entirely about *delivering* them, not composing them. Resist rewording the log messages while you are in there.
ltms added this to the 1.1 — single-host close-out milestone 2026-08-16 17:25:27 +02:00
ltms commented 2026-08-16 17:54:57 +02:00
Author
Owner
Copy Link
Copy Source

Delivered and merged to main as PR #93.

Both spawn paths now catch PlacementException. REST returns 503 with {"error":"no_capacity","detail":...}; MCP returns the same reason in the isError shape it already uses for every other spawn failure. 503 rather than 400 because the request was valid and will likely succeed later — the caller did nothing wrong.

The implementer confirmed something the ticket only suspected: on the MCP path the exception was not merely mapped to the wrong code, it was never caught at all. So that surface was worse off than REST, not equally off.

Every other throw site funnels through the same type, so one catch covers them all — the quarantine cooldown message, weight 0 (excluded ...), and the all-at-cap / all-quarantined / all-unreachable trio. That last group is the real win: those three distinguish "wait a moment" from "your backends are gone", and all three used to arrive as the identical blank 500.

Both new tests assert the caller can read the reason, not just that the status code changed. That distinction was in the acceptance criteria for a reason — a test that only checks 503 would pass against an empty body and prove nothing.

Verified by the lead: mvn -f bridged/pom.xml clean install unpiped — Tests run: 824, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS.

No exception message was reworded, as asked. The messages were already good; they just never reached anyone.

Delivered and merged to `main` as PR #93. Both spawn paths now catch `PlacementException`. REST returns **503** with `{"error":"no_capacity","detail":...}`; MCP returns the same reason in the `isError` shape it already uses for every other spawn failure. 503 rather than 400 because the request was valid and will likely succeed later — the caller did nothing wrong. The implementer confirmed something the ticket only suspected: on the MCP path the exception was not merely mapped to the wrong code, it was **never caught at all**. So that surface was worse off than REST, not equally off. Every other throw site funnels through the same type, so one catch covers them all — the quarantine cooldown message, `weight 0 (excluded ...)`, and the all-at-cap / all-quarantined / all-unreachable trio. That last group is the real win: those three distinguish "wait a moment" from "your backends are gone", and all three used to arrive as the identical blank 500. Both new tests assert the caller can read the **reason**, not just that the status code changed. That distinction was in the acceptance criteria for a reason — a test that only checks 503 would pass against an empty body and prove nothing. Verified by the lead: `mvn -f bridged/pom.xml clean install` unpiped — Tests run: 824, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS. No exception message was reworded, as asked. The messages were already good; they just never reached anyone.
ltms closed this issue 2026-08-16 17:55:05 +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 1.1 — single-host close-out
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#89
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?