Classify review start timeouts · Entire

Classify review start timeouts

b1bbb69→main·

dipree·3w ago·3 files·+75 added/-2 removed

Sessions

f6581d3bbb26View transcript

[?
Review Range Handling and Timeout ClassificationPi·GPT-5.5·5 steps](/content/gh/entireio/cli/session/019ee08c-a5ac-7745-9a1c-53e23fc07061#timeline-f6581d3bbb26/index.html)

Changes

3

143 unmodified lines

144
145
146
147
148
149
150
151
152
153
154
3 unmodified lines

158
159
160
156
161
162
163
164
1 unmodified line

166
167
168
164
169
170
171
172

143 unmodified lines

// No event-stream signals available since Start failed before producing any.
    finished := time.Now()
    status := classifyStatus(ctx, err, eventOutcome{})
    runErr := err
    if reviewerDeadlineFired(ctx, agentCtx, err) {
        status = reviewtypes.AgentStatusFailed
        runErr = timedOutError(displayName, timeout)
    }
    summary := reviewtypes.RunSummary{
        StartedAt:  started,
        FinishedAt: finished,
3 unmodified lines

AgentName: agentName,
            Model:     modelName,
            Status:    status,
            Err:       err,
            Err:       runErr,
            StartedAt: started,
            Duration:  finished.Sub(started),
        }},
1 unmodified line

for _, sink := range sinks {
        sink.RunFinished(summary)
    }
    return summary, err //nolint:wrapcheck // interface-boundary passthrough; wrapping breaks classifyStatus's ctx.Err() identity check for cancelled-during-Start scenarios
    return summary, runErr //nolint:wrapcheck // interface-boundary passthrough; wrapping breaks classifyStatus's ctx.Err() identity check for cancelled-during-Start scenarios
}

var (

Mcmd/entire/cli/review/run.go+7/-2

157 unmodified lines

158
159
160
161
162
163
164
165
166
167
168
169

157 unmodified lines

// Cancel immediately to release the per-agent timeout timer while
        // siblings continue running. Queue the terminal marker so the dispatch
        // loop remains the single writer of perAgentState terminal fields.
        timedOut := reviewerDeadlineFired(ctx, agentCtx, err)
        cancelAgent()
        startTerminals = append(startTerminals, taggedEvent{agentIdx: i, terminal: &agentTerminal{
            startErr:   err,
            finishedAt: time.Now(),
            timedOut:   timedOut,
        }})
        continue
    }
}

Mcmd/entire/cli/review/run_multi.go+2

19 unmodified lines

20
21
22
23
24
25
26
27
28
29
30
31
32
33
665 unmodified lines

699
700
701
702
703
704
705
706
707
708
709
710
711
712
713
714
715
716
717
718
719
720
721
722
723
724
725
726
727
728
729
730
75 unmodified lines

806
807
808
809
810
811
812
813
814
815
816
817
818
819
820
821
822
823
824
825
826
827
828
829
830
831
832
833
834
835
836
837
838
839
840
841
842
843

19 unmodified lines

return &ctxProcess{ctx: ctx}, nil
}

type contextBlockingStartReviewer struct{ name string }

func (r *contextBlockingStartReviewer) Name() string { return r.name }
func (r *contextBlockingStartReviewer) Start(ctx context.Context, _ reviewtypes.RunConfig) (reviewtypes.Process, error) {
    <-ctx.Done()
    return nil, ctx.Err()
}

type ctxProcess struct{ ctx context.Context }
type deadlineHidingContext struct{ context.Context }
665 unmodified lines

}

func TestRun_ReviewerTimeoutDuringStart(t *testing.T) {

t.Parallel()
summary, err := Run(
    context.Background(),
    &contextBlockingStartReviewer{name: "slow-start"},
    reviewtypes.RunConfig{ReviewerTimeout: 30 * time.Millisecond},
    nil,
)
if err == nil || !strings.Contains(err.Error(), "timed out") {
    t.Fatalf("err = %v, want a 'timed out' error", err)
}
if summary.Cancelled {
    t.Error("Cancelled should be false for a per-reviewer timeout")
}
if len(summary.AgentRuns) != 1 {
    t.Fatalf("expected 1 AgentRun, got %d", len(summary.AgentRuns))
}
run := summary.AgentRuns[0]
if run.Status != reviewtypes.AgentStatusFailed {
    t.Errorf("status = %v, want Failed", run.Status)
}
if run.Err == nil || !strings.Contains(run.Err.Error(), "timed out") {
    t.Errorf("run.Err = %v, want 'timed out'", run.Err)
}
}

func TestRun_ReviewerTimeoutWithStringWrappedContextError(t *testing.T) {

t.Parallel()
summary, err := Run(
75 unmodified lines

}

func TestRunMulti_ReviewerTimeoutDuringStart(t *testing.T) {

t.Parallel()
slowStart := &contextBlockingStartReviewer{name: "slow-start"}
fast := &stubReviewer{name: "fast", events: []reviewtypes.Event{
    reviewtypes.Started{},
    reviewtypes.Finished{Success: true},
}}
summary, err := RunMulti(
    context.Background(),
    []reviewtypes.AgentReviewer{slowStart, fast},
    reviewtypes.RunConfig{ReviewerTimeout: 30 * time.Millisecond},
    nil,
)
if err == nil || !strings.Contains(err.Error(), "timed out") {
    t.Fatalf("err = %v, want the timed-out agent's error", err)
}
if summary.Cancelled {
    t.Error("Cancelled should be false")
}
byName := map[string]reviewtypes.AgentRun{}
for _, r := range summary.AgentRuns {
    byName[r.Name] = r
}
if got := byName["slow-start"]; got.Status != reviewtypes.AgentStatusFailed ||
    got.Err == nil || !strings.Contains(got.Err.Error(), "timed out") {
    t.Errorf("slow-start = %+v, want Failed with 'timed out'", got)
}
if byName["fast"].Status != reviewtypes.AgentStatusSucceeded {
    t.Errorf("fast status = %v, want Succeeded", byName["fast"].Status)
}
}

// TestRun_ParentCancelIsNotTimeout pins the timeout-vs-cancel distinction: when
// the parent context is cancelled (user Ctrl+C) before a reviewer's deadline
// can fire, the reviewer is classified Cancelled, not failed-by-timeout.