fix(rebuild): keep the fresh agent model routing on config restore (#7011) - #7066
Closed
yanyunl1991 wants to merge 1 commit into
Closed
fix(rebuild): keep the fresh agent model routing on config restore (#7011)#7066yanyunl1991 wants to merge 1 commit into
yanyunl1991 wants to merge 1 commit into
Conversation
…7011) After switching the inference provider/model (e.g. via `nemoclaw onboard`), the host route and `inference get` show the new model, but the OpenClaw agent inside the sandbox kept routing to the previous one, and a rebuild did not fix it. The rebuild config-restore merge treats the whole `agents` section as durable (restored from the pre-rebuild backup), so `agents.defaults.model.primary` -- the ref the agent/TUI routes on -- reverted to the backed-up (pre-switch) model. This is inconsistent with the `models` section, where the fresh rebuild already owns the routing identity while the backup restores user tuning. Add `mergeOpenClawAgents`: the backup still restores durable agent config (per-agent settings, thinkingDefault / timeoutSeconds / compaction, etc.), but the fresh rebuild owns the model routing -- `agents.defaults.model` and each agent-list entry's `model` ref. Reproduced and fixed end-to-end on a DGX Spark (build provider, super->ultra): before, `inference get` showed ultra while the sandbox openclaw.json primary stayed super across a rebuild; after, the rebuild keeps primary=ultra and the durable agent tuning is preserved. The bug is provider/model-agnostic (any switch + rebuild reverts), so it does not require the reporter's DGX Station / DeepSeek setup. Unit test covers the fresh-routing / durable-tuning split; existing #5202 restore tests still pass. Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
Contributor
📝 WalkthroughWalkthroughSelective restore now keeps fresh agent model routing while restoring durable agent tuning and unrelated MCP settings. Per-agent model references are reconciled by agent ID, preventing stale backup routing from returning. ChangesAgent routing restore
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant BackupConfig
participant CurrentConfig
participant mergeOpenClawAgents
participant RestoredConfig
BackupConfig->>mergeOpenClawAgents: durable agent settings
CurrentConfig->>mergeOpenClawAgents: fresh model routing
mergeOpenClawAgents->>RestoredConfig: reconciled agents configuration
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/state/openclaw-config-merge.ts`:
- Around line 395-427: Update mergeOpenClawAgents in
src/lib/state/openclaw-config-merge.ts (lines 395-427) so agents.list is
reconciled as an ID-based union that retains fresh-only agents, and make fresh
omission of defaults.model authoritative by removing any backup value. Add
coverage in src/lib/state/openclaw-config-merge.test.ts (lines 246-294) for a
fresh-only agent and a fresh configuration that omits defaults.model.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d8ae7023-ea71-4a5d-b25d-a6224613a41d
📒 Files selected for processing (2)
src/lib/state/openclaw-config-merge.test.tssrc/lib/state/openclaw-config-merge.ts
Comment on lines
+395
to
+427
| function mergeOpenClawAgents(backupAgents: unknown, currentAgents: unknown): unknown { | ||
| if (!isPlainObject(currentAgents)) return cloneJson(backupAgents ?? currentAgents); | ||
| const backup = isPlainObject(backupAgents) ? backupAgents : {}; | ||
| const merged = mergeJsonObjects(currentAgents, backup); | ||
|
|
||
| // Fresh rebuild owns agents.defaults.model (the routing block). | ||
| if (isPlainObject(currentAgents.defaults) && "model" in currentAgents.defaults) { | ||
| const mergedDefaults = isPlainObject(merged.defaults) | ||
| ? merged.defaults | ||
| : ((merged.defaults = {}) as Record<string, unknown>); | ||
| mergedDefaults.model = cloneJson(currentAgents.defaults.model); | ||
| } | ||
|
|
||
| // Fresh rebuild owns each agent-list entry's `model` routing ref (by id). An | ||
| // agent that the fresh config routes via defaults (no `model` key) must not | ||
| // resurrect a stale per-agent ref from the backup. | ||
| if (Array.isArray(merged.list) && Array.isArray(currentAgents.list)) { | ||
| const freshById = new Map<string, Record<string, unknown>>(); | ||
| for (const entry of currentAgents.list) { | ||
| const id = agentEntryId(entry); | ||
| if (id && isPlainObject(entry) && !freshById.has(id)) freshById.set(id, entry); | ||
| } | ||
| merged.list = merged.list.map((entry) => { | ||
| if (!isPlainObject(entry)) return entry; | ||
| const id = agentEntryId(entry); | ||
| const fresh = id ? freshById.get(id) : undefined; | ||
| if (!fresh) return entry; | ||
| const next: Record<string, unknown> = { ...entry }; | ||
| if ("model" in fresh) next.model = cloneJson(fresh.model); | ||
| else delete next.model; | ||
| return next; | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Complete fresh ownership of the agent configuration.
The backup overlay replaces the fresh list wholesale and only preserves defaults.model when fresh explicitly provides it. This can drop newly rebuilt agents or resurrect stale default routing.
src/lib/state/openclaw-config-merge.ts#L395-L427: reconcileagents.listas an ID-based union and make fresh absence ofdefaults.modelauthoritative.src/lib/state/openclaw-config-merge.test.ts#L246-L294: cover a fresh-only agent and a fresh config that intentionally omitsdefaults.model.
📍 Affects 2 files
src/lib/state/openclaw-config-merge.ts#L395-L427(this comment)src/lib/state/openclaw-config-merge.test.ts#L246-L294
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/state/openclaw-config-merge.ts` around lines 395 - 427, Update
mergeOpenClawAgents in src/lib/state/openclaw-config-merge.ts (lines 395-427) so
agents.list is reconciled as an ID-based union that retains fresh-only agents,
and make fresh omission of defaults.model authoritative by removing any backup
value. Add coverage in src/lib/state/openclaw-config-merge.test.ts (lines
246-294) for a fresh-only agent and a fresh configuration that omits
defaults.model.
Source: Path instructions
Contributor
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 2 optional E2E recommendations
1 warning · 0 suggestionsWarningsWarnings do not block.
|
21 tasks
Contributor
Author
|
Closing in favor of #7063 (same diagnosis, tighter scoping that re-owns exactly the switch-set routing refs). Added the real-hardware E2E reproduction there. |
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
After switching the inference provider/model (e.g.
nemoclaw onboard→ NVIDIA Endpoints), the host route andnemoclaw inference getshow the new model, but the OpenClaw agent inside the sandbox keeps routing to the previous model — and arebuilddoes not fix it.Root cause: the rebuild config-restore merge (
mergeOpenClawRestoredConfig) treats the wholeagentssection as durable (restored from the pre-rebuild backup). Soagents.defaults.model.primary— the ref the agent/TUI routes on — is reverted to the backed-up (pre-switch) model, overriding the fresh rebuild's new routing. This is inconsistent with themodelssection, where the fresh rebuild already owns the routing identity while the backup restores user tuning (#5202).Fix
Add
mergeOpenClawAgents: the backup still restores durable agent config (per-agent settings,thinkingDefault/timeoutSeconds/compaction, …), but the fresh rebuild owns the model routing —agents.defaults.modeland each agent-list entry'smodelref.Reproduced and fixed end-to-end (DGX Spark, build provider, super→ultra)
The bug is provider/model-agnostic (any switch + rebuild reverts), so it does not require the reporter's DGX Station / DeepSeek setup.
inference getopenclaw.jsonprimaryrebuild --yes(before fix)rebuild --yes(with this fix)Durable agent tuning (
thinkingDefault=off) is preserved across the rebuild. Also verified at the merge level directly.Unit test covers the fresh-routing / durable-tuning split; the existing #5202 restore tests still pass.
Fixes #7011.
Signed-off-by: Yanyun Liao yanyunl@nvidia.com
🤖 Generated with Claude Code
Summary by CodeRabbit