inspect: add a per-inspector timeout (default 10m) · Entire
inspect: add a per-inspector timeout (default 10m)
478379b→main·
dipree·1mo ago·6 files·+180 added/-16 removed
Inspectors previously ran under a plain cancel context with no deadline, so a stuck agent could hang the review forever. Each inspector now runs under its own context.WithTimeout (RunConfig.InspectorTimeout, default 10m via defaultInspectorTimeout, overridable with --timeout). On timeout the inspector's process is killed and it is marked failed-by-timeout; sibling inspectors and the judge proceed. A parent cancellation (Ctrl+C) is still classified as cancelled, not timed out.
- RunConfig.InspectorTimeout + inspectorTimeout() default helper. - Run and RunMulti wrap each Start in a per-agent deadline and detect DeadlineExceeded (vs parent cancel) to set the failed-by-timeout error. - --timeout duration flag (default 10m) wired through runReview to both the single- and multi-inspector paths. - Tests: Run times out a hanging inspector; RunMulti times out one inspector while a sibling still succeeds. - Docs updated (command surface + architecture note).
Sessions
c8c400c372beView transcript
[?
Checkout the hand off doc that I just added.Pi·Opus 4.8·2 steps](/content/gh/entireio/cli/session/019eca64-8c2c-7b00-90c6-3aa49738c497#timeline-c8c400c372be/index.html)
Changes
6
cmd/entire/cli/review
Mcmd.go+15/-6
Mrun.go+33/-2
Mrun_multi.go+22/-7
Mrun_test.go+90
types
Mreviewer.go+12/-1
docs/architecture
Mreview-command.md+8
15 unmodified lines
16
17
18
19
20
21
22
67 unmodified lines
90
91
92
93
94
95
96
36 unmodified lines
133
134
135
136
137
138
139
140
65 unmodified lines
206
207
208
205
209
210
211
212
14 unmodified lines
227
228
229
230
231
232
233
451 unmodified lines
685
686
687
683
688
689
690
691
130 unmodified lines
822
823
824
820
825
826
827
828
12 unmodified lines
841
842
843
839
844
845
846
847
14 unmodified lines
862
863
864
860
865
866
867
868
61 unmodified lines
930
931
932
933
934
935
936
70 unmodified lines
1007
1008
1009
1010
1011
1012
1013
81 unmodified lines
1095
1096
1097
1098
1099
1100
1101
99 unmodified lines
1201
1202
1203
1196
1204
1205
1206
1207
1208
15 unmodified lines
"os"
"sort"
"strings"
"time"
"charm.land/huh/v2"
"github.com/spf13/cobra"
67 unmodified lines
var setJudge string
var setOutput string
var setLocal bool
var inspectTimeout time.Duration
var setTask string
var setModels []string
var setSlots []string
36 unmodified lines
--models list the models each agent advertises (optionally --agent NAME)
--profile NAME select a profile (also accepted as positional arg)
--prompt TEXT add one-off per-run instructions for this invocation
--timeout DUR max time each inspector may run before it's cancelled and
marked failed (default 10m). Siblings and the judge proceed.
--base REF scope against REF instead of mainline. Useful for stacked
PRs where the base is the parent feature branch, not main.
Default: first existing of origin/HEAD, origin/main,
65 unmodified lines
if findings {
return runReviewFindings(ctx, cmd, deps.NewSilentError)
}
return runReview(ctx, cmd, agentOverride, modelOverride, baseOverride, profileName, perRunPrompt, deps)
return runReview(ctx, cmd, agentOverride, modelOverride, baseOverride, profileName, perRunPrompt, inspectTimeout, deps)
},
}
cmd.Flags().BoolVar(&configure, "configure", false, "set up a review profile; shows available agents and accepts --set-* flags for non-interactive config")
14 unmodified lines
cmd.Flags().StringVar(&profileOverride, "profile", "", "review profile to run (default: review_default_profile or general)")
cmd.Flags().StringVar(&perRunPrompt, "prompt", "", "one-off instructions appended to this review run")
cmd.Flags().StringVar(&baseOverride, "base", "", "git ref to scope the review against (default: origin/HEAD → origin/main → origin/master → main → master)")
cmd.Flags().DurationVar(&inspectTimeout, "timeout", defaultInspectorTimeout, "max time each inspector may run before it is cancelled and marked failed")
// The listing modes and the action modes each select a distinct command
// behavior; combining them silently runs one and drops the rest, so reject
// the combination up front with a clear cobra error.
451 unmodified lines
}
// runReview executes the main review flow.
func runReview(ctx context.Context, cmd *cobra.Command, agentOverride, modelOverride, baseOverride, profileOverride, perRunPrompt string, deps Deps) error {
func runReview(ctx context.Context, cmd *cobra.Command, agentOverride, modelOverride, baseOverride, profileOverride, perRunPrompt string, timeout time.Duration, deps Deps) error {
out := cmd.OutOrStdout()
silentErr := deps.NewSilentError
130 unmodified lines
if modelOverride != "" {
cfg.Model = modelOverride
}
return runSingleAgentPath(ctx, cmd, profileName, workerName, baseOverride, perRunPrompt, profile.Task, outputMode, cfg, installed, deps, out)
return runSingleAgentPath(ctx, cmd, profileName, workerName, baseOverride, perRunPrompt, profile.Task, outputMode, timeout, cfg, installed, deps, out)
}
if missing := missingInstalledProfileAgents(profile.Agents, installed); len(missing) > 0 {
12 unmodified lines
return silentErr(err)
case 1:
cfg := profile.Agents[eligible[0].Name]
return runSingleAgentPath(ctx, cmd, profileName, eligible[0].Name, baseOverride, perRunPrompt, profile.Task, outputMode, cfg, installed, deps, out)
return runSingleAgentPath(ctx, cmd, profileName, eligible[0].Name, baseOverride, perRunPrompt, profile.Task, outputMode, timeout, cfg, installed, deps, out)
default:
launchableEligible := computeLaunchableEligibleForProfile(profile, installed, deps.ReviewerFor)
if len(launchableEligible) != len(eligible) {
14 unmodified lines
fmt.Fprintln(cmd.ErrOrStderr(), err.Error())
return silentErr(err)
}
return runMultiAgentPath(ctx, cmd, profileName, profile, launchableEligible, judge, outputMode, baseOverride, perRunPrompt, deps, out)
return runMultiAgentPath(ctx, cmd, profileName, profile, launchableEligible, judge, outputMode, timeout, baseOverride, perRunPrompt, deps, out)
}
}
61 unmodified lines
ctx context.Context,
cmd *cobra.Command,
profileName, workerName, baseOverride, perRunPrompt, task, outputMode string,
timeout time.Duration,
cfg settings.ReviewConfig,
installed []types.AgentName,
deps Deps,
70 unmodified lines
ScopeBaseRef: scopeBaseRef,
CheckpointContext: checkpointContext,
StartingSHA: headSHA,
InspectorTimeout: timeout,
}
applyReviewConfig(&runCfg, cfg)
81 unmodified lines
launchableEligible []AgentChoice,
judge judgeSpec,
outputMode string,
timeout time.Duration,
baseOverride string,
perRunPrompt string,
deps Deps,
99 unmodified lines
}
summary, waitErr := RunMulti(runCtx, reviewers, reviewtypes.RunConfig{
EnrichAgentRun: reviewAgentRunTokenEnricher(worktreeRoot, headSHA),
EnrichAgentRun: reviewAgentRunTokenEnricher(worktreeRoot, headSHA),
InspectorTimeout: timeout,
}, sinks)
writePostReviewManifest(ctx, out, worktreeRoot, headSHA, summary, aggregateOutput)
maybePostReviewToTrail(ctx, out, deps, outputMode, profileName, summary, aggregateOutput)
Mcmd/entire/cli/review/cmd.go+15/-6
32 unmodified lines
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
16 unmodified lines
72
73
74
58
75
76
77
78
79
80
81
82
83
84
48 unmodified lines
133
134
135
136
137
138
139
140
141
142
7 unmodified lines
150
151
152
126
153
154
155
156
157
158
159
160
32 unmodified lines
return ""
}
// defaultInspectorTimeout bounds a single inspector's run when the caller
// doesn't set RunConfig.InspectorTimeout. A stuck agent is cancelled (its
// process killed) and marked failed rather than hanging the review forever.
const defaultInspectorTimeout = 10 * time.Minute
func inspectorTimeout(cfg reviewtypes.RunConfig) time.Duration {
if cfg.InspectorTimeout > 0 {
return cfg.InspectorTimeout
}
return defaultInspectorTimeout
}
// timedOutError reports the per-inspector timeout as a user-facing error.
func timedOutError(agent string, timeout time.Duration) error {
return fmt.Errorf("review agent %s timed out after %s", agent, timeout)
}
// Run executes a single-agent review. Events from the agent are forwarded
// to all sinks via AgentEvent as they arrive; on completion, RunFinished
// is called on each sink with the populated RunSummary.
16 unmodified lines
modelName = cfg.Model
}
proc, err := reviewer.Start(ctx, cfg)
// Bound the inspector so a stuck agent can't hang the review forever. The
// deadline applies only to this agent; cancellation kills its process.
timeout := inspectorTimeout(cfg)
agentCtx, cancelAgent := context.WithTimeout(ctx, timeout)
defer cancelAgent()
proc, err := reviewer.Start(agentCtx, cfg)
if err != nil {
// Construction failed — classify (cancellation vs failure), fan out, return.
// No event-stream signals available since Start failed before producing any.
}
48 unmodified lines
waitErr := proc.Wait()
finished := time.Now()
// A per-inspector timeout fired iff this agent's deadline elapsed while the
// parent run context is still live (a parent cancellation is a user Ctrl+C,
// classified as Cancelled instead).
timedOut := agentCtx.Err() == context.DeadlineExceeded && ctx.Err() == nil
if shouldEmitSyntheticRunError(ctx, waitErr) {
synthEvent := reviewtypes.RunError{Err: waitErr}
buffer = append(buffer, synthEvent)
7 unmodified lines
}
status := classifyStatus(ctx, waitErr, eventOutcome{finishedSeen: finishedSeen, finishedOk: finishedOk, sawRunError: sawRunError})
runErr := waitErr
if runErr == nil && status == reviewtypes.AgentStatusFailed {
switch {
case timedOut:
status = reviewtypes.AgentStatusFailed
runErr = timedOutError(displayName, timeout)
case runErr == nil && status == reviewtypes.AgentStatusFailed:
runErr = agentRunFailureError(displayName, firstRunErr)
}
}
Mcmd/entire/cli/review/run.go+33/-2
37 unmodified lines
38
39
40
41
42
43
44
41
42
43
44
45
46
47
18 unmodified lines
66
67
68
69
70
71
72
58 unmodified lines
131
132
133
134
135
136
137
138
135
139
140
141
142
143
144
145
146
147
148
143
149
150
151
152
153
154
1 unmodified line
156
157
158
159
160
161
162
163
164
165
5 unmodified lines
171
172
173
163
174
175
176
177
48 unmodified lines
226
227
228
229
230
231
232
233
234
235
37 unmodified lines
//
// Write paths (no mutex; the close-after-wait protocol below provides
// happens-before for both):
// - waitErr and finishedAt are written by the per-agent forwarding
// goroutine after its proc.Events range loop exits. That goroutine may
// still send final derived events (synthetic RunError, enriched Tokens)
// before wg.Done.
// - waitErr, finishedAt, and timedOut are written by the per-agent
// forwarding goroutine after its proc.Events range loop exits. That
// goroutine may still send final derived events (synthetic RunError,
// enriched Tokens) before wg.Done.
// - All other mutable fields (events buffer, tokens, finishedSeen,
// finishedOk, sawRunError) are written from the single dispatch loop
// reading the fan-in channel.
18 unmodified lines
finishedSeen bool
finishedOk bool
sawRunError bool
timedOut bool
waitErr error
}
58 unmodified lines
// scheduling jitter without holding an unbounded queue.
fanIn := make(chan taggedEvent, len(reviewers)*16)
// Each inspector gets its own deadline so a stuck agent is cancelled (its
// process killed) without hanging the run; siblings and the judge proceed.
timeout := inspectorTimeout(cfg)
var wg sync.WaitGroup
for i, r := range reviewers {
proc, err := r.Start(ctx, cfg)
agentCtx, cancelAgent := context.WithTimeout(ctx, timeout)
proc, err := r.Start(agentCtx, cfg)
if err != nil {
cancelAgent()
states[i].startErr = err
states[i].finishedAt = time.Now()
continue
}
states[i].proc = proc
wg.Add(1)
go func(idx int, p reviewtypes.Process) {
go func(idx int, p reviewtypes.Process, ac context.Context, cancel context.CancelFunc) {
defer wg.Done()
defer cancel()
for ev := range p.Events() {
fanIn <- taggedEvent{agentIdx: idx, ev: ev}
}
1 unmodified line
finishedAt := time.Now()
states[idx].waitErr = waitErr
states[idx].finishedAt = finishedAt
// A per-inspector timeout fired iff this agent's deadline elapsed
// while the parent run context is still live (a parent cancellation
// is a user Ctrl+C, classified as Cancelled).
states[idx].timedOut = ac.Err() == context.DeadlineExceeded && ctx.Err() == nil
if shouldEmitSyntheticRunError(ctx, waitErr) {
fanIn <- taggedEvent{agentIdx: idx, ev: reviewtypes.RunError{Err: waitErr}}
}
5 unmodified lines
Duration: finishedAt.Sub(states[idx].startedAt),
Err: waitErr,
})
}
}
// Close fanIn after all forwarding goroutines finish. This goroutine
48 unmodified lines
if agentErr == nil {
agentErr = st.waitErr
}
if st.timedOut {
status = reviewtypes.AgentStatusFailed
agentErr = timedOutError(st.name, timeout)
}
agentRuns[i] = reviewtypes.AgentRun{
Name: st.name,
AgentName: st.agentName,
Mcmd/entire/cli/review/run_multi.go+22/-7
2 unmodified lines
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
487 unmodified lines
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
2 unmodified lines
import (
"context"
"errors"
"strings"
"testing"
"time"
reviewtypes "github.com/entireio/cli/cmd/entire/cli/review/types"
)
// ctxReviewer's process hangs until its run context is done, then reports the
// context error — modeling an agent that would run forever until the
// orchestrator's per-inspector deadline cancels (kills) it.
type ctxReviewer struct{ name string }
func (r *ctxReviewer) Name() string { return r.name }
func (r *ctxReviewer) Start(ctx context.Context, _ reviewtypes.RunConfig) (reviewtypes.Process, error) {
return &ctxProcess{ctx: ctx}, nil
}
type ctxProcess struct{ ctx context.Context }
func (p *ctxProcess) Events() <-chan reviewtypes.Event {
out := make(chan reviewtypes.Event)
go func() {
<-p.ctx.Done()
close(out)
}()
return out
}
func (p *ctxProcess) Wait() error {
<-p.ctx.Done()
return p.ctx.Err()
}
// stubReviewer is a test double for reviewtypes.AgentReviewer.
type stubReviewer struct {
name string
487 unmodified lines
}
func TestRun_InspectorTimeout(t *testing.T) {
t.Parallel()
rec := &stubSinkRecorder{}
summary, err := Run(
context.Background(),
&ctxReviewer{name: "claude-code"},
reviewtypes.RunConfig{InspectorTimeout: 30 * time.Millisecond},
[]reviewtypes.Sink{rec},
)
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-inspector timeout (parent ctx not cancelled)")
}
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 TestRunMulti_InspectorTimeoutIsolated(t *testing.T) {
t.Parallel()
// One inspector hangs (times out); a sibling finishes cleanly. The run is
// not cancelled, the hung one is failed-by-timeout, the sibling succeeds.
hang := &ctxReviewer{name: "slow"}
fast := &stubReviewer{name: "fast", events: []reviewtypes.Event{
reviewtypes.Started{},
reviewtypes.Finished{Success: true},
}}
rec := &stubSinkRecorder{}
summary, err := RunMulti(
context.Background(),
[]reviewtypes.AgentReviewer{hang, fast},
reviewtypes.RunConfig{InspectorTimeout: 40 * time.Millisecond},
[]reviewtypes.Sink{rec},
)
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"]; got.Status != reviewtypes.AgentStatusFailed ||
got.Err == nil || !strings.Contains(got.Err.Error(), "timed out") {
t.Errorf("slow = %+v, want Failed with 'timed out'", got)
}
if byName["fast"].Status != reviewtypes.AgentStatusSucceeded {
t.Errorf("fast status = %v, want Succeeded", byName["fast"].Status)
}
}
Mcmd/entire/cli/review/run_test.go+90
17 unmodified lines
18
19
20
21
21
22
23
24
25
26
27
93 unmodified lines
121
122
123
124
125
126
127
128
129
130
131
132
133
134
17 unmodified lines
// without depending on each other.
package types
import "context"
import (
"context"
"time"
)
// AgentReviewer drives a single agent's review run.
type AgentReviewer interface {
93 unmodified lines
// the commit that was reviewed.
StartingSHA string
// InspectorTimeout bounds how long a single inspector may run before the
// orchestrator cancels it (its process is killed and the run is marked
// failed-by-timeout) so a stuck agent can't hang the review forever. Zero
// or negative means use the orchestrator default (defaultInspectorTimeout).
// Sibling inspectors and the judge are unaffected by one inspector's
// timeout.
InspectorTimeout time.Duration
// EnrichSummary optionally updates the completed run summary before sinks
// receive RunFinished. It is used for post-process data such as token
// totals that are only available after agent lifecycle hooks flush state.