Skip to content

feat(cli): add read-only host mounts - #8280

Open
ericksoa wants to merge 14 commits into
mainfrom
feat/read-only-host-mounts-8274
Open

feat(cli): add read-only host mounts#8280
ericksoa wants to merge 14 commits into
mainfrom
feat/read-only-host-mounts-8274

Conversation

@ericksoa

@ericksoa ericksoa commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Add repeatable, read-only host directory mounts to NemoClaw onboarding on Linux and WSL2. Before this change, users had to copy host project files into a sandbox; after it, they can explicitly expose selected existing directories below /sandbox while NemoClaw validates, records, reports, and safely reuses the declarations.

Related Issue

Closes #8274

Changes

  • Add nemoclaw onboard --host-mount <absolute-host-directory:/sandbox/directory> with duplicate, path-normalization, directory-existence, and symlink-component validation.
  • Materialize Docker bind mounts with read_only: true and enable OpenShell's Docker bind-mount gate only for runs that require it. This gateway capability switch is needed because OpenShell rejects driver bind mounts unless the managed gateway explicitly opts in; creation-plan tests cover the merged bind/tmpfs driver configuration.
  • Persist declarations across interrupted onboarding, authoritative rebuilds, registry updates, and status output while revalidating user-editable durable state before reuse.
  • Document security, platform, lifecycle, resume, status, and compatibility-alias behavior in the sandbox-state guide and command reference.

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:
  • 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: security review requested through normal PR review
  • 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: docs-updated
  • Evidence: docs/manage-sandboxes/workspace-files.mdx; docs/reference/commands.mdx; src/lib/onboard.ts; src/lib/onboard/host-mount/index.ts; src/lib/state/registry/host-mount.ts; src/lib/state/registry/types.ts; src/lib/onboard/resume-config.ts; src/lib/onboard/machine/handlers/sandbox-resume.ts; src/lib/onboard/docker-driver-gateway-runtime.ts; src/lib/actions/sandbox/rebuild-gpu-opt-out.ts; src/lib/actions/sandbox/status-text.ts
  • 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 — 274 final host-mount, create-plan, gateway, resume, and lifecycle tests passed; after merging current main, the corrected rebuild, host-mount, create-plan, and shared gateway-health tests passed 71/71 and the CLI typecheck passed
  • Applicable broad gate passed — npm test built successfully; its all-project parallel run was stopped after widespread unrelated five-second timeouts under local host saturation, so CI is the broad test signal
  • 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) — passed with 0 errors and 2 pre-existing Fern warnings
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Aaron Erickson aerickson@nvidia.com

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR adds repeated read-only host-directory mounts to onboarding. It validates mounts and platforms, enables managed Docker bind mounts, propagates mounts through creation and persistence, preserves them during rebuilds, reports them in status output, and documents their lifecycle.

Changes

Read-only host mount onboarding

Layer / File(s) Summary
Mount contracts and persisted validation
src/lib/state/registry/*, src/lib/state/onboard-session.ts, src/lib/onboard/host-mount/*, src/lib/onboard/resume-config.ts
Defines SandboxHostMount, validates paths and persisted entries, rejects duplicates, compares canonical mount sets, and clones mount data across state boundaries.
CLI onboarding and gateway capability
src/lib/onboard/command*, src/lib/onboard.ts, src/lib/onboard/docker-driver-gateway-*, src/lib/onboard/machine/*, src/lib/onboard/session-bootstrap*
Adds repeated --host-mount handling, platform validation, resume support, scoped bind-mount enablement, gateway drift detection, and external-gateway rejection.
Sandbox intent and driver configuration
src/lib/onboard/sandbox-create-*, src/lib/onboard/machine/handlers/sandbox.ts, src/lib/onboard/sandbox-registration*
Propagates mounts into sandbox creation, emits read-only Docker and Podman bind mounts, and stores cloned registry metadata.
Rebuilds, status, and documentation
src/lib/actions/sandbox/*, test/support/status-flow-test-harness.ts, docs/manage-sandboxes/workspace-files.mdx, docs/reference/commands.mdx
Preserves mounts during rebuilds, displays mappings in status output, and documents setup, validation, persistence, cleanup, and inspection.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested labels: area: cli, area: onboarding, area: sandbox, feature

Sequence Diagram(s)

sequenceDiagram
  participant NemoClawCLI
  participant OnboardCommand
  participant DockerDriverGateway
  participant SandboxCreatePlan
  participant SandboxRegistry
  NemoClawCLI->>OnboardCommand: submit --host-mount declarations
  OnboardCommand->>DockerDriverGateway: enable managed bind mounts
  OnboardCommand->>SandboxCreatePlan: pass validated read-only mounts
  SandboxCreatePlan->>DockerDriverGateway: generate read-only bind configuration
  SandboxCreatePlan->>SandboxRegistry: store cloned mount metadata
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.32% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The implementation covers [#8274]: validated read-only mounts, Docker gateway opt-in, persistence, rebuild handling, status reporting, tests, and documentation.
Out of Scope Changes check ✅ Passed All changes support [#8274] through host-mount validation, onboarding, lifecycle handling, gateway configuration, tests, or documentation.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding read-only host mounts to the CLI.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/read-only-host-mounts-8274

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

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

@github-code-quality

github-code-quality Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 974066d in the feat/read-only-host-... branch remains at 96%, unchanged from commit 9c2ca36 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 974066d in the feat/read-only-host-... branch remains at 81%, unchanged from commit 89e17bf in the main branch.

Show a code coverage summary of the most impacted files.
File main 89e17bf feat/read-only-host-... 974066d +/-
src/lib/onboard...-health-wait.ts 93% 82% -11%
src/lib/policy/...ne-exclusion.ts 96% 92% -4%
src/lib/state/registry.ts 90% 87% -3%
src/lib/onboard.ts 31% 30% -1%
src/lib/shields/index.ts 74% 73% -1%
src/lib/state/o...oard-session.ts 89% 88% -1%
src/lib/actions.../status-text.ts 42% 43% +1%
src/lib/onboard...on-bootstrap.ts 76% 77% +1%
src/lib/state/r...y/host-mount.ts 0% 88% +88%
src/lib/onboard...-mount/index.ts 0% 100% +100%

Updated August 05, 2026 08:30 UTC

Comment thread src/lib/onboard.ts Fixed
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@github-actions

github-actions Bot commented Aug 5, 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 terminology decisions differ; normalized E2E selections differ; severity counts match.
5 terminology differences from the second opinion

Advisory only. These are normalized differences from the primary terminology receipt.

  • read-only host mount at docs/manage-sandboxes/workspace-files.mdx:6: primary classified it as justified; the second opinion classified it as define.
  • bind-mount at docs/manage-sandboxes/workspace-files.mdx:44: selected only by the second-opinion lane as established.
  • sourceIdentity at src/lib/state/registry/host-mount.ts:65: selected only by the second-opinion lane as established.
  • host mount at docs/manage-sandboxes/workspace-files.mdx:6: selected only by the second-opinion lane as define.
  • NEMOCLAW_DOCKER_ENABLE_BIND_MOUNTS at src/lib/onboard/docker-driver-gateway-config.ts:78: selected only by the second-opinion lane as define.
2 additional E2E selections from the second opinion

Advisory only. The primary lane did not select these E2E jobs or targets.

  • sandbox-survival: The completed second-opinion lane identified E2E coverage that the primary lane omitted.
  • rebuild-hermes: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate.

4 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

  • justified — read-only host mount at docs/manage-sandboxes/workspace-files.mdx:6: Use `read-only host mount` consistently for this onboarding feature.
  • replace — host mount at docs/manage-sandboxes/workspace-files.mdx:39: Use `read-only host mount` when referring to this feature; retain unqualified `host mount` only where a broader or existing interface context is intended.
  • define — mount declaration at docs/manage-sandboxes/workspace-files.mdx:45: Define `mount declaration` at first use, or use `read-only host mount` consistently for the persisted configuration.
  • established — Docker-driver gateway at docs/manage-sandboxes/workspace-files.mdx:18: Keep the established driver qualifier and the `NemoClaw-managed` ownership qualifier where host-mount support depends on them.

E2E guidance

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

Recommended E2E: cloud-onboard, onboard-repair, onboard-resume, rebuild-openclaw, state-backup-restore, ubuntu-repo-docker-post-reboot-recovery

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.

Comment thread src/lib/onboard.ts Fixed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/onboard/command.test.ts`:
- Around line 113-125: The test around resolve must actually pass two distinct
valid host-mount values through the public resolver boundary. Create two
temporary source directories, provide both mappings in the Linux input, and
assert both read-only hostMounts are returned; retain the Darwin rejection using
that same two-mount input and clean up both directories.

In `@src/lib/onboard/docker-driver-gateway-env.ts`:
- Line 227: Update the gateway runtime drift validation to require
NEMOCLAW_DOCKER_ENABLE_BIND_MOUNTS to be absent or disabled when the current
onboarding has no host mounts, so a reused mount-enabled process is rejected.
Add a regression test covering reuse from mount-enabled to mount-disabled
configuration, while preserving the enabled case in
docker-driver-gateway-env.ts.

In `@src/lib/onboard/machine/handlers/sandbox.ts`:
- Line 1204: Update resolveResumeDecision() and the reuseSandbox() flow to
normalize and compare this.options.hostMounts against the persisted host-mount
declaration before returning reuse. Ensure matching mounts continue reuse, while
added, removed, or changed mounts trigger recreate or fail closed, and cover all
cases in resume tests so reuse converges with buildSandboxCreateIntent().

In `@src/lib/onboard/sandbox-create-intent.ts`:
- Line 147: Preserve checkpoint fingerprint compatibility in
resolveSandboxCreateIntent by omitting hostMounts when no mounts are requested,
or by canonicalizing absent and empty hostMounts in
SandboxStateFlow.currentSandboxCreateFingerprint and
assertCheckpointCreateInputsStillMatch. Add a resume test using a pre-mount
checkpoint to verify interrupted onboarding does not report input drift.

In `@src/lib/state/onboard-session.ts`:
- Around line 470-486: Update persisted host-mount handling around the
hostMounts normalization and prepareResumeSession so invalid declarations are
rejected rather than silently filtered out. Preserve validation failures through
persisted-state recovery and make resume reject sessions containing malformed,
missing, or symlinked mounts when no replacement mounts are supplied; ensure
replacement mounts still follow the normal validation path. Add coverage for
each invalid persisted-mount case during resume.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 05ba0d39-62c4-4226-99b3-df0a184aef4e

📥 Commits

Reviewing files that changed from the base of the PR and between 2b18817 and 6a8b0fd.

📒 Files selected for processing (39)
  • docs/manage-sandboxes/workspace-files.mdx
  • docs/reference/commands.mdx
  • src/lib/actions/sandbox/rebuild-gpu-opt-out.test.ts
  • src/lib/actions/sandbox/rebuild-gpu-opt-out.ts
  • src/lib/actions/sandbox/status-flow.test.ts
  • src/lib/actions/sandbox/status-snapshot.ts
  • src/lib/actions/sandbox/status-text.ts
  • src/lib/actions/sandbox/status.test.ts
  • src/lib/onboard.ts
  • src/lib/onboard/command-support.ts
  • src/lib/onboard/command.test.ts
  • src/lib/onboard/command.ts
  • src/lib/onboard/docker-driver-gateway-config.ts
  • src/lib/onboard/docker-driver-gateway-env.ts
  • src/lib/onboard/docker-driver-gateway-launch.test.ts
  • src/lib/onboard/docker-driver-gateway-runtime.ts
  • src/lib/onboard/host-mount/host-mount.test.ts
  • src/lib/onboard/host-mount/index.ts
  • src/lib/onboard/machine/core-flow-phases.ts
  • src/lib/onboard/machine/handlers/gateway.ts
  • src/lib/onboard/machine/handlers/sandbox.ts
  • src/lib/onboard/machine/initial-flow-phases.ts
  • src/lib/onboard/sandbox-create-intent-resolution.ts
  • src/lib/onboard/sandbox-create-intent-types.ts
  • src/lib/onboard/sandbox-create-intent.ts
  • src/lib/onboard/sandbox-create-plan-materialization.ts
  • src/lib/onboard/sandbox-create-plan.test.ts
  • src/lib/onboard/sandbox-create-plan.ts
  • src/lib/onboard/sandbox-registration.test.ts
  • src/lib/onboard/sandbox-registration.ts
  • src/lib/onboard/session-bootstrap.test.ts
  • src/lib/onboard/session-bootstrap.ts
  • src/lib/onboard/types.ts
  • src/lib/state/onboard-session.test.ts
  • src/lib/state/onboard-session.ts
  • src/lib/state/registry.ts
  • src/lib/state/registry/host-mount.ts
  • src/lib/state/registry/types.ts
  • test/support/status-flow-test-harness.ts

Comment thread src/lib/onboard/command.test.ts
Comment thread src/lib/onboard/docker-driver-gateway-env.ts
hermesToolGateways,
extraProviders,
staleExtraProviders,
hostMounts: this.options.hostMounts,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Revalidate host mounts before sandbox reuse.

hostMounts reaches buildSandboxCreateIntent() only after resolveResumeDecision() selects create or recreate. When the decision is reuse, reuseSandbox() does not compare this.options.hostMounts with the persisted declaration. A new --host-mount can be silently ignored, and a changed mount declaration is not revalidated.

Compare normalized host mounts before returning reuse. Recreate or fail closed when they differ. Include matching, added, removed, and changed mount cases in resume tests.

As per path instructions, resume and rebuild flows must revalidate durable state and converge with the authoritative creation path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/onboard/machine/handlers/sandbox.ts` at line 1204, Update
resolveResumeDecision() and the reuseSandbox() flow to normalize and compare
this.options.hostMounts against the persisted host-mount declaration before
returning reuse. Ensure matching mounts continue reuse, while added, removed, or
changed mounts trigger recreate or fail closed, and cover all cases in resume
tests so reuse converges with buildSandboxCreateIntent().

Source: Path instructions

Comment thread src/lib/onboard/sandbox-create-intent.ts
Comment thread src/lib/state/onboard-session.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/onboard/gateway-health-wait.ts`:
- Around line 10-29: Remove the duplicate getGatewayHealthWaitConfig
implementation from onboard.ts and import the authoritative function from
gateway-health-wait.ts, preserving a re-export only if the existing public API
requires it. Update all onboarding call sites to use this imported symbol, and
add public-boundary coverage proving fresh, resumed, repair, and failure flows
reach the module implementation.

In `@src/lib/onboard/host-mount/index.ts`:
- Around line 13-35: Update the onboarding cleanup flow around
finishOnboardTrace() to wrap trace-finalization and related cleanup in a nested
try/finally, ensuring hostMountScope.restore() is always executed even when
trace writing fails. Preserve the existing restoration behavior and use the
scope returned by beginHostMountScope().
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ad63b8df-3f5d-4f3a-bc5a-7d23fb1175a0

📥 Commits

Reviewing files that changed from the base of the PR and between 6a8b0fd and e01efa7.

📒 Files selected for processing (5)
  • src/lib/onboard.ts
  • src/lib/onboard/gateway-health-wait.ts
  • src/lib/onboard/host-mount/host-mount.test.ts
  • src/lib/onboard/host-mount/index.ts
  • src/lib/onboard/session-bootstrap.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/lib/onboard/session-bootstrap.ts
  • src/lib/onboard/host-mount/host-mount.test.ts
  • src/lib/onboard.ts

Comment on lines +10 to +29
export function getGatewayHealthWaitConfig(_startStatus = 0, containerState = "") {
const isArm64 = process.arch === "arm64";
const standardCount = envInt("NEMOCLAW_HEALTH_POLL_COUNT", isArm64 ? 30 : 12);
const standardInterval = envInt("NEMOCLAW_HEALTH_POLL_INTERVAL", isArm64 ? 10 : 5);
const extendedCount = envInt("NEMOCLAW_GATEWAY_START_POLL_COUNT", standardCount);
const extendedInterval = envInt("NEMOCLAW_GATEWAY_START_POLL_INTERVAL", standardInterval);
const normalizedState = String(containerState || "")
.trim()
.toLowerCase();
const normalizedContainerState = normalizedState || "missing";
const useExtendedWait = normalizedContainerState !== "missing";

return {
count: useExtendedWait ? extendedCount : standardCount,
interval: useExtendedWait ? extendedInterval : standardInterval,
extended: useExtendedWait,
containerState: normalizedContainerState,
};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Remove the superseded health-wait implementation.

The supplied graph context shows getGatewayHealthWaitConfig also defined in src/lib/onboard.ts, Lines 10-28. The onboarding call sites at Lines 1793-1796 and 2155-2158 therefore can resolve to the old local implementation instead of this module.

Keep src/lib/onboard/gateway-health-wait.ts authoritative. Import it from src/lib/onboard.ts and delete the old implementation. If the legacy export must remain, make it a re-export. Add a public-boundary test that proves onboarding reaches this module.

As per path instructions, a replacement path must be authoritative, and fresh, resumed, repair, and failure flows must prove the cutover.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/onboard/gateway-health-wait.ts` around lines 10 - 29, Remove the
duplicate getGatewayHealthWaitConfig implementation from onboard.ts and import
the authoritative function from gateway-health-wait.ts, preserving a re-export
only if the existing public API requires it. Update all onboarding call sites to
use this imported symbol, and add public-boundary coverage proving fresh,
resumed, repair, and failure flows reach the module implementation.

Source: Path instructions

Comment thread src/lib/onboard/host-mount/index.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/state/registry/host-mount.ts`:
- Around line 112-119: Update both canonicalHostMounts implementations to use a
locale-independent binary total-order comparator for the combined source and
target keys instead of localeCompare(). Ensure distinct Unicode paths never
compare equal, so equivalent mount sets serialize identically regardless of
declaration order.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 40dd5930-1526-418f-b1a0-c1460fd0edf8

📥 Commits

Reviewing files that changed from the base of the PR and between e01efa7 and 1668693.

📒 Files selected for processing (17)
  • src/lib/onboard/command.test.ts
  • src/lib/onboard/docker-driver-gateway-runtime.test.ts
  • src/lib/onboard/docker-driver-gateway-runtime.ts
  • src/lib/onboard/host-mount/host-mount.test.ts
  • src/lib/onboard/machine/handlers/gateway.test.ts
  • src/lib/onboard/machine/handlers/sandbox-resume.test.ts
  • src/lib/onboard/machine/handlers/sandbox-resume.ts
  • src/lib/onboard/machine/handlers/sandbox.ts
  • src/lib/onboard/resume-config.test.ts
  • src/lib/onboard/resume-config.ts
  • src/lib/onboard/sandbox-create-intent.ts
  • src/lib/onboard/sandbox-create-plan.test.ts
  • src/lib/onboard/session-bootstrap.test.ts
  • src/lib/onboard/session-bootstrap.ts
  • src/lib/state/onboard-session.test.ts
  • src/lib/state/onboard-session.ts
  • src/lib/state/registry/host-mount.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • src/lib/onboard/sandbox-create-plan.test.ts
  • src/lib/onboard/sandbox-create-intent.ts
  • src/lib/onboard/host-mount/host-mount.test.ts
  • src/lib/onboard/command.test.ts
  • src/lib/onboard/session-bootstrap.ts
  • src/lib/onboard/machine/handlers/sandbox.ts
  • src/lib/onboard/session-bootstrap.test.ts

Comment on lines +112 to +119
function canonicalHostMounts(mounts: readonly SandboxHostMount[]): string {
return JSON.stringify(
mounts
.map(({ source, target }) => ({ source, target, readOnly: true as const }))
.sort((left, right) =>
`${left.source}\0${left.target}`.localeCompare(`${right.source}\0${right.target}`),
),
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

node - <<'NODE'
const first = "\u00F1";
const second = "n\u0303";
console.log({
  localeCompare: first.localeCompare(second),
  binaryCompare: first < second ? -1 : first > second ? 1 : 0,
});
NODE

Repository: NVIDIA/NemoClaw

Length of output: 192


🏁 Script executed:

set -euo pipefail

file="$(fd -t f 'host-mount\.ts$' . | head -n 1)"
printf '%s\n' "== $file =="
cat -n "$file" | sed -n '90,145p'

printf '%s\n' '== equality callers and tests =='
rg -n -C 3 'canonicalHostMounts|persistedSandboxHostMountsEqual|SandboxHostMount' --glob '!node_modules' --glob '!dist' --glob '!build' . | head -n 240

printf '%s\n' '== deterministic probe =='
node - <<'NODE'
const inputs = [
  ["\u00F1", "n\u0303"],
  ["A", "a"],
  ["\u212B", "\u00C5"],
];
for (const [left, right] of inputs) {
  const locale = left.localeCompare(right);
  const binary = left < right ? -1 : left > right ? 1 : 0;
  console.log(JSON.stringify({left, right, locale, binary}));
}
NODE

Repository: NVIDIA/NemoClaw

Length of output: 19865


🏁 Script executed:

set -euo pipefail

file="$(fd -t f 'host-mount\.ts$' . | head -n 1)"
cat -n "$file" | sed -n '100,140p'

rg -n -C 3 'canonicalHostMounts|persistedSandboxHostMountsEqual' . \
  --glob '!node_modules' --glob '!dist' --glob '!build'

node - <<'NODE'
const first = "\u00F1";
const second = "n\u0303";
console.log({
  localeCompare: first.localeCompare(second),
  binaryCompare: first < second ? -1 : first > second ? 1 : 0,
});
NODE

Repository: NVIDIA/NemoClaw

Length of output: 7306


🏁 Script executed:

set -euo pipefail

printf '%s\n' '== host-mount validation and tests =='
cat -n src/lib/state/registry/host-mount.ts | sed -n '1,90p'
cat -n src/lib/onboard/host-mount/host-mount.test.ts | sed -n '88,112p'

printf '%s\n' '== second canonicalization path and resume decision =='
cat -n src/lib/onboard/resume-config.ts | sed -n '20,50p'
cat -n src/lib/onboard/resume-config.ts | sed -n '165,200p'
cat -n src/lib/onboard/machine/handlers/sandbox-resume.ts | sed -n '25,45p'
rg -n -C 4 'hasHostMountConfigDrift|resolveResume|resume.*config|host mounts' src/lib/onboard --glob '*.ts' | head -n 260

printf '%s\n' '== exact ordering failure model =='
node - <<'NODE'
function canonical(mounts) {
  return JSON.stringify(
    mounts
      .map(({source, target}) => ({source, target, readOnly: true}))
      .sort((left, right) =>
        `${left.source}\0${left.target}`.localeCompare(`${right.source}\0${right.target}`),
      ),
  );
}
const first = {source: "/tmp/\u00F1", target: "/sandbox/a", readOnly: true};
const second = {source: "/tmp/n\u0303", target: "/sandbox/b", readOnly: true};
const left = [first, second];
const right = [second, first];
console.log({
  left: canonical(left),
  right: canonical(right),
  equal: canonical(left) === canonical(right),
});
NODE

Repository: NVIDIA/NemoClaw

Length of output: 30002


🏁 Script executed:

set -euo pipefail

cat -n src/lib/state/registry/host-mount.ts | sed -n '1,90p'
cat -n src/lib/onboard/host-mount/host-mount.test.ts | sed -n '88,112p'
cat -n src/lib/onboard/resume-config.ts | sed -n '20,50p;165,200p'
cat -n src/lib/onboard/machine/handlers/sandbox-resume.ts | sed -n '25,45p'

node - <<'NODE'
function canonical(mounts) {
  return JSON.stringify(
    mounts
      .map(({source, target}) => ({source, target, readOnly: true}))
      .sort((a, b) =>
        `${a.source}\0${a.target}`.localeCompare(`${b.source}\0${b.target}`),
      ),
  );
}
const first = {source: "/tmp/\u00F1", target: "/sandbox/a", readOnly: true};
const second = {source: "/tmp/n\u0303", target: "/sandbox/b", readOnly: true};
const left = [first, second];
const right = [second, first];
console.log(canonical(left) === canonical(right));
NODE

Repository: NVIDIA/NemoClaw

Length of output: 8995


🏁 Script executed:

set -euo pipefail

node - <<'NODE'
const cases = [
  [
    {source: "/tmp/\u00F1", target: "/sandbox/\u00E9"},
    {source: "/tmp/n\u0303", target: "/sandbox/e\u0301"},
  ],
  [
    {source: "/tmp/\u00E5", target: "/sandbox/\u00E9"},
    {source: "/tmp/a\u030A", target: "/sandbox/e\u0301"},
  ],
  [
    {source: "/tmp/\u00F1", target: "/sandbox/a"},
    {source: "/tmp/n\u0303", target: "/sandbox/a\u0301"},
  ],
];

function key(mount) {
  return `${mount.source}\0${mount.target}`;
}
function canonical(mounts) {
  return JSON.stringify(
    mounts
      .map(({source, target}) => ({source, target, readOnly: true}))
      .sort((left, right) => key(left).localeCompare(key(right))),
  );
}

for (const mounts of cases) {
  const comparison = key(mounts[0]).localeCompare(key(mounts[1]));
  console.log(JSON.stringify({
    keys: mounts.map(key),
    comparison,
    swappedCanonicalEqual: canonical(mounts) === canonical([...mounts].reverse()),
    canonical: canonical(mounts),
  }));
}
NODE

printf '%s\n' '== all localeCompare call sites =='
rg -n -C 2 '\.localeCompare\(' src/lib/state/registry/host-mount.ts src/lib/onboard/resume-config.ts

Repository: NVIDIA/NemoClaw

Length of output: 1796


🏁 Script executed:

set -euo pipefail

node - <<'NODE'
const mounts = [
  {source: "/tmp/\u00F1", target: "/sandbox/\u00E9"},
  {source: "/tmp/n\u0303", target: "/sandbox/e\u0301"},
];

const key = ({source, target}) => `${source}\0${target}`;
const canonical = (items) =>
  JSON.stringify(
    items
      .map(({source, target}) => ({source, target, readOnly: true}))
      .sort((a, b) => key(a).localeCompare(key(b))),
  );

console.log({
  comparison: key(mounts[0]).localeCompare(key(mounts[1])),
  swappedCanonicalEqual: canonical(mounts) === canonical([...mounts].reverse()),
  canonical: canonical(mounts),
});
NODE

rg -n -C 2 '\.localeCompare\(' \
  src/lib/state/registry/host-mount.ts \
  src/lib/onboard/resume-config.ts

Repository: NVIDIA/NemoClaw

Length of output: 1163


Use a locale-independent total order for host-mount keys.

localeCompare() can return 0 for distinct Unicode paths. Stable sorting can preserve different declaration orders, so the same mount set can produce different JSON and trigger false sandbox recreation. Apply the binary comparison in both canonicalHostMounts implementations: src/lib/state/registry/host-mount.ts and src/lib/onboard/resume-config.ts.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/state/registry/host-mount.ts` around lines 112 - 119, Update both
canonicalHostMounts implementations to use a locale-independent binary
total-order comparator for the combined source and target keys instead of
localeCompare(). Ensure distinct Unicode paths never compare equal, so
equivalent mount sets serialize identically regardless of declaration order.

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@ericksoa

ericksoa commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the remaining automated review findings on the current head 19c269a5f433c910752f6e5b7647bea43afd6cc2:

  • The final sandbox-create boundary now revalidates every host mount's no-symlink path and captured device/inode after prebuild and immediately before creation. The regression test replaces a previously validated directory with a symlink and verifies creation is rejected.
  • Mount canonicalization already uses a locale-independent binary total order in both implementations (commit dfadc4ee7), so the CodeRabbit comment from the earlier reviewed range is resolved.
  • NemoClaw pins OpenShell v0.0.85. That release contains the upstream Docker bind-mount gate and read-only rendering tests from OpenShell PR #2092: bind mounts are rejected unless explicitly enabled, and enabled binds render with :ro by default.

ericksoa and others added 2 commits August 4, 2026 23:43
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Aug 5, 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.

@ericksoa ericksoa self-assigned this Aug 5, 2026
@ericksoa ericksoa added v0.0.103 Release target release-target feature PR adds or expands user-visible functionality labels Aug 5, 2026
@ericksoa ericksoa added area: cli Command line interface, flags, terminal UX, or output area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery area: security Security controls, permissions, secrets, or hardening platform: ubuntu Affects Ubuntu Linux environments platform: linux Affects non-Ubuntu Linux environments platform: wsl Affects Windows Subsystem for Linux platform: container Affects Docker, containerd, Podman, or images labels Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli Command line interface, flags, terminal UX, or output area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery area: security Security controls, permissions, secrets, or hardening feature PR adds or expands user-visible functionality platform: container Affects Docker, containerd, Podman, or images platform: linux Affects non-Ubuntu Linux environments platform: ubuntu Affects Ubuntu Linux environments platform: wsl Affects Windows Subsystem for Linux release-target v0.0.103 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add read-only host bind mounts to onboarding

2 participants