fix(e2e): pin old gateway upgrade install#4041
Merged
jyaunches merged 3 commits intoMay 22, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR adds a Docker wrapper mechanism to the OpenShell gateway upgrade test to enforce deterministic build arguments for legacy NemoClaw/OpenClaw versions during regression testing. Configuration constants and environment setup ensure the old installer path uses pinned base images and versions rather than defaults. ChangesDocker Wrapper for Old Version Pinning
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/e2e/docs/parity-map.yaml (2)
1-1:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd the required SPDX header at the top of this YAML file.
This file was modified, but it still has no SPDX copyright/license header. That leaves the change out of compliance with the repo’s source-file policy.
As per coding guidelines,
**/*.{js,ts,tsx,jsx,sh,yaml,yml,json,md,mdx}: Every source file must include an SPDX license header for copyright and Apache-2.0 license.🤖 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 `@test/e2e/docs/parity-map.yaml` at line 1, Add the required SPDX header lines at the very top of the YAML file (before the existing "scripts:" key) so the file includes both copyright and license identifiers; specifically insert SPDX-FileCopyrightText (with the appropriate year/owner) and SPDX-License-Identifier: Apache-2.0 as the first lines in the file so the repository’s source-file policy is satisfied.
8117-8175:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRestore the truncated
reasontext in these deferred assertions.Several edited entries now end in fragments like
prerequisite failure for,expected green summary after, andlive regression guard assertion for. That strips the explanation the parity map is supposed to preserve for why coverage is deferred or what the assertion is guarding.Also applies to: 11251-11264
🤖 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 `@test/e2e/docs/parity-map.yaml` around lines 8117 - 8175, Several deferred entries were truncated—find the entries with the shown legacy keys (e.g. "Docker is not running", "NVIDIA_API_KEY is set", "NVIDIA_API_KEY is required and must start with nvapi-", "nemoclaw is available: $(nemoclaw --version...)", "nemoclaw not found after install", "fresh sandbox onboard completed", "fresh sandbox onboard failed (exit ${onboard_rc}); see ${ONBOARD_LOG}", "OpenClaw-style plugin runtime deps replacement hit `#3513` EXDEV failure", "runtime deps replacement exited ${agent_rc}; see ${AGENT_LOG}", "OpenClaw-style plugin runtime-deps replacement completed across filesystems", "runtime deps replacement exited 0 but success marker was missing; see ${AGENT_LOG}", "OpenClaw plugin runtime-deps EXDEV guard passed") and restore their full reason text (undo the truncation like "prerequisite failure for", "expected green summary after", etc.) by copying the original reason strings from the previous commit or backup and replacing the incomplete reason fields so each deferred entry again contains the full explanatory sentence the parity map must preserve.
🤖 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.
Inline comments:
In `@test/e2e/test-openshell-gateway-upgrade.sh`:
- Around line 175-177: When rewrote_base is "0" the script only logs but doesn't
enforce a pinned BASE_IMAGE, allowing a mutable Dockerfile default to be used;
update the branch handling rewrote_base (the if [ "$rewrote_base" = "0" ] block)
to assign and export a pinned fallback (e.g. set BASE_IMAGE to a
PINNED_BASE_IMAGE constant or env var) before appending the log to $log_file so
builds use the deterministic pinned base when no override is provided.
---
Outside diff comments:
In `@test/e2e/docs/parity-map.yaml`:
- Line 1: Add the required SPDX header lines at the very top of the YAML file
(before the existing "scripts:" key) so the file includes both copyright and
license identifiers; specifically insert SPDX-FileCopyrightText (with the
appropriate year/owner) and SPDX-License-Identifier: Apache-2.0 as the first
lines in the file so the repository’s source-file policy is satisfied.
- Around line 8117-8175: Several deferred entries were truncated—find the
entries with the shown legacy keys (e.g. "Docker is not running",
"NVIDIA_API_KEY is set", "NVIDIA_API_KEY is required and must start with
nvapi-", "nemoclaw is available: $(nemoclaw --version...)", "nemoclaw not found
after install", "fresh sandbox onboard completed", "fresh sandbox onboard failed
(exit ${onboard_rc}); see ${ONBOARD_LOG}", "OpenClaw-style plugin runtime deps
replacement hit `#3513` EXDEV failure", "runtime deps replacement exited
${agent_rc}; see ${AGENT_LOG}", "OpenClaw-style plugin runtime-deps replacement
completed across filesystems", "runtime deps replacement exited 0 but success
marker was missing; see ${AGENT_LOG}", "OpenClaw plugin runtime-deps EXDEV guard
passed") and restore their full reason text (undo the truncation like
"prerequisite failure for", "expected green summary after", etc.) by copying the
original reason strings from the previous commit or backup and replacing the
incomplete reason fields so each deferred entry again contains the full
explanatory sentence the parity map must preserve.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2adba7d1-75db-41a3-8886-d3e050543edd
📒 Files selected for processing (9)
test/e2e/docs/parity-map.yamltest/e2e/nemoclaw_scenarios/scenarios.yamltest/e2e/scenario-framework-tests/e2e-lib-helpers.test.tstest/e2e/test-openshell-gateway-upgrade.shtest/e2e/validation_suites/baseline-onboarding/00-cli-and-openshell.shtest/e2e/validation_suites/baseline-onboarding/01-sandbox-state.shtest/e2e/validation_suites/baseline-onboarding/02-route-and-smoke.shtest/e2e/validation_suites/lib/baseline_onboarding.shtest/e2e/validation_suites/suites.yaml
Comment on lines
+175
to
+177
| if [ "$rewrote_base" = "0" ]; then | ||
| printf 'no BASE_IMAGE override seen; old Dockerfile default remains unchanged\n' >>"$log_file" | ||
| fi |
Contributor
There was a problem hiding this comment.
Force the pinned BASE_IMAGE when no override is provided.
This branch only logs when BASE_IMAGE isn’t present, which means the build can still consume a mutable Dockerfile default and bypass the intended pin for old-install determinism. Inject the pinned value as a fallback.
Suggested fix
if [ "$rewrote_base" = "0" ]; then
- printf 'no BASE_IMAGE override seen; old Dockerfile default remains unchanged\n' >>"$log_file"
+ args+=("--build-arg" "BASE_IMAGE=${base_ref}")
+ printf 'add build-arg BASE_IMAGE=%s\n' "$base_ref" >>"$log_file"
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if [ "$rewrote_base" = "0" ]; then | |
| printf 'no BASE_IMAGE override seen; old Dockerfile default remains unchanged\n' >>"$log_file" | |
| fi | |
| if [ "$rewrote_base" = "0" ]; then | |
| args+=("--build-arg" "BASE_IMAGE=${base_ref}") | |
| printf 'add build-arg BASE_IMAGE=%s\n' "$base_ref" >>"$log_file" | |
| fi |
🤖 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 `@test/e2e/test-openshell-gateway-upgrade.sh` around lines 175 - 177, When
rewrote_base is "0" the script only logs but doesn't enforce a pinned
BASE_IMAGE, allowing a mutable Dockerfile default to be used; update the branch
handling rewrote_base (the if [ "$rewrote_base" = "0" ] block) to assign and
export a pinned fallback (e.g. set BASE_IMAGE to a PINNED_BASE_IMAGE constant or
env var) before appending the log to $log_file so builds use the deterministic
pinned base when no override is provided.
cv
approved these changes
May 22, 2026
jyaunches
force-pushed
the
fix/openshell-upgrade-e2e-old-base-pin
branch
from
e8d3304 to
cf8fadf
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
test/e2e/test-openshell-gateway-upgrade.sh (1)
145-151:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce pinned
BASE_IMAGEinstead of logging-only fallback.Line 175 currently logs and continues when no rewrite happened, so old-install builds can still use mutable/default base image behavior. Also, Line 145 and Line 160 only rewrite one exact
BASE_IMAGEvalue, which can miss other explicitBASE_IMAGE=inputs.Suggested fix
- if [ "$#" -ge 2 ] && [ "$2" = "BASE_IMAGE=ghcr.io/nvidia/nemoclaw/sandbox-base:latest" ]; then + if [ "$#" -ge 2 ] && [ "${2#BASE_IMAGE=}" != "$2" ]; then args+=("--build-arg" "BASE_IMAGE=${base_ref}") rewrote_base=1 printf 'rewrite build-arg %s -> BASE_IMAGE=%s\n' "$2" "$base_ref" >>"$log_file" shift 2 continue fi @@ - --build-arg=BASE_IMAGE=ghcr.io/nvidia/nemoclaw/sandbox-base:latest) + --build-arg=BASE_IMAGE=*) args+=("--build-arg=BASE_IMAGE=${base_ref}") rewrote_base=1 printf 'rewrite build-arg %s -> BASE_IMAGE=%s\n' "$1" "$base_ref" >>"$log_file" shift continue ;; @@ if [ "$rewrote_base" = "0" ]; then - printf 'no BASE_IMAGE override seen; old Dockerfile default remains unchanged\n' >>"$log_file" + args+=("--build-arg" "BASE_IMAGE=${base_ref}") + printf 'add build-arg BASE_IMAGE=%s\n' "$base_ref" >>"$log_file" fiAlso applies to: 160-166, 175-177
🤖 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 `@test/e2e/test-openshell-gateway-upgrade.sh` around lines 145 - 151, The script only rewrites a single exact "BASE_IMAGE=ghcr.io/nvidia/nemoclaw/sandbox-base:latest" token and otherwise just logs a fallback, allowing builds to use mutable images; update the logic in the blocks that touch args/rewrote_base (the branch that inspects "$2", the later branch at 160-166, and the post-loop 175-177 handling) to: detect any argument that begins with "BASE_IMAGE=" (use prefix matching), replace its value with the pinned ${base_ref} (push "--build-arg" "BASE_IMAGE=${base_ref}" into args and set rewrote_base=1), and if after processing no rewrite occurred, enforce the pinned base by appending "--build-arg" "BASE_IMAGE=${base_ref}" (not just logging) and log the enforced rewrite to ${log_file}; keep using the same variables/functions: args, rewrote_base, base_ref, and log_file to locate and change the code.
🤖 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.
Duplicate comments:
In `@test/e2e/test-openshell-gateway-upgrade.sh`:
- Around line 145-151: The script only rewrites a single exact
"BASE_IMAGE=ghcr.io/nvidia/nemoclaw/sandbox-base:latest" token and otherwise
just logs a fallback, allowing builds to use mutable images; update the logic in
the blocks that touch args/rewrote_base (the branch that inspects "$2", the
later branch at 160-166, and the post-loop 175-177 handling) to: detect any
argument that begins with "BASE_IMAGE=" (use prefix matching), replace its value
with the pinned ${base_ref} (push "--build-arg" "BASE_IMAGE=${base_ref}" into
args and set rewrote_base=1), and if after processing no rewrite occurred,
enforce the pinned base by appending "--build-arg" "BASE_IMAGE=${base_ref}" (not
just logging) and log the enforced rewrite to ${log_file}; keep using the same
variables/functions: args, rewrote_base, base_ref, and log_file to locate and
change the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7f12d1f7-6239-4f6b-951b-3ef856eb4ae6
📒 Files selected for processing (1)
test/e2e/test-openshell-gateway-upgrade.sh
jyaunches
added a commit
that referenced
this pull request
May 22, 2026
## Summary - force the old gateway-upgrade Docker wrapper to add the historical BASE_IMAGE build arg when the old installer does not pass one - keep the old install fixture pinned to the intended sandbox base digest instead of falling through to the mutable current base - preserve wrapper diagnostics by logging the added BASE_IMAGE fallback ## Why The openshell-gateway-upgrade E2E prepares a v0.0.36 install before exercising the current upgrade path. PR #4041 pinned the old OpenClaw version, but if the old installer does not pass `BASE_IMAGE=ghcr.io/nvidia/nemoclaw/sandbox-base:latest`, the wrapper only logged that no override was seen and left the old Dockerfile default in place. That allowed the fixture to consume a current base containing OpenClaw 2026.5.18, causing the old `rcf_patch.py` Patch 4 to fail before the upgrade path ran. ## Test plan - `bash -n test/e2e/test-openshell-gateway-upgrade.sh` - `git diff --check` - `npm run typecheck:cli` - `npm run test -- test/onboard-openshell-version.test.ts` Fixes the nightly `openshell-gateway-upgrade-e2e` preparation failure after PR #4041. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Enhanced Docker wrapper generator logic for upgrade regression testing to properly handle base image configuration when no override is specified, ensuring accurate testing scenarios during gateway upgrades. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4047?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 -->
](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4047?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
This was referenced May 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
The gateway-upgrade E2E prepares a real old-install scenario by running the v0.0.36 installer. That historical installer can otherwise consume today's mutable sandbox base / OpenClaw layout, causing the old Dockerfile's removed
rcf_patch.pymonkey patch to fail before the current upgrade path is exercised.Test plan
bash -n test/e2e/test-openshell-gateway-upgrade.shgit diff --checknpm run typecheck:clinpm run test -- test/onboard-openshell-version.test.tsSummary by CodeRabbit