fix(dockerfile): drop Patch 4 entirely#3500
Conversation
…range Patch 4 monkey-patches OpenClaw's `replaceConfigFile` to suppress EACCES inside the sandbox. Until the upstream fix in openclaw/openclaw#72950 lands, the patch's only effect is downgrading a hard EACCES crash to a warning during plugin metadata persistence (plugins still load via auto-discovery from extensions/). But the patch's regex tracks OpenClaw's internal source shape, and each OpenClaw refactor has broken it -- v0.0.29 hit one such break (#2686); v0.0.40 on macOS hit a stale-install variant (#3497). Each break is a hard image-build failure even though the patch is, by design, a fallback for noisier behaviour in the sandbox. Replace the hard-assert with a version-gated soft-fail: * `rcf_patch.py` now reads `OPENCLAW_VERSION` from env. When it exceeds the new `LAST_OPENCLAW_NEEDING_RCF_PATCH` sentinel, the script exits 0 without touching the source on the assumption that the upstream fix has landed and the patch is no longer needed. Maintainers bump the sentinel DOWN once that becomes true. * When the regex fails to match -- the OpenClaw refactor case the hard-assert was guarding -- the script now emits a loud `[nemoclaw] WARN` line and exits 0. Plugin metadata persistence in the sandbox surfaces raw EACCES at runtime instead of being suppressed, but the image still builds. The Dockerfile's post-patch grep follows the same shape (`echo WARN`, not `exit 1`). * `test/rcf-patch.test.ts` covers the four new branches: pattern miss (soft-warn), missing function (soft-warn), past-sentinel skip, and unparseable / empty `OPENCLAW_VERSION` (still attempts the patch). Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (4)
📝 WalkthroughWalkthroughRemoves the in-image OpenClaw “Patch 4” invocation and the Dockerfile copy of ChangesRemove in-image Patch 4 & adjust staging
rcf_patch script behavior (repo-only) and tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/rcf_patch.py (1)
1-3:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd the required SPDX header at the top of this Python source file.
This file is missing the repository-mandated SPDX header block.
Suggested patch
+# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 """ Patch replaceConfigFile in OpenClaw dist to wrap the tryWriteSingleTopLevelIncludeMutation/writeConfigFile block in a try/catchAs per coding guidelines "
**/*.{js,ts,tsx,sh,py}: Include SPDX license header at the top of every source file..."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/rcf_patch.py` around lines 1 - 3, Add the repository-mandated SPDX header block at the very top of this Python file (rcf_patch.py) above the existing module docstring: insert the standard SPDX-License-Identifier line and the required copyright/ownership header used by the repo so every .py file includes the license header; ensure the header is formatted as a Python comment block (leading #) and placed before any other code or docstrings.
🧹 Nitpick comments (1)
Dockerfile (1)
205-212: Run the Dockerfile E2E matrix for this fail-open behavior change.Since this modifies patching behavior during image build (warn-and-continue path), I recommend running the four listed jobs on this branch before merge:
cloud-e2e,sandbox-survival-e2e,hermes-e2e, andrebuild-openclaw-e2e.As per coding guidelines "
Dockerfile: This file affects the sandbox container image... E2E test recommendation: cloud-e2e, sandbox-survival-e2e, hermes-e2e, rebuild-openclaw-e2e."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` around lines 205 - 212, The Dockerfile change introduces a fail-open path when patching replaceConfigFile (see rcf_file, OPENCLAW_VERSION, and python3 /usr/local/lib/nemoclaw/rcf_patch.py) so before merging run the full Dockerfile E2E matrix to validate behavior: trigger the cloud-e2e, sandbox-survival-e2e, hermes-e2e, and rebuild-openclaw-e2e jobs against this branch and confirm the warn-and-continue path (grep for OPENSHELL_SANDBOX.*EACCES and the echo warnings about Patch 4) produces acceptable logs and no regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@scripts/rcf_patch.py`:
- Around line 1-3: Add the repository-mandated SPDX header block at the very top
of this Python file (rcf_patch.py) above the existing module docstring: insert
the standard SPDX-License-Identifier line and the required copyright/ownership
header used by the repo so every .py file includes the license header; ensure
the header is formatted as a Python comment block (leading #) and placed before
any other code or docstrings.
---
Nitpick comments:
In `@Dockerfile`:
- Around line 205-212: The Dockerfile change introduces a fail-open path when
patching replaceConfigFile (see rcf_file, OPENCLAW_VERSION, and python3
/usr/local/lib/nemoclaw/rcf_patch.py) so before merging run the full Dockerfile
E2E matrix to validate behavior: trigger the cloud-e2e, sandbox-survival-e2e,
hermes-e2e, and rebuild-openclaw-e2e jobs against this branch and confirm the
warn-and-continue path (grep for OPENSHELL_SANDBOX.*EACCES and the echo warnings
about Patch 4) produces acceptable logs and no regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e8093075-ecbf-4caa-aae0-887cec8f3bc8
📒 Files selected for processing (3)
Dockerfilescripts/rcf_patch.pytest/rcf-patch.test.ts
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Patch 4 (Dockerfile step 17 + scripts/rcf_patch.py) used a regex over OpenClaw's compiled JS to wrap `replaceConfigFile` in an EACCES try/catch suppression for the read-only sandbox. The regex tracked OpenClaw's internal source shape, which has already broken twice (#2686, #3497) and will break again every time the upstream refactors the function. Switch to a runtime trap via `NODE_OPTIONS=--require` instead: * nemoclaw-blueprint/scripts/openclaw-rcf-shim.js hooks `Module.prototype.require`, detects when the OpenClaw `mutate.js` module loads, and wraps the exported `replaceConfigFile` with the same try/catch behaviour. The wrap is gated on the running OpenClaw version being at or below a sentinel (`last_openclaw_needing_rcf_shim` in blueprint.yaml, initial value `9999.99.99`). Bump the sentinel DOWN when openclaw/openclaw#72950 lands; the shim then self-disables for newer versions. * scripts/nemoclaw-start.sh::install_openclaw_rcf_shim() reads the sentinel from blueprint.yaml and the current OpenClaw version from `openclaw --version`, exports both as env vars the shim consumes, and threads `--require` into `NODE_OPTIONS`. * The Dockerfile's Step 16 emits a subtle INFO note when the sentinel still sits above min_openclaw_version — a maintenance nudge to keep the sentinel current as openclaw/openclaw#72950 progresses. * Drop scripts/rcf_patch.py and test/rcf-patch.test.ts entirely. * Add test/openclaw-rcf-shim.test.ts covering the six behaviour branches (no-op outside sandbox, EACCES swallow + synthesised ConfigReplaceResult, non-EACCES re-throw, happy-path pass-through, past-sentinel skip, non-matching filename skip). * schemas/blueprint.schema.json gains the new sentinel key; the blueprint validation test covers it. * src/lib/sandbox/build-context.ts no longer stages rcf_patch.py. Resolves #2686 Resolves #3497 Refs openclaw/openclaw#72950 (upstream defect; this PR only stops the crash, the real fix has to land there) Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Patch 4 is a regex-based monkey-patch over OpenClaw's compiled JS that suppresses EACCES inside replaceConfigFile. Its source-shape coupling has broken three times in eight days (#2377, #2484, #2876) chasing upstream refactors; #2686 and #3497 report the latest casualty, where the regex no longer finds the function in 2026.4.24 and the image build fails. Patch 4 is also unnecessary by design: * In mutable-default mode, openclaw.json is chmod 660 sandbox:sandbox and the gateway UID is in the sandbox group (#2681), so plugin install writes through without ever hitting EACCES. * In shields-up mode, the entire config tree (file + parent dir + the plugin/extensions state dirs in HIGH_RISK_STATE_DIRS) is locked to root:root by design — refusing runtime mutations is the whole point of shields-up. Suppressing the EACCES masked that refusal and made the install appear to succeed silently while only the auto-discovery half landed. The expected flow is configure-in-mutable-mode → shields up → run. Plugin install attempted while shielded should fail cleanly, which is what happens without Patch 4. Reverts the rcf-shim replacement attempt; the require-hook approach does not catch OpenClaw's ESM named imports anyway (capture-at-import- time semantics). Resolves #2686 Resolves #3497 Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Selective E2E Results — ✅ All requested jobs passedRun: 25859905690
|
|
❌ Brev E2E (full): FAILED on branch |
Selective E2E Results — ✅ All requested jobs passedRun: 25895823458
|
## Summary - clarify that `scripts/rcf_patch.py` is intentionally absent from current blueprints - replace the stale Patch-4 fail-closed QA expectation with the current mutable-default / shields-up validation path - add regression coverage so troubleshooting keeps pointing QA at the supported checks ## Root Cause Patch 4 was intentionally deleted by #3500 after NemoClaw moved away from the build-time `replaceConfigFile` monkey-patch. The issue reproduces because the old QA test plan still expects `scripts/rcf_patch.py` to exist even though the supported behavior is now the mutable-default config model plus shields-up lockdown. Closes #3944 ## Verification - `test ! -f scripts/rcf_patch.py && test ! -f nemoclaw-blueprint/scripts/rcf_patch.py` - `./node_modules/.bin/vitest run test/rcf-patch-removal.test.ts test/repro-2681-group-writable.test.ts test/sandbox-build-context.test.ts` - `npm run build:cli` - `npm run docs:strict` - `bash -n test/e2e/test-shields-config.sh` Signed-off-by: Chengjie Wang <chengjiew@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Enhanced troubleshooting guide clarifying current sandbox configuration lifecycle, deprecated test behavior, and updated validation instructions for shields/config checks. * **Tests** * Added automated test to verify removal of the retired patch script from blueprints and repo locations to prevent regressions in setup and validation. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4202?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
Summary
Patch 4 in the sandbox
Dockerfileis a regex-based monkey-patch over OpenClaw's compiled JS that wrapsreplaceConfigFilein an EACCES try/catch suppression. It is source-shape-coupled and has been rewritten three times in eight days chasing OpenClaw refactors:fefd69fa2(fix(dockerfile): patch plugin install for symlinked extensions dir #2377) — original literal-string match against openclaw/openclaw@v2026.4.9.5dcb0a9b9(chore: upgrade OpenClaw from 2026.4.9 to 2026.4.24 #2484) — updated the literal string for the restructured write block in openclaw/openclaw@v2026.4.24.e0290e153(fix: harden Dockerfile replaceConfigFile patch #2876) — hardened to a tolerant whitespace/property-order-aware regex against openclaw/openclaw@v2026.4.24.#2686 and #3497 are the latest break: in current OpenClaw, the regex no longer finds the function shape and the image build aborts at Step 17/56.
Patch 4 is also unnecessary by design. The EACCES it was suppressing does not happen in the supported flows:
nemoclaw shields up):openclaw.jsonischmod 660 sandbox:sandboxand the gateway UID is in the sandbox group, courtesy of fix(sandbox): make .openclaw group-writable so OpenClaw config mutations land cleanly (#2681) #2851 (closing OpenClaw UI "Enable Dreaming" doesn't work because of GatewayRequestError: EACCES: permission denied #2681; superseding the EACCES-swallow attempt in fix(sandbox): graceful EACCES in mutateConfigFile (#2681) #2693).openclaw plugins installwrites through normally; no EACCES.extensions/pluginsstate dirs from HIGH_RISK_STATE_DIRS — is locked toroot:rootby design. Shields-up exists to refuse runtime config and plugin mutations. Suppressing the EACCES masked that refusal and madeopenclaw plugins installappear to succeed silently while only the auto-discovery half landed.The expected lifecycle is configure-in-mutable-mode →
shields up→ run. Plugin install attempted while shielded should fail cleanly; that is exactly what happens without Patch 4.This PR therefore deletes Patch 4 entirely.
Related Issue
Resolves #2686
Resolves #3497
Related context:
.openclawgroup-writable" issue, closed by fix(sandbox): make .openclaw group-writable so OpenClaw config mutations land cleanly (#2681) #2851.plugins.entries.<id>.config); the real fix has to land there.Changes
Dockerfile: drop the Patch 4 block (theCOPY scripts/rcf_patch.pyline and the inline Python invocation + grep guard).scripts/rcf_patch.py: deleted.src/lib/sandbox/build-context.ts: stop stagingscripts/rcf_patch.pyinto the sandbox build context.test/rcf-patch.test.ts: deleted.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
Chores
Tests