fix(model): unify LiteLLM model_name contract, remove double-strip

cmd/obol/model.go

@@ -240,9 +240,9 @@ func modelSetupCustomCommand(cfg *config.Config) *cli.Command {
 		Name:  "custom",
 		Usage: "Add a custom OpenAI-compatible endpoint (validates before adding)",
 		Flags: []cli.Flag{
-			&cli.StringFlag{Name: "name", Usage: "Short name for the endpoint (e.g. my-vllm)", Required: true},
+			&cli.StringFlag{Name: "name", Usage: "Short label for the endpoint (informational only — LiteLLM keys the route by --model, not --name)", Required: true},
 			&cli.StringFlag{Name: "endpoint", Usage: "Full base URL (e.g. http://host:8000/v1)", Required: true},
-			&cli.StringFlag{Name: "model", Usage: "Model name at the endpoint", Required: true},
+			&cli.StringFlag{Name: "model", Usage: "Model identifier at the endpoint — this is also the LiteLLM model_name the agent will call", Required: true},
 			&cli.StringFlag{Name: "api-key", Usage: "API key (optional, some endpoints don't require it)"},
 			&cli.BoolFlag{Name: "no-sync", Usage: "Skip the agent model sync (batch with other model commands, then run `obol model sync` once)"},
 		},

internal/hermes/hermes.go

@@ -987,11 +987,20 @@ func syncObolSkills(cfg *config.Config, id string) error {
 	return nil
 }
 
+// configuredModels returns the agent-facing model list and the primary model
+// name. Both are returned as round-trippable LiteLLM `model_name` strings:
+// the agent passes `primary` back as the `model` field on chat-completion
+// calls, and LiteLLM matches by exact string against the entries in the
+// returned slice. NO provider-prefix stripping happens on this path —
+// LiteLLM `model_name` is the contract identifier end-to-end.
+//
+// See internal/model/model.go (AddCustomEndpoint, buildModelEntries) for
+// where the bare-name convention is written into the LiteLLM ConfigMap.
 func configuredModels(cfg *config.Config, u *ui.UI) ([]string, string, error) {
 	models, err := model.GetConfiguredModels(cfg)
 	if err == nil && len(models) > 0 {
 		primary, _ := rankModels(models)
-		return stripProviderPrefixes(models), stripProviderPrefix(primary), nil
+		return models, primary, nil
 	}
 
 	ollamaModels, ollamaErr := model.ListOllamaModels()
@@ -1024,7 +1033,7 @@ func configuredModels(cfg *config.Config, u *ui.UI) ([]string, string, error) {
 	}
 
 	primary, _ := rankModels(names)
-	return names, stripProviderPrefix(primary), nil
+	return names, primary, nil
 }
 
 func generateConfig(cfg *config.Config, primary string) ([]byte, error) {
@@ -1183,38 +1192,17 @@ func litellmMasterKey(cfg *config.Config) string {
 	return "sk-obol-" + strings.TrimSpace(string(data))
 }
 
-func stripProviderPrefix(modelName string) string {
-	modelName = strings.TrimSpace(strings.Trim(modelName, `"'`))
-	if before, after, ok := strings.Cut(modelName, "/"); ok && before != "" && after != "" {
-		return after
-	}
-	return modelName
-}
-
-func stripProviderPrefixes(modelNames []string) []string {
-	if len(modelNames) == 0 {
-		return nil
-	}
-
-	out := make([]string, 0, len(modelNames))
-	for _, name := range modelNames {
-		if trimmed := stripProviderPrefix(name); trimmed != "" {
-			out = append(out, trimmed)
-		}
-	}
-	return out
-}
-
 // rankModels delegates to model.Rank, which knows how to prefer larger local
 // models and frontier cloud models. Kept as a thin wrapper so call sites
-// don't need to import internal/model directly and to preserve the existing
-// stripProviderPrefix shape on the inputs.
+// don't need to import internal/model directly.
+//
+// IMPORTANT: do NOT pre-strip provider prefixes here. model.Rank strips
+// internally for ranking heuristics but returns the ORIGINAL strings so the
+// agent can round-trip them back to LiteLLM. Stripping at this layer would
+// break that round-trip — that's exactly the double-strip bug that
+// ca820c9 worked around for custom endpoints.
 func rankModels(models []string) (primary string, fallbacks []string) {
-	stripped := make([]string, len(models))
-	for i, m := range models {
-		stripped[i] = stripProviderPrefix(m)
-	}
-	return model.Rank(stripped)
+	return model.Rank(models)
 }
 
 func k3dNodeExec(cfg *config.Config, hostPath, shellCmd string) error {

internal/hermes/hermes_test.go

@@ -23,6 +23,51 @@ func testConfig(t *testing.T) *config.Config {
 	return &config.Config{ConfigDir: dir, DataDir: dir, BinDir: dir}
 }
 
+// TestGenerateConfig_PrimaryIsRoundTrippable guards the LiteLLM model_name
+// contract end-to-end: whatever string the agent's `model.default` is set to
+// MUST match a `model_name` entry in the LiteLLM ConfigMap byte-for-byte,
+// because Hermes will pass it back as the `model` field on every
+// chat-completion call. Stripping anywhere on this path causes the agent to
+// call LiteLLM with a key that no longer matches a registered route — the
+// flow-14 / ca820c9 regression.
+func TestGenerateConfig_PrimaryIsRoundTrippable(t *testing.T) {
+	cases := []struct {
+		name    string
+		primary string
+	}{
+		{"bare ollama tag", "qwen3.5:9b"},
+		{"bare claude id", "claude-opus-4-7"},
+		{"bare openai id", "gpt-5.4"},
+		// Wildcard-expanded entries can carry the provider prefix; the
+		// agent must still send back the exact string LiteLLM published.
+		{"provider-prefixed", "anthropic/claude-3-5-sonnet-latest"},
+		// Custom endpoints write `model_name: <bare>` after the contract
+		// fix; this case guards that the agent picks up that bare name
+		// without re-namespacing.
+		{"custom endpoint bare", "qwen36-fast"},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			raw, err := generateConfig(testConfig(t), tc.primary)
+			if err != nil {
+				t.Fatalf("generateConfig: %v", err)
+			}
+			var cfg map[string]any
+			if err := yaml.Unmarshal(raw, &cfg); err != nil {
+				t.Fatalf("yaml.Unmarshal: %v", err)
+			}
+			modelCfg, ok := cfg["model"].(map[string]any)
+			if !ok {
+				t.Fatalf("model config missing")
+			}
+			if got := modelCfg["default"]; got != tc.primary {
+				t.Fatalf("model.default = %q, want %q (round-trip mismatch)", got, tc.primary)
+			}
+		})
+	}
+}
+
 func TestGenerateConfig_UsesLiteLLMCustomProvider(t *testing.T) {
 	raw, err := generateConfig(testConfig(t), "gpt-5.2")
 	if err != nil {

internal/hermes/rankmodels_test.go

@@ -6,13 +6,17 @@ import "testing"
 // from the colleague's screenshot: Hermes was deploying with `llama3.2:1b` as
 // the default model, which then parroted its own tool list back on every
 // "hello" prompt. The fix moved capability ranking into model.Rank; this test
-// just confirms the Hermes-side wrapper still calls into it correctly and
-// keeps the openai/-prefix-stripping shape intact.
+// just confirms the Hermes-side wrapper still calls into it correctly.
+//
+// Contract: bare LiteLLM model_name strings come in, the SAME bare strings
+// come back out — no provider-prefix stripping at this layer. The agent must
+// be able to round-trip the returned primary back to LiteLLM without
+// modification.
 func TestRankModels_HermesWrapper_PrefersLargerLocalModel(t *testing.T) {
 	primary, fallbacks := rankModels([]string{
-		"openai/llama3.2:1b",
-		"openai/qwen3.5:9b",
-		"openai/llama3.2:3b",
+		"llama3.2:1b",
+		"qwen3.5:9b",
+		"llama3.2:3b",
 	})
 	if primary != "qwen3.5:9b" {
 		t.Fatalf("primary: got %q, want qwen3.5:9b", primary)
@@ -22,13 +26,47 @@ func TestRankModels_HermesWrapper_PrefersLargerLocalModel(t *testing.T) {
 	}
 }
 
+// TestRankModels_HermesWrapper_PrefersClaudeOverLocal exercises the cloud
+// tier. Cloud entries written by buildModelEntries are bare (e.g.
+// `claude-opus-4-7`, not `anthropic/claude-opus-4-7`), and the wrapper must
+// preserve that.
 func TestRankModels_HermesWrapper_PrefersClaudeOverLocal(t *testing.T) {
 	primary, _ := rankModels([]string{
 		"qwen3.5:9b",
-		"anthropic/claude-opus-4-7",
+		"claude-opus-4-7",
 		"llama3.2:1b",
 	})
 	if primary != "claude-opus-4-7" {
 		t.Fatalf("primary: got %q, want claude-opus-4-7", primary)
 	}
 }
+
+// TestRankModels_HermesWrapper_PreservesProviderPrefixIfPresent guards the
+// round-trip property. If something upstream (a wildcard expansion, a
+// hand-edited ConfigMap, an older release) writes a `provider/model` shape
+// into LiteLLM, we still need to return the EXACT string so the agent's
+// chat-completion call matches by literal string. Stripping at this layer
+// was the double-strip bug fixed in ca820c9 — this test guards against
+// reintroducing it.
+func TestRankModels_HermesWrapper_PreservesProviderPrefixIfPresent(t *testing.T) {
+	primary, _ := rankModels([]string{
+		"anthropic/claude-opus-4-7",
+		"openai/gpt-4o",
+		"qwen3.5:9b",
+	})
+	if primary != "anthropic/claude-opus-4-7" {
+		t.Fatalf("primary: got %q, want anthropic/claude-opus-4-7 (unstripped)", primary)
+	}
+}
+
+// TestRankModels_HermesWrapper_CustomNamespacedEntryRoundTrips guards the
+// specific shape that broke flow-14: a legacy `custom/<name>/<model>` entry
+// that double-stripping would mangle to `<model>`, leaving the agent calling
+// LiteLLM with a key that no longer matched the registered route.
+func TestRankModels_HermesWrapper_CustomNamespacedEntryRoundTrips(t *testing.T) {
+	in := []string{"custom/spark1-vllm/qwen36-fast"}
+	primary, _ := rankModels(in)
+	if primary != in[0] {
+		t.Fatalf("primary: got %q, want %q (must round-trip unchanged)", primary, in[0])
+	}
+}

internal/model/model.go

@@ -630,6 +630,22 @@ func RemoveModel(cfg *config.Config, u *ui.UI, modelName string) error {
 
 // AddCustomEndpoint adds a custom OpenAI-compatible endpoint to LiteLLM
 // after validating it works.
+//
+// LiteLLM `model_name` contract — the canonical identifier is the bare
+// `modelName`. Same convention every other code path in this stack uses:
+// Ollama writes `qwen3.5:9b`, Anthropic writes `claude-opus-4-7`, OpenAI
+// writes `gpt-5.4`. The agent (Hermes / OpenClaw) reads `model_name` straight
+// back as the `model` field on chat-completion calls — any provider-prefix
+// namespacing (`custom/<name>/<model>`) on this side breaks that round-trip
+// because the agent then strips it and calls LiteLLM with a key that doesn't
+// match.
+//
+// The `name` arg is informational only. It is surfaced via
+// `obol model status` / `list` for human reference but does NOT participate
+// in the LiteLLM route key. Two custom endpoints that publish the same
+// `modelName` will overwrite each other in the LiteLLM ConfigMap; that is
+// the natural "repoint my model" behavior an operator running
+// `obol model setup custom` wants when they re-run the command.
 func AddCustomEndpoint(cfg *config.Config, u *ui.UI, name, endpoint, modelName, apiKey string) error {
 	kubectlBinary := filepath.Join(cfg.BinDir, "kubectl")
 	kubeconfigPath := filepath.Join(cfg.ConfigDir, "kubeconfig.yaml")
@@ -658,38 +674,17 @@ func AddCustomEndpoint(cfg *config.Config, u *ui.UI, name, endpoint, modelName,
 		u.Infof("Cluster endpoint: %s (translated from %s)", clusterEndpoint, endpoint)
 	}
 
-	// Build model entry. The LiteLLM `model_name` is the user-facing
-	// identifier the agent will pass on chat-completion calls. We use the
-	// bare `modelName` so the agent's request matches the LiteLLM entry by
-	// exact string — the `name` flag is still surfaced in `obol model
-	// status` / `list` for human reference, but LiteLLM keys the route by
-	// the model alone. Re-running `obol model setup custom --name X
-	// --model Y` with the same Y simply re-binds, which is the natural
-	// "repoint my model" behavior an operator wants.
-	//
-	// (The historical `custom/<name>/<model>` namespaced ID caused
-	// Hermes to call LiteLLM with a stripped name that no longer matched
-	// the entry, surfacing as 400 "no healthy deployments for this model"
-	// on every agent invocation.)
-	litellmModel := "openai/" + modelName
-	modelID := modelName
-	_ = name // currently informational only; reserved for future multi-endpoint namespacing
+	entry := buildCustomEndpointEntry(modelName, clusterEndpoint, apiKey)
 
-	entry := ModelEntry{
-		ModelName: modelID,
-		LiteLLMParams: LiteLLMParams{
-			Model:   litellmModel,
-			APIBase: clusterEndpoint,
-			APIKey:  apiKey,
-		},
-	}
-	if apiKey == "" {
-		entry.LiteLLMParams.APIKey = "none"
+	// Patch ConfigMap for persistence. The display label is logged so an
+	// operator can correlate the call with their `--name` arg, but it isn't
+	// part of the route key.
+	if name != "" {
+		u.Infof("Adding custom endpoint %q (model: %s) to LiteLLM config", name, modelName)
+	} else {
+		u.Infof("Adding custom endpoint (model: %s) to LiteLLM config", modelName)
 	}
 
-	// Patch ConfigMap for persistence.
-	u.Infof("Adding custom endpoint %q to LiteLLM config", name)
-
 	if err := patchLiteLLMConfig(kubectlBinary, kubeconfigPath, []ModelEntry{entry}); err != nil {
 		return fmt.Errorf("failed to update LiteLLM config: %w", err)
 	}
@@ -700,7 +695,7 @@ func AddCustomEndpoint(cfg *config.Config, u *ui.UI, name, endpoint, modelName,
 		return RestartLiteLLM(cfg, u, name)
 	}
 
-	u.Successf("Custom endpoint %q added (model: %s)", name, modelID)
+	u.Successf("Custom endpoint %q added (model: %s)", name, modelName)
 
 	return nil
 }
@@ -1143,6 +1138,27 @@ func buildModelEntries(provider string, models []string) []ModelEntry {
 	return entries
 }
 
+// buildCustomEndpointEntry constructs the LiteLLM ModelEntry for a custom
+// OpenAI-compatible endpoint added via `obol model setup custom`. The
+// `model_name` is the bare `modelName` — see the AddCustomEndpoint doc
+// comment for the round-trip contract this enforces. Extracted as a
+// standalone helper so the entry shape is unit-testable without going
+// through the full kubectl-driven AddCustomEndpoint path.
+func buildCustomEndpointEntry(modelName, clusterEndpoint, apiKey string) ModelEntry {
+	entry := ModelEntry{
+		ModelName: modelName,
+		LiteLLMParams: LiteLLMParams{
+			Model:   "openai/" + modelName,
+			APIBase: clusterEndpoint,
+			APIKey:  apiKey,
+		},
+	}
+	if apiKey == "" {
+		entry.LiteLLMParams.APIKey = "none"
+	}
+	return entry
+}
+
 // patchLiteLLMConfig reads the current config.yaml from the ConfigMap,
 // merges new model entries (replacing existing by model_name), and patches back.
 func patchLiteLLMConfig(kubectlBinary, kubeconfigPath string, entries []ModelEntry) error {

internal/model/model_test.go

@@ -88,6 +88,57 @@ func TestBuildModelEntries(t *testing.T) {
 	})
 }
 
+// TestBuildCustomEndpointEntry encodes the LiteLLM `model_name` contract for
+// custom endpoints added via `obol model setup custom`:
+//
+//   - `model_name` is the BARE model identifier (no `custom/<name>/` namespace
+//     and no `openai/` provider prefix). The agent reads this string and
+//     passes it back as the `model` field on chat-completion calls; any
+//     namespacing here breaks the round-trip — the flow-14 / ca820c9 bug.
+//   - `litellm_params.model` carries the `openai/` provider hint so LiteLLM
+//     routes through its OpenAI-compatible adapter to the user's upstream.
+//   - The `name` (label) flag does NOT participate in the route key.
+func TestBuildCustomEndpointEntry(t *testing.T) {
+	t.Run("bare model_name with openai-compat routing", func(t *testing.T) {
+		entry := buildCustomEndpointEntry("qwen36-fast", "http://host.k3d.internal:8000/v1", "secret-key")
+		if entry.ModelName != "qwen36-fast" {
+			t.Errorf("ModelName = %q, want bare %q (contract: bare LiteLLM model_name)", entry.ModelName, "qwen36-fast")
+		}
+		if strings.HasPrefix(entry.ModelName, "custom/") {
+			t.Errorf("ModelName = %q must NOT carry custom/<name>/ namespace", entry.ModelName)
+		}
+		if entry.LiteLLMParams.Model != "openai/qwen36-fast" {
+			t.Errorf("litellm_params.model = %q, want openai/qwen36-fast", entry.LiteLLMParams.Model)
+		}
+		if entry.LiteLLMParams.APIBase != "http://host.k3d.internal:8000/v1" {
+			t.Errorf("api_base = %q", entry.LiteLLMParams.APIBase)
+		}
+		if entry.LiteLLMParams.APIKey != "secret-key" {
+			t.Errorf("api_key = %q, want secret-key", entry.LiteLLMParams.APIKey)
+		}
+	})
+
+	t.Run("empty api_key falls back to none", func(t *testing.T) {
+		// Some self-hosted OpenAI-compatible servers (vLLM, llama.cpp, mlx-lm)
+		// don't require auth. LiteLLM still wants a non-empty api_key field
+		// or the openai client errors before even hitting the endpoint.
+		entry := buildCustomEndpointEntry("any-model", "http://host:8000/v1", "")
+		if entry.LiteLLMParams.APIKey != "none" {
+			t.Errorf("api_key = %q, want %q (LiteLLM placeholder)", entry.LiteLLMParams.APIKey, "none")
+		}
+	})
+
+	t.Run("model with colon tag survives intact", func(t *testing.T) {
+		// A custom endpoint can serve a tagged model id (e.g. mlx-lm publishing
+		// `qwen3:9b-mlx`). The colon must NOT be stripped or interpreted as a
+		// provider separator.
+		entry := buildCustomEndpointEntry("qwen3:9b-mlx", "http://host:8000/v1", "")
+		if entry.ModelName != "qwen3:9b-mlx" {
+			t.Errorf("ModelName = %q, want qwen3:9b-mlx unchanged", entry.ModelName)
+		}
+	})
+}
+
 func TestExpandWildcard(t *testing.T) {
 	t.Run("uses live models when available", func(t *testing.T) {
 		live := []string{"claude-sonnet-4-6", "claude-opus-4", "gpt-4o"}

internal/model/rank.go

@@ -88,6 +88,17 @@ func IsCloudModel(name string) bool {
 	return false
 }
 
+// stripProviderPrefix is an internal helper for ranking only. It is NOT
+// exported and MUST NOT be used to mutate model identifiers that the agent
+// will pass back to LiteLLM on chat-completion calls.
+//
+// LiteLLM `model_name` is bare (no provider prefix) — see the contract
+// documented on AddCustomEndpoint and buildModelEntries. The only place a
+// `provider/model` shape can sneak in is wildcard entries like `anthropic/*`,
+// or legacy entries from older releases that namespaced custom endpoints as
+// `custom/<name>/<model>`. We strip those here so size/family parsing in
+// IsCloudModel/cloudRank/localRank still works on tagged tokens, but the
+// caller in Rank() returns the ORIGINAL string — never the stripped form.
 func stripProviderPrefix(name string) string {
 	if idx := strings.Index(name, "/"); idx >= 0 {
 		return name[idx+1:]