diff --git a/fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java b/fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java index 2f9869a..902f9a4 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java +++ b/fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java @@ -408,10 +408,28 @@ public final class MessageService { * as the zero-candidate case does, and let {@link #abandon} apply the eventual recovery * deterministically instead. * + *
{@code content} is required (fleetd #302). Both doors that reach this
+ * method must reject a missing/blank reply the same way, so the check lives here rather than in
+ * either caller: {@code FleetMcp.reply} already refuses a {@code null} content before it ever
+ * calls this method (its own required-arg guard), and no test or production call site anywhere
+ * in the codebase relies on replying with empty content — confirmed by searching every call site
+ * of this method before adding the check, not assumed. Without this guard, a REST {@code
+ * POST /sessions/{id}/reply} whose body omits {@code content} (or a client library that maps a
+ * missing field to {@code ""}) used to reach {@link Rendezvous#resolve} with an empty string,
+ * silently completing the lead's blocking wait with nothing — indistinguishable from a worker
+ * that genuinely replied with nothing, which is worse than a loud failure because it destroys the
+ * information that the reply never arrived.
+ *
+ * @throws IllegalArgumentException if {@code content} is {@code null} or blank — the caller must
+ * report this as a client error (REST: 400 {@code bad_request}) rather than resolve
+ * anything
* @return always {@code true} — the reply resolved a live send, completed a parked ticket, or
* was queued
*/
public boolean reply(String session, String content) {
+ if (content == null || content.isBlank()) {
+ throw new IllegalArgumentException("content is required");
+ }
if (rendezvous.resolve(session, content)) {
count(FleetMetrics.REPLIES, "path", "rendezvous");
return true; // a live send took it — unchanged fast path
diff --git a/fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java b/fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java
index 56a36fc..2541660 100644
--- a/fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java
+++ b/fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java
@@ -684,7 +684,19 @@ public final class FleetApp {
ctx.status(400).json(Map.of("error", "bad_request", "detail", "body must be JSON"));
return;
}
- messages.reply(id, content);
+ // fleetd #302: content is required. `.path("content").asText("")` above turns a missing key
+ // into "" rather than throwing, so without this check an empty/blank reply used to reach
+ // messages.reply(...) and silently resolve the lead's waiter — the same class of bug as the
+ // sibling "content is required" guards on sendMessage/askMessage below, except this one wrote
+ // a WRONG value instead of failing loudly. The check lives in MessageService.reply so both
+ // this door and FleetMcp.reply inherit the same rule; this catch only translates it into the
+ // {error, detail} envelope this file uses everywhere else.
+ try {
+ messages.reply(id, content);
+ } catch (IllegalArgumentException e) {
+ ctx.status(400).json(Map.of("error", "bad_request", "detail", e.getMessage()));
+ return;
+ }
ctx.status(200).json(Map.of("sessionId", id, "delivered", true));
}
diff --git a/fleetd/src/test/java/dev/ltms/fleet/rest/FleetAppTest.java b/fleetd/src/test/java/dev/ltms/fleet/rest/FleetAppTest.java
index dc9166b..a3e7a67 100644
--- a/fleetd/src/test/java/dev/ltms/fleet/rest/FleetAppTest.java
+++ b/fleetd/src/test/java/dev/ltms/fleet/rest/FleetAppTest.java
@@ -532,6 +532,53 @@ class FleetAppTest {
assertEquals("orphan", body.get("replies").get(0).get("content").asText());
}
+ @Test
+ void replyWithMissingContentIsRejectedAndDoesNotResolveTheWaiter() throws Exception {
+ // fleetd #302: `.path("content").asText("")` used to turn a missing "content" key into an
+ // empty string that reached rendezvous.resolve, silently completing the lead's blocking wait
+ // with nothing. Prove the fix two ways: the bad call is rejected with 400, AND the real send
+ // it would have wrongly resolved is still open afterwards — a real reply completes it.
+ FakeHerdr herdr = new FakeHerdr().agentStatus("idle");
+ int port = start(herdr, "http://gx00.gw:8000", Set.of("gx00.gw"));
+
+ var send = java.util.concurrent.CompletableFuture.supplyAsync(() -> {
+ try { return postMessage(port, "{\"content\":\"review this\",\"timeoutMs\":4000}"); }
+ catch (Exception e) { throw new RuntimeException(e); }
+ });
+
+ Thread.sleep(200); // let the background send open its rendezvous waiter
+
+ HttpResponse