agents/proxy: fix red replay tests and a crash-class race in failAllTrackedTurns

[ElanHasson]

Stacked on top of #1893 (base: juliaye/doctl-proxy2). Two mechanical fixes found while reviewing that PR; both are verified, and neither changes intended behavior.

1. All four --replay tests are currently red

agentproxytest.Harness only registers the data-plane route:

mux.HandleFunc("GET /v2/agents/sessions/{id}/events", h.handleStream)

but godo's StreamSession sends replay-only reads to the control plane at .../stream?replay_only=true (see hostedAgentSessionStreamPath, and StreamSession's own doc comment about the two SSE surfaces). So every replay test 404s:

codex facade: replay history fetch failed, will retry on next connect:
GET .../v2/agents/sessions/sess_test123/stream?replay_only=true: 404 404 page not found

--- FAIL: TestFacade_Replay_ThreadResume
--- FAIL: TestFacade_Replay_FiresOnlyOnce
--- FAIL: TestFacade_Replay_UnaffectedByConcurrentLiveTurnsReset
--- FAIL: TestFacade_Replay_RetriesAfterAbortedAttempt

handleStream already branches on replay_only and QueueReplayHistory already documents serving it, so only the route registration was missing. This looks like a merge artifact: the .../events migration landed on feat/agents-subcommands, and the merge kept the base's route list while the branch's new replay code needs .../stream. Registering it makes all four pass.

The stale comment claiming .../stream is "deliberately not registered" is updated to describe what the harness actually does.

2. failAllTrackedTurns can crash the process

f.mu.Lock()
turns := f.turns
f.mu.Unlock()

for runID, ts := range turns { // ranges the *live* map
    f.finishTurn(runID, ts, "failed", ...)
}

turns aliases f.turns, so this ranges the live map. Meanwhile finishTurn deletes from it, a concurrent turn/start inserts into it via trackTurn, and a --replay goroutine's own finishTurn deletes from it too — all under f.mu, but holding the lock on only the writer's side does not protect an open iterator. That's fatal error: concurrent map iteration and map write: an unrecoverable crash, not a benign stale read. It fires precisely on the connection-loss path, which is where users are least able to afford one.

Confirmed under -race:

WARNING: DATA RACE
Write at 0x00c0003c78c0 by goroutine 10:
  runtime.mapassign_faststr()
Previous read at 0x00c0003c78c0 by goroutine 9:
  runtime.mapIterStart()
  codex.(*Facade).failAllTrackedTurns() facade.go:1173

Fixed by copying the map under f.mu — chosen over detaching with f.turns = nil, which would have widened the existing window where a turn/start racing runEventLoop's exit registers a turn into a map that the deferred cleanup then discards. The copy is a pure race fix with no behavior change.

Added TestFacade_FailAllTrackedTurns_ConcurrentTurnsWrite, which fails under -race without the fix and passes with it.

3. gofmt

harness.go's godo import was inside the stdlib group; gofmt -l flagged the file.

Test plan

• go test -race ./internal/agentproxy/... — green (was 4 failures)
• Regression test verified to fail under -race with the failAllTrackedTurns fix reverted
• gofmt -l internal/agentproxy/ — clean
• GOOS=windows go build ./commands/ — builds

Remaining review findings on #1893 (attach-race regression, shared-Facade notifier race, replayed HITL re-prompts) are behavioral/design calls and are left to the author; they're written up in a review comment there rather than changed here.

Made with Cursor