From 738d34a609b12d04bafb0e6f80d1871fcc91aa40 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 17:08:44 +0700 Subject: [PATCH] fleetd #556 rework: pin registration on the #553 finally backstop path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comment 17009: there are TWO registrar.register(target, sent.token()) call sites in Injector's delivery method — the ordinary path inside `if (sent != null)`, and the fleetd #553 finally backstop, reached only when an earlier block throws before the ordinary path ever runs. The lead's mutation on Injector.java:703 (the backstop call) survived the full suite: the existing #553 regression test for this exact scenario (aRuntimeExceptionFromOnTurnCompleteStillCompletesTheNextDelivery) asserts only that the delivered future completes, never that the turn is registered with CompletionResolver — so a redesign that dropped registration from the backstop would reopen this ticket's own defect on precisely the path #553 exists for, with every existing test green. Adds aRuntimeExceptionFromOnTurnCompleteStillLeavesTheNextDeliveryRegisteredOnTheRecoveryPath: drives the same construction as the existing #553 test (onTurnComplete throws for a previous turn, forcing the next turn's delivery down the finally backstop) and additionally asserts the new turn is registered with CompletionResolver and carries the correct waiter — the same assertion the ordinary-path test makes, now made on the recovery path. Proven by mutation: removing Injector.java:703 alone (exact-line anchor 1 -> 0) turns the new test red with its own assertion message; restored and confirmed byte-identical (sha256 8fcb698afccc254b0c99d3a4bf9e960c0e7c85e542024e870c1e815dce6d62a9, matching the pre-mutation tree); re-run green as a control. Full suite after restore: 1750 tests, 0 failures, 0 errors, 0 skipped (Maven's own summary and an independent sum over surefire-reports/*.txt agree), BUILD SUCCESS. --- .../dev/ltms/fleet/inject/InjectorTest.java | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/fleetd/src/test/java/dev/ltms/fleet/inject/InjectorTest.java b/fleetd/src/test/java/dev/ltms/fleet/inject/InjectorTest.java index e55e06e..4efaf91 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/inject/InjectorTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/inject/InjectorTest.java @@ -1310,4 +1310,53 @@ class InjectorTest { + "is the CB-116 cross-turn stale reply fleetd #553 fixed, and this redesign " + "must not reopen it"); } + + @Test + void aRuntimeExceptionFromOnTurnCompleteStillLeavesTheNextDeliveryRegisteredOnTheRecoveryPath() { + // fleetd #556 rework (PR #566, comment 17009): there are TWO registrar.register call sites + // in the delivery method — the ordinary path inside `if (sent != null)`, and the fleetd + // #553 `finally` backstop, reached only when an earlier block (here, onTurnComplete for the + // PREVIOUS turn) throws before the ordinary path ever runs. The existing #553 regression + // test for this exact scenario + // (aRuntimeExceptionFromOnTurnCompleteStillCompletesTheNextDelivery, above) asserts only + // that "second"'s DELIVERED FUTURE completes — never that "second" is REGISTERED with + // CompletionResolver — so a redesign that dropped registrar.register() from the backstop + // passed every existing test while reopening this ticket's own defect on the one path + // fleetd #553 exists for. This test closes that gap: on the recovery path, the backstop is + // the ONLY thing that registers "second", so its waiter must still be resolvable afterward. + RuntimeException boom = new RuntimeException("fleetd #556 rework: boom from onTurnComplete"); + TurnListener throwing = new TurnListener() { + @Override + public void onTurnComplete(String target) { + throw boom; + } + }; + Rendezvous rendezvous = new Rendezvous(); + CompletionResolver completion = new CompletionResolver(new AgentControl(herdr), rendezvous, + ExhaustedPatternLookup.none(), ExhaustionSink.none()); + Injector inj = new Injector(new AgentControl(herdr), throwing, _ -> true, _ -> { + }, completion::register); + + inj.enqueue(T, "first", TestTurnTokens.inert(T)); + CompletableFuture secondWaiter = rendezvous.open(T); + inj.enqueue(T, "second", new TurnToken(T, secondWaiter)); + + inj.onStatus(T, AgentStatus.IDLE); // delivers "first" (inert token — nothing to register) + inj.onStatus(T, AgentStatus.WORKING); // picked up + + RuntimeException thrown = assertThrows(RuntimeException.class, + // "first" completes (onTurnComplete throws) before "second"'s ordinary delivery + // path ever runs; only the finally backstop is left to register "second". + () -> inj.onStatus(T, AgentStatus.IDLE), + "the throwable from onTurnComplete must still escape onStatus"); + assertSame(boom, thrown); + + CompletionResolver.InFlight inFlight = completion.inFlight(T); + assertNotNull(inFlight, + "fleetd #556: \"second\" must be registered by the fleetd #553 finally backstop " + + "even though onTurnComplete threw for \"first\"'s completion before the " + + "ordinary registration path ever ran for \"second\""); + assertSame(secondWaiter, inFlight.waiter(), + "the registered entry must carry \"second\"'s own waiter, not some other value"); + } }