fix(rebuild): reconcile stale pinned session models after inference switch (#7102) - #7109
Merged
apurvvkumaria merged 7 commits intoJul 17, 2026
Merged
Conversation
…witch (#7102) OpenClaw pins a `{ modelProvider, model }` on each stored session in `agents/<id>/sessions/sessions.json`. `nemoclaw inference set` + `rebuild` update the config default (`agents.defaults.model.primary`) but leave those per-session pins on the pre-switch model. When the OpenClaw TUI resumes the last session on the first `connect` after a switch, its status bar shows the old pinned model; a reconnect looks correct once the session is reconciled. Reconcile the persisted pins during the rebuild post-restore window, while the gateway is down (OpenClaw owns `sessions.json` while live, so editing it there would race its writes). For the default agent, clear the pin of any session whose managed-`inference` model no longer matches the current default so it falls back to the config default — matching OpenClaw's own clean-entry semantics after `sessions.reset`. Sessions already on the default, and sessions pinned to a different provider (an intentional per-session choice), are left untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds reconciliation for stale managed-provider session model pins after sandbox rebuilds, atomically persists cleared pins, logs outcomes, and invokes the process after ChangesSession model reconciliation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant runRebuildPostRestorePhase
participant reconcileStalePinnedSessionModelsAfterRebuild
participant executeSandboxCommand
participant sessions.json
runRebuildPostRestorePhase->>reconcileStalePinnedSessionModelsAfterRebuild: Reconcile after openclaw doctor --fix
reconcileStalePinnedSessionModelsAfterRebuild->>executeSandboxCommand: Read primary model and sessions.json
executeSandboxCommand-->>reconcileStalePinnedSessionModelsAfterRebuild: Return sandbox data
reconcileStalePinnedSessionModelsAfterRebuild->>reconcilePinnedSessionModels: Clear stale managed pins
reconcileStalePinnedSessionModelsAfterRebuild->>executeSandboxCommand: Atomically replace sessions.json
executeSandboxCommand->>sessions.json: Guarded write
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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
PR Review Advisor — InformationalAdvisor assessment: Informational / medium 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: 3 optional E2E recommendations
2 warnings · 0 suggestionsWarningsWarnings do not block.
|
![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/actions/sandbox/reconcile-session-models.ts`:
- Around line 115-120: Update the sessions.json publish flow around
executeSandboxCommand to use the repository’s descriptor-safe temporary-file and
atomic-replace primitive instead of shell redirection to tmpPath followed by mv.
Ensure the write cannot follow a pre-existing symlink and that the replacement
preserves the original file’s owner and mode.
🪄 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: 995c0a0c-28a1-4eed-8def-aee151619fca
📒 Files selected for processing (3)
src/lib/actions/sandbox/rebuild-post-restore-phase.tssrc/lib/actions/sandbox/reconcile-session-models.test.tssrc/lib/actions/sandbox/reconcile-session-models.ts
wscurran
added
area: cli
Replace restored session state through a no-follow, inode-checked atomic write. Preserve the original owner and mode, refuse symlinked or concurrently changed stores, and clean up owned staging files on failure. Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/actions/sandbox/reconcile-session-models.test.ts (2)
105-120: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest safety behavior rather than Python source text.
These assertions lock the test to
os.*calls and a temporary-name implementation. Keep observable checks—rejection, metadata preservation, and no staging artifacts—and remove command substring assertions.As per path instructions, “Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions.”
🤖 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/actions/sandbox/reconcile-session-models.test.ts` around lines 105 - 120, Update the buildSessionStoreReplaceCommand test to exercise the public command behavior rather than asserting Python source substrings. Replace the os.* and temporary-name checks with observable assertions covering rejection, preservation of target metadata, atomic replacement, and cleanup of staging artifacts; retain the existing test inputs and public boundary.Source: Path instructions
132-136: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftKeep host-process execution out of this unit test. These cases invoke real
shandpython3; inject/mock the process boundary here and place the end-to-end shell/Python contract in an appropriate integration/E2E suite.
src/lib/actions/sandbox/reconcile-session-models.test.ts#L132-L136: replace the real process invocation with a mocked process boundary.src/lib/actions/sandbox/reconcile-session-models.test.ts#L159-L166: move the symlink behavior check to integration/E2E coverage.src/lib/actions/sandbox/reconcile-session-models.test.ts#L183-L190: move the stale-source behavior check to integration/E2E coverage.As per coding guidelines, “Mock external dependencies in unit tests.”
🤖 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/actions/sandbox/reconcile-session-models.test.ts` around lines 132 - 136, The unit test currently executes real sh and python3 processes. In src/lib/actions/sandbox/reconcile-session-models.test.ts lines 132-136, mock or inject the process boundary used by buildSessionStoreReplaceCommand instead of calling spawnSync directly; move the symlink behavior coverage at lines 159-166 and stale-source behavior coverage at lines 183-190 to an appropriate integration/E2E suite, with no direct change required there beyond removal from this unit test.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@src/lib/actions/sandbox/reconcile-session-models.test.ts`:
- Around line 105-120: Update the buildSessionStoreReplaceCommand test to
exercise the public command behavior rather than asserting Python source
substrings. Replace the os.* and temporary-name checks with observable
assertions covering rejection, preservation of target metadata, atomic
replacement, and cleanup of staging artifacts; retain the existing test inputs
and public boundary.
- Around line 132-136: The unit test currently executes real sh and python3
processes. In src/lib/actions/sandbox/reconcile-session-models.test.ts lines
132-136, mock or inject the process boundary used by
buildSessionStoreReplaceCommand instead of calling spawnSync directly; move the
symlink behavior coverage at lines 159-166 and stale-source behavior coverage at
lines 183-190 to an appropriate integration/E2E suite, with no direct change
required there beyond removal from this unit test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d4bbddda-c32f-4e99-a39b-ca25f7493a3f
📒 Files selected for processing (2)
src/lib/actions/sandbox/reconcile-session-models.test.tssrc/lib/actions/sandbox/reconcile-session-models.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/actions/sandbox/reconcile-session-models.ts
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/actions/sandbox/reconcile-session-models.test.ts (1)
238-241: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep shell hardening assertions in the command-builder tests.
Checking
os.O_NOFOLLOWand temporary-file names couples this orchestration test to generated source text without proving behavior. The dedicated replacement-command tests already own those guarantees; here, retain only the dispatch and outcome assertions.Proposed cleanup
- const writeCommand = executeSandboxCommandMock.mock.calls[2][1]; - expect(writeCommand).toContain("python3 -c"); - expect(writeCommand).toContain("os.O_NOFOLLOW"); - expect(writeCommand).not.toContain(".nemoclaw-tmp");As per path instructions, “Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions.”
🤖 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/actions/sandbox/reconcile-session-models.test.ts` around lines 238 - 241, Remove the generated-command string assertions for “python3 -c”, “os.O_NOFOLLOW”, and “.nemoclaw-tmp” from the reconciliation test. Keep this orchestration test focused on dispatch and observable outcome assertions, leaving shell-hardening coverage to the dedicated replacement-command tests.Source: Path instructions
🤖 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.
Nitpick comments:
In `@src/lib/actions/sandbox/reconcile-session-models.test.ts`:
- Around line 238-241: Remove the generated-command string assertions for
“python3 -c”, “os.O_NOFOLLOW”, and “.nemoclaw-tmp” from the reconciliation test.
Keep this orchestration test focused on dispatch and observable outcome
assertions, leaving shell-hardening coverage to the dedicated
replacement-command tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 51612216-9632-4252-8b15-5c8927ef1e8d
📒 Files selected for processing (1)
src/lib/actions/sandbox/reconcile-session-models.test.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
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/actions/sandbox/reconcile-session-models.test.ts`:
- Around line 223-227: Update the buildSessionStoreReplaceCommand call in the
symlink reconciliation test to pass the exact original fixture content for hash
calculation, replacing original.trim() with original while leaving the
newline-terminated replacement content unchanged.
🪄 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: 72609c49-4c2a-4080-8f0f-2d665e5171e7
📒 Files selected for processing (3)
src/lib/actions/sandbox/rebuild-post-restore-phase.test.tssrc/lib/actions/sandbox/reconcile-session-models.test.tssrc/lib/actions/sandbox/reconcile-session-models.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/actions/sandbox/reconcile-session-models.ts
Comment on lines
+223
to
+227
| buildSessionStoreReplaceCommand( | ||
| join(linkedParent, "sessions.json"), | ||
| '{"replace":true}\n', | ||
| original.trim(), | ||
| ), |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Hash the exact fixture content in this symlink test.
original.trim() differs from the file’s newline-terminated content, so the command may reject the stale hash before testing the symlinked parent path. Pass original to ensure this test exercises its stated claim.
Proposed fix
- original.trim(),
+ original,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| buildSessionStoreReplaceCommand( | |
| join(linkedParent, "sessions.json"), | |
| '{"replace":true}\n', | |
| original.trim(), | |
| ), | |
| buildSessionStoreReplaceCommand( | |
| join(linkedParent, "sessions.json"), | |
| '{"replace":true}\n', | |
| original, | |
| ), |
🤖 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/actions/sandbox/reconcile-session-models.test.ts` around lines 223 -
227, Update the buildSessionStoreReplaceCommand call in the symlink
reconciliation test to pass the exact original fixture content for hash
calculation, replacing original.trim() with original while leaving the
newline-terminated replacement content unchanged.
Source: Path instructions
…ale-session-model-on-rebuild
21 tasks
jyaunches
approved these changes
Jul 17, 2026
jyaunches
left a comment
jyaunches
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the rebuild reconciliation and hardening changes. The stale managed session-pin fix is narrowly scoped; atomic replacement rejects symlink traversal and stale-source races, preserves metadata, and sanitizes sandbox-derived model references. Focused unit/orchestration coverage, full CI, security scans, advisor review, and the trusted exact-diff E2E gate (including upgrade-stale-sandbox) are green.
apurvvkumaria
deleted the
fix/7102-reconcile-stale-session-model-on-rebuild
branch
July 17, 2026 21:01
21 tasks
apurvvkumaria
pushed a commit
that referenced
this pull request
Jul 18, 2026
<!-- markdownlint-disable MD041 --> ## Summary Add the v0.0.87 changelog entry and align the DGX Station, platform-support, and rebuild documentation with behavior merged since v0.0.86. The Station documentation retains the Deferred support status while recording the two exact factory-image qualification profiles and the post-reboot receipt compatibility fix from #7130. ## Changes - Add the v0.0.87 changelog summary, including the merged Station resume receipt fix, with links to the owning documentation pages. - Document the exact April 2026 Colossus BaseOS and June 2026 AI Developer Tools Station identities, validation boundaries, and permitted host preparation. - Synchronize those Station qualification paths into the canonical platform matrix and generated provider/platform pages. - Document how an OpenClaw rebuild clears stale managed-provider session-model pins after an inference switch. ### Source summary - [#7130](#7130) -> `docs/changelog/2026-07-17.mdx`: Document compatibility with current six-field and legacy three-field Station resume receipts after host preparation. - [#7128](#7128) -> `docs/changelog/2026-07-17.mdx`: Document restart-safe managed DCode startup and required Docker resource limits. - [#7126](#7126) -> `docs/changelog/2026-07-17.mdx`, `docs/get-started/dgx-station-preparation.mdx`, `ci/platform-matrix.json`: Document the two bounded Station factory-image qualification profiles without promoting Deferred support and synchronize the generated platform/provider references. - [#6947](#6947) -> `docs/changelog/2026-07-17.mdx`: Document streaming sandbox backup archive creation. - [#7117](#7117) -> `docs/changelog/2026-07-17.mdx`: Document Hermes post-restore gateway and managed MCP health verification. - [#7109](#7109) -> `docs/changelog/2026-07-17.mdx`, `docs/manage-sandboxes/recover-rebuild-sandboxes.mdx`: Document stale managed session-model pin reconciliation after rebuild. - [#7068](#7068) -> `docs/changelog/2026-07-17.mdx`: Document strict-provider compatibility for Hermes tool schemas. - [#6965](#6965) -> `docs/changelog/2026-07-17.mdx`: Document managed vLLM download storage estimation. - [#7114](#7114) -> `docs/changelog/2026-07-17.mdx`: Document preserved, redacted rebuild diagnostics. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: Documentation-only release-prep update; the changelog, platform-generation contracts, and docs build validate the changed pages and links. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/generate-platform-docs.test.ts test/station-doc-ownership.test.ts test/changelog-docs.test.ts`: 29 passed; `python3 scripts/generate-platform-docs.py --check`: all generated tables in sync - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [x] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added filesystem-aware managed vLLM storage preflight (cold download sizing; interactive vs non-interactive capacity checks). - Improved tool-schema compatibility for strict OpenAI-compatible providers (including Gemini schema handling) using a strict single envelope. - Enhanced sandbox backup creation with streamed archive generation and incremental entry validation. - **Bug Fixes** - Strengthened rebuild/recovery checks with Hermes sandbox health validation and cleanup of stale managed-provider session pins. - Persisted onboarding startup commands with required `nproc`/`nofile` limits across sandbox recreation. - Improved replacement-image rebuild diagnostics with bounded, redacted output handling. - For OpenCLAW “rebuild while preserving state,” stale model/provider pins are cleared when appropriate. - **Documentation** - Expanded DGX Station GB300 no-OTA factory profile/qualification criteria and clarified managed vLLM provider/sandbox constraints. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
This was referenced Jul 20, 2026
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
5 participants
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
nemoclaw inference set+rebuild, the firstnemoclaw connectsession shows the old model name in the OpenClaw TUI status bar; a disconnect + reconnect shows the correct new name. This PR reconciles the stale per-session model state during rebuild so the first connect shows the new model.Closes #7102.
Reproduction
On our Ubuntu 24.04 x86_64 test host (no GPU), against a managed-inference sandbox (the status-bar model uses the
inference/<model>managed ref regardless of the concrete provider):agent:main:main— pinned to A). The reporter's QA flow does this by chatting before switching.nemoclaw inference set --provider <p> --model <B>nemoclaw <sb> rebuild --yesnemoclaw <sb> connectthenopenclaw tui— observe the status-bar model.Observed on
main(before fix) — first connect shows the OLD model:Observed on
fix/...(after fix) — first connect shows the NEW model:Analysis
OpenClaw pins a
{ modelProvider, model }on each stored session in/sandbox/.openclaw/agents/<id>/sessions/sessions.json(visible viaopenclaw status→ the per-sessionModelcolumn).nemoclaw inference setonly rewrites the OpenClaw config (agents.defaults.model.primaryinopenclaw.json, viapatchOpenClawInferenceConfig/writeSandboxConfig), andrebuildrestores theagents/state dir (includingsessions.json) verbatim — neither reconciles the per-session pinned model. The OpenClaw TUI resumes the last session on connect and renders that session's pinned model, so the first connect after a switch shows the pre-switch model. There is no OpenClaw-native "set session model" command; the only native lever (sessions.reset) requires a live gateway.Fix
Reconcile the persisted pins during the rebuild post-restore window (
rebuild-post-restore-phase.ts, right afteropenclaw doctor --fix), while the gateway is still down — OpenClaw ownssessions.jsonwhile live, so editing it duringinference setwould race its writes; the post-restore window does not, and the reporter'sinference set→rebuild→connectflow always passes through it.Contract (
reconcilePinnedSessionModels, pure + unit-tested): for the default agent's session store, clear the pin (model+modelProvider) of any session whosemodelProvideris the managed provider (inference) and whoseinference/<model>no longer equals the currentagents.defaults.model.primary. Cleared sessions fall back to the config default (OpenClaw's own clean-entry semantics aftersessions.reset). Sessions already on the default, and sessions pinned to a different provider (an intentional per-session choice), are left untouched.Changes
src/lib/actions/sandbox/reconcile-session-models.ts: new — purereconcilePinnedSessionModelscontract +reconcileStalePinnedSessionModelsAfterRebuild(reads the restored primary + session store, writes back only if stale pins were cleared).src/lib/actions/sandbox/rebuild-post-restore-phase.ts: invoke the reconcile in the openclaw post-restore window.src/lib/actions/sandbox/reconcile-session-models.test.ts: new — locks the contract (clears stale managed pins; leaves current/other-provider/unpinned sessions and malformed/missing input alone).Type of Change
Verification
reconcile-session-models.test.ts)npm run typecheck:cliandnpm run build:clipassAI Disclosure
Signed-off-by: Yanyun Liao yanyunl@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests