Skip to content

fix(onboard): suppress sandbox-base local build log on success#3586

Merged
cv merged 3 commits into
mainfrom
fix/sandbox-base-build-output
May 15, 2026
Merged

fix(onboard): suppress sandbox-base local build log on success#3586
cv merged 3 commits into
mainfrom
fix/sandbox-base-build-output

Conversation

@laitingsheng
Copy link
Copy Markdown
Contributor

@laitingsheng laitingsheng commented May 15, 2026

Summary

At step [6/8], when the published sandbox-base image is incompatible (glibc < 2.39) and NemoClaw rebuilds locally, the full Docker build log (~200 lines: apt-get output, debconf warnings, dpkg messages, layer hashes) is forwarded to the user terminal. #3311 already fixed the [2/8] gateway setup leak the same way; the [6/8] sandbox-base rebuild path was not covered.

Related Issue

Fixes #3584

Changes

  • Add a quiet?: boolean option to dockerBuild that prepends --quiet to the build argv.
  • Switch the sandbox-base local rebuild to quiet: true + suppressOutput: true + ignoreError: true, and surface the captured stderr (plus a one-line failure summary) when the build does not succeed.
  • Add a "This is a one-time step and can take several minutes" notice so users do not mistake the silent build window for a hang.
  • Add docker-helper tests covering --quiet argv injection on dockerBuild and the default-omit behaviour.

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)

