fix(onboard): keep the pre-rollback cause when a sandbox container cannot start - #7998
fix(onboard): keep the pre-rollback cause when a sandbox container cannot start#7998yanyunl1991 wants to merge 4 commits into
Conversation
…nnot start
Onboarding recreates the sandbox container with a restart-safe startup
command and waits for the OpenShell supervisor to reconnect. When the
container cannot start, NemoClaw captures a pre-rollback diagnostics
bundle that already classifies why it died, then rolls back and deletes
that container.
The user-facing failure block ran after the rollback, re-inspected the
container, found it gone, and degraded to the weakest verdict available
("OpenShell sandbox entered Error phase") while the accurate one
("Patched GPU container exited with code 127") had been computed moments
earlier and written only to disk. Operators were left with GPU escape
hatches that do not apply.
Carry the pre-rollback classification through to the failure printer and
prefer it only when the post-rollback re-inspection can no longer observe
the container, so first-hand evidence is never overridden by a stale
verdict. Classify exit code 127 explicitly: the sandbox entrypoint
launches its startup command through `env`, which exits 127 only when
that command is absent from the image, so the signature unambiguously
means the image lacks the NemoClaw-managed runtime. Attach the remedy as
prose hints kept separate from the machine-readable summary lines.
Fixes #7996
Signed-off-by: yanyunl1991 <yanyunl@nvidia.com>
|
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)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe onboarding flow now captures Docker GPU patch failure evidence before rollback, reports structured replacement-container cleanup state, and provides conditional guidance for images missing the managed ChangesDocker GPU failure diagnostics
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DockerGpuSandboxCreate
participant DockerGpuPreRollbackDiagnostics
participant PatchedContainer
participant DockerGpuPatchRollback
participant FailureHandler
DockerGpuSandboxCreate->>DockerGpuPreRollbackDiagnostics: capture logs and failure classification
DockerGpuPreRollbackDiagnostics->>PatchedContainer: inspect failed replacement container
DockerGpuPreRollbackDiagnostics-->>DockerGpuSandboxCreate: return classification and diagnostics
DockerGpuSandboxCreate->>DockerGpuPatchRollback: restore sandbox and record replacement state
DockerGpuSandboxCreate->>FailureHandler: pass preserved classification and diagnostics
FailureHandler-->>DockerGpuSandboxCreate: print hints, cleanup state, and exit
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 21f737c in the TypeScript / code-coverage/cliThe overall coverage in commit 21f737c in the Show a code coverage summary of the most impacted files.
Updated |
|
🌿 Preview your docs: https://nvidia-preview-pr-7998.docs.buildwithfern.com/nemoclaw |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/onboard/docker-gpu-pre-rollback-diagnostics.ts (1)
122-181: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftRetain the classification when bundle collection fails.
At Line 181, a
nulldiagnostics bundle discards the classification already captured from the replacement container.
The caller then forwardsnullafter rollback and loses the exit code127remedy.
Capture the classification before best-effort bundle work, and return it independently when bundle collection fails.
Add a test for this failure path.The PR objective requires preserving the diagnosis after rollback.
🤖 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-pre-rollback-diagnostics.ts` around lines 122 - 181, Update captureDockerGpuPreRollbackDiagnostics so classification remains available when collectDockerGpuPatchDiagnostics returns null: capture it before best-effort bundle collection and return a non-null result containing the classification with an absent diagnostics bundle, while preserving the existing bundled result on success. Add a test covering bundle-collection failure and verifying the classification, including the exit-code-127 remedy, survives rollback.
🤖 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/troubleshooting.mdx`:
- Around line 578-581: Update the troubleshooting text around
finalizeDockerGpuPatchBackup to make the diagnostics directory conditional on
NemoClaw successfully saving pre-rollback diagnostics. Also state that the
failed container is unavailable after a successful rollback, rather than
presenting restoration as guaranteed; preserve the existing failure and remedy
details.
---
Outside diff comments:
In `@src/lib/onboard/docker-gpu-pre-rollback-diagnostics.ts`:
- Around line 122-181: Update captureDockerGpuPreRollbackDiagnostics so
classification remains available when collectDockerGpuPatchDiagnostics returns
null: capture it before best-effort bundle collection and return a non-null
result containing the classification with an absent diagnostics bundle, while
preserving the existing bundled result on success. Add a test covering
bundle-collection failure and verifying the classification, including the
exit-code-127 remedy, survives rollback.
🪄 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: 918cbcf7-fb75-422c-9048-e1e22a4e3384
📒 Files selected for processing (8)
docs/reference/troubleshooting.mdxsrc/lib/onboard/docker-gpu-patch-diagnostics-classification.test.tssrc/lib/onboard/docker-gpu-patch-failure-print.test.tssrc/lib/onboard/docker-gpu-patch-types.tssrc/lib/onboard/docker-gpu-patch.tssrc/lib/onboard/docker-gpu-pre-rollback-diagnostics.tssrc/lib/onboard/docker-gpu-sandbox-create-diagnostics.test.tssrc/lib/onboard/docker-gpu-sandbox-create.ts
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence Model lanes
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
wscurran
added
area: onboarding
|
✨ Thanks for the PR. This fixes onboarding error reporting by preserving the pre-rollback diagnostics when a sandbox container fails to start, so users see the accurate failure cause instead of a degraded post-rollback message. Maintainers will review the error flow and test coverage. Related open issues: |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Correct the earlier contributor rationale: exit 127 alone is ambiguous. Only the exact missing nemoclaw-start diagnostic selects the startup-command-absent path. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/lib/onboard/docker-gpu-patch-diagnostics.ts (1)
218-253: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the cleanup-state decision into a pure helper.
This block computes
cleanupPendingRollback,prePatchRestored,replacementPresence,cleanupDisposition, andcleanupCommandsthrough several nested ternaries, inside a function that also performs filesystem and Docker I/O. Extract this into a small pure function, for exampleresolveDockerGpuCleanupState(options, snapshot, context, inspectedTargets), that takes only the already-computed inputs and returns{ cleanupDisposition, cleanupCommands, replacementPresence }.This lowers the complexity of
collectDockerGpuPatchDiagnosticsand lets the decision table be unit-tested directly, without fakes fordockerCapture,dockerLogs, or the filesystem.As per coding guidelines, "
**/*.{js,ts,tsx}: Keep function complexity low", and per thesrc/lib/README.mdpath instructions to "Separate pure classification/planning logic from Docker, filesystem, and process interactions; keep host-boundary operations injectable so ... tests can use fakes."🤖 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-diagnostics.ts` around lines 218 - 253, Extract the cleanup-state calculation from collectDockerGpuPatchDiagnostics into a pure helper such as resolveDockerGpuCleanupState, accepting only options, snapshot, context, and inspectedTargets. Move the replacement-presence, cleanup-disposition, and cleanup-command logic into the helper and have it return cleanupDisposition, cleanupCommands, and replacementPresence; leave Docker, filesystem, and process I/O in the caller.Sources: Coding guidelines, Path instructions
src/lib/onboard/docker-gpu-sandbox-create.ts (1)
298-304: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
failureDiagnosticDepsin this call.Lines 299-303 repeat the exact five keys that
failureDiagnosticDepsalready holds at lines 150-156. The other three call sites spread the shared object. Spread it here too so a future dependency addition reaches every failure path.♻️ Proposed consolidation
onPatchFailureExit(options.sandboxName, new Error(failureMessage), { - runCaptureOpenshell: options.deps.runCaptureOpenshell, - dockerCapture: options.deps.dockerCapture, - dockerLogs: options.deps.dockerLogs, - homedir: options.deps.homedir, - now: options.deps.now, + ...failureDiagnosticDeps, additionalSummaryLines: routeAdapter.additionalSummaryLines, preRollbackClassification,🤖 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-sandbox-create.ts` around lines 298 - 304, Update the onPatchFailureExit call in the failure path to spread the existing failureDiagnosticDeps object instead of repeating runCaptureOpenshell, dockerCapture, dockerLogs, homedir, and now; preserve additionalSummaryLines alongside the spread so the call retains its route-specific summary data.src/lib/onboard/docker-gpu-patch-finalize.test.ts (1)
149-197: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
unknownreplacement presence outcome.The two new tests cover
presentandabsent.observeReplacementPresencealso returnsunknownon two paths: a container ID that is not a 64-character hex string, and adocker pscall that exits non-zero.unknowndrivescleanup_disposition=unknownandcleanup_required=unknownincollectDockerGpuPatchDiagnostics, so the branch changes user-visible cleanup guidance. Add a test that fails the presence query.💚 Proposed test for the inconclusive presence query
+ it("records unknown replacement presence when the presence query fails (`#7996`)", () => { + const newContainerId = "c".repeat(64); + const outcome = finalizeDockerGpuPatchBackup( + { + result: { ...deferredCreateResult(), newContainerId }, + supervisorReady: false, + }, + { + dockerStop: vi.fn(() => ({ status: 0 })), + dockerRm: vi.fn(() => ({ status: 1 })), + dockerRun: vi.fn(() => ({ status: 1, stdout: "" })), + dockerRename: vi.fn(() => ({ status: 0 })), + dockerStart: vi.fn(() => ({ status: 0 })), + }, + ); + + expect(outcome).toEqual({ + backupRemoved: false, + rolledBack: true, + replacementStopConfirmed: true, + replacementRemovalConfirmed: false, + replacementPresence: "unknown", + }); + });🤖 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-finalize.test.ts` around lines 149 - 197, Add coverage in the finalizeDockerGpuPatch tests for the `unknown` replacement presence outcome by making the replacement presence query inconclusive, such as returning a non-64-character replacement ID or a non-zero `docker ps` result. Assert that `finalizeDockerGpuPatchBackup` reports `replacementPresence: "unknown"` while preserving the other expected cleanup fields.src/lib/onboard/docker-gpu-sandbox-create-diagnostics.test.ts (1)
285-292: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the pre-rollback directory before reading the summary.
Line 287 falls back to
""whenpreRollbackis undefined.path.join("", "summary.txt")resolves tosummary.txtrelative to the working directory. The test then fails with an ENOENT path that does not name the real defect, or reads an unrelated file if one exists at the repository root. Assert the directory first so the failure message identifies the missing diagnostics bundle.♻️ Proposed guard
const preRollback = (captured as DockerGpuPreRollbackDiagnostics | null)?.diagnostics; - const preRollbackSummary = fs.readFileSync( - path.join(preRollback?.dir ?? "", "summary.txt"), - "utf-8", - ); + expect(preRollback?.dir).toBeTruthy(); + const preRollbackSummary = fs.readFileSync( + path.join(String(preRollback?.dir), "summary.txt"), + "utf-8", + );🤖 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-sandbox-create-diagnostics.test.ts` around lines 285 - 292, In the pre-rollback diagnostics assertions, validate that preRollback?.dir is defined before calling fs.readFileSync. Update the flow around preRollback and preRollbackSummary so the test fails with the missing diagnostics bundle assertion instead of falling back to an empty path; retain the existing summary and cleanupCommands assertions after this guard.
🤖 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/docker-gpu-pre-rollback-diagnostics.ts`:
- Around line 28-31: Add a BusyBox-formatted alternative to
MISSING_MANAGED_STARTUP_COMMAND_LOG so it matches `env: can't execute
'nemoclaw-start': No such file or directory` while preserving the existing GNU
env format, and add a regression test covering the custom Alpine/BusyBox failure
and missing-command hint.
---
Nitpick comments:
In `@src/lib/onboard/docker-gpu-patch-diagnostics.ts`:
- Around line 218-253: Extract the cleanup-state calculation from
collectDockerGpuPatchDiagnostics into a pure helper such as
resolveDockerGpuCleanupState, accepting only options, snapshot, context, and
inspectedTargets. Move the replacement-presence, cleanup-disposition, and
cleanup-command logic into the helper and have it return cleanupDisposition,
cleanupCommands, and replacementPresence; leave Docker, filesystem, and process
I/O in the caller.
In `@src/lib/onboard/docker-gpu-patch-finalize.test.ts`:
- Around line 149-197: Add coverage in the finalizeDockerGpuPatch tests for the
`unknown` replacement presence outcome by making the replacement presence query
inconclusive, such as returning a non-64-character replacement ID or a non-zero
`docker ps` result. Assert that `finalizeDockerGpuPatchBackup` reports
`replacementPresence: "unknown"` while preserving the other expected cleanup
fields.
In `@src/lib/onboard/docker-gpu-sandbox-create-diagnostics.test.ts`:
- Around line 285-292: In the pre-rollback diagnostics assertions, validate that
preRollback?.dir is defined before calling fs.readFileSync. Update the flow
around preRollback and preRollbackSummary so the test fails with the missing
diagnostics bundle assertion instead of falling back to an empty path; retain
the existing summary and cleanupCommands assertions after this guard.
In `@src/lib/onboard/docker-gpu-sandbox-create.ts`:
- Around line 298-304: Update the onPatchFailureExit call in the failure path to
spread the existing failureDiagnosticDeps object instead of repeating
runCaptureOpenshell, dockerCapture, dockerLogs, homedir, and now; preserve
additionalSummaryLines alongside the spread so the call retains its
route-specific summary data.
🪄 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: bf22637f-b843-4245-9b92-95630ac16993
📒 Files selected for processing (16)
docs/reference/troubleshooting.mdxsrc/lib/onboard/docker-gpu-patch-clone.tssrc/lib/onboard/docker-gpu-patch-diagnostics-classification.test.tssrc/lib/onboard/docker-gpu-patch-diagnostics-collection.test.tssrc/lib/onboard/docker-gpu-patch-diagnostics.tssrc/lib/onboard/docker-gpu-patch-failure-print.test.tssrc/lib/onboard/docker-gpu-patch-finalize.test.tssrc/lib/onboard/docker-gpu-patch-finalize.tssrc/lib/onboard/docker-gpu-patch-recreate.tssrc/lib/onboard/docker-gpu-patch-rollback.tssrc/lib/onboard/docker-gpu-patch-types.tssrc/lib/onboard/docker-gpu-patch.tssrc/lib/onboard/docker-gpu-pre-rollback-diagnostics.test.tssrc/lib/onboard/docker-gpu-pre-rollback-diagnostics.tssrc/lib/onboard/docker-gpu-sandbox-create-diagnostics.test.tssrc/lib/onboard/docker-gpu-sandbox-create.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/onboard/docker-gpu-patch-diagnostics-classification.test.ts
- src/lib/onboard/docker-gpu-patch-failure-print.test.ts
- docs/reference/troubleshooting.mdx
| const PRE_ROLLBACK_DIAGNOSTICS_TOTAL_BUDGET_MS = 10_000; | ||
| const PRE_ROLLBACK_DIAGNOSTICS_CALL_TIMEOUT_MS = 2_000; | ||
| const MISSING_MANAGED_STARTUP_COMMAND_LOG = | ||
| /(?:^|\n)(?:\/usr\/bin\/)?env: [\u0027\u2018]?nemoclaw-start[\u0027\u2019]?: No such file or directory(?:\r?\n|$)/u; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Look for Alpine/busybox base-image references and how nemoclaw-start
# is invoked, to assess whether the busybox env error format is reachable.
rg -n -i 'alpine|busybox' --type=ts --type=dockerfile 2>/dev/null
fd -i 'Dockerfile' | xargs -I{} rg -n -i 'FROM ' {}
rg -n 'nemoclaw-start' --type=ts -C3
rg -n 'openshellSandboxCommandEnvValue' --type=ts -C5Repository: NVIDIA/NemoClaw
Length of output: 25767
🏁 Script executed:
# Inspect the diagnostic implementation, its tests, and the custom-Dockerfile contract.
fd -i 'docker-gpu-pre-rollback-diagnostics' .
fd -i 'Dockerfile*' .
ast-grep outline src/lib/onboard/docker-gpu-pre-rollback-diagnostics.ts
sed -n '1,240p' src/lib/onboard/docker-gpu-pre-rollback-diagnostics.ts
rg -n -i 'custom Dockerfile|custom image|BASE_IMAGE|alpine|busybox|nemoclaw-start' src tests Dockerfile* 2>/dev/null | head -n 300Repository: NVIDIA/NemoClaw
Length of output: 46325
🌐 Web query:
Current BusyBox env applet error output for a missing executable in current Alpine Linux releases, including whether it uses "can't execute '<name>': No such file or directory"
💡 Result:
In current Alpine Linux releases, when the BusyBox env applet fails to find or execute a requested binary, it outputs the error message: "env: can't execute '': No such file or directory" [1][2][3]. This behavior is standard for the BusyBox env implementation found in Alpine Linux [4][5]. The "No such file or directory" message is a generic error that indicates the shell or system utility cannot locate the specified executable within the current PATH or, in some cases, that the executable itself is present but missing required dynamic libraries (such as when a glibc-linked binary is run on an Alpine musl-based system) [6][1][7]. This error is distinct from "env: unrecognized option", which occurs if a script attempts to use GNU env features (like the -S flag) that are not supported by the BusyBox version of env [4][5][8]. If you encounter this specific "can't execute" error, it confirms that env successfully parsed its own arguments but failed during the execvp call to launch the target command [1][3].
Citations:
- 1: https://stackoverflow.com/questions/45973481/alpine-linux-in-docker-container-env-cant-execute-node-no-such-file-or
- 2: [Bug]: env: can't execute 'bash': No such file or directory smallstep/certificates#1115
- 3: env: can't execute 'bash': No such file or directory concourse/concourse#8662
- 4: https://forum.gitlab.com/t/error-usr-bin-env-unrecognized-option-s-with-alpine-linux-image-causes-ci-script-to-fail/64063
- 5: Terminus 3 doesn't work with alpine images pantheon-systems/terminus#2272
- 6: https://stackoverflow.com/questions/2195664/my-busybox-does-not-execute-non-applet-utility-commands
- 7: https://unix.stackexchange.com/questions/433444/cant-run-script-file-in-docker-no-such-file-or-directory
- 8: Alpine compatibility: BusyBox-safe entrypoint + portable non-interactive shell path google-gemini/gemini-cli#24198
🏁 Script executed:
# Exercise the exact diagnostic pattern against GNU and BusyBox formats, and inspect
# the documented custom-image path without executing repository code.
python3 - <<'PY'
import re
pattern = re.compile(
r"(?:^|\n)(?:/usr/bin/)?env: ['\u2018]?nemoclaw-start['\u2019]?: "
r"No such file or directory(?:\r?\n|$)"
)
samples = {
"gnu_ascii": "/usr/bin/env: 'nemoclaw-start': No such file or directory\n",
"gnu_curly": "env: ‘nemoclaw-start’: No such file or directory\n",
"busybox": "env: can't execute 'nemoclaw-start': No such file or directory\n",
"unrelated": "env: can't execute 'other-command': No such file or directory\n",
}
for name, value in samples.items():
print(f"{name}={bool(pattern.search(value))}")
PY
rg -n -C 5 -i 'custom Dockerfile|--from|custom image|base image' README.md docs src/lib/onboard test 2>/dev/null | head -n 240
sed -n '1,210p' src/lib/onboard/build-context.ts
sed -n '1,180p' test/onboard-custom-dockerfile.test.tsRepository: NVIDIA/NemoClaw
Length of output: 34745
Match BusyBox env failures for custom images.
When a custom Alpine/BusyBox image cannot start nemoclaw-start, its log uses env: can't execute 'nemoclaw-start': No such file or directory. The current pattern does not match this format, so the missing-command hint is omitted. Add a BusyBox alternative and a regression test.
🤖 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-pre-rollback-diagnostics.ts` around lines 28 - 31,
Add a BusyBox-formatted alternative to MISSING_MANAGED_STARTUP_COMMAND_LOG so it
matches `env: can't execute 'nemoclaw-start': No such file or directory` while
preserving the existing GNU env format, and add a regression test covering the
custom Alpine/BusyBox failure and missing-command hint.
apurvvkumaria
left a comment
apurvvkumaria
left a comment
There was a problem hiding this comment.
Reviewed current head 21f737c. The change is safe to merge and I found no blocking defect. The current audit failures are inherited or time-dependent and this PR does not change dependencies, the lockfile, or audit configuration.\n\nNon-blocking fast-follow: recognize the BusyBox form env: cannot execute nemoclaw-start: No such file or directory in MISSING_MANAGED_STARTUP_COMMAND_LOG and add a narrow regression test. This would improve compatibility diagnostics without needing to delay this PR.
|
Current-main readiness follow-up for exact head 21f737c: this approved head is now 44 commits behind current main and still has red required aggregate and E2E checks from the older dependency/audit state. Please refresh the author branch onto current main so the inherited security remediations and current workflow contracts are included, then let exact-head required checks run. I am not rerunning the stale workflows or editing the contributor branch. |
6 participants
Summary
nemoclaw onboardrecreates the sandbox container with a restart-safe startup command, then waits for the OpenShell supervisor to reconnect. When the container cannot start, NemoClaw captures a pre-rollback diagnostics bundle — which already classifies why the container died, including its exit code — and then rolls back, deleting that container.The user-facing failure block was printed after the rollback. It re-inspected the container, found it gone, and degraded to the weakest verdict available:
The accurate verdict had been computed moments earlier and written only to disk:
So the cause was known, discarded, and the operator was shown GPU escape hatches that do not apply to it.
Changes
env: 'nemoclaw-start': No such file or directoryselects the missing-managed-startup diagnosis; other exit-127 failures retain the generic patched-container verdict.hints, kept separate fromsummaryLinesso the on-disk summary stays machine-readablekey=value.Why this shape
#7996 was reported as "
onboard --fromdoes not inject the NemoClaw Python runtime scripts", with a suggested fix indockerfile-patch.ts. That fix would contradict a documented contract:--fromuses the supplied Dockerfile as the complete sandbox image and deliberately does not layer it over the managed runtime. Seedocs/deployment/install-openclaw-plugins.mdx("Do not usesandbox-baseas the final custom image. It is an intermediate dependency image."),docs/reference/troubleshooting.mdx, and theREMOVE-WHENnote insrc/lib/onboard/custom-openclaw-runtime-diagnosis.ts.That module already carries the right message for this scenario, but it runs inside
verifyDeployment. This failure aborts onboarding during container recreate, well before deployment verification is reached, so the message never had a chance to fire. This PR makes the earlier abort say the same thing.The change is not specific to custom images: any recreated container that dies before the supervisor reconnects now keeps its cause.
Quality Gates
137c1746ecfa9180dc7fc4a9200b8992e9b87fadagainst based5b64a72a5fcb8299e8cec1c2f22746b5c0a7f32(treec4b722391333e26541b2be6788b9fbfa254fd41c; stable patch292629d4d92ef59ecd3986540d562d94d237eeec). All nine security categories PASS; no findings.Documentation Writer Review
docs-updateddocs/reference/troubleshooting.mdxto document exact-log-gated exit-127 diagnosis, conditional pre-rollback diagnostics availability, rollback state, and safe exact-container cleanup. Reviewed every changed user-facing string, code comment, test title, commit message, and the OpenClaw-specific guidance against the final source and tests.npm run docspassed with 0 errors and 2 existing warnings; focused tests passed 46/46 andnpm run validate:prpassed.Verification
Reproduced on Ubuntu 24.04 x86_64 with the reporter's Dockerfile verbatim, on the same base commit before and after the change. The container crash-loops with
env: 'nemoclaw-start': No such file or directoryand exit 127.Before — cause discarded:
After — cause preserved and actionable:
The diagnostics path is printed only when collection returned one. If rollback restored the pre-patch sandbox, no sandbox-deletion command is suggested. If a replacement container is confirmed present, cleanup names only that exact container; unknown state produces no deletion command.
Regression tests cover the exact-log-gated missing-startup classification, exit 127 without that log, exit 125, stale-vs-fresh verdict preference in both directions, diagnostics availability, rollback state, exact-container cleanup, and the hand-off from the create path. Exact-head focused tests passed 46/46;
npm run validate:prpassed;npm run docscompleted with 0 errors and 2 existing warnings.Platform scope: reproduced and verified on Ubuntu 24.04 x86_64, matching the reporter's environment. Other platforms not exercised; the changed code is platform-independent.
Fixes #7996
Signed-off-by: yanyunl1991 yanyunl@nvidia.com
Summary by CodeRabbit
Bug Fixes
Documentation