From 9ca3b7c464e45d48fd147e95a52583c80e338953 Mon Sep 17 00:00:00 2001 From: Peyton Montei Date: Thu, 16 Jul 2026 18:07:01 -0400 Subject: [PATCH 01/11] feat: resolve dispatch summary agents Entire-Checkpoint: 01KXPFG4E8FBAX2C5W6XYBZYGC --- cmd/entire/cli/explain_summary_provider.go | 37 +- .../cli/explain_summary_provider_test.go | 330 ++++++++++++++++++ 2 files changed, 358 insertions(+), 9 deletions(-) diff --git a/cmd/entire/cli/explain_summary_provider.go b/cmd/entire/cli/explain_summary_provider.go index b0e7e6cb66..27c15b601f 100644 --- a/cmd/entire/cli/explain_summary_provider.go +++ b/cmd/entire/cli/explain_summary_provider.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "strings" "github.com/entireio/cli/cmd/entire/cli/agent" "github.com/entireio/cli/cmd/entire/cli/agent/external" @@ -30,10 +31,25 @@ var ( ) type checkpointSummaryProvider struct { - Name types.AgentName - DisplayName string - Model string - Generator summarize.Generator + Name types.AgentName + DisplayName string + Model string + TextGenerator agent.TextGenerator + Generator summarize.Generator +} + +func resolveDispatchSummaryProvider(ctx context.Context, w io.Writer, override string) (*checkpointSummaryProvider, error) { + override = strings.TrimSpace(override) + if override == "" { + return resolveCheckpointSummaryProvider(ctx, w) + } + + providerName := types.AgentName(override) + discoverSummaryProviderIfMissing(ctx, providerName) + if err := ensureSummaryProviderPresent(ctx, providerName); err != nil { + return nil, err + } + return buildCheckpointSummaryProviderWithEffectiveModel(providerName, "") } func resolveCheckpointSummaryProvider(ctx context.Context, w io.Writer) (*checkpointSummaryProvider, error) { @@ -167,6 +183,10 @@ func promptForSummaryProvider(providers []checkpointSummaryProvider) (types.Agen } func buildCheckpointSummaryProvider(name types.AgentName, model string) (*checkpointSummaryProvider, error) { + return buildCheckpointSummaryProviderWithEffectiveModel(name, summarize.ResolveModel(name, model)) +} + +func buildCheckpointSummaryProviderWithEffectiveModel(name types.AgentName, effectiveModel string) (*checkpointSummaryProvider, error) { ag, err := getSummaryAgent(name) if err != nil { return nil, fmt.Errorf("loading summary provider %s: %w", name, err) @@ -177,12 +197,11 @@ func buildCheckpointSummaryProvider(name types.AgentName, model string) (*checkp return nil, fmt.Errorf("agent %s does not support summary generation", name) } - effectiveModel := summarize.ResolveModel(name, model) - return &checkpointSummaryProvider{ - Name: name, - DisplayName: string(ag.Type()), - Model: effectiveModel, + Name: name, + DisplayName: string(ag.Type()), + Model: effectiveModel, + TextGenerator: textGenerator, Generator: &summarize.TextGeneratorAdapter{ TextGenerator: textGenerator, Model: effectiveModel, diff --git a/cmd/entire/cli/explain_summary_provider_test.go b/cmd/entire/cli/explain_summary_provider_test.go index cf37380d8c..2e440ef202 100644 --- a/cmd/entire/cli/explain_summary_provider_test.go +++ b/cmd/entire/cli/explain_summary_provider_test.go @@ -3,6 +3,7 @@ package cli import ( "bytes" "context" + "errors" "os" "os/exec" "path/filepath" @@ -81,6 +82,10 @@ func (s *stubTextAgent) GenerateText(context.Context, string, string) (string, e return `{"intent":"Intent","outcome":"Outcome","learnings":{"repo":[],"code":[],"workflow":[]},"friction":[],"open_items":[]}`, nil } +type stubNonTextAgent struct { + agent.Agent +} + func TestResolveCheckpointSummaryProvider_UsesConfiguredProvider(t *testing.T) { // Cannot use t.Parallel() because we use t.Chdir and package-level var stubs ctx := context.Background() @@ -130,6 +135,331 @@ func TestResolveCheckpointSummaryProvider_UsesConfiguredProvider(t *testing.T) { if provider.Model != "haiku" { t.Fatalf("provider.Model = %q, want %q", provider.Model, "haiku") } + if provider.TextGenerator == nil { + t.Fatal("provider.TextGenerator = nil, want configured provider's raw text generator") + } +} + +func TestResolveDispatchSummaryProvider_ExplicitCodexUsesDefaultModelWithoutPersistence(t *testing.T) { + // Cannot use t.Parallel(): mutates package-level resolution seams. + ctx := context.Background() + codex := &stubTextAgent{name: agent.AgentNameCodex, kind: agent.AgentTypeCodex} + + originalLoad := loadSummarySettings + originalLoadFile := loadSummarySettingsFromFile + originalSave := saveLocalSummarySettings + originalGet := getSummaryAgent + originalCLI := isSummaryCLIAvailable + originalDiscover := discoverSummaryProvidersAlways + t.Cleanup(func() { + loadSummarySettings = originalLoad + loadSummarySettingsFromFile = originalLoadFile + saveLocalSummarySettings = originalSave + getSummaryAgent = originalGet + isSummaryCLIAvailable = originalCLI + discoverSummaryProvidersAlways = originalDiscover + }) + + loadSummarySettings = func(context.Context) (*settings.EntireSettings, error) { + t.Fatal("explicit dispatch provider must not load summary settings") + return nil, errors.New("unexpected settings load") + } + loadSummarySettingsFromFile = func(string) (*settings.EntireSettings, error) { + t.Fatal("explicit dispatch provider must not load settings for persistence") + return nil, errors.New("unexpected settings load for persistence") + } + saveLocalSummarySettings = func(context.Context, *settings.EntireSettings) error { + t.Fatal("explicit dispatch provider must not persist settings") + return nil + } + getSummaryAgent = func(name types.AgentName) (agent.Agent, error) { + if name != agent.AgentNameCodex { + t.Fatalf("getSummaryAgent(%q), want %q", name, agent.AgentNameCodex) + } + return codex, nil + } + isSummaryCLIAvailable = func(name types.AgentName) bool { + return name == agent.AgentNameCodex + } + discoverSummaryProvidersAlways = func(context.Context) { + t.Fatal("registered explicit provider should not trigger external discovery") + } + + provider, err := resolveDispatchSummaryProvider(ctx, &bytes.Buffer{}, " codex ") + if err != nil { + t.Fatalf("resolveDispatchSummaryProvider() error = %v", err) + } + if provider.Name != agent.AgentNameCodex { + t.Fatalf("provider.Name = %q, want %q", provider.Name, agent.AgentNameCodex) + } + if provider.Model != "" { + t.Fatalf("provider.Model = %q, want provider CLI default", provider.Model) + } + if provider.TextGenerator != codex { + t.Fatalf("provider.TextGenerator = %T %p, want raw generator %T %p", provider.TextGenerator, provider.TextGenerator, codex, codex) + } +} + +func TestResolveDispatchSummaryProvider_EmptyOverrideUsesConfiguredProviderAndModel(t *testing.T) { + // Cannot use t.Parallel(): mutates package-level resolution seams. + ctx := context.Background() + configured := &stubTextAgent{name: agent.AgentNameGemini, kind: agent.AgentTypeGemini} + + originalLoad := loadSummarySettings + originalGet := getSummaryAgent + originalCLI := isSummaryCLIAvailable + originalDiscover := discoverSummaryProvidersAlways + t.Cleanup(func() { + loadSummarySettings = originalLoad + getSummaryAgent = originalGet + isSummaryCLIAvailable = originalCLI + discoverSummaryProvidersAlways = originalDiscover + }) + + loadSummarySettings = func(context.Context) (*settings.EntireSettings, error) { + return &settings.EntireSettings{SummaryGeneration: &settings.SummaryGenerationSettings{ + Provider: string(agent.AgentNameGemini), + Model: "gemini-saved-model", + }}, nil + } + getSummaryAgent = func(name types.AgentName) (agent.Agent, error) { + if name != agent.AgentNameGemini { + t.Fatalf("getSummaryAgent(%q), want %q", name, agent.AgentNameGemini) + } + return configured, nil + } + isSummaryCLIAvailable = func(name types.AgentName) bool { + return name == agent.AgentNameGemini + } + discoverSummaryProvidersAlways = func(context.Context) { + t.Fatal("configured registered provider should not trigger external discovery") + } + + provider, err := resolveDispatchSummaryProvider(ctx, &bytes.Buffer{}, " \t\n") + if err != nil { + t.Fatalf("resolveDispatchSummaryProvider() error = %v", err) + } + if provider.Name != agent.AgentNameGemini { + t.Fatalf("provider.Name = %q, want %q", provider.Name, agent.AgentNameGemini) + } + if provider.Model != "gemini-saved-model" { + t.Fatalf("provider.Model = %q, want configured model", provider.Model) + } + if provider.TextGenerator != configured { + t.Fatalf("provider.TextGenerator = %T, want configured raw generator", provider.TextGenerator) + } +} + +func TestResolveDispatchSummaryProvider_ExplicitProviderIgnoresSavedProviderAndModel(t *testing.T) { + // Cannot use t.Parallel(): mutates package-level resolution seams. + ctx := context.Background() + codex := &stubTextAgent{name: agent.AgentNameCodex, kind: agent.AgentTypeCodex} + loadCalls := 0 + + originalLoad := loadSummarySettings + originalGet := getSummaryAgent + originalCLI := isSummaryCLIAvailable + t.Cleanup(func() { + loadSummarySettings = originalLoad + getSummaryAgent = originalGet + isSummaryCLIAvailable = originalCLI + }) + + loadSummarySettings = func(context.Context) (*settings.EntireSettings, error) { + loadCalls++ + return &settings.EntireSettings{SummaryGeneration: &settings.SummaryGenerationSettings{ + Provider: string(agent.AgentNameClaudeCode), + Model: "sonnet", + }}, nil + } + getSummaryAgent = func(types.AgentName) (agent.Agent, error) { return codex, nil } + isSummaryCLIAvailable = func(types.AgentName) bool { return true } + + provider, err := resolveDispatchSummaryProvider(ctx, &bytes.Buffer{}, string(agent.AgentNameCodex)) + if err != nil { + t.Fatalf("resolveDispatchSummaryProvider() error = %v", err) + } + if loadCalls != 0 { + t.Fatalf("loadSummarySettings calls = %d, want 0 for explicit override", loadCalls) + } + if provider.Name != agent.AgentNameCodex || provider.Model != "" { + t.Fatalf("provider = %+v, want explicit Codex with provider-default model", provider) + } +} + +func TestResolveDispatchSummaryProvider_ExplicitClaudePreservesEmptyModel(t *testing.T) { + // Cannot use t.Parallel(): mutates package-level resolution seams. + ctx := context.Background() + claude := &stubTextAgent{name: agent.AgentNameClaudeCode, kind: agent.AgentTypeClaudeCode} + loadCalls := 0 + + originalLoad := loadSummarySettings + originalGet := getSummaryAgent + originalCLI := isSummaryCLIAvailable + t.Cleanup(func() { + loadSummarySettings = originalLoad + getSummaryAgent = originalGet + isSummaryCLIAvailable = originalCLI + }) + + loadSummarySettings = func(context.Context) (*settings.EntireSettings, error) { + loadCalls++ + return &settings.EntireSettings{SummaryGeneration: &settings.SummaryGenerationSettings{ + Provider: string(agent.AgentNameClaudeCode), + Model: "sonnet", + }}, nil + } + getSummaryAgent = func(types.AgentName) (agent.Agent, error) { return claude, nil } + isSummaryCLIAvailable = func(types.AgentName) bool { return true } + + provider, err := resolveDispatchSummaryProvider(ctx, &bytes.Buffer{}, string(agent.AgentNameClaudeCode)) + if err != nil { + t.Fatalf("resolveDispatchSummaryProvider() error = %v", err) + } + if loadCalls != 0 { + t.Fatalf("loadSummarySettings calls = %d, want 0 for explicit override", loadCalls) + } + if provider.Model != "" { + t.Fatalf("provider.Model = %q, want Claude CLI default rather than saved/summary default", provider.Model) + } +} + +func TestResolveDispatchSummaryProvider_ExplicitValidationErrors(t *testing.T) { + // Cannot use t.Parallel(): subtests mutate package-level resolution seams. + tests := []struct { + name string + override string + agent agent.Agent + getErr error + available bool + wantError string + }{ + { + name: "unknown provider", + override: "missing-provider", + getErr: errors.New("not registered"), + available: true, + wantError: "unknown summary provider", + }, + { + name: "no text generator capability", + override: "no-text", + agent: &stubNonTextAgent{Agent: &stubTextAgent{ + name: "no-text", + kind: agent.AgentTypeUnknown, + }}, + available: true, + wantError: "does not support summary generation", + }, + { + name: "CLI unavailable", + override: string(agent.AgentNameCodex), + agent: &stubTextAgent{name: agent.AgentNameCodex, kind: agent.AgentTypeCodex}, + available: false, + wantError: "not on PATH", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + originalGet := getSummaryAgent + originalCLI := isSummaryCLIAvailable + originalDiscover := discoverSummaryProvidersAlways + t.Cleanup(func() { + getSummaryAgent = originalGet + isSummaryCLIAvailable = originalCLI + discoverSummaryProvidersAlways = originalDiscover + }) + + getSummaryAgent = func(types.AgentName) (agent.Agent, error) { + if tt.getErr != nil { + return nil, tt.getErr + } + return tt.agent, nil + } + isSummaryCLIAvailable = func(types.AgentName) bool { return tt.available } + discoverSummaryProvidersAlways = func(context.Context) {} + + _, err := resolveDispatchSummaryProvider(context.Background(), &bytes.Buffer{}, tt.override) + if err == nil { + t.Fatalf("resolveDispatchSummaryProvider(%q) error = nil, want %q", tt.override, tt.wantError) + } + if !strings.Contains(err.Error(), tt.wantError) { + t.Fatalf("resolveDispatchSummaryProvider(%q) error = %q, want substring %q", tt.override, err, tt.wantError) + } + }) + } +} + +func TestResolveDispatchSummaryProvider_ExplicitExternalProviderDoesNotWriteLocalSettings(t *testing.T) { + // Cannot use t.Parallel(): subtests mutate cwd, PATH, and the agent registry. + if _, err := exec.LookPath("sh"); err != nil { + t.Skip("sh not available") + } + + tests := []struct { + name string + providerName string + localContent string + }{ + {name: "does not create settings.local.json", providerName: "external-dispatch-no-create"}, + { + name: "does not update settings.local.json", + providerName: "external-dispatch-no-update", + localContent: `{"external_agents":false,"summary_generation":{"provider":"codex","model":"saved-model"}}`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + testutil.InitRepo(t, tmpDir) + t.Chdir(tmpDir) + + if err := os.MkdirAll(filepath.Join(tmpDir, ".entire"), 0o755); err != nil { + t.Fatalf("mkdir .entire: %v", err) + } + if err := os.WriteFile(filepath.Join(tmpDir, ".entire", "settings.json"), []byte(`{"enabled":true,"external_agents":false}`), 0o644); err != nil { + t.Fatalf("write settings.json: %v", err) + } + + localPath := filepath.Join(tmpDir, ".entire", "settings.local.json") + if tt.localContent != "" { + if err := os.WriteFile(localPath, []byte(tt.localContent), 0o644); err != nil { + t.Fatalf("write settings.local.json: %v", err) + } + } + + externalDir := t.TempDir() + writeExternalSummaryAgentBinary(t, externalDir, tt.providerName) + t.Setenv("PATH", externalDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + provider, err := resolveDispatchSummaryProvider(ctx, &bytes.Buffer{}, tt.providerName) + if err != nil { + t.Fatalf("resolveDispatchSummaryProvider() error = %v", err) + } + if provider.Name != types.AgentName(tt.providerName) { + t.Fatalf("provider.Name = %q, want %q", provider.Name, tt.providerName) + } + if provider.Model != "" { + t.Fatalf("provider.Model = %q, want external CLI default", provider.Model) + } + if provider.TextGenerator == nil { + t.Fatal("provider.TextGenerator = nil, want external raw generator") + } + + got, err := os.ReadFile(localPath) + switch { + case tt.localContent == "" && !errors.Is(err, os.ErrNotExist): + t.Fatalf("settings.local.json read error = %v, want file to remain absent (content %q)", err, got) + case tt.localContent != "" && err != nil: + t.Fatalf("read settings.local.json: %v", err) + case tt.localContent != "" && string(got) != tt.localContent: + t.Fatalf("settings.local.json changed:\n got: %s\nwant: %s", got, tt.localContent) + } + }) + } } func TestResolveCheckpointSummaryProvider_SavesSingleInstalledProvider(t *testing.T) { From 74a7be8428a44a054e3ebccaeaca03a7c7e23113 Mon Sep 17 00:00:00 2001 From: Peyton Montei Date: Thu, 16 Jul 2026 19:23:23 -0400 Subject: [PATCH 02/11] feat: add local dispatch agent selection Entire-Checkpoint: 01KXPKVZ0XZVYM3S01VQKDHBT5 --- cmd/entire/cli/agent/external/discovery.go | 88 ++++-- cmd/entire/cli/dispatch.go | 22 ++ cmd/entire/cli/dispatch/dispatch.go | 6 + cmd/entire/cli/dispatch/generate.go | 30 +- cmd/entire/cli/dispatch/generate_test.go | 55 +++- cmd/entire/cli/dispatch/mode_local.go | 2 +- cmd/entire/cli/dispatch/mode_local_test.go | 98 +++--- cmd/entire/cli/dispatch_test.go | 295 ++++++++++++++++++ cmd/entire/cli/explain_summary_provider.go | 25 +- .../cli/explain_summary_provider_test.go | 77 ++++- cmd/entire/cli/setup_test.go | 3 + 11 files changed, 552 insertions(+), 149 deletions(-) diff --git a/cmd/entire/cli/agent/external/discovery.go b/cmd/entire/cli/agent/external/discovery.go index 2fb5fcb1f7..d48775f405 100644 --- a/cmd/entire/cli/agent/external/discovery.go +++ b/cmd/entire/cli/agent/external/discovery.go @@ -4,6 +4,7 @@ import ( "context" "log/slog" "os" + "os/exec" "path/filepath" "runtime" "strings" @@ -43,6 +44,24 @@ func DiscoverAndRegisterAlways(ctx context.Context) { discoverAndRegister(ctx) } +// DiscoverAndRegisterNamedAlways discovers and registers only the external +// agent binary matching name. It bypasses the external_agents setting for +// explicit, one-invocation selections without executing unrelated plugins. +func DiscoverAndRegisterNamedAlways(ctx context.Context, name types.AgentName) { + if name == "" { + return + } + if _, err := agent.Get(name); err == nil { + return + } + + binPath, err := exec.LookPath(binaryPrefix + string(name)) + if err != nil { + return + } + registerExternalAgent(ctx, binPath, name) +} + // discoverAndRegister contains the shared scanning logic for external agent discovery. func discoverAndRegister(ctx context.Context) { ctx, cancel := context.WithTimeout(ctx, discoveryTimeout) @@ -93,42 +112,47 @@ func discoverAndRegister(ctx context.Context) { continue } - finfo, err := os.Stat(binPath) //nolint:gosec // PATH entries are trusted - if err != nil || finfo.IsDir() { - continue - } - // Check executable bit (on Unix; Windows doesn't set execute bits) - if runtime.GOOS != osWindows && finfo.Mode()&0o111 == 0 { - continue + if registerExternalAgent(ctx, binPath, agentName) { + registered[agentName] = true } + } + } +} - ea, err := New(ctx, binPath) - if err != nil { - logging.Debug(ctx, "skipping external agent (info failed)", - slog.String("binary", binPath), - slog.String("error", err.Error())) - continue - } +func registerExternalAgent(ctx context.Context, binPath string, name types.AgentName) bool { + finfo, err := os.Stat(binPath) //nolint:gosec // PATH entries are trusted + if err != nil || finfo.IsDir() { + return false + } + // Check executable bit (on Unix; Windows doesn't set execute bits). + if runtime.GOOS != osWindows && finfo.Mode()&0o111 == 0 { + return false + } - // Wrap with capability interfaces and register - wrapped, err := Wrap(ea) - if err != nil { - logging.Debug(ctx, "skipping external agent (wrap failed)", - slog.String("binary", binPath), - slog.String("error", err.Error())) - continue - } - agent.Register(agentName, func() agent.Agent { - return wrapped - }) - registered[agentName] = true - - logging.Debug(ctx, "registered external agent", - slog.String("name", string(agentName)), - slog.String("type", string(ea.Type())), - slog.String("binary", binPath)) - } + ea, err := New(ctx, binPath) + if err != nil { + logging.Debug(ctx, "skipping external agent (info failed)", + slog.String("binary", binPath), + slog.String("error", err.Error())) + return false + } + + wrapped, err := Wrap(ea) + if err != nil { + logging.Debug(ctx, "skipping external agent (wrap failed)", + slog.String("binary", binPath), + slog.String("error", err.Error())) + return false } + agent.Register(name, func() agent.Agent { + return wrapped + }) + + logging.Debug(ctx, "registered external agent", + slog.String("name", string(name)), + slog.String("type", string(ea.Type())), + slog.String("binary", binPath)) + return true } // StripExeExt removes Windows executable extensions (.exe, .bat, .cmd, .com) diff --git a/cmd/entire/cli/dispatch.go b/cmd/entire/cli/dispatch.go index 52804b859a..b9ffad1d4f 100644 --- a/cmd/entire/cli/dispatch.go +++ b/cmd/entire/cli/dispatch.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "os" + "strings" dispatchpkg "github.com/entireio/cli/cmd/entire/cli/dispatch" "github.com/entireio/cli/cmd/entire/cli/interactive" @@ -18,6 +19,7 @@ var renderDispatchMarkdown = dispatchpkg.RenderMarkdown var dispatchTerminalMode = interactive.IsTerminalWriter var runInteractiveDispatch = defaultRunInteractiveDispatch var renderTerminalMarkdown = defaultRenderTerminalMarkdown +var resolveDispatchProvider = resolveDispatchSummaryProvider func newDispatchCmd() *cobra.Command { var ( @@ -27,6 +29,7 @@ func newDispatchCmd() *cobra.Command { flagAllBranches bool flagRepos []string flagVoice string + flagAgent string flagInsecureHTTPAuth bool ) @@ -38,9 +41,19 @@ func newDispatchCmd() *cobra.Command { Examples: entire dispatch entire dispatch --local --all-branches + entire dispatch --local --agent codex entire dispatch --repos entireio/cli entire dispatch --voice neutral`, RunE: func(cmd *cobra.Command, _ []string) error { + agentOverride := strings.TrimSpace(flagAgent) + agentFlagSet := cmd.Flags().Changed("agent") + if agentFlagSet && !flagLocal { + return errors.New("--agent only applies to --local (cloud dispatch uses Entire's server-side generator)") + } + if agentFlagSet && agentOverride == "" { + return errors.New("--agent requires a non-empty value") + } + var ( opts dispatchpkg.Options err error @@ -57,6 +70,14 @@ Examples: } return err } + if opts.Mode == dispatchpkg.ModeLocal { + provider, err := resolveDispatchProvider(cmd.Context(), cmd.ErrOrStderr(), agentOverride) + if err != nil { + return err + } + opts.TextGenerator = provider.TextGenerator + opts.Model = provider.Model + } if err := runDispatchCommand(cmd.Context(), cmd.OutOrStdout(), opts); err != nil { if errors.Is(err, errDispatchCancelled) { @@ -74,6 +95,7 @@ Examples: cmd.Flags().BoolVar(&flagAllBranches, "all-branches", false, "include every existing local branch (--local only; renamed or deleted branches are skipped)") cmd.Flags().StringSliceVar(&flagRepos, "repos", nil, fmt.Sprintf("cloud repo slugs, up to %d (for example entireio/cli)", dispatchpkg.CloudRepoLimit)) cmd.Flags().StringVar(&flagVoice, "voice", "", "voice preset name or literal description") + cmd.Flags().StringVar(&flagAgent, "agent", "", "local text-generation agent (requires --local)") cmd.Flags().BoolVar(&flagInsecureHTTPAuth, "insecure-http-auth", false, "Allow authentication over plain HTTP (insecure, for local development only)") if err := cmd.Flags().MarkHidden("insecure-http-auth"); err != nil { panic(fmt.Sprintf("hide insecure-http-auth flag: %v", err)) diff --git a/cmd/entire/cli/dispatch/dispatch.go b/cmd/entire/cli/dispatch/dispatch.go index 73dcd23c2f..ca05516764 100644 --- a/cmd/entire/cli/dispatch/dispatch.go +++ b/cmd/entire/cli/dispatch/dispatch.go @@ -12,6 +12,10 @@ const ( ModeLocal ) +type TextGenerator interface { + GenerateText(ctx context.Context, prompt string, model string) (string, error) +} + func (m Mode) String() string { switch m { case ModeServer: @@ -33,6 +37,8 @@ type Options struct { ImplicitCurrentBranch bool Voice string InsecureHTTPAuth bool + TextGenerator TextGenerator + Model string } // CloudRepoLimit caps how many repos the cloud mode may query in one request. diff --git a/cmd/entire/cli/dispatch/generate.go b/cmd/entire/cli/dispatch/generate.go index 339a295b07..6767e0cf58 100644 --- a/cmd/entire/cli/dispatch/generate.go +++ b/cmd/entire/cli/dispatch/generate.go @@ -8,28 +8,18 @@ import ( "strings" "time" - "github.com/entireio/cli/cmd/entire/cli/agent" - "github.com/entireio/cli/cmd/entire/cli/agent/claudecode" "github.com/entireio/cli/cmd/entire/cli/jsonutil" - "github.com/entireio/cli/cmd/entire/cli/summarize" ) -type dispatchTextGenerator interface { - GenerateText(ctx context.Context, prompt string, model string) (string, error) -} - -var dispatchTextGeneratorFactory = func() (dispatchTextGenerator, error) { - textGenerator, ok := agent.AsTextGenerator(claudecode.NewClaudeCodeAgent()) - if !ok { - return nil, errors.New("default dispatch generator does not support text generation") - } - return textGenerator, nil -} - -func generateLocalDispatch(ctx context.Context, dispatch *Dispatch, voice string) (string, error) { - textGenerator, err := dispatchTextGeneratorFactory() - if err != nil { - return "", err +func generateLocalDispatch( + ctx context.Context, + dispatch *Dispatch, + voice string, + textGenerator TextGenerator, + model string, +) (string, error) { + if textGenerator == nil { + return "", errors.New("local dispatch text generator is not configured") } prompt, err := buildDispatchPrompt(dispatch, voice) @@ -37,7 +27,7 @@ func generateLocalDispatch(ctx context.Context, dispatch *Dispatch, voice string return "", err } - text, err := textGenerator.GenerateText(ctx, prompt, summarize.DefaultModel) + text, err := textGenerator.GenerateText(ctx, prompt, model) if err != nil { return "", fmt.Errorf("generate dispatch text: %w", err) } diff --git a/cmd/entire/cli/dispatch/generate_test.go b/cmd/entire/cli/dispatch/generate_test.go index a89eb0271c..6202b5ccf9 100644 --- a/cmd/entire/cli/dispatch/generate_test.go +++ b/cmd/entire/cli/dispatch/generate_test.go @@ -10,10 +10,10 @@ import ( ) func TestGenerateLocalDispatch_UsesVoiceAndBullets(t *testing.T) { - mock := &stubTextGenerator{text: "generated dispatch"} - oldFactory := dispatchTextGeneratorFactory - dispatchTextGeneratorFactory = func() (dispatchTextGenerator, error) { return mock, nil } - t.Cleanup(func() { dispatchTextGeneratorFactory = oldFactory }) + t.Parallel() + + mock := &stubTextGenerator{text: " generated dispatch\n"} + var generator TextGenerator = mock dispatch := &Dispatch{ Repos: []RepoGroup{{ @@ -27,13 +27,20 @@ func TestGenerateLocalDispatch_UsesVoiceAndBullets(t *testing.T) { }}, } - got, err := generateLocalDispatch(context.Background(), dispatch, "marvin") + expectedPrompt, err := buildDispatchPrompt(dispatch, "marvin") + if err != nil { + t.Fatal(err) + } + got, err := generateLocalDispatch(context.Background(), dispatch, "marvin", generator, "test-model") if err != nil { t.Fatal(err) } if got != "generated dispatch" { t.Fatalf("unexpected text: %q", got) } + if mock.prompt != expectedPrompt { + t.Fatalf("unexpected prompt:\n%s\nwant:\n%s", mock.prompt, expectedPrompt) + } if !strings.Contains(mock.prompt, "You write concise markdown engineering dispatches.") { t.Fatalf("missing server instruction block in prompt: %s", mock.prompt) } @@ -52,9 +59,14 @@ func TestGenerateLocalDispatch_UsesVoiceAndBullets(t *testing.T) { if !strings.Contains(mock.prompt, "Write the final dispatch in markdown.") { t.Fatalf("missing final dispatch instruction in prompt: %s", mock.prompt) } + if mock.model != "test-model" { + t.Fatalf("unexpected model: %q", mock.model) + } } func TestBuildDispatchPrompt_SanitizesVoiceAndEscapesPromptTags(t *testing.T) { + t.Parallel() + dispatch := &Dispatch{ CoveredRepos: []string{"entireio/cli"}, Repos: []RepoGroup{{ @@ -257,26 +269,43 @@ func TestMarshalDispatchPromptPayload_OmitsRepoURLWhenFullNameSanitized(t *testi } func TestGenerateLocalDispatch_PropagatesGeneratorError(t *testing.T) { - oldFactory := dispatchTextGeneratorFactory - dispatchTextGeneratorFactory = func() (dispatchTextGenerator, error) { - return &stubTextGenerator{err: errors.New("boom")}, nil + t.Parallel() + + providerErr := errors.New("boom") + _, err := generateLocalDispatch( + context.Background(), + &Dispatch{}, + "", + &stubTextGenerator{err: providerErr}, + "test-model", + ) + if !errors.Is(err, providerErr) { + t.Fatalf("expected wrapped provider error, got %v", err) + } + if err.Error() != "generate dispatch text: boom" { + t.Fatalf("unexpected error: %v", err) } - t.Cleanup(func() { dispatchTextGeneratorFactory = oldFactory }) +} + +func TestGenerateLocalDispatch_RejectsNilGenerator(t *testing.T) { + t.Parallel() - _, err := generateLocalDispatch(context.Background(), &Dispatch{}, "") - if err == nil || !strings.Contains(err.Error(), "boom") { - t.Fatalf("expected generator error, got %v", err) + _, err := generateLocalDispatch(context.Background(), &Dispatch{}, "", nil, "test-model") + if err == nil || err.Error() != "local dispatch text generator is not configured" { + t.Fatalf("unexpected error: %v", err) } } type stubTextGenerator struct { prompt string + model string text string err error } -func (s *stubTextGenerator) GenerateText(_ context.Context, prompt string, _ string) (string, error) { +func (s *stubTextGenerator) GenerateText(_ context.Context, prompt string, model string) (string, error) { s.prompt = prompt + s.model = model if s.err != nil { return "", s.err } diff --git a/cmd/entire/cli/dispatch/mode_local.go b/cmd/entire/cli/dispatch/mode_local.go index d4015036a0..cf2a93d291 100644 --- a/cmd/entire/cli/dispatch/mode_local.go +++ b/cmd/entire/cli/dispatch/mode_local.go @@ -89,7 +89,7 @@ func runLocal(ctx context.Context, opts Options) (*Dispatch, error) { }, } - text, err := generateLocalDispatch(ctx, dispatch, opts.Voice) + text, err := generateLocalDispatch(ctx, dispatch, opts.Voice, opts.TextGenerator, opts.Model) if err != nil { return nil, err } diff --git a/cmd/entire/cli/dispatch/mode_local_test.go b/cmd/entire/cli/dispatch/mode_local_test.go index dd1af532eb..b4d899d5f2 100644 --- a/cmd/entire/cli/dispatch/mode_local_test.go +++ b/cmd/entire/cli/dispatch/mode_local_test.go @@ -22,7 +22,6 @@ import ( func TestLocalMode_EnumeratesCheckpoints(t *testing.T) { dir := t.TempDir() - stubGeneratedLocalDispatch(t) testutil.InitRepo(t, dir) testutil.WriteFile(t, dir, "a.txt", "x") testutil.GitAdd(t, dir, "a.txt") @@ -47,9 +46,10 @@ func TestLocalMode_EnumeratesCheckpoints(t *testing.T) { t.Chdir(dir) got, err := Run(context.Background(), Options{ - Mode: ModeLocal, - Since: "7d", - Branches: []string{"main"}, + Mode: ModeLocal, + Since: "7d", + Branches: []string{"main"}, + TextGenerator: stubGeneratedLocalDispatch(), }) if err != nil { t.Fatal(err) @@ -74,7 +74,6 @@ func TestLocalMode_EnumeratesCheckpoints(t *testing.T) { func TestLocalMode_ExplicitRepoUsesTargetRepoCheckpointSettings(t *testing.T) { cwdDir := t.TempDir() targetDir := t.TempDir() - stubGeneratedLocalDispatch(t) testutil.InitRepo(t, cwdDir) if err := os.MkdirAll(filepath.Join(cwdDir, ".entire"), 0o755); err != nil { @@ -112,10 +111,11 @@ func TestLocalMode_ExplicitRepoUsesTargetRepoCheckpointSettings(t *testing.T) { t.Chdir(cwdDir) got, err := Run(context.Background(), Options{ - Mode: ModeLocal, - RepoPaths: []string{targetDir}, - Since: "7d", - Branches: []string{"main"}, + Mode: ModeLocal, + RepoPaths: []string{targetDir}, + Since: "7d", + Branches: []string{"main"}, + TextGenerator: stubGeneratedLocalDispatch(), }) if err != nil { t.Fatal(err) @@ -130,7 +130,6 @@ func TestLocalMode_ExplicitRepoUsesTargetRepoCheckpointSettings(t *testing.T) { func TestLocalMode_UsesUntilWindow(t *testing.T) { dir := t.TempDir() - stubGeneratedLocalDispatch(t) testutil.InitRepo(t, dir) testutil.WriteFile(t, dir, "a.txt", "x") testutil.GitAdd(t, dir, "a.txt") @@ -155,10 +154,11 @@ func TestLocalMode_UsesUntilWindow(t *testing.T) { t.Chdir(dir) got, err := Run(context.Background(), Options{ - Mode: ModeLocal, - Since: "7d", - Until: now.Add(-time.Hour).Format(time.RFC3339), - Branches: []string{"main"}, + Mode: ModeLocal, + Since: "7d", + Until: now.Add(-time.Hour).Format(time.RFC3339), + Branches: []string{"main"}, + TextGenerator: stubGeneratedLocalDispatch(), }) if err != nil { t.Fatal(err) @@ -170,7 +170,6 @@ func TestLocalMode_UsesUntilWindow(t *testing.T) { func TestLocalMode_FallsBackToCommitSubjectWhenSummaryMissing(t *testing.T) { dir := t.TempDir() - stubGeneratedLocalDispatch(t) testutil.InitRepo(t, dir) testutil.WriteFile(t, dir, "a.txt", "x") testutil.GitAdd(t, dir, "a.txt") @@ -199,9 +198,10 @@ func TestLocalMode_FallsBackToCommitSubjectWhenSummaryMissing(t *testing.T) { t.Chdir(dir) got, err := Run(context.Background(), Options{ - Mode: ModeLocal, - Since: "7d", - Branches: []string{"main"}, + Mode: ModeLocal, + Since: "7d", + Branches: []string{"main"}, + TextGenerator: stubGeneratedLocalDispatch(), }) if err != nil { t.Fatal(err) @@ -232,23 +232,20 @@ func TestLocalMode_GenerateProducesInlineText(t *testing.T) { }) oldNow := nowUTC - oldFactory := dispatchTextGeneratorFactory nowUTC = func() time.Time { return createdAt.Add(2 * time.Hour) } mock := &stubTextGenerator{text: "generated inline dispatch"} - dispatchTextGeneratorFactory = func() (dispatchTextGenerator, error) { - return mock, nil - } t.Cleanup(func() { nowUTC = oldNow - dispatchTextGeneratorFactory = oldFactory }) t.Chdir(dir) got, err := Run(context.Background(), Options{ - Mode: ModeLocal, - Since: "7d", - Branches: []string{"main"}, + Mode: ModeLocal, + Since: "7d", + Branches: []string{"main"}, + TextGenerator: mock, + Model: "test-model", }) if err != nil { t.Fatal(err) @@ -256,6 +253,9 @@ func TestLocalMode_GenerateProducesInlineText(t *testing.T) { if got.GeneratedText != "generated inline dispatch" { t.Fatalf("expected generated text, got %q", got.GeneratedText) } + if mock.model != "test-model" { + t.Fatalf("unexpected model: %q", mock.model) + } } func TestLocalMode_FailsWhenGeneratedMarkdownIsEmpty(t *testing.T) { @@ -276,22 +276,18 @@ func TestLocalMode_FailsWhenGeneratedMarkdownIsEmpty(t *testing.T) { }) oldNow := nowUTC - oldFactory := dispatchTextGeneratorFactory nowUTC = func() time.Time { return createdAt.Add(2 * time.Hour) } - dispatchTextGeneratorFactory = func() (dispatchTextGenerator, error) { - return &stubTextGenerator{text: " \n\t "}, nil - } t.Cleanup(func() { nowUTC = oldNow - dispatchTextGeneratorFactory = oldFactory }) t.Chdir(dir) _, err := Run(context.Background(), Options{ - Mode: ModeLocal, - Since: "7d", - Branches: []string{"main"}, + Mode: ModeLocal, + Since: "7d", + Branches: []string{"main"}, + TextGenerator: &stubTextGenerator{text: " \n\t "}, }) if err == nil { t.Fatal("expected error when local generation returns empty markdown") @@ -303,7 +299,6 @@ func TestLocalMode_FailsWhenGeneratedMarkdownIsEmpty(t *testing.T) { func TestLocalMode_ImplicitCurrentBranchUsesHEADReachability(t *testing.T) { dir := t.TempDir() - stubGeneratedLocalDispatch(t) testutil.InitRepo(t, dir) testutil.WriteFile(t, dir, "a.txt", "x") testutil.GitAdd(t, dir, "a.txt") @@ -360,6 +355,7 @@ func TestLocalMode_ImplicitCurrentBranchUsesHEADReachability(t *testing.T) { Since: "7d", Branches: []string{"entire-dispatch-codex"}, ImplicitCurrentBranch: true, + TextGenerator: stubGeneratedLocalDispatch(), }) if err != nil { t.Fatal(err) @@ -371,7 +367,6 @@ func TestLocalMode_ImplicitCurrentBranchUsesHEADReachability(t *testing.T) { func TestLocalMode_ExplicitBranchesRemainExact(t *testing.T) { dir := t.TempDir() - stubGeneratedLocalDispatch(t) testutil.InitRepo(t, dir) testutil.WriteFile(t, dir, "a.txt", "x") testutil.GitAdd(t, dir, "a.txt") @@ -423,9 +418,10 @@ func TestLocalMode_ExplicitBranchesRemainExact(t *testing.T) { t.Chdir(dir) got, err := Run(context.Background(), Options{ - Mode: ModeLocal, - Since: "7d", - Branches: []string{"entire-dispatch-codex"}, + Mode: ModeLocal, + Since: "7d", + Branches: []string{"entire-dispatch-codex"}, + TextGenerator: stubGeneratedLocalDispatch(), }) if err != nil { t.Fatal(err) @@ -437,7 +433,6 @@ func TestLocalMode_ExplicitBranchesRemainExact(t *testing.T) { func TestLocalMode_ImplicitCurrentBranchUsesCheckpointBranchWithoutTrailerReachability(t *testing.T) { dir := t.TempDir() - stubGeneratedLocalDispatch(t) testutil.InitRepo(t, dir) testutil.WriteFile(t, dir, "a.txt", "x") testutil.GitAdd(t, dir, "a.txt") @@ -468,6 +463,7 @@ func TestLocalMode_ImplicitCurrentBranchUsesCheckpointBranchWithoutTrailerReacha Since: "7d", Branches: []string{"entire-dispatch-codex"}, ImplicitCurrentBranch: true, + TextGenerator: stubGeneratedLocalDispatch(), }) if err != nil { t.Fatal(err) @@ -479,7 +475,6 @@ func TestLocalMode_ImplicitCurrentBranchUsesCheckpointBranchWithoutTrailerReacha func TestLocalMode_ImplicitCurrentBranchExcludesDefaultBranchHistory(t *testing.T) { dir := t.TempDir() - stubGeneratedLocalDispatch(t) testutil.InitRepo(t, dir) addOriginRemote(t, dir) @@ -522,6 +517,7 @@ func TestLocalMode_ImplicitCurrentBranchExcludesDefaultBranchHistory(t *testing. Since: "7d", Branches: []string{"my-feature"}, ImplicitCurrentBranch: true, + TextGenerator: stubGeneratedLocalDispatch(), }) if err != nil { t.Fatal(err) @@ -540,7 +536,6 @@ func TestLocalMode_ImplicitCurrentBranchExcludesDefaultBranchHistory(t *testing. func TestLocalMode_AllBranchesRestrictsToLocalBranches(t *testing.T) { dir := t.TempDir() - stubGeneratedLocalDispatch(t) testutil.InitRepo(t, dir) testutil.WriteFile(t, dir, "a.txt", "x") testutil.GitAdd(t, dir, "a.txt") @@ -572,9 +567,10 @@ func TestLocalMode_AllBranchesRestrictsToLocalBranches(t *testing.T) { t.Chdir(dir) got, err := Run(context.Background(), Options{ - Mode: ModeLocal, - Since: "7d", - AllBranches: true, + Mode: ModeLocal, + Since: "7d", + AllBranches: true, + TextGenerator: stubGeneratedLocalDispatch(), }) if err != nil { t.Fatal(err) @@ -808,16 +804,8 @@ type seededCheckpoint struct { outcome string } -func stubGeneratedLocalDispatch(t *testing.T) { - t.Helper() - - oldFactory := dispatchTextGeneratorFactory - dispatchTextGeneratorFactory = func() (dispatchTextGenerator, error) { - return &stubTextGenerator{text: "generated dispatch"}, nil - } - t.Cleanup(func() { - dispatchTextGeneratorFactory = oldFactory - }) +func stubGeneratedLocalDispatch() TextGenerator { + return &stubTextGenerator{text: "generated dispatch"} } func seedCommittedCheckpoint(t *testing.T, repoDir string, cp seededCheckpoint) { diff --git a/cmd/entire/cli/dispatch_test.go b/cmd/entire/cli/dispatch_test.go index 5b16ba988a..c8495e5a14 100644 --- a/cmd/entire/cli/dispatch_test.go +++ b/cmd/entire/cli/dispatch_test.go @@ -3,10 +3,12 @@ package cli import ( "bytes" "context" + "errors" "io" "strings" "testing" + "github.com/entireio/cli/cmd/entire/cli/agent" dispatchpkg "github.com/entireio/cli/cmd/entire/cli/dispatch" "github.com/spf13/cobra" ) @@ -201,6 +203,299 @@ func TestNewDispatchCmd_LocalHelpText(t *testing.T) { } } +func TestNewDispatchCmd_AgentFlagHelpText(t *testing.T) { + t.Parallel() + + cmd := newDispatchCmd() + flag := cmd.Flags().Lookup("agent") + if flag == nil { + t.Fatal("expected --agent flag to be registered") + } + want := "local text-generation agent (requires --local)" + if flag.Usage != want { + t.Fatalf("unexpected --agent help text: %q", flag.Usage) + } + if modelFlag := cmd.Flags().Lookup("model"); modelFlag != nil { + t.Fatal("did not expect --model flag to be registered") + } +} + +func TestNewDispatchCmd_LongHelpIncludesLocalAgentExample(t *testing.T) { + t.Parallel() + + cmd := newDispatchCmd() + if !strings.Contains(cmd.Long, "entire dispatch --local --agent codex") { + t.Fatalf("long help missing local-agent example:\n%s", cmd.Long) + } +} + +func TestNewDispatchCmd_CloudAgentFailsBeforeProviderOrDispatch(t *testing.T) { + oldProvider := resolveDispatchProvider + oldRunDispatch := runDispatch + unexpectedCallErr := errors.New("unexpected command dependency call") + resolveDispatchProvider = func(context.Context, io.Writer, string) (*checkpointSummaryProvider, error) { + t.Fatal("local provider resolution must not run for cloud --agent validation") + return nil, unexpectedCallErr + } + runDispatch = func(context.Context, dispatchpkg.Options) (*dispatchpkg.Dispatch, error) { + t.Fatal("dispatch must not run after invalid cloud --agent validation") + return nil, unexpectedCallErr + } + t.Cleanup(func() { + resolveDispatchProvider = oldProvider + runDispatch = oldRunDispatch + }) + + cmd := newDispatchCmd() + cmd.SilenceErrors = true + cmd.SilenceUsage = true + cmd.SetArgs([]string{"--agent", string(agent.AgentNameCodex)}) + + err := cmd.Execute() + if err == nil { + t.Fatal("expected cloud --agent validation error") + } + want := "--agent only applies to --local (cloud dispatch uses Entire's server-side generator)" + if err.Error() != want { + t.Fatalf("unexpected error: %q", err) + } +} + +func TestNewDispatchCmd_CloudExplicitEmptyAgentUsesLocalOnlyErrorPrecedence(t *testing.T) { + oldProvider := resolveDispatchProvider + oldRunDispatch := runDispatch + providerCalled := false + dispatchCalled := false + unexpectedCallErr := errors.New("unexpected command dependency call") + resolveDispatchProvider = func(context.Context, io.Writer, string) (*checkpointSummaryProvider, error) { + providerCalled = true + return nil, unexpectedCallErr + } + runDispatch = func(context.Context, dispatchpkg.Options) (*dispatchpkg.Dispatch, error) { + dispatchCalled = true + return nil, unexpectedCallErr + } + t.Cleanup(func() { + resolveDispatchProvider = oldProvider + runDispatch = oldRunDispatch + }) + + cmd := newDispatchCmd() + cmd.SilenceErrors = true + cmd.SilenceUsage = true + cmd.SetArgs([]string{"--agent="}) + + err := cmd.Execute() + want := "--agent only applies to --local (cloud dispatch uses Entire's server-side generator)" + if err == nil || err.Error() != want { + t.Fatalf("unexpected error: %v", err) + } + if providerCalled { + t.Fatal("local provider resolution must not run for cloud --agent validation") + } + if dispatchCalled { + t.Fatal("dispatch must not run after invalid cloud --agent validation") + } +} + +func TestNewDispatchCmd_LocalExplicitEmptyAgentFailsBeforeProviderOrDispatch(t *testing.T) { + oldProvider := resolveDispatchProvider + oldRunDispatch := runDispatch + providerCalled := false + dispatchCalled := false + unexpectedCallErr := errors.New("unexpected command dependency call") + resolveDispatchProvider = func(context.Context, io.Writer, string) (*checkpointSummaryProvider, error) { + providerCalled = true + return nil, unexpectedCallErr + } + runDispatch = func(context.Context, dispatchpkg.Options) (*dispatchpkg.Dispatch, error) { + dispatchCalled = true + return nil, unexpectedCallErr + } + t.Cleanup(func() { + resolveDispatchProvider = oldProvider + runDispatch = oldRunDispatch + }) + + for _, args := range [][]string{ + {"--local", "--all-branches", "--agent="}, + {"--local", "--all-branches", "--agent", " "}, + } { + providerCalled = false + dispatchCalled = false + cmd := newDispatchCmd() + cmd.SilenceErrors = true + cmd.SilenceUsage = true + cmd.SetArgs(args) + + err := cmd.Execute() + if err == nil || err.Error() != "--agent requires a non-empty value" { + t.Fatalf("args %q: unexpected error: %v", args, err) + } + if providerCalled { + t.Fatalf("args %q: provider resolution must not run", args) + } + if dispatchCalled { + t.Fatalf("args %q: dispatch must not run", args) + } + } +} + +func TestNewDispatchCmd_LocalAgentInjectsProviderAndKeepsOutputSeparated(t *testing.T) { + oldProvider := resolveDispatchProvider + oldRunDispatch := runDispatch + oldTerminalMode := dispatchTerminalMode + oldMarkdown := renderDispatchMarkdown + generator := &stubTextAgent{} + resolveDispatchProvider = func(_ context.Context, w io.Writer, override string) (*checkpointSummaryProvider, error) { + if override != string(agent.AgentNameCodex) { + t.Fatalf("provider override = %q, want codex", override) + } + if _, err := io.WriteString(w, "provider notice\n"); err != nil { + t.Fatal(err) + } + return &checkpointSummaryProvider{TextGenerator: generator, Model: "exact-model"}, nil + } + runDispatch = func(_ context.Context, opts dispatchpkg.Options) (*dispatchpkg.Dispatch, error) { + if opts.Mode != dispatchpkg.ModeLocal { + t.Fatalf("mode = %v, want local", opts.Mode) + } + if opts.TextGenerator != generator { + t.Fatalf("TextGenerator = %T, want raw provider generator", opts.TextGenerator) + } + if opts.Model != "exact-model" { + t.Fatalf("Model = %q, want exact-model", opts.Model) + } + return &dispatchpkg.Dispatch{GeneratedText: "generated dispatch"}, nil + } + dispatchTerminalMode = func(io.Writer) bool { return false } + renderDispatchMarkdown = func(*dispatchpkg.Dispatch) string { return testDispatchGeneratedMarkdown } + t.Cleanup(func() { + resolveDispatchProvider = oldProvider + runDispatch = oldRunDispatch + dispatchTerminalMode = oldTerminalMode + renderDispatchMarkdown = oldMarkdown + }) + + cmd := newDispatchCmd() + cmd.SilenceErrors = true + cmd.SilenceUsage = true + var stdout bytes.Buffer + var stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs([]string{"--local", "--all-branches", "--agent", " " + string(agent.AgentNameCodex) + " "}) + + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + if got := stdout.String(); got != testDispatchGeneratedMarkdown { + t.Fatalf("unexpected stdout: %q", got) + } + if got := stderr.String(); got != "provider notice\n" { + t.Fatalf("unexpected stderr: %q", got) + } +} + +func TestNewDispatchCmd_LocalWithoutAgentResolvesConfiguredProvider(t *testing.T) { + oldProvider := resolveDispatchProvider + oldRunDispatch := runDispatch + oldTerminalMode := dispatchTerminalMode + resolveDispatchProvider = func(_ context.Context, _ io.Writer, override string) (*checkpointSummaryProvider, error) { + if override != "" { + t.Fatalf("provider override = %q, want empty", override) + } + return &checkpointSummaryProvider{TextGenerator: &stubTextAgent{}}, nil + } + runDispatch = func(context.Context, dispatchpkg.Options) (*dispatchpkg.Dispatch, error) { + return &dispatchpkg.Dispatch{}, nil + } + dispatchTerminalMode = func(io.Writer) bool { return false } + t.Cleanup(func() { + resolveDispatchProvider = oldProvider + runDispatch = oldRunDispatch + dispatchTerminalMode = oldTerminalMode + }) + + cmd := newDispatchCmd() + cmd.SetArgs([]string{"--local", "--all-branches"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } +} + +func TestNewDispatchCmd_CloudDispatchDoesNotResolveLocalProvider(t *testing.T) { + oldProvider := resolveDispatchProvider + oldRunDispatch := runDispatch + oldTerminalMode := dispatchTerminalMode + unexpectedCallErr := errors.New("unexpected local provider resolution") + resolveDispatchProvider = func(context.Context, io.Writer, string) (*checkpointSummaryProvider, error) { + t.Fatal("normal cloud dispatch must not resolve a local provider") + return nil, unexpectedCallErr + } + runDispatch = func(_ context.Context, opts dispatchpkg.Options) (*dispatchpkg.Dispatch, error) { + if opts.Mode != dispatchpkg.ModeServer { + t.Fatalf("mode = %v, want server", opts.Mode) + } + return &dispatchpkg.Dispatch{}, nil + } + dispatchTerminalMode = func(io.Writer) bool { return false } + t.Cleanup(func() { + resolveDispatchProvider = oldProvider + runDispatch = oldRunDispatch + dispatchTerminalMode = oldTerminalMode + }) + + cmd := newDispatchCmd() + cmd.SetArgs([]string{"--repos", "entireio/cli"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } +} + +func TestNewDispatchCmd_ProviderErrorUsesStderrAndSkipsDispatch(t *testing.T) { + oldProvider := resolveDispatchProvider + oldRunDispatch := runDispatch + unexpectedCallErr := errors.New("unexpected dispatch call") + resolveDispatchProvider = func(_ context.Context, w io.Writer, override string) (*checkpointSummaryProvider, error) { + if override != string(agent.AgentNameCodex) { + t.Fatalf("provider override = %q, want codex", override) + } + if _, err := io.WriteString(w, "provider warning\n"); err != nil { + t.Fatal(err) + } + return nil, errors.New("provider failed") + } + runDispatch = func(context.Context, dispatchpkg.Options) (*dispatchpkg.Dispatch, error) { + t.Fatal("dispatch must not run after provider resolution fails") + return nil, unexpectedCallErr + } + t.Cleanup(func() { + resolveDispatchProvider = oldProvider + runDispatch = oldRunDispatch + }) + + cmd := newDispatchCmd() + cmd.SilenceErrors = true + cmd.SilenceUsage = true + var stdout bytes.Buffer + var stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs([]string{"--local", "--all-branches", "--agent", string(agent.AgentNameCodex)}) + + err := cmd.Execute() + if err == nil || err.Error() != "provider failed" { + t.Fatalf("unexpected error: %v", err) + } + if got := stdout.String(); got != "" { + t.Fatalf("unexpected stdout: %q", got) + } + if got := stderr.String(); got != "provider warning\n" { + t.Fatalf("unexpected stderr: %q", got) + } +} + func TestShouldRunDispatchWizard(t *testing.T) { t.Parallel() diff --git a/cmd/entire/cli/explain_summary_provider.go b/cmd/entire/cli/explain_summary_provider.go index 27c15b601f..937afc9da0 100644 --- a/cmd/entire/cli/explain_summary_provider.go +++ b/cmd/entire/cli/explain_summary_provider.go @@ -20,14 +20,15 @@ import ( ) var ( - loadSummarySettings = LoadEntireSettings - loadSummarySettingsFromFile = settings.LoadFromFile - saveLocalSummarySettings = SaveEntireSettingsLocal - getSummaryAgent = agent.Get - listRegisteredAgents = agent.List - isSummaryCLIAvailable = agent.IsSummaryCLIAvailable - discoverSummaryProviders = external.DiscoverAndRegister - discoverSummaryProvidersAlways = external.DiscoverAndRegisterAlways + loadSummarySettings = LoadEntireSettings + loadSummarySettingsFromFile = settings.LoadFromFile + saveLocalSummarySettings = SaveEntireSettingsLocal + getSummaryAgent = agent.Get + listRegisteredAgents = agent.List + isSummaryCLIAvailable = agent.IsSummaryCLIAvailable + discoverSummaryProviders = external.DiscoverAndRegister + discoverSummaryProvidersAlways = external.DiscoverAndRegisterAlways + discoverDispatchSummaryProvider = external.DiscoverAndRegisterNamedAlways ) type checkpointSummaryProvider struct { @@ -45,8 +46,10 @@ func resolveDispatchSummaryProvider(ctx context.Context, w io.Writer, override s } providerName := types.AgentName(override) - discoverSummaryProviderIfMissing(ctx, providerName) - if err := ensureSummaryProviderPresent(ctx, providerName); err != nil { + if _, err := getSummaryAgent(providerName); err != nil { + discoverDispatchSummaryProvider(ctx, providerName) + } + if err := validateSummaryProvider(override); err != nil { return nil, err } return buildCheckpointSummaryProviderWithEffectiveModel(providerName, "") @@ -238,7 +241,7 @@ func validateSummaryProvider(provider string) error { return fmt.Errorf("agent %q does not support summary generation", provider) } if !isSummaryProviderAvailable(name, ag) { - return fmt.Errorf("summary provider %q is configured but its CLI binary is not on PATH; install it or choose another provider", provider) + return fmt.Errorf("summary provider %q CLI binary is not on PATH; install it or choose another provider", provider) } return nil } diff --git a/cmd/entire/cli/explain_summary_provider_test.go b/cmd/entire/cli/explain_summary_provider_test.go index 2e440ef202..bd0aeaace5 100644 --- a/cmd/entire/cli/explain_summary_provider_test.go +++ b/cmd/entire/cli/explain_summary_provider_test.go @@ -86,6 +86,21 @@ type stubNonTextAgent struct { agent.Agent } +func writeInfoSentinelExternalAgentBinary(t *testing.T, dir, name string) { + t.Helper() + + script := `#!/bin/sh +if [ "$1" = "info" ]; then + : > "$ENTIRE_TEST_UNRELATED_INFO_SENTINEL" + exit 1 +fi +echo '{}' +` + if err := os.WriteFile(filepath.Join(dir, "entire-agent-"+name), []byte(script), 0o755); err != nil { + t.Fatalf("write unrelated external agent binary: %v", err) + } +} + func TestResolveCheckpointSummaryProvider_UsesConfiguredProvider(t *testing.T) { // Cannot use t.Parallel() because we use t.Chdir and package-level var stubs ctx := context.Background() @@ -150,14 +165,14 @@ func TestResolveDispatchSummaryProvider_ExplicitCodexUsesDefaultModelWithoutPers originalSave := saveLocalSummarySettings originalGet := getSummaryAgent originalCLI := isSummaryCLIAvailable - originalDiscover := discoverSummaryProvidersAlways + originalDiscover := discoverDispatchSummaryProvider t.Cleanup(func() { loadSummarySettings = originalLoad loadSummarySettingsFromFile = originalLoadFile saveLocalSummarySettings = originalSave getSummaryAgent = originalGet isSummaryCLIAvailable = originalCLI - discoverSummaryProvidersAlways = originalDiscover + discoverDispatchSummaryProvider = originalDiscover }) loadSummarySettings = func(context.Context) (*settings.EntireSettings, error) { @@ -181,7 +196,7 @@ func TestResolveDispatchSummaryProvider_ExplicitCodexUsesDefaultModelWithoutPers isSummaryCLIAvailable = func(name types.AgentName) bool { return name == agent.AgentNameCodex } - discoverSummaryProvidersAlways = func(context.Context) { + discoverDispatchSummaryProvider = func(context.Context, types.AgentName) { t.Fatal("registered explicit provider should not trigger external discovery") } @@ -327,12 +342,13 @@ func TestResolveDispatchSummaryProvider_ExplicitClaudePreservesEmptyModel(t *tes func TestResolveDispatchSummaryProvider_ExplicitValidationErrors(t *testing.T) { // Cannot use t.Parallel(): subtests mutate package-level resolution seams. tests := []struct { - name string - override string - agent agent.Agent - getErr error - available bool - wantError string + name string + override string + agent agent.Agent + getErr error + available bool + wantError string + unwantedError string }{ { name: "unknown provider", @@ -352,11 +368,12 @@ func TestResolveDispatchSummaryProvider_ExplicitValidationErrors(t *testing.T) { wantError: "does not support summary generation", }, { - name: "CLI unavailable", - override: string(agent.AgentNameCodex), - agent: &stubTextAgent{name: agent.AgentNameCodex, kind: agent.AgentTypeCodex}, - available: false, - wantError: "not on PATH", + name: "CLI unavailable", + override: string(agent.AgentNameCodex), + agent: &stubTextAgent{name: agent.AgentNameCodex, kind: agent.AgentTypeCodex}, + available: false, + wantError: "install it or choose another provider", + unwantedError: "configured", }, } @@ -364,11 +381,11 @@ func TestResolveDispatchSummaryProvider_ExplicitValidationErrors(t *testing.T) { t.Run(tt.name, func(t *testing.T) { originalGet := getSummaryAgent originalCLI := isSummaryCLIAvailable - originalDiscover := discoverSummaryProvidersAlways + originalDiscover := discoverDispatchSummaryProvider t.Cleanup(func() { getSummaryAgent = originalGet isSummaryCLIAvailable = originalCLI - discoverSummaryProvidersAlways = originalDiscover + discoverDispatchSummaryProvider = originalDiscover }) getSummaryAgent = func(types.AgentName) (agent.Agent, error) { @@ -378,7 +395,7 @@ func TestResolveDispatchSummaryProvider_ExplicitValidationErrors(t *testing.T) { return tt.agent, nil } isSummaryCLIAvailable = func(types.AgentName) bool { return tt.available } - discoverSummaryProvidersAlways = func(context.Context) {} + discoverDispatchSummaryProvider = func(context.Context, types.AgentName) {} _, err := resolveDispatchSummaryProvider(context.Background(), &bytes.Buffer{}, tt.override) if err == nil { @@ -387,6 +404,9 @@ func TestResolveDispatchSummaryProvider_ExplicitValidationErrors(t *testing.T) { if !strings.Contains(err.Error(), tt.wantError) { t.Fatalf("resolveDispatchSummaryProvider(%q) error = %q, want substring %q", tt.override, err, tt.wantError) } + if tt.unwantedError != "" && strings.Contains(err.Error(), tt.unwantedError) { + t.Fatalf("resolveDispatchSummaryProvider(%q) error = %q, do not want substring %q", tt.override, err, tt.unwantedError) + } }) } } @@ -433,7 +453,12 @@ func TestResolveDispatchSummaryProvider_ExplicitExternalProviderDoesNotWriteLoca externalDir := t.TempDir() writeExternalSummaryAgentBinary(t, externalDir, tt.providerName) + writeInfoSentinelExternalAgentBinary(t, externalDir, tt.providerName+"-unrelated") t.Setenv("PATH", externalDir+string(os.PathListSeparator)+os.Getenv("PATH")) + unrelatedInfoSentinel := filepath.Join(t.TempDir(), "unrelated-info-called") + t.Setenv("ENTIRE_TEST_UNRELATED_INFO_SENTINEL", unrelatedInfoSentinel) + modelRecord := filepath.Join(t.TempDir(), "model-args") + t.Setenv("ENTIRE_TEST_EXTERNAL_MODEL_RECORD", modelRecord) provider, err := resolveDispatchSummaryProvider(ctx, &bytes.Buffer{}, tt.providerName) if err != nil { @@ -448,6 +473,24 @@ func TestResolveDispatchSummaryProvider_ExplicitExternalProviderDoesNotWriteLoca if provider.TextGenerator == nil { t.Fatal("provider.TextGenerator = nil, want external raw generator") } + if _, err := os.Stat(unrelatedInfoSentinel); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("unrelated plugin info was invoked: stat error = %v", err) + } + + generated, err := provider.TextGenerator.GenerateText(ctx, "generate a summary", provider.Model) + if err != nil { + t.Fatalf("provider.TextGenerator.GenerateText() error = %v", err) + } + if !strings.Contains(generated, `"intent":"Intent"`) { + t.Fatalf("provider.TextGenerator.GenerateText() = %q, want generated summary", generated) + } + modelArgs, err := os.ReadFile(modelRecord) + if err != nil { + t.Fatalf("read external model args: %v", err) + } + if string(modelArgs) != "--model\n\n" { + t.Fatalf("external generate-text args = %q, want empty model argument", modelArgs) + } got, err := os.ReadFile(localPath) switch { diff --git a/cmd/entire/cli/setup_test.go b/cmd/entire/cli/setup_test.go index c9c1175b44..1255289548 100644 --- a/cmd/entire/cli/setup_test.go +++ b/cmd/entire/cli/setup_test.go @@ -176,6 +176,9 @@ case "$1" in echo '{"present": true}' ;; generate-text) + if [ -n "$ENTIRE_TEST_EXTERNAL_MODEL_RECORD" ]; then + printf '%s\n%s\n' "$2" "$3" > "$ENTIRE_TEST_EXTERNAL_MODEL_RECORD" + fi echo '{"text":"{\"intent\":\"Intent\",\"outcome\":\"Outcome\",\"learnings\":{\"repo\":[],\"code\":[],\"workflow\":[]},\"friction\":[],\"open_items\":[]}"}' ;; *) From 20bb2f2b61446da6b09a79592acb176dec602e7f Mon Sep 17 00:00:00 2001 From: Peyton Montei Date: Fri, 17 Jul 2026 14:02:48 -0400 Subject: [PATCH 03/11] fix: address dispatch agent review findings Entire-Checkpoint: 01KXRKXP0E2Z7X5PVB982TEX37 --- cmd/entire/cli/agent/external/discovery.go | 7 +++++ .../cli/agent/external/discovery_test.go | 28 +++++++++++++++++++ cmd/entire/cli/explain_summary_provider.go | 2 +- .../cli/explain_summary_provider_test.go | 8 +++--- 4 files changed, 40 insertions(+), 5 deletions(-) diff --git a/cmd/entire/cli/agent/external/discovery.go b/cmd/entire/cli/agent/external/discovery.go index d48775f405..849d346b26 100644 --- a/cmd/entire/cli/agent/external/discovery.go +++ b/cmd/entire/cli/agent/external/discovery.go @@ -48,6 +48,10 @@ func DiscoverAndRegisterAlways(ctx context.Context) { // agent binary matching name. It bypasses the external_agents setting for // explicit, one-invocation selections without executing unrelated plugins. func DiscoverAndRegisterNamedAlways(ctx context.Context, name types.AgentName) { + discoverAndRegisterNamed(ctx, name, discoveryTimeout) +} + +func discoverAndRegisterNamed(ctx context.Context, name types.AgentName, timeout time.Duration) { if name == "" { return } @@ -55,6 +59,9 @@ func DiscoverAndRegisterNamedAlways(ctx context.Context, name types.AgentName) { return } + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + binPath, err := exec.LookPath(binaryPrefix + string(name)) if err != nil { return diff --git a/cmd/entire/cli/agent/external/discovery_test.go b/cmd/entire/cli/agent/external/discovery_test.go index 60057b094d..07dd751930 100644 --- a/cmd/entire/cli/agent/external/discovery_test.go +++ b/cmd/entire/cli/agent/external/discovery_test.go @@ -2,11 +2,13 @@ package external import ( "context" + "fmt" "os" "os/exec" "path/filepath" "runtime" "testing" + "time" "github.com/entireio/cli/cmd/entire/cli/agent" "github.com/entireio/cli/cmd/entire/cli/agent/types" @@ -290,6 +292,32 @@ func TestDiscoverAndRegisterAlways_FindsAgentWithoutSettings(t *testing.T) { } } +func TestDiscoverAndRegisterNamedAlways_TimesOutStalledInfo(t *testing.T) { + sleepPath, err := exec.LookPath("sleep") + if err != nil { + t.Skip("sleep not available") + } + + name := types.AgentName("disc-named-timeout") + dir := t.TempDir() + binPath := filepath.Join(dir, binaryPrefix+string(name)) + script := fmt.Sprintf("#!/bin/sh\nexec %q 60\n", sleepPath) + if err := os.WriteFile(binPath, []byte(script), 0o755); err != nil { + t.Fatalf("write stalled mock binary: %v", err) + } + t.Setenv("PATH", dir) + + const timeout = 100 * time.Millisecond + started := time.Now() + discoverAndRegisterNamed(context.Background(), name, timeout) + if elapsed := time.Since(started); elapsed > 2*time.Second { + t.Fatalf("named discovery took %v, want cancellation near %v", elapsed, timeout) + } + if _, err := agent.Get(name); err == nil { + t.Fatal("stalled external agent was registered") + } +} + func TestIsExternal_WrappedAgent(t *testing.T) { if _, err := exec.LookPath("sh"); err != nil { t.Skip("sh not available") diff --git a/cmd/entire/cli/explain_summary_provider.go b/cmd/entire/cli/explain_summary_provider.go index 937afc9da0..dc2c8d6c9c 100644 --- a/cmd/entire/cli/explain_summary_provider.go +++ b/cmd/entire/cli/explain_summary_provider.go @@ -52,7 +52,7 @@ func resolveDispatchSummaryProvider(ctx context.Context, w io.Writer, override s if err := validateSummaryProvider(override); err != nil { return nil, err } - return buildCheckpointSummaryProviderWithEffectiveModel(providerName, "") + return buildCheckpointSummaryProvider(providerName, "") } func resolveCheckpointSummaryProvider(ctx context.Context, w io.Writer) (*checkpointSummaryProvider, error) { diff --git a/cmd/entire/cli/explain_summary_provider_test.go b/cmd/entire/cli/explain_summary_provider_test.go index bd0aeaace5..b71e02f57c 100644 --- a/cmd/entire/cli/explain_summary_provider_test.go +++ b/cmd/entire/cli/explain_summary_provider_test.go @@ -302,7 +302,7 @@ func TestResolveDispatchSummaryProvider_ExplicitProviderIgnoresSavedProviderAndM } } -func TestResolveDispatchSummaryProvider_ExplicitClaudePreservesEmptyModel(t *testing.T) { +func TestResolveDispatchSummaryProvider_ExplicitClaudeUsesSummaryDefaultModel(t *testing.T) { // Cannot use t.Parallel(): mutates package-level resolution seams. ctx := context.Background() claude := &stubTextAgent{name: agent.AgentNameClaudeCode, kind: agent.AgentTypeClaudeCode} @@ -321,7 +321,7 @@ func TestResolveDispatchSummaryProvider_ExplicitClaudePreservesEmptyModel(t *tes loadCalls++ return &settings.EntireSettings{SummaryGeneration: &settings.SummaryGenerationSettings{ Provider: string(agent.AgentNameClaudeCode), - Model: "sonnet", + Model: "opus", }}, nil } getSummaryAgent = func(types.AgentName) (agent.Agent, error) { return claude, nil } @@ -334,8 +334,8 @@ func TestResolveDispatchSummaryProvider_ExplicitClaudePreservesEmptyModel(t *tes if loadCalls != 0 { t.Fatalf("loadSummarySettings calls = %d, want 0 for explicit override", loadCalls) } - if provider.Model != "" { - t.Fatalf("provider.Model = %q, want Claude CLI default rather than saved/summary default", provider.Model) + if provider.Model != summarize.DefaultModel { + t.Fatalf("provider.Model = %q, want summary default %q", provider.Model, summarize.DefaultModel) } } From ded59d389b1ed17fe2975b94ff290595a91d50c3 Mon Sep 17 00:00:00 2001 From: Peyton Montei Date: Fri, 17 Jul 2026 15:53:33 -0400 Subject: [PATCH 04/11] fix: surface explicit agent discovery errors Entire-Checkpoint: 01KXRT8F8Z6NTJXMGBQDZGFEKV --- cmd/entire/cli/agent/external/discovery.go | 72 +++++++---- .../cli/agent/external/discovery_test.go | 122 +++++++++++++++++- cmd/entire/cli/explain_summary_provider.go | 4 +- .../cli/explain_summary_provider_test.go | 113 +++++++++++++++- 4 files changed, 282 insertions(+), 29 deletions(-) diff --git a/cmd/entire/cli/agent/external/discovery.go b/cmd/entire/cli/agent/external/discovery.go index 849d346b26..594d391781 100644 --- a/cmd/entire/cli/agent/external/discovery.go +++ b/cmd/entire/cli/agent/external/discovery.go @@ -2,6 +2,8 @@ package external import ( "context" + "errors" + "fmt" "log/slog" "os" "os/exec" @@ -24,6 +26,11 @@ const ( // discoveryTimeout caps the total time spent scanning $PATH for external agents. const discoveryTimeout = 10 * time.Second +var ( + statExternalAgent = os.Stat //nolint:gochecknoglobals // narrow test seam for stat failures + lookPathExternalAgent = exec.LookPath //nolint:gochecknoglobals // narrow test seam for lookup failures +) + // DiscoverAndRegister scans $PATH for executables matching "entire-agent-", // calls their "info" subcommand, and registers them in the agent registry. // Binaries whose name conflicts with an already-registered agent are skipped. @@ -47,26 +54,34 @@ func DiscoverAndRegisterAlways(ctx context.Context) { // DiscoverAndRegisterNamedAlways discovers and registers only the external // agent binary matching name. It bypasses the external_agents setting for // explicit, one-invocation selections without executing unrelated plugins. -func DiscoverAndRegisterNamedAlways(ctx context.Context, name types.AgentName) { - discoverAndRegisterNamed(ctx, name, discoveryTimeout) +func DiscoverAndRegisterNamedAlways(ctx context.Context, name types.AgentName) error { + return discoverAndRegisterNamed(ctx, name, discoveryTimeout) } -func discoverAndRegisterNamed(ctx context.Context, name types.AgentName, timeout time.Duration) { +func discoverAndRegisterNamed(ctx context.Context, name types.AgentName, timeout time.Duration) error { if name == "" { - return + return nil } if _, err := agent.Get(name); err == nil { - return + return nil } ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() + if err := ctx.Err(); err != nil { + return fmt.Errorf("discovering external agent %q: %w", name, err) + } - binPath, err := exec.LookPath(binaryPrefix + string(name)) + binName := binaryPrefix + string(name) + binPath, err := lookPathExternalAgent(binName) if err != nil { - return + if errors.Is(err, exec.ErrNotFound) { + return nil + } + return fmt.Errorf("looking up external agent %q binary %q: %w", name, binName, err) } - registerExternalAgent(ctx, binPath, name) + _, err = registerExternalAgent(ctx, binPath, name) + return err } // discoverAndRegister contains the shared scanning logic for external agent discovery. @@ -119,37 +134,48 @@ func discoverAndRegister(ctx context.Context) { continue } - if registerExternalAgent(ctx, binPath, agentName) { + registeredAgent, err := registerExternalAgent(ctx, binPath, agentName) + if err != nil { + logging.Debug(ctx, "skipping external agent (registration failed)", + slog.String("binary", binPath), + slog.String("agent", string(agentName)), + slog.String("error", err.Error())) + continue + } + if registeredAgent { registered[agentName] = true } } } } -func registerExternalAgent(ctx context.Context, binPath string, name types.AgentName) bool { - finfo, err := os.Stat(binPath) //nolint:gosec // PATH entries are trusted - if err != nil || finfo.IsDir() { - return false +func registerExternalAgent(ctx context.Context, binPath string, name types.AgentName) (bool, error) { + finfo, err := statExternalAgent(binPath) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return false, nil + } + return false, fmt.Errorf("inspecting external agent %q binary %q: %w", name, binPath, err) + } + if finfo.IsDir() { + return false, nil } // Check executable bit (on Unix; Windows doesn't set execute bits). if runtime.GOOS != osWindows && finfo.Mode()&0o111 == 0 { - return false + return false, nil } ea, err := New(ctx, binPath) if err != nil { - logging.Debug(ctx, "skipping external agent (info failed)", - slog.String("binary", binPath), - slog.String("error", err.Error())) - return false + if ctxErr := ctx.Err(); ctxErr != nil { + return false, fmt.Errorf("loading info for external agent %q from binary %q: %w: %w", name, binPath, ctxErr, err) + } + return false, fmt.Errorf("loading info for external agent %q from binary %q: %w", name, binPath, err) } wrapped, err := Wrap(ea) if err != nil { - logging.Debug(ctx, "skipping external agent (wrap failed)", - slog.String("binary", binPath), - slog.String("error", err.Error())) - return false + return false, fmt.Errorf("wrapping external agent %q from binary %q: %w", name, binPath, err) } agent.Register(name, func() agent.Agent { return wrapped @@ -159,7 +185,7 @@ func registerExternalAgent(ctx context.Context, binPath string, name types.Agent slog.String("name", string(name)), slog.String("type", string(ea.Type())), slog.String("binary", binPath)) - return true + return true, nil } // StripExeExt removes Windows executable extensions (.exe, .bat, .cmd, .com) diff --git a/cmd/entire/cli/agent/external/discovery_test.go b/cmd/entire/cli/agent/external/discovery_test.go index 07dd751930..ed6cb1f969 100644 --- a/cmd/entire/cli/agent/external/discovery_test.go +++ b/cmd/entire/cli/agent/external/discovery_test.go @@ -2,11 +2,13 @@ package external import ( "context" + "errors" "fmt" "os" "os/exec" "path/filepath" "runtime" + "strings" "testing" "time" @@ -307,17 +309,131 @@ func TestDiscoverAndRegisterNamedAlways_TimesOutStalledInfo(t *testing.T) { } t.Setenv("PATH", dir) - const timeout = 100 * time.Millisecond + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + started := time.Now() - discoverAndRegisterNamed(context.Background(), name, timeout) + err = DiscoverAndRegisterNamedAlways(ctx, name) if elapsed := time.Since(started); elapsed > 2*time.Second { - t.Fatalf("named discovery took %v, want cancellation near %v", elapsed, timeout) + t.Fatalf("named discovery took %v, want cancellation near context deadline", elapsed) + } + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("DiscoverAndRegisterNamedAlways() error = %v, want context deadline exceeded", err) } if _, err := agent.Get(name); err == nil { t.Fatal("stalled external agent was registered") } } +func TestDiscoverAndRegisterNamedAlways_CanceledContext(t *testing.T) { + if _, err := exec.LookPath("sh"); err != nil { + t.Skip("sh not available") + } + + name := types.AgentName("disc-named-canceled") + dir := setupDiscoveryDir(t, string(name), makeInfoJSON(string(name))) + t.Setenv("PATH", dir) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := DiscoverAndRegisterNamedAlways(ctx, name) + if !errors.Is(err, context.Canceled) { + t.Fatalf("DiscoverAndRegisterNamedAlways() error = %v, want context canceled", err) + } +} + +func TestDiscoverAndRegisterNamedAlways_InvalidInfo(t *testing.T) { + if _, err := exec.LookPath("sh"); err != nil { + t.Skip("sh not available") + } + + name := types.AgentName("disc-named-invalid-info") + dir := setupDiscoveryDir(t, string(name), "not json") + t.Setenv("PATH", dir) + + err := DiscoverAndRegisterNamedAlways(context.Background(), name) + if err == nil { + t.Fatal("DiscoverAndRegisterNamedAlways() error = nil, want invalid info error") + } + if !strings.Contains(err.Error(), string(name)) { + t.Fatalf("DiscoverAndRegisterNamedAlways() error = %q, want agent name %q", err, name) + } + if !strings.Contains(err.Error(), "info: invalid JSON") { + t.Fatalf("DiscoverAndRegisterNamedAlways() error = %q, want invalid info context", err) + } +} + +func TestDiscoverAndRegisterNamedAlways_MissingHelper(t *testing.T) { + name := types.AgentName("disc-named-missing") + t.Setenv("PATH", t.TempDir()) + + if err := DiscoverAndRegisterNamedAlways(context.Background(), name); err != nil { + t.Fatalf("DiscoverAndRegisterNamedAlways() error = %v, want nil for missing helper", err) + } +} + +func TestDiscoverAndRegisterNamedAlways_StatError(t *testing.T) { + name := types.AgentName("disc-named-stat-error") + binPath := filepath.Join(t.TempDir(), binaryPrefix+string(name)) + wantErr := errors.New("stat failed") + + originalLookPath := lookPathExternalAgent + originalStat := statExternalAgent + t.Cleanup(func() { + lookPathExternalAgent = originalLookPath + statExternalAgent = originalStat + }) + lookPathExternalAgent = func(string) (string, error) { return binPath, nil } + statExternalAgent = func(string) (os.FileInfo, error) { return nil, wantErr } + + err := DiscoverAndRegisterNamedAlways(context.Background(), name) + if !errors.Is(err, wantErr) { + t.Fatalf("DiscoverAndRegisterNamedAlways() error = %v, want stat error", err) + } + if !strings.Contains(err.Error(), string(name)) { + t.Fatalf("DiscoverAndRegisterNamedAlways() error = %q, want agent name %q", err, name) + } +} + +func TestDiscoverAndRegisterNamedAlways_LookPathError(t *testing.T) { + name := types.AgentName("disc-named-lookpath-error") + wantErr := errors.New("lookup failed") + + originalLookPath := lookPathExternalAgent + t.Cleanup(func() { lookPathExternalAgent = originalLookPath }) + lookPathExternalAgent = func(string) (string, error) { return "", wantErr } + + err := DiscoverAndRegisterNamedAlways(context.Background(), name) + if !errors.Is(err, wantErr) { + t.Fatalf("DiscoverAndRegisterNamedAlways() error = %v, want lookup error", err) + } + if !strings.Contains(err.Error(), string(name)) { + t.Fatalf("DiscoverAndRegisterNamedAlways() error = %q, want agent name %q", err, name) + } +} + +func TestDiscoverAndRegister_ContinuesAfterRegistrationError(t *testing.T) { + if _, err := exec.LookPath("sh"); err != nil { + t.Skip("sh not available") + } + + badName := "disc-scan-a-invalid" + badDir := setupDiscoveryDir(t, badName, "not json") + goodName := "disc-scan-z-valid" + goodDir := setupDiscoveryDir(t, goodName, makeInfoJSON(goodName)) + t.Setenv("PATH", badDir+string(os.PathListSeparator)+goodDir) + + DiscoverAndRegisterAlways(context.Background()) + + if _, err := agent.Get(types.AgentName(badName)); err == nil { + t.Fatalf("invalid external agent %q was registered", badName) + } + if _, err := agent.Get(types.AgentName(goodName)); err != nil { + t.Fatalf("valid external agent %q was not registered after earlier failure: %v", goodName, err) + } +} + func TestIsExternal_WrappedAgent(t *testing.T) { if _, err := exec.LookPath("sh"); err != nil { t.Skip("sh not available") diff --git a/cmd/entire/cli/explain_summary_provider.go b/cmd/entire/cli/explain_summary_provider.go index dc2c8d6c9c..c2d30fca15 100644 --- a/cmd/entire/cli/explain_summary_provider.go +++ b/cmd/entire/cli/explain_summary_provider.go @@ -47,7 +47,9 @@ func resolveDispatchSummaryProvider(ctx context.Context, w io.Writer, override s providerName := types.AgentName(override) if _, err := getSummaryAgent(providerName); err != nil { - discoverDispatchSummaryProvider(ctx, providerName) + if err := discoverDispatchSummaryProvider(ctx, providerName); err != nil { + return nil, err + } } if err := validateSummaryProvider(override); err != nil { return nil, err diff --git a/cmd/entire/cli/explain_summary_provider_test.go b/cmd/entire/cli/explain_summary_provider_test.go index b71e02f57c..21523e5e73 100644 --- a/cmd/entire/cli/explain_summary_provider_test.go +++ b/cmd/entire/cli/explain_summary_provider_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "fmt" "os" "os/exec" "path/filepath" @@ -196,8 +197,9 @@ func TestResolveDispatchSummaryProvider_ExplicitCodexUsesDefaultModelWithoutPers isSummaryCLIAvailable = func(name types.AgentName) bool { return name == agent.AgentNameCodex } - discoverDispatchSummaryProvider = func(context.Context, types.AgentName) { + discoverDispatchSummaryProvider = func(context.Context, types.AgentName) error { t.Fatal("registered explicit provider should not trigger external discovery") + return nil } provider, err := resolveDispatchSummaryProvider(ctx, &bytes.Buffer{}, " codex ") @@ -339,6 +341,113 @@ func TestResolveDispatchSummaryProvider_ExplicitClaudeUsesSummaryDefaultModel(t } } +func TestResolveDispatchSummaryProvider_PropagatesDiscoveryDeadline(t *testing.T) { + // Cannot use t.Parallel(): mutates package-level resolution seams. + providerName := types.AgentName("external-discovery-deadline") + + originalGet := getSummaryAgent + originalDiscover := discoverDispatchSummaryProvider + t.Cleanup(func() { + getSummaryAgent = originalGet + discoverDispatchSummaryProvider = originalDiscover + }) + + getSummaryAgent = func(types.AgentName) (agent.Agent, error) { + return nil, errors.New("not registered") + } + discoverDispatchSummaryProvider = func(context.Context, types.AgentName) error { + return fmt.Errorf("discovering external agent %q: %w", providerName, context.DeadlineExceeded) + } + + _, err := resolveDispatchSummaryProvider(context.Background(), &bytes.Buffer{}, string(providerName)) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("resolveDispatchSummaryProvider() error = %v, want context deadline exceeded", err) + } + if strings.Contains(err.Error(), "unknown summary provider") { + t.Fatalf("resolveDispatchSummaryProvider() error = %q, do not want unknown-provider rewrite", err) + } +} + +func TestResolveDispatchSummaryProvider_PropagatesDiscoveryCancellation(t *testing.T) { + // Cannot use t.Parallel(): mutates package-level resolution seams. + providerName := types.AgentName("external-discovery-canceled") + + originalGet := getSummaryAgent + originalDiscover := discoverDispatchSummaryProvider + t.Cleanup(func() { + getSummaryAgent = originalGet + discoverDispatchSummaryProvider = originalDiscover + }) + + getSummaryAgent = func(types.AgentName) (agent.Agent, error) { + return nil, errors.New("not registered") + } + discoverDispatchSummaryProvider = func(context.Context, types.AgentName) error { + return fmt.Errorf("discovering external agent %q: %w", providerName, context.Canceled) + } + + _, err := resolveDispatchSummaryProvider(context.Background(), &bytes.Buffer{}, string(providerName)) + if !errors.Is(err, context.Canceled) { + t.Fatalf("resolveDispatchSummaryProvider() error = %v, want context canceled", err) + } + if strings.Contains(err.Error(), "unknown summary provider") { + t.Fatalf("resolveDispatchSummaryProvider() error = %q, do not want unknown-provider rewrite", err) + } +} + +func TestResolveDispatchSummaryProvider_PropagatesInvalidExternalInfo(t *testing.T) { + // Cannot use t.Parallel(): mutates package-level resolution seams. + providerName := types.AgentName("external-discovery-invalid-info") + infoErr := errors.New("invalid helper info") + + originalGet := getSummaryAgent + originalDiscover := discoverDispatchSummaryProvider + t.Cleanup(func() { + getSummaryAgent = originalGet + discoverDispatchSummaryProvider = originalDiscover + }) + + getSummaryAgent = func(types.AgentName) (agent.Agent, error) { + return nil, errors.New("not registered") + } + discoverDispatchSummaryProvider = func(context.Context, types.AgentName) error { + return fmt.Errorf("loading info for external agent %q: info: invalid JSON: %w", providerName, infoErr) + } + + _, err := resolveDispatchSummaryProvider(context.Background(), &bytes.Buffer{}, string(providerName)) + if !errors.Is(err, infoErr) { + t.Fatalf("resolveDispatchSummaryProvider() error = %v, want invalid-info cause", err) + } + if !strings.Contains(err.Error(), string(providerName)) || !strings.Contains(err.Error(), "info: invalid JSON") { + t.Fatalf("resolveDispatchSummaryProvider() error = %q, want provider and invalid-info context", err) + } + if strings.Contains(err.Error(), "unknown summary provider") { + t.Fatalf("resolveDispatchSummaryProvider() error = %q, do not want unknown-provider rewrite", err) + } +} + +func TestResolveDispatchSummaryProvider_MissingExternalKeepsUnknownProviderError(t *testing.T) { + // Cannot use t.Parallel(): mutates package-level resolution seams. + providerName := types.AgentName("external-discovery-missing") + + originalGet := getSummaryAgent + originalDiscover := discoverDispatchSummaryProvider + t.Cleanup(func() { + getSummaryAgent = originalGet + discoverDispatchSummaryProvider = originalDiscover + }) + + getSummaryAgent = func(types.AgentName) (agent.Agent, error) { + return nil, errors.New("not registered") + } + discoverDispatchSummaryProvider = func(context.Context, types.AgentName) error { return nil } + + _, err := resolveDispatchSummaryProvider(context.Background(), &bytes.Buffer{}, string(providerName)) + if err == nil || !strings.Contains(err.Error(), "unknown summary provider") { + t.Fatalf("resolveDispatchSummaryProvider() error = %v, want existing unknown-provider error", err) + } +} + func TestResolveDispatchSummaryProvider_ExplicitValidationErrors(t *testing.T) { // Cannot use t.Parallel(): subtests mutate package-level resolution seams. tests := []struct { @@ -395,7 +504,7 @@ func TestResolveDispatchSummaryProvider_ExplicitValidationErrors(t *testing.T) { return tt.agent, nil } isSummaryCLIAvailable = func(types.AgentName) bool { return tt.available } - discoverDispatchSummaryProvider = func(context.Context, types.AgentName) {} + discoverDispatchSummaryProvider = func(context.Context, types.AgentName) error { return nil } _, err := resolveDispatchSummaryProvider(context.Background(), &bytes.Buffer{}, tt.override) if err == nil { From fb7316aed261d0687c3d75cd9ca7ec9ca83eb9bc Mon Sep 17 00:00:00 2001 From: Peyton Montei Date: Fri, 17 Jul 2026 16:09:10 -0400 Subject: [PATCH 05/11] fix: preserve named discovery boundary errors Entire-Checkpoint: 01KXRV52M3989P5VTHYBKFVQQ7 --- cmd/entire/cli/agent/external/discovery.go | 13 +++++- .../cli/agent/external/discovery_test.go | 43 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/cmd/entire/cli/agent/external/discovery.go b/cmd/entire/cli/agent/external/discovery.go index 594d391781..1f60ad2f4f 100644 --- a/cmd/entire/cli/agent/external/discovery.go +++ b/cmd/entire/cli/agent/external/discovery.go @@ -74,14 +74,23 @@ func discoverAndRegisterNamed(ctx context.Context, name types.AgentName, timeout binName := binaryPrefix + string(name) binPath, err := lookPathExternalAgent(binName) + if ctxErr := ctx.Err(); ctxErr != nil { + return fmt.Errorf("looking up external agent %q binary %q: %w", name, binName, ctxErr) + } if err != nil { if errors.Is(err, exec.ErrNotFound) { return nil } return fmt.Errorf("looking up external agent %q binary %q: %w", name, binName, err) } - _, err = registerExternalAgent(ctx, binPath, name) - return err + registered, err := registerExternalAgent(ctx, binPath, name) + if err != nil { + return err + } + if !registered { + return fmt.Errorf("external agent %q binary %q was found but could not be registered", name, binPath) + } + return nil } // discoverAndRegister contains the shared scanning logic for external agent discovery. diff --git a/cmd/entire/cli/agent/external/discovery_test.go b/cmd/entire/cli/agent/external/discovery_test.go index ed6cb1f969..b26cdba22b 100644 --- a/cmd/entire/cli/agent/external/discovery_test.go +++ b/cmd/entire/cli/agent/external/discovery_test.go @@ -373,6 +373,49 @@ func TestDiscoverAndRegisterNamedAlways_MissingHelper(t *testing.T) { } } +func TestDiscoverAndRegisterNamedAlways_DeadlineWhileLookingUpMissingHelper(t *testing.T) { + name := types.AgentName("disc-named-lookup-deadline") + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + + originalLookPath := lookPathExternalAgent + t.Cleanup(func() { lookPathExternalAgent = originalLookPath }) + lookPathExternalAgent = func(string) (string, error) { + <-ctx.Done() + return "", exec.ErrNotFound + } + + err := DiscoverAndRegisterNamedAlways(ctx, name) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("DiscoverAndRegisterNamedAlways() error = %v, want context deadline exceeded", err) + } +} + +func TestDiscoverAndRegisterNamedAlways_HelperDisappearsAfterLookup(t *testing.T) { + name := types.AgentName("disc-named-helper-disappeared") + binPath := filepath.Join(t.TempDir(), binaryPrefix+string(name)) + + originalLookPath := lookPathExternalAgent + originalStat := statExternalAgent + t.Cleanup(func() { + lookPathExternalAgent = originalLookPath + statExternalAgent = originalStat + }) + lookPathExternalAgent = func(string) (string, error) { return binPath, nil } + statExternalAgent = func(string) (os.FileInfo, error) { return nil, os.ErrNotExist } + + err := DiscoverAndRegisterNamedAlways(context.Background(), name) + if err == nil { + t.Fatal("DiscoverAndRegisterNamedAlways() error = nil, want helper-disappeared error") + } + if !strings.Contains(err.Error(), string(name)) || !strings.Contains(err.Error(), binPath) { + t.Fatalf("DiscoverAndRegisterNamedAlways() error = %q, want agent and binary context", err) + } + if !strings.Contains(err.Error(), "was found but could not be registered") { + t.Fatalf("DiscoverAndRegisterNamedAlways() error = %q, want actionable registration context", err) + } +} + func TestDiscoverAndRegisterNamedAlways_StatError(t *testing.T) { name := types.AgentName("disc-named-stat-error") binPath := filepath.Join(t.TempDir(), binaryPrefix+string(name)) From 18d5b63d7becd5247b277494ab31cb497cc02ffa Mon Sep 17 00:00:00 2001 From: Peyton Montei Date: Fri, 17 Jul 2026 16:54:27 -0400 Subject: [PATCH 06/11] fix: preflight local dispatch before agent selection Entire-Checkpoint: 01KXRXQZFHBN48GYRKPNREGKYV --- cmd/entire/cli/dispatch.go | 5 + cmd/entire/cli/dispatch/dispatch.go | 1 + cmd/entire/cli/dispatch/mode_local.go | 49 +++++-- cmd/entire/cli/dispatch/mode_local_test.go | 135 +++++++++++++++++++ cmd/entire/cli/dispatch_test.go | 144 +++++++++++++++++++++ cmd/entire/cli/dispatch_tui_test.go | 74 +++++++++++ 6 files changed, 399 insertions(+), 9 deletions(-) diff --git a/cmd/entire/cli/dispatch.go b/cmd/entire/cli/dispatch.go index b9ffad1d4f..a80ab2527d 100644 --- a/cmd/entire/cli/dispatch.go +++ b/cmd/entire/cli/dispatch.go @@ -19,6 +19,7 @@ var renderDispatchMarkdown = dispatchpkg.RenderMarkdown var dispatchTerminalMode = interactive.IsTerminalWriter var runInteractiveDispatch = defaultRunInteractiveDispatch var renderTerminalMarkdown = defaultRenderTerminalMarkdown +var prepareLocalDispatch = dispatchpkg.PrepareLocal var resolveDispatchProvider = resolveDispatchSummaryProvider func newDispatchCmd() *cobra.Command { @@ -71,6 +72,10 @@ Examples: return err } if opts.Mode == dispatchpkg.ModeLocal { + opts, err = prepareLocalDispatch(cmd.Context(), opts) + if err != nil { + return err + } provider, err := resolveDispatchProvider(cmd.Context(), cmd.ErrOrStderr(), agentOverride) if err != nil { return err diff --git a/cmd/entire/cli/dispatch/dispatch.go b/cmd/entire/cli/dispatch/dispatch.go index ca05516764..8dedd4e5cf 100644 --- a/cmd/entire/cli/dispatch/dispatch.go +++ b/cmd/entire/cli/dispatch/dispatch.go @@ -39,6 +39,7 @@ type Options struct { InsecureHTTPAuth bool TextGenerator TextGenerator Model string + localPreflight *localPreflight } // CloudRepoLimit caps how many repos the cloud mode may query in one request. diff --git a/cmd/entire/cli/dispatch/mode_local.go b/cmd/entire/cli/dispatch/mode_local.go index 61e2c56fe3..8c7954dc84 100644 --- a/cmd/entire/cli/dispatch/mode_local.go +++ b/cmd/entire/cli/dispatch/mode_local.go @@ -35,7 +35,20 @@ var ( nowUTC = func() time.Time { return time.Now().UTC() } ) -func runLocal(ctx context.Context, opts Options) (*Dispatch, error) { +type localPreflight struct { + normalizedSince time.Time + normalizedUntil time.Time + repoRoots []string +} + +// PrepareLocal validates and resolves the inputs needed before local dispatch +// generation can begin. The returned options can be passed to Run without +// repeating time-window parsing or repository-root discovery. +func PrepareLocal(ctx context.Context, opts Options) (Options, error) { + if opts.Mode != ModeLocal { + return Options{}, errors.New("local dispatch preflight requires local mode") + } + now := nowUTC() sinceInput := strings.TrimSpace(opts.Since) if sinceInput == "" { @@ -43,28 +56,46 @@ func runLocal(ctx context.Context, opts Options) (*Dispatch, error) { } since, err := ParseSinceAtNow(sinceInput, now) if err != nil { - return nil, err + return Options{}, err } until, err := ParseUntilAtNow(opts.Until, now) if err != nil { - return nil, err + return Options{}, err } normalizedSince, normalizedUntil := NormalizeWindow(since, until) if !normalizedSince.Before(normalizedUntil) { - return nil, errors.New("--since must be before --until") + return Options{}, errors.New("--since must be before --until") } repoRoots, err := resolveRepoRoots(ctx, opts.RepoPaths) if err != nil { - return nil, err + return Options{}, err + } + + opts.localPreflight = &localPreflight{ + normalizedSince: normalizedSince, + normalizedUntil: normalizedUntil, + repoRoots: repoRoots, + } + return opts, nil +} + +func runLocal(ctx context.Context, opts Options) (*Dispatch, error) { + if opts.localPreflight == nil { + prepared, err := PrepareLocal(ctx, opts) + if err != nil { + return nil, err + } + opts = prepared } + preflight := opts.localPreflight allCandidates := make([]candidate, 0) var candidatesMu sync.Mutex group, groupCtx := errgroup.WithContext(ctx) - for _, repoRoot := range repoRoots { + for _, repoRoot := range preflight.repoRoots { group.Go(func() error { - candidates, err := enumerateRepoCandidates(groupCtx, repoRoot, opts, normalizedSince, normalizedUntil) + candidates, err := enumerateRepoCandidates(groupCtx, repoRoot, opts, preflight.normalizedSince, preflight.normalizedUntil) if err != nil { return err } @@ -83,8 +114,8 @@ func runLocal(ctx context.Context, opts Options) (*Dispatch, error) { CoveredRepos: coveredRepos(allCandidates), Repos: groupBulletsByRepo(fallback.Used), Window: Window{ - NormalizedSince: normalizedSince, - NormalizedUntil: normalizedUntil, + NormalizedSince: preflight.normalizedSince, + NormalizedUntil: preflight.normalizedUntil, FirstCheckpointAt: firstAt(fallback.Used), LastCheckpointAt: lastAt(fallback.Used), }, diff --git a/cmd/entire/cli/dispatch/mode_local_test.go b/cmd/entire/cli/dispatch/mode_local_test.go index bade933846..64a20f0b7b 100644 --- a/cmd/entire/cli/dispatch/mode_local_test.go +++ b/cmd/entire/cli/dispatch/mode_local_test.go @@ -20,6 +20,141 @@ import ( "github.com/go-git/go-git/v6/plumbing/object" ) +func TestPrepareLocal_RejectsServerMode(t *testing.T) { + t.Parallel() + + _, err := PrepareLocal(context.Background(), Options{Mode: ModeServer}) + if err == nil || !strings.Contains(err.Error(), "local") { + t.Fatalf("expected local-mode error, got %v", err) + } +} + +func TestPrepareLocal_RejectsInvalidSince(t *testing.T) { + t.Parallel() + + _, err := PrepareLocal(context.Background(), Options{ + Mode: ModeLocal, + Since: "definitely-not-a-time", + }) + if err == nil || !strings.Contains(err.Error(), "unparseable time") { + t.Fatalf("expected invalid --since error, got %v", err) + } +} + +func TestPrepareLocal_RejectsInvalidUntil(t *testing.T) { + t.Parallel() + + _, err := PrepareLocal(context.Background(), Options{ + Mode: ModeLocal, + Since: "2026-07-16T12:00:00Z", + Until: "definitely-not-a-time", + }) + if err == nil || !strings.Contains(err.Error(), "unparseable time") { + t.Fatalf("expected invalid --until error, got %v", err) + } +} + +func TestPrepareLocal_RejectsReversedNormalizedWindow(t *testing.T) { + t.Parallel() + + _, err := PrepareLocal(context.Background(), Options{ + Mode: ModeLocal, + Since: "2026-07-17T12:01:00Z", + Until: "2026-07-17T12:00:00Z", + }) + if err == nil || err.Error() != "--since must be before --until" { + t.Fatalf("expected reversed-window error, got %v", err) + } +} + +func TestPrepareLocal_RejectsEqualNormalizedWindow(t *testing.T) { + t.Parallel() + + _, err := PrepareLocal(context.Background(), Options{ + Mode: ModeLocal, + Since: "2026-07-17T12:00:00Z", + Until: "2026-07-17T12:00:00Z", + }) + if err == nil || err.Error() != "--since must be before --until" { + t.Fatalf("expected equal-window error, got %v", err) + } +} + +func TestPrepareLocal_ValidWindowOutsideGitFailsRepoResolution(t *testing.T) { + t.Chdir(t.TempDir()) + + _, err := PrepareLocal(context.Background(), Options{ + Mode: ModeLocal, + Since: "2026-07-16T12:00:00Z", + Until: "2026-07-17T12:00:00Z", + }) + if err == nil || !strings.Contains(err.Error(), "not in a git repository") { + t.Fatalf("expected repository-root error, got %v", err) + } +} + +func TestPrepareLocal_RunAutoPreparesDirectCall(t *testing.T) { + t.Chdir(t.TempDir()) + + _, err := Run(context.Background(), Options{ + Mode: ModeLocal, + Since: "2026-07-16T12:00:00Z", + Until: "2026-07-17T12:00:00Z", + AllBranches: true, + TextGenerator: stubGeneratedLocalDispatch(), + }) + if err == nil || !strings.Contains(err.Error(), "not in a git repository") { + t.Fatalf("expected direct Run to perform repository preflight, got %v", err) + } +} + +func TestRunLocal_UsesPreparedWindowAndRepoRoots(t *testing.T) { + repoDir := t.TempDir() + testutil.InitRepo(t, repoDir) + testutil.WriteFile(t, repoDir, "a.txt", "x") + testutil.GitAdd(t, repoDir, "a.txt") + testutil.GitCommit(t, repoDir, "initial") + addOriginRemote(t, repoDir) + + preparedAt := time.Date(2026, 7, 17, 12, 34, 45, 0, time.UTC) + oldNow := nowUTC + nowUTC = func() time.Time { return preparedAt } + t.Cleanup(func() { nowUTC = oldNow }) + + t.Chdir(repoDir) + generator := stubGeneratedLocalDispatch() + prepared, err := PrepareLocal(context.Background(), Options{ + Mode: ModeLocal, + Since: "1h", + AllBranches: true, + TextGenerator: generator, + Model: "prepared-model", + }) + if err != nil { + t.Fatal(err) + } + if prepared.TextGenerator != generator || prepared.Model != "prepared-model" { + t.Fatal("preflight must preserve injected generation options") + } + + nowUTC = func() time.Time { return preparedAt.Add(24 * time.Hour) } + t.Chdir(t.TempDir()) + got, err := Run(context.Background(), prepared) + if err != nil { + t.Fatal(err) + } + + wantSince := time.Date(2026, 7, 17, 11, 34, 0, 0, time.UTC) + wantUntil := time.Date(2026, 7, 17, 12, 35, 0, 0, time.UTC) + if !got.Window.NormalizedSince.Equal(wantSince) || !got.Window.NormalizedUntil.Equal(wantUntil) { + t.Fatalf("prepared window was recomputed: got [%s, %s), want [%s, %s)", + got.Window.NormalizedSince, got.Window.NormalizedUntil, wantSince, wantUntil) + } + if got.GeneratedText != "generated dispatch" { + t.Fatalf("unexpected generated text: %q", got.GeneratedText) + } +} + func TestLocalMode_EnumeratesCheckpoints(t *testing.T) { dir := t.TempDir() testutil.InitRepo(t, dir) diff --git a/cmd/entire/cli/dispatch_test.go b/cmd/entire/cli/dispatch_test.go index c8495e5a14..88ffe1b1a7 100644 --- a/cmd/entire/cli/dispatch_test.go +++ b/cmd/entire/cli/dispatch_test.go @@ -229,6 +229,150 @@ func TestNewDispatchCmd_LongHelpIncludesLocalAgentExample(t *testing.T) { } } +func TestDispatchPreflight_InvalidTimeBeforeProvider(t *testing.T) { + oldProvider := resolveDispatchProvider + oldRunDispatch := runDispatch + providerCalled := false + resolveDispatchProvider = func(context.Context, io.Writer, string) (*checkpointSummaryProvider, error) { + providerCalled = true + return nil, errors.New("provider must not run") + } + runDispatch = func(context.Context, dispatchpkg.Options) (*dispatchpkg.Dispatch, error) { + t.Fatal("dispatch must not run after preflight fails") + return nil, errors.New("dispatch must not run") + } + t.Cleanup(func() { + resolveDispatchProvider = oldProvider + runDispatch = oldRunDispatch + }) + + cmd := newDispatchCmd() + cmd.SilenceErrors = true + cmd.SilenceUsage = true + cmd.SetArgs([]string{"--local", "--all-branches", "--since", "not-a-time"}) + + err := cmd.Execute() + if err == nil || !strings.Contains(err.Error(), "unparseable time") { + t.Fatalf("expected invalid time preflight error, got %v", err) + } + if providerCalled { + t.Fatal("provider resolution ran before invalid time preflight returned") + } +} + +func TestDispatchPreflight_RepositoryFailureBeforeProvider(t *testing.T) { + oldProvider := resolveDispatchProvider + oldRunDispatch := runDispatch + providerCalled := false + resolveDispatchProvider = func(context.Context, io.Writer, string) (*checkpointSummaryProvider, error) { + providerCalled = true + return nil, errors.New("provider must not run") + } + runDispatch = func(context.Context, dispatchpkg.Options) (*dispatchpkg.Dispatch, error) { + t.Fatal("dispatch must not run after preflight fails") + return nil, errors.New("dispatch must not run") + } + t.Cleanup(func() { + resolveDispatchProvider = oldProvider + runDispatch = oldRunDispatch + }) + t.Chdir(t.TempDir()) + + cmd := newDispatchCmd() + cmd.SilenceErrors = true + cmd.SilenceUsage = true + cmd.SetArgs([]string{ + "--local", "--all-branches", + "--since", "2026-07-16T12:00:00Z", + "--until", "2026-07-17T12:00:00Z", + }) + + err := cmd.Execute() + if err == nil || !strings.Contains(err.Error(), "not in a git repository") { + t.Fatalf("expected repository preflight error, got %v", err) + } + if providerCalled { + t.Fatal("provider resolution ran before repository preflight returned") + } +} + +func TestDispatchProvider_LocalRunsAfterPreflightAndInjectsOptions(t *testing.T) { + oldPrepare := prepareLocalDispatch + oldProvider := resolveDispatchProvider + oldRunDispatch := runDispatch + oldTerminalMode := dispatchTerminalMode + oldMarkdown := renderDispatchMarkdown + var calls []string + generator := &stubTextAgent{} + prepareLocalDispatch = func(_ context.Context, opts dispatchpkg.Options) (dispatchpkg.Options, error) { + calls = append(calls, "prepare") + return opts, nil + } + resolveDispatchProvider = func(context.Context, io.Writer, string) (*checkpointSummaryProvider, error) { + calls = append(calls, "provider") + return &checkpointSummaryProvider{TextGenerator: generator, Model: "ordered-model"}, nil + } + runDispatch = func(_ context.Context, opts dispatchpkg.Options) (*dispatchpkg.Dispatch, error) { + calls = append(calls, "dispatch") + if opts.TextGenerator != generator || opts.Model != "ordered-model" { + t.Fatalf("provider options not passed to dispatch: generator=%T model=%q", opts.TextGenerator, opts.Model) + } + return &dispatchpkg.Dispatch{}, nil + } + dispatchTerminalMode = func(io.Writer) bool { return false } + renderDispatchMarkdown = func(*dispatchpkg.Dispatch) string { return "" } + t.Cleanup(func() { + prepareLocalDispatch = oldPrepare + resolveDispatchProvider = oldProvider + runDispatch = oldRunDispatch + dispatchTerminalMode = oldTerminalMode + renderDispatchMarkdown = oldMarkdown + }) + + cmd := newDispatchCmd() + cmd.SetArgs([]string{"--local", "--all-branches"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + if got := strings.Join(calls, ","); got != "prepare,provider,dispatch" { + t.Fatalf("call order = %q, want prepare,provider,dispatch", got) + } +} + +func TestDispatchPreflight_CloudSkipsLocalPreparationAndProvider(t *testing.T) { + oldPrepare := prepareLocalDispatch + oldProvider := resolveDispatchProvider + oldRunDispatch := runDispatch + oldTerminalMode := dispatchTerminalMode + prepareLocalDispatch = func(context.Context, dispatchpkg.Options) (dispatchpkg.Options, error) { + t.Fatal("cloud dispatch must not run local preflight") + return dispatchpkg.Options{}, errors.New("local preflight must not run") + } + resolveDispatchProvider = func(context.Context, io.Writer, string) (*checkpointSummaryProvider, error) { + t.Fatal("cloud dispatch must not resolve a local provider") + return nil, errors.New("provider must not run") + } + runDispatch = func(_ context.Context, opts dispatchpkg.Options) (*dispatchpkg.Dispatch, error) { + if opts.Mode != dispatchpkg.ModeServer { + t.Fatalf("mode = %v, want server", opts.Mode) + } + return &dispatchpkg.Dispatch{}, nil + } + dispatchTerminalMode = func(io.Writer) bool { return false } + t.Cleanup(func() { + prepareLocalDispatch = oldPrepare + resolveDispatchProvider = oldProvider + runDispatch = oldRunDispatch + dispatchTerminalMode = oldTerminalMode + }) + + cmd := newDispatchCmd() + cmd.SetArgs([]string{"--repos", "entireio/cli"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } +} + func TestNewDispatchCmd_CloudAgentFailsBeforeProviderOrDispatch(t *testing.T) { oldProvider := resolveDispatchProvider oldRunDispatch := runDispatch diff --git a/cmd/entire/cli/dispatch_tui_test.go b/cmd/entire/cli/dispatch_tui_test.go index b866d4103b..779a1526b7 100644 --- a/cmd/entire/cli/dispatch_tui_test.go +++ b/cmd/entire/cli/dispatch_tui_test.go @@ -23,6 +23,80 @@ func (p fakeDispatchProgram) Run() (tea.Model, error) { return model, nil } +type dispatchProgramFunc func() (tea.Model, error) + +func (f dispatchProgramFunc) Run() (tea.Model, error) { + return f() +} + +func TestDispatchTerminal_ResolvesProviderBeforeProgramRun(t *testing.T) { + oldPrepare := prepareLocalDispatch + oldProvider := resolveDispatchProvider + oldRunDispatch := runDispatch + oldTerminalMode := dispatchTerminalMode + oldInteractiveDispatch := runInteractiveDispatch + oldRenderTerminal := renderTerminalMarkdown + oldProgramFactory := newDispatchProgram + providerResolved := false + programRunning := false + generator := &stubTextAgent{} + prepareLocalDispatch = func(_ context.Context, opts dispatchpkg.Options) (dispatchpkg.Options, error) { + return opts, nil + } + resolveDispatchProvider = func(context.Context, io.Writer, string) (*checkpointSummaryProvider, error) { + if programRunning { + t.Fatal("provider resolution ran after Bubble Tea took terminal ownership") + } + providerResolved = true + return &checkpointSummaryProvider{TextGenerator: generator, Model: "terminal-model"}, nil + } + runDispatch = func(_ context.Context, opts dispatchpkg.Options) (*dispatchpkg.Dispatch, error) { + if !programRunning { + t.Fatal("interactive dispatch callback ran before program Run") + } + if opts.TextGenerator != generator || opts.Model != "terminal-model" { + t.Fatalf("provider options not passed to interactive dispatch: generator=%T model=%q", opts.TextGenerator, opts.Model) + } + return &dispatchpkg.Dispatch{GeneratedText: "# terminal dispatch\n"}, nil + } + dispatchTerminalMode = func(io.Writer) bool { return true } + runInteractiveDispatch = defaultRunInteractiveDispatch + renderTerminalMarkdown = func(_ io.Writer, markdown string) (string, error) { return markdown, nil } + newDispatchProgram = func(model tea.Model, _ io.Writer, _ bool) dispatchProgram { + if !providerResolved { + t.Fatal("Bubble Tea program was created before provider resolution completed") + } + return dispatchProgramFunc(func() (tea.Model, error) { + programRunning = true + status, ok := model.(dispatchStatusModel) + if !ok { + t.Fatalf("unexpected model type %T", model) + } + markdown, err := status.run(context.Background()) + status.result = dispatchRenderResult{markdown: markdown, err: err} + return status, nil + }) + } + t.Cleanup(func() { + prepareLocalDispatch = oldPrepare + resolveDispatchProvider = oldProvider + runDispatch = oldRunDispatch + dispatchTerminalMode = oldTerminalMode + runInteractiveDispatch = oldInteractiveDispatch + renderTerminalMarkdown = oldRenderTerminal + newDispatchProgram = oldProgramFactory + }) + + cmd := newDispatchCmd() + cmd.SetArgs([]string{"--local", "--all-branches"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + if !providerResolved { + t.Fatal("provider was not resolved") + } +} + func TestDefaultRunInteractiveDispatch_DoesNotUseAltScreen(t *testing.T) { // Cannot run in parallel: mutates package-level newDispatchProgram, which // races with TestDefaultRunInteractiveDispatch_ClearsLoadingCardBeforeReturn. From 4d5ad96d09c78cfd7a9e082de9292f437910a76c Mon Sep 17 00:00:00 2001 From: Peyton Montei Date: Fri, 17 Jul 2026 17:19:51 -0400 Subject: [PATCH 07/11] test: cover interactive dispatch agent selection Entire-Checkpoint: 01KXRZ6FQ85EQGF1S3PP3S9JDA --- cmd/entire/cli/dispatch.go | 6 +- cmd/entire/cli/dispatch_test.go | 101 +++++++++++++++++++++ cmd/entire/cli/explain_summary_provider.go | 6 +- 3 files changed, 109 insertions(+), 4 deletions(-) diff --git a/cmd/entire/cli/dispatch.go b/cmd/entire/cli/dispatch.go index a80ab2527d..569bf706bb 100644 --- a/cmd/entire/cli/dispatch.go +++ b/cmd/entire/cli/dispatch.go @@ -19,6 +19,8 @@ var renderDispatchMarkdown = dispatchpkg.RenderMarkdown var dispatchTerminalMode = interactive.IsTerminalWriter var runInteractiveDispatch = defaultRunInteractiveDispatch var renderTerminalMarkdown = defaultRenderTerminalMarkdown +var shouldRunDispatchWizardForCommand = shouldRunDispatchWizard +var runDispatchWizardForCommand = runDispatchWizard var prepareLocalDispatch = dispatchpkg.PrepareLocal var resolveDispatchProvider = resolveDispatchSummaryProvider @@ -60,8 +62,8 @@ Examples: err error ) - if shouldRunDispatchWizard(cmd.Flags().NFlag(), isTerminalStdin(os.Stdin), interactive.IsTerminalWriter(cmd.OutOrStdout())) { - opts, err = runDispatchWizard(cmd) + if shouldRunDispatchWizardForCommand(cmd.Flags().NFlag(), isTerminalStdin(os.Stdin), interactive.IsTerminalWriter(cmd.OutOrStdout())) { + opts, err = runDispatchWizardForCommand(cmd) } else { opts, err = parseDispatchFlags(cmd, flagLocal, flagSince, flagUntil, flagAllBranches, flagRepos, flagVoice, flagInsecureHTTPAuth) } diff --git a/cmd/entire/cli/dispatch_test.go b/cmd/entire/cli/dispatch_test.go index 88ffe1b1a7..c52f616f3b 100644 --- a/cmd/entire/cli/dispatch_test.go +++ b/cmd/entire/cli/dispatch_test.go @@ -9,7 +9,10 @@ import ( "testing" "github.com/entireio/cli/cmd/entire/cli/agent" + "github.com/entireio/cli/cmd/entire/cli/agent/types" dispatchpkg "github.com/entireio/cli/cmd/entire/cli/dispatch" + "github.com/entireio/cli/cmd/entire/cli/settings" + "github.com/entireio/cli/cmd/entire/cli/testutil" "github.com/spf13/cobra" ) @@ -568,6 +571,104 @@ func TestNewDispatchCmd_LocalWithoutAgentResolvesConfiguredProvider(t *testing.T } } +func TestDispatchWizard_LocalWithoutConfiguredAgentPromptsAndPersistsSelection(t *testing.T) { + repoDir := t.TempDir() + testutil.InitRepo(t, repoDir) + t.Chdir(repoDir) + + oldShouldRunWizard := shouldRunDispatchWizardForCommand + oldRunWizard := runDispatchWizardForCommand + oldLoad := loadSummarySettings + oldLoadFile := loadSummarySettingsFromFile + oldSave := saveLocalSummarySettings + oldDiscover := discoverSummaryProvidersAlways + oldList := listRegisteredAgents + oldGet := getSummaryAgent + oldAvailable := isSummaryCLIAvailable + oldCanPrompt := canPromptForSummaryProvider + oldPrompt := promptSummaryProvider + oldRunDispatch := runDispatch + oldTerminalMode := dispatchTerminalMode + oldMarkdown := renderDispatchMarkdown + t.Cleanup(func() { + shouldRunDispatchWizardForCommand = oldShouldRunWizard + runDispatchWizardForCommand = oldRunWizard + loadSummarySettings = oldLoad + loadSummarySettingsFromFile = oldLoadFile + saveLocalSummarySettings = oldSave + discoverSummaryProvidersAlways = oldDiscover + listRegisteredAgents = oldList + getSummaryAgent = oldGet + isSummaryCLIAvailable = oldAvailable + canPromptForSummaryProvider = oldCanPrompt + promptSummaryProvider = oldPrompt + runDispatch = oldRunDispatch + dispatchTerminalMode = oldTerminalMode + renderDispatchMarkdown = oldMarkdown + }) + + var calls []string + shouldRunDispatchWizardForCommand = func(int, bool, bool) bool { return true } + runDispatchWizardForCommand = func(*cobra.Command) (dispatchpkg.Options, error) { + calls = append(calls, "wizard") + return dispatchpkg.Options{Mode: dispatchpkg.ModeLocal, Since: "7d", AllBranches: true}, nil + } + loadSummarySettings = func(context.Context) (*settings.EntireSettings, error) { + return &settings.EntireSettings{Enabled: true}, nil + } + loadSummarySettingsFromFile = func(string) (*settings.EntireSettings, error) { + return &settings.EntireSettings{}, nil + } + discoverSummaryProvidersAlways = func(context.Context) {} + listRegisteredAgents = func() []types.AgentName { + return []types.AgentName{agent.AgentNameCodex, agent.AgentNameGemini} + } + getSummaryAgent = func(name types.AgentName) (agent.Agent, error) { + kind := agent.AgentTypeCodex + if name == agent.AgentNameGemini { + kind = agent.AgentTypeGemini + } + return &stubTextAgent{name: name, kind: kind}, nil + } + isSummaryCLIAvailable = func(types.AgentName) bool { return true } + canPromptForSummaryProvider = func() bool { return true } + promptSummaryProvider = func(providers []checkpointSummaryProvider) (types.AgentName, error) { + calls = append(calls, "picker") + if len(providers) != 2 || providers[0].Name != agent.AgentNameCodex || providers[1].Name != agent.AgentNameGemini { + t.Fatalf("picker providers = %+v, want enabled codex and gemini", providers) + } + return agent.AgentNameGemini, nil + } + var persistedProvider string + saveLocalSummarySettings = func(_ context.Context, s *settings.EntireSettings) error { + if s.SummaryGeneration != nil { + persistedProvider = s.SummaryGeneration.Provider + } + return nil + } + runDispatch = func(_ context.Context, opts dispatchpkg.Options) (*dispatchpkg.Dispatch, error) { + calls = append(calls, "dispatch") + selected, ok := opts.TextGenerator.(*stubTextAgent) + if !ok || selected.name != agent.AgentNameGemini { + t.Fatalf("dispatch generator = %#v, want selected gemini agent", opts.TextGenerator) + } + return &dispatchpkg.Dispatch{}, nil + } + dispatchTerminalMode = func(io.Writer) bool { return false } + renderDispatchMarkdown = func(*dispatchpkg.Dispatch) string { return "" } + + cmd := newDispatchCmd() + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + if got := strings.Join(calls, ","); got != "wizard,picker,dispatch" { + t.Fatalf("call order = %q, want wizard,picker,dispatch", got) + } + if persistedProvider != string(agent.AgentNameGemini) { + t.Fatalf("persisted provider = %q, want %q", persistedProvider, agent.AgentNameGemini) + } +} + func TestNewDispatchCmd_CloudDispatchDoesNotResolveLocalProvider(t *testing.T) { oldProvider := resolveDispatchProvider oldRunDispatch := runDispatch diff --git a/cmd/entire/cli/explain_summary_provider.go b/cmd/entire/cli/explain_summary_provider.go index c2d30fca15..41180ea5f2 100644 --- a/cmd/entire/cli/explain_summary_provider.go +++ b/cmd/entire/cli/explain_summary_provider.go @@ -29,6 +29,8 @@ var ( discoverSummaryProviders = external.DiscoverAndRegister discoverSummaryProvidersAlways = external.DiscoverAndRegisterAlways discoverDispatchSummaryProvider = external.DiscoverAndRegisterNamedAlways + canPromptForSummaryProvider = interactive.CanPromptInteractively + promptSummaryProvider = promptForSummaryProvider ) type checkpointSummaryProvider struct { @@ -86,11 +88,11 @@ func resolveCheckpointSummaryProvider(ctx context.Context, w io.Writer) (*checkp case 1: return autoSelectSummaryProvider(ctx, w, candidates[0].Name, "non-interactive auto-select: single installed provider") default: - if !interactive.CanPromptInteractively() { + if !canPromptForSummaryProvider() { return autoSelectSummaryProvider(ctx, w, candidates[0].Name, "non-interactive auto-select: first detected of multiple") } - selected, err := promptForSummaryProvider(candidates) + selected, err := promptSummaryProvider(candidates) if err != nil { return nil, err } From 11b6479c7e873cda808b220bc2533baa4648d744 Mon Sep 17 00:00:00 2001 From: Peyton Montei Date: Fri, 17 Jul 2026 17:33:50 -0400 Subject: [PATCH 08/11] test: cover named agent discovery on Windows Entire-Checkpoint: 01KXS001P21KYQBC8961Z63TKP --- .github/workflows/ci.yml | 12 +++++++ .../cli/agent/external/discovery_test.go | 32 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4c010de0a0..2969e7bba0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,12 +67,23 @@ jobs: E2E_CHECKPOINT_STORE: ${{ matrix.checkpoint_store }} run: mise run test:e2e:canary + test-windows-external-discovery: + runs-on: windows-latest + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} + - uses: jdx/mise-action@dad1bfd3df957f44999b559dd69dc1671cb4e9ea # v4 + - name: Named external-agent discovery + run: go test ./cmd/entire/cli/agent/external -run '^TestDiscoverAndRegisterNamedAlways_RegistersBatOnWindows$' -count=1 + test: runs-on: ubuntu-latest needs: - test-core - test-integration - test-canary + - test-windows-external-discovery if: ${{ always() }} steps: - name: Check dependencies @@ -80,3 +91,4 @@ jobs: [ "${{ needs.test-core.result }}" = "success" ] [ "${{ needs.test-integration.result }}" = "success" ] [ "${{ needs.test-canary.result }}" = "success" ] + [ "${{ needs.test-windows-external-discovery.result }}" = "success" ] diff --git a/cmd/entire/cli/agent/external/discovery_test.go b/cmd/entire/cli/agent/external/discovery_test.go index b26cdba22b..a1b0dabd90 100644 --- a/cmd/entire/cli/agent/external/discovery_test.go +++ b/cmd/entire/cli/agent/external/discovery_test.go @@ -598,3 +598,35 @@ func TestDiscoverAndRegister_RegistersBatOnWindows(t *testing.T) { t.Errorf("agent Name() = %q, want %q", ag.Name(), name) } } + +// TestDiscoverAndRegisterNamedAlways_RegistersBatOnWindows covers the explicit +// named-discovery path, which uses exec.LookPath and therefore depends on +// Windows PATHEXT handling rather than the scan-all filepath.Glob path above. +func TestDiscoverAndRegisterNamedAlways_RegistersBatOnWindows(t *testing.T) { + if runtime.GOOS != osWindows { + t.Skip("this test only applies on Windows") + } + + name := types.AgentName("disc-named-bat") + infoJSON := `{"protocol_version":1,"name":"` + string(name) + `","type":"` + string(name) + ` Agent","description":"Named Windows agent","is_preview":false,"protected_dirs":[],"hook_names":[],"capabilities":{}}` + script := "@echo off\r\nif not \"%1\"==\"info\" goto :notinfo\r\necho " + infoJSON + "\r\ngoto :eof\r\n:notinfo\r\necho unknown subcommand: %1 1>&2\r\nexit /b 1\r\n" + + dir := t.TempDir() + binPath := filepath.Join(dir, binaryPrefix+string(name)+".bat") + if err := os.WriteFile(binPath, []byte(script), 0o755); err != nil { + t.Fatalf("write mock binary: %v", err) + } + t.Setenv("PATH", dir) + t.Setenv("PATHEXT", ".COM;.EXE;.BAT;.CMD") + + if err := DiscoverAndRegisterNamedAlways(context.Background(), name); err != nil { + t.Fatalf("DiscoverAndRegisterNamedAlways() error = %v", err) + } + ag, err := agent.Get(name) + if err != nil { + t.Fatalf("expected named .bat agent %q to be registered: %v", name, err) + } + if ag.Name() != name { + t.Fatalf("agent Name() = %q, want %q", ag.Name(), name) + } +} From 8cb45040651dcb612218207e9eefa2bd04e1d821 Mon Sep 17 00:00:00 2001 From: Peyton Montei Date: Fri, 17 Jul 2026 18:13:30 -0400 Subject: [PATCH 09/11] fix(dispatch): harden local agent selection Entire-Checkpoint: 01KXS28PXFVTCQBQ8GTQCWH3EE --- cmd/entire/cli/agent/external/discovery.go | 3 + .../cli/agent/external/discovery_test.go | 21 +++++++ cmd/entire/cli/dispatch/mode_local.go | 16 ++++- cmd/entire/cli/dispatch/mode_local_test.go | 58 +++++++++++++++++++ cmd/entire/cli/dispatch_test.go | 15 +++++ 5 files changed, 112 insertions(+), 1 deletion(-) diff --git a/cmd/entire/cli/agent/external/discovery.go b/cmd/entire/cli/agent/external/discovery.go index 1f60ad2f4f..7a7ca7ff18 100644 --- a/cmd/entire/cli/agent/external/discovery.go +++ b/cmd/entire/cli/agent/external/discovery.go @@ -62,6 +62,9 @@ func discoverAndRegisterNamed(ctx context.Context, name types.AgentName, timeout if name == "" { return nil } + if strings.ContainsAny(string(name), `/\`) { + return fmt.Errorf("invalid external agent name %q: contains path separators", name) + } if _, err := agent.Get(name); err == nil { return nil } diff --git a/cmd/entire/cli/agent/external/discovery_test.go b/cmd/entire/cli/agent/external/discovery_test.go index a1b0dabd90..0358f8c294 100644 --- a/cmd/entire/cli/agent/external/discovery_test.go +++ b/cmd/entire/cli/agent/external/discovery_test.go @@ -373,6 +373,27 @@ func TestDiscoverAndRegisterNamedAlways_MissingHelper(t *testing.T) { } } +func TestDiscoverAndRegisterNamedAlways_RejectsPathSeparators(t *testing.T) { + originalLookPath := lookPathExternalAgent + t.Cleanup(func() { lookPathExternalAgent = originalLookPath }) + + for _, name := range []types.AgentName{"foo/../../agent", `foo\bar`} { + lookedUp := false + lookPathExternalAgent = func(string) (string, error) { + lookedUp = true + return "", exec.ErrNotFound + } + + err := DiscoverAndRegisterNamedAlways(context.Background(), name) + if err == nil || !strings.Contains(err.Error(), "path separators") { + t.Errorf("DiscoverAndRegisterNamedAlways(%q) error = %v, want path separator error", name, err) + } + if lookedUp { + t.Errorf("DiscoverAndRegisterNamedAlways(%q) called exec.LookPath for an invalid name", name) + } + } +} + func TestDiscoverAndRegisterNamedAlways_DeadlineWhileLookingUpMissingHelper(t *testing.T) { name := types.AgentName("disc-named-lookup-deadline") ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) diff --git a/cmd/entire/cli/dispatch/mode_local.go b/cmd/entire/cli/dispatch/mode_local.go index 8c7954dc84..3e262a357a 100644 --- a/cmd/entire/cli/dispatch/mode_local.go +++ b/cmd/entire/cli/dispatch/mode_local.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "os/exec" + "slices" "sort" "strings" "sync" @@ -39,6 +40,16 @@ type localPreflight struct { normalizedSince time.Time normalizedUntil time.Time repoRoots []string + sinceInput string + untilInput string + repoPathsInput []string +} + +func (p *localPreflight) matches(opts Options) bool { + return p != nil && + p.sinceInput == opts.Since && + p.untilInput == opts.Until && + slices.Equal(p.repoPathsInput, opts.RepoPaths) } // PrepareLocal validates and resolves the inputs needed before local dispatch @@ -76,12 +87,15 @@ func PrepareLocal(ctx context.Context, opts Options) (Options, error) { normalizedSince: normalizedSince, normalizedUntil: normalizedUntil, repoRoots: repoRoots, + sinceInput: opts.Since, + untilInput: opts.Until, + repoPathsInput: slices.Clone(opts.RepoPaths), } return opts, nil } func runLocal(ctx context.Context, opts Options) (*Dispatch, error) { - if opts.localPreflight == nil { + if !opts.localPreflight.matches(opts) { prepared, err := PrepareLocal(ctx, opts) if err != nil { return nil, err diff --git a/cmd/entire/cli/dispatch/mode_local_test.go b/cmd/entire/cli/dispatch/mode_local_test.go index 64a20f0b7b..4241a87d7e 100644 --- a/cmd/entire/cli/dispatch/mode_local_test.go +++ b/cmd/entire/cli/dispatch/mode_local_test.go @@ -155,6 +155,64 @@ func TestRunLocal_UsesPreparedWindowAndRepoRoots(t *testing.T) { } } +func TestRunLocal_RepreparesWhenPreflightInputsChange(t *testing.T) { + repoDir := t.TempDir() + testutil.InitRepo(t, repoDir) + testutil.WriteFile(t, repoDir, "a.txt", "x") + testutil.GitAdd(t, repoDir, "a.txt") + testutil.GitCommit(t, repoDir, "initial") + addOriginRemote(t, repoDir) + t.Chdir(repoDir) + + tests := []struct { + name string + mutate func(*Options) + wantError string + }{ + { + name: "since", + mutate: func(opts *Options) { + opts.Since = "definitely-not-a-time" + }, + wantError: "unparseable time", + }, + { + name: "until", + mutate: func(opts *Options) { + opts.Until = "definitely-not-a-time" + }, + wantError: "unparseable time", + }, + { + name: "repo paths", + mutate: func(opts *Options) { + opts.RepoPaths = []string{filepath.Join(t.TempDir(), "missing")} + }, + wantError: "resolve repo root", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + prepared, err := PrepareLocal(context.Background(), Options{ + Mode: ModeLocal, + Since: "7d", + AllBranches: true, + TextGenerator: stubGeneratedLocalDispatch(), + }) + if err != nil { + t.Fatal(err) + } + + tt.mutate(&prepared) + _, err = Run(context.Background(), prepared) + if err == nil || !strings.Contains(err.Error(), tt.wantError) { + t.Fatalf("Run() error = %v, want error containing %q", err, tt.wantError) + } + }) + } +} + func TestLocalMode_EnumeratesCheckpoints(t *testing.T) { dir := t.TempDir() testutil.InitRepo(t, dir) diff --git a/cmd/entire/cli/dispatch_test.go b/cmd/entire/cli/dispatch_test.go index c52f616f3b..2e5dd29dd2 100644 --- a/cmd/entire/cli/dispatch_test.go +++ b/cmd/entire/cli/dispatch_test.go @@ -489,11 +489,15 @@ func TestNewDispatchCmd_LocalExplicitEmptyAgentFailsBeforeProviderOrDispatch(t * } func TestNewDispatchCmd_LocalAgentInjectsProviderAndKeepsOutputSeparated(t *testing.T) { + oldPrepare := prepareLocalDispatch oldProvider := resolveDispatchProvider oldRunDispatch := runDispatch oldTerminalMode := dispatchTerminalMode oldMarkdown := renderDispatchMarkdown generator := &stubTextAgent{} + prepareLocalDispatch = func(_ context.Context, opts dispatchpkg.Options) (dispatchpkg.Options, error) { + return opts, nil + } resolveDispatchProvider = func(_ context.Context, w io.Writer, override string) (*checkpointSummaryProvider, error) { if override != string(agent.AgentNameCodex) { t.Fatalf("provider override = %q, want codex", override) @@ -518,6 +522,7 @@ func TestNewDispatchCmd_LocalAgentInjectsProviderAndKeepsOutputSeparated(t *test dispatchTerminalMode = func(io.Writer) bool { return false } renderDispatchMarkdown = func(*dispatchpkg.Dispatch) string { return testDispatchGeneratedMarkdown } t.Cleanup(func() { + prepareLocalDispatch = oldPrepare resolveDispatchProvider = oldProvider runDispatch = oldRunDispatch dispatchTerminalMode = oldTerminalMode @@ -545,9 +550,13 @@ func TestNewDispatchCmd_LocalAgentInjectsProviderAndKeepsOutputSeparated(t *test } func TestNewDispatchCmd_LocalWithoutAgentResolvesConfiguredProvider(t *testing.T) { + oldPrepare := prepareLocalDispatch oldProvider := resolveDispatchProvider oldRunDispatch := runDispatch oldTerminalMode := dispatchTerminalMode + prepareLocalDispatch = func(_ context.Context, opts dispatchpkg.Options) (dispatchpkg.Options, error) { + return opts, nil + } resolveDispatchProvider = func(_ context.Context, _ io.Writer, override string) (*checkpointSummaryProvider, error) { if override != "" { t.Fatalf("provider override = %q, want empty", override) @@ -559,6 +568,7 @@ func TestNewDispatchCmd_LocalWithoutAgentResolvesConfiguredProvider(t *testing.T } dispatchTerminalMode = func(io.Writer) bool { return false } t.Cleanup(func() { + prepareLocalDispatch = oldPrepare resolveDispatchProvider = oldProvider runDispatch = oldRunDispatch dispatchTerminalMode = oldTerminalMode @@ -699,9 +709,13 @@ func TestNewDispatchCmd_CloudDispatchDoesNotResolveLocalProvider(t *testing.T) { } func TestNewDispatchCmd_ProviderErrorUsesStderrAndSkipsDispatch(t *testing.T) { + oldPrepare := prepareLocalDispatch oldProvider := resolveDispatchProvider oldRunDispatch := runDispatch unexpectedCallErr := errors.New("unexpected dispatch call") + prepareLocalDispatch = func(_ context.Context, opts dispatchpkg.Options) (dispatchpkg.Options, error) { + return opts, nil + } resolveDispatchProvider = func(_ context.Context, w io.Writer, override string) (*checkpointSummaryProvider, error) { if override != string(agent.AgentNameCodex) { t.Fatalf("provider override = %q, want codex", override) @@ -716,6 +730,7 @@ func TestNewDispatchCmd_ProviderErrorUsesStderrAndSkipsDispatch(t *testing.T) { return nil, unexpectedCallErr } t.Cleanup(func() { + prepareLocalDispatch = oldPrepare resolveDispatchProvider = oldProvider runDispatch = oldRunDispatch }) From f71c9828d7f1c19658fa180555a5d5a623e30049 Mon Sep 17 00:00:00 2001 From: Peyton Montei Date: Thu, 23 Jul 2026 14:04:39 -0400 Subject: [PATCH 10/11] ci: slim windows discovery job and cover both bat tests Replace mise-action with setup-go (the job only needs a Go toolchain) and widen the -run pattern so the pre-existing scan-all .bat test runs in CI alongside the named-discovery one. Document why the job is scoped to the Windows-gated tests. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2969e7bba0..d0bc04d376 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,9 +73,15 @@ jobs: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: ref: ${{ github.event.pull_request.head.sha || github.sha }} - - uses: jdx/mise-action@dad1bfd3df957f44999b559dd69dc1671cb4e9ea # v4 - - name: Named external-agent discovery - run: go test ./cmd/entire/cli/agent/external -run '^TestDiscoverAndRegisterNamedAlways_RegistersBatOnWindows$' -count=1 + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: go.mod + # Runs only the Windows-gated discovery tests: exec.LookPath resolves + # external agents via PATHEXT (.bat/.cmd), a code path the ubuntu jobs + # cannot exercise. The rest of the package mocks agents as #!/bin/sh + # scripts, which Windows cannot execute, so it stays on ubuntu. + - name: Windows external-agent discovery + run: go test ./cmd/entire/cli/agent/external -run 'RegistersBatOnWindows$' -count=1 test: runs-on: ubuntu-latest From 003604958aa31ddb3410a324ebf740cf7596458a Mon Sep 17 00:00:00 2001 From: Peyton Montei Date: Thu, 23 Jul 2026 15:09:22 -0400 Subject: [PATCH 11/11] ci: drop windows discovery job per review, tracked in #1843 Remove the test-windows-external-discovery merge gate added earlier in this PR. Review feedback: per-PR Windows coverage should be a repo-wide CI decision covering all Windows-gated tests, not a gate scoped to the two newest ones. The .bat discovery tests stay (they self-skip off Windows) and will run once #1843 lands a proper Windows job. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 873f98b9ed..a8b8e3c44f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,29 +67,12 @@ jobs: E2E_CHECKPOINT_STORE: ${{ matrix.checkpoint_store }} run: mise run test:e2e:canary - test-windows-external-discovery: - runs-on: windows-latest - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - ref: ${{ github.event.pull_request.head.sha || github.sha }} - - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 - with: - go-version-file: go.mod - # Runs only the Windows-gated discovery tests: exec.LookPath resolves - # external agents via PATHEXT (.bat/.cmd), a code path the ubuntu jobs - # cannot exercise. The rest of the package mocks agents as #!/bin/sh - # scripts, which Windows cannot execute, so it stays on ubuntu. - - name: Windows external-agent discovery - run: go test ./cmd/entire/cli/agent/external -run 'RegistersBatOnWindows$' -count=1 - test: runs-on: ubuntu-latest needs: - test-core - test-integration - test-canary - - test-windows-external-discovery if: ${{ always() }} steps: - name: Check dependencies @@ -97,4 +80,3 @@ jobs: [ "${{ needs.test-core.result }}" = "success" ] [ "${{ needs.test-integration.result }}" = "success" ] [ "${{ needs.test-canary.result }}" = "success" ] - [ "${{ needs.test-windows-external-discovery.result }}" = "success" ]