Skip to content

fix(onboard): surface cluster image build failures - #6623

Merged
cv merged 4 commits into
mainfrom
fix/6622_docker_build_diagnostics
Jul 10, 2026
Merged

fix(onboard): surface cluster image build failures#6623
cv merged 4 commits into
mainfrom
fix/6622_docker_build_diagnostics

Conversation

@chengjiew

@chengjiew chengjiew commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Preserve quiet cluster-image builds on success, but surface captured and redacted Docker diagnostics when the internal build fails. This turns the opaque exit-only failure reported on WSL2 into actionable output without exposing credential-shaped values.

Related Issue

Refs #6622

Changes

  • Return captured stdout and stderr from the cluster-image Docker runner boundary.
  • Append unified, redacted build diagnostics to ClusterImagePatchError only on failure.
  • Add regression coverage proving the Docker reason is visible while token-shaped content stays redacted.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: failure diagnostics now expose the Docker error already produced by the runtime; no command, option, or documented workflow changes.
  • 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: pending maintainer review of the onboarding diagnostic boundary.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npx vitest run --project cli src/lib/cluster-image-patch.test.ts (23 passed); npm run typecheck:cli passed; npx prek run --all-files passed.
  • Applicable broad gate passed — npm test completed with the changed suite passing but the unrelated src/lib/shields/flow.test.ts:500 process-preemption test failing; the same test fails when rerun alone.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Chengjie Wang chengjiew@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced Docker image build-failure diagnostics to include clearer context in thrown errors.
    • Redacted embedded credentials/tokens from build and spawn error output, including in logs.
    • Improved diagnostic formatting by stripping terminal control characters, normalizing host paths, and truncating overly long diagnostics.
  • Tests
    • Updated and added test cases to simulate Docker build/spawn failures containing credential material and verify redaction.
    • Added assertions for diagnostic truncation behavior and that non-sensitive failure context is still surfaced.

Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a2e70aa5-e285-47e1-96c7-335a8354cd48

📥 Commits

Reviewing files that changed from the base of the PR and between c88c1c1 and 1e1972e.

📒 Files selected for processing (5)
  • src/lib/cluster-image-patch.test.ts
  • src/lib/cluster-image-patch.ts
  • src/lib/sandbox-base-image-resolution.test.ts
  • src/lib/sandbox-base-image.test.ts
  • src/lib/sandbox-base-image.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/lib/cluster-image-patch.test.ts
  • src/lib/cluster-image-patch.ts

📝 Walkthrough

Walkthrough

ensurePatchedClusterImage now propagates Docker build output, formats failure diagnostics, and verifies that diagnostic messages expose useful errors without leaking credentials. Sandbox base image failures apply the same redaction, path normalization, control-character stripping, and truncation behavior.

Changes

Docker build diagnostic handling

Layer / File(s) Summary
Diagnostic formatting and redaction
src/lib/sandbox-base-image.ts, src/lib/sandbox-base-image.test.ts, src/lib/sandbox-base-image-resolution.test.ts
Build diagnostics include process errors and output, redact credentials, remove terminal controls, normalize host paths, enforce a size limit, and are validated across sandbox failure paths.
Cluster image failure propagation
src/lib/cluster-image-patch.ts, src/lib/cluster-image-patch.test.ts
The runner returns process output through RunResult, and Docker build errors append sanitized diagnostics with coverage for bearer and basic-auth redaction.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested labels: bug-fix, security, area: onboarding

Suggested reviewers: cv, ericksoa

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: surfacing cluster image build failures during onboarding.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/6622_docker_build_diagnostics

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

@github-code-quality

github-code-quality Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the main branch.

TypeScript / code-coverage/cli

The overall coverage in the fix/6622_docker_buil... branch remains at 77%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.
File main 2b84a04 fix/6622_docker_buil... 1e1972e +/-
src/lib/sandbox-base-image.ts 99% 94% -5%
src/lib/onboard...eway-cleanup.ts 58% 55% -3%
src/lib/cluster-image-patch.ts 90% 90% 0%

Updated July 10, 2026 17:28 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: overlayfs-autofix, sandbox-rebuild, cloud-onboard
Optional E2E: full-e2e, rebuild-openclaw

Dispatch hint: overlayfs-autofix,sandbox-rebuild,cloud-onboard

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • overlayfs-autofix (high): Validates the live onboarding overlayfs auto-fix path that invokes ensurePatchedClusterImage and builds/uses the patched OpenShell cluster image.
  • sandbox-rebuild (high): Exercises the sandbox rebuild lifecycle and base-image resolution/rebuild integration affected by sandbox-base-image diagnostics and failure handling changes.
  • cloud-onboard (high): Covers full hosted onboarding through the public install path and sandbox base image resolution on a clean runner, guarding against regressions in installer/onboarding behavior from the shared base-image changes.

Optional E2E

  • full-e2e (high): Useful broad smoke coverage for install, onboard, sandbox startup, and inference against the changed base image and cluster image plumbing, but less focused than the required overlayfs/base-image jobs.
  • rebuild-openclaw (high): Adjacent confidence for OpenClaw image rebuild behavior and base-image integration, especially if reviewers want additional validation beyond sandbox-rebuild.

New E2E recommendations

  • security-diagnostics-redaction (high): Existing live E2E jobs mainly validate successful image/onboarding paths and may not force docker build/spawn failures with secrets in stdout/stderr/error. The changed behavior is specifically failure-path diagnostic redaction and truncation.
    • Suggested test: Add a fault-injection live E2E that forces sandbox base image and patched cluster image docker build failures, injects representative registry credentials/tokens/credentialed URLs/control characters/host paths into captured output, and asserts uploaded logs/artifacts contain useful diagnostics but no secrets or raw host paths.

Dispatch hint

  • Workflow: .github/workflows/e2e.yaml
  • jobs input: overlayfs-autofix,sandbox-rebuild,cloud-onboard

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: ubuntu-repo-cloud-openclaw
Optional E2E targets: ubuntu-repo-cloud-langchain-deepagents-code

Dispatch required E2E targets:

  • gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-openclaw

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • ubuntu-repo-cloud-openclaw: Changes affect sandbox base-image resolution/build diagnostics and patched cluster image build failure handling, both on the Docker-backed onboarding path exercised by the live-supported Ubuntu OpenClaw target.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-openclaw

