From cde894050179158ddd5bfb1045be9e0d0f8e6512 Mon Sep 17 00:00:00 2001 From: Bruno Clermont Date: Sat, 4 Jul 2026 12:57:30 -0400 Subject: [PATCH] fix(agent): fix race in TestMaybeSummarize_LogsTriggerDecisionOverThreshold (#1347) maybeSummarize spawns a background goroutine that logs via slog.Info before calling the LLM provider -- correct, intentional async design, not a bug. The test read the shared log buffer (buf.String(), inside captureSlog) immediately after maybeSummarize returned, i.e. right after the goroutine was merely spawned, not after it finished logging -- an unsynchronized concurrent read/write on bytes.Buffer, caught by go test -race. This was surfaced by an unrelated PR's CI run (nextlevelbuilder/goclaw#1346), confirmed unrelated to that PR's actual changes. Moved the existing done-channel wait inside captureSlog's closure, so the buffer read only happens after the background goroutine has signalled it reached provider.Chat -- which happens strictly after the slog.Info write on this path. Proper happens-before via the channel, no sleep/poll, no production code changed. Verified 10/10 passes under go test -race -count=10, plus full package regression run clean. Co-authored-by: Bruno Clermont --- .../loop_history_sanitize_max_tokens_test.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/internal/agent/loop_history_sanitize_max_tokens_test.go b/internal/agent/loop_history_sanitize_max_tokens_test.go index 99d2df1b..79c23f36 100644 --- a/internal/agent/loop_history_sanitize_max_tokens_test.go +++ b/internal/agent/loop_history_sanitize_max_tokens_test.go @@ -234,13 +234,19 @@ func TestMaybeSummarize_LogsTriggerDecisionOverThreshold(t *testing.T) { logs := captureSlog(t, func() { loop.maybeSummarize(context.Background(), "test-session-key") - }) - select { - case <-done: - case <-time.After(5 * time.Second): - t.Fatal("timed out waiting for maybeSummarize to call provider.Chat") - } + // Wait for the background summarize goroutine to reach the point where it + // calls provider.Chat (which happens strictly after the "compact_budget" + // slog.Info write). This must happen INSIDE the captureSlog closure so the + // buffer is not read until the background goroutine's logging is done — + // otherwise buf.String() races with the goroutine's concurrent slog.Info + // write to the same bytes.Buffer under -race. + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for maybeSummarize to call provider.Chat") + } + }) assertLogContains(t, logs, "compaction_decision",