test(e2e): restore OpenClaw rebuild fixture parity - #5855
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe old-base build-context logic moved into a shared helper module, and the live rebuild-openclaw test now imports it. New support tests verify staged context copying, direct ChangesOpenClaw rebuild context
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
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. |
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 28241415036
|
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 28242031263
|
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
test/e2e-scenario/live/rebuild-openclaw-old-base-context.ts (1)
21-42: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle multiline
COPYinstructions before tokenizing.This parser works on physical lines, so a valid continued form like
COPY a \/b /destwill record onlyaand silently missb. Because the support test calls the same helper, that drift would also evade the new guard and only surface later whencreateOldBaseBuildContext()stages an incomplete context. Please join\-continued lines into logical Dockerfile instructions before applying the current token checks.🤖 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-scenario/live/rebuild-openclaw-old-base-context.ts` around lines 21 - 42, Update the Dockerfile COPY parser in the helper used by the rebuild-openclaw-old-base-context test so it processes logical instructions, not just physical lines. Right now the loop tokenizes each raw line directly, so multiline COPY forms with backslash continuations can lose later path tokens and slip past the existing COPY validation. Fold `\`-continued lines together before the current `instructionMatch`/token checks, then keep the existing source extraction and stage-source guard in place so `createOldBaseBuildContext()` sees the full COPY source list.
🧹 Nitpick comments (1)
test/e2e-scenario/support-tests/rebuild-openclaw-old-base-context.test.ts (1)
47-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the
COPY --from buildform too.This guard only exercises
--from=..., butdirectDockerfileBaseCopySources()also has a separate branch for the space-delimited--fromsyntax. Add that variant here so the support test actually locks both exclusions down.Suggested test fixture update
[ "FROM base AS build", "copy scripts/lib/sandbox-rlimits.sh /tmp/lowercase", "COPY\tnemoclaw-blueprint/blueprint.yaml /tmp/tabbed", + "COPY --from build /tmp/ignored-space /tmp/ignored-space", "COPY --from=build /tmp/ignored /tmp/ignored", ].join("\n"), "utf8", );🤖 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-scenario/support-tests/rebuild-openclaw-old-base-context.test.ts` around lines 47 - 59, The support test for directDockerfileBaseCopySources only covers the equals-style --from form, so it misses the space-delimited --from branch. Update the fixture in rebuild-openclaw-old-base-context.test.ts to include a COPY --from build case alongside the existing COPY --from=build case, and keep the expectation focused on verifying both forms are excluded from the returned base copy sources.
🤖 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 `@test/e2e-scenario/live/rebuild-openclaw-old-base-context.ts`:
- Around line 21-42: Update the Dockerfile COPY parser in the helper used by the
rebuild-openclaw-old-base-context test so it processes logical instructions, not
just physical lines. Right now the loop tokenizes each raw line directly, so
multiline COPY forms with backslash continuations can lose later path tokens and
slip past the existing COPY validation. Fold `\`-continued lines together before
the current `instructionMatch`/token checks, then keep the existing source
extraction and stage-source guard in place so `createOldBaseBuildContext()` sees
the full COPY source list.
---
Nitpick comments:
In `@test/e2e-scenario/support-tests/rebuild-openclaw-old-base-context.test.ts`:
- Around line 47-59: The support test for directDockerfileBaseCopySources only
covers the equals-style --from form, so it misses the space-delimited --from
branch. Update the fixture in rebuild-openclaw-old-base-context.test.ts to
include a COPY --from build case alongside the existing COPY --from=build case,
and keep the expectation focused on verifying both forms are excluded from the
returned base copy sources.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1eea2611-f030-4df0-90eb-d0df986711fd
📒 Files selected for processing (2)
test/e2e-scenario/live/rebuild-openclaw-old-base-context.tstest/e2e-scenario/support-tests/rebuild-openclaw-old-base-context.test.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 28242713033
|
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 28243892150
|
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 28244425002
|
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 28246460827
|
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Vitest E2E Scenario Results — ✅ All requested jobs passedRun: 28247006896
|
Vitest E2E Scenario Results — ✅ All requested jobs passedRun: 28248247005
|
wscurran
added
area: e2e
|
✨
Related open issues: |
## Summary Restore issue NVIDIA#5849 Package F parity for the migrated `rebuild-openclaw-vitest` fixture. The legacy bash test builds `Dockerfile.base` with the full repository as Docker context after temporarily lowering `nemoclaw-blueprint/blueprint.yaml`. The migrated Vitest kept the checkout read-only by staging a minimal context, but after NVIDIA#5682 `Dockerfile.base` also copies `scripts/lib/sandbox-rlimits.sh`, so the Vitest old-base build failed before reaching the rebuild assertions. ## Related Issues Refs NVIDIA#5849 Refs NVIDIA#5800 Refs NVIDIA#5682 ## Scope gate - Package: `Package F — OpenClaw rebuild fixture parity` - Included input: NVIDIA#5682 is the explicit Vitest-fixture exception documented in NVIDIA#5849; it did not touch legacy bash, but exposed migrated Vitest setup drift against the bash reference. - Bash reference: `test/e2e/test-rebuild-openclaw.sh` - Out of scope: shell lane retirement / PR NVIDIA#5756 cleanup; OpenClaw version bump PR NVIDIA#5595; Hermes rebuild fixes. ## Parity map | ID | Source / evidence | Contract | Vitest assertion / fix | Status | | --- | --- | --- | --- | --- | | F1 | NVIDIA#5682, run 28214953054 | Migrated OpenClaw rebuild Vitest must build the old base with every direct `Dockerfile.base` context dependency needed by the bash full-repo build. | `createOldBaseBuildContext()` now stages `nemoclaw-blueprint/blueprint.yaml` and `scripts/lib/sandbox-rlimits.sh` before building `Dockerfile.base`. | covered | | F2 | Future `Dockerfile.base` COPY drift | If `Dockerfile.base` gains another direct `COPY`, the Package F fixture must fail fast before live E2E runtime. | New support test `test/e2e-scenario/support-tests/rebuild-openclaw-old-base-context.test.ts` checks direct `Dockerfile.base` COPY sources are represented in the old-base context list. | covered | ## Inference mode support - Default mode for touched live target: existing `public-nvidia` / hosted path for `rebuild-openclaw-vitest`; no inference mode behavior changed. - Real inference support preserved: yes; this PR only fixes pre-rebuild Docker build fixture setup. - Modes validated in this PR: local support/unit/build/typecheck. Live Docker validation requires GitHub runner because local Docker daemon is unavailable. ## Validation - [x] `npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/rebuild-openclaw-old-base-context.test.ts` - [x] `npm run build:cli` - [x] `npm run typecheck:cli` - [x] `git diff --check` - [ ] Selective `rebuild-openclaw-vitest` workflow on PR branch ## Follow-ups / waivers - Local live run not attempted because Docker daemon is unavailable on this machine (`Cannot connect to the Docker daemon at unix:///Users/jyaunches/.docker/run/docker.sock`). Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added end-to-end helper utilities to generate a temporary legacy Docker build context, stage required direct `COPY` sources, and rewrite the blueprint’s minimum OpenClaw version. * **Bug Fixes** * Strengthened Dockerfile parsing to include only supported direct `COPY` sources while excluding non-matching variants and secret-related inputs. * **Tests** * Added e2e test coverage for legacy context creation, correct `COPY` source selection, error handling for unsupported paths, and automatic cleanup of temp directories. * **Refactor** * Reused the shared legacy build-context helper in the rebuild scenario. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
3 participants
Summary
Restore issue #5849 Package F parity for the migrated
rebuild-openclaw-vitestfixture.The legacy bash test builds
Dockerfile.basewith the full repository as Docker context after temporarily loweringnemoclaw-blueprint/blueprint.yaml. The migrated Vitest kept the checkout read-only by staging a minimal context, but after #5682Dockerfile.basealso copiesscripts/lib/sandbox-rlimits.sh, so the Vitest old-base build failed before reaching the rebuild assertions.Related Issues
Refs #5849
Refs #5800
Refs #5682
Scope gate
Package F — OpenClaw rebuild fixture paritytest/e2e/test-rebuild-openclaw.shParity map
Dockerfile.basecontext dependency needed by the bash full-repo build.createOldBaseBuildContext()now stagesnemoclaw-blueprint/blueprint.yamlandscripts/lib/sandbox-rlimits.shbefore buildingDockerfile.base.Dockerfile.baseCOPY driftDockerfile.basegains another directCOPY, the Package F fixture must fail fast before live E2E runtime.test/e2e-scenario/support-tests/rebuild-openclaw-old-base-context.test.tschecks directDockerfile.baseCOPY sources are represented in the old-base context list.Inference mode support
public-nvidia/ hosted path forrebuild-openclaw-vitest; no inference mode behavior changed.Validation
npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/rebuild-openclaw-old-base-context.test.tsnpm run build:clinpm run typecheck:cligit diff --checkrebuild-openclaw-vitestworkflow on PR branchFollow-ups / waivers
Cannot connect to the Docker daemon at unix:///Users/jyaunches/.docker/run/docker.sock).Signed-off-by: Julie Yaunches jyaunches@nvidia.com
Summary by CodeRabbit
COPYsources, and rewrite the blueprint’s minimum OpenClaw version.COPYsources while excluding non-matching variants and secret-related inputs.COPYsource selection, error handling for unsupported paths, and automatic cleanup of temp directories.