From f6870464504bee13f4adaa1b24eef2ab5b90ad1a Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 07:45:36 +0700 Subject: [PATCH] fleetd #489 follow-up: fix stale class javadoc, off-by-one nudge count, weak test Three review corrections on top of the previous commit: 1. The class javadoc's four-step continuation list (lines 47-58) was stale. Step 1 said "report an injectable state", but waitUntilAtTurnBoundary's own javadoc excludes BLOCKED - fixed to say IDLE or DONE. Step 3 still described the old plain re-check ("the original, pre-correction wait... still here") - fixed to describe what waitForClearPickupAndSettle actually does: nudge while unpicked-up, then wait for a real WORKING -> IDLE/DONE boundary, releasing rather than wedging if WORKING never shows. 2. PICKUP_GRACE_POLLS=8 bounds the number of consecutive not-yet-picked-up polls, not the number of nudges - the 8th poll releases instead of nudging again, so 8 polls produce 7 nudges. The log.info in the release branch and two javadoc spots said "8 nudges"; fixed all three to state the poll count and the nudge count separately and correctly. Behavior and the constant are unchanged. 3. pickupSeenStopsNudgingAndBootstrapTextIsSent asserted only promptCallCount and sendKeysCallCount, both of which a return-true stub also satisfies. Added an assertion on the already-tracked postClearGetCalls counter (>= 2), which only a real post-/clear poll loop can produce - this is what makes the test fail against a return-true mutant. --- .../dev/ltms/fleet/lead/LeadRollover.java | 48 +++++++++++-------- .../dev/ltms/fleet/lead/LeadRolloverTest.java | 6 +++ 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java b/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java index 07bfd3a..b2054cd 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java +++ b/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java @@ -44,15 +44,18 @@ import java.util.function.Supplier; * {@code continuationRunner} before returning. That continuation is what actually touches the pane, * once the calling turn has ended, in this order: *
    - *
  1. wait for the lead's own pane to report an injectable state — i.e. wait for the very - * {@code confirm()} call that approved this roll to finish its turn — bounded by - * {@code turnSettleSeconds}. If this never happens, nothing else in this list runs: - * no {@code /clear} is ever sent. A lead that never goes idle is a lead still doing - * real work, and clearing it would throw away live context — exactly the failure this - * correction exists to prevent.
    2. + *
  2. wait for the lead's own pane to report a real turn boundary — {@code IDLE} or {@code + * DONE}, never merely {@code BLOCKED} — i.e. wait for the very {@code confirm()} call that + * approved this roll to finish its turn — bounded by {@code turnSettleSeconds}. If + * this never happens, nothing else in this list runs: no {@code /clear} is ever sent. + * A lead that never goes idle is a lead still doing real work, and clearing it would throw + * away live context — exactly the failure this correction exists to prevent.
    3. *
  3. {@code agents.send(lead, "/clear")}
    4. - *
  4. wait again for the pane to report injectable, bounded by {@code clearSettleSeconds} (this - * is the original, pre-correction wait — still here, just no longer the only one)
    5. + *
  5. wait for {@code /clear} to be picked up and settle, bounded by {@code clearSettleSeconds} + * (fleetd #489: no longer a plain re-check of the same boundary — {@code /clear} starts no + * turn of its own, so this instead nudges the submit keystroke while no pickup has been seen, + * then waits for a real {@code WORKING} → {@code IDLE}/{@code DONE} boundary once one has; + * see {@link #waitForClearPickupAndSettle})
    6. *
  6. {@code agents.send(lead, cfg.bootstrapTextFor(p.handoverPath()))}
    7. *
* A {@link #confirm} that returns {@link RollDecision#approved()} therefore means "every gate @@ -112,10 +115,13 @@ public final class LeadRollover { static final long SETTLE_POLL_MS = 250; /** - * How many consecutive not-yet-picked-up polls {@link #waitForClearPickupAndSettle} nudges the - * submit keystroke before releasing rather than wedging the roll — the same constant and the - * same release-not-wedge choice {@link dev.ltms.fleet.inject.Injector} already makes for its own - * post-turn {@code /clear} housekeeping (fleetd #306). + * How many consecutive not-yet-picked-up polls {@link #waitForClearPickupAndSettle} allows + * before releasing rather than wedging the roll — the same constant and the same + * release-not-wedge choice {@link dev.ltms.fleet.inject.Injector} already makes for its own + * post-turn {@code /clear} housekeeping (fleetd #306). This bounds the number of + * consecutive polls, not the number of nudges: the first {@code PICKUP_GRACE_POLLS - 1} + * of those polls each send a nudge, and the {@code PICKUP_GRACE_POLLS}th releases instead of + * nudging again — so 8 polls produce 7 nudges, not 8. */ static final int PICKUP_GRACE_POLLS = 8; @@ -510,13 +516,16 @@ public final class LeadRollover { * @@ -558,8 +567,9 @@ public final class LeadRollover { } if (++idlePollsAwaitingPickup >= PICKUP_GRACE_POLLS) { log.info("lead-rollover: /clear on {} was never observed as WORKING after {} " - + "nudges — releasing rather than wedging the roll", - target, PICKUP_GRACE_POLLS); + + "consecutive IDLE/DONE polls ({} of those were nudged) — " + + "releasing rather than wedging the roll", + target, PICKUP_GRACE_POLLS, PICKUP_GRACE_POLLS - 1); return true; } try { diff --git a/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java b/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java index 94ea58e..6d85796 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java @@ -548,6 +548,12 @@ class LeadRolloverTest { + "still complete the full roll — /clear then bootstrapText"); assertEquals(0, sendKeysCallCount(fake), "once WORKING was observed, nudging must stop " + "immediately — no agent.send_keys call should ever have been needed or sent"); + assertTrue(postClearGetCalls.get() >= 2, "the pane's status must have been polled AGAIN " + + "after the WORKING sample, before bootstrapText was sent — this is what proves " + + "the method actually waited for the WORKING -> IDLE completion boundary instead " + + "of returning as soon as pickup was seen (or worse, without polling at all, as a " + + "stub that just returns true would); got " + postClearGetCalls.get() + + " agent.get call(s) after /clear"); } @Test