inspect: avoid timeout fallback false positives

8b8c615→main·

dipree·4w ago·2 files·+83 added/-0 removed

The lost-sentinel timeout fallback should only classify errors that actually look like formatted context deadline errors. Otherwise an ordinary Wait failure that returns after the inspector deadline fires could be mislabeled as a timeout. Keep errors.Is(DeadlineExceeded) as the primary signal, but require the fallback error text to contain the context deadline message before sampling the per-inspector context.

Add single and multi-agent regressions where Wait returns a normal failure afther the deadline has fired; both remain ordinary failures, not timeouts.

Sessions

c16f7eef84afView transcript

[?
Checkout the hand off doc that I just added.Pi·Opus 4.8·1 step](/content/gh/entireio/cli/session/019eca64-8c2c-7b00-90c6-3aa49738c497#timeline-c16f7eef84af/index.html)

Changes

2

• cmd/entire/cli/review

  • Mrun.go+8

  • Mrun_test.go+75

8 unmodified lines

9
10
11
12
13
14
15
46 unmodified lines

62
63
64
65
66
67
68
69
70
71
72
73
74

8 unmodified lines

\t"context"
\t"errors"
\t"fmt"
\t"strings"
\t"time"

\nreviewtypes "github.com/entireio/cli/cmd/entire/cli/review/types"
46 unmodified lines

\tif errors.Is(waitErr, context.DeadlineExceeded) {
\t\treturn true
\t}
\t// Fallback only for adapters that formatted ctx.Err() without %w (for
\t// example "agent failed: context deadline exceeded"). Do not classify an
\t// unrelated non-nil Wait error as a timeout just because the inspector
\t// deadline fired while/after Wait was returning.
\tif !strings.Contains(waitErr.Error(), context.DeadlineExceeded.Error()) {
\t\treturn false
\t}
\tif !errors.Is(agentCtx.Err(), context.DeadlineExceeded) {
\t\treturn false
\t}

Mcmd/entire/cli/review/run.go+8

60 unmodified lines

61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
537 unmodified lines

631
632
633
634
635
636
637
638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
55 unmodified lines

716
717
718
719
720
721
722
723
724
725
726
727
728
729
730
731
732
733
734
735
736
737
738
739
740
741
742
743
744
745

60 unmodified lines

\treturn errors.New("agent failed: " + p.ctx.Err().Error())
}

type delayedWaitReviewer struct {
\tname    string
\tdelay   time.Duration
\twaitErr error
}

func (r *delayedWaitReviewer) Name() string { return r.name }
func (r *delayedWaitReviewer) Start(context.Context, reviewtypes.RunConfig) (reviewtypes.Process, error) {
\treturn &delayedWaitProcess{delay: r.delay, waitErr: r.waitErr}, nil
}

type delayedWaitProcess struct {
\tdelay   time.Duration
\twaitErr error
}

func (p *delayedWaitProcess) Events() <-chan reviewtypes.Event {
\tout := make(chan reviewtypes.Event)
\tclose(out)
\treturn out
}

func (p *delayedWaitProcess) Wait() error {
\ttime.Sleep(p.delay)
\treturn p.waitErr
}

// stubReviewer is a test double for reviewtypes.AgentReviewer.
type stubReviewer struct {
\tname     string
537 unmodified lines

}\n}

func TestRun_DeadlineDuringOrdinaryWaitFailureIsNotTimeout(t *testing.T) {
\tt.Parallel()
\tordinaryErr := errors.New("exit status 1")
\tsummary, err := Run(
\t\tcontext.Background(),
\t\t&delayedWaitReviewer{name: "claude-code", delay: 30 * time.Millisecond, waitErr: ordinaryErr},
\t\treviewtypes.RunConfig{InspectorTimeout: 5 * time.Millisecond},
\t\tnil,
\t)
\tif !errors.Is(err, ordinaryErr) {
\tt.Fatalf("err = %v, want ordinary wait error", err)
\t}
\tif len(summary.AgentRuns) != 1 {
\tt.Fatalf("expected 1 AgentRun, got %d", len(summary.AgentRuns))
\t}
\trun := summary.AgentRuns[0]
\tif run.Status != reviewtypes.AgentStatusFailed {
\tt.Fatalf("status = %v, want Failed", run.Status)
\t}
\tif run.Err == nil || strings.Contains(run.Err.Error(), "timed out") {
\tt.Fatalf("run.Err = %v, want ordinary failure, not timeout", run.Err)
\t}
}

func TestRunMulti_InspectorTimeoutIsolated(t *testing.T) {
\tt.Parallel()
\t// One inspector hangs (times out); a sibling finishes cleanly. The run is
55 unmodified lines

}\n}

func TestRunMulti_DeadlineDuringOrdinaryWaitFailureIsNotTimeout(t *testing.T) {
\tt.Parallel()
\tordinaryErr := errors.New("exit status 1")
\tsummary, err := RunMulti(
\t\tcontext.Background(),
\t\t[]reviewtypes.AgentReviewer{&delayedWaitReviewer{name: "slow-fail", delay: 30 * time.Millisecond, waitErr: ordinaryErr}},
\t\treviewtypes.RunConfig{InspectorTimeout: 5 * time.Millisecond},
\t\tnil,
\t)
\tif !errors.Is(err, ordinaryErr) {
\tt.Fatalf("err = %v, want ordinary wait error", err)
\t}
\tif len(summary.AgentRuns) != 1 {
\tt.Fatalf("expected 1 AgentRun, got %d", len(summary.AgentRuns))
\t}
\trun := summary.AgentRuns[0]
\tif run.Status != reviewtypes.AgentStatusFailed {
\tt.Fatalf("status = %v, want Failed", run.Status)
\t}
\tif run.Err == nil || strings.Contains(run.Err.Error(), "timed out") {
\tt.Fatalf("run.Err = %v, want ordinary failure, not timeout", run.Err)
\t}
}

func TestRun_ParentCancelIsNotTimeout(t *testing.T) {
\tt.Parallel()
\tctx, cancel := context.WithCancel(context.Background())