Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions lib/msgfmt/message_box.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,22 @@ import (
"strings"
)

// containsHorizontalBorder reports whether the line contains a
// horizontal border made of box-drawing characters (─ or ╌).
func containsHorizontalBorder(line string) bool {
return strings.Contains(line, "───────────────") ||
strings.Contains(line, "╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌")
}

// Usually something like
// ───────────────
// ─────────────── (or ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌)
// >
// ───────────────
// ─────────────── (or ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌)
// Used by Claude Code, Goose, and Aider.
func findGreaterThanMessageBox(lines []string) int {
for i := len(lines) - 1; i >= max(len(lines)-6, 0); i-- {
if strings.Contains(lines[i], ">") {
if i > 0 && strings.Contains(lines[i-1], "───────────────") {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obs findGenericSlimMessageBox accepts mixed border styles — e.g. ─── on top and ╌╌╌ on bottom would match. (Edge Case Analyst, Contract Auditor)

The top and bottom borders are checked independently with ||. In practice, Claude Code uses the same character for both borders, so this is defensive rather than problematic.

Agreed this is fine — more permissive detection reduces false negatives, and the narrow scan window (last 9 lines) plus the prompt-character requirement on the middle line make false positives unlikely. Worth knowing if a false-positive concern arises later.

if i > 0 && containsHorizontalBorder(lines[i-1]) {
return i - 1
}
return i
Expand All @@ -22,14 +29,14 @@ func findGreaterThanMessageBox(lines []string) int {
}

// Usually something like
// ───────────────
// ─────────────── (or ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌)
// |
// ───────────────
// ─────────────── (or ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌)
func findGenericSlimMessageBox(lines []string) int {
for i := len(lines) - 3; i >= max(len(lines)-9, 0); i-- {
if strings.Contains(lines[i], "───────────────") &&
if containsHorizontalBorder(lines[i]) &&
(strings.Contains(lines[i+1], "|") || strings.Contains(lines[i+1], "│") || strings.Contains(lines[i+1], "❯")) &&
strings.Contains(lines[i+2], "───────────────") {
containsHorizontalBorder(lines[i+2]) {
return i
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
1 function greet() {
2 - console.log("Hello, World!");
2 + console.log("Hello, Claude!");
3 }
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌
> Try "what does this code do?"
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌
Syntax theme: Monokai Extended (ctrl+t to disable)
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
╭────────────────────────────────────────────╮
│ ✻ Welcome to Claude Code! │
│ │
│ /help for help │
╰────────────────────────────────────────────╯
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌
│ Type your message...
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌
8 changes: 8 additions & 0 deletions lib/screentracker/pty_conversation.go
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,14 @@ func (c *PTYConversation) statusLocked() ConversationStatus {
return ConversationStatusChanging
}

// The send loop gates stableSignal on initialPromptReady.
// Report "changing" until readiness is detected so that Send()
// rejects with ErrMessageValidationChanging instead of blocking
// indefinitely on a stableSignal that will never fire.
if !c.initialPromptReady {
return ConversationStatusChanging
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obs ConversationStatusChanging now carries two distinct meanings: "screen content is actively changing" and "agent readiness detection hasn't fired." (Edge Case Analyst, Contract Auditor)

An external consumer watching /status cannot distinguish between transient screen instability (which resolves on its own) and permanent readiness failure (which may never resolve without user intervention).

This is strictly better than the old behavior (status said "stable" but Send() hung). Both changing and initializing map to running in the HTTP layer anyway, so the public API surface doesn't change. If a future feature needs to surface "agent not recognized" vs. "screen still settling," a new status value or a separate readiness field would be needed — but that's out of scope here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note Adding to this thread: when readiness never fires, the system now reports "running" indefinitely with no log line or error explaining why. The user's only recourse is inspecting the terminal directly. This is strictly better than the old hang, and the dual-meaning limitation is acknowledged as out of scope here. Noting for awareness since there's no planned follow-up or tracking issue. (Mafuuu)

🤖

}

// Handle initial prompt readiness: report "changing" until the queue is drained
// to avoid the status flipping "changing" -> "stable" -> "changing"
if len(c.outboundQueue) > 0 || c.sendingMessage {
Expand Down
61 changes: 48 additions & 13 deletions lib/screentracker/pty_conversation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1200,7 +1200,7 @@ func TestStatePersistence(t *testing.T) {
func TestInitialPromptReadiness(t *testing.T) {
discardLogger := slog.New(slog.NewTextHandler(io.Discard, nil))

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

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

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

// Agent not ready initially, status should be stable
// Agent not ready initially, status should be changing.
advanceFor(ctx, t, mClock, 1*time.Second)
assert.Equal(t, st.ConversationStatusStable, c.Status())

assert.Equal(t, st.ConversationStatusChanging, c.Status())
// Agent becomes ready, prompt gets enqueued, status becomes "changing"
agent.setScreen("ready")
advanceFor(ctx, t, mClock, 1*time.Second)
Expand Down Expand Up @@ -1283,10 +1282,9 @@ func TestInitialPromptReadiness(t *testing.T) {
c := st.NewPTY(ctx, cfg, &testEmitter{})
c.Start(ctx)

// Status is "stable" while waiting for readiness (prompt not yet enqueued).
// Status is "changing" while waiting for readiness (prompt not yet enqueued).
advanceFor(ctx, t, mClock, 1*time.Second)
assert.Equal(t, st.ConversationStatusStable, c.Status())

assert.Equal(t, st.ConversationStatusChanging, c.Status())
// Agent becomes ready. The snapshot loop detects this, enqueues the prompt,
// then sees queue + stable + ready and signals the send loop.
// writeStabilize runs with onWrite changing the screen, so it completes.
Expand All @@ -1304,7 +1302,7 @@ func TestInitialPromptReadiness(t *testing.T) {
assert.Equal(t, st.ConversationStatusStable, c.Status())
})

t.Run("no initial prompt - normal status logic applies", func(t *testing.T) {
t.Run("ReadyForInitialPrompt always false - status is changing", func(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
t.Cleanup(cancel)
mClock := quartz.NewMock(t)
Expand All @@ -1325,8 +1323,10 @@ func TestInitialPromptReadiness(t *testing.T) {

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

// Status should be stable because no initial prompt to wait for.
assert.Equal(t, st.ConversationStatusStable, c.Status())
// Even without an initial prompt, stableSignal gates on
// initialPromptReady. Status must reflect that Send()
// would block.
assert.Equal(t, st.ConversationStatusChanging, c.Status())
})

t.Run("no initial prompt configured - normal status logic applies", func(t *testing.T) {
Expand Down Expand Up @@ -1743,3 +1743,38 @@ func TestInitialPromptSent(t *testing.T) {
}
})
}

func TestSendRejectsWhenInitialPromptNotReady(t *testing.T) {
// Regression test for https://github.com/coder/agentapi/issues/209.
// Send() used to block forever when ReadyForInitialPrompt never
// returned true, because statusLocked() reported "stable" while
// stableSignal required initialPromptReady. Now statusLocked()
// returns "changing" and Send() rejects immediately.
ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
t.Cleanup(cancel)

mClock := quartz.NewMock(t)
agent := &testAgent{screen: "onboarding screen without message box"}
cfg := st.PTYConversationConfig{
Clock: mClock,
SnapshotInterval: 100 * time.Millisecond,
ScreenStabilityLength: 200 * time.Millisecond,
AgentIO: agent,
ReadyForInitialPrompt: func(message string) bool {
return false // Simulates failed message box detection.
},
Logger: slog.New(slog.NewTextHandler(io.Discard, nil)),
}
c := st.NewPTY(ctx, cfg, &testEmitter{})
c.Start(ctx)

// Fill snapshot buffer to reach stability.
advanceFor(ctx, t, mClock, 300*time.Millisecond)

// Status reports "changing" because initialPromptReady is false.
assert.Equal(t, st.ConversationStatusChanging, c.Status())

// Send() rejects immediately instead of blocking forever.
err := c.Send(st.MessagePartText{Content: "hello"})
assert.ErrorIs(t, err, st.ErrMessageValidationChanging)
}
Loading