Skip to content

Commit f6f14b8

Browse files
committed
fix(screentracker): fix context-cancellation test race with mock clock
The test had a race: advanceFor could complete before the Send() goroutine enqueued, so the stableSignal never fired, and sendCancel ran while the message was still queued (never reaching writeStabilize). Fix: use onWrite callback as a synchronization point. advanceUntil waits for writeStabilize to start writing (onWrite fires), then cancels. This guarantees Phase 1 WaitFor is running when ctx is cancelled, and its sleep select sees ctx.Done() immediately.
1 parent a610148 commit f6f14b8

File tree

1 file changed

+32
-8
lines changed

1 file changed

+32
-8
lines changed

lib/screentracker/pty_conversation_test.go

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -515,35 +515,59 @@ func TestMessages(t *testing.T) {
515515

516516
t.Run("send-message-no-echo-context-cancelled", func(t *testing.T) {
517517
// Given: a non-echoing agent and a cancellable context.
518+
// The onWrite signals when writeStabilize starts writing
519+
// message parts — this is used to synchronize the cancel.
518520
sendCtx, sendCancel := context.WithCancel(context.Background())
519521
t.Cleanup(sendCancel)
520522

523+
writeStarted := make(chan struct{}, 1)
521524
c, _, mClock := newConversation(sendCtx, t, func(cfg *st.PTYConversationConfig) {
522525
a := &testAgent{screen: "prompt"}
523-
a.onWrite = func(data []byte) {}
526+
a.onWrite = func(data []byte) {
527+
select {
528+
case writeStarted <- struct{}{}:
529+
default:
530+
}
531+
}
524532
cfg.AgentIO = a
525533
})
526534
advanceFor(sendCtx, t, mClock, interval*threshold)
527535

528536
// When: a message is sent and the context is cancelled
529-
// during Phase 1 (before echo detection completes).
537+
// during Phase 1 (after the message is written to the
538+
// PTY, before echo detection completes).
530539
var sendErr error
531540
var sendDone atomic.Bool
532541
go func() {
533542
sendErr = c.Send(st.MessagePartText{Content: "hello"})
534543
sendDone.Store(true)
535544
}()
536545

537-
// Advance past the snapshot interval so the send loop
538-
// picks up the message, then cancel the context.
539-
advanceFor(sendCtx, t, mClock, interval)
546+
// Advance tick-by-tick until writeStabilize starts
547+
// (onWrite fires). This gives the send loop goroutine
548+
// scheduling time between advances.
549+
advanceUntil(sendCtx, t, mClock, func() bool {
550+
select {
551+
case <-writeStarted:
552+
return true
553+
default:
554+
return false
555+
}
556+
})
557+
558+
// writeStabilize Phase 1 is now running. Its WaitFor is
559+
// blocked on a mock timer sleep select. Cancel: the
560+
// select sees ctx.Done() immediately.
540561
sendCancel()
541562

542-
// Wait for Send to complete (it should fail quickly
543-
// after cancellation).
563+
// WaitFor returns ctx.Err(). The errors.Is guard in
564+
// Phase 1 propagates it as fatal. Use Eventually since
565+
// the goroutine needs scheduling time.
544566
require.Eventually(t, sendDone.Load, 5*time.Second, 10*time.Millisecond)
545567

546-
// Then: the error wraps context.Canceled, not a Phase 2 error.
568+
// Then: the error wraps context.Canceled, not a Phase 2
569+
// timeout. This validates the errors.Is(WaitTimedOut)
570+
// guard.
547571
require.Error(t, sendErr)
548572
assert.ErrorIs(t, sendErr, context.Canceled)
549573
assert.NotContains(t, sendErr.Error(), "failed to wait for processing to start")

0 commit comments

Comments
 (0)