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-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.ts b/src/lib/onboard/dockerfile-patch.ts index 7258a4bf1f4..23dd8099e80 100644 --- a/src/lib/onboard/dockerfile-patch.ts +++ b/src/lib/onboard/dockerfile-patch.ts @@ -77,6 +77,12 @@ function encodeSanitizedDockerJsonArg(value: unknown): string { return sanitizeDockerArg(encodeDockerJsonArg(value)); } +export type DockerfileBuildIdPolicy = "preserve" | "rewrite"; + +export interface PatchStagedDockerfileOptions { + buildIdPolicy?: DockerfileBuildIdPolicy; +} + export function isValidProxyHost(value: string): boolean { return PROXY_HOST_RE.test(value); } @@ -99,6 +105,7 @@ export function patchStagedDockerfile( darwinVmCompat = false, inferenceBaseUrlOverride: string | null = null, hermesToolGateways: string[] = [], + options: PatchStagedDockerfileOptions = {}, ): void { const sanitizedModel = sanitizeDockerArg(model); const sandboxInference = getSandboxInferenceConfig( @@ -171,10 +178,15 @@ 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)}`, - ); + // 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)}`, + ); + } dockerfile = dockerfile.replace( /^ARG NEMOCLAW_DARWIN_VM_COMPAT=.*$/m, `ARG NEMOCLAW_DARWIN_VM_COMPAT=${sanitizeDockerArg(darwinVmCompat ? "1" : "0")}`, 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 new file mode 100644 index 00000000000..bf56e0f6582 --- /dev/null +++ b/test/fixtures/warm-build-cache-evidence.md @@ -0,0 +1,127 @@ + + + +# 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. | + +## 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 `--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 +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.