fix(cli): show pull progress during sandbox onboard base image download - #1897
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds detection of Docker image pull output and a new internal "pull" phase; the stream parser now sets phase="pull", announces "Pulling base image from registry...", and emits "Still pulling base image..." heartbeats while pull-related lines are observed. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as Caller
participant Proc as ChildProcess
participant Parser as StreamParser
participant Logger as Logger/Heartbeat
CLI->>Proc: start sandbox build
Proc-->>Parser: stdout/stderr lines (Step, Pulling..., Downloading..., Status...)
alt pull-related lines detected
Parser->>Parser: set phase = "pull"
Parser->>Logger: "Pulling base image from registry..."
loop while pull lines continue
Proc-->>Parser: pull progress lines
Parser->>Logger: "Still pulling base image..." heartbeat
end
else other build lines
Parser->>Logger: emit build lines and "Still building..." heartbeats
end
Proc-->>CLI: exit/close
Parser->>Logger: final messages
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/sandbox-create-stream.ts (1)
238-263:⚠️ Potential issue | 🟠 MajorHeartbeat still allows indefinite hangs without a hard timeout.
The new heartbeat improves visibility, but if
docker build/pullstalls and never closes, this promise can remain pending forever. Add a configurable hard timeout and terminate/finish deterministically.🛡️ Proposed fix
export interface StreamSandboxCreateOptions { readyCheck?: (() => boolean) | null; pollIntervalMs?: number; heartbeatIntervalMs?: number; silentPhaseMs?: number; + buildTimeoutMs?: number; logLine?: (line: string) => void; spawnImpl?: ( @@ const heartbeatIntervalMs = options.heartbeatIntervalMs || 5000; const silentPhaseMs = options.silentPhaseMs || 15000; + const buildTimeoutMs = options.buildTimeoutMs ?? 15 * 60_000; @@ function finish(status: number, overrides: Partial<StreamSandboxCreateResult> = {}) { if (settled) return; settled = true; if (pending) flushLine(pending); if (readyTimer) clearInterval(readyTimer); + clearTimeout(hardTimeoutTimer); clearInterval(heartbeatTimer); @@ + const hardTimeoutTimer = setTimeout(() => { + if (settled) return; + const detail = `build timed out after ${Math.floor(buildTimeoutMs / 1000)}s`; + lines.push(detail); + try { + child.kill?.("SIGTERM"); + } catch { + // Best effort. + } + detachChild(); + finish(124); + }, buildTimeoutMs); + hardTimeoutTimer.unref?.(); + return new Promise((resolve) => {🤖 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 238 - 263, The heartbeat loop (heartbeatTimer) can hang indefinitely if docker build/pull never finishes; add a configurable hard timeout (e.g., heartbeatHardTimeoutMs or maxHeartbeatMs) that starts when the heartbeatTimer is created and, when reached, deterministically stops the interval, marks the operation settled, and triggers the same termination/cleanup path used for normal completion. Concretely: introduce a timeout using setTimeout alongside heartbeatTimer, clear both timer handles (clearInterval and clearTimeout) when finished, set settled = true, and call the existing cleanup/finish logic (the same code path used when the operation completes) so functions like printProgressLine, trimDisplayLine, lastHeartbeatPhase/lastHeartbeatBucket and any promise resolution/rejection behave consistently; make the hard timeout value configurable via the surrounding options where heartbeatIntervalMs is defined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/sandbox-create-stream.ts`:
- Around line 155-156: The regex that detects "Pulling from" only allows
lowercase letter prefixes ([a-z]+:) which misses tags like "12.4:", "v1.2.3:" or
"cuda-12.5:"; update both occurrences of /^\s*(?:[a-z]+:\s+)?Pulling from
\S+/.test(line) (also the one at lines 162-163) to accept broader tag prefixes
such as /^\s*(?:[^\s:]+:\s+)?Pulling from \S+/.test(line) or
/^\s*(?:[\w.-]+:\s+)?Pulling from \S+/.test(line) so numeric, dot, dash and
underscore tags are matched. Ensure you change both instances in
src/lib/sandbox-create-stream.ts.
---
Outside diff comments:
In `@src/lib/sandbox-create-stream.ts`:
- Around line 238-263: The heartbeat loop (heartbeatTimer) can hang indefinitely
if docker build/pull never finishes; add a configurable hard timeout (e.g.,
heartbeatHardTimeoutMs or maxHeartbeatMs) that starts when the heartbeatTimer is
created and, when reached, deterministically stops the interval, marks the
operation settled, and triggers the same termination/cleanup path used for
normal completion. Concretely: introduce a timeout using setTimeout alongside
heartbeatTimer, clear both timer handles (clearInterval and clearTimeout) when
finished, set settled = true, and call the existing cleanup/finish logic (the
same code path used when the operation completes) so functions like
printProgressLine, trimDisplayLine, lastHeartbeatPhase/lastHeartbeatBucket and
any promise resolution/rejection behave consistently; make the hard timeout
value configurable via the surrounding options where heartbeatIntervalMs is
defined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f88534c4-106c-42e5-ad26-fe1121afd44e
📒 Files selected for processing (2)
src/lib/sandbox-create-stream.test.tssrc/lib/sandbox-create-stream.ts
Collaborator
Author
|
Broadened the tag-prefix regex as suggested ( Deferring the hard-timeout suggestion to a separate PR. The indefinite-hang risk predates this change — the heartbeat loop in |
Contributor
|
✨ Thanks for submitting this PR, which proposes a way to improve the NemoClaw CLI by showing pull progress during sandbox creation. Possibly related open issues: |
1 similar comment
Contributor
|
✨ Thanks for submitting this PR, which proposes a way to improve the NemoClaw CLI by showing pull progress during sandbox creation. Possibly related open issues: |
latenighthackathon
force-pushed
the
fix/sandbox-pull-progress
branch
from
April 17, 2026 01:34
af4b8ad to
3304dcc
Compare
3 tasks
latenighthackathon
force-pushed
the
fix/sandbox-pull-progress
branch
from
April 25, 2026 05:02
3304dcc to
0d90f15
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/sandbox-create-stream.ts (1)
148-176: Optional: Extract shared pull regexes to avoid drift.
shouldShowLineandisPullLineduplicate pull-related patterns. Centralizing them will reduce future mismatch risk when patterns evolve.♻️ Suggested refactor
+const PULL_FROM_RE = /^\s*(?:[^:\s]+:\s+)?Pulling from \S+/; +const PULL_STATUS_RE = /^\s*Status: (?:Downloaded|Image is up to date)/; function shouldShowLine(line: string) { return ( @@ - /^\s*(?:[^:\s]+:\s+)?Pulling from \S+/.test(line) || - /^\s*Status: (?:Downloaded|Image is up to date)/.test(line) + PULL_FROM_RE.test(line) || + PULL_STATUS_RE.test(line) ); } function isPullLine(line: string) { return ( - /^\s*(?:[^:\s]+:\s+)?Pulling from \S+/.test(line) || + PULL_FROM_RE.test(line) || @@ - /^\s*Status: (?:Downloaded|Image is up to date)/.test(line) || + PULL_STATUS_RE.test(line) ||🤖 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 148 - 176, shouldShowLine and isPullLine duplicate the same pull-related regexes; extract those shared patterns into a single set of constants or a helper matcher (e.g., const PULL_PATTERNS = [...]; function matchesPullPattern(line: string): boolean) and replace the duplicated regex checks in both shouldShowLine and isPullLine to call that helper or iterate PULL_PATTERNS, keeping other unique tests in each function unchanged so both use the single source of truth for pull-related patterns.
🤖 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 148-176: shouldShowLine and isPullLine duplicate the same
pull-related regexes; extract those shared patterns into a single set of
constants or a helper matcher (e.g., const PULL_PATTERNS = [...]; function
matchesPullPattern(line: string): boolean) and replace the duplicated regex
checks in both shouldShowLine and isPullLine to call that helper or iterate
PULL_PATTERNS, keeping other unique tests in each function unchanged so both use
the single source of truth for pull-related patterns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 48577dc3-0aea-4b5f-97c4-e1641cbac7d3
📒 Files selected for processing (2)
src/lib/sandbox-create-stream.test.tssrc/lib/sandbox-create-stream.ts
latenighthackathon
force-pushed
the
fix/sandbox-pull-progress
branch
from
April 26, 2026 18:31
0d90f15 to
9c7143b
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/sandbox-create-stream.test.ts (1)
162-225: Optional: factor repeated stream setup into a small helper.The new tests repeat the same
streamSandboxCreatewiring; extracting a helper would reduce boilerplate and keep intent tighter.Refactor sketch
+function startStream(child: FakeChild, logLine = vi.fn()) { + return { + logLine, + promise: streamSandboxCreate("echo create", process.env, { + logLine, + spawnImpl: () => child as never, + heartbeatIntervalMs: 1_000, + silentPhaseMs: 10_000, + }), + }; +}🤖 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` around lines 162 - 225, The tests repeatedly create the same FakeChild/logLine/promise wiring for streamSandboxCreate; extract a small helper (e.g., makeStreamTest or createStreamSandboxFixture) that returns { child: FakeChild, logLine: vi.fn(), promise } and internally calls streamSandboxCreate with spawnImpl: () => child as never and default heartbeatIntervalMs and silentPhaseMs so each test can call the helper and then emit on child.stdout and child.close; update existing specs to use the helper to remove duplication around FakeChild, logLine, spawnImpl, heartbeatIntervalMs, and silentPhaseMs.src/lib/sandbox-create-stream.ts (1)
245-270: Consider adding a configurable hard stream timeout in follow-up.This improves phase accuracy, but a stuck pull/build stream can still run indefinitely. A max duration guard around the stream lifecycle would improve failure recovery and operator UX.
🤖 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 245 - 270, Add a configurable hard stream timeout to the stream lifecycle to avoid indefinite runs: introduce a maxStreamDurationMs option (or ENV/config) and record streamStart = Date.now() when the stream begins, then in the existing heartbeat interval (heartbeatTimer callback) check if Date.now() - streamStart > maxStreamDurationMs and if so call the same cleanup/settle logic used when streams fail (respecting settled flag) and emit/throw a clear timeout error tied to the sandbox creation flow; update references in the same module (heartbeatTimer, settled, lastOutputAt, elapsedSeconds, currentPhase, printProgressLine) so the timeout triggers cleanup, logging, and unref behavior consistently and make maxStreamDurationMs configurable/defaulted.
🤖 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`:
- Around line 162-225: The tests repeatedly create the same
FakeChild/logLine/promise wiring for streamSandboxCreate; extract a small helper
(e.g., makeStreamTest or createStreamSandboxFixture) that returns { child:
FakeChild, logLine: vi.fn(), promise } and internally calls streamSandboxCreate
with spawnImpl: () => child as never and default heartbeatIntervalMs and
silentPhaseMs so each test can call the helper and then emit on child.stdout and
child.close; update existing specs to use the helper to remove duplication
around FakeChild, logLine, spawnImpl, heartbeatIntervalMs, and silentPhaseMs.
In `@src/lib/sandbox-create-stream.ts`:
- Around line 245-270: Add a configurable hard stream timeout to the stream
lifecycle to avoid indefinite runs: introduce a maxStreamDurationMs option (or
ENV/config) and record streamStart = Date.now() when the stream begins, then in
the existing heartbeat interval (heartbeatTimer callback) check if Date.now() -
streamStart > maxStreamDurationMs and if so call the same cleanup/settle logic
used when streams fail (respecting settled flag) and emit/throw a clear timeout
error tied to the sandbox creation flow; update references in the same module
(heartbeatTimer, settled, lastOutputAt, elapsedSeconds, currentPhase,
printProgressLine) so the timeout triggers cleanup, logging, and unref behavior
consistently and make maxStreamDurationMs configurable/defaulted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4e1fa159-69e1-4ec8-b4a3-88fcb5e25439
📒 Files selected for processing (2)
src/lib/sandbox-create-stream.test.tssrc/lib/sandbox-create-stream.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/sandbox-create-stream.test.ts (1)
194-216: Tighten BuildKit assertion scope.Line 214 currently verifies only the phase banner. Consider asserting at least one BuildKit progress line is emitted to prevent silent regressions in
shouldShowLine.Proposed test-strengthening diff
await expect(promise).resolves.toMatchObject({ status: 0 }); expect(logLine).toHaveBeenCalledWith(" Pulling base image from registry..."); + expect(logLine).toHaveBeenCalledWith( + "#3 resolve ghcr.io/nvidia/nemoclaw/sandbox-base:latest", + );🤖 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` around lines 194 - 216, The test currently only asserts the phase banner; add an assertion that a BuildKit progress line was emitted by checking logLine was also called with a progress substring from the fake stdout (e.g. matching "sha256" or "MB" / "12.34MB") so the test validates that streamSandboxCreate/shouldShowLine actually forwards BuildKit progress; locate the test case using streamSandboxCreate, FakeChild, child.stdout.emit and logLine and add an expect that logLine was called with a string containing the BuildKit progress token (e.g. "sha256" or "12.34MB").
🤖 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`:
- Around line 194-216: The test currently only asserts the phase banner; add an
assertion that a BuildKit progress line was emitted by checking logLine was also
called with a progress substring from the fake stdout (e.g. matching "sha256" or
"MB" / "12.34MB") so the test validates that streamSandboxCreate/shouldShowLine
actually forwards BuildKit progress; locate the test case using
streamSandboxCreate, FakeChild, child.stdout.emit and logLine and add an expect
that logLine was called with a string containing the BuildKit progress token
(e.g. "sha256" or "12.34MB").
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cb3307e2-dccf-44d6-a7a7-71f667422b9f
📒 Files selected for processing (2)
src/lib/sandbox-create-stream.test.tssrc/lib/sandbox-create-stream.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/sandbox-create-stream.ts
cv
enabled auto-merge (squash)
April 27, 2026 17:20
auto-merge was automatically disabled
April 27, 2026 22:20
Head branch was pushed to by a user without write access
latenighthackathon
force-pushed
the
fix/sandbox-pull-progress
branch
3 times, most recently
from
April 29, 2026 07:58
7d618f1 to
e80e539
Compare
Collaborator
Author
|
Rebased onto current upstream/main — dropped the unsigned maintainer Update-Branch merge so DCO is green. Single signed commit on top now. Cheers! |
nemoclaw onboard's sandbox-create output filter recognized build and upload progress but dropped Docker pull output, so when the base image ghcr.io/nvidia/nemoclaw/sandbox-base:latest was not cached locally the user saw only "Still building sandbox image... (Ns elapsed)" for up to five minutes while the pull completed (NVIDIA#1829). Add a "pull" phase alongside build/upload/create/ready, with detection patterns for both classic Docker pull output (`<tag>: Pulling from <ref>`, `<id>: Pulling fs layer / Downloading / Extracting / Pull complete`, `Status: Downloaded`, `Digest:`) and BuildKit pull progress (`#N resolve <ref>`, `#N sha256:<id> <size> / <total>`). The tag prefix regex uses `[^:\s]+` so non-lowercase tags (`v1.2.3`, `cuda-12.5`, `12.4`) match. Spread `PULL_PROGRESS_PATTERNS` into `VISIBLE_PROGRESS_PATTERNS` so pull lines reach `logLine` (not just the phase banner). Emit a " Pulling base image from registry..." banner on transition and a " Still pulling base image from registry... (Ns elapsed)" heartbeat during the silence so users know the stall is a download, not a hang. Tests cover four angles: - classic Docker pull progress triggers the pull phase + visible lines - BuildKit pull progress triggers the phase + lines reach logLine (guards against silent regressions in shouldShowLine's pattern set) - non-lowercase tag prefixes (`v1.2.3`, `cuda-12.5`, `12.4`) recognized - pull-phase heartbeat fires (not the build-phase heartbeat) when only pull lines are flowing Closes NVIDIA#1829. Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
latenighthackathon
force-pushed
the
fix/sandbox-pull-progress
branch
from
May 5, 2026 02:04
e80e539 to
7ffc5b9
Compare
cv
approved these changes
May 5, 2026
wscurran
added
area: cli
23 tasks
jyaunches
added a commit
that referenced
this pull request
Aug 19, 2026
## Summary Cold sandbox base-image resolution currently runs synchronous, captured Docker pulls before the later sandbox-create stream starts. Users can therefore see no output while a large uncached image downloads. This change emits a bounded heartbeat during that pull without exposing captured Docker output or changing the pull result. PR #1897 covers pull output emitted by the later sandbox-create child process. This PR covers the separate earlier `resolveSandboxBaseImage()` pull identified as still silent in #3990. ## Related Issue Related to #3990. This PR addresses the cold base-image pull heartbeat only. Download concurrency and aggregate size estimation remain tracked in #3990; maintainer acceptance of this slice boundary is still pending. ## Changes - Generalize the existing out-of-process base-image heartbeat so it can identify build and pull work. - Wrap every synchronous remote pull and refresh in base-image resolution with the pull heartbeat. - Verify that all pull attempts pass through the heartbeat while preserving the existing Docker result. ## 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: The change adds progress feedback to an existing pull and does not change commands, defaults, configuration, APIs, or documented workflows. - [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: The wrapper preserves captured output, prints no image reference or credentials, and leaves the authoritative Docker result unchanged. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: The code change adds a heartbeat to an existing silent synchronous base-image pull. It does not change commands, defaults, configuration, or documented behavior, and it does not contradict existing documentation. - Agent: Codex Desktop <!-- docs-review-head-sha: a70ffa0 --> <!-- docs-review-agents-blob-sha: e30afb2 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## 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 validate:pr` passed after refreshing `origin/main` 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/sandbox-base-image/local-build-heartbeat.test.ts src/lib/sandbox-base-image-resolution.test.ts` passed 36 tests at latest PR commit `a70ffa0c54`, including execution of the real spawned Node heartbeat with captured pull output and SIGTERM cleanup. - [x] Applicable broad gate passed — `npm run validate:pr` passed at latest PR commit `a70ffa0c54`, including repository checks, secret scanning, formatting/lint, and CLI type checks. - [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: Ho Lim <subhoya@gmail.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved progress feedback while pulling sandbox base images. * Heartbeat activity is now accurately labeled during image pulls and builds. * Ensured heartbeat monitoring covers both initial and refreshed Docker image pulls. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Ho Lim <subhoya@gmail.com> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Julie Yaunches <jyaunches@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
nemoclaw onboard's sandbox-create stream filter recognizes build and upload progress but drops Docker pull output, so when the base imageghcr.io/nvidia/nemoclaw/sandbox-base:latestis not cached locally the user sees onlyStill building sandbox image... (Ns elapsed)for up to five minutes while the pull completes. The stall looks indistinguishable from a hang.Problem
src/lib/sandbox-create-stream.tspattern-matches lines againstshouldShowLineto decide what to render, and againstsetPhasetransitions to pick the heartbeat message. Both recognize build (Step N/N :,Building image) and upload (Pushing image,[progress]) but not Docker pull output, so:"build", so the heartbeat keeps printingStill building sandbox image... (315s elapsed)even though the childprocess is actually pulling a large base image.
Reported in #1829: the exact scenario is an uncached base image on macOS Docker where the pull emits BuildKit progress for 5+ minutes.
Fix
Add a new
"pull"phase alongsidebuild/upload/create/ready. Detect pull-indicative lines from both classic Docker (Pulling from,<id>: Pulling fs layer,: Downloading,: Extracting,: Pull complete,Status: Downloaded,Digest: sha256:...) and BuildKit (#N resolve <ref>,#N sha256:<id> <size> / <total>). On transition, emit a one-linePulling base image from registry...banner and use aStill pulling base image from registry... (Ns elapsed)heartbeat so users know the stall is a download, not a hang. A small subset of informative pull lines (Pulling from,Status: Downloaded) is allowed through for context.Test plan
"Pulling base image from registry..."banner, pull-phase heartbeat, pass-through of informative lines#N resolve,#N sha256:... MB / MB) → same pull-phase handling"build"behavior preservedStill building sandbox image...toStill pulling base image from registry...when pull lines appearsandbox-create-stream.test.tstests pass (7 existing + 3 new)npm run lint/npm run typecheck/npm run build:clicleanCloses #1829
Signed-off-by: latenighthackathon latenighthackathon@users.noreply.github.com
Summary by CodeRabbit
New Features
Tests