mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 16:17:25 +00:00
fix(providers): use DB name for Anthropic/ClaudeCLI in HTTP onboarding (#942)
* fix(providers): use DB name for Anthropic/ClaudeCLI in HTTP onboarding
Commit 7fcf0327 added WithAnthropicName/WithClaudeCLIName on the startup
registration path (cmd/gateway_providers.go) but missed the HTTP onboarding
path (internal/http/providers.go). Providers created via POST /v1/providers
were registered in-memory with the hardcoded default name ("anthropic",
"claude-cli") instead of the user's chosen DB name.
Symptom: user onboards an Anthropic provider named e.g. "my-anthropic",
picks a model, hits verify — handleVerifyProvider looks up via
GetForTenant(tenantID, p.Name) and fails with "provider not registered:
my-anthropic" because the registry key was "anthropic".
Fix: mirror the startup path by passing WithAnthropicName(p.Name) and
WithClaudeCLIName(p.Name) in registerInMemory. Adds two regression tests
asserting custom-named providers are locatable by their DB name and NOT
by the hardcoded default.
Closes incomplete-refactor gap in #742.
* test(providers): add negative assertion for ClaudeCLI default name
Mirrors the Anthropic test's negative guard: when a user picks a custom
provider name, the hardcoded "claude-cli" default must NOT be present in
the registry. Per Claude review on PR #942 — symmetric test coverage.
This commit is contained in:
1 parent
b1f6eadf85
commit
3e542e7087
2 files changed
+70
-2
No files matched your search
@@ -162,8 +162,10 @@ func (h *ProvidersHandler) registerInMemory(p *store.LLMProviderData) {
|
||||
if cliPath == "" {
|
||||
cliPath = "claude"
|
||||
}
|
||||
var cliOpts []providers.ClaudeCLIOption
|
||||
cliOpts = append(cliOpts, providers.WithClaudeCLISecurityHooks("", true))
|
||||
cliOpts := []providers.ClaudeCLIOption{
|
||||
providers.WithClaudeCLIName(p.Name),
|
||||
providers.WithClaudeCLISecurityHooks("", true),
|
||||
}
|
||||
if h.gatewayAddr != "" {
|
||||
mcpData := providers.BuildCLIMCPConfigData(nil, h.gatewayAddr, pkgGatewayToken)
|
||||
mcpData.AgentMCPLookup = h.mcpLookup
|
||||
@@ -197,6 +199,7 @@ func (h *ProvidersHandler) registerInMemory(p *store.LLMProviderData) {
|
||||
h.providerReg.RegisterForTenant(p.TenantID, codex)
|
||||
case store.ProviderAnthropicNative:
|
||||
h.providerReg.RegisterForTenant(p.TenantID, providers.NewAnthropicProvider(p.APIKey,
|
||||
providers.WithAnthropicName(p.Name),
|
||||
providers.WithAnthropicBaseURL(apiBase)))
|
||||
case store.ProviderDashScope:
|
||||
h.providerReg.RegisterForTenant(p.TenantID, providers.NewDashScopeProvider(p.Name, p.APIKey, apiBase, ""))
|
||||
|
||||
@@ -57,6 +57,71 @@ func TestProvidersHandlerRegisterInMemoryAppliesCodexPoolDefaults(t *testing.T)
|
||||
}
|
||||
}
|
||||
|
||||
// TestProvidersHandlerRegisterInMemoryUsesDBNameForAnthropic guards the onboarding verify flow:
|
||||
// when an Anthropic provider is created via HTTP with a custom name, the in-memory registry
|
||||
// must key the provider by that DB name — not the hardcoded "anthropic" default. Otherwise
|
||||
// handleVerifyProvider's GetForTenant(p.TenantID, p.Name) lookup fails with "provider not registered".
|
||||
// See commit 7fcf0327 for the matching fix on the startup path (cmd/gateway_providers.go).
|
||||
func TestProvidersHandlerRegisterInMemoryUsesDBNameForAnthropic(t *testing.T) {
|
||||
providerReg := providers.NewRegistry(nil)
|
||||
handler := NewProvidersHandler(newMockProviderStore(), newMockSecretsStore(), providerReg, "")
|
||||
|
||||
provider := &store.LLMProviderData{
|
||||
BaseModel: store.BaseModel{ID: uuid.New()},
|
||||
TenantID: uuid.New(),
|
||||
Name: "my-anthropic",
|
||||
ProviderType: store.ProviderAnthropicNative,
|
||||
APIKey: "sk-ant-test",
|
||||
Enabled: true,
|
||||
}
|
||||
|
||||
handler.registerInMemory(provider)
|
||||
|
||||
got, err := providerReg.GetForTenant(provider.TenantID, provider.Name)
|
||||
if err != nil {
|
||||
t.Fatalf("GetForTenant(%q) error = %v, want provider registered under DB name", provider.Name, err)
|
||||
}
|
||||
if got.Name() != provider.Name {
|
||||
t.Fatalf("Name() = %q, want %q", got.Name(), provider.Name)
|
||||
}
|
||||
|
||||
// Negative: the hardcoded default "anthropic" must NOT be registered when the user chose a different name.
|
||||
if _, err := providerReg.GetForTenant(provider.TenantID, "anthropic"); err == nil {
|
||||
t.Fatal("GetForTenant(\"anthropic\") succeeded, want not-found — provider should only live under its DB name")
|
||||
}
|
||||
}
|
||||
|
||||
// TestProvidersHandlerRegisterInMemoryUsesDBNameForClaudeCLI mirrors the Anthropic guard for Claude CLI.
|
||||
// Custom-named CLI providers must be registered under their DB name to be locatable via verify.
|
||||
func TestProvidersHandlerRegisterInMemoryUsesDBNameForClaudeCLI(t *testing.T) {
|
||||
providerReg := providers.NewRegistry(nil)
|
||||
handler := NewProvidersHandler(newMockProviderStore(), newMockSecretsStore(), providerReg, "")
|
||||
|
||||
provider := &store.LLMProviderData{
|
||||
BaseModel: store.BaseModel{ID: uuid.New()},
|
||||
TenantID: uuid.New(),
|
||||
Name: "claude-max",
|
||||
ProviderType: store.ProviderClaudeCLI,
|
||||
APIBase: "claude",
|
||||
Enabled: true,
|
||||
}
|
||||
|
||||
handler.registerInMemory(provider)
|
||||
|
||||
got, err := providerReg.GetForTenant(provider.TenantID, provider.Name)
|
||||
if err != nil {
|
||||
t.Fatalf("GetForTenant(%q) error = %v, want provider registered under DB name", provider.Name, err)
|
||||
}
|
||||
if got.Name() != provider.Name {
|
||||
t.Fatalf("Name() = %q, want %q", got.Name(), provider.Name)
|
||||
}
|
||||
|
||||
// Negative: the hardcoded default "claude-cli" must NOT be registered when the user chose a different name.
|
||||
if _, err := providerReg.GetForTenant(provider.TenantID, "claude-cli"); err == nil {
|
||||
t.Fatal("GetForTenant(\"claude-cli\") succeeded, want not-found — provider should only live under its DB name")
|
||||
}
|
||||
}
|
||||
|
||||
func setupProvidersAdminToken(t *testing.T) string {
|
||||
t.Helper()
|
||||
token := "system-admin-key"
|
||||
|
||||
Reference in new issue
Block a user