fleetd #480 Unit E: BLOCKED is not a settled turn boundary #484
@@ -281,11 +281,12 @@ public final class LeadRollover {
|
|||||||
*/
|
*/
|
||||||
private void runRollover(PendingRollover p, FleetConfig.LeadRollover cfg) {
|
private void runRollover(PendingRollover p, FleetConfig.LeadRollover cfg) {
|
||||||
String lead = p.leadTerminal();
|
String lead = p.leadTerminal();
|
||||||
boolean turnSettled = waitUntilInjectable(lead, cfg.turnSettleSeconds());
|
boolean turnSettled = waitUntilAtTurnBoundary(lead, cfg.turnSettleSeconds());
|
||||||
if (!turnSettled) {
|
if (!turnSettled) {
|
||||||
log.warn("lead-rollover: pane {} never went idle within {}s after confirm() — refusing "
|
log.warn("lead-rollover: pane {} did not reach a turn boundary (IDLE or DONE) within {}s "
|
||||||
+ "to send /clear at all; the calling lead's own turn is still live and "
|
+ "after confirm() — refusing to send /clear at all; the calling lead's "
|
||||||
+ "clearing it now would destroy live context (token={})",
|
+ "own turn is still live and clearing it now would destroy live context "
|
||||||
|
+ "(token={})",
|
||||||
lead, cfg.turnSettleSeconds(), p.token());
|
lead, cfg.turnSettleSeconds(), p.token());
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -294,10 +295,10 @@ public final class LeadRollover {
|
|||||||
// /clear is housekeeping, not a delegated turn, and routing it through Injector wedges the
|
// /clear is housekeeping, not a delegated turn, and routing it through Injector wedges the
|
||||||
// pane forever (see this class's javadoc).
|
// pane forever (see this class's javadoc).
|
||||||
agents.send(lead, "/clear");
|
agents.send(lead, "/clear");
|
||||||
boolean clearSettled = waitUntilInjectable(lead, cfg.clearSettleSeconds());
|
boolean clearSettled = waitUntilAtTurnBoundary(lead, cfg.clearSettleSeconds());
|
||||||
if (!clearSettled) {
|
if (!clearSettled) {
|
||||||
log.warn("lead-rollover: pane {} did not become injectable within {}s after /clear — "
|
log.warn("lead-rollover: pane {} did not reach a turn boundary (IDLE or DONE) within {}s "
|
||||||
+ "NOT sending bootstrapText (token={})",
|
+ "after /clear — NOT sending bootstrapText (token={})",
|
||||||
lead, cfg.clearSettleSeconds(), p.token());
|
lead, cfg.clearSettleSeconds(), p.token());
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -350,15 +351,27 @@ public final class LeadRollover {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Poll {@link AgentControl#status} until {@code target} reports an injectable state, bounded by
|
* Poll {@link AgentControl#status} until {@code target} reports a real turn boundary — {@link
|
||||||
* {@code settleSeconds}. Used twice by {@link #runRollover}: once to wait for the CALLING
|
* AgentStatus#IDLE} or {@link AgentStatus#DONE} — bounded by {@code settleSeconds}. Used twice
|
||||||
* turn's own pane to settle (the {@code turnSettleSeconds} gate that makes this correction
|
* by {@link #runRollover}: once to wait for the CALLING turn's own pane to settle (the {@code
|
||||||
* safe), and once to wait for the pane to re-settle after {@code /clear}. A failed status read
|
* turnSettleSeconds} gate that makes this correction safe), and once to wait for the pane to
|
||||||
* degrades to "not yet settled" and is retried on the next poll, the same posture {@code
|
* re-settle after {@code /clear}. A failed status read degrades to "not yet settled" and is
|
||||||
* LeadHeartbeatLoop} and {@code HerdrPeerLauncher}'s readiness gate already take toward an
|
* retried on the next poll, the same posture {@code LeadHeartbeatLoop} and {@code
|
||||||
* unreadable status.
|
* HerdrPeerLauncher}'s readiness gate already take toward an unreadable status.
|
||||||
|
*
|
||||||
|
* <p><strong>Deliberately not {@link AgentStatus#injectable()}.</strong> {@code injectable()}
|
||||||
|
* answers the {@code Injector}'s question — "may I deliver a message without stepping on a live
|
||||||
|
* turn" — and it accepts {@link AgentStatus#BLOCKED} for that purpose, because a pane paused on
|
||||||
|
* an approval prompt is safe to queue a message behind. This class asks a stricter question —
|
||||||
|
* "has the turn actually ended" — and {@code BLOCKED} answers no: it is a live turn that is
|
||||||
|
* merely paused, not one that has finished. Reusing {@code injectable()} here would let both
|
||||||
|
* waits fire into an open approval prompt mid-turn (the first wait would send {@code /clear}
|
||||||
|
* while the lead's own {@code confirm()}-calling turn is still live and paused on a prompt; the
|
||||||
|
* second would send {@code bootstrapText} the same way after {@code /clear}) — exactly the
|
||||||
|
* live-context-destroying failure the {@code turnSettleSeconds} gate exists to prevent. Do not
|
||||||
|
* "simplify" this back to {@code injectable()}.
|
||||||
*/
|
*/
|
||||||
private boolean waitUntilInjectable(String target, int settleSeconds) {
|
private boolean waitUntilAtTurnBoundary(String target, int settleSeconds) {
|
||||||
long deadline = nowMillis.getAsLong() + TimeUnit.SECONDS.toMillis(settleSeconds);
|
long deadline = nowMillis.getAsLong() + TimeUnit.SECONDS.toMillis(settleSeconds);
|
||||||
while (nowMillis.getAsLong() < deadline) {
|
while (nowMillis.getAsLong() < deadline) {
|
||||||
AgentStatus status;
|
AgentStatus status;
|
||||||
@@ -369,7 +382,7 @@ public final class LeadRollover {
|
|||||||
target, e.toString());
|
target, e.toString());
|
||||||
status = null;
|
status = null;
|
||||||
}
|
}
|
||||||
if (status != null && status.injectable()) {
|
if (status == AgentStatus.IDLE || status == AgentStatus.DONE) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
settleSleeper.run();
|
settleSleeper.run();
|
||||||
|
|||||||
@@ -250,6 +250,102 @@ class LeadRolloverTest {
|
|||||||
+ "same live context");
|
+ "same live context");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ---- fleetd #480 Unit E: BLOCKED is not a settled turn boundary ------------------------
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@DisplayName("[UNIT E] the calling lead's turn reporting BLOCKED the whole window is NOT settled — /clear is never sent")
|
||||||
|
void blockedTheWholeTurnSettleWindowSendsNoClearAtAll() throws IOException {
|
||||||
|
FakeHerdr herdr = new FakeHerdr();
|
||||||
|
herdr.agentStatus("blocked"); // the calling lead's own pane — paused on a prompt the whole window
|
||||||
|
Path handover = writeHandover("handover contents");
|
||||||
|
FleetConfig.LeadRollover config =
|
||||||
|
new FleetConfig.LeadRollover(handover.toString(), true, 3600, 1 /*turnSettleSeconds*/, 20, "text");
|
||||||
|
// The clock must ADVANCE across waitUntilAtTurnBoundary's poll loop, or a bounded loop
|
||||||
|
// against a frozen clock never reaches its own deadline.
|
||||||
|
AtomicLong clock = new AtomicLong(1_000);
|
||||||
|
LeadRollover rollover = newRollover(herdr, config, () -> clock.addAndGet(500));
|
||||||
|
|
||||||
|
LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full");
|
||||||
|
LeadRollover.RollDecision decision = rollover.confirm(LEAD, pending.token(), true);
|
||||||
|
|
||||||
|
assertTrue(decision.accepted(), "every synchronous gate should pass; the refusal happens "
|
||||||
|
+ "only inside the deferred continuation, which this test's synchronous runner has "
|
||||||
|
+ "already run to completion by the time confirm() returns");
|
||||||
|
assertEquals(0, promptCallCount(herdr),
|
||||||
|
"BLOCKED means the calling lead's turn is paused on a prompt, not ended — it is NOT "
|
||||||
|
+ "a turn boundary, so /clear must never be sent while the pane sits on an "
|
||||||
|
+ "open prompt (that is exactly what injectable() would wrongly allow, since "
|
||||||
|
+ "it treats BLOCKED as safe to inject into)");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@DisplayName("[UNIT E] a pane that goes BLOCKED after /clear is NOT settled — bootstrapText is never sent")
|
||||||
|
void blockedAfterClearNeverSendsBootstrapText() throws IOException {
|
||||||
|
// Idle until /clear is sent, then permanently blocked (paused on a prompt) — isolates the
|
||||||
|
// SECOND wait (clearSettleSeconds) from the first (turnSettleSeconds), which passes
|
||||||
|
// immediately here.
|
||||||
|
FakeHerdr fake = new FakeHerdr();
|
||||||
|
HerdrClient blocksAfterClear = new HerdrClient() {
|
||||||
|
@Override
|
||||||
|
public JsonNode call(String method, Object params) throws HerdrException {
|
||||||
|
JsonNode result = fake.call(method, params);
|
||||||
|
if ("agent.prompt".equals(method) && String.valueOf(params).contains("/clear")) {
|
||||||
|
fake.agentStatus("blocked");
|
||||||
|
}
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void close() {
|
||||||
|
fake.close();
|
||||||
|
}
|
||||||
|
};
|
||||||
|
Path handover = writeHandover("handover contents");
|
||||||
|
FleetConfig.LeadRollover config =
|
||||||
|
new FleetConfig.LeadRollover(handover.toString(), true, 3600, 20, 1 /*clearSettleSeconds*/, "boot text");
|
||||||
|
AtomicLong clock = new AtomicLong(1_000);
|
||||||
|
LeadRollover rollover = newRollover(blocksAfterClear, config, () -> clock.addAndGet(500));
|
||||||
|
|
||||||
|
LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full");
|
||||||
|
LeadRollover.RollDecision decision = rollover.confirm(LEAD, pending.token(), true);
|
||||||
|
|
||||||
|
assertTrue(decision.accepted(), "every synchronous gate passes; the refusal is logged only, "
|
||||||
|
+ "deep inside the deferred continuation");
|
||||||
|
assertEquals(1, promptCallCount(fake), "exactly one agent.prompt call — the /clear — and "
|
||||||
|
+ "nothing else: a freshly-cleared pane sitting on a prompt must not be typed into");
|
||||||
|
long bootstrapSends = fake.calls.stream()
|
||||||
|
.filter(c -> "agent.prompt".equals(c.method()))
|
||||||
|
.filter(c -> String.valueOf(c.params()).contains("boot text"))
|
||||||
|
.count();
|
||||||
|
assertEquals(0, bootstrapSends,
|
||||||
|
"bootstrapText must never be sent while the freshly-cleared pane reports BLOCKED — "
|
||||||
|
+ "typing into an open prompt after /clear is exactly as destructive as "
|
||||||
|
+ "typing /clear into one");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@DisplayName("[UNIT E] DONE still counts as a settled turn boundary — the fix must not over-tighten to IDLE-only")
|
||||||
|
void doneStatusStillCompletesTheFullRoll() throws IOException {
|
||||||
|
FakeHerdr herdr = new FakeHerdr();
|
||||||
|
herdr.agentStatus("done"); // both waits must accept DONE as much as IDLE
|
||||||
|
Path handover = writeHandover("handover contents");
|
||||||
|
AtomicLong clock = new AtomicLong(1_000);
|
||||||
|
FleetConfig.LeadRollover config = cfg(handover.toString());
|
||||||
|
LeadRollover rollover = newRollover(herdr, config, fixedClock(clock));
|
||||||
|
|
||||||
|
LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full");
|
||||||
|
LeadRollover.RollDecision decision = rollover.confirm(LEAD, pending.token(), true);
|
||||||
|
|
||||||
|
assertTrue(decision.accepted(), "expected approval; got: " + decision.reason() + " / " + decision.detail());
|
||||||
|
var prompts = herdr.calls.stream().filter(c -> "agent.prompt".equals(c.method())).toList();
|
||||||
|
assertEquals(2, prompts.size(),
|
||||||
|
"a pane reporting DONE the whole time must complete the full roll — /clear then "
|
||||||
|
+ "bootstrapText — exactly like IDLE; DONE is a real turn-boundary equivalent "
|
||||||
|
+ "to IDLE (see AgentStatus#DONE), not merely 'injectable'");
|
||||||
|
assertTrue(prompts.get(0).params().toString().contains("/clear"));
|
||||||
|
assertTrue(prompts.get(1).params().toString().contains("read the handover file"));
|
||||||
|
}
|
||||||
|
|
||||||
// ---- Correction 2: only the terminal that opened a request may confirm it -------------
|
// ---- Correction 2: only the terminal that opened a request may confirm it -------------
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
Reference in New Issue
Block a user