diff --git a/scripts/post-rebuild-setup.sh b/scripts/post-rebuild-setup.sh index afa370e4b..51a056c97 100644 --- a/scripts/post-rebuild-setup.sh +++ b/scripts/post-rebuild-setup.sh @@ -41,6 +41,13 @@ until curl -s --max-time 5 "$PLATFORM_URL/health" >/dev/null 2>&1; do done echo " platform up" +# MINIMAX_BASE_URL defaults to the global endpoint (api.minimax.io) — +# matches the operator-host MINIMAX_API_KEY scope. Override by setting +# MINIMAX_BASE_URL before running this script (e.g. for the China +# endpoint api.minimaxi.com). See RFC internal#417 for why the global +# URL is the safe default. +: "${MINIMAX_BASE_URL:=https://api.minimax.io}" + echo "=== Inserting global secrets ===" docker exec "$DB_CONTAINER" psql -U "$DB_USER" -d "$DB_NAME" -c " INSERT INTO global_secrets (key, encrypted_value, encryption_version) VALUES @@ -50,10 +57,12 @@ INSERT INTO global_secrets (key, encrypted_value, encryption_version) VALUES ('ANTHROPIC_SMALL_FAST_MODEL', 'MiniMax-M2.7', 0), ('API_TIMEOUT_MS', '3000000', 0), ('CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC', '1', 0), -('GITHUB_TOKEN', '$GITHUB_PAT', 0) +('GITHUB_TOKEN', '$GITHUB_PAT', 0), +('MINIMAX_API_KEY', '$MINIMAX_API_KEY', 0), +('MINIMAX_BASE_URL', '$MINIMAX_BASE_URL', 0) ON CONFLICT (key) DO UPDATE SET encrypted_value = EXCLUDED.encrypted_value; " -echo " 7 global secrets set" +echo " 9 global secrets set" echo "=== Importing org template ===" curl -s --max-time 600 -X POST "$PLATFORM_URL/org/import" \ diff --git a/workspace-server/internal/handlers/provider_defaults.go b/workspace-server/internal/handlers/provider_defaults.go new file mode 100644 index 000000000..5a1e123f8 --- /dev/null +++ b/workspace-server/internal/handlers/provider_defaults.go @@ -0,0 +1,97 @@ +package handlers + +// Mirror of the third-party LLM provider registry baked into adapter +// templates (workspace-configs-templates/openclaw/adapter.py + +// workspace-configs-templates/claude-code-default/adapter.py + the +// hermes counterparts). Must stay in sync — if a provider is added in +// any of those adapters' base_url defaults, add it here too. +// +// Why this exists (RFC internal#417): +// +// Operators saving a vendor-scoped API key (e.g. MINIMAX_API_KEY) into +// the platform `global_secrets` table expect the workspace to talk to +// the canonical regional endpoint for that key. Pre-fix, when no +// matching _BASE_URL was also saved, the adapter inside the +// workspace fell back to its hardcoded registry default — which for +// MiniMax is api.minimaxi.com (China), while every operator key issued +// against api.minimax.io (global) hits 401 there. The fix is platform- +// side: when we see the API key but no URL, we inject the canonical URL +// so the workspace's adapter precedence chain (operator URL > runtime +// config > adapter default) ends up pointing at the right region. +// +// This is a FALLBACK, never an override. If the operator already +// saved _BASE_URL we leave it alone; their explicit choice +// always wins. If the API key isn't set, we don't inject a stray URL +// for a provider the workspace never uses. +// +// MINIMAX_BASE_URL is set to api.minimax.io (NOT api.minimaxi.com) — +// the global endpoint matches the keys all operator-host secrets are +// issued against. See RFC internal#417 §Phase 1 for the regional +// ambiguity table. +var ProviderBaseURLDefaults = map[string]string{ + "OPENAI_BASE_URL": "https://api.openai.com/v1", + "GROQ_BASE_URL": "https://api.groq.com/openai/v1", + "OPENROUTER_BASE_URL": "https://openrouter.ai/api/v1", + "QIANFAN_BASE_URL": "https://qianfan.baidubce.com/v2", + "MINIMAX_BASE_URL": "https://api.minimax.io", + "MOONSHOT_BASE_URL": "https://api.moonshot.ai/v1", +} + +// applyProviderBaseURLDefaults injects each provider's canonical +// BASE_URL into envVars when the matching _API_KEY is set +// and non-empty AND no _BASE_URL is already present. +// +// The function mutates envVars in place. It is a pure helper (no DB, +// no I/O) so unit tests drive it directly without sqlmock. +// +// Pairing rule: each entry in ProviderBaseURLDefaults names a key of +// the form _BASE_URL; the paired API-key var is derived by +// swapping the suffix to _API_KEY (e.g. MINIMAX_BASE_URL ↔ +// MINIMAX_API_KEY). This mirrors how every adapter registry pairs the +// two — see workspace-configs-templates/*/adapter.py for the canonical +// pairing and RFC internal#417 §Phase 2 for the design rationale. +// +// Returns the list of base-URL keys that were injected, so the caller +// can log which fallbacks fired without re-iterating the map. +func applyProviderBaseURLDefaults(envVars map[string]string) []string { + if envVars == nil { + return nil + } + var injected []string + for baseURLKey, defaultURL := range ProviderBaseURLDefaults { + // Derive the paired API-key var name. Every entry in the map + // MUST end in `_BASE_URL`; if a future entry breaks that + // convention this strip-and-swap silently misses it. The unit + // test TestApplyProviderBaseURLDefaults_KeyShape pins the + // shape so the miss surfaces immediately on next test run. + const baseSuffix = "_BASE_URL" + if len(baseURLKey) <= len(baseSuffix) || + baseURLKey[len(baseURLKey)-len(baseSuffix):] != baseSuffix { + continue + } + apiKeyKey := baseURLKey[:len(baseURLKey)-len(baseSuffix)] + "_API_KEY" + + // API key must be present AND non-empty. An empty string is + // treated as unset — operators sometimes save a placeholder + // row and we don't want to silently route them to a real + // endpoint with no credential. + apiKeyVal, hasKey := envVars[apiKeyKey] + if !hasKey || apiKeyVal == "" { + continue + } + + // Don't overwrite an explicit operator URL. The operator may + // have legitimately scoped this provider to a custom or + // regional endpoint (e.g. a corporate proxy, a different + // MiniMax region). Their explicit value always wins — same + // precedence rule as molecule_runtime.adapter_base's + // resolve_provider_routing. + if existing, ok := envVars[baseURLKey]; ok && existing != "" { + continue + } + + envVars[baseURLKey] = defaultURL + injected = append(injected, baseURLKey) + } + return injected +} diff --git a/workspace-server/internal/handlers/provider_defaults_test.go b/workspace-server/internal/handlers/provider_defaults_test.go new file mode 100644 index 000000000..ab154a9e0 --- /dev/null +++ b/workspace-server/internal/handlers/provider_defaults_test.go @@ -0,0 +1,118 @@ +package handlers + +import ( + "strings" + "testing" +) + +// TestApplyProviderBaseURLDefaults_InjectsWhenKeySetAndURLAbsent — the +// happy path that closes RFC internal#417's reported bug: operator +// saves MINIMAX_API_KEY only, fallback fills in the canonical URL. +func TestApplyProviderBaseURLDefaults_InjectsWhenKeySetAndURLAbsent(t *testing.T) { + envVars := map[string]string{ + "MINIMAX_API_KEY": "sk-real-key", + } + + injected := applyProviderBaseURLDefaults(envVars) + + if got := envVars["MINIMAX_BASE_URL"]; got != "https://api.minimax.io" { + t.Fatalf("MINIMAX_BASE_URL: got %q, want %q", got, "https://api.minimax.io") + } + if len(injected) != 1 || injected[0] != "MINIMAX_BASE_URL" { + t.Fatalf("injected list: got %v, want [MINIMAX_BASE_URL]", injected) + } +} + +// TestApplyProviderBaseURLDefaults_DoesNotOverrideExplicitURL — operator +// override wins. If they saved both MINIMAX_API_KEY and a custom URL +// (say a corporate-proxy or a different region), we leave their URL +// alone. This is the precedence rule from RFC §Phase 2. +func TestApplyProviderBaseURLDefaults_DoesNotOverrideExplicitURL(t *testing.T) { + envVars := map[string]string{ + "MINIMAX_API_KEY": "sk-real-key", + "MINIMAX_BASE_URL": "https://custom.example.com", + } + + injected := applyProviderBaseURLDefaults(envVars) + + if got := envVars["MINIMAX_BASE_URL"]; got != "https://custom.example.com" { + t.Fatalf("MINIMAX_BASE_URL: got %q, want operator value %q", + got, "https://custom.example.com") + } + for _, k := range injected { + if k == "MINIMAX_BASE_URL" { + t.Fatalf("MINIMAX_BASE_URL should not appear in injected list when operator set it explicitly; got %v", injected) + } + } +} + +// TestApplyProviderBaseURLDefaults_NoKeyNoInjection — provider whose +// API key isn't set should not get a stray URL injected. Keeps the +// workspace env clean and avoids accidentally signalling that the +// provider is available when it isn't. +func TestApplyProviderBaseURLDefaults_NoKeyNoInjection(t *testing.T) { + envVars := map[string]string{ + // Some unrelated key set, but no MINIMAX_API_KEY. + "OPENAI_API_KEY": "sk-openai", + } + + injected := applyProviderBaseURLDefaults(envVars) + + if _, ok := envVars["MINIMAX_BASE_URL"]; ok { + t.Fatalf("MINIMAX_BASE_URL was injected without MINIMAX_API_KEY being set; envVars=%v", envVars) + } + // OPENAI_API_KEY is set, so OPENAI_BASE_URL should be injected — + // keeps this test honest about what "no injection" means (it's + // per-provider, not blanket). + if envVars["OPENAI_BASE_URL"] != "https://api.openai.com/v1" { + t.Fatalf("OPENAI_BASE_URL: got %q, want %q", + envVars["OPENAI_BASE_URL"], "https://api.openai.com/v1") + } + for _, k := range injected { + if k == "MINIMAX_BASE_URL" { + t.Fatalf("MINIMAX_BASE_URL should not appear in injected list; got %v", injected) + } + } +} + +// TestApplyProviderBaseURLDefaults_EmptyKeyTreatedAsUnset — operators +// sometimes save a placeholder empty-string row (e.g. they cleared the +// key but didn't delete the row). Treat that the same as the key being +// absent: don't route to a real endpoint with no credential. +func TestApplyProviderBaseURLDefaults_EmptyKeyTreatedAsUnset(t *testing.T) { + envVars := map[string]string{ + "MINIMAX_API_KEY": "", // explicitly empty + } + + injected := applyProviderBaseURLDefaults(envVars) + + if _, ok := envVars["MINIMAX_BASE_URL"]; ok { + t.Fatalf("MINIMAX_BASE_URL should not be injected when MINIMAX_API_KEY=\"\"; envVars=%v", envVars) + } + if len(injected) != 0 { + t.Fatalf("injected list should be empty when key is empty; got %v", injected) + } +} + +// TestApplyProviderBaseURLDefaults_KeyShape — every key in the +// registry must end in `_BASE_URL` so the strip-and-swap in +// applyProviderBaseURLDefaults can derive the paired API-key var +// name. If a future entry breaks that convention the fallback +// silently misses it; this test catches the drift at next CI run. +func TestApplyProviderBaseURLDefaults_KeyShape(t *testing.T) { + for k := range ProviderBaseURLDefaults { + if !strings.HasSuffix(k, "_BASE_URL") { + t.Errorf("ProviderBaseURLDefaults key %q must end in _BASE_URL", k) + } + } +} + +// TestApplyProviderBaseURLDefaults_NilMap — defensive: the helper is +// called from loadWorkspaceSecrets right before return, after both +// query loops. If a future refactor passes nil (e.g. an error path +// returns the bare nil map literal), the helper must not panic. The +// function's nil-guard pins this. +func TestApplyProviderBaseURLDefaults_NilMap(t *testing.T) { + // Must not panic. Return value can be nil/empty either way. + _ = applyProviderBaseURLDefaults(nil) +} diff --git a/workspace-server/internal/handlers/workspace_provision.go b/workspace-server/internal/handlers/workspace_provision.go index 821b313bf..46a5de779 100644 --- a/workspace-server/internal/handlers/workspace_provision.go +++ b/workspace-server/internal/handlers/workspace_provision.go @@ -830,6 +830,21 @@ func loadWorkspaceSecrets(ctx context.Context, workspaceID string) (map[string]s log.Printf("Provisioner: workspace_secrets rows.Err workspace=%s: %v", workspaceID, err) } } + + // Provider BASE_URL fallback (RFC internal#417): when the operator + // saved a vendor API key (e.g. MINIMAX_API_KEY) but no matching + // _BASE_URL, inject the canonical regional URL so the + // in-workspace adapter doesn't fall back to its hardcoded default + // (which is wrong for at least MiniMax — api.minimaxi.com China vs + // api.minimax.io global). Operator-saved URLs win; this only fills + // holes. See provider_defaults.go for the registry + precedence. + if injected := applyProviderBaseURLDefaults(envVars); len(injected) > 0 { + for _, k := range injected { + log.Printf("Provisioner: provider BASE_URL fallback applied for %s: %s=%s", + workspaceID, k, envVars[k]) + } + } + return envVars, "" }