Skip to content

Commit 00ff7ff

Browse files
johnstcn35C4n0r
andauthored
fix(screentracker): make writeStabilize Phase 1 non-fatal when agents don't echo input (#208)
Fixes #123. - Make Phase 1 (echo detection) of `writeStabilize` non-fatal on timeout -- proceed to Phase 2 instead of returning HTTP 500 - Guard non-fatal path with `errors.Is(err, util.WaitTimedOut)` so context cancellation still propagates - Reduce Phase 1 timeout from 15s to 2s (terminal echo is near-instant) - Extract `writeStabilizeEchoTimeout` and `writeStabilizeProcessTimeout` constants - Log at Info level (not Warn) since non-echoing agents hit this on every message - Add doc comment on `formatPaste` in `claude.go` documenting the ESC limitation with TUI selection prompts - Add tests for non-echoing agents (reacts, unresponsive, context-cancelled) ## Known limitation For TUI selection prompts (numbered/arrow-key lists), this fix eliminates the 500 but does **not** deliver the correct selection -- the `\x1b` (ESC) in bracketed paste cancels the selection widget. The correct approach is `MessageTypeRaw`. Documented via a comment on `formatPaste` in `lib/httpapi/claude.go`. Also discovered a separate issue during smoke-testing: #209 (fixed in #215, included in this branch). > 🤖 Written by a Coder Agent. Reviewed by several humans and a veritable army of robots. Co-authored-by: 35C4n0r <70096901+35C4n0r@users.noreply.github.com>
1 parent d42b9ac commit 00ff7ff

File tree

6 files changed

+266
-28
lines changed

6 files changed

+266
-28
lines changed

lib/httpapi/claude.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ import (
55
st "github.com/coder/agentapi/lib/screentracker"
66
)
77

