Skip to content

test(cli): lock in #1555 destroy-side session clear so list cannot resurrect destroyed sandboxes (#1641) #1677

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.

Sign up for GitHub

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

Open
ColinM-sys wants to merge 1 commit into
base: main
Choose a base branch
from
Open

test(cli): lock in #1555 destroy-side session clear so list cannot resurrect destroyed sandboxes (#1641) #1677

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions src/lib/inventory-commands.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,43 @@ describe("inventory commands", () => {
);
});

it("regression #1641: after destroy clears the session, list shows the onboard hint instead of resurrecting the sandbox", async () => {
// Reproduces the loop the reporter described in #1641: a sandbox
// is destroyed, registry is empty, the destroy code clears
// session.sandboxName to null (added in #1555), and the next
// `nemoclaw list` MUST NOT recover the destroyed sandbox from
// the session. Without the destroy-side clear, list would print
// "Recovered sandbox inventory from the last onboard session"
// and the user would loop between `list` (showing the sandbox)
// and `connect` (removing it as stale).
const lines: string[] = [];
await listSandboxesCommand({
recoverRegistryEntries: async () => ({
sandboxes: [],
defaultSandbox: null,
recoveredFromSession: false,
recoveredFromGateway: 0,
}),
getLiveInference: () => null,
// Post-destroy session: sandboxName has been cleared. The
// session object itself still exists (other fields populated)
// but loadLastSession returning a session with sandboxName=null
// must NOT trip the "last onboarded sandbox was 'X'" hint.
loadLastSession: () => ({ sandboxName: null }),
log: (message = "") => lines.push(message),
});

expect(lines).toContain(
" No sandboxes registered. Run `nemoclaw onboard` to get started.",
);
expect(
lines.some((line) =>
line.includes("Recovered sandbox inventory from the last onboard session"),
),
).toBe(false);
expect(lines.some((line) => line.includes("last onboarded sandbox was"))).toBe(false);
});
Comment on lines +23 to +58
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Regression scope is too narrow to catch destroy-path regressions.

Line 33–46 hardcodes the post-destroy state (loadLastSession already returns { sandboxName: null }), so this test only verifies list rendering. If destroy stops clearing the session again, this test will still pass because destroy is never exercised. Please add/convert this to a flow that performs destroy and then list against shared state so #1555 is actually pinned.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/lib/inventory-commands.test.ts` around lines 23 - 58, The test currently
stubs loadLastSession to { sandboxName: null } so it never exercises the destroy
codepath; instead create a shared in-memory session/registry object and call the
actual destroy flow before invoking listSandboxesCommand so the test verifies
the destroy logic clears the session. Concretely: introduce a mutable shared
object (e.g., const sharedState = { session: { sandboxName: "X" }, registry:
{...} }), wire your mocks passed to listSandboxesCommand to read from
sharedState (loadLastSession returns sharedState.session, recoverRegistryEntries
reads sharedState.registry), then import and invoke the real destroy routine
used in tests (the project’s destroy command function) to mutate sharedState
(clearing session.sandboxName) and finally call listSandboxesCommand to assert
the onboarding hint and absence of recovery messages.


it("prints recovered sandbox inventory details", async () => {
const lines: string[] = [];
await listSandboxesCommand({
Expand Down