fix: preserve models.json baseUrls on regen (#67893) (thanks @lawrence3699)

* models-config: preserve existing models.json baseUrls * test: distill models-config baseUrl regression test * fix: preserve models.json baseUrls on regen (#67893) (thanks @lawrence3699) --------- Co-authored-by: lawrence3699 <lawrence3699@users.noreply.github.com> Co-authored-by: Ayaan Zaidi <hi@obviy.us>
2026-05-06 10:20:42 +00:00 · 2026-04-17 13:02:35 +10:00
parent a189394590
commit 35fb3f7e1c
6 changed files with 76 additions and 46 deletions
--- a/src/agents/models-config.merge.test.ts
+++ b/src/agents/models-config.merge.test.ts
@@ -134,14 +134,13 @@ describe("models-config merge helpers", () => {
        } as ExistingProviderConfig,
      },
      secretRefManagedProviders: new Set<string>(),
-      explicitBaseUrlProviders: new Set<string>(["custom-proxy"]),
    });

    expect(merged.existing?.baseUrl).toBe("http://localhost:1234/v1");
    expect(merged["custom-proxy"]?.baseUrl).toBe("http://localhost:4000/v1");
  });

-  it("preserves non-empty existing apiKey while explicit baseUrl wins", async () => {
+  it("preserves non-empty existing apiKey and baseUrl from models.json", async () => {
    const merged = mergeWithExistingProviderSecrets({
      nextProviders: {
        custom: createConfigProvider(),
@@ -150,14 +149,13 @@ describe("models-config merge helpers", () => {
        custom: createExistingProvider(),
      },
      secretRefManagedProviders: new Set<string>(),
-      explicitBaseUrlProviders: new Set<string>(["custom"]),
    });

    expect(merged.custom?.apiKey).toBe(preservedApiKey);
-    expect(merged.custom?.baseUrl).toBe("https://config.example/v1");
+    expect(merged.custom?.baseUrl).toBe("https://agent.example/v1");
  });

-  it("preserves existing apiKey after explicit provider key normalization", async () => {
+  it("preserves existing baseUrl after explicit provider key normalization", async () => {
    const normalized = mergeProviders({
      explicit: {
        " custom ": createConfigProvider(),
@@ -169,11 +167,10 @@ describe("models-config merge helpers", () => {
        custom: createExistingProvider(),
      },
      secretRefManagedProviders: new Set<string>(),
-      explicitBaseUrlProviders: new Set<string>(["custom"]),
    });

    expect(merged.custom?.apiKey).toBe(preservedApiKey);
-    expect(merged.custom?.baseUrl).toBe("https://config.example/v1");
+    expect(merged.custom?.baseUrl).toBe("https://agent.example/v1");
  });

  it("preserves implicit provider headers when explicit config adds extra headers", async () => {
@@ -228,7 +225,6 @@ describe("models-config merge helpers", () => {
        } as ExistingProviderConfig,
      },
      secretRefManagedProviders: new Set<string>(),
-      explicitBaseUrlProviders: new Set<string>(),
    });

    expect(merged.custom).toEqual(
@@ -255,7 +251,6 @@ describe("models-config merge helpers", () => {
        custom: existingProvider,
      },
      secretRefManagedProviders: new Set<string>(),
-      explicitBaseUrlProviders: new Set<string>(["custom"]),
    });

    expect(merged.custom?.apiKey).toBe(preservedApiKey);
@@ -277,7 +272,6 @@ describe("models-config merge helpers", () => {
        } as ExistingProviderConfig,
      },
      secretRefManagedProviders: new Set<string>(),
-      explicitBaseUrlProviders: new Set<string>(),
    });

    expect(merged.custom?.apiKey).toBe("GOOGLE_API_KEY"); // pragma: allowlist secret
@@ -294,11 +288,10 @@ describe("models-config merge helpers", () => {
        }),
      },
      secretRefManagedProviders: new Set<string>(),
-      explicitBaseUrlProviders: new Set<string>(["custom"]),
    });

    expect(merged.custom?.apiKey).toBe("ALLCAPS_SAMPLE"); // pragma: allowlist secret
-    expect(merged.custom?.baseUrl).toBe("https://config.example/v1");
+    expect(merged.custom?.baseUrl).toBe("https://agent.example/v1");
  });

  it("uses config apiKey/baseUrl when existing values are empty", async () => {
@@ -313,7 +306,6 @@ describe("models-config merge helpers", () => {
        }),
      },
      secretRefManagedProviders: new Set<string>(),
-      explicitBaseUrlProviders: new Set<string>(["custom"]),
    });

    expect(merged.custom?.apiKey).toBe(configApiKey);
--- a/src/agents/models-config.merge.ts
+++ b/src/agents/models-config.merge.ts
@@ -196,17 +196,11 @@ function shouldPreserveExistingApiKey(params: {
 }

 function shouldPreserveExistingBaseUrl(params: {
-  providerKey: string;
  existing: ExistingProviderConfig;
  nextEntry: ProviderConfig;
-  explicitBaseUrlProviders: ReadonlySet<string>;
 }): boolean {
-  const { providerKey, existing, nextEntry, explicitBaseUrlProviders } = params;
-  if (
-    explicitBaseUrlProviders.has(providerKey) ||
-    typeof existing.baseUrl !== "string" ||
-    existing.baseUrl.length === 0
-  ) {
+  const { existing, nextEntry } = params;
+  if (typeof existing.baseUrl !== "string" || existing.baseUrl.length === 0) {
    return false;
  }

@@ -219,10 +213,8 @@ export function mergeWithExistingProviderSecrets(params: {
  nextProviders: Record<string, ProviderConfig>;
  existingProviders: Record<string, ExistingProviderConfig>;
  secretRefManagedProviders: ReadonlySet<string>;
-  explicitBaseUrlProviders: ReadonlySet<string>;
 }): Record<string, ProviderConfig> {
-  const { nextProviders, existingProviders, secretRefManagedProviders, explicitBaseUrlProviders } =
-    params;
+  const { nextProviders, existingProviders, secretRefManagedProviders } = params;
  const mergedProviders: Record<string, ProviderConfig> = {};
  for (const [key, entry] of Object.entries(existingProviders)) {
    mergedProviders[key] = entry;
@@ -246,10 +238,8 @@ export function mergeWithExistingProviderSecrets(params: {
    }
    if (
      shouldPreserveExistingBaseUrl({
-        providerKey: key,
        existing,
        nextEntry: newEntry,
-        explicitBaseUrlProviders,
      })
    ) {
      preserved.baseUrl = existing.baseUrl;
--- a/src/agents/models-config.plan.ts
+++ b/src/agents/models-config.plan.ts
@@ -58,26 +58,11 @@ export async function resolveProvidersForModelsJsonWithDeps(
  });
 }

-function resolveExplicitBaseUrlProviders(
-  providers: OpenClawConfig["models"] | undefined,
-): ReadonlySet<string> {
-  return new Set(
-    Object.entries(providers?.providers ?? {})
-      .map(([key, provider]) => [key.trim(), provider] as const)
-      .filter(
-        ([key, provider]) =>
-          Boolean(key) && typeof provider?.baseUrl === "string" && provider.baseUrl.trim(),
-      )
-      .map(([key]) => key),
-  );
-}
-
 function resolveProvidersForMode(params: {
  mode: NonNullable<ModelsConfig["mode"]>;
  existingParsed: unknown;
  providers: Record<string, ProviderConfig>;
  secretRefManagedProviders: ReadonlySet<string>;
-  explicitBaseUrlProviders: ReadonlySet<string>;
 }): Record<string, ProviderConfig> {
  if (params.mode !== "merge") {
    return params.providers;
@@ -94,7 +79,6 @@ function resolveProvidersForMode(params: {
    nextProviders: params.providers,
    existingProviders: existingProviders as Record<string, ExistingProviderConfig>,
    secretRefManagedProviders: params.secretRefManagedProviders,
-    explicitBaseUrlProviders: params.explicitBaseUrlProviders,
  });
 }

@@ -135,7 +119,6 @@ export async function planOpenClawModelsJsonWithDeps(
    existingParsed: params.existingParsed,
    providers: normalizedProviders,
    secretRefManagedProviders,
-    explicitBaseUrlProviders: resolveExplicitBaseUrlProviders(cfg.models),
  });
  const secretEnforcedProviders =
    enforceSourceManagedProviderSecrets({
--- a/src/agents/models-config.runtime-source-snapshot.test.ts
+++ b/src/agents/models-config.runtime-source-snapshot.test.ts
@@ -279,6 +279,9 @@ describe("models-config runtime source snapshot", () => {
              openai: {
                ...runtimeConfig.models!.providers!.openai,
                baseUrl: "https://api.openai.com/v1",
+                headers: {
+                  "X-OpenClaw-Test": "one",
+                },
              },
            },
          },
@@ -290,6 +293,9 @@ describe("models-config runtime source snapshot", () => {
              openai: {
                ...runtimeConfig.models!.providers!.openai,
                baseUrl: "https://mirror.example/v1",
+                headers: {
+                  "X-OpenClaw-Test": "two",
+                },
              },
            },
          },
@@ -299,17 +305,26 @@ describe("models-config runtime source snapshot", () => {
          setRuntimeConfigSnapshot(runtimeConfig, sourceConfig);
          await ensureOpenClawModelsJson(firstCandidate);
          let parsed = await readGeneratedModelsJson<{
-            providers: Record<string, { baseUrl?: string; apiKey?: string }>;
+            providers: Record<
+              string,
+              { baseUrl?: string; apiKey?: string; headers?: Record<string, string> }
+            >;
          }>();
          expect(parsed.providers.openai?.baseUrl).toBe("https://api.openai.com/v1");
          expect(parsed.providers.openai?.apiKey).toBe("OPENAI_API_KEY"); // pragma: allowlist secret
+          expect(parsed.providers.openai?.headers?.["X-OpenClaw-Test"]).toBe("one");

+          // Header changes still rewrite models.json, but merge mode preserves the existing baseUrl.
          await ensureOpenClawModelsJson(secondCandidate);
          parsed = await readGeneratedModelsJson<{
-            providers: Record<string, { baseUrl?: string; apiKey?: string }>;
+            providers: Record<
+              string,
+              { baseUrl?: string; apiKey?: string; headers?: Record<string, string> }
+            >;
          }>();
-          expect(parsed.providers.openai?.baseUrl).toBe("https://mirror.example/v1");
+          expect(parsed.providers.openai?.baseUrl).toBe("https://api.openai.com/v1");
          expect(parsed.providers.openai?.apiKey).toBe("OPENAI_API_KEY"); // pragma: allowlist secret
+          expect(parsed.providers.openai?.headers?.["X-OpenClaw-Test"]).toBe("two");
        } finally {
          clearRuntimeConfigSnapshot();
          clearConfigCache();
--- a/src/agents/models-config.uses-first-github-copilot-profile-env-tokens.test.ts
+++ b/src/agents/models-config.uses-first-github-copilot-profile-env-tokens.test.ts
@@ -124,6 +124,55 @@ describe("models-config", () => {
    });
  });

+  it("keeps a non-empty existing models.json baseUrl when merge mode regenerates the provider", async () => {
+    const kilocodeProvider = {
+      baseUrl: "https://api.kilo.ai/api/gateway/v1",
+      api: "openai-completions" as const,
+      models: [],
+    };
+    const existingContents = `${JSON.stringify(
+      {
+        providers: {
+          kilocode: {
+            baseUrl: "https://api.kilo.ai/api/gateway",
+            api: "openai-completions",
+            models: [],
+          },
+        },
+      },
+      null,
+      2,
+    )}\n`;
+
+    const plan = await planOpenClawModelsJsonWithDeps(
+      {
+        cfg: {
+          models: {
+            providers: {
+              kilocode: kilocodeProvider,
+            },
+          },
+        },
+        sourceConfigForSecrets: {
+          models: {
+            providers: {
+              kilocode: kilocodeProvider,
+            },
+          },
+        },
+        agentDir: "/tmp/openclaw-agent",
+        env: {} as NodeJS.ProcessEnv,
+        existingRaw: existingContents,
+        existingParsed: JSON.parse(existingContents),
+      },
+      {
+        resolveImplicitProviders: async () => ({}),
+      },
+    );
+
+    expect(plan).toEqual({ action: "noop" });
+  });
+
  it("uses tokenRef env var when github-copilot profile omits plaintext token", () => {
    const auth = createProviderAuthResolver(
      {