Skip to content

fix(sandbox): preserve name in host-side hints - #7896

Closed
deepujain wants to merge 1 commit into
NVIDIA:mainfrom
deepujain:fix/7292-sandbox-host-hint-name
Closed

fix(sandbox): preserve name in host-side hints#7896
deepujain wants to merge 1 commit into
NVIDIA:mainfrom
deepujain:fix/7292-sandbox-host-hint-name

Conversation

@deepujain

@deepujain deepujain commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Preserve the validated sandbox name in managed startup commands so in-sandbox channel guards can print a copyable host command. This replaces the literal <name> fallback when supported OpenShell versions expose OPENSHELL_SANDBOX as a boolean marker.

Related Issue

Fixes #7292

Changes

  • Pass NEMOCLAW_SANDBOX_NAME to every managed agent during sandbox creation and supervisor relaunch.
  • Prefer the NemoClaw-provided name in host-side hints while retaining the existing RFC-1123 output allowlist.
  • Cover OpenShell's boolean-marker behavior for channel add/remove, policy breadcrumbs, initial launch, and relaunch.

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: The existing host command is unchanged; this fix supplies its missing runtime value.
  • 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: Reviewed the runtime value source, create and relaunch propagation, shell allowlist, boolean-marker compatibility, and injection fallback.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: No documentation paths changed; the documented command and environment variable remain unchanged.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit:
  • Station profile/scenario:
  • Result:
  • Supporting evidence:

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 validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: 44 assertions passed across the channel guard, policy breadcrumb, launch renderer, observability compatibility, and supervisor relaunch suites.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result: npm run build:cli, npm run typecheck:cli, npm run lint, and npm run check:diff
  • 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: Deepak Jain deepujain@gmail.com

Summary by CodeRabbit

  • Bug Fixes

    • Improved sandbox name handling for recovery and policy-denial hints.
    • Preserved valid sandbox names when only a boolean sandbox marker is available.
    • Replaced invalid or unsafe names with a generic placeholder to prevent injection.
  • Tests

    • Added regression coverage for sandbox name forwarding, validation, and secure hint rendering.

Propagate the validated sandbox name through managed startup and recovery commands.

Channel guards can then render copyable host commands when OpenShell
exposes only a boolean marker.

Fixes NVIDIA#7292

Signed-off-by: Deepak Jain <deepujain@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: 3424dbc4-73b0-4e6e-ac7d-192a511e703d

📥 Commits

Reviewing files that changed from the base of the PR and between 4dcb89e and b8659b4.

📒 Files selected for processing (6)
  • scripts/nemoclaw-start.sh
  • src/lib/actions/sandbox/supervisor-relaunch.test.ts
  • src/lib/onboard/sandbox-create-launch.test.ts
  • src/lib/onboard/sandbox-create-launch.ts
  • test/nemoclaw-start.test.ts
  • test/repro-5978-policy-denial-hint.test.ts

📝 Walkthrough

Walkthrough

The change forwards resolved sandbox names to managed sandbox environments and updates policy-denial hint rendering to prefer and validate NEMOCLAW_SANDBOX_NAME, with regression coverage for valid, boolean-marker, and injection-like values.

Changes

Sandbox name propagation and policy-denial hints

Layer / File(s) Summary
Runtime sandbox name forwarding
src/lib/onboard/sandbox-create-launch.ts, src/lib/onboard/sandbox-create-launch.test.ts, src/lib/actions/sandbox/supervisor-relaunch.test.ts
NEMOCLAW_SANDBOX_NAME is injected for any managed agent with a resolved sandbox name, and tests verify that the resolved value is serialized while ambient and rendered names are excluded.
Validated policy-denial hint names
scripts/nemoclaw-start.sh, test/nemoclaw-start.test.ts, test/repro-5978-policy-denial-hint.test.ts
Hint rendering prefers NEMOCLAW_SANDBOX_NAME, falls back to OPENSHELL_SANDBOX, and emits <name> for invalid candidates, including injection-like values.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#7870: Updates the same policy-denial hint validation and sandbox-name fallback behavior.

Suggested labels: area: sandbox, area: cli, bug-fix, integration: openclaw, v0.0.98

Suggested reviewers: cv, prekshivyas, brandonpelfrey

🚥 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: preserving the sandbox name for host-side hints.
Linked Issues check ✅ Passed The changes address #7292 by ensuring validated sandbox names are available when rendering the host-side add/remove guard hint.
Out of Scope Changes check ✅ Passed The modified code and tests all support the sandbox-name preservation fix; no unrelated changes are evident.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@deepujain

Copy link
Copy Markdown
Contributor Author

Closing in favor of #7870, which already covers the same runtime name propagation and connect-shell hint path with live OpenShell evidence.

@deepujain deepujain closed this Jul 30, 2026
@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

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, cloud-onboard

1 optional E2E recommendation
  • channels-add-remove

Workflow run details

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

openclaw channels add/remove guard inside sandbox omits the host-side hint

1 participant