perf: parallelize toolset startup and prefetch MCP catalog

[dgageot]

Profiling examples/gopher.yaml on Apple Silicon revealed that agent startup blocked sequentially on MCP handshakes: every toolset was started one after another, so total startup time was the sum of all handshake latencies. For a sub-agent like librarian that reaches three MCP servers, that summed to around 5.2 s on a warm cache.

This PR attacks that sequential work at every layer. gateway.Prefetch() kicks off the memoized catalog fetch in the background as soon as a config with ref: toolsets is loaded, so the network round-trip overlaps model and environment resolution instead of blocking LoadTeam. Inside EmitStartupInfo, agent.Run, and tools/list, toolsets are now started and queried concurrently using a new concurrent.MapSlice fan-out helper that replaces five hand-rolled results-slice + WaitGroup patterns (including pre-existing ones in pkg/hooks/executor.go and pkg/runtime/toolexec/dispatcher.go). Merged tool lists remain in configuration order so dedup, collision detection, and warning semantics are unchanged.

Two correctness fixes accompany the parallelism. The deferred aggregator implements a new tools.PeerDependent marker so it is started only after all its peer toolsets have settled — preserving the ordering invariant that existed before when startup was sequential. A process-wide mutex in pkg/tools/mcp/oauth.go serializes interactive OAuth flows so concurrent starts cannot race on the fixed callback port, issue simultaneous prompts, or confuse legacy MCP clients that answer elicitations without an elicitation_id.

Measured on Apple Silicon, warm caches: LoadTeam drops from ~150–190 ms to ~130–145 ms; gopher root-agent tools ready in 1.34 s (from 1.47 s, now bound by the inherent ~1.3 s gopls MCP handshake); librarian sub-agent first turn drops from 5.2 s to ~3.4 s. The single-flight guarantee on Start is preserved, configuration-order warnings and dedup are preserved, and the deferred toolset ordering invariant is preserved.

[docker-agent]

Assessment: 🟢 APPROVE

The parallelisation and prefetch changes are well-structured. concurrent.MapSlice/ForEach are correct — Go 1.26.5 gives per-iteration loop variable semantics so there is no closure capture race. The two-wave start ordering (independent → peer-dependent) correctly preserves the invariant that the deferred aggregator starts only after its source toolsets have settled. gateway.Prefetch is safe: loader.load immediately wraps its fetch in context.WithoutCancel, so the background goroutine completes even if the caller's context is cancelled. The interactiveOAuthMu locking order (per-transport oauthFlowMu → global interactiveOAuthMu) is consistent at every call-site, ruling out deadlock.

Lower-confidence findings (not posted inline)

• [medium] pkg/tools/mcp/oauth.go:492 — interactiveOAuthMu.Lock() does not honour context cancellation (confidence: 🟠 weak 52/100): In the interactive-prompt path (ensureToolSetsAreStarted), multiple toolsets starting concurrently could each queue on interactiveOAuthMu. A sync.Mutex lock is not cancellable, so if the agent run context is cancelled while a goroutine waits for the mutex, that goroutine remains blocked until the in-progress OAuth flow finishes. In practice this is the intended serialisation behaviour and the goroutine is not permanently leaked (it will proceed once the mutex is released and then return quickly on the cancelled context); the non-interactive startup path (emitToolsProgressively) already guards this with WithoutInteractivePrompts so the mutex is never contended there. Mentioning for awareness — a context-aware mutex (or a TryLock with ctx-done select) would make cancellation instantaneous, but it is not required for correctness.

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

This PR's concurrent toolset startup is well-structured — the two-wave approach (independents first, then PeerDependent toolsets) is sound, concurrent.MapSlice/ForEach is correctly implemented, and the wrapper chain for PeerDependent detection works because all wrapper types implement Unwrapper. One liveness concern was found in the new OAuth serialization mutex.

[docker-agent]

[medium] interactiveOAuthMu.Lock() is not context-aware: a cancelled toolset blocks indefinitely waiting behind an active OAuth flow