Verification

  • `npx prek run --all-files` passes
  • `npm test` passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • `make 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: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

  • New Features

    • Docker build accepts an optional quiet flag to suppress build output.
  • Bug Fixes

    • Improved handling of build failures: capture and present combined diagnostics, suppress streaming output on failure, and return a clear failure result.
    • Diagnostics now redact sensitive information before display.
  • Tests

    • Added tests for quiet-flag behavior and comprehensive build-failure diagnostics (including binary stream handling).

Review Change Stack

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 15, 2026

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

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: fb9fd352-1087-43fa-9587-53652952282c

📥 Commits

Reviewing files that changed from the base of the PR and between 5dd713a and 5716045.

📒 Files selected for processing (2)
  • src/lib/adapters/docker/image.ts
  • src/lib/adapters/docker/index.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/lib/adapters/docker/index.test.ts
  • src/lib/adapters/docker/image.ts

📝 Walkthrough

Walkthrough

Suppress Docker build output during local sandbox base-image rebuilds by adding a quiet option to dockerBuild, implement formatBuildFailureDiagnostics to normalize and redact build stderr/stdout, and integrate both so failures log only redacted diagnostics while successful builds proceed.

Changes

Suppress and redact sandbox base-image build output

Layer / File(s) Summary
Add quiet option to dockerBuild
src/lib/adapters/docker/image.ts, src/lib/adapters/docker/index.test.ts
DockerBuildOptions adds optional quiet; dockerBuild constructs docker build args, conditionally adds --quiet, ensures DOCKER_BUILDKIT=1, and forwards remaining options to dockerRun. Tests cover quiet and non-quiet cases.
Format and redact build failure diagnostics
src/lib/sandbox-base-image.ts, src/lib/sandbox-base-image.test.ts
New exported formatBuildFailureDiagnostics normalizes stderr/stdout (including Buffers), trims/filters lines, concatenates stderr then stdout, redacts secrets, and returns a combined diagnostic string (or empty). Tests cover stderr-only, stdout-only, combined, empty, redaction, and Buffer inputs.
Integrate quiet option and diagnostics into sandbox base-image build
src/lib/sandbox-base-image.ts
Local base-image build calls dockerBuild with quiet:true and captured/suppressed stdio; on failure logs redacted diagnostics plus exit/error detail and returns null; success continues to glibc compatibility checks with a computed label.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#3585: Similar dockerBuild changes merging DOCKER_BUILDKIT=1 into forwarded env/options.
  • NVIDIA/NemoClaw#3311: Related adjustments to suppress noisy Docker build output during image patching.

Suggested reviewers

  • cv
  • zyang-dev

Poem

🐰 I hopped through logs both long and loud,
Whispered "--quiet" to calm the crowd.
I stitched stderr with stdout's song,
Redacted secrets, kept output strong.
A tidy build — the rabbit's proud.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% 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 'fix(onboard): suppress sandbox-base local build log on success' accurately summarizes the main change of suppressing verbose Docker build output during sandbox-base local rebuilds.
Linked Issues check ✅ Passed The PR fully addresses issue #3584 by adding a quiet flag to dockerBuild, suppressing output during local sandbox-base rebuilds, capturing and redacting diagnostics on failure, and providing user-friendly feedback.
Out of Scope Changes check ✅ Passed All changes are tightly scoped to addressing the Docker build output suppression problem: docker adapter enhancements, sandbox-base build refactoring, and corresponding tests. No unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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/sandbox-base-build-output

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 15, 2026

E2E Advisor Recommendation

Required E2E: cloud-onboard-e2e, rebuild-openclaw-e2e
Optional E2E: rebuild-hermes-stale-base-e2e, sandbox-operations-e2e

Dispatch hint: cloud-onboard-e2e,rebuild-openclaw-e2e

Auto-dispatched E2E: cloud-onboard-e2e, rebuild-openclaw-e2e via nightly-e2e.yaml at 571604578b99cdf96ea01a24e7c5a29afce815c2nightly run

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • cloud-onboard-e2e (high): Validates the real install/onboard path that consumes sandbox base image resolution before sandbox creation. This is the highest-signal existing E2E for user-visible onboarding regressions from the shared base-image resolver.
  • rebuild-openclaw-e2e (high): Exercises OpenClaw sandbox rebuild/upgrade lifecycle with Dockerfile.base image construction and recreation. The touched base-image and docker build code are directly adjacent to this rebuild path and can affect sandbox lifecycle safety.

Optional E2E

  • rebuild-hermes-stale-base-e2e (high): Shared sandbox-base-image plumbing also supports Hermes base image resolution. This stale-base rebuild test gives extra confidence that agent base refresh/rebuild behavior remains intact, but it is not merge-blocking for an OpenClaw-focused base fallback change.
  • sandbox-operations-e2e (high): Broad sandbox lifecycle coverage for create/list/status/logs/destroy and gateway recovery. Useful as an adjacent confidence check if the PR is risky, but less targeted than onboarding and rebuild coverage.

New E2E recommendations

  • sandbox base image local fallback (high): Existing E2Es do not appear to force resolveSandboxBaseImage through the no-compatible-GHCR-image local Dockerfile.base fallback while asserting successful quiet build behavior and useful redacted diagnostics on failure.
    • Suggested test: Add a targeted sandbox-base-local-build-fallback E2E that disables or poisons published base-image candidates, forces local Dockerfile.base build through resolveLocalCandidate, verifies onboarding still succeeds, and verifies failed builds surface redacted stderr/stdout diagnostics.

Dispatch hint

  • Workflow: nightly-e2e.yaml
  • jobs input: cloud-onboard-e2e,rebuild-openclaw-e2e

…failure

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@laitingsheng laitingsheng marked this pull request as ready for review May 15, 2026 12:55
@github-actions
Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 25919013123
Target ref: 5dd713a9884c09cd0339cfd97adf0b2cbf56dadf
Workflow ref: main
Requested jobs: cloud-onboard-e2e,rebuild-openclaw-e2e,rebuild-hermes-stale-base-e2e
Summary: 3 passed, 0 failed, 0 skipped

Job Result
cloud-onboard-e2e ✅ success
rebuild-hermes-stale-base-e2e ✅ success
rebuild-openclaw-e2e ✅ success

@laitingsheng laitingsheng added v0.0.44 Release target Docker Support for Docker containerization labels May 15, 2026
…d-output

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

# Conflicts:
#	src/lib/adapters/docker/image.ts
#	src/lib/adapters/docker/index.test.ts
@cv cv merged commit 88e3936 into main May 15, 2026
27 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 25922625416
Target ref: 571604578b99cdf96ea01a24e7c5a29afce815c2
Workflow ref: main
Requested jobs: cloud-onboard-e2e,rebuild-openclaw-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job Result
cloud-onboard-e2e ✅ success
rebuild-openclaw-e2e ✅ success

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Docker Support for Docker containerization fix v0.0.44 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[DGX Spark][Sandbox] sandbox base image local build leaks full Docker build log to user terminal at step [6/8]

3 participants