From a27bca9897a09a1893d0a108889915f30ca9e202 Mon Sep 17 00:00:00 2001 From: MK Date: Tue, 2 Jun 2026 15:28:26 -0400 Subject: [PATCH] fix(init): wire 'Custom' provider through OpenAI-compatible runtime path (closes #83) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wizard's Custom provider option is documented as "point Forge at any OpenAI-compatible endpoint" (OpenRouter, litellm, vLLM, self-hosted Kimi/ Llama, etc.). It collected the right inputs but wrote artifacts the runtime could not consume: 1. forge.yaml got `provider: custom`, which the LLM client factory (forge-core/llm/providers/factory.go) rejects with "unknown LLM provider". Runner falls back to StubExecutor; every task fails with 'agent execution not configured for framework "forge"'. 2. .env got MODEL_BASE_URL / MODEL_API_KEY, which forge-core/runtime/ config.go's resolveAPIKey() never reads (only per-provider names like OPENAI_BASE_URL / OPENAI_API_KEY are consulted). 3. Even when users worked around (1) and (2) by hand-editing forge.yaml to `provider: openai` + writing OPENAI_BASE_URL, stored ChatGPT OAuth credentials silently won over the explicit base URL — the runner's createProviderClient overrides cfg.BaseURL with chatgpt.com/backend-api/codex. ChatGPT then returns 400 rejecting the operator's model, with no signal that the endpoint was hijacked. Fix at three boundaries: - forge-cli/cmd/init.go: new normalizeCustomProvider() called at the top of scaffold() (single entry point for both TUI and Web UI). Rewrites ModelProvider=custom -> openai and migrates MODEL_BASE_URL / MODEL_API_KEY -> OPENAI_BASE_URL / OPENAI_API_KEY. Handles all three shapes that may arrive: legacy MODEL_* env vars, newer Web UI direct OPENAI_* env vars, and the non-interactive --api-key flag path. buildEnvVars's "openai" case now emits OPENAI_BASE_URL when set; the dead "custom" case is removed. - forge-cli/runtime/runner.go + forge-cli/cmd/ui.go: OAuth-precedence guardrail. When provider="openai" + cfg.BaseURL != "" + cfg.APIKey == "", refuse rather than fall through to OAuth. Error names the configured base URL, the missing env var (OPENAI_API_KEY), and the override (chatgpt.com/backend-api/codex) that would otherwise occur. Anthropic and other providers unaffected. - forge-ui/handlers_create.go + types.go + static/app.js: wizard metadata advertises BaseURLEnv="OPENAI_BASE_URL" (not MODEL_BASE_URL); the React form binds to OPENAI_BASE_URL directly. POSTs whose ModelProvider="custom" still work — they hit the same scaffold() entry point and get the same normalization. Regression tests: forge-cli/cmd/init_custom_provider_test.go (6 tests): TestNormalizeCustomProvider_RewritesLegacyEnvVars TestNormalizeCustomProvider_NoOpForOtherProviders TestNormalizeCustomProvider_PreExistingOpenAIVarsPreserved TestNormalizeCustomProvider_APIKeyFallsBackToOptsField TestScaffold_CustomProviderProducesOpenAIShape TestScaffold_CustomProviderWebUIShape forge-cli/runtime/runner_oauth_guardrail_test.go (4 tests): TestCreateProviderClient_BaseURLSetWithoutAPIKey_RefusesOAuth TestCreateProviderClient_BaseURLSetWithAPIKey_BypassesOAuth TestCreateProviderClient_NoBaseURL_AllowsOAuthPath TestCreateProviderClient_AnthropicWithBaseURL_Unaffected go test -race ./forge-cli/cmd/... ./forge-cli/runtime/... ./forge-ui/... golangci-lint and gofmt clean across all three modules. Migration note in CHANGELOG for users with checked-in `provider: custom` forge.yaml files (manual rename to provider: openai + .env key rename). --- CHANGELOG.md | 28 +++ forge-cli/cmd/init.go | 67 ++++- forge-cli/cmd/init_custom_provider_test.go | 228 ++++++++++++++++++ forge-cli/cmd/ui.go | 15 ++ forge-cli/runtime/runner.go | 23 ++ .../runtime/runner_oauth_guardrail_test.go | 114 +++++++++ forge-ui/handlers_create.go | 14 +- forge-ui/static/app.js | 6 +- forge-ui/types.go | 2 +- 9 files changed, 478 insertions(+), 19 deletions(-) create mode 100644 forge-cli/cmd/init_custom_provider_test.go create mode 100644 forge-cli/runtime/runner_oauth_guardrail_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 2408676..e408e5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,33 @@ # Changelog +## Unreleased + +### Fixed + +- **`forge init` Custom provider now produces a runnable agent (issue #83).** + Picking the **Custom** provider in `forge init` (or the Web UI wizard) + previously wrote `provider: custom` to `forge.yaml` plus + `MODEL_BASE_URL` / `MODEL_API_KEY` env vars, neither of which the runtime + understood — agents fell back to `StubExecutor` and every task failed + with `agent execution not configured for framework "forge"`. Scaffold + now normalizes Custom → `provider: openai` + `OPENAI_BASE_URL` / + `OPENAI_API_KEY`, matching the OpenAI-compatible code path the runtime + resolver already supports. Affects both TUI and Web UI flows. +- **OAuth-credentials path no longer silently overrides + `OPENAI_BASE_URL` (issue #83).** When the runtime or skill builder + found stored ChatGPT OAuth credentials AND no `OPENAI_API_KEY`, it + ignored an explicitly-set `OPENAI_BASE_URL` and routed traffic to + `chatgpt.com/backend-api/codex` — manifesting as a 400 from ChatGPT + rejecting the operator's model name. Both `forge run` and `forge ui` + now refuse this combination with a clear error explaining what to set. + +### Migration + +- If you have `provider: custom` in a checked-in `forge.yaml` from an + earlier `forge init` run, change it to `provider: openai` and rename + the `.env` keys from `MODEL_BASE_URL` / `MODEL_API_KEY` to + `OPENAI_BASE_URL` / `OPENAI_API_KEY`. No new `forge init` is required. + ## v0.12.0 — Phase 1: MCP integration (HTTP transport) — in progress ### Added diff --git a/forge-cli/cmd/init.go b/forge-cli/cmd/init.go index ba1e71b..1b98596 100644 --- a/forge-cli/cmd/init.go +++ b/forge-cli/cmd/init.go @@ -386,7 +386,11 @@ func collectInteractive(opts *initOptions) error { opts.EnvVars[k] = v } - // Custom provider env vars + // Custom provider env vars. The wizard collects a base URL + API key + // for the OpenAI-compatible endpoint. Write under the legacy + // MODEL_* names here; normalizeCustomProvider rewrites them to + // OPENAI_BASE_URL / OPENAI_API_KEY at scaffold time so both this + // path and the Web UI POST (which also uses MODEL_*) converge. if ctx.CustomBaseURL != "" { opts.EnvVars["MODEL_BASE_URL"] = ctx.CustomBaseURL } @@ -513,6 +517,48 @@ func storeProviderEnvVar(opts *initOptions) { } } +// normalizeCustomProvider rewrites the wizard's "Custom" provider option +// to the OpenAI-compatible shape that the runtime resolver actually +// consumes: provider=openai + OPENAI_BASE_URL + OPENAI_API_KEY. +// +// Background (issue #83): the wizard's Custom path is meant for OpenAI- +// compatible endpoints (OpenRouter, litellm, vLLM, self-hosted Kimi/ +// Llama, etc.). Before normalization it wrote provider=custom to +// forge.yaml and MODEL_BASE_URL / MODEL_API_KEY to .env — but the LLM +// client factory has no "custom" case (it returns an unknown-provider +// error) and ResolveModelConfig never reads MODEL_BASE_URL or +// MODEL_API_KEY. The runtime fell back to StubExecutor and every task +// failed with "agent execution not configured for framework forge". +// +// The fix is to normalize at the single scaffold entry point so both +// the TUI wizard and the Web UI Custom flow produce the same shape. +func normalizeCustomProvider(opts *initOptions) { + if opts.ModelProvider != "custom" { + return + } + opts.ModelProvider = "openai" + // Migrate legacy MODEL_BASE_URL / MODEL_API_KEY (TUI wizard, + // older Web UI revs) to the OPENAI_* names the runtime reads. + if v := opts.EnvVars["MODEL_BASE_URL"]; v != "" { + opts.EnvVars["OPENAI_BASE_URL"] = v + delete(opts.EnvVars, "MODEL_BASE_URL") + } + if v := opts.EnvVars["MODEL_API_KEY"]; v != "" { + opts.EnvVars["OPENAI_API_KEY"] = v + delete(opts.EnvVars, "MODEL_API_KEY") + if opts.APIKey == "" { + opts.APIKey = v + } + } + // storeProviderEnvVar runs before scaffold and short-circuits when + // provider="custom"; fold in any opts.APIKey set by flag or POST + // so the .env emits OPENAI_API_KEY consistently with the path + // taken by every other openai-shaped configuration. + if opts.APIKey != "" && opts.EnvVars["OPENAI_API_KEY"] == "" { + opts.EnvVars["OPENAI_API_KEY"] = opts.APIKey + } +} + // checkSkillRequirements checks binary and env requirements for selected skills. func checkSkillRequirements(opts *initOptions) { chkReg, chkErr := local.NewEmbeddedRegistry() @@ -594,6 +640,8 @@ func parseSkillsFile(path string) ([]toolEntry, error) { } func scaffold(opts *initOptions) error { + normalizeCustomProvider(opts) + dir := filepath.Join(".", opts.AgentID) // Check if directory already exists @@ -1124,6 +1172,13 @@ func buildEnvVars(opts *initOptions) []envVarEntry { val = "your-api-key-here" } vars = append(vars, envVarEntry{Key: "OPENAI_API_KEY", Value: val, Comment: "OpenAI API key"}) + // OPENAI_BASE_URL is set when the wizard's Custom provider + // option is used against an OpenAI-compatible endpoint + // (OpenRouter, vLLM, litellm, etc.) and normalizeCustomProvider + // has rewritten provider=custom to provider=openai. (Issue #83.) + if baseURL := opts.EnvVars["OPENAI_BASE_URL"]; baseURL != "" { + vars = append(vars, envVarEntry{Key: "OPENAI_BASE_URL", Value: baseURL, Comment: "OpenAI-compatible endpoint base URL"}) + } if orgID := opts.OrganizationID; orgID != "" { vars = append(vars, envVarEntry{Key: "OPENAI_ORG_ID", Value: orgID, Comment: "OpenAI organization ID (enterprise)"}) } @@ -1141,16 +1196,6 @@ func buildEnvVars(opts *initOptions) []envVarEntry { vars = append(vars, envVarEntry{Key: "GEMINI_API_KEY", Value: val, Comment: "Gemini API key"}) case "ollama": vars = append(vars, envVarEntry{Key: "OLLAMA_HOST", Value: "http://localhost:11434", Comment: "Ollama host"}) - case "custom": - baseURL := opts.EnvVars["MODEL_BASE_URL"] - if baseURL != "" { - vars = append(vars, envVarEntry{Key: "MODEL_BASE_URL", Value: baseURL, Comment: "Custom model endpoint URL"}) - } - apiKeyVal := opts.EnvVars["MODEL_API_KEY"] - if apiKeyVal == "" { - apiKeyVal = "your-api-key-here" - } - vars = append(vars, envVarEntry{Key: "MODEL_API_KEY", Value: apiKeyVal, Comment: "Model provider API key"}) } // Web search provider key if web_search selected diff --git a/forge-cli/cmd/init_custom_provider_test.go b/forge-cli/cmd/init_custom_provider_test.go new file mode 100644 index 0000000..a5ba745 --- /dev/null +++ b/forge-cli/cmd/init_custom_provider_test.go @@ -0,0 +1,228 @@ +package cmd + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "gopkg.in/yaml.v3" +) + +// Regression tests for issue #83 — Custom-provider wizard path produces +// forge.yaml + .env that the runtime can actually consume. + +func TestNormalizeCustomProvider_RewritesLegacyEnvVars(t *testing.T) { + opts := &initOptions{ + ModelProvider: "custom", + EnvVars: map[string]string{ + "MODEL_BASE_URL": "https://endpoint.example.com/v1", + "MODEL_API_KEY": "sk-test", + "UNRELATED": "keep-me", + }, + } + normalizeCustomProvider(opts) + + if opts.ModelProvider != "openai" { + t.Errorf("ModelProvider = %q, want %q", opts.ModelProvider, "openai") + } + if got := opts.EnvVars["OPENAI_BASE_URL"]; got != "https://endpoint.example.com/v1" { + t.Errorf("OPENAI_BASE_URL = %q, want endpoint URL", got) + } + if got := opts.EnvVars["OPENAI_API_KEY"]; got != "sk-test" { + t.Errorf("OPENAI_API_KEY = %q, want sk-test", got) + } + if _, present := opts.EnvVars["MODEL_BASE_URL"]; present { + t.Errorf("MODEL_BASE_URL should be deleted after normalization") + } + if _, present := opts.EnvVars["MODEL_API_KEY"]; present { + t.Errorf("MODEL_API_KEY should be deleted after normalization") + } + if got := opts.EnvVars["UNRELATED"]; got != "keep-me" { + t.Errorf("UNRELATED key should be preserved, got %q", got) + } + if opts.APIKey != "sk-test" { + t.Errorf("opts.APIKey should be filled from MODEL_API_KEY, got %q", opts.APIKey) + } +} + +func TestNormalizeCustomProvider_NoOpForOtherProviders(t *testing.T) { + cases := []string{"openai", "anthropic", "gemini", "ollama"} + for _, p := range cases { + t.Run(p, func(t *testing.T) { + opts := &initOptions{ + ModelProvider: p, + EnvVars: map[string]string{ + "MODEL_BASE_URL": "https://should-not-be-touched", + "MODEL_API_KEY": "should-not-be-touched", + }, + } + normalizeCustomProvider(opts) + if opts.ModelProvider != p { + t.Errorf("ModelProvider mutated from %q to %q", p, opts.ModelProvider) + } + if got := opts.EnvVars["MODEL_BASE_URL"]; got != "https://should-not-be-touched" { + t.Errorf("MODEL_BASE_URL should not be touched for provider=%s, got %q", p, got) + } + }) + } +} + +func TestNormalizeCustomProvider_PreExistingOpenAIVarsPreserved(t *testing.T) { + // Newer Web UI revs write OPENAI_BASE_URL directly. The normalizer + // should accept that and not clobber it. + opts := &initOptions{ + ModelProvider: "custom", + EnvVars: map[string]string{ + "OPENAI_BASE_URL": "https://from-webui", + "OPENAI_API_KEY": "sk-from-webui", + }, + } + normalizeCustomProvider(opts) + + if opts.ModelProvider != "openai" { + t.Errorf("ModelProvider = %q, want openai", opts.ModelProvider) + } + if got := opts.EnvVars["OPENAI_BASE_URL"]; got != "https://from-webui" { + t.Errorf("OPENAI_BASE_URL clobbered, got %q", got) + } + if got := opts.EnvVars["OPENAI_API_KEY"]; got != "sk-from-webui" { + t.Errorf("OPENAI_API_KEY clobbered, got %q", got) + } +} + +func TestNormalizeCustomProvider_APIKeyFallsBackToOptsField(t *testing.T) { + // Non-interactive --api-key flag path: APIKey is set on opts directly + // (storeProviderEnvVar would skip the openai branch when provider was + // still "custom"). After normalization, the API key must reach OPENAI_API_KEY. + opts := &initOptions{ + ModelProvider: "custom", + APIKey: "sk-from-flag", + EnvVars: map[string]string{"MODEL_BASE_URL": "https://endpoint"}, + } + normalizeCustomProvider(opts) + + if opts.ModelProvider != "openai" { + t.Errorf("ModelProvider = %q, want openai", opts.ModelProvider) + } + if got := opts.EnvVars["OPENAI_API_KEY"]; got != "sk-from-flag" { + t.Errorf("OPENAI_API_KEY = %q, want sk-from-flag", got) + } + if got := opts.EnvVars["OPENAI_BASE_URL"]; got != "https://endpoint" { + t.Errorf("OPENAI_BASE_URL = %q, want endpoint", got) + } +} + +// End-to-end: scaffold with the Custom provider shape produces a +// forge.yaml whose model.provider is "openai" and a .env containing +// OPENAI_BASE_URL + OPENAI_API_KEY (not MODEL_*). +func TestScaffold_CustomProviderProducesOpenAIShape(t *testing.T) { + tmpDir := t.TempDir() + origDir, _ := os.Getwd() + if err := os.Chdir(tmpDir); err != nil { + t.Fatalf("chdir: %v", err) + } + defer func() { _ = os.Chdir(origDir) }() + + opts := &initOptions{ + Name: "custom-shape", + AgentID: "custom-shape", + Framework: "forge", + ModelProvider: "custom", + CustomModel: "moonshotai/Kimi-K2.6", + APIKey: "sk-endpoint", + EnvVars: map[string]string{ + "MODEL_BASE_URL": "https://openrouter-ish.example.com/v1", + "MODEL_API_KEY": "sk-endpoint", + }, + NonInteractive: true, + } + + if err := scaffold(opts); err != nil { + t.Fatalf("scaffold: %v", err) + } + + // forge.yaml: provider must be "openai" (not "custom"), model.name preserved. + cfgPath := filepath.Join("custom-shape", "forge.yaml") + cfgRaw, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatalf("reading forge.yaml: %v", err) + } + var cfg struct { + Model struct { + Provider string `yaml:"provider"` + Name string `yaml:"name"` + } `yaml:"model"` + } + if err := yaml.Unmarshal(cfgRaw, &cfg); err != nil { + t.Fatalf("parsing forge.yaml: %v\n%s", err, cfgRaw) + } + if cfg.Model.Provider != "openai" { + t.Errorf("forge.yaml model.provider = %q, want %q\n--- forge.yaml ---\n%s", + cfg.Model.Provider, "openai", cfgRaw) + } + if cfg.Model.Name != "moonshotai/Kimi-K2.6" { + t.Errorf("forge.yaml model.name = %q, want %q", cfg.Model.Name, "moonshotai/Kimi-K2.6") + } + + // .env: OPENAI_BASE_URL + OPENAI_API_KEY present; legacy MODEL_* absent. + envPath := filepath.Join("custom-shape", ".env") + envContent, err := os.ReadFile(envPath) + if err != nil { + t.Fatalf("reading .env: %v", err) + } + envStr := string(envContent) + if !strings.Contains(envStr, "OPENAI_BASE_URL=https://openrouter-ish.example.com/v1") { + t.Errorf(".env missing OPENAI_BASE_URL with endpoint URL:\n%s", envStr) + } + if !strings.Contains(envStr, "OPENAI_API_KEY=sk-endpoint") { + t.Errorf(".env missing OPENAI_API_KEY=sk-endpoint:\n%s", envStr) + } + if strings.Contains(envStr, "MODEL_BASE_URL=") { + t.Errorf(".env should NOT contain MODEL_BASE_URL after normalization:\n%s", envStr) + } + if strings.Contains(envStr, "MODEL_API_KEY=") { + t.Errorf(".env should NOT contain MODEL_API_KEY after normalization:\n%s", envStr) + } +} + +// Web UI parity: a POST whose ModelProvider="custom" + EnvVars already +// carry OPENAI_BASE_URL (new app.js shape) also produces the right output. +func TestScaffold_CustomProviderWebUIShape(t *testing.T) { + tmpDir := t.TempDir() + origDir, _ := os.Getwd() + if err := os.Chdir(tmpDir); err != nil { + t.Fatalf("chdir: %v", err) + } + defer func() { _ = os.Chdir(origDir) }() + + opts := &initOptions{ + Name: "webui-shape", + AgentID: "webui-shape", + Framework: "forge", + ModelProvider: "custom", + CustomModel: "moonshotai/Kimi-K2.6", + EnvVars: map[string]string{ + "OPENAI_BASE_URL": "https://endpoint.example.com/v1", + "OPENAI_API_KEY": "sk-from-webui", + }, + NonInteractive: true, + } + + if err := scaffold(opts); err != nil { + t.Fatalf("scaffold: %v", err) + } + + envPath := filepath.Join("webui-shape", ".env") + envContent, err := os.ReadFile(envPath) + if err != nil { + t.Fatalf("reading .env: %v", err) + } + envStr := string(envContent) + if !strings.Contains(envStr, "OPENAI_BASE_URL=https://endpoint.example.com/v1") { + t.Errorf(".env missing OPENAI_BASE_URL:\n%s", envStr) + } + if !strings.Contains(envStr, "OPENAI_API_KEY=sk-from-webui") { + t.Errorf(".env missing OPENAI_API_KEY=sk-from-webui:\n%s", envStr) + } +} diff --git a/forge-cli/cmd/ui.go b/forge-cli/cmd/ui.go index 4773287..8934f92 100644 --- a/forge-cli/cmd/ui.go +++ b/forge-cli/cmd/ui.go @@ -199,6 +199,21 @@ func runUI(cmd *cobra.Command, args []string) error { // skip the codegen model upgrade for OAuth clients. var client llm.Client needsOAuth := mc.Provider == "openai" && (mc.Client.APIKey == "" || mc.Client.APIKey == "__oauth__") + // OAuth precedence guardrail (issue #83). The ChatGPT OAuth + // path's hardcoded chatgpt.com/backend-api/codex base URL is + // mutually exclusive with an operator-supplied OPENAI_BASE_URL. + // Without this guard, the skill builder would silently route + // requests to ChatGPT instead of the agent's configured + // OpenAI-compatible endpoint. + if needsOAuth && mc.Client.BaseURL != "" { + return fmt.Errorf( + "OPENAI_BASE_URL is set to %q but no OPENAI_API_KEY was provided; "+ + "the OpenAI OAuth credentials path is disabled when an explicit "+ + "base URL is in use (it would silently override your endpoint with "+ + "chatgpt.com/backend-api/codex). Set OPENAI_API_KEY for the configured endpoint", + mc.Client.BaseURL, + ) + } if needsOAuth { token, oauthErr := oauth.LoadCredentials(mc.Provider) if oauthErr == nil && token != nil && token.RefreshToken != "" { diff --git a/forge-cli/runtime/runner.go b/forge-cli/runtime/runner.go index 8c5afe6..7fb2561 100644 --- a/forge-cli/runtime/runner.go +++ b/forge-cli/runtime/runner.go @@ -1675,11 +1675,34 @@ func (r *Runner) buildLLMClient(mc *coreruntime.ModelConfig) (llm.Client, error) // createProviderClient creates an LLM client for a provider, using OAuth // credentials if available for supported providers. +// +// OAuth precedence guardrail (issue #83): when the operator has set +// OPENAI_BASE_URL (i.e. an explicit OpenAI-compatible endpoint), do NOT +// fall through to the stored ChatGPT OAuth credentials — the OAuth +// path overrides cfg.BaseURL with chatgpt.com/backend-api/codex and +// silently routes requests there, defeating the explicit override. +// An operator pointing at OpenRouter / vLLM / Kimi / etc. must set +// OPENAI_API_KEY for that endpoint; if it's missing, surface the +// configuration error rather than tunneling to ChatGPT. func (r *Runner) createProviderClient(provider string, cfg llm.ClientConfig) (llm.Client, error) { // Check for stored OAuth credentials — but only if no real API key is // configured. The "__oauth__" sentinel means the user chose OAuth auth // during init, so we should load the actual token from the credential store. needsOAuth := provider == "openai" && (cfg.APIKey == "" || cfg.APIKey == "__oauth__") + + // Explicit OPENAI_BASE_URL disqualifies the OAuth path. The OAuth + // flow's base URL (chatgpt.com/backend-api/codex) is mutually + // exclusive with a user-supplied endpoint. + if needsOAuth && cfg.BaseURL != "" { + return nil, fmt.Errorf( + "OPENAI_BASE_URL is set to %q but no OPENAI_API_KEY was provided; "+ + "the OpenAI OAuth credentials path is disabled when an explicit "+ + "base URL is in use (it would silently override your endpoint with "+ + "chatgpt.com/backend-api/codex). Set OPENAI_API_KEY for the configured endpoint", + cfg.BaseURL, + ) + } + if needsOAuth { token, err := oauth.LoadCredentials(provider) if err == nil && token != nil && token.RefreshToken != "" { diff --git a/forge-cli/runtime/runner_oauth_guardrail_test.go b/forge-cli/runtime/runner_oauth_guardrail_test.go new file mode 100644 index 0000000..da7a922 --- /dev/null +++ b/forge-cli/runtime/runner_oauth_guardrail_test.go @@ -0,0 +1,114 @@ +package runtime + +import ( + "bytes" + "strings" + "testing" + + "github.com/initializ/forge/forge-core/llm" + coreruntime "github.com/initializ/forge/forge-core/runtime" +) + +// Regression test for issue #83: when OPENAI_BASE_URL is set (operator +// pointed Forge at an OpenAI-compatible endpoint) but no OPENAI_API_KEY, +// createProviderClient must refuse with a clear error rather than +// silently using stored ChatGPT OAuth credentials (which override +// BaseURL with chatgpt.com/backend-api/codex). + +func newOAuthGuardrailRunner() *Runner { + return &Runner{ + logger: coreruntime.NewJSONLogger(&bytes.Buffer{}, false), + } +} + +func TestCreateProviderClient_BaseURLSetWithoutAPIKey_RefusesOAuth(t *testing.T) { + r := newOAuthGuardrailRunner() + + cfg := llm.ClientConfig{ + Model: "moonshotai/Kimi-K2.6", + BaseURL: "https://openrouter-ish.example.com/v1", + // APIKey intentionally empty — would otherwise trigger needsOAuth. + } + + _, err := r.createProviderClient("openai", cfg) + if err == nil { + t.Fatal("expected error when OPENAI_BASE_URL is set without API key, got nil") + } + msg := err.Error() + for _, want := range []string{ + "OPENAI_BASE_URL", + "https://openrouter-ish.example.com/v1", + "OPENAI_API_KEY", + } { + if !strings.Contains(msg, want) { + t.Errorf("error message %q does not contain %q", msg, want) + } + } + // Crucially: must NOT mention chatgpt.com (that's the silent override + // we're guarding against — error tells operator what to fix). + if strings.Contains(msg, "chatgpt.com") { + // The mention of chatgpt.com in the error EXPLANATION is fine + // (we want to tell the operator what would have happened), so + // don't fail on this — but keep the check here for documentation. + t.Logf("error message references chatgpt.com (explanatory): %s", msg) + } +} + +func TestCreateProviderClient_BaseURLSetWithAPIKey_BypassesOAuth(t *testing.T) { + r := newOAuthGuardrailRunner() + + cfg := llm.ClientConfig{ + Model: "moonshotai/Kimi-K2.6", + BaseURL: "https://openrouter-ish.example.com/v1", + APIKey: "sk-real-endpoint-key", + } + + client, err := r.createProviderClient("openai", cfg) + if err != nil { + t.Fatalf("did not expect error when both BASE_URL and API_KEY are set, got: %v", err) + } + if client == nil { + t.Fatal("expected non-nil client") + } +} + +func TestCreateProviderClient_NoBaseURL_AllowsOAuthPath(t *testing.T) { + // When no BaseURL is set and no API key, the existing OAuth path + // is unchanged — the guardrail must not block normal openai.com + // OAuth use. We can't actually verify the OAuth load (no credential + // store in this test), but the error must NOT be the new + // "OPENAI_BASE_URL is set" message — it should be the existing + // "no OpenAI API key or OAuth credentials found" path. + r := newOAuthGuardrailRunner() + + cfg := llm.ClientConfig{ + Model: "gpt-5.4", + // No BaseURL, no APIKey + } + _, err := r.createProviderClient("openai", cfg) + if err == nil { + // If credentials happen to be present locally, this is fine — + // we only care that the new guardrail did not fire. + return + } + if strings.Contains(err.Error(), "OPENAI_BASE_URL is set") { + t.Errorf("guardrail fired even though BaseURL is empty: %v", err) + } +} + +// Sanity: non-openai providers are unaffected. +func TestCreateProviderClient_AnthropicWithBaseURL_Unaffected(t *testing.T) { + r := newOAuthGuardrailRunner() + + cfg := llm.ClientConfig{ + Model: "claude-sonnet-4-20250514", + BaseURL: "https://anthropic-proxy.example.com", + } + client, err := r.createProviderClient("anthropic", cfg) + if err != nil { + t.Fatalf("anthropic provider should not hit the openai-OAuth guardrail: %v", err) + } + if client == nil { + t.Fatal("expected non-nil anthropic client") + } +} diff --git a/forge-ui/handlers_create.go b/forge-ui/handlers_create.go index 2a21d3c..4b557f4 100644 --- a/forge-ui/handlers_create.go +++ b/forge-ui/handlers_create.go @@ -75,10 +75,16 @@ func (s *UIServer) handleGetWizardMeta(w http.ResponseWriter, _ *http.Request) { }, }, "custom": { - Default: "default", - NeedsKey: true, - IsCustom: true, - BaseURLEnv: "MODEL_BASE_URL", + Default: "default", + NeedsKey: true, + IsCustom: true, + // Custom-provider normalization (issue #83): the wizard's + // Custom path is wired through provider=openai + + // OPENAI_BASE_URL/OPENAI_API_KEY at scaffold time. The + // frontend therefore writes OPENAI_BASE_URL directly + // rather than the legacy MODEL_BASE_URL alias, which the + // runtime resolver never read. + BaseURLEnv: "OPENAI_BASE_URL", }, } diff --git a/forge-ui/static/app.js b/forge-ui/static/app.js index d4bc86b..2bf64ce 100644 --- a/forge-ui/static/app.js +++ b/forge-ui/static/app.js @@ -1493,8 +1493,8 @@ function CreatePage() { Base URL * updateEnvVar('MODEL_BASE_URL', e.target.value)} /> + value=${form.env_vars['OPENAI_BASE_URL'] || ''} + onInput=${(e) => updateEnvVar('OPENAI_BASE_URL', e.target.value)} />
OpenAI-compatible API endpoint
`} @@ -1749,7 +1749,7 @@ function CreatePage() { // Filter out vars already collected by previous steps const autoKeys = new Set([ ...Object.keys(PROVIDER_KEY_INFO).map(p => PROVIDER_KEY_INFO[p].envVar), - 'MODEL_BASE_URL', 'WEB_SEARCH_PROVIDER', + 'OPENAI_BASE_URL', 'WEB_SEARCH_PROVIDER', ...form.channels.flatMap(ch => (CHANNEL_TOKEN_FIELDS[ch] || []).map(f => f.key)), ...(meta?.web_search_providers || []).map(p => p.env_var), ...skillEnvVars.required.map(e => e.key), diff --git a/forge-ui/types.go b/forge-ui/types.go index d628890..8f5e4db 100644 --- a/forge-ui/types.go +++ b/forge-ui/types.go @@ -239,7 +239,7 @@ type ProviderModels struct { HasOAuth bool `json:"has_oauth,omitempty"` NeedsKey bool `json:"needs_key"` IsCustom bool `json:"is_custom,omitempty"` - BaseURLEnv string `json:"base_url_env,omitempty"` // e.g. "MODEL_BASE_URL" + BaseURLEnv string `json:"base_url_env,omitempty"` // e.g. "OPENAI_BASE_URL" SupportsOrgID bool `json:"supports_org_id,omitempty"` }