perf(docker): preserve warm sandbox build cache

[amata-human]

Summary

Move runtime defaults after build-time layers so identical sandbox rebuilds can reuse Docker cache. Keep stock build IDs stable unless custom Dockerfiles consume the build ID.

Related Issue

Fixes #4682

Changes

• Move OpenClaw and Hermes runtime ENV defaults after build-time config, messaging, plugin install, and permission layers.
• Keep NEMOCLAW_BUILD_ID stable for stock Dockerfiles, while preserving cache-bust compatibility for custom Dockerfiles that consume it.
• Update Dockerfile patch and security regression tests for the new cache-safe build contract.
• Verified clean live build measurements: OpenClaw cold 20.9s vs warm 0.1s; Hermes cold 21.5s vs warm 0.4s.

Type of Change

• 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

• Tests added or updated for changed behavior
• Existing tests cover changed behavior — justification:
• Tests not applicable — justification:
• Docs updated for user-facing behavior changes
• Docs not applicable — justification: internal Docker build-cache behavior; runtime/operator contract is unchanged.
• Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
• Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: affected paths were reviewed against Dockerfile ARG/ENV consumption, gateway auth/token handling, and credential boundary tests; no credential behavior changed.
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

• PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
• Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
• Targeted tests pass for changed behavior
• Full npm test passes (broad runtime changes only)
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: Angel Mata amata@nvidia.com

Summary by CodeRabbit

• Bug Fixes
  • Improved Docker image build behavior so build-time values are only carried into runtime when needed, reducing unintended changes across layers.
  • Fixed handling of build identifiers so they stay stable unless the image actually uses them.
  • Strengthened container startup behavior for auth-related settings to keep image builds more cache-safe and predictable.

[copy-pr-bot]

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]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4b830157-acab-492c-81b4-f8130a8d7011

📥 Commits

Reviewing files that changed from the base of the PR and between 59c8680 and 117800b.

📒 Files selected for processing (5)

• Dockerfile
• agents/hermes/Dockerfile
• src/lib/onboard/dockerfile-patch.test.ts
• src/lib/onboard/dockerfile-patch.ts
• test/security-c2-dockerfile-injection.test.ts

---

📝 Walkthrough

Walkthrough

The ARG→ENV promotion blocks in both Dockerfile and agents/hermes/Dockerfile are relocated to later positions so build-time config/install layers consume ARG values before runtime ENV contracts are set. patchStagedDockerfile gains a dockerfileConsumesBuildId helper that makes NEMOCLAW_BUILD_ID replacement conditional on whether the Dockerfile actually references it outside of its declaration. Tests and security assertions are updated to match.

Changes

Dockerfile ENV deferral and conditional BUILD_ID patching

Layer / File(s)	Summary
Dockerfile ENV block deferral Dockerfile, agents/hermes/Dockerfile	Removes early ARG→ENV promotion blocks and reintroduces them later in both Dockerfiles, after build-time config/install/patch layers; updates comments to state NEMOCLAW_BUILD_ID is not consumed by NemoClaw-managed images and that gateway tokens are generated at startup.
Conditional dockerfileConsumesBuildId helper and patch logic src/lib/onboard/dockerfile-patch.ts	Adds dockerfileConsumesBuildId internal predicate that scans non-comment Dockerfile lines for NEMOCLAW_BUILD_ID references outside the ARG declaration; wraps the ARG NEMOCLAW_BUILD_ID rewrite in patchStagedDockerfile in this conditional.
Updated tests and security ordering assertions src/lib/onboard/dockerfile-patch.test.ts, test/security-c2-dockerfile-injection.test.ts	Updates existing assertions so NEMOCLAW_BUILD_ID stays unchanged when not consumed; adds new test covering both the stable and consumed cases; replaces ENV-promotion-centric security assertion with an ARG < generator RUN < runtime interpolation ordering check.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• NVIDIA/NemoClaw#3300: Modifies patchStagedDockerfile and the surrounding helper/test suite in src/lib/onboard/dockerfile-patch.ts, directly overlapping with this PR's conditional NEMOCLAW_BUILD_ID patching logic.
• NVIDIA/NemoClaw#3512: Adds a variable to the same agents/hermes/Dockerfile ARG→ENV promotion block that this PR relocates.

Suggested labels

area: performance, platform: container

Suggested reviewers

• ericksoa
• jyaunches
• prekshivyas
• cjagwani

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: Docker caching improvements for warm sandbox builds.
Linked Issues check	✅ Passed	The Dockerfile changes and regression tests address warm-build cache misses, preserve runtime token generation, and keep NEMOCLAW_BUILD_ID behavior aligned with the issue.
Out of Scope Changes check	✅ Passed	The changes stay focused on Docker cache behavior and related tests, with no evident unrelated code paths.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 4682-reduce-dockerfile-cache-misses-in-warm-sandbox-builds

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the 4682-reduce-dockerfi... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.

File	main	4682-reduce-dockerfi... 3813836	+/-
nemoclaw/src/se...cret-scanner.ts	—	100%	—
nemoclaw/src/commands/slash.ts	—	100%	—
nemoclaw/src/li...bprocess-env.ts	—	100%	—
nemoclaw/src/bl...eprint/state.ts	—	98%	—
nemoclaw/src/onboard/config.ts	—	98%	—
nemoclaw/src/bl...int/snapshot.ts	—	97%	—
nemoclaw/src/bl...print/runner.ts	—	95%	—
nemoclaw/src/co...ration-state.ts	—	94%	—
nemoclaw/src/bl...ate-networks.ts	—	94%	—
nemoclaw/src/index.ts	—	94%	—

TypeScript / code-coverage/cli

The overall coverage in the 4682-reduce-dockerfi... branch is 68%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.

File	main	4682-reduce-dockerfi... 3813836	+/-
src/lib/actions...all/run-plan.ts	—	80%	—
src/lib/state/o...oard-session.ts	—	79%	—
src/lib/actions...dbox/rebuild.ts	—	74%	—
src/lib/state/sandbox.ts	—	72%	—
src/lib/shields/index.ts	—	70%	—
src/lib/onboard/preflight.ts	—	69%	—
src/lib/actions...licy-channel.ts	—	60%	—
src/lib/onboard...er-gpu-patch.ts	—	59%	—
src/lib/policy/index.ts	—	52%	—
src/lib/onboard.ts	—	20%	—

---

_{Updated June 30, 2026 03:53 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.}

[github-actions]

E2E Advisor Recommendation

Required E2E: full-e2e-vitest, hermes-e2e-vitest, cloud-onboard-vitest
Optional E2E: rebuild-openclaw-vitest, rebuild-hermes-vitest, runtime-overrides-vitest

Dispatch hint: full-e2e-vitest,hermes-e2e-vitest,cloud-onboard-vitest

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• full-e2e-vitest (high): Required because the default OpenClaw Dockerfile and onboard Dockerfile patching changed. This validates install from source, non-interactive onboard, real sandbox image build, OpenClaw startup, generated config, and live inference/user flow.
• hermes-e2e-vitest (high): Required because agents/hermes/Dockerfile changed. This validates the real Hermes onboard/build/start path and catches missing build ARG visibility or broken persisted runtime ENV defaults after the late ENV move.
• cloud-onboard-vitest (high): Required because the changes can affect full hosted onboarding and deployment-time image generation: staged Dockerfile patching, provider/model build args, OpenClaw config generation, and Docker build/runtime environment contract.

Optional E2E

• rebuild-openclaw-vitest (high): Useful adjacent coverage for OpenClaw sandbox lifecycle/rebuild after Dockerfile and cache-boundary changes. It exercises real image rebuild, state preservation, config hash/token behavior, and rebuilt sandbox readiness.
• rebuild-hermes-vitest (high): Useful adjacent coverage for Hermes rebuild after Dockerfile ARG/ENV ordering changes, including real Docker/OpenShell rebuild and Hermes config/runtime state preservation.
• runtime-overrides-vitest (medium): Useful focused OpenClaw image-level check that the built image still has correct runtime defaults and entrypoint config override behavior after moving ENV persistence later in the Dockerfile.

New E2E recommendations

• warm Docker build cache for onboard images (medium): The PR's primary cache behavior is currently documented by a manual fixture, but there is no existing automated E2E that runs two same-input onboard builds for OpenClaw and Hermes and asserts the second build reuses layers while custom Dockerfiles that consume NEMOCLAW_BUILD_ID still bust cache.
  • Suggested test: Add a warm-build-cache E2E scenario that performs cold and warm non-interactive onboard builds for OpenClaw and Hermes, records Docker build timing/cache-hit evidence, and verifies stock staged Dockerfiles keep ARG NEMOCLAW_BUILD_ID stable.

Dispatch hint

• Workflow: .github/workflows/e2e-vitest-scenarios.yaml
• jobs input: full-e2e-vitest,hermes-e2e-vitest,cloud-onboard-vitest

[github-actions]

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: runtime-overrides-vitest, cloud-onboard-vitest, hermes-root-entrypoint-smoke-vitest
Optional Vitest E2E scenarios: full-e2e-vitest

Dispatch required Vitest E2E scenarios:

• gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=runtime-overrides-vitest
• gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=cloud-onboard-vitest
• gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=hermes-root-entrypoint-smoke-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

• runtime-overrides-vitest: The root Dockerfile changes ARG/ENV ordering and NEMOCLAW_BUILD_ID cache behavior. This free-standing live job builds the real OpenClaw sandbox image and validates generated runtime config and config-hash behavior at the Docker image/entrypoint boundary.
  • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=runtime-overrides-vitest
• cloud-onboard-vitest: src/lib/onboard/dockerfile-patch.ts changes how staged Dockerfiles are patched during onboarding, especially NEMOCLAW_BUILD_ID handling. This live job exercises a real cloud OpenClaw onboard path that stages, patches, builds, and boots the sandbox image.
  • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=cloud-onboard-vitest
• hermes-root-entrypoint-smoke-vitest: agents/hermes/Dockerfile changes Hermes build-time ARG consumption and persisted runtime ENV placement. This free-standing live job builds the real Hermes image and verifies root entrypoint startup, health, privilege separation, and runtime layout.
  • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=hermes-root-entrypoint-smoke-vitest

Optional Vitest E2E scenarios

• full-e2e-vitest: Broader adjacent OpenClaw coverage for the full install, onboard, inference, CLI operations, and cleanup path after the Dockerfile and staged-Dockerfile patching changes.
  • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=full-e2e-vitest

Relevant changed files

• Dockerfile
• agents/hermes/Dockerfile
• src/lib/onboard/dockerfile-patch.ts

[github-actions]

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-2: Update #2001 or #3776 with timing evidence; then add or justify PRA-T1.
Open items: 1 required · 2 warnings · 0 suggestions · 6 test follow-ups
Since last review: 3 prior items resolved · 0 still apply · 2 new items found

Action checklist

• PRA-2 Fix: Update perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776 with timing evidence
• PRA-1 Resolve or justify: Source-of-truth review needed: Hermes config-generation ARG/ENV contract
• PRA-3 Resolve or justify: Add Hermes structural guard for the late runtime ENV contract in agents/hermes/Dockerfile:437
• PRA-T1 Add or justify test follow-up: Runtime validation
• PRA-T2 Add or justify test follow-up: Runtime validation
• PRA-T3 Add or justify test follow-up: Runtime validation
• PRA-T4 Add or justify test follow-up: Runtime validation
• PRA-T5 Add or justify test follow-up: Acceptance clause
• PRA-T6 Add or justify test follow-up: Hermes config-generation ARG/ENV contract

Findings index

ID	Severity	Category	Location	Required action
PRA-1	Resolve/justify	architecture	—	Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2	Required	acceptance	—	Add the timing evidence to #2001 or #3776, or add a checked-in repository note that clearly records where that required issue update was made and references the cold/warm OpenClaw and Hermes timings from this PR.
PRA-3	Resolve/justify	security	agents/hermes/Dockerfile:437	Add a small structural test, either near `test/security-c2-dockerfile-injection.test.ts` or a Hermes Dockerfile test, that parses `agents/hermes/Dockerfile` and asserts the relevant `ARG NEMOCLAW_MODEL`, `ARG NEMOCLAW_INFERENCE_BASE_URL`, messaging/tool-gateway ARGs appear before `generate-config.ts`, and the final `ENV NEMOCLAW_MODEL=${NEMOCLAW_MODEL}` block appears after the Hermes config-hash layers.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-2 Required — Update #2001 or #3776 with timing evidence

• Location: not file-specific
• Category: acceptance
• Problem: Linked issue perf(onboard): reduce Dockerfile cache misses in warm sandbox builds #4682 includes the acceptance criterion “perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776 is updated with timing evidence if the change materially improves warm onboard/rebuild time.” The changed fixture records material cold-vs-warm improvements for both OpenClaw and Hermes, but the deterministic linked-issue context shows no comments on perf(onboard): reduce Dockerfile cache misses in warm sandbox builds #4682 and the diff does not add a stable repository reference showing that perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776 was updated.
• Impact: Without an auditable parent/performance-budget update, the performance evidence can remain isolated to this PR/fixture and the linked acceptance criterion is not fully satisfied for future readers of the broader warm-build investigation.
• Required action: Add the timing evidence to perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776, or add a checked-in repository note that clearly records where that required issue update was made and references the cold/warm OpenClaw and Hermes timings from this PR.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: Read the linked performance issue perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776, or a repository-tracked reference in this PR, and confirm it contains the OpenClaw `20.9s` to `0.1s` and Hermes `21.5s` to `0.4s` timing evidence or equivalent build-cache timing data.
• Missing regression test: This is an acceptance-traceability requirement rather than executable behavior; the existing `test/fixtures/warm-build-cache-evidence.md` is the relevant checked-in evidence, and the missing coverage is an auditable parent/performance issue update or repository reference to that update.
• Done when: The required change is committed and verification passes: Read the linked performance issue perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776, or a repository-tracked reference in this PR, and confirm it contains the OpenClaw `20.9s` to `0.1s` and Hermes `21.5s` to `0.4s` timing evidence or equivalent build-cache timing data.
• Evidence: Issue perf(onboard): reduce Dockerfile cache misses in warm sandbox builds #4682 acceptance criterion: “perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776 is updated with timing evidence if the change materially improves warm onboard/rebuild time.” `test/fixtures/warm-build-cache-evidence.md` records material improvements, while the linked issue context contains no comments and no changed file records a perf: investigate and reduce networking latency during onboard and validation #2001/perf(onboard): define onboard performance budget and CI regression signal #3776 update.

Review findings by urgency: 1 required fix, 2 items to resolve/justify, 0 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: Hermes config-generation ARG/ENV contract

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Missing: add a Hermes structural test that checks relevant ARGs precede `generate-config.ts` and the final runtime `ENV` block follows the Hermes config-hash layers.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: `agents/hermes/Dockerfile` now has ARGs before `generate-config.ts` and a final `ENV` block at the end, but the changed tests only include an OpenClaw ordering assertion.

PRA-3 Resolve/justify — Add Hermes structural guard for the late runtime ENV contract

• Location: agents/hermes/Dockerfile:437
• Category: security
• Problem: OpenClaw now has a structural test that `ARG NEMOCLAW_DISABLE_DEVICE_AUTH` is available before config generation and that the final runtime `ENV` block appears after the generator. Hermes has the same security-sensitive ARG-to-config flow and the same late runtime `ENV` relocation, but no equivalent test currently guards `agents/hermes/Dockerfile` ordering around `generate-config.ts`, config hashes, and the final runtime `ENV` block.
• Impact: A future edit could move Hermes runtime metadata back before config generation or hashing, reintroducing avoidable cache busting or changing the build-time/runtime configuration boundary without a focused test failure. Because Hermes `.env` and config hashing participate in the secret-boundary and startup-integrity paths, this should be guarded the same way as OpenClaw.
• Recommended action: Add a small structural test, either near `test/security-c2-dockerfile-injection.test.ts` or a Hermes Dockerfile test, that parses `agents/hermes/Dockerfile` and asserts the relevant `ARG NEMOCLAW_MODEL`, `ARG NEMOCLAW_INFERENCE_BASE_URL`, messaging/tool-gateway ARGs appear before `generate-config.ts`, and the final `ENV NEMOCLAW_MODEL=${NEMOCLAW_MODEL}` block appears after the Hermes config-hash layers.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Search the changed tests for a Hermes-specific ordering assertion mentioning `agents/hermes/Dockerfile`, `generate-config.ts`, and `ENV NEMOCLAW_MODEL=${NEMOCLAW_MODEL}`; currently nearby Hermes tests cover runtime plans, secret boundaries, and hashes but not this changed ARG/ENV ordering.
• Missing regression test: Add a test named like `Hermes Dockerfile exposes ARGs to generate-config before persisting runtime ENV defaults` that checks ARG line numbers are before the `generate-config.ts` RUN and the final runtime ENV line is after both `/etc/nemoclaw/hermes.config-hash` and `/sandbox/.hermes/.config-hash` creation.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Search the changed tests for a Hermes-specific ordering assertion mentioning `agents/hermes/Dockerfile`, `generate-config.ts`, and `ENV NEMOCLAW_MODEL=${NEMOCLAW_MODEL}`; currently nearby Hermes tests cover runtime plans, secret boundaries, and hashes but not this changed ARG/ENV ordering.
• Evidence: `agents/hermes/Dockerfile` defines build ARGs around line 158, runs `node --experimental-strip-types /opt/nemoclaw-hermes-config/generate-config.ts` around line 206, writes config hashes before line 437, and now persists runtime `ENV NEMOCLAW_MODEL=${NEMOCLAW_MODEL}` at line 437. `test/security-c2-dockerfile-injection.test.ts` only guards the root OpenClaw Dockerfile ordering.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

• None.

Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Runtime validation — Repeat an OpenClaw onboard/rebuild with identical effective config and verify stock `ARG NEMOCLAW_BUILD_ID=default`, `openclaw.json` has no baked gateway token, and plugin/install layers are reused.. The changed behavior affects Docker layer-cache behavior and sandbox image build/runtime configuration. Static tests now cover the patcher and OpenClaw ordering, and the fixture provides manual cold/warm evidence, but the high-risk Dockerfile boundary still benefits from targeted runtime validation and a missing Hermes structural guard.
• PRA-T2 Runtime validation — Repeat a Hermes onboard/rebuild with identical model, messaging, and tool-gateway config and verify runtime setup, config generation, agent-install, permission, and config-hash layers are reused after the runtime ENV relocation.. The changed behavior affects Docker layer-cache behavior and sandbox image build/runtime configuration. Static tests now cover the patcher and OpenClaw ordering, and the fixture provides manual cold/warm evidence, but the high-risk Dockerfile boundary still benefits from targeted runtime validation and a missing Hermes structural guard.
• PRA-T3 Runtime validation — Patch or build a custom Dockerfile that consumes `NEMOCLAW_BUILD_ID` in a non-declaration line and verify the supplied build ID is written while CR/LF payloads remain inert.. The changed behavior affects Docker layer-cache behavior and sandbox image build/runtime configuration. Static tests now cover the patcher and OpenClaw ordering, and the fixture provides manual cold/warm evidence, but the high-risk Dockerfile boundary still benefits from targeted runtime validation and a missing Hermes structural guard.
• PRA-T4 Runtime validation — Add a Hermes structural test that asserts build ARGs precede `generate-config.ts` and the final runtime `ENV NEMOCLAW_*` block follows the Hermes config-hash layers.. The changed behavior affects Docker layer-cache behavior and sandbox image build/runtime configuration. Static tests now cover the patcher and OpenClaw ordering, and the fixture provides manual cold/warm evidence, but the high-risk Dockerfile boundary still benefits from targeted runtime validation and a missing Hermes structural guard.
• PRA-T5 Acceptance clause — perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776 is updated with timing evidence if the change materially improves warm onboard/rebuild time. — add test evidence or identify existing coverage. The fixture records material timing improvements, but the deterministic linked issue context has no comments and no changed file provides evidence that perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776 was updated.
• PRA-T6 Hermes config-generation ARG/ENV contract — Missing: add a Hermes structural test that checks relevant ARGs precede `generate-config.ts` and the final runtime `ENV` block follows the Hermes config-hash layers.. `agents/hermes/Dockerfile` now has ARGs before `generate-config.ts` and a final `ENV` block at the end, but the changed tests only include an OpenClaw ordering assertion.

Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: Hermes config-generation ARG/ENV contract

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Missing: add a Hermes structural test that checks relevant ARGs precede `generate-config.ts` and the final runtime `ENV` block follows the Hermes config-hash layers.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: `agents/hermes/Dockerfile` now has ARGs before `generate-config.ts` and a final `ENV` block at the end, but the changed tests only include an OpenClaw ordering assertion.

PRA-2 Required — Update #2001 or #3776 with timing evidence

• Location: not file-specific
• Category: acceptance
• Problem: Linked issue perf(onboard): reduce Dockerfile cache misses in warm sandbox builds #4682 includes the acceptance criterion “perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776 is updated with timing evidence if the change materially improves warm onboard/rebuild time.” The changed fixture records material cold-vs-warm improvements for both OpenClaw and Hermes, but the deterministic linked-issue context shows no comments on perf(onboard): reduce Dockerfile cache misses in warm sandbox builds #4682 and the diff does not add a stable repository reference showing that perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776 was updated.
• Impact: Without an auditable parent/performance-budget update, the performance evidence can remain isolated to this PR/fixture and the linked acceptance criterion is not fully satisfied for future readers of the broader warm-build investigation.
• Required action: Add the timing evidence to perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776, or add a checked-in repository note that clearly records where that required issue update was made and references the cold/warm OpenClaw and Hermes timings from this PR.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: Read the linked performance issue perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776, or a repository-tracked reference in this PR, and confirm it contains the OpenClaw `20.9s` to `0.1s` and Hermes `21.5s` to `0.4s` timing evidence or equivalent build-cache timing data.
• Missing regression test: This is an acceptance-traceability requirement rather than executable behavior; the existing `test/fixtures/warm-build-cache-evidence.md` is the relevant checked-in evidence, and the missing coverage is an auditable parent/performance issue update or repository reference to that update.
• Done when: The required change is committed and verification passes: Read the linked performance issue perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776, or a repository-tracked reference in this PR, and confirm it contains the OpenClaw `20.9s` to `0.1s` and Hermes `21.5s` to `0.4s` timing evidence or equivalent build-cache timing data.
• Evidence: Issue perf(onboard): reduce Dockerfile cache misses in warm sandbox builds #4682 acceptance criterion: “perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776 is updated with timing evidence if the change materially improves warm onboard/rebuild time.” `test/fixtures/warm-build-cache-evidence.md` records material improvements, while the linked issue context contains no comments and no changed file records a perf: investigate and reduce networking latency during onboard and validation #2001/perf(onboard): define onboard performance budget and CI regression signal #3776 update.

PRA-3 Resolve/justify — Add Hermes structural guard for the late runtime ENV contract

• Location: agents/hermes/Dockerfile:437
• Category: security
• Problem: OpenClaw now has a structural test that `ARG NEMOCLAW_DISABLE_DEVICE_AUTH` is available before config generation and that the final runtime `ENV` block appears after the generator. Hermes has the same security-sensitive ARG-to-config flow and the same late runtime `ENV` relocation, but no equivalent test currently guards `agents/hermes/Dockerfile` ordering around `generate-config.ts`, config hashes, and the final runtime `ENV` block.
• Impact: A future edit could move Hermes runtime metadata back before config generation or hashing, reintroducing avoidable cache busting or changing the build-time/runtime configuration boundary without a focused test failure. Because Hermes `.env` and config hashing participate in the secret-boundary and startup-integrity paths, this should be guarded the same way as OpenClaw.
• Recommended action: Add a small structural test, either near `test/security-c2-dockerfile-injection.test.ts` or a Hermes Dockerfile test, that parses `agents/hermes/Dockerfile` and asserts the relevant `ARG NEMOCLAW_MODEL`, `ARG NEMOCLAW_INFERENCE_BASE_URL`, messaging/tool-gateway ARGs appear before `generate-config.ts`, and the final `ENV NEMOCLAW_MODEL=${NEMOCLAW_MODEL}` block appears after the Hermes config-hash layers.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Search the changed tests for a Hermes-specific ordering assertion mentioning `agents/hermes/Dockerfile`, `generate-config.ts`, and `ENV NEMOCLAW_MODEL=${NEMOCLAW_MODEL}`; currently nearby Hermes tests cover runtime plans, secret boundaries, and hashes but not this changed ARG/ENV ordering.
• Missing regression test: Add a test named like `Hermes Dockerfile exposes ARGs to generate-config before persisting runtime ENV defaults` that checks ARG line numbers are before the `generate-config.ts` RUN and the final runtime ENV line is after both `/etc/nemoclaw/hermes.config-hash` and `/sandbox/.hermes/.config-hash` creation.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Search the changed tests for a Hermes-specific ordering assertion mentioning `agents/hermes/Dockerfile`, `generate-config.ts`, and `ENV NEMOCLAW_MODEL=${NEMOCLAW_MODEL}`; currently nearby Hermes tests cover runtime plans, secret boundaries, and hashes but not this changed ARG/ENV ordering.
• Evidence: `agents/hermes/Dockerfile` defines build ARGs around line 158, runs `node --experimental-strip-types /opt/nemoclaw-hermes-config/generate-config.ts` around line 206, writes config hashes before line 437, and now persists runtime `ENV NEMOCLAW_MODEL=${NEMOCLAW_MODEL}` at line 437. `test/security-c2-dockerfile-injection.test.ts` only guards the root OpenClaw Dockerfile ordering.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

[github-actions]

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-1: Missing automated warm-build cache regression test; then add or justify PRA-T1.
Open items: 1 required · 3 warnings · 3 suggestions · 6 test follow-ups
Since last review: 2 prior items resolved · 6 still apply · 3 new items found

Action checklist

