fix: share option resolution between the opencode and Pi builders

AGENTS.md

@@ -56,7 +56,7 @@ The binary lives under `cmd/`; the domain logic is split into `internal/` packag
 
 **Pi config (`internal/pi`).** Pi's `models.json` is plain JSON of the form `{"providers": {"<id>": {baseUrl, api, apiKey, models: [...]}}}`. The whole file is read into a generic map so unknown top-level keys, sibling providers, and unknown provider fields all round-trip untouched; only the managed provider is merged in (models unioned by `id`). `apiKey` is written as a `$ENV_VAR` interpolation (never the resolved secret), and **keyless local providers get a dummy literal placeholder** (`piPlaceholderAPIKey`) because Pi hides a provider's models from `/model` until *some* auth is configured. The reference is otherwise always written, even when the variable is currently unset, because Pi resolves it at run time — the key can be exported after the Outfit is applied. The one exception is `apiKeyOptional` (see the traps below), where an unset variable means a genuinely keyless server rather than a key yet to be supplied. Pi has no top-level default-model setting, so `export` relies on the provider selection alone and `add` tells the user which model to pick.
 
-**Key/option resolution.** `catalog.BuildProviderBlock` sets `options.apiKey` (opencode) to the provider's `apiKeyEnv` as an `{env:VAR}` reference — never the secret — validating `apiKeyPrefix` and erroring when `apiKeyRequired` against the value it resolves. Resolution is `opencode.EnvResolver(dir)`: a `.env` **beside the Outfit being applied**, then the environment — `dir` is empty for a flags-only `add`, which falls back to the working directory. It deliberately is not relative to the binary or the source file (the old `runtime.Caller` form resolved to the build machine's path, so an installed binary never found one). The option is omitted only for a provider with no key variable, or an `apiKeyOptional` one at a local endpoint. Both harnesses therefore rely on the variable being set when the agent runs, which `harnessEnv` in the CLI arranges at launch. `optionsFromEnv` injects other options (e.g. `baseURL`, `region`) when their env vars are set. The opencode top-level `model` is `<providerId>/<modelKey>`. Base-URL precedence for both builders: `--base-url`/`-u` flag > `OUTFIT_BASE_URL` > the catalogue's `pi.baseUrl`/`options.baseURL`.
+**Key/option resolution.** `catalog.BuildProviderBlock` sets `options.apiKey` (opencode) to the provider's `apiKeyEnv` as an `{env:VAR}` reference — never the secret — validating `apiKeyPrefix` and erroring when `apiKeyRequired` against the value it resolves. Resolution is `opencode.EnvResolver(dir)`: a `.env` **beside the Outfit being applied**, then the environment — `dir` is empty for a flags-only `add`, which falls back to the working directory. It deliberately is not relative to the binary or the source file (the old `runtime.Caller` form resolved to the build machine's path, so an installed binary never found one). The option is omitted only for a provider with no key variable, or an `apiKeyOptional` one at a local endpoint. Both harnesses therefore rely on the variable being set when the agent runs, which `harnessEnv` in the CLI arranges at launch. `optionsFromEnv` injects other options (e.g. `baseURL`, `region`) when their env vars are set. Both builders share `Provider.ResolveOptions` for that and `Provider.RequireOptions` for `optionsRequired`, deliberately: when only the opencode builder applied them, a per-provider variable like `OLLAMA_BASE_URL` was silently dropped under Pi, which then handed a *remote* server the keyless placeholder because `IsLocalEndpoint` still saw `localhost`. The opencode top-level `model` is `<providerId>/<modelKey>`. Base-URL precedence for both builders: `--base-url`/`-u` flag > `OUTFIT_BASE_URL` > the provider's own `optionsFromEnv` variable > the catalogue's `pi.baseUrl` (Pi only) > `options.baseURL`.
 
 **`serve`.** `outfit serve` launches a local inference server for an Outfit without touching any harness config. The engine comes from `PROVIDER` via `engineFor` (`cmd/outfit/serve.go`), the local twin of `runnerFor`: `llamacpp` runs `llama-server`, `omlx` runs the Apple-Silicon oMLX CLI, and anything else is an error. A `serveEngine` carries the binary, any subcommand, the preset dialect, the Outfit→flags mapping, and the install hint. With a `PRESET` it flattens the matching `.ini` section (plus `[*]` defaults) into flags in that engine's dialect; otherwise it derives a command from the Outfit alone. Anything the Outfit states overrides the preset. `--dry-run`/`-n` prints the command without running it. oMLX serves a whole `--model-dir` and picks per request, so only `BASEURL` becomes a launch flag for it, and it starts happily with neither `PRESET` nor `MODEL`. `serve` never resolves or passes an API key — it prints the command it runs.
 

