ci(e2e): delegate protected gate approvals#7342
Conversation
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
📝 WalkthroughWalkthroughThe PR adds protected-environment approval handling for internal credentialed E2E runs, including controller commands, workflow orchestration, evidence finalization, reviewer-based authorization, targeted cancellation, tests, and updated operational documentation. ChangesInternal E2E approval
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Coordinator
participant ApproveInternalE2E
participant Controller
participant GitHub
Coordinator->>ApproveInternalE2E: pass approval PR and SHA outputs
ApproveInternalE2E->>Controller: run start-approved-control-plane
Controller->>GitHub: validate workflow run and environment approval
Controller->>GitHub: dispatch approved E2E run
ApproveInternalE2E->>GitHub: wait for and verify evidence
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 8031a94 in the TypeScript / code-coverage/cliThe overall coverage in commit 8031a94 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. 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.
🧹 Nitpick comments (1)
.github/workflows/pr-e2e-gate.yaml (1)
335-337: 🩺 Stability & Availability | 🔵 TrivialConsider
cancel-in-progress: falsehere, mirroring thecoordinatejob's rationale.
coordinate's concurrency group deliberately usescancel-in-progress: falseso the outgoing run can observe cancellation and close its own check.approve-internal-e2eis gated by the protected environment before any step runs (including "Checkout controller"), so if a newer push cancels it viacancel-in-progress: truewhile it's still "Waiting" for reviewer approval, no step — not even thealways()"Close incomplete approved check" step — ever executes. The check for that stale head/base SHA pair is left permanently "in_progress" with the authorization title. Blast radius is limited (checks are scoped to the exact head/base SHA pair, so it won't block the required check for the PR's current head), but it does leave dangling pending-approval checks on abandoned commits that require no cleanup path.If the intent is to let a new push immediately supersede the pending approval wait (rather than queue behind it for up to
timeout-minutes: 180), that's a reasonable tradeoff — but it's worth confirming this was a deliberate choice given the explicit contrast with the sibling job's documented reasoning.🤖 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 335 - 337, Update the approve-internal-e2e concurrency configuration to use cancel-in-progress: false, matching the coordinate job’s behavior and allowing the pending protected-environment run to execute its cleanup steps. Keep the existing concurrency group unchanged.
🤖 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 335-337: Update the approve-internal-e2e concurrency configuration
to use cancel-in-progress: false, matching the coordinate job’s behavior and
allowing the pending protected-environment run to execute its cleanup steps.
Keep the existing concurrency group unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 61e6c3fd-d422-432c-a19f-a06b55aabada
📒 Files selected for processing (9)
.github/workflows/pr-e2e-gate.yamltest/e2e/README.mdtest/pr-e2e-gate-command.test.tstest/pr-e2e-gate-fork-skip.test.tstest/pr-e2e-gate-internal-approval.test.tstest/pr-e2e-gate-retry-history.test.tstest/pr-e2e-gate-workflow.test.tstest/pr-e2e-required.test.tstools/e2e/pr-e2e-gate.mts
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical dated `## v0.0.93` release entry to `docs/changelog/2026-07-23.mdx`. The entry records user-visible behavior, release validation, and documentation controls merged after `v0.0.92`, while preserving the pending DGX OS `7.6.x` Station Express qualification caveat. ## Changes - Adds the parser-safe dated release entry with a summary, grouped details, and published-route links. - Reconciles the `v0.0.92..origin/main` commit range with merged `v0.0.93` PRs. - Records that no-OTA DGX OS `7.6.x` passed bounded host preflight, while full Station Express end-to-end qualification remains pending. - Leaves existing product pages unchanged because the source PRs already document their supported behavior. ### Source summary - #7285 -> `docs/changelog/2026-07-23.mdx`: Records the existing-vLLM ownership choice and resumable Station handoff. - #7419 -> `docs/changelog/2026-07-23.mdx`: Records bounded no-OTA DGX OS `7.6.x` recognition and its pending end-to-end qualification. - #7268 -> `docs/changelog/2026-07-23.mdx`: Records optional Hugging Face authentication, output sanitization, and resumable HTTP `429` recovery. - #7442 -> `docs/changelog/2026-07-23.mdx`: Records clean SIGINT handling at hidden credential prompts. - #7299 -> `docs/changelog/2026-07-23.mdx`: Records Intel macOS rejection before ref resolution or network work. - #7296 -> `docs/changelog/2026-07-23.mdx`: Records the DGX Spark non-interactive local-vLLM selection order. - #7342 -> `docs/changelog/2026-07-23.mdx`: Records delegated protected E2E approvals in the grouped release-validation bullet. - #7373 -> `docs/changelog/2026-07-23.mdx`: Records base-image publication gating before final-main fanout. - #7388 -> `docs/changelog/2026-07-23.mdx`: Records semantic phase runtime summaries. - #7397 -> `docs/changelog/2026-07-23.mdx`: Records progress coverage hardening. - #7391 -> `docs/changelog/2026-07-23.mdx`: Records centralized larger-runner routing. - #7423 -> `docs/changelog/2026-07-23.mdx`: Records one retry for confirmed hosted-runner loss. - #7399 -> `docs/changelog/2026-07-23.mdx`: Records runner-comparison telemetry. - #7270 -> `docs/changelog/2026-07-23.mdx`: Records staging Brev Launchable validation. - #7426 -> `docs/changelog/2026-07-23.mdx`: Records filtering of irrelevant base-image run history. - #7333 -> `docs/changelog/2026-07-23.mdx`: Records aligned Quickstart platform guidance. - #7343 -> `docs/changelog/2026-07-23.mdx`: Records documentation-writer receipt collection. - #7400 -> `docs/changelog/2026-07-23.mdx`: Records the documentation-writer receipt requirement for docs-only PRs. - #7413 -> `docs/changelog/2026-07-23.mdx`: Records removal of redundant receipt PR metadata. - #7405 -> `docs/changelog/2026-07-23.mdx`: Records corrected inference CLI references. - #7389 -> `docs/changelog/2026-07-23.mdx`: Records completion of the v0.0.91 documentation audit. `#7384` is an internal refactor with no intended runtime behavior change. `#7401` updates internal CodeQL Actions dependencies. `#7376` is already contained in `v0.0.92`, so it is outside the release-entry scan range despite its retained planning label. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates dated changelog structure, SPDX syntax, and version headings. - [ ] Tests not applicable — justification: - [x] 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: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: Reviewed `docs/changelog/2026-07-23.mdx` against `WRITING.md`, `docs/CONTRIBUTING.md`, `docs/.docs-skip`, `docs/index.yml`, the six user-visible source PRs, and the remaining grouped release commits. The review corrected an ambiguous qualification claim, confirmed all published routes, preserved the DGX OS `7.6.x` caveat, and found no remaining action. - Agent: Codex Desktop <!-- docs-review-head-sha: ec0a866 --> <!-- docs-review-agents-blob-sha: 9c9b36d --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable. This PR does not change `scripts/prepare-dgx-station-host.sh`. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## Verification - [x] PR description includes a `Signed-off-by:` line 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 test/changelog-docs.test.ts`: 1 file and 6 tests passed. - [ ] Applicable broad gate passed — Not applicable to one native changelog file. - [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) — completed with 0 errors and 2 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) — not applicable because native changelog entries use a parser-safe MDX SPDX comment without frontmatter. --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added the v0.0.93 changelog covering onboarding and validation improvements. * Documented support for additional DGX Station Express workstation releases and clearer handling of existing vLLM workloads. * Added guidance for optional Hugging Face authentication, resumable rate-limit recovery, and DGX Spark provider selection. * Clarified installer behavior on Intel macOS, release validation requirements, hosted-runner retries, documentation checks, and supported CLI quickstart paths. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
Repository-configuration follow-up: the required rollout configuration in this PR was not completed. The live environment API currently reports:
This is now fail-closing every protected internal E2E request. #7492 approval run 30126807957, #7493 run 30119974452, and #7494 run 30124567776 all auto-started instead of showing Review deployments, then failed with A repository administrator needs to complete this PR's documented setup: add one or more required reviewers, enable prevent-self-review, disable admin bypass, restrict deployment to protected @prekshivyas @cv @cjagwani — please coordinate the reviewer/team selection with a repository admin. The manual maintainer fallback can unblock one PR, but it does not repair this repository-wide gate. |
Summary
Internal PRs that change credential-bearing E2E trust boundaries now request an exact-SHA protected-environment review automatically after ordinary CI. Repository-configured E2E reviewers can approve that environment without merge rights, while fork PR code still never receives repository secrets.
Required Repository Configuration
Before merging or enabling this workflow, a repository administrator must:
approve-credentialed-e2e-for-internal-pr.nemoclaw-e2e-approvers; members need repository read access but do not need merge rights. Individuals can also be listed directly.main.Only one configured reviewer is required to approve a run. The approval must cover the exact test plan. Complete this configuration before merging or enabling the workflow.
Changes
approve-credentialed-e2e-for-internal-pras a protected, non-deployment environment gate before internal credentialed E2E dispatch.maintainoradminchecks on both manual fallback operations.main, no environment secrets, variables, or custom protection apps, and exact plan review.run-control-planeandapprove-fork-e2e-skipoperations as maintainer fallbacks.This intentionally excludes the feedback-loop and observer-removal changes in #7262. Both PRs touch the gate lifecycle, so whichever merges second will require security-sensitive conflict resolution rather than an automatic merge.
Type of Change
Quality Gates
test/e2e/README.mddocuments the operational change and no shipped user surface changes.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run test/pr-e2e-gate-command.test.ts test/pr-e2e-gate-workflow.test.ts test/pr-e2e-gate-fork-skip.test.ts test/pr-e2e-gate-internal-approval.test.ts test/pr-e2e-gate-retry-history.test.ts test/pr-e2e-required.test.ts(94 passed);npm run typecheck:cli;npm run test-size:check.npm testcompleted with 18,865 passed, 201 skipped, and 20 failures in untouched local macOS/tooling-dependent tests. Failures include/private/varpath normalization, missing GNUtimeoutandstat -c, Bash-version assumptions, Docker timing/memory, Python build tooling, and unrelated timeouts. Approval-specific tests are green; CI must provide the authoritative Linux broad result.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Prekshi Vyas prekshiv@nvidia.com