Optional E2E targets

  • ubuntu-repo-cloud-langchain-deepagents-code: Optional adjacent coverage for the same shared sandbox base-image and Docker build diagnostic surface under the other live-supported onboarding profile.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-langchain-deepagents-code

Relevant changed files

  • src/lib/cluster-image-patch.ts
  • src/lib/sandbox-base-image.ts

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Large sandbox base-image resolution test hotspot grew instead of being offset or extracted.
Open items: 0 required · 1 warning · 0 suggestions · 8 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 1 new item found

Action checklist

  • PRA-1 Resolve or justify: Large sandbox base-image resolution test hotspot grew instead of being offset or extracted in src/lib/sandbox-base-image-resolution.test.ts:198
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Acceptance clause
  • PRA-T5 Add or justify test follow-up: Acceptance clause
  • PRA-T6 Add or justify test follow-up: Acceptance clause
  • PRA-T7 Add or justify test follow-up: Acceptance clause
  • PRA-T8 Add or justify test follow-up: Acceptance clause

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture src/lib/sandbox-base-image-resolution.test.ts:198 Extract the new local rebuild spawn-error diagnostic case, or shared resolution-mock setup it needs, into a focused helper or smaller test module, or offset the added lines by shrinking existing duplicated setup in this file.
Review findings by urgency: 0 required fixes, 1 item to resolve/justify, 0 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Large sandbox base-image resolution test hotspot grew instead of being offset or extracted

  • Location: src/lib/sandbox-base-image-resolution.test.ts:198
  • Category: architecture
  • Problem: The PR adds another 28 lines to an already large sandbox-base-image resolution test file, continuing monolith growth in a current hotspot rather than extracting focused helpers or offsetting growth elsewhere.
  • Impact: Continued growth in this already-large regression suite makes sandbox base-image resolution behavior harder to audit and increases the cost of future deterministic regression review around image trust and local-build fallback paths.
  • Recommended action: Extract the new local rebuild spawn-error diagnostic case, or shared resolution-mock setup it needs, into a focused helper or smaller test module, or offset the added lines by shrinking existing duplicated setup in this file.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/sandbox-base-image-resolution.test.ts` around the inserted test and compare the file length/growth reported by the scope context; confirm whether the new diagnostic coverage was extracted or offset.
  • Missing regression test: Existing checked-in coverage exercises the new behavior; this finding is about test-suite structure, so no additional behavior regression test is missing if the same assertion coverage is preserved after extraction.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/sandbox-base-image-resolution.test.ts` around the inserted test and compare the file length/growth reported by the scope context; confirm whether the new diagnostic coverage was extracted or offset.
  • Evidence: Scope/risk context reports src/lib/sandbox-base-image-resolution.test.ts grew from 452 to 480 lines (+28) and flags it as a current monolith hotspot with rationale: "Current monolith grew by 20 or more lines; extract or offset the growth before merge." Diff shows a new test, "redacts a local rebuild spawn error before logging it", inserted into the existing describe block rather than extracting shared setup or moving focused diagnostics coverage to a smaller file.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

  • None.
Simplification opportunities: 1 possible cut

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-1 shrink (src/lib/sandbox-base-image-resolution.test.ts:198): Inline growth in the large resolution test hotspot
    • Replacement: Move the diagnostic-specific case or repeated resolution mock setup into a focused helper/smaller test file while preserving the same assertions.
    • Net: 0 lines
    • Safety boundary: Do not remove the redaction/local-build failure assertions; preserve security-sensitive diagnostic coverage.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Add or identify targeted runtime/integration validation for the changed behavior; do not report external E2E job pass/fail here.. Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/cluster-image-patch.ts, src/lib/sandbox-base-image.ts. Checked-in unit/mocked-boundary coverage is sufficient for the changed deterministic behavior, including diagnostic visibility, redaction, truncation, spawn errors, and quiet-success output. Because the changed source files are Docker/sandbox infrastructure, targeted runtime validation would improve confidence.
  • PRA-T2 Runtime validation — Optional runtime follow-up: force a real `docker build` failure through the `runner.run`/`dockerBuild` path with `suppressOutput: true` and verify the user-facing failure includes redacted BuildKit diagnostics while success output remains quiet.. Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/cluster-image-patch.ts, src/lib/sandbox-base-image.ts. Checked-in unit/mocked-boundary coverage is sufficient for the changed deterministic behavior, including diagnostic visibility, redaction, truncation, spawn errors, and quiet-success output. Because the changed source files are Docker/sandbox infrastructure, targeted runtime validation would improve confidence.
  • PRA-T3 Runtime validation — Optional platform follow-up: reproduce the linked WSL2 Windows 11 ARM64 rebuild scenario and confirm the newly surfaced diagnostics identify the underlying failure cause.. Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/cluster-image-patch.ts, src/lib/sandbox-base-image.ts. Checked-in unit/mocked-boundary coverage is sufficient for the changed deterministic behavior, including diagnostic visibility, redaction, truncation, spawn errors, and quiet-success output. Because the changed source files are Docker/sandbox infrastructure, targeted runtime validation would improve confidence.
  • PRA-T4 Acceptance clause — After successfully adding a messaging channel (e.g. Slack) via `nemoclaw my-assistant channels add slack`, the subsequent sandbox rebuild fails with "docker build exited with status 1". — add test evidence or identify existing coverage. The PR does not make the rebuild succeed; it changes diagnostics for Docker build failures.
  • PRA-T5 Acceptance clause — Manual docker build using the same Dockerfile with `--build-arg BASE_IMAGE=ghcr.io/nvidia/nemoclaw/sandbox-base:v0.0.77` completes successfully (all 77 build steps pass), confirming the Dockerfile itself is not the issue on this platform. — add test evidence or identify existing coverage. Static review did not execute Docker builds, and the diff does not add platform runtime validation for this manual build.
  • PRA-T6 Acceptance clause — - **Platform scope:** Platform-specific: fails on WSL2 (Windows 11 ARM64); verified working on DGX Spark — add test evidence or identify existing coverage. The PR adds platform-neutral diagnostic surfacing and no WSL2/ARM64-specific branch or checked-in platform validation.
  • PRA-T7 Acceptance clause — - **Regression:** Yes — worked with lkg v0.0.55 — add test evidence or identify existing coverage. The diff does not compare behavior against v0.0.55; it improves diagnostics available when the regression occurs.
  • PRA-T8 Acceptance clause — 1. On a Windows 11 ARM64 machine with WSL2 Ubuntu-24.04 and Docker Desktop, install NemoClaw: ``` curl -fsSL https://www.nvidia.com/nemoclaw.sh | bash ``` — add test evidence or identify existing coverage. No installer files changed and static review did not execute the installer or WSL2 runtime path.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Large sandbox base-image resolution test hotspot grew instead of being offset or extracted

  • Location: src/lib/sandbox-base-image-resolution.test.ts:198
  • Category: architecture
  • Problem: The PR adds another 28 lines to an already large sandbox-base-image resolution test file, continuing monolith growth in a current hotspot rather than extracting focused helpers or offsetting growth elsewhere.
  • Impact: Continued growth in this already-large regression suite makes sandbox base-image resolution behavior harder to audit and increases the cost of future deterministic regression review around image trust and local-build fallback paths.
  • Recommended action: Extract the new local rebuild spawn-error diagnostic case, or shared resolution-mock setup it needs, into a focused helper or smaller test module, or offset the added lines by shrinking existing duplicated setup in this file.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/sandbox-base-image-resolution.test.ts` around the inserted test and compare the file length/growth reported by the scope context; confirm whether the new diagnostic coverage was extracted or offset.
  • Missing regression test: Existing checked-in coverage exercises the new behavior; this finding is about test-suite structure, so no additional behavior regression test is missing if the same assertion coverage is preserved after extraction.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/sandbox-base-image-resolution.test.ts` around the inserted test and compare the file length/growth reported by the scope context; confirm whether the new diagnostic coverage was extracted or offset.
  • Evidence: Scope/risk context reports src/lib/sandbox-base-image-resolution.test.ts grew from 452 to 480 lines (+28) and flags it as a current monolith hotspot with rationale: "Current monolith grew by 20 or more lines; extract or offset the growth before merge." Diff shows a new test, "redacts a local rebuild spawn error before logging it", inserted into the existing describe block rather than extracting shared setup or moving focused diagnostics coverage to a smaller file.

Workflow run details

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.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Spawn error code discarded in local-build failure diagnostic.
Open items: 0 required · 2 warnings · 2 suggestions · 6 test follow-ups
Since last review: 2 prior items resolved · 0 still apply · 4 new items found

Action checklist

  • PRA-1 Resolve or justify: Spawn error code discarded in local-build failure diagnostic in src/lib/sandbox-base-image.ts:292
  • PRA-4 Resolve or justify: Missing regression test for spawn error code preservation in local-build failure in src/lib/sandbox-base-image-resolution.test.ts:215
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Missing regression test for spawn error code preservation in local-build failure
  • PRA-T5 Add or justify test follow-up: Acceptance clause
  • PRA-T6 Add or justify test follow-up: Local-build spawn error redaction workaround (sandbox-base-image.ts:292)
  • PRA-2 In-scope improvement: Single-use config array for home/tmp path redaction in src/lib/sandbox-base-image.ts:69
  • PRA-3 In-scope improvement: Over-engineered test credential construction in src/lib/sandbox-base-image.test.ts:59

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify correctness src/lib/sandbox-base-image.ts:292 Preserve the spawn error code (e.g., error.code) in the logged diagnostic while still redacting secrets, or add a structured error code that callers can act on. Align with cluster-image-patch.ts which surfaces spawnCause in the thrown error.
PRA-2 Improvement architecture src/lib/sandbox-base-image.ts:69 Replace with a named constant or inline the three string replacements directly. Hoist to a shared constant only when a second consumer exists.
PRA-3 Improvement architecture src/lib/sandbox-base-image.test.ts:59 Replace with a single direct behavior test passing a raw string containing all credential types inline, or use a shared fixture constant. Remove the join/template scaffolding.
PRA-4 Resolve/justify tests src/lib/sandbox-base-image-resolution.test.ts:215 Add a test case that mocks dockerBuild to return a spawn error with error.code === "ENOENT" (or EACCES), triggers resolveLocalCandidate with local build enabled, and asserts the console.error output contains the error code while still redacting any secrets.
Review findings by urgency: 0 required fixes, 2 items to resolve/justify, 2 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Spawn error code discarded in local-build failure diagnostic

  • Location: src/lib/sandbox-base-image.ts:292
  • Category: correctness
  • Problem: The local-build failure path in resolveLocalCandidate replaces the raw spawn error message with a generic "(process launch failed)" string, discarding actionable error codes (ENOENT, EACCES, etc.). The cluster-image-patch path preserves the spawn cause in the thrown error, creating a behavior mismatch between two similar Docker build failure paths.
  • Impact: Users and automation cannot distinguish between missing docker, permission issues, or other spawn failures when local sandbox base image build fails, increasing MTTR for onboarding failures.
  • Recommended action: Preserve the spawn error code (e.g., error.code) in the logged diagnostic while still redacting secrets, or add a structured error code that callers can act on. Align with cluster-image-patch.ts which surfaces spawnCause in the thrown error.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read src/lib/sandbox-base-image.ts lines 285-295 and compare with cluster-image-patch.ts lines 300-310; run local build failure scenario and observe console.error output.
  • Missing regression test: Add test in sandbox-base-image-resolution.test.ts that triggers a local build spawn failure (ENOENT/EACCES) and asserts the logged diagnostic contains the error code or a non-generic actionable message, while still redacting any secrets.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read src/lib/sandbox-base-image.ts lines 285-295 and compare with cluster-image-patch.ts lines 300-310; run local build failure scenario and observe console.error output.
  • Evidence: src/lib/sandbox-base-image.ts:292 changes error detail to generic string src/lib/cluster-image-patch.ts:305-307 preserves spawnCause in thrown error Test in cluster-image-patch.test.ts:330-345 expects spawnCause in error

