inspect: require earlier agent deadline for timeout fallback

bdce33a→main·

dipree·4w ago·2 files·+32 added/-11 removed

Classify inspector timeouts only when the agent context has a deadline that is strictly earlier than the parent deadline, or when the parent has no deadline. Equal deadlines can mean the agent context inherited the parent deadline, so do not report those parent timeouts as per-inspector failures. This applies before both the preserved DeadlineExceeded sentinel path and the lost-sentinel string fallback.

Update tests for equal-deadline parent timeouts and strictly-earlier inspector timeouts.

Sessions

a6e547b1c6a8View transcript

Changes

2

• cmd/entire/cli/review

• Mrun.go+12/-9

• Mrun_test.go+20/-2

58 unmodified lines

59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
4 unmodified lines

80
81
82
72
73
74
75
76
77
78
79
80
83
84
85
86

58 unmodified lines

if waitErr == nil {
        return false
    }
    agentDeadline, ok := agentCtx.Deadline()
    if !ok {
        return false
    }
    // Only an agent deadline that is strictly earlier than the parent deadline
    // (or no parent deadline at all) proves this was the per-inspector timeout.
    // Equal deadlines mean the child may have inherited the parent's deadline, so
    // parent cancellation/timeout must not be reported as an inspector timeout.
    if parentDeadline, parentHasDeadline := parentCtx.Deadline(); parentHasDeadline && !agentDeadline.Before(parentDeadline) {
        return false
    }
    if errors.Is(waitErr, context.DeadlineExceeded) {
        return true
    }

Mcmd/entire/cli/review/run.go+12/-9

581 unmodified lines

582
583
584
585
585
586
587
588
1 unmodified line

590
591
592
593
594
595
596
597
598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
1 unmodified line

615
616
617
600
618
619
620
621

581 unmodified lines

}
}

func TestInspectorDeadlineFiredFallbackAllowsEqualParentDeadline(t *testing.T) {
func TestInspectorDeadlineFired_EqualParentDeadlineIsNotInspectorTimeout(t *testing.T) {
    t.Parallel()
    deadline := time.Now().Add(20 * time.Millisecond)
    parentCtx, cancelParent := context.WithDeadline(context.Background(), deadline)
1 unmodified line

agentCtx, cancelAgent := context.WithDeadline(parentCtx, deadline)
    defer cancelAgent()

select {
    case <-agentCtx.Done():
    case <-time.After(time.Second):
        t.Fatal("agent context deadline did not fire")
    }
    waitErr := errors.New("agent failed: " + context.DeadlineExceeded.Error())
    if inspectorDeadlineFired(parentCtx, agentCtx, waitErr) {
        t.Fatal("equal parent/agent deadlines should be treated as parent deadline, not inspector timeout")
    }
}

func TestInspectorDeadlineFired_EarlierAgentDeadlineIsInspectorTimeout(t *testing.T) {
    t.Parallel()
    parentCtx, cancelParent := context.WithDeadline(context.Background(), time.Now().Add(time.Second))
    defer cancelParent()
    agentCtx, cancelAgent := context.WithTimeout(parentCtx, 20*time.Millisecond)
    defer cancelAgent()

select {
    case <-agentCtx.Done():
    case <-time.After(time.Second):
1 unmodified line

}
    waitErr := errors.New("agent failed: " + context.DeadlineExceeded.Error())
    if !inspectorDeadlineFired(parentCtx, agentCtx, waitErr) {
        t.Fatal("equal parent/agent deadlines should still classify as inspector deadline fallback")
        t.Fatal("earlier agent deadline should classify as inspector timeout")
    }
}