mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 14:17:40 +00:00
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 <bruno.clermont@gmail.com>
This commit is contained in:
1 parent
f87cdaf616
commit
cde8940501
1 file changed
+12
-6
@@ -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",
|
||||
|
||||
Reference in new issue
Block a user