CHANGELOG.md

@@ -16,6 +16,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   are not self-hosted engines (it previously ran llama-server whatever the
   Outfit said)
 
+### Fixed
+- fix: apply optionsFromEnv and optionsRequired when building a Pi provider, so
+  per-provider endpoint variables (OLLAMA_BASE_URL, LLAMACPP_BASE_URL,
+  OMLX_BASE_URL, VLLM_BASE_URL, OPENAI_BASE_URL) reach the Pi harness and a
+  remote server is no longer handed the keyless placeholder
+
 ## [1.9.0] - 2026-07-29
 ### Added
 - feat: add a vllm provider to the catalogue

internal/catalog/catalog.go

@@ -119,6 +119,49 @@ func (c *Catalog) SortedProviderNames() []string {
 	return names
 }
 
+// ResolveOptions returns the provider's options with its optionsFromEnv mapping
+// applied over the static ones, which is the view *both* harness builders need:
+// a per-provider variable like OLLAMA_BASE_URL is a user's runtime statement
+// about where the server is, so it is not opencode-specific. resolve looks up
+// env vars (typically a .env, then the environment).
+//
+// The returned map is a fresh copy, so callers may layer their own overrides
+// over it without mutating the catalogue.
+func (p *Provider) ResolveOptions(resolve func(string) string) map[string]any {
+	options := make(map[string]any, len(p.Options)+len(p.OptionsFromEnv))
+	for k, v := range p.Options {
+		options[k] = v
+	}
+	for optKey, envVar := range p.OptionsFromEnv {
+		if v := resolve(envVar); v != "" {
+			options[optKey] = v
+		}
+	}
+	return options
+}
+
+// RequireOptions checks that every option in OptionsRequired resolved to a
+// non-empty value. A provider may require caller-supplied options that have no
+// usable default (e.g. a Vertex AI project); unlike the API key, these are plain
+// options, so apiKeyRequired does not cover them.
+//
+// Both builders call this. Pi's schema carries no general options map, so a
+// provider that both requires an option and declares a `pi` block cannot express
+// it there — failing loudly beats writing an entry that is missing something
+// essential. The embedded catalogue never pairs the two (TestCatalogPiIntegrity
+// enforces that), but a runtime catalogue supplied via --providers can.
+func (p *Provider) RequireOptions(id string, options map[string]any) error {
+	for _, optKey := range p.OptionsRequired {
+		if v, ok := options[optKey]; !ok || v == nil || v == "" {
+			if env := p.OptionsFromEnv[optKey]; env != "" {
+				return fmt.Errorf("the %q option is required for provider %q; set %s in your .env or environment", optKey, id, env)
+			}
+			return fmt.Errorf("the %q option is required for provider %q; set it in the catalogue options", optKey, id)
+		}
+	}
+	return nil
+}
+
 // BuildProviderBlock turns a provider plus an explicit model into an opencode
 // provider block, returning the block and the fully-qualified default model
 // (provider/model), or "" if none was selected. resolve looks up env vars
@@ -137,31 +180,15 @@ func BuildProviderBlock(id string, p *Provider, modelOverride, baseURLOverride s
 		block["npm"] = p.NPM
 	}
 
-	options := map[string]any{}
-	for k, v := range p.Options {
-		options[k] = v
-	}
-	for optKey, envVar := range p.OptionsFromEnv {
-		if v := resolve(envVar); v != "" {
-			options[optKey] = v
-		}
-	}
+	options := p.ResolveOptions(resolve)
 	if baseURLOverride == "" {
 		baseURLOverride = resolve(baseURLEnv)
 	}
 	if baseURLOverride != "" {
 		options["baseURL"] = baseURLOverride
 	}
-	// A provider may require caller-supplied options that have no usable default
-	// (e.g. a Vertex AI project). Unlike the API key, these are plain options,
-	// so apiKeyRequired does not cover them.
-	for _, optKey := range p.OptionsRequired {
-		if v, ok := options[optKey]; !ok || v == nil || v == "" {
-			if env := p.OptionsFromEnv[optKey]; env != "" {
-				return nil, "", fmt.Errorf("the %q option is required for provider %q; set %s in your .env or environment", optKey, id, env)
-			}
-			return nil, "", fmt.Errorf("the %q option is required for provider %q; set it in the catalogue options", optKey, id)
-		}
+	if err := p.RequireOptions(id, options); err != nil {
+		return nil, "", err
 	}
 	if p.APIKeyEnv != "" {
 		key := resolve(p.APIKeyEnv)
@@ -245,17 +272,35 @@ func BuildPiProvider(id string, p *Provider, modelOverride, baseURLOverride stri
 
 	prov := PiProvider{API: p.Pi.API}
 
+	options := p.ResolveOptions(resolve)
 	if baseURLOverride == "" {
 		baseURLOverride = resolve(baseURLEnv)
 	}
+	// Precedence: the explicit override (--base-url, then OUTFIT_BASE_URL), then
+	// the provider's own endpoint variable, then the catalogue's Pi endpoint,
+	// then its opencode one. The per-provider variable sits above both catalogue
+	// values because it is the user speaking about their own machine, and below
+	// the explicit override because that is the user speaking more specifically.
+	var envBaseURL string
+	if v := p.OptionsFromEnv["baseURL"]; v != "" {
+		envBaseURL = resolve(v)
+	}
 	switch {
 	case baseURLOverride != "":
 		prov.BaseURL = baseURLOverride
+	case envBaseURL != "":
+		prov.BaseURL = envBaseURL
 	case p.Pi.BaseURL != "":
 		prov.BaseURL = p.Pi.BaseURL
 	default:
 		prov.BaseURL, _ = p.Options["baseURL"].(string)
 	}
+	if prov.BaseURL != "" {
+		options["baseURL"] = prov.BaseURL
+	}
+	if err := p.RequireOptions(id, options); err != nil {
+		return PiProvider{}, "", err
+	}
 
 	// Pi only surfaces a provider's models in /model once auth is configured;
 	// with no apiKey at all the models load but stay unavailable. Keyless local

internal/catalog/catalog_test.go

@@ -745,3 +745,130 @@ func TestBuildPiProvider_OMLXPlaceholderThenReference(t *testing.T) {
 		t.Errorf("remote apiKey = %q, want $OPENAI_API_KEY", remote.APIKey)
 	}
 }
+
+// TestBuildPiProvider_HonoursPerProviderBaseURLEnv is the regression test for a
+// bug that made the Pi harness unusable against any non-local server: the Pi
+// builder read the catalogue's static options.baseURL directly and never applied
+// optionsFromEnv, so a per-provider endpoint variable was silently dropped.
+//
+// The dropped base URL was not the damaging part. IsLocalEndpoint then saw
+// "localhost", so an apiKeyOptional provider took the keyless branch and Pi was
+// handed the literal placeholder to authenticate a remote server with.
+func TestBuildPiProvider_HonoursPerProviderBaseURLEnv(t *testing.T) {
+	cat, _ := Load()
+	for _, tc := range []struct{ id, env string }{
+		{"omlx", "OMLX_BASE_URL"},
+		{"llamacpp", "LLAMACPP_BASE_URL"},
+		{"vllm", "VLLM_BASE_URL"},
+		{"ollama", "OLLAMA_BASE_URL"},
+	} {
+		t.Run(tc.id, func(t *testing.T) {
+			p := cat.Providers[tc.id]
+			if p == nil {
+				t.Fatalf("%s missing from the catalogue", tc.id)
+			}
+			const remote = "http://box.local:9999/v1"
+			prov, _, err := BuildPiProvider(tc.id, p, "m", "", envMap(map[string]string{tc.env: remote}))
+			if err != nil {
+				t.Fatalf("BuildPiProvider: %v", err)
+			}
+			if prov.BaseURL != remote {
+				t.Errorf("baseUrl = %q, want %q", prov.BaseURL, remote)
+			}
+			// A provider with a key variable must get the reference Pi resolves at
+			// run time, not the placeholder meant for keyless local servers.
+			if p.APIKeyEnv != "" && prov.APIKey != "$"+p.APIKeyEnv {
+				t.Errorf("apiKey = %q, want $%s for a remote endpoint", prov.APIKey, p.APIKeyEnv)
+			}
+		})
+	}
+}
+
+// TestBuildPiProvider_BaseURLPrecedence pins the order the two overrides and the
+// two catalogue values resolve in.
+func TestBuildPiProvider_BaseURLPrecedence(t *testing.T) {
+	cat, _ := Load()
+	omlx := cat.Providers["omlx"]
+
+	t.Run("explicit override beats the provider variable", func(t *testing.T) {
+		prov, _, err := BuildPiProvider("omlx", omlx, "m", "http://explicit:1/v1",
+			envMap(map[string]string{"OMLX_BASE_URL": "http://from-env:2/v1"}))
+		if err != nil {
+			t.Fatalf("BuildPiProvider: %v", err)
+		}
+		if prov.BaseURL != "http://explicit:1/v1" {
+			t.Errorf("baseUrl = %q, want the explicit override", prov.BaseURL)
+		}
+	})
+
+	t.Run("OUTFIT_BASE_URL beats the provider variable", func(t *testing.T) {
+		prov, _, err := BuildPiProvider("omlx", omlx, "m", "", envMap(map[string]string{
+			"OUTFIT_BASE_URL": "http://generic:1/v1",
+			"OMLX_BASE_URL":   "http://from-env:2/v1",
+		}))
+		if err != nil {
+			t.Fatalf("BuildPiProvider: %v", err)
+		}
+		if prov.BaseURL != "http://generic:1/v1" {
+			t.Errorf("baseUrl = %q, want the generic override", prov.BaseURL)
+		}
+	})
+
+	t.Run("unset variable leaves the catalogue value", func(t *testing.T) {
+		prov, _, err := BuildPiProvider("omlx", omlx, "m", "", noEnv)
+		if err != nil {
+			t.Fatalf("BuildPiProvider: %v", err)
+		}
+		if prov.BaseURL != "http://localhost:8000/v1" {
+			t.Errorf("baseUrl = %q, want the catalogue default", prov.BaseURL)
+		}
+	})
+
+	t.Run("pi.baseUrl still wins where no variable applies", func(t *testing.T) {
+		prov, _, err := BuildPiProvider("openrouter", cat.Providers["openrouter"], "m", "", noEnv)
+		if err != nil {
+			t.Fatalf("BuildPiProvider: %v", err)
+		}
+		if prov.BaseURL != "https://openrouter.ai/api/v1" {
+			t.Errorf("baseUrl = %q, want the pi block's endpoint", prov.BaseURL)
+		}
+	})
+}
+
+// TestBuildPiProvider_RequiresOptions closes the second half of the same split:
+// optionsRequired was enforced when building an opencode block but not a Pi one,
+// so a Pi-capable provider missing a required option produced an entry silently
+// lacking it. Pi's schema has no general options map, so failing is the only
+// honest outcome. The catalogue cannot currently express this pairing, but a
+// runtime one supplied via --providers can.
+func TestBuildPiProvider_RequiresOptions(t *testing.T) {
+	p := &Provider{
+		Description:     "test",
+		Options:         map[string]any{"baseURL": "https://example.invalid/v1"},
+		OptionsRequired: []string{"project"},
+		OptionsFromEnv:  map[string]string{"project": "TEST_PROJECT"},
+		Pi:              &PiConfig{API: "openai-completions"},
+	}
+
+	if _, _, err := BuildPiProvider("custom", p, "m", "", noEnv); err == nil {
+		t.Error("expected an error when a required option is unset")
+	} else if !strings.Contains(err.Error(), "TEST_PROJECT") {
+		t.Errorf("error should name the variable to set, got %v", err)
+	}
+
+	if _, _, err := BuildPiProvider("custom", p, "m", "", envMap(map[string]string{"TEST_PROJECT": "p"})); err != nil {
+		t.Errorf("a satisfied requirement should build: %v", err)
+	}
+}
+
+// TestCatalogRequiredOptionsAreNotPiCapable holds the invariant that lets the
+// embedded catalogue avoid the case above: Pi's schema carries no options map,
+// so a provider needing one cannot be served by Pi.
+func TestCatalogRequiredOptionsAreNotPiCapable(t *testing.T) {
+	cat, _ := Load()
+	for name, p := range cat.Providers {
+		if len(p.OptionsRequired) > 0 && p.Pi != nil {
+			t.Errorf("provider %q requires options %v but declares a pi block; Pi cannot carry them", name, p.OptionsRequired)
+		}
+	}
+}

internal/catalog/providers.yaml

@@ -30,8 +30,12 @@
 #                     api      Pi protocol: openai-completions | openai-responses
 #                              | anthropic-messages | google-generative-ai
 #                     baseUrl  Pi endpoint (optional; falls back to
-#                              options.baseURL). Providers without a pi block
-#                              are reported as unsupported by the Pi harness.
+#                              options.baseURL, and is itself overridden by this
+#                              provider's optionsFromEnv baseURL variable).
+#                              Providers without a pi block are reported as
+#                              unsupported by the Pi harness. Pi's schema carries
+#                              no general options map, so a provider using
+#                              optionsRequired cannot declare a pi block.
 
 providers:
   openrouter:

openspec/changes/fix-pi-option-resolution/.openspec.yaml

@@ -0,0 +1,2 @@
+schema: spec-driven
+created: 2026-08-01

openspec/changes/fix-pi-option-resolution/design.md

@@ -0,0 +1,54 @@
+## Context
+
+`BuildProviderBlock` and `BuildPiProvider` translate the same catalogue entry into two
+config formats. They necessarily differ in output shape, but they had also come to differ
+in *input interpretation*, which is not a difference either format requires.
+
+## Decisions
+
+### Shared helpers rather than a parallel fix
+
+The obvious fix — copy the `optionsFromEnv` loop into `BuildPiProvider` — would restore
+correctness and leave the same trap for the next option kind. `optionsRequired` is the
+proof: it was added to one builder only, months after `optionsFromEnv` was, by an author
+with no reason to suspect the other builder existed as a separate code path.
+
+So the resolution moves to `Provider.ResolveOptions` and `Provider.RequireOptions`, and
+both builders call them. A third harness would inherit both for free.
+
+### Where the per-provider variable sits in the precedence
+
+`--base-url` > `OUTFIT_BASE_URL` > `optionsFromEnv` > `pi.baseUrl` > `options.baseURL`.
+
+The interesting placement is `optionsFromEnv` above `pi.baseUrl`. Both are "where is the
+server", but one is written by the catalogue and one by the user, and a user who exports
+`OMLX_BASE_URL` has said something specific about their machine that a shipped default
+cannot know. The two cannot actually collide today — every provider with an
+`optionsFromEnv.baseURL` falls through to `options.baseURL`, and the only provider with a
+`pi.baseUrl` (`openrouter`) has no endpoint variable — so this ordering is about what
+happens when someone adds one, not about current behaviour.
+
+The alternative, slotting it below `pi.baseUrl`, has a nastier failure mode: adding a
+`pi.baseUrl` to `omlx` later would silently stop `OMLX_BASE_URL` from working, which is
+the same class of bug being fixed here.
+
+### Failing on a required option Pi cannot express
+
+A Pi provider entry is `{baseUrl, api, apiKey, models}`. There is nowhere to put a Vertex
+`project`, so a provider that needs one genuinely cannot be served by Pi.
+
+Two guards, because they cover different things. The runtime check in `BuildPiProvider`
+catches a **runtime** catalogue (`--providers`/`OUTFIT_PROVIDERS`), which no test can see.
+The integrity test catches the **embedded** catalogue, and states the invariant somewhere
+a reader will find it. Without the integrity test the runtime error would be the only
+documentation; without the runtime check the invariant would hold only for the shipped
+file.
+
+## Risks / Trade-offs
+
+- **Users relying on the broken behaviour**: implausible. The prior behaviour was a Pi
+  config pointing at a server the user had explicitly redirected away from, which fails at
+  first use rather than working differently.
+- **Ordering choice is forward-looking**: `optionsFromEnv` above `pi.baseUrl` changes
+  nothing today and is asserted by a precedence test, so a future provider combining both
+  gets the documented behaviour rather than an accident.