Skip to content

Commit 52f0a23

Browse files
committed
fix: add close(errCh) comments and sendingMessage race fix
Add comments explaining why close(msg.errCh) is called at both sites. Add sendingMessage flag to prevent statusLocked() from returning 'stable' while the send loop is processing a dequeued message outside the lock.
1 parent 2b64027 commit 52f0a23

1 file changed

Lines changed: 13 additions & 1 deletion

File tree

lib/screentracker/pty_conversation.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,10 @@ type PTYConversation struct {
9999
// layer holds s.mu, and Send blocks until the message is processed),
100100
// so ordering is preserved.
101101
outboundQueue chan outboundMessage
102+
// sendingMessage is true while the send loop is processing a message.
103+
// Set under lock in the snapshot loop when signaling, cleared under
104+
// lock in the send loop after sendMessage returns.
105+
sendingMessage bool
102106
// stableSignal is used by the snapshot loop to signal the send loop
103107
// when the agent is stable and there are items in the outbound queue.
104108
stableSignal chan struct{}
@@ -162,6 +166,7 @@ func (c *PTYConversation) Start(ctx context.Context) {
162166
if c.initialPromptReady && len(c.outboundQueue) > 0 && c.isScreenStableLocked() {
163167
select {
164168
case c.stableSignal <- struct{}{}:
169+
c.sendingMessage = true
165170
default:
166171
// Signal already pending
167172
}
@@ -181,6 +186,8 @@ func (c *PTYConversation) Start(ctx context.Context) {
181186
case msg := <-c.outboundQueue:
182187
if msg.errCh != nil {
183188
msg.errCh <- ctx.Err()
189+
// Close so the Send() caller's <-errCh never blocks
190+
// if it has already consumed the error value.
184191
close(msg.errCh)
185192
}
186193
default:
@@ -198,8 +205,13 @@ func (c *PTYConversation) Start(ctx context.Context) {
198205
return
199206
case msg := <-c.outboundQueue:
200207
err := c.sendMessage(ctx, msg.parts...)
208+
c.lock.Lock()
209+
c.sendingMessage = false
210+
c.lock.Unlock()
201211
if msg.errCh != nil {
202212
msg.errCh <- err
213+
// Close so the Send() caller's <-errCh never blocks
214+
// if it has already consumed the error value.
203215
close(msg.errCh)
204216
}
205217
default:
@@ -448,7 +460,7 @@ func (c *PTYConversation) statusLocked() ConversationStatus {
448460

449461
// Handle initial prompt readiness: report "changing" until the queue is drained
450462
// to avoid the status flipping "changing" -> "stable" -> "changing"
451-
if len(c.outboundQueue) > 0 {
463+
if len(c.outboundQueue) > 0 || c.sendingMessage {
452464
return ConversationStatusChanging
453465
}
454466

0 commit comments

Comments
 (0)