fix(cli): stop overstating workspace durability on sandbox stop by Dongni-Yang · Pull Request #7283 · NVIDIA/NemoClaw

Dongni-Yang · 2026-07-21T03:11:03Z

Summary

nemoclaw <sandbox> stop printed Workspace state is preserved. unconditionally whenever docker stop returned 0, promising a workspace-durability guarantee it cannot make: start is a direct docker start of the same container, and the sandbox /sandbox workspace lives on an OpenShell-owned PVC whose re-attachment on restart NemoClaw does not control (#7199). After the change, stop states only what it guarantees — the sandbox stays registered for a later start — and no longer promises workspace-data preservation.

Related Issue

Refs #7199 — this is the message-honesty slice only. Per the issue's triage, the actual workspace data-loss root cause is upstream OpenShell PVC re-attachment on a direct docker start and is being escalated there; this PR intentionally does not claim to fix that, so it uses Refs rather than Fixes/Closes.

Changes

src/lib/actions/sandbox/stop.ts: reword the post-stop message from Workspace state is preserved. to The sandbox stays registered for restart., with a comment explaining why no workspace-durability promise is made here.
src/lib/actions/sandbox/stop.test.ts: the [All Platforms][CLI&UX] nemoclaw sandbox has no stop or pause command — only destroy is available #6026 test now asserts the honest message and adds a guard that the old absolute claim never regresses. No new abstraction, config, or fallback introduced.

Type of Change

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

Tests added or updated for changed behavior
Existing tests cover changed behavior — justification:
Tests not applicable — justification:
Docs updated for user-facing behavior changes
Docs not applicable — justification: no user-facing doc references this CLI string; only stop.ts and its test mention it (verified repo-wide).
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: diff is a user-facing CLI string plus its test only; no sandbox lifecycle, policy, credential, or control-flow logic changed — pending maintainer review.
Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result: npx vitest run --project cli src/lib/actions/sandbox/stop.test.ts → 18/18 passed; npm run test:changed → 40/40 passed
Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result: not applicable (single user-facing string + its test; no runtime/harness or repo-wide validation change)
Quality Gates section completed with required justifications or waivers
No secrets, API keys, or credentials committed
npm run docs builds without warnings (doc changes only)
Doc pages follow the style guide (doc changes only)
New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Dongni Yang dongniy@nvidia.com

Summary by CodeRabbit

Bug Fixes
- Updated sandbox stop messaging to clarify that the sandbox remains registered for restart.
- Removed the misleading claim that workspace state is preserved, reflecting the actual behavior of the stop action.
Tests
- Updated coverage to verify the revised stop confirmation message.

`nemoclaw <sandbox> stop` printed "Workspace state is preserved" unconditionally after `docker stop` succeeded. `start` is a direct `docker start` of the same container, and the sandbox /sandbox workspace lives on an OpenShell-owned PVC whose re-attachment NemoClaw does not control, so the message promised a durability guarantee it cannot make (#7199). Reword to state only what stop guarantees — the sandbox stays registered for a later `start` — and guard the #6026 test against the old absolute claim regressing. Refs #7199 Signed-off-by: Dongni Yang <dongniy@nvidia.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-07-21T03:11:33Z

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: 5fbd4dc3-8b6b-437c-8564-69b40aecb0bc

📥 Commits

Reviewing files that changed from the base of the PR and between d5fa194 and b56cf38.

📒 Files selected for processing (2)

src/lib/actions/sandbox/stop.test.ts
src/lib/actions/sandbox/stop.ts

📝 Walkthrough

Walkthrough

The sandbox stop action now reports that the sandbox remains registered for restart instead of claiming workspace-state preservation. Its test verifies the updated message and absence of the previous wording.

Changes

Sandbox stop messaging

Layer / File(s)	Summary
Update stop message and assertion `src/lib/actions/sandbox/stop.ts`, `src/lib/actions/sandbox/stop.test.ts`	The stop log now describes restart registration, and the test checks for that message while rejecting the workspace-preservation message.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested labels: area: sandbox, bug-fix

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: adjusting the sandbox stop CLI message to stop overstating workspace durability.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/7199-stop-workspace-message-honesty

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

github-actions · 2026-07-21T03:18:20Z

PR Review Advisor — Informational

Advisor assessment: Informational / high confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: onboard-repair, onboard-resume

1 optional E2E recommendation

sandbox-operations

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

github-code-quality · 2026-07-21T03:18:50Z

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit b56cf38 in the fix/7199-stop-worksp... branch remains at 96%, unchanged from commit 65d78ee in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit b56cf38 in the fix/7199-stop-worksp... branch remains at 80%, unchanged from commit 65d78ee in the main branch.

Show a code coverage summary of the most impacted files.

File	`main` 65d78ee	`fix/7199-stop-worksp...` b56cf38	+/-
`src/lib/domain/.../connect-env.ts`	97%	89%	-8%
`src/lib/platform.ts`	89%	84%	-5%
`src/lib/messagi...nnels/policy.ts`	100%	100%	0%
`src/lib/sandbox...rce-identity.ts`	91%	91%	0%
`src/lib/securit...ntial-filter.ts`	93%	93%	0%
`src/lib/trace.ts`	94%	94%	0%
`src/lib/state/g...way-registry.ts`	94%	95%	+1%

_{Updated July 21, 2026 03:28 UTC}

Dongni-Yang · 2026-07-21T03:26:24Z

reviewed-npm-audit is failing on pre-existing advisories unrelated to this change.

This PR changes only src/lib/actions/sandbox/stop.ts and its test — no dependency manifests — so its production npm graph is identical to base d5fa1942a. The reviewed-npm-audit failure comes from two newly-published HIGH-severity DoS advisories in transitive production dependencies:

brace-expansion — GHSA-jxxr-4gwj-5jf2, GHSA-3jxr-9vmj-r5cp
js-yaml — GHSA-h67p-54hq-rp68, GHSA-52cp-r559-cp3m

The same check is currently red on unrelated open PRs (#7284, #7278) and passed on #7280 before these advisories landed, which indicates repo-wide supply-chain state rather than anything introduced here. npm audit fix resolves both; the remedy is a dependency bump on main, and I will rebase this branch once that lands. Flagging for maintainer acceptance of the non-success check per the PR template in the meantime.

Dongni-Yang closed this Jul 21, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(cli): stop overstating workspace durability on sandbox stop#7283

fix(cli): stop overstating workspace durability on sandbox stop#7283
Dongni-Yang wants to merge 1 commit into
mainfrom
fix/7199-stop-workspace-message-honesty

Dongni-Yang commented Jul 21, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 21, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

github-actions Bot commented Jul 21, 2026 •

edited

Loading

Uh oh!

github-code-quality Bot commented Jul 21, 2026 •

edited

Loading

Uh oh!

Dongni-Yang commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Dongni-Yang commented Jul 21, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Type of Change

Quality Gates

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

github-actions Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review Advisor — Informational

Model lanes

E2E guidance

Uh oh!

github-code-quality Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Overview

TypeScript / code-coverage/plugin

TypeScript / code-coverage/cli

Uh oh!

Dongni-Yang commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Dongni-Yang commented Jul 21, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 21, 2026 •

edited

Loading

github-actions Bot commented Jul 21, 2026 •

edited

Loading

github-code-quality Bot commented Jul 21, 2026 •

edited

Loading