fix(onboard): debounce transient sandbox Error during readiness wait (#6043) - #6164
Merged
Conversation
On a fresh onboard the OpenShell gateway can (re)start its supervisor session and re-register the just-created sandbox. During that window `openshell sandbox list` briefly reports the sandbox in Error phase before it flips to Ready (observed on DGX Spark, where the dashboard port fallback + supervisor restart race the sandbox bootstrap). The create/readiness waiter fast-failed on the first Error poll, turning that transient into a terminal onboard failure: Sandbox '<name>' entered Error phase before it became ready (waited up to 1500s). Apply a bounded consecutive-Error debounce in waitForCreatedSandboxReadyWithTrace, mirroring the Docker GPU supervisor-reconnect path: tolerate a transient Error (default 30 polls / ~60s, env NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE) and recover if the sandbox flips to Ready, while still fast-failing (with full failure diagnostics) on sustained Error well before the readiness timeout. Callers can pass errorPhaseDebouncePolls: 1 to restore the original fast-fail behavior. Fixes #6043 Signed-off-by: Yimo Jiang <yimoj@nvidia.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 (6)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds configurable debounce for sandbox Error-phase readiness polling, updates readiness handling for terminal versus transient failure phases, aligns reconnect rounding, and moves related coverage and docs to match the new behavior. ChangesSandbox error-phase debounce handling
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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) — Changes requestedMerge posture: Do not merge yet 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 (2)
src/lib/onboard/docker-gpu-patch.test.ts (2)
963-1013: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo direct unit tests for
getSandboxReadyErrorDebouncePolls.The new exported helper (env override, default fallback,
Math.max(1, ...)clamping) has no dedicated coverage in this file — only exercised indirectly via the readiness-wait tests, which don't pass invalid/overridden env values.Want me to draft tests covering default value, env override, and clamping of non-positive/invalid env values?
🤖 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/docker-gpu-patch.test.ts` around lines 963 - 1013, Add direct unit coverage for getSandboxReadyErrorDebouncePolls, since it is only exercised indirectly by waitForCreatedSandboxReadyWithTrace. Write tests around the helper’s env override behavior, its default fallback, and the Math.max(1, ...) clamping for non-positive or invalid values, so the exported helper is verified independently of the sandbox readiness flow.
963-988: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest relies on the implicit environment-derived default debounce value.
This test doesn't pass
errorPhaseDebouncePolls, so it depends ongetSandboxReadyErrorDebouncePolls()readingprocess.env.NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCEat run time. If another test in the suite stubs/sets this env var (e.g., to test the override path) without restoring it, or a.envfile sets it, this test's poll-count assertion (toHaveBeenCalledTimes(4)) could flake.♻️ Suggested fix: pass an explicit debounce value
const ready = waitForCreatedSandboxReadyWithTrace({ sandboxName: "my-sandbox", timeoutSecs: 600, runCaptureOpenshell, isSandboxReady, getSandboxFailurePhase, + errorPhaseDebouncePolls: 30, sleep, });🤖 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/docker-gpu-patch.test.ts` around lines 963 - 988, This test is relying on the environment-derived default debounce, which makes the polling assertion brittle if NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE is changed elsewhere. Update the test around waitForCreatedSandboxReadyWithTrace to pass an explicit errorPhaseDebouncePolls value instead of depending on getSandboxReadyErrorDebouncePolls() at runtime. Keep the existing recovery scenario and assertion, but make the debounce setting local to the test so the expected runCaptureOpenshell call count stays deterministic.
🤖 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/sandbox-readiness-tracing.ts`:
- Around line 116-122: The debounce in the sandbox readiness wait is currently
applied to every non-null failure phase, but it should only apply to Error;
update the logic around getSandboxFailurePhase so Failed and CrashLoopBackOff
are treated as immediately terminal while only Error uses
errorPhaseDebouncePolls. Use the existing sandbox readiness wait loop and the
getSandboxFailurePhase helper to add a separate non-Error terminal-phase check,
preserving the current fast-fail behavior for genuinely terminal failures.
---
Nitpick comments:
In `@src/lib/onboard/docker-gpu-patch.test.ts`:
- Around line 963-1013: Add direct unit coverage for
getSandboxReadyErrorDebouncePolls, since it is only exercised indirectly by
waitForCreatedSandboxReadyWithTrace. Write tests around the helper’s env
override behavior, its default fallback, and the Math.max(1, ...) clamping for
non-positive or invalid values, so the exported helper is verified independently
of the sandbox readiness flow.
- Around line 963-988: This test is relying on the environment-derived default
debounce, which makes the polling assertion brittle if
NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE is changed elsewhere. Update the test
around waitForCreatedSandboxReadyWithTrace to pass an explicit
errorPhaseDebouncePolls value instead of depending on
getSandboxReadyErrorDebouncePolls() at runtime. Keep the existing recovery
scenario and assertion, but make the debounce setting local to the test so the
expected runCaptureOpenshell call count stays deterministic.
🪄 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: 8456ff24-a143-49bc-824c-82e24680e73a
📒 Files selected for processing (2)
src/lib/onboard/docker-gpu-patch.test.tssrc/lib/onboard/sandbox-readiness-tracing.ts
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 — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision. |
Address review feedback on the #6043 create/readiness Error debounce: - Scope the debounce to the transient "Error" phase only. "Failed" and "CrashLoopBackOff" are genuinely terminal and now fast-fail immediately instead of burning the debounce window (CodeRabbit r3510182513, advisor PRA-2). - Add a source-of-truth / removal-contract comment block mirroring docker-gpu-supervisor-reconnect.ts: invalid state, OpenShell sandbox-list cache boundary, why tolerated locally, regression evidence, removal condition (advisor PRA-3). - Move the readiness-wait tests into a focused sandbox-readiness-tracing.test.ts (out of the docker-gpu-patch.test.ts hotspot) and add direct env-contract coverage (default 30, override, empty/non-finite/NaN/Infinity fallback, clamp-to-1, fractional round/truncate), non-Error immediate-terminal, counter reset on flap, and a deterministic replay of the reporter's DGX Spark sandbox-list sequence through the real waiter (advisor PRA-4, PRA-5). Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
…tract Address second-round review on the #6043 readiness Error debounce: - Surface the terminal Error phase (not a phase-less timeout) when the sandbox is still in Error on the final poll — happens when the debounce window outlasts a low readiness timeout, which previously misreported a stuck Error as "did not become ready" and dropped the phase (advisor PRA-1). - Document the ~60s default-debounce latency trade-off and the intentional trunc-vs-envInt-rounding difference on the errorPhaseDebouncePolls JSDoc (advisor PRA-4, PRA-6). - Add a maintainer-enableable removal-signal test (upstream_openshell_sandbox_list_error_transient_fixed) and a real-DGX E2E follow-up note on the replay fixture (advisor PRA-3, PRA-5). Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Third-round review (PR Review Advisor Nemotron): - Round the programmatic errorPhaseDebouncePolls override (Math.round) so it matches the env-var path's envInt rounding — one consistent rule across both entry points (PRA-3). - Document the removal-signal tracking mechanism in the source-of-truth block: the maintainer-enabled upstream_openshell_sandbox_list_error_transient_fixed test is the executable checkpoint; escalate to a tracking issue if the workaround outlives a release cycle (PRA-2). - Expand the errorPhaseDebouncePolls JSDoc to call out the fresh-create latency trade-off and justify the conservative 30-poll default (re-registration scales with host/gateway speed; a too-low default risks re-introducing #6043; env-tunable) (PRA-4). Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
…racker Fourth-round review (PR Review Advisor Nemotron): - Align the docker-gpu-supervisor-reconnect.ts programmatic override to Math.round so both onboard debounce modules and the env-var path share one rounding rule (PRA-5). - Reference NemoClaw #6043 as the removal tracker in the source-of-truth block and the removal-signal test comment, so the workaround has a concrete tracking handle without inventing a duplicate issue (PRA-2, PRA-3). Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Fifth-round review (PR Review Advisor Nemotron): - Document the new NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE env var (default 30, Error-only scope, K=1 fast-fail opt-out) in the commands + commands-nemohermes env-var tables, and add a troubleshooting entry for the "entered Error phase before it became ready" message — mirroring the existing sibling docs for NEMOCLAW_DOCKER_GPU_SUPERVISOR_RECONNECT_ERROR_DEBOUNCE (PRA-5). - Add the full NemoClaw #6043 issue URL to the source-of-truth block and the removal-signal test comment as the removal tracker (PRA-2, PRA-3). Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-6164.docs.buildwithfern.com/nemoclaw |
wscurran
added
area: onboarding
Contributor
Contributor
Vitest E2E Target Results — ✅ All selected jobs passedRun: 28636624709
|
Contributor
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28636624715
|
cv
approved these changes
Jul 3, 2026
cv
left a comment
cv
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the Error-only debounce, bounded failure behavior, source-of-truth/removal contract, deterministic DGX replay, and focused tests. I accept the 30-poll default: it covers slower DGX-class registration while remaining bounded well below the readiness timeout and is operator-configurable. I am overriding the separate scheduled-nightly suggestion because #6043 plus the executable removal checkpoint provide a concrete tracking path, while no stable upstream nightly signal is defined; adding a privileged scheduled workflow is not warranted in this fix. All four required exact-head E2E targets passed; CI, CodeRabbit, GPT PR Advisor, DCO, and verification are clean. Approved for v0.0.74.
21 tasks
ericksoa
added a commit
that referenced
this pull request
Jul 4, 2026
<!-- markdownlint-disable MD041 --> ## Summary This PR prepares the user-facing documentation for v0.0.74 before the release plan is frozen. It expands the release notes across the 56-commit train and closes durable documentation gaps found during the pre-tag commit scan. ## Changes - Expand the `v0.0.74` release notes to cover OpenShell 0.0.72, managed MCP, progressive tool disclosure, LangChain Deep Agents Code, onboarding, local inference, messaging, recovery, and contributor workflows. - Correct the `destroy` contract for retained per-name volumes, gateway-unreachable `--force` cleanup, managed MCP ownership, and same-name recovery. - Document separate remediation for an unreachable container DNS resolver versus one that answers with `NXDOMAIN` or `REFUSED`. - Document the Windows on Arm N1X automatic Ollama safeguard and its remaining large-model limitations. - State that messaging conflicts abort rebuild before backup or deletion, leaving the original sandbox intact. - Link the agent-runnable value benchmark from the contributor task index. - Synchronize generated agent command variants. - Validate with `npm run docs:sync-agent-variants` and `npm run docs`; Fern completed with 0 errors and 2 existing warnings. - Source summary: - [#6020](#6020) and [#5876](#5876) -> `docs/about/release-notes.mdx`: Consolidate the OpenShell 0.0.72 policy boundary and managed MCP lifecycle. - [#6251](#6251) and [#5989](#5989) -> `docs/about/release-notes.mdx`: Summarize progressive tool disclosure and sandbox-first inference controls. - [#6232](#6232), [#6082](#6082), [#6219](#6219), [#6214](#6214), [#6215](#6215), [#6230](#6230), and [#6260](#6260) -> `docs/about/release-notes.mdx`: Summarize the experimental LangChain Deep Agents Code status, secret, version, rebuild, snapshot, and MCP boundaries. - [#6166](#6166), [#6254](#6254), [#6265](#6265), [#6164](#6164), and [#6017](#6017) -> `docs/about/release-notes.mdx`: Summarize BuildKit prebuild, validated image reuse, bounded readiness, and preflight improvements. - [#6150](#6150) -> `docs/about/release-notes.mdx` and `docs/reference/troubleshooting.mdx`: Separate unreachable-resolver remediation from reachable-but-rejected DNS responses. - [#6234](#6234) -> `docs/about/release-notes.mdx`, `docs/inference/use-local-inference.mdx`, and `docs/get-started/windows-preparation.mdx`: Document N1X automatic 9B selection and the remaining explicit-large-model boundary. - [#6129](#6129), [#5987](#5987), [#5955](#5955), and [#6220](#6220) -> `docs/about/release-notes.mdx`, `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, and `docs/reference/commands-nemohermes.mdx`: Document messaging policy persistence, status, and the pre-destructive conflict check. - [#5963](#5963), [#6050](#6050), [#6094](#6094), [#6238](#6238), [#5988](#5988), [#6235](#6235), [#6181](#6181), and [#5986](#5986) -> `docs/about/release-notes.mdx`, `docs/reference/commands.mdx`, and `docs/reference/commands-nemohermes.mdx`: Summarize day-two recovery and clarify retained-volume and local-only destroy semantics. - [#6200](#6200), [#6248](#6248), [#6168](#6168), [#6270](#6270), and [#5649](#5649) -> `docs/about/release-notes.mdx` and `CONTRIBUTING.md`: Summarize contributor setup and verification improvements and expose the advisory value benchmark. ## 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) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: documentation-only release preparation; generated-variant synchronization and the Fern docs build validate 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 applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: tests are not applicable to this documentation-only change; `npm run docs` validates the source and generated routes. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [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: Aaron Erickson <aerickson@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Expanded setup guidance for Windows on Arm devices with safer default local model selection. * Clarified local inference and sandbox messaging behavior, including conflict checks before rebuilds and safer recovery steps. * Updated destroy/rebuild/reference docs with more detailed warnings, failure handling, and volume-retention guidance. * Improved troubleshooting instructions for Docker DNS issues with clearer paths for unreachable vs. blocked resolvers. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
…VIDIA#6043) (NVIDIA#6164) ## Summary On a fresh `nemoclaw onboard`, the OpenShell gateway can (re)start its supervisor session and re-register the just-created sandbox. During that window `openshell sandbox list` briefly reports the sandbox in the transient **Error** phase before it flips to **Ready** (observed on DGX Spark, where the dashboard port fallback `18789 → 18794` and the supervisor restart race the sandbox bootstrap). The create/readiness waiter fast-failed on the *first* Error poll, turning a recoverable transient into a terminal onboard failure. This PR applies a bounded consecutive-**Error** debounce so the transient recovers, while genuinely terminal phases still fail immediately. ## Related Issue Fixes NVIDIA#6043 ## Changes - `src/lib/onboard/sandbox-readiness-tracing.ts`: `waitForCreatedSandboxReadyWithTrace` now requires **consecutive Error** polls before declaring a terminal failure, instead of bailing on the first Error poll. Default is 30 polls (~60s at the 2s poll interval), configurable via `NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE`; the counter resets on any non-Error poll so only *sustained* Error is terminal. Mirrors the existing `docker-gpu-supervisor-reconnect.ts` debounce. - **Debounce is scoped to `Error` only.** `Failed` and `CrashLoopBackOff` are genuinely terminal and still fast-fail immediately (addresses CodeRabbit `r3510182513` and PR Review Advisor PRA-2). - Terminal failures are **not** hidden: sustained Error still fast-fails after the bounded window (well before the readiness timeout), and the caller still captures full failure diagnostics (`collectSandboxCreateFailureDiagnostics`). Callers can pass `errorPhaseDebouncePolls: 1` to restore the original fast-fail. - Added a source-of-truth / removal-contract comment block (invalid state → OpenShell `sandbox list` cache boundary → why tolerated locally → regression evidence → removal condition), mirroring `docker-gpu-supervisor-reconnect.ts` (PRA-3). - Added `transient_failure_phase` trace event and `last_failure_phase` on the timeout trace. - Tests moved into a focused `src/lib/onboard/sandbox-readiness-tracing.test.ts` (out of the `docker-gpu-patch.test.ts` hotspot, which shrinks; PRA-4). New direct coverage: default `30`, env override, empty/non-finite (`""`/`abc`/`NaN`/`Infinity`) fallback, clamp-to-1, fractional rounding (env) and truncation (param) semantics, non-Error immediate-terminal, counter reset on flap, and a **deterministic replay of the reporter's DGX Spark `sandbox list` sequence** through the real waiter (PRA-5). ## Type of Change - [x] Code change (feature, bug fix, or refactor) ## Quality Gates - [x] Tests added or updated for changed behavior - [x] Docs not applicable — justification: internal onboarding readiness-wait timing/recovery; no user-facing doc surface. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: high-effort `/code-review` on the diff returned no findings; CodeRabbit `r3510182513` (Error-only scoping) and PR Review Advisor items PRA-2/PRA-3/PRA-4/PRA-5 addressed in code/tests; change reuses the reviewed supervisor-reconnect debounce pattern. ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Targeted tests pass — `vitest run src/lib/onboard/sandbox-readiness-tracing.test.ts src/lib/onboard/docker-gpu-patch.test.ts` (70 tests) - [x] Full `npm test` (cli lane) passes — `vitest run --project cli`: 508 files / 5336 tests - [x] Biome check clean on changed files - [x] No secrets, API keys, or credentials committed ### E2E / reproduction DGX Spark hardware was unavailable in this session, and the failure is a **timing-dependent transient Error** during gateway re-registration that cannot be forced deterministically on substitute GPU hardware (a healthy onboard on the available Linux/GPU host `yimoj-colossus-dev` would not enter the Error branch). Per the acceptance guidance, the fix is gated by a **checked-in deterministic replay** (`sandbox-readiness-tracing.test.ts` → "DGX Spark fresh-onboard readiness replay (NVIDIA#6043)") that drives the reporter's exact `sandbox list` sequence through the real readiness waiter: the pre-fix (`K=1`) path reproduces the exact reporter line and the shipped default recovers to Ready. The same replay against the built `dist/` shipped code: ``` [transient Error (default) -> recovers] polls=5 {"ready":true,"reason":"ready","failurePhase":null} [Failed (default) -> immediate terminal] polls=2 {"ready":false,"reason":"terminal_failure_phase","failurePhase":"Failed"} [CrashLoopBackOff (default) -> immediate terminal] polls=2 {"ready":false,"reason":"terminal_failure_phase","failurePhase":"CrashLoopBackOff"} ``` A **real worktree-CLI onboard** was also run on this host to prove the changed readiness waiter runs in the real command path without regression (`node ./bin/nemoclaw.js onboard --non-interactive --yes --fresh --no-gpu --no-sandbox-gpu --agent openclaw --name nemoclaw-6043-e2e`): ``` Creating sandbox in gateway... Built image openshell/sandbox-from:1782969910 Waiting for sandbox to become ready... <- waitForCreatedSandboxReadyWithTrace (changed code) Sandbox reported Ready before create stream exited; continuing. ✓ Sandbox 'nemoclaw-6043-e2e' created ✓ OpenClaw gateway launched inside sandbox ``` `openshell sandbox list` afterward: `nemoclaw-6043-e2e ... Ready`. This exercises the changed waiter on the happy path (no Error branch, since the DGX Spark transient cannot be forced on a healthy non-DGX host); the Error-recovery branch is covered by the deterministic replay above. --- Signed-off-by: Yimo Jiang <yimoj@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable tolerance for transient **Error** phases during post-create sandbox readiness, with env-based default and a minimum of 1. * Supports overriding the tolerated **Error** poll count (including rounding for non-integers). * **Bug Fixes** * Improved readiness failure reporting: sustained **Error** now ends as a terminal **Error** (even when the debounce window expires), while other terminal phases still fail immediately. * **Documentation** * Documented the new tuning variable and the troubleshooting scenario for “entered Error phase before it became ready”. * **Tests** * Expanded and reorganized readiness tracing coverage for debounce, recovery, streak reset, and timeout/debounce edge cases. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Yimo Jiang <yimoj@nvidia.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.74 before the release plan is frozen. It expands the release notes across the 56-commit train and closes durable documentation gaps found during the pre-tag commit scan. ## Changes - Expand the `v0.0.74` release notes to cover OpenShell 0.0.72, managed MCP, progressive tool disclosure, LangChain Deep Agents Code, onboarding, local inference, messaging, recovery, and contributor workflows. - Correct the `destroy` contract for retained per-name volumes, gateway-unreachable `--force` cleanup, managed MCP ownership, and same-name recovery. - Document separate remediation for an unreachable container DNS resolver versus one that answers with `NXDOMAIN` or `REFUSED`. - Document the Windows on Arm N1X automatic Ollama safeguard and its remaining large-model limitations. - State that messaging conflicts abort rebuild before backup or deletion, leaving the original sandbox intact. - Link the agent-runnable value benchmark from the contributor task index. - Synchronize generated agent command variants. - Validate with `npm run docs:sync-agent-variants` and `npm run docs`; Fern completed with 0 errors and 2 existing warnings. - Source summary: - [NVIDIA#6020](NVIDIA#6020) and [NVIDIA#5876](NVIDIA#5876) -> `docs/about/release-notes.mdx`: Consolidate the OpenShell 0.0.72 policy boundary and managed MCP lifecycle. - [NVIDIA#6251](NVIDIA#6251) and [NVIDIA#5989](NVIDIA#5989) -> `docs/about/release-notes.mdx`: Summarize progressive tool disclosure and sandbox-first inference controls. - [NVIDIA#6232](NVIDIA#6232), [NVIDIA#6082](NVIDIA#6082), [NVIDIA#6219](NVIDIA#6219), [NVIDIA#6214](NVIDIA#6214), [NVIDIA#6215](NVIDIA#6215), [NVIDIA#6230](NVIDIA#6230), and [NVIDIA#6260](NVIDIA#6260) -> `docs/about/release-notes.mdx`: Summarize the experimental LangChain Deep Agents Code status, secret, version, rebuild, snapshot, and MCP boundaries. - [NVIDIA#6166](NVIDIA#6166), [NVIDIA#6254](NVIDIA#6254), [NVIDIA#6265](NVIDIA#6265), [NVIDIA#6164](NVIDIA#6164), and [NVIDIA#6017](NVIDIA#6017) -> `docs/about/release-notes.mdx`: Summarize BuildKit prebuild, validated image reuse, bounded readiness, and preflight improvements. - [NVIDIA#6150](NVIDIA#6150) -> `docs/about/release-notes.mdx` and `docs/reference/troubleshooting.mdx`: Separate unreachable-resolver remediation from reachable-but-rejected DNS responses. - [NVIDIA#6234](NVIDIA#6234) -> `docs/about/release-notes.mdx`, `docs/inference/use-local-inference.mdx`, and `docs/get-started/windows-preparation.mdx`: Document N1X automatic 9B selection and the remaining explicit-large-model boundary. - [NVIDIA#6129](NVIDIA#6129), [NVIDIA#5987](NVIDIA#5987), [NVIDIA#5955](NVIDIA#5955), and [NVIDIA#6220](NVIDIA#6220) -> `docs/about/release-notes.mdx`, `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, and `docs/reference/commands-nemohermes.mdx`: Document messaging policy persistence, status, and the pre-destructive conflict check. - [NVIDIA#5963](NVIDIA#5963), [NVIDIA#6050](NVIDIA#6050), [NVIDIA#6094](NVIDIA#6094), [NVIDIA#6238](NVIDIA#6238), [NVIDIA#5988](NVIDIA#5988), [NVIDIA#6235](NVIDIA#6235), [NVIDIA#6181](NVIDIA#6181), and [NVIDIA#5986](NVIDIA#5986) -> `docs/about/release-notes.mdx`, `docs/reference/commands.mdx`, and `docs/reference/commands-nemohermes.mdx`: Summarize day-two recovery and clarify retained-volume and local-only destroy semantics. - [NVIDIA#6200](NVIDIA#6200), [NVIDIA#6248](NVIDIA#6248), [NVIDIA#6168](NVIDIA#6168), [NVIDIA#6270](NVIDIA#6270), and [NVIDIA#5649](NVIDIA#5649) -> `docs/about/release-notes.mdx` and `CONTRIBUTING.md`: Summarize contributor setup and verification improvements and expose the advisory value benchmark. ## 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) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: documentation-only release preparation; generated-variant synchronization and the Fern docs build validate 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 applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: tests are not applicable to this documentation-only change; `npm run docs` validates the source and generated routes. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [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: Aaron Erickson <aerickson@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Expanded setup guidance for Windows on Arm devices with safer default local model selection. * Clarified local inference and sandbox messaging behavior, including conflict checks before rebuilds and safer recovery steps. * Updated destroy/rebuild/reference docs with more detailed warnings, failure handling, and volume-retention guidance. * Improved troubleshooting instructions for Docker DNS issues with clearer paths for unreachable vs. blocked resolvers. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Aaron Erickson <aerickson@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
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
On a fresh
nemoclaw onboard, the OpenShell gateway can (re)start its supervisor session and re-register the just-created sandbox. During that windowopenshell sandbox listbriefly reports the sandbox in the transient Error phase before it flips to Ready (observed on DGX Spark, where the dashboard port fallback18789 → 18794and the supervisor restart race the sandbox bootstrap). The create/readiness waiter fast-failed on the first Error poll, turning a recoverable transient into a terminal onboard failure. This PR applies a bounded consecutive-Error debounce so the transient recovers, while genuinely terminal phases still fail immediately.Related Issue
Fixes #6043
Changes
src/lib/onboard/sandbox-readiness-tracing.ts:waitForCreatedSandboxReadyWithTracenow requires consecutive Error polls before declaring a terminal failure, instead of bailing on the first Error poll. Default is 30 polls (~60s at the 2s poll interval), configurable viaNEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE; the counter resets on any non-Error poll so only sustained Error is terminal. Mirrors the existingdocker-gpu-supervisor-reconnect.tsdebounce.Erroronly.FailedandCrashLoopBackOffare genuinely terminal and still fast-fail immediately (addresses CodeRabbitr3510182513and PR Review Advisor PRA-2).collectSandboxCreateFailureDiagnostics). Callers can passerrorPhaseDebouncePolls: 1to restore the original fast-fail.sandbox listcache boundary → why tolerated locally → regression evidence → removal condition), mirroringdocker-gpu-supervisor-reconnect.ts(PRA-3).transient_failure_phasetrace event andlast_failure_phaseon the timeout trace.src/lib/onboard/sandbox-readiness-tracing.test.ts(out of thedocker-gpu-patch.test.tshotspot, which shrinks; PRA-4). New direct coverage: default30, env override, empty/non-finite (""/abc/NaN/Infinity) fallback, clamp-to-1, fractional rounding (env) and truncation (param) semantics, non-Error immediate-terminal, counter reset on flap, and a deterministic replay of the reporter's DGX Sparksandbox listsequence through the real waiter (PRA-5).Type of Change
Quality Gates
/code-reviewon the diff returned no findings; CodeRabbitr3510182513(Error-only scoping) and PR Review Advisor items PRA-2/PRA-3/PRA-4/PRA-5 addressed in code/tests; change reuses the reviewed supervisor-reconnect debounce pattern.Verification
Verifiedin GitHubvitest run src/lib/onboard/sandbox-readiness-tracing.test.ts src/lib/onboard/docker-gpu-patch.test.ts(70 tests)npm test(cli lane) passes —vitest run --project cli: 508 files / 5336 testsE2E / reproduction
DGX Spark hardware was unavailable in this session, and the failure is a timing-dependent transient Error during gateway re-registration that cannot be forced deterministically on substitute GPU hardware (a healthy onboard on the available Linux/GPU host
yimoj-colossus-devwould not enter the Error branch). Per the acceptance guidance, the fix is gated by a checked-in deterministic replay (sandbox-readiness-tracing.test.ts→ "DGX Spark fresh-onboard readiness replay (#6043)") that drives the reporter's exactsandbox listsequence through the real readiness waiter: the pre-fix (K=1) path reproduces the exact reporter line and the shipped default recovers to Ready.The same replay against the built
dist/shipped code:A real worktree-CLI onboard was also run on this host to prove the changed readiness waiter runs in the real command path without regression (
node ./bin/nemoclaw.js onboard --non-interactive --yes --fresh --no-gpu --no-sandbox-gpu --agent openclaw --name nemoclaw-6043-e2e):openshell sandbox listafterward:nemoclaw-6043-e2e ... Ready. This exercises the changed waiter on the happy path (no Error branch, since the DGX Spark transient cannot be forced on a healthy non-DGX host); the Error-recovery branch is covered by the deterministic replay above.Signed-off-by: Yimo Jiang yimoj@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests