CB-115/CB-116: reliable turn completion — status refinement, clean scrape, waiter identity
A 5-turn primary↔worker conversation test (see the e2e harness) surfaced three delegation-channel gaps; this closes them. CB-115 — status + scrape correctness: - AgentStatus gains DONE (herdr's explicit turn-complete marker) so a finished turn is no longer misread as UNKNOWN and left to wedge or false-fail. - StatusRefiner reclassifies content-bearing UNKNOWN samples (StatusPoller wired to it), and the Injector baselines pane content on delivery (TurnListener gains onDelivered) to guard completion against previous-turn misattribution. - CompletionResolver.lastAssistantBlock stops at the first hard TUI boundary, so a scrape returns only the assistant answer — no input box, prompt echo, spinner, tips or warnings. CB-116 — waiter identity (the cross-turn stale reply): - The completion/failure fallback ran on a virtual thread and resolved whichever waiter was currently registered for the session. Since the rendezvous holds one waiter per session and sends serialize, turn N's late completion could land on turn N+1's waiter and deliver turn N's stale scrape as turn N+1's answer. The baseline guard missed it because turn N was resolved by bridge_reply, which never updates the completion baseline. - Fix: capture the exact waiter (and pre-turn baseline) when a turn is delivered, on the poller thread before any next-turn delivery can overwrite it, and resolve THAT waiter — a no-op if it was already resolved. Rendezvous.resolveCompletion/ resolveFailure now take the captured CompletableFuture; currentWaiter exposes the registered one for capture. A late completion for turn N can no longer touch turn N+1's send. Verified: 128 unit tests green (incl. a CB-116 regression asserting a late completion never resolves the next turn's waiter); a re-run of the conversation test passes with turn 5 resolving to its own reply rather than turn 4's text.
This commit is contained in:
Dai Ha
@@ -0,0 +1,43 @@
|
||||
package dev.ltms.bridged.herdr;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
/** Wire mapping and injectability of {@link AgentStatus}, including the CB-115 {@code done} state. */
|
||||
class AgentStatusTest {
|
||||
|
||||
@Test
|
||||
void mapsTheKnownWireStrings() {
|
||||
assertEquals(AgentStatus.IDLE, AgentStatus.fromWire("idle"));
|
||||
assertEquals(AgentStatus.WORKING, AgentStatus.fromWire("working"));
|
||||
assertEquals(AgentStatus.BLOCKED, AgentStatus.fromWire("blocked"));
|
||||
assertEquals(AgentStatus.DONE, AgentStatus.fromWire("done"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void mapsDoneCaseInsensitively() {
|
||||
assertEquals(AgentStatus.DONE, AgentStatus.fromWire("DONE"));
|
||||
assertEquals(AgentStatus.DONE, AgentStatus.fromWire("Done"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void unknownAndNullFallToUnknown() {
|
||||
assertEquals(AgentStatus.UNKNOWN, AgentStatus.fromWire("unknown"));
|
||||
assertEquals(AgentStatus.UNKNOWN, AgentStatus.fromWire("something-else"));
|
||||
assertEquals(AgentStatus.UNKNOWN, AgentStatus.fromWire(null));
|
||||
}
|
||||
|
||||
@Test
|
||||
void doneIsInjectableLikeIdle() {
|
||||
// The whole point of CB-115: a finished worker herdr reports as `done` must be deliverable,
|
||||
// not treated as UNKNOWN (which wedged delivery and mis-fired the stall failure).
|
||||
assertTrue(AgentStatus.DONE.injectable());
|
||||
assertTrue(AgentStatus.IDLE.injectable());
|
||||
assertTrue(AgentStatus.BLOCKED.injectable());
|
||||
assertFalse(AgentStatus.WORKING.injectable());
|
||||
assertFalse(AgentStatus.UNKNOWN.injectable());
|
||||
}
|
||||
}
|
||||
@@ -5,7 +5,9 @@ import dev.ltms.bridged.herdr.FakeHerdr;
|
||||
import dev.ltms.bridged.msg.Rendezvous;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
/** Unit behaviour of the CB-106 completion resolver in isolation from the injector. */
|
||||
class CompletionResolverTest {
|
||||
@@ -16,7 +18,7 @@ class CompletionResolverTest {
|
||||
Rendezvous rendezvous = new Rendezvous(); // no waiter opened
|
||||
CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous);
|
||||
|
||||
resolver.resolve("term_a");
|
||||
resolver.resolve("term_a", null); // no in-flight turn captured for this target
|
||||
|
||||
assertFalse(herdr.called("agent.read"),
|
||||
"a turn nobody is blocked on must not cost a transcript scrape");
|
||||
@@ -28,9 +30,173 @@ class CompletionResolverTest {
|
||||
Rendezvous rendezvous = new Rendezvous(); // no waiter opened
|
||||
CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous);
|
||||
|
||||
resolver.fail("term_a");
|
||||
resolver.fail("term_a", null); // no in-flight turn, and no registered waiter to fall back to
|
||||
|
||||
assertFalse(herdr.called("agent.read"),
|
||||
"a wedge nobody is blocked on must not cost a transcript scrape");
|
||||
}
|
||||
|
||||
@Test
|
||||
void captureBaselineSkipsTheReadWhenNoSendIsWaiting() {
|
||||
FakeHerdr herdr = new FakeHerdr().readText("⏺ X\n❯ ");
|
||||
Rendezvous rendezvous = new Rendezvous(); // no waiter opened
|
||||
CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous);
|
||||
|
||||
resolver.captureBaseline("term_a"); // no send to attribute a later completion to
|
||||
|
||||
assertFalse(herdr.called("agent.read"),
|
||||
"with no waiting send there is no turn to baseline — skip the scrape");
|
||||
}
|
||||
|
||||
// --- CB-115 clean scrape: extract the last assistant block ----------------
|
||||
|
||||
@Test
|
||||
void extractsTheLastAssistantBlockStrippingChrome() {
|
||||
String raw = """
|
||||
⏺ Reading the file…
|
||||
|
||||
⏺ Done. The bug was an off-by-one in the loop bound.
|
||||
|
||||
╭──────────────────────────────────────╮
|
||||
│ > │
|
||||
╰──────────────────────────────────────╯
|
||||
⏵⏵ auto mode on · ? for shortcuts
|
||||
""";
|
||||
assertEquals("Done. The bug was an off-by-one in the loop bound.",
|
||||
CompletionResolver.lastAssistantBlock(raw));
|
||||
}
|
||||
|
||||
@Test
|
||||
void keepsMultiLineAssistantContent() {
|
||||
String raw = "⏺ Line one.\nLine two.\n❯ ";
|
||||
assertEquals("Line one.\nLine two.", CompletionResolver.lastAssistantBlock(raw));
|
||||
}
|
||||
|
||||
@Test
|
||||
void fallsBackToRawTextWhenThereIsNoMarker() {
|
||||
String raw = "plain worker output with no glyph";
|
||||
assertEquals("plain worker output with no glyph", CompletionResolver.lastAssistantBlock(raw));
|
||||
}
|
||||
|
||||
@Test
|
||||
void blankScrapeYieldsEmpty() {
|
||||
assertTrue(CompletionResolver.lastAssistantBlock("").isEmpty());
|
||||
assertTrue(CompletionResolver.lastAssistantBlock(null).isEmpty());
|
||||
}
|
||||
|
||||
@Test
|
||||
void stripsSpinnerAndRuleChrome() {
|
||||
String raw = """
|
||||
⏺ Channel check confirmed — your message got through.
|
||||
|
||||
✻ Brewed for 11s
|
||||
|
||||
─────────────────────────────────────
|
||||
""";
|
||||
assertEquals("Channel check confirmed — your message got through.",
|
||||
CompletionResolver.lastAssistantBlock(raw));
|
||||
}
|
||||
|
||||
@Test
|
||||
void cutsANextTurnPromptEchoAndTrailingTipsFromTheBlock() {
|
||||
// The exact turn-2 leak: the scrape captured the settled answer, then a "✻ Cooked" spinner,
|
||||
// then the NEXT turn's echoed prompt, then a "✶ Forming…" spinner and trailing tips/warnings
|
||||
// whose lines (⎿, ⚠) are not themselves chrome-terminated. Stopping at the first boundary
|
||||
// (the ✻ spinner) is what keeps every one of those interface lines out of the reply.
|
||||
String raw = """
|
||||
⏺ Channel confirmed — the bridge reply delivered successfully.
|
||||
|
||||
✻ Cooked for 9s
|
||||
|
||||
❯ Thanks. Now a small task: what is 17 * 23? Show just the number.
|
||||
|
||||
|
||||
|
||||
✶ Forming…
|
||||
⎿ Tip: Name your conversations with /rename
|
||||
⚠ claude.ai connectors are disabled because ANTHROPIC_API_KEY is set
|
||||
""";
|
||||
assertEquals("Channel confirmed — the bridge reply delivered successfully.",
|
||||
CompletionResolver.lastAssistantBlock(raw));
|
||||
}
|
||||
|
||||
// --- CB-115 misattribution guard: suppress a stale (unchanged) completion -------
|
||||
|
||||
@Test
|
||||
void suppressesACompletionWhoseScrapeIsUnchangedFromDelivery() {
|
||||
// Rapid back-to-back turn: the pane still shows the PREVIOUS turn's answer when this turn's
|
||||
// (misattributed) completion boundary fires. The scrape == the delivery baseline, so the
|
||||
// send must NOT be resolved with the stale answer.
|
||||
FakeHerdr herdr = new FakeHerdr().readText("⏺ 391\n❯ ");
|
||||
Rendezvous rendezvous = new Rendezvous();
|
||||
CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous);
|
||||
|
||||
var waiter = rendezvous.open("term_a"); // a send is blocked on this turn
|
||||
// The turn as captured at delivery: its waiter, and the previous turn's answer still on screen.
|
||||
var turn = new CompletionResolver.InFlight(waiter, "391");
|
||||
resolver.resolve("term_a", turn); // scrape still "391" == baseline → suppress
|
||||
|
||||
assertFalse(waiter.isDone(), "a completion with no output change must not resolve the send");
|
||||
assertTrue(rendezvous.isWaiting("term_a"), "the send stays waiting for a real reply");
|
||||
}
|
||||
|
||||
@Test
|
||||
void resolvesACompletionWhoseScrapeChangedSinceDelivery() {
|
||||
FakeHerdr herdr = new FakeHerdr().readText("⏺ No, 391 = 17 × 23.\n❯ "); // the worker's real answer
|
||||
Rendezvous rendezvous = new Rendezvous();
|
||||
CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous);
|
||||
|
||||
var waiter = rendezvous.open("term_a");
|
||||
// Delivery baseline was the previous turn's "391"; the scrape now differs → resolve.
|
||||
var turn = new CompletionResolver.InFlight(waiter, "391");
|
||||
resolver.resolve("term_a", turn);
|
||||
|
||||
assertTrue(waiter.isDone(), "a completion with new output must resolve the send");
|
||||
assertEquals(Rendezvous.Kind.COMPLETION, waiter.getNow(null).kind());
|
||||
assertEquals("No, 391 = 17 × 23.", waiter.getNow(null).text());
|
||||
}
|
||||
|
||||
@Test
|
||||
void resolvesWhenThereIsNoBaseline() {
|
||||
// No delivery baseline (e.g. the pre-turn read failed) ⇒ never suppress; the completion resolves.
|
||||
FakeHerdr herdr = new FakeHerdr().readText("⏺ hello\n❯ ");
|
||||
Rendezvous rendezvous = new Rendezvous();
|
||||
CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous);
|
||||
|
||||
var waiter = rendezvous.open("term_a");
|
||||
resolver.resolve("term_a", new CompletionResolver.InFlight(waiter, null));
|
||||
|
||||
assertTrue(waiter.isDone(), "with no baseline a completion resolves as before");
|
||||
assertEquals("hello", waiter.getNow(null).text());
|
||||
}
|
||||
|
||||
// --- CB-116 waiter identity: a late completion never crosses into the next turn ---------
|
||||
|
||||
@Test
|
||||
void aLateCompletionForOneTurnNeverResolvesTheNextTurnsWaiter() {
|
||||
// The cross-turn stale reply the conversation test surfaced: turn N's completion fallback
|
||||
// fires AFTER turn N was resolved by an explicit bridge_reply and turn N+1 has opened its own
|
||||
// waiter on the same session. Resolving "whatever is waiting now" would hand turn N's stale
|
||||
// scrape to turn N+1; targeting turn N's captured waiter makes the late completion a no-op.
|
||||
FakeHerdr herdr = new FakeHerdr().readText("⏺ turn N answer\n❯ ");
|
||||
Rendezvous rendezvous = new Rendezvous();
|
||||
CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous);
|
||||
|
||||
var waiterN = rendezvous.open("term_a"); // turn N's send
|
||||
// The turn as the injector captured it at delivery (waiter + pre-turn baseline).
|
||||
var turnN = new CompletionResolver.InFlight(waiterN, "an earlier answer");
|
||||
|
||||
// Turn N is resolved by the worker's explicit reply.
|
||||
assertTrue(rendezvous.resolve("term_a", "N replied"));
|
||||
|
||||
// Turn N+1's send opens its own waiter on the same session (replacing the registered one).
|
||||
var waiterN1 = rendezvous.open("term_a");
|
||||
|
||||
resolver.resolve("term_a", turnN); // turn N's completion fallback finally fires
|
||||
|
||||
assertFalse(waiterN1.isDone(), "turn N's late completion must not resolve turn N+1's waiter");
|
||||
assertEquals(Rendezvous.Kind.REPLY, waiterN.getNow(null).kind(),
|
||||
"turn N stays resolved by its own reply");
|
||||
assertTrue(rendezvous.isWaiting("term_a"), "turn N+1 is still awaiting its own resolution");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
package dev.ltms.bridged.inject;
|
||||
|
||||
import dev.ltms.bridged.herdr.AgentControl;
|
||||
import dev.ltms.bridged.herdr.AgentStatus;
|
||||
import dev.ltms.bridged.herdr.FakeHerdr;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
/** Content-based refinement of an unreliable {@code UNKNOWN} status (CB-115). */
|
||||
class StatusRefinerTest {
|
||||
|
||||
// --- pure classification -------------------------------------------------
|
||||
|
||||
@Test
|
||||
void classifiesAnIdlePromptAsIdle() {
|
||||
String pane = """
|
||||
⏺ All done — the file compiles cleanly.
|
||||
|
||||
╭──────────────────────────────────────╮
|
||||
│ > │
|
||||
╰──────────────────────────────────────╯
|
||||
⏵⏵ auto mode on (shift+tab to cycle)
|
||||
""";
|
||||
assertEquals(AgentStatus.IDLE, StatusRefiner.classify(pane));
|
||||
}
|
||||
|
||||
@Test
|
||||
void classifiesABarePromptGlyphAsIdle() {
|
||||
assertEquals(AgentStatus.IDLE, StatusRefiner.classify("some output\n❯ "));
|
||||
}
|
||||
|
||||
@Test
|
||||
void classifiesActiveGenerationAsWorking() {
|
||||
String pane = """
|
||||
⏺ Working on it…
|
||||
✳ Thinking… (12s · esc to interrupt)
|
||||
""";
|
||||
assertEquals(AgentStatus.WORKING, StatusRefiner.classify(pane));
|
||||
}
|
||||
|
||||
@Test
|
||||
void anEscToInterruptScreenIsWorkingEvenWithAPromptBox() {
|
||||
// "esc to interrupt" wins over a prompt box: the turn is still generating.
|
||||
String pane = "│ > │\n esc to interrupt";
|
||||
assertEquals(AgentStatus.WORKING, StatusRefiner.classify(pane));
|
||||
}
|
||||
|
||||
@Test
|
||||
void anUnrecognizableScreenStaysUnknown() {
|
||||
assertEquals(AgentStatus.UNKNOWN, StatusRefiner.classify("garbled ansi noise with no prompt"));
|
||||
assertEquals(AgentStatus.UNKNOWN, StatusRefiner.classify(""));
|
||||
assertEquals(AgentStatus.UNKNOWN, StatusRefiner.classify(null));
|
||||
}
|
||||
|
||||
// --- refine() wiring -----------------------------------------------------
|
||||
|
||||
@Test
|
||||
void refinePassesNonUnknownStatusesThroughWithoutReading() {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
StatusRefiner refiner = new StatusRefiner(new AgentControl(herdr));
|
||||
|
||||
assertEquals(AgentStatus.WORKING, refiner.refine("term_a", AgentStatus.WORKING));
|
||||
assertEquals(AgentStatus.IDLE, refiner.refine("term_a", AgentStatus.IDLE));
|
||||
|
||||
assertFalse(herdr.called("agent.read"),
|
||||
"a trusted status must not cost a pane read");
|
||||
}
|
||||
|
||||
@Test
|
||||
void refineUpgradesUnknownToIdleFromPaneContent() {
|
||||
FakeHerdr herdr = new FakeHerdr().readText("⏺ answer\n❯ ");
|
||||
StatusRefiner refiner = new StatusRefiner(new AgentControl(herdr));
|
||||
|
||||
assertEquals(AgentStatus.IDLE, refiner.refine("term_a", AgentStatus.UNKNOWN));
|
||||
assertTrue(herdr.called("agent.read"), "an UNKNOWN must trigger a pane read");
|
||||
}
|
||||
|
||||
@Test
|
||||
void refineLeavesUnknownWhenContentIsUnclassifiable() {
|
||||
FakeHerdr herdr = new FakeHerdr().readText("nothing recognizable here");
|
||||
StatusRefiner refiner = new StatusRefiner(new AgentControl(herdr));
|
||||
|
||||
assertEquals(AgentStatus.UNKNOWN, refiner.refine("term_a", AgentStatus.UNKNOWN));
|
||||
}
|
||||
}
|
||||
@@ -49,8 +49,10 @@ class MessageServiceTest {
|
||||
CompletableFuture<MessageService.Reply> send = sendAsync();
|
||||
awaitWaiting();
|
||||
|
||||
injector.onStatus(T, AgentStatus.IDLE); // deliver the task
|
||||
herdr.readText("$ prompt"); // pre-turn pane: no answer yet (baseline reference)
|
||||
injector.onStatus(T, AgentStatus.IDLE); // deliver the task (baselines the pre-turn content)
|
||||
injector.onStatus(T, AgentStatus.WORKING); // worker picks it up and works
|
||||
herdr.readText("BUILD GREEN: 391 files"); // the worker's turn produced new output
|
||||
injector.onStatus(T, AgentStatus.IDLE); // working → idle: turn complete, no bridge_reply
|
||||
|
||||
MessageService.Reply reply = send.get(5, TimeUnit.SECONDS);
|
||||
|
||||
Reference in New Issue
Block a user