Improve review failure and trail posting handling · Entire

Improve review failure and trail posting handling

3055e84→main·

dipree·4w ago·9 files·+302 added/-22 removed

Sessions

b14c959dbfd5View transcript

[?
List me all the potential command combinations for review.Pi·Opus 4.8·1 step](/content/gh/entireio/cli/session/019e9fb5-004d-7145-8f48-db6a240d0ab3#timeline-b14c959dbfd5/index.html)

Changes

9

1210 unmodified lines

1211
1212
1213
1214
1215
1216
1217
1218
1216
1217
1218
1219
1220
1221
108 unmodified lines

1330
1331
1332
1333
1340
1341
1342
1343
1344
1345
1346
1347
1348
1349
1350
1351
1352
1353
1354
1355
1356
1357
1358
1359
1360
1361
1362
1363
1364
1365
1366
1367
1368

1210 unmodified lines

EnrichAgentRun:  reviewAgentRunTokenEnricher(worktreeRoot, headSHA),
        ReviewerTimeout: timeout,
    }, sinks)
    if shouldAbortMultiReview(summary, waitErr) && runCtx.Err() == nil && ctx.Err() == nil {
        return multiReviewFailureError(waitErr)
    }
    writePostReviewManifest(ctx, out, worktreeRoot, headSHA, summary, aggregateOutput)
    maybePostReviewToTrail(ctx, out, deps, outputMode, profileName, summary, aggregateOutput)
    if waitErr != nil && runCtx.Err() == nil && ctx.Err() == nil {
        return fmt.Errorf("review run: %w", waitErr)
    }
    return nil
}

108 unmodified lines

return "judge: " + masterName
// shouldAbortMultiReview reports whether the profile-native fan-out produced no
// successful reviewer at all. Individual reviewer infrastructure failures (for
// example quota/auth/tool failures) should not fail the entire review when at
// least one sibling produced a usable review; the failed reviewer remains
// visible in terminal output only. With zero successful reviewers, there is no
// review result to manifest or post, so the command fails loudly.
func shouldAbortMultiReview(summary reviewtypes.RunSummary, waitErr error) bool {
    if len(summary.AgentRuns) == 0 {
        return waitErr != nil
    }
    for _, run := range summary.AgentRuns {
        if run.Status == reviewtypes.AgentStatusSucceeded {
            return false
        }
    }
    if waitErr != nil {
        return true
    }
    for _, run := range summary.AgentRuns {
        if run.Status == reviewtypes.AgentStatusFailed {
            return true
        }
    }
    return false
}

func multiReviewFailureError(waitErr error) error {
    if waitErr != nil {
        return fmt.Errorf("review run: %w", waitErr)
    }
    return errors.New("review run: all reviewers failed")
}

// maybePostReviewToTrail delivers the final review output to the branch's trail
// when the profile selects the "trail" destination. It never fails the run:
// the review already happened, so a posting error (or a missing hook) is

Mcmd/entire/cli/review/cmd.go+36/-3

513 unmodified lines

514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
86 unmodified lines

638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
689
690
691
692
693
694
695
696
697
698
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
731
732
733
734
735
736
737
738
739

513 unmodified lines

func (p *stubDispatchProcess) Wait() error { return nil }

type scriptedDispatchReviewer struct {
    name    string
    events  []reviewtypes.Event
    waitErr error
}

func (r *scriptedDispatchReviewer) Name() string { return r.name }
func (r *scriptedDispatchReviewer) Start(context.Context, reviewtypes.RunConfig) (reviewtypes.Process, error) {
    return &scriptedDispatchProcess{events: r.events, waitErr: r.waitErr}, nil
}

type scriptedDispatchProcess struct {
    events  []reviewtypes.Event
    waitErr error
}

func (p *scriptedDispatchProcess) Events() <-chan reviewtypes.Event {
    ch := make(chan reviewtypes.Event, len(p.events))
    for _, ev := range p.events {
        ch <- ev
    }
    close(ch)
    return ch
}

func (p *scriptedDispatchProcess) Wait() error { return p.waitErr }

