Files
coder/cli
Ethan d219f96ba5 fix(cli): join MCP reporter and watcher goroutines before exit (#26847)
## Problem

`TestExpMcpReporter/Reconnect` flakes under the race detector with a
data race on the shared `*serpent.Invocation`'s `inv.Stderr` field.

The MCP server's reporter and watcher goroutines write status warnings
via `cliui.Warnf(inv.Stderr, ...)`, but they were launched
fire-and-forget with nothing tying their lifetime to the command
handler. On shutdown, `startServer`'s deferred restore of
`inv.Stdin/Stdout/Stderr` could run concurrently with a still-running
goroutine reading `inv.Stderr`, which the race detector flags. The
reporter's error suppression only swallows `context.Canceled`, so a
shutdown error from an in-flight `UpdateAppStatus` RPC (a drpc "closed"
error, not `context.Canceled`) reaches the `Warnf` call and races the
restore.

## Fix

Track the reporter and watcher goroutines on a `sync.WaitGroup`. After
`startServer` returns, cancel the context, close the queue and socket
client, then `wg.Wait()` for the goroutines to exit before returning.
All three unblocks are needed: cancel stops the watcher retry loop and a
reporter blocked on `Pop`, `queue.Close` also unblocks `Pop`, and
`socketClient.Close` unblocks a reporter parked in an in-flight RPC.

This also removes the stdin/stdout/stderr save/restore in `startServer`,
which only ever wrote back identical values and was the racing write.

This mirrors the existing precedent in `cli/ssh.go`, where a
`sync.WaitGroup` guards against "logging while closing the log file in a
defer."

Verified with `go test ./cli -run 'TestExpMcpReporter/Reconnect' -race
-count=50` (the reproducer from the issue) plus a 240-execution parallel
stress run of the full `TestExpMcp` suite under `-race`, all green.

Closes CODAGT-710
Closes https://github.com/coder/internal/issues/1610
2026-07-01 00:16:25 +10:00
..
…