From bb867c6fdbc4739751f0fc6d8fd2ade20684041d Mon Sep 17 00:00:00 2001 From: Parth Sareen Date: Fri, 13 Mar 2026 11:45:36 -0700 Subject: [PATCH] launch: fix headless --yes integration flow and policy scoping (#14815) --- cmd/launch/command_test.go | 104 ++++++++++++++++++++++ cmd/launch/launch.go | 90 ++++++++++--------- cmd/launch/launch_test.go | 166 +++++++++++++++++++++++++++++++++++ cmd/launch/selector_hooks.go | 18 ++-- cmd/launch/selector_test.go | 2 +- 5 files changed, 320 insertions(+), 60 deletions(-) diff --git a/cmd/launch/command_test.go b/cmd/launch/command_test.go index cb90bb3d1..632ae459f 100644 --- a/cmd/launch/command_test.go +++ b/cmd/launch/command_test.go @@ -160,6 +160,27 @@ func TestLaunchCmdTUICallback(t *testing.T) { } }) + t.Run("--yes flag without integration returns error", func(t *testing.T) { + tuiCalled := false + mockTUI := func(cmd *cobra.Command) { + tuiCalled = true + } + + cmd := LaunchCmd(mockCheck, mockTUI) + cmd.SetArgs([]string{"--yes"}) + err := cmd.Execute() + + if err == nil { + t.Fatal("expected --yes without an integration to fail") + } + if !strings.Contains(err.Error(), "require an integration name") { + t.Fatalf("expected integration-name guidance, got %v", err) + } + if tuiCalled { + t.Error("TUI callback should NOT be called when --yes is provided without an integration") + } + }) + t.Run("extra args without integration return error", func(t *testing.T) { tuiCalled := false mockTUI := func(cmd *cobra.Command) { @@ -492,3 +513,86 @@ func TestLaunchCmdIntegrationArgPromptsForModelWithSavedSelection(t *testing.T) t.Fatalf("saved models mismatch (-want +got):\n%s", diff) } } + +func TestLaunchCmdHeadlessYes_IntegrationRequiresModelEvenWhenSaved(t *testing.T) { + tmpDir := t.TempDir() + setLaunchTestHome(t, tmpDir) + withLauncherHooks(t) + withInteractiveSession(t, false) + + if err := config.SaveIntegration("stubapp", []string{"llama3.2"}); err != nil { + t.Fatalf("failed to seed saved config: %v", err) + } + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/show": + fmt.Fprint(w, `{"model":"llama3.2"}`) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer srv.Close() + t.Setenv("OLLAMA_HOST", srv.URL) + + stub := &launcherSingleRunner{} + restore := OverrideIntegration("stubapp", stub) + defer restore() + + oldSelector := DefaultSingleSelector + defer func() { DefaultSingleSelector = oldSelector }() + DefaultSingleSelector = func(title string, items []ModelItem, current string) (string, error) { + t.Fatal("selector should not be called for headless --yes saved-model launch") + return "", nil + } + + cmd := LaunchCmd(func(cmd *cobra.Command, args []string) error { return nil }, func(cmd *cobra.Command) {}) + cmd.SetArgs([]string{"stubapp", "--yes"}) + err := cmd.Execute() + if err == nil { + t.Fatal("expected launch command to fail when --yes is used headlessly without --model") + } + if !strings.Contains(err.Error(), "requires --model ") { + t.Fatalf("expected actionable --model guidance, got %v", err) + } + if stub.ranModel != "" { + t.Fatalf("expected launch to abort before run, got %q", stub.ranModel) + } +} + +func TestLaunchCmdHeadlessYes_IntegrationWithoutSavedModelReturnsError(t *testing.T) { + tmpDir := t.TempDir() + setLaunchTestHome(t, tmpDir) + withLauncherHooks(t) + withInteractiveSession(t, false) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + defer srv.Close() + t.Setenv("OLLAMA_HOST", srv.URL) + + stub := &launcherSingleRunner{} + restore := OverrideIntegration("stubapp", stub) + defer restore() + + oldSelector := DefaultSingleSelector + defer func() { DefaultSingleSelector = oldSelector }() + DefaultSingleSelector = func(title string, items []ModelItem, current string) (string, error) { + t.Fatal("selector should not be called for headless --yes without saved model") + return "", nil + } + + cmd := LaunchCmd(func(cmd *cobra.Command, args []string) error { return nil }, func(cmd *cobra.Command) {}) + cmd.SetArgs([]string{"stubapp", "--yes"}) + err := cmd.Execute() + if err == nil { + t.Fatal("expected launch command to fail when --yes is used headlessly without --model") + } + if !strings.Contains(err.Error(), "requires --model ") { + t.Fatalf("expected actionable --model guidance, got %v", err) + } + if stub.ranModel != "" { + t.Fatalf("expected launch to abort before run, got %q", stub.ranModel) + } +} diff --git a/cmd/launch/launch.go b/cmd/launch/launch.go index 2109b05f7..8d22c785f 100644 --- a/cmd/launch/launch.go +++ b/cmd/launch/launch.go @@ -40,6 +40,7 @@ type LauncherIntegrationState struct { // RunModelRequest controls how the root launcher resolves the chat model. type RunModelRequest struct { ForcePicker bool + Policy *LaunchPolicy } // LaunchConfirmMode controls confirmation behavior across launch flows. @@ -72,12 +73,18 @@ type LaunchPolicy struct { MissingModel LaunchMissingModelMode } -func defaultLaunchPolicy(interactive bool) LaunchPolicy { +func defaultLaunchPolicy(interactive bool, yes bool) LaunchPolicy { policy := LaunchPolicy{ Confirm: LaunchConfirmPrompt, MissingModel: LaunchMissingModelPromptToPull, } - if !interactive { + switch { + case yes: + // if yes flag is set, auto approve and auto pull + policy.Confirm = LaunchConfirmAutoApprove + policy.MissingModel = LaunchMissingModelAutoPull + case !interactive: + // otherwise make sure to stop when needed policy.Confirm = LaunchConfirmRequireYes policy.MissingModel = LaunchMissingModelFail } @@ -149,37 +156,6 @@ type ModelItem struct { Recommended bool } -// ConfigureIntegrationWithSelectors allows the user to select/change the model for an integration using custom selectors. -func ConfigureIntegrationWithSelectors(ctx context.Context, name string, single SingleSelector, multi MultiSelector) error { - oldSingle := DefaultSingleSelector - oldMulti := DefaultMultiSelector - if single != nil { - DefaultSingleSelector = single - } - if multi != nil { - DefaultMultiSelector = multi - } - defer func() { - DefaultSingleSelector = oldSingle - DefaultMultiSelector = oldMulti - }() - - return LaunchIntegration(ctx, IntegrationLaunchRequest{ - Name: name, - ForceConfigure: true, - ConfigureOnly: true, - }) -} - -// ConfigureIntegration allows the user to select/change the model for an integration. -func ConfigureIntegration(ctx context.Context, name string) error { - return LaunchIntegration(ctx, IntegrationLaunchRequest{ - Name: name, - ForceConfigure: true, - ConfigureOnly: true, - }) -} - // LaunchCmd returns the cobra command for launching integrations. // The runTUI callback is called when the root launcher UI should be shown. func LaunchCmd(checkServerHeartbeat func(cmd *cobra.Command, args []string) error, runTUI func(cmd *cobra.Command)) *cobra.Command { @@ -214,13 +190,8 @@ Examples: Args: cobra.ArbitraryArgs, PreRunE: checkServerHeartbeat, RunE: func(cmd *cobra.Command, args []string) error { - policy := defaultLaunchPolicy(isInteractiveSession()) - if yesFlag { - policy.Confirm = LaunchConfirmAutoApprove - if policy.MissingModel == LaunchMissingModelFail { - policy.MissingModel = LaunchMissingModelAutoPull - } - } + policy := defaultLaunchPolicy(isInteractiveSession(), yesFlag) + // reset when done to make sure state doens't leak between launches restoreConfirmPolicy := withLaunchConfirmPolicy(policy.confirmPolicy()) defer restoreConfirmPolicy() @@ -246,7 +217,7 @@ Examples: } if name == "" { - if cmd.Flags().Changed("model") || cmd.Flags().Changed("config") || len(passArgs) > 0 { + if cmd.Flags().Changed("model") || cmd.Flags().Changed("config") || cmd.Flags().Changed("yes") || len(passArgs) > 0 { return fmt.Errorf("flags and extra args require an integration name, for example: 'ollama launch claude --model qwen3.5'") } runTUI(cmd) @@ -262,10 +233,11 @@ Examples: } } + headlessYes := yesFlag && !isInteractiveSession() err := LaunchIntegration(cmd.Context(), IntegrationLaunchRequest{ Name: name, ModelOverride: modelFlag, - ForceConfigure: configFlag || modelFlag == "", + ForceConfigure: configFlag || (modelFlag == "" && !headlessYes), ConfigureOnly: configFlag, ExtraArgs: passArgs, Policy: &policy, @@ -304,7 +276,7 @@ func newLauncherClient(policy LaunchPolicy) (*launcherClient, error) { // BuildLauncherState returns the launch-owned root launcher menu snapshot. func BuildLauncherState(ctx context.Context) (*LauncherState, error) { - launchClient, err := newLauncherClient(defaultLaunchPolicy(isInteractiveSession())) + launchClient, err := newLauncherClient(defaultLaunchPolicy(isInteractiveSession(), false)) if err != nil { return nil, err } @@ -313,7 +285,15 @@ func BuildLauncherState(ctx context.Context) (*LauncherState, error) { // ResolveRunModel returns the model that should be used for interactive chat. func ResolveRunModel(ctx context.Context, req RunModelRequest) (string, error) { - launchClient, err := newLauncherClient(defaultLaunchPolicy(isInteractiveSession())) + // Called by the launcher TUI "Run a model" action (cmd/runLauncherAction), + // which resolves models separately from LaunchIntegration. Callers can pass + // Policy directly; otherwise we fall back to ambient --yes/session defaults. + policy := defaultLaunchPolicy(isInteractiveSession(), currentLaunchConfirmPolicy.yes) + if req.Policy != nil { + policy = *req.Policy + } + + launchClient, err := newLauncherClient(policy) if err != nil { return "", err } @@ -332,8 +312,11 @@ func LaunchIntegration(ctx context.Context, req IntegrationLaunchRequest) error } } - policy := defaultLaunchPolicy(isInteractiveSession()) - if req.Policy != nil { + var policy LaunchPolicy + if req.Policy == nil { + policy = defaultLaunchPolicy(isInteractiveSession(), false) + fmt.Fprintln(os.Stderr, "Launch policy not found from request, using default") + } else { policy = *req.Policy } @@ -342,6 +325,10 @@ func LaunchIntegration(ctx context.Context, req IntegrationLaunchRequest) error return err } saved, _ := loadStoredIntegrationConfig(name) + // In headless --yes mode we cannot prompt, so require an explicit --model. + if policy.Confirm == LaunchConfirmAutoApprove && !isInteractiveSession() && req.ModelOverride == "" { + return fmt.Errorf("headless --yes launch for %s requires --model ", name) + } if editor, ok := runner.(Editor); ok { return launchClient.launchEditorIntegration(ctx, name, runner, editor, saved, req) @@ -421,6 +408,17 @@ func (c *launcherClient) launcherModelState(ctx context.Context, name string, is func (c *launcherClient) resolveRunModel(ctx context.Context, req RunModelRequest) (string, error) { current := config.LastModel() + if !req.ForcePicker && current != "" && c.policy.Confirm == LaunchConfirmAutoApprove && !isInteractiveSession() { + if err := c.ensureModelsReady(ctx, []string{current}); err != nil { + return "", err + } + fmt.Fprintf(os.Stderr, "Headless mode: auto-selected last used model %q\n", current) + if err := config.SetLastModel(current); err != nil { + return "", err + } + return current, nil + } + if !req.ForcePicker { usable, err := c.savedModelUsable(ctx, current) if err != nil { diff --git a/cmd/launch/launch_test.go b/cmd/launch/launch_test.go index 3c77bd0f2..f9a9f79a9 100644 --- a/cmd/launch/launch_test.go +++ b/cmd/launch/launch_test.go @@ -98,6 +98,49 @@ func withLauncherHooks(t *testing.T) { }) } +func TestDefaultLaunchPolicy(t *testing.T) { + tests := []struct { + name string + interactive bool + yes bool + want LaunchPolicy + }{ + { + name: "interactive default prompts and prompt-pull", + interactive: true, + yes: false, + want: LaunchPolicy{Confirm: LaunchConfirmPrompt, MissingModel: LaunchMissingModelPromptToPull}, + }, + { + name: "headless without yes requires yes and fail-missing", + interactive: false, + yes: false, + want: LaunchPolicy{Confirm: LaunchConfirmRequireYes, MissingModel: LaunchMissingModelFail}, + }, + { + name: "interactive yes auto-approves and auto-pulls", + interactive: true, + yes: true, + want: LaunchPolicy{Confirm: LaunchConfirmAutoApprove, MissingModel: LaunchMissingModelAutoPull}, + }, + { + name: "headless yes auto-approves and auto-pulls", + interactive: false, + yes: true, + want: LaunchPolicy{Confirm: LaunchConfirmAutoApprove, MissingModel: LaunchMissingModelAutoPull}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := defaultLaunchPolicy(tt.interactive, tt.yes) + if got != tt.want { + t.Fatalf("defaultLaunchPolicy(%v, %v) = %+v, want %+v", tt.interactive, tt.yes, got, tt.want) + } + }) + } +} + func TestBuildLauncherState_InstalledAndCloudDisabled(t *testing.T) { tmpDir := t.TempDir() setLaunchTestHome(t, tmpDir) @@ -284,6 +327,129 @@ func TestResolveRunModel_UsesSavedModelWithoutSelector(t *testing.T) { } } +func TestResolveRunModel_HeadlessYesAutoPicksLastModel(t *testing.T) { + tmpDir := t.TempDir() + setLaunchTestHome(t, tmpDir) + withLauncherHooks(t) + withInteractiveSession(t, false) + + if err := config.SetLastModel("missing-model"); err != nil { + t.Fatalf("failed to save last model: %v", err) + } + + DefaultSingleSelector = func(title string, items []ModelItem, current string) (string, error) { + t.Fatal("selector should not be called in headless --yes mode") + return "", nil + } + + restoreConfirm := withLaunchConfirmPolicy(launchConfirmPolicy{yes: true}) + defer restoreConfirm() + + pullCalled := false + modelPulled := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/tags": + fmt.Fprint(w, `{"models":[{"name":"llama3.2"}]}`) + case "/api/show": + var req apiShowRequest + _ = json.NewDecoder(r.Body).Decode(&req) + if req.Model == "missing-model" && !modelPulled { + w.WriteHeader(http.StatusNotFound) + fmt.Fprint(w, `{"error":"model not found"}`) + return + } + fmt.Fprintf(w, `{"model":%q}`, req.Model) + case "/api/pull": + pullCalled = true + modelPulled = true + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, `{"status":"success"}`) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + t.Setenv("OLLAMA_HOST", srv.URL) + + var model string + stderr := captureStderr(t, func() { + var err error + model, err = ResolveRunModel(context.Background(), RunModelRequest{}) + if err != nil { + t.Fatalf("ResolveRunModel returned error: %v", err) + } + }) + + if model != "missing-model" { + t.Fatalf("expected saved last model to be selected, got %q", model) + } + if !pullCalled { + t.Fatal("expected missing saved model to be auto-pulled in headless --yes mode") + } + if !strings.Contains(stderr, `Headless mode: auto-selected last used model "missing-model"`) { + t.Fatalf("expected headless auto-pick message in stderr, got %q", stderr) + } +} + +func TestResolveRunModel_UsesRequestPolicy(t *testing.T) { + tmpDir := t.TempDir() + setLaunchTestHome(t, tmpDir) + withLauncherHooks(t) + withInteractiveSession(t, false) + + if err := config.SetLastModel("missing-model"); err != nil { + t.Fatalf("failed to save last model: %v", err) + } + + DefaultSingleSelector = func(title string, items []ModelItem, current string) (string, error) { + t.Fatal("selector should not be called when request policy enables headless auto-pick") + return "", nil + } + + pullCalled := false + modelPulled := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/tags": + fmt.Fprint(w, `{"models":[{"name":"llama3.2"}]}`) + case "/api/show": + var req apiShowRequest + _ = json.NewDecoder(r.Body).Decode(&req) + if req.Model == "missing-model" && !modelPulled { + w.WriteHeader(http.StatusNotFound) + fmt.Fprint(w, `{"error":"model not found"}`) + return + } + fmt.Fprintf(w, `{"model":%q}`, req.Model) + case "/api/pull": + pullCalled = true + modelPulled = true + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, `{"status":"success"}`) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + t.Setenv("OLLAMA_HOST", srv.URL) + + reqPolicy := LaunchPolicy{ + Confirm: LaunchConfirmAutoApprove, + MissingModel: LaunchMissingModelAutoPull, + } + model, err := ResolveRunModel(context.Background(), RunModelRequest{Policy: &reqPolicy}) + if err != nil { + t.Fatalf("ResolveRunModel returned error: %v", err) + } + if model != "missing-model" { + t.Fatalf("expected saved last model to be selected, got %q", model) + } + if !pullCalled { + t.Fatal("expected missing saved model to be auto-pulled when request policy enables auto-pull") + } +} + func TestResolveRunModel_ForcePickerAlwaysUsesSelector(t *testing.T) { tmpDir := t.TempDir() setLaunchTestHome(t, tmpDir) diff --git a/cmd/launch/selector_hooks.go b/cmd/launch/selector_hooks.go index 1204023bb..8710bb4f8 100644 --- a/cmd/launch/selector_hooks.go +++ b/cmd/launch/selector_hooks.go @@ -52,26 +52,18 @@ type launchConfirmPolicy struct { var currentLaunchConfirmPolicy launchConfirmPolicy -func (p launchConfirmPolicy) chain(next launchConfirmPolicy) launchConfirmPolicy { - chained := launchConfirmPolicy{ - yes: p.yes || next.yes, - requireYesMessage: p.requireYesMessage || next.requireYesMessage, - } - if chained.yes { - chained.requireYesMessage = false - } - return chained -} - func withLaunchConfirmPolicy(policy launchConfirmPolicy) func() { old := currentLaunchConfirmPolicy - currentLaunchConfirmPolicy = old.chain(policy) + currentLaunchConfirmPolicy = policy return func() { currentLaunchConfirmPolicy = old } } -// ConfirmPrompt asks the user to confirm an action using the configured prompt hook. +// ConfirmPrompt is the shared confirmation gate for launch flows (integration +// edits, missing-model pulls, sign-in prompts, OpenClaw install/security, etc). +// Behavior is controlled by currentLaunchConfirmPolicy, typically scoped by +// withLaunchConfirmPolicy in LaunchCmd (e.g. auto-approve with --yes). func ConfirmPrompt(prompt string) (bool, error) { if currentLaunchConfirmPolicy.yes { return true, nil diff --git a/cmd/launch/selector_test.go b/cmd/launch/selector_test.go index 92fa28460..fa6a635b0 100644 --- a/cmd/launch/selector_test.go +++ b/cmd/launch/selector_test.go @@ -19,7 +19,7 @@ func TestErrCancelled(t *testing.T) { }) } -func TestWithLaunchConfirmPolicy_ChainsAndRestores(t *testing.T) { +func TestWithLaunchConfirmPolicy_ScopesAndRestores(t *testing.T) { oldPolicy := currentLaunchConfirmPolicy oldHook := DefaultConfirmPrompt t.Cleanup(func() {