-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(rebuild): reconcile stale pinned session models after inference switch (#7102) #7109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
apurvvkumaria
merged 7 commits into
main
from
fix/7102-reconcile-stale-session-model-on-rebuild
Jul 17, 2026
Merged
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
26f5d5d
fix(rebuild): reconcile stale pinned session models after inference s…
yanyunl1991 520d3c0
fix(rebuild): harden session model reconciliation
jyaunches b4378e9
test(rebuild): cover session model reconciliation flow
jyaunches 2ccc4ed
test(rebuild): use descriptor-safe session reads
jyaunches 16a7fe4
test(rebuild): keep session assertions linear
jyaunches d0889ea
fix(rebuild): harden session model reconciliation
jyaunches 05965bb
Merge remote-tracking branch 'origin/main' into fix/7102-reconcile-st…
jyaunches File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
292 changes: 292 additions & 0 deletions
292
src/lib/actions/sandbox/reconcile-session-models.test.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,292 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import { spawnSync } from "node:child_process"; | ||
| import { | ||
| mkdtempSync, | ||
| readdirSync, | ||
| readFileSync, | ||
| rmSync, | ||
| statSync, | ||
| symlinkSync, | ||
| writeFileSync, | ||
| } from "node:fs"; | ||
| import { tmpdir } from "node:os"; | ||
| import { join } from "node:path"; | ||
| import { beforeEach, describe, expect, it, vi } from "vitest"; | ||
| import { executeSandboxCommand } from "./process-recovery"; | ||
| import { | ||
| buildSessionStoreReplaceCommand, | ||
| reconcilePinnedSessionModels, | ||
| reconcileStalePinnedSessionModelsAfterRebuild, | ||
| } from "./reconcile-session-models"; | ||
|
|
||
| vi.mock("./process-recovery", () => ({ executeSandboxCommand: vi.fn() })); | ||
|
|
||
| const executeSandboxCommandMock = vi.mocked(executeSandboxCommand); | ||
|
|
||
| beforeEach(() => { | ||
| executeSandboxCommandMock.mockReset(); | ||
| }); | ||
|
|
||
| function store(entries: Record<string, unknown>): string { | ||
| return JSON.stringify(entries); | ||
| } | ||
|
|
||
| describe("reconcilePinnedSessionModels (#7102)", () => { | ||
| const primary = "inference/nvidia/llama-3.3-nemotron-super-49b-v1.5"; | ||
|
|
||
| it("clears a managed pin that no longer matches the current default", () => { | ||
| const raw = store({ | ||
| "agent:main:main": { | ||
| sessionId: "s1", | ||
| updatedAt: 1, | ||
| modelProvider: "inference", | ||
| model: "meta/llama-3.1-8b-instruct", | ||
| }, | ||
| }); | ||
| const result = reconcilePinnedSessionModels(raw, primary); | ||
| expect(result.changed).toBe(true); | ||
| expect(result.clearedSessionKeys).toEqual(["agent:main:main"]); | ||
| const parsed = JSON.parse(result.content); | ||
| expect(parsed["agent:main:main"].model).toBeUndefined(); | ||
| expect(parsed["agent:main:main"].modelProvider).toBeUndefined(); | ||
| // Non-model fields are preserved. | ||
| expect(parsed["agent:main:main"].sessionId).toBe("s1"); | ||
| }); | ||
|
|
||
| it("leaves a session already on the current default untouched", () => { | ||
| const raw = store({ | ||
| "agent:main:main": { | ||
| modelProvider: "inference", | ||
| model: "nvidia/llama-3.3-nemotron-super-49b-v1.5", | ||
| }, | ||
| }); | ||
| const result = reconcilePinnedSessionModels(raw, primary); | ||
| expect(result.changed).toBe(false); | ||
| expect(result.content).toBe(raw); | ||
| }); | ||
|
|
||
| it("leaves an intentional non-managed provider pin untouched", () => { | ||
| const raw = store({ | ||
| "agent:main:main": { modelProvider: "openai", model: "gpt-5.6-terra" }, | ||
| }); | ||
| const result = reconcilePinnedSessionModels(raw, primary); | ||
| expect(result.changed).toBe(false); | ||
| }); | ||
|
|
||
| it("only clears the stale managed sessions in a mixed store", () => { | ||
| const raw = store({ | ||
| stale: { modelProvider: "inference", model: "meta/llama-3.1-8b-instruct" }, | ||
| current: { modelProvider: "inference", model: "nvidia/llama-3.3-nemotron-super-49b-v1.5" }, | ||
| intentional: { modelProvider: "openai", model: "gpt-5.6-terra" }, | ||
| unpinned: { sessionId: "x" }, | ||
| }); | ||
| const result = reconcilePinnedSessionModels(raw, primary); | ||
| expect(result.clearedSessionKeys).toEqual(["stale"]); | ||
| const parsed = JSON.parse(result.content); | ||
| expect(parsed.stale.model).toBeUndefined(); | ||
| expect(parsed.current.model).toBe("nvidia/llama-3.3-nemotron-super-49b-v1.5"); | ||
| expect(parsed.intentional.model).toBe("gpt-5.6-terra"); | ||
| expect(parsed.unpinned.sessionId).toBe("x"); | ||
| }); | ||
|
|
||
| it("is a no-op when the primary ref is missing", () => { | ||
| const raw = store({ | ||
| "agent:main:main": { modelProvider: "inference", model: "meta/llama-3.1-8b-instruct" }, | ||
| }); | ||
| expect(reconcilePinnedSessionModels(raw, null).changed).toBe(false); | ||
| }); | ||
|
|
||
| it("is a no-op on malformed session json", () => { | ||
| expect(reconcilePinnedSessionModels("not json", primary).changed).toBe(false); | ||
| expect(reconcilePinnedSessionModels("[]", primary).changed).toBe(false); | ||
| }); | ||
|
|
||
| it("ignores an entry with a non-string model", () => { | ||
| const raw = store({ | ||
| "agent:main:main": { modelProvider: "inference", model: 42 }, | ||
| }); | ||
| expect(reconcilePinnedSessionModels(raw, primary).changed).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe("buildSessionStoreReplaceCommand", () => { | ||
| it("uses no-follow exclusive staging with atomic replacement and cleanup (#7102)", () => { | ||
| const command = buildSessionStoreReplaceCommand( | ||
| "/sandbox/.openclaw/agents/main/sessions/sessions.json", | ||
| '{"new":true}\n', | ||
| '{"old":true}', | ||
| ); | ||
|
|
||
| expect(command).toContain("os.O_EXCL"); | ||
| expect(command).toContain("os.O_NOFOLLOW"); | ||
| expect(command).toContain("os.fchown(staged_fd, source_stat.st_uid, source_stat.st_gid)"); | ||
| expect(command).toContain("os.fchmod(staged_fd, stat.S_IMODE(source_stat.st_mode))"); | ||
| expect(command).toContain("os.replace(staged_name, target_name"); | ||
| expect(command).toContain("if not installed and staged_name"); | ||
| expect(command).toContain("os.unlink(staged_name, dir_fd=parent_fd)"); | ||
| expect(command).not.toContain(".nemoclaw-tmp"); | ||
| }); | ||
|
|
||
| it("atomically replaces a regular store while preserving its metadata (#7102)", () => { | ||
| const root = mkdtempSync(join(tmpdir(), "nemoclaw-session-reconcile-")); | ||
| try { | ||
| const sessionsPath = join(root, "sessions.json"); | ||
| const original = '{"old":true}\n'; | ||
| const replacement = '{"new":true}\n'; | ||
| writeFileSync(sessionsPath, original, { mode: 0o640 }); | ||
| const before = statSync(sessionsPath); | ||
|
|
||
| const result = spawnSync( | ||
| "sh", | ||
| ["-c", buildSessionStoreReplaceCommand(sessionsPath, replacement, original.trim())], | ||
| { encoding: "utf8" }, | ||
| ); | ||
|
|
||
| expect(result.status, result.stderr).toBe(0); | ||
| expect(readFileSync(sessionsPath, "utf8")).toBe(replacement); | ||
| const after = statSync(sessionsPath); | ||
| expect(after.mode & 0o777).toBe(before.mode & 0o777); | ||
| expect(after.uid).toBe(before.uid); | ||
| expect(after.gid).toBe(before.gid); | ||
| expect(readdirSync(root)).toEqual(["sessions.json"]); | ||
| } finally { | ||
| rmSync(root, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
|
|
||
| it("refuses a symlinked store without changing its target (#7102)", () => { | ||
| const root = mkdtempSync(join(tmpdir(), "nemoclaw-session-reconcile-link-")); | ||
| try { | ||
| const targetPath = join(root, "target.json"); | ||
| const sessionsPath = join(root, "sessions.json"); | ||
| const original = '{"keep":true}\n'; | ||
| writeFileSync(targetPath, original); | ||
| symlinkSync(targetPath, sessionsPath); | ||
|
|
||
| const result = spawnSync( | ||
| "sh", | ||
| [ | ||
| "-c", | ||
| buildSessionStoreReplaceCommand(sessionsPath, '{"replace":true}\n', original.trim()), | ||
| ], | ||
| { encoding: "utf8" }, | ||
| ); | ||
|
|
||
| expect(result.status).not.toBe(0); | ||
| expect(readFileSync(targetPath, "utf8")).toBe(original); | ||
| expect(readdirSync(root).sort()).toEqual(["sessions.json", "target.json"]); | ||
| } finally { | ||
| rmSync(root, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
|
|
||
| it("refuses stale source content without changing the store (#7102)", () => { | ||
| const root = mkdtempSync(join(tmpdir(), "nemoclaw-session-reconcile-race-")); | ||
| try { | ||
| const sessionsPath = join(root, "sessions.json"); | ||
| const current = '{"current":true}\n'; | ||
| writeFileSync(sessionsPath, current); | ||
|
|
||
| const result = spawnSync( | ||
| "sh", | ||
| [ | ||
| "-c", | ||
| buildSessionStoreReplaceCommand(sessionsPath, '{"replacement":true}\n', '{"stale":true}'), | ||
| ], | ||
| { encoding: "utf8" }, | ||
| ); | ||
|
|
||
| expect(result.status).not.toBe(0); | ||
| expect(readFileSync(sessionsPath, "utf8")).toBe(current); | ||
| expect(readdirSync(root)).toEqual(["sessions.json"]); | ||
| } finally { | ||
| rmSync(root, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| describe("reconcileStalePinnedSessionModelsAfterRebuild", () => { | ||
| const primary = "inference/nvidia/llama-3.3-nemotron-super-49b-v1.5"; | ||
| const config = JSON.stringify({ agents: { defaults: { model: { primary } } } }); | ||
| const staleStore = store({ | ||
| "agent:main:main": { | ||
| modelProvider: "inference", | ||
| model: "meta/llama-3.1-8b-instruct", | ||
| }, | ||
| }); | ||
|
|
||
| it("reads restored state and dispatches a guarded write for stale pins (#7102)", () => { | ||
| executeSandboxCommandMock | ||
| .mockReturnValueOnce({ status: 0, stdout: config, stderr: "" }) | ||
| .mockReturnValueOnce({ status: 0, stdout: staleStore, stderr: "" }) | ||
| .mockReturnValueOnce({ status: 0, stdout: "", stderr: "" }); | ||
| const log = vi.fn(); | ||
|
|
||
| reconcileStalePinnedSessionModelsAfterRebuild("alpha", log); | ||
|
|
||
| expect(executeSandboxCommandMock).toHaveBeenCalledTimes(3); | ||
| expect(executeSandboxCommandMock.mock.calls[0]).toEqual([ | ||
| "alpha", | ||
| "cat /sandbox/.openclaw/openclaw.json 2>/dev/null", | ||
| ]); | ||
| expect(executeSandboxCommandMock.mock.calls[1]).toEqual([ | ||
| "alpha", | ||
| "cat /sandbox/.openclaw/agents/main/sessions/sessions.json 2>/dev/null", | ||
| ]); | ||
| const writeCommand = executeSandboxCommandMock.mock.calls[2][1]; | ||
| expect(writeCommand).toContain("python3 -c"); | ||
| expect(writeCommand).toContain("os.O_NOFOLLOW"); | ||
| expect(writeCommand).not.toContain(".nemoclaw-tmp"); | ||
| expect(log).toHaveBeenLastCalledWith( | ||
| `Session model reconcile: cleared stale pinned model on 1 session(s) so they follow ${primary}`, | ||
| ); | ||
| }); | ||
|
|
||
| it("stops when the restored config has no primary model (#7102)", () => { | ||
| executeSandboxCommandMock.mockReturnValueOnce({ | ||
| status: 0, | ||
| stdout: '{"agents":{"defaults":{}}}', | ||
| stderr: "", | ||
| }); | ||
| const log = vi.fn(); | ||
|
|
||
| reconcileStalePinnedSessionModelsAfterRebuild("alpha", log); | ||
|
|
||
| expect(executeSandboxCommandMock).toHaveBeenCalledTimes(1); | ||
| expect(log).toHaveBeenLastCalledWith( | ||
| "Session model reconcile skipped: could not read agents.defaults.model.primary", | ||
| ); | ||
| }); | ||
|
|
||
| it("stops when the restored session store cannot be read (#7102)", () => { | ||
| executeSandboxCommandMock | ||
| .mockReturnValueOnce({ status: 0, stdout: config, stderr: "" }) | ||
| .mockReturnValueOnce({ status: 1, stdout: "", stderr: "missing" }); | ||
| const log = vi.fn(); | ||
|
|
||
| reconcileStalePinnedSessionModelsAfterRebuild("alpha", log); | ||
|
|
||
| expect(executeSandboxCommandMock).toHaveBeenCalledTimes(2); | ||
| expect(log).toHaveBeenLastCalledWith( | ||
| "Session model reconcile skipped: no session store at /sandbox/.openclaw/agents/main/sessions/sessions.json", | ||
| ); | ||
| }); | ||
|
|
||
| it("reports an atomic write failure without retrying or claiming success (#7102)", () => { | ||
| executeSandboxCommandMock | ||
| .mockReturnValueOnce({ status: 0, stdout: config, stderr: "" }) | ||
| .mockReturnValueOnce({ status: 0, stdout: staleStore, stderr: "" }) | ||
| .mockReturnValueOnce({ status: 9, stdout: "", stderr: "refused" }); | ||
| const log = vi.fn(); | ||
|
|
||
| reconcileStalePinnedSessionModelsAfterRebuild("alpha", log); | ||
|
|
||
| expect(executeSandboxCommandMock).toHaveBeenCalledTimes(3); | ||
| expect(log).toHaveBeenLastCalledWith( | ||
| "Session model reconcile: failed to write /sandbox/.openclaw/agents/main/sessions/sessions.json (status=9)", | ||
| ); | ||
| expect(log.mock.calls.flat()).not.toContainEqual(expect.stringContaining("cleared stale")); | ||
| }); | ||
| }); | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.