fix(share): pre-flight remote path before invoking sshfs (#3414)#3647
Merged
cv merged 2 commits intoMay 16, 2026
Merged
Conversation
Closes NVIDIA#3414. `nemoclaw <sandbox> share mount /missing/path` invokes `sshfs` directly without first checking that the remote path exists inside the sandbox. `sshfs` exits non-zero with empty stderr on a missing remote path, and the existing "SSHFS mount failed." line that `runShareMount` emits leaves the user with no actionable error. Add a `checkSandboxPathExists()` dep that calls `openshell sandbox exec <name> -- test -e <remotePath>` with a bounded probe timeout and returns a boolean. `runShareMount` calls it after the sandbox-live check (so we know exec is reachable) but before the filesystem-writability check (so we can avoid creating an empty mount directory when the source path is missing). On failure the user sees a three-line message naming the missing path, explaining the exec probe either could not run or returned non-zero, and pointing at `openshell sandbox exec ls <parent>` as a follow-up. Tests: `test/share-command-remote-path.test.ts` covers 6 paths: the preflight passing for an existing path, exit on missing path, exit on exec timeout, exit on exec error, mkdir cleanup on missing path, and ordering after `ensureLive`. 6/6 pass; existing 6/6 share-command.test.ts pass unchanged after wiring `checkSandboxPathExists` into the test ShareCommandDeps factory. Signed-off-by: latenighthackathon <support@latenighthackathon.com> Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR implements a pre-flight check for the remote sandbox path before invoking sshfs in the ChangesPre-flight Path Existence Check
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
9 tasks
cv
approved these changes
May 16, 2026
12 tasks
ericksoa
pushed a commit
that referenced
this pull request
May 18, 2026
## Summary Updates the NemoClaw documentation for the v0.0.45 release by summarizing the user-facing changes merged since v0.0.44 and bumping the docs version metadata. Refreshes generated user skills so agent-facing references match the source docs. ## Changes - Added v0.0.45 release notes covering onboarding recovery, local inference, channel cleanup, share mount diagnostics, uninstall cleanup, and security redaction updates. - Updated command and troubleshooting docs for sandbox name limits, GPU gateway reuse, DNS preflight behavior, channel removal cleanup, and share mount path validation. - Bumped docs version metadata to 0.0.45 and regenerated NemoClaw user skills from the docs. - Source summary: #3672 -> `docs/reference/commands.md`: documented channel removal detaching bridge providers and un-applying channel policy presets. - Source summary: #3678 -> `docs/about/release-notes.md`: documented Ollama streamed usage accounting in the release notes. - Source summary: #3670 -> `docs/reference/commands.md`, `docs/reference/troubleshooting.md`: documented safe GPU gateway replacement behavior. - Source summary: #3664 -> `docs/about/release-notes.md`: documented blueprint permission normalization in the release notes. - Source summary: #3181 -> `docs/reference/troubleshooting.md`: documented GPU toolkit guidance when host drivers work but passthrough is disabled. - Source summary: #3554 -> `docs/about/release-notes.md`: documented host `openshell-gateway` cleanup during uninstall. - Source summary: #3651 -> `docs/reference/troubleshooting.md`: documented the uncached `.invalid` DNS preflight probe. - Source summary: #3643 -> `docs/reference/commands.md`: included existing `NEMOCLAW_PROVIDER` interactive-mode behavior in generated docs. - Source summary: #3647 -> `docs/reference/commands.md`: documented remote sandbox path verification for `share mount`. - Source summary: #3646 -> `docs/reference/commands.md`: included existing local writable mount target guidance in generated docs. - Source summary: #3642 -> `docs/inference/use-local-inference.md`, `docs/reference/commands.md`: documented managed-vLLM model override and gated-model token checks. - Source summary: #3639 -> `docs/reference/commands.md`: documented the 63-character sandbox name limit. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Verification - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [x] `make docs` builds without warnings (doc changes only) - [x] 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) Commit hooks passed for the staged files. A standalone `npx prek run --all-files` attempt was blocked by sandbox access to `/Users/miyoungc/.cache/prek/prek.log`, so that checkbox is left unchecked. --- <!-- DCO sign-off required by CI. Run: git config user.name && git config user.email --> Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Enhanced CLI command reference documentation with clearer guidance on onboarding, GPU passthrough, inference configuration, channel removal, and shared mounts. * Improved troubleshooting sections with better DNS resolution and GPU passthrough remediation steps. * Added documentation for overriding managed vLLM model selection. * Updated release notes for v0.0.45 reflecting infrastructure and workflow improvements. * **Version Bump** * Released v0.0.45. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/3755?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 tasks
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #3414.
nemoclaw <sandbox> share mountexits non-zero with no stderr when the remote sandbox path does not exist (e.g. a typo). sshfs is silent in this case, so the CLI surfaced only the bareSSHFS mount failed.line and the user had no signal that the source path was the problem. Reported by @SeseMueller in #3235 as a follow-up to the writable-fs error fix.Related Issue
Closes #3414
Changes
checkSandboxPathExistsdep onShareCommandDeps(src/lib/share-command-deps.ts). Default impl runsopenshell sandbox exec <name> -- test -e <remotePath>with the sameOPENSHELL_PROBE_TIMEOUT_MSbudget as the existinggetSshConfigprobe, and treats a zero exit status as "path exists" (everything else is missing or unreachable).assertSandboxPathExistsOrExit(deps, sandboxName, remotePath)insrc/lib/share-command.ts. Consults the dep, emits a structured error when the probe returns non-zero, and exits 1. The error is phrased as a verification failure rather than a definitive "path is missing" claim, because the probe also returns false on transient exec failures (sandbox just restarted, gRPC hiccup), and a wrong-cause message wastes the user's time. Extracted so the behavior is testable without driving the full sshfs lifecycle.runShareMountcalls the helper afterensureLiveand beforegetSshConfig, so a non-verifiable remote path fails fast with:No SSH config is written, no sshfs binary is invoked, no local mount directory is created when the pre-flight fails.
Type of Change
Verification
npx prek run --all-filespasses for the staged files (includes the source-shape test budget gate).test/share-command-remote-path.test.tscover the happy path (no exit, no stderr), the missing-path exit (verifies error contents and that the dep was called with the right args), and thecliNamebranding hint (so thenemohermesalias surfacesnemohermes hermes connectrather thannemoclaw hermes connect).npx vitest run test/share-command-remote-path.test.ts- 3/3 pass.npx tsx scripts/find-source-shape-tests.ts- 0 source-shape cases (no source-text assertions).npm run build:cliclean.Signed-off-by: latenighthackathon latenighthackathon@users.noreply.github.com
Summary by CodeRabbit
New Features
share mountto verify the remote path exists in the sandbox before attempting to connect.Bug Fixes