Skip to content

fix: improve setup.sh and brev-setup.sh for CI and non-interactive use#1090

Merged
cv merged 2 commits intoNVIDIA:mainfrom
jyaunches:improve/setup-ci-friendliness
Mar 30, 2026
Merged

fix: improve setup.sh and brev-setup.sh for CI and non-interactive use#1090
cv merged 2 commits intoNVIDIA:mainfrom
jyaunches:improve/setup-ci-friendliness

Conversation

@jyaunches
Copy link
Copy Markdown
Contributor

@jyaunches jyaunches commented Mar 30, 2026

Summary

Improves setup.sh and brev-setup.sh for CI pipelines and non-interactive environments.

setup.sh

  • Timestamped log output[HH:MM:SS] prefix on all info/warn/fail messages for CI log correlation
  • NEMOCLAW_SANDBOX_NAME env var — allows callers to specify the sandbox name without positional args (falls back to nemoclaw default)
  • --no-tty flag on openshell sandbox create — prevents hangs when running in non-interactive SSH sessions or CI
  • Base image fallback — pre-builds Dockerfile.base locally when the GHCR image isn't available (common on forks before the first base-image workflow run)
  • Progress reporter — background process prints a heartbeat every 30s during sandbox build with Docker step info (secrets filtered from output), so CI doesn't look hung during long builds
  • Build timing — logs elapsed time on sandbox build completion

brev-setup.sh

  • Timestamped log output — matches setup.sh format
  • SKIP_VLLM=1 support — skips vLLM installation on CPU-only instances where GPU inference isn't available

Motivation

These changes came from building E2E test infrastructure that bootstraps NemoClaw on ephemeral Brev instances via SSH. Without them, setup.sh would hang on TTY prompts, produce un-timestamped output making CI logs hard to debug, and fail on forks without GHCR access.

Testing

  • All existing tests pass
  • Changes tested on ephemeral Brev CPU instances via the e2e-brev workflow
  • No behavioral changes to the sandbox creation flow itself — only logging, configuration, and non-interactive mode improvements

Summary by CodeRabbit

  • New Features

    • Added configurable sandbox name via environment variable override.
    • Introduced skip option for vLLM installation.
    • Added automatic base image detection, download, and build.
    • Added real-time progress reporting during sandbox creation.
  • Improvements

    • Log messages now include timestamps for better tracking.

setup.sh:
- Add timestamped log output ([HH:MM:SS] prefix) for CI visibility
- Support NEMOCLAW_SANDBOX_NAME env var for custom sandbox names
- Add --no-tty flag to openshell sandbox create for non-interactive mode
- Pre-build base image locally when GHCR image is unavailable (forks)
- Add background progress reporter during sandbox build (heartbeat
  every 30s with Docker step info, filters secrets from output)
- Show build elapsed time on completion

brev-setup.sh:
- Add timestamped log output matching setup.sh format
- Support SKIP_VLLM=1 to skip vLLM install on CPU-only instances
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 39e8225e-baec-42cd-b9c4-90d3870a4fab

📥 Commits

Reviewing files that changed from the base of the PR and between f59f58e and 673554d.

📒 Files selected for processing (3)
  • scripts/brev-setup.sh
  • scripts/setup.sh
  • test/setup-sandbox-name.test.js

📝 Walkthrough

Walkthrough

Added timestamping to logging functions in setup scripts. Modified sandbox name resolution to support NEMOCLAW_SANDBOX_NAME environment variable override. Added pre-build base image verification and local build fallback. Introduced background progress reporter during sandbox creation with periodic status and elapsed-time tracking.

Changes

Cohort / File(s) Summary
Logging Enhancements
scripts/brev-setup.sh, scripts/setup.sh
Added _ts() function to emit current timestamps and updated info(), warn(), and fail() functions to prefix log messages with [$(_ts)] for better traceability across both scripts.
Sandbox Setup & Build Process
scripts/setup.sh
Modified sandbox name resolution to cascade through positional arg, NEMOCLAW_SANDBOX_NAME env var, then default "nemoclaw". Added pre-build base image check with docker pull fallback and local Dockerfile build if missing. Introduced background progress reporter during openshell sandbox create that tails build logs, extracts Docker step progress, and logs periodic heartbeats with elapsed time. Added --no-tty -- true args to sandbox create invocation.
vLLM Control Flow
scripts/brev-setup.sh
Added conditional opt-out gate SKIP_VLLM=1 to bypass vLLM installation and start logic entirely when set.
Sandbox Tests
test/setup-sandbox-name.test.js
Updated test assertions to validate cascading SANDBOX_NAME resolution with positional arg taking precedence, followed by NEMOCLAW_SANDBOX_NAME env var, then default. Added dedicated test case for env var override when no positional arg provided.

Sequence Diagram(s)

sequenceDiagram
    participant Script as Setup Script
    participant Registry as Docker Registry
    participant Docker as Local Docker
    participant FS as Filesystem
    participant Build as Docker Build
    participant Sandbox as openshell

    Script->>Docker: Check if BASE_IMAGE exists locally
    alt Image Not Found
        Script->>Registry: docker pull BASE_IMAGE
        alt Pull Succeeds
            Registry-->>Docker: Image loaded
        else Pull Fails
            Script->>FS: Check for Dockerfile.base
            alt Dockerfile exists
                Script->>Build: docker build -f Dockerfile.base
                Build-->>Docker: Image created locally
            else File Not Found
                Script->>Script: Continue without base image
            end
        end
    end

    Script->>Sandbox: openshell sandbox create with progress tracking
    par Main Process
        Sandbox->>Sandbox: Build sandbox
    and Background Reporter
        Script->>Script: Start progress reporter loop
        loop Every interval
            Script->>FS: Tail build log
            Script->>Script: Extract last Docker Step X/Y
            Script->>Script: Log heartbeat with elapsed time
        end
    end

    Sandbox-->>Script: Build completes
    Script->>Script: Terminate reporter, log duration & exit code
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 Timestamps now dance in logs so bright,
Sandbox names cascade with delight,
Base images pre-built before the show,
Progress reporters steal the glow,
A setup script that's both wise and fleet!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: improving setup.sh and brev-setup.sh for CI and non-interactive environments, which aligns with the primary objectives of adding timestamped logs, environment variable support, progress reporting, and skip flags.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@cv cv merged commit e9a59a2 into NVIDIA:main Mar 30, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants