Skip to content

Commit 46a0da1

Browse files
authored
fix(onboard): honor explicit sandbox recreation (#9318)
<!-- markdownlint-disable MD041 --> ## Summary Explicit `--recreate-sandbox` requests now bypass a stale saved build or policy fingerprint during resumed onboarding. Previously, the recovery path rejected the request while instructing the user to supply the flag that was already present. ## Related Issue Fixes #9297 ## Changes - Honor explicit sandbox recreation before comparing saved create-input fingerprints. - Add regression coverage for build, policy, and durable create-intent drift while preserving denial without the flag. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: Existing command documentation already covers resumable onboarding and explicit sandbox recreation. This fix adds no command, flag, default, configuration, or user-visible text. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval: #9318 (comment) - [x] Non-success, skipped, or missing CI check accepted by maintainer — the advisory Nemotron second-opinion lane failed, while the primary Advisor completed with no blockers, warnings, or suggestions and recommended merge as-is: #9318 (comment) ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: Exact-revision review confirmed that existing command and recovery documentation already covers resumable onboarding and explicit sandbox recreation. This fix makes the documented `--recreate-sandbox` recovery instruction work after saved build, policy, or durable create inputs drift; it adds no command, flag, default, configuration, or user-visible text. `git diff --check` passed; exact-revision CI and automated review passed apart from the accepted advisory second-opinion failure. - Agent: Codex <!-- docs-review-head-sha: c797ec9 --> <!-- docs-review-agents-blob-sha: b9fb6a9 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## 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 validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run --project cli src/lib/onboard/machine/handlers/sandbox-checkpoint-crash-recovery.test.ts` passed 42/42; `npx vitest run --project cli src/lib/onboard/machine/handlers/sandbox-recreate-journal.test.ts` passed 15/15. - [ ] 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 - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] 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: San Dang <sdang@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved sandbox recovery when build or policy settings change. * Explicit sandbox recreation now proceeds without incorrectly reporting a fingerprint mismatch error. * Prevented unnecessary failures when checkpoint configuration changes during intentional recreation. * **Tests** * Added coverage to verify successful sandbox recreation after build or policy checkpoint changes. * Confirmed recreation is performed once and completes without errors. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 7534ae3 commit 46a0da1

2 files changed

Lines changed: 74 additions & 3 deletions

File tree

src/lib/onboard/machine/handlers/sandbox-checkpoint-crash-recovery.test.ts

Lines changed: 73 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,13 @@ vi.mock("../../messaging-channel-setup", () => ({
2626

2727
vi.mocked(detectMessagingChannelsFromEnv).mockReturnValue([]);
2828

29-
function defaultCreateFingerprint(sandboxName = "my-assistant"): string {
29+
function defaultCreateFingerprint(
30+
builtFingerprint = "my-assistant",
31+
policyFingerprint = "default",
32+
): string {
3033
return [
31-
sandboxName,
32-
"default",
34+
builtFingerprint,
35+
policyFingerprint,
3336
"provider",
3437
"model",
3538
"openai-completions",
@@ -925,6 +928,34 @@ describe("sandbox crash-recovery replay (#5961, #6228)", () => {
925928
expect(calls.error.mock.calls.flat().join("\n")).toContain("--recreate-sandbox");
926929
});
927930

931+
it.each([
932+
["build", defaultCreateFingerprint("v0.0.108")],
933+
["policy", defaultCreateFingerprint("my-assistant", "previous-policy")],
934+
] as const)("recreates after %s drift when explicitly requested (#9297)", async (_drift, fingerprint) => {
935+
const session = sessionWithCheckpoint(
936+
crashedCheckpoint({
937+
effectGroups: {
938+
sandbox_create: { completedAt: "2026-01-01T00:00:00.000Z", fingerprint },
939+
},
940+
}),
941+
);
942+
session.machine.state = "openclaw";
943+
const { deps, calls } = createDeps({ getSandboxReuseState: () => "ready" }, session);
944+
945+
await handleSandboxState({
946+
...baseOptions(deps, session),
947+
resume: true,
948+
sandboxName: "my-assistant",
949+
recreateSandbox: () => true,
950+
});
951+
952+
expect(calls.createSandbox).toHaveBeenCalledOnce();
953+
expect(calls.createSandbox.mock.calls[0]?.at(-1)).toEqual(
954+
expect.objectContaining({ recreate: true }),
955+
);
956+
expect(calls.error).not.toHaveBeenCalled();
957+
});
958+
928959
it("rejects reuse when a resolved policy or package input drifted despite an unchanged build version and policy tier (#7022)", async () => {
929960
const { deps, calls } = createDeps({ getSandboxReuseState: () => "ready" });
930961
const session = sessionWithCheckpoint(crashedCheckpoint());
@@ -1021,6 +1052,45 @@ describe("sandbox crash-recovery replay (#5961, #6228)", () => {
10211052
expect(resumedRun.calls.error.mock.calls.flat().join("\n")).toContain("--recreate-sandbox");
10221053
});
10231054

1055+
it("recreates after stable resolved create-intent drift when explicitly requested (#9297)", async () => {
1056+
const session = createSession({ sessionId: "sess-1", agent: "openclaw" });
1057+
const updateSession = vi.fn((mutator: (value: typeof session) => void) => {
1058+
mutator(session);
1059+
return session;
1060+
});
1061+
const firstRun = createDeps({ getSandboxReuseState: () => "missing", updateSession });
1062+
1063+
await handleSandboxState({
1064+
...baseOptions(firstRun.deps, session),
1065+
resume: false,
1066+
sandboxName: "my-assistant",
1067+
});
1068+
1069+
const resumedRun = createDeps({ getSandboxReuseState: () => "missing", updateSession });
1070+
const defaultResolve = resumedRun.calls.resolveCreateIntent.getMockImplementation();
1071+
expect(defaultResolve).toBeDefined();
1072+
resumedRun.calls.resolveCreateIntent.mockImplementation(async (input) => {
1073+
const resolved = await defaultResolve!(input);
1074+
return {
1075+
...resolved,
1076+
policy: { ...resolved.policy, basePolicyPath: "/repo/changed-policy.yaml" },
1077+
};
1078+
});
1079+
1080+
await handleSandboxState({
1081+
...baseOptions(resumedRun.deps, session),
1082+
resume: true,
1083+
recreateSandbox: () => true,
1084+
sandboxName: "my-assistant",
1085+
});
1086+
1087+
expect(resumedRun.calls.createSandbox).toHaveBeenCalledOnce();
1088+
expect(resumedRun.calls.createSandbox.mock.calls[0]?.at(-1)).toEqual(
1089+
expect.objectContaining({ recreate: true }),
1090+
);
1091+
expect(resumedRun.calls.error).not.toHaveBeenCalled();
1092+
});
1093+
10241094
it("rejects reasoning capability drift before replaying a recorded sandbox create (#7570)", async () => {
10251095
const session = createSession({ sessionId: "sess-1", agent: "openclaw" });
10261096
const updateSession = vi.fn((mutator: (value: typeof session) => void) => {

src/lib/onboard/machine/handlers/sandbox.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -851,6 +851,7 @@ class SandboxStateFlow<
851851
sandboxName: string,
852852
createIntent: ResolvedSandboxCreateIntent,
853853
): void {
854+
if (this.options.recreateSandbox(false)) return;
854855
const recordedFingerprint = state.session?.checkpoint?.effectGroups.sandbox_create?.fingerprint;
855856
if (!recordedFingerprint) return;
856857
// Older and reuse-backfilled receipts contain the stable create-input prefix.

0 commit comments

Comments
 (0)