• PRA-1 Fix: Missing automated warm-build cache regression test in test/fixtures/warm-build-cache-evidence.md:1
• PRA-2 Resolve or justify: Parent issues (perf: investigate and reduce networking latency during onboard and validation #2001, perf(onboard): define onboard performance budget and CI regression signal #3776) not updated with timing evidence in test/fixtures/warm-build-cache-evidence.md:50
• PRA-3 Resolve or justify: dockerfileConsumesBuildId regex may miss indirect references in custom Dockerfiles in src/lib/onboard/dockerfile-patch.ts:55
• PRA-4 Resolve or justify: dockerfileConsumesBuildId test coverage missing multi-line RUN and multiple refs on one line in src/lib/onboard/dockerfile-patch.test.ts:316
• PRA-T1 Add or justify test follow-up: Runtime validation
• PRA-T2 Add or justify test follow-up: Runtime validation
• PRA-T3 Add or justify test follow-up: Runtime validation
• PRA-T4 Add or justify test follow-up: Security test updated but could add Hermes equivalent
• PRA-T5 Add or justify test follow-up: Acceptance clause
• PRA-T6 Add or justify test follow-up: Acceptance clause
• PRA-5 In-scope improvement: Test file monolith growth — extract dockerfileConsumesBuildId tests in src/lib/onboard/dockerfile-patch.test.ts:1
• PRA-6 In-scope improvement: Security test updated but could add Hermes equivalent in test/security-c2-dockerfile-injection.test.ts:85
• PRA-7 In-scope improvement: C-2 Dockerfile injection fix preserved with cache-safe boundary in Dockerfile:942

Findings index

ID	Severity	Category	Location	Required action
PRA-1	Required	acceptance	test/fixtures/warm-build-cache-evidence.md:1	Add automated warm-build cache regression test in test/security-c2-dockerfile-injection.test.ts or new test file. Can be a Vitest integration test using docker build --no-cache vs docker build (cached) with timing threshold or BuildKit cache metadata. Defer only with explicit rationale in PR.
PRA-2	Resolve/justify	acceptance	test/fixtures/warm-build-cache-evidence.md:50	Confirm parent issues updated with timing evidence or add follow-up task to update them before merge.
PRA-3	Resolve/justify	security	src/lib/onboard/dockerfile-patch.ts:55	Add unit test for indirect reference case (should return false — current behavior is correct). Document that custom Dockerfiles consuming NEMOCLAW_BUILD_ID indirectly must declare explicit consumption.
PRA-4	Resolve/justify	correctness	src/lib/onboard/dockerfile-patch.test.ts:316	Add subtests for multi-line RUN continuation and multiple references on one line to strengthen confidence.
PRA-5	Improvement	architecture	src/lib/onboard/dockerfile-patch.test.ts:1	Extract the new dockerfileConsumesBuildId test suite to a dedicated file (e.g., dockerfile-patch-consumes-build-id.test.ts).
PRA-6	Improvement	tests	test/security-c2-dockerfile-injection.test.ts:85	Add mirror security test for agents/hermes/Dockerfile verifying same cache-safe boundary: ARG declarations before config generation RUN, ENV promotion after all build-time layers.
PRA-7	Improvement	security	Dockerfile:942	No action needed; pattern is secure.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-1 Required — Missing automated warm-build cache regression test

• Location: test/fixtures/warm-build-cache-evidence.md:1
• Category: acceptance
• Problem: PR documents manual warm-build cache evidence (OpenClaw 20.9s→0.1s, Hermes 21.5s→0.4s) but no automated regression test exists. Prior review PRA-1/T6/T7 requested automated Vitest integration test that builds twice with identical args and asserts layer reuse. Acceptance clause 'Tests/CI compare cold vs warm cache reuse' unmet.
• Impact: Without automated validation, a future change could inadvertently reintroduce cache misses that degrade warm onboard/rebuild performance from ~0.1s back to ~20s.
• Required action: Add automated warm-build cache regression test in test/security-c2-dockerfile-injection.test.ts or new test file. Can be a Vitest integration test using docker build --no-cache vs docker build (cached) with timing threshold or BuildKit cache metadata. Defer only with explicit rationale in PR.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: Check if test/security-c2-dockerfile-injection.test.ts or new test contains warm-cache regression test running docker build twice with identical args and validating layer cache hits.
• Missing regression test: Automated warm-build cache reuse test: build image twice with same args, verify second build completes in <5s (indicating cache reuse) and BuildKit reports CACHED for all layers except final ENV.
• Done when: The required change is committed and verification passes: Check if test/security-c2-dockerfile-injection.test.ts or new test contains warm-cache regression test running docker build twice with identical args and validating layer cache hits.
• Evidence: PR body: 'Verified clean live build measurements: OpenClaw cold 20.9s vs warm 0.1s; Hermes cold 21.5s vs warm 0.4s.' No automated test present in diff. Issue perf(onboard): reduce Dockerfile cache misses in warm sandbox builds #4682 acceptance criteria: 'Tests or CI/manual benchmark guidance compare cold vs warm builds and show the expected cache reuse.'

Review findings by urgency: 1 required fix, 3 items to resolve/justify, 3 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-2 Resolve/justify — Parent issues (#2001, #3776) not updated with timing evidence

• Location: test/fixtures/warm-build-cache-evidence.md:50
• Category: acceptance
• Problem: Acceptance clause 'Parent issues (perf: investigate and reduce networking latency during onboard and validation #2001, perf(onboard): define onboard performance budget and CI regression signal #3776) updated with timing' — not visible in PR diff or linked issue comments.
• Impact: Performance tracking issue loses visibility; future reviewers cannot verify improvement was communicated upstream.
• Recommended action: Confirm parent issues updated with timing evidence or add follow-up task to update them before merge.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check GitHub issues perf: investigate and reduce networking latency during onboard and validation #2001 and perf(onboard): define onboard performance budget and CI regression signal #3776 for timing updates from this PR.
• Missing regression test: N/A — process item.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check GitHub issues perf: investigate and reduce networking latency during onboard and validation #2001 and perf(onboard): define onboard performance budget and CI regression signal #3776 for timing updates from this PR.
• Evidence: Issue perf(onboard): reduce Dockerfile cache misses in warm sandbox builds #4682 acceptance criteria: 'perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776 is updated with timing evidence if the change materially improves warm onboard/rebuild time.' PR body and diff show no evidence of updates.

PRA-3 Resolve/justify — dockerfileConsumesBuildId regex may miss indirect references in custom Dockerfiles

• Location: src/lib/onboard/dockerfile-patch.ts:55
• Category: security
• Problem: dockerfileConsumesBuildId uses word-boundary regex (\bNEMOCLAW_BUILD_ID\b) which correctly detects ENV/RUN/ARG/LABEL direct references in tests, but may miss edge cases in custom Dockerfiles: (1) indirect references via intermediate variables (ARG X=${NEMOCLAW_BUILD_ID}; ENV Y=${X}), (2) concatenated references without word boundaries in unusual contexts. Risk is low since function operates on NemoClaw-generated staged Dockerfiles (trusted structure), but custom Dockerfiles could bypass cache busting.
• Impact: If a custom Dockerfile consumes NEMOCLAW_BUILD_ID in a way the regex misses, the build ID will remain stable and the custom Dockerfile's cache-busting logic won't trigger, potentially serving stale config.
• Recommended action: Add unit test for indirect reference case (should return false — current behavior is correct). Document that custom Dockerfiles consuming NEMOCLAW_BUILD_ID indirectly must declare explicit consumption.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Run npx vitest run src/lib/onboard/dockerfile-patch.test.ts and verify new test 'leaves NEMOCLAW_BUILD_ID stable unless the Dockerfile consumes it' passes for ENV/RUN/ARG/LABEL patterns. Check function at line 55-63.
• Missing regression test: Test for indirect reference: ARG X=${NEMOCLAW_BUILD_ID}; ENV Y=${X} should return false (current behavior correct — only direct textual references bust cache).
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Run npx vitest run src/lib/onboard/dockerfile-patch.test.ts and verify new test 'leaves NEMOCLAW_BUILD_ID stable unless the Dockerfile consumes it' passes for ENV/RUN/ARG/LABEL patterns. Check function at line 55-63.
• Evidence: Function at lines 55-63: splits Dockerfile by lines, skips comments and ARG declarations, tests word-boundary regex. Tests cover 4 direct patterns but no indirect variable references.

PRA-4 Resolve/justify — dockerfileConsumesBuildId test coverage missing multi-line RUN and multiple refs on one line

• Location: src/lib/onboard/dockerfile-patch.test.ts:316
• Category: correctness
• Problem: New test 'leaves NEMOCLAW_BUILD_ID stable unless the Dockerfile consumes it' covers 4 consumption patterns (ENV, RUN, ARG, LABEL) but does not test: (1) multi-line RUN with backslash continuation, (2) multiple references on same line (e.g., ENV A=${NEMOCLAW_BUILD_ID} B=${NEMOCLAW_BUILD_ID}).
• Impact: Edge cases in real custom Dockerfiles could go undetected, leading to unexpected cache behavior.
• Recommended action: Add subtests for multi-line RUN continuation and multiple references on one line to strengthen confidence.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Run vitest and check test coverage for dockerfileConsumesBuildId. Look for test cases with RUN echo ${NEMOCLAW_BUILD_ID} \\ && echo done and ENV A=${NEMOCLAW_BUILD_ID} B=${NEMOCLAW_BUILD_ID}.
• Missing regression test: Unit tests for: (1) RUN echo ${NEMOCLAW_BUILD_ID} \\ && echo done (multi-line), (2) ENV A=${NEMOCLAW_BUILD_ID} B=${NEMOCLAW_BUILD_ID} (multiple).
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Run vitest and check test coverage for dockerfileConsumesBuildId. Look for test cases with RUN echo ${NEMOCLAW_BUILD_ID} \\ && echo done and ENV A=${NEMOCLAW_BUILD_ID} B=${NEMOCLAW_BUILD_ID}.
• Evidence: Test file lines 316-360 cover 4 patterns but no multi-line or multiple-ref cases.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-5 Improvement — Test file monolith growth — extract dockerfileConsumesBuildId tests

• Location: src/lib/onboard/dockerfile-patch.test.ts:1
• Category: architecture
• Problem: Test file grew by 63 lines (1226→1289), triggering monolith growth blocker. Growth is from new test 'leaves NEMOCLAW_BUILD_ID stable unless the Dockerfile consumes it' which adds valuable coverage.
• Impact: Large test file harder to maintain, navigate, and review. Related tests scattered.
• Suggested action: Extract the new dockerfileConsumesBuildId test suite to a dedicated file (e.g., dockerfile-patch-consumes-build-id.test.ts).
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check src/lib/onboard/ directory for test file count; verify new file exists with the extracted tests.
• Missing regression test: N/A — architecture item.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: Static analysis flagged dockerfile-patch.test.ts as large-file hotspot with +63 delta. Monolith growth blocker rationale: 'Current monolith grew by 20 or more lines; extract or offset the growth before merge.'

PRA-6 Improvement — Security test updated but could add Hermes equivalent

• Location: test/security-c2-dockerfile-injection.test.ts:85
• Category: tests
• Problem: Security test validates cache-safe config boundary for OpenClaw Dockerfile (ARG → generator RUN → runtime ENV). Hermes Dockerfile follows same pattern but has no dedicated security test mirroring this.
• Impact: Future changes to Hermes Dockerfile could inadvertently reintroduce early ENV cache-busting without test coverage.
• Suggested action: Add mirror security test for agents/hermes/Dockerfile verifying same cache-safe boundary: ARG declarations before config generation RUN, ENV promotion after all build-time layers.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Verify agents/hermes/Dockerfile has no early ENV block; ENV promotion at line 354 comes after post-agent-install RUN. Add test asserting ordering.
• Missing regression test: Security test for Hermes Dockerfile: assert ARG NEMOCLAW_DISABLE_DEVICE_AUTH line < generator RUN line < runtime ENV promotion line.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: OpenClaw security test at test/security-c2-dockerfile-injection.test.ts:85 validates hasCacheSafeConfigBoundary. Hermes Dockerfile at line 354 has ENV promotion after post-agent-install RUN at line ~320.

PRA-7 Improvement — C-2 Dockerfile injection fix preserved with cache-safe boundary

• Location: Dockerfile:942
• Category: security
• Problem: Build args promoted to ENV only after config generation RUN layer; generate-openclaw-config.mts reads from process.env (data), not source interpolation. Security test validates cache-safe boundary (ARG → generator RUN → runtime ENV).
• Impact: No credential leakage, no code injection vector. Build-time config reads safe; runtime ENV contract preserved.
• Suggested action: No action needed; pattern is secure.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check test/security-c2-dockerfile-injection.test.ts:85 for cache-safe boundary assertion; verify Dockerfile has ARG declarations before generate-openclaw-config.mts RUN and ENV block after plugin install layers.
• Missing regression test: Existing security test covers this.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: Dockerfile: ARG block at line ~550, generate-openclaw-config.mts RUN at line ~620, ENV promotion at line 942. Security test asserts argLine < generatorLine < runtimeEnvLine.

Simplification opportunities: 1 possible cut

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

• PRA-5 shrink (src/lib/onboard/dockerfile-patch.test.ts:1): dockerfileConsumesBuildId test suite (lines 316-360 approx)
  • Replacement: New file src/lib/onboard/dockerfile-patch-consumes-build-id.test.ts importing and testing the exported function
  • Net: 0 lines
  • Safety boundary: Must not weaken security test coverage; function must remain exported or test must import from source

Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Runtime validation — Integration test: docker build twice with identical args → assert layer reuse (CACHED) and <5s warm build. Runtime/sandbox/infrastructure paths need behavioral runtime validation: Dockerfile, agents/hermes/Dockerfile, scripts/generate-openclaw-config.mts, src/lib/onboard/dockerfile-patch.ts. Unit tests cover dockerfile-patch.ts logic but dockerfileConsumesBuildId edge cases and warm-build cache behavior need integration validation.
• PRA-T2 Runtime validation — dockerfileConsumesBuildId edge cases in actual Docker builds: multi-line RUN continuation, multiple refs on one line, indirect variable references. Runtime/sandbox/infrastructure paths need behavioral runtime validation: Dockerfile, agents/hermes/Dockerfile, scripts/generate-openclaw-config.mts, src/lib/onboard/dockerfile-patch.ts. Unit tests cover dockerfile-patch.ts logic but dockerfileConsumesBuildId edge cases and warm-build cache behavior need integration validation.
• PRA-T3 Runtime validation — Hermes Dockerfile cache-safe boundary security test (mirror of OpenClaw test). Runtime/sandbox/infrastructure paths need behavioral runtime validation: Dockerfile, agents/hermes/Dockerfile, scripts/generate-openclaw-config.mts, src/lib/onboard/dockerfile-patch.ts. Unit tests cover dockerfile-patch.ts logic but dockerfileConsumesBuildId edge cases and warm-build cache behavior need integration validation.
• PRA-T4 Security test updated but could add Hermes equivalent — Add mirror security test for agents/hermes/Dockerfile verifying same cache-safe boundary: ARG declarations before config generation RUN, ENV promotion after all build-time layers.
• PRA-T5 Acceptance clause — Tests/CI compare cold vs warm cache reuse — add test evidence or identify existing coverage. Manual evidence only in PR body and warm-build-cache-evidence.md; no automated test in test/ directory
• PRA-T6 Acceptance clause — Parent issues (perf: investigate and reduce networking latency during onboard and validation #2001, perf(onboard): define onboard performance budget and CI regression signal #3776) updated with timing — add test evidence or identify existing coverage. Not visible in PR diff, linked issue comments, or PR timeline

Since last review details

Current findings, using the urgency labels above:

PRA-1 Required — Missing automated warm-build cache regression test

• Location: test/fixtures/warm-build-cache-evidence.md:1
• Category: acceptance
• Problem: PR documents manual warm-build cache evidence (OpenClaw 20.9s→0.1s, Hermes 21.5s→0.4s) but no automated regression test exists. Prior review PRA-1/T6/T7 requested automated Vitest integration test that builds twice with identical args and asserts layer reuse. Acceptance clause 'Tests/CI compare cold vs warm cache reuse' unmet.
• Impact: Without automated validation, a future change could inadvertently reintroduce cache misses that degrade warm onboard/rebuild performance from ~0.1s back to ~20s.
• Required action: Add automated warm-build cache regression test in test/security-c2-dockerfile-injection.test.ts or new test file. Can be a Vitest integration test using docker build --no-cache vs docker build (cached) with timing threshold or BuildKit cache metadata. Defer only with explicit rationale in PR.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: Check if test/security-c2-dockerfile-injection.test.ts or new test contains warm-cache regression test running docker build twice with identical args and validating layer cache hits.
• Missing regression test: Automated warm-build cache reuse test: build image twice with same args, verify second build completes in <5s (indicating cache reuse) and BuildKit reports CACHED for all layers except final ENV.
• Done when: The required change is committed and verification passes: Check if test/security-c2-dockerfile-injection.test.ts or new test contains warm-cache regression test running docker build twice with identical args and validating layer cache hits.
• Evidence: PR body: 'Verified clean live build measurements: OpenClaw cold 20.9s vs warm 0.1s; Hermes cold 21.5s vs warm 0.4s.' No automated test present in diff. Issue perf(onboard): reduce Dockerfile cache misses in warm sandbox builds #4682 acceptance criteria: 'Tests or CI/manual benchmark guidance compare cold vs warm builds and show the expected cache reuse.'

PRA-2 Resolve/justify — Parent issues (#2001, #3776) not updated with timing evidence

• Location: test/fixtures/warm-build-cache-evidence.md:50
• Category: acceptance
• Problem: Acceptance clause 'Parent issues (perf: investigate and reduce networking latency during onboard and validation #2001, perf(onboard): define onboard performance budget and CI regression signal #3776) updated with timing' — not visible in PR diff or linked issue comments.
• Impact: Performance tracking issue loses visibility; future reviewers cannot verify improvement was communicated upstream.
• Recommended action: Confirm parent issues updated with timing evidence or add follow-up task to update them before merge.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check GitHub issues perf: investigate and reduce networking latency during onboard and validation #2001 and perf(onboard): define onboard performance budget and CI regression signal #3776 for timing updates from this PR.
• Missing regression test: N/A — process item.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check GitHub issues perf: investigate and reduce networking latency during onboard and validation #2001 and perf(onboard): define onboard performance budget and CI regression signal #3776 for timing updates from this PR.
• Evidence: Issue perf(onboard): reduce Dockerfile cache misses in warm sandbox builds #4682 acceptance criteria: 'perf: investigate and reduce networking latency during onboard and validation #2001 or perf(onboard): define onboard performance budget and CI regression signal #3776 is updated with timing evidence if the change materially improves warm onboard/rebuild time.' PR body and diff show no evidence of updates.

PRA-3 Resolve/justify — dockerfileConsumesBuildId regex may miss indirect references in custom Dockerfiles

• Location: src/lib/onboard/dockerfile-patch.ts:55
• Category: security
• Problem: dockerfileConsumesBuildId uses word-boundary regex (\bNEMOCLAW_BUILD_ID\b) which correctly detects ENV/RUN/ARG/LABEL direct references in tests, but may miss edge cases in custom Dockerfiles: (1) indirect references via intermediate variables (ARG X=${NEMOCLAW_BUILD_ID}; ENV Y=${X}), (2) concatenated references without word boundaries in unusual contexts. Risk is low since function operates on NemoClaw-generated staged Dockerfiles (trusted structure), but custom Dockerfiles could bypass cache busting.
• Impact: If a custom Dockerfile consumes NEMOCLAW_BUILD_ID in a way the regex misses, the build ID will remain stable and the custom Dockerfile's cache-busting logic won't trigger, potentially serving stale config.
• Recommended action: Add unit test for indirect reference case (should return false — current behavior is correct). Document that custom Dockerfiles consuming NEMOCLAW_BUILD_ID indirectly must declare explicit consumption.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Run npx vitest run src/lib/onboard/dockerfile-patch.test.ts and verify new test 'leaves NEMOCLAW_BUILD_ID stable unless the Dockerfile consumes it' passes for ENV/RUN/ARG/LABEL patterns. Check function at line 55-63.
• Missing regression test: Test for indirect reference: ARG X=${NEMOCLAW_BUILD_ID}; ENV Y=${X} should return false (current behavior correct — only direct textual references bust cache).
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Run npx vitest run src/lib/onboard/dockerfile-patch.test.ts and verify new test 'leaves NEMOCLAW_BUILD_ID stable unless the Dockerfile consumes it' passes for ENV/RUN/ARG/LABEL patterns. Check function at line 55-63.
• Evidence: Function at lines 55-63: splits Dockerfile by lines, skips comments and ARG declarations, tests word-boundary regex. Tests cover 4 direct patterns but no indirect variable references.

PRA-4 Resolve/justify — dockerfileConsumesBuildId test coverage missing multi-line RUN and multiple refs on one line

• Location: src/lib/onboard/dockerfile-patch.test.ts:316
• Category: correctness
• Problem: New test 'leaves NEMOCLAW_BUILD_ID stable unless the Dockerfile consumes it' covers 4 consumption patterns (ENV, RUN, ARG, LABEL) but does not test: (1) multi-line RUN with backslash continuation, (2) multiple references on same line (e.g., ENV A=${NEMOCLAW_BUILD_ID} B=${NEMOCLAW_BUILD_ID}).
• Impact: Edge cases in real custom Dockerfiles could go undetected, leading to unexpected cache behavior.
• Recommended action: Add subtests for multi-line RUN continuation and multiple references on one line to strengthen confidence.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Run vitest and check test coverage for dockerfileConsumesBuildId. Look for test cases with RUN echo ${NEMOCLAW_BUILD_ID} \\ && echo done and ENV A=${NEMOCLAW_BUILD_ID} B=${NEMOCLAW_BUILD_ID}.
• Missing regression test: Unit tests for: (1) RUN echo ${NEMOCLAW_BUILD_ID} \\ && echo done (multi-line), (2) ENV A=${NEMOCLAW_BUILD_ID} B=${NEMOCLAW_BUILD_ID} (multiple).
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Run vitest and check test coverage for dockerfileConsumesBuildId. Look for test cases with RUN echo ${NEMOCLAW_BUILD_ID} \\ && echo done and ENV A=${NEMOCLAW_BUILD_ID} B=${NEMOCLAW_BUILD_ID}.
• Evidence: Test file lines 316-360 cover 4 patterns but no multi-line or multiple-ref cases.

PRA-5 Improvement — Test file monolith growth — extract dockerfileConsumesBuildId tests

• Location: src/lib/onboard/dockerfile-patch.test.ts:1
• Category: architecture
• Problem: Test file grew by 63 lines (1226→1289), triggering monolith growth blocker. Growth is from new test 'leaves NEMOCLAW_BUILD_ID stable unless the Dockerfile consumes it' which adds valuable coverage.
• Impact: Large test file harder to maintain, navigate, and review. Related tests scattered.
• Suggested action: Extract the new dockerfileConsumesBuildId test suite to a dedicated file (e.g., dockerfile-patch-consumes-build-id.test.ts).
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check src/lib/onboard/ directory for test file count; verify new file exists with the extracted tests.
• Missing regression test: N/A — architecture item.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: Static analysis flagged dockerfile-patch.test.ts as large-file hotspot with +63 delta. Monolith growth blocker rationale: 'Current monolith grew by 20 or more lines; extract or offset the growth before merge.'

PRA-6 Improvement — Security test updated but could add Hermes equivalent

• Location: test/security-c2-dockerfile-injection.test.ts:85
• Category: tests
• Problem: Security test validates cache-safe config boundary for OpenClaw Dockerfile (ARG → generator RUN → runtime ENV). Hermes Dockerfile follows same pattern but has no dedicated security test mirroring this.
• Impact: Future changes to Hermes Dockerfile could inadvertently reintroduce early ENV cache-busting without test coverage.
• Suggested action: Add mirror security test for agents/hermes/Dockerfile verifying same cache-safe boundary: ARG declarations before config generation RUN, ENV promotion after all build-time layers.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Verify agents/hermes/Dockerfile has no early ENV block; ENV promotion at line 354 comes after post-agent-install RUN. Add test asserting ordering.
• Missing regression test: Security test for Hermes Dockerfile: assert ARG NEMOCLAW_DISABLE_DEVICE_AUTH line < generator RUN line < runtime ENV promotion line.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: OpenClaw security test at test/security-c2-dockerfile-injection.test.ts:85 validates hasCacheSafeConfigBoundary. Hermes Dockerfile at line 354 has ENV promotion after post-agent-install RUN at line ~320.

PRA-7 Improvement — C-2 Dockerfile injection fix preserved with cache-safe boundary

• Location: Dockerfile:942
• Category: security
• Problem: Build args promoted to ENV only after config generation RUN layer; generate-openclaw-config.mts reads from process.env (data), not source interpolation. Security test validates cache-safe boundary (ARG → generator RUN → runtime ENV).
• Impact: No credential leakage, no code injection vector. Build-time config reads safe; runtime ENV contract preserved.
• Suggested action: No action needed; pattern is secure.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check test/security-c2-dockerfile-injection.test.ts:85 for cache-safe boundary assertion; verify Dockerfile has ARG declarations before generate-openclaw-config.mts RUN and ENV block after plugin install layers.
• Missing regression test: Existing security test covers this.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: Dockerfile: ARG block at line ~550, generate-openclaw-config.mts RUN at line ~620, ENV promotion at line 942. Security test asserts argLine < generatorLine < runtimeEnvLine.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

[amata-human]

Addressed the PR Advisor items in 381383684:

• PRA-2 / acceptance evidence: added test/fixtures/warm-build-cache-evidence.md with the cold/warm measurement method, command shape, observed OpenClaw and Hermes timings, and expected cache-hit behavior.
• PRA-3 / PRA-1: updated scripts/generate-openclaw-config.mts to describe the new ARG-to-process.env build contract and late runtime ENV persistence.
• Build-id edge cases: expanded src/lib/onboard/dockerfile-patch.test.ts coverage for comment-only references plus ENV, RUN, concatenated ARG, and LABEL consumers.

Validation rerun:

• npm test -- --run src/lib/onboard/dockerfile-patch.test.ts test/security-c2-dockerfile-injection.test.ts
• npm run build:cli && npm run typecheck:cli
• npm run source-shape:check
• npm run test:projects:check
• npx prek run --files Dockerfile agents/hermes/Dockerfile scripts/generate-openclaw-config.mts src/lib/onboard/dockerfile-patch.ts src/lib/onboard/dockerfile-patch.test.ts test/security-c2-dockerfile-injection.test.ts test/fixtures/warm-build-cache-evidence.md

[github-actions]

E2E Target Recommendation

Required E2E targets: e2e-all
Optional E2E targets: None

Dispatch required E2E targets:

• gh workflow run e2e.yaml --ref <pr-head-ref>

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required E2E targets

• e2e-all: The PR changes the stock OpenClaw and Hermes Dockerfiles plus the onboarding Dockerfile patcher/config-generation path that determines sandbox image build inputs and runtime ENV persistence. This affects core live onboarding/build behavior across multiple targets, so run the full E2E target fan-out.
  • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref>

Optional E2E targets

• None.

Relevant changed files

• Dockerfile
• agents/hermes/Dockerfile
• scripts/generate-openclaw-config.mts
• src/lib/onboard/dockerfile-patch.ts

[cv]

Superseded by #6034, a signed same-repo salvage that preserves Angel Mata's authorship and timing evidence. The replacement narrows the change to the verified cache lever (stable stock NEMOCLAW_BUILD_ID), adds byte-identical OpenClaw/Hermes build-context regressions, and drops the late-ENV relocation after a BuildKit control showed changed ARG values still invalidate following RUN layers.