Skip to content

fix(ci): write Brev credentials file so CLI authenticates on runner#1639

Open
cjagwani wants to merge 3 commits intomainfrom
fix/brev-e2e-auth-credentials
Open

fix(ci): write Brev credentials file so CLI authenticates on runner#1639
cjagwani wants to merge 3 commits intomainfrom
fix/brev-e2e-auth-credentials

Conversation

@cjagwani
Copy link
Copy Markdown
Contributor

@cjagwani cjagwani commented Apr 8, 2026

Summary

Root Cause

PR #1470 replaced brev("login", "--token", process.env.BREV_API_TOKEN) with a brev("ls") pre-check (hasAuthenticatedBrev), but nothing writes the credentials file on the ephemeral GH Actions runner. Result: brev ls fails silently, hasAuthenticatedBrev returns false, describe.runIf() skips the entire suite, and CI reports success with 0 tests run.

Evidence

Test plan

  • Merge to main, trigger e2e-brev workflow with TEST_SUITE=full — confirm tests actually run (not skip)

Fixes #1638

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Improved CI end-to-end workflow: requires and validates an API token before running, securely writes and protects credential data, and verifies authentication to ensure reliable automated test runs.

The `brev login --token` call was removed in 374a847 (#1470),
replaced with a `brev ls` pre-check that assumes the CLI is already
authenticated. But on ephemeral GH Actions runners there is no
~/.brev/credentials.json, so `brev ls` fails and the entire E2E
suite silently skips (0 tests run, reports success).

Write the credentials file during the "Install Brev CLI" step so
the `hasAuthenticatedBrev` check in brev-e2e.test.js passes.

Fixes #1638

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 8, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2691f9d2-29ed-4455-acc5-a1539ed65cca

📥 Commits

Reviewing files that changed from the base of the PR and between 7af8b06 and 7079b27.

📒 Files selected for processing (1)
  • .github/workflows/e2e-brev.yaml

📝 Walkthrough

Walkthrough

The workflow now injects BREV_API_TOKEN into the "Install Brev CLI" step, validates it, writes ~/.brev/credentials.json with {"refresh_token":"$BREV_API_TOKEN"} (secure perms), and runs brev ls to confirm authentication for the CI test harness.

Changes

Cohort / File(s) Summary
GitHub Actions Workflow
.github/workflows/e2e-brev.yaml
Pass BREV_API_TOKEN into the step, validate it's non-empty, create ~/.brev/credentials.json with the refresh token (set restrictive permissions), and run brev ls to verify authentication.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant GH as GH Actions runner
participant Step as "Install Brev CLI" step
participant FS as Filesystem (~/.brev/credentials.json)
participant Brev as Brev CLI
participant Tests as Vitest/CI tests

GH->>Step: start step (env BREV_API_TOKEN)
Step->>Step: validate BREV_API_TOKEN non-empty
Step->>FS: write credentials.json {"refresh_token": token}
Step->>FS: set umask/chmod 600
Step->>Brev: run brev ls
Brev-->>Step: authentication status
Step->>Tests: proceed if authenticated

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰
A token tucked in a tiny file,
I hopped it in with a careful smile,
Now brev can speak and tests can run,
CI sings softly — hop, job done! 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately reflects the main change: writing the Brev credentials file in CI to enable CLI authentication on runners.
Linked Issues check ✅ Passed The PR directly addresses all objectives from issue #1638: creates ~/.brev/credentials.json, validates BREV_API_TOKEN, enables brev ls to succeed, and restores full E2E test execution.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing CI authentication: adding BREV_API_TOKEN handling, credentials file creation, and validation in the GitHub Actions workflow step.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/brev-e2e-auth-credentials

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/e2e-brev.yaml:
- Around line 155-160: The workflow currently writes ~/.brev/credentials.json
from $BREV_API_TOKEN without checking it's present and doesn't verify
authentication, so CI can silently skip tests; update the steps that create the
credentials (the mkdir + printf block) to first fail-fast if $BREV_API_TOKEN is
empty or invalid, write the credentials file with restricted permissions (e.g.,
chmod 600) and immediately run a quick verification command (brev ls) and exit
non-zero on failure so the pipeline fails fast instead of allowing the test
harness's hasAuthenticatedBrev to swallow auth errors.
🪄 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: c8ab42b2-e63d-42ad-b9d8-06d54c156b75

📥 Commits

Reviewing files that changed from the base of the PR and between 8f6995c and 7af8b06.

📒 Files selected for processing (1)
  • .github/workflows/e2e-brev.yaml

@cjagwani cjagwani self-assigned this Apr 8, 2026
@cjagwani cjagwani requested review from brandonpelfrey and cv April 8, 2026 22:25
Address CodeRabbit review: guard against empty BREV_API_TOKEN,
restrict credentials file to 600, and verify auth with `brev ls`
immediately so the pipeline fails fast instead of silently skipping.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@wscurran wscurran added CI/CD Use this label to identify issues with NemoClaw CI/CD pipeline or GitHub Actions. fix github_actions Pull requests that update GitHub Actions code labels Apr 8, 2026
Copy link
Copy Markdown
Contributor

@cv cv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — security review WARNING (non-blocking).

Minor suggestion: The printf '{"refresh_token":"%s"}' could malform JSON if the token ever contains quotes/backslashes. Since Node.js is already set up on the runner, consider:

node -e "process.stdout.write(JSON.stringify({refresh_token: process.env.BREV_API_TOKEN}))" > ~/.brev/credentials.json

Otherwise clean:

  • File permissions correct (umask 077 + chmod 600)
  • Secret never echoed to logs (masked by GHA, written to file only)
  • Fail-fast guard on empty token
  • brev ls >/dev/null verification step
  • CI-only, no production impact
  • All checks green

No blocking concerns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI/CD Use this label to identify issues with NemoClaw CI/CD pipeline or GitHub Actions. fix github_actions Pull requests that update GitHub Actions code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(ci): Brev E2E suite silently skips — brev login removed in #1470

3 participants