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 3a722a3..04ebc21 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java +++ b/fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java @@ -1317,6 +1317,27 @@ public final class MessageService { return new TaskView(ticket, Phase.FAILED, null, null, detail, null); } + /** + * Test seam only — carries no production behaviour, and nothing in this class calls it; + * {@link #pruneTerminalTickets} still reads {@link Task#completedNanos} directly. + * + *
Reports whether {@code ticket}'s completion hook (the {@code whenComplete} registered in + * {@link Task}'s constructor) has actually run yet. Exists because {@link #poll} can report + * {@link Phase#DONE} for a ticket before that hook fires: {@code CompletableFuture.complete()} + * publishes its result and only afterwards runs dependent actions such as {@code whenComplete} + * (fleetd #399), so a caller that observes the future done via {@link #poll} is not thereby + * guaranteed to also observe {@link Task#completedNanos} stamped. A test that must order both + * events — e.g. before advancing an injected clock past the TTL, to avoid stamping the + * *advanced* time and masking a real eviction bug — waits on this instead of on + * {@link Phase#DONE}. + * + * @return {@code false} for an unknown ticket or one whose completion hook has not run yet + */ + boolean isCompletionStampedForTest(String ticket) { + Task task = tasks.get(ticket); + return task != null && task.completedNanos != null; + } + /** Best-effort live worker status for a pending poll; never throws (a lookup error is just noise). */ private String liveStatus(String target) { try { diff --git a/fleetd/src/test/java/dev/ltms/fleet/msg/MessageServiceTest.java b/fleetd/src/test/java/dev/ltms/fleet/msg/MessageServiceTest.java index 23c9b70..60f1470 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/msg/MessageServiceTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/msg/MessageServiceTest.java @@ -1917,6 +1917,12 @@ class MessageServiceTest { // Only now does it reply. Under the old clock this reply was born already expired. assertTrue(rendezvous.resolve(T, "the long report")); awaitTicketPhaseOn(wiring.service(), slow, MessageService.Phase.DONE); + // #399: same barrier fix as the sibling eviction test, for consistency — this test's + // own assertion happens to survive a late stamp today (the clock is not advanced any + // further between here and the sweep below, so cutoff cannot move past whichever + // value completedNanos ends up stamped with), but DONE is still the wrong thing to + // order on before a sweep that matters for the TTL. + awaitCompletionStamped(wiring.service(), slow); // A second delegation runs pruneTerminalTickets before it returns. wiring.service().sendAsync(T, "an unrelated second task"); @@ -1946,6 +1952,11 @@ class MessageServiceTest { injectDelivery(); assertTrue(rendezvous.resolve(T, "quick result")); awaitTicketPhaseOn(wiring.service(), done, MessageService.Phase.DONE); + // #399: DONE can be observed before the completion hook stamps completedNanos. Wait + // for the real stamp before advancing the clock, or the hook can run late and stamp + // the ADVANCED time — making cutoff = advanced - TTL unreachable and hiding the very + // eviction this test exists to pin. + awaitCompletionStamped(wiring.service(), done); // Nobody collected it, and the TTL has now passed since it FINISHED. clock.addAndGet(MessageService.TICKET_TTL_NANOS + TimeUnit.SECONDS.toNanos(1)); @@ -1990,6 +2001,25 @@ class MessageServiceTest { return view; } + /** + * fleetd #399: waits until {@code ticket}'s completion hook has actually stamped + * {@code completedNanos}, not just until {@link MessageService#poll} reports + * {@link MessageService.Phase#DONE} for it. {@code poll} can observe {@code DONE} the instant + * the task's future resolves, before the {@code whenComplete} hook that stamps the completion + * time has run — {@code CompletableFuture.complete()} publishes its result and only then runs + * dependents. A test that is about to advance an injected clock past the TTL must order itself + * after the stamp, not after {@code DONE}: winning the race the other way stamps the + * *advanced* clock value and can hide a real eviction bug behind a false pass. + */ + private void awaitCompletionStamped(MessageService svc, String ticket) throws Exception { + long deadline = System.currentTimeMillis() + 3000; + while (!svc.isCompletionStampedForTest(ticket)) { + assertTrue(System.currentTimeMillis() < deadline, + "completedNanos for " + ticket + " was never stamped"); + Thread.sleep(5); + } + } + // --- CB-640: fleet health evidence accessors -------------------------------------------- @Test