// Compile-time interface check.
var _ reviewtypes.AgentReviewer = (*stubDispatchReviewer)(nil)
var _ reviewtypes.Process = (*stubDispatchProcess)(nil)
var _ reviewtypes.AgentReviewer = (*scriptedDispatchReviewer)(nil)
var _ reviewtypes.Process = (*scriptedDispatchProcess)(nil)

type captureRunConfigReviewer struct {
    name   string
86 unmodified lines

}

func TestDispatchFork_MultiAgentIgnoresFailedSiblingWhenAnotherSucceeds(t *testing.T) {
    setupCmdTestRepo(t)

if err := seedReviewConfig(context.Background(), map[string]settings.ReviewConfig{
        "agent-a": {Prompt: "review"},
        "agent-b": {Prompt: "review"},
    }); err != nil {
        t.Fatal(err)
    }

quotaErr := errors.New("quota exhausted")
    reviewers := map[string]reviewtypes.AgentReviewer{
        "agent-a": &scriptedDispatchReviewer{
            name: "agent-a",
            events: []reviewtypes.Event{
                reviewtypes.Started{},
                reviewtypes.Finished{Success: false},
            },
            waitErr: quotaErr,
        },
        "agent-b": &scriptedDispatchReviewer{
            name: "agent-b",
            events: []reviewtypes.Event{
                reviewtypes.Started{},
                reviewtypes.AssistantText{Text: "agent-b found no blockers."},
                reviewtypes.Finished{Success: true},
            },
        },
    }
    deps := newDispatchTestDeps(t, []types.AgentName{"agent-a", "agent-b"}, []string{"agent-a", "agent-b"})
    deps.ReviewerFor = func(agentName string) reviewtypes.AgentReviewer { return reviewers[agentName] }

buf := &bytes.Buffer{}
    cmd := review.NewCommand(deps)
    cmd.SetOut(buf)
    cmd.SetErr(&bytes.Buffer{})
    cmd.SetArgs([]string{"general"})

if err := cmd.Execute(); err != nil {
        t.Fatalf("partial reviewer failure should not fail command: %v\nOutput:\n%s", err, buf.String())
    }
    out := buf.String()
    if !strings.Contains(out, "2 agent(s) done — 1 succeeded, 1 failed") {
        t.Fatalf("output missing partial-failure counts:\n%s", out)
    }
    if !strings.Contains(out, "agent-b found no blockers") {
        t.Fatalf("output missing successful reviewer narrative:\n%s", out)
    }
}

func TestDispatchFork_MultiAgentFailsWhenAllReviewersFail(t *testing.T) {
    setupCmdTestRepo(t)

reviewers := map[string]reviewtypes.AgentReviewer{
        "agent-a": &scriptedDispatchReviewer{
            name: "agent-a",
            events: []reviewtypes.Event{
                reviewtypes.Started{},
                reviewtypes.Finished{Success: false},
            },
            waitErr: errors.New("agent-a quota exhausted"),
        },
        "agent-b": &scriptedDispatchReviewer{
            name: "agent-b",
            events: []reviewtypes.Event{
                reviewtypes.Started{},
                reviewtypes.Finished{Success: false},
            },
            waitErr: errors.New("agent-b quota exhausted"),
        },
    }
    deps := newDispatchTestDeps(t, []types.AgentName{"agent-a", "agent-b"}, []string{"agent-a", "agent-b"})
    deps.ReviewerFor = func(agentName string) reviewtypes.AgentReviewer { return reviewers[agentName] }

buf := &bytes.Buffer{}
    cmd := review.NewCommand(deps)
    cmd.SetOut(buf)
    cmd.SetErr(&bytes.Buffer{})
    cmd.SetArgs([]string{"general"})

error := cmd.Execute()
    if err == nil {
        t.Fatalf("expected error when all reviewers fail\nOutput:\n%s", buf.String())
    }
    if !strings.Contains(err.Error(), "review run") {
        t.Fatalf("error should identify review run failure, got %v", err)
    }
}

func TestDispatchFork_MultiAgentPassesPerAgentConfigs(t *testing.T) {
    setupCmdTestRepo(t)