Trigger: This PR introduces concurrent toolset startup. If toolset A is in an interactive OAuth flow (waiting for user browser interaction, which can take minutes), toolset B — started concurrently — can also reach startInteractiveFlowLocked and block at interactiveOAuthMu.Lock() on line 492. If toolset B's ctx is cancelled before it acquires the mutex (e.g. the agent session is torn down, a startup timeout fires, or the user cancels), sync.Mutex.Lock() is not context-aware and cannot be interrupted. Toolset B's goroutine is stuck behind the user interaction for toolset A.

Impact: While blocked at interactiveOAuthMu.Lock(), toolset B's goroutine still holds oauthFlowMu (acquired by the caller authorizeOnce). Every concurrent RPC to toolset B's transport that also needs oauthFlowMu is therefore also stalled — potentially for minutes — even though toolset B's context is already cancelled. Once toolset B finally acquires interactiveOAuthMu, it calls handleOAuthFlow(ctx, ...) and returns immediately (cancelled context), but the delay was already paid. Before this PR sequential startup meant only one toolset could be doing OAuth at a time, so this scenario could not arise.

Suggested fix: Replace the bare sync.Mutex with a context-aware acquire that selects on ctx.Done():

acquired := make(chan struct{}, 1)
go func() { interactiveOAuthMu.Lock(); acquired <- struct{}{} }()
select {
case <-acquired:
    defer interactiveOAuthMu.Unlock()
case <-ctx.Done():
    return ctx.Err()
}

Or use a sync.Cond-based approach, or a channel-based semaphore, to allow waiting goroutines to be preempted on context cancellation.

Confidence	Score
🟢 strong	97/100

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

Two LIKELY findings in the concurrency changes introduced by this PR. Neither is a crash or data-loss bug, but both represent new regressions vs. the sequential-startup behaviour this PR replaces.

[docker-agent]

[medium] Holding per-transport oauthFlowMu while blocking on global interactiveOAuthMu can stall concurrent RPC calls for the duration of another transport's interactive OAuth flow

authorizeOnce acquires t.oauthFlowMu, then calls startInteractiveFlowLocked, which blocks on the process-wide interactiveOAuthMu. Before this PR, toolsets started sequentially, so Transport A's OAuth would never begin while Transport B's user-facing flow was in progress. Now that toolsets start concurrently, Transport A can be holding its own per-transport t.oauthFlowMu while waiting for Transport B's user to click "Authorize" in the browser — a wait measured in minutes.

During that entire window, any concurrent RoundTrip call for Transport A that encounters a 401 and calls authorizeOnce will block on t.oauthFlowMu with its request context ticking down, potentially timing out. The comment in the code acknowledges this lock ordering but does not address the extended hold duration introduced by concurrency.

A low-risk mitigation: acquire interactiveOAuthMu before t.oauthFlowMu (i.e. restructure authorizeOnce to try the cheap token-check first without holding oauthFlowMu, and only take t.oauthFlowMu for the minimal critical section). Alternatively, document that callers must use a context with a timeout long enough to survive a full interactive flow.

Confidence	Score
🟡 moderate	75/100

[docker-agent]

[low] Prefetch winning the sync.Once race with a transient error permanently prevents catalog loading for this process lifetime

Prefetch fires go func() { _, _ = loader.load(ctx) }() in the background. Inside load, sync.Once memoizes the first result — including errors — so a transient failure during the prefetch window permanently caches the error; no subsequent ServerSpec call can retry.

Before this PR, Prefetch did not exist, so the once.Do was triggered only by a real ServerSpec call (at which point the caller could propagate the error and the user could retry by restarting). Now the once.Do fires at load time, in the background, when the caller has no visibility.

fetchAndCache does have a disk-cache fallback, so a pure network blip is unlikely to produce an error (it would return the cached catalog instead). The residual scenario is: no network AND no on-disk cache (cold start on first run with no connectivity). In that case, the process must be restarted to retry, whereas before this PR the first real call would fail immediately and the user could react.

Consider returning the error through a channel or logging it at WARN level so the failure is visible, and/or not entering once.Do from the background goroutine (only trigger the fetch eagerly without committing the error result — keep once.Do solely on the real call path).

Confidence	Score
🟡 moderate	62/100