Models: keep Moonshot CN baseUrl from config in merge mode by liuxiaopai-ai · Pull Request #32722 · openclaw/openclaw

liuxiaopai-ai · 2026-03-03T07:42:50Z

Summary

Problem: in models.mode=merge, an existing ~/.openclaw/agents/*/models.json Moonshot baseUrl could override a newer explicit config models.providers.moonshot.baseUrl.
Why it matters: users selecting Moonshot CN (https://api.moonshot.cn/v1) could still end up with stale .ai endpoint in regenerated models.json, causing auth failures.
What changed: merge logic now keeps explicit config baseUrl for moonshot when present, while preserving previous merge behavior for other providers.
What did NOT change (scope boundary): no changes to provider auth wiring, model catalogs, or non-Moonshot baseUrl merge semantics.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

With models.mode: merge, explicit models.providers.moonshot.baseUrl from config now wins over stale models.json Moonshot baseUrl.

Security Impact (required)

New permissions/capabilities? (No)
Secrets/tokens handling changed? (No)
New/changed network calls? (No)
Command/tool execution surface changed? (No)
Data access scope changed? (No)
If any Yes, explain risk + mitigation:

Repro + Verification

Environment

OS: macOS
Runtime/container: Node 22 + pnpm
Model/provider: Moonshot
Integration/channel (if any): N/A
Relevant config (redacted): models.providers.moonshot.baseUrl = https://api.moonshot.cn/v1

Steps

Seed agent models.json with Moonshot baseUrl https://api.moonshot.ai/v1.
Set config models.providers.moonshot.baseUrl to https://api.moonshot.cn/v1 and run models generation (ensureOpenClawModelsJson).
Inspect generated models.json.

Expected

Generated Moonshot baseUrl remains https://api.moonshot.cn/v1.

Actual

Verified expected behavior in regression test.

Evidence

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios:
- pnpm test src/agents/models-config.fills-missing-provider-apikey-from-env-var.test.ts
- pnpm test src/commands/auth-choice.moonshot.test.ts
Edge cases checked:
- existing merge behavior for non-Moonshot provider baseUrl preservation remains unchanged.
What you did not verify:
- live Moonshot CN API calls end-to-end in a real runtime.

Compatibility / Migration

Backward compatible? (Yes)
Config/env changes? (No)
Migration needed? (No)
If yes, exact upgrade steps:

Failure Recovery (if this breaks)

How to disable/revert this change quickly:
- Revert this PR commit.
Files/config to restore:
- src/agents/models-config.ts
- src/agents/models-config.fills-missing-provider-apikey-from-env-var.test.ts
Known bad symptoms reviewers should watch for:
- Moonshot baseUrl unexpectedly reverting to stale models.json value

Risks and Mitigations

Risk: provider-specific merge exception could accidentally widen in future refactors.
- Mitigation: targeted regression test explicitly asserts Moonshot CN precedence while existing merge-preservation tests for generic providers still pass.

greptile-apps · 2026-03-03T07:44:44Z

Greptile Summary

This PR fixes a real bug in models.mode=merge: when regenerating models.json, the existing merge logic would always preserve a non-empty baseUrl from the stale agent file, silently overriding an explicit models.providers.moonshot.baseUrl the user had set in their config. The fix correctly threads the explicit config providers down to mergeWithExistingProviderSecrets and skips the stale-value preservation when the user has an explicit config value.

Key changes:

mergeWithExistingProviderSecrets gains an explicitProviders parameter (optional, backward compatible).
resolveProvidersForMode and ensureOpenClawModelsJson capture and forward cfg.models?.providers as explicitProviders.
A regression test seeds a stale baseUrl (*.ai), applies an explicit config baseUrl (*.cn), and asserts the config value wins.

Notable concern:

The hasExplicitBaseUrl guard includes key === "moonshot", restricting the fix to Moonshot only. The same stale-override bug would still affect any other provider where the user supplies an explicit baseUrl in their config — this is acknowledged as an intentional scope boundary in the PR description, but removing the guard would generalise the fix at no additional risk (see inline comment).

Confidence Score: 4/5

Safe to merge — the bug fix is correct and backward compatible; the only concern is a narrower-than-necessary scope.
The core logic is sound: explicitProviders is correctly captured once from cfg.models?.providers and threaded through to the merge function, the preserved object correctly omits baseUrl when the user has an explicit config value, and newEntry.baseUrl (which already reflects the explicit config via mergeProviders) fills in correctly. The regression test covers the intended scenario. The one issue is that key === "moonshot" restricts a generally-applicable fix to a single provider, leaving an identical bug open for all other providers that expose a configurable baseUrl.
Pay attention to src/agents/models-config.ts lines 164–167 — consider generalising the hasExplicitBaseUrl guard beyond Moonshot.

_{Last reviewed commit: 68e6762}

greptile-apps

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1e99c9b4c6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

openclaw-barnacle Bot added agents Agent runtime and tooling size: S experienced-contributor labels Mar 3, 2026

greptile-apps Bot reviewed Mar 3, 2026

View reviewed changes

Comment thread src/agents/models-config.ts Outdated

liuxiaopai-ai added 2 commits March 3, 2026 16:35

Models: prefer moonshot config baseUrl in merge mode

70f830c

models: honor explicit provider baseUrl in merge mode

1e99c9b

chatgpt-codex-connector Bot reviewed Mar 3, 2026

View reviewed changes

Comment thread src/agents/models-config.ts

models: keep merge baseUrl override scoped to moonshot

a621b6a

thewilloftheshadow closed this Mar 6, 2026

BigUncle mentioned this pull request Mar 7, 2026

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

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Models: keep Moonshot CN baseUrl from config in merge mode#32722

Models: keep Moonshot CN baseUrl from config in merge mode#32722
liuxiaopai-ai wants to merge 3 commits intoopenclaw:mainfrom
liuxiaopai-ai:codex/issue-32607-moonshot-cn-baseurl-merge-mode

liuxiaopai-ai commented Mar 3, 2026

Uh oh!

greptile-apps Bot commented Mar 3, 2026

Uh oh!

greptile-apps Bot left a comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

liuxiaopai-ai commented Mar 3, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Compatibility / Migration

Failure Recovery (if this breaks)

Risks and Mitigations

Uh oh!

greptile-apps Bot commented Mar 3, 2026

Greptile Summary

Confidence Score: 4/5

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants