Skip to content

ci(e2e): remove person-specific Launchable approval - #7829

Merged
jyaunches merged 3 commits into
mainfrom
codex/remove-brev-e2e-person-gate
Jul 29, 2026
Merged

ci(e2e): remove person-specific Launchable approval#7829
jyaunches merged 3 commits into
mainfrom
codex/remove-brev-e2e-person-gate

Conversation

@jyaunches

@jyaunches jyaunches commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Full and Launchable E2E runs no longer wait on the person-specific approve-brev-launchable-e2e environment. The trusted main workflow now verifies that the dispatching and rerunning actors have repository maintain or admin permission before the Launchable path's source checkout, so any maintainer can run the credential-bearing job without a separate approval.

Changes

  • Remove the GitHub environment from Exact staging Brev Launchable.
  • Add a fail-closed collaborator-role check before the Launchable path's source checkout for exact Launchable and full-suite dispatches, including reruns.
  • Reject future GitHub environments or authorization drift while preserving trusted-main selection, repository-secret guards, non-cancelling concurrency, exact-SHA evidence, and cleanup verification.
  • Update maintainer skills and E2E operator guidance to describe role-based authorization and distinguish concurrency queueing from approval.

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: Maintainer-directed authorization simplification. The removed environment stores no secrets; required credentials are repository secrets. A fail-closed GitHub collaborator-role check requires maintain or admin permission for dispatch and rerun actors before the Launchable path's source checkout. Focused tests preserve trusted-main selection, secret guards, exact dispatch identity, non-cancelling concurrency, and cleanup evidence.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: docs-updated
  • Evidence: Updated .agents/skills/nemoclaw-maintainer-e2e/SKILL.md, .agents/skills/nemoclaw-maintainer-cut-release-tag/SKILL.md, and test/e2e/README.md. Reviewed terminology, structure, voice, and security-behavior alignment at head e3c9e25d38da7640f3e78a8829cc2d24c6adc335. The guidance accurately describes automatic maintain/admin checks for dispatching and rerunning actors before the Launchable path's source checkout, removal of GitHub environment approval, and retained non-cancelling queue behavior. E2E support passed 45/45; maintainer policy tests passed 28/28; Biome, markdownlint, and amend hooks passed.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable
  • Station profile/scenario: Not applicable
  • Result: Not applicable
  • Supporting evidence: Not applicable

Verification

  • PR description includes a Signed-off-by: line 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 — npm exec -- vitest run --project e2e-support test/e2e/support/e2e-workflow.test.ts passed 45 tests; npm exec -- vitest run --project integration test/maintainer-e2e-skill.test.ts test/maintainer-skills-policy.test.ts passed 28 tests; the post-format test/maintainer-e2e-skill.test.ts rerun passed 12 tests; npm run typecheck:cli and npm run test-size:check passed.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repository-wide validation/coverage changes — command/result:
  • 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)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new docs pages only)

Signed-off-by: Julie Yaunches jyaunches@nvidia.com

Summary by CodeRabbit

  • New Features

    • Added maintainer authorization checks for manually dispatched Launchable E2E runs.
    • Launchable E2E runs now remain queued when concurrency limits are reached instead of requiring environment approval.
  • Documentation

    • Clarified dispatch authorization, queueing behavior, and recovery guidance for E2E workflows.
  • Tests

    • Expanded workflow validation to cover authorization checks, required configuration, ordering, and prevention of unintended environment gates.

Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
@jyaunches jyaunches self-assigned this Jul 29, 2026
@github-code-quality

github-code-quality Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit e3c9e25 in the codex/remove-brev-e2... branch remains at 96%, unchanged from commit ae71359 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit e3c9e25 in the codex/remove-brev-e2... branch remains at 81%, unchanged from commit ae71359 in the main branch.

Show a code coverage summary of the most impacted files.
File main ae71359 codex/remove-brev-e2... e3c9e25 +/-
src/lib/onboard...ndbox-create.ts 83% 33% -50%
src/lib/onboard...-create-plan.ts 88% 75% -13%
src/lib/onboard...ndbox-create.ts 91% 83% -8%
src/lib/sandbox...rce-identity.ts 88% 88% 0%

Updated July 29, 2026 15:13 UTC

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The staging Brev Launchable E2E job now uses trusted-main maintainer authorization and non-cancelling queueing instead of GitHub environment approval. Workflow validation, maintainer guidance, release recovery instructions, documentation, and tests reflect the updated behavior.

Changes

Launchable E2E routing

Layer / File(s) Summary
Workflow authorization boundary
.github/workflows/e2e.yaml, tools/e2e/workflow-boundary.mts, test/e2e/support/e2e-workflow.test.ts
The workflow validates maintainer/admin permissions for Launchable dispatches, removes the job environment, and tests authorization, secret, and ordering drift.
Authorization and queue guidance
.agents/skills/nemoclaw-maintainer-*/SKILL.md, test/e2e/README.md, test/maintainer-e2e-skill.test.ts
Guidance and assertions describe trusted-main authorization, no separate environment approval, and non-cancelling Launchable queue behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

Possibly related PRs

Suggested labels: area: security

Suggested reviewers: cv, dnandakumar-nv

Sequence Diagram(s)

sequenceDiagram
  participant Maintainer
  participant GitHubActions
  participant CollaboratorAPI
  participant LaunchableE2E
  Maintainer->>GitHubActions: dispatch trusted-main Launchable workflow
  GitHubActions->>CollaboratorAPI: verify maintain/admin permission
  CollaboratorAPI-->>GitHubActions: return permission
  GitHubActions->>LaunchableE2E: start authorized run or keep it queued
Loading
🚥 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: removing the person-specific Launchable approval from E2E runs.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/remove-brev-e2e-person-gate

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

Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · medium confidence · 0 blockers · 1 warning · 0 suggestions
  • Model comparison: normalized findings differ; normalized E2E selections match; Nemotron reported the same number of blockers, 1 more warning, the same number of suggestions.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: cloud-inference, cloud-onboard, security-posture

1 optional E2E recommendation
  • staging-brev-launchable

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@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 Jul 29, 2026
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
@jyaunches
jyaunches merged commit 11cfdd6 into main Jul 29, 2026
90 of 97 checks passed
@jyaunches
jyaunches deleted the codex/remove-brev-e2e-person-gate branch July 29, 2026 15:21
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.

2 participants