Skip to content

Fix stale provider baseUrl overriding explicit config in merge mode#39103

Closed
BigUncle wants to merge 1 commit intoopenclaw:mainfrom
BigUncle:codex/fix-models-json-baseurl-sync-upstream
Closed

Fix stale provider baseUrl overriding explicit config in merge mode#39103
BigUncle wants to merge 1 commit intoopenclaw:mainfrom
BigUncle:codex/fix-models-json-baseurl-sync-upstream

Conversation

@BigUncle
Copy link
Copy Markdown
Contributor

@BigUncle BigUncle commented Mar 7, 2026

Summary

This fixes a merge-mode bug where a stale provider baseUrl already stored in:

  • ~/.openclaw/agents/<agent>/agent/models.json

could override a newer explicit baseUrl from openclaw.json.

After this change, an explicit provider baseUrl from config wins over stale runtime state.

Problem

In models.mode: "merge", updating a provider endpoint in openclaw.json could appear to have no effect.

A typical failure mode was:

  • runtime models.json still had an old provider URL
  • openclaw.json was corrected
  • OpenClaw continued using the stale runtime URL
  • users had to manually edit or delete models.json

Root Cause

src/agents/models-config.ts preserved stale provider values from existing runtime models.json during merge.

That meant later explicit config changes could be masked by old runtime state.

There is also a provider-key matching edge case here: explicit-config detection must stay consistent with trimmed /
normalized provider keys, so entries like " custom " still match runtime key custom.

What Changed

  • track which providers explicitly define baseUrl in the current config
  • trim / normalize those provider keys consistently before merge comparison
  • preserve existing runtime baseUrl only when the current config does not explicitly provide one
  • keep the existing apiKey / SecretRef preservation behavior intact

Tests

Updated regression coverage in:

  • src/agents/models-config.fills-missing-provider-apikey-from-env-var.test.ts

Added coverage for:

  • explicit config baseUrl overrides stale runtime models.json baseUrl
  • the same behavior still works when the config provider key is written in a trimmed / normalized form (for example
    " custom " -> custom)

Validated with:

pnpm vitest run src/agents/models-config*.test.ts

Result:

  • 23 test files passed
  • 89 tests passed

User-visible Effect

After this patch:

  • changing a provider endpoint in openclaw.json will correctly propagate into regenerated models.json
  • OpenClaw will stop silently using a stale runtime provider URL
  • explicit baseUrl config will still work correctly when provider keys need trimmed / normalized matching

Reproduction Case

Concrete case that reproduced this bug:

  • old runtime file had http://192.168.1.8/v1
  • config was updated to http://192.168.1.8:8317/v1
  • OpenClaw continued using the old runtime URL until models.json was manually fixed

This patch makes the explicit config URL win automatically.

Related

This PR addresses the same bug family discussed in:

It is also closely related to earlier PR attempts:

My understanding is that those earlier PRs were addressing the same stale merge behavior family.

This patch focuses specifically on explicit provider baseUrl precedence in merge mode, and adds regression coverage for the trimmed / normalized provider-key matching case.

There is also prior provider-specific precedent in:

This PR applies the same principle more generally to merge-mode provider baseUrl handling.

@openclaw-barnacle openclaw-barnacle bot added agents Agent runtime and tooling size: S labels Mar 7, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 7, 2026

Greptile Summary

This PR correctly fixes a merge-mode bug where a stale baseUrl in the runtime models.json could silently override an updated baseUrl in openclaw.json. The production-code changes in models-config.ts are sound: an explicitBaseUrlProviders set is derived from trimmed config keys (consistent with mergeProviders's own key normalization) and threaded through to mergeWithExistingProviderSecrets, where it prevents the runtime baseUrl from overwriting an explicitly configured one.

Issues found:

  • The "uses config apiKey/baseUrl when existing agent values are empty" test (line 362) was not updated to use the new seedProvider wrapper introduced by this PR. It passes baseUrl, apiKey, api, and models directly at the top level, making the required seedProvider field absent. This is a TypeScript type error; at runtime under Vitest, params.seedProvider is undefined, so writeAgentModelsJson silently writes no provider, meaning the test no longer exercises the intended scenario (empty existing values) — it now trivially passes because no prior runtime state exists at all. The regression coverage this test was supposed to provide is effectively lost.

Confidence Score: 3/5

  • Production fix is correct and safe, but a test regression silently removes coverage for the "empty existing values" case.
  • The models-config.ts production change is logically correct — the explicitBaseUrlProviders set is built consistently with how provider keys are normalized elsewhere, and the guard in mergeWithExistingProviderSecrets is correctly conditioned. The score is reduced because one test call site was not updated to the new seedProvider-wrapped signature, causing a TypeScript type error and silently losing the intended regression coverage. The test still passes at runtime (for the wrong reason), which means the gap may not be caught without a tsc check.
  • src/agents/models-config.fills-missing-provider-apikey-from-env-var.test.ts — the "uses config apiKey/baseUrl when existing agent values are empty" test at line 362 needs its argument wrapped in { seedProvider: { ... } }.

Comments Outside Diff (1)

  1. src/agents/models-config.fills-missing-provider-apikey-from-env-var.test.ts, line 362-367 (link)

    Stale call site not updated to new seedProvider wrapper

    runCustomProviderMergeTest was refactored in this PR to accept { seedProvider, existingProviderKey?, configProviderKey? }, but this call site still passes baseUrl, apiKey, api, and models at the top level — the seedProvider required field is missing entirely.

    At the TypeScript level this is a type error (required property seedProvider is absent). At runtime under Vitest (which transpiles without type-checking), params.seedProvider is undefined, so writeAgentModelsJson({ providers: { custom: undefined } }) silently writes {"providers": {}} — no seed provider is created on disk. The merge then finds no existing custom provider and falls through to using the config values, causing the assertions to pass — but for the wrong reason. The test is no longer covering the "empty existing baseUrl/apiKey" scenario it describes; it is effectively testing the "no prior runtime state" case.

    The fix is to wrap the seed provider fields under seedProvider:

Last reviewed commit: 5f3a194

@BigUncle
Copy link
Copy Markdown
Contributor Author

BigUncle commented Mar 7, 2026

Thanks — addressed.

I rebased onto the latest main, resolved the merge conflict, and fixed the stale runCustomProviderMergeTest call site to use seedProvider: { ... }.

I also reran:

pnpm vitest run src/agents/models-config*.test.ts

Result: 23 files passed / 89 tests passed locally.

steipete added a commit that referenced this pull request Mar 7, 2026
@steipete
Copy link
Copy Markdown
Contributor

steipete commented Mar 7, 2026

Landed via maintainer flow.

What I did:

SHAs:

  • Original PR head: 1219318a001e18aa4f3d19503e0a881febeb1b2b
  • Landed on main: c06014d503e710650f2201f9539b788d278f4573

Thanks @BigUncle for the contribution.

@steipete steipete closed this Mar 7, 2026
mrosmarin added a commit to mrosmarin/openclaw that referenced this pull request Mar 7, 2026
* main: (133 commits)
  reduce image size, offer slim image (openclaw#38479)
  fix(security): harden install base drift cleanup
  fix(agents): respect explicit provider baseUrl in merge mode (openclaw#39103)
  fix(agents): apply contextTokens cap for compaction threshold (openclaw#39099)
  fix(exec): block dangerous override-only env pivots
  fix(security): stage installs before publish
  fix(daemon): normalise whitespace in checkTokenDrift to prevent false-positive warning (openclaw#39108)
  fix(security): harden fs-safe copy writes
  refactor: dedupe bluebubbles webhook auth test setup
  refactor: dedupe discord native command test scaffolding
  refactor: dedupe anthropic probe target test setup
  refactor: dedupe minimax provider auth test setup
  refactor: dedupe runtime snapshot test fixtures
  fix: harden zip extraction writes
  fix(tests): stabilize diffs localReq headers (supersedes openclaw#39063)
  fix: harden workspace skill path containment
  fix(agents): land openclaw#38935 from @MumuTW
  fix(models): land openclaw#38947 from @davidemanuelDEV
  fix(gateway): land openclaw#39064 from @Narcooo
  fix(models-auth): land openclaw#38951 from @MumuTW
  ...
vincentkoc pushed a commit to BryanTegomoh/openclaw-fork that referenced this pull request Mar 8, 2026
openperf pushed a commit to openperf/moltbot that referenced this pull request Mar 8, 2026
mcaxtr pushed a commit to mcaxtr/openclaw that referenced this pull request Mar 8, 2026
Saitop pushed a commit to NomiciAI/openclaw that referenced this pull request Mar 8, 2026
GordonSH-oss pushed a commit to GordonSH-oss/openclaw that referenced this pull request Mar 9, 2026
jenawant pushed a commit to jenawant/openclaw that referenced this pull request Mar 10, 2026
dhoman pushed a commit to dhoman/chrono-claw that referenced this pull request Mar 11, 2026
senw-developers pushed a commit to senw-developers/va-openclaw that referenced this pull request Mar 17, 2026
V-Gutierrez pushed a commit to V-Gutierrez/openclaw-vendor that referenced this pull request Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants