From 22c39549bbb6d2aa1796647ecc26973293252545 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Tue, 30 Jun 2026 00:24:30 -0700 Subject: [PATCH 1/2] perf(docker): preserve warm sandbox build cache Co-authored-by: Angel Mata Signed-off-by: Angel Mata Signed-off-by: Carlos Villela --- Dockerfile | 6 +- src/lib/onboard/dockerfile-patch.test.ts | 125 ++++++++++++++++++++- src/lib/onboard/dockerfile-patch.ts | 21 +++- test/fixtures/warm-build-cache-evidence.md | 74 ++++++++++++ 4 files changed, 216 insertions(+), 10 deletions(-) create mode 100644 test/fixtures/warm-build-cache-evidence.md diff --git a/Dockerfile b/Dockerfile index 7b25ff2e2f9..558f013f15b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -613,9 +613,9 @@ ARG NEMOCLAW_EXTRA_AGENTS_JSON_B64=W10= # since terminal-based pairing is impossible in those contexts. # Default: "0" (device auth enabled for local deployments — secure by default). ARG NEMOCLAW_DISABLE_DEVICE_AUTH=0 -# Unique per build — busts the Docker cache for the token-injection layer -# so each image gets a fresh gateway auth token. -# Pass --build-arg NEMOCLAW_BUILD_ID=$(date +%s) to bust the cache. +# Compatibility build arg for older custom Dockerfiles and rebuild tooling. +# NemoClaw-managed images intentionally do not consume it; gateway auth tokens +# are generated at container startup and are never baked into image layers. ARG NEMOCLAW_BUILD_ID=default # macOS OpenShell VM backend imports the Docker image into a virtiofs rootfs # where image uid/gid ownership is presented as the host user. The VM also diff --git a/src/lib/onboard/dockerfile-patch.test.ts b/src/lib/onboard/dockerfile-patch.test.ts index 2d860d17b97..6c7224f7fae 100644 --- a/src/lib/onboard/dockerfile-patch.test.ts +++ b/src/lib/onboard/dockerfile-patch.test.ts @@ -16,6 +16,7 @@ import { } from "./dockerfile-patch"; const tmpRoots: string[] = []; +const REPO_ROOT = path.resolve(import.meta.dirname, "../../.."); beforeEach(() => { delete process.env.NEMOCLAW_MESSAGING_PLAN_B64; @@ -196,7 +197,7 @@ describe("dockerfile patch helpers", () => { expect(patched).toContain("ARG NEMOCLAW_PRIMARY_MODEL_REF=inference/custom-model"); expect(patched).toContain("ARG CHAT_UI_URL=https://chat.example"); expect(patched).toContain("ARG NEMOCLAW_INFERENCE_COMPAT_B64="); - expect(patched).toContain("ARG NEMOCLAW_BUILD_ID=build-1"); + expect(patched).toContain("ARG NEMOCLAW_BUILD_ID=old"); expect(patched).toContain("ARG NEMOCLAW_DARWIN_VM_COMPAT=1"); expect(patched).toContain("ARG NEMOCLAW_PROXY_HOST=host.docker.internal"); expect(patched).toContain("ARG NEMOCLAW_PROXY_PORT=3128"); @@ -393,7 +394,8 @@ describe("dockerfile patch helpers", () => { expect(patched).not.toMatch(/\r|\nRUN touch/); expect(patched).toContain("ARG NEMOCLAW_MODEL=modelRUN touch /tmp/model-pwn"); expect(patched).toContain("ARG CHAT_UI_URL=https://chat.exampleRUN touch /tmp/chat-pwn"); - expect(patched).toContain("ARG NEMOCLAW_BUILD_ID=build-1RUN touch /tmp/build-pwn"); + expect(patched).toContain("ARG NEMOCLAW_BUILD_ID=old"); + expect(patched).not.toContain("build-1RUN touch /tmp/build-pwn"); expect(patched).toContain("ARG NEMOCLAW_INFERENCE_API=openai-responsesRUN touch /tmp/api-pwn"); expect(patched).toContain( "ARG BASE_IMAGE=ghcr.io/nvidia/nemoclaw/sandbox-base@sha256:abcRUN touch /tmp/base-pwn", @@ -429,13 +431,130 @@ describe("dockerfile patch helpers", () => { assert.match(patched, /^ARG NEMOCLAW_PROVIDER_KEY=openai$/m); assert.match(patched, /^ARG NEMOCLAW_PRIMARY_MODEL_REF=openai\/gpt-5\.4$/m); assert.match(patched, /^ARG CHAT_UI_URL=http:\/\/127\.0\.0\.1:19999$/m); - assert.match(patched, /^ARG NEMOCLAW_BUILD_ID=build-123$/m); + assert.match(patched, /^ARG NEMOCLAW_BUILD_ID=default$/m); assert.match(patched, /^ARG NEMOCLAW_DARWIN_VM_COMPAT=0$/m); } finally { fs.rmSync(tmpDir, { recursive: true, force: true }); } }); + it("leaves NEMOCLAW_BUILD_ID stable unless the Dockerfile consumes it (#4682)", () => { + const unconsumedDockerfilePath = dockerfileWith( + [ + "ARG NEMOCLAW_MODEL=old", + "ARG NEMOCLAW_PROVIDER_KEY=old", + "ARG NEMOCLAW_PRIMARY_MODEL_REF=old", + "ARG CHAT_UI_URL=old", + "ARG NEMOCLAW_INFERENCE_BASE_URL=old", + "ARG NEMOCLAW_INFERENCE_API=old", + "ARG NEMOCLAW_INFERENCE_COMPAT_B64=old", + "# NEMOCLAW_BUILD_ID is documented here but not consumed.", + "ARG NEMOCLAW_BUILD_ID=default", + "ARG NEMOCLAW_DARWIN_VM_COMPAT=0", + ].join("\n"), + ); + + patchStagedDockerfile( + unconsumedDockerfilePath, + "gpt-5.4", + "http://127.0.0.1:19999", + "build-cache-stable", + "openai-api", + ); + + expect(fs.readFileSync(unconsumedDockerfilePath, "utf8")).toMatch( + /^ARG NEMOCLAW_BUILD_ID=default$/m, + ); + + for (const [index, consumerLine] of [ + "ENV LEGACY_BUILD_ID=${NEMOCLAW_BUILD_ID}", + "RUN printf '%s\\n' ${NEMOCLAW_BUILD_ID}", + "ARG LEGACY_LABEL=prefix-${NEMOCLAW_BUILD_ID}-suffix", + "LABEL legacy.build=${NEMOCLAW_BUILD_ID}", + ].entries()) { + const consumedDockerfilePath = dockerfileWith( + [ + "ARG NEMOCLAW_MODEL=old", + "ARG NEMOCLAW_PROVIDER_KEY=old", + "ARG NEMOCLAW_PRIMARY_MODEL_REF=old", + "ARG CHAT_UI_URL=old", + "ARG NEMOCLAW_INFERENCE_BASE_URL=old", + "ARG NEMOCLAW_INFERENCE_API=old", + "ARG NEMOCLAW_INFERENCE_COMPAT_B64=old", + "ARG NEMOCLAW_BUILD_ID=default", + consumerLine, + "ARG NEMOCLAW_DARWIN_VM_COMPAT=0", + ].join("\n"), + ); + patchStagedDockerfile( + consumedDockerfilePath, + "gpt-5.4", + "http://127.0.0.1:19999", + `build-cache-bust-${index}`, + "openai-api", + ); + + expect(fs.readFileSync(consumedDockerfilePath, "utf8")).toMatch( + new RegExp(`^ARG NEMOCLAW_BUILD_ID=build-cache-bust-${index}$`, "m"), + ); + } + }); + + it.each([ + ["OpenClaw", path.join(REPO_ROOT, "Dockerfile")], + ["Hermes", path.join(REPO_ROOT, "agents", "hermes", "Dockerfile")], + ])("keeps the stock %s build context byte-identical when only the per-run build ID changes (#4682)", (_agent, stockDockerfile) => { + const stockSource = fs.readFileSync(stockDockerfile, "utf8"); + const firstBuild = dockerfileWith(stockSource); + const secondBuild = dockerfileWith(stockSource); + + patchStagedDockerfile( + firstBuild, + "gpt-5.4", + "http://127.0.0.1:19999", + "first-per-run-id", + "openai-api", + ); + patchStagedDockerfile( + secondBuild, + "gpt-5.4", + "http://127.0.0.1:19999", + "second-per-run-id", + "openai-api", + ); + + expect(fs.readFileSync(firstBuild, "utf8")).toBe(fs.readFileSync(secondBuild, "utf8")); + }); + + it("sanitizes a per-run build ID when a custom Dockerfile consumes it (#4682)", () => { + const dockerfilePath = dockerfileWith( + [ + "ARG NEMOCLAW_MODEL=old", + "ARG NEMOCLAW_PROVIDER_KEY=old", + "ARG NEMOCLAW_PRIMARY_MODEL_REF=old", + "ARG CHAT_UI_URL=old", + "ARG NEMOCLAW_INFERENCE_BASE_URL=old", + "ARG NEMOCLAW_INFERENCE_API=old", + "ARG NEMOCLAW_INFERENCE_COMPAT_B64=old", + "ARG NEMOCLAW_BUILD_ID=default", + "LABEL legacy.build=${NEMOCLAW_BUILD_ID}", + "ARG NEMOCLAW_DARWIN_VM_COMPAT=0", + ].join("\n"), + ); + + patchStagedDockerfile( + dockerfilePath, + "gpt-5.4", + "http://127.0.0.1:19999", + "build-safe\nRUN touch /tmp/build-id-injection", + "openai-api", + ); + + const patched = fs.readFileSync(dockerfilePath, "utf8"); + expect(patched).toContain("ARG NEMOCLAW_BUILD_ID=build-safeRUN touch /tmp/build-id-injection"); + expect(patched).not.toMatch(/\nRUN touch \/tmp\/build-id-injection/); + }); + it("patches the staged Dockerfile for macOS VM rootfs ownership compatibility", () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-darwin-vm-")); const dockerfilePath = path.join(tmpDir, "Dockerfile"); diff --git a/src/lib/onboard/dockerfile-patch.ts b/src/lib/onboard/dockerfile-patch.ts index 7258a4bf1f4..a9375ff24d2 100644 --- a/src/lib/onboard/dockerfile-patch.ts +++ b/src/lib/onboard/dockerfile-patch.ts @@ -77,6 +77,17 @@ function encodeSanitizedDockerJsonArg(value: unknown): string { return sanitizeDockerArg(encodeDockerJsonArg(value)); } +function dockerfileConsumesBuildId(dockerfile: string): boolean { + return dockerfile.split("\n").some((line) => { + const trimmed = line.trimStart(); + return ( + !trimmed.startsWith("#") && + !/^ARG\s+NEMOCLAW_BUILD_ID(?:=.*)?$/.test(trimmed) && + /\bNEMOCLAW_BUILD_ID\b/.test(trimmed) + ); + }); +} + export function isValidProxyHost(value: string): boolean { return PROXY_HOST_RE.test(value); } @@ -171,10 +182,12 @@ export function patchStagedDockerfile( /^ARG NEMOCLAW_INFERENCE_COMPAT_B64=.*$/m, `ARG NEMOCLAW_INFERENCE_COMPAT_B64=${encodeSanitizedDockerJsonArg(inferenceCompat)}`, ); - dockerfile = dockerfile.replace( - /^ARG NEMOCLAW_BUILD_ID=.*$/m, - `ARG NEMOCLAW_BUILD_ID=${sanitizeDockerArg(buildId)}`, - ); + if (dockerfileConsumesBuildId(dockerfile)) { + dockerfile = dockerfile.replace( + /^ARG NEMOCLAW_BUILD_ID=.*$/m, + `ARG NEMOCLAW_BUILD_ID=${sanitizeDockerArg(buildId)}`, + ); + } dockerfile = dockerfile.replace( /^ARG NEMOCLAW_DARWIN_VM_COMPAT=.*$/m, `ARG NEMOCLAW_DARWIN_VM_COMPAT=${sanitizeDockerArg(darwinVmCompat ? "1" : "0")}`, diff --git a/test/fixtures/warm-build-cache-evidence.md b/test/fixtures/warm-build-cache-evidence.md new file mode 100644 index 00000000000..b3b657628f1 --- /dev/null +++ b/test/fixtures/warm-build-cache-evidence.md @@ -0,0 +1,74 @@ + + + +# Warm Sandbox Build Cache Evidence + +This fixture records the manual cache validation for issue #4682. It is not a +user-facing guide; it gives reviewers an auditable command shape and expected +cache behavior for stabilizing the otherwise-unused per-run build ID. + +## Method + +The measurement keeps shared base images on the host and removes only generated +NemoClaw/OpenShell sandbox images (`openshell/sandbox-from:*`) for the cold run. +That isolates final-image layer reuse instead of measuring base-image pulls. + +For each agent: + +1. Delete the measurement sandbox if it exists: + + ```bash + openshell sandbox delete warm-cache-openclaw || true + openshell sandbox delete warm-cache-hermes || true + ``` + +2. Delete the generated measurement image before the cold run: + + ```bash + docker image rm openshell/sandbox-from: + ``` + +3. Run onboard with stable inputs and record the + `Sandbox image build completed in ...` line: + + ```bash + NEMOCLAW_NON_INTERACTIVE=1 \ + NEMOCLAW_RECREATE_SANDBOX=1 \ + NEMOCLAW_SANDBOX_NAME=warm-cache-openclaw \ + NEMOCLAW_PROVIDER=custom \ + NEMOCLAW_MODEL=test-model \ + NEMOCLAW_ENDPOINT_URL=http://host.openshell.internal:11434/v1 \ + COMPATIBLE_API_KEY=warm-cache-dummy-key \ + node bin/nemoclaw.js onboard \ + --non-interactive --yes --fresh --recreate-sandbox \ + --name warm-cache-openclaw \ + --yes-i-accept-third-party-software + ``` + + For Hermes, add `--agent hermes` and use + `NEMOCLAW_SANDBOX_NAME=warm-cache-hermes` / `--name warm-cache-hermes`. + +4. Stop the post-build readiness wait after the timing line, delete the sandbox, + keep the generated image, and rerun the same command for the warm run. + +## Observed Results + +| Agent | Cold build | Warm build | Expected warm-cache behavior | +| --- | ---: | ---: | --- | +| OpenClaw | `20.9s` | `0.1s` | Stable Dockerfile/build context reuses build-time config, plugin install, proxy, OTEL, permission, and hash layers. | +| Hermes | `21.5s` | `0.4s` | Stable Dockerfile/build context reuses runtime setup, config generation, agent-install, permission, and config-hash layers. | + +Warm builds showed the derived-image Docker steps completing at `0.0s` or +`0.1s`. `ARG NEMOCLAW_BUILD_ID=default` remained stable in stock staged +Dockerfiles; custom Dockerfiles that reference `NEMOCLAW_BUILD_ID` still receive +the supplied build ID. + +A separate BuildKit control probe confirmed that changing an in-scope `ARG` +invalidates a following `RUN` layer even when that instruction does not mention +the argument. This change therefore does not claim that moving `ENV` instructions +can protect layers from other changed build arguments. The automated regression +instead patches both stock Dockerfiles with two different per-run build IDs and +requires the resulting build contexts to remain byte-identical. + +The post-build OpenShell GPU reconnect/readiness step is outside this cache +measurement and can be handled separately from Docker build-layer reuse. From c740d4cc3b63d541e7881e3d8206dc31d581501b Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Tue, 30 Jun 2026 00:48:03 -0700 Subject: [PATCH 2/2] fix(docker): preserve custom build-id compatibility Signed-off-by: Carlos Villela --- .../onboard/dockerfile-patch-build-id.test.ts | 100 ++++++++++++++ src/lib/onboard/dockerfile-patch.test.ts | 125 +----------------- src/lib/onboard/dockerfile-patch.ts | 19 ++- .../sandbox-dockerfile-patch-flow.test.ts | 42 +++++- .../onboard/sandbox-dockerfile-patch-flow.ts | 11 ++ test/fixtures/warm-build-cache-evidence.md | 57 +++++++- 6 files changed, 217 insertions(+), 137 deletions(-) create mode 100644 src/lib/onboard/dockerfile-patch-build-id.test.ts diff --git a/src/lib/onboard/dockerfile-patch-build-id.test.ts b/src/lib/onboard/dockerfile-patch-build-id.test.ts new file mode 100644 index 00000000000..7dfee8f8fe5 --- /dev/null +++ b/src/lib/onboard/dockerfile-patch-build-id.test.ts @@ -0,0 +1,100 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, describe, expect, it } from "vitest"; + +import { type DockerfileBuildIdPolicy, patchStagedDockerfile } from "./dockerfile-patch"; + +const REPO_ROOT = path.resolve(import.meta.dirname, "../../.."); +const tmpRoots: string[] = []; + +function dockerfileWith(content: string): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-build-id-test-")); + tmpRoots.push(root); + const dockerfile = path.join(root, "Dockerfile"); + fs.writeFileSync(dockerfile, content, "utf8"); + return dockerfile; +} + +function patchBuildId( + dockerfile: string, + buildId: string, + buildIdPolicy?: DockerfileBuildIdPolicy, +): void { + patchStagedDockerfile( + dockerfile, + "gpt-5.4", + "http://127.0.0.1:19999", + buildId, + "openai-api", + null, + null, + null, + false, + null, + [], + buildIdPolicy ? { buildIdPolicy } : undefined, + ); +} + +afterEach(() => { + for (const root of tmpRoots.splice(0)) { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +describe("Dockerfile build-id cache policy (#4682)", () => { + it("rewrites custom Dockerfiles even when an invoked script is the indirect consumer", () => { + const dockerfile = dockerfileWith( + [ + "FROM scratch", + "ARG NEMOCLAW_BUILD_ID=default", + "COPY generate-token.js /tmp/generate-token.js", + "RUN node /tmp/generate-token.js", + '# Literal documentation: "NEMOCLAW_BUILD_ID".', + ].join("\n"), + ); + + patchBuildId(dockerfile, "custom-per-run-id"); + + expect(fs.readFileSync(dockerfile, "utf8")).toMatch( + /^ARG NEMOCLAW_BUILD_ID=custom-per-run-id$/m, + ); + }); + + it.each([ + ["OpenClaw", path.join(REPO_ROOT, "Dockerfile")], + ["Hermes", path.join(REPO_ROOT, "agents", "hermes", "Dockerfile")], + ])("keeps the managed stock %s context byte-identical across per-run IDs", (agentName, stockDockerfile) => { + expect(fs.existsSync(stockDockerfile), `missing managed ${agentName} Dockerfile`).toBe(true); + const stockSource = fs.readFileSync(stockDockerfile, "utf8"); + const buildIdLines = stockSource + .split("\n") + .filter((line) => line.includes("NEMOCLAW_BUILD_ID") && !line.trimStart().startsWith("#")); + expect(buildIdLines, `${agentName} must not consume the preserved build ID`).toEqual([ + "ARG NEMOCLAW_BUILD_ID=default", + ]); + + const firstBuild = dockerfileWith(stockSource); + const secondBuild = dockerfileWith(stockSource); + patchBuildId(firstBuild, "first-per-run-id", "preserve"); + patchBuildId(secondBuild, "second-per-run-id", "preserve"); + + expect(fs.readFileSync(firstBuild, "utf8")).toBe(fs.readFileSync(secondBuild, "utf8")); + expect(fs.readFileSync(firstBuild, "utf8")).toMatch(/^ARG NEMOCLAW_BUILD_ID=default$/m); + }); + + it("sanitizes the custom per-run build ID", () => { + const dockerfile = dockerfileWith("ARG NEMOCLAW_BUILD_ID=default\n"); + + patchBuildId(dockerfile, "build-safe\nRUN touch /tmp/build-id-injection"); + + const patched = fs.readFileSync(dockerfile, "utf8"); + expect(patched).toContain("ARG NEMOCLAW_BUILD_ID=build-safeRUN touch /tmp/build-id-injection"); + expect(patched).not.toMatch(/\nRUN touch \/tmp\/build-id-injection/); + }); +}); diff --git a/src/lib/onboard/dockerfile-patch.test.ts b/src/lib/onboard/dockerfile-patch.test.ts index 6c7224f7fae..2d860d17b97 100644 --- a/src/lib/onboard/dockerfile-patch.test.ts +++ b/src/lib/onboard/dockerfile-patch.test.ts @@ -16,7 +16,6 @@ import { } from "./dockerfile-patch"; const tmpRoots: string[] = []; -const REPO_ROOT = path.resolve(import.meta.dirname, "../../.."); beforeEach(() => { delete process.env.NEMOCLAW_MESSAGING_PLAN_B64; @@ -197,7 +196,7 @@ describe("dockerfile patch helpers", () => { expect(patched).toContain("ARG NEMOCLAW_PRIMARY_MODEL_REF=inference/custom-model"); expect(patched).toContain("ARG CHAT_UI_URL=https://chat.example"); expect(patched).toContain("ARG NEMOCLAW_INFERENCE_COMPAT_B64="); - expect(patched).toContain("ARG NEMOCLAW_BUILD_ID=old"); + expect(patched).toContain("ARG NEMOCLAW_BUILD_ID=build-1"); expect(patched).toContain("ARG NEMOCLAW_DARWIN_VM_COMPAT=1"); expect(patched).toContain("ARG NEMOCLAW_PROXY_HOST=host.docker.internal"); expect(patched).toContain("ARG NEMOCLAW_PROXY_PORT=3128"); @@ -394,8 +393,7 @@ describe("dockerfile patch helpers", () => { expect(patched).not.toMatch(/\r|\nRUN touch/); expect(patched).toContain("ARG NEMOCLAW_MODEL=modelRUN touch /tmp/model-pwn"); expect(patched).toContain("ARG CHAT_UI_URL=https://chat.exampleRUN touch /tmp/chat-pwn"); - expect(patched).toContain("ARG NEMOCLAW_BUILD_ID=old"); - expect(patched).not.toContain("build-1RUN touch /tmp/build-pwn"); + expect(patched).toContain("ARG NEMOCLAW_BUILD_ID=build-1RUN touch /tmp/build-pwn"); expect(patched).toContain("ARG NEMOCLAW_INFERENCE_API=openai-responsesRUN touch /tmp/api-pwn"); expect(patched).toContain( "ARG BASE_IMAGE=ghcr.io/nvidia/nemoclaw/sandbox-base@sha256:abcRUN touch /tmp/base-pwn", @@ -431,130 +429,13 @@ describe("dockerfile patch helpers", () => { assert.match(patched, /^ARG NEMOCLAW_PROVIDER_KEY=openai$/m); assert.match(patched, /^ARG NEMOCLAW_PRIMARY_MODEL_REF=openai\/gpt-5\.4$/m); assert.match(patched, /^ARG CHAT_UI_URL=http:\/\/127\.0\.0\.1:19999$/m); - assert.match(patched, /^ARG NEMOCLAW_BUILD_ID=default$/m); + assert.match(patched, /^ARG NEMOCLAW_BUILD_ID=build-123$/m); assert.match(patched, /^ARG NEMOCLAW_DARWIN_VM_COMPAT=0$/m); } finally { fs.rmSync(tmpDir, { recursive: true, force: true }); } }); - it("leaves NEMOCLAW_BUILD_ID stable unless the Dockerfile consumes it (#4682)", () => { - const unconsumedDockerfilePath = dockerfileWith( - [ - "ARG NEMOCLAW_MODEL=old", - "ARG NEMOCLAW_PROVIDER_KEY=old", - "ARG NEMOCLAW_PRIMARY_MODEL_REF=old", - "ARG CHAT_UI_URL=old", - "ARG NEMOCLAW_INFERENCE_BASE_URL=old", - "ARG NEMOCLAW_INFERENCE_API=old", - "ARG NEMOCLAW_INFERENCE_COMPAT_B64=old", - "# NEMOCLAW_BUILD_ID is documented here but not consumed.", - "ARG NEMOCLAW_BUILD_ID=default", - "ARG NEMOCLAW_DARWIN_VM_COMPAT=0", - ].join("\n"), - ); - - patchStagedDockerfile( - unconsumedDockerfilePath, - "gpt-5.4", - "http://127.0.0.1:19999", - "build-cache-stable", - "openai-api", - ); - - expect(fs.readFileSync(unconsumedDockerfilePath, "utf8")).toMatch( - /^ARG NEMOCLAW_BUILD_ID=default$/m, - ); - - for (const [index, consumerLine] of [ - "ENV LEGACY_BUILD_ID=${NEMOCLAW_BUILD_ID}", - "RUN printf '%s\\n' ${NEMOCLAW_BUILD_ID}", - "ARG LEGACY_LABEL=prefix-${NEMOCLAW_BUILD_ID}-suffix", - "LABEL legacy.build=${NEMOCLAW_BUILD_ID}", - ].entries()) { - const consumedDockerfilePath = dockerfileWith( - [ - "ARG NEMOCLAW_MODEL=old", - "ARG NEMOCLAW_PROVIDER_KEY=old", - "ARG NEMOCLAW_PRIMARY_MODEL_REF=old", - "ARG CHAT_UI_URL=old", - "ARG NEMOCLAW_INFERENCE_BASE_URL=old", - "ARG NEMOCLAW_INFERENCE_API=old", - "ARG NEMOCLAW_INFERENCE_COMPAT_B64=old", - "ARG NEMOCLAW_BUILD_ID=default", - consumerLine, - "ARG NEMOCLAW_DARWIN_VM_COMPAT=0", - ].join("\n"), - ); - patchStagedDockerfile( - consumedDockerfilePath, - "gpt-5.4", - "http://127.0.0.1:19999", - `build-cache-bust-${index}`, - "openai-api", - ); - - expect(fs.readFileSync(consumedDockerfilePath, "utf8")).toMatch( - new RegExp(`^ARG NEMOCLAW_BUILD_ID=build-cache-bust-${index}$`, "m"), - ); - } - }); - - it.each([ - ["OpenClaw", path.join(REPO_ROOT, "Dockerfile")], - ["Hermes", path.join(REPO_ROOT, "agents", "hermes", "Dockerfile")], - ])("keeps the stock %s build context byte-identical when only the per-run build ID changes (#4682)", (_agent, stockDockerfile) => { - const stockSource = fs.readFileSync(stockDockerfile, "utf8"); - const firstBuild = dockerfileWith(stockSource); - const secondBuild = dockerfileWith(stockSource); - - patchStagedDockerfile( - firstBuild, - "gpt-5.4", - "http://127.0.0.1:19999", - "first-per-run-id", - "openai-api", - ); - patchStagedDockerfile( - secondBuild, - "gpt-5.4", - "http://127.0.0.1:19999", - "second-per-run-id", - "openai-api", - ); - - expect(fs.readFileSync(firstBuild, "utf8")).toBe(fs.readFileSync(secondBuild, "utf8")); - }); - - it("sanitizes a per-run build ID when a custom Dockerfile consumes it (#4682)", () => { - const dockerfilePath = dockerfileWith( - [ - "ARG NEMOCLAW_MODEL=old", - "ARG NEMOCLAW_PROVIDER_KEY=old", - "ARG NEMOCLAW_PRIMARY_MODEL_REF=old", - "ARG CHAT_UI_URL=old", - "ARG NEMOCLAW_INFERENCE_BASE_URL=old", - "ARG NEMOCLAW_INFERENCE_API=old", - "ARG NEMOCLAW_INFERENCE_COMPAT_B64=old", - "ARG NEMOCLAW_BUILD_ID=default", - "LABEL legacy.build=${NEMOCLAW_BUILD_ID}", - "ARG NEMOCLAW_DARWIN_VM_COMPAT=0", - ].join("\n"), - ); - - patchStagedDockerfile( - dockerfilePath, - "gpt-5.4", - "http://127.0.0.1:19999", - "build-safe\nRUN touch /tmp/build-id-injection", - "openai-api", - ); - - const patched = fs.readFileSync(dockerfilePath, "utf8"); - expect(patched).toContain("ARG NEMOCLAW_BUILD_ID=build-safeRUN touch /tmp/build-id-injection"); - expect(patched).not.toMatch(/\nRUN touch \/tmp\/build-id-injection/); - }); - it("patches the staged Dockerfile for macOS VM rootfs ownership compatibility", () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-darwin-vm-")); const dockerfilePath = path.join(tmpDir, "Dockerfile"); diff --git a/src/lib/onboard/dockerfile-patch.ts b/src/lib/onboard/dockerfile-patch.ts index a9375ff24d2..23dd8099e80 100644 --- a/src/lib/onboard/dockerfile-patch.ts +++ b/src/lib/onboard/dockerfile-patch.ts @@ -77,15 +77,10 @@ function encodeSanitizedDockerJsonArg(value: unknown): string { return sanitizeDockerArg(encodeDockerJsonArg(value)); } -function dockerfileConsumesBuildId(dockerfile: string): boolean { - return dockerfile.split("\n").some((line) => { - const trimmed = line.trimStart(); - return ( - !trimmed.startsWith("#") && - !/^ARG\s+NEMOCLAW_BUILD_ID(?:=.*)?$/.test(trimmed) && - /\bNEMOCLAW_BUILD_ID\b/.test(trimmed) - ); - }); +export type DockerfileBuildIdPolicy = "preserve" | "rewrite"; + +export interface PatchStagedDockerfileOptions { + buildIdPolicy?: DockerfileBuildIdPolicy; } export function isValidProxyHost(value: string): boolean { @@ -110,6 +105,7 @@ export function patchStagedDockerfile( darwinVmCompat = false, inferenceBaseUrlOverride: string | null = null, hermesToolGateways: string[] = [], + options: PatchStagedDockerfileOptions = {}, ): void { const sanitizedModel = sanitizeDockerArg(model); const sandboxInference = getSandboxInferenceConfig( @@ -182,7 +178,10 @@ export function patchStagedDockerfile( /^ARG NEMOCLAW_INFERENCE_COMPAT_B64=.*$/m, `ARG NEMOCLAW_INFERENCE_COMPAT_B64=${encodeSanitizedDockerJsonArg(inferenceCompat)}`, ); - if (dockerfileConsumesBuildId(dockerfile)) { + // Rewriting is the compatibility-safe default for custom and legacy + // Dockerfiles. Only callers with explicit knowledge of a managed stock + // Dockerfile may preserve the declaration to keep warm builds cacheable. + if (options.buildIdPolicy !== "preserve") { dockerfile = dockerfile.replace( /^ARG NEMOCLAW_BUILD_ID=.*$/m, `ARG NEMOCLAW_BUILD_ID=${sanitizeDockerArg(buildId)}`, diff --git a/src/lib/onboard/sandbox-dockerfile-patch-flow.test.ts b/src/lib/onboard/sandbox-dockerfile-patch-flow.test.ts index c1f270617eb..bb625e9b236 100644 --- a/src/lib/onboard/sandbox-dockerfile-patch-flow.test.ts +++ b/src/lib/onboard/sandbox-dockerfile-patch-flow.test.ts @@ -2,9 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 import { describe, expect, it, vi } from "vitest"; - -import type { SandboxGpuConfig } from "./sandbox-gpu-mode"; import { prepareSandboxDockerfilePatch } from "./sandbox-dockerfile-patch-flow"; +import type { SandboxGpuConfig } from "./sandbox-gpu-mode"; const sandboxGpuConfig: SandboxGpuConfig = { mode: "auto", @@ -74,12 +73,14 @@ describe("prepareSandboxDockerfilePatch", () => { false, null, ["github"], + { buildIdPolicy: "preserve" }, ); }); it("skips base-image resolution for agent default Dockerfiles", async () => { const pullAndResolveBaseImageDigest = vi.fn(); const dockerImageInspect = vi.fn(); + const patchStagedDockerfile = vi.fn(); const result = await prepareSandboxDockerfilePatch({ agent: { name: "hermes" } as any, fromDockerfile: null, @@ -98,7 +99,7 @@ describe("prepareSandboxDockerfilePatch", () => { pullAndResolveBaseImageDigest, dockerImageInspect, enforceDockerGpuPatchPreserveNetwork: vi.fn(async () => false), - patchStagedDockerfile: vi.fn(), + patchStagedDockerfile, now: () => 1, }, }); @@ -106,6 +107,9 @@ describe("prepareSandboxDockerfilePatch", () => { expect(result.resolvedBaseImage).toBeNull(); expect(pullAndResolveBaseImageDigest).not.toHaveBeenCalled(); expect(dockerImageInspect).not.toHaveBeenCalled(); + expect(patchStagedDockerfile.mock.calls[0]?.[11]).toEqual({ + buildIdPolicy: "preserve", + }); }); it("resolves the base image when an agent uses a custom Dockerfile", async () => { @@ -148,6 +152,38 @@ describe("prepareSandboxDockerfilePatch", () => { expect(patchStagedDockerfile.mock.calls[0]?.[7]).toBe( "ghcr.io/nvidia/nemoclaw/sandbox-base@sha256:customagent", ); + expect(patchStagedDockerfile.mock.calls[0]?.[11]).toEqual({ + buildIdPolicy: "rewrite", + }); + }); + + it("keeps the per-run rewrite for managed agents that consume the build id", async () => { + const patchStagedDockerfile = vi.fn(); + + await prepareSandboxDockerfilePatch({ + agent: { name: "langchain-deepagents-code" } as any, + fromDockerfile: null, + sandboxBaseImage: "ghcr.io/nvidia/nemoclaw/sandbox-base", + sandboxBaseTag: "latest", + stagedDockerfile: "/tmp/Dockerfile", + model: "model-a", + chatUiUrl: "http://127.0.0.1:7000", + provider: null, + preferredInferenceApi: null, + webSearchConfig: null, + hermesToolGateways: [], + sandboxGpuConfig, + deps: { + isLinuxDockerDriverGatewayEnabled: vi.fn(() => false), + enforceDockerGpuPatchPreserveNetwork: vi.fn(async () => false), + patchStagedDockerfile, + now: () => 1, + }, + }); + + expect(patchStagedDockerfile.mock.calls[0]?.[11]).toEqual({ + buildIdPolicy: "rewrite", + }); }); it("warns when the base image cannot be resolved but cached latest exists", async () => { diff --git a/src/lib/onboard/sandbox-dockerfile-patch-flow.ts b/src/lib/onboard/sandbox-dockerfile-patch-flow.ts index 70fa4ffb2a2..2b1050c44d7 100644 --- a/src/lib/onboard/sandbox-dockerfile-patch-flow.ts +++ b/src/lib/onboard/sandbox-dockerfile-patch-flow.ts @@ -12,6 +12,8 @@ type EnforceDockerGpuPatchPreserveNetwork = typeof import("./docker-gpu-local-inference").enforceDockerGpuPatchPreserveNetwork; type PatchStagedDockerfile = typeof import("./dockerfile-patch").patchStagedDockerfile; +const STABLE_MANAGED_BUILD_ID_AGENTS = new Set(["openclaw", "hermes"]); + export type SandboxDockerfilePatchDeps = { pullAndResolveBaseImageDigest?: PullAndResolveBaseImageDigest; dockerImageInspect?: (target: string, opts?: Record) => DockerRunResult; @@ -137,6 +139,14 @@ export async function prepareSandboxDockerfilePatch({ }, ); const darwinVmCompat = false; + // Preserve the compatibility ARG only for managed Dockerfiles that are + // checked in here and known not to consume it. Custom --from Dockerfiles + // and other managed agents retain the historical per-run rewrite. + const managedAgentName = agent?.name ?? "openclaw"; + const buildIdPolicy = + !fromDockerfile && STABLE_MANAGED_BUILD_ID_AGENTS.has(managedAgentName) + ? "preserve" + : "rewrite"; (deps.patchStagedDockerfile ?? patchStagedDockerfile)( stagedDockerfile, model, @@ -149,6 +159,7 @@ export async function prepareSandboxDockerfilePatch({ darwinVmCompat, null, hermesToolGateways, + { buildIdPolicy }, ); return { buildId, resolvedBaseImage: resolved }; diff --git a/test/fixtures/warm-build-cache-evidence.md b/test/fixtures/warm-build-cache-evidence.md index b3b657628f1..bf56e0f6582 100644 --- a/test/fixtures/warm-build-cache-evidence.md +++ b/test/fixtures/warm-build-cache-evidence.md @@ -58,10 +58,63 @@ For each agent: | OpenClaw | `20.9s` | `0.1s` | Stable Dockerfile/build context reuses build-time config, plugin install, proxy, OTEL, permission, and hash layers. | | Hermes | `21.5s` | `0.4s` | Stable Dockerfile/build context reuses runtime setup, config generation, agent-install, permission, and config-hash layers. | +## Representative BuildKit Trace + +The timing table above came from the onboard measurement. The following +independent local control was captured with Docker 29.2.1 and Buildx 0.31.1 +against the checked-in OpenClaw and Hermes Dockerfiles. A first build with +`NEMOCLAW_BUILD_ID=evidence-pre-a` primed every other input. Changing only that +argument to `evidence-pre-b` reproduced the old per-run rewrite and rebuilt all +downstream `RUN` layers. Repeating `evidence-pre-b` represented the managed +stable-ID path and reused those same layers. + +The largest avoidable OpenClaw misses were the plugin installation and legacy +layout/permission normalization: + +```text +#49 [stage-2 35/45] RUN openclaw plugins install /opt/nemoclaw ... +#49 DONE 3.7s +#52 [stage-2 38/45] RUN set -eu; config_dir=/sandbox/.openclaw; ... +#52 DONE 10.9s +``` + +On the stable-ID rerun, BuildKit reported the identical instruction numbers as +cache hits: + +```text +#57 [stage-2 35/45] RUN openclaw plugins install /opt/nemoclaw ... +#57 CACHED +#16 [stage-2 38/45] RUN set -eu; config_dir=/sandbox/.openclaw; ... +#16 CACHED +``` + +For Hermes, the top misses were doctor/config generation and legacy layout +normalization: + +```text +#33 [29/36] RUN HERMES_HOME=/sandbox/.hermes /usr/local/bin/hermes doctor --fix ... +#33 DONE 9.0s +#37 [33/36] RUN set -eu; config_dir=/sandbox/.hermes; ... +#37 DONE 4.8s +``` + +The stable-ID rerun reused both layers: + +```text +#6 [29/36] RUN HERMES_HOME=/sandbox/.hermes /usr/local/bin/hermes doctor --fix ... +#6 CACHED +#21 [33/36] RUN set -eu; config_dir=/sandbox/.hermes; ... +#21 CACHED +``` + +These traces identify the concrete avoidable misses behind the aggregate +timings. The step numbers before the slash are Dockerfile instruction numbers; +the leading BuildKit job numbers vary between runs. + Warm builds showed the derived-image Docker steps completing at `0.0s` or `0.1s`. `ARG NEMOCLAW_BUILD_ID=default` remained stable in stock staged -Dockerfiles; custom Dockerfiles that reference `NEMOCLAW_BUILD_ID` still receive -the supplied build ID. +Dockerfiles; custom `--from` Dockerfiles retain the historical unconditional, +sanitized per-run build-ID rewrite, including indirect consumers. A separate BuildKit control probe confirmed that changing an in-scope `ARG` invalidates a following `RUN` layer even when that instruction does not mention