8+
// formatPaste wraps message in bracketed paste escape sequences.
9+
// These sequences start with ESC (\x1b), which TUI selection
10+
// widgets (e.g. Claude Code's numbered-choice prompt) interpret
11+
// as "cancel". For selection prompts, callers should use
12+
// MessageTypeRaw to send raw keystrokes directly instead.
813
func formatPaste(message string) []st.MessagePart {
914
return []st.MessagePart{
1015
// Bracketed paste mode start sequence

lib/msgfmt/message_box.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,22 @@ import (
44
"strings"
55
)
66

7+
// containsHorizontalBorder reports whether the line contains a
8+
// horizontal border made of box-drawing characters (─ or ╌).
9+
func containsHorizontalBorder(line string) bool {
10+
return strings.Contains(line, "───────────────") ||
11+
strings.Contains(line, "╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌")
12+
}
13+
714
// Usually something like
8-
// ───────────────
15+
// ─────────────── (or ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌)
916
// >
10-
// ───────────────
17+
// ─────────────── (or ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌)
1118
// Used by Claude Code, Goose, and Aider.
1219
func findGreaterThanMessageBox(lines []string) int {
1320
for i := len(lines) - 1; i >= max(len(lines)-6, 0); i-- {
1421
if strings.Contains(lines[i], ">") {
15-
if i > 0 && strings.Contains(lines[i-1], "───────────────") {
22+
if i > 0 && containsHorizontalBorder(lines[i-1]) {
1623
return i - 1
1724
}
1825
return i
@@ -22,14 +29,14 @@ func findGreaterThanMessageBox(lines []string) int {
2229
}
2330

2431
// Usually something like
25-
// ───────────────
32+
// ─────────────── (or ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌)
2633
// |
27-
// ───────────────
34+
// ─────────────── (or ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌)
2835
func findGenericSlimMessageBox(lines []string) int {
2936
for i := len(lines) - 3; i >= max(len(lines)-9, 0); i-- {
30-
if strings.Contains(lines[i], "───────────────") &&
37+
if containsHorizontalBorder(lines[i]) &&
3138
(strings.Contains(lines[i+1], "|") || strings.Contains(lines[i+1], "│") || strings.Contains(lines[i+1], "❯")) &&
32-
strings.Contains(lines[i+2], "───────────────") {
39+
containsHorizontalBorder(lines[i+2]) {
3340
return i
3441
}
3542
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
1 function greet() {
2+
2 - console.log("Hello, World!");
3+
2 + console.log("Hello, Claude!");
4+
3 }
5+
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌
6+
> Try "what does this code do?"
7+
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌
8+
Syntax theme: Monokai Extended (ctrl+t to disable)
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
╭────────────────────────────────────────────╮
2+
│ ✻ Welcome to Claude Code! │
3+
│ │
4+
│ /help for help │
5+
╰────────────────────────────────────────────╯
6+
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌
7+
│ Type your message...
8+
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌

lib/screentracker/pty_conversation.go

Lines changed: 62 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package screentracker
33
import (
44
"context"
55
"encoding/json"
6+
"errors"
67
"fmt"
78
"log/slog"
89
"os"
@@ -16,6 +17,26 @@ import (
1617
"golang.org/x/xerrors"
1718
)
1819

20+
const (
21+
// writeStabilizeEchoTimeout is the timeout for the echo
22+
// detection WaitFor loop in writeStabilize Phase 1. The
23+
// effective ceiling may be slightly longer because the
24+
// stability check inside the condition runs outside
25+
// WaitFor's timeout select. Non-echoing agents (e.g. TUI
26+
// agents using bracketed paste) will hit this timeout,
27+
// which is non-fatal.
28+
//
29+
// TODO: move to PTYConversationConfig if agents need
30+
// different echo detection windows.
31+
writeStabilizeEchoTimeout = 2 * time.Second
32+
33+
// writeStabilizeProcessTimeout is the maximum time to wait
34+
// for the screen to change after sending a carriage return.
35+
// This detects whether the agent is actually processing the
36+
// input.
37+
writeStabilizeProcessTimeout = 15 * time.Second
38+
)
39+
1940
// A screenSnapshot represents a snapshot of the PTY at a specific time.
2041
type screenSnapshot struct {
2142
timestamp time.Time
@@ -411,17 +432,30 @@ func (c *PTYConversation) sendMessage(ctx context.Context, messageParts ...Messa
411432
return nil
412433
}
413434

414-
// writeStabilize writes messageParts to the screen and waits for the screen to stabilize after the message is written.
435+
// writeStabilize writes messageParts to the PTY and waits for
436+
// the agent to process them. It operates in two phases:
437+
//
438+
// Phase 1 (echo detection): writes the message text and waits
439+
// for the screen to change and stabilize. This detects agents
440+
// that echo typed input. If the screen doesn't change within
441+
// writeStabilizeEchoTimeout, this is non-fatal. Many TUI
442+
// agents buffer bracketed-paste input without rendering it.
443+
//
444+
// Phase 2 (processing detection): writes a carriage return
445+
// and waits for the screen to change, indicating the agent
446+
// started processing. This phase is fatal on timeout: if the
447+
// agent doesn't react to Enter, it's unresponsive.
415448
func (c *PTYConversation) writeStabilize(ctx context.Context, messageParts ...MessagePart) error {
416449
screenBeforeMessage := c.cfg.AgentIO.ReadScreen()
417450
for _, part := range messageParts {
418451
if err := part.Do(c.cfg.AgentIO); err != nil {
419452
return xerrors.Errorf("failed to write message part: %w", err)
420453
}
421454
}
422-
// wait for the screen to stabilize after the message is written
455+
// Phase 1: wait for the screen to stabilize after the
456+
// message is written (echo detection).
423457
if err := util.WaitFor(ctx, util.WaitTimeout{
424-
Timeout: 15 * time.Second,
458+
Timeout: writeStabilizeEchoTimeout,
425459
MinInterval: 50 * time.Millisecond,
426460
InitialWait: true,
427461
Clock: c.cfg.Clock,
@@ -441,14 +475,26 @@ func (c *PTYConversation) writeStabilize(ctx context.Context, messageParts ...Me
441475
}
442476
return false, nil
443477
}); err != nil {
444-
return xerrors.Errorf("failed to wait for screen to stabilize: %w", err)
445-
}
446-
447-
// wait for the screen to change after the carriage return is written
478+
if !errors.Is(err, util.WaitTimedOut) {
479+
// Context cancellation or condition errors are fatal.
480+
return xerrors.Errorf("failed to wait for screen to stabilize: %w", err)
481+
}
482+
// Phase 1 timeout is non-fatal: the agent may not echo
483+
// input (e.g. TUI agents buffer bracketed-paste content
484+
// internally). Proceed to Phase 2 to send the carriage
485+
// return.
486+
c.cfg.Logger.Info(
487+
"echo detection timed out, sending carriage return",
488+
"timeout", writeStabilizeEchoTimeout,
489+
)
490+
}
491+
492+
// Phase 2: wait for the screen to change after the
493+
// carriage return is written (processing detection).
448494
screenBeforeCarriageReturn := c.cfg.AgentIO.ReadScreen()
449495
lastCarriageReturnTime := time.Time{}
450496
if err := util.WaitFor(ctx, util.WaitTimeout{
451-
Timeout: 15 * time.Second,
497+
Timeout: writeStabilizeProcessTimeout,
452498
MinInterval: 25 * time.Millisecond,
453499
Clock: c.cfg.Clock,
454500
}, func() (bool, error) {
@@ -527,6 +573,14 @@ func (c *PTYConversation) statusLocked() ConversationStatus {
527573
return ConversationStatusChanging
528574
}
529575

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

0 commit comments

Comments
 (0)