inspect: remove dead code and collapse the synthesis path · Entire
Log in
inspect: remove dead code and collapse the synthesis path
48d27b1→main·
dipree·4w ago·23 files·+162 added/-2,072 removed
Cleanup pass over the review/inspect feature after a long iteration.
Dead code: - delete the unused agent multi-picker (multipicker.go) and PromptForAgent; the multi-agent path fans out over all eligible agents, no picker - delete the unused trail.Store local-storage layer (store.go) and the ID/Priority/Type/Reviewer/Discussion/Checkpoints types it alone used - drop TrailDetailResponse and write-request fields the CLI never sends (TrailUpdateRequest.{Branch,Base,Assignees,Priority,Type}, TrailCreateRequest.{Assignees,Labels,Priority,Type}, BranchCreated) - delete EntireSettings.ReviewConfigFor (only its own test read it); the legacy Review/ReviewFixAgent/ReviewMigrationDismissed fields stay as parse-tolerance shims (the loader uses DisallowUnknownFields)
Collapse the synthesis path: autoSynthesis was always true in production, so the prompted (Auto=false) branch was unreachable. Drop SynthesisSink's Auto/InputTTY/PromptYN, the legacy compose branch, realPromptYN, and the now-dead canPrompt/promptYN sink inputs. The master report now runs unconditionally in TTY and redirected output alike.
Tidy: delegate top-level `trail watch` to the shared trail-review resolver (deleting the bespoke resolveTrailWatch* duplicates), make RunReviewProfileConfigPicker return only error, inline single-caller wrappers (detectScopeBaseRef, hydrateTrailReviewCommentSuggestions, reviewTrailFindingInput, saveDefaultReviewProfile), and fix flushBuffer's dead error-handling return.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Sessions
8fa27a4b7754View transcript
?\ Review Inspect Feature CleanupClaude Code·1 step
Changes
23
cmd/entire/cli
api
Mtrail_types.go+26/-49
review
Mcmd.go+16/-49
Mcmd_test.go+18/-208
Mexport_test.go-12
Dmultipicker.go-106
Dmultipicker_test.go-96
Mpicker.go+16/-38
Mpostrun_sinks.go+3/-3
Mprofile.go-4
Mscope.go+17/-23
Mscope_test.go+10/-14
Msynthesis_sink.go+22/-69
Msynthesis_sink_test.go+17/-119
Mreview_bridge.go+3/-9
Mreview_bridge_test.go+2/-2
settings
Msettings.go-11
Msettings_test.go-13
trail
Dstore.go-489
Dstore_test.go-481
Mtrail.go+3/-126
Mtrail_test.go-81
Mtrail_review_cmd.go+1/-6
Mtrail_watch_cmd.go+8/-64
20 unmodified lines
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
10 unmodified lines
55
56
57
61
62
63
58
59
60
14 unmodified lines
75
76
77
84
85
86
87
88
89
90
78
79
80
81
82
83
84
95
96
97
98
99
100
101
102
103
85
86
87
88
89
90
91
110
111
112
113
114
115
116
117
118
92
93
94
95
96
97
98
20 unmodified lines
// TrailResource represents a single trail from the API.
type TrailResource struct {
ID string `json:"id,omitempty"`
Number int `json:"number,omitempty"`
Branch string `json:"branch"`
Base string `json:"base"`
Title string `json:"title"`
Body string `json:"body"`
Status string `json:"status"`
Phase string `json:"phase,omitempty"`
Author *trail.Author `json:"author"`
Assignees []string `json:"assignees"`
Labels []string `json:"labels"`
Priority string `json:"priority,omitempty"`
Type string `json:"type,omitempty"`
Reviewers []trail.Reviewer `json:"reviewers,omitempty"`
CreatedAt time.Time `json:"created_at"`
UpdatedAt time.Time `json:"updated_at"`
MergedAt *time.Time `json:"merged_at,omitempty"`
CommentCount int `json:"comment_count,omitempty"`
UnresolvedCount int `json:"unresolved_count,omitempty"`
CheckpointCount int `json:"checkpoint_count,omitempty"`
CommitsAhead int `json:"commits_ahead,omitempty"`
ID string `json:"id,omitempty"`
Number int `json:"number,omitempty"`
Branch string `json:"branch"`
Base string `json:"base"`
Title string `json:"title"`
Body string `json:"body"`
Status string `json:"status"`
Phase string `json:"phase,omitempty"`
Author *trail.Author `json:"author"`
Assignees []string `json:"assignees"`
Labels []string `json:"labels"`
CreatedAt time.Time `json:"created_at"`
UpdatedAt time.Time `json:"updated_at"`
MergedAt *time.Time `json:"merged_at,omitempty"`
CommentCount int `json:"comment_count,omitempty"`
UnresolvedCount int `json:"unresolved_count,omitempty"`
CheckpointCount int `json:"checkpoint_count,omitempty"`
CommitsAhead int `json:"commits_ahead,omitempty"`
}
// ToMetadata converts a TrailResource to a trail.Metadata for display.
10 unmodified lines
Author: r.Author,
Assignees: r.Assignees,
Labels: r.Labels,
Priority: trail.Priority(r.Priority),
Type: trail.Type(r.Type),
Reviewers: r.Reviewers,
CreatedAt: r.CreatedAt,
UpdatedAt: r.UpdatedAt,
MergedAt: r.MergedAt,
14 unmodified lines
BranchName string `json:"branch_name"`
// BranchAction is "create" (default) or "link". The CLI sends "link" to
// attach the already-pushed branch instead of backfilling it at base.
BranchAction string `json:"branch_action,omitempty"`
Base string `json:"base,omitempty"`
Status string `json:"status,omitempty"`
Assignees []string `json:"assignees,omitempty"`
Labels []string `json:"labels,omitempty"`
Priority string `json:"priority,omitempty"`
Type string `json:"type,omitempty"`
BranchAction string `json:"branch_action,omitempty"`
Base string `json:"base,omitempty"`
Status string `json:"status,omitempty"`
}
// TrailCreateResponse is the response from POST /api/v1/trails/:org/:repo.
type TrailCreateResponse struct {
Trail TrailResource `json:"trail"`
BranchCreated bool `json:"branch_created"`
}
// TrailDetailResponse is the response from GET /api/v1/trails/:org/:repo/:trailId.
type TrailDetailResponse struct {
Trail TrailResource `json:"trail"`
Discussion trail.Discussion `json:"discussion"`
Checkpoints trail.Checkpoints `json:"checkpoints"`
Trail TrailResource `json:"trail"`
}
// TrailUpdateRequest is the body for PATCH /api/v1/trails/:host/:owner/:repo/:trailId.
// Pointer fields distinguish "not provided" (nil) from "set to value".
// For slices, *[]string is used so nil means "no change" while &[]string{} means "clear".
type TrailUpdateRequest struct {
Branch *string `json:"branch,omitempty"`
Base *string `json:"base,omitempty"`
Status *string `json:"status,omitempty"`
Title *string `json:"title,omitempty"`
Body *string `json:"body,omitempty"`
Assignees *[]string `json:"assignees,omitempty"`
Labels *[]string `json:"labels,omitempty"`
Priority *string `json:"priority,omitempty"`
Type *string `json:"type,omitempty"`
Status *string `json:"status,omitempty"`
Title *string `json:"title,omitempty"`
Body *string `json:"body,omitempty"`
Labels *[]string `json:"labels,omitempty"`
}
// TrailUpdateResponse is the response from PATCH /api/v1/trails/:org/:repo/:trailId.
Mcmd/entire/cli/api/trail_types.go+26/-49
201 unmodified lines
202
203
204
205
206
205
206
207
208
574 unmodified lines
783
784
785
787
786
787
788
789
403 unmodified lines
1193
1194
1195
1197
1196
1197
1198
2 unmodified lines
1201
1202
1203
1206
1204
1205
1206
15 unmodified lines
1222
1223
1224
1228
1225
1226
1227
1231
1228
1229
1230
5 unmodified lines
1236
1237
1238
1243
1244
1239
1240
1241
1242
1243
4 unmodified lines
1248
1249
1250
1255
1251
1252
1253
1254
1260
1255
1256
1257
1258
1265
1259
1260
1261
5 unmodified lines
1267
1268
1269
1277
1270
1271
1272
1273
1274
1279
1280
1281
1282
1283
1284
1285
1286
1287
1288
1289
1290
1291
1275
1276
1277
1278
1279
1280
1281
1282
1283
1284
1297
1285
1286
1287
1288
1289
1290
1303
1304
1291
1292
1293
1294
1309
1295
1296
1297
8 unmodified lines
1306
1307
1308
1324
1325
1326
1327
1328
1329
1330
1331
1332
1333
1334
1335
1336
1337
1338
1309
1310
1311
1312
1343
1313
1314
1315
1316
1347
1348
1317
1318
1319
1320
1353
1321
1322
1323
201 unmodified lines
}, deps)
}
if edit {
_, err := RunReviewProfileConfigPicker(ctx, cmd.OutOrStdout(), deps.GetAgentsWithHooksInstalled, profileName)
return err
return RunReviewProfileConfigPicker(ctx, cmd.OutOrStdout(), deps.GetAgentsWithHooksInstalled, profileName)
}
if findings {
return runReviewFindings(ctx, cmd, deps.NewSilentError)
574 unmodified lines
fmt.Fprintln(out, "Configure later with `entire inspect --configure`.")
fmt.Fprintln(out)
}
if saveErr := saveDefaultReviewProfile(ctx, profileForSetup, profile, saveScope); saveErr != nil {
if saveErr := saveReviewProfile(ctx, profileForSetup, profile, false, saveScope); saveErr != nil {
return saveErr
}
s.ReviewProfiles = map[string]settings.ReviewProfileConfig{profileForSetup: profile}
403 unmodified lines
sinks := composeMultiAgentSinks(multiAgentSinkInputs{
out: out,
isTTY: interactive.IsTerminalWriter(out) && interactive.CanPromptInteractively(),
canPrompt: interactive.CanPromptInteractively(),
agentNames: agentNames,
cancelRun: cancelRun,
runContext: runCtx,
2 unmodified lines
profileName: profileName,
task: profile.Task,
masterName: masterLabel,
autoSynthesis: true,
onSynthesisResult: func(result string) {
aggregateOutput = result
},
15 unmodified lines
return nil
}
// handlePickerError maps multi-picker error sentinels to the appropriate
// handlePickerError maps picker error sentinels to the appropriate
// command-layer response.
// - ErrPickerCancelled → return nil (user cancelled; no error shown)
// - ErrNoAgentsSelected → surface error to user
// - other errors → surface to user
func handlePickerError(cmd *cobra.Command, silentErr func(error) error, pickErr error) error {
if errors.Is(pickErr, ErrPickerCancelled) {
5 unmodified lines
}
// multiAgentSinkInputs collects the parameters composeMultiAgentSinks needs.
// It exists so tests can drive the helper with explicit isTTY / canPrompt
// values instead of monkey-patching interactive helpers at run time.
// It exists so tests can drive the helper with an explicit isTTY value
// instead of monkey-patching interactive helpers at run time.
//
// isTTY here means "the TUI sink is safe to compose" — production callers
// AND IsTerminalWriter(out) with CanPromptInteractively() before passing
4 unmodified lines
type multiAgentSinkInputs struct {
out io.Writer
isTTY bool
canPrompt bool
agentNames []string
cancelRun context.CancelFunc
runContext context.Context
synthesisProvider SynthesisProvider
promptYN func(ctx context.Context, question string, def bool) (bool, error)
perRunPrompt string
profileName string
task string
masterName string
autoSynthesis bool
onSynthesisResult func(result string)
}
5 unmodified lines
cancelRun context.CancelFunc
}
// composeMultiAgentSinks builds the sink slice for a multi-agent run.
// composeMultiAgentSinks builds the sink slice for a multi-agent run. The
// master adjudication phase (SynthesisSink) runs unconditionally when a
// provider is configured — it needs no stdin, so it is available in TTY,
// redirected, and CI output alike.
//
// - Non-TTY: [DumpSink, SynthesisSink?] — narrative dump plus profile-native
// final report when autoSynthesis is enabled.
// - TTY + profile-native auto synthesis: [TUISink, buffered DumpSink,\
// buffered SynthesisSink, TUI finalizer, buffer flusher]. The TUI stays up
// during the judge phase and post-run stdout is flushed after the alt-screen
// exits.
// - TTY without auto synthesis: [TUISink, TUI finalizer, DumpSink,\
// SynthesisSink?]. Legacy prompted synthesis runs after the TUI exits so the
// prompt can safely use stdin/stdout.
//
// Prompted legacy synthesis is still only appended when canPrompt is true.
// Profile-native auto synthesis does not need stdin, so it is available in
// redirected and CI output too.
// - Non-TTY: [DumpSink, SynthesisSink?] — narrative dump plus the final report.
// - TTY: [TUISink, buffered DumpSink, buffered SynthesisSink, buffer flusher].
// The TUI stays up during the judge phase and post-run stdout is flushed
// after the alt-screen exits.
// - TTY without a provider: [TUISink, TUI finalizer, DumpSink].
func composeMultiAgentSinks(in multiAgentSinkInputs) []reviewtypes.Sink {
sinks := []reviewtypes.Sink{}
if in.isTTY {
tui := NewTUISink(in.agentNames, in.cancelRun, in.out, os.Stdin)
sinks = append(sinks, tui)
if in.autoSynthesis && in.synthesisProvider != nil {
if in.synthesisProvider != nil {
postRunOut := &bytes.Buffer{}
sinks = append(sinks, DumpSink{W: postRunOut})
sinks = append(sinks, SynthesisSink{
Provider: in.synthesisProvider,
Writer: postRunOut,
InputTTY: in.canPrompt,
PromptYN: in.promptYN,
PerRunPrompt: in.perRunPrompt,
ProfileName: in.profileName,
Task: in.task,
MasterName: in.masterName,
Auto: true,
RunContext: in.runContext,
OnResult: in.onSynthesisResult,
OnStart: func() {
8 unmodified lines
}
sinks = append(sinks, tuiPostRunCompleteSink{tui: tui})
sinks = append(sinks, DumpSink{W: in.out})
if in.synthesisProvider != nil && in.canPrompt {
sinks = append(sinks, SynthesisSink{
Provider: in.synthesisProvider,
Writer: in.out,
InputTTY: in.canPrompt,
PromptYN: in.promptYN,
PerRunPrompt: in.perRunPrompt,
ProfileName: in.profileName,
Task: in.task,
MasterName: in.masterName,
Auto: false,
RunContext: in.runContext,
OnResult: in.onSynthesisResult,
})
}
return sinks
}
sinks = append(sinks, DumpSink{W: in.out})
if in.synthesisProvider != nil && (in.autoSynthesis || in.canPrompt) {
if in.synthesisProvider != nil {
sinks = append(sinks, SynthesisSink{
Provider: in.synthesisProvider,
Writer: in.out,
InputTTY: in.canPrompt,
PromptYN: in.promptYN,
PerRunPrompt: in.perRunPrompt,
ProfileName: in.profileName,
Task: in.task,
MasterName: in.masterName,
Auto: in.autoSynthesis,
RunContext: in.runContext,
OnResult: in.onSynthesisResult,
})
Mcmd/entire/cli/review/cmd.go+16/-49
466 unmodified lines
467
468
469
470
471
470
471
472
473
474
475
478
479
476
477
478
105 unmodified lines
584
585
586
591
592
587
588
589
590
4 unmodified lines
595
596
597
603
604
605
606
607
608
609
610
611
612
598
614
599
600
601
602
4 unmodified lines
607
608
609
625
626
627
610
611
612
69 unmodified lines
682
683
684
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
740
741
742
743
744
745
746
747
748
749
750
751
752
753
754
755
756
757
758
759
760
761
762
763
764
765
766
767
768
769
770
771
772
773
774
775
776
777
778
779
780
781
782
783
784
785
786
787
788
789
790
791
792
793
794
795
796
797
798
799
800
801
802
803
804
805
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
844
845
846
847
848
849
850
851
852
853
854
855
856
857
685
686
687
18 unmodified lines
706
707
708
882
709
710
711
1 unmodified line
713
714
715
890
716
717
892
718
719
895
720
721
722
723
898
724
725
900
726
727
728
3 unmodified lines
732
733
734
910
735
736
737
738
739
740
917
918
919
920
921
741
742
743
744
923
745
746
747
748
3 unmodified lines
752
753
754
933
755
756
757
113 unmodified lines
871
872
873
1053
874
875
876
4 unmodified lines
881
882
883
1064
1065
884
885
886
1067
1068
887
888
889
890
891
38 unmodified lines
930
931
932
1113
1114
1115
1116
1117
1118
1119
1120
933
1122
934
935
936
937
29 unmodified lines
967
968
969
1158
970
971
972
973
17 unmodified lines
991
992
993
1182
994
995
996
997
1187
998
999
1000
8 unmodified lines
1009
1010
1011
1202
1203
1012
1013
1014
1015
1016
466 unmodified lines
t *testing.T,
installed []types.AgentName,
launchableAgents []string,
multiPickerFn func(ctx context.Context, eligible []review.AgentChoice) (review.PickedAgents, error),
promptForAgentFn func(ctx context.Context, eligible []review.AgentChoice) (string, error),
) review.Deps {
t.Helper()
launchableSet := make(map[string]struct{}, len(launchableAgents))
for _, name := range launchableAgents {
launchableSet[name] = struct{}{}
}
_ = promptForAgentFn
_ = multiPickerFn
return review.Deps{
GetAgentsWithHooksInstalled: func(_ context.Context) []types.AgentName {
return installed
105 unmodified lines
}
// TestDispatchFork_TwoLaunchableNoOverride verifies that when 2+ launchable
// agents are configured and --agent is empty, the profile fan-out runs without
// invoking the old per-run multi-picker.
// agents are configured and --agent is empty, the profile fan-out runs cleanly.
func TestDispatchFork_TwoLaunchableNoOverride(t *testing.T) {
setupCmdTestRepo(t)
4 unmodified lines
t.Fatal(err)
}
multiPickerCalled := false
multiPickerFn := func(_ context.Context, eligible []review.AgentChoice) (review.PickedAgents, error) {
multiPickerCalled = true
names := make([]string, 0, len(eligible))
for _, e := range eligible {
names = append(names, e.Name)
}
return review.PickedAgents{Names: names, PerRun: ""}, nil
}
installed := []types.AgentName{"agent-a", "agent-b"}
deps := newDispatchTestDeps(t, installed, []string{"agent-a", "agent-b"}, multiPickerFn, nil)
deps := newDispatchTestDeps(t, installed, []string{"agent-a", "agent-b"})
buf := &bytes.Buffer{}
cmd := review.NewCommand(deps)
4 unmodified lines
if err := cmd.Execute(); err != nil {
t.Fatalf("unexpected error: %v", err)
}
if multiPickerCalled {
t.Error("multi-picker should not be invoked; profile config is the fan-out contract")
}
}
func TestDispatchFork_MultiAgentPassesPerAgentConfigs(t *testing.T) {
69 unmodified lines
}
}
// TestDispatchFork_OneLaunchableOneNonLaunchableNoOverride verifies that when
// only 1 agent is launchable (the other is non-launchable), the single-agent
// path is taken (no multi-picker). Uses cursor (real non-launchable agent with
// hooks) + agent-a (fake launchable stub).
func TestDispatchFork_OneLaunchableOneNonLaunchableNoOverride(t *testing.T) {
setupCmdTestRepo(t)
installHooksForCmdTest(t, "cursor")
if err := seedReviewConfig(context.Background(), map[string]settings.ReviewConfig{
"cursor": {Prompt: "review"},
"agent-a": {Prompt: "review"},
}); err != nil {
t.Fatal(err)
}
multiPickerCalled := false
multiPickerFn := func(_ context.Context, _ []review.AgentChoice) (review.PickedAgents, error) {
multiPickerCalled = true
return review.PickedAgents{}, errors.New("should not be called")
}
// Stub single-select picker to avoid TTY: always picks cursor.
singlePickerFn := func(_ context.Context, _ []review.AgentChoice) (string, error) {
return "cursor", nil
}
installed := []types.AgentName{"cursor", "agent-a"}
// Only agent-a is launchable. With 1 launchable agent, computeLaunchableEligible
// returns 1 entry, so multi-path is skipped. The single-select picker picks cursor.
// ReviewerFor("cursor") returns nil → marker fallback path (writes marker file).
deps := newDispatchTestDeps(t, installed, []string{"agent-a"}, multiPickerFn, singlePickerFn)
cmd := review.NewCommand(deps)
cmd.SetOut(&bytes.Buffer{})
cmd.SetErr(&bytes.Buffer{})
cmd.SetArgs([]string{"general"})
executeErr := cmd.Execute() // may error (agent-a not a real agent); we only care about picker routing
_ = executeErr // intentionally ignored: this test only asserts picker routing
if multiPickerCalled {
t.Error("multi-picker should NOT be invoked when only 1 launchable agent is configured")
}
}
// TestDispatchFork_TwoLaunchableWithAgentOverride verifies that --agent flag
// bypasses the multi-picker even when 2+ launchable agents are configured.
// The test uses cursor (non-launchable, real agent) + agent-a (fake launchable)
// with --agent cursor so the single-agent path runs to completion via marker
// fallback (cursor is non-launchable in reviewerFor, so nil → marker fallback).
func TestDispatchFork_TwoLaunchableWithAgentOverride(t *testing.T) {
setupCmdTestRepo(t)
installHooksForCmdTest(t, "cursor") // cursor needs real hooks
multiPickerCalled := false
multiPickerFn := func(_ context.Context, _ []review.AgentChoice) (review.PickedAgents, error) {
multiPickerCalled = true
return review.PickedAgents{}, errors.New("should not be called")
}
// cursor + agent-a both installed; agent-a is launchable but cursor is not.
// With 1 launchable agent (agent-a) among the 2 eligible agents, the
// multi-agent path would NOT fire (needs 2+ launchable). But when we
// additionally pass --agent cursor, the multi-picker is bypassed by the
// agentOverride check at the top of step 3.
installed := []types.AgentName{"cursor", "agent-a"}
deps := newDispatchTestDeps(t, installed, []string{"agent-a"}, multiPickerFn, nil)
buf := &bytes.Buffer{}
cmd := review.NewCommand(deps)
cmd.SetOut(buf)
cmd.SetErr(&bytes.Buffer{})
cmd.SetArgs([]string{"general", "--agent", "cursor"})
// cursor is not launchable in our stub (reviewerFor returns nil), so it
// falls through to RunMarkerFallback. That's fine — we only care that
// multiPickerCalled is false.
executeErr := cmd.Execute()
_ = executeErr // intentionally ignored: this test only asserts picker routing
if multiPickerCalled {
t.Error("multi-picker should NOT be invoked when --agent override is set")
}
}
// TestDispatchFork_MultiPickerCancellationExitsCleanly verifies that when
// the multi-picker is cancelled (ErrPickerCancelled), the command exits with
// nil error (no user-facing error).
func TestDispatchFork_MultiPickerCancellationExitsCleanly(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)
}
multiPickerFn := func(_ context.Context, _ []review.AgentChoice) (review.PickedAgents, error) {
return review.PickedAgents{}, review.ErrPickerCancelled
}
installed := []types.AgentName{"agent-a", "agent-b"}
deps := newDispatchTestDeps(t, installed, []string{"agent-a", "agent-b"}, multiPickerFn, nil)
errBuf := &bytes.Buffer{}
cmd := review.NewCommand(deps)
cmd.SetOut(&bytes.Buffer{})
cmd.SetErr(errBuf)
cmd.SetArgs([]string{"general"})
err := cmd.Execute()
if err != nil {
t.Errorf("ErrPickerCancelled should produce nil command error, got: %v", err)
}
}
// TestDispatchFork_MultiPickerNoSelectionNotUsed verifies profile fan-out no
// longer asks a per-run multi-picker, so picker selection errors are irrelevant.
func TestDispatchFork_MultiPickerNoSelectionSurfacesError(t *testing.T) {
setupCmdTestRepo(t)
multiPickerCalled := false
multiPickerFn := func(_ context.Context, _ []review.AgentChoice) (review.PickedAgents, error) {
multiPickerCalled = true
return review.PickedAgents{}, review.ErrNoAgentsSelected
}
installed := []types.AgentName{"agent-a", "agent-b"}
deps := newDispatchTestDeps(t, installed, []string{"agent-a", "agent-b"}, multiPickerFn, nil)
cmd := review.NewCommand(deps)
cmd.SetOut(&bytes.Buffer{})
cmd.SetErr(&bytes.Buffer{})
cmd.SetArgs([]string{"general"})
if err := cmd.Execute(); err != nil {
t.Fatalf("unexpected error: %v", err)
}
if multiPickerCalled {
t.Error("multi-picker should not be called by profile fan-out")
}
}
// --- Synthesis sink dispatch tests (CU10) ---
// stubCmdSynthesisProvider is a minimal SynthesisProvider for cmd-level tests.
18 unmodified lines
tests := []struct {
name string
isTTY bool
canPrompt bool
provider review.SynthesisProvider
wantTUI bool
wantDump bool
1 unmodified line
wantTotal int
}{
{
name: "non-tty omits tui and synth",
name: "non-tty omits tui but auto-synthesizes with provider",
isTTY: false,
canPrompt: false,
provider: provider,
wantDump: true,
wantTotal: 1,
wantSynth: true,
wantTotal: 2,
},
{
name: "tty with provider and prompt appends synth after tui finalizer",
name: "tty with provider buffers dump and synth before the flusher",
isTTY: true,
canPrompt: true,
provider: provider,
wantTUI: true,
wantDump: true,
3 unmodified lines
{
name: "tty without provider skips synth",
isTTY: true,
canPrompt: true,
provider: nil,
wantTUI: true,
wantDump: true,
wantTotal: 3,
},
{
name: "tty without prompt skips legacy synth even with provider",
isTTY: true,
canPrompt: false,
provider: provider,
wantTUI: true,
name: "non-tty without provider is dump only",
isTTY: false,
provider: nil,
wantDump: true,
wantTotal: 3,
wantTotal: 1,
},
}
3 unmodified lines
sinks := review.ExposedComposeMultiAgentSinks(review.SinkComposeInputs{
Out: &bytes.Buffer{},
IsTTY: tt.isTTY,
CanPrompt: tt.canPrompt,
AgentNames: []string{"a", "b"},
CancelRun: noopCancel,
SynthesisProvider: tt.provider,
113 unmodified lines
multi := review.ExposedComposeMultiAgentSinks(review.SinkComposeInputs{
Out: &bytes.Buffer{},
IsTTY: true,
CanPrompt: true,
AgentNames: []string{"a", "b"},
CancelRun: func() {},
SynthesisProvider: provider,
4 unmodified lines
if _, ok := multi[0].(*review.TUISink); !ok {
t.Fatalf("multi sink[0] = %T, want *TUISink", multi[0])
}
if _, ok := multi[2].(review.DumpSink); !ok {
t.Fatalf("multi sink[2] = %T, want DumpSink", multi[2])
if _, ok := multi[1].(review.DumpSink); !ok {
t.Fatalf("multi sink[1] = %T, want buffered DumpSink", multi[1])
}
if _, ok := multi[3].(review.SynthesisSink); !ok {
t.Fatalf("multi sink[3] = %T, want SynthesisSink", multi[3])
if _, ok := multi[2].(review.SynthesisSink); !ok {
t.Fatalf("multi sink[2] = %T, want SynthesisSink", multi[2])
}
single := review.ExposedComposeSingleAgentSinks(review.SingleAgentSinkComposeInputs{
38 unmodified lines
t.Fatal(err)
}
multiPickerFn := func(_ context.Context, eligible []review.AgentChoice) (review.PickedAgents, error) {
names := make([]string, 0, len(eligible))
for _, e := range eligible {
names = append(names, e.Name)
}
return review.PickedAgents{Names: names, PerRun: ""}, nil
}
installed := []types.AgentName{"agent-a", "agent-b"}
deps := newDispatchTestDeps(t, installed, []string{"agent-a", "agent-b"}, multiPickerFn, nil)
deps := newDispatchTestDeps(t, installed, []string{"agent-a", "agent-b"})
// Profile-native review uses the profile master rather than deps-level synthesis.
buf := &bytes.Buffer{}
29 unmodified lines
// cursor is installed but not launchable (ReviewerFor returns nil).
installed := []types.AgentName{"cursor"}
deps := newDispatchTestDeps(t, installed, nil /* no launchable */, nil, nil)
deps := newDispatchTestDeps(t, installed, nil /* no launchable */)
_ = provider
buf := &bytes.Buffer{}
17 unmodified lines
sinks := review.ExposedComposeMultiAgentSinks(review.SinkComposeInputs{
Out: &bytes.Buffer{},
IsTTY: true,
CanPrompt: true,
AgentNames: []string{"a", "b"},
CancelRun: func() {},
SynthesisProvider: provider,
MasterName: testAgentName,
AutoSynthesis: true,
})
if len(sinks) != 4 {
t.Fatalf("len(sinks) = %d, want 4", len(sinks))
8 unmodified lines
if !ok {
t.Fatalf("sink[2] = %T, want SynthesisSink", sinks[2])
}
if !synth.Auto || synth.MasterName != testAgentName {
t.Fatalf("synthesis sink = Auto:%v MasterName:%q, want true/%s", synth.Auto, synth.MasterName, testAgentName)
if synth.MasterName != testAgentName {
t.Fatalf("synthesis sink MasterName = %q, want %s", synth.MasterName, testAgentName)
}
if synth.OnStart == nil || synth.OnComplete == nil {
t.Fatal("auto synthesis should notify the TUI when the final judge starts/completes")
Mcmd/entire/cli/review/cmd_test.go+18/-208
3 unmodified lines
4
5
6
7
8
7
8
9
10 unmodified lines
20
21
22
25
23
24
25
29
26
27
32
28
29
30
9 unmodified lines
40
41
42
48
43
44
45
52
46
47
55
48
49
50
8 unmodified lines
59
60
61
70
71
72
73
62
63
64
3 unmodified lines
"context"
"io"
"charm.land/huh/v2"
reviewtypes "github.com/entireio/cli/cmd/entire/cli/review/types"
)
10 unmodified lines
type SinkComposeInputs struct {
Out io.Writer
IsTTY bool
CanPrompt bool
AgentNames []string
CancelRun context.CancelFunc
SynthesisProvider SynthesisProvider
PromptYN func(ctx context.Context, question string, def bool) (bool, error)
PerRunPrompt string
MasterName string
AutoSynthesis bool
}
type SingleAgentSinkComposeInputs struct {
9 unmodified lines
return composeMultiAgentSinks(multiAgentSinkInputs{
out: in.Out,
isTTY: in.IsTTY,
canPrompt: in.CanPrompt,
agentNames: in.AgentNames,
cancelRun: in.CancelRun,
synthesisProvider: in.SynthesisProvider,
promptYN: in.PromptYN,
perRunPrompt: in.PerRunPrompt,
masterName: in.MasterName,
autoSynthesis: in.AutoSynthesis,
})
}
8 unmodified lines
})
}
func ExposedBuildAgentMultiSelect(options []huh.Option[string], picked *[]string) *huh.MultiSelect[string] {
return buildAgentMultiSelect(options, picked)
}
// ExposedFindTUISink exposes findTUISink for tests.
func ExposedFindTUISink(sinks []reviewtypes.Sink) (*TUISink, bool) {
return findTUISink(sinks)
Mcmd/entire/cli/review/export_test.go-12
1
2
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
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
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
94
95
96
97
98
99
100
101
102
103
104
105
106
// Package review — see env.go for package-level rationale.
//
// multipicker.go provides spawn-time agent multi-selection and per-run
// prompt collection for multi-agent review runs. When 2+ launchable agents
// are configured AND the user has not passed --agent, the dispatch logic
// in cmd.go calls PickAgents to let the user choose a subset and optionally
// add a one-off prompt without editing settings.
package review
import (
"context"
"errors"
"fmt"
"sort"
"charm.land/huh/v2"
)
// PickedAgents is the result of PickAgents: the agents the user selected
// for this run, plus an optional per-run prompt to append to the composed
// review prompt for each agent.
type PickedAgents struct {
// Names contains the agent registry keys selected by the user,
// e.g. ["claude-code", "codex"]. Sorted alphabetically.
Names []string
// PerRun is optional textarea content; "" when the user skipped or cleared it.
PerRun string
}
// ErrPickerCancelled is returned when the user aborts the multi-select.
var ErrPickerCancelled = errors.New("agent picker cancelled")
// ErrNoAgentsSelected is returned when the user unchecks all agents.
// Caller should surface a clear error rather than running with zero agents.
var ErrNoAgentsSelected = errors.New("no agents selected for review")
// PickAgents shows a multi-select form populated from eligible (the agents
// that are both configured AND have an AgentReviewer), pre-checks all of
// them, and returns the user's selection plus an optional per-run prompt.
//
// Returns ErrPickerCancelled if the user aborts. An empty selection (user
// unchecked all boxes) returns ErrNoAgentsSelected.
//
// Requires len(eligible) >= 2; returns an error if the caller passes fewer
// than 2 choices — this function is for multi-agent flows only.
func PickAgents(ctx context.Context, eligible []AgentChoice) (PickedAgents, error) {
if len(eligible) < 2 {
return PickedAgents{}, fmt.Errorf("PickAgents requires at least 2 eligible agents, got %d", len(eligible))
}
if ctx.Err() != nil {
return PickedAgents{}, ErrPickerCancelled
}
// Sort alphabetically for stable display order regardless of how the
// caller populated the slice.
sorted := make([]AgentChoice, len(eligible))
copy(sorted, eligible)
sort.Slice(sorted, func(i, j int) bool { return sorted[i].Name < sorted[j].Name })
// Build options pre-selected (all agents checked by default — mirrors
// PR #1018 behaviour so the user can just press Enter to run all).
options := make([]huh.Option[string], 0, len(sorted))
for _, c := range sorted {
options = append(options, huh.NewOption(c.Label, c.Name).Selected(true))
}
var picked []string
multiForm := newAccessibleForm(huh.NewGroup(
buildAgentMultiSelect(options, &picked),
))
if err := multiForm.RunWithContext(ctx); err != nil {
return PickedAgents{}, ErrPickerCancelled
}
if len(picked) == 0 {
return PickedAgents{}, ErrNoAgentsSelected
}
// Sort the selection alphabetically so the caller receives a stable slice.
sort.Strings(picked)
// Per-run prompt: optional textarea presented after agent selection.
var perRun string
promptForm := newAccessibleForm(huh.NewGroup(
huh.NewText().
Title("Optional per-run prompt").
Description("e.g. 'focus on auth' — appended to the review prompt for this run only. Leave blank to skip.").
Value(&perRun),
))
if err := promptForm.RunWithContext(ctx); err != nil {
// Cancellation on the prompt step (Ctrl+C) propagates as picker
// cancelled — we don't want an empty prompt here; user can retry.
return PickedAgents{}, ErrPickerCancelled
}
return PickedAgents{Names: picked, PerRun: perRun}, nil
}
func buildAgentMultiSelect(options []huh.Option[string], picked *[]string) *huh.MultiSelect[string] {
return huh.NewMultiSelect[string]().
Title("Which agents should run this review?").
Options(options...).
Height(len(options) + 1).
Value(picked)
}
Dcmd/entire/cli/review/multipicker.go-106
1
2
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
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
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
94
95
96
package review_test
import (
"context"
"errors"
"strings"
"testing"
"charm.land/huh/v2"
"github.com/entireio/cli/cmd/entire/cli/review"
)
// TestPickAgents_TooFewEligibleReturnsError verifies that calling PickAgents
// with fewer than 2 choices returns an error — it is the caller's
// responsibility to route single-agent flows through the single-agent path.
func TestPickAgents_TooFewEligibleReturnsError(t *testing.T) {
t.Parallel()
_, err := review.PickAgents(context.Background(), []review.AgentChoice{
{Name: "claude-code", Label: "claude-code (1 skill configured)"},
})
if err == nil {
t.Fatal("expected error for single-element eligible list")
}
// Must NOT be ErrPickerCancelled or ErrNoAgentsSelected — it's a caller
// contract violation, not a user action.
if errors.Is(err, review.ErrPickerCancelled) {
t.Errorf("should not return ErrPickerCancelled for too-few-eligible")
}
if errors.Is(err, review.ErrNoAgentsSelected) {
t.Errorf("should not return ErrNoAgentsSelected for too-few-eligible")
}
}
// TestPickAgents_EmptyEligibleReturnsError covers the zero-length case.
func TestPickAgents_EmptyEligibleReturnsError(t *testing.T) {
t.Parallel()
_, err := review.PickAgents(context.Background(), nil)
if err == nil {
t.Fatal("expected error for empty eligible list")
}
if errors.Is(err, review.ErrPickerCancelled) || errors.Is(err, review.ErrNoAgentsSelected) {
t.Errorf("wrong error sentinel for empty eligible: %v", err)
}
}
// TestPickAgents_CancelledContextReturnsPickerCancelled verifies that a
// pre-cancelled context causes PickAgents to return ErrPickerCancelled
// (not a raw context.Canceled). The huh RunWithContext method returns an
// error for a cancelled context, which PickAgents maps to ErrPickerCancelled.
func TestPickAgents_CancelledContextReturnsPickerCancelled(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithCancel(context.Background())
cancel() // cancel before calling PickAgents
_, err := review.PickAgents(ctx, []review.AgentChoice{
{Name: "claude-code", Label: "claude-code (1 skill configured)"},
{Name: "codex", Label: "codex (2 skills configured)"},
})
if err == nil {
t.Fatal("expected error from cancelled context")
}
if !errors.Is(err, review.ErrPickerCancelled) {
t.Errorf("expected ErrPickerCancelled, got: %v", err)
}
}
// TestPickedAgentsSentinels verifies the exported error sentinels are distinct
// values so callers can distinguish them cleanly.
func TestPickedAgentsSentinels(t *testing.T) {
t.Parallel()
if errors.Is(review.ErrPickerCancelled, review.ErrNoAgentsSelected) {
t.Error("ErrPickerCancelled and ErrNoAgentsSelected must be distinct")
}
if errors.Is(review.ErrNoAgentsSelected, review.ErrPickerCancelled) {
t.Error("ErrNoAgentsSelected and ErrPickerCancelled must be distinct")
}
}
func TestAgentMultiSelectRendersAllEligibleAgents(t *testing.T) {
t.Parallel()
var picked []string
field := review.ExposedBuildAgentMultiSelect([]huh.Option[string]{
huh.NewOption("claude-code (3 skills configured)", "claude-code").Selected(true),
huh.NewOption("codex (1 skills configured)", "codex").Selected(true),
}, &picked).WithWidth(80)
field.Focus()
view := field.View()
for _, want := range []string{"claude-code", "codex"} {
if !strings.Contains(view, want) {
t.Fatalf("agent picker did not render %q:\n%s", want, view)
}
}
}
Dcmd/entire/cli/review/multipicker_test.go-96
25 unmodified lines
26
27
28
29
30
31
32
33
34
35
36
732 unmodified lines
769
770
771
767
772
773
774
775
776
777
778
774
779
780
781
782
20 unmodified lines
803
804
805
801
806
807
808
809
810
811
807
812
813
814
815
83 unmodified lines
899
900
901
897
902
903
904
905
901
906
907
908
909
17 unmodified lines
927
928
929
925
930
931
932
933
934
930
935
936
937
938
934
939
940
941
937
942
943
944
940
945
946
947
948
194 unmodified lines
1143
1144
1145
1141
1142
1143
1144
1145
1146
1147
1148
1149
1150
1151
1152
1153
1154
1155
1156
1157
1158
1159
1160
1161
1162
1163
1164
1165
1166
1167
1146
1147
1148
25 unmodified lines
"github.com/entireio/cli/cmd/entire/cli/uiform"
)
// ErrPickerCancelled is returned when the user aborts an interactive picker
// or confirmation (Ctrl+C / Esc). Callers map it to a clean, silent exit
// rather than a command error.
var ErrPickerCancelled = errors.New("picker cancelled")
// AgentChoice is one row in the spawn-time picker. Name is the agent
// registry key (used for marker/override); Label is the picker-visible
// string ("<name> (N skills configured)" or "<name> (prompt-only)").
732 unmodified lines
return runNow, nil
}
func RunReviewProfileConfigPicker(ctx context.Context, out io.Writer, getInstalled func(context.Context) []types.AgentName, profileName string) (map[string]settings.ReviewConfig, error) {
func RunReviewProfileConfigPicker(ctx context.Context, out io.Writer, getInstalled func(context.Context) []types.AgentName, profileName string) error {
profileName = strings.TrimSpace(profileName)
if profileName == "" {
profileName = DefaultProfileName
}
installed := getInstalled(ctx)
if len(installed) == 0 {
return nil, errors.New(
return errors.New(
"no agents with hooks installed; " +
"run 'entire configure --agent <name>' to install hooks for one, " +
"or 'entire enable' to set up the repo",
20 unmodified lines
if len(configurable) == 0 {
prefsPath, pathErr := settings.ClonePreferencesPath(ctx)
if pathErr != nil {
return nil, errors.New(
return errors.New(
"no installed agents have curated review skills; " +
"install an eligible agent and run `entire inspect --edit`, " +
"or edit clone-local review preferences under review.<agent-name>",
)
}
return nil, fmt.Errorf(
return fmt.Errorf(
"no installed agents have curated review skills; "+
"install an eligible agent and run `entire inspect --edit`, "+
"or edit clone-local review preferences (%s) under review.<agent-name>",
83 unmodified lines
form := newAccessibleForm(huh.NewGroup(fields...))
if err := form.RunWithContext(ctx); err != nil {
return nil, fmt.Errorf("picker for %s: %w", c.name, err)
return fmt.Errorf("picker for %s: %w", c.name, err)
}
model, err := resolvePickedReviewModel(ctx, string(c.name), pickedModel)
if err != nil {
return nil, err
return err
}
cfg := settings.ReviewConfig{
17 unmodified lines
// The emptiness check runs on `merged`, not `selected`.
if len(merged) == 0 {
return nil, errors.New("no review skills or prompt configured")
return errors.New("no review skills or prompt configured")
}
judgeAgent, err := pickReviewJudgeAgentPreference(ctx, merged, existingJudge)
if err != nil {
return nil, err
return err
}
scope, err := promptForSettingsScope(ctx, false)
if err != nil {
return nil, err
return err
}
if err := saveReviewProfileConfig(ctx, profileName, merged, judgeAgent, scope); err != nil {
return nil, err
return err
}
fmt.Fprintf(out, "Saved review profile %q to %s. Edit later with `entire inspect --edit --profile %s`.\n", profileName, scope.file(), profileName)
return merged, nil
return nil
}
// MergePickerResults combines the picker's output with existing review
194 unmodified lines
return out
}
// PromptForAgent renders the single-select agent picker shown when more than
// one eligible agent is configured. Returns the chosen agent name. Respects
// accessibility mode via newAccessibleForm.
func PromptForAgent(ctx context.Context, eligible []AgentChoice) (string, error) {
if err := ctx.Err(); err != nil {
return "", fmt.Errorf("agent picker: %w", err)
}
if len(eligible) == 0 {
return "", errors.New("no eligible agents to prompt for")
}
options := make([]huh.Option[string], 0, len(eligible))
for _, c := range eligible {
options = append(options, huh.NewOption(c.Label, c.Name))
}
picked := eligible[0].Name
form := newAccessibleForm(huh.NewGroup(
huh.NewSelect[string]().
Title("Which agent should run this review?").
Options(options...).
Value(&picked),
))
if err := form.RunWithContext(ctx); err != nil {
return "", fmt.Errorf("agent picker: %w", err)
}
return picked, nil
}
// VerifyConfiguredSkillsInstalled is the spawn-time backstop for the
// silent-failure vector. For each skill in cfg.Skills, check it's either a
// curated built-in or returned by the agent's SkillDiscoverer; fail with a
Mcmd/entire/cli/review/picker.go+16/-38
25 unmodified lines
26
27
28
29
30
31
29
30
31
32
25 unmodified lines
if s.buf == nil || s.out == nil || s.buf.Len() == 0 {
return
}
if _, err := s.out.Write(s.buf.Bytes()); err != nil {
return
}
// Best-effort flush of buffered post-run output; a write error here means
// the terminal is gone and there is nothing actionable to do.
_, _ = s.out.Write(s.buf.Bytes()) //nolint:errcheck // best-effort terminal flush
}
Mcmd/entire/cli/review/postrun_sinks.go+3/-3
432 unmodified lines
433
434
435
436
437
438
439
436
437
438
432 unmodified lines
return settings.EntireSettingsFile
}
func saveDefaultReviewProfile(ctx context.Context, profileName string, profile settings.ReviewProfileConfig, scope reviewSettingsScope) error {
return saveReviewProfile(ctx, profileName, profile, false, scope)
}
// saveReviewProfile persists one profile into the chosen settings file via a
// raw read-modify-write so unrelated keys (and other profiles) are preserved.
func saveReviewProfile(ctx context.Context, profileName string, profile settings.ReviewProfileConfig, makeDefault bool, scope reviewSettingsScope) error {
Mcmd/entire/cli/review/profile.go-4
89 unmodified lines
90
91
92
93
93
94
95
96
25 unmodified lines
122
123
124
125
126
127
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
10 unmodified lines
154
155
156
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
157
158
159
89 unmodified lines
}
baseRef = baseOverride
} else {
baseRef, err = detectScopeBaseRef(ctx, repo)
baseRef, err = fallbackScopeRef(repo)
if err != nil {
return ScopeStats{}, fmt.Errorf("detect scope base ref: %w", err)
}
25 unmodified lines
}, nil
}
// detectScopeBaseRef returns the mainline ref the review should be scoped
// against, walking the fallback chain origin/HEAD → origin/main →
// origin/master → main → master and returning the first that exists.
// repoWorktreePath returns the working-tree path for repo, or an error if the
// repo is bare or its worktree can't be resolved. ComputeScopeStats uses this
// as the cwd for the runGit invocations in countCommits / countFilesChanged /
// countUncommitted.
func repoWorktreePath(repo *git.Repository) (string, error) {
wt, err := repo.Worktree()
if err != nil {
return "", fmt.Errorf("resolve worktree: %w", err)
}
return wt.Filesystem().Root(), nil
}
// fallbackScopeRef returns the mainline ref the review should be scoped
// against: the first existing ref from the fallback chain origin/HEAD →
// origin/main → origin/master → main → master. Returns an error naming the
// tried refs if none exist.
//
// A previous implementation tried to be clever: it picked the merged-into-HEAD
// branch with the most recent committerdate, on the theory that stacked PRs
10 unmodified lines
// Stacked PR review is now served by the explicit `--base <ref>` flag at
// the command surface, not an inference. The default stays predictable
// (always mainline); the override is explicit when users actually want it.
func detectScopeBaseRef(_ context.Context, repo *git.Repository) (string, error) {
return fallbackScopeRef(repo)
}
// repoWorktreePath returns the working-tree path for repo, or an error if the
// repo is bare or its worktree can't be resolved. ComputeScopeStats uses this
// as the cwd for the runGit invocations in countCommits / countFilesChanged /
// countUncommitted.
func repoWorktreePath(repo *git.Repository) (string, error) {
wt, err := repo.Worktree()
if err != nil {
return "", fmt.Errorf("resolve worktree: %w", err)
}
return wt.Filesystem().Root(), nil
}
// fallbackScopeRef returns the first existing ref from the fallback chain:
// origin/HEAD → origin/main → origin/master → main → master.
// Returns an error naming the tried refs if none exist.
func fallbackScopeRef(repo *git.Repository) (string, error) {
chain := []string{"origin/HEAD", "origin/main", "origin/master", "main", "master"}
for _, name := range chain {
Mcmd/entire/cli/review/scope.go+17/-23
131 unmodified lines
132
133
134
135
135
136
137
137
138
139
140
95 unmodified lines
236
237
238
239
239
240
242
241
242
244
243
244
245
246
20 unmodified lines
267
268
269
271
270
271
274
272
273
276
274
275
276
277
27 unmodified lines
305
306
307
310
308
309
312
310
311
312
313
65 unmodified lines
379
380
381
384
382
383
384
385
2 unmodified lines
388
389
390
393
394
391
392
393
398
394
395
396
397
131 unmodified lines
ctx := context.Background()
repo := openTestRepo(t, dir)
baseRef, err := detectScopeBaseRef(ctx, repo)
baseRef, err := fallbackScopeRef(repo)
if err != nil {
t.Fatalf("detectScopeBaseRef: %v", err)
t.Fatalf("fallbackScopeRef: %v", err)
}
if baseRef != defaultBranchName {
t.Errorf("baseRef = %q, want %q", baseRef, defaultBranchName)
95 unmodified lines
commitFile(t, dir, "child1.go", "package main", "child commit 1")
commitFile(t, dir, "child2.go", "package main", "child commit 2")
ctx := context.Background()
repo := openTestRepo(t, dir)
baseRef, err := detectScopeBaseRef(ctx, repo)
baseRef, err := fallbackScopeRef(repo)
if err != nil {
t.Fatalf("detectScopeBaseRef: %v", err)
t.Fatalf("fallbackScopeRef: %v", err)
}
// Mainline must win even though feat/parent's tip is newer.
if baseRef != defaultBranchName {
20 unmodified lines
t.Fatalf("detach HEAD: %v\n%s", err, out)
}
ctx := context.Background()
repo := openTestRepo(t, dir)
baseRef, err := detectScopeBaseRef(ctx, repo)
baseRef, err := fallbackScopeRef(repo)
if err != nil {
t.Fatalf("detectScopeBaseRef: %v", err)
t.Fatalf("fallbackScopeRef: %v", err)
}
// With no ancestor branches (detached HEAD, no origin), falls back to "main".
if baseRef != defaultBranchName {
27 unmodified lines
ctx := context.Background()
repo := openTestRepo(t, dir)
baseRef, err := detectScopeBaseRef(ctx, repo)
baseRef, err := fallbackScopeRef(repo)
if err != nil {
t.Fatalf("detectScopeBaseRef: %v", err)
t.Fatalf("fallbackScopeRef: %v", err)
}
commits, err := countCommits(ctx, dir, baseRef)
65 unmodified lines
}
defaultBranch := strings.TrimSpace(string(branchOut))
// Rename default branch to a non-fallback name so detectScopeBaseRef
// Rename default branch to a non-fallback name so fallbackScopeRef
// cannot resolve any fallback.
//nolint:noctx // test helper
cmd := exec.Command("git", "branch", "-m", defaultBranch, "custom-branch")
2 unmodified lines
t.Fatalf("rename branch: %v\n%s", cmdErr, out)
}
ctx := context.Background()
// Re-open repo after rename.
repo := openTestRepo(t, dir)
_, detectErr := detectScopeBaseRef(ctx, repo)
_, detectErr := fallbackScopeRef(repo)
if detectErr == nil {
t.Error("expected error when no suitable ancestor branch exists, got nil")
}
Mcmd/entire/cli/review/scope_test.go+10/-14
1
2
3
4
5
6
7
3
4
5
6
7
8
9
9
10
10
11
12
13
14
15
16
17
17
18
19
20
21
22
22
23
25
24
25
26
29
30
31
27
28
29
30
31
24 unmodified lines
56
57
58
62
63
64
65
66
59
60
61
62
63
64
65
70
71
66
67
68
69
76
70
71
72
9 unmodified lines
82
83
84
92
85
86
87
95
88
89
90
91
100
101
102
103
92
93
94
95
96
97
2 unmodified lines
100
101
102
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
103
104
105
139
140
141
142
143
144
106
107
108
146
109
110
111
112
113
114
115
153
154
155
156
157
116
117
118
119
41 unmodified lines
161
162
163
205
206
207
208
209
210
// Package review — see env.go for package-level rationale.
//
// synthesis_sink.go provides SynthesisSink, an opt-in Sink that prompts the
// user (y/N, default N) after all agents finish, then asks a configured
// summary provider to synthesize a unified verdict across the per-agent
// narratives. Skipped silently in non-TTY mode, on cancellation, or when
// fewer than 2 agents produced usable output.
// synthesis_sink.go provides SynthesisSink, the master adjudication phase of a
// multi-agent review: after all worker agents finish, it asks a configured
// provider to consolidate the per-agent narratives into a final report.
// Skipped silently on cancellation or when fewer than 2 agents produced
// usable output. The report runs unconditionally (no y/N prompt) and works in
// both TTY and redirected/CI output.
//
// Composition: appended AFTER DumpSink in TTY-mode sink slices, so the
// y/N prompt appears below the per-agent narrative dump.
// Composition: appended AFTER DumpSink in the multi-agent sink slice, so the
// final report renders below the per-agent narrative dump.
package review
import (
"context"
"fmt"
"io"
"log/slog"
"time"
"github.com/entireio/cli/cmd/entire/cli/agent"
agenttypes "github.com/entireio/cli/cmd/entire/cli/agent/types"
"github.com/entireio/cli/cmd/entire/cli/logging"
"github.com/entireio/cli/cmd/entire/cli/mdrender"
reviewtypes "github.com/entireio/cli/cmd/entire/cli/review/types"
"github.com/entireio/cli/cmd/entire/cli/uiform"
)
// SynthesisProvider abstracts the LLM call that produces the cross-agent
// verdict. Injected via Deps so tests can stub the provider call without a
// real API roundtrip. Production wiring (in review_bridge.go) calls into
// the same provider entire explain uses.
// verdict. Injected so tests can stub the provider call without a real API
// roundtrip; production wiring uses AgentSynthesisProvider.
type SynthesisProvider interface {
// Synthesize takes the composed synthesis prompt and returns the
// verdict text. Errors are surfaced to the caller; SynthesisSink
24 unmodified lines
}
// SynthesisSink composes a multi-agent verdict by calling a configured
// provider after the run finishes. In normal profile-native `entire review`
// runs this is the profile's master adjudication phase: Auto is true and
// Provider is the profile master, so the master report is produced without a
// y/N prompt. The legacy opt-in path (Auto false) keeps the prompted-synthesis
// behavior. AgentEvent is a no-op; all work happens in RunFinished.
// provider after the run finishes — the profile's master adjudication phase.
// Provider is the profile master, so the final report is produced
// unconditionally (no y/N prompt). AgentEvent is a no-op; all work happens in
// RunFinished.
type SynthesisSink struct {
Provider SynthesisProvider
Writer io.Writer
InputTTY bool // true if stdin can prompt the user
PromptYN func(ctx context.Context, question string, def bool) (bool, error)
PerRunPrompt string // if non-empty, included in the synthesis prompt for context
ProfileName string
Task string
MasterName string
Auto bool // when true, run without a y/N prompt (profile-native final report)
RunContext context.Context // optional; nil falls back to context.Background()
ProviderTimeout time.Duration // optional; zero uses defaultSynthesisProviderTimeout
OnResult func(result string)
9 unmodified lines
// AgentEvent is a no-op; SynthesisSink only acts in RunFinished.
func (SynthesisSink) AgentEvent(_ string, _ reviewtypes.Event) {}
// RunFinished optionally synthesizes a cross-agent verdict.
// RunFinished synthesizes a cross-agent final report.
//
// Skip silently when:
// - stdin isn't a TTY (s.InputTTY == false)
// - the run was cancelled (summary.Cancelled)
// - fewer than 2 agents produced usable output (status Succeeded or Failed
// with non-empty narrative buffer)
//
// In profile-native mode (Auto=true), the master phase is mandatory and runs
// without a y/N prompt. In legacy sink mode (Auto=false), prompt y/N (default
// N). On provider failure: print "final report unavailable: <err>" with the
// underlying error; user can still commit.
// The master phase is mandatory and runs without a y/N prompt, in TTY and
// redirected output alike. On provider failure: print "final report
// unavailable: <err>" with the underlying error; the user can still commit.
func (s SynthesisSink) RunFinished(summary reviewtypes.RunSummary) {
if summary.Cancelled {
return
2 unmodified lines
return
}
ctx := s.runContext()
if !s.Auto {
if !s.InputTTY {
return
}
promptFn := s.PromptYN
if promptFn == nil {
promptFn = realPromptYN
}
yes, err := promptFn(ctx, "Synthesize a unified verdict across all agent reviews?", false)
if err != nil {
// huh form errors (terminal-resize anomalies, stdin EOF, stub
// failures) shouldn't block the user from committing — they get the
// same silent skip as a "no" answer. Logged at debug for diagnostics.
logging.Debug(ctx, "synthesis prompt error",
slog.String("error", err.Error()))
return
}
if !yes {
return
}
}
synthesisPrompt := composeSynthesisPrompt(summary, s.PerRunPrompt, s.ProfileName, s.Task)
providerCtx, cancelProvider := s.providerContext()
defer cancelProvider()
if s.Auto {
if s.MasterName != "" {
fmt.Fprintf(s.Writer, "Generating final report with %s...\n", s.MasterName)
} else {
fmt.Fprintln(s.Writer, "Generating final report...")
}
if s.MasterName != "" {
fmt.Fprintf(s.Writer, "Generating final report with %s...\n", s.MasterName)
} else {
fmt.Fprintln(s.Writer, "Generating summary...")
fmt.Fprintln(s.Writer, "Generating final report...")
}
if s.OnStart != nil {
s.OnStart()
}
result, provErr := s.Provider.Synthesize(providerCtx, synthesisPrompt)
if provErr != nil {
if s.Auto {
fmt.Fprintf(s.Writer, "final report unavailable: %v\n", provErr)
} else {
fmt.Fprintf(s.Writer, "synthesis unavailable: %v\n", provErr)
}
fmt.Fprintf(s.Writer, "final report unavailable: %v\n", provErr)
if s.OnComplete != nil {
s.OnComplete(provErr)
}
41 unmodified lines
func usableAgentCount(summary reviewtypes.RunSummary) int {
return len(usableAgentRuns(summary))
}
// realPromptYN is the production y/N prompt; delegates to uiform.PromptYN
// so the review and investigate packages share one implementation.
func realPromptYN(ctx context.Context, question string, def bool) (bool, error) {
return uiform.PromptYN(ctx, question, def) //nolint:wrapcheck // uiform already wraps
}
Mcmd/entire/cli/review/synthesis_sink.go+22/-69
43 unmodified lines
44
45
46
47
48
47
48
49
50
51
54
55
52
53
54
32 unmodified lines
87
88
89
94
90
91
92
93
12 unmodified lines
106
107
108
113
114
115
116
117
118
109
110
111
112
113
114
124
125
126
115
116
117
2 unmodified lines
120
121
122
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
123
124
125
50 unmodified lines
176
177
178
214
215
216
217
218
219
179
180
181
222
223
224
182
183
184
1 unmodified line
186
187
188
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
189
190
191
192
193
194
259
260
261
262
195
196
197
198
199
267
200
201
202
270
203
204
205
206
5 unmodified lines
212
213
214
282
283
284
215
286
216
217
218
219
11 unmodified lines
231
232
233
304
305
306
234
235
309
236
237
238
239
12 unmodified lines
252
253
254
328
329
330
331
255
256
257
258
7 unmodified lines
266
267
268
345
269
270
271
272
1 unmodified line
274
275
276
353
354
355
356
277
278
279
280
281
282
362
363
283
284
285
286
287
6 unmodified lines
294
295
296
376
377
378
297
380
298
299
300
301
1 unmodified line
303
304
305
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
43 unmodified lines
func buildSink(
provider review.SynthesisProvider,
w *bytes.Buffer,
inputTTY bool,
promptYN func(ctx context.Context, question string, def bool) (bool, error),
perRunPrompt string,
) review.SynthesisSink {
return review.SynthesisSink{
Provider: provider,
Writer: w,
InputTTY: inputTTY,
PromptYN: promptYN,
PerRunPrompt: perRunPrompt,
}
}
32 unmodified lines
t.Parallel()
w := &bytes.Buffer{}
stub := &stubSynthesisProvider{response: "verdict"}
sink := buildSink(stub, w, true, nil, "")
sink := buildSink(stub, w, "")
sink.AgentEvent("agent-a", reviewtypes.AssistantText{Text: "hello"})
sink.AgentEvent("agent-b", reviewtypes.ToolCall{Name: "Bash", Args: "ls"})
12 unmodified lines
t.Parallel()
w := &bytes.Buffer{}
stub := &stubSynthesisProvider{response: "verdict"}
promptCalled := false
promptFn := func(_ context.Context, _ string, _ bool) (bool, error) {
promptCalled = true
return true, nil
}
sink := buildSink(stub, w, true, promptFn, "")
sink := buildSink(stub, w, "")
summary := makeTwoAgentSummary()
summary.Cancelled = true
sink.RunFinished(summary)
if promptCalled {
t.Error("prompt should not be shown when run was cancelled")
}
if stub.capturedPrompt != "" {
t.Error("provider should not be called when run was cancelled")
}
2 unmodified lines
}
}
// TestSynthesisSink_SkipsWhenNonTTY verifies RunFinished is a no-op when
// InputTTY is false (CI, piped output).
func TestSynthesisSink_SkipsWhenNonTTY(t *testing.T) {
t.Parallel()
w := &bytes.Buffer{}
stub := &stubSynthesisProvider{response: "verdict"}
promptCalled := false
promptFn := func(_ context.Context, _ string, _ bool) (bool, error) {
promptCalled = true
return true, nil
}
sink := buildSink(stub, w, false, promptFn, "")
sink.RunFinished(makeTwoAgentSummary())
if promptCalled {
t.Error("prompt should not be shown in non-TTY mode")
}
if stub.capturedPrompt != "" {
t.Error("provider should not be called in non-TTY mode")
}
}
// TestSynthesisSink_SkipsWhenFewerThanTwoUsableAgents verifies that synthesis
// is skipped when fewer than 2 agents produced usable narrative output.
func TestSynthesisSink_SkipsWhenFewerThanTwoUsableAgents(t *testing.T) {
50 unmodified lines
t.Parallel()
w := &bytes.Buffer{}
stub := &stubSynthesisProvider{response: "verdict"}
promptCalled := false
promptFn := func(_ context.Context, _ string, _ bool) (bool, error) {
promptCalled = true
return true, nil
}
sink := buildSink(stub, w, true, promptFn, "")
sink := buildSink(stub, w, "")
sink.RunFinished(tc.summary)
if promptCalled {
t.Errorf("[%s] prompt should not be shown with <2 usable agents", tc.name)
}
if stub.capturedPrompt != "" {
t.Errorf("[%s] provider should not be called with <2 usable agents", tc.name)
}
1 unmodified line
}
}
// TestSynthesisSink_UserPicksNo verifies that when the user picks N, the
// provider is not called and nothing is written.
func TestSynthesisSink_UserPicksNo(t *testing.T) {
t.Parallel()
w := &bytes.Buffer{}
stub := &stubSynthesisProvider{response: "verdict"}
promptFn := func(_ context.Context, _ string, _ bool) (bool, error) {
return false, nil // user picks N
}
sink := buildSink(stub, w, true, promptFn, "")
sink.RunFinished(makeTwoAgentSummary())
if stub.capturedPrompt != "" {
t.Error("provider should not be called when user picks N")
}
if w.Len() > 0 {
t.Errorf("no output expected when user picks N, got: %q", w.String())
}
}
// TestSynthesisSink_UserPicksYes verifies that when the user picks Y, the
// provider is called and its response is written to the writer.
func TestSynthesisSink_UserPicksYes(t *testing.T) {
// TestSynthesisSink_WritesFinalReport verifies that with 2+ usable agents the
// provider is called unconditionally and its response is written to the writer.
func TestSynthesisSink_WritesFinalReport(t *testing.T) {
t.Parallel()
w := &bytes.Buffer{}
stub := &stubSynthesisProvider{response: "Unified verdict: looks good."}
promptFn := func(_ context.Context, _ string, _ bool) (bool, error) {
return true, nil // user picks Y
}
sink := buildSink(stub, w, true, promptFn, "")
sink := buildSink(stub, w, "")
sink.RunFinished(makeTwoAgentSummary())
if stub.capturedPrompt == "" {
t.Fatal("provider should have been called when user picks Y")
t.Fatal("provider should have been called")
}
out := w.String()
if !strings.Contains(out, "Generating summary...") {
if !strings.Contains(out, "Generating final report...") {
t.Errorf("writer should show progress before provider response, got: %q", out)
}
if !strings.Contains(out, "Unified verdict: looks good.") {
5 unmodified lines
t.Parallel()
w := &bytes.Buffer{}
stub := &stubSynthesisProvider{response: "Unified verdict: fix H1."}
promptFn := func(_ context.Context, _ string, _ bool) (bool, error) {
return true, nil
}
var captured string
sink := buildSink(stub, w, true, promptFn, "")
sink := buildSink(stub, w, "")
sink.OnResult = func(result string) {
captured = result
}
11 unmodified lines
t.Parallel()
w := &bytes.Buffer{}
provider := &contextWaitingSynthesisProvider{}
promptFn := func(_ context.Context, _ string, _ bool) (bool, error) {
return true, nil
}
runCtx, cancelRun := context.WithCancel(context.Background())
cancelRun()
sink := buildSink(provider, w, true, promptFn, "")
sink := buildSink(provider, w, "")
sink.RunContext = runCtx
sink.RunFinished(makeTwoAgentSummary())
12 unmodified lines
t.Parallel()
w := &bytes.Buffer{}
provider := &contextWaitingSynthesisProvider{}
promptFn := func(_ context.Context, _ string, _ bool) (bool, error) {
return true, nil
}
sink := buildSink(provider, w, true, promptFn, "")
sink := buildSink(provider, w, "")
sink.ProviderTimeout = time.Nanosecond
sink.RunFinished(makeTwoAgentSummary())
7 unmodified lines
}
// TestSynthesisSink_ProviderErrorDegradeGracefully verifies that a provider
// error results in a "synthesis unavailable" message rather than a panic or
// error results in a "final report unavailable" message rather than a panic or
// swallowed error.
func TestSynthesisSink_ProviderErrorDegradeGracefully(t *testing.T) {
t.Parallel()
1 unmodified line
stub := &stubSynthesisProvider{
err: errors.New("API quota exceeded"),
}
promptFn := func(_ context.Context, _ string, _ bool) (bool, error) {
return true, nil // user picks Y
}
sink := buildSink(stub, w, true, promptFn, "")
sink := buildSink(stub, w, "")
// Must not panic.
sink.RunFinished(makeTwoAgentSummary())
out := w.String()
if !strings.Contains(out, "synthesis unavailable") {
t.Errorf("expected 'synthesis unavailable' in output, got: %q", out)
if !strings.Contains(out, "final report unavailable") {
t.Errorf("expected 'final report unavailable' in output, got: %q", out)
}
if !strings.Contains(out, "API quota exceeded") {
t.Errorf("expected error message in output, got: %q", out)
6 unmodified lines
t.Parallel()
w := &bytes.Buffer{}
stub := &stubSynthesisProvider{response: "verdict"}
promptFn := func(_ context.Context, _ string, _ bool) (bool, error) {
return true, nil
}
perRunPrompt := "Focus specifically on security vulnerabilities."
sink := buildSink(stub, w, true, promptFn, perRunPrompt)
sink := buildSink(stub, w, perRunPrompt)
sink.RunFinished(makeTwoAgentSummary())
1 unmodified line
t.Errorf("per-run prompt %q not found in provider prompt:\n%s", perRunPrompt, stub.capturedPrompt)
}
}
// TestSynthesisSink_PromptDefaultIsNo verifies the default value passed to
// the PromptYN function is false (N), so pressing Enter accepts the default N.
func TestSynthesisSink_PromptDefaultIsNo(t *testing.T) {
t.Parallel()
w := &bytes.Buffer{}
stub := &stubSynthesisProvider{response: "verdict"}
var capturedDefault bool
promptFn := func(_ context.Context, _ string, def bool) (bool, error) {
capturedDefault = def
return false, nil // user picks N
}
sink := buildSink(stub, w, true, promptFn, "")
sink.RunFinished(makeTwoAgentSummary())
if capturedDefault {
t.Error("default for synthesis prompt should be false (N), got true")
}
}
Mcmd/entire/cli/review/synthesis_sink_test.go+17/-119
83 unmodified lines
84
85
86
87
88
89
90
91
92
93
94
87
88
89
5 unmodified lines
95
96
97
106
98
99
100
101
102
103
83 unmodified lines
})
}
// reviewTrailFindingInput builds the trail finding payload for one review
// verdict. The verdict spans the whole change, so it uses "whole_change"
// granularity: the API requires a valid granularity and rejects a zero/empty
// value with a 400.
func reviewTrailFindingInput(profileName, verdict string) api.TrailReviewCommentInput {
return reviewTrailFindingInputWithKind(profileName, verdict, "verdict")
}
// reviewTrailFindingInputs turns a final review verdict into trail findings.
// It first accepts the runner-style last JSON line format
// {"summary":"","comments":[...]}; when absent, it falls back to splitting
5 unmodified lines
}
items := splitReviewVerdictFindings(verdict)
if len(items) <= 1 {
return []api.TrailReviewCommentInput{reviewTrailFindingInput(profileName, verdict)}
// The verdict spans the whole change, so it uses "verdict" kind:
// the API requires a valid granularity and rejects an empty value.
return []api.TrailReviewCommentInput{reviewTrailFindingInputWithKind(profileName, verdict, "verdict")}
}
inputs := make([]api.TrailReviewCommentInput, 0, len(items))
for _, item := range items {
Mcmd/entire/cli/review_bridge.go+3/-9
12 unmodified lines
13
14
15
16
16
17
18
19
5 unmodified lines
25
26
27
28
28
29
30
31
12 unmodified lines
// Regression: a review verdict is not tied to a file/line, so the finding
// must use whole_change granularity. An empty granularity is rejected by
// the API with a 400.
in := reviewTrailFindingInput("general", " the verdict ")
in := reviewTrailFindingInputWithKind("general", " the verdict ", "verdict")
if in.Location.Granularity != testWholeChangeGranularity {
t.Errorf("granularity = %q, want whole_change", in.Location.Granularity)
}
5 unmodified lines
}
// No profile: the body is exactly the trimmed verdict.
bare := reviewTrailFindingInput("", " bare verdict ")
bare := reviewTrailFindingInputWithKind("", " bare verdict ", "verdict")
if bare.Body == nil || *bare.Body != "bare verdict" {
t.Errorf("body = %v, want exactly %q", bare.Body, "bare verdict")
}
Mcmd/entire/cli/review_bridge_test.go+2/-2
370 unmodified lines
371
372
373
374
375
376
377
378
379
380
381
382
383
384
374
375
376
370 unmodified lines
return c.Agent == "" && c.Model == "" && len(c.Skills) == 0 && c.Prompt == ""
}
// ReviewConfigFor returns the configured review config for the given agent.
// Returns a zero-value config when the agent has no entry; callers should
// check IsZero (or the individual fields) to decide whether configuration
// is present.
func (s *EntireSettings) ReviewConfigFor(agentName string) ReviewConfig {
if s == nil {
return ReviewConfig{}
}
return s.Review[agentName]
}
// InvestigateConfig holds the configuration for `entire investigate`.
// Unlike ReviewConfig, investigate runs the same shared prompt across
// all configured agents, so the schema is a flat agent list with global
Mcmd/entire/cli/settings/settings.go-11
1124 unmodified lines
1125
1126
1127
1128
1129
1130
1131
1132
1133
1134
1135
1136
1137
1138
1139
1140
1128
1129
1130
1124 unmodified lines
}
}
func TestEntireSettings_ReviewConfigFor(t *testing.T) {
t.Parallel()
s := &EntireSettings{Review: map[string]ReviewConfig{
"claude-code": {Skills: []string{"/pr-review-toolkit:review-pr"}},
}}
if cfg := s.ReviewConfigFor("claude-code"); len(cfg.Skills) != 1 {
t.Fatalf("expected 1 skill, got %v", cfg.Skills)
}
if cfg := s.ReviewConfigFor("codex"); !cfg.IsZero() {
t.Fatalf("expected zero config for unconfigured agent, got %+v", cfg)
}
}
func TestReviewConfig_IsZero(t *testing.T) {
t.Parallel()
tests := []struct {
Mcmd/entire/cli/settings/settings_test.go-13
1
2
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
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
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
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
package trail
import (
"context"
"encoding/json"
"errors"
"fmt"
"strings"
"time"
"github.com/entireio/cli/cmd/entire/cli/checkpoint"
"github.com/entireio/cli/cmd/entire/cli/paths"
"github.com/go-git/go-git/v6"
"github.com/go-git/go-git/v6/plumbing"
"github.com/go-git/go-git/v6/plumbing/filemode"
"github.com/go-git/go-git/v6/plumbing/object"
)
const (
metadataFile = "metadata.json"
discussionFile = "discussion.json"
checkpointsFile = "checkpoints.json"
)
// ErrTrailNotFound is returned when a trail cannot be found.
var ErrTrailNotFound = errors.New("trail not found")
// Store provides CRUD operations for trail metadata on the entire/trails/v1 branch.
type Store struct {
repo *git.Repository
}
// NewStore creates a new trail store backed by the given git repository.
func NewStore(repo *git.Repository) *Store {
return &Store{repo: repo}
}
// EnsureBranch creates the entire/trails/v1 orphan branch if it doesn't exist.
func (s *Store) EnsureBranch(ctx context.Context) error {
refName := plumbing.NewBranchReferenceName(paths.TrailsBranchName)
_, err := s.repo.Reference(refName, true)
if err == nil {
return nil // Branch already exists
}
if !errors.Is(err, plumbing.ErrReferenceNotFound) {
return fmt.Errorf("failed to check trails branch: %w", err)
}
// Create orphan branch with empty tree
emptyTreeHash, err := checkpoint.BuildTreeFromEntries(ctx, s.repo, make(map[string]object.TreeEntry))
if err != nil {
return fmt.Errorf("failed to build empty tree: %w", err)
}
authorName, authorEmail := checkpoint.GetGitAuthorFromRepo(s.repo)
commitHash, err := checkpoint.CreateCommit(ctx, s.repo, emptyTreeHash, plumbing.ZeroHash, "Initialize trails branch", authorName, authorEmail)
if err != nil {
return fmt.Errorf("failed to create initial commit: %w", err)
}
newRef := plumbing.NewHashReference(refName, commitHash)
if err := s.repo.Storer.SetReference(newRef); err != nil {
return fmt.Errorf("failed to set branch reference: %w", err)
}
return nil
}
// Write writes trail metadata, discussion, and checkpoints to the entire/trails/v1 branch.
// If checkpoints is nil, an empty checkpoints list is written.
func (s *Store) Write(ctx context.Context, metadata *Metadata, discussion *Discussion, checkpoints *Checkpoints) error {
if metadata.TrailID.IsEmpty() {
return errors.New("trail ID is required")
}
if err := s.EnsureBranch(ctx); err != nil {
return fmt.Errorf("failed to ensure trails branch: %w", err)
}
commitHash, rootTreeHash, err := s.getBranchRef()
if err != nil {
return fmt.Errorf("failed to get branch ref: %w", err)
}
// Build blob entries for the trail's 3 files
trailEntries, err := s.buildTrailEntries(metadata, discussion, checkpoints)
if err != nil {
return err
}
// Splice into tree at [shard, suffix] — preserves sibling trails automatically
shard, suffix := metadata.TrailID.ShardParts()
newTreeHash, err := checkpoint.UpdateSubtree(
s.repo, rootTreeHash,
[]string{shard, suffix},
trailEntries,
checkpoint.UpdateSubtreeOptions{MergeMode: checkpoint.ReplaceAll},
)
if err != nil {
return fmt.Errorf("failed to update subtree: %w", err)
}
commitMsg := fmt.Sprintf("Trail: %s (%s)", metadata.Title, metadata.TrailID)
return s.commitAndUpdateRef(ctx, newTreeHash, commitHash, commitMsg)
}
// buildTrailEntries creates blob objects for a trail's 3 files and returns them as tree entries.
func (s *Store) buildTrailEntries(metadata *Metadata, discussion *Discussion, checkpoints *Checkpoints) ([]object.TreeEntry, error) {
if discussion == nil {
discussion = &Discussion{Comments: []Comment{}}
}
if checkpoints == nil {
checkpoints = &Checkpoints{Checkpoints: []CheckpointRef{}}
}
type fileSpec struct {
name string
data any
}
files := []fileSpec{
{metadataFile, metadata},
{discussionFile, discussion},
{checkpointsFile, checkpoints},
}
entries := make([]object.TreeEntry, 0, len(files))
for _, f := range files {
jsonBytes, err := json.MarshalIndent(f.data, "", " ")
if err != nil {
return nil, fmt.Errorf("failed to marshal %s: %w", f.name, err)
}
blobHash, err := checkpoint.CreateBlobFromContent(s.repo, jsonBytes)
if err != nil {
return nil, fmt.Errorf("failed to create %s blob: %w", f.name, err)
}
entries = append(entries, object.TreeEntry{
Name: f.name,
Mode: filemode.Regular,
Hash: blobHash,
})
}
return entries, nil
}
// Read reads a trail by its ID from the entire/trails/v1 branch.
func (s *Store) Read(trailID ID) (*Metadata, *Discussion, *Checkpoints, error) {
if err := ValidateID(string(trailID)); err != nil {
return nil, nil, nil, err
}
tree, err := s.getBranchTree()
if err != nil {
return nil, nil, nil, err
}
basePath := trailID.Path() + "/"
// Read metadata
metadataEntry, err := tree.FindEntry(basePath + metadataFile)
if err != nil {
return nil, nil, nil, fmt.Errorf("trail %s not found: %w", trailID, err)
}
metadataBlob, err := s.repo.BlobObject(metadataEntry.Hash)
if err != nil {
return nil, nil, nil, fmt.Errorf("failed to read metadata blob: %w", err)
}
metadataReader, err := metadataBlob.Reader()
if err != nil {
return nil, nil, nil, fmt.Errorf("failed to open metadata reader: %w", err)
}
defer metadataReader.Close()
var metadata Metadata
if err := json.NewDecoder(metadataReader).Decode(&metadata); err != nil {
return nil, nil, nil, fmt.Errorf("failed to decode metadata: %w", err)
}
// Read discussion (optional, may not exist yet)
var discussion Discussion
discussionEntry, err := tree.FindEntry(basePath + discussionFile)
if err == nil {
discussionBlob, blobErr := s.repo.BlobObject(discussionEntry.Hash)
if blobErr == nil {
discussionReader, readerErr := discussionBlob.Reader()
if readerErr == nil {
//nolint:errcheck,gosec // best-effort decode of optional discussion
json.NewDecoder(discussionReader).Decode(&discussion)
_ = discussionReader.Close()
}
}
}
// Read checkpoints (optional, may not exist yet)
var checkpoints Checkpoints
checkpointsEntry, err := tree.FindEntry(basePath + checkpointsFile)
if err == nil {
checkpointsBlob, blobErr := s.repo.BlobObject(checkpointsEntry.Hash)
if blobErr == nil {
checkpointsReader, readerErr := checkpointsBlob.Reader()
if readerErr == nil {
//nolint:errcheck,gosec // best-effort decode of optional checkpoints
json.NewDecoder(checkpointsReader).Decode(&checkpoints)
_ = checkpointsReader.Close()
}
}
}
return &metadata, &discussion, &checkpoints, nil
}
// FindByBranch finds a trail for the given branch name.
// Returns (nil, nil) if no trail exists for the branch.
func (s *Store) FindByBranch(branchName string) (*Metadata, error) {
trails, err := s.List()
if err != nil {
return nil, err
}
for _, t := range trails {
if t.Branch == branchName {
return t, nil
}
}
return nil, nil //nolint:nilnil // nil, nil means "not found" — callers check both
}
// List returns all trail metadata from the entire/trails/v1 branch.
func (s *Store) List() ([]*Metadata, error) {
tree, err := s.getBranchTree()
if err != nil {
// Branch doesn't exist yet — no trails
return nil, nil //nolint:nilerr // Expected when no trails exist yet
}
var trails []*Metadata
entries := make(map[string]object.TreeEntry)
if err := checkpoint.FlattenTree(s.repo, tree, "", entries); err != nil {
return nil, fmt.Errorf("failed to flatten tree: %w", err)
}
// Find all metadata.json files
for path, entry := range entries {
if !strings.HasSuffix(path, "/"+metadataFile) {
continue
}
blob, err := s.repo.BlobObject(entry.Hash)
if err != nil {
continue
}
reader, err := blob.Reader()
if err != nil {
continue
}
var metadata Metadata
decodeErr := json.NewDecoder(reader).Decode(&metadata)
_ = reader.Close()
if decodeErr != nil {
continue
}
trails = append(trails, &metadata)
}
return trails, nil
}
// Update updates an existing trail's metadata. It reads the current metadata,
// applies the provided update function, and writes it back.
func (s *Store) Update(ctx context.Context, trailID ID, updateFn func(*Metadata)) error {
// ValidateID is called by Read, no need to duplicate here
metadata, discussion, checkpoints, err := s.Read(trailID)
if err != nil {
return fmt.Errorf("failed to read trail for update: %w", err)
}
updateFn(metadata)
metadata.UpdatedAt = time.Now()
return s.Write(ctx, metadata, discussion, checkpoints)
}
// AddCheckpoint prepends a checkpoint reference to a trail's checkpoints list (newest first).
// Only reads and writes the checkpoints.json file — metadata and discussion are untouched.
func (s *Store) AddCheckpoint(ctx context.Context, trailID ID, ref CheckpointRef) error {
if err := ValidateID(string(trailID)); err != nil {
return err
}
if err := s.EnsureBranch(ctx); err != nil {
return fmt.Errorf("failed to ensure trails branch: %w", err)
}
commitHash, rootTreeHash, err := s.getBranchRef()
if err != nil {
return fmt.Errorf("failed to get branch ref: %w", err)
}
// Navigate to the trail's subtree and read only checkpoints.json
shard, suffix := trailID.ShardParts()
trailTree, err := s.navigateToTrailTree(rootTreeHash, shard, suffix)
if err != nil {
return fmt.Errorf("failed to read checkpoints for trail %s: %w", trailID, err)
}
checkpoints, err := s.readCheckpointsFromTrailTree(trailTree)
if err != nil {
return fmt.Errorf("failed to read checkpoints for trail %s: %w", trailID, err)
}
// Prepend new ref (newest first)
checkpoints.Checkpoints = append(checkpoints.Checkpoints, CheckpointRef{})
copy(checkpoints.Checkpoints[1:], checkpoints.Checkpoints[:len(checkpoints.Checkpoints)-1])
checkpoints.Checkpoints[0] = ref
// Create new blob and splice back — MergeKeepExisting preserves metadata.json and discussion.json
checkpointsJSON, err := json.MarshalIndent(checkpoints, "", " ")
if err != nil {
return fmt.Errorf("failed to marshal checkpoints: %w", err)
}
blobHash, err := checkpoint.CreateBlobFromContent(s.repo, checkpointsJSON)
if err != nil {
return fmt.Errorf("failed to create checkpoints blob: %w", err)
}
newTreeHash, err := checkpoint.UpdateSubtree(
s.repo, rootTreeHash,
[]string{shard, suffix},
[]object.TreeEntry{{Name: checkpointsFile, Mode: filemode.Regular, Hash: blobHash}},
checkpoint.UpdateSubtreeOptions{MergeMode: checkpoint.MergeKeepExisting},
)
if err != nil {
return fmt.Errorf("failed to update subtree: %w", err)
}
commitMsg := fmt.Sprintf("Add checkpoint to trail: %s", trailID)
return s.commitAndUpdateRef(ctx, newTreeHash, commitHash, commitMsg)
}
// Delete removes a trail from the entire/trails/v1 branch.
func (s *Store) Delete(ctx context.Context, trailID ID) error {
if err := ValidateID(string(trailID)); err != nil {
return err
}
if err := s.EnsureBranch(ctx); err != nil {
return fmt.Errorf("failed to ensure trails branch: %w", err)
}
commitHash, rootTreeHash, err := s.getBranchRef()
if err != nil {
return fmt.Errorf("failed to get branch ref: %w", err)
}
// Verify the trail exists by navigating the tree at O(depth)
shard, suffix := trailID.ShardParts()
if _, err := s.navigateToTrailTree(rootTreeHash, shard, suffix); err != nil {
return err
}
// Delete the trail's subtree by removing it from the shard directory
newTreeHash, err := checkpoint.UpdateSubtree(
s.repo, rootTreeHash,
[]string{shard},
nil,
checkpoint.UpdateSubtreeOptions{
MergeMode: checkpoint.MergeKeepExisting,
DeleteNames: []string{suffix},
},
)
if err != nil {
return fmt.Errorf("failed to update subtree: %w", err)
}
commitMsg := fmt.Sprintf("Delete trail: %s", trailID)
return s.commitAndUpdateRef(ctx, newTreeHash, commitHash, commitMsg)
}
// navigateToTrailTree walks rootTree → shard → suffix and returns the trail's subtree.
func (s *Store) navigateToTrailTree(rootTreeHash plumbing.Hash, shard, suffix string) (*object.Tree, error) {
rootTree, err := s.repo.TreeObject(rootTreeHash)
if err != nil {
return nil, fmt.Errorf("trail %s/%s not found: %w", shard, suffix, err)
}
shardEntry, err := rootTree.FindEntry(shard)
if err != nil {
return nil, fmt.Errorf("trail %s/%s not found: %w", shard, suffix, err)
}
shardTree, err := s.repo.TreeObject(shardEntry.Hash)
if err != nil {
return nil, fmt.Errorf("trail %s/%s not found: %w", shard, suffix, err)
}
trailEntry, err := shardTree.FindEntry(suffix)
if err != nil {
return nil, fmt.Errorf("trail %s/%s not found: %w", shard, suffix, err)
}
trailTree, err := s.repo.TreeObject(trailEntry.Hash)
if err != nil {
return nil, fmt.Errorf("trail %s/%s not found: %w", shard, suffix, err)
}
return trailTree, nil
}
// readCheckpointsFromTrailTree reads checkpoints.json from a trail's subtree.
// Returns empty checkpoints if the file doesn't exist yet.
func (s *Store) readCheckpointsFromTrailTree(trailTree *object.Tree) (*Checkpoints, error) {
cpEntry, err := trailTree.FindEntry(checkpointsFile)
if err != nil {
// No checkpoints file yet — return empty
return &Checkpoints{Checkpoints: []CheckpointRef{}}, nil
}
blob, err := s.repo.BlobObject(cpEntry.Hash)
if err != nil {
return nil, fmt.Errorf("failed to read checkpoints blob: %w", err)
}
reader, err := blob.Reader()
if err != nil {
return nil, fmt.Errorf("failed to open checkpoints reader: %w", err)
}
defer reader.Close()
var checkpoints Checkpoints
if err := json.NewDecoder(reader).Decode(&checkpoints); err != nil {
return nil, fmt.Errorf("failed to decode checkpoints: %w", err)
}
return &checkpoints, nil
}
// commitAndUpdateRef creates a commit and updates the trails branch reference.
func (s *Store) commitAndUpdateRef(ctx context.Context, treeHash, parentHash plumbing.Hash, message string) error {
authorName, authorEmail := checkpoint.GetGitAuthorFromRepo(s.repo)
commitHash, err := checkpoint.CreateCommit(ctx, s.repo, treeHash, parentHash, message, authorName, authorEmail)
if err != nil {
return fmt.Errorf("failed to create commit: %w", err)
}
newRef := plumbing.NewHashReference(plumbing.NewBranchReferenceName(paths.TrailsBranchName), commitHash)
if err := s.repo.Storer.SetReference(newRef); err != nil {
return fmt.Errorf("failed to update branch reference: %w", err)
}
return nil
}
// getBranchRef returns the commit hash and root tree hash for the entire/trails/v1 branch HEAD
// without flattening the tree. Falls back to remote tracking branch if local is missing.
func (s *Store) getBranchRef() (commitHash, rootTreeHash plumbing.Hash, err error) {
refName := plumbing.NewBranchReferenceName(paths.TrailsBranchName)
ref, refErr := s.repo.Reference(refName, true)
if refErr != nil {
// Try remote tracking branch
remoteRefName := plumbing.NewRemoteReferenceName("origin", paths.TrailsBranchName)
ref, refErr = s.repo.Reference(remoteRefName, true)
if refErr != nil {
return plumbing.ZeroHash, plumbing.ZeroHash, fmt.Errorf("trails branch not found: %w", refErr)
}
}
commit, err := s.repo.CommitObject(ref.Hash())
if err != nil {
return plumbing.ZeroHash, plumbing.ZeroHash, fmt.Errorf("failed to get commit: %w", err)
}
return ref.Hash(), commit.TreeHash, nil
}
// getBranchTree returns the tree for the entire/trails/v1 branch HEAD.
func (s *Store) getBranchTree() (*object.Tree, error) {
_, rootTreeHash, err := s.getBranchRef()
if err != nil {
return nil, err
}
tree, err := s.repo.TreeObject(rootTreeHash)
if err != nil {
return nil, fmt.Errorf("failed to get tree: %w", err)
}
return tree, nil
}
Dcmd/entire/cli/trail/store.go-489
package trail
import ( "context" "os" "os/exec" "path/filepath" "testing" "time"
"github.com/go-git/go-git/v6" "github.com/stretchr/testify/require" )
func strPtr(s string) *string { return &s }
// initTestRepo creates a test git repository with an initial commit. func initTestRepo(t *testing.T) *git.Repository { t.Helper()
dir := t.TempDir()
ctx := context.Background() cmds := [][]string{ {"git", "init", dir}, {"git", "-C", dir, "config", "user.name", "Test"}, {"git", "-C", dir, "config", "user.email", "test@test.com"}, {"git", "-C", dir, "config", "commit.gpgsign", "false"}, } for _, args := range cmds { cmd := exec.CommandContext(ctx, args[0], args[1:]...) if out, err := cmd.CombinedOutput(); err != nil { t.Fatalf("command %v failed: %v\n%s", args, err, out) } }
// Create a file and commit if err := os.WriteFile(filepath.Join(dir, "README.md"), []byte("# Test"), 0o644); err != nil { t.Fatalf("failed to write file: %v", err) } commitCmds := [][]string{ {"git", "-C", dir, "add", "."}, {"git", "-C", dir, "commit", "-m", "Initial commit"}, } for _, args := range commitCmds { cmd := exec.CommandContext(ctx, args[0], args[1:]...) if out, err := cmd.CombinedOutput(); err != nil { t.Fatalf("command %v failed: %v\n%s", args, err, out) } }
repo, err := git.PlainOpen(dir) if err != nil { t.Fatalf("failed to open repo: %v", err) } return repo }
func TestStore_EnsureBranch(t *testing.T) { t.Parallel() repo := initTestRepo(t) store := NewStore(repo)
// First call should create the branch if err := store.EnsureBranch(context.Background()); err != nil { t.Fatalf("EnsureBranch() error = %v", err) }
// Second call should be idempotent if err := store.EnsureBranch(context.Background()); err != nil { t.Fatalf("EnsureBranch() second call error = %v", err) } }
func TestStore_WriteAndRead(t *testing.T) { t.Parallel() repo := initTestRepo(t) store := NewStore(repo)
trailID, err := GenerateID() if err != nil { t.Fatalf("GenerateID() error = %v", err) }
now := time.Now().Truncate(time.Second) metadata := &Metadata{ TrailID: trailID, Branch: "feature/test", Base: "main", Title: "Test trail", Body: "A test trail", Status: StatusDraft, Author: &Author{ID: "1", Login: strPtr("tester")}, Assignees: []string{}, Labels: []string{"test"}, CreatedAt: now, UpdatedAt: now, }
discussion := &Discussion{Comments: []Comment{}}
if err := store.Write(context.Background(), metadata, discussion, nil); err != nil { t.Fatalf("Write() error = %v", err) }
// Read it back gotMeta, gotDisc, _, err := store.Read(trailID) if err != nil { t.Fatalf("Read() error = %v", err) }
if gotMeta.TrailID != trailID { t.Errorf("Read() trail_id = %s, want %s", gotMeta.TrailID, trailID) } if gotMeta.Branch != "feature/test" { t.Errorf("Read() branch = %q, want %q", gotMeta.Branch, "feature/test") } if gotMeta.Title != "Test trail" { t.Errorf("Read() title = %q, want %q", gotMeta.Title, "Test trail") } if gotMeta.Status != StatusDraft { t.Errorf("Read() status = %q, want %q", gotMeta.Status, StatusDraft) } if len(gotMeta.Labels) != 1 || gotMeta.Labels[0] != "test" { t.Errorf("Read() labels = %v, want [test]", gotMeta.Labels) } if gotDisc == nil { t.Error("Read() discussion should not be nil") } }
func TestStore_FindByBranch(t *testing.T) { t.Parallel() repo := initTestRepo(t) store := NewStore(repo)
now := time.Now()
// Create two trails for different branches for _, branch := range []string{"feature/a", "feature/b"} { id, err := GenerateID() if err != nil { t.Fatalf("GenerateID() error = %v", err) } meta := &Metadata{ TrailID: id, Branch: branch, Base: "main", Title: HumanizeBranchName(branch), Status: StatusDraft, Author: &Author{ID: "1", Login: strPtr("test")}, Assignees: []string{}, Labels: []string{}, CreatedAt: now, UpdatedAt: now, } if err := store.Write(context.Background(), meta, nil, nil); err != nil { t.Fatalf("Write() error = %v", err) } }
// Find by branch found, err := store.FindByBranch("feature/a") if err != nil { t.Fatalf("FindByBranch() error = %v", err) } require.NotNil(t, found, "FindByBranch() returned nil, expected trail") if found.Branch != "feature/a" { t.Errorf("FindByBranch() branch = %q, want %q", found.Branch, "feature/a") }
// Not found notFound, err := store.FindByBranch("feature/c") if err != nil { t.Fatalf("FindByBranch() error = %v", err) } if notFound != nil { t.Error("FindByBranch() should return nil for non-existent branch") } }
func TestStore_List(t *testing.T) { t.Parallel() repo := initTestRepo(t) store := NewStore(repo)
// List when no trails exist (branch doesn't exist yet) trails, err := store.List() if err != nil { t.Fatalf("List() error = %v", err) } if trails != nil { t.Errorf("List() = %v, want nil for empty store", trails) }
// Create a trail now := time.Now() id, err := GenerateID() if err != nil { t.Fatalf("GenerateID() error = %v", err) } meta := &Metadata{ TrailID: id, Branch: "feature/test", Base: "main", Title: "Test", Status: StatusDraft, Author: &Author{ID: "1", Login: strPtr("test")}, Assignees: []string{}, Labels: []string{}, CreatedAt: now, UpdatedAt: now, } if err := store.Write(context.Background(), meta, nil, nil); err != nil { t.Fatalf("Write() error = %v", err) }
trails, err = store.List() if err != nil { t.Fatalf("List() error = %v", err) } if len(trails) != 1 { t.Fatalf("List() returned %d trails, want 1", len(trails)) } if trails[0].TrailID != id { t.Errorf("List()[0].TrailID = %s, want %s", trails[0].TrailID, id) } }
func TestStore_Update(t *testing.T) { t.Parallel() repo := initTestRepo(t) store := NewStore(repo)
now := time.Now() id, err := GenerateID() if err != nil { t.Fatalf("GenerateID() error = %v", err) } meta := &Metadata{ TrailID: id, Branch: "feature/test", Base: "main", Title: "Original", Status: StatusDraft, Author: &Author{ID: "1", Login: strPtr("test")}, Assignees: []string{}, Labels: []string{}, CreatedAt: now, UpdatedAt: now, } if err := store.Write(context.Background(), meta, nil, nil); err != nil { t.Fatalf("Write() error = %v", err) }
// Update if err := store.Update(context.Background(), id, func(m *Metadata) { m.Title = "Updated" m.Status = StatusOpen m.Labels = []string{"urgent"} }); err != nil { t.Fatalf("Update() error = %v", err) }
// Verify updated, _, _, err := store.Read(id) if err != nil { t.Fatalf("Read() error = %v", err) } if updated.Title != "Updated" { t.Errorf("Read() title = %q, want %q", updated.Title, "Updated") } if updated.Status != StatusOpen { t.Errorf("Read() status = %q, want %q", updated.Status, StatusOpen) } if len(updated.Labels) != 1 || updated.Labels[0] != "urgent" { t.Errorf("Read() labels = %v, want [urgent]", updated.Labels) } if !updated.UpdatedAt.After(now) { t.Error("Read() updated_at should be after original") } }
func TestStore_Delete(t *testing.T) { t.Parallel() repo := initTestRepo(t) store := NewStore(repo)
now := time.Now() id, err := GenerateID() if err != nil { t.Fatalf("GenerateID() error = %v", err) } meta := &Metadata{ TrailID: id, Branch: "feature/test", Base: "main", Title: "To delete", Status: StatusDraft, Author: &Author{ID: "1", Login: strPtr("test")}, Assignees: []string{}, Labels: []string{}, CreatedAt: now, UpdatedAt: now, } if err := store.Write(context.Background(), meta, nil, nil); err != nil { t.Fatalf("Write() error = %v", err) }
// Delete if err := store.Delete(context.Background(), id); err != nil { t.Fatalf("Delete() error = %v", err) }
// Verify it's gone _, _, _, err = store.Read(id) if err == nil { t.Error("Read() should fail after delete") } }
func TestStore_ReadNonExistent(t *testing.T) { t.Parallel() repo := initTestRepo(t) store := NewStore(repo)
if err := store.EnsureBranch(context.Background()); err != nil { t.Fatalf("EnsureBranch() error = %v", err) }
_, _, _, err := store.Read(ID("abcdef123456")) if err == nil { t.Error("Read() should fail for non-existent trail") } }
func TestStore_ReadInvalidID(t *testing.T) { t.Parallel() repo := initTestRepo(t) store := NewStore(repo)
// Invalid format: too short _, _, _, err := store.Read(ID("abc")) if err == nil { t.Error("Read() should fail for invalid trail ID") }
// Path traversal attempt _, _, _, err = store.Read(ID("../../etc/pass")) if err == nil { t.Error("Read() should fail for path traversal ID") } }
func TestStore_DeleteInvalidID(t *testing.T) { t.Parallel() repo := initTestRepo(t) store := NewStore(repo)
// Invalid format: uppercase hex err := store.Delete(context.Background(), ID("ABCDEF123456")) if err == nil { t.Error("Delete() should fail for invalid trail ID") }
// Path traversal attempt err = store.Delete(context.Background(), ID("../../../etc")) if err == nil { t.Error("Delete() should fail for path traversal ID") } }
func TestStore_AddCheckpointPreservesOtherFields(t *testing.T) { t.Parallel() repo := initTestRepo(t) store := NewStore(repo)
trailID, err := GenerateID() if err != nil { t.Fatalf("GenerateID() error = %v", err) }
now := time.Now().Truncate(time.Second) metadata := &Metadata{ TrailID: trailID, Branch: "feature/preserve", Base: "main", Title: "Preservation test", Body: "Verify AddCheckpoint doesn't corrupt other fields", Status: StatusOpen, Author: &Author{ID: "1", Login: strPtr("tester")}, Assignees: []string{"alice"}, Labels: []string{"important"}, CreatedAt: now, UpdatedAt: now, } discussion := &Discussion{Comments: []Comment{ {ID: "c1", Author: "bob", Body: "looks good", CreatedAt: now}, }}
if err := store.Write(context.Background(), metadata, discussion, nil); err != nil { t.Fatalf("Write() error = %v", err) }
// Add a checkpoint firstSummary := "first checkpoint" cpRef := CheckpointRef{ CheckpointID: "aabbccddeeff", CommitSHA: "deadbeef1234", CreatedAt: now, Summary: &firstSummary, } if err := store.AddCheckpoint(context.Background(), trailID, cpRef); err != nil { t.Fatalf("AddCheckpoint() error = %v", err) }
// Read back and verify metadata + discussion are unchanged gotMeta, gotDisc, gotCPs, err := store.Read(trailID) if err != nil { t.Fatalf("Read() error = %v", err) }
// Metadata unchanged if gotMeta.Title != "Preservation test" { t.Errorf("metadata title changed: got %q, want %q", gotMeta.Title, "Preservation test") } if gotMeta.Body != "Verify AddCheckpoint doesn't corrupt other fields" { t.Errorf("metadata body changed: got %q", gotMeta.Body) } if gotMeta.Status != StatusOpen { t.Errorf("metadata status changed: got %q, want %q", gotMeta.Status, StatusOpen) } if len(gotMeta.Assignees) != 1 || gotMeta.Assignees[0] != "alice" { t.Errorf("metadata assignees changed: got %v", gotMeta.Assignees) } if len(gotMeta.Labels) != 1 || gotMeta.Labels[0] != "important" { t.Errorf("metadata labels changed: got %v", gotMeta.Labels) }
// Discussion unchanged if len(gotDisc.Comments) != 1 { t.Fatalf("discussion comments count = %d, want 1", len(gotDisc.Comments)) } if gotDisc.Comments[0].ID != "c1" || gotDisc.Comments[0].Body != "looks good" { t.Errorf("discussion comment changed: got %+v", gotDisc.Comments[0]) }
// Checkpoint added correctly if len(gotCPs.Checkpoints) != 1 { t.Fatalf("checkpoints count = %d, want 1", len(gotCPs.Checkpoints)) } if gotCPs.Checkpoints[0].CheckpointID != "aabbccddeeff" { t.Errorf("checkpoint ID = %q, want %q", gotCPs.Checkpoints[0].CheckpointID, "aabbccddeeff") }
// Add a second checkpoint — should prepend secondSummary := "second checkpoint" cpRef2 := CheckpointRef{ CheckpointID: "112233445566", CommitSHA: "cafebabe5678", CreatedAt: now, Summary: &secondSummary, } if err := store.AddCheckpoint(context.Background(), trailID, cpRef2); err != nil { t.Fatalf("AddCheckpoint() second call error = %v", err) }
_, _, gotCPs2, err := store.Read(trailID) if err != nil { t.Fatalf("Read() error = %v", err) } if len(gotCPs2.Checkpoints) != 2 { t.Fatalf("checkpoints count = %d, want 2", len(gotCPs2.Checkpoints)) } if gotCPs2.Checkpoints[0].CheckpointID != "112233445566" { t.Errorf("newest checkpoint should be first, got %q", gotCPs2.Checkpoints[0].CheckpointID) } if gotCPs2.Checkpoints[1].CheckpointID != "aabbccddeeff" { t.Errorf("older checkpoint should be second, got %q", gotCPs2.Checkpoints[1].CheckpointID) } }
Dcmd/entire/cli/trail/store\_test.go-481
1 2 3 4 2 3 4 5 6 7 8 9 10 11 8 9 10 11 16 17 12 13 14 15 16 17 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 18 19 20 4 unmodified lines
25 26 27 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 28 29 30 27 unmodified lines
58 59 60 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 61 62 63 19 unmodified lines
83 84 85 168 169 170 86 87 88 5 unmodified lines
94 95 96 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 97 98 99 4 unmodified lines
104 105 106 217 218 219 220 221 222 223 224 225 226 227 228 229 107 108 109
// Package trail provides types and helpers for managing trail metadata. // Trails are branch-centric work tracking abstractions stored on the // entire/trails/v1 orphan branch. They answer "why/what" (human intent) // while checkpoints answer "how/when" (machine snapshots). // Trails are branch-centric work-tracking abstractions served by the core // API. They answer "why/what" (human intent) while checkpoints answer // "how/when" (machine snapshots). package trail
import ( "crypto/rand" "encoding/hex" "fmt" "regexp" "strings" "time" )
const idLength = 6 // 6 bytes = 12 hex chars
// ID is a 12-character hex identifier for trails. type ID string
// EmptyID represents an unset or invalid trail ID. const EmptyID ID = ""
// idRegex validates the format: exactly 12 lowercase hex characters.
var idRegex = regexp.MustCompile(^[0-9a-f]{12}$)
// GenerateID creates a new random 12-character hex trail ID. func GenerateID() (ID, error) { bytes := make([]byte, idLength) if _, err := rand.Read(bytes); err != nil { return EmptyID, fmt.Errorf("failed to generate random trail ID: %w", err) } return ID(hex.EncodeToString(bytes)), nil }
// ValidateID checks if a string is a valid trail ID format. func ValidateID(s string) error { if !idRegex.MatchString(s) { return fmt.Errorf("invalid trail ID %q: must be 12 lowercase hex characters", s) } return nil }
// String returns the trail ID as a string. func (id ID) String() string { return string(id) 4 unmodified lines
return id == EmptyID }
// Path returns the sharded storage path for this trail ID. // Uses first 2 characters as shard (256 buckets), remaining as folder name. // Example: "a3b2c4d5e6f7" -> "a3/b2c4d5e6f7" func (id ID) Path() string { if len(id) < 3 { return string(id) } return string(id[:2]) + "/" + string(id[2:]) }
// ShardParts returns the shard prefix and suffix separately. // Example: "a3b2c4d5e6f7" -> ("a3", "b2c4d5e6f7") func (id ID) ShardParts() (shard, suffix string) { if len(id) < 3 { return string(id), "" } return string(id[:2]), string(id[2:]) }
// Status represents the lifecycle status of a trail. type Status string
27 unmodified lines
return false }
// Priority represents the priority level of a trail. type Priority string
const ( PriorityUrgent Priority = "urgent" PriorityHigh Priority = "high" PriorityMedium Priority = "medium" PriorityLow Priority = "low" PriorityNone Priority = "none" )
// Type represents the type/category of a trail. type Type string
const ( TypeBug Type = "bug" TypeFeature Type = "feature" TypeChore Type = "chore" TypeDocs Type = "docs" TypeRefactor Type = "refactor" )
// ReviewerStatus represents the review status for a reviewer. type ReviewerStatus string
const ( ReviewerPending ReviewerStatus = "pending" ReviewerApproved ReviewerStatus = "approved" ReviewerChangesRequested ReviewerStatus = "changes_requested" )
// Reviewer represents a reviewer assigned to a trail.
type Reviewer struct {
Login string json:"login"
Status ReviewerStatus json:"status"
}
// Author identifies the user who created a trail. // On the wire the whole object may be null when the original author can no // longer be resolved (e.g. the GitHub user no longer exists), and login may 19 unmodified lines
CreatedAt time.Time json:"created_at"
UpdatedAt time.Time json:"updated_at"
MergedAt *time.Time json:"merged_at"
Priority Priority json:"priority,omitempty"
Type Type json:"type,omitempty"
Reviewers []Reviewer json:"reviewers,omitempty"
}
// AuthorLogin returns the trail author's login, or an empty string if the 5 unmodified lines
return *m.Author.Login }
// Discussion holds the discussion/comments for a trail.
type Discussion struct {
Comments []Comment json:"comments"
}
// Comment represents a single comment on a trail.
type Comment struct {
ID string json:"id"
Author string json:"author"
Body string json:"body"
CreatedAt time.Time json:"created_at"
Resolved bool json:"resolved"
ResolvedBy *string json:"resolved_by"
ResolvedAt *time.Time json:"resolved_at"
Replies []CommentReply json:"replies,omitempty"
}
// CommentReply represents a reply to a comment.
type CommentReply struct {
ID string json:"id"
Author string json:"author"
Body string json:"body"
CreatedAt time.Time json:"created_at"
}
// commonBranchPrefixes are stripped from branch names when humanizing. var commonBranchPrefixes = []string{ "feature/", 4 unmodified lines
"release/", }
// CheckpointRef links a checkpoint to a trail.
type CheckpointRef struct {
CheckpointID string json:"checkpoint_id"
CommitSHA string json:"commit_sha"
CreatedAt time.Time json:"created_at"
Summary *string json:"summary"
}
// Checkpoints holds the list of checkpoint references for a trail.
type Checkpoints struct {
Checkpoints []CheckpointRef json:"checkpoints"
}
// HumanizeBranchName converts a branch name into a human-readable title. // It strips common prefixes (feature/, fix/, etc.), replaces dashes/underscores // with spaces, and capitalizes the first word.
Mcmd/entire/cli/trail/trail.go+3/-126
3 unmodified lines
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 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 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 7 8 9
3 unmodified lines
"testing" )
func TestGenerateID(t *testing.T) { t.Parallel()
id, err := GenerateID() if err != nil { t.Fatalf("GenerateID() error = %v", err) } if len(id) != 12 { t.Errorf("expected 12-char ID, got %d: %q", len(id), id) } if err := ValidateID(id.String()); err != nil { t.Errorf("generated ID failed validation: %v", err) } }
func TestGenerateID_Unique(t *testing.T) { t.Parallel()
seen := make(map[ID]bool) for range 100 { id, err := GenerateID() if err != nil { t.Fatalf("GenerateID() error = %v", err) } if seen[id] { t.Errorf("duplicate ID generated: %s", id) } seen[id] = true } }
func TestValidateID(t *testing.T) { t.Parallel()
tests := []struct { name string id string wantErr bool }{ {"valid", "abcdef123456", false}, {"valid_all_hex", "0123456789ab", false}, {"too_short", "abcdef", true}, {"too_long", "abcdef1234567", true}, {"uppercase", "ABCDEF123456", true}, {"non_hex", "ghijkl123456", true}, {"empty", "", true}, }
for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() err := ValidateID(tt.id) if (err != nil) != tt.wantErr { t.Errorf("ValidateID(%q) error = %v, wantErr %v", tt.id, err, tt.wantErr) } }) } }
func TestID_Path(t *testing.T) { t.Parallel()
tests := []struct { id ID want string }{ {"abcdef123456", "ab/cdef123456"}, {"0123456789ab", "01/23456789ab"}, {"ab", "ab"}, }
for _, tt := range tests { t.Run(string(tt.id), func(t *testing.T) { t.Parallel() if got := tt.id.Path(); got != tt.want { t.Errorf("ID(%q).Path() = %q, want %q", tt.id, got, tt.want) } }) } }
func TestID_IsEmpty(t *testing.T) { t.Parallel()
Mcmd/entire/cli/trail/trail\_test.go-81
355 unmodified lines
356 357 358 359 359 360 361 362 556 unmodified lines
919 920 921 922 923 924 925 926 922 923 924
355 unmodified lines
if err != nil { return err } if hydrated, hydrateErr := hydrateTrailReviewCommentSuggestions(cmd.Context(), client, target.Trail.ID, comment); hydrateErr == nil { if hydrated, _, hydrateErr := hydrateTrailReviewCommentWithState(cmd.Context(), client, target.Trail.ID, comment); hydrateErr == nil { comment = hydrated } printTrailReviewCommentDetail(cmd.OutOrStdout(), comment) 556 unmodified lines
} }
func hydrateTrailReviewCommentSuggestions(ctx context.Context, client *api.Client, trailID string, comment api.TrailReviewComment) (api.TrailReviewComment, error) { hydrated, _, err := hydrateTrailReviewCommentWithState(ctx, client, trailID, comment) return hydrated, err }
func hydrateTrailReviewCommentWithState(ctx context.Context, client *api.Client, trailID string, comment api.TrailReviewComment) (api.TrailReviewComment, api.TrailReviewStateResponse, error) { state, err := fetchTrailReviewState(ctx, client, trailID, comment.ReviewID) if err != nil {
Mcmd/entire/cli/trail\_review\_cmd.go+1/-6
8 unmodified lines
9 10 11 12 12 13 14 26 unmodified lines
41 42 43 45 44 45 46 49 47 48 49 50 51 52 55 53 54 55 56 57 6 unmodified lines
64 65 66 67 68 69 70 71 72 73 69 70 75 71 72 73 74 75 76 4 unmodified lines
81 82 83 86 87 88 89 90 91 92 93 94 95 84 85 86 68 unmodified lines
155 156 157 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 158 159 160
8 unmodified lines
"io" "net/http" "net/url" "strconv" "strings" "time"
26 unmodified lines
jsonOutput bool showPings bool once bool number int )
cmd := &cobra.Command{
Use: "watch [
If
This command resolves the trail's id internally and streams
GET /api/v1/trails/
error server-side error; treated as reconnect`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { selector := "" if len(args) == 1 { n, err := strconv.Atoi(args[0]) if err != nil || n <= 0 { return fmt.Errorf("invalid trail number %q", args[0]) } number = n selector = args[0] } return runTrailWatch(cmd, number, jsonOutput, showPings, once) // Delegates to the shared trail-review resolver so number/id/branch // selectors and insecure-HTTP handling stay in one place. return runTrailReviewWatch(cmd, selector, jsonOutput, showPings, once) }, }
4 unmodified lines
return cmd }
func runTrailWatch(cmd *cobra.Command, number int, jsonOutput, showPings, once bool) error { return runAuthenticatedDataAPI(cmd.Context(), cmd.ErrOrStderr(), trailInsecureHTTP(cmd), func(ctx context.Context, client *api.Client) error { trailID, description, err := resolveTrailWatchTarget(ctx, client, number) if err != nil { return err } return runTrailWatchResolved(cmd, client, trailID, description, jsonOutput, showPings, once) }) }
func runTrailWatchResolved(cmd *cobra.Command, client *api.Client, trailID, description string, jsonOutput, showPings, once bool) error { ctx := cmd.Context() w := cmd.OutOrStdout() 68 unmodified lines
} }
func resolveTrailWatchTarget(ctx context.Context, client *api.Client, number int) (trailID, description string, err error) { if number > 0 { return resolveTrailWatchNumber(ctx, client, number) }
forge, owner, repo, err := resolveTrailRemote(ctx) if err != nil { return "", "", err } branch, err := GetCurrentBranch(ctx) if err != nil { return "", "", fmt.Errorf("no trail number given and current branch is unknown: %w", err) } found, err := findTrailByBranch(ctx, client, forge, owner, repo, branch) if err != nil { return "", "", err } if found == nil { return "", "", fmt.Errorf("no trail found for branch %q (pass an explicit trail number)", branch) } if found.ID == "" { return "", "", fmt.Errorf("trail for branch %q has no id yet", branch) } return found.ID, trailWatchDescription(forge, owner, repo, found.Number, found.ID), nil }
func resolveTrailWatchNumber(ctx context.Context, client *api.Client, number int) (trailID, description string, err error) { forge, owner, repo, err := resolveTrailRemote(ctx) if err != nil { return "", "", err } found, err := findTrailByNumber(ctx, client, forge, owner, repo, number) if err != nil { return "", "", err } if found == nil { return "", "", fmt.Errorf("no trail #%d found in %s/%s/%s", number, forge, owner, repo) } if found.ID == "" { return "", "", fmt.Errorf("trail #%d has no id yet", number) } return found.ID, trailWatchDescription(forge, owner, repo, found.Number, found.ID), nil }
func trailWatchDescription(forge, owner, repo string, number int, trailID string) string { if number > 0 { return fmt.Sprintf("trail #%d (%s/%s/%s, id %s)", number, forge, owner, repo, trailID)
Mcmd/entire/cli/trail\_watch\_cmd.go+8/-64