+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *
+ * http://www.apache.org/licenses/LICENSE-2.0 + *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package cmd + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/conductor-oss/conductor-cli/internal" + "github.com/conductor-oss/conductor-cli/internal/transport" +) + +// pointTransportAt swaps the shared transport for the duration of a test. +func pointTransportAt(t *testing.T, status int, body string) { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, _ = w.Write([]byte(body)) + })) + previous := internal.Transport() + internal.SetTransport(transport.Config{BaseURL: srv.URL + "/api"}) + t.Cleanup(func() { + srv.Close() + internal.SetTransport(previous) + }) +} + +func TestResolveInitModelReturnsAnExplicitModelVerbatim(t *testing.T) { + // Pointed at a server that would fail the lookup: an explicit model must not + // depend on the server being reachable at all. + pointTransportAt(t, http.StatusInternalServerError, `{"message":"boom"}`) + + got, err := resolveInitModel(context.Background(), "anthropic/some-model") + if err != nil { + t.Fatalf("resolveInitModel: %v", err) + } + if got != "anthropic/some-model" { + t.Errorf("model = %q, want it passed through unchanged", got) + } +} + +func TestResolveInitModelNamesConfiguredProviders(t *testing.T) { + pointTransportAt(t, http.StatusOK, `{"managedByHost":false,"providers":[ + {"name":"openai","configured":false}, + {"name":"anthropic","configured":true}, + {"name":"perplexity","configured":true}]}`) + + _, err := resolveInitModel(context.Background(), "") + if err == nil { + t.Fatal("expected an error when no model is given") + } + msg := err.Error() + if !strings.Contains(msg, "--model") { + t.Errorf("error should name the flag to pass: %q", msg) + } + if !strings.Contains(msg, "anthropic") || !strings.Contains(msg, "perplexity") { + t.Errorf("error should name the configured providers: %q", msg) + } + if strings.Contains(msg, "openai") { + t.Errorf("error should not offer an unconfigured provider: %q", msg) + } +} + +// init scaffolds a local file. It must stay usable with no server reachable, so the +// provider list is a courtesy rather than a precondition. +func TestResolveInitModelStillFailsUsefullyWithoutAServer(t *testing.T) { + pointTransportAt(t, http.StatusNotFound, `{"status":404,"message":"Not Found"}`) + + _, err := resolveInitModel(context.Background(), "") + if err == nil { + t.Fatal("expected an error when no model is given") + } + msg := err.Error() + if !strings.Contains(msg, "--model") { + t.Errorf("error should name the flag to pass: %q", msg) + } + if !strings.Contains(msg, "doctor") { + t.Errorf("error should point at a way to find the answer: %q", msg) + } +} + +func TestResolveInitModelReportsHostManagedConfiguration(t *testing.T) { + pointTransportAt(t, http.StatusOK, `{"managedByHost":true,"providers":[]}`) + + _, err := resolveInitModel(context.Background(), "") + if err == nil { + t.Fatal("expected an error when no model is given") + } + if !strings.Contains(err.Error(), "host") { + t.Errorf("error should explain why no providers are listed: %q", err.Error()) + } +} diff --git a/cmd/doctor.go b/cmd/doctor.go index c7e3677..ebbc73c 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -14,22 +14,32 @@ package cmd import ( + "context" + "errors" "fmt" + "io" "os" "os/exec" "strings" + "time" "github.com/spf13/cobra" "github.com/conductor-oss/conductor-cli/internal" + "github.com/conductor-oss/conductor-cli/internal/providers" ) // aiProvider describes an LLM provider and the env vars that configure it. +// +// It deliberately names no models. doctor can know which providers are configured; +// which model ids a provider currently serves is not something a CLI release can keep +// true, and stale ids fail at execution time on the server rather than at validation +// (see #103). type aiProvider struct { - name string - envVars []string // all must be set for the provider to be "configured" - warns []providerWarning // conditional warnings, checked when the provider is opted into - models []string // example model ids + name string + serverName string // identifier the server reports for this provider + envVars []string // all must be set for the provider to be "configured" + warns []providerWarning // conditional warnings, checked when the provider is opted into } type providerWarning struct { @@ -41,11 +51,12 @@ type providerWarning struct { // aiProviders is the data-driven registry doctor reports on. Adding a provider is a // data change, not new control flow. var aiProviders = []aiProvider{ - {name: "OpenAI", envVars: []string{"OPENAI_API_KEY"}, models: []string{"openai/gpt-4o", "openai/gpt-4o-mini"}}, - {name: "Anthropic", envVars: []string{"ANTHROPIC_API_KEY"}, models: []string{"anthropic/claude-sonnet-4-20250514", "anthropic/claude-3-5-sonnet-20241022"}}, + {name: "OpenAI", serverName: "openai", envVars: []string{"OPENAI_API_KEY"}}, + {name: "Anthropic", serverName: "anthropic", envVars: []string{"ANTHROPIC_API_KEY"}}, { - name: "Google Gemini", - envVars: []string{"GEMINI_API_KEY", "GOOGLE_CLOUD_PROJECT"}, + name: "Google Gemini", + serverName: "gemini", + envVars: []string{"GEMINI_API_KEY", "GOOGLE_CLOUD_PROJECT"}, warns: []providerWarning{{ condition: func() bool { return os.Getenv("GEMINI_API_KEY") != "" && os.Getenv("GOOGLE_CLOUD_PROJECT") == "" @@ -53,11 +64,11 @@ var aiProviders = []aiProvider{ message: "GEMINI_API_KEY is set but GOOGLE_CLOUD_PROJECT is missing", fix: "export GOOGLE_CLOUD_PROJECT=your-gcp-project-id", }}, - models: []string{"google_gemini/gemini-2.0-flash", "google_gemini/gemini-1.5-pro"}, }, { - name: "Azure OpenAI", - envVars: []string{"AZURE_OPENAI_API_KEY", "AZURE_OPENAI_ENDPOINT"}, + name: "Azure OpenAI", + serverName: "azureopenai", + envVars: []string{"AZURE_OPENAI_API_KEY", "AZURE_OPENAI_ENDPOINT"}, warns: []providerWarning{{ condition: func() bool { return os.Getenv("AZURE_OPENAI_API_KEY") != "" && os.Getenv("AZURE_OPENAI_DEPLOYMENT") == "" @@ -65,11 +76,11 @@ var aiProviders = []aiProvider{ message: "AZURE_OPENAI_DEPLOYMENT is not set (required to route requests)", fix: "export AZURE_OPENAI_DEPLOYMENT=your-deployment-name", }}, - models: []string{"azure_openai/gpt-4o"}, }, { - name: "AWS Bedrock", - envVars: []string{"AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY"}, + name: "AWS Bedrock", + serverName: "aws_bedrock", + envVars: []string{"AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY"}, warns: []providerWarning{{ condition: func() bool { return os.Getenv("AWS_ACCESS_KEY_ID") != "" && os.Getenv("AWS_DEFAULT_REGION") == "" && os.Getenv("AWS_REGION") == "" @@ -77,14 +88,33 @@ var aiProviders = []aiProvider{ message: "No AWS region set — defaults to us-east-1", fix: "export AWS_DEFAULT_REGION=us-east-1", }}, - models: []string{"aws_bedrock/anthropic.claude-3-5-sonnet-20241022-v2:0"}, }, - {name: "Mistral", envVars: []string{"MISTRAL_API_KEY"}, models: []string{"mistral/mistral-large-latest"}}, - {name: "Cohere", envVars: []string{"COHERE_API_KEY"}, models: []string{"cohere/command-r-plus"}}, - {name: "Grok", envVars: []string{"XAI_API_KEY"}, models: []string{"grok/grok-3"}}, - {name: "Perplexity", envVars: []string{"PERPLEXITY_API_KEY"}, models: []string{"perplexity/sonar-pro"}}, - {name: "Hugging Face", envVars: []string{"HUGGINGFACE_API_KEY"}, models: []string{"hugging_face/meta-llama/Llama-3-70b-chat-hf"}}, - {name: "Stability AI", envVars: []string{"STABILITY_API_KEY"}, models: []string{"stabilityai/sd3.5-large"}}, + {name: "Mistral", serverName: "mistral", envVars: []string{"MISTRAL_API_KEY"}}, + {name: "Cohere", serverName: "cohere", envVars: []string{"COHERE_API_KEY"}}, + {name: "Grok", serverName: "grok", envVars: []string{"XAI_API_KEY"}}, + {name: "Perplexity", serverName: "perplexity", envVars: []string{"PERPLEXITY_API_KEY"}}, + {name: "Hugging Face", serverName: "huggingface", envVars: []string{"HUGGINGFACE_API_KEY"}}, + {name: "Stability AI", serverName: "stabilityai", envVars: []string{"STABILITY_API_KEY"}}, +} + +// serverOnlyProviders are reported by the server but configured without env vars, so +// they have no entry in the local registry. Only their display name is needed. +var serverOnlyProviders = map[string]string{"ollama": "Ollama"} + +// displayProviderName maps a server provider identifier to the CLI's display name, so +// one provider reads as one provider rather than two spellings of it. Unknown names +// pass through unchanged — a provider the server gained since this release should be +// reported, not hidden. +func displayProviderName(serverName string) string { + for _, p := range aiProviders { + if p.serverName == serverName { + return p.name + } + } + if name, ok := serverOnlyProviders[serverName]; ok { + return name + } + return serverName } var doctorCmd = &cobra.Command{ @@ -119,36 +149,111 @@ func runDoctor(cmd *cobra.Command, args []string) error { fmt.Println(" -- No authentication configured (anonymous; OSS only)") } - fmt.Println("\nAI Providers") - configured := 0 + // Agents run on the server, so ask it what it can dial before reporting what this + // shell happens to hold. The transport imposes no timeout of its own; a + // diagnostic must not hang on an unreachable server. + ctx, cancel := context.WithTimeout(cmd.Context(), providerStatusTimeout) + defer cancel() + st, err := providers.Fetch(ctx, t) + printServerProviders(os.Stdout, st, err) + + configured, warnings := printLocalProviders(os.Stdout) + issues += warnings + + fmt.Printf("\n%d AI provider(s) configured in this environment", configured) + if issues > 0 { + fmt.Printf(", %d warning(s)", issues) + } + fmt.Println(".") + return nil +} + +// providerStatusTimeout bounds the provider-status lookup. doctor reports on a server +// that may be unreachable, which is itself a finding rather than a reason to block. +// +// The bound is generous because the endpoint is measurably slow: a local OSS server +// took 7-8s to answer, and not because of the ollama reachability probe it advertises +// (a refused connect returns immediately). A timeout tight enough to feel snappy would +// report "no providers" on a server that has them, which is the very confusion this +// section exists to remove. +const providerStatusTimeout = 20 * time.Second + +// printServerProviders reports the providers the server has configured. Neither a +// host-managed deployment nor a server without the endpoint is a fault in the user's +// setup, so both are stated plainly and neither counts as a warning. +func printServerProviders(w io.Writer, st providers.Status, err error) { + fmt.Fprintln(w, "\nAI Providers (server)") + + switch { + case errors.Is(err, providers.ErrUnsupported): + fmt.Fprintln(w, " -- This server does not report provider status") + fmt.Fprintln(w, " (AI integrations are disabled, or the server predates the endpoint)") + return + case err != nil: + fmt.Fprintf(w, " -- Could not read provider status: %v\n", err) + return + case st.ManagedByHost: + fmt.Fprintln(w, " -- Provider configuration is owned by the host deployment") + fmt.Fprintln(w, " (per-provider detail is not reported by this server)") + return + case len(st.Providers) == 0: + fmt.Fprintln(w, " -- The server reported no providers") + return + } + + for _, p := range st.Providers { + mark := "--" + if p.Configured { + mark = "ok" + } + fmt.Fprintf(w, " %s %s%s\n", mark, displayProviderName(p.Name), serverProviderDetail(p)) + } +} + +// serverProviderDetail renders the extras the server reports for URL-based providers. +// Reachability is probed from the server's own network, which is precisely the fact a +// client cannot determine for itself. +func serverProviderDetail(p providers.Provider) string { + if p.BaseURL == "" && p.Reachable == nil { + return "" + } + parts := []string{} + if p.BaseURL != "" { + parts = append(parts, p.BaseURL) + } + if p.Reachable != nil { + if *p.Reachable { + parts = append(parts, "reachable") + } else { + parts = append(parts, "unreachable from the server") + } + } + return " (" + strings.Join(parts, ", ") + ")" +} + +// printLocalProviders reports what this shell has configured, which still governs the +// deploy and worker paths. It returns the configured count and the number of warnings +// raised. +func printLocalProviders(w io.Writer) (configured, warnings int) { + fmt.Fprintln(w, "\nAI Providers (local environment)") for _, p := range aiProviders { - opted := providerOptedIn(p) if isProviderConfigured(p) { configured++ - fmt.Printf(" ok %s (%s)\n", p.name, strings.Join(p.envVars, ", ")) - for _, m := range p.models { - fmt.Printf(" %s\n", m) - } + fmt.Fprintf(w, " ok %s (%s)\n", p.name, strings.Join(p.envVars, ", ")) } else { - fmt.Printf(" -- %s (%s)\n", p.name, strings.Join(p.envVars, ", ")) + fmt.Fprintf(w, " -- %s (%s)\n", p.name, strings.Join(p.envVars, ", ")) } - if opted { - for _, w := range p.warns { - if w.condition() { - fmt.Printf(" ! %s\n", w.message) - fmt.Printf(" %s\n", w.fix) - issues++ + if providerOptedIn(p) { + for _, warn := range p.warns { + if warn.condition() { + fmt.Fprintf(w, " ! %s\n", warn.message) + fmt.Fprintf(w, " %s\n", warn.fix) + warnings++ } } } } - - fmt.Printf("\n%d AI provider(s) configured", configured) - if issues > 0 { - fmt.Printf(", %d warning(s)", issues) - } - fmt.Println(".") - return nil + return configured, warnings } // isProviderConfigured reports whether every required env var for a provider is set. diff --git a/cmd/doctor_test.go b/cmd/doctor_test.go index a4575b2..ce9f1f8 100644 --- a/cmd/doctor_test.go +++ b/cmd/doctor_test.go @@ -13,7 +13,118 @@ package cmd -import "testing" +import ( + "errors" + "strings" + "testing" + + "github.com/conductor-oss/conductor-cli/internal/providers" +) + +func boolPtr(b bool) *bool { return &b } + +func TestServerProvidersReportConfiguredState(t *testing.T) { + var sb strings.Builder + printServerProviders(&sb, providers.Status{Providers: []providers.Provider{ + {Name: "openai", Configured: true}, + {Name: "anthropic", Configured: false}, + }}, nil) + out := sb.String() + + if !strings.Contains(out, "OpenAI") || !strings.Contains(out, "Anthropic") { + t.Errorf("both providers should be listed:\n%s", out) + } + // The whole point of the section: the reader must not mistake it for the local + // environment check that follows. + if !strings.Contains(strings.ToLower(out), "server") { + t.Errorf("section should attribute the report to the server:\n%s", out) + } +} + +func TestServerProvidersShowReachabilityWhenProbed(t *testing.T) { + var sb strings.Builder + printServerProviders(&sb, providers.Status{Providers: []providers.Provider{ + {Name: "ollama", Configured: true, BaseURL: "http://localhost:11434", Reachable: boolPtr(false)}, + }}, nil) + out := sb.String() + + if !strings.Contains(out, "http://localhost:11434") { + t.Errorf("base url should be shown:\n%s", out) + } + if !strings.Contains(strings.ToLower(out), "unreachable") { + t.Errorf("a failed probe should be called out:\n%s", out) + } +} + +func TestServerProvidersReportHostManagedConfiguration(t *testing.T) { + var sb strings.Builder + printServerProviders(&sb, providers.Status{ManagedByHost: true}, nil) + out := strings.ToLower(sb.String()) + + if !strings.Contains(out, "host") { + t.Errorf("host-managed configuration should be named as such:\n%s", out) + } + if strings.Contains(out, "error") || strings.Contains(out, "failed") { + t.Errorf("host-managed configuration is not a failure:\n%s", out) + } +} + +// A server with AI integrations disabled is a normal deployment, not a broken one. +func TestServerProvidersReportMissingEndpointPlainly(t *testing.T) { + var sb strings.Builder + printServerProviders(&sb, providers.Status{}, providers.ErrUnsupported) + out := sb.String() + + if strings.TrimSpace(out) == "" { + t.Fatal("an unsupported endpoint should still say something") + } + if strings.Contains(strings.ToLower(out), "error") { + t.Errorf("should read as information, not an error:\n%s", out) + } +} + +func TestServerProvidersReportUnreachableServer(t *testing.T) { + var sb strings.Builder + printServerProviders(&sb, providers.Status{}, errors.New("dial tcp: connection refused")) + + if strings.TrimSpace(sb.String()) == "" { + t.Fatal("a failed lookup should still say something") + } +} + +// Server and CLI spell the same provider differently. One provider, one name. +func TestServerProviderNamesMapToDisplayNames(t *testing.T) { + for serverName, want := range map[string]string{ + "gemini": "Google Gemini", + "azureopenai": "Azure OpenAI", + "huggingface": "Hugging Face", + "aws_bedrock": "AWS Bedrock", + "openai": "OpenAI", + "ollama": "Ollama", // server-only: not in the local registry at all + } { + if got := displayProviderName(serverName); got != want { + t.Errorf("displayProviderName(%q) = %q, want %q", serverName, got, want) + } + } +} + +// Regression guard for #103: doctor reported model ids it could not keep current, and +// two of them had already been withdrawn by the provider. Model identifiers carry a +// "provider/model" shape; nothing else in this section contains a slash. +func TestLocalProvidersAdvertiseNoModelIdentifiers(t *testing.T) { + var sb strings.Builder + printLocalProviders(&sb) + out := sb.String() + + for _, line := range strings.Split(out, "\n") { + if strings.Contains(line, "/") { + t.Errorf("doctor should name no model identifiers, got: %q", line) + } + } + if !strings.Contains(out, "Anthropic") { + t.Errorf("providers should still be listed:\n%s", out) + } +} func TestProviderConfiguredRequiresAllEnvVars(t *testing.T) { p := aiProvider{name: "Test", envVars: []string{"DOC_TEST_KEY", "DOC_TEST_ENDPOINT"}} diff --git a/internal/providers/providers.go b/internal/providers/providers.go new file mode 100644 index 0000000..55cae40 --- /dev/null +++ b/internal/providers/providers.go @@ -0,0 +1,84 @@ +/* + * Copyright 2026 Conductor Authors. + *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *
+ * http://www.apache.org/licenses/LICENSE-2.0 + *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +// Package providers reads the server's view of which AI providers it can dial. +// +// Agents execute on the server, which resolves provider credentials from its own +// environment, credential store, or properties. A client shell's OPENAI_API_KEY says +// nothing about that, so the CLI asks rather than guesses. +// +// The transport applies no timeout of its own (request lifetime belongs to the +// caller's context), so callers bound this themselves — a diagnostic command must not +// hang on an unreachable server. +package providers + +import ( + "context" + "errors" + "fmt" + "net/http" + + "github.com/conductor-oss/conductor-cli/internal/transport" +) + +const statusPath = "/providers/status" + +// ErrUnsupported means the server has no provider-status route: AI integrations are +// disabled, or the deployment predates the endpoint. It describes the deployment, not +// a failure, and callers are expected to report it and carry on. +var ErrUnsupported = errors.New("server does not expose provider status") + +// Status is the server's provider configuration. +type Status struct { + // ManagedByHost is true when the server is embedded in a host that owns provider + // configuration. Providers is then empty by design rather than by omission; the + // wire contract is forward-compatible, so per-provider detail may appear later. + ManagedByHost bool `json:"managedByHost"` + Providers []Provider `json:"providers"` +} + +// Provider is one provider's status as the server sees it. +type Provider struct { + Name string `json:"name"` + Configured bool `json:"configured"` + // BaseURL is reported only for URL-based providers (ollama). + BaseURL string `json:"baseUrl,omitempty"` + // Reachable is probed from the server's own network — a fact no client can + // observe for itself. It is a pointer because "not probed" and "probed, and it + // failed" are different answers. + Reachable *bool `json:"reachable,omitempty"` +} + +// Fetch reads provider status from the server. A missing route yields ErrUnsupported. +func Fetch(ctx context.Context, t transport.Config) (Status, error) { + var st Status + if err := t.DoJSON(ctx, http.MethodGet, statusPath, nil, &st); err != nil { + var apiErr *transport.APIError + if errors.As(err, &apiErr) && apiErr.Status == http.StatusNotFound { + return Status{}, ErrUnsupported + } + return Status{}, fmt.Errorf("read provider status: %w", err) + } + return st, nil +} + +// Configured returns the names of the providers the server reports as configured. +func (s Status) Configured() []string { + var names []string + for _, p := range s.Providers { + if p.Configured { + names = append(names, p.Name) + } + } + return names +} diff --git a/internal/providers/providers_test.go b/internal/providers/providers_test.go new file mode 100644 index 0000000..f3ab172 --- /dev/null +++ b/internal/providers/providers_test.go @@ -0,0 +1,120 @@ +/* + * Copyright 2026 Conductor Authors. + *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *
+ * http://www.apache.org/licenses/LICENSE-2.0 + *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package providers + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/conductor-oss/conductor-cli/internal/transport" +) + +// stub serves one canned response and records the paths it was asked for. +func stub(t *testing.T, status int, body string) (transport.Config, *[]string) { + t.Helper() + var paths []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(srv.Close) + return transport.Config{BaseURL: srv.URL + "/api"}, &paths +} + +func TestFetchReadsPerProviderStatus(t *testing.T) { + cfg, paths := stub(t, http.StatusOK, `{"managedByHost":false,"providers":[ + {"name":"openai","configured":true}, + {"name":"anthropic","configured":false}, + {"name":"ollama","configured":true,"baseUrl":"http://localhost:11434","reachable":false}]}`) + + st, err := Fetch(context.Background(), cfg) + if err != nil { + t.Fatalf("Fetch: %v", err) + } + if st.ManagedByHost { + t.Error("ManagedByHost should be false") + } + if len(st.Providers) != 3 { + t.Fatalf("got %d providers, want 3", len(st.Providers)) + } + if !st.Providers[0].Configured || st.Providers[0].Name != "openai" { + t.Errorf("first provider = %+v", st.Providers[0]) + } + if st.Providers[1].Configured { + t.Error("anthropic should be unconfigured") + } + + ollama := st.Providers[2] + if ollama.BaseURL != "http://localhost:11434" { + t.Errorf("ollama BaseURL = %q", ollama.BaseURL) + } + if ollama.Reachable == nil || *ollama.Reachable { + t.Errorf("ollama Reachable = %v, want a non-nil false", ollama.Reachable) + } + + // Reachability is only reported for providers the server probes; the absence of + // the field must stay distinguishable from a probe that failed. + if st.Providers[0].Reachable != nil { + t.Error("openai should carry no reachability") + } + + if len(*paths) != 1 { + t.Fatalf("made %d requests, want exactly 1: %v", len(*paths), *paths) + } + if (*paths)[0] != "/api/providers/status" { + t.Errorf("requested %q", (*paths)[0]) + } +} + +func TestFetchReportsHostManagedConfiguration(t *testing.T) { + cfg, _ := stub(t, http.StatusOK, `{"managedByHost":true,"providers":[]}`) + + st, err := Fetch(context.Background(), cfg) + if err != nil { + t.Fatalf("Fetch: %v", err) + } + if !st.ManagedByHost { + t.Error("ManagedByHost should be true") + } + if len(st.Providers) != 0 { + t.Errorf("host-managed response should carry no per-provider detail, got %d", len(st.Providers)) + } +} + +// A server with AI integrations disabled has no such route. That is a fact about the +// deployment, not a failure, so callers need to tell it apart from a real error. +func TestFetchTreatsMissingEndpointAsUnsupported(t *testing.T) { + cfg, _ := stub(t, http.StatusNotFound, `{"status":404,"message":"Not Found"}`) + + if _, err := Fetch(context.Background(), cfg); !errors.Is(err, ErrUnsupported) { + t.Fatalf("err = %v, want ErrUnsupported", err) + } +} + +func TestFetchSurfacesOtherErrors(t *testing.T) { + cfg, _ := stub(t, http.StatusInternalServerError, `{"status":500,"message":"boom"}`) + + _, err := Fetch(context.Background(), cfg) + if err == nil { + t.Fatal("expected an error") + } + if errors.Is(err, ErrUnsupported) { + t.Error("a 500 is a real failure, not an unsupported endpoint") + } +} diff --git a/test/e2e/agent.bats b/test/e2e/agent.bats index bd33457..f83b63d 100644 --- a/test/e2e/agent.bats +++ b/test/e2e/agent.bats @@ -14,8 +14,9 @@ AGENT_NAME="e2e_agent_probe" -# A model verified to exist; the `agent init` default (openai/gpt-4o) is not -# usable unless an OpenAI key happens to be configured — see #103. +# A model verified to exist. `agent init` no longer supplies a default at all — the +# server reports which providers it can dial but no model names, so every config has +# to name one explicitly (see #103). LLM_MODEL="anthropic/claude-haiku-4-5-20251001" setup_file() { @@ -84,7 +85,7 @@ run_bounded() { # bats test_tags=tier:pr @test "1. Agent init creates a YAML config" { - run bash -c "cd '$BATS_TEST_TMPDIR' && '$PWD/conductor' agent init inittest 2>&1" + run bash -c "cd '$BATS_TEST_TMPDIR' && '$PWD/conductor' agent init inittest --model $LLM_MODEL 2>&1" echo "Output: $output" [ "$status" -eq 0 ] [ -f "$BATS_TEST_TMPDIR/inittest.yaml" ] @@ -92,7 +93,7 @@ run_bounded() { # bats test_tags=tier:pr @test "2. Agent init --format json creates a JSON config" { - run bash -c "cd '$BATS_TEST_TMPDIR' && '$PWD/conductor' agent init jsontest --format json 2>&1" + run bash -c "cd '$BATS_TEST_TMPDIR' && '$PWD/conductor' agent init jsontest --format json --model $LLM_MODEL 2>&1" echo "Output: $output" [ "$status" -eq 0 ] [ -f "$BATS_TEST_TMPDIR/jsontest.json" ] @@ -100,12 +101,23 @@ run_bounded() { # bats test_tags=tier:pr @test "3. Agent init --strategy records the strategy" { - run bash -c "cd '$BATS_TEST_TMPDIR' && '$PWD/conductor' agent init strattest --strategy handoff 2>&1" + run bash -c "cd '$BATS_TEST_TMPDIR' && '$PWD/conductor' agent init strattest --strategy handoff --model $LLM_MODEL 2>&1" echo "Output: $output" [ "$status" -eq 0 ] grep -q 'handoff' "$BATS_TEST_TMPDIR/strattest.yaml" } +# Regression guard for #103: init used to write a fixed OpenAI model whatever was +# configured, then print a run command that could not succeed. +# bats test_tags=tier:pr +@test "3b. Agent init without --model fails and writes nothing" { + run bash -c "cd '$BATS_TEST_TMPDIR' && '$PWD/conductor' agent init nomodel 2>&1" + echo "Output: $output" + [ "$status" -ne 0 ] + [[ "$output" == *"--model"* ]] + [ ! -f "$BATS_TEST_TMPDIR/nomodel.yaml" ] +} + # bats test_tags=tier:pr @test "4. Agent list succeeds" { require_agents_api diff --git a/test/e2e/doctor.bats b/test/e2e/doctor.bats index 96853e3..306e23f 100644 --- a/test/e2e/doctor.bats +++ b/test/e2e/doctor.bats @@ -64,14 +64,17 @@ setup_file() { [[ "$output" == *"OPENAI_API_KEY"* ]] } -# Regression guard for #103: doctor advertises specific model strings that have +# Regression guard for #103: doctor advertised specific model strings that had # drifted out of date (two Anthropic models return 404 from the provider API). -# Asserts the desired end state — that doctor does not print known-dead models. +# doctor now names no models at all, so the two dead ids are covered by the general +# assertion as well as by name. @test "7. Doctor does not advertise retired model identifiers" { - skip "known broken: #103 — doctor hardcodes stale model strings" run bash -c "./conductor doctor 2>&1" echo "Output: $output" [ "$status" -eq 0 ] [[ "$output" != *"claude-sonnet-4-20250514"* ]] [[ "$output" != *"claude-3-5-sonnet-20241022"* ]] + # No model identifier of any provider/model shape, dead or alive. + run bash -c "./conductor doctor 2>&1 | grep -E '^ +[a-z_]+/[A-Za-z0-9._-]+'" + [ "$status" -ne 0 ] }