Fix enforced connection fields overridden by user env vars

memory-bank/aiConfig.md

@@ -126,7 +126,8 @@ Config flows through three stages: **assemble sources → resolve → watch**. P
    so the sealed `enforced` overlay can never be overwritten, apply the
    `PlatformBaseline` beneath, and build `ResolvedProvider[]` via `build-catalog.ts`.
    Objects deep-merge per leaf-key (`mergeConfigFragments`), `allow`/`deny` arrays
-   wholesale-replace, and a non-secret env-var overlay applies on top.
+   wholesale-replace. Connection env vars are a resolver-owned source ranked
+   below `enforced` but above `user`/`host`/`default` — not a post-resolution overlay.
    `loadResolvedProviderCatalog()` (`src/node/load-catalog.ts`) is the public read
    seam that composes assembly + resolve and returns `readonly ResolvedProvider[]`.
    (`mergeEnforced` — the two-layer merge — remains exported as a low-level
@@ -149,7 +150,7 @@ reusable independent of the catalog builder.
 - **Enablement** (`resolveEnabled`): enforced per-provider > enforced default >
   user per-provider > user default > platform-baseline per-provider > baseline
   default.
-- **Connection**: non-secret env-var overlay (highest) > enforced > user file >
+- **Connection**: enforced > connection env vars > user file >
   injected `host` sources (e.g. Positron `authentication.*` via
   `additionalSources`) > built-in defaults. Object keys deep-merge across layers.
 - **Model routing**: user config (override/custom) > provider config > discovered
@@ -298,7 +299,7 @@ the bridge's `ModelInfo` — compatible by contract, not by import.
 | `src/node/paths.ts`                   | `AI_CONFIG_DIR`, `PROVIDERS_CONFIG_PATH`, enforced env-var name, lockfile path                                      |
 | `src/node/types.ts`                   | Node seam option/result types (`LoadCatalogOptions`, `ProviderCatalogChange`, `Disposable`, …)                      |
 | `src/resolve-catalog.ts`              | `resolveProviderCatalog()` — pure deep resolver seam; owns the precedence stack + sealed-enforced invariant         |
-| `src/build-catalog.ts`                | `buildCatalog()` — assemble `ResolvedProvider[]` from the resolved config + baseline + env overlay (pure entry)     |
+| `src/build-catalog.ts`                | `buildCatalog()` — assemble `ResolvedProvider[]` from merged config + enablement layers + baseline (pure entry)     |
 | `src/node/load-config.ts`             | `loadConfigSources()` / `readFileConfig()` / `readEnvFragment()` — assemble the ordered `ProviderConfigSource` list |
 | `src/node/load-catalog.ts`            | `loadResolvedProviderCatalog()` — public read seam (assemble sources → `resolveProviderCatalog`)                    |
 | `src/node/mutate-config.ts`           | `mutateProvidersConfig()` — locked, atomic, serialized mutation                                                     |

packages/ai-config/README.md

@@ -101,11 +101,11 @@ The **deep resolver seam** — the one place that owns the entire precedence sta
 function resolveProviderCatalog(opts: {
   sources: readonly ProviderConfigSource[]; // any order — ranked by `kind`
   baseline: PlatformBaseline;
-  envVars?: Record<string, string | undefined>; // non-secret connection overlay (default {})
+  envVars?: Record<string, string | undefined>; // non-secret connection source ranked below enforced (default {})
 }): readonly ResolvedProvider[];
 ```
 
-A `ProviderConfigSource` declares _what it is_ via `kind`, not _where it sits_. The resolver maps each kind to a fixed rank, highest → lowest: **`enforced` > `user` > `host` > `default`**, with the `PlatformBaseline` beneath all sources. The `enforced` layer is a **sealed overlay** — no lower source can overwrite an enforced key (a correctness invariant). Object fields deep-merge per leaf-key; `allow`/`deny` arrays wholesale-replace.
+A `ProviderConfigSource` declares _what it is_ via `kind`, not _where it sits_. The resolver maps each kind to a fixed rank, highest → lowest: **`enforced` > connection env > `user` > `host` > `default`**, with the `PlatformBaseline` beneath all sources. The `enforced` layer is a **sealed overlay** — no lower source can overwrite an enforced key (a correctness invariant). Connection env vars (from `envVars`) are converted into a resolver-owned source ranked below `enforced` but above `user`, so admin pins always win while env vars still override file-based config. Object fields deep-merge per leaf-key; `allow`/`deny` arrays wholesale-replace.
 
 `mergeEnforced` (two-layer) and `mergeConfigFragments` (layered) remain exported as the low-level merge primitives, but consumers should assemble sources and call `resolveProviderCatalog` rather than merging by hand.
 
@@ -186,15 +186,15 @@ const catalog = await loadResolvedProviderCatalog({
 
 `LoadCatalogOptions`:
 
-| Field                 | Description                                                                                                                                    |
-| --------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------- |
-| `baseline` (required) | `PlatformBaseline` — enablement defaults for this platform.                                                                                    |
-| `configPath?`         | Override the file path (testing).                                                                                                              |
-| `enforcedEnvVar?`     | Override the enforced env-var name (defaults to `POSIT_AI_PROVIDERS_ENFORCED`; testing).                                                       |
-| `defaultEnvVar?`      | Override the defaults env-var name (defaults to `POSIT_AI_PROVIDERS_DEFAULT`; testing).                                                        |
-| `envVars?`            | Source for the non-secret connection overlay **and** the enforced/default fragment env vars (defaults to `process.env`).                       |
-| `additionalSources?`  | Extra watchable `ProviderConfigSourceProvider`s (e.g. a Positron `authentication.*` `host` source). Folded into both the load and watch paths. |
-| `logger?`             | `LoggerLike` for diagnostics/validation warnings.                                                                                              |
+| Field                 | Description                                                                                                                                                                                        |
+| --------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `baseline` (required) | `PlatformBaseline` — enablement defaults for this platform.                                                                                                                                        |
+| `configPath?`         | Override the file path (testing).                                                                                                                                                                  |
+| `enforcedEnvVar?`     | Override the enforced env-var name (defaults to `POSIT_AI_PROVIDERS_ENFORCED`; testing).                                                                                                           |
+| `defaultEnvVar?`      | Override the defaults env-var name (defaults to `POSIT_AI_PROVIDERS_DEFAULT`; testing).                                                                                                            |
+| `envVars?`            | Source for non-secret connection env vars (converted into a resolver-owned source ranked below `enforced`) **and** for reading the enforced/default fragment env vars (defaults to `process.env`). |
+| `additionalSources?`  | Extra watchable `ProviderConfigSourceProvider`s (e.g. a Positron `authentication.*` `host` source). Folded into both the load and watch paths.                                                     |
+| `logger?`             | `LoggerLike` for diagnostics/validation warnings.                                                                                                                                                  |
 
 #### `mutateProvidersConfig(mutator, opts?): Promise<void>`
 
@@ -272,7 +272,7 @@ sub.dispose();
 Config flows through **assemble sources → resolve → watch**:
 
 1. **Assemble sources**: the node seam reads the user file (missing → `{}`, validated against `providersConfigSchema`), the enforced fragment from `POSIT_AI_PROVIDERS_ENFORCED`, and the defaults fragment from `POSIT_AI_PROVIDERS_DEFAULT` (both validated against the relaxed `enforcedProvidersConfigSchema`), plus any `additionalSources` (e.g. a Positron `authentication.*` `host` source). Each becomes a `ProviderConfigSource` tagged with its `kind`.
-2. **Resolve** (`resolveProviderCatalog`): rank the sources by kind (`enforced` > `user` > `host` > `default`), fold them low → high so the sealed `enforced` overlay can never be overwritten, apply the `PlatformBaseline` beneath, and build `ResolvedProvider[]` — objects deep-merge per leaf-key, `allow`/`deny` arrays wholesale-replace, and a non-secret env-var overlay applies on top.
+2. **Resolve** (`resolveProviderCatalog`): rank the sources by kind (`enforced` > connection env > `user` > `host` > `default`), fold them low → high so the sealed `enforced` overlay can never be overwritten, apply the `PlatformBaseline` beneath, and build `ResolvedProvider[]` — objects deep-merge per leaf-key, `allow`/`deny` arrays wholesale-replace. Connection env vars are a resolver-owned source below `enforced`, not a post-resolution overlay.
 3. **Watch** (`watchResolvedProviderCatalog`): debounced (~300ms), ancestor-aware watch over **every** source (file via `fs.watch`; host sources emit their own change signals; env sources are static) that re-resolves, diffs against the previous catalog, and emits a typed change only when something actually changed.
 
 The read path **degrades gracefully**: malformed or missing files log a warning and fall back rather than throwing.

packages/ai-config/src/__tests__/build-catalog-custom.test.ts

@@ -32,15 +32,9 @@ function configWithCustom(): ProvidersConfig {
 
 describe("buildCatalog custom providers", () => {
 	it("includes custom providers", () => {
-		const catalog = buildCatalog(configWithCustom(), layersOf(configWithCustom()), BASELINE, {});
-		const customEntry = catalog.find((p) => p.id === "my-gateway");
-		expect(customEntry).toBeDefined();
-		expect(customEntry!.clientKind).toBe("openai-compatible");
-	});
-
-	it("includes custom providers when options is undefined (default)", () => {
 		const catalog = buildCatalog(configWithCustom(), layersOf(configWithCustom()), BASELINE);
 		const customEntry = catalog.find((p) => p.id === "my-gateway");
 		expect(customEntry).toBeDefined();
+		expect(customEntry!.clientKind).toBe("openai-compatible");
 	});
 });

packages/ai-config/src/__tests__/resolve-catalog.test.ts

@@ -389,13 +389,66 @@ describe("resolveProviderCatalog — host layer merge semantics (Phase 6)", () =
 	});
 });
 
+describe("resolveProviderCatalog — enforced beats connection env", () => {
+	it("enforced baseUrl wins over connection env var", () => {
+		const catalog = resolveProviderCatalog({
+			sources: [
+				source("enforced", {
+					providers: { anthropic: { baseUrl: "https://enforced.example.com" } },
+				}),
+				source("user", { providers: {} }),
+			],
+			baseline: STANDALONE,
+			envVars: { ANTHROPIC_BASE_URL: "https://env.example.com" },
+		});
+		expect(find(catalog, "anthropic")?.connection.baseUrl).toBe("https://enforced.example.com");
+	});
+
+	it("env beats user/default when no enforced source pins the field", () => {
+		const catalog = resolveProviderCatalog({
+			sources: [
+				source("user", {
+					providers: { anthropic: { baseUrl: "https://user.example.com" } },
+				}),
+				source("default", {
+					providers: { anthropic: { baseUrl: "https://default.example.com" } },
+				}),
+			],
+			baseline: STANDALONE,
+			envVars: { ANTHROPIC_BASE_URL: "https://env.example.com" },
+		});
+		expect(find(catalog, "anthropic")?.connection.baseUrl).toBe("https://env.example.com");
+	});
+
+	it("enforced positaiLogin.host wins over POSITAI_AUTH_HOST env, env-only sub-key still lands", () => {
+		const catalog = resolveProviderCatalog({
+			sources: [
+				source("enforced", {
+					providers: { positai: { positaiLogin: { host: "enforced.login.com" } } },
+				}),
+				source("user", { providers: {} }),
+			],
+			baseline: STANDALONE,
+			envVars: {
+				POSITAI_AUTH_HOST: "env.login.com",
+				POSITAI_CLIENT_ID: "env-client-id",
+			},
+		});
+		const login = find(catalog, "positai")?.connection.positaiLogin;
+		// Enforced host wins over env
+		expect(login?.host).toBe("enforced.login.com");
+		// Env-only sub-key (clientId not in enforced) still lands
+		expect(login?.clientId).toBe("env-client-id");
+	});
+});
+
 describe("recoverValidStack — choose dropped source", () => {
 	/** Custom entry with no `type` — uncompletable unless another source supplies it. */
 	const badCustom = (name: string): ProviderConfigSource["config"] => ({
 		providers: { custom: { [name]: { enabled: false } } },
 	});
 
-	function keptKinds(sources: readonly ProviderConfigSource[]): string[] {
+	function keptKinds(sources: readonly { readonly kind: string }[]): string[] {
 		return sources.map((s) => s.kind);
 	}
 

packages/ai-config/src/build-catalog.ts

@@ -27,55 +27,6 @@ import { mintCustomProviderId } from "./types.js";
 import { BUILTIN_PROVIDER_IDS } from "./vocabulary.js";
 import type { BuiltinProviderId, ClientKind } from "./vocabulary.js";
 
-// ---------------------------------------------------------------------------
-// Non-secret connection env var mappings
-// ---------------------------------------------------------------------------
-
-/**
- * Maps environment variable names to non-secret connection fields for
- * built-in providers. Env vars have the highest precedence in the
- * connection resolution chain: env > file (providers.json) > defaults.
- *
- * Only non-secret connection config goes here. Secret env vars (API keys,
- * AWS secret keys) are handled by the separate `envCredentialResolver` in
- * `@assistant/node`.
- */
-interface ConnectionEnvMapping {
-	baseUrl?: string;
-	endpoint?: string;
-	positaiLogin?: { host?: string; clientId?: string; scope?: string };
-	aws?: { region?: string; profile?: string };
-	googleCloud?: { project?: string; location?: string };
-}
-
-const CONNECTION_ENV_MAPPINGS: Readonly<Record<string, ConnectionEnvMapping>> = {
-	anthropic: { baseUrl: "ANTHROPIC_BASE_URL" },
-	openai: { baseUrl: "OPENAI_BASE_URL" },
-	gemini: { baseUrl: "GEMINI_BASE_URL" },
-	positai: {
-		baseUrl: "POSITAI_BASE_URL",
-		positaiLogin: {
-			host: "POSITAI_AUTH_HOST",
-			clientId: "POSITAI_CLIENT_ID",
-			scope: "POSITAI_SCOPE",
-		},
-	},
-	openrouter: { baseUrl: "OPENROUTER_BASE_URL" },
-	ollama: { endpoint: "OLLAMA_ENDPOINT" },
-	lmstudio: { endpoint: "LMSTUDIO_ENDPOINT" },
-	bedrock: { aws: { region: "AWS_REGION", profile: "AWS_PROFILE" } },
-	"google-vertex": {
-		googleCloud: {
-			project: "GOOGLE_CLOUD_PROJECT",
-			location: "GOOGLE_CLOUD_LOCATION",
-		},
-	},
-	"openai-compatible": { baseUrl: "OPENAI_COMPATIBLE_BASE_URL" },
-	"ms-foundry": { baseUrl: "MS_FOUNDRY_BASE_URL" },
-	"snowflake-cortex": { baseUrl: "SNOWFLAKE_BASE_URL" },
-	deepseek: { baseUrl: "DEEPSEEK_BASE_URL" },
-};
-
 // ---------------------------------------------------------------------------
 // Built-in provider id → client kind mapping
 // ---------------------------------------------------------------------------
@@ -113,7 +64,8 @@ const BUILTIN_CLIENT_KIND = {
  * This is the **catalog builder** behind `resolveProviderCatalog` — consumers
  * iterate the result instead of the static registry. Connection, model
  * policy, and client kind are read from `mergedConfig` (the deep-merged
- * result where higher-precedence sources win per key). Enablement is resolved
+ * result where higher-precedence sources win per key, including connection
+ * env vars already folded by the resolver). Enablement is resolved
  * separately from `enabledLayers` (highest precedence first) so the sealed
  * enforced overlay can never be overridden and per-layer "id beats default"
  * semantics are preserved.
@@ -122,27 +74,15 @@ export function buildCatalog(
 	mergedConfig: ProvidersConfig,
 	enabledLayers: readonly EnablementLayer[],
 	baseline: PlatformBaseline,
-	options?: {
-		/**
-		 * Environment variables for the non-secret connection overlay. Pure —
-		 * defaults to `{}` (no overlay) when omitted, never `process.env`. Node
-		 * callers inject `process.env` explicitly.
-		 */
-		envVars?: Record<string, string | undefined>;
-	},
 ): readonly ResolvedProvider[] {
 	const providers = mergedConfig.providers;
 	const catalog: ResolvedProvider[] = [];
-	// This builder is part of the PURE entry — never reach for `process.env`
-	// here (a browser/renderer/notebooks caller may have no `process`). Node
-	// callers inject `process.env` via the ai-config/node seams.
-	const envVars = options?.envVars ?? {};
 
 	// 1. Built-in providers
 	for (const id of BUILTIN_PROVIDER_IDS) {
 		const block = getBuiltinBlock(providers, id);
 		const enabled = resolveEnabled(id, enabledLayers, baseline);
-		const connection = applyEnvOverlay(id, resolveConnection(id, block), envVars);
+		const connection = resolveConnection(id, block);
 
 		catalog.push({
 			id,
@@ -275,92 +215,3 @@ function mergeOptionalSection<T extends Record<string, unknown>>(
 	}
 	return { ...defaults, ...block };
 }
-
-// ---------------------------------------------------------------------------
-// Environment variable overlay
-// ---------------------------------------------------------------------------
-
-/**
- * Apply non-secret connection env vars on top of the resolved connection.
- * Env vars have the highest precedence: env > file (providers.json) > defaults.
- *
- * Only overrides fields where the corresponding env var is set (non-empty).
- */
-function applyEnvOverlay(
-	providerId: string,
-	connection: ResolvedConnection,
-	envVars: Record<string, string | undefined>,
-): ResolvedConnection {
-	const mapping = CONNECTION_ENV_MAPPINGS[providerId];
-	if (!mapping) return connection;
-
-	let result = connection;
-	let changed = false;
-
-	// Top-level scalar fields
-	if (mapping.baseUrl) {
-		const val = envVars[mapping.baseUrl];
-		if (val) {
-			result = changed ? result : { ...result };
-			result.baseUrl = val;
-			changed = true;
-		}
-	}
-	if (mapping.endpoint) {
-		const val = envVars[mapping.endpoint];
-		if (val) {
-			result = changed ? result : { ...result };
-			result.endpoint = val;
-			changed = true;
-		}
-	}
-
-	// Nested sections — only override fields where the env var is set
-	if (mapping.positaiLogin) {
-		const overlay = readEnvSection(mapping.positaiLogin, envVars);
-		if (overlay) {
-			result = changed ? result : { ...result };
-			result.positaiLogin = result.positaiLogin ? { ...result.positaiLogin, ...overlay } : overlay;
-			changed = true;
-		}
-	}
-	if (mapping.aws) {
-		const overlay = readEnvSection(mapping.aws, envVars);
-		if (overlay) {
-			result = changed ? result : { ...result };
-			result.aws = result.aws ? { ...result.aws, ...overlay } : overlay;
-			changed = true;
-		}
-	}
-	if (mapping.googleCloud) {
-		const overlay = readEnvSection(mapping.googleCloud, envVars);
-		if (overlay) {
-			result = changed ? result : { ...result };
-			result.googleCloud = result.googleCloud ? { ...result.googleCloud, ...overlay } : overlay;
-			changed = true;
-		}
-	}
-
-	return result;
-}
-
-/**
- * Read a nested env-mapping section (e.g. `{ host: "ENV_VAR_NAME", ... }`)
- * and return an object with only the fields whose env vars are set.
- * Returns `undefined` if no env vars in the section are set.
- */
-function readEnvSection<T extends Record<string, string | undefined>>(
-	mapping: T,
-	envVars: Record<string, string | undefined>,
-): Record<string, string> | undefined {
-	let result: Record<string, string> | undefined;
-	for (const [field, envVarName] of Object.entries(mapping)) {
-		if (!envVarName) continue;
-		const val = envVars[envVarName];
-		if (val) {
-			result ??= {};
-			result[field] = val;
-		}
-	}
-	return result;
-}