Skip to content

Commit 761b7d5

Browse files
committed
fix(screentracker): make statusLocked() honest about initialPromptReady
statusLocked() used to report 'stable' when the screen was calm, blissfully unaware that initialPromptReady was false. Send() would trust this optimistic status, enqueue a message, and then wait forever for a stableSignal that would never arrive — like waiting for a bus that got cancelled. Return 'changing' when initialPromptReady is false so Send() rejects immediately with ErrMessageValidationChanging instead of ghosting the caller. Fixes: #209
1 parent 99b9c11 commit 761b7d5

2 files changed

Lines changed: 63 additions & 20 deletions

File tree

lib/screentracker/pty_conversation.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,14 @@ func (c *PTYConversation) statusLocked() ConversationStatus {
571571
return ConversationStatusChanging
572572
}
573573

574+
// The send loop gates outbound messages on initialPromptReady.
575+
// Report "changing" until readiness is detected so that Send()
576+
// rejects with ErrMessageValidationChanging instead of blocking
577+
// indefinitely on a stableSignal that will never fire.
578+
if !c.initialPromptReady {
579+
return ConversationStatusChanging
580+
}
581+
574582
// Handle initial prompt readiness: report "changing" until the queue is drained
575583
// to avoid the status flipping "changing" -> "stable" -> "changing"
576584
if len(c.outboundQueue) > 0 || c.sendingMessage {

lib/screentracker/pty_conversation_test.go

Lines changed: 55 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,7 +1200,7 @@ func TestStatePersistence(t *testing.T) {
12001200
func TestInitialPromptReadiness(t *testing.T) {
12011201
discardLogger := slog.New(slog.NewTextHandler(io.Discard, nil))
12021202

1203-
t.Run("agent not ready - status is stable until agent becomes ready", func(t *testing.T) {
1203+
t.Run("agent not ready - status is changing until agent becomes ready", func(t *testing.T) {
12041204
ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
12051205
t.Cleanup(cancel)
12061206
mClock := quartz.NewMock(t)
@@ -1223,9 +1223,9 @@ func TestInitialPromptReadiness(t *testing.T) {
12231223
// Take a snapshot with "loading...". Threshold is 1 (stability 0 / interval 1s = 0 + 1 = 1).
12241224
advanceFor(ctx, t, mClock, 1*time.Second)
12251225

1226-
// Screen is stable and agent is not ready, so initial prompt hasn't been enqueued yet.
1227-
// Status should be stable.
1228-
assert.Equal(t, st.ConversationStatusStable, c.Status())
1226+
// Screen is stable but agent is not ready. Status must be
1227+
// "changing" so that Send() rejects instead of blocking.
1228+
assert.Equal(t, st.ConversationStatusChanging, c.Status())
12291229
})
12301230

12311231
t.Run("agent becomes ready - prompt enqueued and status changes to changing", func(t *testing.T) {
@@ -1248,10 +1248,9 @@ func TestInitialPromptReadiness(t *testing.T) {
12481248
c := st.NewPTY(ctx, cfg, &testEmitter{})
12491249
c.Start(ctx)
12501250

1251-
// Agent not ready initially, status should be stable
1252-
advanceFor(ctx, t, mClock, 1*time.Second)
1253-
assert.Equal(t, st.ConversationStatusStable, c.Status())
1254-
1251+
// Agent not ready initially, status should be changing.
1252+
advanceFor(ctx, t, mClock, 1*time.Second)
1253+
assert.Equal(t, st.ConversationStatusChanging, c.Status())
12551254
// Agent becomes ready, prompt gets enqueued, status becomes "changing"
12561255
agent.setScreen("ready")
12571256
advanceFor(ctx, t, mClock, 1*time.Second)
@@ -1283,10 +1282,9 @@ func TestInitialPromptReadiness(t *testing.T) {
12831282
c := st.NewPTY(ctx, cfg, &testEmitter{})
12841283
c.Start(ctx)
12851284

1286-
// Status is "stable" while waiting for readiness (prompt not yet enqueued).
1287-
advanceFor(ctx, t, mClock, 1*time.Second)
1288-
assert.Equal(t, st.ConversationStatusStable, c.Status())
1289-
1285+
// Status is "changing" while waiting for readiness (prompt not yet enqueued).
1286+
advanceFor(ctx, t, mClock, 1*time.Second)
1287+
assert.Equal(t, st.ConversationStatusChanging, c.Status())
12901288
// Agent becomes ready. The snapshot loop detects this, enqueues the prompt,
12911289
// then sees queue + stable + ready and signals the send loop.
12921290
// writeStabilize runs with onWrite changing the screen, so it completes.
@@ -1304,7 +1302,7 @@ func TestInitialPromptReadiness(t *testing.T) {
13041302
assert.Equal(t, st.ConversationStatusStable, c.Status())
13051303
})
13061304

1307-
t.Run("no initial prompt - normal status logic applies", func(t *testing.T) {
1305+
t.Run("no initial prompt - status is changing when readiness never fires", func(t *testing.T) {
13081306
ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
13091307
t.Cleanup(cancel)
13101308
mClock := quartz.NewMock(t)
@@ -1325,8 +1323,10 @@ func TestInitialPromptReadiness(t *testing.T) {
13251323

13261324
advanceFor(ctx, t, mClock, 1*time.Second)
13271325

1328-
// Status should be stable because no initial prompt to wait for.
1329-
assert.Equal(t, st.ConversationStatusStable, c.Status())
1326+
// Even without an initial prompt, the send loop gates on
1327+
// initialPromptReady. Status must reflect that Send()
1328+
// would block.
1329+
assert.Equal(t, st.ConversationStatusChanging, c.Status())
13301330
})
13311331

13321332
t.Run("no initial prompt configured - normal status logic applies", func(t *testing.T) {
@@ -1736,10 +1736,45 @@ func TestInitialPromptSent(t *testing.T) {
17361736

17371737
// Verify no prompt was sent (should only have the initial screen message)
17381738
messages := c.Messages()
1739-
for _, msg := range messages {
1740-
if msg.Role == st.ConversationRoleUser {
1741-
t.Errorf("Unexpected user message sent: %q (empty prompt should not be restored)", msg.Message)
1739+
for _, msg := range messages {
1740+
if msg.Role == st.ConversationRoleUser {
1741+
t.Errorf("Unexpected user message sent: %q (empty prompt should not be restored)", msg.Message)
1742+
}
17421743
}
1743-
}
1744-
})
1744+
})
17451745
}
1746+
1747+
func TestSendRejectsWhenInitialPromptNotReady(t *testing.T) {
1748+
// Regression test for https://github.com/coder/agentapi/issues/209.
1749+
// Send() used to block forever when ReadyForInitialPrompt never
1750+
// returned true, because statusLocked() reported "stable" while
1751+
// stableSignal required initialPromptReady. Now statusLocked()
1752+
// returns "changing" and Send() rejects immediately.
1753+
ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
1754+
t.Cleanup(cancel)
1755+
1756+
mClock := quartz.NewMock(t)
1757+
agent := &testAgent{screen: "onboarding screen without message box"}
1758+
cfg := st.PTYConversationConfig{
1759+
Clock: mClock,
1760+
SnapshotInterval: 100 * time.Millisecond,
1761+
ScreenStabilityLength: 200 * time.Millisecond,
1762+
AgentIO: agent,
1763+
ReadyForInitialPrompt: func(screen string) bool {
1764+
return false // Simulates failed message box detection.
1765+
},
1766+
Logger: slog.New(slog.NewTextHandler(io.Discard, nil)),
1767+
}
1768+
c := st.NewPTY(ctx, cfg, &testEmitter{})
1769+
c.Start(ctx)
1770+
1771+
// Fill snapshot buffer to reach stability.
1772+
advanceFor(ctx, t, mClock, 300*time.Millisecond)
1773+
1774+
// Status reports "changing" because initialPromptReady is false.
1775+
assert.Equal(t, st.ConversationStatusChanging, c.Status())
1776+
1777+
// Send() rejects immediately instead of blocking forever.
1778+
err := c.Send(st.MessagePartText{Content: "hello"})
1779+
assert.ErrorIs(t, err, st.ErrMessageValidationChanging)
1780+
}

0 commit comments

Comments
 (0)