Skip to content

Commit 9402f99

Browse files
committed
fix(onboard): show base image pull heartbeat
Signed-off-by: Ho Lim <subhoya@gmail.com>
1 parent f5198b8 commit 9402f99

3 files changed

Lines changed: 47 additions & 20 deletions

File tree

src/lib/sandbox-base-image-resolution.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ const sourceMocks = vi.hoisted(() => ({
2121
inputsChanged: vi.fn(),
2222
nearestTags: vi.fn(),
2323
}));
24+
const heartbeatMocks = vi.hoisted(() => ({
25+
run: vi.fn((operation: () => unknown, _options?: { activity?: string }) => operation()),
26+
}));
2427

2528
vi.mock("./adapters/docker", () => ({
2629
dockerBuild: dockerMocks.build,
@@ -35,6 +38,10 @@ vi.mock("./trace", () => ({
3538
addTraceEvent: traceMocks.add,
3639
}));
3740

41+
vi.mock("./sandbox-base-image/local-build-heartbeat", () => ({
42+
withLocalBuildHeartbeat: heartbeatMocks.run,
43+
}));
44+
3845
vi.mock("./sandbox-base-image/source-identity", async (importOriginal) => ({
3946
...(await importOriginal<typeof import("./sandbox-base-image/source-identity")>()),
4047
baseImageInputsDirty: sourceMocks.inputsDirty,
@@ -179,6 +186,8 @@ describe("sandbox base-image warm resolution", () => {
179186
).toBeNull();
180187
expect(dockerMocks.imageInspect).toHaveBeenCalled();
181188
expect(dockerMocks.pull).toHaveBeenCalled();
189+
expect(heartbeatMocks.run).toHaveBeenCalledTimes(dockerMocks.pull.mock.calls.length);
190+
expect(heartbeatMocks.run.mock.calls.every((call) => call[1]?.activity === "pull")).toBe(true);
182191
expect(traceMocks.add).toHaveBeenCalledWith("nemoclaw.sandbox_base_image.cache_miss", {
183192
has_hint: true,
184193
});

src/lib/sandbox-base-image.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ function resolvePulledCandidate(
336336
: "nemoclaw.sandbox_base_image.remote_pull",
337337
{ source },
338338
);
339-
const pullResult = dockerPull(imageRef, { ignoreError: true, suppressOutput: true });
339+
const pullResult = pullSandboxBaseImage(imageRef);
340340
if (pullResult.status !== 0) return null;
341341
}
342342

@@ -356,14 +356,21 @@ function resolvePulledCandidate(
356356
imageRefCanRefresh(imageRef)
357357
) {
358358
addTraceEvent("nemoclaw.sandbox_base_image.remote_refresh", { source });
359-
const pullResult = dockerPull(imageRef, { ignoreError: true, suppressOutput: true });
359+
const pullResult = pullSandboxBaseImage(imageRef);
360360
if (pullResult.status !== 0) return null;
361361
return validatePulledCandidate(imageName, imageRef, source, options, candidateOptions, true);
362362
}
363363

364364
return null;
365365
}
366366

367+
function pullSandboxBaseImage(imageRef: string) {
368+
return withLocalBuildHeartbeat(
369+
() => dockerPull(imageRef, { ignoreError: true, suppressOutput: true }),
370+
{ activity: "pull" },
371+
);
372+
}
373+
367374
function resolveLocalCandidate(
368375
options: ResolveBaseImageOptions,
369376
forceBuild = false,

src/lib/sandbox-base-image/local-build-heartbeat.ts

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,32 +4,38 @@
44
import { spawn } from "node:child_process";
55

66
const DEFAULT_HEARTBEAT_INTERVAL_MS = 30_000;
7-
const HEARTBEAT_CHILD_SCRIPT = [
8-
"const intervalMs = Number(process.argv[1]);",
9-
"const parentPid = Number(process.argv[2]);",
10-
"const startedAt = Date.now();",
11-
"const stop = () => process.exit(0);",
12-
'process.on("SIGINT", stop);',
13-
'process.on("SIGTERM", stop);',
14-
"setInterval(() => {",
15-
" try { process.kill(parentPid, 0); } catch { stop(); return; }",
16-
" const elapsedSeconds = Math.max(0, Math.round((Date.now() - startedAt) / 1000));",
17-
" process.stdout.write(` ⏳ Still working on sandbox base image build… (${elapsedSeconds}s elapsed)\\n`);",
18-
"}, intervalMs);",
19-
].join("\n");
7+
8+
type HeartbeatActivity = "build" | "pull";
9+
10+
function heartbeatChildScript(activity: HeartbeatActivity): string {
11+
return [
12+
"const intervalMs = Number(process.argv[1]);",
13+
"const parentPid = Number(process.argv[2]);",
14+
"const startedAt = Date.now();",
15+
"const stop = () => process.exit(0);",
16+
'process.on("SIGINT", stop);',
17+
'process.on("SIGTERM", stop);',
18+
"setInterval(() => {",
19+
" try { process.kill(parentPid, 0); } catch { stop(); return; }",
20+
" const elapsedSeconds = Math.max(0, Math.round((Date.now() - startedAt) / 1000));",
21+
` process.stdout.write(\` ⏳ Still working on sandbox base image ${activity}… (\${elapsedSeconds}s elapsed)\\n\`);`,
22+
"}, intervalMs);",
23+
].join("\n");
24+
}
2025

2126
type SpawnHeartbeat = typeof spawn;
2227

2328
export interface LocalBuildHeartbeatOptions {
29+
activity?: HeartbeatActivity;
2430
intervalMs?: number;
2531
nodeExecutable?: string;
2632
parentPid?: number;
2733
spawnImpl?: SpawnHeartbeat;
2834
}
2935

30-
/** Keep quiet synchronous Docker builds observable without exposing their captured logs. */
36+
/** Keep quiet synchronous Docker work observable without exposing its captured logs. */
3137
export function withLocalBuildHeartbeat<T>(
32-
build: () => T,
38+
operation: () => T,
3339
options: LocalBuildHeartbeatOptions = {},
3440
): T {
3541
const intervalMs =
@@ -40,7 +46,12 @@ export function withLocalBuildHeartbeat<T>(
4046
try {
4147
child = (options.spawnImpl ?? spawn)(
4248
options.nodeExecutable ?? process.execPath,
43-
["-e", HEARTBEAT_CHILD_SCRIPT, String(intervalMs), String(options.parentPid ?? process.pid)],
49+
[
50+
"-e",
51+
heartbeatChildScript(options.activity ?? "build"),
52+
String(intervalMs),
53+
String(options.parentPid ?? process.pid),
54+
],
4455
{ env: {}, stdio: ["ignore", "inherit", "inherit"] },
4556
);
4657
child.on("error", () => undefined);
@@ -50,12 +61,12 @@ export function withLocalBuildHeartbeat<T>(
5061
}
5162

5263
try {
53-
return build();
64+
return operation();
5465
} finally {
5566
try {
5667
child?.kill("SIGTERM");
5768
} catch {
58-
// Progress reporting must never replace the Docker build result.
69+
// Progress reporting must never replace the Docker operation result.
5970
}
6071
}
6172
}

0 commit comments

Comments
 (0)