fix(state): preserve reporter-owned model metadata on openclaw.json rebuild restore

[yimoj]

Summary

After a NemoClaw upgrade + nemoclaw <name> rebuild, an OpenClaw sandbox's reporter-tuned openclaw.json model metadata reset to regenerated defaults. Fixing this required two layers: (1) the rebuild restore merge replaced matching models.providers entries wholesale, and (2) even after a correct merge, OpenClaw's own config-integrity protection reverted the restored config to its .last-good baseline. This PR fixes both, verified end-to-end through the real nemoclaw CLI on a live sandbox.

Related Issue

Fixes #5202

Changes

• src/lib/state/openclaw-config-merge.ts: reconcile matching models.providers entries by ownership instead of replacing them wholesale. The fresh rebuild keeps runtime-owned routing/credentials (baseUrl, api, apiKey) and a model's identity (id, name); the backup restores user-owned non-secret tuning (reasoning, cost, contextWindow, maxTokens, compat, input). Fresh rebuild stays authoritative for the provider/model set and order; backup-only/id-less stale models are not resurrected; backup-only providers are inherited. No raw secrets restored.
• src/lib/state/sandbox.ts: buildStateFileRestoreCommand now refreshes OpenClaw's .last-good recovery anchor for the openclaw.json merge restore — staged through a temp + atomic rename and failing closed before the live config is swapped. Without this, OpenClaw archives the restored config as openclaw.json.clobbered.<ts> and reverts to the freshly generated baseline.
• Tests: reporter-shaped unit regression (openclaw-config-merge.test.ts), a command-string unit test for the anchor refresh + ordering + fail-closed (state-file-restore-command.test.ts), and an extended backup/restore integration test through the real backupSandboxState()/restoreSandboxState() path asserting both the merged metadata, mcp.servers retention, and the .last-good anchor (openclaw-config-snapshot.test.ts).

Notes on mcp.servers

The reporter also saw mcp.servers become {}. The fresh generated config emits no mcp section, so the backed-up mcp survives the generic object merge — confirmed live (fresh in-sandbox config showed mcp: null) and locked in by the tests. The merge layer needed no change for it; the .last-good revert was what dropped it end-to-end, which layer 2 fixes.

Type of Change

• Code change (feature, bug fix, or refactor)

Verification

• Reporter-workflow E2E on a live OpenClaw sandbox via the worktree CLI (no mocks):
  • node ./bin/nemoclaw.js onboard --non-interactive --yes --no-gpu --name i5202e2e — created a real OpenClaw sandbox (cloud inference, no GPU).
  • node ./bin/nemoclaw.js i5202e2e exec -- ... — tuned the in-sandbox /sandbox/.openclaw/openclaw.json to the reporter state (reasoning:true, custom cost, maxTokens:32768, compat, mcp.servers.filesystem).
  • node ./bin/nemoclaw.js i5202e2e rebuild --yes — the user-facing operation under test (backup → recreate → restore-merge → .last-good refresh).
  • Post-rebuild inspection of /sandbox/.openclaw/openclaw.json: kept reasoning:true, maxTokens:32768, cost {0.5,1.5,0.1,0.2}, compat, input ["text","image"], mcp.servers.filesystem; runtime fields fresh (baseUrl https://inference.local/v1, apiKey unused, fresh id/name); zero openclaw.json.clobbered.* files. Survived a subsequent container restart (OpenClaw integrity check re-ran). Reproduced cleanly across two consecutive fixed rebuilds.
• Pre-fix, the same workflow showed the bug: restored config reverted to reasoning:false/maxTokens:4096/no compat/no mcp, with the correctly-merged config archived as openclaw.json.clobbered.* — proving the merge worked but OpenClaw reverted it.
• npx vitest run on the affected unit + integration tests — passing.
• tsc -p tsconfig.cli.json typecheck passes.
• npx @biomejs/biome check clean on changed files.
• codex review --uncommitted clean.
• Tests added for new/changed behavior.
• No secrets, API keys, or credentials committed; only the already-sanitized backup is merged.

---

Signed-off-by: Yimo Jiang yimoj@nvidia.com

Summary by CodeRabbit

• Bug Fixes
  • Improved restore behavior to keep user-provided model tuning/metadata and durable server settings across rebuilds while ensuring fresh runtime credentials and routing take precedence; backup/restore now strips secrets but preserves non-sensitive config.
• Tests
  • Added coverage validating config merge, durable server preservation, reporter-owned metadata restoration, and atomic state-file restore behavior (.last-good anchor).
• Chores
  • Exposed state-restore command builder for safer restore orchestration.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0333c18e-2f6d-4997-bf42-598aa067ef75

📥 Commits

Reviewing files that changed from the base of the PR and between df06f5a and b623586.

📒 Files selected for processing (5)

• src/lib/state/openclaw-config-merge.test.ts
• src/lib/state/openclaw-config-merge.ts
• src/lib/state/sandbox.ts
• test/openclaw-config-snapshot.test.ts
• test/state-file-restore-command.test.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• test/state-file-restore-command.test.ts
• src/lib/state/openclaw-config-merge.ts
• test/openclaw-config-snapshot.test.ts

---

📝 Walkthrough

Walkthrough

Enforces fresh runtime-owned provider/model fields while restoring reporter-owned model tuning and durable MCP server settings, exports a safer state-file restore command with staged .last-good anchoring, and adds unit/integration tests plus a sandbox test helper for backup/restore flows.

Changes

OpenClaw Config Restore Ownership and Merge

Layer / File(s)	Summary
Ownership contract and runtime field definitions src/lib/state/openclaw-config-merge.ts	OPENCLAW_CONFIG_RESTORE_OWNERSHIP updated with narrowed generated maps, adjusted durable sections, and new providerRuntimeOwnedFields / modelRuntimeOwnedFields.
Provider and model reconciliation merge helpers src/lib/state/openclaw-config-merge.ts	New helpers reconcile providers and models by id, enforce runtime-owned fields from the fresh config, avoid resurrecting backup-only id-less models, and mergeOpenClawModels now uses provider reconciliation.
Config merge unit tests src/lib/state/openclaw-config-merge.test.ts	Vitest case verifying merged result preserves fresh provider routing/credentials, retains fresh model identity while restoring backup tuning/metadata, preserves fresh gateway token, and keeps durable mcp.servers settings.
State restore command adjustments src/lib/state/sandbox.ts, test/state-file-restore-command.test.ts	Exported buildStateFileRestoreCommand and updated its OpenClaw path to stage and atomically install dst.last-good from a parent-dir temp file; added tests validating staging, symlink refusal, atomic install ordering, and hash refresh behavior.
Integration tests & sandbox helper test/openclaw-config-snapshot.test.ts	Added writeFakeSandboxBins to emit fake openshell/ssh scripts; refactored existing test to use it and added #5202 integration test verifying backup sanitization and restore precedence across rebuild.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• NVIDIA/NemoClaw#5177: Related changes to mergeOpenClawRestoredConfig addressing provider/model ownership semantics.
• NVIDIA/NemoClaw#5174: Related ownership-aware merge updates and wiring into restore flows.

Suggested labels

integration: openclaw, area: sandbox, bug-fix

Suggested reviewers

• cv
• prekshivyas

Poem

🐰 I hopped through backups, with a curious nose,
Kept the fresh keys shining where the runtime wind blows,
Restored the quiet tuning that reporters compose,
Anchored the last-good safe where durable moss grows,
Rebuilds hum along while old wisdom still shows.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 35.71% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main fix: preserving reporter-owned model metadata during openclaw.json rebuild restore, matching the primary change objective.
Linked Issues check	✅ Passed	The PR comprehensively addresses all coding requirements from #5202: reconciles matching providers by ownership instead of wholesale replacement, refreshes .last-good anchor to prevent reversion, restores user-owned tuning while keeping runtime-owned fields from fresh rebuilds, and includes unit/integration tests validating merged metadata and mcp.servers retention.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing #5202: config-merge logic updates, sandbox restore command refresh, and related test additions. The refactoring of test helpers (writeFakeSandboxBins) is necessary infrastructure for new integration tests and remains within scope.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[prekshivyas]

Reviewed (code + 9-cat security), verified against source at the head SHA.

✅ Approve — correct, well-scoped fix for #5202. Reconciles models.providers by ownership during rebuild restore (fresh rebuild keeps routing/credentials + model id/name; sanitized backup restores non-secret tuning like reasoning/cost/maxTokens/compat), and refreshes OpenClaw's .last-good recovery anchor before the live config swap so the integrity watcher doesn't revert the merge.

Verified: mergeOpenClawProviderMap/...Entry/...ModelArray exist and replace the prior wholesale-replace behavior that was the real cause of the bug; restoreRuntimeOwnedFields deletes a field when absent from current, so a stale backup apiKey/baseUrl can't resurrect; the backup is sanitized at backup time (sanitizeConfigFile) so adding mcp to durable sections inherits already-scrubbed content (the snapshot test asserts GITHUB_TOKEN === [STRIPPED_BY_MIGRATION]); and the .last-good anchor is staged via temp + atomic same-dir rename, symlink-rejected, fail-closed before the swap.

Security: all 9 pass (symlink rejection fail-closed; no secret leakage; anchor-before-swap closes the integrity-watcher race). Nits: .last-good chmod 660 is looser than the config's 640 (single-tenant, negligible — align for least-privilege); a redundant models merge before the explicit array overwrite (harmless). Tests adequate (ownership-merge unit, command-ordering + symlink-rejection + non-OpenClaw negative, full backup→restore integration).

[wscurran]

✨
Related open issues:

• #5202 Openclaw config partially restored after nemoclaw <name> rebuild after upgrading nemoclaw version