PRA-4 Resolve/justify — Missing regression test for spawn error code preservation in local-build failure

  • Location: src/lib/sandbox-base-image-resolution.test.ts:215
  • Category: tests
  • Problem: The new test "redacts a local rebuild spawn error before logging it" (lines 215-240) verifies token redaction and generic "process launch failed" message, but does not assert that actionable error codes (ENOENT, EACCES, etc.) are preserved in the logged diagnostic. This leaves the behavior mismatch in F-001 without automated coverage.
  • Impact: If a future change accidentally preserves or discards error codes, there is no test to catch the regression. Users continue to receive non-actionable diagnostics for spawn failures.
  • Recommended action: Add a test case that mocks dockerBuild to return a spawn error with error.code === "ENOENT" (or EACCES), triggers resolveLocalCandidate with local build enabled, and asserts the console.error output contains the error code while still redacting any secrets.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read src/lib/sandbox-base-image-resolution.test.ts lines 215-240; the test mocks dockerMocks.build with status: null, error: new Error("spawn docker EACCES: Bearer ..."). Add a variant with error.code set and assert the code appears in logged output.
  • Missing regression test: Add test: "preserves spawn error code (ENOENT/EACCES) in local-build failure diagnostic while redacting secrets" — mock buildResult.error.code = "ENOENT", call resolveLocalCandidate with local build enabled, assert console.error output contains "ENOENT" and not the Bearer token.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read src/lib/sandbox-base-image-resolution.test.ts lines 215-240; the test mocks dockerMocks.build with status: null, error: new Error("spawn docker EACCES: Bearer ..."). Add a variant with error.code set and assert the code appears in logged output.
  • Evidence: src/lib/sandbox-base-image-resolution.test.ts:215-240 test only checks for generic "process launch failed" src/lib/sandbox-base-image.ts:292 discards error.code F-001 documents the behavior mismatch

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-2 Improvement — Single-use config array for home/tmp path redaction

  • Location: src/lib/sandbox-base-image.ts:69
  • Category: architecture
  • Problem: The array [[process.env.HOME, ""], [os.homedir(), ""], [os.tmpdir(), "<tmp>"]] at line 69 is used only once in formatBuildFailureDiagnostics. No evidence users/CI override these paths; a constant or inline replacements would be clearer until a second consumer appears.
  • Impact: Adds cognitive overhead (config pattern implies configurability) without benefit. Minor but accumulates.
  • Suggested action: Replace with a named constant or inline the three string replacements directly. Hoist to a shared constant only when a second consumer exists.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read src/lib/sandbox-base-image.ts lines 69-75; grep for other usages of the pattern.
  • Missing regression test: N/A — simplification candidate; no regression test needed.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: src/lib/sandbox-base-image.ts:69 single-use config array No other references to this pattern in codebase

