refactor(e2e): simplify Brev E2E to use bare CPU instances - #1205
Conversation
Changes: - Update Brev CLI from v0.6.310 to v0.6.322 - Replace broken --cpu flag with 'brev search cpu | brev create' pattern - Remove launchable startup script in favor of direct brev-setup.sh - Simplify secret passing to use plain env vars (ephemeral CI VM) - Add brev_token input for manual token override - Increase SSH wait timeout from 5 min to 7.5 min The --cpu flag in Brev CLI v0.6.310 was silently ignored, always creating GPU instances. The new approach uses 'brev search cpu' to find CPU-only instance types and pipes them to 'brev create'. The launchable startup script from OpenShell-Community was unreliable, causing 40-minute timeouts. Direct SSH-based setup is more reliable and easier to debug.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRemoves launchable VM provisioning from the e2e workflow, adds an optional Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Workflow
participant Brev as Brev API/CLI
participant Instance as Provisioned VM
participant SSH as SSH Runner
participant Tests as E2E Test Runner
CI->>Brev: call `brev search cpu --min-vcpu/--min-ram ... | brev create --detached` (use `brev_token` or secret)
Brev-->>CI: returns instance metadata/IP
CI->>Instance: wait for SSH (up to 90 attempts)
CI->>SSH: connect (sshEnv: export env vars then run remote bootstrap) --streaming
SSH->>Instance: run `brev-setup.sh` and install nemoclaw CLI, write sandbox registry
SSH-->>CI: stream logs/stdout (streaming mode)
CI->>SSH: run remote E2E tests (streaming)
SSH-->>Tests: execute tests on Instance
Tests-->>CI: test results/exit status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 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 `@test/e2e/brev-e2e.test.js`:
- Around line 61-71: The sshEnv helper embeds env values into a single-quoted
shell string which breaks if NVIDIA_API_KEY or GITHUB_TOKEN contain single
quotes; update sshEnv to sanitize/escape those values before building envPrefix
(e.g., replace each single quote in process.env.NVIDIA_API_KEY and
process.env.GITHUB_TOKEN with the shell-safe sequence '\'' or alternatively
base64-encode the values and decode in the remote command) so that the envPrefix
construction in sshEnv never produces an unbalanced/invalid quoted string.
🪄 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
Run ID: 7169957e-68c0-42ed-a64f-dc9e796056c2
📒 Files selected for processing (2)
.github/workflows/e2e-brev.yamltest/e2e/brev-e2e.test.js
Addresses CodeRabbit review feedback: if NVIDIA_API_KEY or GITHUB_TOKEN contain single quotes, the shell string would break. Add shellEscape() helper to properly escape single quotes with the '\'' sequence.
## Summary Simplifies the Brev E2E test infrastructure by using bare CPU instances instead of the launchable startup script. ## Changes - **Update Brev CLI** from v0.6.310 to v0.6.322 - **Fix CPU instance creation**: Replace broken `--cpu` flag with `brev search cpu | brev create` pattern - **Remove launchable startup script**: Use direct `brev-setup.sh` instead of OpenShell-Community launch script - **Simplify secret passing**: Use plain env vars instead of stdin piping (ephemeral CI VM) - **Add `brev_token` input**: Allow manual token override in workflow dispatch - **Increase SSH timeout**: 5 min → 7.5 min for slower cloud providers ## Rationale ### CPU Instance Fix The `--cpu` flag in Brev CLI v0.6.310 was silently ignored, always creating GPU instances (expensive). The new approach uses `brev search cpu` to find CPU-only instance types and pipes them to `brev create`. ### Launchable Removal The startup script from OpenShell-Community was unreliable, causing 40-minute timeouts waiting for log files that were never created. Direct SSH-based setup via `brev-setup.sh` is more reliable and easier to debug. ## Files Changed | File | Changes | |------|---------| | `.github/workflows/e2e-brev.yaml` | Update CLI, remove launchable inputs | | `test/e2e/brev-e2e.test.js` | Simplify instance creation and secret handling | <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated Brev CLI version and added an optional brev_token workflow input for flexible token handling. * Removed legacy workflow input to simplify CI configuration. * **Tests** * Simplified instance provisioning to a single bare-instance flow and increased SSH retry allowance. * Streamlined remote SSH credential handling and made long-running bootstrap/test execution use streaming. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Simplifies the Brev E2E test infrastructure by using bare CPU instances instead of the launchable startup script. ## Changes - **Update Brev CLI** from v0.6.310 to v0.6.322 - **Fix CPU instance creation**: Replace broken `--cpu` flag with `brev search cpu | brev create` pattern - **Remove launchable startup script**: Use direct `brev-setup.sh` instead of OpenShell-Community launch script - **Simplify secret passing**: Use plain env vars instead of stdin piping (ephemeral CI VM) - **Add `brev_token` input**: Allow manual token override in workflow dispatch - **Increase SSH timeout**: 5 min → 7.5 min for slower cloud providers ## Rationale ### CPU Instance Fix The `--cpu` flag in Brev CLI v0.6.310 was silently ignored, always creating GPU instances (expensive). The new approach uses `brev search cpu` to find CPU-only instance types and pipes them to `brev create`. ### Launchable Removal The startup script from OpenShell-Community was unreliable, causing 40-minute timeouts waiting for log files that were never created. Direct SSH-based setup via `brev-setup.sh` is more reliable and easier to debug. ## Files Changed | File | Changes | |------|---------| | `.github/workflows/e2e-brev.yaml` | Update CLI, remove launchable inputs | | `test/e2e/brev-e2e.test.js` | Simplify instance creation and secret handling | <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated Brev CLI version and added an optional brev_token workflow input for flexible token handling. * Removed legacy workflow input to simplify CI configuration. * **Tests** * Simplified instance provisioning to a single bare-instance flow and increased SSH retry allowance. * Streamlined remote SSH credential handling and made long-running bootstrap/test execution use streaming. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Simplifies the Brev E2E test infrastructure by using bare CPU instances instead of the launchable startup script. ## Changes - **Update Brev CLI** from v0.6.310 to v0.6.322 - **Fix CPU instance creation**: Replace broken `--cpu` flag with `brev search cpu | brev create` pattern - **Remove launchable startup script**: Use direct `brev-setup.sh` instead of OpenShell-Community launch script - **Simplify secret passing**: Use plain env vars instead of stdin piping (ephemeral CI VM) - **Add `brev_token` input**: Allow manual token override in workflow dispatch - **Increase SSH timeout**: 5 min → 7.5 min for slower cloud providers ## Rationale ### CPU Instance Fix The `--cpu` flag in Brev CLI v0.6.310 was silently ignored, always creating GPU instances (expensive). The new approach uses `brev search cpu` to find CPU-only instance types and pipes them to `brev create`. ### Launchable Removal The startup script from OpenShell-Community was unreliable, causing 40-minute timeouts waiting for log files that were never created. Direct SSH-based setup via `brev-setup.sh` is more reliable and easier to debug. ## Files Changed | File | Changes | |------|---------| | `.github/workflows/e2e-brev.yaml` | Update CLI, remove launchable inputs | | `test/e2e/brev-e2e.test.js` | Simplify instance creation and secret handling | <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated Brev CLI version and added an optional brev_token workflow input for flexible token handling. * Removed legacy workflow input to simplify CI configuration. * **Tests** * Simplified instance provisioning to a single bare-instance flow and increased SSH retry allowance. * Streamlined remote SSH credential handling and made long-running bootstrap/test execution use streaming. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Simplifies the Brev E2E test infrastructure by using bare CPU instances instead of the launchable startup script.
Changes
--cpuflag withbrev search cpu | brev createpatternbrev-setup.shinstead of OpenShell-Community launch scriptbrev_tokeninput: Allow manual token override in workflow dispatchRationale
CPU Instance Fix
The
--cpuflag in Brev CLI v0.6.310 was silently ignored, always creating GPU instances (expensive). The new approach usesbrev search cputo find CPU-only instance types and pipes them tobrev create.Launchable Removal
The startup script from OpenShell-Community was unreliable, causing 40-minute timeouts waiting for log files that were never created. Direct SSH-based setup via
brev-setup.shis more reliable and easier to debug.Files Changed
.github/workflows/e2e-brev.yamltest/e2e/brev-e2e.test.jsSummary by CodeRabbit
Chores
Tests