fix(installer): skip unreachable running sandboxes in pre-upgrade backup - #6199
Merged
Conversation
Classify a running sandbox whose SSH endpoint is unreachable as such, and let NEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP=1 skip it so the pre-upgrade backup no longer loops the installer. Onboarding then recovers it from its latest validated backup. Fixes #6188 Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds SSH-unreachable sandbox handling in backup state, an opt-in skip path for ChangesUnreachable sandbox backup skip
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant install.sh
participant backupAll
participant backupSandboxState
install.sh->>backupAll: run pre-upgrade backup
loop running sandboxes
backupAll->>backupSandboxState: backupSandboxState()
backupSandboxState-->>backupAll: backup result
alt unreachable and skip flag set
backupAll->>backupAll: skip sandbox
else unreachable and skip flag unset
backupAll->>backupAll: record unreachable failure
end
end
backupAll-->>install.sh: success or failure guidance
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
laitingsheng
added
NV QA
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
Contributor
PR Review Advisor (Nemotron Ultra) — BlockedMerge posture: Do not merge until addressed Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
Contributor
PR Review Advisor — BlockedMerge posture: Do not merge until addressed Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/lib/state/sandbox.ts (1)
124-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument when
unreachableis set.Sibling optional fields (
manifest,error) have comments explaining when they're populated;unreachablehas none. Given it's only set on one of several SSH-failure paths (see the completeness comment above), a short comment clarifying its scope would help avoid future gaps.🤖 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/sandbox.ts` at line 124, Add a brief doc comment for the unreachable field in the Sandbox state type so its population scope is clear, matching the style used for manifest and error. Update the Sandbox interface near unreachable to note that it is only set on specific SSH failure paths and not for all connectivity errors, using the surrounding completeness/comment context to place it correctly.docs/reference/commands.mdx (1)
1762-1762: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit into one sentence per line.
This line packs two sentences together. As per coding guidelines,
docs/**/*.{md,mdx}files should "Keep one sentence per line in Markdown and MDX source files."📝 Proposed fix
-A running sandbox whose in-sandbox SSH endpoint does not answer fails its backup and aborts the run. Set `NEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP=1` to skip such sandboxes and continue the upgrade; they are recovered from their latest validated backup during onboarding. +A running sandbox whose in-sandbox SSH endpoint does not answer fails its backup and aborts the run. +Set `NEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP=1` to skip such sandboxes and continue the upgrade. +They are recovered from their latest validated backup during onboarding.🤖 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 `@docs/reference/commands.mdx` at line 1762, The markdown/MDX text in the referenced command docs currently combines two sentences on one line, which violates the one-sentence-per-line guideline. Split the content into separate lines so each sentence stands alone, keeping the same wording and updating the relevant prose near the sandbox backup note in the commands reference.Source: Coding guidelines
src/lib/actions/maintenance.ts (1)
34-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the backup-skip decision out of
actions.
shouldSkipUnreachableSandboxBackupis a pure decision function with no I/O — exactly the kind of helper the layering doc calls out forstate/domain, notactions. As per path instructions,src/lib/README.mdstates: "When adding behavior like SSH-unreachable classification and backup-skip decisions for the installer, place orchestration/user-facing flow inactions(e.g.,backupAll), and keep classification helpers testable/pure where possible instate/domain(e.g.,isSshTransportFailure)."Consider relocating this predicate alongside
isSshTransportFailure(or another domain module) and importing it intobackupAll, keepingactions/maintenance.tsfocused on orchestration.#!/bin/bash # Survey existing domain modules to find the right home for this predicate fd . src/lib/domain🤖 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/actions/maintenance.ts` around lines 34 - 36, The pure backup-skip predicate is in the wrong layer and should be moved out of actions. Relocate shouldSkipUnreachableSandboxBackup from maintenance.ts into a domain/state helper alongside isSshTransportFailure (or another appropriate testable module), then import and use it from backupAll so actions/maintenance.ts stays orchestration-only. Keep the helper pure and preserve its current env-based decision logic in the new home.Source: Path instructions
🤖 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/sandbox.ts`:
- Around line 1022-1026: Propagate the unreachable/skippable state through SSH
failure handling so later audit and SSH+tar paths don’t keep retrying after the
initial dir probe marks the sandbox unreachable. Update isSshTransportFailure
and the related audit/tar failure handling in sandbox.ts to carry forward the
unreachable flag from the probe result, and use that in the SSH retry/skip
decision instead of returning a plain failure.
---
Nitpick comments:
In `@docs/reference/commands.mdx`:
- Line 1762: The markdown/MDX text in the referenced command docs currently
combines two sentences on one line, which violates the one-sentence-per-line
guideline. Split the content into separate lines so each sentence stands alone,
keeping the same wording and updating the relevant prose near the sandbox backup
note in the commands reference.
In `@src/lib/actions/maintenance.ts`:
- Around line 34-36: The pure backup-skip predicate is in the wrong layer and
should be moved out of actions. Relocate shouldSkipUnreachableSandboxBackup from
maintenance.ts into a domain/state helper alongside isSshTransportFailure (or
another appropriate testable module), then import and use it from backupAll so
actions/maintenance.ts stays orchestration-only. Keep the helper pure and
preserve its current env-based decision logic in the new home.
In `@src/lib/state/sandbox.ts`:
- Line 124: Add a brief doc comment for the unreachable field in the Sandbox
state type so its population scope is clear, matching the style used for
manifest and error. Update the Sandbox interface near unreachable to note that
it is only set on specific SSH failure paths and not for all connectivity
errors, using the surrounding completeness/comment context to place it
correctly.
🪄 Autofix (Beta)
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: 8f7cd8eb-38a8-4bcb-85c8-82631ee48981
📒 Files selected for processing (6)
docs/reference/commands.mdxscripts/install.shsrc/lib/actions/maintenance.test.tssrc/lib/actions/maintenance.tssrc/lib/state/sandbox.test.tssrc/lib/state/sandbox.ts
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…-skip-unreachable
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-6199.docs.buildwithfern.com/nemoclaw |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/install-openshell-upgrade-prompt.test.ts (1)
221-234: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSequence assertions don't actually check order.
This test is meant to verify the ordered sequence
old:backup-all→prepare-current→current:backup-all, but.toContainon the split array only checks membership, so it would still pass even if the calls happened out of order (e.g. if a future regression calledprepare-currentbefore the old backup attempt).🧪 Proposed fix to assert actual order
- expect(cliLog.split(/\r?\n/)).toContain("old:backup-all"); - expect(cliLog.split(/\r?\n/)).toContain("prepare-current"); - expect(cliLog.split(/\r?\n/)).toContain("current:backup-all"); + const cliLogLines = cliLog.split(/\r?\n/).filter(Boolean); + expect(cliLogLines.indexOf("old:backup-all")).toBeGreaterThanOrEqual(0); + expect(cliLogLines.indexOf("old:backup-all")).toBeLessThan(cliLogLines.indexOf("prepare-current")); + expect(cliLogLines.indexOf("prepare-current")).toBeLessThan(cliLogLines.indexOf("current:backup-all"));As per path instructions, "Flag copied production algorithms, broad mocks that bypass the behavior under test, and conditionals that make a test pass without exercising its claim."
🤖 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 `@test/install-openshell-upgrade-prompt.test.ts` around lines 221 - 234, The sequence check in the retry test does not verify the call order, so a regression could still pass even if the backup steps happen out of order. Update the assertions in the test that uses runPreinstallUpgradeGuard, cliLog, and the expected old:backup-all → prepare-current → current:backup-all flow so it compares indices or otherwise enforces ordering, not just membership. Keep the same symbols and log markers, but make the test fail unless the commands occur in the intended sequence.Source: Path instructions
🤖 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 `@docs/reference/commands-nemohermes.mdx`:
- Around line 1402-1403: Split the text in the affected MDX paragraph so each
sentence is on its own line, keeping the wording the same. Update the sentence
in the command reference content near the sandbox backup note so the statement
about skipping unreachable sandboxes and the statement about recovery from the
latest validated backup are separated. Preserve the surrounding markdown
structure and ensure the one-sentence-per-line guideline is followed.
---
Nitpick comments:
In `@test/install-openshell-upgrade-prompt.test.ts`:
- Around line 221-234: The sequence check in the retry test does not verify the
call order, so a regression could still pass even if the backup steps happen out
of order. Update the assertions in the test that uses runPreinstallUpgradeGuard,
cliLog, and the expected old:backup-all → prepare-current → current:backup-all
flow so it compares indices or otherwise enforces ordering, not just membership.
Keep the same symbols and log markers, but make the test fail unless the
commands occur in the intended sequence.
🪄 Autofix (Beta)
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: af67cd1a-90cc-4f75-a403-62777823ab4b
📒 Files selected for processing (4)
docs/reference/commands-nemohermes.mdxdocs/reference/commands.mdxscripts/install.shtest/install-openshell-upgrade-prompt.test.ts
✅ Files skipped from review due to trivial changes (1)
- docs/reference/commands.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/install.sh
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/state/sandbox.ts (1)
1128-1141: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftMark SSH-config failures as unreachable in
src/lib/state/sandbox.ts:1129-1141
getSshConfig()collapses transport failures intonull, so this return loses the SSH-unreachable signal.backupAll()won’t honorNEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUPfor sandboxes that die at the ssh-config step.🤖 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/sandbox.ts` around lines 1128 - 1141, The SSH config lookup path in backupAll currently treats getSshConfig(sandboxName) returning null as a generic failure, which drops the unreachable signal. Update the SSH+tar download branch to preserve this as an SSH-unreachable case so the existing skip-unreachable behavior still applies, using the backupAll flow and the getSshConfig result handling in src/lib/state/sandbox.ts.
🤖 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.
Outside diff comments:
In `@src/lib/state/sandbox.ts`:
- Around line 1128-1141: The SSH config lookup path in backupAll currently
treats getSshConfig(sandboxName) returning null as a generic failure, which
drops the unreachable signal. Update the SSH+tar download branch to preserve
this as an SSH-unreachable case so the existing skip-unreachable behavior still
applies, using the backupAll flow and the getSshConfig result handling in
src/lib/state/sandbox.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3957ccbc-20f0-4a81-bb5a-2ea52f3b155f
📒 Files selected for processing (4)
docs/reference/commands-nemohermes.mdxdocs/reference/commands.mdxsrc/lib/state/sandbox.tstest/install-openshell-upgrade-prompt.test.ts
✅ Files skipped from review due to trivial changes (2)
- docs/reference/commands.mdx
- docs/reference/commands-nemohermes.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- test/install-openshell-upgrade-prompt.test.ts
…docs (#6188) Address advisor findings on #6199: - PRA-5/PRA-8: isSshTransportFailure now checks result.signal for SIGHUP and SIGPIPE explicitly (matches connect.ts). Signal-killed SSH probes (common when the sandbox gateway dies mid-connection) are classified as transport-level, so the skip flag activates. - PRA-2: warning + guidance messages at maintenance.ts:134 and 158, and the installer error at scripts/install.sh, now explicitly state that any uncommitted state since the last successful backup will be lost. - PRA-3/PRA-4: docs at commands.mdx and commands-nemohermes.mdx call out that only the exact value '1' is accepted (true/yes/0 are not). - CodeRabbit minor: split the two-clause sentence per line at both mdx sites while updating. Adds two sandbox.test.ts cases for the new signal handling. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
… paths (#6188) Address advisor PRA-1 / PRA-7 and CodeRabbit Major #2: - backupStateFile now returns { outcome, unreachable } instead of a bare string enum. Transport-level SSH failures (exit 255, signal kill, spawn error) during state-file backup set unreachable=true. The caller in backupSandboxState propagates this to the outer BackupResult.unreachable so NEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP=1 activates for state-file failures, not only the initial dir probe. - getSshConfig-null branch in backupSandboxState now returns unreachable=true. For a sandbox already confirmed running (the maintenance.ts loop only reaches BackupResult.unreachable for running sandboxes), an ssh-config lookup failure is transport-level. Addresses CodeRabbit's Major finding at sandbox.ts:1128-1141. Both branches now feed the same skip path already exercised by the initial dir probe. Propagation is covered by manual E2E for the running-sandbox-goes-unreachable-mid-loop case; unit tests would require heavy spawnSync mocking that this PR does not introduce. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
…ycle-flags row (#6188) Close remaining GPT-5.5 advisor items: - PRA-2: the docs promised 'recovered from their latest validated backup during onboarding' without distinguishing between the installer's automatic pre-upgrade backup-all (which exports NEMOCLAW_RESTORE_LATEST_BACKUP_ON_RECREATE=1 at install.sh:1912) and standalone 'nemoclaw backup-all' runs (which do not). Split the paragraph in both mdx files so the automatic restore is clearly installer-scoped; standalone runs only skip the failure. - PRA-4: add a Lifecycle Behavior Flags table row for NEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP in both commands.mdx and commands-nemohermes.mdx so it shows up alongside the other sandbox-scoped lifecycle env vars. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
#6188) Address advisor PRA-6 and PRA-10: - Move isSshTransportFailure from src/lib/state/sandbox.ts into a new src/lib/state/ssh-transport.ts. The function was already a pure utility with no sandbox.ts internals; extraction keeps sandbox.ts smaller and gives future SSH-transport helpers a natural home. Re- exported from sandbox.ts for backwards compatibility. - Rename src/lib/state/sandbox.test.ts to src/lib/state/ssh-transport.test.ts so the test file name matches its actual scope. No behavior change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
…ion (#6188) The re-export syntax 'export { isSshTransportFailure } from "./ssh-transport"' doesn't bind the name into local module scope in TS, so internal call sites in sandbox.ts stopped resolving after the previous extraction commit. Add a real import so both external re-export and local uses resolve. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
…retry (#6188) Add advisor PRA-9 coverage: prove the installer passes NEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP=1 through unchanged when it retries pre-upgrade backup with the current CLI. The env var is consumed by the CLI's backup-all path (maintenance.ts); the installer's contract is just to propagate it. - Extend the current-CLI mock to echo the skip env var into cli.log so the test can assert propagation without relying on install.sh internals. - Add a new test case where NEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP=1 is set, the old CLI fails, the current CLI is used for retry, and cli.log records skip-env=1 alongside current:backup-all. - Update the pre-existing 'aborts current-gateway upgrades' assertion to match the new install.sh error text (which now includes the data-loss disclosure per PRA-2). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cjagwani
approved these changes
Jul 2, 2026
cjagwani
left a comment
cjagwani
left a comment
Collaborator
There was a problem hiding this comment.
Thanks @laitingsheng, pushed a bunch of follow-ups tackling everything the advisors flagged. Walked through each one:
backupStateFilenow returns a typed result so transport failures during state-file backup carry theunreachableflag up; same treatment on thegetSshConfignull path. Both feed the existing skip flow.- Data-loss disclosure added to all three warning/error sites, and the docs now spell out that only exactly
=1counts (nottrue/yes) plus a row in the Lifecycle Flags table. isSshTransportFailuremoved into its own module and the test file renamed to match. Also added SIGHUP/SIGPIPE handling since spawnSync surfaces those withstatus=nulland we want the diagnostic explicit.- Installer integration test now proves the skip env var actually propagates through the current-CLI retry, not just that install.sh mentions it.
Left shouldSkipUnreachableSandboxBackup exported since the unit test imports it directly. CodeRabbit came back clean on the re-run, cloud-onboard E2E already green, gateway-upgrade and state-backup-restore still running.
Approving pending the last CI checks. Nice fix.
Contributor
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28617740925
|
21 tasks
cv
added a commit
that referenced
this pull request
Jul 2, 2026
<!-- markdownlint-disable MD041 --> ## Summary This PR prepares the user-facing documentation for v0.0.73 before the release plan is frozen. It adds release notes for the merged runtime changes and closes documentation gaps around DNS-backed HTTPS endpoint validation and LangChain Deep Agents Code proxy recovery. ## Changes - Add the `v0.0.73` release-note section with links to the detailed command, inference, recovery, lifecycle, platform, and setup documentation. - Correct the custom endpoint guidance so DNS-backed HTTPS rejection and the supported alternatives match the fail-closed runtime behavior. - Document the managed `inference.local` proxy boundary and rebuild requirement for existing LangChain Deep Agents Code sandboxes. - Add troubleshooting guidance for the DNS-backed HTTPS validation error. - Validate with `npm run docs:sync-agent-variants` and `npm run docs`; Fern completed with 0 errors and 2 existing warnings. - Source summary: - [#6139](#6139) -> `docs/about/release-notes.mdx`, `docs/inference/inference-options.mdx`, `docs/reference/commands.mdx`, `docs/reference/commands-nemohermes.mdx`, and `docs/reference/troubleshooting.mdx`: Document fail-closed DNS-backed HTTPS endpoint handling and recovery options. - [#6142](#6142) -> `docs/about/release-notes.mdx`: Summarize native OpenShell GPU injection and compatibility-path diagnostics. - [#6197](#6197) -> `docs/about/release-notes.mdx`: Summarize agent-aware messaging preset rejection. - [#6199](#6199) -> `docs/about/release-notes.mdx`: Summarize the unreachable-sandbox backup opt-in, restore behavior, and data-loss boundary. - [#6204](#6204) and [#6206](#6206) -> `docs/about/release-notes.mdx` and `docs/get-started/quickstart-langchain-deepagents-code.mdx`: Document the corrected managed proxy contract and required sandbox rebuild. - [#6213](#6213) -> `docs/about/release-notes.mdx`: Summarize the merged setup, recovery, and host-state documentation. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check all that apply. For any "covered by existing tests", "not applicable", or waiver entry, add a brief justification on the same line or in the Changes section. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: documentation-only release preparation; the Fern docs build validates the changed pages and routes. - [x] 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: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each item you ran and confirmed. Leave unchecked items you skipped. Doc-only changes do not require npm test unless you ran it. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [ ] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run 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) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a new **v0.0.73** release notes section with six highlights at the top of the changelog. * Expanded **Custom Endpoint URL Validation** guidance in inference option docs, including explicit acceptance/rejection rules for HTTP vs DNS-backed HTTPS and how validated IPs are stored. * Updated command references (`nemohermes inference set`, `$$nemoclaw inference set`) to match the new validation behavior. * Added troubleshooting documentation for unsupported **DNS-backed HTTPS endpoints**, plus clarified Deep Agents Code routing and post-upgrade sandbox rebuild guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
…kup (NVIDIA#6199) <!-- markdownlint-disable MD041 --> ## Summary Pre-upgrade `backup-all` aborted the `curl | bash` installer whenever a running sandbox's in-sandbox SSH endpoint did not answer, with no override, looping the upgrade forever. This classifies such a sandbox as unreachable and adds `NEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP=1` to skip it so the upgrade proceeds and onboarding recovers it from its latest validated backup. ## Related Issue Fixes NVIDIA#6188 ## Changes - `src/lib/state/sandbox.ts`: add `unreachable` to `BackupResult` and set it on an SSH transport-level dir-check failure (exit 255, timeout, spawn error) via a new `isSshTransportFailure` predicate. - `src/lib/actions/maintenance.ts`: an unreachable running sandbox is skipped when `NEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP=1`, otherwise it still fails but prints actionable guidance before exit. - `scripts/install.sh`: reword the pre-upgrade backup abort to name the override and the recovery path. - `docs/reference/commands.mdx`: document the flag in the `backup-all` section. - Tests: `maintenance.test.ts` gains skip-with-flag, fail-with-guidance, and flag truth-table cases; new `sandbox.test.ts` covers `isSshTransportFailure`. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] 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: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [x] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run 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) --- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * `backup-all` now detects running sandboxes with an unreachable in-sandbox SSH endpoint and can skip them when `NEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP=1`. * Skipped sandboxes are recovered during onboarding from the latest validated backup; any uncommitted state since then is not preserved. * **Bug Fixes** * Improved failure handling and remediation when SSH transport/unreachability occurs, including clearer guidance and installer/upgrade retry behavior. * **Documentation** * Updated `nemoclaw` and `nemohermes` `backup-all` docs to state the default abort behavior and the skip flag’s exact `=1` requirement. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Co-authored-by: Charan Jagwani <cjagwani@nvidia.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
<!-- markdownlint-disable MD041 --> ## Summary This PR prepares the user-facing documentation for v0.0.73 before the release plan is frozen. It adds release notes for the merged runtime changes and closes documentation gaps around DNS-backed HTTPS endpoint validation and LangChain Deep Agents Code proxy recovery. ## Changes - Add the `v0.0.73` release-note section with links to the detailed command, inference, recovery, lifecycle, platform, and setup documentation. - Correct the custom endpoint guidance so DNS-backed HTTPS rejection and the supported alternatives match the fail-closed runtime behavior. - Document the managed `inference.local` proxy boundary and rebuild requirement for existing LangChain Deep Agents Code sandboxes. - Add troubleshooting guidance for the DNS-backed HTTPS validation error. - Validate with `npm run docs:sync-agent-variants` and `npm run docs`; Fern completed with 0 errors and 2 existing warnings. - Source summary: - [NVIDIA#6139](NVIDIA#6139) -> `docs/about/release-notes.mdx`, `docs/inference/inference-options.mdx`, `docs/reference/commands.mdx`, `docs/reference/commands-nemohermes.mdx`, and `docs/reference/troubleshooting.mdx`: Document fail-closed DNS-backed HTTPS endpoint handling and recovery options. - [NVIDIA#6142](NVIDIA#6142) -> `docs/about/release-notes.mdx`: Summarize native OpenShell GPU injection and compatibility-path diagnostics. - [NVIDIA#6197](NVIDIA#6197) -> `docs/about/release-notes.mdx`: Summarize agent-aware messaging preset rejection. - [NVIDIA#6199](NVIDIA#6199) -> `docs/about/release-notes.mdx`: Summarize the unreachable-sandbox backup opt-in, restore behavior, and data-loss boundary. - [NVIDIA#6204](NVIDIA#6204) and [NVIDIA#6206](NVIDIA#6206) -> `docs/about/release-notes.mdx` and `docs/get-started/quickstart-langchain-deepagents-code.mdx`: Document the corrected managed proxy contract and required sandbox rebuild. - [NVIDIA#6213](NVIDIA#6213) -> `docs/about/release-notes.mdx`: Summarize the merged setup, recovery, and host-state documentation. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check all that apply. For any "covered by existing tests", "not applicable", or waiver entry, add a brief justification on the same line or in the Changes section. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: documentation-only release preparation; the Fern docs build validates the changed pages and routes. - [x] 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: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each item you ran and confirmed. Leave unchecked items you skipped. Doc-only changes do not require npm test unless you ran it. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [ ] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run 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) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a new **v0.0.73** release notes section with six highlights at the top of the changelog. * Expanded **Custom Endpoint URL Validation** guidance in inference option docs, including explicit acceptance/rejection rules for HTTP vs DNS-backed HTTPS and how validated IPs are stored. * Updated command references (`nemohermes inference set`, `$$nemoclaw inference set`) to match the new validation behavior. * Added troubleshooting documentation for unsupported **DNS-backed HTTPS endpoints**, plus clarified Deep Agents Code routing and post-upgrade sandbox rebuild guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
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
4 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
Pre-upgrade
backup-allaborted thecurl | bashinstaller whenever a running sandbox's in-sandbox SSH endpoint did not answer, with no override, looping the upgrade forever. This classifies such a sandbox as unreachable and addsNEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP=1to skip it so the upgrade proceeds and onboarding recovers it from its latest validated backup.Related Issue
Fixes #6188
Changes
src/lib/state/sandbox.ts: addunreachabletoBackupResultand set it on an SSH transport-level dir-check failure (exit 255, timeout, spawn error) via a newisSshTransportFailurepredicate.src/lib/actions/maintenance.ts: an unreachable running sandbox is skipped whenNEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP=1, otherwise it still fails but prints actionable guidance before exit.scripts/install.sh: reword the pre-upgrade backup abort to name the override and the recovery path.docs/reference/commands.mdx: document the flag in thebackup-allsection.maintenance.test.tsgains skip-with-flag, fail-with-guidance, and flag truth-table cases; newsandbox.test.tscoversisSshTransportFailure.Type of Change
Quality Gates
Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
backup-allnow detects running sandboxes with an unreachable in-sandbox SSH endpoint and can skip them whenNEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP=1.nemoclawandnemohermesbackup-alldocs to state the default abort behavior and the skip flag’s exact=1requirement.