From d51e4c21348b6d6a949256e57b6c482593fad561 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 4 Feb 2026 11:29:14 +0000 Subject: [PATCH 01/24] refactor(screentracker): internalize initial prompt handling in PTYConversation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes: - Add InitialPrompt []MessagePart and OnSnapshot callback to PTYConversationConfig - Remove initialPrompt string parameter from NewPTY function (now reads from config) - Add initialPromptReady chan struct{} field for signaling readiness - Add sendLocked() helper (same as Send but without lock) - Add messagesLocked() helper that returns a copy of messages - Update statusLocked() to return ConversationStatusChanging while initial prompt is pending, fixing the status flip-flop issue (changing → stable → changing) - Update Start() to use select with: - Ticker for snapshots (calling OnSnapshot callback if set) - initialPromptReady channel to send initial prompt when ready This consolidates initial prompt logic inside PTYConversation.Start() instead of requiring the server to manipulate internal fields directly. The server.go changes to use this new API will be done in a separate commit. --- lib/screentracker/pty_conversation.go | 71 ++++++++++++++++++++++++--- 1 file changed, 63 insertions(+), 8 deletions(-) diff --git a/lib/screentracker/pty_conversation.go b/lib/screentracker/pty_conversation.go index 38d8e40..cd49436 100644 --- a/lib/screentracker/pty_conversation.go +++ b/lib/screentracker/pty_conversation.go @@ -66,7 +66,11 @@ type PTYConversationConfig struct { ReadyForInitialPrompt func(message string) bool // FormatToolCall removes the coder report_task tool call from the agent message and also returns the array of removed tool calls FormatToolCall func(message string) (string, []string) - Logger *slog.Logger + // InitialPrompt is the initial prompt to send to the agent once ready + InitialPrompt []MessagePart + // OnSnapshot is called after each snapshot with current status, messages, and screen content + OnSnapshot func(status ConversationStatus, messages []ConversationMessage, screen string) + Logger *slog.Logger } func (cfg PTYConversationConfig) getStableSnapshotsThreshold() int { @@ -96,13 +100,15 @@ type PTYConversation struct { InitialPromptSent bool // ReadyForInitialPrompt keeps track if the agent is ready to accept the initial prompt ReadyForInitialPrompt bool + // initialPromptReady is closed when the agent is ready to receive the initial prompt + initialPromptReady chan struct{} // toolCallMessageSet keeps track of the tool calls that have been detected & logged in the current agent message toolCallMessageSet map[string]bool } var _ Conversation = &PTYConversation{} -func NewPTY(ctx context.Context, cfg PTYConversationConfig, initialPrompt string) *PTYConversation { +func NewPTY(ctx context.Context, cfg PTYConversationConfig) *PTYConversation { if cfg.Clock == nil { cfg.Clock = quartz.NewReal() } @@ -118,10 +124,13 @@ func NewPTY(ctx context.Context, cfg PTYConversationConfig, initialPrompt string Time: cfg.Clock.Now(), }, }, - InitialPrompt: initialPrompt, - InitialPromptSent: len(initialPrompt) == 0, + InitialPromptSent: len(cfg.InitialPrompt) == 0, toolCallMessageSet: make(map[string]bool), } + // Initialize the channel only if we have an initial prompt to send + if len(cfg.InitialPrompt) > 0 { + c.initialPromptReady = make(chan struct{}) + } return c } @@ -129,6 +138,14 @@ func (c *PTYConversation) Start(ctx context.Context) { go func() { ticker := c.cfg.Clock.NewTicker(c.cfg.SnapshotInterval) defer ticker.Stop() + + // Create a nil channel if no initial prompt - select will never receive from it + initialPromptReady := c.initialPromptReady + if initialPromptReady == nil { + initialPromptReady = make(chan struct{}) + // Don't close it - we want it to block forever + } + for { select { case <-ctx.Done(): @@ -144,7 +161,27 @@ func (c *PTYConversation) Start(ctx context.Context) { c.lock.Lock() screen := c.cfg.AgentIO.ReadScreen() c.snapshotLocked(screen) + status := c.statusLocked() + messages := c.messagesLocked() + c.lock.Unlock() + + // Call snapshot callback if configured + if c.cfg.OnSnapshot != nil { + c.cfg.OnSnapshot(status, messages, screen) + } + case <-initialPromptReady: + // Agent is ready for initial prompt - send it + c.lock.Lock() + if !c.InitialPromptSent && len(c.cfg.InitialPrompt) > 0 { + if err := c.sendLocked(c.cfg.InitialPrompt...); err != nil { + c.cfg.Logger.Error("failed to send initial prompt", "error", err) + } else { + c.InitialPromptSent = true + } + } c.lock.Unlock() + // Nil out to prevent this case from triggering again + initialPromptReady = nil } } }() @@ -222,6 +259,11 @@ func (c *PTYConversation) Send(messageParts ...MessagePart) error { c.lock.Lock() defer c.lock.Unlock() + return c.sendLocked(messageParts...) +} + +// sendLocked sends a message to the agent. Caller MUST hold c.lock. +func (c *PTYConversation) sendLocked(messageParts ...MessagePart) error { if !c.cfg.SkipSendMessageStatusCheck && c.statusLocked() != ConversationStatusStable { return ErrMessageValidationChanging } @@ -356,11 +398,19 @@ func (c *PTYConversation) statusLocked() ConversationStatus { } } - if !c.InitialPromptSent && !c.ReadyForInitialPrompt { - if len(snapshots) > 0 && c.cfg.ReadyForInitialPrompt(snapshots[len(snapshots)-1].screen) { - c.ReadyForInitialPrompt = true - return ConversationStatusStable + // Handle initial prompt readiness: report "changing" until the prompt is sent + // to avoid the status flipping "changing" → "stable" → "changing" + if !c.InitialPromptSent { + if !c.ReadyForInitialPrompt { + if len(snapshots) > 0 && c.cfg.ReadyForInitialPrompt != nil && c.cfg.ReadyForInitialPrompt(snapshots[len(snapshots)-1].screen) { + c.ReadyForInitialPrompt = true + // Signal that we're ready - close the channel if it exists + if c.initialPromptReady != nil { + close(c.initialPromptReady) + } + } } + // Keep returning "changing" until initial prompt is actually sent return ConversationStatusChanging } @@ -371,6 +421,11 @@ func (c *PTYConversation) Messages() []ConversationMessage { c.lock.Lock() defer c.lock.Unlock() + return c.messagesLocked() +} + +// messagesLocked returns a copy of messages. Caller MUST hold c.lock. +func (c *PTYConversation) messagesLocked() []ConversationMessage { result := make([]ConversationMessage, len(c.messages)) copy(result, c.messages) return result From 241671a0c0f2cf9636d6ad7dfbf57b81c5cd272f Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 4 Feb 2026 11:31:47 +0000 Subject: [PATCH 02/24] refactor(server): use PTYConversation's internal snapshot loop and InitialPrompt handling - Update NewServer to format InitialPrompt into []MessagePart via FormatMessage - Pass InitialPrompt and OnSnapshot callback in PTYConversationConfig - OnSnapshot callback emits status/messages/screen changes to EventEmitter - Remove initialPrompt string parameter from NewPTY call (now in config) - Simplify StartSnapshotLoop to just call s.conversation.Start(ctx) - Remove redundant goroutine, ticker, and initial prompt send logic The snapshot loop and initial prompt handling are now internalized in PTYConversation.Start(), which calls the OnSnapshot callback after each snapshot. --- lib/httpapi/server.go | 48 +++++++++++++++---------------------------- 1 file changed, 17 insertions(+), 31 deletions(-) diff --git a/lib/httpapi/server.go b/lib/httpapi/server.go index cb13a29..def8d11 100644 --- a/lib/httpapi/server.go +++ b/lib/httpapi/server.go @@ -244,6 +244,15 @@ func NewServer(ctx context.Context, config ServerConfig) (*Server, error) { return mf.FormatToolCall(config.AgentType, message) } + emitter := NewEventEmitter(1024) + agentType := config.AgentType + + // Format initial prompt into message parts if provided + var initialPrompt []st.MessagePart + if config.InitialPrompt != "" { + initialPrompt = FormatMessage(config.AgentType, config.InitialPrompt) + } + conversation := st.NewPTY(ctx, st.PTYConversationConfig{ AgentType: config.AgentType, AgentIO: config.Process, @@ -253,9 +262,14 @@ func NewServer(ctx context.Context, config ServerConfig) (*Server, error) { FormatMessage: formatMessage, ReadyForInitialPrompt: isAgentReadyForInitialPrompt, FormatToolCall: formatToolCall, - Logger: logger, - }, config.InitialPrompt) - emitter := NewEventEmitter(1024) + InitialPrompt: initialPrompt, + OnSnapshot: func(status st.ConversationStatus, messages []st.ConversationMessage, screen string) { + emitter.UpdateStatusAndEmitChanges(status, agentType) + emitter.UpdateMessagesAndEmitChanges(messages) + emitter.UpdateScreenAndEmitChanges(screen) + }, + Logger: logger, + }) // Create temporary directory for uploads tempDir, err := os.MkdirTemp("", "agentapi-uploads-") @@ -338,34 +352,6 @@ func sseMiddleware(ctx huma.Context, next func(huma.Context)) { func (s *Server) StartSnapshotLoop(ctx context.Context) { s.conversation.Start(ctx) - go func() { - ticker := s.clock.NewTicker(snapshotInterval) - defer ticker.Stop() - for { - currentStatus := s.conversation.Status() - - // Send initial prompt when agent becomes stable for the first time - if !s.conversation.InitialPromptSent && convertStatus(currentStatus) == AgentStatusStable { - if err := s.conversation.Send(FormatMessage(s.agentType, s.conversation.InitialPrompt)...); err != nil { - s.logger.Error("Failed to send initial prompt", "error", err) - } else { - s.conversation.InitialPromptSent = true - s.conversation.ReadyForInitialPrompt = false - currentStatus = st.ConversationStatusChanging - s.logger.Info("Initial prompt sent successfully") - } - } - s.emitter.UpdateStatusAndEmitChanges(currentStatus, s.agentType) - s.emitter.UpdateMessagesAndEmitChanges(s.conversation.Messages()) - s.emitter.UpdateScreenAndEmitChanges(s.conversation.Text()) - - select { - case <-ctx.Done(): - return - case <-ticker.C: - } - } - }() } // registerRoutes sets up all API endpoints From c80b288db33ae256830116158f9baf726082ab55 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 4 Feb 2026 11:38:46 +0000 Subject: [PATCH 03/24] Update pty_conversation tests for new NewPTY API and status behavior - Update all NewPTY calls to use new signature (config only, no initialPrompt param) - For tests needing initial prompt, use InitialPrompt config field with []MessagePart - Update tests to expect status 'changing' until InitialPromptSent is true (new behavior prevents status flip 'changing' -> 'stable' -> 'changing') - Remove direct manipulation of internal fields where possible, use Status() API - Keep minimal internal field access (InitialPromptSent) where needed for testing post-send behavior without running the Start() goroutine --- lib/screentracker/pty_conversation_test.go | 92 +++++++++------------- 1 file changed, 38 insertions(+), 54 deletions(-) diff --git a/lib/screentracker/pty_conversation_test.go b/lib/screentracker/pty_conversation_test.go index e290322..4c78e4b 100644 --- a/lib/screentracker/pty_conversation_test.go +++ b/lib/screentracker/pty_conversation_test.go @@ -40,7 +40,7 @@ func statusTest(t *testing.T, params statusTestParams) { if params.cfg.Clock == nil { params.cfg.Clock = quartz.NewReal() } - c := st.NewPTY(ctx, params.cfg, "") + c := st.NewPTY(ctx, params.cfg) assert.Equal(t, st.ConversationStatusInitializing, c.Status()) for i, step := range params.steps { @@ -147,7 +147,7 @@ func TestMessages(t *testing.T) { for _, opt := range opts { opt(&cfg) } - return st.NewPTY(context.Background(), cfg, "") + return st.NewPTY(context.Background(), cfg) } t.Run("messages are copied", func(t *testing.T) { @@ -361,19 +361,18 @@ func TestInitialPromptReadiness(t *testing.T) { ReadyForInitialPrompt: func(message string) bool { return message == "ready" }, + InitialPrompt: []st.MessagePart{st.MessagePartText{Content: "initial prompt here"}}, } - c := st.NewPTY(context.Background(), cfg, "initial prompt here") + c := st.NewPTY(context.Background(), cfg) // Fill buffer with stable snapshots, but agent is not ready c.Snapshot("loading...") // Even though screen is stable, status should be changing because agent is not ready assert.Equal(t, st.ConversationStatusChanging, c.Status()) - assert.False(t, c.ReadyForInitialPrompt) - assert.False(t, c.InitialPromptSent) }) - t.Run("agent becomes ready - status changes to stable", func(t *testing.T) { + t.Run("agent becomes ready - status stays changing until initial prompt sent", func(t *testing.T) { mClock := quartz.NewMock(t) mClock.Set(now) cfg := st.PTYConversationConfig{ @@ -384,21 +383,21 @@ func TestInitialPromptReadiness(t *testing.T) { ReadyForInitialPrompt: func(message string) bool { return message == "ready" }, + InitialPrompt: []st.MessagePart{st.MessagePartText{Content: "initial prompt here"}}, } - c := st.NewPTY(context.Background(), cfg, "initial prompt here") + c := st.NewPTY(context.Background(), cfg) // Agent not ready initially c.Snapshot("loading...") assert.Equal(t, st.ConversationStatusChanging, c.Status()) - // Agent becomes ready + // Agent becomes ready, but status stays "changing" until initial prompt is sent + // This is the new behavior - we don't flip to "stable" then back to "changing" c.Snapshot("ready") - assert.Equal(t, st.ConversationStatusStable, c.Status()) - assert.True(t, c.ReadyForInitialPrompt) - assert.False(t, c.InitialPromptSent) + assert.Equal(t, st.ConversationStatusChanging, c.Status()) }) - t.Run("ready for initial prompt lifecycle: false -> true -> false", func(t *testing.T) { + t.Run("initial prompt lifecycle - status stays changing until sent", func(t *testing.T) { mClock := quartz.NewMock(t) mClock.Set(now) agent := &testAgent{screen: "loading..."} @@ -410,35 +409,21 @@ func TestInitialPromptReadiness(t *testing.T) { ReadyForInitialPrompt: func(message string) bool { return message == "ready" }, + InitialPrompt: []st.MessagePart{st.MessagePartText{Content: "initial prompt here"}}, SkipWritingMessage: true, SkipSendMessageStatusCheck: true, } - c := st.NewPTY(context.Background(), cfg, "initial prompt here") + c := st.NewPTY(context.Background(), cfg) - // Initial state: ReadyForInitialPrompt should be false + // Initial state: status should be changing while waiting for readiness c.Snapshot("loading...") - assert.False(t, c.ReadyForInitialPrompt, "should start as false") - assert.False(t, c.InitialPromptSent) assert.Equal(t, st.ConversationStatusChanging, c.Status()) - // Agent becomes ready: ReadyForInitialPrompt should become true + // Agent becomes ready: status still "changing" until initial prompt is actually sent + // This prevents the status from flipping "changing" → "stable" → "changing" agent.screen = "ready" c.Snapshot("ready") - assert.Equal(t, st.ConversationStatusStable, c.Status()) - assert.True(t, c.ReadyForInitialPrompt, "should become true when ready") - assert.False(t, c.InitialPromptSent) - - // Send the initial prompt - assert.NoError(t, c.Send(st.MessagePartText{Content: "initial prompt here"})) - - // After sending initial prompt: ReadyForInitialPrompt should be set back to false - // (simulating what happens in the actual server code) - c.InitialPromptSent = true - c.ReadyForInitialPrompt = false - - // Verify final state - assert.False(t, c.ReadyForInitialPrompt, "should be false after initial prompt sent") - assert.True(t, c.InitialPromptSent) + assert.Equal(t, st.ConversationStatusChanging, c.Status()) }) t.Run("no initial prompt - normal status logic applies", func(t *testing.T) { @@ -452,56 +437,55 @@ func TestInitialPromptReadiness(t *testing.T) { ReadyForInitialPrompt: func(message string) bool { return false // Agent never ready }, + // No InitialPrompt set - means no need to wait for readiness } - // Empty initial prompt means no need to wait for readiness - c := st.NewPTY(context.Background(), cfg, "") + c := st.NewPTY(context.Background(), cfg) c.Snapshot("loading...") // Status should be stable because no initial prompt to wait for assert.Equal(t, st.ConversationStatusStable, c.Status()) - assert.False(t, c.ReadyForInitialPrompt) - assert.True(t, c.InitialPromptSent) // Set to true when initial prompt is empty }) - t.Run("initial prompt sent - normal status logic applies", func(t *testing.T) { + t.Run("status after initial prompt sent - normal status logic applies", func(t *testing.T) { mClock := quartz.NewMock(t) mClock.Set(now) agent := &testAgent{screen: "ready"} cfg := st.PTYConversationConfig{ Clock: mClock, SnapshotInterval: 1 * time.Second, - ScreenStabilityLength: 0, + ScreenStabilityLength: 2 * time.Second, // threshold = 3 AgentIO: agent, ReadyForInitialPrompt: func(message string) bool { return message == "ready" }, + InitialPrompt: []st.MessagePart{st.MessagePartText{Content: "initial prompt here"}}, SkipWritingMessage: true, SkipSendMessageStatusCheck: true, } - c := st.NewPTY(context.Background(), cfg, "initial prompt here") + c := st.NewPTY(context.Background(), cfg) - // First, agent becomes ready + // Fill buffer to reach stability with "ready" screen + // But since initial prompt not sent, status stays "changing" c.Snapshot("ready") - assert.Equal(t, st.ConversationStatusStable, c.Status()) - assert.True(t, c.ReadyForInitialPrompt) - assert.False(t, c.InitialPromptSent) - - // Send the initial prompt - agent.screen = "processing..." - assert.NoError(t, c.Send(st.MessagePartText{Content: "initial prompt here"})) + c.Snapshot("ready") + c.Snapshot("ready") + assert.Equal(t, st.ConversationStatusChanging, c.Status()) - // Mark initial prompt as sent (simulating what the server does) + // Simulate initial prompt being sent (this is normally done by Start() goroutine) c.InitialPromptSent = true - c.ReadyForInitialPrompt = false - // Now test that status logic works normally after initial prompt is sent + // Now that initial prompt is marked as sent, and screen is stable, status becomes stable + assert.Equal(t, st.ConversationStatusStable, c.Status()) + + // After screen changes, status becomes changing + agent.screen = "processing..." c.Snapshot("processing...") + assert.Equal(t, st.ConversationStatusChanging, c.Status()) - // Status should be stable because initial prompt was already sent - // and the readiness check is bypassed + // After screen is stable again (3 identical snapshots), status becomes stable + c.Snapshot("processing...") + c.Snapshot("processing...") assert.Equal(t, st.ConversationStatusStable, c.Status()) - assert.False(t, c.ReadyForInitialPrompt) - assert.True(t, c.InitialPromptSent) }) } From 35fed243bd77f5c2e17f8e92201ea0697e1c2f9b Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 4 Feb 2026 11:43:05 +0000 Subject: [PATCH 04/24] fix: race condition in initial prompt sending sendLocked() was failing with ErrMessageValidationChanging because statusLocked() returns ConversationStatusChanging when InitialPromptSent is false. This is a chicken-and-egg problem: we need to send the initial prompt before we can set InitialPromptSent=true. Solution: Add skipStatusCheck parameter to sendLocked() to bypass the status check for the initial prompt case. The Start() goroutine passes true to skip the check, while external Send() calls pass false to preserve the existing validation behavior. --- lib/screentracker/pty_conversation.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/screentracker/pty_conversation.go b/lib/screentracker/pty_conversation.go index cd49436..a1c9df9 100644 --- a/lib/screentracker/pty_conversation.go +++ b/lib/screentracker/pty_conversation.go @@ -173,7 +173,7 @@ func (c *PTYConversation) Start(ctx context.Context) { // Agent is ready for initial prompt - send it c.lock.Lock() if !c.InitialPromptSent && len(c.cfg.InitialPrompt) > 0 { - if err := c.sendLocked(c.cfg.InitialPrompt...); err != nil { + if err := c.sendLocked(true, c.cfg.InitialPrompt...); err != nil { c.cfg.Logger.Error("failed to send initial prompt", "error", err) } else { c.InitialPromptSent = true @@ -259,12 +259,14 @@ func (c *PTYConversation) Send(messageParts ...MessagePart) error { c.lock.Lock() defer c.lock.Unlock() - return c.sendLocked(messageParts...) + return c.sendLocked(false, messageParts...) } // sendLocked sends a message to the agent. Caller MUST hold c.lock. -func (c *PTYConversation) sendLocked(messageParts ...MessagePart) error { - if !c.cfg.SkipSendMessageStatusCheck && c.statusLocked() != ConversationStatusStable { +// skipStatusCheck bypasses the status check - used for initial prompt sending +// where status is "changing" until the prompt is sent. +func (c *PTYConversation) sendLocked(skipStatusCheck bool, messageParts ...MessagePart) error { + if !skipStatusCheck && !c.cfg.SkipSendMessageStatusCheck && c.statusLocked() != ConversationStatusStable { return ErrMessageValidationChanging } From 2c907a53fcf3fb898abd5199aad6c05d3e11c65b Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 4 Feb 2026 12:26:56 +0000 Subject: [PATCH 05/24] httpapi: replace StartSnapshotLoop with Conversation accessor Remove the StartSnapshotLoop method which only delegated to s.conversation.Start(ctx), and add a Conversation() accessor method instead. This allows callers to invoke Start() directly on the conversation. Part of refactoring to move snapshot loop logic inside PTYConversation. --- lib/httpapi/server.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/httpapi/server.go b/lib/httpapi/server.go index def8d11..0fc63e0 100644 --- a/lib/httpapi/server.go +++ b/lib/httpapi/server.go @@ -350,8 +350,9 @@ func sseMiddleware(ctx huma.Context, next func(huma.Context)) { next(ctx) } -func (s *Server) StartSnapshotLoop(ctx context.Context) { - s.conversation.Start(ctx) +// Conversation returns the underlying PTYConversation for direct access. +func (s *Server) Conversation() *st.PTYConversation { + return s.conversation } // registerRoutes sets up all API endpoints From ccec2330da9e3fa4d186db45ccfef5a0fec43063 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 4 Feb 2026 12:27:58 +0000 Subject: [PATCH 06/24] refactor(screentracker): remove redundant exported fields from PTYConversation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove exported InitialPromptSent and ReadyForInitialPrompt boolean fields from PTYConversation struct: - InitialPromptSent → initialPromptSent (unexported) - ReadyForInitialPrompt boolean removed entirely The initialPromptReady channel now handles readiness signaling entirely. When the agent is ready (detected via cfg.ReadyForInitialPrompt callback), the channel is closed and set to nil to prevent double-close. This simplifies statusLocked() by removing the intermediate boolean state and using the channel's nil state to track whether readiness was already signaled. Note: Tests will need updates to verify behavior through Status() API rather than setting internal fields directly. --- lib/screentracker/pty_conversation.go | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/lib/screentracker/pty_conversation.go b/lib/screentracker/pty_conversation.go index a1c9df9..9e1b1e4 100644 --- a/lib/screentracker/pty_conversation.go +++ b/lib/screentracker/pty_conversation.go @@ -96,10 +96,8 @@ type PTYConversation struct { // InitialPrompt is the initial prompt passed to the agent InitialPrompt string - // InitialPromptSent keeps track if the InitialPrompt has been successfully sent to the agents - InitialPromptSent bool - // ReadyForInitialPrompt keeps track if the agent is ready to accept the initial prompt - ReadyForInitialPrompt bool + // initialPromptSent keeps track if the InitialPrompt has been successfully sent to the agent + initialPromptSent bool // initialPromptReady is closed when the agent is ready to receive the initial prompt initialPromptReady chan struct{} // toolCallMessageSet keeps track of the tool calls that have been detected & logged in the current agent message @@ -124,7 +122,7 @@ func NewPTY(ctx context.Context, cfg PTYConversationConfig) *PTYConversation { Time: cfg.Clock.Now(), }, }, - InitialPromptSent: len(cfg.InitialPrompt) == 0, + initialPromptSent: len(cfg.InitialPrompt) == 0, toolCallMessageSet: make(map[string]bool), } // Initialize the channel only if we have an initial prompt to send @@ -172,11 +170,11 @@ func (c *PTYConversation) Start(ctx context.Context) { case <-initialPromptReady: // Agent is ready for initial prompt - send it c.lock.Lock() - if !c.InitialPromptSent && len(c.cfg.InitialPrompt) > 0 { + if !c.initialPromptSent && len(c.cfg.InitialPrompt) > 0 { if err := c.sendLocked(true, c.cfg.InitialPrompt...); err != nil { c.cfg.Logger.Error("failed to send initial prompt", "error", err) } else { - c.InitialPromptSent = true + c.initialPromptSent = true } } c.lock.Unlock() @@ -402,15 +400,11 @@ func (c *PTYConversation) statusLocked() ConversationStatus { // Handle initial prompt readiness: report "changing" until the prompt is sent // to avoid the status flipping "changing" → "stable" → "changing" - if !c.InitialPromptSent { - if !c.ReadyForInitialPrompt { - if len(snapshots) > 0 && c.cfg.ReadyForInitialPrompt != nil && c.cfg.ReadyForInitialPrompt(snapshots[len(snapshots)-1].screen) { - c.ReadyForInitialPrompt = true - // Signal that we're ready - close the channel if it exists - if c.initialPromptReady != nil { - close(c.initialPromptReady) - } - } + if !c.initialPromptSent { + // Check if agent is ready for initial prompt and signal if so + if c.initialPromptReady != nil && len(snapshots) > 0 && c.cfg.ReadyForInitialPrompt != nil && c.cfg.ReadyForInitialPrompt(snapshots[len(snapshots)-1].screen) { + close(c.initialPromptReady) + c.initialPromptReady = nil // Prevent double-close } // Keep returning "changing" until initial prompt is actually sent return ConversationStatusChanging From d16c552ee5558ae5052ab7bb23ef25af1e33c155 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 4 Feb 2026 12:29:06 +0000 Subject: [PATCH 07/24] refactor: use srv.Conversation().Start(ctx) instead of removed StartSnapshotLoop The StartSnapshotLoop method was removed from Server in favor of exposing a Conversation() accessor that returns the PTYConversation, which has its own Start(ctx) method. --- cmd/server/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/server/server.go b/cmd/server/server.go index 6a7fa7f..2211cbb 100644 --- a/cmd/server/server.go +++ b/cmd/server/server.go @@ -136,7 +136,7 @@ func runServer(ctx context.Context, logger *slog.Logger, argsToPass []string) er fmt.Println(srv.GetOpenAPI()) return nil } - srv.StartSnapshotLoop(ctx) + srv.Conversation().Start(ctx) logger.Info("Starting server on port", "port", port) processExitCh := make(chan error, 1) go func() { From 1fa447e964578c5080a278bed982626ceaac9a24 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 4 Feb 2026 12:30:10 +0000 Subject: [PATCH 08/24] refactor(tests): remove direct field access to InitialPromptSent The InitialPromptSent field was unexported as initialPromptSent. Rework the test to verify the same behavior (normal status logic applies after initial prompt handling) by configuring no InitialPrompt instead of manually setting the field. When no InitialPrompt is configured, initialPromptSent defaults to true, which achieves the same testing outcome through the public API. --- lib/screentracker/pty_conversation_test.go | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/lib/screentracker/pty_conversation_test.go b/lib/screentracker/pty_conversation_test.go index 4c78e4b..7fe061e 100644 --- a/lib/screentracker/pty_conversation_test.go +++ b/lib/screentracker/pty_conversation_test.go @@ -447,7 +447,9 @@ func TestInitialPromptReadiness(t *testing.T) { assert.Equal(t, st.ConversationStatusStable, c.Status()) }) - t.Run("status after initial prompt sent - normal status logic applies", func(t *testing.T) { + t.Run("no initial prompt configured - normal status logic applies", func(t *testing.T) { + // When no InitialPrompt is configured, the conversation behaves as if + // the initial prompt has already been sent, so normal status logic applies. mClock := quartz.NewMock(t) mClock.Set(now) agent := &testAgent{screen: "ready"} @@ -456,26 +458,17 @@ func TestInitialPromptReadiness(t *testing.T) { SnapshotInterval: 1 * time.Second, ScreenStabilityLength: 2 * time.Second, // threshold = 3 AgentIO: agent, - ReadyForInitialPrompt: func(message string) bool { - return message == "ready" - }, - InitialPrompt: []st.MessagePart{st.MessagePartText{Content: "initial prompt here"}}, + // No InitialPrompt configured - normal status logic applies immediately SkipWritingMessage: true, SkipSendMessageStatusCheck: true, } c := st.NewPTY(context.Background(), cfg) // Fill buffer to reach stability with "ready" screen - // But since initial prompt not sent, status stays "changing" c.Snapshot("ready") c.Snapshot("ready") c.Snapshot("ready") - assert.Equal(t, st.ConversationStatusChanging, c.Status()) - - // Simulate initial prompt being sent (this is normally done by Start() goroutine) - c.InitialPromptSent = true - - // Now that initial prompt is marked as sent, and screen is stable, status becomes stable + // Since no initial prompt is configured, screen stability determines status assert.Equal(t, st.ConversationStatusStable, c.Status()) // After screen changes, status becomes changing From 24ff1061cfa95185c3a095199851adf0c616e154 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 4 Feb 2026 12:45:31 +0000 Subject: [PATCH 09/24] refactor: move conversation.Start() into NewServer() and remove Conversation() accessor - Move the s.conversation.Start(ctx) call into NewServer(), just before the return statement, so the conversation starts immediately when the server is created. - Add nil check for config.Process to handle test scenarios where no process is configured. - Remove the Conversation() accessor method from Server since it is no longer needed externally. - Remove the external srv.Conversation().Start(ctx) call from cmd/server/server.go. --- cmd/server/server.go | 1 - lib/httpapi/server.go | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/cmd/server/server.go b/cmd/server/server.go index 2211cbb..6d5cdec 100644 --- a/cmd/server/server.go +++ b/cmd/server/server.go @@ -136,7 +136,6 @@ func runServer(ctx context.Context, logger *slog.Logger, argsToPass []string) er fmt.Println(srv.GetOpenAPI()) return nil } - srv.Conversation().Start(ctx) logger.Info("Starting server on port", "port", port) processExitCh := make(chan error, 1) go func() { diff --git a/lib/httpapi/server.go b/lib/httpapi/server.go index 0fc63e0..0ec1fd4 100644 --- a/lib/httpapi/server.go +++ b/lib/httpapi/server.go @@ -295,6 +295,11 @@ func NewServer(ctx context.Context, config ServerConfig) (*Server, error) { // Register API routes s.registerRoutes() + // Start the conversation polling loop if we have a process + if config.Process != nil { + s.conversation.Start(ctx) + } + return s, nil } @@ -350,11 +355,6 @@ func sseMiddleware(ctx huma.Context, next func(huma.Context)) { next(ctx) } -// Conversation returns the underlying PTYConversation for direct access. -func (s *Server) Conversation() *st.PTYConversation { - return s.conversation -} - // registerRoutes sets up all API endpoints func (s *Server) registerRoutes() { // GET /status endpoint From 9a197fc3a92496666cc8c850a94e5373c4c0d210 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 4 Feb 2026 12:44:42 +0000 Subject: [PATCH 10/24] refactor(screentracker): move status check from sendLocked to Send() Remove the skipStatusCheck parameter from sendLocked and move the status check into Send() where it belongs. This simplifies the code since: - Start() always skipped the check (for initial prompt) - Send() always respected cfg.SkipSendMessageStatusCheck Now the check happens in Send() before calling sendLocked, and the initial prompt in Start() naturally bypasses it by calling sendLocked directly. --- lib/screentracker/pty_conversation.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/screentracker/pty_conversation.go b/lib/screentracker/pty_conversation.go index 9e1b1e4..50854ed 100644 --- a/lib/screentracker/pty_conversation.go +++ b/lib/screentracker/pty_conversation.go @@ -171,7 +171,7 @@ func (c *PTYConversation) Start(ctx context.Context) { // Agent is ready for initial prompt - send it c.lock.Lock() if !c.initialPromptSent && len(c.cfg.InitialPrompt) > 0 { - if err := c.sendLocked(true, c.cfg.InitialPrompt...); err != nil { + if err := c.sendLocked(c.cfg.InitialPrompt...); err != nil { c.cfg.Logger.Error("failed to send initial prompt", "error", err) } else { c.initialPromptSent = true @@ -257,17 +257,15 @@ func (c *PTYConversation) Send(messageParts ...MessagePart) error { c.lock.Lock() defer c.lock.Unlock() - return c.sendLocked(false, messageParts...) -} - -// sendLocked sends a message to the agent. Caller MUST hold c.lock. -// skipStatusCheck bypasses the status check - used for initial prompt sending -// where status is "changing" until the prompt is sent. -func (c *PTYConversation) sendLocked(skipStatusCheck bool, messageParts ...MessagePart) error { - if !skipStatusCheck && !c.cfg.SkipSendMessageStatusCheck && c.statusLocked() != ConversationStatusStable { + if !c.cfg.SkipSendMessageStatusCheck && c.statusLocked() != ConversationStatusStable { return ErrMessageValidationChanging } + return c.sendLocked(messageParts...) +} + +// sendLocked sends a message to the agent. Caller MUST hold c.lock. +func (c *PTYConversation) sendLocked(messageParts ...MessagePart) error { var sb strings.Builder for _, part := range messageParts { sb.WriteString(part.String()) From 3dd8c56ec7a9d9535d8230d21627fd9255e0a5af Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 4 Feb 2026 13:03:39 +0000 Subject: [PATCH 11/24] docs: add clarifying comments addressing review feedback - Expand comment for process nil check to explain: - Process is nil only for --print-openapi mode - Process is already running (termexec.StartProcess blocks) - Agent readiness is handled asynchronously via ReadyForInitialPrompt - Add comment for OnSnapshot callback explaining: - Callback pattern keeps screentracker decoupled from httpapi - Preserves clean package boundaries and avoids import cycles --- lib/httpapi/server.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/httpapi/server.go b/lib/httpapi/server.go index 0ec1fd4..24e5e55 100644 --- a/lib/httpapi/server.go +++ b/lib/httpapi/server.go @@ -263,6 +263,9 @@ func NewServer(ctx context.Context, config ServerConfig) (*Server, error) { ReadyForInitialPrompt: isAgentReadyForInitialPrompt, FormatToolCall: formatToolCall, InitialPrompt: initialPrompt, + // OnSnapshot uses a callback rather than passing the emitter directly + // to keep the screentracker package decoupled from httpapi concerns. + // This preserves clean package boundaries and avoids import cycles. OnSnapshot: func(status st.ConversationStatus, messages []st.ConversationMessage, screen string) { emitter.UpdateStatusAndEmitChanges(status, agentType) emitter.UpdateMessagesAndEmitChanges(messages) @@ -295,7 +298,12 @@ func NewServer(ctx context.Context, config ServerConfig) (*Server, error) { // Register API routes s.registerRoutes() - // Start the conversation polling loop if we have a process + // Start the conversation polling loop if we have a process. + // Process is nil only when --print-openapi is used (no agent runs). + // The process is already running at this point - termexec.StartProcess() + // blocks until the PTY is created and the process is active. Agent + // readiness (waiting for the prompt) is handled asynchronously inside + // conversation.Start() via ReadyForInitialPrompt. if config.Process != nil { s.conversation.Start(ctx) } From f5bd08d091ff89b57b4e6bed9f2ddad1c04dbd89 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 4 Feb 2026 15:48:36 +0000 Subject: [PATCH 12/24] refactor: use Conversation interface in Server struct Change Server.conversation from *st.PTYConversation to st.Conversation to program against the interface abstraction rather than the concrete type. This ensures the Conversation interface is a complete abstraction. --- lib/httpapi/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/httpapi/server.go b/lib/httpapi/server.go index 24e5e55..b14510f 100644 --- a/lib/httpapi/server.go +++ b/lib/httpapi/server.go @@ -41,7 +41,7 @@ type Server struct { srv *http.Server mu sync.RWMutex logger *slog.Logger - conversation *st.PTYConversation + conversation st.Conversation agentio *termexec.Process agentType mf.AgentType emitter *EventEmitter From 86c9d91fd40ec4ff64ce2b594391cc070fc96f8c Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 4 Feb 2026 15:51:21 +0000 Subject: [PATCH 13/24] refactor: remove unnecessary agentType local variable Use config.AgentType directly in the OnSnapshot closure instead of creating a redundant local variable. --- lib/httpapi/server.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/httpapi/server.go b/lib/httpapi/server.go index b14510f..e43315b 100644 --- a/lib/httpapi/server.go +++ b/lib/httpapi/server.go @@ -245,7 +245,6 @@ func NewServer(ctx context.Context, config ServerConfig) (*Server, error) { } emitter := NewEventEmitter(1024) - agentType := config.AgentType // Format initial prompt into message parts if provided var initialPrompt []st.MessagePart @@ -267,7 +266,7 @@ func NewServer(ctx context.Context, config ServerConfig) (*Server, error) { // to keep the screentracker package decoupled from httpapi concerns. // This preserves clean package boundaries and avoids import cycles. OnSnapshot: func(status st.ConversationStatus, messages []st.ConversationMessage, screen string) { - emitter.UpdateStatusAndEmitChanges(status, agentType) + emitter.UpdateStatusAndEmitChanges(status, config.AgentType) emitter.UpdateMessagesAndEmitChanges(messages) emitter.UpdateScreenAndEmitChanges(screen) }, From b5437f748623084e595a169f061ff8970a51e9fb Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 4 Feb 2026 16:02:12 +0000 Subject: [PATCH 14/24] refactor: simplify nil channel handling in Start() Remove unnecessary channel creation for nil initialPromptReady. In Go's select statement, nil channel cases are simply skipped (never selected), so we don't need to create a new channel that blocks forever - the nil channel already has the desired behavior. Addresses PR review feedback. --- lib/screentracker/pty_conversation.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/screentracker/pty_conversation.go b/lib/screentracker/pty_conversation.go index 50854ed..2295be3 100644 --- a/lib/screentracker/pty_conversation.go +++ b/lib/screentracker/pty_conversation.go @@ -137,12 +137,8 @@ func (c *PTYConversation) Start(ctx context.Context) { ticker := c.cfg.Clock.NewTicker(c.cfg.SnapshotInterval) defer ticker.Stop() - // Create a nil channel if no initial prompt - select will never receive from it + // If nil, the select case below is simply never chosen (nil channels are skipped in select) initialPromptReady := c.initialPromptReady - if initialPromptReady == nil { - initialPromptReady = make(chan struct{}) - // Don't close it - we want it to block forever - } for { select { From 593b65f91202dcb4de798bfb84b6099ba8df8e88 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 4 Feb 2026 16:24:25 +0000 Subject: [PATCH 15/24] refactor: remove unused InitialPrompt field from PTYConversation The public InitialPrompt string field is no longer used after refactoring. The initial prompt is now stored in cfg.InitialPrompt (as []MessagePart) and managed internally. Removing this field avoids confusion and maintains clean encapsulation. Addresses PR review feedback. --- lib/screentracker/pty_conversation.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/screentracker/pty_conversation.go b/lib/screentracker/pty_conversation.go index 2295be3..b386214 100644 --- a/lib/screentracker/pty_conversation.go +++ b/lib/screentracker/pty_conversation.go @@ -94,9 +94,7 @@ type PTYConversation struct { screenBeforeLastUserMessage string lock sync.Mutex - // InitialPrompt is the initial prompt passed to the agent - InitialPrompt string - // initialPromptSent keeps track if the InitialPrompt has been successfully sent to the agent + // initialPromptSent keeps track if the initial prompt has been successfully sent to the agent initialPromptSent bool // initialPromptReady is closed when the agent is ready to receive the initial prompt initialPromptReady chan struct{} From 6079777c4860a4eed4433caf825e054e6149e2df Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 5 Feb 2026 11:39:35 +0000 Subject: [PATCH 16/24] refactor: replace initialPromptReady/Sent with outbound message queue Replace initialPromptSent bool and initialPromptReady chan with: - outboundQueue chan []MessagePart (buffered, size 1) - agentReady chan struct{} (nil if no initial prompt) The initial prompt is now enqueued in NewPTY() and sent via the queue in Start(). This makes the code more extensible for future queued message handling. Start() now uses a two-phase loop: - Phase 1: Wait for agentReady while still processing ticker snapshots - Phase 2: Normal loop with ticker + outboundQueue select cases No external API behavior changes. --- lib/screentracker/pty_conversation.go | 80 +++++++++++++++------------ 1 file changed, 46 insertions(+), 34 deletions(-) diff --git a/lib/screentracker/pty_conversation.go b/lib/screentracker/pty_conversation.go index b386214..d4391ec 100644 --- a/lib/screentracker/pty_conversation.go +++ b/lib/screentracker/pty_conversation.go @@ -92,12 +92,13 @@ type PTYConversation struct { snapshotBuffer *RingBuffer[screenSnapshot] messages []ConversationMessage screenBeforeLastUserMessage string - lock sync.Mutex + lock sync.Mutex - // initialPromptSent keeps track if the initial prompt has been successfully sent to the agent - initialPromptSent bool - // initialPromptReady is closed when the agent is ready to receive the initial prompt - initialPromptReady chan struct{} + // outboundQueue holds messages waiting to be sent to the agent + outboundQueue chan []MessagePart + // agentReady is closed when the agent is ready to receive the initial prompt. + // It is nil if no initial prompt was configured. + agentReady chan struct{} // toolCallMessageSet keeps track of the tool calls that have been detected & logged in the current agent message toolCallMessageSet map[string]bool } @@ -120,12 +121,13 @@ func NewPTY(ctx context.Context, cfg PTYConversationConfig) *PTYConversation { Time: cfg.Clock.Now(), }, }, - initialPromptSent: len(cfg.InitialPrompt) == 0, + outboundQueue: make(chan []MessagePart, 1), toolCallMessageSet: make(map[string]bool), } - // Initialize the channel only if we have an initial prompt to send + // If we have an initial prompt, enqueue it and create the ready signal if len(cfg.InitialPrompt) > 0 { - c.initialPromptReady = make(chan struct{}) + c.agentReady = make(chan struct{}) + c.outboundQueue <- cfg.InitialPrompt } return c } @@ -135,21 +137,39 @@ func (c *PTYConversation) Start(ctx context.Context) { ticker := c.cfg.Clock.NewTicker(c.cfg.SnapshotInterval) defer ticker.Stop() - // If nil, the select case below is simply never chosen (nil channels are skipped in select) - initialPromptReady := c.initialPromptReady + // Phase 1: Wait for agent to be ready (only if we have an initial prompt) + agentReady := c.agentReady + if agentReady != nil { + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + c.lock.Lock() + screen := c.cfg.AgentIO.ReadScreen() + c.snapshotLocked(screen) + status := c.statusLocked() + messages := c.messagesLocked() + c.lock.Unlock() + + if c.cfg.OnSnapshot != nil { + c.cfg.OnSnapshot(status, messages, screen) + } + case <-agentReady: + // Agent is ready, proceed to phase 2 + agentReady = nil + goto phase2 + } + } + } + phase2: + // Phase 2: Normal loop with ticker snapshots and outbound queue processing for { select { case <-ctx.Done(): return case <-ticker.C: - // It's important that we hold the lock while reading the screen. - // There's a race condition that occurs without it: - // 1. The screen is read - // 2. Independently, Send is called and takes the lock. - // 3. snapshotLocked is called and waits on the lock. - // 4. Send modifies the terminal state, releases the lock - // 5. snapshotLocked adds a snapshot from a stale screen c.lock.Lock() screen := c.cfg.AgentIO.ReadScreen() c.snapshotLocked(screen) @@ -157,23 +177,15 @@ func (c *PTYConversation) Start(ctx context.Context) { messages := c.messagesLocked() c.lock.Unlock() - // Call snapshot callback if configured if c.cfg.OnSnapshot != nil { c.cfg.OnSnapshot(status, messages, screen) } - case <-initialPromptReady: - // Agent is ready for initial prompt - send it + case parts := <-c.outboundQueue: c.lock.Lock() - if !c.initialPromptSent && len(c.cfg.InitialPrompt) > 0 { - if err := c.sendLocked(c.cfg.InitialPrompt...); err != nil { - c.cfg.Logger.Error("failed to send initial prompt", "error", err) - } else { - c.initialPromptSent = true - } + if err := c.sendLocked(parts...); err != nil { + c.cfg.Logger.Error("failed to send queued message", "error", err) } c.lock.Unlock() - // Nil out to prevent this case from triggering again - initialPromptReady = nil } } }() @@ -390,15 +402,15 @@ func (c *PTYConversation) statusLocked() ConversationStatus { } } - // Handle initial prompt readiness: report "changing" until the prompt is sent + // Handle initial prompt readiness: report "changing" until the queue is drained // to avoid the status flipping "changing" → "stable" → "changing" - if !c.initialPromptSent { + if c.agentReady != nil || len(c.outboundQueue) > 0 { // Check if agent is ready for initial prompt and signal if so - if c.initialPromptReady != nil && len(snapshots) > 0 && c.cfg.ReadyForInitialPrompt != nil && c.cfg.ReadyForInitialPrompt(snapshots[len(snapshots)-1].screen) { - close(c.initialPromptReady) - c.initialPromptReady = nil // Prevent double-close + if c.agentReady != nil && len(snapshots) > 0 && c.cfg.ReadyForInitialPrompt != nil && c.cfg.ReadyForInitialPrompt(snapshots[len(snapshots)-1].screen) { + close(c.agentReady) + c.agentReady = nil // Prevent double-close } - // Keep returning "changing" until initial prompt is actually sent + // Keep returning "changing" until outbound queue is drained return ConversationStatusChanging } From 002fd67f650396d43334e981b83541571be398ca Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 5 Feb 2026 12:06:35 +0000 Subject: [PATCH 17/24] screentracker: use no-op defaults for optional callbacks Add no-op default functions for OnSnapshot and ReadyForInitialPrompt in NewPTY() constructor instead of checking for nil throughout the code. This removes: - Two nil checks for OnSnapshot in Start() phase 1 and phase 2 loops - One nil check for ReadyForInitialPrompt in statusLocked() --- lib/screentracker/pty_conversation.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/screentracker/pty_conversation.go b/lib/screentracker/pty_conversation.go index d4391ec..f680319 100644 --- a/lib/screentracker/pty_conversation.go +++ b/lib/screentracker/pty_conversation.go @@ -129,6 +129,13 @@ func NewPTY(ctx context.Context, cfg PTYConversationConfig) *PTYConversation { c.agentReady = make(chan struct{}) c.outboundQueue <- cfg.InitialPrompt } + // Set no-op defaults for optional callbacks + if c.cfg.OnSnapshot == nil { + c.cfg.OnSnapshot = func(ConversationStatus, []ConversationMessage, string) {} + } + if c.cfg.ReadyForInitialPrompt == nil { + c.cfg.ReadyForInitialPrompt = func(string) bool { return true } + } return c } @@ -152,9 +159,7 @@ func (c *PTYConversation) Start(ctx context.Context) { messages := c.messagesLocked() c.lock.Unlock() - if c.cfg.OnSnapshot != nil { - c.cfg.OnSnapshot(status, messages, screen) - } + c.cfg.OnSnapshot(status, messages, screen) case <-agentReady: // Agent is ready, proceed to phase 2 agentReady = nil @@ -177,9 +182,7 @@ func (c *PTYConversation) Start(ctx context.Context) { messages := c.messagesLocked() c.lock.Unlock() - if c.cfg.OnSnapshot != nil { - c.cfg.OnSnapshot(status, messages, screen) - } + c.cfg.OnSnapshot(status, messages, screen) case parts := <-c.outboundQueue: c.lock.Lock() if err := c.sendLocked(parts...); err != nil { @@ -406,7 +409,7 @@ func (c *PTYConversation) statusLocked() ConversationStatus { // to avoid the status flipping "changing" → "stable" → "changing" if c.agentReady != nil || len(c.outboundQueue) > 0 { // Check if agent is ready for initial prompt and signal if so - if c.agentReady != nil && len(snapshots) > 0 && c.cfg.ReadyForInitialPrompt != nil && c.cfg.ReadyForInitialPrompt(snapshots[len(snapshots)-1].screen) { + if c.agentReady != nil && len(snapshots) > 0 && c.cfg.ReadyForInitialPrompt(snapshots[len(snapshots)-1].screen) { close(c.agentReady) c.agentReady = nil // Prevent double-close } From a6b54a4c3afada68f4150851b564fa52828f96f2 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 5 Feb 2026 12:08:47 +0000 Subject: [PATCH 18/24] screentracker: remove duplicate agentReady nil check --- lib/screentracker/pty_conversation.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/screentracker/pty_conversation.go b/lib/screentracker/pty_conversation.go index f680319..844b51b 100644 --- a/lib/screentracker/pty_conversation.go +++ b/lib/screentracker/pty_conversation.go @@ -407,13 +407,15 @@ func (c *PTYConversation) statusLocked() ConversationStatus { // Handle initial prompt readiness: report "changing" until the queue is drained // to avoid the status flipping "changing" → "stable" → "changing" - if c.agentReady != nil || len(c.outboundQueue) > 0 { + if c.agentReady != nil { // Check if agent is ready for initial prompt and signal if so - if c.agentReady != nil && len(snapshots) > 0 && c.cfg.ReadyForInitialPrompt(snapshots[len(snapshots)-1].screen) { + if len(snapshots) > 0 && c.cfg.ReadyForInitialPrompt(snapshots[len(snapshots)-1].screen) { close(c.agentReady) c.agentReady = nil // Prevent double-close } - // Keep returning "changing" until outbound queue is drained + return ConversationStatusChanging + } + if len(c.outboundQueue) > 0 { return ConversationStatusChanging } From 3259c1f78ddd657d46a77bd52131dcae67af4b1c Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 5 Feb 2026 12:24:38 +0000 Subject: [PATCH 19/24] screentracker: remove unnecessary comment and replace goto with labeled break --- lib/screentracker/pty_conversation.go | 37 ++++++++++++--------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/lib/screentracker/pty_conversation.go b/lib/screentracker/pty_conversation.go index 844b51b..fde50d0 100644 --- a/lib/screentracker/pty_conversation.go +++ b/lib/screentracker/pty_conversation.go @@ -129,7 +129,6 @@ func NewPTY(ctx context.Context, cfg PTYConversationConfig) *PTYConversation { c.agentReady = make(chan struct{}) c.outboundQueue <- cfg.InitialPrompt } - // Set no-op defaults for optional callbacks if c.cfg.OnSnapshot == nil { c.cfg.OnSnapshot = func(ConversationStatus, []ConversationMessage, string) {} } @@ -146,29 +145,25 @@ func (c *PTYConversation) Start(ctx context.Context) { // Phase 1: Wait for agent to be ready (only if we have an initial prompt) agentReady := c.agentReady - if agentReady != nil { - for { - select { - case <-ctx.Done(): - return - case <-ticker.C: - c.lock.Lock() - screen := c.cfg.AgentIO.ReadScreen() - c.snapshotLocked(screen) - status := c.statusLocked() - messages := c.messagesLocked() - c.lock.Unlock() - - c.cfg.OnSnapshot(status, messages, screen) - case <-agentReady: - // Agent is ready, proceed to phase 2 - agentReady = nil - goto phase2 - } + phase1: + for agentReady != nil { + select { + case <-ctx.Done(): + return + case <-ticker.C: + c.lock.Lock() + screen := c.cfg.AgentIO.ReadScreen() + c.snapshotLocked(screen) + status := c.statusLocked() + messages := c.messagesLocked() + c.lock.Unlock() + + c.cfg.OnSnapshot(status, messages, screen) + case <-agentReady: + break phase1 } } - phase2: // Phase 2: Normal loop with ticker snapshots and outbound queue processing for { select { From 54b8ef4fec18c4e21f3a73c6aa6b19b7145b0a5f Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 5 Feb 2026 14:44:01 +0000 Subject: [PATCH 20/24] refactor: use separate snapshot/send goroutines with stableSignal channel Replace the two-phase loop design in PTYConversation.Start() with two independent goroutines coordinated by an unbuffered stableSignal channel: - Snapshot loop: takes periodic snapshots and signals when stable with queued items - Send loop: waits for stable signal, then processes the outbound queue This replaces the agentReady channel mechanism with a simpler coordination pattern that eliminates duplicated snapshot logic. --- lib/screentracker/pty_conversation.go | 59 ++++++++++++--------------- 1 file changed, 25 insertions(+), 34 deletions(-) diff --git a/lib/screentracker/pty_conversation.go b/lib/screentracker/pty_conversation.go index fde50d0..d734f2d 100644 --- a/lib/screentracker/pty_conversation.go +++ b/lib/screentracker/pty_conversation.go @@ -96,9 +96,9 @@ type PTYConversation struct { // outboundQueue holds messages waiting to be sent to the agent outboundQueue chan []MessagePart - // agentReady is closed when the agent is ready to receive the initial prompt. - // It is nil if no initial prompt was configured. - agentReady chan struct{} + // stableSignal is used by the snapshot loop to signal the send loop + // when the agent is stable and there are items in the outbound queue. + stableSignal chan struct{} // toolCallMessageSet keeps track of the tool calls that have been detected & logged in the current agent message toolCallMessageSet map[string]bool } @@ -122,11 +122,11 @@ func NewPTY(ctx context.Context, cfg PTYConversationConfig) *PTYConversation { }, }, outboundQueue: make(chan []MessagePart, 1), + stableSignal: make(chan struct{}), toolCallMessageSet: make(map[string]bool), } - // If we have an initial prompt, enqueue it and create the ready signal + // If we have an initial prompt, enqueue it if len(cfg.InitialPrompt) > 0 { - c.agentReady = make(chan struct{}) c.outboundQueue <- cfg.InitialPrompt } if c.cfg.OnSnapshot == nil { @@ -139,14 +139,12 @@ func NewPTY(ctx context.Context, cfg PTYConversationConfig) *PTYConversation { } func (c *PTYConversation) Start(ctx context.Context) { + // Snapshot loop go func() { ticker := c.cfg.Clock.NewTicker(c.cfg.SnapshotInterval) defer ticker.Stop() - // Phase 1: Wait for agent to be ready (only if we have an initial prompt) - agentReady := c.agentReady - phase1: - for agentReady != nil { + for { select { case <-ctx.Done(): return @@ -159,31 +157,32 @@ func (c *PTYConversation) Start(ctx context.Context) { c.lock.Unlock() c.cfg.OnSnapshot(status, messages, screen) - case <-agentReady: - break phase1 + + // Signal send loop if stable and queue has items + if status == ConversationStatusStable && len(c.outboundQueue) > 0 { + c.stableSignal <- struct{}{} + } } } + }() - // Phase 2: Normal loop with ticker snapshots and outbound queue processing + // Send loop + go func() { for { select { case <-ctx.Done(): return - case <-ticker.C: - c.lock.Lock() - screen := c.cfg.AgentIO.ReadScreen() - c.snapshotLocked(screen) - status := c.statusLocked() - messages := c.messagesLocked() - c.lock.Unlock() - - c.cfg.OnSnapshot(status, messages, screen) - case parts := <-c.outboundQueue: - c.lock.Lock() - if err := c.sendLocked(parts...); err != nil { - c.cfg.Logger.Error("failed to send queued message", "error", err) + case <-c.stableSignal: + select { + case parts := <-c.outboundQueue: + c.lock.Lock() + if err := c.sendLocked(parts...); err != nil { + c.cfg.Logger.Error("failed to send queued message", "error", err) + } + c.lock.Unlock() + default: + c.cfg.Logger.Error("received stable signal but outbound queue is empty") } - c.lock.Unlock() } } }() @@ -402,14 +401,6 @@ func (c *PTYConversation) statusLocked() ConversationStatus { // Handle initial prompt readiness: report "changing" until the queue is drained // to avoid the status flipping "changing" → "stable" → "changing" - if c.agentReady != nil { - // Check if agent is ready for initial prompt and signal if so - if len(snapshots) > 0 && c.cfg.ReadyForInitialPrompt(snapshots[len(snapshots)-1].screen) { - close(c.agentReady) - c.agentReady = nil // Prevent double-close - } - return ConversationStatusChanging - } if len(c.outboundQueue) > 0 { return ConversationStatusChanging } From b157ff795a3b7445b8e591ba1978930b3837db74 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 5 Feb 2026 15:52:27 +0000 Subject: [PATCH 21/24] e2e: always rebuild binary when AGENTAPI_BINARY_PATH is not set Previously the test only built the binary if it didn't exist, causing stale binary issues when testing changes. Now the binary is always rebuilt unless AGENTAPI_BINARY_PATH is set to use a custom binary. --- e2e/echo_test.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/e2e/echo_test.go b/e2e/echo_test.go index fbc1efc..861fb42 100644 --- a/e2e/echo_test.go +++ b/e2e/echo_test.go @@ -133,14 +133,11 @@ func setup(ctx context.Context, t testing.TB, p *params) ([]ScriptEntry, *agenta cwd, err := os.Getwd() require.NoError(t, err, "Failed to get current working directory") binaryPath = filepath.Join(cwd, "..", "out", "agentapi") - _, err = os.Stat(binaryPath) - if err != nil { - t.Logf("Building binary at %s", binaryPath) - buildCmd := exec.CommandContext(ctx, "go", "build", "-o", binaryPath, ".") - buildCmd.Dir = filepath.Join(cwd, "..") - t.Logf("run: %s", buildCmd.String()) - require.NoError(t, buildCmd.Run(), "Failed to build binary") - } + t.Logf("Building binary at %s", binaryPath) + buildCmd := exec.CommandContext(ctx, "go", "build", "-o", binaryPath, ".") + buildCmd.Dir = filepath.Join(cwd, "..") + t.Logf("run: %s", buildCmd.String()) + require.NoError(t, buildCmd.Run(), "Failed to build binary") } serverPort, err := getFreePort() From 7d488f39b612a0b50b29834d7f8415b05dd7dd98 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 5 Feb 2026 15:53:00 +0000 Subject: [PATCH 22/24] Fix stableSignal deadlock in pty_conversation.go The previous implementation had a deadlock condition: - Snapshot loop signals stableSignal when status == Stable && queue > 0 - But statusLocked() returns 'changing' when queue > 0 - These conditions were mutually exclusive Fixed by: 1. Adding isScreenStableLocked() helper to check screen stability independently 2. Using the helper in statusLocked() (refactor, same behavior) 3. Making stableSignal a buffered channel (size 1) to allow non-blocking sends 4. Updating snapshot loop to check screen stability and ReadyForInitialPrompt independently of statusLocked(), with non-blocking send to avoid deadlock --- lib/screentracker/pty_conversation.go | 41 ++++++++++++++++++++------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/lib/screentracker/pty_conversation.go b/lib/screentracker/pty_conversation.go index d734f2d..1f810c8 100644 --- a/lib/screentracker/pty_conversation.go +++ b/lib/screentracker/pty_conversation.go @@ -122,7 +122,7 @@ func NewPTY(ctx context.Context, cfg PTYConversationConfig) *PTYConversation { }, }, outboundQueue: make(chan []MessagePart, 1), - stableSignal: make(chan struct{}), + stableSignal: make(chan struct{}, 1), toolCallMessageSet: make(map[string]bool), } // If we have an initial prompt, enqueue it @@ -154,14 +154,20 @@ func (c *PTYConversation) Start(ctx context.Context) { c.snapshotLocked(screen) status := c.statusLocked() messages := c.messagesLocked() + + // Signal send loop if agent is ready and queue has items. + // We check readiness independently of statusLocked() because + // statusLocked() returns "changing" when queue has items. + if len(c.outboundQueue) > 0 && c.isScreenStableLocked() && c.cfg.ReadyForInitialPrompt(screen) { + select { + case c.stableSignal <- struct{}{}: + default: + // Signal already pending + } + } c.lock.Unlock() c.cfg.OnSnapshot(status, messages, screen) - - // Signal send loop if stable and queue has items - if status == ConversationStatusStable && len(c.outboundQueue) > 0 { - c.stableSignal <- struct{}{} - } } } }() @@ -371,6 +377,21 @@ func (c *PTYConversation) Status() ConversationStatus { return c.statusLocked() } +// isScreenStableLocked returns true if the screen content has been stable +// for the required number of snapshots. Caller MUST hold c.lock. +func (c *PTYConversation) isScreenStableLocked() bool { + snapshots := c.snapshotBuffer.GetAll() + if len(snapshots) != c.stableSnapshotsThreshold { + return false + } + for i := 1; i < len(snapshots); i++ { + if snapshots[0].screen != snapshots[i].screen { + return false + } + } + return true +} + // caller MUST hold c.lock func (c *PTYConversation) statusLocked() ConversationStatus { // sanity checks @@ -393,14 +414,12 @@ func (c *PTYConversation) statusLocked() ConversationStatus { return ConversationStatusInitializing } - for i := 1; i < len(snapshots); i++ { - if snapshots[0].screen != snapshots[i].screen { - return ConversationStatusChanging - } + if !c.isScreenStableLocked() { + return ConversationStatusChanging } // Handle initial prompt readiness: report "changing" until the queue is drained - // to avoid the status flipping "changing" → "stable" → "changing" + // to avoid the status flipping "changing" -> "stable" -> "changing" if len(c.outboundQueue) > 0 { return ConversationStatusChanging } From f725683f4f419027a9845fd676f50a43da77c349 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 5 Feb 2026 16:13:38 +0000 Subject: [PATCH 23/24] Optimize ReadyForInitialPrompt check and fix stability comparison - Use separate goroutine to poll ReadyForInitialPrompt instead of checking on every snapshot tick (reduces overhead) - Use atomic.Bool to track when initial prompt readiness is achieved - Change isScreenStableLocked() to use < instead of != for robustness --- lib/screentracker/pty_conversation.go | 29 +++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/lib/screentracker/pty_conversation.go b/lib/screentracker/pty_conversation.go index 1f810c8..e5fd809 100644 --- a/lib/screentracker/pty_conversation.go +++ b/lib/screentracker/pty_conversation.go @@ -6,6 +6,7 @@ import ( "log/slog" "strings" "sync" + "sync/atomic" "time" "github.com/coder/agentapi/lib/msgfmt" @@ -101,6 +102,9 @@ type PTYConversation struct { stableSignal chan struct{} // toolCallMessageSet keeps track of the tool calls that have been detected & logged in the current agent message toolCallMessageSet map[string]bool + // initialPromptReady is set to true when ReadyForInitialPrompt returns true. + // This is checked by a separate goroutine to avoid calling ReadyForInitialPrompt on every tick. + initialPromptReady atomic.Bool } var _ Conversation = &PTYConversation{} @@ -139,6 +143,27 @@ func NewPTY(ctx context.Context, cfg PTYConversationConfig) *PTYConversation { } func (c *PTYConversation) Start(ctx context.Context) { + // Initial prompt readiness loop - polls ReadyForInitialPrompt until it returns true, + // then sets initialPromptReady and exits. This avoids calling ReadyForInitialPrompt + // on every snapshot tick. + go func() { + ticker := c.cfg.Clock.NewTicker(100 * time.Millisecond) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + screen := c.cfg.AgentIO.ReadScreen() + if c.cfg.ReadyForInitialPrompt(screen) { + c.initialPromptReady.Store(true) + return + } + } + } + }() + // Snapshot loop go func() { ticker := c.cfg.Clock.NewTicker(c.cfg.SnapshotInterval) @@ -158,7 +183,7 @@ func (c *PTYConversation) Start(ctx context.Context) { // Signal send loop if agent is ready and queue has items. // We check readiness independently of statusLocked() because // statusLocked() returns "changing" when queue has items. - if len(c.outboundQueue) > 0 && c.isScreenStableLocked() && c.cfg.ReadyForInitialPrompt(screen) { + if len(c.outboundQueue) > 0 && c.isScreenStableLocked() && c.initialPromptReady.Load() { select { case c.stableSignal <- struct{}{}: default: @@ -381,7 +406,7 @@ func (c *PTYConversation) Status() ConversationStatus { // for the required number of snapshots. Caller MUST hold c.lock. func (c *PTYConversation) isScreenStableLocked() bool { snapshots := c.snapshotBuffer.GetAll() - if len(snapshots) != c.stableSnapshotsThreshold { + if len(snapshots) < c.stableSnapshotsThreshold { return false } for i := 1; i < len(snapshots); i++ { From 0b9e68729308a4c0596558c64fc969a2867eb55a Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 5 Feb 2026 18:23:43 +0000 Subject: [PATCH 24/24] refactor: use channel for initialPromptReady one-time signaling Replace atomic.Bool with a channel that gets closed for one-time signaling, which is more idiomatic Go. Remove sync/atomic import since it's no longer needed. --- lib/screentracker/pty_conversation.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/screentracker/pty_conversation.go b/lib/screentracker/pty_conversation.go index e5fd809..5ae29b2 100644 --- a/lib/screentracker/pty_conversation.go +++ b/lib/screentracker/pty_conversation.go @@ -6,7 +6,6 @@ import ( "log/slog" "strings" "sync" - "sync/atomic" "time" "github.com/coder/agentapi/lib/msgfmt" @@ -102,9 +101,9 @@ type PTYConversation struct { stableSignal chan struct{} // toolCallMessageSet keeps track of the tool calls that have been detected & logged in the current agent message toolCallMessageSet map[string]bool - // initialPromptReady is set to true when ReadyForInitialPrompt returns true. + // initialPromptReady is closed when ReadyForInitialPrompt returns true. // This is checked by a separate goroutine to avoid calling ReadyForInitialPrompt on every tick. - initialPromptReady atomic.Bool + initialPromptReady chan struct{} } var _ Conversation = &PTYConversation{} @@ -128,6 +127,7 @@ func NewPTY(ctx context.Context, cfg PTYConversationConfig) *PTYConversation { outboundQueue: make(chan []MessagePart, 1), stableSignal: make(chan struct{}, 1), toolCallMessageSet: make(map[string]bool), + initialPromptReady: make(chan struct{}), } // If we have an initial prompt, enqueue it if len(cfg.InitialPrompt) > 0 { @@ -157,7 +157,7 @@ func (c *PTYConversation) Start(ctx context.Context) { case <-ticker.C: screen := c.cfg.AgentIO.ReadScreen() if c.cfg.ReadyForInitialPrompt(screen) { - c.initialPromptReady.Store(true) + close(c.initialPromptReady) return } } @@ -183,7 +183,13 @@ func (c *PTYConversation) Start(ctx context.Context) { // Signal send loop if agent is ready and queue has items. // We check readiness independently of statusLocked() because // statusLocked() returns "changing" when queue has items. - if len(c.outboundQueue) > 0 && c.isScreenStableLocked() && c.initialPromptReady.Load() { + isReady := false + select { + case <-c.initialPromptReady: + isReady = true + default: + } + if len(c.outboundQueue) > 0 && c.isScreenStableLocked() && isReady { select { case c.stableSignal <- struct{}{}: default: