Revert "ci(nightly): enable brev-e2e job with long-lived BREV_API_TOKEN (#3350)" by ericksoa · Pull Request #3373 · NVIDIA/NemoClaw

ericksoa · 2026-05-12T00:23:21Z

Summary

Reverts #3350 because the new brev-e2e reusable workflow call breaks nightly-e2e at startup on main.

GitHub rejects nightly-e2e.yaml before jobs are created because the caller only grants contents: read, while the called workflow requests checks: write and pull-requests: write.

Failing runs:

Verification

git diff --check origin/main..HEAD
Confirmed nightly-e2e.yaml no longer contains the new brev-e2e job wiring.

Summary by CodeRabbit

Release Notes

Chores
- Updated E2E testing authentication and provisioning mechanisms in CI workflows
- Removed brev-e2e from nightly test pipeline
- Simplified E2E test setup and instance creation logic

…EN (#3350)" This reverts commit 0776ea0. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

coderabbitai · 2026-05-12T00:23:32Z

📝 Walkthrough

Walkthrough

Unified Brev E2E provisioning from dual-mode (published launchable vs startup-script) to startup-script-only instances using BREV_API_TOKEN refresh-token authentication. Removed nightly brev-e2e job. Updated workflow inputs, secrets, and test helpers accordingly.

Changes

Brev E2E Provisioning Simplification

Layer / File(s)	Summary
Workflow Contract & Input/Secret Updates `.github/workflows/e2e-branch-validation.yaml`	Updated `workflow_dispatch` inputs to replace `use_published_launchable` and `launchable_id` with a single `use_launchable` boolean flag. Added optional `brev_token` input. Updated callable workflow secret interface to require `BREV_API_TOKEN` instead of `BREV_API_KEY` and `BREV_ORG_ID`.
Job Execution & Brev Authentication Flow `.github/workflows/e2e-branch-validation.yaml`	Modified checkout `ref` precedence logic. Changed Brev CLI setup to validate `BREV_API_TOKEN` presence and write `refresh_token`-based credentials file, removing the prior `brev login --api-key` flow. Updated E2E test runner environment to pass `BREV_API_TOKEN` and derived `USE_LAUNCHABLE` flag, removing published-launchable selection env vars.
Nightly Job Removal `.github/workflows/nightly-e2e.yaml`	Removed `brev-e2e` from `inputs.jobs` allowlist, deleted the entire `brev-e2e` job definition, and removed it from `notify-on-failure` and `report-to-pr` dependency lists.
Test Suite: Startup-Script-Only Provisioning `test/e2e/brev-e2e.test.ts`	Consolidated provisioning configuration to single `DEFAULT_SETUP_SCRIPT_PATH`. Simplified `listBrevInstances()` to directly parse `brev ls --json`. Rewrote `createBrevInstance()` to always use `brev create --startup-script` with local or HTTP-downloadable script resolution. Updated `waitForLaunchableReady()` to poll only for `LAUNCHABLE_SENTINEL` file and tail `/tmp/launch-plugin.log`. Removed `createPublishedLaunchableInstance()` and `createStartupScriptInstance()` helper functions and associated mode-selection branching.
Vitest Project Gating `vitest.config.ts`	Updated `e2e-branch-validation` project `enabled` condition to check only `BREV_API_TOKEN` presence. Removed project-level `silent: false` override.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

NVIDIA/NemoClaw#3350: Related through opposing provisioning changes in the same E2E Brev workflows and tests; this PR consolidates to startup-script-only mode with refresh-token auth while that PR moves to published-launchable mode with long-lived API keys.

Suggested labels

CI/CD, fix, E2E

Poem

🐰 No more dual-mode dances, just startup scripts and tokens!
Refresh flows replace old keys in cryptic closets.
Nightly jobs hop away, leaving cleaner code.
Sentinel files sing their readiness songs,
And Brev CLI learns to trust again. 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.56% 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 describes the main change: reverting PR `#3350` that enabled the brev-e2e job, which matches the file-level changes removing the brev-e2e job from nightly-e2e.yaml and modifying related workflows.
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 revert-3350-nightly-brev-signed

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

github-actions · 2026-05-12T00:23:49Z

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

Pi Semantic E2E Advisor

Failed: pi exited with status 1; see /home/runner/work/NemoClaw/NemoClaw/artifacts/e2e-advisor/e2e-advisor-pi-raw-output.txt

+  let setupScriptPath: string;
+  if (DEFAULT_SETUP_SCRIPT_PATH.startsWith("http")) {
+    setupScriptPath = "/tmp/brev-ci-setup.sh";
+    execSync(`curl -fsSL -o ${setupScriptPath} "${DEFAULT_SETUP_SCRIPT_PATH}"`, {


+      `brev search cpu --min-vcpu ${BREV_MIN_VCPU} --min-ram ${BREV_MIN_RAM} --min-disk ${BREV_MIN_DISK} --provider ${BREV_PROVIDER} --sort price | ` +
+        `brev create ${INSTANCE_NAME} --startup-script @${setupScriptPath} --detached`,


+      `brev search cpu --min-vcpu ${BREV_MIN_VCPU} --min-ram ${BREV_MIN_RAM} --min-disk ${BREV_MIN_DISK} --provider ${BREV_PROVIDER} --sort price | ` +
+        `brev create ${INSTANCE_NAME} --startup-script @${setupScriptPath} --detached`,


coderabbitai

Actionable comments posted: 1

🤖 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 @.github/workflows/e2e-branch-validation.yaml:
- Around line 125-126: Uncomment and restore the repository restriction on the
job by re-enabling the conditional `if: github.repository == 'NVIDIA/NemoClaw'`
(currently commented out above `runs-on: ubuntu-latest`) so the workflow only
runs in the main repository and not on forks where secrets like `BREV_API_TOKEN`
are unavailable; ensure the `if:` line is present and active before merging.

🪄 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: 81ef3d8e-4ccc-41ac-bdba-9081dd9a6c23

📥 Commits

Reviewing files that changed from the base of the PR and between a2e251d and c9f2440.

📒 Files selected for processing (4)

.github/workflows/e2e-branch-validation.yaml
.github/workflows/nightly-e2e.yaml
test/e2e/brev-e2e.test.ts
vitest.config.ts

coderabbitai · 2026-05-12T00:26:08Z

+    # if: github.repository == 'NVIDIA/NemoClaw'  # Disabled for fork testing — re-enable before merge
    runs-on: ubuntu-latest


⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Re-enable repository restriction before merging.

The repository restriction if: github.repository == 'NVIDIA/NemoClaw' is commented out with a note to "re-enable before merge". Without this guard, the job will attempt to run on forks where BREV_API_TOKEN is unavailable, causing failures. Uncomment this line before merging.

🔧 Proposed fix

- # if: github.repository == 'NVIDIA/NemoClaw' # Disabled for fork testing — re-enable before merge + if: github.repository == 'NVIDIA/NemoClaw'

📝 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: github.repository == 'NVIDIA/NemoClaw' # Disabled for fork testing — re-enable before merge

runs-on: ubuntu-latest

if: github.repository == 'NVIDIA/NemoClaw'

runs-on: ubuntu-latest

🤖 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 @.github/workflows/e2e-branch-validation.yaml around lines 125 - 126, Uncomment and restore the repository restriction on the job by re-enabling the conditional `if: github.repository == 'NVIDIA/NemoClaw'` (currently commented out above `runs-on: ubuntu-latest`) so the workflow only runs in the main repository and not on forks where secrets like `BREV_API_TOKEN` are unavailable; ensure the `if:` line is present and active before merging.

Revert "ci(nightly): enable brev-e2e job with long-lived BREV_API_TOK…

c9f2440

…EN (#3350)" This reverts commit 0776ea0. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

github-advanced-security AI found potential problems May 12, 2026

View reviewed changes

coderabbitai Bot reviewed May 12, 2026

View reviewed changes

ericksoa merged commit 0007f4d into main May 12, 2026
27 of 28 checks passed

ericksoa deleted the revert-3350-nightly-brev-signed branch May 12, 2026 00:30

ericksoa mentioned this pull request May 13, 2026

fix(ci): unblock nightly startup after Brev reusable workflow merge #3376

Closed

wscurran added the feature PR adds or expands user-visible functionality label Jun 9, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert "ci(nightly): enable brev-e2e job with long-lived BREV_API_TOKEN (#3350)"#3373

Revert "ci(nightly): enable brev-e2e job with long-lived BREV_API_TOKEN (#3350)"#3373
ericksoa merged 1 commit into
mainfrom
revert-3350-nightly-brev-signed

ericksoa commented May 12, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 12, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented May 12, 2026

Pi Semantic E2E Advisor

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot May 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		`brev search cpu --min-vcpu ${BREV_MIN_VCPU} --min-ram ${BREV_MIN_RAM} --min-disk ${BREV_MIN_DISK} --provider ${BREV_PROVIDER} --sort price \| ` +
		`brev create ${INSTANCE_NAME} --startup-script @${setupScriptPath} --detached`,

		# if: github.repository == 'NVIDIA/NemoClaw' # Disabled for fork testing — re-enable before merge
		runs-on: ubuntu-latest

Conversation

ericksoa commented May 12, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented May 12, 2026

E2E Advisor Recommendation

Pi Semantic E2E Advisor

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ericksoa commented May 12, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 12, 2026 •

edited

Loading