diff --git a/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md b/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md index 00d1b497097..62d973cb8f7 100644 --- a/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md +++ b/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md @@ -59,7 +59,7 @@ The checker requires these status-rollup entries: A first-time fork contributor might need **Approve and run** before `pull_request` checks appear. The E2E controller records the PR SHA and base SHA without running fork code. Do not waive a missing, neutral, or skipped E2E gate. -Do not run fork code with privileged credentials to create a result. +Fork code can receive E2E credentials only through the protected exact-revision approval below. ### GitHub Actions evidence @@ -74,7 +74,11 @@ Required PR workflows must identify the PR number, PR SHA, and base SHA. - A closed event must use `E2E / PR Gate (not applicable)` for its skipped observer. It must not publish the required check name. - GitHub usually associates the run with the PR. - If that association is empty, require the controller path, branch, repository, and run time to match the coordination check. + For a fork run with an empty association, require the Actions event, workflow path, fork repository, branch, and PR SHA to match the current PR. + The PR and installer workflows must also name the exact PR, PR SHA, and base SHA. + The E2E controller must still enclose the trusted coordination check. +- If GitHub omits `headRepository.nameWithOwner`, derive it only from the returned repository name and repository-owner login. + Fail closed when those fields are missing, malformed, or contradictory. - Treat an all-skipped `gate false` run from an older workflow version as non-evidence. - Fail closed when identity, state, or timing evidence is missing, malformed, stale, contradictory, or changed. @@ -133,73 +137,38 @@ Malformed or unsafe evidence is a terminal controller error. Schema mismatches, identity mismatches, and traversal-limit errors are also terminal. The coordination check, required job, and controller must fail closed. -### Approve an E2E skip for a fork PR +### Authorize E2E for a fork PR -Use the protected-environment path when the coordination check reports `Maintainer approval required to skip credentialed E2E`. +Use the protected-environment path when coordination reports `E2E reviewer authorization required to run fork E2E`. 1. Follow the `E2E / PR Gate Controller run ` link in the coordination summary. -2. Select **Review deployments**. -3. Select `approve-credentialed-e2e-skip-for-fork-pr`. -4. Add a comment when useful, and approve. +2. Verify the fork repository, PR SHA, base SHA, selected jobs and targets, and risk-plan artifact. +3. Select **Review deployments**. +4. Select `approve-credentialed-e2e-for-fork-pr`. +5. Add a comment when useful, and approve. -This approval records that credential-bearing jobs and targets did not run. -It does not authorize fork code to use repository secrets. -The waiting job has `deployment: false`, no secrets, and no PR-controlled execution. +This approval authorizes the exact fork revision to run the selected work with E2E credentials. +Before approval, no selected credential-bearing work runs. +The trusted workflow definition comes from `main`; each PR-code checkout uses the reviewed fork repository and exact PR SHA. The controller reads the approval history and requires one approval for that environment. -The reviewer must still have `maintain` or `admin` access. -The controller also checks the PR number, PR SHA, base SHA, plan, failed check, compatible `main`, and PR state. +The environment's required reviewers are the authorization allowlist. +The controller also checks the PR number, head repository, PR SHA, base SHA, plan, pending coordination check, compatible `main`, and PR state. +Immediately before dispatch, it confirms that the PR is open and that the PR SHA, base SHA, and coordination identity still match. -An accepted approval completes the coordination check with this result: -`Credentialed E2E skipped for fork PR — approved by @`. -The summary starts with `Outcome: APPROVED SKIP — credentialed E2E did not run.` -Treat this result as an audited skip, not as E2E evidence. +Approval returns coordination to `Running E2E check(s)`. +The gate passes only after the selected jobs and targets return verified passing evidence. +Failed, missing, skipped, pending, or mismatched evidence keeps the gate from passing. Configure the environment before rollout. -Require reviewers with `maintain` or `admin` access. +Require the intended E2E reviewers. Do not add secrets, variables, or a protection app. Disable administrator bypass when possible. If **Review deployments** is absent, the environment might be missing, unprotected, or no longer waiting. -Configure it, push a change, and run PR CI again. You can also use the manual fallback below. +Configure it, push a change, and run PR CI again. Do not rerun the waiting workflow. The controller accepts an environment approval only on the first attempt. Per-PR concurrency cancels a waiting approval when another SHA reaches the gate. -### Use the manual fork-skip fallback - -Use this fallback only after the gate reports `Maintainer approval required to skip credentialed E2E`. -Review the fork change and non-secret PR CI first. Prefer the protected-environment path. - -The controller rejects these states: - -- The PR is closed. -- The PR SHA or base SHA changed. -- The PR is internal. -- The E2E plan is empty. -- The gate did not fail. -- The controller commit is not `main` or a validated ancestor of `main`. -- The reviewer does not have maintainer access. - -A permitted `main` advance must keep the controller commit as its merge base. -It must contain fewer than 300 listed changed files and no `e2e-control-plane` changes. -Other advances fail closed. -Immediately before it writes success, the controller confirms that the PR is open and that the PR SHA, base SHA, and coordination identity still match. -It fails closed if any value changed or does not match. -Its result records the reviewer, optional comment, approval-run URL, plan, and work that did not run. - -To use the fallback, select **Run workflow** on `main` and then select `approve-fork-e2e-skip`. -Provide these inputs: - -- The fork PR number. -- The 40-character PR SHA as `expected_head_sha`. -- The 40-character base SHA as `expected_base_sha`. -- A reason of 10 to 500 characters. - -Read both SHAs before dispatch. -You can give an `evidence_url` that matches `https://github.com/NVIDIA/NemoClaw/actions/runs/`. -Leave it blank when no supporting run exists. -The workflow rejects PR, issue, comment, job, and external URLs. -It validates the repository and run-ID format. It does not inspect that run's SHA, jobs, targets, or result. - ### Authorize E2E control-plane changes The `e2e-control-plane` path group includes these areas: diff --git a/.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts b/.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts index 6564ef18d58..84bd371ddb4 100644 --- a/.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts +++ b/.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts @@ -448,6 +448,7 @@ const INSTALLER_HASH_RUN_TITLE = /^Installer Hash PR #([1-9][0-9]*) head ([a-f0-9]{40}) base ([a-f0-9]{40}) gate (true|false)$/u; const E2E_GATE_RUN_TITLE = /^E2E Gate PR #([1-9][0-9]*) head ([a-f0-9]{40}) base ([a-f0-9]{40}) gate (true|false)$/u; +const REPOSITORY_NAME_PATTERN = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/u; const REQUIRED_CHECK_WORKFLOW_PATHS = new Map([ ["checks", ".github/workflows/pr.yaml"], ["changes", ".github/workflows/pr.yaml"], @@ -945,27 +946,53 @@ function currentCheckRollup( check.detailsUrl?.match(/\/runs\/(\d+)(?:[/?#]|$)/u)?.[1] === String(e2eCoordinationEvidence.trustedLegacyCheckId); + const associationLessHeadBinding = ( + metadata: ActionRunMetadata, + ): "current" | "other" | "unknown" => { + if ( + metadata.hasPullRequests !== false || + (metadata.event !== "pull_request" && metadata.event !== "pull_request_target") + ) { + return "unknown"; + } + if ( + metadata.headShaMatches === false || + metadata.headRefNameMatches === false || + metadata.headRepositoryMatches === false + ) { + return "other"; + } + return metadata.headShaMatches === true && + metadata.headRefNameMatches === true && + metadata.headRepositoryMatches === true + ? "current" + : "unknown"; + }; + function runIdentityEvidence( runId: string, requiresExactDiff: boolean, ): "current" | "other" | "unknown" { const metadata = actionRunMetadata(runId); if (!metadata?.event || !metadata.path) return "unknown"; + const headBinding = associationLessHeadBinding(metadata); if ( metadata.event === "pull_request" && - metadata.path === ".github/workflows/installer-hash-check.yaml" + (metadata.path === ".github/workflows/installer-hash-check.yaml" || + metadata.path === ".github/workflows/pr.yaml") ) { if ( metadata.immutablePrDiff === false || metadata.exactDiff === false || - metadata.headShaMatches === false + metadata.headShaMatches === false || + headBinding === "other" ) { return "other"; } if ( metadata.immutablePrDiff === true && metadata.headShaMatches === true && - (metadata.exactDiff === true || metadata.hasPullRequests === false) + (metadata.exactDiff === true || headBinding === "current") ) { return "current"; } @@ -973,11 +1000,6 @@ function currentCheckRollup( } if (metadata.exactDiff === true) { if (metadata.headShaMatches === true) { - if (metadata.event === "pull_request" && metadata.path === ".github/workflows/pr.yaml") { - if (metadata.immutablePrDiff === true) return "current"; - if (metadata.immutablePrDiff === false) return "other"; - return "unknown"; - } return "current"; } if (metadata.headShaMatches === false) return "other"; @@ -989,6 +1011,13 @@ function currentCheckRollup( if (e2eHeadBinding === "current") { return e2eCoordinationIsEnclosed(metadata) ? "current" : "unknown"; } + if ( + exactDiff.headRepository !== repo && + metadata.path !== ".github/workflows/pr-e2e-gate.yaml" && + headBinding !== "unknown" + ) { + return headBinding; + } if ( !requiresExactDiff && metadata.hasPullRequests === false && @@ -1654,6 +1683,37 @@ interface PrRevisionSnapshot { headRepository: string; } +function parseHeadRepository(headRepository: unknown, headRepositoryOwner: unknown): string | null { + if ( + typeof headRepository !== "object" || + headRepository === null || + Array.isArray(headRepository) + ) { + return null; + } + const repository = headRepository as Record; + const direct = + typeof repository.nameWithOwner === "string" && + REPOSITORY_NAME_PATTERN.test(repository.nameWithOwner) + ? repository.nameWithOwner + : null; + let derived: string | null = null; + if ( + typeof repository.name === "string" && + /^[A-Za-z0-9_.-]+$/u.test(repository.name) && + typeof headRepositoryOwner === "object" && + headRepositoryOwner !== null && + !Array.isArray(headRepositoryOwner) + ) { + const login = (headRepositoryOwner as Record).login; + if (typeof login === "string" && /^[A-Za-z0-9_.-]+$/u.test(login)) { + derived = `${login}/${repository.name}`; + } + } + if (direct && derived && direct !== derived) return null; + return direct ?? derived; +} + function fetchPrRevisionSnapshot(repo: string, number: number): PrRevisionSnapshot | null { const value = ghJson([ "pr", @@ -1662,11 +1722,11 @@ function fetchPrRevisionSnapshot(repo: string, number: number): PrRevisionSnapsh "--repo", repo, "--json", - "title,body,state,isDraft,mergeable,mergeStateStatus,headRefOid,baseRefOid,headRefName,baseRefName,headRepository", + "title,body,state,isDraft,mergeable,mergeStateStatus,headRefOid,baseRefOid,headRefName,baseRefName,headRepository,headRepositoryOwner", ]); if (typeof value !== "object" || value === null || Array.isArray(value)) return null; const record = value as Record; - const headRepository = record.headRepository; + const headRepository = parseHeadRepository(record.headRepository, record.headRepositoryOwner); if ( typeof record.title !== "string" || typeof record.body !== "string" || @@ -1678,10 +1738,7 @@ function fetchPrRevisionSnapshot(repo: string, number: number): PrRevisionSnapsh typeof record.baseRefOid !== "string" || typeof record.headRefName !== "string" || typeof record.baseRefName !== "string" || - typeof headRepository !== "object" || - headRepository === null || - Array.isArray(headRepository) || - typeof (headRepository as Record).nameWithOwner !== "string" + !headRepository ) { return null; } @@ -1696,7 +1753,7 @@ function fetchPrRevisionSnapshot(repo: string, number: number): PrRevisionSnapsh baseRefOid: record.baseRefOid, headRefName: record.headRefName, baseRefName: record.baseRefName, - headRepository: (headRepository as Record).nameWithOwner as string, + headRepository, }; } @@ -1773,7 +1830,7 @@ function main(): void { "--repo", repo, "--json", - "number,title,url,body,files,statusCheckRollup,state,isDraft,mergeable,mergeStateStatus,headRefOid,baseRefOid,headRefName,baseRefName,headRepository,author", + "number,title,url,body,files,statusCheckRollup,state,isDraft,mergeable,mergeStateStatus,headRefOid,baseRefOid,headRefName,baseRefName,headRepository,headRepositoryOwner,author", ]) as { number: number; title: string; @@ -1789,7 +1846,8 @@ function main(): void { baseRefOid: string; headRefName: string; baseRefName: string; - headRepository: { nameWithOwner: string }; + headRepository: { name: string; nameWithOwner: string }; + headRepositoryOwner: { login: string } | null; author: PrIdentity | null; } | null; @@ -1797,13 +1855,18 @@ function main(): void { console.error(`Failed to fetch PR #${prNumber} from ${repo}`); process.exit(1); } + const headRepository = parseHeadRepository(prData.headRepository, prData.headRepositoryOwner); + if (!headRepository) { + console.error(`Failed to resolve PR #${prNumber} head repository from ${repo}`); + process.exit(1); + } const ci = checkCi(prData.statusCheckRollup, repo, { number: prNumber, headSha: prData.headRefOid, baseSha: prData.baseRefOid, headRefName: prData.headRefName, - headRepository: prData.headRepository.nameWithOwner, + headRepository, }); const coderabbit = checkCodeRabbit(repo, prNumber); const riskyCodeTested = checkRiskyCodeTested(prData.files ?? []); @@ -1832,7 +1895,7 @@ function main(): void { baseRefOid: prData.baseRefOid, headRefName: prData.headRefName, baseRefName: prData.baseRefName, - headRepository: prData.headRepository.nameWithOwner, + headRepository, }, currentRevision, currentBaseSha, diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 4421d9ebeb1..68a9ea58752 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -48,6 +48,16 @@ on: required: false default: "" type: string + checkout_repository: + description: PR head repository selected by the controller. + required: false + default: "" + type: string + controller_check_id: + description: Controller-owned check run that authorizes this exact E2E run. + required: false + default: "" + type: string base_sha: description: PR base commit selected by the controller. required: false @@ -70,6 +80,7 @@ on: type: string permissions: + checks: read contents: read pull-requests: read @@ -189,8 +200,69 @@ jobs: runner_routing="$(jq -cn --arg standard "ubuntu-latest" --arg larger "${larger_runner}" '{"channels-stop-start-hermes":$larger,"channels-stop-start-openclaw":$standard,"common-egress-agent":$larger,"hermes-dashboard":$larger,"hermes-discord":$larger,"hermes-e2e":$larger,"hermes-inference-switch":$larger,"hermes-shields-config":$larger,"mcp-bridge-deepagents":$larger,"mcp-bridge-hermes":$larger,"mcp-bridge-openclaw":$standard,"rebuild-hermes":$larger,"rebuild-hermes-stale-base":$larger,"security-posture-hermes":$larger,"security-posture-openclaw":$standard}')" printf 'runner_routing=%s\n' "${runner_routing}" >> "${GITHUB_OUTPUT}" + - name: Authenticate controller dispatch + if: ${{ inputs.checkout_sha != '' }} + env: + ACTOR: ${{ github.actor }} + BASE_SHA: ${{ inputs.base_sha }} + CHECKOUT_SHA: ${{ inputs.checkout_sha }} + CONTROLLER_CHECK_ID: ${{ inputs.controller_check_id }} + CORRELATION_ID: ${{ inputs.correlation_id }} + GITHUB_TOKEN: ${{ github.token }} + JOBS: ${{ inputs.jobs }} + PLAN_HASH: ${{ inputs.plan_hash }} + PR_NUMBER: ${{ inputs.pr_number }} + RUN_ATTEMPT: ${{ github.run_attempt }} + RUN_ID: ${{ github.run_id }} + TARGETS: ${{ inputs.targets }} + shell: bash + run: | + set -euo pipefail + [[ "$ACTOR" == "github-actions[bot]" ]] || { echo "::error::PR E2E must be dispatched by the trusted controller"; exit 1; } + [[ "$RUN_ATTEMPT" == "1" ]] || { echo "::error::controller-authorized PR E2E cannot be rerun"; exit 1; } + [[ "$CONTROLLER_CHECK_ID" =~ ^[1-9][0-9]*$ ]] || { echo "::error::controller_check_id must be a positive integer"; exit 1; } + expected_external_id="nemoclaw-pr-e2e:v2:${PR_NUMBER}:${CHECKOUT_SHA}:${BASE_SHA}" + expected_run_url="https://github.com/${GITHUB_REPOSITORY}/actions/runs/${RUN_ID}" + jobs_summary="${JOBS//,/, }" + targets_summary="${TARGETS//,/, }" + expected_summary="Risk plan ${PLAN_HASH} selected jobs: ${jobs_summary:-none}; targets: ${targets_summary:-none}." + check_json="" + for _attempt in {1..15}; do + if check_json="$(curl --fail --silent --show-error --proto '=https' \ + --header "Authorization: Bearer ${GITHUB_TOKEN}" \ + --header "Accept: application/vnd.github+json" \ + --header "X-GitHub-Api-Version: 2022-11-28" \ + "https://api.github.com/repos/${GITHUB_REPOSITORY}/check-runs/${CONTROLLER_CHECK_ID}")" && + [[ "$(jq -r '.details_url // ""' <<< "$check_json")" == "$expected_run_url" ]]; then + break + fi + check_json="" + sleep 2 + done + [[ -n "$check_json" ]] || { echo "::error::trusted controller authorization was not published for this run"; exit 1; } + jq -e \ + --argjson check_id "$CONTROLLER_CHECK_ID" \ + --arg external_id "$expected_external_id" \ + --arg head_sha "$CHECKOUT_SHA" \ + --arg run_url "$expected_run_url" \ + --arg summary "$expected_summary" \ + ' + .id == $check_id and + .name == "E2E / PR Gate Coordination" and + .app.id == 15368 and + .app.slug == "github-actions" and + .head_sha == $head_sha and + .external_id == $external_id and + .status == "in_progress" and + .conclusion == null and + .details_url == $run_url and + .output.summary == $summary + ' <<< "$check_json" >/dev/null || + { echo "::error::controller check does not authorize this exact E2E run"; exit 1; } + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} fetch-depth: 0 persist-credentials: false @@ -199,6 +271,7 @@ jobs: if: ${{ inputs.checkout_sha != '' }} env: BASE_SHA: ${{ inputs.base_sha }} + CHECKOUT_REPOSITORY: ${{ inputs.checkout_repository }} CHECKOUT_SHA: ${{ inputs.checkout_sha }} EXPECTED_WORKFLOW_SHA: ${{ inputs.workflow_sha }} GITHUB_TOKEN: ${{ github.token }} @@ -213,6 +286,7 @@ jobs: run: | set -euo pipefail [[ "$WORKFLOW_EVENT" == "workflow_dispatch" && "$WORKFLOW_REF" == "refs/heads/main" ]] || { echo "::error::PR E2E runs must be dispatched from main"; exit 1; } + [[ "$CHECKOUT_REPOSITORY" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]] || { echo "::error::checkout_repository must be an owner/repository name"; exit 1; } [[ "$CHECKOUT_SHA" =~ ^[a-f0-9]{40}$ ]] || { echo "::error::checkout_sha must be a lowercase 40-character SHA"; exit 1; } [[ "$BASE_SHA" =~ ^[a-f0-9]{40}$ ]] || { echo "::error::base_sha must be a lowercase 40-character SHA"; exit 1; } [[ "$EXPECTED_WORKFLOW_SHA" =~ ^[a-f0-9]{40}$ && "$WORKFLOW_SHA" == "$EXPECTED_WORKFLOW_SHA" ]] || { echo "::error::workflow_sha must match the trusted workflow commit"; exit 1; } @@ -229,7 +303,7 @@ jobs: --header "X-GitHub-Api-Version: 2022-11-28" \ "https://api.github.com/repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}")" [[ "$(jq -r '.state' <<< "$pull_json")" == "open" ]] || { echo "::error::pull request must still be open"; exit 1; } - [[ "$(jq -r '.head.repo.full_name // ""' <<< "$pull_json")" == "$GITHUB_REPOSITORY" ]] || { echo "::error::pull request must originate from this repository"; exit 1; } + [[ "$(jq -r '.head.repo.full_name // ""' <<< "$pull_json")" == "$CHECKOUT_REPOSITORY" ]] || { echo "::error::checkout_repository must match the PR head repository"; exit 1; } [[ "$(jq -r '.head.sha' <<< "$pull_json")" == "$CHECKOUT_SHA" ]] || { echo "::error::checkout_sha must match the PR SHA"; exit 1; } [[ "$(jq -r '.base.sha' <<< "$pull_json")" == "$BASE_SHA" ]] || { echo "::error::base_sha must match the PR base commit"; exit 1; } @@ -343,6 +417,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -536,6 +611,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -577,6 +653,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -843,6 +920,7 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -978,6 +1056,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1082,6 +1161,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1157,6 +1237,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1194,6 +1275,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1238,6 +1320,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1289,6 +1372,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1366,6 +1450,7 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1422,6 +1507,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1508,6 +1594,7 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1566,6 +1653,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1621,6 +1709,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1664,6 +1753,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1722,6 +1812,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1790,6 +1881,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1834,6 +1926,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1885,6 +1978,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -1942,6 +2036,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -2015,6 +2110,7 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -2082,6 +2178,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -2355,6 +2452,7 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -2430,6 +2528,7 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -2505,6 +2604,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -2606,6 +2706,7 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -2690,6 +2791,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -2761,6 +2863,7 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -2812,6 +2915,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -2882,6 +2986,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -2996,6 +3101,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -3102,6 +3208,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -3165,6 +3272,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -3228,6 +3336,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -3271,6 +3380,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -3333,6 +3443,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -3391,6 +3502,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -3456,6 +3568,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -3522,6 +3635,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -3577,6 +3691,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -3631,6 +3746,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -3729,6 +3845,7 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -3793,6 +3910,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -3906,6 +4024,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -3959,6 +4078,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4013,6 +4133,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4065,6 +4186,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4116,6 +4238,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4224,6 +4347,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4274,6 +4398,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4333,6 +4458,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4377,6 +4503,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4421,6 +4548,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4471,6 +4599,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4529,6 +4658,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4576,6 +4706,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4627,6 +4758,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4694,6 +4826,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4762,6 +4895,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4839,6 +4973,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -4934,6 +5069,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -5012,6 +5148,7 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -5058,6 +5195,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -5099,6 +5237,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -5154,6 +5293,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -5223,6 +5363,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -5315,6 +5456,7 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -5400,6 +5542,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -5464,6 +5607,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -5521,6 +5665,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -5585,6 +5730,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -5662,6 +5808,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false @@ -5847,6 +5994,7 @@ jobs: - name: Checkout scorecard builders uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + repository: ${{ inputs.checkout_repository || github.repository }} ref: ${{ inputs.checkout_sha || github.sha }} persist-credentials: false sparse-checkout: | diff --git a/.github/workflows/pr-e2e-gate.yaml b/.github/workflows/pr-e2e-gate.yaml index 8aed89d1a41..2d196c3fc36 100644 --- a/.github/workflows/pr-e2e-gate.yaml +++ b/.github/workflows/pr-e2e-gate.yaml @@ -22,10 +22,9 @@ on: operation: description: E2E gate action to perform. required: true - default: approve-fork-e2e-skip + default: run-control-plane type: choice options: - - approve-fork-e2e-skip - run-control-plane pr_number: description: Pull request number for the selected E2E gate action. @@ -40,14 +39,9 @@ on: required: true type: string review_reason: - description: Why this fork PR may skip credentialed E2E or this internal PR may run control-plane E2E. + description: Why this internal PR may run control-plane E2E. required: true type: string - evidence_url: - description: Fork credentialed-E2E skip only; optional Actions run URL. Ignored by run-control-plane, whose evidence comes from the dispatched jobs. - required: false - default: "" - type: string permissions: {} @@ -135,7 +129,7 @@ jobs: --timeout-seconds 21480 cancel-superseded: - if: ${{ github.run_attempt == 1 && github.event_name == 'pull_request_target' && github.repository == 'NVIDIA/NemoClaw' && github.event.pull_request.head.repo.full_name == github.repository && (github.event.action != 'edited' || github.event.changes.base != null) }} + if: ${{ github.run_attempt == 1 && github.event_name == 'pull_request_target' && github.repository == 'NVIDIA/NemoClaw' && (github.event.action != 'edited' || github.event.changes.base != null) }} runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -176,14 +170,11 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 330 outputs: - control_plane_approval_mode: ${{ steps.start.outputs.control_plane_approval_mode }} - control_plane_approval_pr_number: ${{ steps.start.outputs.control_plane_approval_pr_number }} - control_plane_approval_head_sha: ${{ steps.start.outputs.control_plane_approval_head_sha }} - control_plane_approval_base_sha: ${{ steps.start.outputs.control_plane_approval_base_sha }} - fork_skip_mode: ${{ steps.start.outputs.fork_skip_mode }} - fork_skip_pr_number: ${{ steps.start.outputs.fork_skip_pr_number }} - fork_skip_head_sha: ${{ steps.start.outputs.fork_skip_head_sha }} - fork_skip_base_sha: ${{ steps.start.outputs.fork_skip_base_sha }} + approval_mode: ${{ steps.start.outputs.approval_mode }} + approval_environment: ${{ steps.start.outputs.approval_environment }} + approval_pr_number: ${{ steps.start.outputs.approval_pr_number }} + approval_head_sha: ${{ steps.start.outputs.approval_head_sha }} + approval_base_sha: ${{ steps.start.outputs.approval_base_sha }} permissions: actions: write checks: write @@ -406,14 +397,14 @@ jobs: if: ${{ always() && steps.workspace.outputs.work_dir != '' }} run: rm -rf -- "${{ steps.workspace.outputs.work_dir }}" - approve-internal-e2e: - name: Approve credentialed E2E for internal PR + approve-e2e: + name: Approve credentialed E2E for reviewed PR needs: coordinate - if: ${{ needs.coordinate.result == 'success' && needs.coordinate.outputs.control_plane_approval_mode != '' && github.run_attempt == 1 }} + if: ${{ needs.coordinate.result == 'success' && needs.coordinate.outputs.approval_mode != '' && github.run_attempt == 1 }} runs-on: ubuntu-latest timeout-minutes: 330 environment: - name: approve-credentialed-e2e-for-internal-pr + name: ${{ needs.coordinate.outputs.approval_environment }} deployment: false permissions: actions: write @@ -421,7 +412,7 @@ jobs: contents: read pull-requests: read concurrency: - group: pr-e2e-gate-${{ github.repository }}-${{ needs.coordinate.outputs.control_plane_approval_pr_number }}-${{ needs.coordinate.outputs.control_plane_approval_head_sha }}-${{ needs.coordinate.outputs.control_plane_approval_base_sha }} + group: pr-e2e-gate-${{ github.repository }}-${{ needs.coordinate.outputs.approval_pr_number }}-${{ needs.coordinate.outputs.approval_head_sha }}-${{ needs.coordinate.outputs.approval_base_sha }} queue: max cancel-in-progress: false steps: @@ -449,36 +440,45 @@ jobs: printf 'work_dir=%s\n' "$work_dir" >> "$GITHUB_OUTPUT" - id: start - name: Start approved internal E2E + name: Start approved E2E env: + APPROVAL_MODE: ${{ needs.coordinate.outputs.approval_mode }} APPROVAL_RUN_ATTEMPT: ${{ github.run_attempt }} APPROVAL_RUN_ID: ${{ github.run_id }} - EXPECTED_BASE_SHA: ${{ needs.coordinate.outputs.control_plane_approval_base_sha }} - EXPECTED_HEAD_SHA: ${{ needs.coordinate.outputs.control_plane_approval_head_sha }} + EXPECTED_BASE_SHA: ${{ needs.coordinate.outputs.approval_base_sha }} + EXPECTED_HEAD_SHA: ${{ needs.coordinate.outputs.approval_head_sha }} GATE_RUN_ID: ${{ github.run_id }} GITHUB_TOKEN: ${{ github.token }} - PR_NUMBER: ${{ needs.coordinate.outputs.control_plane_approval_pr_number }} + PR_NUMBER: ${{ needs.coordinate.outputs.approval_pr_number }} WORKFLOW_RUN_ATTEMPT: ${{ github.run_attempt }} WORKFLOW_SHA: ${{ github.workflow_sha }} WORK_DIR: ${{ steps.workspace.outputs.work_dir }} - run: >- - node --experimental-strip-types tools/e2e/pr-e2e-gate.mts - --mode start-approved-control-plane - --pr "$PR_NUMBER" - --head "$EXPECTED_HEAD_SHA" - --base "$EXPECTED_BASE_SHA" - --workflow-sha "$WORKFLOW_SHA" - --approval-run-id "$APPROVAL_RUN_ID" - --approval-run-attempt "$APPROVAL_RUN_ATTEMPT" - --gate-run-id "$GATE_RUN_ID" - --workflow-run-attempt "$WORKFLOW_RUN_ATTEMPT" - --work-dir "$WORK_DIR" + run: | + set -euo pipefail + case "$APPROVAL_MODE" in + start-approved-control-plane | start-approved-fork) ;; + *) + echo "::error::approval mode is invalid" >&2 + exit 1 + ;; + esac + node --experimental-strip-types tools/e2e/pr-e2e-gate.mts \ + --mode "$APPROVAL_MODE" \ + --pr "$PR_NUMBER" \ + --head "$EXPECTED_HEAD_SHA" \ + --base "$EXPECTED_BASE_SHA" \ + --workflow-sha "$WORKFLOW_SHA" \ + --approval-run-id "$APPROVAL_RUN_ID" \ + --approval-run-attempt "$APPROVAL_RUN_ATTEMPT" \ + --gate-run-id "$GATE_RUN_ID" \ + --workflow-run-attempt "$WORKFLOW_RUN_ATTEMPT" \ + --work-dir "$WORK_DIR" - name: Upload approved risk plan if: ${{ always() && steps.workspace.outputs.work_dir != '' }} uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: - name: pr-e2e-approved-risk-plan-${{ needs.coordinate.outputs.control_plane_approval_head_sha }} + name: pr-e2e-approved-risk-plan-${{ needs.coordinate.outputs.approval_head_sha }} path: ${{ steps.workspace.outputs.work_dir }}/risk-plan.json if-no-files-found: ignore retention-days: 14 @@ -609,104 +609,3 @@ jobs: - name: Remove private workspace if: ${{ always() && steps.workspace.outputs.work_dir != '' }} run: rm -rf -- "${{ steps.workspace.outputs.work_dir }}" - - approve-fork-e2e-skip: - name: Approve credentialed E2E skip for fork PR - needs: coordinate - if: ${{ needs.coordinate.result == 'success' && needs.coordinate.outputs.fork_skip_mode != '' && github.run_attempt == 1 }} - runs-on: ubuntu-latest - timeout-minutes: 10 - environment: - name: approve-credentialed-e2e-skip-for-fork-pr - deployment: false - permissions: - actions: read - checks: write - contents: read - pull-requests: read - concurrency: - group: pr-e2e-gate-${{ github.repository }}-${{ needs.coordinate.outputs.fork_skip_pr_number }}-${{ needs.coordinate.outputs.fork_skip_head_sha }}-${{ needs.coordinate.outputs.fork_skip_base_sha }} - queue: max - cancel-in-progress: false - steps: - - name: Checkout controller - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - with: - ref: ${{ github.workflow_sha }} - persist-credentials: false - - - name: Setup Node - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 - with: - node-version: "22" - - - name: Install controller dependencies - run: npm ci --ignore-scripts --no-audit --no-fund - - - name: Record approved credentialed E2E skip - env: - APPROVAL_RUN_ATTEMPT: ${{ github.run_attempt }} - APPROVAL_RUN_ID: ${{ github.run_id }} - EXPECTED_BASE_SHA: ${{ needs.coordinate.outputs.fork_skip_base_sha }} - EXPECTED_HEAD_SHA: ${{ needs.coordinate.outputs.fork_skip_head_sha }} - GITHUB_TOKEN: ${{ github.token }} - PR_NUMBER: ${{ needs.coordinate.outputs.fork_skip_pr_number }} - WORKFLOW_SHA: ${{ github.workflow_sha }} - run: >- - node --experimental-strip-types tools/e2e/pr-e2e-gate.mts - --mode record-approved-fork-e2e-skip - --pr "$PR_NUMBER" - --head "$EXPECTED_HEAD_SHA" - --base "$EXPECTED_BASE_SHA" - --workflow-sha "$WORKFLOW_SHA" - --approval-run-id "$APPROVAL_RUN_ID" - --approval-run-attempt "$APPROVAL_RUN_ATTEMPT" - - record-fork-e2e-skip: - name: Record credentialed E2E skip for fork PR - if: ${{ github.run_attempt == 1 && github.event_name == 'workflow_dispatch' && github.repository == 'NVIDIA/NemoClaw' && github.ref == 'refs/heads/main' && inputs.operation == 'approve-fork-e2e-skip' }} - runs-on: ubuntu-latest - timeout-minutes: 10 - permissions: - checks: write - contents: read - pull-requests: read - concurrency: - group: pr-e2e-gate-${{ github.repository }}-${{ inputs.pr_number }}-${{ inputs.expected_head_sha }}-${{ inputs.expected_base_sha }} - queue: max - cancel-in-progress: false - steps: - - name: Checkout controller - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - with: - ref: ${{ github.workflow_sha }} - persist-credentials: false - - - name: Setup Node - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 - with: - node-version: "22" - - - name: Install controller dependencies - run: npm ci --ignore-scripts --no-audit --no-fund - - - name: Record credentialed E2E skip - env: - EVIDENCE_URL: ${{ inputs.evidence_url }} - EXPECTED_BASE_SHA: ${{ inputs.expected_base_sha }} - EXPECTED_HEAD_SHA: ${{ inputs.expected_head_sha }} - GITHUB_TOKEN: ${{ github.token }} - MAINTAINER: ${{ github.triggering_actor }} - PR_NUMBER: ${{ inputs.pr_number }} - REVIEW_REASON: ${{ inputs.review_reason }} - WORKFLOW_SHA: ${{ github.workflow_sha }} - run: >- - node --experimental-strip-types tools/e2e/pr-e2e-gate.mts - --mode record-fork-e2e-skip - --pr "$PR_NUMBER" - --head "$EXPECTED_HEAD_SHA" - --base "$EXPECTED_BASE_SHA" - --workflow-sha "$WORKFLOW_SHA" - --maintainer "$MAINTAINER" - --reason "$REVIEW_REASON" - --evidence-url "$EVIDENCE_URL" diff --git a/test/e2e/README.md b/test/e2e/README.md index 9c6cc982782..9546fed156b 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -426,8 +426,8 @@ the deterministic risk plan. Runtime families and changes to workflow-wired live tests select canonical selectors from the trusted `e2e.yaml` inventory independently of advisor output. Ordinary internal changes execute those focused selections. -Gate initialization, CI coordination, protected approval, and manual fork-skip -recording share one non-cancelling FIFO concurrency group for the exact +Gate initialization, CI coordination, and protected approval share one +non-cancelling FIFO concurrency group for the exact repository, PR number, PR SHA, and base SHA. `queue: max` keeps pending jobs for that exact identity instead of replacing them, up to GitHub's 100-job bound. Before the controller creates or updates coordination for the current revision, @@ -478,11 +478,12 @@ control-plane change, or second advance fails closed. The accepted `main` commit is recorded as the workflow SHA and passed as `workflow_sha`. Before matrix or secret-bearing jobs can run, `e2e.yaml` requires `github.workflow_sha` to match that accepted commit. Each selected job checks -out `checkout_sha`. The same validation verifies that the PR remains open, -belongs to `NVIDIA/NemoClaw`, and still has both the dispatched head and base -commits. The dispatch includes selected jobs, allowlisted typed targets, and -valid plan and correlation metadata. Controller-bound targets are restricted -to the trusted allowlist. Before checking out PR code, the trusted workflow +out `checkout_sha` from the live PR head repository. The same validation +verifies that the PR remains open in `NVIDIA/NemoClaw`, the checkout repository +is still the PR head repository, and both the dispatched head and base commits +still match. The dispatch includes selected jobs, allowlisted typed targets, +and valid plan and correlation metadata. Controller-bound targets are +restricted to the trusted allowlist. Before checking out PR code, the trusted workflow projects each controller-selected target into a fixed target ID and hosted runner mapping. The generated live matrix must exactly match those trusted IDs and runners, and only the trusted projection can configure credential-bearing @@ -498,7 +499,7 @@ trusted controller and observer boundaries leaves coordination in progress with `E2E reviewer authorization required to run E2E`. The native required job keeps waiting for the authorization flow. No selected job or target runs and no repository secret is exposed. The same controller run starts `Approve -credentialed E2E for internal PR`, which waits on the protected +credentialed E2E for reviewed PR`, which waits on the protected `approve-credentialed-e2e-for-internal-pr` environment. With `deployment: false`, the job does not create a deployment record. After reviewing the exact head SHA, base SHA, and risk plan as described below, an environment reviewer @@ -515,7 +516,13 @@ the first attempt of the trusted `workflow_run` controller. It then revalidates the internal repository origin, open PR, PR SHA and base SHA, risk plan, matching pending coordination state, compatible trusted controller commit, and final live revision. It updates coordination to `Running E2E check(s)` -and dispatches the selected jobs and targets in one workflow run. +and dispatches the selected jobs and targets in one workflow run. The child +workflow receives the controller-owned coordination check ID. Before checking +out the PR revision, it requires a GitHub Actions dispatch and verifies that +the exact check is owned by the GitHub Actions app, matches the PR head and base +identity, names the selected plan, and links to the current child run. A direct +manual dispatch that supplies otherwise-valid PR inputs cannot forge that +one-run authorization and fails before checkout. The manual maintainer path remains available as a fallback. A repository maintainer or administrator chooses **Run workflow** on `main`, selects @@ -558,37 +565,33 @@ candidates fails closed. Selected-job product or assertion failures, evidence policy or integrity failures, schema or identity mismatches, traversal or provenance failures, reconciliation, controller errors, unknown states, and failures recorded before retry reasons existed -remain terminal for that PR/base SHA pair. Fork approval failures are not retried by -PR CI; follow the protected or manual skip path, or update the PR to create a -new head. Update the PR and run fresh CI for the other terminal outcomes. The +remain terminal for that PR/base SHA pair. Update the PR and run fresh CI for +terminal outcomes. The normal wait, evidence download, and finish path is the only path that can record success; the authorization itself cannot make the gate green. A changed head or base requires a new authorization. -A fork revision that selects jobs or typed targets completes coordination as -failed while the native required job waits for the skip-approval flow. The -controller does not dispatch the selected credential-bearing jobs or targets -or expose repository secrets. -Non-secret PR CI remains required. The failed coordination summary -embeds an explicit link to the same `E2E / PR Gate Controller` run; maintainers -follow that link rather than relying on the coordination check's **Details** -destination. The coordination check publishes only allowlisted skip-approval -metadata for its PR number, mode, head SHA, and base SHA. The native required -job recognizes the approval-required title as an intermediate waiting state. -That controller run starts -`Approve credentialed E2E skip for fork PR`, which waits on the protected -`approve-credentialed-e2e-skip-for-fork-pr` environment. With -`deployment: false`, the job does not create a deployment record. A maintainer -or delegated E2E reviewer reviews the exact head SHA, base SHA, and risk plan as -described below, opens the linked run, chooses **Review deployments**, selects -that environment, and approves it. The approval records that the selected -credential-bearing jobs and targets will not run; it does not authorize fork -code to run with repository secrets. The comment is optional, and the workflow -reads both the reviewer and comment from GitHub's run approval history rather -than accepting an actor supplied by the job. +A fork revision that selects jobs or typed targets leaves coordination in +progress with `E2E reviewer authorization required to run fork E2E`. Non-secret +PR CI remains required. Before approval, the controller does not dispatch the +credential-bearing work or expose repository secrets. The coordination summary +links to the same `E2E / PR Gate Controller` run and publishes the exact PR +number, head repository, head SHA, base SHA, plan, jobs, and targets under +review. + +That controller run starts `Approve credentialed E2E for reviewed PR`, which +waits on the protected `approve-credentialed-e2e-for-fork-pr` environment. +With `deployment: false`, the job does not create a deployment record. A +maintainer or delegated E2E reviewer reviews the exact repository, head SHA, +base SHA, and risk plan, opens the linked run, chooses **Review deployments**, +selects that environment, and approves it. This approval authorizes the exact +fork revision to run the selected work with E2E credentials. It is not a skip +and cannot make the gate pass by itself. The workflow reads the reviewer and +optional comment from GitHub's run approval history rather than accepting an +actor supplied by the job. Before rollout, create both `approve-credentialed-e2e-for-internal-pr` and -`approve-credentialed-e2e-skip-for-fork-pr` in the repository. Configure each +`approve-credentialed-e2e-for-fork-pr` in the repository. Configure each environment with one or more required reviewers. Protected-environment reviewers are the authorization allowlist and may have repository read access without merge rights. Do not add environment secrets, variables, or custom @@ -598,47 +601,33 @@ approval history. Restrict deployment branches to protected `main`. Before either decision, verify the exact head SHA, base SHA, and selected jobs and targets in the coordination check summary and the `pr-e2e-risk-plan-` artifact from the linked controller run. The -internal approval job receives only its job-scoped token after approval and -executes the trusted controller from `main`; the fork approval job records a -skip and runs no PR-controlled code. If **Review deployments** is absent, the -environment may be missing or unprotected, or the run may no longer be waiting. -Configure the environment, update the PR to create a new head, and trigger fresh -upstream PR CI to create a new gate run, or use the corresponding manual -maintainer fallback. GitHub approval +approval job receives only its job-scoped token after approval and executes the +trusted controller from `main`. The trusted E2E workflow definition stays on +`main`, while every PR-code checkout is pinned to the approved head repository +and SHA. If **Review deployments** is absent, the environment may be missing or +unprotected, or the run may no longer be waiting. Configure the environment, +update the PR to create a new head, and trigger fresh upstream PR CI to create a +new gate run. GitHub approval history is not bound to a run attempt, so the controller rejects reruns of an approval run. Approval concurrency is bound to the exact PR SHA and base SHA. A newer revision creates a separate approval request, while an obsolete request cannot authorize it. -For the fork button path, the controller requires a first-attempt, in-progress run +For the fork approval path, the controller requires a first-attempt, in-progress run of this exact workflow on `main`, at the trusted workflow SHA and with the `workflow_run` event. It requires exactly one approved review that names only the exact environment. The environment's required-reviewer configuration is the authority for this protected path. The shared resolver revalidates -the open PR, repository origin, PR SHA and base SHA, deterministic plan, -matching failed coordination check, and that the controller commit is either +the open PR, head repository, PR SHA and base SHA, deterministic plan, +matching pending coordination check, and that the controller commit is either still `main` or has only a compatible safe descendant as described above. Immediately before -recording success, it reads the live PR again and requires the same PR SHA and -base SHA. The result records the reviewer, bounded optional comment, validated -approval-run URL, plan hash, and jobs and targets that did not run. The -successful skip coordination check is titled -`Credentialed E2E skipped for fork PR — approved by @` and begins -with `Outcome: APPROVED SKIP — credentialed E2E did not run.` It never claims -that the selected checks passed. The native required job mirrors this -approved-skip success. - -The manual fork skip approval on `main` remains available as a fallback. Choose -`approve-fork-e2e-skip` and provide the PR number, current `expected_head_sha`, -current `expected_base_sha`, a 10–500-character `review_reason`, and optionally -an Actions run URL in the exact form -`https://github.com/NVIDIA/NemoClaw/actions/runs/`. Leave -`evidence_url` blank when no supporting run exists. PR, issue, comment, job, and -external URLs are rejected. The controller validates the optional URL's shape -but does not inspect that run's contents. It applies the same PR, role, plan, -failed-check, compatible-`main`, and final stale-revision checks. Any new commit -receives a different gate and requires a new decision; a base change also -invalidates the decision. +dispatch, it reads the live PR again and requires the same head repository, PR +SHA, and base SHA. The trusted workflow runs the selected jobs and targets, +downloads their evidence, and verifies its identity and outcome. Only passing +evidence for every selected item completes coordination successfully. Failed, +missing, skipped, pending, or mismatched evidence keeps the required gate from +passing. Any new commit or base change requires a new approval. The Vitest reporter writes one `risk-signal.json` for each selected job shard and typed target. Typed targets bind the signal identity to the exact matrix ID diff --git a/test/e2e/docs/README.md b/test/e2e/docs/README.md index c2c463a044d..4ea968b8a34 100644 --- a/test/e2e/docs/README.md +++ b/test/e2e/docs/README.md @@ -255,9 +255,9 @@ test/e2e/ matches are drawn only from the trusted controller workflow and scripts. Other or mixed internal control-plane revisions require a maintainer-authorized run for the PR SHA; only - its verified evidence can pass coordination. Risky forks retain the audited - credentialed-E2E skip approval. See [NemoClaw E2E CI](../README.md) for the - full lifecycle. + its verified evidence can pass coordination. Risky forks require protected + approval before the trusted workflow runs their exact repository and SHA with + E2E credentials. See [NemoClaw E2E CI](../README.md) for the full lifecycle. - `.github/workflows/e2e.yaml` runs selected or all supported live E2E targets and uploads an explicit artifact allowlist with @@ -276,10 +276,13 @@ test/e2e/ These per-target timing summaries are artifact evidence only. The Slack and GitHub scorecard timing comparison remains scoped to the dedicated `cloud-onboard` artifact. - PR E2E dispatches validate the PR SHA and controller metadata before - preparation, attach `test/e2e/risk-signal-reporter.ts` to live Vitest - invocations, and suppress PR reporting and scorecards. The workflow boundary - requires every selected job shard to upload its evidence artifact. + PR E2E dispatches authenticate the controller-owned coordination check before + checking out the PR revision, then validate the PR SHA and controller + metadata before preparation. Direct manual dispatches cannot reuse the PR + input shape to run fork code. Selected runs attach + `test/e2e/risk-signal-reporter.ts` to live Vitest invocations and suppress PR + reporting and scorecards. The workflow boundary requires every selected job + shard to upload its evidence artifact. - `.github/workflows/e2e-branch-validation.yaml`, `macos-e2e.yaml`, `wsl-e2e.yaml`, and `regression-e2e.yaml` call focused E2E targets directly for their E2E coverage. Individual repository-hosted targets, including diff --git a/test/e2e/support/e2e-operations-workflow-boundary.test.ts b/test/e2e/support/e2e-operations-workflow-boundary.test.ts index 31c41e523c7..8c2e4c633ce 100644 --- a/test/e2e/support/e2e-operations-workflow-boundary.test.ts +++ b/test/e2e/support/e2e-operations-workflow-boundary.test.ts @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { spawnSync } from "node:child_process"; import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -104,41 +105,172 @@ describe("E2E operations workflow boundary", () => { it("rejects controller protocol and PR validation drift", () => { const workflow = readE2eOperationsWorkflow(); delete workflow.on?.workflow_dispatch?.inputs?.base_sha; + delete workflow.on?.workflow_dispatch?.inputs?.checkout_repository; + delete workflow.on?.workflow_dispatch?.inputs?.controller_check_id; delete workflow.on?.workflow_dispatch?.inputs?.workflow_sha; delete workflow.on?.workflow_dispatch?.inputs?.plan_hash; + delete (workflow.permissions as Record).checks; workflow.env!.NEMOCLAW_E2E_PLAN_HASH = "${{ inputs.checkout_sha }}"; workflow.concurrency!["cancel-in-progress"] = false; + const authentication = workflow.jobs["generate-matrix"].steps!.find( + (step) => step.name === "Authenticate controller dispatch", + )!; + delete authentication.env?.CONTROLLER_CHECK_ID; + authentication.if = "${{ inputs.plan_hash != '' }}"; + authentication.run = "echo unchecked"; const validation = workflow.jobs["generate-matrix"].steps!.find( (step) => step.name === "Validate controller dispatch", )!; delete validation.env?.BASE_SHA; + delete validation.env?.CHECKOUT_REPOSITORY; delete validation.env?.EXPECTED_WORKFLOW_SHA; validation.if = "${{ inputs.plan_hash != '' }}"; validation.run = "echo unchecked"; const checkout = workflow.jobs["generate-matrix"].steps!.find((step) => step.uses?.startsWith("actions/checkout@"), )!; + delete checkout.with!.repository; checkout.with!.ref = "${{ github.sha }}"; expect(validateE2eOperationsWorkflow(workflow)).toEqual( expect.arrayContaining([ "workflow_dispatch base_sha must be an optional string with an empty default", + "workflow_dispatch checkout_repository must be an optional string with an empty default", + "workflow_dispatch controller_check_id must be an optional string with an empty default", "workflow_dispatch workflow_sha must be an optional string with an empty default", "workflow_dispatch plan_hash must be an optional string with an empty default", "E2E workflow must bind NEMOCLAW_E2E_PLAN_HASH to controller metadata", "PR E2E concurrency must cancel obsolete runs", + "E2E workflow must grant read-only check access for controller authentication", + "Controller authentication must be activated only by checkout_sha", + "Controller authentication must bind CONTROLLER_CHECK_ID", + 'Controller authentication must retain "$ACTOR" == "github-actions[bot]"', + "Controller authentication must retain .app.id == 15368", + "Controller authentication must retain .details_url == $run_url", "Controller validation must be activated only by checkout_sha", "Controller validation must bind BASE_SHA", + "Controller validation must bind CHECKOUT_REPOSITORY", "Controller validation must bind EXPECTED_WORKFLOW_SHA", 'Controller validation must retain "$BASE_SHA" =~ ^[a-f0-9]{40}$', 'Controller validation must retain "$WORKFLOW_SHA" == "$EXPECTED_WORKFLOW_SHA"', 'Controller validation must retain [[ "$(jq -r \'.base.sha\' <<< "$pull_json")" == "$BASE_SHA" ]]', 'Controller validation must retain "$PR_NUMBER" =~ ^[1-9][0-9]*$', "generate-matrix checkout must use the selected PR commit", + "generate-matrix checkout must use the selected PR head repository", ]), ); }); + it("rejects an authorized-looking direct fork dispatch before untrusted checkout", () => { + const workflow = readE2eOperationsWorkflow(); + workflow.jobs["generate-matrix"].steps = workflow.jobs["generate-matrix"].steps!.filter( + (step) => step.name !== "Authenticate controller dispatch", + ); + + expect(validateE2eOperationsWorkflow(workflow)).toEqual( + expect.arrayContaining([ + "Controller authentication must be activated only by checkout_sha", + "Controller authentication must run before untrusted checkout and PR validation", + "Controller authentication must bind CONTROLLER_CHECK_ID", + "Controller authentication must retain nemoclaw-pr-e2e:v2:${PR_NUMBER}:${CHECKOUT_SHA}:${BASE_SHA}", + "Controller authentication must retain .external_id == $external_id", + "Controller authentication must retain .output.summary == $summary", + ]), + ); + }); + + it("fails a valid-looking manual fork dispatch before controller API authentication", () => { + const workflow = readE2eOperationsWorkflow(); + const authentication = workflow.jobs["generate-matrix"].steps!.find( + (step) => step.name === "Authenticate controller dispatch", + )!; + const result = spawnSync( + "bash", + ["--noprofile", "--norc", "-e", "-o", "pipefail", "-c", authentication.run!], + { + encoding: "utf8", + env: { + ...process.env, + ACTOR: "maintainer", + BASE_SHA: "b".repeat(40), + CHECKOUT_REPOSITORY: "contributor/NemoClaw", + CHECKOUT_SHA: "a".repeat(40), + CONTROLLER_CHECK_ID: "17", + CORRELATION_ID: "123e4567-e89b-42d3-a456-426614174000", + GITHUB_REPOSITORY: "NVIDIA/NemoClaw", + GITHUB_TOKEN: "unused", + JOBS: "credential-sanitization", + PLAN_HASH: "c".repeat(64), + PR_NUMBER: "42", + RUN_ATTEMPT: "1", + RUN_ID: "23", + TARGETS: "", + }, + }, + ); + + expect(result.status).toBe(1); + expect(result.stdout).toContain("PR E2E must be dispatched by the trusted controller"); + expect(result.stderr).not.toContain("curl:"); + }); + + it("accepts a controller check bound to the exact child run and selected plan", () => { + const workflow = readE2eOperationsWorkflow(); + const authentication = workflow.jobs["generate-matrix"].steps!.find( + (step) => step.name === "Authenticate controller dispatch", + )!; + const headSha = "a".repeat(40); + const baseSha = "b".repeat(40); + const planHash = "c".repeat(64); + const check = JSON.stringify({ + id: 17, + name: "E2E / PR Gate Coordination", + app: { id: 15368, slug: "github-actions" }, + head_sha: headSha, + external_id: `nemoclaw-pr-e2e:v2:42:${headSha}:${baseSha}`, + status: "in_progress", + conclusion: null, + details_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/23", + output: { + summary: `Risk plan ${planHash} selected jobs: credential-sanitization; targets: none.`, + }, + }); + const result = spawnSync( + "bash", + [ + "--noprofile", + "--norc", + "-e", + "-o", + "pipefail", + "-c", + `curl() { printf '%s' "$FAKE_CHECK"; }\n${authentication.run!}`, + ], + { + encoding: "utf8", + env: { + ...process.env, + ACTOR: "github-actions[bot]", + BASE_SHA: baseSha, + CHECKOUT_SHA: headSha, + CONTROLLER_CHECK_ID: "17", + CORRELATION_ID: "123e4567-e89b-42d3-a456-426614174000", + FAKE_CHECK: check, + GITHUB_REPOSITORY: "NVIDIA/NemoClaw", + GITHUB_TOKEN: "unused", + JOBS: "credential-sanitization", + PLAN_HASH: planHash, + PR_NUMBER: "42", + RUN_ATTEMPT: "1", + RUN_ID: "23", + TARGETS: "", + }, + }, + ); + + expect(result.status, result.stderr || result.stdout).toBe(0); + }); + it("binds controller dispatch to the exact checkout, plan, and correlation identity (#6955)", () => { const workflow = readE2eOperationsWorkflow(); const validation = workflow.jobs["generate-matrix"].steps!.find( diff --git a/test/e2e/support/sandbox-operations-workflow-boundary.test.ts b/test/e2e/support/sandbox-operations-workflow-boundary.test.ts index 889905d72db..6c5acac0e3f 100644 --- a/test/e2e/support/sandbox-operations-workflow-boundary.test.ts +++ b/test/e2e/support/sandbox-operations-workflow-boundary.test.ts @@ -140,7 +140,7 @@ describe("sandbox operations workflow boundary", () => { expect( validateCentralWorkflowMutation((source) => - source.replace("permissions:\n contents: read", "permissions:\n contents: write"), + source.replace(" contents: read", " contents: write"), ), ).toContain("workflow permissions.contents must be read"); }); diff --git a/test/pr-e2e-gate-command.test.ts b/test/pr-e2e-gate-command.test.ts index bed5752509b..c54e6c776ee 100644 --- a/test/pr-e2e-gate-command.test.ts +++ b/test/pr-e2e-gate-command.test.ts @@ -64,7 +64,7 @@ describe("PR E2E controller commands", () => { expect(result.status).toBe(1); expect(result.stderr).toContain( - "--mode must be seed, start, start-control-plane, start-approved-control-plane, finish", + "--mode must be seed, start, start-control-plane, start-approved-control-plane, start-approved-fork, finish", ); expect(result.stderr).not.toContain("ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX"); }); @@ -287,35 +287,43 @@ describe("PR E2E controller commands", () => { }); }); - it("parses a fork credentialed E2E skip resolution", () => { - expect( - parseControllerCommand([ - "--mode", - "record-fork-e2e-skip", - "--pr", - "42", - "--head", - HEAD_SHA, - "--base", - BASE_SHA, - "--workflow-sha", - WORKFLOW_SHA, - "--maintainer", - "maintainer", - "--reason", - "Reviewed exact fork revision", - "--evidence-url", - "https://github.com/NVIDIA/NemoClaw/actions/runs/123", - ]), - ).toEqual({ - mode: "record-fork-e2e-skip", - prNumber: 42, - headSha: HEAD_SHA, - baseSha: BASE_SHA, - workflowSha: WORKFLOW_SHA, - maintainer: "maintainer", - reason: "Reviewed exact fork revision", - evidenceUrl: "https://github.com/NVIDIA/NemoClaw/actions/runs/123", + it("parses an approved fork E2E run", () => { + withPrivateWorkDir((workDir) => { + expect( + parseControllerCommand([ + "--mode", + "start-approved-fork", + "--pr", + "42", + "--head", + HEAD_SHA, + "--base", + BASE_SHA, + "--workflow-sha", + WORKFLOW_SHA, + "--approval-run-id", + "101", + "--approval-run-attempt", + "1", + "--gate-run-id", + "102", + "--workflow-run-attempt", + "1", + "--work-dir", + workDir, + ]), + ).toMatchObject({ + mode: "start-approved-fork", + prNumber: 42, + headSha: HEAD_SHA, + baseSha: BASE_SHA, + workflowSha: WORKFLOW_SHA, + approvalRunId: 101, + approvalRunAttempt: 1, + gateRunId: 102, + workflowRunAttempt: 1, + planPath: path.join(workDir, "risk-plan.json"), + }); }); }); diff --git a/test/pr-e2e-gate-fork-skip.test.ts b/test/pr-e2e-gate-fork-approval.test.ts similarity index 71% rename from test/pr-e2e-gate-fork-skip.test.ts rename to test/pr-e2e-gate-fork-approval.test.ts index 035be950c08..856d8efe828 100644 --- a/test/pr-e2e-gate-fork-skip.test.ts +++ b/test/pr-e2e-gate-fork-approval.test.ts @@ -10,8 +10,7 @@ import { type PullRequest, parseControllerCommand, prGateExternalId, - recordApprovedForkE2ESkip, - recordManualForkE2ESkip, + startApprovedForkPrGate, startControlPlanePrGate, startPrGate, } from "../tools/e2e/pr-e2e-gate.mts"; @@ -219,22 +218,40 @@ function approvalReview(comment: string | null = null, overrides: Record, + { mode: "start-approved-fork" } + >; } function approvalRunRoute(value: unknown) { @@ -252,7 +269,7 @@ function approvalHistoryRoute(value: unknown) { ); } -function successfulApprovedForkRoutes(approvals: unknown) { +function successfulApprovedForkRoutes(approvals: unknown, requests: RecordedGitHubRequest[]) { return [ approvalRunRoute(approvalWorkflowRun()), approvalHistoryRoute(approvals), @@ -265,20 +282,49 @@ function successfulApprovedForkRoutes(approvals: unknown) { () => githubResponse([{ filename: "src/lib/onboard.ts" }]), ), existingPrGateCheckRunsRoute({ - status: "completed", - conclusion: "failure", - output: { title: "Maintainer approval required to skip credentialed E2E" }, + output: { title: "E2E reviewer authorization required to run fork E2E" }, }), mainWorkflowRefRoute(), githubFetchRoute( ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", (request) => prGateMutationResponse(request), ), + githubFetchRoute( + ({ url, method }) => + url.endsWith("/actions/workflows/e2e.yaml/dispatches") && method === "POST", + () => + githubResponse({ + workflow_run_id: 23, + run_url: "https://api.github.com/repos/NVIDIA/NemoClaw/actions/runs/23", + html_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/23", + }), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/actions/runs/23") && method === "GET", + () => { + const dispatch = requests.find((request) => request.url.endsWith("/dispatches")); + const inputs = (dispatch?.body as { inputs?: Record } | undefined)?.inputs; + const correlationId = inputs?.correlation_id ?? "missing"; + return githubResponse({ + id: 23, + name: `E2E PR #42 (${correlationId})`, + path: ".github/workflows/e2e.yaml", + workflow_id: 7, + run_attempt: 1, + event: "workflow_dispatch", + head_sha: WORKFLOW_SHA, + status: "queued", + conclusion: null, + display_title: `E2E PR #42 (${correlationId})`, + html_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/23", + }); + }, + ), ]; } -describe("PR E2E controller fork credentialed E2E skip approval safety", () => { - it("plans a risky fork without dispatching secret-bearing E2E", async () => { +describe("PR E2E controller fork credentialed E2E approval safety", () => { + it("requires protected approval before a risky fork can run credentialed E2E", async () => { const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-fork-")); const outputPath = path.join(workDir, "github-output"); fs.writeFileSync(outputPath, "", { mode: 0o600 }); @@ -320,34 +366,37 @@ describe("PR E2E controller fork credentialed E2E skip approval safety", () => { startPrGate({ ...startCommand(workDir), headRepository: "contributor/NemoClaw" }), ).resolves.toBeUndefined(); expect(requests.some((request) => request.url.endsWith("/dispatches"))).toBe(false); - const completion = requests - .filter((request) => request.url.endsWith("/check-runs/17")) - .at(-1); - expect(completion?.body).toMatchObject({ - status: "completed", - conclusion: "failure", - details_url: `https://github.com/NVIDIA/NemoClaw/actions/runs/${GATE_RUN_ID}`, + const pending = requests.filter((request) => request.url.endsWith("/check-runs/17")).at(-1); + expect(pending?.body).toMatchObject({ + status: "in_progress", output: { - title: "Maintainer approval required to skip credentialed E2E", - summary: expect.stringContaining("The selected jobs and targets were not run"), + title: "E2E reviewer authorization required to run fork E2E", + summary: expect.stringContaining( + "No selected E2E job or target ran. No repository credential was exposed to fork code.", + ), }, }); - expect(JSON.stringify(completion?.body)).toContain("Review deployments"); - expect(JSON.stringify(completion?.body)).toContain( + expect(JSON.stringify(pending?.body)).toContain("Review deployments"); + expect(JSON.stringify(pending?.body)).toContain( `[E2E / PR Gate Controller run ${GATE_RUN_ID}](https://github.com/NVIDIA/NemoClaw/actions/runs/${GATE_RUN_ID})`, ); - expect(JSON.stringify(completion?.body)).toContain( - "approve-credentialed-e2e-skip-for-fork-pr", + expect(JSON.stringify(pending?.body)).toContain("approve-credentialed-e2e-for-fork-pr"); + expect(JSON.stringify(pending?.body)).toContain( + "Approval authorizes the selected fork code to run with E2E credentials.", ); - expect(JSON.stringify(completion?.body)).toContain("If Review deployments is absent"); - expect(JSON.stringify(completion?.body)).toContain("update the PR to create a new head"); - expect(JSON.stringify(completion?.body)).toContain("approve-fork-e2e-skip"); + expect(JSON.stringify(pending?.body)).toContain("Review scope: PR #42"); + expect(JSON.stringify(pending?.body)).toContain("head repository `contributor/NemoClaw`"); + expect(JSON.stringify(pending?.body)).toContain(`head SHA \`${HEAD_SHA}\``); + expect(JSON.stringify(pending?.body)).toContain(`base SHA \`${BASE_SHA}\``); + expect(JSON.stringify(pending?.body)).toContain("jobs:"); + expect(JSON.stringify(pending?.body)).toContain("deterministic plan"); expect(fs.readFileSync(outputPath, "utf8")).toContain( [ - "fork_skip_mode=record-fork-e2e-skip", - "fork_skip_pr_number=42", - `fork_skip_head_sha=${HEAD_SHA}`, - `fork_skip_base_sha=${BASE_SHA}`, + "approval_mode=start-approved-fork", + "approval_environment=approve-credentialed-e2e-for-fork-pr", + "approval_pr_number=42", + `approval_head_sha=${HEAD_SHA}`, + `approval_base_sha=${BASE_SHA}`, ].join("\n"), ); expect(fs.readFileSync(outputPath, "utf8")).toContain("finalized=true"); @@ -536,13 +585,13 @@ describe("PR E2E controller fork credentialed E2E skip approval safety", () => { expect(summary).toContain("approve-credentialed-e2e-for-internal-pr"); expect(fs.readFileSync(outputPath, "utf8")).toContain( [ - "control_plane_approval_mode=start-approved-control-plane", - "control_plane_approval_pr_number=42", - `control_plane_approval_head_sha=${HEAD_SHA}`, - `control_plane_approval_base_sha=${BASE_SHA}`, + "approval_mode=start-approved-control-plane", + "approval_environment=approve-credentialed-e2e-for-internal-pr", + "approval_pr_number=42", + `approval_head_sha=${HEAD_SHA}`, + `approval_base_sha=${BASE_SHA}`, ].join("\n"), ); - expect(fs.readFileSync(outputPath, "utf8")).not.toContain("fork_skip_mode="); expect(fs.readFileSync(outputPath, "utf8")).toContain("finalized=true"); } finally { fs.rmSync(workDir, { recursive: true, force: true }); @@ -592,240 +641,116 @@ describe("PR E2E controller fork credentialed E2E skip approval safety", () => { } }); - it.each([ - { name: "without a comment", comment: null, expectedReason: "approval confirmed" }, - { - name: "with an optional comment", - comment: " Independently\nreviewed without secrets. ", - expectedReason: "Reviewer comment: Independently reviewed without secrets.", - }, - { - name: "with an overlong optional comment", - comment: "x".repeat(1000), - expectedReason: `Reviewer comment: ${"x".repeat(100)}`, - }, - ])("records a validated environment approval $name", async ({ comment, expectedReason }) => { - vi.stubEnv("GITHUB_TOKEN", "token"); - vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); - const requests: RecordedGitHubRequest[] = []; - vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter(successfulApprovedForkRoutes([approvalReview(comment)]), requests), - ); - - await expect(recordApprovedForkE2ESkip(approvedForkSkipCommand())).resolves.toBeUndefined(); - - const completion = requests.filter((request) => request.method === "PATCH").at(-1); - expect(completion?.body).toMatchObject({ - status: "completed", - conclusion: "success", - details_url: `https://github.com/NVIDIA/NemoClaw/actions/runs/${APPROVAL_RUN_ID}`, - output: { - title: "Credentialed E2E skipped for fork PR — approved by @e2e-reviewer", - summary: expect.stringContaining( - "**Outcome: APPROVED SKIP — credentialed E2E did not run.**", - ), - }, - }); - const summary = JSON.stringify(completion?.body); - expect(summary).toContain("Validated environment approval run"); - expect(summary).toContain(expectedReason); - expect(summary).not.toContain("not validated by this controller"); - expect(summary.length).toBeLessThan(2000); - expect(requests.some((request) => request.url.includes("/collaborators/"))).toBe(false); - }); - - it("explains how to recover when the approval environment is not protected", async () => { - vi.stubEnv("GITHUB_TOKEN", "token"); - vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); - const requests: RecordedGitHubRequest[] = []; - vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter( - [approvalRunRoute(approvalWorkflowRun()), approvalHistoryRoute([])], - requests, - ), - ); - - await expect(recordApprovedForkE2ESkip(approvedForkSkipCommand())).rejects.toThrow( - /No required-reviewer approval was recorded.*Review deployments was absent.*missing or unprotected.*update the PR to create a new head.*trigger fresh PR CI.*manual maintainer fallback/u, - ); - expect(requests.some((request) => request.method === "PATCH")).toBe(false); - }); - - it.each([ - { name: "a malformed history object", approvals: {} }, - { - name: "a malformed review", - approvals: [approvalReview(null, { comment: 42 })], - }, - { - name: "the wrong environment", - approvals: [approvalReview(null, { environments: [{ name: "production" }] })], - }, - { - name: "a rejected review", - approvals: [approvalReview(null, { state: "rejected" })], - }, - { - name: "an approval spanning multiple environments", - approvals: [ - approvalReview(null, { - environments: [ - { name: "approve-credentialed-e2e-skip-for-fork-pr" }, - { name: "production" }, - ], - }), - ], - }, - { - name: "ambiguous matching approvals", - approvals: [approvalReview(), approvalReview("second approval")], - }, - ])("fails closed for $name", async ({ approvals }) => { + it("dispatches the exact fork repository and PR SHA after protected approval", async () => { + const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-fork-approved-")); + const outputPath = path.join(workDir, "github-output"); + fs.writeFileSync(outputPath, "", { mode: 0o600 }); vi.stubEnv("GITHUB_TOKEN", "token"); vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + vi.stubEnv("GITHUB_OUTPUT", outputPath); const requests: RecordedGitHubRequest[] = []; vi.spyOn(globalThis, "fetch").mockImplementation( createGitHubFetchRouter( - [approvalRunRoute(approvalWorkflowRun()), approvalHistoryRoute(approvals)], + successfulApprovedForkRoutes( + [approvalReview("Reviewed the exact fork PR and selected E2E plan.")], + requests, + ), requests, ), ); - await expect(recordApprovedForkE2ESkip(approvedForkSkipCommand())).rejects.toThrow(); - expect(requests.some((request) => request.method === "PATCH")).toBe(false); - }); - - it.each([ - { name: "wrong run id", overrides: { id: APPROVAL_RUN_ID + 1 } }, - { name: "wrong event", overrides: { event: "workflow_dispatch" } }, - { - name: "untrusted workflow path suffix", - overrides: { path: ".github/workflows/pr-e2e-gate.yaml@refs/heads/main" }, - }, - { name: "wrong head branch", overrides: { head_branch: "feature" } }, - { name: "wrong workflow SHA", overrides: { head_sha: ADVANCED_WORKFLOW_SHA } }, - { name: "completed run", overrides: { status: "completed", conclusion: "success" } }, - { name: "second run attempt", overrides: { run_attempt: 2 } }, - { - name: "noncanonical URL", - overrides: { - html_url: `https://github.com/NVIDIA/NemoClaw/actions/runs/${APPROVAL_RUN_ID}/`, - }, - }, - ])("rejects approval from a $name", async ({ overrides }) => { - vi.stubEnv("GITHUB_TOKEN", "token"); - vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); - const requests: RecordedGitHubRequest[] = []; - vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter([approvalRunRoute(approvalWorkflowRun(overrides))], requests), - ); - - await expect(recordApprovedForkE2ESkip(approvedForkSkipCommand())).rejects.toThrow( - /trusted first-attempt gate run/u, - ); - expect(requests.some((request) => request.method === "PATCH")).toBe(false); - }); - - it("parses only first-attempt protected-environment resolutions", () => { - const args = [ - "--mode", - "record-approved-fork-e2e-skip", - "--pr", - "42", - "--head", - HEAD_SHA, - "--base", - BASE_SHA, - "--workflow-sha", - WORKFLOW_SHA, - "--approval-run-id", - String(APPROVAL_RUN_ID), - "--approval-run-attempt", - "1", - ]; - - expect(parseControllerCommand(args)).toEqual(approvedForkSkipCommand()); - expect(() => parseControllerCommand([...args.slice(0, -1), "2"])).toThrow(/must be exactly 1/u); - }); - - it("rejects a command for a rerun before reading GitHub approval state", async () => { - vi.stubEnv("GITHUB_TOKEN", "token"); - vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); - const requests: RecordedGitHubRequest[] = []; - vi.spyOn(globalThis, "fetch").mockImplementation(createGitHubFetchRouter([], requests)); + try { + await expect(startApprovedForkPrGate(approvedForkCommand(workDir))).resolves.toBeUndefined(); - await expect( - recordApprovedForkE2ESkip({ ...approvedForkSkipCommand(), approvalRunAttempt: 2 }), - ).rejects.toThrow(/must be exactly 1/u); - expect(requests).toHaveLength(0); + expect(requests.some((request) => request.url.includes("/collaborators/"))).toBe(false); + expect(requests.find((request) => request.url.endsWith("/dispatches"))?.body).toMatchObject({ + ref: "main", + inputs: { + controller_check_id: "17", + pr_number: "42", + checkout_repository: "contributor/NemoClaw", + checkout_sha: HEAD_SHA, + base_sha: BASE_SHA, + workflow_sha: WORKFLOW_SHA, + }, + }); + const authorization = requests.find( + (request) => + request.url.endsWith("/check-runs/17") && + (request.body as { output?: { title?: string } } | undefined)?.output?.title === + "E2E execution authorized by @e2e-reviewer", + ); + expect(authorization?.body).toMatchObject({ + status: "in_progress", + output: { + summary: expect.stringContaining("Reviewed the exact fork PR and selected E2E plan."), + }, + }); + expect(fs.readFileSync(outputPath, "utf8")).toContain("dispatched=true"); + } finally { + fs.rmSync(workDir, { recursive: true, force: true }); + } }); - it("records an approved credentialed E2E skip for the reviewed head/base after a compatible main advance", async () => { + it("fails closed when the fork approval environment did not record an approval", async () => { + const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-fork-no-review-")); vi.stubEnv("GITHUB_TOKEN", "token"); vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); const requests: RecordedGitHubRequest[] = []; vi.spyOn(globalThis, "fetch").mockImplementation( createGitHubFetchRouter( - [ - githubFetchRoute( - ({ url }) => url.endsWith("/collaborators/maintainer/permission"), - () => - githubResponse({ - role_name: "maintain", - permission: "write", - user: { login: "maintainer" }, - }), - ), - githubFetchRoute( - ({ url }) => url.endsWith("/pulls/42"), - () => githubResponse(forkPullRequest()), - ), - githubFetchRoute( - ({ url }) => url.includes("/pulls/42/files?"), - () => githubResponse([{ filename: "src/lib/onboard.ts" }]), - ), - existingPrGateCheckRunsRoute({ - status: "completed", - conclusion: "failure", - output: { title: "Maintainer approval required to skip credentialed E2E" }, - }), - mainWorkflowRefRoute(ADVANCED_WORKFLOW_SHA), - compatibleMainComparisonRoute([{ filename: "docs/get-started/quickstart.mdx" }]), - githubFetchRoute( - ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", - (request) => prGateMutationResponse(request), - ), - ], + [approvalRunRoute(approvalWorkflowRun()), approvalHistoryRoute([])], requests, ), ); - await recordManualForkE2ESkip({ - mode: "record-fork-e2e-skip", - prNumber: 42, - headSha: HEAD_SHA, - baseSha: BASE_SHA, - workflowSha: WORKFLOW_SHA, - maintainer: "maintainer", - reason: "The fork cannot safely receive credential-bearing test secrets.", - }); + try { + await expect(startApprovedForkPrGate(approvedForkCommand(workDir))).rejects.toThrow( + /No required-reviewer approval was recorded for approve-credentialed-e2e-for-fork-pr/u, + ); + expect(requests.some((request) => request.url.endsWith("/dispatches"))).toBe(false); + expect(requests.some((request) => request.method === "PATCH")).toBe(false); + } finally { + fs.rmSync(workDir, { recursive: true, force: true }); + } + }); - const completion = requests.at(-1); - expect(completion?.body).toMatchObject({ - status: "completed", - conclusion: "success", - output: { - title: "Credentialed E2E skipped for fork PR — approved by @maintainer", - summary: expect.stringContaining( - "**Outcome: APPROVED SKIP — credentialed E2E did not run.**", - ), - }, - }); - expect(JSON.stringify(completion?.body)).toContain("Selected jobs and targets not run"); - expect(JSON.stringify(completion?.body)).toContain( - "Approval source: manual fallback; no supporting Actions run was supplied.", - ); - expect(JSON.stringify(completion?.body)).not.toContain("tests passed"); + it("parses only first-attempt protected fork execution", () => { + const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-fork-command-")); + try { + const command = approvedForkCommand(workDir); + expect(command).toMatchObject({ + mode: "start-approved-fork", + approvalRunAttempt: 1, + workflowRunAttempt: 1, + }); + expect(() => + parseControllerCommand([ + "--mode", + "start-approved-fork", + "--pr", + "42", + "--head", + HEAD_SHA, + "--base", + BASE_SHA, + "--workflow-sha", + WORKFLOW_SHA, + "--approval-run-id", + String(APPROVAL_RUN_ID), + "--approval-run-attempt", + "2", + "--gate-run-id", + String(APPROVAL_RUN_ID), + "--workflow-run-attempt", + "1", + "--work-dir", + workDir, + ]), + ).toThrow(/must be exactly 1/u); + } finally { + fs.rmSync(workDir, { recursive: true, force: true }); + } }); it("dispatches an authorized control-plane run for the PR SHA without clearing the gate", async () => { @@ -1052,7 +977,7 @@ describe("PR E2E controller fork credentialed E2E skip approval safety", () => { }, }); await expect(startControlPlanePrGate(startControlPlaneCommand(workDirs[1]!))).rejects.toThrow( - /matching pending control-plane authorization state/u, + /matching pending E2E authorization state/u, ); expect(requests.filter((request) => request.url.endsWith("/dispatches"))).toHaveLength(1); expect(fs.readFileSync(outputPath, "utf8")).toContain("finalized=true"); @@ -1061,41 +986,6 @@ describe("PR E2E controller fork credentialed E2E skip approval safety", () => { } }); - it("rejects a credentialed E2E skip from a collaborator below maintainer role", async () => { - vi.stubEnv("GITHUB_TOKEN", "token"); - vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); - const requests: RecordedGitHubRequest[] = []; - vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter( - [ - githubFetchRoute( - ({ url }) => url.endsWith("/collaborators/contributor/permission"), - () => - githubResponse({ - role_name: "write", - permission: "write", - user: { login: "contributor" }, - }), - ), - ], - requests, - ), - ); - - await expect( - recordManualForkE2ESkip({ - mode: "record-fork-e2e-skip", - prNumber: 42, - headSha: HEAD_SHA, - baseSha: BASE_SHA, - workflowSha: WORKFLOW_SHA, - maintainer: "contributor", - reason: "A write-role collaborator tried to record a credentialed E2E skip.", - }), - ).rejects.toThrow(/maintainer or administrator/u); - expect(requests.some((request) => request.method === "PATCH")).toBe(false); - }); - it("rejects control-plane authorization from a collaborator below maintainer role", async () => { const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-role-")); vi.stubEnv("GITHUB_TOKEN", "token"); @@ -1164,40 +1054,6 @@ describe("PR E2E controller fork credentialed E2E skip approval safety", () => { } }); - it("rejects a fork credentialed E2E skip for an internal pull request", async () => { - vi.stubEnv("GITHUB_TOKEN", "token"); - vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); - const requests: RecordedGitHubRequest[] = []; - vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter( - [ - githubFetchRoute( - ({ url }) => url.endsWith("/collaborators/maintainer/permission"), - () => githubResponse({ role_name: "maintain", user: { login: "maintainer" } }), - ), - githubFetchRoute( - ({ url }) => url.endsWith("/pulls/42"), - () => githubResponse(pullRequest()), - ), - ], - requests, - ), - ); - - const common = { - prNumber: 42, - headSha: HEAD_SHA, - baseSha: BASE_SHA, - workflowSha: WORKFLOW_SHA, - maintainer: "maintainer", - reason: "The resolver operation must match the pull request origin.", - }; - await expect( - recordManualForkE2ESkip({ mode: "record-fork-e2e-skip", ...common }), - ).rejects.toThrow(/credentialed E2E skips require a fork pull request/u); - expect(requests.some((request) => request.method === "PATCH")).toBe(false); - }); - it("rejects control-plane authorization when the gate is already completed", async () => { const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-title-")); vi.stubEnv("GITHUB_TOKEN", "token"); @@ -1230,7 +1086,7 @@ describe("PR E2E controller fork credentialed E2E skip approval safety", () => { try { await expect(startControlPlanePrGate(startControlPlaneCommand(workDir))).rejects.toThrow( - /matching pending control-plane authorization state/u, + /matching pending E2E authorization state/u, ); expect(requests.some((request) => request.method === "PATCH")).toBe(false); } finally { @@ -1367,82 +1223,6 @@ describe("PR E2E controller fork credentialed E2E skip approval safety", () => { } }); - it("rejects a stale fork credentialed E2E skip before changing the gate", async () => { - vi.stubEnv("GITHUB_TOKEN", "token"); - vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); - const requests: RecordedGitHubRequest[] = []; - vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter( - [ - githubFetchRoute( - ({ url }) => url.endsWith("/collaborators/maintainer/permission"), - () => githubResponse({ role_name: "maintain", user: { login: "maintainer" } }), - ), - githubFetchRoute( - ({ url }) => url.endsWith("/pulls/42"), - () => - githubResponse({ - ...forkPullRequest(), - head: { ...forkPullRequest().head, sha: "c".repeat(40) }, - }), - ), - ], - requests, - ), - ); - - await expect( - recordManualForkE2ESkip({ - mode: "record-fork-e2e-skip", - prNumber: 42, - headSha: HEAD_SHA, - baseSha: BASE_SHA, - workflowSha: WORKFLOW_SHA, - maintainer: "maintainer", - reason: "The reviewed revision has since changed upstream.", - }), - ).rejects.toThrow(/no longer matches/u); - expect(requests.some((request) => request.method === "PATCH")).toBe(false); - }); - - it("rejects a fork credentialed E2E skip after the pull request is retargeted", async () => { - vi.stubEnv("GITHUB_TOKEN", "token"); - vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); - const requests: RecordedGitHubRequest[] = []; - vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter( - [ - githubFetchRoute( - ({ url }) => url.endsWith("/collaborators/maintainer/permission"), - () => githubResponse({ role_name: "maintain", user: { login: "maintainer" } }), - ), - githubFetchRoute( - ({ url }) => url.endsWith("/pulls/42"), - () => - githubResponse({ - ...forkPullRequest(), - base: { ...forkPullRequest().base, sha: "f".repeat(40) }, - }), - ), - ], - requests, - ), - ); - - await expect( - recordManualForkE2ESkip({ - mode: "record-fork-e2e-skip", - prNumber: 42, - headSha: HEAD_SHA, - baseSha: BASE_SHA, - workflowSha: WORKFLOW_SHA, - maintainer: "maintainer", - reason: "The reviewed base revision has since changed upstream.", - }), - ).rejects.toThrow(/no longer matches the reviewed PR SHA and base SHA/u); - expect(requests.some((request) => request.method === "PATCH")).toBe(false); - }); - it("rejects control-plane authorization when the base changes before dispatch", async () => { const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-base-")); vi.stubEnv("GITHUB_TOKEN", "token"); diff --git a/test/pr-e2e-gate-internal-approval.test.ts b/test/pr-e2e-gate-internal-approval.test.ts index 66e364453cd..d7cf8dc4ee2 100644 --- a/test/pr-e2e-gate-internal-approval.test.ts +++ b/test/pr-e2e-gate-internal-approval.test.ts @@ -224,6 +224,7 @@ describe("PR E2E protected internal approval", () => { expect(requests.find((request) => request.url.endsWith("/dispatches"))?.body).toMatchObject({ ref: "main", inputs: { + controller_check_id: "17", pr_number: "42", checkout_sha: HEAD_SHA, base_sha: BASE_SHA, diff --git a/test/pr-e2e-gate-lifecycle.test.ts b/test/pr-e2e-gate-lifecycle.test.ts index 9e2717951c4..ec98afa3de9 100644 --- a/test/pr-e2e-gate-lifecycle.test.ts +++ b/test/pr-e2e-gate-lifecycle.test.ts @@ -175,9 +175,10 @@ function pullRequestListItem(pull = pullRequest()): Omit { }, }); expect(JSON.stringify(completion?.body)).toContain("Review deployments"); - expect(fs.readFileSync(outputPath, "utf8")).not.toContain("fork_skip_mode="); + expect(fs.readFileSync(outputPath, "utf8")).toContain( + "approval_mode=start-approved-control-plane", + ); expect(fs.readFileSync(outputPath, "utf8")).toContain("check_id=18"); expect(fs.readFileSync(outputPath, "utf8")).toContain("finalized=true"); } finally { diff --git a/test/pr-e2e-gate-runner-loss-retry.test.ts b/test/pr-e2e-gate-runner-loss-retry.test.ts index d8c881f77f6..7caac32f81a 100644 --- a/test/pr-e2e-gate-runner-loss-retry.test.ts +++ b/test/pr-e2e-gate-runner-loss-retry.test.ts @@ -61,9 +61,10 @@ function sha256(value: string): string { function state(): PrGateState { return { - version: 3, + version: 4, commitSha: HEAD_SHA, baseSha: BASE_SHA, + checkoutRepository: "NVIDIA/NemoClaw", workflowSha: WORKFLOW_SHA, planHash: "c".repeat(64), correlationId: ORIGINAL_CORRELATION_ID, diff --git a/test/pr-e2e-gate-typed-target.test.ts b/test/pr-e2e-gate-typed-target.test.ts index 3dc9245504b..73f1343e449 100644 --- a/test/pr-e2e-gate-typed-target.test.ts +++ b/test/pr-e2e-gate-typed-target.test.ts @@ -23,9 +23,10 @@ const DCODE_CHECK = function state(): PrGateState { const plan = buildRiskPlan({ headSha: HEAD_SHA, changedFiles: [DCODE_CHECK] }); return { - version: 3, + version: 4, commitSha: HEAD_SHA, baseSha: BASE_SHA, + checkoutRepository: "NVIDIA/NemoClaw", workflowSha: WORKFLOW_SHA, planHash: plan.planHash, correlationId: CORRELATION_ID, @@ -80,7 +81,9 @@ describe("PR E2E typed-target gate (#7031)", () => { await expect( dispatchPrGate({ repository: "NVIDIA/NemoClaw", + checkoutRepository: "NVIDIA/NemoClaw", token: "token", + controllerCheckId: 101, jobs: [], targets: ["ubuntu-repo-cloud-openclaw"], prNumber: 42, diff --git a/test/pr-e2e-gate-workflow.test.ts b/test/pr-e2e-gate-workflow.test.ts index 6f28cd0d550..9c7b087afee 100644 --- a/test/pr-e2e-gate-workflow.test.ts +++ b/test/pr-e2e-gate-workflow.test.ts @@ -150,12 +150,9 @@ function runControlPlaneStartStep(reviewReason: string) { } } -function runApprovedForkSkipStep() { +function runApprovedForkStartStep() { const workflow = readYaml(PR_GATE_PATH); - const approve = step( - workflow.jobs["approve-fork-e2e-skip"], - "Record approved credentialed E2E skip", - ); + const approve = step(workflow.jobs["approve-e2e"], "Start approved E2E"); const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-approve-")); const binDir = path.join(tempDir, "bin"); const argumentsPath = path.join(tempDir, "node-arguments"); @@ -171,15 +168,19 @@ function runApprovedForkSkipStep() { encoding: "utf8", env: { ...process.env, + APPROVAL_MODE: "start-approved-fork", APPROVAL_RUN_ATTEMPT: "1", APPROVAL_RUN_ID: "101", EXPECTED_BASE_SHA: BASE_SHA, EXPECTED_HEAD_SHA: HEAD_SHA, FAKE_NODE_ARGUMENTS: argumentsPath, + GATE_RUN_ID: "101", GITHUB_TOKEN: "token", PATH: `${binDir}:${process.env.PATH ?? ""}`, PR_NUMBER: "42", + WORKFLOW_RUN_ATTEMPT: "1", WORKFLOW_SHA, + WORK_DIR: tempDir, }, timeout: 5_000, }); @@ -270,6 +271,7 @@ esac env: { ...process.env, BASE_SHA, + CHECKOUT_REPOSITORY: "NVIDIA/NemoClaw", CHECKOUT_SHA: HEAD_SHA, CORRELATION_ID: "12345678-1234-4123-8123-123456789abc", EXPECTED_WORKFLOW_SHA: WORKFLOW_SHA, @@ -307,9 +309,7 @@ describe("PR E2E gate workflow", () => { const required = workflow.jobs.required; const cancel = workflow.jobs["cancel-superseded"]; const coordinate = workflow.jobs.coordinate; - const approveInternal = workflow.jobs["approve-internal-e2e"]; - const approveForkSkip = workflow.jobs["approve-fork-e2e-skip"]; - const recordForkSkip = workflow.jobs["record-fork-e2e-skip"]; + const approveE2e = workflow.jobs["approve-e2e"]; const longestSelectedE2eMinutes = 130; const controllerWaitMinutes = 140; const evidenceAndKillGraceMinutes = 10.5; @@ -339,9 +339,9 @@ describe("PR E2E gate workflow", () => { operation: { description: "E2E gate action to perform.", required: true, - default: "approve-fork-e2e-skip", + default: "run-control-plane", type: "choice", - options: ["approve-fork-e2e-skip", "run-control-plane"], + options: ["run-control-plane"], }, pr_number: { description: "Pull request number for the selected E2E gate action.", @@ -359,18 +359,10 @@ describe("PR E2E gate workflow", () => { type: "string", }, review_reason: { - description: - "Why this fork PR may skip credentialed E2E or this internal PR may run control-plane E2E.", + description: "Why this internal PR may run control-plane E2E.", required: true, type: "string", }, - evidence_url: { - description: - "Fork credentialed-E2E skip only; optional Actions run URL. Ignored by run-control-plane, whose evidence comes from the dispatched jobs.", - required: false, - default: "", - type: "string", - }, }, }, }); @@ -439,7 +431,7 @@ describe("PR E2E gate workflow", () => { expect(observer.run).toContain("--timeout-seconds 21480"); expect(cancel.if).toContain("github.event_name == 'pull_request_target'"); expect(cancel.if).toContain("github.run_attempt == 1"); - expect(cancel.if).toContain( + expect(cancel.if).not.toContain( "github.event.pull_request.head.repo.full_name == github.repository", ); expect(cancel.if).toContain("github.event.action != 'edited'"); @@ -475,92 +467,47 @@ describe("PR E2E gate workflow", () => { expect(coordinate.concurrency?.["cancel-in-progress"]).toBe(false); expect(coordinate["timeout-minutes"]).toBe(330); expect(coordinate.outputs).toEqual({ - control_plane_approval_mode: "${{ steps.start.outputs.control_plane_approval_mode }}", - control_plane_approval_pr_number: - "${{ steps.start.outputs.control_plane_approval_pr_number }}", - control_plane_approval_head_sha: "${{ steps.start.outputs.control_plane_approval_head_sha }}", - control_plane_approval_base_sha: "${{ steps.start.outputs.control_plane_approval_base_sha }}", - fork_skip_mode: "${{ steps.start.outputs.fork_skip_mode }}", - fork_skip_pr_number: "${{ steps.start.outputs.fork_skip_pr_number }}", - fork_skip_head_sha: "${{ steps.start.outputs.fork_skip_head_sha }}", - fork_skip_base_sha: "${{ steps.start.outputs.fork_skip_base_sha }}", + approval_mode: "${{ steps.start.outputs.approval_mode }}", + approval_environment: "${{ steps.start.outputs.approval_environment }}", + approval_pr_number: "${{ steps.start.outputs.approval_pr_number }}", + approval_head_sha: "${{ steps.start.outputs.approval_head_sha }}", + approval_base_sha: "${{ steps.start.outputs.approval_base_sha }}", }); - expect(approveInternal.name).toBe("Approve credentialed E2E for internal PR"); - expect(approveInternal.needs).toBe("coordinate"); - expect(approveInternal.if).toBe( - "${{ needs.coordinate.result == 'success' && needs.coordinate.outputs.control_plane_approval_mode != '' && github.run_attempt == 1 }}", + expect(approveE2e.name).toBe("Approve credentialed E2E for reviewed PR"); + expect(approveE2e.needs).toBe("coordinate"); + expect(approveE2e.if).toBe( + "${{ needs.coordinate.result == 'success' && needs.coordinate.outputs.approval_mode != '' && github.run_attempt == 1 }}", ); - expect(approveInternal.environment).toEqual({ - name: "approve-credentialed-e2e-for-internal-pr", + expect(approveE2e.environment).toEqual({ + name: "${{ needs.coordinate.outputs.approval_environment }}", deployment: false, }); - expect(approveInternal.permissions).toEqual({ + expect(approveE2e.permissions).toEqual({ actions: "write", checks: "write", contents: "read", "pull-requests": "read", }); - expect(approveInternal.concurrency).toEqual({ + expect(approveE2e.concurrency).toEqual({ group: - "pr-e2e-gate-${{ github.repository }}-${{ needs.coordinate.outputs.control_plane_approval_pr_number }}-${{ needs.coordinate.outputs.control_plane_approval_head_sha }}-${{ needs.coordinate.outputs.control_plane_approval_base_sha }}", + "pr-e2e-gate-${{ github.repository }}-${{ needs.coordinate.outputs.approval_pr_number }}-${{ needs.coordinate.outputs.approval_head_sha }}-${{ needs.coordinate.outputs.approval_base_sha }}", queue: "max", "cancel-in-progress": false, }); - expect(approveInternal["timeout-minutes"]).toBe(330); + expect(approveE2e["timeout-minutes"]).toBe(330); expect(controllerWaitMinutes).toBeGreaterThan(longestSelectedE2eMinutes); expect(coordinate["timeout-minutes"]).toBeGreaterThanOrEqual( twoAttemptMinimum + controllerSetupReserveMinutes, ); - expect(approveInternal["timeout-minutes"]).toBeGreaterThanOrEqual( + expect(approveE2e["timeout-minutes"]).toBeGreaterThanOrEqual( twoAttemptMinimum + controllerSetupReserveMinutes, ); expect(observerPollMinutes).toBeGreaterThanOrEqual( coordinate["timeout-minutes"]! + maxPrerequisiteCiMinutes + observerApiSlackMinutes, ); expect(required["timeout-minutes"]).toBeGreaterThan(observerPollMinutes); - expect(approveInternal.secrets).toBeUndefined(); - expect(approveForkSkip.name).toBe("Approve credentialed E2E skip for fork PR"); - expect(approveForkSkip.needs).toBe("coordinate"); - expect(approveForkSkip.if).toBe( - "${{ needs.coordinate.result == 'success' && needs.coordinate.outputs.fork_skip_mode != '' && github.run_attempt == 1 }}", - ); - expect(approveForkSkip.environment).toEqual({ - name: "approve-credentialed-e2e-skip-for-fork-pr", - deployment: false, - }); - expect(approveForkSkip.permissions).toEqual({ - actions: "read", - checks: "write", - contents: "read", - "pull-requests": "read", - }); - expect(approveForkSkip.concurrency).toEqual({ - group: - "pr-e2e-gate-${{ github.repository }}-${{ needs.coordinate.outputs.fork_skip_pr_number }}-${{ needs.coordinate.outputs.fork_skip_head_sha }}-${{ needs.coordinate.outputs.fork_skip_base_sha }}", - queue: "max", - "cancel-in-progress": false, - }); - expect(approveForkSkip.secrets).toBeUndefined(); - expect(recordForkSkip.if).toContain("github.event_name == 'workflow_dispatch'"); - expect(recordForkSkip.if).toContain("github.run_attempt == 1"); - expect(recordForkSkip.if).toContain("github.ref == 'refs/heads/main'"); - expect(recordForkSkip.name).toBe("Record credentialed E2E skip for fork PR"); - expect(recordForkSkip.if).toContain("inputs.operation == 'approve-fork-e2e-skip'"); - expect(recordForkSkip.permissions).toEqual({ - checks: "write", - contents: "read", - "pull-requests": "read", - }); - expect(recordForkSkip.concurrency).toEqual({ - group: - "pr-e2e-gate-${{ github.repository }}-${{ inputs.pr_number }}-${{ inputs.expected_head_sha }}-${{ inputs.expected_base_sha }}", - queue: "max", - "cancel-in-progress": false, - }); + expect(approveE2e.secrets).toBeUndefined(); expect(collectStrings(initialize).some((value) => value.includes("--mode seed"))).toBe(true); - expect( - collectStrings(recordForkSkip).some((value) => value.includes("--mode record-fork-e2e-skip")), - ).toBe(true); expect(step(initialize, "Reserve PR/base SHA gate").run).toContain('--head "$HEAD_SHA"'); expect(step(initialize, "Reserve PR/base SHA gate").env?.BASE_SHA).toBe( "${{ github.event.pull_request.base.sha }}", @@ -575,19 +522,21 @@ describe("PR E2E gate workflow", () => { expect(start.run).toContain("--mode start-control-plane"); expect(start.run).toContain('--ci-display-title "$CI_DISPLAY_TITLE"'); expect(start.run).toContain('--gate-run-id "$GATE_RUN_ID"'); - const approvedStart = step(approveInternal, "Start approved internal E2E"); + const approvedStart = step(approveE2e, "Start approved E2E"); expect(approvedStart.env).toMatchObject({ + APPROVAL_MODE: "${{ needs.coordinate.outputs.approval_mode }}", APPROVAL_RUN_ATTEMPT: "${{ github.run_attempt }}", APPROVAL_RUN_ID: "${{ github.run_id }}", - EXPECTED_BASE_SHA: "${{ needs.coordinate.outputs.control_plane_approval_base_sha }}", - EXPECTED_HEAD_SHA: "${{ needs.coordinate.outputs.control_plane_approval_head_sha }}", + EXPECTED_BASE_SHA: "${{ needs.coordinate.outputs.approval_base_sha }}", + EXPECTED_HEAD_SHA: "${{ needs.coordinate.outputs.approval_head_sha }}", GATE_RUN_ID: "${{ github.run_id }}", GITHUB_TOKEN: "${{ github.token }}", - PR_NUMBER: "${{ needs.coordinate.outputs.control_plane_approval_pr_number }}", + PR_NUMBER: "${{ needs.coordinate.outputs.approval_pr_number }}", WORKFLOW_RUN_ATTEMPT: "${{ github.run_attempt }}", WORKFLOW_SHA: "${{ github.workflow_sha }}", }); - expect(approvedStart.run).toContain("--mode start-approved-control-plane"); + expect(approvedStart.run).toContain("start-approved-control-plane | start-approved-fork"); + expect(approvedStart.run).toContain('--mode "$APPROVAL_MODE"'); expect(approvedStart.run).toContain('--approval-run-id "$APPROVAL_RUN_ID"'); expect(approvedStart.run).toContain('--approval-run-attempt "$APPROVAL_RUN_ATTEMPT"'); expect(approvedStart.run).toContain('--head "$EXPECTED_HEAD_SHA"'); @@ -621,36 +570,6 @@ describe("PR E2E gate workflow", () => { expect(interruptedRetry.if).not.toContain("steps.retry.outcome == 'failure'"); expect(interruptedRetry.if).toContain("steps.retry.outputs.check_id == ''"); expect(interruptedRetry.run).toContain("--mode abandon-runner-loss-retry"); - const approval = step(approveForkSkip, "Record approved credentialed E2E skip"); - expect(approval.env).toEqual({ - APPROVAL_RUN_ATTEMPT: "${{ github.run_attempt }}", - APPROVAL_RUN_ID: "${{ github.run_id }}", - EXPECTED_BASE_SHA: "${{ needs.coordinate.outputs.fork_skip_base_sha }}", - EXPECTED_HEAD_SHA: "${{ needs.coordinate.outputs.fork_skip_head_sha }}", - GITHUB_TOKEN: "${{ github.token }}", - PR_NUMBER: "${{ needs.coordinate.outputs.fork_skip_pr_number }}", - WORKFLOW_SHA: "${{ github.workflow_sha }}", - }); - expect(approval.run).toContain("--mode record-approved-fork-e2e-skip"); - expect(approval.run).not.toContain("--fork-skip-mode"); - expect(approval.run).toContain('--pr "$PR_NUMBER"'); - expect(approval.run).toContain('--head "$EXPECTED_HEAD_SHA"'); - expect(approval.run).toContain('--base "$EXPECTED_BASE_SHA"'); - expect(approval.run).toContain('--workflow-sha "$WORKFLOW_SHA"'); - expect(approval.run).toContain('--approval-run-id "$APPROVAL_RUN_ID"'); - expect(approval.run).toContain('--approval-run-attempt "$APPROVAL_RUN_ATTEMPT"'); - const resolution = step(recordForkSkip, "Record credentialed E2E skip"); - expect(resolution.env?.WORKFLOW_SHA).toBe("${{ github.workflow_sha }}"); - expect(resolution.env?.MAINTAINER).toBe("${{ github.triggering_actor }}"); - expect(resolution.env?.MAINTAINER).not.toBe("${{ github.actor }}"); - expect(resolution.env?.EXPECTED_BASE_SHA).toBe("${{ inputs.expected_base_sha }}"); - expect(resolution.env?.REVIEW_REASON).toBe("${{ inputs.review_reason }}"); - expect(resolution.run).toContain("--mode record-fork-e2e-skip"); - expect(resolution.run).toContain('--head "$EXPECTED_HEAD_SHA"'); - expect(resolution.run).toContain('--base "$EXPECTED_BASE_SHA"'); - expect(resolution.run).toContain('--workflow-sha "$WORKFLOW_SHA"'); - expect(resolution.run).toContain('--reason "$REVIEW_REASON"'); - expect(resolution.run).toContain('--evidence-url "$EVIDENCE_URL"'); expect(collectStrings(workflow).some((value) => value.includes("${{ secrets."))).toBe(false); }); @@ -668,7 +587,7 @@ describe("PR E2E gate workflow", () => { (candidate) => candidate.name === "Install controller dependencies", ); - expect(checkouts).toHaveLength(7); + expect(checkouts).toHaveLength(5); expect( checkouts.every( (checkout) => @@ -676,11 +595,11 @@ describe("PR E2E gate workflow", () => { checkout.with?.["persist-credentials"] === false, ), ).toBe(true); - expect(nodeSetups).toHaveLength(7); + expect(nodeSetups).toHaveLength(5); expect(nodeSetups.every((setup) => setup.uses === TRUSTED_SETUP_NODE_ACTION)).toBe(true); expect(nodeSetups.every((setup) => setup.with?.["node-version"] === "22")).toBe(true); expect(nodeSetups.every((setup) => !("cache" in (setup.with ?? {})))).toBe(true); - expect(installs).toHaveLength(6); + expect(installs).toHaveLength(4); expect( installs.every((install) => install.run === "npm ci --ignore-scripts --no-audit --no-fund"), ).toBe(true); @@ -733,8 +652,8 @@ describe("PR E2E gate workflow", () => { expect(execution.arguments[prFlag + 1]).toBe(""); }); - it("passes the approved fork skip identity as inert arguments", () => { - const execution = runApprovedForkSkipStep(); + it("passes the approved fork E2E identity as inert arguments", () => { + const execution = runApprovedForkStartStep(); expect(execution.result.status).toBe(0); expect(execution.result.stderr).toBe(""); @@ -742,7 +661,7 @@ describe("PR E2E gate workflow", () => { "--experimental-strip-types", "tools/e2e/pr-e2e-gate.mts", "--mode", - "record-approved-fork-e2e-skip", + "start-approved-fork", "--pr", "42", "--head", @@ -755,6 +674,12 @@ describe("PR E2E gate workflow", () => { "101", "--approval-run-attempt", "1", + "--gate-run-id", + "101", + "--workflow-run-attempt", + "1", + "--work-dir", + expect.any(String), ]); }); @@ -809,7 +734,7 @@ describe("PR E2E gate workflow", () => { it("orders the coordinate steps and always finalizes through the controller", () => { const workflow = readYaml(PR_GATE_PATH); const coordinate = workflow.jobs.coordinate; - const approveInternal = workflow.jobs["approve-internal-e2e"]; + const approveE2e = workflow.jobs["approve-e2e"]; expect((coordinate.steps ?? []).map((candidate) => candidate.name)).toEqual([ "Checkout controller", @@ -842,12 +767,12 @@ describe("PR E2E gate workflow", () => { expect(cleanup.if).toContain("steps.workspace.outputs.work_dir"); expect(cleanup.run).toBe('rm -rf -- "${{ steps.workspace.outputs.work_dir }}"'); - expect((approveInternal.steps ?? []).map((candidate) => candidate.name)).toEqual([ + expect((approveE2e.steps ?? []).map((candidate) => candidate.name)).toEqual([ "Checkout controller", "Setup Node", "Install controller dependencies", "Create private workspace", - "Start approved internal E2E", + "Start approved E2E", "Upload approved risk plan", "Wait for approved E2E run", "Download approved evidence", @@ -861,25 +786,25 @@ describe("PR E2E gate workflow", () => { "Close incomplete approved check", "Remove private workspace", ]); - expect(step(approveInternal, "Download approved evidence").if).toContain("always()"); - expect(step(approveInternal, "Verify approved evidence").if).toContain("always()"); - expect(step(approveInternal, "Retry approved E2E after hosted runner loss").if).toContain( + expect(step(approveE2e, "Download approved evidence").if).toContain("always()"); + expect(step(approveE2e, "Verify approved evidence").if).toContain("always()"); + expect(step(approveE2e, "Retry approved E2E after hosted runner loss").if).toContain( "github.run_attempt == 1", ); - expect(step(approveInternal, "Download approved retry evidence").run).toContain( + expect(step(approveE2e, "Download approved retry evidence").run).toContain( "--slot runner-loss-retry", ); - expect(step(approveInternal, "Verify approved retry evidence").if).toContain("always()"); - expect(step(approveInternal, "Close incomplete approved retry check").if).toContain("always()"); + expect(step(approveE2e, "Verify approved retry evidence").if).toContain("always()"); + expect(step(approveE2e, "Close incomplete approved retry check").if).toContain("always()"); const approvedInterruptedRetry = step( - approveInternal, + approveE2e, "Terminalize interrupted approved retry setup", ); expect(approvedInterruptedRetry.if).toContain("steps.retry.outcome != 'success'"); expect(approvedInterruptedRetry.if).not.toContain("steps.retry.outcome == 'failure'"); expect(approvedInterruptedRetry.if).toContain("steps.retry.outputs.check_id == ''"); expect(approvedInterruptedRetry.run).toContain("--mode abandon-runner-loss-retry"); - expect(step(approveInternal, "Close incomplete approved check").if).toContain("always()"); - expect(step(approveInternal, "Remove private workspace").if).toContain("always()"); + expect(step(approveE2e, "Close incomplete approved check").if).toContain("always()"); + expect(step(approveE2e, "Remove private workspace").if).toContain("always()"); }); }); diff --git a/test/pr-e2e-gate.test.ts b/test/pr-e2e-gate.test.ts index 5d89dbc3246..7834ad251f9 100644 --- a/test/pr-e2e-gate.test.ts +++ b/test/pr-e2e-gate.test.ts @@ -161,9 +161,10 @@ function pullRequestListItem(pull = pullRequest()): Omit = {}) } describe("PR E2E controller", () => { - it("explains the accepted evidence URL when a manual fork skip uses another GitHub URL", () => { - expect(() => - parseControllerCommand([ - "--mode", - "record-fork-e2e-skip", - "--pr", - "42", - "--head", - HEAD_SHA, - "--base", - BASE_SHA, - "--workflow-sha", - WORKFLOW_SHA, - "--maintainer", - "maintainer", - "--reason", - "Reviewed exact fork revision", - "--evidence-url", - "https://github.com/NVIDIA/NemoClaw/pull/42#issuecomment-1", - ]), - ).toThrow( - "Evidence URL must be an Actions run URL such as https://github.com/NVIDIA/NemoClaw/actions/runs/123. PR, issue, comment, job, and external URLs are not accepted. Leave the field blank if no run exists.", + it("rejects the removed manual fork skip mode", () => { + expect(() => parseControllerCommand(["--mode", "record-fork-e2e-skip"])).toThrow( + "--mode must be", ); }); @@ -310,6 +292,9 @@ describe("PR E2E controller", () => { expect(riskPlanRequiredTargetIds(targetPlan)).toEqual([DCODE_TARGET]); expect(validatePrGateState(gate)).toEqual(gate); expect(() => validatePrGateState({ ...gate, prNumber: 0 })).toThrow(/PR number/u); + expect(() => validatePrGateState({ ...gate, checkoutRepository: "invalid" })).toThrow( + /checkout repository/u, + ); expect(() => validatePrGateState({ ...gate, expectedShards: {} })).toThrow(/shard selections/u); expect(() => validatePrGateState({ ...gate, expectedTargets: ["unknown-target"] })).toThrow( /State targets/u, @@ -420,7 +405,9 @@ describe("PR E2E controller", () => { await expect( dispatchPrGate({ repository: "NVIDIA/NemoClaw", + checkoutRepository: "NVIDIA/NemoClaw", token: "token", + controllerCheckId: 101, jobs, targets: [DCODE_TARGET], prNumber: 42, @@ -439,8 +426,10 @@ describe("PR E2E controller", () => { inputs: { jobs: jobs.join(","), targets: DCODE_TARGET, + controller_check_id: "101", pr_number: "42", checkout_sha: HEAD_SHA, + checkout_repository: "NVIDIA/NemoClaw", base_sha: BASE_SHA, workflow_sha: WORKFLOW_SHA, plan_hash: "c".repeat(64), @@ -503,7 +492,9 @@ describe("PR E2E controller", () => { await expect( dispatchPrGate({ repository: "NVIDIA/NemoClaw", + checkoutRepository: "NVIDIA/NemoClaw", token: "token", + controllerCheckId: 101, jobs: ["onboard-repair"], prNumber: 42, commitSha: HEAD_SHA, @@ -570,7 +561,9 @@ describe("PR E2E controller", () => { await expect( dispatchPrGate({ repository: "NVIDIA/NemoClaw", + checkoutRepository: "NVIDIA/NemoClaw", token: "token", + controllerCheckId: 101, jobs: ["onboard-repair"], prNumber: 42, commitSha: HEAD_SHA, @@ -637,7 +630,9 @@ describe("PR E2E controller", () => { await expect( dispatchPrGate({ repository: "NVIDIA/NemoClaw", + checkoutRepository: "NVIDIA/NemoClaw", token: "token", + controllerCheckId: 101, jobs: ["onboard-repair"], prNumber: 42, commitSha: HEAD_SHA, @@ -691,7 +686,9 @@ describe("PR E2E controller", () => { await expect( dispatchPrGate({ repository: "NVIDIA/NemoClaw", + checkoutRepository: "NVIDIA/NemoClaw", token: "token", + controllerCheckId: 101, jobs: ["onboard-repair"], prNumber: 42, commitSha: HEAD_SHA, @@ -1251,7 +1248,7 @@ describe("PR E2E controller", () => { }); const outputs = fs.readFileSync(outputPath, "utf8"); expect(outputs).toContain("dispatched=true"); - expect(outputs).not.toContain("fork_skip_mode="); + expect(outputs).not.toContain("approval_mode="); expect(outputs).not.toContain("finalized=true"); } finally { fs.rmSync(workDir, { recursive: true, force: true }); diff --git a/test/pr-e2e-required.test.ts b/test/pr-e2e-required.test.ts index c36b9f0de71..9955158222d 100644 --- a/test/pr-e2e-required.test.ts +++ b/test/pr-e2e-required.test.ts @@ -86,20 +86,23 @@ describe("native PR E2E required job", () => { expect(result.stderr).not.toContain("ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX"); }); - it("classifies fork-skip approval failures as pending", () => { + it("treats completed authorization failures as terminal", () => { expect( classifyCoordinationCheck( check("E2E / PR Gate", { conclusion: "failure", - output: { title: "Maintainer approval required to skip credentialed E2E" }, + output: { title: "E2E reviewer authorization failed" }, }), identity.repository, ), ).toEqual({ - state: "waiting", - description: "Maintainer approval required to skip credentialed E2E", - detailsUrl: "https://github.com/NVIDIA/NemoClaw/actions/runs/99", - logUrls: ["https://github.com/NVIDIA/NemoClaw/actions/runs/99"], + state: "complete", + result: { + conclusion: "failure", + title: "E2E reviewer authorization failed", + detailsUrl: "https://github.com/NVIDIA/NemoClaw/actions/runs/99", + logUrls: ["https://github.com/NVIDIA/NemoClaw/actions/runs/99"], + }, }); }); diff --git a/test/skills/check-gates-fork-evidence.test.ts b/test/skills/check-gates-fork-evidence.test.ts new file mode 100644 index 00000000000..7dc0e525d16 --- /dev/null +++ b/test/skills/check-gates-fork-evidence.test.ts @@ -0,0 +1,202 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; +import { + exactDiffGateRun, + HEAD_SHA, + prWorkflowRun, + runGate, + successfulRequiredChecks, +} from "./check-gates-test-fixtures.ts"; + +describe("maintainer merge-gate fork evidence", () => { + it("accepts association-less PR CI with exact immutable diff and head metadata", () => { + const output = JSON.parse( + runGate({ + body: "Signed-off-by: Example User ", + verified: true, + headRepository: "example/fork", + emptyHeadRepositoryNameWithOwner: true, + actionRunAttempts: { + "90": { + ...prWorkflowRun( + "success", + [ + { id: 1, name: "checks" }, + { id: 2, name: "changes" }, + ], + true, + ), + headRepository: "example/fork", + pullRequests: [], + }, + }, + }).stdout, + ); + + expect(output).toMatchObject({ allPass: true, gates: { ci: { pass: true } } }); + }); + + it("fails closed when GitHub head repository fields contradict each other", () => { + const result = runGate({ + body: "Signed-off-by: Example User ", + verified: true, + headRepository: "example/fork", + headRepositoryNameWithOwner: "other/fork", + }); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("Failed to resolve PR #42 head repository"); + }); + + it.each([ + "pull_request", + "pull_request_target", + ])("accepts an association-less %s check tied to the exact head metadata", (event) => { + const output = JSON.parse( + runGate({ + body: "Signed-off-by: Example User ", + verified: true, + headRepository: "example/fork", + statusChecks: [ + ...successfulRequiredChecks(), + { + __typename: "CheckRun", + name: "optional-check", + workflowName: "CI / Optional", + detailsUrl: "https://github.com/NVIDIA/NemoClaw/actions/runs/449/job/41", + startedAt: "2026-01-01T00:00:00Z", + status: "COMPLETED", + conclusion: "SUCCESS", + }, + ], + actionRunAttempts: { + "449": { + attempt: 1, + headSha: HEAD_SHA, + headBranch: "feature-branch", + headRepository: "example/fork", + pullRequests: [], + event, + path: ".github/workflows/optional.yaml", + status: "completed", + conclusion: "success", + jobs: [{ id: 41, name: "optional-check" }], + }, + }, + }).stdout, + ); + + expect(output).toMatchObject({ allPass: true, gates: { ci: { pass: true } } }); + }); + + it.each([ + ["head SHA", { headSha: "c".repeat(40) }], + ["head branch", { headBranch: "other-branch" }], + ["head repository", { headRepository: "other/fork" }], + ])("rejects association-less evidence with another %s", (_name, override) => { + const output = JSON.parse( + runGate({ + body: "Signed-off-by: Example User ", + verified: true, + headRepository: "example/fork", + statusChecks: [ + ...successfulRequiredChecks(), + { + __typename: "CheckRun", + name: "optional-check", + workflowName: "CI / Optional", + detailsUrl: "https://github.com/NVIDIA/NemoClaw/actions/runs/449/job/41", + startedAt: "2026-01-01T00:00:00Z", + status: "COMPLETED", + conclusion: "SUCCESS", + }, + ], + actionRunAttempts: { + "449": { + attempt: 1, + headSha: HEAD_SHA, + headBranch: "feature-branch", + headRepository: "example/fork", + pullRequests: [], + event: "pull_request", + path: ".github/workflows/optional.yaml", + status: "completed", + conclusion: "success", + jobs: [{ id: 41, name: "optional-check" }], + ...override, + }, + }, + }).stdout, + ); + + expect(output.gates.ci).toMatchObject({ + pass: false, + failingChecks: ["optional-check: latest attempt evidence incomplete"], + }); + }); + + it("does not use association-less head metadata for an internal PR", () => { + const output = JSON.parse( + runGate({ + body: "Signed-off-by: Example User ", + verified: true, + statusChecks: [ + ...successfulRequiredChecks(), + { + __typename: "CheckRun", + name: "optional-check", + workflowName: "CI / Optional", + detailsUrl: "https://github.com/NVIDIA/NemoClaw/actions/runs/449/job/41", + startedAt: "2026-01-01T00:00:00Z", + status: "COMPLETED", + conclusion: "SUCCESS", + }, + ], + actionRunAttempts: { + "449": { + attempt: 1, + headSha: HEAD_SHA, + headBranch: "feature-branch", + headRepository: "NVIDIA/NemoClaw", + pullRequests: [], + event: "pull_request", + path: ".github/workflows/optional.yaml", + status: "completed", + conclusion: "success", + jobs: [{ id: 41, name: "optional-check" }], + }, + }, + }).stdout, + ); + + expect(output.gates.ci).toMatchObject({ + pass: false, + failingChecks: ["optional-check: latest attempt evidence incomplete"], + }); + }); + + it("does not let association-less head metadata bypass E2E coordination", () => { + const output = JSON.parse( + runGate({ + body: "Signed-off-by: Example User ", + verified: true, + headRepository: "example/fork", + coordinationCheckPages: [{ total_count: 0, check_runs: [] }], + actionRunAttempts: { + "94": { + ...exactDiffGateRun("success", [{ id: 1, name: "E2E / PR Gate" }]), + headRepository: "example/fork", + pullRequests: [], + }, + }, + }).stdout, + ); + + expect(output.gates.ci).toMatchObject({ + pass: false, + failingChecks: ["E2E / PR Gate: latest attempt evidence incomplete"], + }); + }); +}); diff --git a/test/skills/check-gates-test-fixtures.ts b/test/skills/check-gates-test-fixtures.ts index c1088dc1de5..adb4a4ea058 100644 --- a/test/skills/check-gates-test-fixtures.ts +++ b/test/skills/check-gates-test-fixtures.ts @@ -112,6 +112,10 @@ interface ComplianceFixture { state: string; submittedAt?: string | null; }>; + headRefName?: string; + headRepository?: string; + emptyHeadRepositoryNameWithOwner?: boolean; + headRepositoryNameWithOwner?: string; prAuthorLogin?: string; mergeable?: string; mergeStateStatus?: string; @@ -254,6 +258,8 @@ function runGate(fixture: ComplianceFixture) { fs.mkdirSync(bin); const ghPath = path.join(bin, "gh"); + const headRepository = fixture.headRepository ?? "NVIDIA/NemoClaw"; + const [headRepositoryOwner, headRepositoryName] = headRepository.split("/"); const pr = { number: 42, title: "fix(policy): align maintainer workflow", @@ -271,9 +277,15 @@ function runGate(fixture: ComplianceFixture) { isDraft: false, headRefOid: HEAD_SHA, baseRefOid: BASE_SHA, - headRefName: "feature-branch", + headRefName: fixture.headRefName ?? "feature-branch", baseRefName: "main", - headRepository: { nameWithOwner: "NVIDIA/NemoClaw" }, + headRepository: { + name: headRepositoryName, + nameWithOwner: + fixture.headRepositoryNameWithOwner ?? + (fixture.emptyHeadRepositoryNameWithOwner ? "" : headRepository), + }, + headRepositoryOwner: { login: headRepositoryOwner }, author: { login: fixture.prAuthorLogin ?? "contributor" }, }; const finalPr = { ...pr, ...fixture.finalPr }; diff --git a/tools/e2e/operations-workflow-boundary.mts b/tools/e2e/operations-workflow-boundary.mts index a457631a9dc..c76acb4cbdf 100644 --- a/tools/e2e/operations-workflow-boundary.mts +++ b/tools/e2e/operations-workflow-boundary.mts @@ -135,12 +135,83 @@ function requireNode24GithubScript(errors: string[], step: WorkflowStep, owner: } } +function validateControllerAuthorization( + errors: string[], + workflow: OperationsWorkflow, + matrixJob: WorkflowJob, +): void { + if (permissionMap(workflow.permissions).checks !== "read") { + errors.push("E2E workflow must grant read-only check access for controller authentication"); + } + const steps = matrixJob.steps ?? []; + const authenticationIndex = steps.findIndex( + (step) => step.name === "Authenticate controller dispatch", + ); + const checkoutIndex = steps.findIndex((step) => step.uses?.startsWith("actions/checkout@")); + const validationIndex = steps.findIndex((step) => step.name === "Validate controller dispatch"); + const authentication = authenticationIndex >= 0 ? steps[authenticationIndex] : {}; + if (authentication.if !== "${{ inputs.checkout_sha != '' }}") { + errors.push("Controller authentication must be activated only by checkout_sha"); + } + if ( + authenticationIndex < 0 || + checkoutIndex < 0 || + validationIndex < 0 || + authenticationIndex >= checkoutIndex || + checkoutIndex >= validationIndex + ) { + errors.push("Controller authentication must run before untrusted checkout and PR validation"); + } + const expectedEnvironment = { + ACTOR: "${{ github.actor }}", + BASE_SHA: "${{ inputs.base_sha }}", + CHECKOUT_SHA: "${{ inputs.checkout_sha }}", + CONTROLLER_CHECK_ID: "${{ inputs.controller_check_id }}", + CORRELATION_ID: "${{ inputs.correlation_id }}", + JOBS: "${{ inputs.jobs }}", + PLAN_HASH: "${{ inputs.plan_hash }}", + PR_NUMBER: "${{ inputs.pr_number }}", + RUN_ATTEMPT: "${{ github.run_attempt }}", + RUN_ID: "${{ github.run_id }}", + TARGETS: "${{ inputs.targets }}", + }; + for (const [name, value] of Object.entries(expectedEnvironment)) { + if (authentication.env?.[name] !== value) { + errors.push(`Controller authentication must bind ${name}`); + } + } + const source = String(authentication.run ?? ""); + for (const fragment of [ + '"$ACTOR" == "github-actions[bot]"', + '"$RUN_ATTEMPT" == "1"', + '"$CONTROLLER_CHECK_ID" =~ ^[1-9][0-9]*$', + "nemoclaw-pr-e2e:v2:${PR_NUMBER}:${CHECKOUT_SHA}:${BASE_SHA}", + "https://github.com/${GITHUB_REPOSITORY}/actions/runs/${RUN_ID}", + "https://api.github.com/repos/${GITHUB_REPOSITORY}/check-runs/${CONTROLLER_CHECK_ID}", + `[[ "$(jq -r '.details_url // ""' <<< "$check_json")" == "$expected_run_url" ]]`, + '.name == "E2E / PR Gate Coordination"', + ".app.id == 15368", + '.app.slug == "github-actions"', + ".external_id == $external_id", + '.status == "in_progress"', + ".conclusion == null", + ".details_url == $run_url", + ".output.summary == $summary", + ]) { + if (!source.includes(fragment)) { + errors.push(`Controller authentication must retain ${fragment}`); + } + } +} + function validatePrGateDispatch(errors: string[], workflow: OperationsWorkflow): void { const inputs = workflow.on?.workflow_dispatch?.inputs ?? {}; for (const name of [ "jobs", "pr_number", "checkout_sha", + "checkout_repository", + "controller_check_id", "base_sha", "workflow_sha", "plan_hash", @@ -178,6 +249,7 @@ function validatePrGateDispatch(errors: string[], workflow: OperationsWorkflow): } const matrixJob = workflow.jobs["generate-matrix"] ?? {}; + validateControllerAuthorization(errors, workflow, matrixJob); const steps = matrixJob.steps ?? []; const validationIndex = steps.findIndex((step) => step.name === "Validate controller dispatch"); const prepareIndex = steps.findIndex((step) => step.name === "Prepare E2E workspace"); @@ -190,6 +262,7 @@ function validatePrGateDispatch(errors: string[], workflow: OperationsWorkflow): } const expectedStepEnvironment = { BASE_SHA: "${{ inputs.base_sha }}", + CHECKOUT_REPOSITORY: "${{ inputs.checkout_repository }}", CHECKOUT_SHA: "${{ inputs.checkout_sha }}", EXPECTED_WORKFLOW_SHA: "${{ inputs.workflow_sha }}", JOBS: "${{ inputs.jobs }}", @@ -208,6 +281,7 @@ function validatePrGateDispatch(errors: string[], workflow: OperationsWorkflow): for (const fragment of [ '"$WORKFLOW_EVENT" == "workflow_dispatch"', '"$WORKFLOW_REF" == "refs/heads/main"', + '"$CHECKOUT_REPOSITORY" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$', '"$CHECKOUT_SHA" =~ ^[a-f0-9]{40}$', '"$BASE_SHA" =~ ^[a-f0-9]{40}$', '"$WORKFLOW_SHA" == "$EXPECTED_WORKFLOW_SHA"', @@ -220,6 +294,7 @@ function validatePrGateDispatch(errors: string[], workflow: OperationsWorkflow): "https://api.github.com/repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}", "'.state'", "'.head.repo.full_name // \"\"'", + `[[ "$(jq -r '.head.repo.full_name // ""' <<< "$pull_json")" == "$CHECKOUT_REPOSITORY" ]]`, `[[ "$(jq -r '.head.sha' <<< "$pull_json")" == "$CHECKOUT_SHA" ]]`, `[[ "$(jq -r '.base.sha' <<< "$pull_json")" == "$BASE_SHA" ]]`, ]) { @@ -248,16 +323,25 @@ function validatePrGateDispatch(errors: string[], workflow: OperationsWorkflow): step.name === "Check out trusted E2E workflow" && step.if === PUBLICATION_REQUIRED_CONDITION && step.with?.ref === "${{ github.sha }}"; + const trustedCheckout = + trustedHermesFixtureCheckout || + trustedReportHelperCheckout || + trustedLaunchableLaneCheckout || + trustedPublicationCheckout; if ( step.uses?.startsWith("actions/checkout@") && step.with?.ref !== "${{ inputs.checkout_sha || github.sha }}" && - !trustedHermesFixtureCheckout && - !trustedReportHelperCheckout && - !trustedLaunchableLaneCheckout && - !trustedPublicationCheckout + !trustedCheckout ) { errors.push(`${jobName} checkout must use the selected PR commit`); } + if ( + step.uses?.startsWith("actions/checkout@") && + !trustedCheckout && + step.with?.repository !== "${{ inputs.checkout_repository || github.repository }}" + ) { + errors.push(`${jobName} checkout must use the selected PR head repository`); + } } } } diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index 41d2e60cbc5..5f40bfbad55 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -38,7 +38,7 @@ import { const E2E_WORKFLOW = "e2e.yaml"; const E2E_WORKFLOW_PATH = `.github/workflows/${E2E_WORKFLOW}`; const PR_GATE_WORKFLOW_PATH = ".github/workflows/pr-e2e-gate.yaml"; -const FORK_SKIP_APPROVAL_ENVIRONMENT = "approve-credentialed-e2e-skip-for-fork-pr"; +const FORK_E2E_APPROVAL_ENVIRONMENT = "approve-credentialed-e2e-for-fork-pr"; const INTERNAL_E2E_APPROVAL_ENVIRONMENT = "approve-credentialed-e2e-for-internal-pr"; const CHECK_NAME = "E2E / PR Gate Coordination"; const WORKFLOW_NAME = "E2E / PR Gate Controller"; @@ -46,6 +46,7 @@ const RESERVED_CHECK_TITLE = "Waiting for PR CI"; const RESERVED_CHECK_SUMMARY = "This PR SHA and base SHA are reserved for deterministic E2E planning after CI completes."; const CONTROL_PLANE_AUTHORIZATION_TITLE = "E2E reviewer authorization required to run E2E"; +const FORK_E2E_AUTHORIZATION_TITLE = "E2E reviewer authorization required to run fork E2E"; const RETRYABLE_FAILURE_MARKER_PREFIX = ""; const RETRYABLE_FAILURE_REASONS = new Set([ @@ -110,8 +111,6 @@ const MAX_REPORTED_WORKFLOW_JOBS = 10; const MAX_WAIVER_REASON_CHARS = 500; const MAX_APPROVAL_REVIEWS = 20; const MAINTAINER_PATTERN = /^[A-Za-z0-9](?:[A-Za-z0-9-]{0,37}[A-Za-z0-9])?$/u; -const EVIDENCE_URL_PATTERN = - /^https:\/\/github\.com\/NVIDIA\/NemoClaw\/actions\/runs\/[1-9][0-9]*$/u; const ACTIVE_WORKFLOW_RUN_STATUSES = [ "requested", "waiting", @@ -151,28 +150,6 @@ type ControllerPathSlot = "initial" | "runner-loss-retry"; type EvidenceStepOutcome = "success" | "failure" | "cancelled" | "skipped"; -type ManualForkSkipCommandBase = { - prNumber: number; - headSha: string; - baseSha: string; - workflowSha: string; - maintainer: string; - reason: string; - evidenceUrl?: string; -}; - -type ManualForkSkipCommand = ManualForkSkipCommandBase & { mode: "record-fork-e2e-skip" }; - -type ApprovedForkSkipCommand = { - mode: "record-approved-fork-e2e-skip"; - prNumber: number; - headSha: string; - baseSha: string; - workflowSha: string; - approvalRunId: number; - approvalRunAttempt: number; -}; - type ControlPlaneCommandBase = { prNumber: number; headSha: string; @@ -194,16 +171,15 @@ type ApprovedControlPlaneDispatchCommand = ControlPlaneCommandBase & { approvalRunAttempt: number; }; -type AuthorizedControlPlaneCommand = ControlPlaneCommandBase & { - maintainer: string; - reason: string; +type ApprovedForkE2EDispatchCommand = ControlPlaneCommandBase & { + mode: "start-approved-fork"; + approvalRunId: number; + approvalRunAttempt: number; }; -type ForkSkipCommand = ManualForkSkipCommand & { - validatedApproval?: { - environment: typeof FORK_SKIP_APPROVAL_ENVIRONMENT; - runUrl: string; - }; +type AuthorizedE2ECommand = ControlPlaneCommandBase & { + maintainer: string; + reason: string; }; export type ControllerCommand = @@ -254,8 +230,7 @@ export type ControllerCommand = } | ControlPlaneDispatchCommand | ApprovedControlPlaneDispatchCommand - | ManualForkSkipCommand - | ApprovedForkSkipCommand; + | ApprovedForkE2EDispatchCommand; type CheckConclusion = "success" | "failure" | "cancelled"; @@ -381,9 +356,10 @@ type WorkflowRunIdentity = { }; export type PrGateState = { - version: 3; + version: 4; commitSha: string; baseSha: string; + checkoutRepository: string; workflowSha: string; planHash: string; correlationId: string; @@ -719,49 +695,36 @@ export function parseControllerCommand(argv: string[]): ControllerCommand { ...privateControllerPaths(requiredArgument(args.workDir, "work-dir")), }; } - if (args.mode === "record-fork-e2e-skip") { - const maintainer = requiredArgument(args.maintainer, "maintainer"); - if (!MAINTAINER_PATTERN.test(maintainer)) throw new Error("--maintainer is invalid"); - const evidenceUrl = args.evidenceUrl?.trim(); - if (evidenceUrl && !EVIDENCE_URL_PATTERN.test(evidenceUrl)) { - throw new Error( - "Evidence URL must be an Actions run URL such as https://github.com/NVIDIA/NemoClaw/actions/runs/123. PR, issue, comment, job, and external URLs are not accepted. Leave the field blank if no run exists.", - ); - } - return { - mode: args.mode, - prNumber: parsePositiveId(requiredArgument(args.pr, "pr"), "--pr"), - headSha: requiredArgument(args.head, "head"), - baseSha: requiredArgument(args.base, "base"), - workflowSha: requiredArgument(args.workflowSha, "workflow-sha"), - maintainer, - reason: normalizedWaiverReason(requiredArgument(args.reason, "reason")), - ...(evidenceUrl ? { evidenceUrl } : {}), - }; - } - if (args.mode === "record-approved-fork-e2e-skip") { + if (args.mode === "start-approved-fork") { + const workflowRunAttempt = parsePositiveId( + requiredArgument(args.workflowRunAttempt, "workflow-run-attempt"), + "--workflow-run-attempt", + ); const approvalRunAttempt = parsePositiveId( requiredArgument(args.approvalRunAttempt, "approval-run-attempt"), "--approval-run-attempt", ); - if (approvalRunAttempt !== 1) { - throw new Error("--approval-run-attempt must be exactly 1"); + if (workflowRunAttempt !== 1 || approvalRunAttempt !== 1) { + throw new Error("workflow and approval run attempts must be exactly 1"); } return { - mode: "record-approved-fork-e2e-skip", + mode: "start-approved-fork", prNumber: parsePositiveId(requiredArgument(args.pr, "pr"), "--pr"), headSha: requiredArgument(args.head, "head"), baseSha: requiredArgument(args.base, "base"), workflowSha: requiredArgument(args.workflowSha, "workflow-sha"), + gateRunId: parsePositiveId(requiredArgument(args.gateRunId, "gate-run-id"), "--gate-run-id"), + workflowRunAttempt, approvalRunId: parsePositiveId( requiredArgument(args.approvalRunId, "approval-run-id"), "--approval-run-id", ), approvalRunAttempt, + ...privateControllerPaths(requiredArgument(args.workDir, "work-dir")), }; } throw new Error( - "--mode must be seed, start, start-control-plane, start-approved-control-plane, finish, abandon, abandon-runner-loss-retry, cancel, wait, download, retry-runner-loss, record-fork-e2e-skip, or record-approved-fork-e2e-skip", + "--mode must be seed, start, start-control-plane, start-approved-control-plane, start-approved-fork, finish, abandon, abandon-runner-loss-retry, cancel, wait, download, or retry-runner-loss", ); } @@ -770,7 +733,7 @@ function readRegularJson(file: string, maxBytes = MAX_PLAN_BYTES): unknown { } export function validatePrGateState(value: unknown): PrGateState { - if (!isObjectRecord(value) || value.version !== 3) { + if (!isObjectRecord(value) || value.version !== 4) { throw new Error("State version is invalid"); } if (typeof value.commitSha !== "string" || !SHA_PATTERN.test(value.commitSha)) { @@ -779,6 +742,10 @@ export function validatePrGateState(value: unknown): PrGateState { if (typeof value.baseSha !== "string" || !SHA_PATTERN.test(value.baseSha)) { throw new Error("State base SHA is invalid"); } + if (typeof value.checkoutRepository !== "string") { + throw new Error("State checkout repository is invalid"); + } + assertRepository(value.checkoutRepository, "state checkout repository"); if (typeof value.workflowSha !== "string" || !SHA_PATTERN.test(value.workflowSha)) { throw new Error("State workflow SHA is invalid"); } @@ -1025,16 +992,16 @@ function appendOutput(name: string, value: string): void { const output = process.env.GITHUB_OUTPUT; if (!output) return; const validators: Readonly boolean>> = { + approval_base_sha: (candidate) => SHA_PATTERN.test(candidate), + approval_environment: (candidate) => + candidate === INTERNAL_E2E_APPROVAL_ENVIRONMENT || + candidate === FORK_E2E_APPROVAL_ENVIRONMENT, + approval_head_sha: (candidate) => SHA_PATTERN.test(candidate), + approval_mode: (candidate) => + candidate === "start-approved-control-plane" || candidate === "start-approved-fork", + approval_pr_number: (candidate) => /^[1-9][0-9]*$/u.test(candidate), check_id: (candidate) => /^[1-9][0-9]*$/u.test(candidate), - control_plane_approval_base_sha: (candidate) => SHA_PATTERN.test(candidate), - control_plane_approval_head_sha: (candidate) => SHA_PATTERN.test(candidate), - control_plane_approval_mode: (candidate) => candidate === "start-approved-control-plane", - control_plane_approval_pr_number: (candidate) => /^[1-9][0-9]*$/u.test(candidate), dispatched: (candidate) => /^(?:true|false)$/u.test(candidate), - fork_skip_base_sha: (candidate) => SHA_PATTERN.test(candidate), - fork_skip_head_sha: (candidate) => SHA_PATTERN.test(candidate), - fork_skip_mode: (candidate) => candidate === "record-fork-e2e-skip", - fork_skip_pr_number: (candidate) => /^[1-9][0-9]*$/u.test(candidate), finalized: (candidate) => /^(?:true|false)$/u.test(candidate), runner_loss_retry_authorized: (candidate) => candidate === "true", run_id: (candidate) => /^[1-9][0-9]*$/u.test(candidate), @@ -1059,11 +1026,18 @@ function appendOutput(name: string, value: string): void { } } -function emitControlPlaneApprovalOutputs(prNumber: number, headSha: string, baseSha: string): void { - appendOutput("control_plane_approval_mode", "start-approved-control-plane"); - appendOutput("control_plane_approval_pr_number", String(prNumber)); - appendOutput("control_plane_approval_head_sha", headSha); - appendOutput("control_plane_approval_base_sha", baseSha); +function emitE2EApprovalOutputs( + mode: "start-approved-control-plane" | "start-approved-fork", + environment: typeof INTERNAL_E2E_APPROVAL_ENVIRONMENT | typeof FORK_E2E_APPROVAL_ENVIRONMENT, + prNumber: number, + headSha: string, + baseSha: string, +): void { + appendOutput("approval_mode", mode); + appendOutput("approval_environment", environment); + appendOutput("approval_pr_number", String(prNumber)); + appendOutput("approval_head_sha", headSha); + appendOutput("approval_base_sha", baseSha); } export function prGateExternalId(prNumber: number, headSha: string, baseSha: string): string { @@ -1078,18 +1052,6 @@ export function prGateExternalId(prNumber: number, headSha: string, baseSha: str return `${CHECK_EXTERNAL_ID_PREFIX}:${prNumber}:${headSha}:${baseSha}`; } -function emitForkSkipOutputs( - mode: ManualForkSkipCommand["mode"], - prNumber: number, - headSha: string, - baseSha: string, -): void { - appendOutput("fork_skip_mode", mode); - appendOutput("fork_skip_pr_number", String(prNumber)); - appendOutput("fork_skip_head_sha", headSha); - appendOutput("fork_skip_base_sha", baseSha); -} - function validateCheckRunsResponse(value: unknown): CheckRunsResponse { if ( !isObjectRecord(value) || @@ -3093,7 +3055,9 @@ function workflowJobEvidenceFingerprint(details: { export async function dispatchPrGate(options: { repository: string; + checkoutRepository: string; token: string; + controllerCheckId: number; jobs: readonly string[]; targets?: readonly string[]; prNumber: number; @@ -3104,6 +3068,7 @@ export async function dispatchPrGate(options: { correlationId: string; }): Promise<{ runId: number; workflowSha: string }> { assertRepository(options.repository, "repository"); + assertRepository(options.checkoutRepository, "checkout repository"); const targets = options.targets ?? []; if ( !options.token || @@ -3113,6 +3078,8 @@ export async function dispatchPrGate(options: { new Set(targets).size !== targets.length || targets.some((target) => !JOB_PATTERN.test(target) || !isPrE2eTypedTargetId(target)) || options.jobs.some((job) => targets.includes(job)) || + !Number.isSafeInteger(options.controllerCheckId) || + options.controllerCheckId < 1 || !Number.isSafeInteger(options.prNumber) || options.prNumber < 1 || !SHA_PATTERN.test(options.commitSha) || @@ -3138,8 +3105,10 @@ export async function dispatchPrGate(options: { inputs: { jobs: options.jobs.join(","), targets: targets.join(","), + controller_check_id: String(options.controllerCheckId), pr_number: String(options.prNumber), checkout_sha: options.commitSha, + checkout_repository: options.checkoutRepository, base_sha: options.baseSha, workflow_sha: workflowSha, plan_hash: options.planHash, @@ -3323,9 +3292,15 @@ async function dispatchSelectedPrGate(options: { if (!CORRELATION_PATTERN.test(correlationId)) { throw new Error("generated correlation ID is invalid"); } + const checkoutRepository = options.pull.head.repo?.full_name; + if (!checkoutRepository) { + throw new Error("PR head repository is unavailable"); + } const dispatch = await dispatchPrGate({ repository: options.repository, + checkoutRepository, token: options.token, + controllerCheckId: options.checkRunId, jobs, targets, prNumber: options.pull.number, @@ -3339,9 +3314,10 @@ async function dispatchSelectedPrGate(options: { try { appendOutput("run_id", String(childRunId)); const state: PrGateState = { - version: 3, + version: 4, commitSha: options.pull.head.sha, baseSha: options.baseSha, + checkoutRepository, workflowSha: dispatch.workflowSha, planHash: options.plan.planHash, correlationId, @@ -3402,7 +3378,9 @@ async function dispatchRunnerLossRetry(options: { } const dispatch = await dispatchPrGate({ repository: options.repository, + checkoutRepository: options.state.checkoutRepository, token: options.token, + controllerCheckId: options.checkRunId, jobs: options.state.expectedJobs, targets: options.state.expectedTargets, prNumber: options.state.prNumber, @@ -3577,8 +3555,8 @@ export async function retryRunnerLossPrGate( headSha: state.commitSha, baseSha: state.baseSha, }); - if (pull.head.repo?.full_name !== repository) { - throw new Error("runner-loss retry requires an internal pull request"); + if (pull.head.repo?.full_name !== state.checkoutRepository) { + throw new Error("runner-loss retry checkout repository no longer matches the PR"); } const confirmedJobDetails = await listNonPassingWorkflowJobs( @@ -3833,26 +3811,35 @@ export async function startPrGate( if (command.headRepository !== repository && selections.length > 0) { const gateRunUrl = `https://github.com/${repository}/actions/runs/${command.gateRunId}`; const gateRunLink = `[${WORKFLOW_NAME} run ${command.gateRunId}](${gateRunUrl})`; - await completeCheck( - { repository, checkRunId }, - token, + await markCheckInProgress( { - conclusion: "failure", - title: "Maintainer approval required to skip credentialed E2E", - summary: [ - `This fork PR diff (head ${command.headSha}, base ${ciIdentity.baseSha}) selected credential-bearing E2E checks (${selectionSummary}).`, - "The selected jobs and targets were not run. No fork code received repository secrets.", - `Open ${gateRunLink}, choose Review deployments, and approve the \`${FORK_SKIP_APPROVAL_ENVIRONMENT}\` environment to record this skip. If Review deployments is absent, the environment is unprotected or the run is no longer waiting; configure it, update the PR to create a new head, and trigger fresh PR CI. GitHub records the reviewer and optional comment. The manual \`approve-fork-e2e-skip\` workflow operation remains available as fallback.`, - ].join("\n\n"), + repository, + checkRunId, + prNumber: ciIdentity.prNumber, + headSha: command.headSha, + baseSha: ciIdentity.baseSha, }, - gateRunUrl, + token, + FORK_E2E_AUTHORIZATION_TITLE, + [ + `Review scope: PR #${pull.number}; head repository \`${command.headRepository}\`; head SHA \`${command.headSha}\`; base SHA \`${ciIdentity.baseSha}\`; ${selectionSummary}; deterministic plan \`${plan.planHash}\`.`, + "No selected E2E job or target ran. No repository credential was exposed to fork code.", + `An authorized E2E reviewer must review the exact fork code and risk plan. Open ${gateRunLink}, choose Review deployments, and approve the \`${FORK_E2E_APPROVAL_ENVIRONMENT}\` environment. Approval authorizes the selected fork code to run with E2E credentials. GitHub records the reviewer and optional comment.`, + "If Review deployments is absent, configure the protected environment. Then, update the PR to create a new PR SHA and run fresh PR CI.", + ].join("\n\n"), + ); + emitE2EApprovalOutputs( + "start-approved-fork", + FORK_E2E_APPROVAL_ENVIRONMENT, + pull.number, + command.headSha, + ciIdentity.baseSha, ); - emitForkSkipOutputs("record-fork-e2e-skip", pull.number, command.headSha, ciIdentity.baseSha); appendOutput("dispatched", "false"); appendOutput("finalized", "true"); finalized = true; console.log( - `Fork not dispatched: pr=${pull.number} sha=${command.headSha} plan=${plan.planHash} jobs=${jobs.join(",")} targets=${targets.join(",")}`, + `Fork authorization required: pr=${pull.number} sha=${command.headSha} plan=${plan.planHash} jobs=${jobs.join(",")} targets=${targets.join(",")}`, ); return; } @@ -3879,7 +3866,13 @@ export async function startPrGate( `Deterministic plan: \`${plan.planHash}\`.`, ].join("\n\n"), ); - emitControlPlaneApprovalOutputs(pull.number, command.headSha, ciIdentity.baseSha); + emitE2EApprovalOutputs( + "start-approved-control-plane", + INTERNAL_E2E_APPROVAL_ENVIRONMENT, + pull.number, + command.headSha, + ciIdentity.baseSha, + ); appendOutput("dispatched", "false"); appendOutput("finalized", "true"); finalized = true; @@ -3925,8 +3918,9 @@ export async function startPrGate( } } -async function startAuthorizedControlPlanePrGate( - command: AuthorizedControlPlaneCommand, +async function startAuthorizedPrGate( + command: AuthorizedE2ECommand, + authorizationKind: "internal-control-plane" | "fork", ): Promise { const { token, repository } = tokenAndRepository(); if (!SHA_PATTERN.test(command.headSha)) throw new Error("PR head SHA is invalid"); @@ -3937,9 +3931,11 @@ async function startAuthorizedControlPlanePrGate( throw new Error("gate run ID is invalid"); } if (command.workflowRunAttempt !== 1) { - throw new Error("control-plane authorization must use the first workflow run attempt"); + throw new Error("E2E authorization must use the first workflow run attempt"); } const reason = normalizedWaiverReason(command.reason); + const pendingTitle = + authorizationKind === "fork" ? FORK_E2E_AUTHORIZATION_TITLE : CONTROL_PLANE_AUTHORIZATION_TITLE; let checkRunId: number | undefined; try { @@ -3950,9 +3946,13 @@ async function startAuthorizedControlPlanePrGate( headSha: command.headSha, baseSha: command.baseSha, }); - if (pull.head.repo?.full_name !== repository) { + const isFork = pull.head.repo?.full_name !== repository; + if (authorizationKind === "internal-control-plane" && isFork) { throw new Error("control-plane E2E authorization requires an internal pull request"); } + if (authorizationKind === "fork" && !isFork) { + throw new Error("fork E2E authorization requires a fork pull request"); + } const changedFiles = await pullChangedFiles(repository, pull, token); const inventory = readFreeStandingJobsInventory(); const plan = validateRiskPlan( @@ -3963,13 +3963,16 @@ async function startAuthorizedControlPlanePrGate( }), new Set(inventory.allowedJobs), ); - if (!requiresCredentialedE2eAuthorization(plan)) { + if ( + authorizationKind === "internal-control-plane" && + !requiresCredentialedE2eAuthorization(plan) + ) { throw new Error("pull request does not require credentialed E2E authorization"); } const jobs = riskPlanRequiredJobIds(plan); const targets = riskPlanRequiredTargetIds(plan); if (jobs.length + targets.length === 0) { - throw new Error("authorized control-plane plan selected no E2E jobs or targets"); + throw new Error("authorized plan selected no E2E jobs or targets"); } writePrivateRegularFile(command.planPath, `${JSON.stringify(plan, null, 2)}\n`); const currentPull = await requireLiveExactDiff({ @@ -3995,8 +3998,8 @@ async function startAuthorizedControlPlanePrGate( } const check = matchingChecks[0]!; const pendingAuthorization = check.status === "in_progress" && check.conclusion === null; - if (!pendingAuthorization || check.output?.title !== CONTROL_PLANE_AUTHORIZATION_TITLE) { - throw new Error("PR gate must have the matching pending control-plane authorization state"); + if (!pendingAuthorization || check.output?.title !== pendingTitle) { + throw new Error("PR gate must have the matching pending E2E authorization state"); } checkRunId = check.id; appendOutput("check_id", String(checkRunId)); @@ -4059,16 +4062,18 @@ async function startAuthorizedControlPlanePrGate( baseSha: command.baseSha, }, token, - CONTROL_PLANE_AUTHORIZATION_TITLE, + pendingTitle, [ `The authorized E2E attempt did not produce an accepted result: \`${reason}\`.`, - "Review the controller error and any linked child run. Then, launch a first-attempt `run-control-plane` workflow for the PR/base SHA pair.", + authorizationKind === "fork" + ? "Review the controller error and any linked child run. Then, update the PR to create a new PR SHA and run fresh PR CI." + : "Review the controller error and any linked child run. Then, launch a first-attempt `run-control-plane` workflow for the PR/base SHA pair.", ].join("\n\n"), ); appendOutput("finalized", "true"); } catch (restoreError) { console.error( - `Failed to restore control-plane authorization after controller error: ${controllerErrorMessage(restoreError)}`, + `Failed to restore E2E authorization after controller error: ${controllerErrorMessage(restoreError)}`, ); } } @@ -4085,10 +4090,10 @@ export async function startControlPlanePrGate(command: ControlPlaneDispatchComma command.maintainer, "Control-plane E2E authorization", ); - await startAuthorizedControlPlanePrGate(command); + await startAuthorizedPrGate(command, "internal-control-plane"); } -function approvedControlPlaneReason(comment: string | null): string { +function approvedE2EReason(comment: string | null): string { const normalizedComment = (comment ?? "") .replace(/[\u0000-\u001f\u007f]+/gu, " ") .replace(/\s{2,}/gu, " ") @@ -4134,11 +4139,56 @@ export async function startApprovedControlPlanePrGate( ), INTERNAL_E2E_APPROVAL_ENVIRONMENT, ); - await startAuthorizedControlPlanePrGate({ - ...command, - maintainer: review.reviewer, - reason: approvedControlPlaneReason(review.comment), - }); + await startAuthorizedPrGate( + { + ...command, + maintainer: review.reviewer, + reason: approvedE2EReason(review.comment), + }, + "internal-control-plane", + ); +} + +export async function startApprovedForkPrGate( + command: ApprovedForkE2EDispatchCommand, +): Promise { + const { token, repository } = tokenAndRepository(); + if (!Number.isSafeInteger(command.approvalRunId) || command.approvalRunId < 1) { + throw new Error("approval run ID is invalid"); + } + if (command.approvalRunAttempt !== 1 || command.workflowRunAttempt !== 1) { + throw new Error("approval and workflow run attempts must be exactly 1"); + } + if (command.gateRunId !== command.approvalRunId) { + throw new Error("approval run ID must match the gate run ID"); + } + validateApprovalWorkflowRun( + await githubApi(`repos/${repository}/actions/runs/${command.approvalRunId}`, token, { + userAgent: USER_AGENT, + }), + { + repository, + runId: command.approvalRunId, + runAttempt: command.approvalRunAttempt, + workflowSha: command.workflowSha, + }, + ); + const review = validateApprovalReview( + await githubApi( + `repos/${repository}/actions/runs/${command.approvalRunId}/approvals`, + token, + { userAgent: USER_AGENT }, + ), + FORK_E2E_APPROVAL_ENVIRONMENT, + ); + await startAuthorizedPrGate( + { + ...command, + maintainer: review.reviewer, + reason: approvedE2EReason(review.comment), + }, + "fork", + ); } export function findSignalFiles( @@ -4548,14 +4598,14 @@ function validateApprovalWorkflowRun( function validateApprovalReview( value: unknown, - environment: typeof FORK_SKIP_APPROVAL_ENVIRONMENT | typeof INTERNAL_E2E_APPROVAL_ENVIRONMENT, + environment: typeof FORK_E2E_APPROVAL_ENVIRONMENT | typeof INTERNAL_E2E_APPROVAL_ENVIRONMENT, ): { reviewer: string; comment: string | null } { if (!Array.isArray(value)) { throw new Error("GitHub returned malformed environment approval history"); } if (value.length === 0) { throw new Error( - `No required-reviewer approval was recorded for ${environment}. If Review deployments was absent, the environment may be missing or unprotected, or the run may no longer be waiting; configure it, update the PR to create a new head, then trigger fresh PR CI, or use the manual maintainer fallback.`, + `No required-reviewer approval was recorded for ${environment}. If Review deployments was absent, the environment may be missing or unprotected, or the run may no longer be waiting; configure it, update the PR to create a new PR SHA, then run fresh PR CI.`, ); } if (value.length > MAX_APPROVAL_REVIEWS) { @@ -4604,19 +4654,6 @@ function validateApprovalReview( return { reviewer: review.reviewer, comment: review.comment }; } -function approvedWaiverReason(comment: string | null): string { - const normalizedComment = (comment ?? "") - .replace(/[\u0000-\u001f\u007f]+/gu, " ") - .replace(/\s{2,}/gu, " ") - .trim(); - const baseReason = "Protected environment approval confirmed for this credentialed E2E skip."; - const commentPrefix = " Reviewer comment: "; - const maxCommentChars = MAX_WAIVER_REASON_CHARS - baseReason.length - commentPrefix.length; - const boundedComment = normalizedComment.slice(0, maxCommentChars); - const reason = boundedComment ? `${baseReason}${commentPrefix}${boundedComment}` : baseReason; - return normalizedWaiverReason(reason); -} - async function requireMaintainerPermission( repository: string, token: string, @@ -4637,186 +4674,6 @@ async function requireMaintainerPermission( } } -async function completeForkE2ESkip(command: ForkSkipCommand): Promise { - const { token, repository } = tokenAndRepository(); - if (!SHA_PATTERN.test(command.headSha)) throw new Error("PR head SHA is invalid"); - if (!SHA_PATTERN.test(command.baseSha)) throw new Error("PR base SHA is invalid"); - if (!SHA_PATTERN.test(command.workflowSha)) throw new Error("workflow SHA is invalid"); - if (!MAINTAINER_PATTERN.test(command.maintainer)) throw new Error("maintainer login is invalid"); - const reason = normalizedWaiverReason(command.reason); - if (command.evidenceUrl && !EVIDENCE_URL_PATTERN.test(command.evidenceUrl)) { - throw new Error("evidence URL must name an NVIDIA/NemoClaw Actions run"); - } - - if (!command.validatedApproval) { - await requireMaintainerPermission( - repository, - token, - command.maintainer, - "credentialed E2E skip approvals", - ); - } - - const pull = validatePullRequest( - await githubApi(`repos/${repository}/pulls/${command.prNumber}`, token, { - userAgent: USER_AGENT, - }), - ); - if ( - pull.state !== "open" || - pull.base.repo.full_name !== repository || - !pull.head.repo || - pull.head.sha !== command.headSha || - pull.base.sha !== command.baseSha - ) { - throw new Error("pull request no longer matches the reviewed PR SHA and base SHA"); - } - const isFork = pull.head.repo.full_name !== repository; - if (!isFork) { - throw new Error("credentialed E2E skips require a fork pull request"); - } - - const changedFiles = await pullChangedFiles(repository, pull, token); - const inventory = readFreeStandingJobsInventory(); - const allowedJobs = new Set(inventory.allowedJobs); - const plan = validateRiskPlan( - buildRiskPlan({ - headSha: command.headSha, - changedFiles, - focusedE2eJobs: focusedE2eJobsForChangedFiles(changedFiles, inventory), - }), - allowedJobs, - ); - const jobs = riskPlanRequiredJobIds(plan); - const targets = riskPlanRequiredTargetIds(plan); - if (jobs.length + targets.length === 0) { - throw new Error("pull request does not require a credentialed E2E skip"); - } - const currentPull = validatePullRequest( - await githubApi(`repos/${repository}/pulls/${command.prNumber}`, token, { - userAgent: USER_AGENT, - }), - ); - assertPullUnchanged(pull, currentPull); - - const matchingChecks = await matchingPrGateChecks({ - repository, - token, - headSha: command.headSha, - baseSha: command.baseSha, - prNumber: command.prNumber, - }); - if (matchingChecks.length !== 1) { - throw new Error( - `Expected one PR gate check for the PR/base SHA pair; found ${matchingChecks.length}`, - ); - } - const check = matchingChecks[0]!; - if ( - check.status !== "completed" || - check.conclusion !== "failure" || - check.output?.title !== "Maintainer approval required to skip credentialed E2E" - ) { - throw new Error("PR gate must first complete with the matching skip-approval failure"); - } - - const safeReason = reason.replace(/`/gu, "'"); - const evidence = command.validatedApproval - ? `Validated environment approval run for \`${command.validatedApproval.environment}\`: [${command.validatedApproval.runUrl}](${command.validatedApproval.runUrl}).` - : command.evidenceUrl - ? `Maintainer-supplied Actions reference (not validated by this controller): [${command.evidenceUrl}](${command.evidenceUrl}).` - : "Approval source: manual fallback; no supporting Actions run was supplied."; - const title = `Credentialed E2E skipped for fork PR — approved by @${command.maintainer}`; - const approval = `Maintainer @${command.maintainer} approved skipping credentialed E2E for fork head \`${command.headSha}\` on base \`${command.baseSha}\`.`; - const nonExecution = `Selected jobs and targets not run: ${riskPlanSelectionSummary(plan)}.`; - await compatibleMainWorkflowCommit(repository, token, command.workflowSha); - const finalPull = await requireLiveExactDiff({ - repository, - token, - prNumber: command.prNumber, - headSha: command.headSha, - baseSha: command.baseSha, - }); - assertPullUnchanged(pull, finalPull); - await completeCheck( - { repository, checkRunId: check.id }, - token, - { - conclusion: "success", - title, - summary: [ - "**Outcome: APPROVED SKIP — credentialed E2E did not run.**", - approval, - nonExecution, - `Reason: ${safeReason}`, - evidence, - `Deterministic plan: \`${plan.planHash}\`.`, - ].join("\n\n"), - }, - command.validatedApproval?.runUrl ?? - command.evidenceUrl ?? - `https://github.com/${repository}/pull/${pull.number}`, - ); - console.log( - `Credentialed E2E skip recorded: mode=${command.mode} pr=${pull.number} head=${command.headSha} base=${command.baseSha} maintainer=${command.maintainer} plan=${plan.planHash}`, - ); -} - -export async function recordManualForkE2ESkip( - command: Extract, -): Promise { - await completeForkE2ESkip(command); -} - -export async function recordApprovedForkE2ESkip(command: ApprovedForkSkipCommand): Promise { - const { token, repository } = tokenAndRepository(); - if (!Number.isSafeInteger(command.prNumber) || command.prNumber < 1) { - throw new Error("PR number is invalid"); - } - if (!SHA_PATTERN.test(command.headSha)) throw new Error("PR head SHA is invalid"); - if (!SHA_PATTERN.test(command.baseSha)) throw new Error("PR base SHA is invalid"); - if (!SHA_PATTERN.test(command.workflowSha)) throw new Error("workflow SHA is invalid"); - if (!Number.isSafeInteger(command.approvalRunId) || command.approvalRunId < 1) { - throw new Error("approval run ID is invalid"); - } - if (command.approvalRunAttempt !== 1) { - throw new Error("approval run attempt must be exactly 1"); - } - - const runUrl = validateApprovalWorkflowRun( - await githubApi(`repos/${repository}/actions/runs/${command.approvalRunId}`, token, { - userAgent: USER_AGENT, - }), - { - repository, - runId: command.approvalRunId, - runAttempt: command.approvalRunAttempt, - workflowSha: command.workflowSha, - }, - ); - const review = validateApprovalReview( - await githubApi( - `repos/${repository}/actions/runs/${command.approvalRunId}/approvals`, - token, - { userAgent: USER_AGENT }, - ), - FORK_SKIP_APPROVAL_ENVIRONMENT, - ); - await completeForkE2ESkip({ - mode: "record-fork-e2e-skip", - prNumber: command.prNumber, - headSha: command.headSha, - baseSha: command.baseSha, - workflowSha: command.workflowSha, - maintainer: review.reviewer, - reason: approvedWaiverReason(review.comment), - validatedApproval: { - environment: FORK_SKIP_APPROVAL_ENVIRONMENT, - runUrl, - }, - }); -} - async function activeSupersededPrGateChecks(options: { repository: string; token: string; @@ -4845,7 +4702,7 @@ async function activeSupersededPrGateChecks(options: { if ( pull.number !== options.prNumber || pull.head.sha !== options.headSha || - pull.head.repo?.full_name !== options.repository || + !pull.head.repo || pull.base.repo.full_name !== options.repository ) { throw new Error("current pull request identity does not match the cancellation event"); @@ -4955,6 +4812,10 @@ async function main(): Promise { await startApprovedControlPlanePrGate(command); return; } + if (command.mode === "start-approved-fork") { + await startApprovedForkPrGate(command); + return; + } if (command.mode === "retry-runner-loss") { await retryRunnerLossPrGate(command); return; @@ -4990,14 +4851,6 @@ async function main(): Promise { await downloadChildRunEvidence(command.childRunId, command.evidencePath); return; } - if (command.mode === "record-fork-e2e-skip") { - await completeForkE2ESkip(command); - return; - } - if (command.mode === "record-approved-fork-e2e-skip") { - await recordApprovedForkE2ESkip(command); - return; - } await cancelPrGate(command.prNumber, command.headSha, command.supersededHeadSha); } diff --git a/tools/e2e/pr-e2e-required.mts b/tools/e2e/pr-e2e-required.mts index b58befcb9f6..53108394c9d 100644 --- a/tools/e2e/pr-e2e-required.mts +++ b/tools/e2e/pr-e2e-required.mts @@ -27,7 +27,6 @@ const GITHUB_ACTIONS_APP_ID = 15368; const USER_AGENT = "nemoclaw-pr-e2e-required"; const SHA_PATTERN = /^[a-f0-9]{40}$/u; const REPOSITORY_PATTERN = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/u; -const AUTHORIZATION_TITLES = new Set(["Maintainer approval required to skip credentialed E2E"]); const MAX_LOG_URLS = 20; type CheckConclusion = "success" | "failure" | "cancelled"; @@ -321,10 +320,7 @@ export function classifyCoordinationCheck( if (check.status !== "completed") { return { state: "waiting", description: title, ...links }; } - if ( - check.conclusion === "failure" && - (AUTHORIZATION_TITLES.has(title) || hasRetryableFailureMarker(check)) - ) { + if (check.conclusion === "failure" && hasRetryableFailureMarker(check)) { return { state: "waiting", description: title, ...links }; } if (