ci(e2e): run every workflow E2E on main - #8477
Conversation
|
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:
📝 WalkthroughWalkthroughThis PR changes E2E execution to workflow-wide main-push and empty-selector runs. It adds DGX Spark qualification configuration and changes release evidence to use one candidate-bound workflow run with successful artifacts. ChangesE2E workflow and release validation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Maintainer
participant ReleaseE2eEvidence
participant GitHubE2eWorkflow
participant WorkflowReceipt
participant EvidenceLedger
Maintainer->>ReleaseE2eEvidence: Inspect candidate activation paths
ReleaseE2eEvidence->>GitHubE2eWorkflow: Dispatch completeRun with empty selectors
GitHubE2eWorkflow-->>WorkflowReceipt: Record emptySelectors and Launchable inclusion
GitHubE2eWorkflow-->>EvidenceLedger: Publish workflow run attempts
EvidenceLedger-->>Maintainer: Validate successful coverage and exact Brev artifacts
Maintainer->>ReleaseE2eEvidence: Refresh origin/main before tagging
ReleaseE2eEvidence-->>Maintainer: Rebuild evidence if candidate SHA changed
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 71e1200 in the TypeScript / code-coverage/cliThe overall coverage in commit 71e1200 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/e2e/mcp-workflow-boundary.mts (1)
193-217: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate the complete empty-selector predicate.
The validator checks only
inputs.jobs == ''. A changed workflow condition can pass this check while omittinginputs.targets == ''. That condition can run an MCP job for an unrelated target selector.Require the complete
inputs.jobs == '' && inputs.targets == ''predicate for both jobs. Prefer one shared expected condition when possible.Proposed fix
+const EMPTY_SELECTOR = "inputs.jobs == '' && inputs.targets == ''"; + requireContains( errors, job.if, - "inputs.jobs == ''", + EMPTY_SELECTOR, "mcp-bridge must run for empty-selector dispatches", );🤖 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 `@tools/e2e/mcp-workflow-boundary.mts` around lines 193 - 217, Update the condition assertions for both MCP jobs in the validator to require the complete empty-selector predicate, including inputs.jobs == '' and inputs.targets == ''. Prefer defining one shared expected condition and reusing it in both requireContains calls while preserving the existing job-specific validation.
🤖 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
@.agents/skills/nemoclaw-maintainer-e2e/scripts/validate-full-e2e-evidence.mts:
- Line 108: Update validate-full-e2e-evidence.mts:108 so full-mode accepts
dispatch receipts and successful Exact staging Brev Launchable evidence from any
earlier attempt of the same workflow run, provided both bind to the candidate
SHA. Update .agents/skills/nemoclaw-maintainer-e2e/SKILL.md:279-289 to collect
and validate attempt-aware evidence, and
.agents/skills/nemoclaw-maintainer-cut-release-tag/SKILL.md:184-204 to document
that earlier successful attempts are valid. Add validator coverage for workflow
attempt 2 using successful receipt and Launchable job evidence from attempt 1.
In @.github/workflows/e2e.yaml:
- Around line 2565-2566: Require the trusted NVIDIA/NemoClaw main workflow ref,
refs/heads/main, in every protected-runner selector before scheduling: update
.github/workflows/e2e.yaml lines 1953, 2295, 2381-2382, 2565-2566, and
4804-4805, preserving the existing selector conditions. Update
tools/e2e/workflow-boundary.mts lines 3953-3985 to enforce the same trusted-main
requirement during selector validation.
In `@tools/e2e/llama-cpp-dgx-spark-qualification-workflow-boundary.mts`:
- Around line 253-259: Add the `"workflow_dispatch"` fragment to the
`requireFragments` contract for `LLAMA_CPP_DGX_SPARK_QUALIFICATION_JOB_ID`,
alongside the existing `"push"` event requirement, so the qualification guard
accepts manually dispatched runs.
---
Outside diff comments:
In `@tools/e2e/mcp-workflow-boundary.mts`:
- Around line 193-217: Update the condition assertions for both MCP jobs in the
validator to require the complete empty-selector predicate, including
inputs.jobs == '' and inputs.targets == ''. Prefer defining one shared expected
condition and reusing it in both requireContains calls while preserving the
existing job-specific validation.
🪄 Autofix
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: 4636c504-cb16-4f01-9ce8-2db25345bcd0
📒 Files selected for processing (35)
.agents/skills/nemoclaw-maintainer-cut-release-tag/SKILL.md.agents/skills/nemoclaw-maintainer-cut-release-tag/scripts/release-e2e-evidence.mts.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md.agents/skills/nemoclaw-maintainer-e2e/SKILL.md.agents/skills/nemoclaw-maintainer-e2e/scripts/validate-full-e2e-evidence.mts.agents/skills/nemoclaw-maintainer-evening/SKILL.md.agents/skills/nemoclaw-maintainer-policies/references/daily-flow.md.agents/skills/nemoclaw-maintainer-policies/references/release-train.md.github/workflows/e2e.yamlci/llama-cpp-dgx-spark-qualification-v1.yamlmanaged-inference/images/llama-cpp/image.yamltest/e2e/README.mdtest/e2e/docs/README.mdtest/e2e/support/e2e-operations-workflow-boundary.test.tstest/e2e/support/e2e-report-to-pr-workflow-boundary.test.tstest/e2e/support/e2e-scorecard.test.tstest/e2e/support/e2e-workflow.test.tstest/e2e/support/managed-image-protected-runtime-workflow.test.tstest/e2e/support/openshell-gateway-auth-contract-workflow-boundary.test.tstest/e2e/support/workflow-plan.test.tstest/llama-cpp-dgx-spark-qualification-plan.test.tstest/maintainer-e2e-skill.test.tstest/maintainer-skills-policy.test.tstest/release-e2e-evidence.test.tstools/e2e/cli-artifact-workflow-boundary.mtstools/e2e/hermes-gpu-startup-workflow-boundary.mtstools/e2e/llama-cpp-dgx-spark-qualification-workflow-boundary.mtstools/e2e/managed-image-multiarch-workflow-boundary.mtstools/e2e/managed-image-protected-runtime-workflow-boundary.mtstools/e2e/mcp-bridge-runtime-compatibility.mtstools/e2e/mcp-workflow-boundary.mtstools/e2e/openshell-gateway-auth-contract-workflow-boundary.mtstools/e2e/operations-workflow-boundary.mtstools/e2e/report-e2e-results.mtstools/e2e/workflow-boundary.mts
There was a problem hiding this comment.
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 @.agents/skills/nemoclaw-maintainer-policies/references/daily-flow.md:
- Line 71: Update the pre-tag confirmation policy around the candidate SHA and
accepted E2E workflow run: require the accepted run’s head_sha and all
associated evidence to match the captured candidate SHA. If a late merge changes
the candidate SHA, invalidate the prior run and repeat the complete
workflow-wide validation before proceeding.
🪄 Autofix
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: a7f8bb18-6952-476b-8fba-7298666ad688
📒 Files selected for processing (17)
.agents/skills/nemoclaw-maintainer-cut-release-tag/SKILL.md.agents/skills/nemoclaw-maintainer-cut-release-tag/scripts/release-e2e-evidence.mts.agents/skills/nemoclaw-maintainer-e2e/SKILL.md.agents/skills/nemoclaw-maintainer-e2e/scripts/validate-full-e2e-evidence.mts.agents/skills/nemoclaw-maintainer-evening/SKILL.md.agents/skills/nemoclaw-maintainer-policies/references/daily-flow.md.agents/skills/nemoclaw-maintainer-policies/references/release-train.md.github/workflows/e2e.yamltest/e2e/support/jetson-workflow-boundary.test.tstest/maintainer-e2e-skill.test.tstest/maintainer-skills-policy.test.tstest/release-e2e-evidence.test.tstools/e2e/cli-artifact-workflow-boundary.mtstools/e2e/llama-cpp-dgx-spark-qualification-workflow-boundary.mtstools/e2e/managed-image-multiarch-workflow-boundary.mtstools/e2e/managed-image-protected-runtime-workflow-boundary.mtstools/e2e/workflow-boundary.mts
🚧 Files skipped from review as they are similar to previous changes (13)
- test/e2e/support/jetson-workflow-boundary.test.ts
- tools/e2e/cli-artifact-workflow-boundary.mts
- tools/e2e/managed-image-protected-runtime-workflow-boundary.mts
- .agents/skills/nemoclaw-maintainer-policies/references/release-train.md
- test/maintainer-skills-policy.test.ts
- .agents/skills/nemoclaw-maintainer-cut-release-tag/SKILL.md
- tools/e2e/llama-cpp-dgx-spark-qualification-workflow-boundary.mts
- tools/e2e/workflow-boundary.mts
- .agents/skills/nemoclaw-maintainer-e2e/SKILL.md
- test/release-e2e-evidence.test.ts
- .agents/skills/nemoclaw-maintainer-cut-release-tag/scripts/release-e2e-evidence.mts
- .github/workflows/e2e.yaml
- tools/e2e/managed-image-multiarch-workflow-boundary.mts
There was a problem hiding this comment.
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 @.agents/skills/nemoclaw-maintainer-policies/references/daily-flow.md:
- Line 71: The daily-flow policy must bind the accepted E2E workflow run and
every execution’s evidence to the captured candidate SHA. Update the pre-tag
validation around the candidate SHA and accepted workflow run so any late drift
that advances the candidate invalidates the prior run and evidence, requiring
complete workflow-wide validation again before tag confirmation.
🪄 Autofix
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: 1d442583-40dd-462e-8e43-2e18c6c48210
📒 Files selected for processing (7)
.agents/skills/nemoclaw-maintainer-cut-release-tag/SKILL.md.agents/skills/nemoclaw-maintainer-e2e/SKILL.md.agents/skills/nemoclaw-maintainer-e2e/scripts/validate-full-e2e-evidence.mts.agents/skills/nemoclaw-maintainer-policies/references/daily-flow.md.github/workflows/e2e.yamltest/maintainer-e2e-skill.test.tstools/e2e/upload-e2e-artifacts-workflow-boundary.mts
🚧 Files skipped from review as they are similar to previous changes (5)
- test/maintainer-e2e-skill.test.ts
- .agents/skills/nemoclaw-maintainer-e2e/scripts/validate-full-e2e-evidence.mts
- .agents/skills/nemoclaw-maintainer-e2e/SKILL.md
- .github/workflows/e2e.yaml
- .agents/skills/nemoclaw-maintainer-cut-release-tag/SKILL.md
Summary
Each push to
mainnow selects every workflow E2E, including the protected hardware, Brev, MCP, and managed-image jobs that previously required selectors. Trusted manual runs retain current-PR-head authorization and release evidence now requires one complete workflow run ID.Changes
mainand on empty-selector trusted manual runs.test/llama-cpp-dgx-spark-qualification-plan.test.tsprotects the enabled contract.Type of Change
Quality Gates
6c6b0087d. Credential exposure remains confined to documented trusted-host, candidate-process, or Brev-guest boundaries; manual PR authorization and candidate/base SHA validation remain fail closed.Documentation Writer Review
docs-updatedDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Documentation