feat(ci): add macOS e2e testing workflow - #2030
Conversation
Add a GitHub Actions workflow that runs the full vitest suite on the macos-26 (Apple Silicon) runner image. Mirrors the structure of the WSL E2E workflow added in #1715. The workflow: - Triggers on PRs that touch code paths, pushes to main, and manual dispatch - Installs Node.js 22, builds the CLI and plugin, then runs vitest - Optionally detects Docker (Colima / Docker Desktop) and runs the full E2E suite when available - Uses a 30-minute timeout and 60s per-test timeout for CI variability - Includes src/** in path triggers since CLI tests import from dist/ The macOS (Apple Silicon) platform is P0 in the platform matrix and already marked ci_tested: true. This workflow provides the CI gate to back that claim. Signed-off-by: Brandon Pelfrey <bpelfrey@nvidia.com>
|
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:
📝 WalkthroughWalkthroughAdds a new GitHub Actions workflow Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Runner as macOS Runner
participant Node as Node.js / npm
participant Test as Vitest
participant Docker as Docker Engine
participant E2E as Full E2E Script
GH->>Runner: Trigger workflow (dispatch / PR / push)
Runner->>Runner: checkout repository
Runner->>Node: setup Node 22 (npm cache)\nnpm ci --ignore-scripts\nbuild steps
Runner->>Test: npx vitest run --testTimeout 60000
Runner->>Docker: docker info
alt Docker available
Runner->>E2E: run test/e2e/test-full-e2e.sh (env/secrets)
E2E->>Docker: use containers
E2E->>Runner: return results
else Docker unavailable
Runner->>Runner: skip full E2E (print message)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/macos-e2e.yaml (1)
40-43: Pin actions to full commit SHAs instead of mutable version tags.
@v6tags can be mutated and retagged; pinning to full commit SHAs (e.g.,actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6) prevents unexpected behavior changes and strengthens supply-chain integrity. Update lines 40 and 43 to match the pattern already used in other workflows (e.g.,.github/workflows/pr.yaml).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/macos-e2e.yaml around lines 40 - 43, Replace the mutable tags "uses: actions/checkout@v6" and "uses: actions/setup-node@v6" with the corresponding full commit SHAs (e.g., "actions/checkout@<full-sha> # v6" and "actions/setup-node@<full-sha> # v6"); locate the two lines containing the exact strings uses: actions/checkout@v6 and uses: actions/setup-node@v6 and update them to the pinned commit SHAs used elsewhere in the repo while preserving the human-readable tag comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/macos-e2e.yaml:
- Around line 9-17: The workflow's PR path filter omits lockfiles so PRs that
only change dependencies (e.g., package-lock.json, yarn.lock, pnpm-lock.yaml)
won't trigger the macos-e2e job; update the "paths" list in
.github/workflows/macos-e2e.yaml to include common lockfile names
(package-lock.json, yarn.lock, pnpm-lock.yaml and any repo-specific lockfiles)
so dependency-only changes trigger the workflow; ensure the new entries follow
the existing path list format and don't change other keys.
- Around line 57-58: Replace the `npm install --ignore-scripts` steps with `npm
ci --ignore-scripts` in the workflow to ensure deterministic installs; update
both steps titled "Install root dependencies" and "Install nemoclaw plugin
dependencies" so they use `npm ci --ignore-scripts` instead of `npm install
--ignore-scripts`.
---
Nitpick comments:
In @.github/workflows/macos-e2e.yaml:
- Around line 40-43: Replace the mutable tags "uses: actions/checkout@v6" and
"uses: actions/setup-node@v6" with the corresponding full commit SHAs (e.g.,
"actions/checkout@<full-sha> # v6" and "actions/setup-node@<full-sha> # v6");
locate the two lines containing the exact strings uses: actions/checkout@v6 and
uses: actions/setup-node@v6 and update them to the pinned commit SHAs used
elsewhere in the repo while preserving the human-readable tag comment.
🪄 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: Pro Plus
Run ID: 3965df19-c0b5-4fe2-b8f0-3fe88f595cad
📒 Files selected for processing (1)
.github/workflows/macos-e2e.yaml
Move NEMOCLAW_NON_INTERACTIVE, NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE, NEMOCLAW_RECREATE_SANDBOX, and NEMOCLAW_SANDBOX_NAME from job-level env to the full E2E step. These vars leak into test subprocesses via `...process.env` spreads in spawnSync calls, causing 43 test failures: - onboard-selection.test.ts (31 tests): NEMOCLAW_NON_INTERACTIVE=1 forces non-interactive mode which requires NVIDIA_API_KEY - install-preflight.test.ts (1 test): NEMOCLAW_SANDBOX_NAME=e2e-macos replaces the expected default sandbox name in installer output The WSL workflow avoids this because its vitest step runs inside a fresh WSL bash session that does not inherit PowerShell-level env vars. Signed-off-by: Brandon Pelfrey <bpelfrey@nvidia.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
ericksoa
left a comment
There was a problem hiding this comment.
Nice work extending cross-platform CI coverage to macOS — the workflow is clean and well-structured. A few questions and suggestions below to tighten things up before merging.
- Pin actions/checkout and actions/setup-node to full commit SHAs matching the convention in pr.yaml (supply-chain hardening) - Use npm ci instead of npm install for deterministic lockfile-only installs (both root and nemoclaw plugin) - Add package-lock.json and nemoclaw/package-lock.json to PR path filters so lockfile-only dependency changes trigger the workflow - Add paths-ignore to push trigger mirroring main.yaml to skip docs-only merges (macOS runner minutes are more expensive) - Add failure artifact upload step mirroring the WSL workflow pattern for debugging flaky runs on runners we cannot SSH into Signed-off-by: Brandon Pelfrey <bpelfrey@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/macos-e2e.yaml:
- Around line 105-107: The workflow uses a mutable tag for the upload artifact
action (uses: actions/upload-artifact@v4); replace that mutable tag with the
pinned commit SHA ea165f8d65b6e75b540449e92b4886f43607fa02 so the step that
uploads logs on failure uses an immutable reference; update the uses line to
reference the full SHA and commit the change so the "Upload logs on failure"
step is supply-chain pinned.
🪄 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: Pro Plus
Run ID: c164e932-6580-4d69-b06f-0b011169e341
📒 Files selected for processing (1)
.github/workflows/macos-e2e.yaml
Pin actions/upload-artifact to ea165f8d (v4.6.2) so no action in the macOS E2E workflow references a mutable tag. Signed-off-by: Brandon Pelfrey <bpelfrey@nvidia.com>
ericksoa
left a comment
There was a problem hiding this comment.
All review feedback addressed — SHA-pinned actions, npm ci for both installs, lockfile path filters, paths-ignore on push trigger, and artifact upload on failure. CI green. LGTM.
Summary
Add a GitHub Actions workflow that validates the NemoClaw build and vitest suite on the
macos-26(Apple Silicon) runner image, mirroring the WSL E2E pattern from #1715.Related Issue
Closes #1715 follow-up — extends cross-platform CI coverage to macOS.
Changes
.github/workflows/macos-e2e.yamlwith:macos-26runner targeting Apple Siliconbuild:cliandnemoclaw/npm run build)vitest runwith 60 s per-test timeout as the primary gatesrc/**in path triggers (CLI tests import fromdist/compiled fromsrc/)Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)AI Disclosure
Signed-off-by: Brandon Pelfrey bpelfrey@nvidia.com
Summary by CodeRabbit