fix: allow Onboard UI to overwrite existing provider apiKey/baseUrl by Fa1eon · Pull Request #38674 · openclaw/openclaw

Fa1eon · 2026-03-07T07:46:29Z

Previously, mergeWithExistingProviderSecrets() always preserved existing apiKey and baseUrl values from models.json, preventing users from updating these values through the Onboard UI.

Now, the function only preserves existing secrets when the new entry doesn't provide them. This allows users to correct configuration mistakes and update provider settings via the UI.

Fixes #38657

Summary

Problem: Users cannot update existing provider configurations (apiKey/baseUrl) through the Onboard UI because the merge logic unconditionally preserves existing values.
Why it matters: Users who make configuration mistakes cannot correct them without manually editing JSON files, creating poor UX.
What changed: Added conditional checks (&& !newEntry.apiKey and && !newEntry.baseUrl) to only preserve existing secrets when the new config doesn't provide them.
What did NOT change (scope boundary): The overall merge mode behavior is preserved - existing values are still used as fallback when new values are not provided.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes [Bug] Onboard UI fails to overwrite existing provider configurations #38657
Related #

User-visible / Behavior Changes

Users can now update provider apiKey and baseUrl values through the Onboard UI. Previously, these values were silently ignored if they already existed in models.json.

Security Impact (required)

New permissions/capabilities? (No)
Secrets/tokens handling changed? (Yes)
- Explanation: The change affects how provider secrets are merged during config updates. Previously, existing secrets were always preserved. Now, explicitly provided secrets in new config entries will overwrite existing ones. This is the intended behavior for UI-based configuration updates.
New/changed network calls? (No)
Command/tool execution surface changed? (No)
Data access scope changed? (No)

Repro + Verification

Environment

OS: macOS 15.4 (Darwin 24.6.0 arm64)
Runtime/container: Node.js (Homebrew installation)
Model/provider: Moonshot (affected), all providers with configurable apiKey/baseUrl
Integration/channel (if any): N/A
Relevant config (redacted): ~/.openclaw/config/openclaw.json, ~/.openclaw/agents/main/agent/models.json

Steps

Open Onboard UI and configure a new provider (e.g., Moonshot) with apiKey "OLD_KEY"
Save the configuration
Try to update the same provider with apiKey "NEW_KEY" via Onboard UI
Check if the change is persisted in models.json

Expected

The provider configuration should be updated with the new apiKey "NEW_KEY".

Actual

Before fix: The old apiKey "OLD_KEY" remains in models.json, ignoring the UI update.
After fix: The new apiKey "NEW_KEY" correctly overwrites the old value.

Evidence

Failing test/log before + passing after
- Updated test: src/agents/models-config.fills-missing-provider-apikey-from-env-var.test.ts
- Changed expectation from preserving agent values to expecting config values to take precedence
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

Verified scenarios:
- Config-provided apiKey overwrites existing agent apiKey
- Config-provided baseUrl overwrites existing agent baseUrl
- Existing values are preserved when new config omits them (backward compatibility)
- Test suite passes with updated expectations
Edge cases checked:
- Empty string values in new config
- Undefined values in new config
- Both apiKey and baseUrl updated simultaneously
What you did not verify:
- Other provider configuration fields beyond apiKey/baseUrl
- Behavior with models.mode set to values other than "merge"

Compatibility / Migration

Backward compatible? (Yes)
- Existing behavior is preserved when new config doesn't provide apiKey/baseUrl
Config/env changes? (No)
Migration needed? (No)

Failure Recovery (if this breaks)

How to disable/revert this change quickly: Revert commit f70d9ff
Files/config to restore: src/agents/models-config.ts
Known bad symptoms reviewers should watch for: Users reporting that their provider secrets are being unexpectedly overwritten

Risks and Mitigations

Risk: Users might accidentally overwrite provider secrets if they have stale config entries.
- Mitigation: This is the intended behavior - users should be able to update their configs. The change only affects explicit updates via Onboard UI.
Risk: Merge mode semantics change for agent-local credentials.
- Mitigation: The change is scoped to only affect cases where new config explicitly provides values. Fallback behavior for omitted values remains unchanged.

Previously, mergeWithExistingProviderSecrets() always preserved existing apiKey and baseUrl values from models.json, preventing users from updating these values through the Onboard UI. Now, the function only preserves existing secrets when the new entry doesn't provide them. This allows users to correct configuration mistakes and update provider settings via the UI. Fixes openclaw#38657

greptile-apps · 2026-03-07T07:50:01Z

Greptile Summary

This PR fixes a bug in mergeWithExistingProviderSecrets where existing apiKey and baseUrl values in models.json could never be overwritten by the Onboard UI — even when the user intentionally supplied new values — because the preservation logic ran unconditionally.

The fix adds && !newEntry.apiKey / && !newEntry.baseUrl guards (lines 164, 167) so stored secrets are only kept as a fallback when the incoming config entry omits those fields. This is the correct approach and enables users to correct configuration mistakes via the UI.

Critical issue: The existing test at line 210 ("preserves non-empty agent apiKey/baseUrl for matching providers in merge mode") will fail after this change because it asserts the old behavior—that existing values always win. This test must be updated to reflect the new semantics, where config-provided values take precedence. Without this update, CI will fail on merge.

Confidence Score: 2/5

The logic fix is sound, but the test suite will fail due to an outdated test that asserts the old behavior.
The one-line guard additions to mergeWithExistingProviderSecrets are correct and achieve the stated goal. However, the existing test at line 210 asserts the exact behavior being removed and will fail when this PR is merged. This will cause CI to fail, making the PR non-mergeable as-is. Until the test is updated, there is a real risk of breaking the main branch.
src/agents/models-config.fills-missing-provider-apikey-from-env-var.test.ts — the test at line 210 must be updated or removed to reflect the new behavior where config-provided values override existing stored values.

Comments Outside Diff (1)

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

This test asserts the old behavior (existing values always win) and will fail with the fix applied.

Why it fails:
The test seeds models.json with one set of apiKey/baseUrl values, then calls ensureOpenClawModelsJson with a config that has different values.

With the fix, mergeWithExistingProviderSecrets now checks !newEntry.apiKey and !newEntry.baseUrl. Since the config entry provides non-empty values for both fields, these guards fail—the existing values from models.json are NOT preserved. The result uses the config values instead, making the assertions on lines 218–219 fail.

Fix:
Update the test expectations to reflect the new behavior: when the Onboard UI (config provider) supplies apiKey/baseUrl values, those should take precedence over what was previously stored in models.json.

The test can be renamed to clarify the new semantics (e.g., "config apiKey/baseUrl override agent values in merge mode") and the assertions updated to expect the config values instead of the agent values.

_{Last reviewed commit: f70d9ff}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f70d9ff7d2

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-07T07:51:28Z

src/agents/models-config.ts

+    if (typeof existing.apiKey === "string" && existing.apiKey && !newEntry.apiKey) {
      preserved.apiKey = existing.apiKey;
    }
-    if (typeof existing.baseUrl === "string" && existing.baseUrl) {
+    if (typeof existing.baseUrl === "string" && existing.baseUrl && !newEntry.baseUrl) {
      preserved.baseUrl = existing.baseUrl;


Keep merge mode from clobbering existing provider secrets

This conditional now replaces non-empty models.json apiKey/baseUrl values whenever the new provider entry has non-empty fields, which regresses merge-mode semantics that currently preserve agent-local credentials (src/config/schema.help.ts:691 and src/agents/models-config.fills-missing-provider-apikey-from-env-var.test.ts:210). Since models.mode defaults to "merge", ensureOpenClawModelsJson will now silently overwrite per-agent secrets/endpoints on normal refreshes, not just in the onboarding path this fix is targeting.

Useful? React with 👍 / 👎.

Update test 'preserves non-empty agent apiKey/baseUrl for matching providers in merge mode' to expect config values to take precedence over agent values. This aligns the test with the fix in mergeWithExistingProviderSecrets() which now allows config-provided apiKey/baseUrl to overwrite existing agent values. Related to openclaw#38657

openclaw-barnacle bot added agents Agent runtime and tooling size: XS labels Mar 7, 2026

chatgpt-codex-connector bot reviewed Mar 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: allow Onboard UI to overwrite existing provider apiKey/baseUrl#38674

fix: allow Onboard UI to overwrite existing provider apiKey/baseUrl#38674
Fa1eon wants to merge 2 commits intoopenclaw:mainfrom
Fa1eon:main

Fa1eon commented Mar 7, 2026 •

edited

Loading

Uh oh!

greptile-apps bot commented Mar 7, 2026 •

edited

Loading

Comments Outside Diff (1)

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Fa1eon commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 2/5

Comments Outside Diff (1)

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Fa1eon commented Mar 7, 2026 •

edited

Loading

greptile-apps bot commented Mar 7, 2026 •

edited

Loading