Skip to content

ci(e2e): add risk-based PR gate - #6662

Merged
cv merged 8 commits into
mainfrom
codex/simplify-required-live
Jul 11, 2026
Merged

ci(e2e): add risk-based PR gate#6662
cv merged 8 commits into
mainfrom
codex/simplify-required-live

Conversation

@cv

@cv cv commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR replaces separate advisory and post-merge control paths with one deterministic PR E2E gate. The gate selects jobs from the checked-in risk policy, verifies complete evidence for the PR head commit, and keeps model output outside merge authority; the final refactor names each workflow, run, and controller by its actual role.

Changes

  • add E2E / PR Gate with one controller for planning, dispatch, cancellation, evidence verification, and check completion
  • reject fork events before trusted check or API mutations
  • reduce the child e2e.yaml dispatch contract to the PR commit, selected jobs, plan hash, and correlation ID
  • require every policy-selected job shard to produce bound evidence and fail the check for every incomplete outcome
  • rename the workflow, controller, tests, artifacts, symbols, steps, and diagnostics without aliases or compatibility paths
  • consolidate the lifecycle documentation in test/e2e/README.md and keep advisor documentation focused on its local relationship

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification:
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: independent code and test reviews found no remaining actionable issues; security boundaries remain covered by workflow tests
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — 43 integration tests, 24 E2E-support tests, npm run typecheck:cli, and npm run test:projects:check
  • Applicable broad gate passed — npm test passed earlier in this PR with 15,850 tests; the current naming refactor was reverified with the focused suites above
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only) — passed with 0 errors and 2 pre-existing Fern warnings
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • New Features

    • Added a required PR E2E gate that builds a deterministic risk plan, dispatches only the needed E2E jobs, and blocks until all evidence signals are complete and passing.
    • Introduced controller-driven validation for manual dispatches, plus automatic cancellation of superseded/invalid PR E2E runs and updated PR reporting/scorecard gating.
  • Documentation

    • Updated E2E and release documentation to describe the PR gate flow and the new “required-live” terminology/behavior.
  • Tests

    • Expanded workflow contract, evidence/shard routing, and risk-plan determinism coverage; updated reporter and prompt-context expectations for the new signal schema.

@cv cv self-assigned this Jul 10, 2026
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This change replaces post-merge risk-shadow orchestration with a required-live PR E2E gate. It adds a controller, validates correlated child workflows and evidence, updates E2E dispatch contracts, removes shadow workflows, and revises risk-plan schemas, tests, and documentation.

Changes

Risk-plan and evidence contracts

Layer / File(s) Summary
Risk-plan and reporter contracts
tools/advisors/risk-plan.mts, tools/e2e/*, test/e2e/*, test/pr-risk-plan.test.ts
Risk plans use schema version 2 without automatic-job fields; E2E signals, private-file operations, and live metadata contracts are added or updated.
Advisor context and test support
tools/pr-review-advisor/analyze.mts, test/support/github-fetch-router.ts
Advisor contexts omit removed risk-plan fields, and GitHub fetch routing helpers support controller tests.

Required-live controller lifecycle

Layer / File(s) Summary
Controller implementation and lifecycle tests
tools/e2e/pr-e2e-gate.mts, test/pr-e2e-gate.test.ts
The controller validates plans, PR identity, dispatches correlated E2E runs, classifies evidence, manages checks, and handles finish, abandon, and cancellation paths.

Child workflow dispatch and evidence wiring

Layer / File(s) Summary
E2E dispatch contract and validation
.github/workflows/e2e.yaml, tools/e2e/operations-workflow-boundary.mts, test/e2e/support/*
Required-live inputs, checkout validation, concurrency, shard routing, reporting gates, reporter attachment, and artifact uploads are enforced and tested.
PR gate workflow contract tests
test/pr-e2e-gate-workflow.test.ts
Workflow triggers, scripts, child validation, wait behavior, timeouts, and command argument handling are tested.

Required-live workflow coordination

Layer / File(s) Summary
PR E2E gate workflow
.github/workflows/pr-e2e-gate.yaml
The workflow cancels superseded runs, starts the controller, waits for the child E2E run, downloads evidence, finalizes or abandons the check, and cleans up temporary state.

Documentation and ownership migration

Layer / File(s) Summary
Required-live documentation
docs/about/release-notes.mdx, test/e2e/README.md, test/e2e/docs/README.md, tools/e2e-advisor/README.md, tools/pr-review-advisor/README.md
Documentation now describes the PR E2E gate, deterministic required-live checks, evidence requirements, and advisor/controller ownership.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CI as CI / Pull Request
  participant Controller as PR E2E Controller
  participant GitHub as GitHub API
  participant E2E as E2E Workflow
  participant Evidence as Evidence Artifacts
  CI->>Controller: trigger after completion
  Controller->>GitHub: resolve PR and changed files
  Controller->>GitHub: dispatch required jobs
  E2E->>Evidence: upload correlated risk signals
  Controller->>GitHub: poll child run
  Controller->>Evidence: validate shard evidence
  Controller->>GitHub: complete PR E2E check
Loading

Suggested labels: area: ci, area: e2e, feature

Suggested reviewers: prekshivyas, jyaunches

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: replacing the old E2E flow with a risk-based PR gate.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/simplify-required-live

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

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: cloud-onboard
Optional E2E: full-e2e

Dispatch hint: cloud-onboard

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • cloud-onboard: Required by the deterministic risk plan floor for platform-install risk from .github/workflows/e2e.yaml. This validates that a clean hosted onboarding path still installs the intended dependencies, runs the selected live E2E job through the unified workflow, and reaches a usable agent with the updated PR-gate dispatch metadata.

Optional E2E

  • full-e2e: Optional adjacent confidence for the broader real assistant flow because the workflow/reporter changes affect shared live E2E execution, artifact upload, and selected-job signaling beyond cloud onboarding.

New E2E recommendations

  • None.

Dispatch hint

  • Workflow: e2e.yaml
  • jobs input: cloud-onboard

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: cloud-onboard, e2e-all
Optional E2E targets: None

Dispatch required E2E targets:

  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=cloud-onboard
  • gh workflow run e2e.yaml --ref <pr-head-ref>

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • cloud-onboard: Installer and platform changes must work on a clean supported host with the pinned runtime dependencies.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=cloud-onboard
  • e2e-all: The PR changes shared E2E target workflow machinery, controller dispatch metadata, risk-signal wiring, and workflow boundary support. Policy requires the full E2E target fan-out for shared workflow machinery changes.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref>

Optional E2E targets

  • None.

Relevant changed files

  • .github/workflows/e2e.yaml
  • .github/workflows/post-merge-e2e-risk-gate-shadow.yaml
  • .github/workflows/pr-e2e-gate.yaml
  • test/e2e/README.md
  • test/e2e/docs/README.md
  • test/e2e/risk-signal-reporter.ts
  • test/e2e/support/e2e-operations-workflow-boundary.test.ts
  • tools/advisors/risk-plan.mts
  • tools/e2e-advisor/post-merge-risk-gate.mts
  • tools/e2e/operations-workflow-boundary.mts
  • tools/e2e/pr-e2e-gate.mts
  • tools/e2e/private-file.ts
  • tools/e2e/risk-signal.ts

@github-code-quality

github-code-quality Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the main branch.

TypeScript / code-coverage/cli

The overall coverage in the codex/simplify-requi... branch remains at 77%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.
File main fcc121d codex/simplify-requi... 6a940a9 +/-
src/lib/inferen...del-registry.ts 100% 66% -34%
src/lib/actions...ge-preflight.ts 94% 74% -20%
src/lib/inference/local.ts 74% 69% -5%
src/lib/messagi...agent-config.ts 69% 64% -5%
src/lib/shields...nsition-lock.ts 82% 87% +5%
src/lib/actions...ateway-state.ts 72% 80% +8%
src/lib/actions...e-validation.ts 81% 90% +9%
src/lib/onboard...der-recovery.ts 80% 94% +14%
src/lib/messagi...ls/wechat/qr.ts 74% 92% +18%
src/lib/onboard...er-discovery.ts 0% 94% +94%

Updated July 10, 2026 23:54 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

Comment thread tools/e2e/pr-e2e-gate.mts Fixed
@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Large PR E2E controller concentrates trusted workflow responsibilities.
Open items: 0 required · 1 warning · 0 suggestions · 2 test follow-ups
Since last review: 1 prior item resolved · 1 still applies · 0 new items found

Action checklist

  • PRA-1 Resolve or justify: Large PR E2E controller concentrates trusted workflow responsibilities in tools/e2e/pr-e2e-gate.mts:1
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture tools/e2e/pr-e2e-gate.mts:1 Split the controller into focused modules or clearly separated helpers for CLI parsing, GitHub PR/workflow validation, dispatch/run identity, state hashing, evidence traversal/classification, check finalization, and cancellation; keep the entrypoint thin and preserve focused tests for each boundary, or add a concrete in-PR justification for why this monolith is safer here.
Review findings by urgency: 0 required fixes, 1 item to resolve/justify, 0 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Large PR E2E controller concentrates trusted workflow responsibilities

  • Location: tools/e2e/pr-e2e-gate.mts:1
  • Category: architecture
  • Problem: The PR replaces the post-merge shadow controller with a large single-file trusted PR E2E gate controller. Because this controller runs from trusted workflow code with actions/checks write permissions and coordinates PR-head E2E dispatch/evidence, concentrating many responsibilities in one module makes boundary review and targeted regression coverage harder.
  • Impact: A defect or future edit in one responsibility can compromise the correctness of the required PR E2E gate, and reviewers/test authors must reason about multiple trusted-code boundaries together instead of validating small contracts independently.
  • Recommended action: Split the controller into focused modules or clearly separated helpers for CLI parsing, GitHub PR/workflow validation, dispatch/run identity, state hashing, evidence traversal/classification, check finalization, and cancellation; keep the entrypoint thin and preserve focused tests for each boundary, or add a concrete in-PR justification for why this monolith is safer here.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read tools/e2e/pr-e2e-gate.mts and confirm whether one module still owns command parsing, PR/workflow validation, dispatch identity, cancellation, state hashing, artifact traversal, evidence classification, and check finalization instead of delegating to focused modules with narrow contracts.
  • Missing regression test: If decomposed, keep or add focused tests for each boundary: unsafe CLI inputs rejected, stale/forked PR heads rejected, non-current workflow SHA rejected before dispatch, mismatched child run identity rejected, state hash tampering rejected, ambiguous artifact traversal rejected, and missing/duplicate/skipped risk signals classified as failure.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read tools/e2e/pr-e2e-gate.mts and confirm whether one module still owns command parsing, PR/workflow validation, dispatch identity, cancellation, state hashing, artifact traversal, evidence classification, and check finalization instead of delegating to focused modules with narrow contracts.
  • Evidence: Diff stat shows tools/e2e/pr-e2e-gate.mts added with 1261 insertions while the prior post-merge controller was deleted. Symbol scan of tools/e2e/pr-e2e-gate.mts shows parseControllerCommand, validatePrGateState, validateRiskPlan, validateSignal, classifyPrGateEvidence, resolvePullRequest, pullChangedFiles, expectedSignalShards, assertCorrelatedWorkflowRun, dispatchPrGate, startPrGate, findSignalFiles, finishPrGate, abandonPrGate, cancelPrGate, and main all in the same file. Read-only inspection of tools/e2e/pr-e2e-gate.mts lines 1-22 shows one module importing crypto, fs/path/url, YAML, GitHub API helpers, argument parsing, risk-plan helpers, private-file helpers, risk-signal types, and workflow inventory helpers.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

  • None.
Simplification opportunities: 1 possible cut

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-1 shrink (tools/e2e/pr-e2e-gate.mts:1): Split the monolithic PR E2E gate controller into focused modules or clearly separated helper files for CLI parsing, GitHub PR/workflow validation, dispatch/run identity, state hashing, evidence traversal/classification, check finalization, and cancellation.
    • Replacement: A thin tools/e2e/pr-e2e-gate.mts entrypoint orchestrating imported, focused boundary helpers with existing tests mapped to those helper contracts.
    • Net: 0 lines
    • Safety boundary: Do not remove or weaken PR identity validation, fork rejection, current-main workflow revision checks, state-hash validation, child-run identity checks, artifact traversal limits, evidence classification, cancellation, or check finalization behavior.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Run the `cloud-onboard` E2E job for Installer and platform changes must work on a clean supported host with the pinned runtime dependencies. Matched files: `.github/workflows/e2e.yaml`.. Deterministic regression risks require live validation: platform-install. The deterministic risk plan is tier 3 for platform-install because .github/workflows/e2e.yaml changed. Static tests cover controller protocol, workflow boundaries, risk-plan selection, evidence validation, and many negative/error paths, but the clean-host installer/platform invariant requires live validation.
  • PRA-T2 Runtime validation — Run or justify the required `cloud-onboard` E2E job for the `.github/workflows/e2e.yaml` platform-install change to prove a clean supported host installs the intended pinned dependencies, reaches a usable agent, and does not silently downgrade required runtime validation.. Deterministic regression risks require live validation: platform-install. The deterministic risk plan is tier 3 for platform-install because .github/workflows/e2e.yaml changed. Static tests cover controller protocol, workflow boundaries, risk-plan selection, evidence validation, and many negative/error paths, but the clean-host installer/platform invariant requires live validation.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Large PR E2E controller concentrates trusted workflow responsibilities

  • Location: tools/e2e/pr-e2e-gate.mts:1
  • Category: architecture
  • Problem: The PR replaces the post-merge shadow controller with a large single-file trusted PR E2E gate controller. Because this controller runs from trusted workflow code with actions/checks write permissions and coordinates PR-head E2E dispatch/evidence, concentrating many responsibilities in one module makes boundary review and targeted regression coverage harder.
  • Impact: A defect or future edit in one responsibility can compromise the correctness of the required PR E2E gate, and reviewers/test authors must reason about multiple trusted-code boundaries together instead of validating small contracts independently.
  • Recommended action: Split the controller into focused modules or clearly separated helpers for CLI parsing, GitHub PR/workflow validation, dispatch/run identity, state hashing, evidence traversal/classification, check finalization, and cancellation; keep the entrypoint thin and preserve focused tests for each boundary, or add a concrete in-PR justification for why this monolith is safer here.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read tools/e2e/pr-e2e-gate.mts and confirm whether one module still owns command parsing, PR/workflow validation, dispatch identity, cancellation, state hashing, artifact traversal, evidence classification, and check finalization instead of delegating to focused modules with narrow contracts.
  • Missing regression test: If decomposed, keep or add focused tests for each boundary: unsafe CLI inputs rejected, stale/forked PR heads rejected, non-current workflow SHA rejected before dispatch, mismatched child run identity rejected, state hash tampering rejected, ambiguous artifact traversal rejected, and missing/duplicate/skipped risk signals classified as failure.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read tools/e2e/pr-e2e-gate.mts and confirm whether one module still owns command parsing, PR/workflow validation, dispatch identity, cancellation, state hashing, artifact traversal, evidence classification, and check finalization instead of delegating to focused modules with narrow contracts.
  • Evidence: Diff stat shows tools/e2e/pr-e2e-gate.mts added with 1261 insertions while the prior post-merge controller was deleted. Symbol scan of tools/e2e/pr-e2e-gate.mts shows parseControllerCommand, validatePrGateState, validateRiskPlan, validateSignal, classifyPrGateEvidence, resolvePullRequest, pullChangedFiles, expectedSignalShards, assertCorrelatedWorkflowRun, dispatchPrGate, startPrGate, findSignalFiles, finishPrGate, abandonPrGate, cancelPrGate, and main all in the same file. Read-only inspection of tools/e2e/pr-e2e-gate.mts lines 1-22 shows one module importing crypto, fs/path/url, YAML, GitHub API helpers, argument parsing, risk-plan helpers, private-file helpers, risk-signal types, and workflow inventory helpers.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (4)
test/e2e/README.md (1)

11-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep each sentence on one Markdown source line.

The changed prose wraps individual sentences across multiple physical lines, contrary to the documentation guideline. Reflow the affected sentences without changing their content.

As per coding guidelines, Markdown and MDX files must keep one sentence per line.

Also applies to: 58-115

🤖 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 `@test/e2e/README.md` around lines 11 - 14, The affected documentation
sentences in the E2E README, including the sections spanning the referenced
range, are wrapped across multiple Markdown source lines. Reflow each sentence
so it occupies one physical line, preserving all wording and content.

Source: Coding guidelines

tools/e2e-advisor/README.md (1)

41-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep each sentence on one Markdown source line.

The changed prose wraps individual sentences across multiple physical lines. Reflow the affected sentences to comply with the documentation guideline.

As per coding guidelines, Markdown and MDX files must keep one sentence per line.

Also applies to: 116-121, 153-154

🤖 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-advisor/README.md` around lines 41 - 91, Reflow the affected prose
in README.md so every sentence occupies a single Markdown source line, including
the sections referenced around the changed content and the additional ranges
116-121 and 153-154. Preserve the wording and paragraph structure while joining
wrapped sentence fragments onto one physical line.

Source: Coding guidelines

test/e2e/docs/README.md (1)

109-123: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep each sentence on one Markdown source line.

The changed documentation wraps individual sentences across multiple physical lines. Reflow these sentences to comply with the repository Markdown guideline.

As per coding guidelines, Markdown and MDX files must keep one sentence per line.

Also applies to: 136-145

🤖 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 `@test/e2e/docs/README.md` around lines 109 - 123, Reflow the affected
documentation in the README so each complete sentence occupies a single Markdown
source line. Update the wrapped sentences in the sections around the shown diff
and the additionally referenced section, preserving wording and paragraph
structure while ensuring no sentence spans multiple physical lines.

Source: Coding guidelines

tools/pr-review-advisor/README.md (1)

59-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep each sentence on one Markdown source line.

The changed documentation wraps individual sentences across multiple physical lines. Reflow these sentences to comply with the repository Markdown guideline.

As per coding guidelines, Markdown and MDX files must keep one sentence per line.

Also applies to: 87-88

🤖 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/pr-review-advisor/README.md` around lines 59 - 63, Reflow the affected
documentation in the README so each sentence occupies a single Markdown source
line. Update the wrapped sentences in the shown section and the corresponding
lines around the referenced additional location, preserving wording and
paragraph structure.

Source: Coding guidelines

🤖 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/required-live-e2e.yaml:
- Around line 83-95: Prevent shell injection in the “start” step by passing
github.event.workflow_run.head_branch through the step’s env as HEAD_BRANCH and
referencing it safely as "$HEAD_BRANCH" in the command. Update the corresponding
assertion in required-live-workflow.test.ts to expect the environment-based
reference.

In `@test/e2e/README.md`:
- Around line 79-80: Update the dispatch contract wording in the E2E guide to
state that selective job dispatch must be performed without the `targets` input,
replacing the ambiguous “empty target fan-out” phrasing.

In `@tools/e2e-advisor/README.md`:
- Around line 61-62: Clarify the required-live dispatch prerequisites in the
README sentence: state that it uses selective jobs with no `targets` input,
rather than describing this as an “empty target fan-out.”

---

Nitpick comments:
In `@test/e2e/docs/README.md`:
- Around line 109-123: Reflow the affected documentation in the README so each
complete sentence occupies a single Markdown source line. Update the wrapped
sentences in the sections around the shown diff and the additionally referenced
section, preserving wording and paragraph structure while ensuring no sentence
spans multiple physical lines.

In `@test/e2e/README.md`:
- Around line 11-14: The affected documentation sentences in the E2E README,
including the sections spanning the referenced range, are wrapped across
multiple Markdown source lines. Reflow each sentence so it occupies one physical
line, preserving all wording and content.

In `@tools/e2e-advisor/README.md`:
- Around line 41-91: Reflow the affected prose in README.md so every sentence
occupies a single Markdown source line, including the sections referenced around
the changed content and the additional ranges 116-121 and 153-154. Preserve the
wording and paragraph structure while joining wrapped sentence fragments onto
one physical line.

In `@tools/pr-review-advisor/README.md`:
- Around line 59-63: Reflow the affected documentation in the README so each
sentence occupies a single Markdown source line. Update the wrapped sentences in
the shown section and the corresponding lines around the referenced additional
location, preserving wording and paragraph structure.
🪄 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: 3152a77a-3a3b-4187-9490-667cde078c34

📥 Commits

Reviewing files that changed from the base of the PR and between fcc121d and 90a425a.

📒 Files selected for processing (25)
  • .github/workflows/e2e.yaml
  • .github/workflows/post-merge-e2e-risk-gate-shadow.yaml
  • .github/workflows/required-live-e2e.yaml
  • docs/about/release-notes.mdx
  • test/e2e-advisor-targets.test.ts
  • test/e2e-private-file.test.ts
  • test/e2e-risk-signal-reporter.test.ts
  • test/e2e/README.md
  • test/e2e/docs/README.md
  • test/e2e/risk-signal-reporter.ts
  • test/e2e/support/e2e-operations-workflow-boundary.test.ts
  • test/post-merge-e2e-risk-gate-workflow.test.ts
  • test/post-merge-e2e-risk-gate.test.ts
  • test/pr-risk-plan.test.ts
  • test/required-live-workflow.test.ts
  • test/required-live.test.ts
  • tools/advisors/risk-plan.mts
  • tools/e2e-advisor/README.md
  • tools/e2e-advisor/post-merge-risk-gate.mts
  • tools/e2e/operations-workflow-boundary.mts
  • tools/e2e/private-file.ts
  • tools/e2e/required-live.mts
  • tools/e2e/risk-signal.ts
  • tools/pr-review-advisor/README.md
  • tools/pr-review-advisor/analyze.mts
💤 Files with no reviewable changes (5)
  • test/post-merge-e2e-risk-gate.test.ts
  • .github/workflows/post-merge-e2e-risk-gate-shadow.yaml
  • test/post-merge-e2e-risk-gate-workflow.test.ts
  • tools/e2e-advisor/post-merge-risk-gate.mts
  • tools/pr-review-advisor/analyze.mts

Comment thread .github/workflows/required-live-e2e.yaml Outdated
Comment thread test/e2e/README.md Outdated
Comment thread tools/e2e-advisor/README.md Outdated
@cv

cv commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

Automated review follow-up

  • PRA-2 and the original PRA-3 are fixed in 466b4fe50: PR-controlled workflow metadata now crosses an env boundary and is referenced only through quoted shell variables. The workflow test executes single quotes, double quotes, command substitution syntax, semicolons, whitespace, and a newline and proves each value remains one inert argument.
  • PRA-1 is intentionally resolved by justification. tools/e2e/required-live.mts is the single trusted executable checked out from github.workflow_sha. It has one current protocol and no compatibility modes. Its main() is thin, while the trust boundaries are separate named functions with narrow inputs and focused tests: command parsing, private paths and state validation, risk-plan and signal validation, PR identity and change revalidation, workflow-revision preflight, dispatch/run identity, cancellation, bounded artifact traversal, evidence classification, and check finalization. Splitting those functions into more modules would not reduce authority or behavior; it would distribute the same trusted audit surface across imports and add navigation/abstraction overhead. Keeping the cohesive controller in one file is the more inspectable shape for this first implementation.
  • PRA-T1 and PRA-T2 are justified as not run in this PR task. The e2e.yaml change is control-plane validation and evidence binding; it does not change the cloud-onboard job's installer or platform steps. A live cloud-onboard dispatch mutates external cloud state and consumes resources, so it was not started implicitly. Current evidence includes the full non-live aggregate suite, focused lifecycle and hostile-input execution tests, E2E workflow-boundary tests, and the PR CI matrix. The first post-merge pull request will exercise the new required-live coordinator through its intended trusted-default-branch path.
  • The CodeQL file-write data flow is also constrained: controller output names and values are reduced to strict single-line allowlists, then written through a no-follow descriptor after verifying it is a regular runner-owned file.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-2: pull_request_target workflow with write permissions lacks workflow-level fork guard; then add or justify PRA-T1.
Open items: 2 required · 6 warnings · 0 suggestions · 8 test follow-ups
Since last review: 0 prior items resolved · 10 still apply · 0 new items found

Action checklist

  • PRA-2 Fix: pull_request_target workflow with write permissions lacks workflow-level fork guard in .github/workflows/pr-e2e-gate.yaml:30
  • PRA-3 Fix: Risk plan version v2 consumers (E2E Advisor, PR Review Advisor) not verified for compatibility in tools/advisors/risk-plan.mts:1
  • PRA-1 Resolve or justify: PR E2E gate controller duplicates risk plan construction instead of sharing risk-plan.mts in tools/e2e/pr-e2e-gate.mts:580
  • PRA-4 Resolve or justify: Missing env var rename completeness verification test in test/support/env-var-rename.test.ts:1
  • PRA-5 Resolve or justify: Missing workflow-level fork PR dispatch test in test/pr-e2e-gate-workflow.test.ts:1
  • PRA-6 Resolve or justify: Missing permissions audit test for write permissions without fork guard in test/support/workflow-permissions.test.ts:1
  • PRA-7 Resolve or justify: Missing cloud-onboard live test invariant verification for platform-install risk family in test/pr-risk-plan.test.ts:1
  • PRA-8 Resolve or justify: Missing risk plan v2 round-trip integration tests for advisor consumers in test/pr-risk-plan.test.ts:1
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Runtime validation
  • PRA-T6 Add or justify test follow-up: Missing env var rename completeness verification test
  • PRA-T7 Add or justify test follow-up: Missing workflow-level fork PR dispatch test
  • PRA-T8 Add or justify test follow-up: Missing permissions audit test for write permissions without fork guard

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture tools/e2e/pr-e2e-gate.mts:580 Extract shared buildAndValidateRiskPlan function in risk-plan.mts used by both controller and advisors.
PRA-2 Required architecture .github/workflows/pr-e2e-gate.yaml:30 Add workflow-level fork guard: if: github.event.pull_request.head.repo.full_name == github.repository on the coordinate job as defense-in-depth.
PRA-3 Required architecture tools/advisors/risk-plan.mts:1 Verify E2E Advisor and PR Review Advisor risk plan consumers accept version 2. Search for RISK_PLAN_VERSION imports and validateRiskPlan usage in tools/e2e-advisor/ and tools/pr-review-advisor/. Add tests confirming version 2 round-trip compatibility.
PRA-4 Resolve/justify tests test/support/env-var-rename.test.ts:1 Add test/support/env-var-rename.test.ts that greps codebase for NEMOCLAW_E2E_RISK_* and fails if found outside test fixtures.
PRA-5 Resolve/justify tests test/pr-e2e-gate-workflow.test.ts:1 Add test in test/pr-e2e-gate-workflow.test.ts that simulates a fork PR dispatch event and verifies the coordinate job evaluates to false (skipped) due to workflow-level fork guard.
PRA-6 Resolve/justify tests test/support/workflow-permissions.test.ts:1 Add test/support/workflow-permissions.test.ts that parses all .github/workflows/*.yaml and fails if any job has write permissions without an if condition checking github.event.pull_request.head.repo.full_name == github.repository.
PRA-7 Resolve/justify tests test/pr-risk-plan.test.ts:1 Add test in test/pr-risk-plan.test.ts that reads test/e2e/live/cloud-onboard.test.ts and verifies it contains assertions for both platform-install invariants.
PRA-8 Resolve/justify tests test/pr-risk-plan.test.ts:1 Add integration tests in test/pr-risk-plan.test.ts (or per-advisor test files) that build a v2 risk plan and verify each advisor consumer handles it without downgrading or rejecting.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-2 Required — pull_request_target workflow with write permissions lacks workflow-level fork guard

  • Location: .github/workflows/pr-e2e-gate.yaml:30
  • Category: architecture
  • Problem: .github/workflows/pr-e2e-gate.yaml coordinate job requests actions:write, checks:write, pull-requests:read permissions but fork rejection exists only in controller code (tools/e2e/pr-e2e-gate.mts startPrGate). No workflow-level if guard on coordinate job to prevent job start for fork PRs.
  • Impact: A malicious fork PR could trigger the workflow; if controller fork rejection is bypassed (prototype pollution, logic error, future refactor), the controller would have access to GITHUB_TOKEN with write permissions before the check executes.
  • Required action: Add workflow-level fork guard: if: github.event.pull_request.head.repo.full_name == github.repository on the coordinate job as defense-in-depth.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check .github/workflows/pr-e2e-gate.yaml coordinate job if condition; verify tools/e2e/pr-e2e-gate.mts startPrGate fork check executes before any GitHub API calls.
  • Missing regression test: Test simulating a fork PR dispatch verifying the workflow job is skipped (not just controller rejection).
  • Done when: The required change is committed and verification passes: Check .github/workflows/pr-e2e-gate.yaml coordinate job if condition; verify tools/e2e/pr-e2e-gate.mts startPrGate fork check executes before any GitHub API calls.
  • Evidence: .github/workflows/pr-e2e-gate.yaml uses pull_request_target trigger with coordinate job permissions actions:write, checks:write, pull-requests:read tools/e2e/pr-e2e-gate.mts startPrGate throws 'PR branch must be in the base repository' before fetch No if condition on coordinate job checking github.event.pull_request.head.repo.full_name

PRA-3 Required — Risk plan version v2 consumers (E2E Advisor, PR Review Advisor) not verified for compatibility

  • Location: tools/advisors/risk-plan.mts:1
  • Category: architecture
  • Problem: tools/advisors/risk-plan.mts exports RISK_PLAN_VERSION = 2 (was 1). PR E2E gate controller validates version 2. However, E2E Advisor (tools/e2e-advisor/) and PR Review Advisor (tools/pr-review-advisor/) consumption of v2 risk plans is not verified in this PR. Model advice is documented as additive and cannot downgrade the deterministic floor, requiring all consumers to accept v2.
  • Impact: If E2E Advisor or PR Review Advisor still expect version 1, they may reject valid risk plans or produce incorrect recommendations, breaking the 'additive model advice' guarantee and potentially causing silent fallback or incorrect job selection.
  • Required action: Verify E2E Advisor and PR Review Advisor risk plan consumers accept version 2. Search for RISK_PLAN_VERSION imports and validateRiskPlan usage in tools/e2e-advisor/ and tools/pr-review-advisor/. Add tests confirming version 2 round-trip compatibility.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: grep -r 'RISK_PLAN_VERSION\|validateRiskPlan\|buildRiskPlan' tools/e2e-advisor/ tools/pr-review-advisor/ --include='*.mts' --include='*.ts'
  • Missing regression test: Integration tests in each advisor that build a risk plan with version 2 and verify the consumer handles it without downgrading or rejecting.
  • Done when: The required change is committed and verification passes: grep -r 'RISK_PLAN_VERSION\|validateRiskPlan\|buildRiskPlan' tools/e2e-advisor/ tools/pr-review-advisor/ --include='*.mts' --include='*.ts'.
  • Evidence: tools/advisors/risk-plan.mts:1 exports RISK_PLAN_VERSION = 2 tools/e2e/pr-e2e-gate.mts:32 imports and validates version in validateRiskPlan test/e2e-advisor-targets.test.ts:53 expects '"version":2' in context tool result PR description: 'model advice is additive and cannot downgrade the deterministic floor'
Review findings by urgency: 2 required fixes, 6 items to resolve/justify, 0 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — PR E2E gate controller duplicates risk plan construction instead of sharing risk-plan.mts

  • Location: tools/e2e/pr-e2e-gate.mts:580
  • Category: architecture
  • Problem: tools/e2e/pr-e2e-gate.mts (1261 lines) builds its own risk plan from changed files and GitHub API rather than importing buildRiskPlan/validateRiskPlan from tools/advisors/risk-plan.mts. This duplicates deterministic selection logic, creating divergence risk between the controller and advisor consumers.
  • Impact: Divergent risk plan logic between controller and advisors can cause silent job selection mismatches, missed regression coverage, or incorrect job dispatch.
  • Recommended action: Extract shared buildAndValidateRiskPlan function in risk-plan.mts used by both controller and advisors.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: grep -n 'buildRiskPlan\|validateRiskPlan\|RISK_RULES' tools/e2e/pr-e2e-gate.mts tools/advisors/risk-plan.mts — verify imports and shared usage
  • Missing regression test: Integration test calling shared buildAndValidateRiskPlan from both controller and advisor entry points with identical inputs, asserting identical RiskPlan output.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: grep -n 'buildRiskPlan\|validateRiskPlan\|RISK_RULES' tools/e2e/pr-e2e-gate.mts tools/advisors/risk-plan.mts — verify imports and shared usage.
  • Evidence: tools/e2e/pr-e2e-gate.mts reimplements risk family matching tools/advisors/risk-plan.mts exports buildRiskPlan and validateRiskPlan Prior review PRA-1 identified this duplication

PRA-4 Resolve/justify — Missing env var rename completeness verification test

  • Location: test/support/env-var-rename.test.ts:1
  • Category: tests
  • Problem: Env vars renamed from NEMOCLAW_E2E_RISK_* to NEMOCLAW_E2E_PLAN_HASH/NEMOCLAW_E2E_CORRELATION_ID/NEMOCLAW_E2E_SHARD across workflow, controller, and reporter. No test greps codebase for old names to prevent regression.
  • Impact: Stale references to old env var names could silently disable risk signal reporting or break evidence correlation.
  • Recommended action: Add test/support/env-var-rename.test.ts that greps codebase for NEMOCLAW_E2E_RISK_* and fails if found outside test fixtures.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: grep -r 'NEMOCLAW_E2E_RISK_' --include='*.yaml' --include='*.mts' --include='*.ts' . | grep -v test/fixtures
  • Missing regression test: Test file test/support/env-var-rename.test.ts that scans all workflow, controller, and reporter files for old env var names.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: grep -r 'NEMOCLAW_E2E_RISK_' --include='*.yaml' --include='*.mts' --include='*.ts' . | grep -v test/fixtures.
  • Evidence: e2e.yaml uses NEMOCLAW_E2E_PLAN_HASH, NEMOCLAW_E2E_CORRELATION_ID, NEMOCLAW_E2E_SHARD pr-e2e-gate.yaml dispatch inputs use plan_hash, correlation_id risk-signal-reporter.ts reads new env var names No test/support/env-var-rename.test.ts exists

PRA-5 Resolve/justify — Missing workflow-level fork PR dispatch test

  • Location: test/pr-e2e-gate-workflow.test.ts:1
  • Category: tests
  • Problem: PR E2E gate workflow coordinate job lacks workflow-level fork guard. No test simulates a fork PR dispatch to verify the job is skipped (not just controller rejection).
  • Impact: Without a test, a future refactor could remove the controller fork check without detection, allowing fork PRs to trigger write-permission jobs.
  • Recommended action: Add test in test/pr-e2e-gate-workflow.test.ts that simulates a fork PR dispatch event and verifies the coordinate job evaluates to false (skipped) due to workflow-level fork guard.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check test/pr-e2e-gate-workflow.test.ts for fork dispatch simulation test
  • Missing regression test: Test that dispatches workflow_run event with head_repository != base repository and asserts coordinate job if condition evaluates to false.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check test/pr-e2e-gate-workflow.test.ts for fork dispatch simulation test.
  • Evidence: .github/workflows/pr-e2e-gate.yaml coordinate job has no if condition checking github.event.pull_request.head.repo.full_name cancel-superseded job has fork guard but coordinate does not Prior review PRA-9 identified this gap

PRA-6 Resolve/justify — Missing permissions audit test for write permissions without fork guard

  • Location: test/support/workflow-permissions.test.ts:1
  • Category: tests
  • Problem: No test parses workflow files to detect jobs with actions:write, checks:write, or permissions: write-all without workflow-level fork guard.
  • Impact: Workflows with excessive permissions and missing fork guards could be introduced without detection.
  • Recommended action: Add test/support/workflow-permissions.test.ts that parses all .github/workflows/*.yaml and fails if any job has write permissions without an if condition checking github.event.pull_request.head.repo.full_name == github.repository.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check test/support/workflow-permissions.test.ts exists and scans workflows
  • Missing regression test: Test file that validates all workflow jobs with write permissions have fork guards.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check test/support/workflow-permissions.test.ts exists and scans workflows.
  • Evidence: .github/workflows/pr-e2e-gate.yaml coordinate job has actions:write, checks:write without workflow-level fork guard No test/support/workflow-permissions.test.ts exists Prior review PRA-10 identified this gap

PRA-7 Resolve/justify — Missing cloud-onboard live test invariant verification for platform-install risk family

  • Location: test/pr-risk-plan.test.ts:1
  • Category: tests
  • Problem: Risk plan platform-install family requires cloud-onboard job with invariants: clean host installs pinned dependencies, platform detection doesn't downgrade validation. No test verifies test/e2e/live/cloud-onboard.test.ts contains assertions for both invariants.
  • Impact: If cloud-onboard test loses its invariant assertions, the platform-install risk family would pass without actual validation.
  • Recommended action: Add test in test/pr-risk-plan.test.ts that reads test/e2e/live/cloud-onboard.test.ts and verifies it contains assertions for both platform-install invariants.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check test/pr-risk-plan.test.ts for cloud-onboard invariant verification
  • Missing regression test: Test that parses cloud-onboard.test.ts and asserts presence of pinned dependency verification and platform detection no-downgrade checks.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check test/pr-risk-plan.test.ts for cloud-onboard invariant verification.
  • Evidence: tools/advisors/risk-plan.mts platform-install family invariants: 'a clean host installs the intended pinned dependencies and reaches a usable agent' and 'platform detection does not silently downgrade required runtime validation' Required job: cloud-onboard Prior review PRA-7 identified this gap

PRA-8 Resolve/justify — Missing risk plan v2 round-trip integration tests for advisor consumers

  • Location: test/pr-risk-plan.test.ts:1
  • Category: tests
  • Problem: E2E Advisor and PR Review Advisor consume buildRiskPlan from risk-plan.mts (v2). No integration tests verify they handle v2 plans without rejection/downgrade.
  • Impact: If advisor consumers regress to expecting v1, they could reject valid plans or produce incorrect recommendations, breaking the 'additive model advice' guarantee.
  • Recommended action: Add integration tests in test/pr-risk-plan.test.ts (or per-advisor test files) that build a v2 risk plan and verify each advisor consumer handles it without downgrading or rejecting.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: grep -r 'RISK_PLAN_VERSION\|validateRiskPlan\|buildRiskPlan' tools/e2e-advisor/ tools/pr-review-advisor/ --include='*.mts' --include='*.ts'
  • Missing regression test: Integration test per advisor that builds v2 risk plan and asserts consumer accepts it.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: grep -r 'RISK_PLAN_VERSION\|validateRiskPlan\|buildRiskPlan' tools/e2e-advisor/ tools/pr-review-advisor/ --include='*.mts' --include='*.ts'.
  • Evidence: tools/advisors/risk-plan.mts exports RISK_PLAN_VERSION = 2 tools/e2e-advisor/analyze.mts and targets.mts import buildRiskPlan tools/pr-review-advisor/analyze.mts imports buildRiskPlan test/e2e-advisor-targets.test.ts expects version 2 in context tool results Prior review PRA-11 identified this gap

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

  • None.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Run the `cloud-onboard` E2E job for Installer and platform changes must work on a clean supported host with the pinned runtime dependencies. Matched files: `.github/workflows/e2e.yaml`.. Deterministic regression risks require live validation: platform-install. Deterministic regression risks require live validation: platform-install family matched by .github/workflows/e2e.yaml changes. cloud-onboard E2E job must run on clean host to validate pinned dependency installation and platform detection no-downgrade invariants. Unit/mocked tests cannot validate real host behavior.
  • PRA-T2 Runtime validation — Run the cloud-onboard E2E job for Installer and platform changes must work on a clean supported host with the pinned runtime dependencies. Matched files: .github/workflows/e2e.yaml.. Deterministic regression risks require live validation: platform-install. Deterministic regression risks require live validation: platform-install family matched by .github/workflows/e2e.yaml changes. cloud-onboard E2E job must run on clean host to validate pinned dependency installation and platform detection no-downgrade invariants. Unit/mocked tests cannot validate real host behavior.
  • PRA-T3 Runtime validation — Add test/support/env-var-rename.test.ts that greps codebase for NEMOCLAW_E2E_RISK_* and fails if found outside test fixtures (F-005).. Deterministic regression risks require live validation: platform-install. Deterministic regression risks require live validation: platform-install family matched by .github/workflows/e2e.yaml changes. cloud-onboard E2E job must run on clean host to validate pinned dependency installation and platform detection no-downgrade invariants. Unit/mocked tests cannot validate real host behavior.
  • PRA-T4 Runtime validation — Add test in test/pr-e2e-gate-workflow.test.ts that simulates a fork PR dispatch event and verifies the coordinate job evaluates to false due to workflow-level fork guard (F-006).. Deterministic regression risks require live validation: platform-install. Deterministic regression risks require live validation: platform-install family matched by .github/workflows/e2e.yaml changes. cloud-onboard E2E job must run on clean host to validate pinned dependency installation and platform detection no-downgrade invariants. Unit/mocked tests cannot validate real host behavior.
  • PRA-T5 Runtime validation — Add test/support/workflow-permissions.test.ts that parses all .github/workflows/*.yaml and fails if any job has write permissions without fork guard (F-007).. Deterministic regression risks require live validation: platform-install. Deterministic regression risks require live validation: platform-install family matched by .github/workflows/e2e.yaml changes. cloud-onboard E2E job must run on clean host to validate pinned dependency installation and platform detection no-downgrade invariants. Unit/mocked tests cannot validate real host behavior.
  • PRA-T6 Missing env var rename completeness verification test — Add test/support/env-var-rename.test.ts that greps codebase for NEMOCLAW_E2E_RISK_* and fails if found outside test fixtures.
  • PRA-T7 Missing workflow-level fork PR dispatch test — Add test in test/pr-e2e-gate-workflow.test.ts that simulates a fork PR dispatch event and verifies the coordinate job evaluates to false (skipped) due to workflow-level fork guard.
  • PRA-T8 Missing permissions audit test for write permissions without fork guard — Add test/support/workflow-permissions.test.ts that parses all .github/workflows/*.yaml and fails if any job has write permissions without an if condition checking github.event.pull_request.head.repo.full_name == github.repository.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — PR E2E gate controller duplicates risk plan construction instead of sharing risk-plan.mts

  • Location: tools/e2e/pr-e2e-gate.mts:580
  • Category: architecture
  • Problem: tools/e2e/pr-e2e-gate.mts (1261 lines) builds its own risk plan from changed files and GitHub API rather than importing buildRiskPlan/validateRiskPlan from tools/advisors/risk-plan.mts. This duplicates deterministic selection logic, creating divergence risk between the controller and advisor consumers.
  • Impact: Divergent risk plan logic between controller and advisors can cause silent job selection mismatches, missed regression coverage, or incorrect job dispatch.
  • Recommended action: Extract shared buildAndValidateRiskPlan function in risk-plan.mts used by both controller and advisors.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: grep -n 'buildRiskPlan\|validateRiskPlan\|RISK_RULES' tools/e2e/pr-e2e-gate.mts tools/advisors/risk-plan.mts — verify imports and shared usage
  • Missing regression test: Integration test calling shared buildAndValidateRiskPlan from both controller and advisor entry points with identical inputs, asserting identical RiskPlan output.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: grep -n 'buildRiskPlan\|validateRiskPlan\|RISK_RULES' tools/e2e/pr-e2e-gate.mts tools/advisors/risk-plan.mts — verify imports and shared usage.
  • Evidence: tools/e2e/pr-e2e-gate.mts reimplements risk family matching tools/advisors/risk-plan.mts exports buildRiskPlan and validateRiskPlan Prior review PRA-1 identified this duplication

PRA-2 Required — pull_request_target workflow with write permissions lacks workflow-level fork guard

  • Location: .github/workflows/pr-e2e-gate.yaml:30
  • Category: architecture
  • Problem: .github/workflows/pr-e2e-gate.yaml coordinate job requests actions:write, checks:write, pull-requests:read permissions but fork rejection exists only in controller code (tools/e2e/pr-e2e-gate.mts startPrGate). No workflow-level if guard on coordinate job to prevent job start for fork PRs.
  • Impact: A malicious fork PR could trigger the workflow; if controller fork rejection is bypassed (prototype pollution, logic error, future refactor), the controller would have access to GITHUB_TOKEN with write permissions before the check executes.
  • Required action: Add workflow-level fork guard: if: github.event.pull_request.head.repo.full_name == github.repository on the coordinate job as defense-in-depth.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check .github/workflows/pr-e2e-gate.yaml coordinate job if condition; verify tools/e2e/pr-e2e-gate.mts startPrGate fork check executes before any GitHub API calls.
  • Missing regression test: Test simulating a fork PR dispatch verifying the workflow job is skipped (not just controller rejection).
  • Done when: The required change is committed and verification passes: Check .github/workflows/pr-e2e-gate.yaml coordinate job if condition; verify tools/e2e/pr-e2e-gate.mts startPrGate fork check executes before any GitHub API calls.
  • Evidence: .github/workflows/pr-e2e-gate.yaml uses pull_request_target trigger with coordinate job permissions actions:write, checks:write, pull-requests:read tools/e2e/pr-e2e-gate.mts startPrGate throws 'PR branch must be in the base repository' before fetch No if condition on coordinate job checking github.event.pull_request.head.repo.full_name

PRA-3 Required — Risk plan version v2 consumers (E2E Advisor, PR Review Advisor) not verified for compatibility

  • Location: tools/advisors/risk-plan.mts:1
  • Category: architecture
  • Problem: tools/advisors/risk-plan.mts exports RISK_PLAN_VERSION = 2 (was 1). PR E2E gate controller validates version 2. However, E2E Advisor (tools/e2e-advisor/) and PR Review Advisor (tools/pr-review-advisor/) consumption of v2 risk plans is not verified in this PR. Model advice is documented as additive and cannot downgrade the deterministic floor, requiring all consumers to accept v2.
  • Impact: If E2E Advisor or PR Review Advisor still expect version 1, they may reject valid risk plans or produce incorrect recommendations, breaking the 'additive model advice' guarantee and potentially causing silent fallback or incorrect job selection.
  • Required action: Verify E2E Advisor and PR Review Advisor risk plan consumers accept version 2. Search for RISK_PLAN_VERSION imports and validateRiskPlan usage in tools/e2e-advisor/ and tools/pr-review-advisor/. Add tests confirming version 2 round-trip compatibility.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: grep -r 'RISK_PLAN_VERSION\|validateRiskPlan\|buildRiskPlan' tools/e2e-advisor/ tools/pr-review-advisor/ --include='*.mts' --include='*.ts'
  • Missing regression test: Integration tests in each advisor that build a risk plan with version 2 and verify the consumer handles it without downgrading or rejecting.
  • Done when: The required change is committed and verification passes: grep -r 'RISK_PLAN_VERSION\|validateRiskPlan\|buildRiskPlan' tools/e2e-advisor/ tools/pr-review-advisor/ --include='*.mts' --include='*.ts'.
  • Evidence: tools/advisors/risk-plan.mts:1 exports RISK_PLAN_VERSION = 2 tools/e2e/pr-e2e-gate.mts:32 imports and validates version in validateRiskPlan test/e2e-advisor-targets.test.ts:53 expects '"version":2' in context tool result PR description: 'model advice is additive and cannot downgrade the deterministic floor'

PRA-4 Resolve/justify — Missing env var rename completeness verification test

  • Location: test/support/env-var-rename.test.ts:1
  • Category: tests
  • Problem: Env vars renamed from NEMOCLAW_E2E_RISK_* to NEMOCLAW_E2E_PLAN_HASH/NEMOCLAW_E2E_CORRELATION_ID/NEMOCLAW_E2E_SHARD across workflow, controller, and reporter. No test greps codebase for old names to prevent regression.
  • Impact: Stale references to old env var names could silently disable risk signal reporting or break evidence correlation.
  • Recommended action: Add test/support/env-var-rename.test.ts that greps codebase for NEMOCLAW_E2E_RISK_* and fails if found outside test fixtures.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: grep -r 'NEMOCLAW_E2E_RISK_' --include='*.yaml' --include='*.mts' --include='*.ts' . | grep -v test/fixtures
  • Missing regression test: Test file test/support/env-var-rename.test.ts that scans all workflow, controller, and reporter files for old env var names.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: grep -r 'NEMOCLAW_E2E_RISK_' --include='*.yaml' --include='*.mts' --include='*.ts' . | grep -v test/fixtures.
  • Evidence: e2e.yaml uses NEMOCLAW_E2E_PLAN_HASH, NEMOCLAW_E2E_CORRELATION_ID, NEMOCLAW_E2E_SHARD pr-e2e-gate.yaml dispatch inputs use plan_hash, correlation_id risk-signal-reporter.ts reads new env var names No test/support/env-var-rename.test.ts exists

PRA-5 Resolve/justify — Missing workflow-level fork PR dispatch test

  • Location: test/pr-e2e-gate-workflow.test.ts:1
  • Category: tests
  • Problem: PR E2E gate workflow coordinate job lacks workflow-level fork guard. No test simulates a fork PR dispatch to verify the job is skipped (not just controller rejection).
  • Impact: Without a test, a future refactor could remove the controller fork check without detection, allowing fork PRs to trigger write-permission jobs.
  • Recommended action: Add test in test/pr-e2e-gate-workflow.test.ts that simulates a fork PR dispatch event and verifies the coordinate job evaluates to false (skipped) due to workflow-level fork guard.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check test/pr-e2e-gate-workflow.test.ts for fork dispatch simulation test
  • Missing regression test: Test that dispatches workflow_run event with head_repository != base repository and asserts coordinate job if condition evaluates to false.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check test/pr-e2e-gate-workflow.test.ts for fork dispatch simulation test.
  • Evidence: .github/workflows/pr-e2e-gate.yaml coordinate job has no if condition checking github.event.pull_request.head.repo.full_name cancel-superseded job has fork guard but coordinate does not Prior review PRA-9 identified this gap

PRA-6 Resolve/justify — Missing permissions audit test for write permissions without fork guard

  • Location: test/support/workflow-permissions.test.ts:1
  • Category: tests
  • Problem: No test parses workflow files to detect jobs with actions:write, checks:write, or permissions: write-all without workflow-level fork guard.
  • Impact: Workflows with excessive permissions and missing fork guards could be introduced without detection.
  • Recommended action: Add test/support/workflow-permissions.test.ts that parses all .github/workflows/*.yaml and fails if any job has write permissions without an if condition checking github.event.pull_request.head.repo.full_name == github.repository.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check test/support/workflow-permissions.test.ts exists and scans workflows
  • Missing regression test: Test file that validates all workflow jobs with write permissions have fork guards.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check test/support/workflow-permissions.test.ts exists and scans workflows.
  • Evidence: .github/workflows/pr-e2e-gate.yaml coordinate job has actions:write, checks:write without workflow-level fork guard No test/support/workflow-permissions.test.ts exists Prior review PRA-10 identified this gap

PRA-7 Resolve/justify — Missing cloud-onboard live test invariant verification for platform-install risk family

  • Location: test/pr-risk-plan.test.ts:1
  • Category: tests
  • Problem: Risk plan platform-install family requires cloud-onboard job with invariants: clean host installs pinned dependencies, platform detection doesn't downgrade validation. No test verifies test/e2e/live/cloud-onboard.test.ts contains assertions for both invariants.
  • Impact: If cloud-onboard test loses its invariant assertions, the platform-install risk family would pass without actual validation.
  • Recommended action: Add test in test/pr-risk-plan.test.ts that reads test/e2e/live/cloud-onboard.test.ts and verifies it contains assertions for both platform-install invariants.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check test/pr-risk-plan.test.ts for cloud-onboard invariant verification
  • Missing regression test: Test that parses cloud-onboard.test.ts and asserts presence of pinned dependency verification and platform detection no-downgrade checks.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check test/pr-risk-plan.test.ts for cloud-onboard invariant verification.
  • Evidence: tools/advisors/risk-plan.mts platform-install family invariants: 'a clean host installs the intended pinned dependencies and reaches a usable agent' and 'platform detection does not silently downgrade required runtime validation' Required job: cloud-onboard Prior review PRA-7 identified this gap

PRA-8 Resolve/justify — Missing risk plan v2 round-trip integration tests for advisor consumers

  • Location: test/pr-risk-plan.test.ts:1
  • Category: tests
  • Problem: E2E Advisor and PR Review Advisor consume buildRiskPlan from risk-plan.mts (v2). No integration tests verify they handle v2 plans without rejection/downgrade.
  • Impact: If advisor consumers regress to expecting v1, they could reject valid plans or produce incorrect recommendations, breaking the 'additive model advice' guarantee.
  • Recommended action: Add integration tests in test/pr-risk-plan.test.ts (or per-advisor test files) that build a v2 risk plan and verify each advisor consumer handles it without downgrading or rejecting.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: grep -r 'RISK_PLAN_VERSION\|validateRiskPlan\|buildRiskPlan' tools/e2e-advisor/ tools/pr-review-advisor/ --include='*.mts' --include='*.ts'
  • Missing regression test: Integration test per advisor that builds v2 risk plan and asserts consumer accepts it.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: grep -r 'RISK_PLAN_VERSION\|validateRiskPlan\|buildRiskPlan' tools/e2e-advisor/ tools/pr-review-advisor/ --include='*.mts' --include='*.ts'.
  • Evidence: tools/advisors/risk-plan.mts exports RISK_PLAN_VERSION = 2 tools/e2e-advisor/analyze.mts and targets.mts import buildRiskPlan tools/pr-review-advisor/analyze.mts imports buildRiskPlan test/e2e-advisor-targets.test.ts expects version 2 in context tool results Prior review PRA-11 identified this gap

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@cv cv changed the title ci(e2e): require deterministic live validation ci(e2e): add risk-based PR gate Jul 10, 2026
@cv cv added v0.0.80 and removed v0.0.80 labels Jul 10, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
.github/workflows/pr-e2e-gate.yaml (1)

202-223: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Route step outputs through env instead of interpolating into run: (defense-in-depth).

The finish and abandon steps inline ${{ steps.*.outputs.* }} directly into the shell program, which zizmor flags as template-injection. These specific outputs are currently safe (state_hash/check_id/run_id pass the controller's strict appendOutput allowlist, and work_dir is a mktemp path), so this is not exploitable today. However, the start/wait steps already use the env-then-"$VAR" pattern; matching it here keeps the workflow consistent, silences the SAST finding, and future-proofs against a later output on the same path becoming attacker-influenced. The same pattern applies to work_dir in the evidence (Line 196) and cleanup (Line 227) steps.

As per path instructions: "pass untrusted values as data rather than interpolating them into shell programs."

🔒 Proposed hardening for the finish/abandon steps
       - id: finish
         name: Verify evidence
         if: ${{ always() && steps.start.outputs.dispatched == 'true' }}
         env:
           GITHUB_TOKEN: ${{ github.token }}
+          WORK_DIR: ${{ steps.workspace.outputs.work_dir }}
+          STATE_HASH: ${{ steps.start.outputs.state_hash }}
+          CHECK_ID: ${{ steps.start.outputs.check_id }}
+          RUN_ID: ${{ steps.start.outputs.run_id }}
         run: >-
           node --experimental-strip-types tools/e2e/pr-e2e-gate.mts
           --mode finish
-          --work-dir "${{ steps.workspace.outputs.work_dir }}"
-          --state-hash "${{ steps.start.outputs.state_hash }}"
-          --check-id "${{ steps.start.outputs.check_id }}"
-          --run-id "${{ steps.start.outputs.run_id }}"
+          --work-dir "$WORK_DIR"
+          --state-hash "$STATE_HASH"
+          --check-id "$CHECK_ID"
+          --run-id "$RUN_ID"

       - name: Close incomplete check
         if: ${{ always() && steps.start.outputs.check_id != '' && steps.start.outputs.finalized != 'true' && steps.finish.outputs.finalized != 'true' }}
         env:
           GITHUB_TOKEN: ${{ github.token }}
+          CHECK_ID: ${{ steps.start.outputs.check_id }}
+          RUN_ID: ${{ steps.start.outputs.run_id }}
         run: >-
           node --experimental-strip-types tools/e2e/pr-e2e-gate.mts
           --mode abandon
-          --check-id "${{ steps.start.outputs.check_id }}"
-          --run-id "${{ steps.start.outputs.run_id }}"
+          --check-id "$CHECK_ID"
+          --run-id "$RUN_ID"
🤖 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/pr-e2e-gate.yaml around lines 202 - 223, Route all GitHub
step outputs used by the “Verify evidence”, “Close incomplete check”,
“evidence”, and cleanup steps through step-level env variables instead of
interpolating them directly into run commands. Update the scripts to reference
those values as quoted shell environment variables, including work_dir,
state_hash, check_id, and run_id, matching the existing start/wait pattern.

Sources: Path instructions, Linters/SAST tools

🤖 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.

Nitpick comments:
In @.github/workflows/pr-e2e-gate.yaml:
- Around line 202-223: Route all GitHub step outputs used by the “Verify
evidence”, “Close incomplete check”, “evidence”, and cleanup steps through
step-level env variables instead of interpolating them directly into run
commands. Update the scripts to reference those values as quoted shell
environment variables, including work_dir, state_hash, check_id, and run_id,
matching the existing start/wait pattern.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: be139e3e-70aa-45dd-9efc-8d97270f9ef9

📥 Commits

Reviewing files that changed from the base of the PR and between e6366d6 and 3bee59c.

📒 Files selected for processing (13)
  • .github/workflows/e2e.yaml
  • .github/workflows/pr-e2e-gate.yaml
  • docs/about/release-notes.mdx
  • test/e2e-risk-signal-reporter.test.ts
  • test/e2e/README.md
  • test/e2e/docs/README.md
  • test/e2e/support/e2e-operations-workflow-boundary.test.ts
  • test/pr-e2e-gate-workflow.test.ts
  • test/pr-e2e-gate.test.ts
  • tools/e2e-advisor/README.md
  • tools/e2e/operations-workflow-boundary.mts
  • tools/e2e/pr-e2e-gate.mts
  • tools/pr-review-advisor/README.md
✅ Files skipped from review due to trivial changes (2)
  • docs/about/release-notes.mdx
  • tools/pr-review-advisor/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • test/e2e/support/e2e-operations-workflow-boundary.test.ts
  • test/e2e-risk-signal-reporter.test.ts
  • .github/workflows/e2e.yaml

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Comment thread tools/e2e/pr-e2e-gate.mts Dismissed
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv
cv merged commit e4d2e91 into main Jul 11, 2026
45 checks passed
@cv
cv deleted the codex/simplify-required-live branch July 11, 2026 02:52
cv added a commit that referenced this pull request Jul 11, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Follow-up to #6662, this replaces repeated credential-free E2E jobs with
source discovery and one shared Ubuntu E2E job. Tests opt in beside
their code with `e2e/credential-free`; the E2E workflow owns the runner,
setup, permissions, secrets, timeout, and artifact policy.

## Changes

- Discover tagged tests from their real Vitest projects and derive test
IDs from unique filenames, with no separate test catalog or manually
maintained matrix.
- Run five credential-free tests in the `shared-e2e` matrix job,
including the previously unwired Ubuntu repository CLI smoke test.
- Move selector and matrix planning from inline workflow Bash into
tested TypeScript while preserving `jobs`/`targets` selectors, PR
reporting, scorecard inputs, and controller evidence.
- Keep the temporary four-key shell output required by the current base
E2E workflow. It discovers that workflow's actual job IDs, excludes
selectors the base cannot run, and can be deleted once this workflow is
on `main`.
- Reduce `.github/workflows/e2e.yaml` from 5,430 to 5,334 lines and
document the contributor contract in `test/e2e/README.md`.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [x] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: independent diff and
workflow-contract reviews found no credential exposure, path injection,
untrusted runner selection, or unresolved Actions contract mismatch;
boundary tests pin the credential-free shared job's no-secret setup and
artifact contracts.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — `npx vitest run --project e2e-support`
(113 files, 910 tests); PR gate/advisor/mock-parity integration suites
(52 tests); `npm run test:projects:check`; `npm run typecheck:cli`; real
`origin/main...HEAD` mock-parity check; exact current-base shell/matrix
compatibility simulation
- [x] Applicable broad gate passed — earlier PR head: `env -u
SSH_CONNECTION -u SSH_CLIENT -u SSH_TTY npm test` (1,388 files, 15,964
tests passed; 39 skipped; 1 todo); current terminology change set reran
the complete E2E-support project
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — passed
with 0 errors and 2 existing Fern warnings
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only) — no `docs/` pages changed
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added discovery and execution of “credential-free” E2E tests via a
shared workflow job.
* Added a dedicated workflow planner that builds a live test matrix plus
a credential-free test matrix from typed selectors.
* Improved PR reporting to include per-test results (including handling
for unavailable/unknown results).

* **Bug Fixes**
* Improved selector parsing/selection behavior for shared E2E routing
and dispatch targeting.
* Prevented metadata-only changes from being treated as relevant for
mock-parity checks.

* **Documentation**
* Added documentation for credential-free tests, tagging, discovery
rules, and local inspection commands.

* **Tests**
* Extended E2E coverage for selector normalization, credential-free
discovery, workflow boundaries, and PR reporting scenarios (including
unknown/missing results).
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
This PR replaces separate advisory and post-merge control paths with one
deterministic PR E2E gate. The gate selects jobs from the checked-in
risk policy, verifies complete evidence for the PR head commit, and
keeps model output outside merge authority; the final refactor names
each workflow, run, and controller by its actual role.

## Changes

- add `E2E / PR Gate` with one controller for planning, dispatch,
cancellation, evidence verification, and check completion
- reject fork events before trusted check or API mutations
- reduce the child `e2e.yaml` dispatch contract to the PR commit,
selected jobs, plan hash, and correlation ID
- require every policy-selected job shard to produce bound evidence and
fail the check for every incomplete outcome
- rename the workflow, controller, tests, artifacts, symbols, steps, and
diagnostics without aliases or compatibility paths
- consolidate the lifecycle documentation in `test/e2e/README.md` and
keep advisor documentation focused on its local relationship

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [x] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: independent code and
test reviews found no remaining actionable issues; security boundaries
remain covered by workflow tests
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — 43 integration tests, 24 E2E-support
tests, `npm run typecheck:cli`, and `npm run test:projects:check`
- [x] Applicable broad gate passed — `npm test` passed earlier in this
PR with 15,850 tests; the current naming refactor was reverified with
the focused suites above
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — passed
with 0 errors and 2 pre-existing Fern warnings
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Added a required PR E2E gate that builds a deterministic risk plan,
dispatches only the needed E2E jobs, and blocks until all evidence
signals are complete and passing.
- Introduced controller-driven validation for manual dispatches, plus
automatic cancellation of superseded/invalid PR E2E runs and updated PR
reporting/scorecard gating.

- **Documentation**
- Updated E2E and release documentation to describe the PR gate flow and
the new “required-live” terminology/behavior.

- **Tests**
- Expanded workflow contract, evidence/shard routing, and risk-plan
determinism coverage; updated reporter and prompt-context expectations
for the new signal schema.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Follow-up to NVIDIA#6662, this replaces repeated credential-free E2E jobs with
source discovery and one shared Ubuntu E2E job. Tests opt in beside
their code with `e2e/credential-free`; the E2E workflow owns the runner,
setup, permissions, secrets, timeout, and artifact policy.

## Changes

- Discover tagged tests from their real Vitest projects and derive test
IDs from unique filenames, with no separate test catalog or manually
maintained matrix.
- Run five credential-free tests in the `shared-e2e` matrix job,
including the previously unwired Ubuntu repository CLI smoke test.
- Move selector and matrix planning from inline workflow Bash into
tested TypeScript while preserving `jobs`/`targets` selectors, PR
reporting, scorecard inputs, and controller evidence.
- Keep the temporary four-key shell output required by the current base
E2E workflow. It discovers that workflow's actual job IDs, excludes
selectors the base cannot run, and can be deleted once this workflow is
on `main`.
- Reduce `.github/workflows/e2e.yaml` from 5,430 to 5,334 lines and
document the contributor contract in `test/e2e/README.md`.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [x] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: independent diff and
workflow-contract reviews found no credential exposure, path injection,
untrusted runner selection, or unresolved Actions contract mismatch;
boundary tests pin the credential-free shared job's no-secret setup and
artifact contracts.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — `npx vitest run --project e2e-support`
(113 files, 910 tests); PR gate/advisor/mock-parity integration suites
(52 tests); `npm run test:projects:check`; `npm run typecheck:cli`; real
`origin/main...HEAD` mock-parity check; exact current-base shell/matrix
compatibility simulation
- [x] Applicable broad gate passed — earlier PR head: `env -u
SSH_CONNECTION -u SSH_CLIENT -u SSH_TTY npm test` (1,388 files, 15,964
tests passed; 39 skipped; 1 todo); current terminology change set reran
the complete E2E-support project
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — passed
with 0 errors and 2 existing Fern warnings
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only) — no `docs/` pages changed
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added discovery and execution of “credential-free” E2E tests via a
shared workflow job.
* Added a dedicated workflow planner that builds a live test matrix plus
a credential-free test matrix from typed selectors.
* Improved PR reporting to include per-test results (including handling
for unavailable/unknown results).

* **Bug Fixes**
* Improved selector parsing/selection behavior for shared E2E routing
and dispatch targeting.
* Prevented metadata-only changes from being treated as relevant for
mock-parity checks.

* **Documentation**
* Added documentation for credential-free tests, tagging, discovery
rules, and local inspection commands.

* **Tests**
* Extended E2E coverage for selector normalization, credential-free
discovery, workflow boundaries, and PR reporting scenarios (including
unknown/missing results).
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions area: e2e End-to-end tests, nightly failures, or validation infrastructure chore Build, CI, dependency, or tooling maintenance labels Aug 1, 2026
@coderabbitai coderabbitai Bot mentioned this pull request Aug 6, 2026
23 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions area: e2e End-to-end tests, nightly failures, or validation infrastructure chore Build, CI, dependency, or tooling maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants