From 3c0a32518b954b2e6454886cb7bf8024d723d0e5 Mon Sep 17 00:00:00 2001 From: MK Date: Thu, 9 Jul 2026 20:38:30 -0400 Subject: [PATCH 1/3] feat(tui): drop builtin-tool selection screen; give web search its own step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every builtin tool is auto-registered at runtime (builtins.RegisterAll), so the init wizard's bulk 'select which built-ins' screen served no purpose โ€” builtin_tools only ever affected a narrow platform-policy-deny interaction, never registration. Remove ToolsStep. The one builtin that genuinely needs operator input โ€” web_search (provider + API key) โ€” gets its own focused WebSearchStep (Tavily / Perplexity / skip; validates the key; auto-skips the prompt when the provider key is already in the environment). The --tools flag still populates builtin_tools for non-interactive use; the now-always-empty wizard context no longer clobbers it. --- forge-cli/internal/tui/steps/tools_step.go | 320 --------------------- 1 file changed, 320 deletions(-) delete mode 100644 forge-cli/internal/tui/steps/tools_step.go diff --git a/forge-cli/internal/tui/steps/tools_step.go b/forge-cli/internal/tui/steps/tools_step.go deleted file mode 100644 index 5ab4b784..00000000 --- a/forge-cli/internal/tui/steps/tools_step.go +++ /dev/null @@ -1,320 +0,0 @@ -package steps - -import ( - "fmt" - "os" - "strings" - - tea "github.com/charmbracelet/bubbletea" - - "github.com/initializ/forge/forge-cli/internal/tui" - "github.com/initializ/forge/forge-cli/internal/tui/components" -) - -// ToolInfo represents a builtin tool for the tools step. -type ToolInfo struct { - Name string - Description string -} - -// ValidateWebSearchKeyFunc validates a web search API key for a given provider. -type ValidateWebSearchKeyFunc func(provider, key string) error - -type toolsPhase int - -const ( - toolsSelectPhase toolsPhase = iota - toolsWebSearchProviderPhase - toolsWebSearchKeyPhase - toolsWebSearchValidatingPhase - toolsDonePhase -) - -// ToolsStep handles builtin tool selection. -type ToolsStep struct { - styles *tui.StyleSet - phase toolsPhase - multiSelect components.MultiSelect - providerSelect components.SingleSelect - keyInput components.SecretInput - complete bool - selected []string - webSearchKey string - webSearchKeyName string // "TAVILY_API_KEY" or "PERPLEXITY_API_KEY" - webSearchProvider string // "tavily" or "perplexity" - validateFn ValidateWebSearchKeyFunc - validating bool -} - -// NewToolsStep creates a new tools selection step. -func NewToolsStep(styles *tui.StyleSet, tools []ToolInfo, validateFn ValidateWebSearchKeyFunc) *ToolsStep { - var items []components.MultiSelectItem - for _, t := range tools { - icon := toolIcon(t.Name) - items = append(items, components.MultiSelectItem{ - Label: t.Name, - Value: t.Name, - Description: t.Description, - Icon: icon, - }) - } - - ms := components.NewMultiSelect( - items, - styles.Theme.Accent, - styles.Theme.AccentDim, - styles.Theme.Primary, - styles.Theme.Secondary, - styles.Theme.Dim, - styles.ActiveBorder, - styles.InactiveBorder, - styles.KbdKey, - styles.KbdDesc, - ) - - return &ToolsStep{ - styles: styles, - multiSelect: ms, - validateFn: validateFn, - } -} - -func (s *ToolsStep) Title() string { return "Built-in Tools" } -func (s *ToolsStep) Icon() string { return "๐Ÿ”ง" } - -func (s *ToolsStep) Init() tea.Cmd { - return s.multiSelect.Init() -} - -func (s *ToolsStep) Update(msg tea.Msg) (tui.Step, tea.Cmd) { - if s.complete { - return s, nil - } - - if wsm, ok := msg.(tea.WindowSizeMsg); ok { - switch s.phase { - case toolsSelectPhase: - updated, cmd := s.multiSelect.Update(wsm) - s.multiSelect = updated - return s, cmd - case toolsWebSearchProviderPhase: - updated, cmd := s.providerSelect.Update(wsm) - s.providerSelect = updated - return s, cmd - } - return s, nil - } - - switch s.phase { - case toolsSelectPhase: - updated, cmd := s.multiSelect.Update(msg) - s.multiSelect = updated - - if s.multiSelect.Done() { - s.selected = s.multiSelect.SelectedValues() - - // Check if web_search selected and no key is already set - if containsStr(s.selected, "web_search") && - os.Getenv("TAVILY_API_KEY") == "" && - os.Getenv("PERPLEXITY_API_KEY") == "" { - // Show provider selection - s.phase = toolsWebSearchProviderPhase - s.providerSelect = components.NewSingleSelect( - []components.SingleSelectItem{ - {Label: "Tavily (Recommended)", Value: "tavily", Description: "LLM-optimized search with structured results", Icon: "๐Ÿ”"}, - {Label: "Perplexity", Value: "perplexity", Description: "AI-powered search with citations", Icon: "๐ŸŒ"}, - }, - s.styles.Theme.Accent, - s.styles.Theme.Primary, - s.styles.Theme.Secondary, - s.styles.Theme.Dim, - s.styles.Theme.Border, - s.styles.Theme.Accent, - s.styles.Theme.AccentDim, - s.styles.KbdKey, - s.styles.KbdDesc, - ) - return s, s.providerSelect.Init() - } - - // If a key is already set in env, detect the provider - if containsStr(s.selected, "web_search") { - if os.Getenv("TAVILY_API_KEY") != "" { - s.webSearchProvider = "tavily" - } else if os.Getenv("PERPLEXITY_API_KEY") != "" { - s.webSearchProvider = "perplexity" - } - } - - s.complete = true - return s, func() tea.Msg { return tui.StepCompleteMsg{} } - } - - return s, cmd - - case toolsWebSearchProviderPhase: - updated, cmd := s.providerSelect.Update(msg) - s.providerSelect = updated - - if s.providerSelect.Done() { - _, s.webSearchProvider = s.providerSelect.Selected() - s.initKeyInput("") - return s, s.keyInput.Init() - } - - return s, cmd - - case toolsWebSearchKeyPhase: - updated, cmd := s.keyInput.Update(msg) - s.keyInput = updated - - if s.keyInput.Done() { - s.webSearchKey = s.keyInput.Value() - - // Run validation if we have a key and a validateFn - if s.webSearchKey != "" && s.validateFn != nil { - s.phase = toolsWebSearchValidatingPhase - s.validating = true - return s, s.runValidation() - } - - s.complete = true - return s, func() tea.Msg { return tui.StepCompleteMsg{} } - } - - return s, cmd - - case toolsWebSearchValidatingPhase: - if msg, ok := msg.(tui.ValidationResultMsg); ok { - s.validating = false - if msg.Err != nil { - // Validation failed โ€” go back to key input with error - s.initKeyInput(fmt.Sprintf("retry โ€” %s", msg.Err)) - s.keyInput.SetState(components.SecretInputFailed, msg.Err.Error()) - return s, s.keyInput.Init() - } - // Success - s.complete = true - return s, func() tea.Msg { return tui.StepCompleteMsg{} } - } - - return s, nil - } - - return s, nil -} - -// initKeyInput creates a fresh SecretInput for the web search API key. -func (s *ToolsStep) initKeyInput(suffix string) { - keyLabel := "Tavily API key for web_search" - s.webSearchKeyName = "TAVILY_API_KEY" - if s.webSearchProvider == "perplexity" { - keyLabel = "Perplexity API key for web_search" - s.webSearchKeyName = "PERPLEXITY_API_KEY" - } - if suffix != "" { - keyLabel = fmt.Sprintf("%s (%s)", keyLabel, suffix) - } - - s.phase = toolsWebSearchKeyPhase - s.keyInput = components.NewSecretInput( - keyLabel, - false, true, // required โ€” cannot skip; masked - s.styles.Theme.Accent, - s.styles.Theme.Success, - s.styles.Theme.Error, - s.styles.Theme.Border, - s.styles.AccentTxt, - s.styles.InactiveBorder, - s.styles.SuccessTxt, - s.styles.ErrorTxt, - s.styles.DimTxt, - s.styles.KbdKey, - s.styles.KbdDesc, - ) -} - -// runValidation runs the web search key validation asynchronously. -func (s *ToolsStep) runValidation() tea.Cmd { - provider := s.webSearchProvider - key := s.webSearchKey - validateFn := s.validateFn - return func() tea.Msg { - if validateFn == nil { - return tui.ValidationResultMsg{Err: nil} - } - err := validateFn(provider, key) - return tui.ValidationResultMsg{Err: err} - } -} - -func (s *ToolsStep) View(width int) string { - switch s.phase { - case toolsSelectPhase: - return s.multiSelect.View(width) - case toolsWebSearchProviderPhase: - return s.providerSelect.View(width) - case toolsWebSearchKeyPhase: - return s.keyInput.View(width) - case toolsWebSearchValidatingPhase: - if s.validating { - return " " + s.styles.AccentTxt.Render("โฃพ Validating...") + "\n" - } - return s.keyInput.View(width) - } - return "" -} - -func (s *ToolsStep) Complete() bool { - return s.complete -} - -func (s *ToolsStep) Summary() string { - if len(s.selected) == 0 { - return "none" - } - var parts []string - for _, name := range s.selected { - if name == "web_search" && s.webSearchProvider != "" { - parts = append(parts, fmt.Sprintf("web_search [%s]", s.webSearchProvider)) - } else { - parts = append(parts, name) - } - } - return strings.Join(parts, ", ") -} - -func (s *ToolsStep) Apply(ctx *tui.WizardContext) { - ctx.BuiltinTools = s.selected - if s.webSearchKey != "" && s.webSearchKeyName != "" { - ctx.EnvVars[s.webSearchKeyName] = s.webSearchKey - } - if s.webSearchProvider != "" { - ctx.EnvVars["WEB_SEARCH_PROVIDER"] = s.webSearchProvider - } -} - -func toolIcon(name string) string { - icons := map[string]string{ - "http_request": "๐ŸŒ", - "json_parse": "๐Ÿ“‹", - "csv_parse": "๐Ÿ“Š", - "datetime_now": "๐Ÿ•", - "uuid_generate": "๐Ÿ”‘", - "math_calculate": "๐Ÿ”ข", - "web_search": "๐Ÿ”", - } - if icon, ok := icons[name]; ok { - return icon - } - return "๐Ÿ”ง" -} - -func containsStr(slice []string, val string) bool { - for _, s := range slice { - if s == val { - return true - } - } - return false -} From 93246486c5da5e2f0246f18cc7baa19599b129ad Mon Sep 17 00:00:00 2001 From: MK Date: Thu, 9 Jul 2026 21:12:05 -0400 Subject: [PATCH 2/3] feat(tui): add WebSearchStep + rewire init (fix CI: undefined ToolInfo/NewToolsStep) The tools-drop commit only landed the tools_step.go deletion; the replacement WebSearchStep and the init.go rewire that references it were never committed, so the package failed to compile (undefined steps.ToolInfo / steps.NewToolsStep). This adds the missing pieces: - new WebSearchStep: Tavily / Perplexity / skip, validates the key, and auto-adopts a provider key already present in the environment. - init.go: drop the toolInfos build, slot NewWebSearchStep in ChannelStep's place, and guard opts.BuiltinTools so an empty wizard context can't clobber a --tools flag. --- forge-cli/cmd/init.go | 19 +- .../internal/tui/steps/web_search_step.go | 247 ++++++++++++++++++ 2 files changed, 254 insertions(+), 12 deletions(-) create mode 100644 forge-cli/internal/tui/steps/web_search_step.go diff --git a/forge-cli/cmd/init.go b/forge-cli/cmd/init.go index 6956820d..456825bc 100644 --- a/forge-cli/cmd/init.go +++ b/forge-cli/cmd/init.go @@ -241,16 +241,6 @@ func collectInteractive(opts *initOptions) error { theme := tui.DetectTheme(themeOverride) styles := tui.NewStyleSet(theme) - // Load tool info for the tools step - allTools := builtins.All() - var toolInfos []steps.ToolInfo - for _, t := range allTools { - toolInfos = append(toolInfos, steps.ToolInfo{ - Name: t.Name(), - Description: t.Description(), - }) - } - // Load skill info for the skills step var skillInfos []steps.SkillInfo reg, regErr := local.NewEmbeddedRegistry() @@ -325,7 +315,7 @@ func collectInteractive(opts *initOptions) error { steps.NewProviderStep(styles, validateKeyFn, oauthFlowFn), steps.NewFallbackStep(styles, validateKeyFn), steps.NewChannelStep(styles), - steps.NewToolsStep(styles, toolInfos, validateWebSearchKeyFn), + steps.NewWebSearchStep(styles, validateWebSearchKeyFn), steps.NewSkillsStep(styles, skillInfos), steps.NewCompressionStep(styles), steps.NewAuthStep(styles), @@ -375,7 +365,12 @@ func collectInteractive(opts *initOptions) error { opts.Channels = []string{ctx.Channel} } - opts.BuiltinTools = ctx.BuiltinTools + // Only overwrite from the wizard context when it actually collected + // something (e.g. web_search). An empty context must not clobber a + // --tools flag set on the command line. + if len(ctx.BuiltinTools) > 0 { + opts.BuiltinTools = ctx.BuiltinTools + } opts.Skills = ctx.Skills opts.Compression = ctx.Compression diff --git a/forge-cli/internal/tui/steps/web_search_step.go b/forge-cli/internal/tui/steps/web_search_step.go new file mode 100644 index 00000000..7e271bea --- /dev/null +++ b/forge-cli/internal/tui/steps/web_search_step.go @@ -0,0 +1,247 @@ +package steps + +import ( + "fmt" + "os" + + tea "github.com/charmbracelet/bubbletea" + + "github.com/initializ/forge/forge-cli/internal/tui" + "github.com/initializ/forge/forge-cli/internal/tui/components" +) + +// ValidateWebSearchKeyFunc validates a web search API key for a given provider. +type ValidateWebSearchKeyFunc func(provider, key string) error + +type webSearchPhase int + +const ( + webSearchChoosePhase webSearchPhase = iota + webSearchKeyPhase + webSearchValidatingPhase +) + +// WebSearchStep configures the built-in web_search tool: pick a provider +// (Tavily / Perplexity) or skip, and capture + validate its API key. Every +// builtin tool is auto-registered at runtime; web_search is the only one that +// needs operator input, so it gets a focused step of its own rather than being +// buried in a bulk tool checklist. +type WebSearchStep struct { + styles *tui.StyleSet + phase webSearchPhase + choose components.SingleSelect + keyInput components.SecretInput + complete bool + validating bool + + provider string // "tavily" or "perplexity"; "" when web search is disabled + keyName string // "TAVILY_API_KEY" or "PERPLEXITY_API_KEY" + key string + keyFromEnv bool + + validateFn ValidateWebSearchKeyFunc +} + +// NewWebSearchStep creates a new web search configuration step. +func NewWebSearchStep(styles *tui.StyleSet, validateFn ValidateWebSearchKeyFunc) *WebSearchStep { + choose := components.NewSingleSelect( + []components.SingleSelectItem{ + {Label: "Tavily (Recommended)", Value: "tavily", Description: "LLM-optimized search with structured results", Icon: "๐Ÿ”"}, + {Label: "Perplexity", Value: "perplexity", Description: "AI-powered search with citations", Icon: "๐ŸŒ"}, + {Label: "No web search", Value: "", Description: "Skip โ€” agents have no live web access", Icon: "๐Ÿšซ"}, + }, + styles.Theme.Accent, + styles.Theme.Primary, + styles.Theme.Secondary, + styles.Theme.Dim, + styles.Theme.Border, + styles.Theme.Accent, + styles.Theme.AccentDim, + styles.KbdKey, + styles.KbdDesc, + ) + + return &WebSearchStep{ + styles: styles, + choose: choose, + validateFn: validateFn, + } +} + +func (s *WebSearchStep) Title() string { return "Web Search" } +func (s *WebSearchStep) Icon() string { return "๐Ÿ”" } + +func (s *WebSearchStep) Init() tea.Cmd { + return s.choose.Init() +} + +func (s *WebSearchStep) Update(msg tea.Msg) (tui.Step, tea.Cmd) { + if s.complete { + return s, nil + } + + if wsm, ok := msg.(tea.WindowSizeMsg); ok { + switch s.phase { + case webSearchChoosePhase: + updated, cmd := s.choose.Update(wsm) + s.choose = updated + return s, cmd + case webSearchKeyPhase: + updated, cmd := s.keyInput.Update(wsm) + s.keyInput = updated + return s, cmd + } + return s, nil + } + + switch s.phase { + case webSearchChoosePhase: + updated, cmd := s.choose.Update(msg) + s.choose = updated + + if s.choose.Done() { + _, s.provider = s.choose.Selected() + + // "No web search" โ€” nothing more to configure. + if s.provider == "" { + return s.finish() + } + + s.keyName = "TAVILY_API_KEY" + if s.provider == "perplexity" { + s.keyName = "PERPLEXITY_API_KEY" + } + + // If the provider key is already in the environment, adopt it + // without prompting. + if os.Getenv(s.keyName) != "" { + s.keyFromEnv = true + return s.finish() + } + + s.initKeyInput("") + return s, s.keyInput.Init() + } + + return s, cmd + + case webSearchKeyPhase: + updated, cmd := s.keyInput.Update(msg) + s.keyInput = updated + + if s.keyInput.Done() { + s.key = s.keyInput.Value() + + if s.key != "" && s.validateFn != nil { + s.phase = webSearchValidatingPhase + s.validating = true + return s, s.runValidation() + } + + return s.finish() + } + + return s, cmd + + case webSearchValidatingPhase: + if msg, ok := msg.(tui.ValidationResultMsg); ok { + s.validating = false + if msg.Err != nil { + // Validation failed โ€” return to key input with the error. + s.initKeyInput(fmt.Sprintf("retry โ€” %s", msg.Err)) + s.keyInput.SetState(components.SecretInputFailed, msg.Err.Error()) + return s, s.keyInput.Init() + } + return s.finish() + } + return s, nil + } + + return s, nil +} + +func (s *WebSearchStep) finish() (tui.Step, tea.Cmd) { + s.complete = true + return s, func() tea.Msg { return tui.StepCompleteMsg{} } +} + +// initKeyInput creates a fresh SecretInput for the provider API key. +func (s *WebSearchStep) initKeyInput(suffix string) { + keyLabel := "Tavily API key for web_search" + if s.provider == "perplexity" { + keyLabel = "Perplexity API key for web_search" + } + if suffix != "" { + keyLabel = fmt.Sprintf("%s (%s)", keyLabel, suffix) + } + + s.phase = webSearchKeyPhase + s.keyInput = components.NewSecretInput( + keyLabel, + false, true, // required โ€” cannot skip; masked + s.styles.Theme.Accent, + s.styles.Theme.Success, + s.styles.Theme.Error, + s.styles.Theme.Border, + s.styles.AccentTxt, + s.styles.InactiveBorder, + s.styles.SuccessTxt, + s.styles.ErrorTxt, + s.styles.DimTxt, + s.styles.KbdKey, + s.styles.KbdDesc, + ) +} + +// runValidation runs the web search key validation asynchronously. +func (s *WebSearchStep) runValidation() tea.Cmd { + provider := s.provider + key := s.key + validateFn := s.validateFn + return func() tea.Msg { + if validateFn == nil { + return tui.ValidationResultMsg{Err: nil} + } + return tui.ValidationResultMsg{Err: validateFn(provider, key)} + } +} + +func (s *WebSearchStep) View(width int) string { + switch s.phase { + case webSearchChoosePhase: + return s.choose.View(width) + case webSearchKeyPhase: + return s.keyInput.View(width) + case webSearchValidatingPhase: + if s.validating { + return " " + s.styles.AccentTxt.Render("โฃพ Validating...") + "\n" + } + return s.keyInput.View(width) + } + return "" +} + +func (s *WebSearchStep) Complete() bool { return s.complete } + +func (s *WebSearchStep) Summary() string { + if s.provider == "" { + return "disabled" + } + if s.keyFromEnv { + return fmt.Sprintf("%s [key from env]", s.provider) + } + return s.provider +} + +func (s *WebSearchStep) Apply(ctx *tui.WizardContext) { + if s.provider == "" { + return + } + // web_search is auto-registered; recording it in BuiltinTools keeps the + // review/summary and any policy interaction consistent with the choice. + ctx.BuiltinTools = append(ctx.BuiltinTools, "web_search") + ctx.EnvVars["WEB_SEARCH_PROVIDER"] = s.provider + if s.key != "" && s.keyName != "" { + ctx.EnvVars[s.keyName] = s.key + } +} From 446dfe93c54ed6b76776343144f20e93e2a9a23b Mon Sep 17 00:00:00 2001 From: MK Date: Sun, 12 Jul 2026 22:15:12 -0400 Subject: [PATCH 3/3] fix(tui): merge --tools flag with wizard web-search selection; add tests (#263 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blocking fix from the #263 review: the clobber guard only protected the SKIP path. When the user picked a web-search provider interactively, ctx.BuiltinTools was [web_search] and REPLACED a --tools flag value โ€” dropping the flag's other entries from forge.yaml and the init_egress DefaultToolDomains derivation. - Extract mergeBuiltinTools(flag, fromWizard): union, first-seen order, dedupe, drop empties โ€” used in collectInteractive instead of the overwrite. Table test pins all three paths (flag+skip, flag+provider, no-flag+provider) plus dedupe/empty edges. - web_search_step: annotate a provider whose key is already in the environment ("ยท key detected in env") so the user knows selecting it adopts that key and picking the other provider forgoes it (review UX nit). - Add WebSearchStep.Apply test covering the three behaviors: skip writes nothing; chosen provider writes WEB_SEARCH_PROVIDER + web_search + key; env-key path writes the provider but not the key. --- forge-cli/cmd/init.go | 34 ++++++++++-- forge-cli/cmd/init_test.go | 32 +++++++++++ .../internal/tui/steps/web_search_step.go | 16 +++++- .../tui/steps/web_search_step_test.go | 55 +++++++++++++++++++ 4 files changed, 129 insertions(+), 8 deletions(-) create mode 100644 forge-cli/internal/tui/steps/web_search_step_test.go diff --git a/forge-cli/cmd/init.go b/forge-cli/cmd/init.go index 456825bc..5500ce06 100644 --- a/forge-cli/cmd/init.go +++ b/forge-cli/cmd/init.go @@ -365,12 +365,13 @@ func collectInteractive(opts *initOptions) error { opts.Channels = []string{ctx.Channel} } - // Only overwrite from the wizard context when it actually collected - // something (e.g. web_search). An empty context must not clobber a - // --tools flag set on the command line. - if len(ctx.BuiltinTools) > 0 { - opts.BuiltinTools = ctx.BuiltinTools - } + // MERGE the wizard's builtin-tool selection into any --tools flag rather + // than overwriting it. The wizard only records web_search today; a bare + // overwrite would drop the flag's other entries from both forge.yaml and + // the init_egress DefaultToolDomains derivation whenever the user picked a + // provider. Merge + dedupe covers all three paths (flag+skip, flag+ + // provider, no-flag+provider). See #263 review. + opts.BuiltinTools = mergeBuiltinTools(opts.BuiltinTools, ctx.BuiltinTools) opts.Skills = ctx.Skills opts.Compression = ctx.Compression @@ -1412,6 +1413,27 @@ func containsStr(slice []string, val string) bool { return false } +// mergeBuiltinTools returns the union of a --tools flag value and the wizard's +// recorded builtin-tool selection, preserving first-seen order and dropping +// duplicates and empty entries. Merging (not overwriting) keeps a flag entry +// like http_request when the wizard also records web_search โ€” both must reach +// forge.yaml and the init_egress domain derivation. See #263 review. +func mergeBuiltinTools(flag, fromWizard []string) []string { + seen := make(map[string]struct{}, len(flag)+len(fromWizard)) + out := make([]string, 0, len(flag)+len(fromWizard)) + for _, name := range append(append([]string{}, flag...), fromWizard...) { + if name == "" { + continue + } + if _, dup := seen[name]; dup { + continue + } + seen[name] = struct{}{} + out = append(out, name) + } + return out +} + // runOAuthFlow executes the OAuth browser flow for a provider and returns the access token. func runOAuthFlow(provider string) (string, error) { var config oauth.ProviderConfig diff --git a/forge-cli/cmd/init_test.go b/forge-cli/cmd/init_test.go index f6463131..50ba3a08 100644 --- a/forge-cli/cmd/init_test.go +++ b/forge-cli/cmd/init_test.go @@ -951,3 +951,35 @@ func writeTempFile(t *testing.T, name, content string) string { } return path } + +// TestMergeBuiltinTools pins the three paths the #263 review called out: +// a --tools flag merges with the wizard's web_search selection instead of +// being clobbered, empties are dropped, and duplicates are deduped. +func TestMergeBuiltinTools(t *testing.T) { + cases := []struct { + name string + flag []string + fromWizard []string + want []string + }{ + {"flag+skip (wizard records nothing)", []string{"http_request"}, nil, []string{"http_request"}}, + {"flag+provider (must not clobber the flag)", []string{"http_request"}, []string{"web_search"}, []string{"http_request", "web_search"}}, + {"no-flag+provider", nil, []string{"web_search"}, []string{"web_search"}}, + {"dedupe overlap", []string{"web_search", "http_request"}, []string{"web_search"}, []string{"web_search", "http_request"}}, + {"drops empties", []string{"", "http_request"}, []string{""}, []string{"http_request"}}, + {"both empty", nil, nil, []string{}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := mergeBuiltinTools(tc.flag, tc.fromWizard) + if len(got) != len(tc.want) { + t.Fatalf("got %v, want %v", got, tc.want) + } + for i := range tc.want { + if got[i] != tc.want[i] { + t.Fatalf("got %v, want %v (order matters)", got, tc.want) + } + } + }) + } +} diff --git a/forge-cli/internal/tui/steps/web_search_step.go b/forge-cli/internal/tui/steps/web_search_step.go index 7e271bea..df48bf06 100644 --- a/forge-cli/internal/tui/steps/web_search_step.go +++ b/forge-cli/internal/tui/steps/web_search_step.go @@ -44,10 +44,22 @@ type WebSearchStep struct { // NewWebSearchStep creates a new web search configuration step. func NewWebSearchStep(styles *tui.StyleSet, validateFn ValidateWebSearchKeyFunc) *WebSearchStep { + // Annotate a provider whose key is already in the environment so the user + // knows selecting it will silently adopt that key (and picking the other + // provider forgoes an already-available key). See #263 review. + tavilyLabel := "Tavily (Recommended)" + if os.Getenv("TAVILY_API_KEY") != "" { + tavilyLabel += " ยท key detected in env" + } + perplexityLabel := "Perplexity" + if os.Getenv("PERPLEXITY_API_KEY") != "" { + perplexityLabel += " ยท key detected in env" + } + choose := components.NewSingleSelect( []components.SingleSelectItem{ - {Label: "Tavily (Recommended)", Value: "tavily", Description: "LLM-optimized search with structured results", Icon: "๐Ÿ”"}, - {Label: "Perplexity", Value: "perplexity", Description: "AI-powered search with citations", Icon: "๐ŸŒ"}, + {Label: tavilyLabel, Value: "tavily", Description: "LLM-optimized search with structured results", Icon: "๐Ÿ”"}, + {Label: perplexityLabel, Value: "perplexity", Description: "AI-powered search with citations", Icon: "๐ŸŒ"}, {Label: "No web search", Value: "", Description: "Skip โ€” agents have no live web access", Icon: "๐Ÿšซ"}, }, styles.Theme.Accent, diff --git a/forge-cli/internal/tui/steps/web_search_step_test.go b/forge-cli/internal/tui/steps/web_search_step_test.go new file mode 100644 index 00000000..f3e7d2f9 --- /dev/null +++ b/forge-cli/internal/tui/steps/web_search_step_test.go @@ -0,0 +1,55 @@ +package steps + +import ( + "slices" + "testing" + + "github.com/initializ/forge/forge-cli/internal/tui" +) + +// TestWebSearchStep_Apply pins the three meaningful Apply behaviors the #263 +// review flagged: skip writes nothing; a chosen provider writes the provider +// env var + BuiltinTools + key; the env-key path writes the provider but no +// key (it was already in the environment). +func TestWebSearchStep_Apply(t *testing.T) { + t.Run("skip writes nothing", func(t *testing.T) { + ctx := tui.NewWizardContext() + s := &WebSearchStep{provider: ""} // "No web search" + s.Apply(ctx) + if len(ctx.BuiltinTools) != 0 { + t.Errorf("skip must not record a builtin tool, got %v", ctx.BuiltinTools) + } + if len(ctx.EnvVars) != 0 { + t.Errorf("skip must not write env vars, got %v", ctx.EnvVars) + } + }) + + t.Run("chosen provider writes provider + tool + key", func(t *testing.T) { + ctx := tui.NewWizardContext() + s := &WebSearchStep{provider: "tavily", keyName: "TAVILY_API_KEY", key: "tvly-secret"} + s.Apply(ctx) + if !slices.Contains(ctx.BuiltinTools, "web_search") { + t.Errorf("expected web_search recorded, got %v", ctx.BuiltinTools) + } + if ctx.EnvVars["WEB_SEARCH_PROVIDER"] != "tavily" { + t.Errorf("WEB_SEARCH_PROVIDER = %q", ctx.EnvVars["WEB_SEARCH_PROVIDER"]) + } + if ctx.EnvVars["TAVILY_API_KEY"] != "tvly-secret" { + t.Errorf("expected the key written, got %q", ctx.EnvVars["TAVILY_API_KEY"]) + } + }) + + t.Run("env-key path writes provider but no key", func(t *testing.T) { + ctx := tui.NewWizardContext() + // keyFromEnv: the key is already in the environment, so Apply records + // the provider + tool but must NOT re-write the key value (it's ""). + s := &WebSearchStep{provider: "perplexity", keyName: "PERPLEXITY_API_KEY", key: "", keyFromEnv: true} + s.Apply(ctx) + if ctx.EnvVars["WEB_SEARCH_PROVIDER"] != "perplexity" { + t.Errorf("WEB_SEARCH_PROVIDER = %q", ctx.EnvVars["WEB_SEARCH_PROVIDER"]) + } + if _, wrote := ctx.EnvVars["PERPLEXITY_API_KEY"]; wrote { + t.Errorf("env-key path must not write the key, got %v", ctx.EnvVars) + } + }) +}