From 744685d68da4f7d535ea976391b4efa9db92a774 Mon Sep 17 00:00:00 2001 From: patriceckhart Date: Sun, 19 Apr 2026 15:27:09 +0200 Subject: [PATCH] fix(extensions): close ext.commands / ext.tools / lastFrameTime races CI's `go test -race` flagged two races introduced over the recent extension work: 1. ext.commands / ext.tools were appended to from the read-loop goroutine without a lock, while Commands() / Tools() / HasCommand read them under m.mu. Same for the toolIndex pre-check. Fix: take m.mu around the appends and the index dedup so writers and readers serialise on the same lock. 2. assumeReadyAfterIdle read ext.lastFrameTime once on entry without the per-extension lock (the read inside the loop already had it). Fix: take ext.mu for the initial snapshot too. Verified locally with `go test -race ./...`; all packages pass. The corresponding CI run for the scratchpad-persistence commit failed for exactly these two races on linux + macos. --- internal/agent/extensions/manager.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/agent/extensions/manager.go b/internal/agent/extensions/manager.go index ff77c7c..81a7b0e 100644 --- a/internal/agent/extensions/manager.go +++ b/internal/agent/extensions/manager.go @@ -448,7 +448,9 @@ func (m *Manager) spawn(ctx context.Context, ext *Extension) error { const readyIdleWindow = 250 * time.Millisecond func (m *Manager) assumeReadyAfterIdle(ext *Extension) { + ext.mu.Lock() last := ext.lastFrameTime + ext.mu.Unlock() for { select { case <-ext.readyCh: @@ -507,8 +509,12 @@ func (m *Manager) readLoop(ext *Extension, scanner *bufio.Scanner) { case "register_command": var rc extproto.RegisterCommandFromExt if err := json.Unmarshal(line, &rc); err == nil { - ext.commands = append(ext.commands, rc) + // Both ext.commands and m.commandIndex are read by + // the public Commands() / HasCommand() helpers under + // m.mu, so the writes have to take the same lock to + // keep the race detector happy. m.mu.Lock() + ext.commands = append(ext.commands, rc) if _, exists := m.commandIndex[rc.Name]; !exists { m.commandIndex[rc.Name] = ext } @@ -529,8 +535,8 @@ func (m *Manager) readLoop(ext *Extension, scanner *bufio.Scanner) { continue } } - ext.tools = append(ext.tools, rt) m.mu.Lock() + ext.tools = append(ext.tools, rt) if _, exists := m.toolIndex[rt.Name]; !exists { m.toolIndex[rt.Name] = ext }