review: group timedOut with the other wg-published field writes

f13f029→main·

dipree·1mo ago·1 file·+11 added/-4 removed

Address a review note worrying that states[idx].timedOut might not be covered by the wait-group happens-before. It already was (it is written before the deferred wg.Done(), like waitErr/finishedAt, and the main goroutine reads all fields only after wg.Wait() -> close(fanIn) -> the dispatch loop ends). Move the timedOut write up next to waitErr (before finishedAt and before the fanIn sends) and expand the comment to make the wg-based publication explicit. No behavior change; 'go test -race' on the package is clean.

Sessions

d1fe710ae227View transcript

?\ Checkout the hand off doc that I just added.Pi·Opus 4.8·2 steps

Changes

1

• cmd/entire/cli/review

• Mrun_multi.go+11/-4

153 unmodified lines

154
155
156
157
158
157
158
159
160
161
162
163
164
165
161
162
166
167
168
169
170
171
172
173
174

153 unmodified lines

}
            waitErr := p.Wait()
            finishedAt := time.Now()
            states[idx].waitErr = waitErr
            states[idx].finishedAt = finishedAt
            // waitErr, timedOut, and finishedAt are all written here, before this
            // goroutine's deferred wg.Done(). The main goroutine reads them only
            // after the dispatch loop ends, which is sequenced after
            // wg.Wait() -> close(fanIn) by the close goroutine; that wg edge is the
            // happens-before, so it covers these writes regardless of the fanIn
            // sends below.
            //
            // ac.Err() is immutable once set, so DeadlineExceeded means THIS
            // agent's deadline fired first; a parent cancellation (user Ctrl+C)
            // propagates as Canceled instead. Reading only ac is race-free —
            // also sampling the parent ctx could change between the two reads and
            // propagates as Canceled instead. Reading only ac is race-free — also
            // sampling the parent ctx could change between the two reads and
            // misclassify a real timeout as a cancellation.
            states[idx].waitErr = waitErr
            states[idx].timedOut = ac.Err() == context.DeadlineExceeded
            states[idx].finishedAt = finishedAt
            if shouldEmitSyntheticRunError(ctx, waitErr) {
                fanIn <- taggedEvent{agentIdx: idx, ev: reviewtypes.RunError{Err: waitErr}}
            }

Mcmd/entire/cli/review/run_multi.go+11/-4