PRA-3 Improvement — Over-engineered test credential construction

  • Location: src/lib/sandbox-base-image.test.ts:59
  • Category: architecture
  • Problem: Test constructs credential strings via array joins and template literals (e.g., `["registry", "password"].join("-")`, `https://build-user:${urlPassword}&#64;registry.example.test\`\) solely to feed the redaction function. The underlying redactFull already has patterns for Basic, Digest, Bearer, cookies, URL userinfo, and query tokens.
  • Impact: Test scaffolding obscures the actual behavior under test (redaction of known credential patterns) and adds maintenance burden when patterns change.
  • Suggested action: Replace with a single direct behavior test passing a raw string containing all credential types inline, or use a shared fixture constant. Remove the join/template scaffolding.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read src/lib/sandbox-base-image.test.ts lines 55-90; compare with redactFull patterns in src/lib/security/redact.ts.
  • Missing regression test: N/A — simplification candidate; existing test covers behavior, just with extra scaffolding.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: src/lib/sandbox-base-image.test.ts:59,67,69,86 construct credentials via joins/templates src/lib/security/redact.ts has patterns for all tested credential types
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Add or identify targeted runtime/integration validation for the changed behavior; do not report external E2E job pass/fail here.. Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/cluster-image-patch.ts, src/lib/sandbox-base-image.ts. PR adds comprehensive mocked-seam tests for the exact error-message contract change (redacted diagnostics surfaced). 9 new tests in sandbox-base-image.test.ts cover formatBuildFailureDiagnostics boundary; 1 new test in cluster-image-patch.test.ts covers cluster-image-patch error message; 1 new test in sandbox-base-image-resolution.test.ts covers local-build spawn error redaction. All failure branches, empty streams, Buffer inputs, and credential types covered at mocked boundary. riskPlan has zero required invariants/jobs. Runtime (real Docker) validation is a general infrastructure note, not a missing regression test for this PR's scope.
  • PRA-T2 Runtime validation — Add test preserving spawn error code (ENOENT/EACCES) in local-build failure diagnostic while redacting secrets (F-004). Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/cluster-image-patch.ts, src/lib/sandbox-base-image.ts. PR adds comprehensive mocked-seam tests for the exact error-message contract change (redacted diagnostics surfaced). 9 new tests in sandbox-base-image.test.ts cover formatBuildFailureDiagnostics boundary; 1 new test in cluster-image-patch.test.ts covers cluster-image-patch error message; 1 new test in sandbox-base-image-resolution.test.ts covers local-build spawn error redaction. All failure branches, empty streams, Buffer inputs, and credential types covered at mocked boundary. riskPlan has zero required invariants/jobs. Runtime (real Docker) validation is a general infrastructure note, not a missing regression test for this PR's scope.
  • PRA-T3 Runtime validation — Consider focused integration test for docker build failure path in future PR if mock boundary proves insufficient (not required for this merge). Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/cluster-image-patch.ts, src/lib/sandbox-base-image.ts. PR adds comprehensive mocked-seam tests for the exact error-message contract change (redacted diagnostics surfaced). 9 new tests in sandbox-base-image.test.ts cover formatBuildFailureDiagnostics boundary; 1 new test in cluster-image-patch.test.ts covers cluster-image-patch error message; 1 new test in sandbox-base-image-resolution.test.ts covers local-build spawn error redaction. All failure branches, empty streams, Buffer inputs, and credential types covered at mocked boundary. riskPlan has zero required invariants/jobs. Runtime (real Docker) validation is a general infrastructure note, not a missing regression test for this PR's scope.
  • PRA-T4 Missing regression test for spawn error code preservation in local-build failure — Add a test case that mocks dockerBuild to return a spawn error with error.code === "ENOENT" (or EACCES), triggers resolveLocalCandidate with local build enabled, and asserts the console.error output contains the error code while still redacting any secrets.
  • PRA-T5 Acceptance clause — Fix the WSL2 root cause (Issue [WSL2][Sandbox] nemoclaw rebuild fails with "docker build exited with status 1" after channels add #6622 platform-specific failure on Windows 11 ARM64) — add test evidence or identify existing coverage. QA comment (issue [WSL2][Sandbox] nemoclaw rebuild fails with "docker build exited with status 1" after channels add #6622 comment 4937347050) confirms this PR is "a diagnostics fix only… it does not make the reported rebuild succeed"; root cause fix tracked separately
  • PRA-T6 Local-build spawn error redaction workaround (sandbox-base-image.ts:292) — Add test asserting error.code (ENOENT/EACCES) preserved in logged diagnostic while secrets redacted (F-004). src/lib/sandbox-base-image.ts:292 discards error.code; cluster-image-patch.ts preserves spawnCause; test at sandbox-base-image-resolution.test.ts:215-240 only checks generic message
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Spawn error code discarded in local-build failure diagnostic

  • Location: src/lib/sandbox-base-image.ts:292
  • Category: correctness
  • Problem: The local-build failure path in resolveLocalCandidate replaces the raw spawn error message with a generic "(process launch failed)" string, discarding actionable error codes (ENOENT, EACCES, etc.). The cluster-image-patch path preserves the spawn cause in the thrown error, creating a behavior mismatch between two similar Docker build failure paths.
  • Impact: Users and automation cannot distinguish between missing docker, permission issues, or other spawn failures when local sandbox base image build fails, increasing MTTR for onboarding failures.
  • Recommended action: Preserve the spawn error code (e.g., error.code) in the logged diagnostic while still redacting secrets, or add a structured error code that callers can act on. Align with cluster-image-patch.ts which surfaces spawnCause in the thrown error.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read src/lib/sandbox-base-image.ts lines 285-295 and compare with cluster-image-patch.ts lines 300-310; run local build failure scenario and observe console.error output.
  • Missing regression test: Add test in sandbox-base-image-resolution.test.ts that triggers a local build spawn failure (ENOENT/EACCES) and asserts the logged diagnostic contains the error code or a non-generic actionable message, while still redacting any secrets.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read src/lib/sandbox-base-image.ts lines 285-295 and compare with cluster-image-patch.ts lines 300-310; run local build failure scenario and observe console.error output.
  • Evidence: src/lib/sandbox-base-image.ts:292 changes error detail to generic string src/lib/cluster-image-patch.ts:305-307 preserves spawnCause in thrown error Test in cluster-image-patch.test.ts:330-345 expects spawnCause in error

PRA-2 Improvement — Single-use config array for home/tmp path redaction

  • Location: src/lib/sandbox-base-image.ts:69
  • Category: architecture
  • Problem: The array [[process.env.HOME, ""], [os.homedir(), ""], [os.tmpdir(), "<tmp>"]] at line 69 is used only once in formatBuildFailureDiagnostics. No evidence users/CI override these paths; a constant or inline replacements would be clearer until a second consumer appears.
  • Impact: Adds cognitive overhead (config pattern implies configurability) without benefit. Minor but accumulates.
  • Suggested action: Replace with a named constant or inline the three string replacements directly. Hoist to a shared constant only when a second consumer exists.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read src/lib/sandbox-base-image.ts lines 69-75; grep for other usages of the pattern.
  • Missing regression test: N/A — simplification candidate; no regression test needed.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: src/lib/sandbox-base-image.ts:69 single-use config array No other references to this pattern in codebase

PRA-3 Improvement — Over-engineered test credential construction

  • Location: src/lib/sandbox-base-image.test.ts:59
  • Category: architecture
  • Problem: Test constructs credential strings via array joins and template literals (e.g., `["registry", "password"].join("-")`, `https://build-user:${urlPassword}&#64;registry.example.test\`\) solely to feed the redaction function. The underlying redactFull already has patterns for Basic, Digest, Bearer, cookies, URL userinfo, and query tokens.
  • Impact: Test scaffolding obscures the actual behavior under test (redaction of known credential patterns) and adds maintenance burden when patterns change.
  • Suggested action: Replace with a single direct behavior test passing a raw string containing all credential types inline, or use a shared fixture constant. Remove the join/template scaffolding.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read src/lib/sandbox-base-image.test.ts lines 55-90; compare with redactFull patterns in src/lib/security/redact.ts.
  • Missing regression test: N/A — simplification candidate; existing test covers behavior, just with extra scaffolding.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: src/lib/sandbox-base-image.test.ts:59,67,69,86 construct credentials via joins/templates src/lib/security/redact.ts has patterns for all tested credential types

PRA-4 Resolve/justify — Missing regression test for spawn error code preservation in local-build failure

  • Location: src/lib/sandbox-base-image-resolution.test.ts:215
  • Category: tests
  • Problem: The new test "redacts a local rebuild spawn error before logging it" (lines 215-240) verifies token redaction and generic "process launch failed" message, but does not assert that actionable error codes (ENOENT, EACCES, etc.) are preserved in the logged diagnostic. This leaves the behavior mismatch in F-001 without automated coverage.
  • Impact: If a future change accidentally preserves or discards error codes, there is no test to catch the regression. Users continue to receive non-actionable diagnostics for spawn failures.
  • Recommended action: Add a test case that mocks dockerBuild to return a spawn error with error.code === "ENOENT" (or EACCES), triggers resolveLocalCandidate with local build enabled, and asserts the console.error output contains the error code while still redacting any secrets.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read src/lib/sandbox-base-image-resolution.test.ts lines 215-240; the test mocks dockerMocks.build with status: null, error: new Error("spawn docker EACCES: Bearer ..."). Add a variant with error.code set and assert the code appears in logged output.
  • Missing regression test: Add test: "preserves spawn error code (ENOENT/EACCES) in local-build failure diagnostic while redacting secrets" — mock buildResult.error.code = "ENOENT", call resolveLocalCandidate with local build enabled, assert console.error output contains "ENOENT" and not the Bearer token.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read src/lib/sandbox-base-image-resolution.test.ts lines 215-240; the test mocks dockerMocks.build with status: null, error: new Error("spawn docker EACCES: Bearer ..."). Add a variant with error.code set and assert the code appears in logged output.
  • Evidence: src/lib/sandbox-base-image-resolution.test.ts:215-240 test only checks for generic "process launch failed" src/lib/sandbox-base-image.ts:292 discards error.code F-001 documents the behavior mismatch

Workflow run details

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.

Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29067622810
Workflow ref: fix/6622_docker_build_diagnostics
Requested targets: (default — all supported)
Requested jobs: overlayfs-autofix
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
overlayfs-autofix ✅ success

cjagwani added 2 commits July 10, 2026 10:10
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
@cv
cv merged commit 90d5cdf into main Jul 10, 2026
57 of 60 checks passed
@cv
cv deleted the fix/6622_docker_build_diagnostics branch July 10, 2026 18:52
cv pushed a commit that referenced this pull request Jul 10, 2026
…nce (#6002) (#6663)

## Summary

Raise the `full-e2e` cold-onboard acceptance budget
(`ONBOARD_BUDGET_SECS`) from **180s → 205s**. This is the umbrella PR
for today's live-E2E failures on `main`; scope and evidence per job
below.

## Failure triage (main, 2026-07-10)

A full `E2E` dispatch on `main` (run
[29124128082](https://github.com/NVIDIA/NemoClaw/actions/runs/29124128082))
came back **78 passed / 5 skipped / 3 failed**. Each failure was
root-caused, not retried blindly:

| Job | Verdict | Root cause |
|-----|---------|-----------|
| `agent-turn-latency` | ✅ flake, self-cleared | Passed on first retry —
hosted-inference timing variance. |
| `full-e2e` | 🔧 **fixed here** | Consistent ~1s overshoot of a
too-tight 180s onboard budget (see below). |
| `rebuild-hermes` | ⏳ **verification pending** | Both observed failures
were infra (`operation was canceled`, `runner lost communication with
the server`) — **not** a test assertion or image-build error. Retry on
post-bump `main` in flight
([29129133666](https://github.com/NVIDIA/NemoClaw/actions/runs/29129133666)).
See "Hermes v0.18" below. |

## `full-e2e` — full analysis (no gaps)

**Symptom.** The `[1/8]-to-first-response` gate (`full-e2e.test.ts:215`)
failed **3 consecutive times**:

| Run | to-first-response | vs 180s budget |
|-----|-------------------|----------------|
|
[29124128082](https://github.com/NVIDIA/NemoClaw/actions/runs/29124128082)
| 180,829 ms | +0.8s |
|
[29125880976](https://github.com/NVIDIA/NemoClaw/actions/runs/29125880976)
| 181,550 ms | +1.5s |
|
[29127670707](https://github.com/NVIDIA/NemoClaw/actions/runs/29127670707)
| 180,602 ms | +0.6s |

**Not flake, not inference, not a code regression** — proven by the
`onboard-progress-budget.json` artifact decomposition:

| | `onboardSecs` | `totalSecs` | headroom | notes |
|---|---|---|---|---|
| Passing run
[29128496025](https://github.com/NVIDIA/NemoClaw/actions/runs/29128496025)
(`f4cd7ea9`) | **163** | 168 | +12s | BuildKit prebuild ✓, 0 classic
steps |
| Failing run
[29127670707](https://github.com/NVIDIA/NemoClaw/actions/runs/29127670707)
(`fcc121d5`) | **173** | 181 | −1s | BuildKit prebuild ✓, 0 classic
steps |

- The entire delta is in the **cold onboard/BuildKit image-build phase**
(163s → 173s, a ~10s run-to-run swing) on **identical, post-#6265
`main`** — both heads are after the Hermes v0.18 bump, so the bump is
not the cause.
- The first hosted agent turn is only **~5–8s** (`totalSecs −
onboardSecs`); inference is a rounding error.
- The 180s cap (introduced 4 days ago in #6265) left only ~7s of
headroom against a phase that varies ~10s with Docker Hub pull speed and
hosted-runner I/O — so slow-build runs tip over.

**Fix.** Raise to **205s**: covers the observed 173s worst case plus
build-variance headroom, while still catching gross onboard regressions
(a real regression blows well past 205s; `MAX_SILENCE_SECS` and
BuildKit-fallback assertions are unchanged).

## Hermes v0.18 context

Today's `main` includes `feat(hermes): upgrade to v0.18 and enable Slack
Block Kit` (#6507, 17:45Z), plus `#6624` (release-matched sandbox bases)
and `#6623` (surface cluster image build failures). Because
`rebuild-hermes` rebuilds the Hermes image, a v0.18-induced regression
*could* in principle surface as runner resource-exhaustion. **This PR
does not yet claim `rebuild-hermes` is a flake** — the in-flight retry
on post-bump `main` is the deciding evidence:
- retry **passes** → confirmed infra flake, no code change needed, this
PR ships as-is;
- retry **fails** (build error / OOM / repeat comms-loss) → v0.18 is
implicated and a fix is added to this branch before merge.

## Test evidence

- `commitlint`, `gitleaks`, test-size/shape budgets: passed
(pre-commit).
- No product-code change; single test-constant edit. Behavioral proof is
the artifact decomposition above.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>

🤖 Generated with [Claude Code](https://claude.com/claude-code)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
* Adjusted end-to-end onboarding timing thresholds by increasing the
“acceptance budget” to allow for typical variability in image build
times.
* Updated test parity mappings to ensure the added live coverage is
correctly linked with the corresponding faster test set.
* Added inline guidance for why timing can fluctuate between runs, and
why the previous cap needed more headroom.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cv pushed a commit that referenced this pull request Jul 11, 2026
## Summary

Release-prep documentation for **v0.0.80**. Adds the `## v0.0.80`
section to `docs/about/release-notes.mdx` summarizing user-facing
changes since v0.0.79, each bullet linking to the relevant deeper page.

Produced via `nemoclaw-contributor-update-docs` (pre-tag path): scanned
`v0.0.79..HEAD`, applied the docs skip list (no violations), and
confirmed the 8 commits that already shipped in-PR docs are complete. No
new pages needed.

## Source summary

- #6507 -> `docs/about/release-notes.mdx`: Hermes v0.18 + Slack Block
Kit (rich rendering, digest-pinned base image).
- #6584 / #6616 -> `docs/about/release-notes.mdx`: host-local OpenRouter
runtime attribution adapter (port `11437`,
`NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT`) and native Deep Agents
`openrouter` provider.
- #6210 / #6292 -> `docs/about/release-notes.mdx`: host corporate proxy
CA import into sandbox trust (`NEMOCLAW_CORPORATE_CA_BUNDLE`,
`NEMOCLAW_CORPORATE_CA_IMPORT`).
- #6624 / #6623 / #6656 -> `docs/about/release-notes.mdx`:
release-matched base-image selection, surfaced cluster-image build
diagnostics, preserved Nemotron profile registration.
- #6629 / #6637 -> `docs/about/release-notes.mdx`: bare `connect`
default-sandbox behavior and route-probe hardening.
- #6634 / #6626 / #6596 / #5569 / #6610 / #6655 ->
`docs/about/release-notes.mdx`: onboarding/recovery preservation,
stale-gateway-PID fix, installer backup message, vLLM label on managed
platforms.
- #6578 / #5670 -> `docs/about/release-notes.mdx`: automatic Hermes
light terminal skin and non-interactive `npx` MCP server startup.

## Verification

`npm run docs`: 0 errors, all internal links resolve (2 pre-existing
hidden-page warnings). `_build/` variants for OpenClaw, Hermes, and Deep
Agents all regenerate with the v0.0.80 section.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>

🤖 Generated with [Claude Code](https://claude.com/claude-code)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Documentation**
  * Added release notes for v0.0.80.
  * Documented Hermes upgrades, including Slack Block Kit rendering.
  * Added details on OpenRouter traffic routing and attribution headers.
* Documented improved proxy certificate handling and sandbox
reliability.
* Highlighted enhanced connection defaults, route-probing safeguards,
onboarding recovery, and terminal/MCP startup behavior.
  * Added references to relevant user-guide documentation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Preserve quiet cluster-image builds on success, but surface captured and
redacted Docker diagnostics when the internal build fails. This turns
the opaque exit-only failure reported on WSL2 into actionable output
without exposing credential-shaped values.

## Related Issue
Refs NVIDIA#6622

## Changes

- Return captured stdout and stderr from the cluster-image Docker runner
boundary.
- Append unified, redacted build diagnostics to `ClusterImagePatchError`
only on failure.
- Add regression coverage proving the Docker reason is visible while
token-shaped content stays redacted.

## Type of Change

- [x] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: failure diagnostics now
expose the Docker error already produced by the runtime; no command,
option, or documented workflow changes.
- [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: pending maintainer
review of the onboarding diagnostic boundary.
- [ ] 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] 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 — `npx vitest run --project cli
src/lib/cluster-image-patch.test.ts` (23 passed); `npm run
typecheck:cli` passed; `npx prek run --all-files` passed.
- [ ] Applicable broad gate passed — `npm test` completed with the
changed suite passing but the unrelated
`src/lib/shields/flow.test.ts:500` process-preemption test failing; the
same test fails when rerun alone.
- [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)
- [ ] 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: Chengjie Wang <chengjiew@nvidia.com>

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Enhanced Docker image build-failure diagnostics to include clearer
context in thrown errors.
* Redacted embedded credentials/tokens from build and spawn error
output, including in logs.
* Improved diagnostic formatting by stripping terminal control
characters, normalizing host paths, and truncating overly long
diagnostics.
* **Tests**
* Updated and added test cases to simulate Docker build/spawn failures
containing credential material and verify redaction.
* Added assertions for diagnostic truncation behavior and that
non-sensitive failure context is still surfaced.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Co-authored-by: Charan Jagwani <cjagwani@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
…nce (NVIDIA#6002) (NVIDIA#6663)

## Summary

Raise the `full-e2e` cold-onboard acceptance budget
(`ONBOARD_BUDGET_SECS`) from **180s → 205s**. This is the umbrella PR
for today's live-E2E failures on `main`; scope and evidence per job
below.

## Failure triage (main, 2026-07-10)

A full `E2E` dispatch on `main` (run
[29124128082](https://github.com/NVIDIA/NemoClaw/actions/runs/29124128082))
came back **78 passed / 5 skipped / 3 failed**. Each failure was
root-caused, not retried blindly:

| Job | Verdict | Root cause |
|-----|---------|-----------|
| `agent-turn-latency` | ✅ flake, self-cleared | Passed on first retry —
hosted-inference timing variance. |
| `full-e2e` | 🔧 **fixed here** | Consistent ~1s overshoot of a
too-tight 180s onboard budget (see below). |
| `rebuild-hermes` | ⏳ **verification pending** | Both observed failures
were infra (`operation was canceled`, `runner lost communication with
the server`) — **not** a test assertion or image-build error. Retry on
post-bump `main` in flight
([29129133666](https://github.com/NVIDIA/NemoClaw/actions/runs/29129133666)).
See "Hermes v0.18" below. |

## `full-e2e` — full analysis (no gaps)

**Symptom.** The `[1/8]-to-first-response` gate (`full-e2e.test.ts:215`)
failed **3 consecutive times**:

| Run | to-first-response | vs 180s budget |
|-----|-------------------|----------------|
|
[29124128082](https://github.com/NVIDIA/NemoClaw/actions/runs/29124128082)
| 180,829 ms | +0.8s |
|
[29125880976](https://github.com/NVIDIA/NemoClaw/actions/runs/29125880976)
| 181,550 ms | +1.5s |
|
[29127670707](https://github.com/NVIDIA/NemoClaw/actions/runs/29127670707)
| 180,602 ms | +0.6s |

**Not flake, not inference, not a code regression** — proven by the
`onboard-progress-budget.json` artifact decomposition:

| | `onboardSecs` | `totalSecs` | headroom | notes |
|---|---|---|---|---|
| Passing run
[29128496025](https://github.com/NVIDIA/NemoClaw/actions/runs/29128496025)
(`f4cd7ea9`) | **163** | 168 | +12s | BuildKit prebuild ✓, 0 classic
steps |
| Failing run
[29127670707](https://github.com/NVIDIA/NemoClaw/actions/runs/29127670707)
(`fcc121d5`) | **173** | 181 | −1s | BuildKit prebuild ✓, 0 classic
steps |

- The entire delta is in the **cold onboard/BuildKit image-build phase**
(163s → 173s, a ~10s run-to-run swing) on **identical, post-NVIDIA#6265
`main`** — both heads are after the Hermes v0.18 bump, so the bump is
not the cause.
- The first hosted agent turn is only **~5–8s** (`totalSecs −
onboardSecs`); inference is a rounding error.
- The 180s cap (introduced 4 days ago in NVIDIA#6265) left only ~7s of
headroom against a phase that varies ~10s with Docker Hub pull speed and
hosted-runner I/O — so slow-build runs tip over.

**Fix.** Raise to **205s**: covers the observed 173s worst case plus
build-variance headroom, while still catching gross onboard regressions
(a real regression blows well past 205s; `MAX_SILENCE_SECS` and
BuildKit-fallback assertions are unchanged).

## Hermes v0.18 context

Today's `main` includes `feat(hermes): upgrade to v0.18 and enable Slack
Block Kit` (NVIDIA#6507, 17:45Z), plus `NVIDIA#6624` (release-matched sandbox bases)
and `NVIDIA#6623` (surface cluster image build failures). Because
`rebuild-hermes` rebuilds the Hermes image, a v0.18-induced regression
*could* in principle surface as runner resource-exhaustion. **This PR
does not yet claim `rebuild-hermes` is a flake** — the in-flight retry
on post-bump `main` is the deciding evidence:
- retry **passes** → confirmed infra flake, no code change needed, this
PR ships as-is;
- retry **fails** (build error / OOM / repeat comms-loss) → v0.18 is
implicated and a fix is added to this branch before merge.

## Test evidence

- `commitlint`, `gitleaks`, test-size/shape budgets: passed
(pre-commit).
- No product-code change; single test-constant edit. Behavioral proof is
the artifact decomposition above.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>

🤖 Generated with [Claude Code](https://claude.com/claude-code)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
* Adjusted end-to-end onboarding timing thresholds by increasing the
“acceptance budget” to allow for typical variability in image build
times.
* Updated test parity mappings to ensure the added live coverage is
correctly linked with the corresponding faster test set.
* Added inline guidance for why timing can fluctuate between runs, and
why the previous cap needed more headroom.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary

Release-prep documentation for **v0.0.80**. Adds the `## v0.0.80`
section to `docs/about/release-notes.mdx` summarizing user-facing
changes since v0.0.79, each bullet linking to the relevant deeper page.

Produced via `nemoclaw-contributor-update-docs` (pre-tag path): scanned
`v0.0.79..HEAD`, applied the docs skip list (no violations), and
confirmed the 8 commits that already shipped in-PR docs are complete. No
new pages needed.

## Source summary

- NVIDIA#6507 -> `docs/about/release-notes.mdx`: Hermes v0.18 + Slack Block
Kit (rich rendering, digest-pinned base image).
- NVIDIA#6584 / NVIDIA#6616 -> `docs/about/release-notes.mdx`: host-local OpenRouter
runtime attribution adapter (port `11437`,
`NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT`) and native Deep Agents
`openrouter` provider.
- NVIDIA#6210 / NVIDIA#6292 -> `docs/about/release-notes.mdx`: host corporate proxy
CA import into sandbox trust (`NEMOCLAW_CORPORATE_CA_BUNDLE`,
`NEMOCLAW_CORPORATE_CA_IMPORT`).
- NVIDIA#6624 / NVIDIA#6623 / NVIDIA#6656 -> `docs/about/release-notes.mdx`:
release-matched base-image selection, surfaced cluster-image build
diagnostics, preserved Nemotron profile registration.
- NVIDIA#6629 / NVIDIA#6637 -> `docs/about/release-notes.mdx`: bare `connect`
default-sandbox behavior and route-probe hardening.
- NVIDIA#6634 / NVIDIA#6626 / NVIDIA#6596 / NVIDIA#5569 / NVIDIA#6610 / NVIDIA#6655 ->
`docs/about/release-notes.mdx`: onboarding/recovery preservation,
stale-gateway-PID fix, installer backup message, vLLM label on managed
platforms.
- NVIDIA#6578 / NVIDIA#5670 -> `docs/about/release-notes.mdx`: automatic Hermes
light terminal skin and non-interactive `npx` MCP server startup.

## Verification

`npm run docs`: 0 errors, all internal links resolve (2 pre-existing
hidden-page warnings). `_build/` variants for OpenClaw, Hermes, and Deep
Agents all regenerate with the v0.0.80 section.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>

🤖 Generated with [Claude Code](https://claude.com/claude-code)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Documentation**
  * Added release notes for v0.0.80.
  * Documented Hermes upgrades, including Slack Block Kit rendering.
  * Added details on OpenRouter traffic routing and attribution headers.
* Documented improved proxy certificate handling and sandbox
reliability.
* Highlighted enhanced connection defaults, route-probing safeguards,
onboarding recovery, and terminal/MCP startup behavior.
  * Added references to relevant user-guide documentation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
apurvvkumaria added a commit that referenced this pull request Jul 17, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Replacement-image rebuild preflight currently drops Docker diagnostics
when the process result contains `Buffer` streams and falls back to a
generic exit-status message. This change reuses the existing hardened
build-diagnostic formatter so release validation gets actionable,
redacted output needed to identify the affected-host failure tracked in
#7111; it does not claim to fix that still-unknown host-specific build
root cause.

## Related Issue

Refs #7111. Builds on the diagnostic hardening from #6623.

## Changes

- Route rebuild preflight process errors, stderr, and stdout through
`formatBuildFailureDiagnostics`.
- Preserve credential redaction, home/temp path normalization,
terminal-control stripping, and the existing 8 KB diagnostic bound.
- Add a regression covering `Buffer` stderr with a credential and
private host path.

## Type of Change

- [x] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: this corrects an internal
formatting path for an existing failure flow without changing commands,
flags, defaults, configuration, workflows, or documented support
boundaries; the canonical troubleshooting docs already cover failed
rebuilds.
- [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: apurvvkumaria reviewed
the exact two-file diff. It adds no process execution, filesystem,
network, authorization, or policy behavior; the reused formatter applies
both credential-redaction layers, strips control characters, normalizes
host paths, and caps output at 8 KB.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [x] PR description includes a `Signed-off-by:` line 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 — `npx vitest run --project cli
src/lib/actions/sandbox/rebuild-custom-image-preflight.test.ts
src/lib/sandbox-base-image.test.ts` (17 passed)
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — not applicable to this narrow two-file
diagnostic-path correction.
- [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)
- [ ] 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: Apurv Kumaria <akumaria@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Bug Fixes**
- Improved custom image rebuild failure messages with clearer build
diagnostics.
- Sensitive authorization tokens embedded in build error output are now
automatically redacted.
- Prevented credentials from appearing in serialized preflight results.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Co-authored-by: Chengjie Wang <chengjiew@nvidia.com>
Co-authored-by: J. Yaunches <jyaunches@nvidia.com>
@wscurran wscurran added area: install Install, setup, prerequisites, or uninstall flow area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow bug-fix PR fixes a bug or regression labels Aug 1, 2026
@wscurran wscurran added the platform: wsl Affects Windows Subsystem for Linux label Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: install Install, setup, prerequisites, or uninstall flow area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow bug-fix PR fixes a bug or regression platform: wsl Affects Windows Subsystem for Linux

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[WSL2][Sandbox] nemoclaw rebuild fails with "docker build exited with status 1" after channels add

5 participants