Skip to content

refactor(onboard): extract build context staging#5136

Merged
cv merged 18 commits into
mainfrom
codex/onboard-build-context-flow
Jun 11, 2026
Merged

refactor(onboard): extract build context staging#5136
cv merged 18 commits into
mainfrom
codex/onboard-build-context-flow

Conversation

@cv

@cv cv commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Extracts createSandbox build-context staging into a focused helper. This keeps custom Dockerfile validation/copying, agent build-context delegation, default optimized staging, and temp-dir cleanup behavior in one testable module while preserving the existing onboard control flow.

Related Issue

Refs #3802

Changes

  • Added stageCreateSandboxBuildContext in src/lib/onboard/build-context-stage.ts.
  • Replaced inline custom Dockerfile, agent, and default build-context staging in createSandbox with the new helper.
  • Added focused coverage for custom Dockerfile context staging and ignored-path filtering, missing custom Dockerfile failure, directory-path rejection, ignored parent context rejection, large-context warning behavior, EACCES copy cleanup, agent/default delegation, and cleanup behavior.

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

Targeted checks run:

  • npx @biomejs/biome format --write src/lib/onboard/build-context-stage.test.ts
  • npx @biomejs/biome lint src/lib/onboard.ts src/lib/onboard/build-context-stage.ts src/lib/onboard/build-context-stage.test.ts
  • npm run build:cli
  • npx vitest run src/lib/onboard/build-context-stage.test.ts test/onboard-custom-dockerfile.test.ts test/onboard.test.ts
  • npm run typecheck:cli
  • git diff --check

GitHub validation:

Local broad-hook note:

  • npx prek run --all-files was attempted before the test follow-up; static hooks passed, then test/release-latest-tag.test.ts failed because local fixture commits inherit the machine's missing git signing key (/home/cvillela/.ssh/git-signing-key.pub). This unrelated release test file was not modified.

  • 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

  • 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: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • Refactor

    • Improved Docker build context staging process for custom and default builds, with enhanced error handling and clearer messaging.
  • Tests

    • Added comprehensive test coverage for build context staging logic, including custom Dockerfile validation and error scenarios.

@cv cv self-assigned this Jun 10, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jun 10, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jun 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: 8ac14b0a-e602-423c-b423-a9ddc27fa67c

📥 Commits

Reviewing files that changed from the base of the PR and between 8eeacff and 574a384.

📒 Files selected for processing (3)
  • src/lib/onboard.ts
  • src/lib/onboard/build-context-stage.test.ts
  • src/lib/onboard/build-context-stage.ts

📝 Walkthrough

Walkthrough

Extracts Docker build-context staging logic from onboard.ts into a new buildContextStage module with three strategies: custom Dockerfile handling (validation, copying, error handling), agent delegation, and default staging. Introduces input/output interfaces, comprehensive test coverage for validation failures and all staging paths, and refactors the main onboard flow to delegate context staging.

Changes

Build context staging refactoring

Layer / File(s) Summary
Build context staging contracts and cleanup helper
src/lib/onboard/build-context-stage.ts
Establishes CreateSandboxBuildContextInput and CreateSandboxBuildContextResult interfaces defining the staging API contract, and introduces a cleanup helper that removes staged directories recursively.
Build context staging implementation
src/lib/onboard/build-context-stage.ts
Implements stageCreateSandboxBuildContext() with three branching modes: custom Dockerfile validation and copying (with stats and EACCES handling), agent-based delegation, and default/optimized staging fallback.
Build context staging test suite
src/lib/onboard/build-context-stage.test.ts
Comprehensive Vitest suite covering custom Dockerfile staging with filtering, validation failures (missing/directory/ignored-path), size warnings, permission errors with cleanup, and delegation behavior for all three staging modes.
Integration into main onboard module
src/lib/onboard.ts
Refactors createSandbox() to delegate build-context staging to the new module. Updates imports (adds buildContextStage, removes unused errno and staging helpers) and simplifies inline logic by replacing ~82 lines with a single delegating call.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested labels

area: sandbox

Poem

A rabbit hops through staging grounds,
Building contexts, safe and sound!
Three paths converge where Dockerfiles dwell,
Validation, cleanup, tested well. 🐰✨

🚥 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 'refactor(onboard): extract build context staging' clearly and concisely summarizes the main change: extracting build context staging logic into a dedicated helper module (build-context-stage.ts). It directly relates to the primary refactoring objective.
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.

✏️ 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 codex/onboard-build-context-flow

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

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 0 needs attention, 0 worth checking, 0 nice ideas
Since last review: 0 prior items resolved, 0 still apply, 0 new items found

Consider writing more tests for
  • **Runtime validation** — onboard --from <Containerfile> creates a sandbox from the staged Dockerfile and includes expected non-secret context files. Unit and mocked integration coverage is strong for the extracted helper and createSandbox interaction, but the changed path crosses the host filesystem, Docker build-context staging, and OpenShell sandbox creation boundary where targeted runtime validation would add confidence.
  • **Runtime validation** — onboard --from <secret-dir>/Dockerfile rejects before invoking openshell sandbox create. Unit and mocked integration coverage is strong for the extracted helper and createSandbox interaction, but the changed path crosses the host filesystem, Docker build-context staging, and OpenShell sandbox creation boundary where targeted runtime validation would add confidence.
  • **Runtime validation** — sandbox creation failure after custom context staging removes the temporary build context through inline cleanup or the process-exit safety net. Unit and mocked integration coverage is strong for the extracted helper and createSandbox interaction, but the changed path crosses the host filesystem, Docker build-context staging, and OpenShell sandbox creation boundary where targeted runtime validation would add confidence.
  • **Runtime validation** — custom build context entries such as .ssh, .env.local, .aws, .npmrc, credentials.json, and *.pem are absent from the context passed to openshell sandbox create. Unit and mocked integration coverage is strong for the extracted helper and createSandbox interaction, but the changed path crosses the host filesystem, Docker build-context staging, and OpenShell sandbox creation boundary where targeted runtime validation would add confidence.
  • **Acceptance clause:** Refs Umbrella: refactor onboarding into a serializable FSM #3802 — add test evidence or identify existing coverage. The PR body references Umbrella: refactor onboarding into a serializable FSM #3802, but deterministic context reports linkedIssues: [] and provides no issue body or issue comments to quote. Issue-specific acceptance cannot be verified from the provided trusted context.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: cloud-e2e, openclaw-plugin-runtime-exdev-e2e
Optional E2E: rebuild-openclaw-e2e, onboard-negative-paths-e2e

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

Auto-dispatched E2E: cloud-e2e via nightly-e2e.yaml at 574a384a0dfdbe1355c4acf319122a870523bfacnightly run

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • cloud-e2e (high; live Docker/OpenShell sandbox and NVIDIA endpoint inference): Runs install.sh --non-interactive and the standard OpenClaw onboard flow against a live sandbox. This is the primary regression check that the refactored default/agent build-context staging still produces a bootable sandbox and working inference.
  • openclaw-plugin-runtime-exdev-e2e (high; live Docker/OpenShell sandbox and NVIDIA endpoint credential required): Explicitly runs nemoclaw onboard --agent openclaw --from "$REPO/Dockerfile", which exercises the custom Dockerfile build-context path changed by this PR in a real sandbox creation flow.

Optional E2E

  • rebuild-openclaw-e2e (high; install, Docker/OpenShell sandbox rebuild, NVIDIA_API_KEY): Useful additional confidence for sandbox lifecycle/recreate behavior because rebuild creates a fresh current OpenClaw sandbox and verifies state/credential preservation around backup and recreation. Adjacent to the refactored createSandbox build-context path, but less direct than fresh onboard and --from coverage.
  • onboard-negative-paths-e2e (medium; mostly CLI/onboard negative-path checks with some sandbox setup): Optional validation of non-interactive onboard edge/error handling. It does not appear to cover custom --from Dockerfile failures directly, but is adjacent to the onboarding validation paths touched by the refactor.

New E2E recommendations

  • custom Dockerfile build context negative paths (medium): Existing E2E coverage includes a real --from $REPO/Dockerfile onboard, but does not appear to exercise missing Dockerfile, Dockerfile path as directory, ignored context path such as .ssh, EACCES during context copy, or temp-dir cleanup after a failed real CLI invocation.
    • Suggested test: Add a focused E2E script for nemoclaw onboard --from negative-paths that uses temporary Dockerfile contexts and asserts friendly errors, ignored-path rejection, and no persisted temporary build context after failure.
  • custom Dockerfile context filtering / secret boundary (high): The unit tests verify .ssh filtering, but a live or CLI-level E2E would better guard against accidentally sending sensitive host files in a Docker build context when using --from.
    • Suggested test: Add a hermetic CLI-level E2E that creates a custom build context containing .ssh/credential-shaped files plus an allowed file, runs the staging path via nemoclaw onboard --from with sandbox creation mocked or forced to fail after staging, and verifies filtered files are absent from the staged context/logs.

Dispatch hint

  • Workflow: .github/workflows/nightly-e2e.yaml
  • jobs input: cloud-e2e,rebuild-openclaw-e2e,onboard-negative-paths-e2e

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: ubuntu-repo-cloud-openclaw
Optional Vitest E2E scenarios: ubuntu-repo-docker-post-reboot-recovery

Dispatch required Vitest E2E scenarios:

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

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • ubuntu-repo-cloud-openclaw: The PR refactors sandbox build-context staging in the onboarding path used by nemoclaw onboard. The live Ubuntu cloud OpenClaw scenario exercises non-interactive onboarding with Docker and the default optimized sandbox build context.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Optional Vitest E2E scenarios

  • ubuntu-repo-docker-post-reboot-recovery: Optional adjacent coverage: this live-supported scenario also onboards an OpenClaw sandbox through the same build-context staging path before exercising post-reboot recovery invariants.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-docker-post-reboot-recovery

Relevant changed files

  • src/lib/onboard.ts
  • src/lib/onboard/build-context-stage.ts

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27278528343
Target ref: codex/onboard-build-context-flow
Workflow ref: main
Requested jobs: cloud-onboard-e2e,onboard-negative-paths-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job Result
cloud-onboard-e2e ✅ success
onboard-negative-paths-e2e ✅ success

@wscurran wscurran added area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow refactor PR restructures code without intended behavior change labels Jun 10, 2026
@wscurran

Copy link
Copy Markdown
Contributor

Related open issues:

@cv cv added the v0.0.64 Release target label Jun 10, 2026
Base automatically changed from codex/onboard-messaging-prep-flow to main June 11, 2026 09:07
@cv cv marked this pull request as ready for review June 11, 2026 09:14
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27336788635
Target ref: 574a384a0dfdbe1355c4acf319122a870523bfac
Workflow ref: main
Requested jobs: cloud-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job Result
cloud-e2e ✅ success

@cv cv merged commit adc9094 into main Jun 11, 2026
37 of 39 checks passed
@cv cv deleted the codex/onboard-build-context-flow branch June 11, 2026 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow refactor PR restructures code without intended behavior change v0.0.64 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants