fix(cli): recognize BuildKit sandbox build progress (Fixes #2311) by deepujain · Pull Request #2404 · NVIDIA/NemoClaw

deepujain · 2026-04-24T02:05:59Z

Summary

NemoClaw's sandbox create stream only recognized the legacy Docker builder format, so BuildKit output would not be treated as active build progress once OpenShell emits it.

This adds BuildKit progress markers to the same parser path as the existing legacy builder output. It keeps the current legacy behavior and makes #1 [internal] ..., #2 CACHED, and #3 DONE ... visible as build progress.

Changes

src/lib/sandbox-create-stream.ts: recognize BuildKit step and completion lines while tracking the build phase.
src/lib/sandbox-create-stream.test.ts: cover BuildKit progress output and verify it is streamed to the user.

Testing

npm run build:cli passed
npm run typecheck:cli passed
npm test -- src/lib/sandbox-create-stream.test.ts passed
npm test was also attempted. The full suite is not green on current main in this environment; failures are in existing installer/onboard/legacy-guard tests outside this change.

Evidence it works

The new focused test feeds BuildKit-style output into streamSandboxCreate and verifies that the lines are logged, collected in output, and mark sandbox creation as having seen progress.

Fixes #2311

Signed-off-by: Deepak Jain deepujain@gmail.com

Summary by CodeRabbit

Bug Fixes
- Improved detection and display of BuildKit and upload progress so progress markers and completion states are recognized reliably.
Refactor
- Centralized progress-detection logic for more consistent handling of build and upload output.
Tests
- Added a test ensuring BuildKit-formatted progress lines are captured, included in output, and reported to the log callback.

copy-pr-bot · 2026-04-24T02:06:04Z

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 · 2026-04-24T02:06:11Z

📝 Walkthrough

Walkthrough

Centralizes regex-based detection for build/upload progress in the sandbox stream, exports BUILD_PROGRESS_PATTERNS, refactors visible-line filtering to use aggregated pattern sets, and adds a vitest that feeds BuildKit-formatted progress lines into the stream and asserts correct collection and callbacks.

Changes

Cohort / File(s)	Summary
Build / progress parsing `src/lib/sandbox-create-stream.ts`	Centralizes and extends progress-detection regex collections; adds exported `BUILD_PROGRESS_PATTERNS: readonly RegExp[]`; replaces legacy hardcoded checks with `matchesAny` over `BUILD_PROGRESS_PATTERNS` / `UPLOAD_PROGRESS_PATTERNS`; unifies visible-line logic into `VISIBLE_PROGRESS_PATTERNS` to include BuildKit-style lines (`#n [...]`, `#n (DONE
Test: BuildKit progress handling `src/lib/sandbox-create-stream.test.ts`	Adds a vitest that writes multiple newline-terminated BuildKit-style progress lines to a fake child stdout, signals process close with exit code 0, and asserts `streamSandboxCreate` returns `status: 0`, `sawProgress: true`, includes the BuildKit lines in `output`, and calls the provided `logLine` callback once per progress line with exact strings.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through hashes, sharp and true,
Counting BuildKit lines — one, two, three, woo!
Patterns neat in regex rows,
I munched the logs where progress grows.
Stream says "sawProgress" — carrot cheers for you!

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change—adding BuildKit sandbox build progress recognition to the CLI, directly addressing the linked issue `#2311`.
Linked Issues check	✅ Passed	The code changes meet the NemoClaw objectives from issue `#2311`: recognized BuildKit output patterns, added focused tests, and preserved legacy builder support.
Out of Scope Changes check	✅ Passed	All changes are scoped to NemoClaw's sandbox stream parsing; no unrelated modifications to other components or out-of-scope refactoring detected.

_{✏️ 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.}

coderabbitai

🧹 Nitpick comments (1)

src/lib/sandbox-create-stream.ts (1)

125-130: Consider centralizing progress regexes to avoid parser drift.

The same patterns are duplicated between flushLine and shouldShowLine. A shared matcher keeps future edits safer.

♻️ Suggested refactor

+const BUILD_PROGRESS_PATTERNS: RegExp[] = [
+  /^ {2}Building image /,
+  /^ {2}Step \d+\/\d+ : /,
+  /^#\d+ \[/,
+  /^#\d+ (DONE|CACHED)\b/,
+];
+
+const UPLOAD_PROGRESS_PATTERNS: RegExp[] = [
+  /^ {2}Pushing image /,
+  /^\s*\[progress\]/,
+  /^ {2}Image .*available in the gateway/,
+];
+
+const VISIBLE_PROGRESS_PATTERNS: RegExp[] = [
+  ...BUILD_PROGRESS_PATTERNS,
+  /^ {2}Context: /,
+  /^ {2}Gateway: /,
+  /^Successfully built /,
+  /^Successfully tagged /,
+  /^ {2}Built image /,
+  ...UPLOAD_PROGRESS_PATTERNS,
+  /^Created sandbox: /,
+  /^✓ /,
+];
+
+function matchesAny(line: string, patterns: readonly RegExp[]) {
+  return patterns.some((re) => re.test(line));
+}
...
-    if (
-      /^ {2}Building image /.test(line) ||
-      /^ {2}Step \d+\/\d+ : /.test(line) ||
-      /^#\d+ \[/.test(line) ||
-      /^#\d+ (DONE|CACHED)\b/.test(line)
-    ) {
+    if (matchesAny(line, BUILD_PROGRESS_PATTERNS)) {
       setPhase("build");
-    } else if (
-      /^ {2}Pushing image /.test(line) ||
-      /^\s*\[progress\]/.test(line) ||
-      /^ {2}Image .*available in the gateway/.test(line)
-    ) {
+    } else if (matchesAny(line, UPLOAD_PROGRESS_PATTERNS)) {
       setPhase("upload");
...
-    return (
-      /^ {2}Building image /.test(line) ||
-      /^ {2}Step \d+\/\d+ : /.test(line) ||
-      /^#\d+ \[/.test(line) ||
-      /^#\d+ (DONE|CACHED)\b/.test(line) ||
-      ...
-      /^✓ /.test(line)
-    );
+    return matchesAny(line, VISIBLE_PROGRESS_PATTERNS);

Also applies to: 147-164

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/sandbox-create-stream.ts` around lines 125 - 130, The duplicated
Docker progress regexes used in flushLine and shouldShowLine should be
centralized into a single exported constant (e.g., PROGRESS_REGEXES or
progressMatchers) and reused by both functions; update flushLine and
shouldShowLine to iterate over that shared array of RegExp objects (or test each
with .some) instead of repeating the same literal patterns so future changes
only need to be made in one place.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/lib/sandbox-create-stream.ts`:
- Around line 125-130: The duplicated Docker progress regexes used in flushLine
and shouldShowLine should be centralized into a single exported constant (e.g.,
PROGRESS_REGEXES or progressMatchers) and reused by both functions; update
flushLine and shouldShowLine to iterate over that shared array of RegExp objects
(or test each with .some) instead of repeating the same literal patterns so
future changes only need to be made in one place.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 70d3bb59-a42a-43bd-94f1-73854b7510d9

📥 Commits

Reviewing files that changed from the base of the PR and between dc02013 and 065412c.

📒 Files selected for processing (2)

src/lib/sandbox-create-stream.test.ts
src/lib/sandbox-create-stream.ts

deepujain · 2026-04-24T02:18:15Z

Centralized the BuildKit and legacy progress matchers so phase detection and visible-output filtering share the same patterns. Reran npm run build:cli, npm run typecheck:cli, and npm test -- src/lib/sandbox-create-stream.test.ts.

wscurran · 2026-04-24T14:26:42Z

✨ Thanks for submitting this pull request that proposes a way to fix a bug that causes NemoClaw's sandbox create stream to not recognize BuildKit sandbox build progress. This identifies a bug and proposes a change to recognize BuildKit progress markers. The addition of a focused test to verify the lines are logged and collected in output will help ensure the issue does not recur.

Related open issues:

#2311 OpenShell uses legacy Docker builder instead of BuildKit

Fixes NVIDIA#2311 Signed-off-by: Deepak Jain <deepujain@gmail.com>

deepujain · 2026-04-28T16:34:47Z

Rebased on latest main, no code changes needed beyond replay. npm test -- src/lib/sandbox-create-stream.test.ts passes.

coderabbitai

🧹 Nitpick comments (1)

src/lib/sandbox-create-stream.test.ts (1)
76-76: Remove the as never cast from spawnImpl.

Line 76 uses child as never, while all other tests in this file (lines 35, 48, 103, 130, 148, 170, 191) use spawnImpl: () => child directly. Since FakeChild implements StreamableChildProcess, the cast is unnecessary and suppresses type safety without benefit. Removing it ensures consistency across the test suite.
Proposed change
-      spawnImpl: () => child as never,
+      spawnImpl: () => child,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/sandbox-create-stream.test.ts` at line 76, Remove the unnecessary
type cast on the spawnImpl return value: update the test's spawnImpl: () =>
child as never to simply spawnImpl: () => child because FakeChild already
implements StreamableChildProcess; this restores type safety and matches other
tests (see spawnImpl usage and the FakeChild/StreamableChildProcess types).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/lib/sandbox-create-stream.test.ts`:
- Line 76: Remove the unnecessary type cast on the spawnImpl return value:
update the test's spawnImpl: () => child as never to simply spawnImpl: () =>
child because FakeChild already implements StreamableChildProcess; this restores
type safety and matches other tests (see spawnImpl usage and the
FakeChild/StreamableChildProcess types).

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8f44a91a-bdac-49fc-8a88-0f7ed5bcfd73

📥 Commits

Reviewing files that changed from the base of the PR and between d17b8f1 and c468381.

📒 Files selected for processing (2)

src/lib/sandbox-create-stream.test.ts
src/lib/sandbox-create-stream.ts

) ## Summary NemoClaw's sandbox create stream only recognized the legacy Docker builder format, so BuildKit output would not be treated as active build progress once OpenShell emits it. This adds BuildKit progress markers to the same parser path as the existing legacy builder output. It keeps the current legacy behavior and makes `#1 [internal] ...`, `#2 CACHED`, and `#3 DONE ...` visible as build progress. ## Changes - `src/lib/sandbox-create-stream.ts`: recognize BuildKit step and completion lines while tracking the build phase. - `src/lib/sandbox-create-stream.test.ts`: cover BuildKit progress output and verify it is streamed to the user. ## Testing - `npm run build:cli` passed - `npm run typecheck:cli` passed - `npm test -- src/lib/sandbox-create-stream.test.ts` passed - `npm test` was also attempted. The full suite is not green on current main in this environment; failures are in existing installer/onboard/legacy-guard tests outside this change. ## Evidence it works The new focused test feeds BuildKit-style output into `streamSandboxCreate` and verifies that the lines are logged, collected in output, and mark sandbox creation as having seen progress. Fixes #2311 Signed-off-by: Deepak Jain <deepujain@gmail.com>  ## Summary by CodeRabbit * **Bug Fixes** * Improved detection and display of BuildKit and upload progress so progress markers and completion states are recognized reliably. * **Refactor** * Centralized progress-detection logic for more consistent handling of build and upload output. * **Tests** * Added a test ensuring BuildKit-formatted progress lines are captured, included in output, and reported to the log callback.  Signed-off-by: Deepak Jain <deepujain@gmail.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>

… (NVIDIA#2404) ## Summary NemoClaw's sandbox create stream only recognized the legacy Docker builder format, so BuildKit output would not be treated as active build progress once OpenShell emits it. This adds BuildKit progress markers to the same parser path as the existing legacy builder output. It keeps the current legacy behavior and makes `NVIDIA#1 [internal] ...`, `NVIDIA#2 CACHED`, and `NVIDIA#3 DONE ...` visible as build progress. ## Changes - `src/lib/sandbox-create-stream.ts`: recognize BuildKit step and completion lines while tracking the build phase. - `src/lib/sandbox-create-stream.test.ts`: cover BuildKit progress output and verify it is streamed to the user. ## Testing - `npm run build:cli` passed - `npm run typecheck:cli` passed - `npm test -- src/lib/sandbox-create-stream.test.ts` passed - `npm test` was also attempted. The full suite is not green on current main in this environment; failures are in existing installer/onboard/legacy-guard tests outside this change. ## Evidence it works The new focused test feeds BuildKit-style output into `streamSandboxCreate` and verifies that the lines are logged, collected in output, and mark sandbox creation as having seen progress. Fixes NVIDIA#2311 Signed-off-by: Deepak Jain <deepujain@gmail.com>  ## Summary by CodeRabbit * **Bug Fixes** * Improved detection and display of BuildKit and upload progress so progress markers and completion states are recognized reliably. * **Refactor** * Centralized progress-detection logic for more consistent handling of build and upload output. * **Tests** * Added a test ensuring BuildKit-formatted progress lines are captured, included in output, and reported to the log callback.  Signed-off-by: Deepak Jain <deepujain@gmail.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>

coderabbitai Bot reviewed Apr 24, 2026

View reviewed changes

deepujain force-pushed the fix/2311-buildkit-progress branch from 065412c to d17b8f1 Compare April 24, 2026 02:18

wscurran added bug Something fails against expected or documented behavior enhancement: testing labels Apr 24, 2026

wscurran removed the enhancement: cli label Apr 24, 2026

fix(cli): recognize BuildKit sandbox build progress

c468381

Fixes NVIDIA#2311 Signed-off-by: Deepak Jain <deepujain@gmail.com>

deepujain force-pushed the fix/2311-buildkit-progress branch from d17b8f1 to c468381 Compare April 28, 2026 16:34

coderabbitai Bot reviewed Apr 28, 2026

View reviewed changes

cv approved these changes Apr 28, 2026

View reviewed changes

Merge branch 'main' into fix/2311-buildkit-progress

1412828

cv merged commit 6807a6b into NVIDIA:main Apr 28, 2026
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cli): recognize BuildKit sandbox build progress (Fixes #2311)#2404

fix(cli): recognize BuildKit sandbox build progress (Fixes #2311)#2404
cv merged 2 commits into
NVIDIA:mainfrom
deepujain:fix/2311-buildkit-progress

deepujain commented Apr 24, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

copy-pr-bot Bot commented Apr 24, 2026

Uh oh!

coderabbitai Bot commented Apr 24, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

deepujain commented Apr 24, 2026

Uh oh!

wscurran commented Apr 24, 2026 •

edited

Loading

Uh oh!

deepujain commented Apr 28, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

deepujain commented Apr 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Testing

Evidence it works

Summary by CodeRabbit

Uh oh!

copy-pr-bot Bot commented Apr 24, 2026

Uh oh!

coderabbitai Bot commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

deepujain commented Apr 24, 2026

Uh oh!

wscurran commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

deepujain commented Apr 28, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

deepujain commented Apr 24, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 24, 2026 •

edited

Loading

wscurran commented Apr 24, 2026 •

edited

Loading