fix(ci): unblock nightly startup after Brev reusable workflow merge - #3376
fix(ci): unblock nightly startup after Brev reusable workflow merge#3376jyaunches wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe nightly E2E workflow's ChangesNightly E2E Workflow
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 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)
⚔️ Resolve merge conflicts
Comment |
Nightly E2E started failing at workflow startup after NVIDIA#3350 merged: the runs have conclusion=startup_failure and zero jobs/logs, which means GitHub could not instantiate the workflow graph rather than tests failing. The only workflow-graph change from NVIDIA#3350 was the new brev-e2e reusable workflow call. Make that call more conservative: - pass branch: main again for nightly-e2e. Scheduled nightly runs validate main; branch-specific Brev validation should use e2e-branch-validation.yaml directly. - use secrets: inherit for the same-repository reusable workflow call instead of mapping individual secrets in the caller. The callee still declares the required BREV_API_KEY, BREV_ORG_ID, and NVIDIA_API_KEY secrets. This keeps the Brev job wired into nightly while removing the likely startup-time expression/secret mapping issue that prevented any nightly jobs from being created.
986bd52 to
924a61f
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/nightly-e2e.yaml (1)
1800-1813:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMerge duplicate
ifconditions inbrev-e2ejob to fix workflow parse error.Line 1813 introduces a second
ifkey that duplicates the condition at line 1800, which blocks workflow instantiation. Linting tools (actionlint,yamllint) reportkey "if" is duplicated in mapping.Merge the two conditions by prepending the disabling logic to the existing condition:
Suggested fix
brev-e2e: if: >- + false && github.repository == 'NVIDIA/NemoClaw' && (github.event_name != 'workflow_dispatch' || inputs.jobs == '' || contains(format(',{0},', inputs.jobs), ',brev-e2e,')) strategy: fail-fast: false matrix: test_suite: [all, messaging-providers, full] # Re-enable once BREV_API_KEY/BREV_ORG_ID are installed in the upstream # NVIDIA/NemoClaw repo. Referencing a reusable workflow that requires # missing secrets causes GitHub to fail the entire workflow at startup # (zero jobs/logs), which is what broke nightly after `#3350` merged. - if: false🤖 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/nightly-e2e.yaml around lines 1800 - 1813, The job-level duplicate "if" in the brev-e2e job causes a YAML parse error; remove the second "if: false" and merge its disabling logic into the existing "if" condition for brev-e2e by combining them into a single boolean expression (i.e., incorporate the false-disabling clause with an AND into the original complex condition). Edit the brev-e2e job's existing "if" key so it contains the full, merged condition and delete the redundant "if: false" entry to resolve the duplicated key error.
🤖 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.
Outside diff comments:
In @.github/workflows/nightly-e2e.yaml:
- Around line 1800-1813: The job-level duplicate "if" in the brev-e2e job causes
a YAML parse error; remove the second "if: false" and merge its disabling logic
into the existing "if" condition for brev-e2e by combining them into a single
boolean expression (i.e., incorporate the false-disabling clause with an AND
into the original complex condition). Edit the brev-e2e job's existing "if" key
so it contains the full, merged condition and delete the redundant "if: false"
entry to resolve the duplicated key error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a0ceb970-a47a-4a32-a424-0537837939f6
📒 Files selected for processing (1)
.github/workflows/nightly-e2e.yaml
ericksoa
left a comment
There was a problem hiding this comment.
I think this PR should be closed as superseded rather than repaired.
Current main no longer has the brev-e2e block this PR is trying to patch; #3373 already reverted the #3350 nightly wiring that caused the startup failure. When I tested the branch against current main, the conflict was exactly that obsolete block: this branch wants to keep/disable brev-e2e, while main has removed it.
There is also still a real issue on this head: the proposed nightly-e2e.yaml has duplicate job-level if keys in the brev-e2e job, which risks workflow parse/startup failure if merged. Since the underlying broken job is already gone from main, I would not spend time fixing this branch. Please close this PR as superseded by #3373/current main.
|
Closing as superseded by #3373/current main. The broken brev-e2e wiring this PR disables has already been removed from nightly-e2e.yaml, and this branch now conflicts with current main on that obsolete block. |
Summary
Nightly E2E started failing with
startup_failureafter #3350 merged. These runs have zero jobs and no logs, which means GitHub failed to instantiate the workflow graph before any test job could start.Root cause: #3350 added a new
brev-e2ereusable workflow call whose callee declares requiredBREV_API_KEYandBREV_ORG_IDsecrets. Those secrets were validated in the fork, but they are not yet installed in the upstreamNVIDIA/NemoClawrepo (only legacyBREV_API_TOKENandNVIDIA_API_KEYare present there). A reusable workflow with missing required secrets fails during workflow startup, which aborts the entire nightly workflow graph.This PR restores nightly startup immediately by disabling the
brev-e2ejob until the upstream secrets are installed. It also keeps the caller conservative:if: falseonbrev-e2euntil upstreamBREV_API_KEY/BREV_ORG_IDexist.branch: mainfor nightly runs. Branch-specific Brev validation should usee2e-branch-validation.yamldirectly.secrets: inheritfor the same-repo reusable workflow call once it is re-enabled.Validation
.github/workflows/nightly-e2e.yaml.BREV_API_KEYandBREV_ORG_ID; fork has them.Follow-up
After adding upstream repo/org secrets:
BREV_API_KEYBREV_ORG_IDremove the temporary
if: falseand re-enable the nightlybrev-e2ematrix.Related
Summary by CodeRabbit
Note: Internal infrastructure updates only; no user-facing changes.