Skip to content

Commit 587fbba

Browse files
committed
fix: use clock.NewTimer instead of time.After to avoid resource leak
1 parent a33d95e commit 587fbba

File tree

2 files changed

+30
-6
lines changed

2 files changed

+30
-6
lines changed

x/acpio/acp_conversation.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,12 @@ func (c *ACPConversation) executePrompt(messageParts []st.MessagePart) error {
238238

239239
// The ACP SDK dispatches SessionUpdate notifications as goroutines, so
240240
// the chunk may arrive after conn.Prompt() returns. Wait up to 100ms.
241+
timer := c.clock.NewTimer(100 * time.Millisecond)
241242
select {
242243
case <-c.chunkReceived:
243-
case <-time.After(100 * time.Millisecond):
244+
case <-timer.C:
244245
}
246+
timer.Stop()
245247

246248
c.mu.Lock()
247249
c.prompting = false

x/acpio/acp_conversation_test.go

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,9 @@ func Test_Send_AddsUserMessage(t *testing.T) {
227227
assert.Equal(t, "hello", messages[0].Message)
228228
assert.Equal(t, screentracker.ConversationRoleAgent, messages[1].Role)
229229

230+
// Signal a chunk so executePrompt's timer wait doesn't hang on the mock clock.
231+
mock.SimulateChunks("hello response")
232+
230233
// Unblock the write to let Send complete
231234
close(done)
232235
require.NoError(t, <-errCh)
@@ -290,6 +293,9 @@ func Test_Send_RejectsDuplicateSend(t *testing.T) {
290293
err := conv.Send(screentracker.MessagePartText{Content: "second"})
291294
assert.ErrorIs(t, err, screentracker.ErrMessageValidationChanging)
292295

296+
// Signal a chunk so executePrompt's timer wait doesn't hang on the mock clock.
297+
mock.SimulateChunks("first response")
298+
293299
// Unblock the write to let the test complete cleanly
294300
close(done)
295301
require.NoError(t, <-errCh)
@@ -318,6 +324,9 @@ func Test_Status_ChangesWhileProcessing(t *testing.T) {
318324
// Status should be changing while processing
319325
assert.Equal(t, screentracker.ConversationStatusChanging, conv.Status())
320326

327+
// Signal a chunk so executePrompt's timer wait doesn't hang on the mock clock.
328+
mock.SimulateChunks("test response")
329+
321330
// Unblock the write
322331
close(done)
323332

@@ -428,6 +437,9 @@ func Test_InitialPrompt_SentOnStart(t *testing.T) {
428437
assert.Equal(t, screentracker.ConversationRoleUser, messages[0].Role)
429438
assert.Equal(t, "initial prompt", messages[0].Message)
430439

440+
// Signal a chunk so executePrompt's timer wait doesn't hang on the mock clock.
441+
mock.SimulateChunks("initial response")
442+
431443
// Unblock the write to let the test complete cleanly
432444
close(done)
433445
}
@@ -457,6 +469,9 @@ func Test_Messages_AreCopied(t *testing.T) {
457469
originalMessages := conv.Messages()
458470
assert.Equal(t, "test", originalMessages[0].Message)
459471

472+
// Signal a chunk so executePrompt's timer wait doesn't hang on the mock clock.
473+
mock.SimulateChunks("test response")
474+
460475
// Unblock the write to let Send complete
461476
close(done)
462477
require.NoError(t, <-errCh)
@@ -518,12 +533,15 @@ func Test_ErrorRemovesPartialMessage(t *testing.T) {
518533
// Send a second message — IDs must not reuse the removed agent message's ID (1).
519534
mock.mu.Lock()
520535
mock.writeErr = nil
521-
mock.writeBlock = nil
522-
mock.writeStarted = nil
523536
mock.mu.Unlock()
524-
525-
err := conv.Send(screentracker.MessagePartText{Content: "retry"})
526-
require.NoError(t, err)
537+
started2, done2 := mock.BlockWrite()
538+
errCh2 := make(chan error, 1)
539+
go func() { errCh2 <- conv.Send(screentracker.MessagePartText{Content: "retry"}) }()
540+
<-started2
541+
// Signal a chunk so executePrompt's timer wait doesn't hang on the mock clock.
542+
mock.SimulateChunks("retry response")
543+
close(done2)
544+
require.NoError(t, <-errCh2)
527545

528546
messages = conv.Messages()
529547
require.Len(t, messages, 3, "first user + second user + second agent")
@@ -548,6 +566,10 @@ func Test_LateChunkAfterError_DoesNotCorruptUserMessage(t *testing.T) {
548566
mock.mu.Lock()
549567
mock.writeErr = assert.AnError
550568
mock.mu.Unlock()
569+
570+
// Signal a chunk before unblocking; the error path still waits on chunkReceived
571+
// or the timer, so pre-signaling avoids a hang on the mock clock.
572+
mock.SimulateChunks("unexpected chunk")
551573
close(done)
552574

553575
require.ErrorIs(t, <-errCh, assert.AnError)

0 commit comments

Comments
 (0)