From ca4902c990b90bbc2db5aa730a6d388e900d5b96 Mon Sep 17 00:00:00 2001 From: 2xd7 Date: Sat, 7 Mar 2026 22:01:04 +0400 Subject: [PATCH 1/4] test: add goroutine leak detection test --- chat/types_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/chat/types_test.go b/chat/types_test.go index 248e7f9..93d2317 100644 --- a/chat/types_test.go +++ b/chat/types_test.go @@ -5,6 +5,8 @@ import ( "sync" "testing" "time" + + "go.uber.org/goleak" ) // ==================== Messages Tests ==================== @@ -455,3 +457,19 @@ func TestApproveWaiter_Wait_InnerCtxDone(t *testing.T) { t.Fatal("timeout waiting for worker to finish after cancel") } } + +func TestApproveWaiter_ResolveLeaksGoroutine(t *testing.T) { + defer goleak.VerifyNone(t) + + aw := NewApproveWaiter() + + _, cancel := context.WithCancel(context.Background()) + + // Calling Resolve without a reader — goroutine will be spawned via resolveSync + aw.Resolve(Verdict{Accepted: true}) + + // Cancelling context — goroutine should exit via ctx.Done() + cancel() + + // If goroutine leaked, goleak.VerifyNone will catch it and fail the test +} From a1dfc96dc668d5db5f98e7cd27c1979276781b7d Mon Sep 17 00:00:00 2001 From: 2xd7 Date: Sat, 7 Mar 2026 22:01:20 +0400 Subject: [PATCH 2/4] chore: add goleak dependency --- go.mod | 1 + go.sum | 2 ++ 2 files changed, 3 insertions(+) diff --git a/go.mod b/go.mod index f7f8f92..460541f 100644 --- a/go.mod +++ b/go.mod @@ -18,4 +18,5 @@ require ( github.com/tidwall/match v1.1.1 // indirect github.com/tidwall/pretty v1.2.1 // indirect github.com/tidwall/sjson v1.2.5 // indirect + go.uber.org/goleak v1.3.0 ) diff --git a/go.sum b/go.sum index 80e75e9..4e5ebd5 100644 --- a/go.sum +++ b/go.sum @@ -27,6 +27,8 @@ github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY= github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28= github.com/wk8/go-ordered-map/v2 v2.1.8 h1:5h/BUHu93oj4gIdvHHHGsScSTMijfx5PeYkE/fJgbpc= github.com/wk8/go-ordered-map/v2 v2.1.8/go.mod h1:5nJHM5DyteebpVlHnWMV0rPz6Zp7+xBAnxjb1X5vnTw= +go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= +go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= From 907e8b423099bb7e7c0bfbae8541debf8c51c341 Mon Sep 17 00:00:00 2001 From: 2xd7 Date: Sat, 7 Mar 2026 23:16:52 +0400 Subject: [PATCH 3/4] refactor: add ctx field to ApproveWaiter --- chat/chat.go | 4 +++- chat/types.go | 4 +++- chat/types_test.go | 37 ++++++++++++++++++++----------------- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/chat/chat.go b/chat/chat.go index 289666a..46cf678 100644 --- a/chat/chat.go +++ b/chat/chat.go @@ -46,6 +46,7 @@ type sessionState struct { client Client events <-chan StreamEvent send func(StreamEvent) bool + ctx context.Context // session state variables @@ -59,7 +60,7 @@ func (s *sessionState) reset() { s.builder.Reset() s.toolCalls = s.toolCalls[:0] s.lastToolCall = nil - s.approval = NewApproveWaiter() + s.approval = NewApproveWaiter(s.ctx) } func (s *sessionState) flushLastToolCall() bool { @@ -117,6 +118,7 @@ func (c *Chat) Session(ctx context.Context, client Client) <-chan StreamEvent { client: client, events: c.Complete(ctx, client), send: send, + ctx: ctx, } state.reset() diff --git a/chat/types.go b/chat/types.go index fa35aec..4037387 100644 --- a/chat/types.go +++ b/chat/types.go @@ -74,11 +74,13 @@ type Verdict struct { type ApproveWaiter struct { verdicts chan Verdict + ctx context.Context } -func NewApproveWaiter() *ApproveWaiter { +func NewApproveWaiter(ctx context.Context) *ApproveWaiter { return &ApproveWaiter{ verdicts: make(chan Verdict), + ctx: ctx, } } diff --git a/chat/types_test.go b/chat/types_test.go index 93d2317..bef1048 100644 --- a/chat/types_test.go +++ b/chat/types_test.go @@ -129,7 +129,8 @@ func TestMessages_Snapshot_ModifyCopyDoesNotAffectOriginal(t *testing.T) { // ==================== ApproveWaiter Tests ==================== func TestNewApproveWaiter(t *testing.T) { - w := NewApproveWaiter() + ctx := context.Background() + w := NewApproveWaiter(ctx) if w == nil { t.Fatal("NewApproveWaiter returned nil") } @@ -139,9 +140,11 @@ func TestNewApproveWaiter(t *testing.T) { } func TestApproveWaiter_Attach(t *testing.T) { - w := NewApproveWaiter() event := NewEventNewToolCall("call-id", "tool-name", `{"arg": "value"}`) + ctx := context.Background() + w := NewApproveWaiter(ctx) + w.Attach(&event) // The approval field is private, but we can verify by calling Resolve @@ -150,8 +153,8 @@ func TestApproveWaiter_Attach(t *testing.T) { } func TestApproveWaiter_Wait_ZeroAmount(t *testing.T) { - w := NewApproveWaiter() ctx := context.Background() + w := NewApproveWaiter(ctx) verdicts := w.Wait(ctx, 0) @@ -167,8 +170,8 @@ func TestApproveWaiter_Wait_ZeroAmount(t *testing.T) { } func TestApproveWaiter_Wait_SingleVerdict(t *testing.T) { - w := NewApproveWaiter() ctx, cancel := context.WithCancel(context.Background()) + w := NewApproveWaiter(ctx) defer cancel() verdicts := w.Wait(ctx, 1) @@ -188,8 +191,8 @@ func TestApproveWaiter_Wait_SingleVerdict(t *testing.T) { } func TestApproveWaiter_Wait_MultipleVerdicts(t *testing.T) { - w := NewApproveWaiter() ctx, cancel := context.WithCancel(context.Background()) + w := NewApproveWaiter(ctx) defer cancel() amount := 3 @@ -213,8 +216,8 @@ func TestApproveWaiter_Wait_MultipleVerdicts(t *testing.T) { } func TestApproveWaiter_Wait_ContextCancellation(t *testing.T) { - w := NewApproveWaiter() ctx, cancel := context.WithCancel(context.Background()) + w := NewApproveWaiter(ctx) verdicts := w.Wait(ctx, 5) @@ -233,8 +236,8 @@ func TestApproveWaiter_Wait_ContextCancellation(t *testing.T) { } func TestApproveWaiter_Resolve_AcceptTrue(t *testing.T) { - w := NewApproveWaiter() ctx, cancel := context.WithCancel(context.Background()) + w := NewApproveWaiter(ctx) defer cancel() verdicts := w.Wait(ctx, 1) @@ -252,8 +255,9 @@ func TestApproveWaiter_Resolve_AcceptTrue(t *testing.T) { } func TestApproveWaiter_Resolve_AcceptFalse(t *testing.T) { - w := NewApproveWaiter() + ctx, cancel := context.WithCancel(context.Background()) + w := NewApproveWaiter(ctx) defer cancel() verdicts := w.Wait(ctx, 1) @@ -278,7 +282,8 @@ func TestEventNewToolCall_Resolve_NoApproval(t *testing.T) { } func TestEventNewToolCall_Resolve_DoubleCall(t *testing.T) { - w := NewApproveWaiter() + ctx, cancel := context.WithCancel(context.Background()) + w := NewApproveWaiter(ctx) event := NewEventNewToolCall("call-id", "tool-name", `{}`) w.Attach(&event) @@ -288,7 +293,6 @@ func TestEventNewToolCall_Resolve_DoubleCall(t *testing.T) { // Second resolve should be ignored (answered is now true) // We verify this by checking that only one verdict is received - ctx, cancel := context.WithCancel(context.Background()) defer cancel() verdicts := w.Wait(ctx, 1) @@ -311,8 +315,8 @@ func TestEventNewToolCall_Resolve_DoubleCall(t *testing.T) { } func TestApproveWaiter_Resolve_Concurrent(t *testing.T) { - w := NewApproveWaiter() ctx, cancel := context.WithCancel(context.Background()) + w := NewApproveWaiter(ctx) defer cancel() amount := 10 @@ -348,8 +352,8 @@ func TestApproveWaiter_Resolve_Concurrent(t *testing.T) { // inside the collection loop (not immediately after Wait) // This covers: case <-ctx.Done(): return inside the for loop func TestApproveWaiter_Wait_CtxDoneDuringCollection(t *testing.T) { - w := NewApproveWaiter() ctx, cancel := context.WithCancel(context.Background()) + w := NewApproveWaiter(ctx) // Channel for communication between sender and reader goroutines // sender -> ready to send next @@ -423,8 +427,8 @@ func TestApproveWaiter_Wait_CtxDoneDuringCollection(t *testing.T) { // TestApproveWaiter_Wait_InnerCtxDone tests ctx.Done() triggered inside the inner select // This covers: after receiving verdict from a.verdicts, ctx.Done() in inner select causes return func TestApproveWaiter_Wait_InnerCtxDone(t *testing.T) { - w := NewApproveWaiter() ctx, cancel := context.WithCancel(context.Background()) + w := NewApproveWaiter(ctx) defer cancel() resolved := make(chan struct{}) @@ -459,11 +463,10 @@ func TestApproveWaiter_Wait_InnerCtxDone(t *testing.T) { } func TestApproveWaiter_ResolveLeaksGoroutine(t *testing.T) { - defer goleak.VerifyNone(t) + defer goleak.VerifyNone(t, goleak.IgnoreCurrent()) - aw := NewApproveWaiter() - - _, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(context.Background()) + aw := NewApproveWaiter(ctx) // Calling Resolve without a reader — goroutine will be spawned via resolveSync aw.Resolve(Verdict{Accepted: true}) From 232cb207014c376137250590f6915b3f8ab84102 Mon Sep 17 00:00:00 2001 From: 2xd7 Date: Sat, 7 Mar 2026 23:19:05 +0400 Subject: [PATCH 4/4] fix: resolve goroutine leak on context cancellation --- chat/types.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/chat/types.go b/chat/types.go index 4037387..6be48f9 100644 --- a/chat/types.go +++ b/chat/types.go @@ -138,5 +138,9 @@ func (a *ApproveWaiter) Resolve(verdict Verdict) { // resolveSync is a blocking version of Resolve for testing purposes. // It waits until the verdict is read from the channel. func (a *ApproveWaiter) resolveSync(verdict Verdict) { - a.verdicts <- verdict + select { + case a.verdicts <- verdict: + case <-a.ctx.Done(): + return + } }