Skip to content

fix: repair onboard duplicate import and gateway upgrade check#3438

Merged
ericksoa merged 3 commits into
mainfrom
fix/openshell-gateway-upgrade-e2e-check
May 13, 2026
Merged

fix: repair onboard duplicate import and gateway upgrade check#3438
ericksoa merged 3 commits into
mainfrom
fix/openshell-gateway-upgrade-e2e-check

Conversation

@ericksoa

@ericksoa ericksoa commented May 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Related: #3203

This PR now carries two small forward fixes needed after updating from current main:

  • updates the OpenShell gateway upgrade E2E to check the extracted Dockerfile patch helper in src/lib/onboard/dockerfile-patch.ts instead of the old implementation location in src/lib/onboard.ts
  • removes the duplicate web-search verifier require block that current main introduced in src/lib/onboard.ts and that breaks npm run build:cli with TS2451

Validation

  • bash -n test/e2e/test-openshell-gateway-upgrade.sh
  • shellcheck test/e2e/test-openshell-gateway-upgrade.sh
  • exact E2E guard grep checks for Dockerfile, dockerfile-patch.ts, onboard.ts, and OpenClaw state permissions
  • npm run source-shape:check
  • npm test -- src/lib/onboard/dockerfile-patch.test.ts
  • npm test -- test/onboard.test.ts -t "macOS VM rootfs ownership compatibility"
  • npm run build:cli

Nightly Evidence

Summary by CodeRabbit

  • Tests
    • Improved macOS VM compatibility regression testing to validate that the compatibility build argument is injected in sanitized form, ensuring the environment argument is applied correctly during image preparation.

Review Change Stack

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

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: 58002e57-baa1-42ac-b45b-143323a87944

📥 Commits

Reviewing files that changed from the base of the PR and between eab0df0 and 2a2647f.

📒 Files selected for processing (1)
  • src/lib/onboard.ts
💤 Files with no reviewable changes (1)
  • src/lib/onboard.ts

📝 Walkthrough

Walkthrough

The E2E regression test now validates the Dockerfile patch helper (src/lib/onboard/dockerfile-patch.ts) injects the sanitized ARG NEMOCLAW_DARWIN_VM_COMPAT=${sanitizeDockerArg(darwinVmCompat ? "1" : "0")}; a separate import deduplication was applied in src/lib/onboard.ts.

Changes

Darwin VM Compatibility Regression Check

Layer / File(s) Summary
Dockerfile patch compatibility validation
test/e2e/test-openshell-gateway-upgrade.sh, src/lib/onboard/dockerfile-patch.ts
Regression assertion refocused from src/lib/onboard.ts to src/lib/onboard/dockerfile-patch.ts, verifying the sanitized ARG NEMOCLAW_DARWIN_VM_COMPAT=${sanitizeDockerArg(darwinVmCompat ? "1" : "0")} injection pattern.
Onboard import de-duplication
src/lib/onboard.ts
Removed/relocated duplicate verifyWebSearchInsideSandboxWithDeps import from ./onboard/web-search-verify, adjusting import ordering without changing exported APIs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#3383: Modifies the same macOS e2e script to validate Dockerfile compatibility ARG injection versus earlier onboarding logic.
  • NVIDIA/NemoClaw#3305: Related refactor of web-search verification and import placement that overlaps with the import change in src/lib/onboard.ts.

Suggested labels

enhancement: testing, E2E, OpenShell

Suggested reviewers

  • jyaunches
  • prekshivyas

Poem

🐰 A test finds its new home with care,
Sanitized args now patched right there,
Dockerfile helper hums in tune,
Imports tidied up by noon,
Darwin VMs hop into flight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: fixing a duplicate import in onboard.ts and updating the gateway upgrade E2E check to reference the relocated dockerfile-patch helper.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 fix/openshell-gateway-upgrade-e2e-check

Comment @coderabbitai help to get the list of available commands and usage tips.

@ericksoa ericksoa added bug Something fails against expected or documented behavior CI/CD labels May 13, 2026
@github-actions

github-actions Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: openshell-gateway-upgrade-e2e
Optional E2E: double-onboard-e2e, onboard-repair-e2e, onboard-resume-e2e, brave-search-e2e, macos-e2e

Dispatch hint: openshell-gateway-upgrade-e2e

Workflow run

Full advisor summary

Pi Semantic E2E Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • openshell-gateway-upgrade-e2e: The PR directly edits test/e2e/test-openshell-gateway-upgrade.sh (the macOS VM rootfs regression assertion). Re-run the modified script to confirm the new grep target in dockerfile-patch.ts still matches and the rest of the upgrade flow remains green.

Optional E2E

  • double-onboard-e2e: onboard.ts top-level requires were edited (duplicate import removed). A second onboard pass is the cheapest way to catch any module-load regression introduced by the require cleanup.
  • onboard-repair-e2e: Touches onboard.ts module bootstrap; repair flow exercises onboard re-entry paths and would surface require/destructure mistakes early.
  • onboard-resume-e2e: Sibling onboard flow; cheap confidence check that the deduplicated import did not break onboard wizard initialization on resume.
  • brave-search-e2e: The duplicate import removed in onboard.ts is verifyWebSearchInsideSandboxWithDeps from ./onboard/web-search-verify. Brave-search E2E exercises the web-search verification path that consumes this helper.
  • macos-e2e: The updated regression assertion targets darwin VM rootfs compatibility (NEMOCLAW_DARWIN_VM_COMPAT). macos-e2e runs test-full-e2e.sh on macos-26 and is the only job that actually drives darwin sandbox builds end-to-end.

New E2E recommendations

  • None.

Dispatch hint

  • Workflow: .github/workflows/nightly-e2e.yaml
  • jobs input: openshell-gateway-upgrade-e2e

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 25780226018
Branch: fix/openshell-gateway-upgrade-e2e-check
Requested jobs: openshell-gateway-upgrade-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job Result
openshell-gateway-upgrade-e2e ✅ success

ericksoa added 2 commits May 12, 2026 22:48
…-upgrade-e2e-check

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@ericksoa ericksoa changed the title test: update OpenShell gateway upgrade source check fix: repair onboard duplicate import and gateway upgrade check May 13, 2026
@ericksoa ericksoa merged commit d12cce5 into main May 13, 2026
26 of 27 checks passed
@ericksoa ericksoa deleted the fix/openshell-gateway-upgrade-e2e-check branch May 13, 2026 05:55
@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions bug-fix PR fixes a bug or regression chore Build, CI, dependency, or tooling maintenance and removed CI/CD bug Something fails against expected or documented behavior chore Build, CI, dependency, or tooling maintenance labels Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants