diff --git a/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md b/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md index dd263f494f..0406c920cc 100644 --- a/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md +++ b/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md @@ -31,9 +31,12 @@ The script handles the deterministic checks. You handle judgment calls: - **Product scope not established:** Stop before approval when the PR would create a new supported product surface and no accepted issue or design decision establishes ownership and lifecycle expectations. Technical correctness, successful tests, green CI, and positive advisor output do not substitute for product approval. Ask a maintainer for the product decision or route an independent solution through [Community Solutions](../../../docs/resources/community-contributions.mdx). - **Missing required checks:** The checked-in script requires `checks`, `check-hash`, `changes`, `commit-lint`, `dco-check`, and `E2E / PR Gate` in the status rollup, including while the repository ruleset rollout is staged. First-time fork contributors may need "Approve and run" before the ordinary `pull_request` checks appear. The trusted E2E controller reserves its exact head/base context for both internal and fork PRs without executing PR code. Never waive a missing, neutral, or skipped E2E gate, and never run untrusted fork code with privileged credentials to manufacture a result. -- **No-secret exception approval:** The primary path uses the same failed `E2E / PR Gate` run. Open the failed check's **Details** link, choose **Review deployments**, select `e2e-no-secret-exception`, add an optional comment, and approve. The waiting job has `deployment: false`, no secrets, and no PR-controlled execution. The controller reads GitHub's approval history, requires exactly one approval for only that environment, verifies that the recorded reviewer still has `maintain` or `admin`, and then applies the existing exact PR/head/base, deterministic-plan, matching-failed-check, compatible-`main`, and final stale-revision checks. The environment must be configured before rollout with required reviewers whose approving members have `maintain` or `admin`; add no secrets, variables, or custom protection app, and preferably disable administrator bypass. An absent or unprotected environment fails closed. Do not rerun the waiting workflow: environment approvals are not attempt-bound, so the controller accepts only the first attempt. Trigger fresh upstream PR CI for a new gate run, or use the typed manual fallback below. Per-PR concurrency cancels an older waiting approval when a newer exact revision reaches the gate. -- **Fork no-secret exception:** Use this only when an exact-diff gate has failed with `Maintainer fork exception required`. Independently review the fork change and non-secret PR CI, then use the protected-environment approval above. The controller rejects a closed PR, a changed head or base, an internal PR, an empty E2E plan, a non-failed gate, a controller commit that is neither current `main` nor a safely validated ancestor of it, or a recorded reviewer below maintainer role. A safe `main` advance must preserve the controller commit as its merge base, contain fewer than 300 fully enumerated changed files, and avoid `e2e-control-plane` changes; other advances fail closed. It re-reads the open PR's exact head and base immediately before writing success. The result records the reviewer, optional bounded comment, validated approval-run URL, plan, and waived jobs. Credential-bearing E2E was not run; this is an audited no-secret exception, not passing E2E evidence. If the button path is unavailable, choose **Run workflow** on `main`, select `resolve-fork`, and provide the fork PR number, its current 40-character head SHA as `expected_head_sha`, its current 40-character base SHA as `expected_base_sha`, a specific 10–500-character reason, and optionally an `NVIDIA/NemoClaw` Actions run URL. Re-read both PR SHAs immediately before dispatch. The controller validates the optional URL's repository and run-ID shape but does not inspect that run's head, jobs, or conclusion; review the run yourself before citing it. -- **E2E control-plane no-secret exception:** Use this only when an internal exact-diff gate has failed with `Maintainer control-plane exception required`. The deterministic `e2e-control-plane` family is a conservative path boundary: it covers E2E and PR-CI workflows, risk policy, dependency and test configuration, preparation and upload actions, and non-documentation files under `tools/e2e/` and `test/e2e/`, including shell and Python support files. Because a matching revision may influence job selection, credential-bearing execution, or the evidence used by the gate, the controller withholds the selected live jobs instead of trying to classify individual diffs as harmless. Independently review the change and its non-secret CI, then use the protected-environment approval above. The controller rejects closed PRs, forks, stale heads or bases, plans without that family, a mismatched failed gate, a controller commit that is neither current `main` nor a safely validated ancestor of it, or a recorded reviewer below maintainer role. A safe `main` advance must preserve the controller commit as its merge base, contain fewer than 300 fully enumerated changed files, and avoid `e2e-control-plane` changes; other advances fail closed. It re-reads the open PR's exact head and base immediately before writing success. The recorded result identifies the reviewer and validated approval run and states that the credential-bearing jobs were waived; it never claims they passed. If the button path is unavailable, choose **Run workflow** on `main`, select `resolve-control-plane`, and provide the PR number, current `expected_head_sha`, current `expected_base_sha`, a specific 10–500-character reason, and optionally a same-repository Actions run URL. The typed fallback is the maintainer's auditable assertion; it does not query a separate approval record or inspect optional run evidence. +- **Controller workflow versus PR verdict:** `E2E / PR Gate Controller` reports whether the trusted controller safely published an outcome. It can be green while the custom `E2E / PR Gate` check is failed or cancelled. Merge authority comes from the custom check for the current exact head and base, not from the controller workflow. `Superseded by PR update` and `PR closed — gate no longer applies` are cancelled outcomes for an obsolete revision and require no action on that old check; the current revision still needs its own successful gate. The closed-PR outcome also covers a deleted fork repository whose GitHub head-repository field is empty. +- **CI or selected E2E did not pass:** A handled prerequisite-CI or selected-E2E failure leaves `E2E / PR Gate` red while `E2E / PR Gate Controller` completes successfully. Follow the CI run, E2E run, non-passing job, and failed-step links in the custom check, fix or rerun the failing work, and require a successful gate for the resulting exact revision. A green controller run does not override the red verdict. +- **Evidence could not be verified:** If a selected child succeeds but `Download evidence` fails, is cancelled, or is skipped, the custom check fails closed with `Evidence could not be verified` and `E2E / PR Gate Controller` is red. Inspect the `Download evidence` step and rerun the gate; the green child alone is not accepted evidence. A successful download with missing, duplicate, skipped, pending, or reported-failing signals is a handled red PR verdict with a green controller. Malformed or unsafe evidence, schema or exact-identity mismatches, and traversal-limit violations remain controller verification errors: both the check and controller fail closed. +- **No-secret exception approval:** The primary path uses the same failed `E2E / PR Gate` check. Follow the explicit `E2E / PR Gate Controller run ` link embedded in the check summary rather than relying on the custom check's **Details** destination, choose **Review deployments**, select `e2e-no-secret-exception`, add an optional comment, and approve. The waiting job has `deployment: false`, no secrets, and no PR-controlled execution. The controller reads GitHub's approval history, requires exactly one approval for only that environment, verifies that the recorded reviewer still has `maintain` or `admin`, and then applies the existing exact PR/head/base, deterministic-plan, matching-failed-check, compatible-`main`, and final stale-revision checks. A recorded exception completes the custom check successfully with `No E2E run — exception approved by @` and leads with `Outcome: EXCEPTION — credentialed E2E did not run.` Treat this as an audited exception, not passing E2E evidence. The environment must be configured before rollout with required reviewers whose approving members have `maintain` or `admin`; add no secrets, variables, or custom protection app, and preferably disable administrator bypass. An absent or unprotected environment fails closed. Do not rerun the waiting workflow: environment approvals are not attempt-bound, so the controller accepts only the first attempt. Trigger fresh upstream PR CI for a new gate run, or use the typed manual fallback below. Per-PR concurrency cancels an older waiting approval when a newer exact revision reaches the gate. +- **Fork no-secret exception:** Use this only when an exact-diff gate has failed with `Maintainer fork exception required`. Independently review the fork change and non-secret PR CI, then use the protected-environment approval above. The controller rejects a closed PR, a changed head or base, an internal PR, an empty E2E plan, a non-failed gate, a controller commit that is neither current `main` nor a safely validated ancestor of it, or a recorded reviewer below maintainer role. A safe `main` advance must preserve the controller commit as its merge base, contain fewer than 300 fully enumerated changed files, and avoid `e2e-control-plane` changes; other advances fail closed. It re-reads the open PR's exact head and base immediately before writing success. The result records the reviewer, optional bounded comment, validated approval-run URL, plan, and jobs that did not run. If the button path is unavailable, choose **Run workflow** on `main`, select `resolve-fork`, and provide the fork PR number, its current 40-character head SHA as `expected_head_sha`, its current 40-character base SHA as `expected_base_sha`, and a specific 10–500-character reason. Re-read both PR SHAs immediately before dispatch. Optionally set `evidence_url` to an exact `https://github.com/NVIDIA/NemoClaw/actions/runs/` URL that you reviewed, or leave it blank when no supporting run exists. PR, issue, comment, job, and external URLs are rejected. The controller validates the optional URL's repository and run-ID shape but does not inspect that run's head, jobs, or conclusion. +- **E2E control-plane no-secret exception:** Use this only when an internal exact-diff gate has failed with `Maintainer control-plane exception required`. The deterministic `e2e-control-plane` family is a conservative path boundary: it covers E2E and PR-CI workflows, risk policy, dependency and test configuration, preparation and upload actions, and non-documentation files under `tools/e2e/` and `test/e2e/`, including shell and Python support files. Because a matching revision may influence job selection, credential-bearing execution, or the evidence used by the gate, the controller withholds the selected live jobs instead of trying to classify individual diffs as harmless. Independently review the change and its non-secret CI, then use the protected-environment approval above. The controller rejects closed PRs, forks, stale heads or bases, plans without that family, a mismatched failed gate, a controller commit that is neither current `main` nor a safely validated ancestor of it, or a recorded reviewer below maintainer role. A safe `main` advance must preserve the controller commit as its merge base, contain fewer than 300 fully enumerated changed files, and avoid `e2e-control-plane` changes; other advances fail closed. It re-reads the open PR's exact head and base immediately before writing success. The recorded result identifies the reviewer and validated approval run and states which jobs did not run; it never claims they passed. If the button path is unavailable, choose **Run workflow** on `main`, select `resolve-control-plane`, and provide the PR number, current `expected_head_sha`, current `expected_base_sha`, and a specific 10–500-character reason. Optionally set `evidence_url` to an exact `https://github.com/NVIDIA/NemoClaw/actions/runs/` URL that you reviewed, or leave it blank. PR, issue, comment, job, and external URLs are rejected. The typed fallback is the maintainer's auditable assertion; it does not query a separate approval record or inspect optional run evidence. - **Two-phase ruleset rollout and backfill:** First deploy the E2E check producer and its trusted fork handling. Re-run `CI / Pull Request` for every already-open exact PR head/base pair (approving a first-time fork run when necessary), then verify that `E2E / PR Gate` is attached to that same head SHA for the current base. The fail-closed maintainer checker identifies any head still needing backfill. Adding the workflow does not retroactively report the context, and enabling the ruleset first leaves existing PRs at "Waiting for status to be reported." Do not activate the context as a security boundary while it is scoped only to the shared GitHub Actions app: that app identity cannot distinguish this workflow from another workflow. First give the gate distinct provenance through a dedicated GitHub App or an organization required-workflow rule. When the context is finally activated, also enable strict/up-to-date required status checks; otherwise a successful head can remain mergeable after `main` advances and changes the effective merge diff. Preserve the control-plane review and deterministic floor even after those external protections are in place. - **Contributor compliance failed:** Reject the PR and ask the contributor to provide the PR-body DCO declaration or replace unverified commits with a clean verified history. Do not approve, merge, amend, sign, or force-push on the contributor's behalf. - **Contributor/approver overlap:** Surface `advisories.contributorApprovalOverlap` when the same account not recognized as automated by the supported login conventions appears as the current PR opener, commit author, or co-author and its latest opinionated review is approved. The invalid state detected here is contributor and approver identity overlap in the current GitHub PR metadata; the source boundary is the current opener plus all commit-author and review pages fetched through GitHub's GraphQL API. The advisory includes contributors whose commits remain in the current PR head at check time; it does not retain original push actors or authors removed when history is rebased, squashed, or fixed up. A clear result is not proof of independent approval. Missing, invalid, or conflicting review timestamps, or failure to retrieve complete paginated history, produce a warning because the latest opinion cannot be selected reliably. diff --git a/.github/workflows/pr-e2e-gate.yaml b/.github/workflows/pr-e2e-gate.yaml index 2ce8d9aeaf..0e2abd836e 100644 --- a/.github/workflows/pr-e2e-gate.yaml +++ b/.github/workflows/pr-e2e-gate.yaml @@ -1,7 +1,7 @@ # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -name: E2E / PR Gate +name: E2E / PR Gate Controller on: workflow_run: @@ -36,7 +36,7 @@ on: required: true type: string evidence_url: - description: Optional maintainer-supplied NVIDIA/NemoClaw Actions reference. + description: Optional Actions run URL, for example https://github.com/NVIDIA/NemoClaw/actions/runs/123. Leave blank if none. required: false default: "" type: string @@ -68,7 +68,7 @@ jobs: node-version: "22" - name: Install controller dependencies - run: npm ci --ignore-scripts + run: npm ci --ignore-scripts --no-audit --no-fund - name: Reserve exact-diff gate env: @@ -103,7 +103,7 @@ jobs: node-version: "22" - name: Install controller dependencies - run: npm ci --ignore-scripts + run: npm ci --ignore-scripts --no-audit --no-fund - name: Cancel superseded E2E runs env: @@ -145,7 +145,7 @@ jobs: node-version: "22" - name: Install controller dependencies - run: npm ci --ignore-scripts + run: npm ci --ignore-scripts --no-audit --no-fund - id: workspace name: Create private workspace @@ -236,8 +236,8 @@ jobs: "$RUN_ID" "$run_url" ;; completed:failure | completed:cancelled | completed:timed_out | completed:action_required | completed:neutral | completed:skipped | completed:stale | completed:startup_failure) - printf '::error title=E2E run failed::Run %s completed with conclusion %s. %s\n' \ - "$RUN_ID" "${state#*:}" "$run_url" >&2 + printf 'Run %s status=completed conclusion=%s url=%s\n' \ + "$RUN_ID" "${state#*:}" "$run_url" ;; *) printf '::error title=Unexpected run state::Run %s returned an unsupported status/conclusion pair. %s\n' \ @@ -251,7 +251,7 @@ jobs: queued:none | in_progress:none | requested:none | waiting:none | pending:none) sleep 10 ;; - completed:success) + completed:success | completed:failure | completed:cancelled | completed:timed_out | completed:action_required | completed:neutral | completed:skipped | completed:stale | completed:startup_failure) exit 0 ;; *) @@ -262,7 +262,8 @@ jobs: WAIT if [ "$wait_status" -eq 124 ]; then - printf '::error title=E2E run timed out::The run did not complete within 105 minutes.\n' >&2 + printf 'Run %s did not complete within 105 minutes; finalization will cancel it and report the PR gate outcome.\n' "$RUN_ID" + wait_status=0 fi exit "$wait_status" @@ -296,6 +297,7 @@ jobs: --state-hash "${{ steps.start.outputs.state_hash }}" --check-id "${{ steps.start.outputs.check_id }}" --run-id "${{ steps.start.outputs.run_id }}" + --evidence-outcome "${{ steps.evidence.outcome }}" - name: Close incomplete check if: ${{ always() && steps.start.outputs.check_id != '' && steps.start.outputs.finalized != 'true' && steps.finish.outputs.finalized != 'true' }} @@ -341,7 +343,7 @@ jobs: node-version: "22" - name: Install controller dependencies - run: npm ci --ignore-scripts + run: npm ci --ignore-scripts --no-audit --no-fund - name: Record approved E2E exception env: @@ -388,7 +390,7 @@ jobs: node-version: "22" - name: Install controller dependencies - run: npm ci --ignore-scripts + run: npm ci --ignore-scripts --no-audit --no-fund - name: Record E2E exception env: diff --git a/test/e2e/README.md b/test/e2e/README.md index bb90755509..160835a21b 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -10,8 +10,8 @@ before those targets run; local runners must provide it themselves. - `.github/workflows/e2e.yaml` is the scheduled, manually dispatchable, and selectively dispatched live target workflow. -- `.github/workflows/pr-e2e-gate.yaml` is the PR controller for - `E2E / PR Gate`. +- `.github/workflows/pr-e2e-gate.yaml` runs as `E2E / PR Gate Controller` and + publishes the exact-diff `E2E / PR Gate` check. - `.github/workflows/e2e-branch-validation.yaml` provisions Brev instances and runs focused E2E targets from source on a clean machine. - Platform workflows such as macOS, WSL, Ollama proxy, sandbox image, and @@ -84,6 +84,15 @@ map to this consolidated `e2e-artifacts/live/` registry-target artifact layout. ## PR E2E check +The workflow and custom check deliberately use different names and report +different results. `E2E / PR Gate Controller` reports whether the trusted +controller ran and published a terminal result. `E2E / PR Gate` is the +exact-diff merge verdict. A handled prerequisite-CI failure, selected E2E +failure or timeout, stale revision, or closed PR can leave the controller green +while the custom check is failed or cancelled. Only a successful `E2E / PR Gate` +for the current head and base satisfies the gate. An unexpected controller +error still fails the controller workflow and fails the custom check closed. + On open, synchronization, reopen, transition out of draft, or base retarget, `.github/workflows/pr-e2e-gate.yaml` reserves `E2E / PR Gate` for the exact PR head and base commits, including fork heads. A base retarget fails any earlier @@ -102,10 +111,17 @@ 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 and CI coordination share one non-cancelling concurrency -group for the head repository and branch. Before either path creates, fails, or -updates a check, it reads the live PR and requires the event's exact head and -base, including when PR CI failed. This keeps a stale seed or completed CI run -from being applied to a newer exact diff. +group for the head repository and branch. Before either path creates a check or +applies a result to the current revision, it reads the live PR and requires the +event's exact head and base, including when PR CI failed. This keeps a stale seed +or completed CI run from being applied to a newer exact diff. A completed CI +event for an older revision is handled without creating or updating the current +revision's check. +If the older revision still has an in-progress check, the controller completes +it as cancelled with `Superseded by PR update` or +`PR closed — gate no longer applies` and identifies the obsolete head and base. +The closed-PR outcome also applies when a fork repository was deleted and +GitHub consequently returns no head-repository object. Control-plane selections remain hash-bound in the recorded plan, but their credentialed execution is waived only through the exact-diff approval below. Shared sandbox-boundary changes have a floor of `full-e2e`, `hermes-e2e`, and @@ -136,21 +152,24 @@ commits. The dispatch includes selected jobs and valid plan and correlation metadata, but not `targets`. The controller uses GitHub's returned run ID for waiting, evidence download, and completion, then revalidates that the PR is still open with the live head, base, and exact-diff check identity before -recording success. +recording a final result. Credential-bearing E2E is dispatched only for internal revisions whose plan does not include the `e2e-control-plane` family. A fork revision that selects jobs and an internal revision with that family both complete the exact-diff gate as failed without dispatching the selected credential-bearing jobs or exposing repository secrets. Non-secret PR CI remains required. The failed -check links to the same `E2E / PR Gate` run and publishes only allowlisted -exception metadata for its exact PR number, mode, head SHA, and base SHA. That -run starts `Approve no-secret E2E exception`, which waits on the protected -`e2e-no-secret-exception` environment with `deployment: false` and therefore -does not create a deployment record. A maintainer opens the linked run, chooses -**Review deployments**, selects that environment, and approves it. The comment -is optional; the workflow reads both the reviewer and comment from GitHub's run -approval history rather than accepting an actor supplied by the job. +check summary embeds an explicit link to the same `E2E / PR Gate Controller` +run; maintainers follow that link rather than relying on the custom check's +**Details** destination. The check publishes only allowlisted exception +metadata for its exact PR number, mode, head SHA, and base SHA. That controller +run starts `Approve no-secret E2E exception`, which waits on the +protected `e2e-no-secret-exception` environment with `deployment: false` and +therefore does not create a deployment record. A maintainer opens the linked +run, chooses **Review deployments**, selects that environment, and approves it. +The comment is optional; the workflow reads both the reviewer and comment from +GitHub's run approval history rather than accepting an actor supplied by the +job. Before rollout, create `e2e-no-secret-exception` in the repository with one or more required reviewers whose approving members have repository `maintain` or @@ -175,16 +194,22 @@ matching failed gate, 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 exact head and base. The result records the reviewer, bounded optional comment, validated -approval-run URL, plan hash, and waived jobs; it never claims those jobs passed. +approval-run URL, plan hash, and jobs that did not run. The successful exception +check is titled `No E2E run — exception approved by @` and begins +with `Outcome: EXCEPTION — credentialed E2E did not run.` It never claims that +the selected jobs passed. The typed manual dispatch on `main` remains available as a fallback. Choose `resolve-fork` or `resolve-control-plane` and provide the PR number, current `expected_head_sha`, current `expected_base_sha`, a 10–500-character reason, -and optionally an `NVIDIA/NemoClaw` Actions run URL. The controller applies the -same PR, role, plan, failed-check, compatible-`main`, and final stale-revision -checks. It validates the optional URL's shape but does not inspect that run's -contents. Any new commit receives a different gate and requires a new decision; -a base change also invalidates the decision. +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. The Vitest reporter writes one `risk-signal.json` for each selected job and matrix shard. @@ -200,16 +225,31 @@ before downloaded evidence is classified. When the plan selects jobs, the check passes only when the E2E run succeeds and every expected job shard uploads one complete passing signal with no skips or -pending tests. Every other dispatched outcome fails. -The coordinator has a 180-minute job budget and gives evidence download its -own 10-minute limit, so a stalled download fails instead of consuming the -remaining coordination time. +pending tests. For the current exact diff, every other dispatched outcome +fails. A failed result links the selected E2E run and up to 10 non-passing jobs, +including up to three failed step names per job. If GitHub truncates the job +listing or the controller cannot load it, the check directs the maintainer to +the complete run. +The coordinator has a 180-minute job budget and gives the selected E2E run 105 +minutes to finish. When that limit expires, finalization cancels the child and +records the non-passing result in the custom check. Evidence download has its +own 10-minute limit. If the selected child succeeds but the `Download evidence` +step fails, is cancelled, or is skipped, the controller cannot authenticate the +child's artifacts. It fails the custom check closed as +`Evidence could not be verified` and leaves `E2E / PR Gate Controller` red so +maintainers inspect that infrastructure failure and rerun the gate. If the +download step succeeds but signals are missing, duplicated, skipped, pending, +or report a test failure, the controller has completed its work: it publishes +the handled red PR verdict and remains green. Malformed or unsafe evidence, +schema or exact-identity mismatches, and traversal-limit violations remain +controller verification errors, so both the check and controller fail closed. These dispatches suppress PR comments and the scheduled or manual scorecard, including scorecard Slack reporting. -Synchronizing, reopening, or closing an internal PR cancels its active E2E runs. A new -dispatch also cancels the previous run, while the previous controller remains -available to close its check as failed. +Synchronizing, reopening, or closing an internal PR cancels its active E2E +runs. A new dispatch also cancels the previous run. The previous controller +then completes the old exact-diff check as cancelled when the PR revision moved +or closed, or as failed when the current revision's selected E2E did not pass. The controller does not read PR Review Advisor output, so model availability and recommendations are not part of merge authority. diff --git a/test/pr-e2e-gate-command.test.ts b/test/pr-e2e-gate-command.test.ts new file mode 100644 index 0000000000..403e46e450 --- /dev/null +++ b/test/pr-e2e-gate-command.test.ts @@ -0,0 +1,210 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { describe, expect, it } from "vitest"; +import { parseControllerCommand } from "../tools/e2e/pr-e2e-gate.mts"; + +const HEAD_SHA = "a".repeat(40); +const BASE_SHA = "b".repeat(40); +const WORKFLOW_SHA = "d".repeat(40); + +function parseStartCommand(workDir: string, prNumber = "42") { + return parseControllerCommand([ + "--mode", + "start", + "--head", + HEAD_SHA, + "--head-repo", + "NVIDIA/NemoClaw", + "--head-branch", + "feature/pr-e2e-gate", + "--workflow-sha", + WORKFLOW_SHA, + "--ci-conclusion", + "success", + "--ci-display-title", + `CI PR #42 head ${HEAD_SHA} base ${BASE_SHA} gate true`, + "--ci-run-attempt", + "3", + "--ci-run-id", + "99", + "--gate-run-id", + "77", + "--pr", + prNumber, + "--work-dir", + workDir, + ]); +} + +function withPrivateWorkDir(run: (workDir: string) => void) { + const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-command-")); + try { + run(workDir); + } finally { + fs.chmodSync(workDir, 0o700); + fs.rmSync(workDir, { recursive: true, force: true }); + } +} + +describe("PR E2E controller commands", () => { + it("parses a start command inside a private workspace", () => { + withPrivateWorkDir((workDir) => { + expect(parseStartCommand(workDir)).toMatchObject({ + mode: "start", + ciRunAttempt: 3, + ciRunId: 99, + gateRunId: 77, + prNumber: 42, + planPath: path.join(workDir, "risk-plan.json"), + statePath: path.join(workDir, "controller-state.json"), + evidencePath: path.join(workDir, "evidence"), + }); + }); + }); + + it("parses a cancel command", () => { + expect(parseControllerCommand(["--mode", "cancel", "--pr", "42"])).toEqual({ + mode: "cancel", + prNumber: 42, + }); + }); + + it("parses a seed command", () => { + expect( + parseControllerCommand([ + "--mode", + "seed", + "--pr", + "42", + "--head", + HEAD_SHA, + "--base", + BASE_SHA, + ]), + ).toEqual({ + mode: "seed", + prNumber: 42, + headSha: HEAD_SHA, + baseSha: BASE_SHA, + }); + }); + + it("parses a finish command", () => { + withPrivateWorkDir((workDir) => { + expect( + parseControllerCommand([ + "--mode", + "finish", + "--work-dir", + workDir, + "--check-id", + "17", + "--run-id", + "23", + "--state-hash", + "a".repeat(64), + "--evidence-outcome", + "failure", + ]), + ).toMatchObject({ + mode: "finish", + checkRunId: 17, + childRunId: 23, + evidenceOutcome: "failure", + }); + }); + }); + + it("parses a fork-exception resolution", () => { + expect( + parseControllerCommand([ + "--mode", + "resolve-fork", + "--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: "resolve-fork", + 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 a control-plane exception resolution", () => { + expect( + parseControllerCommand([ + "--mode", + "resolve-control-plane", + "--pr", + "42", + "--head", + HEAD_SHA, + "--base", + BASE_SHA, + "--workflow-sha", + WORKFLOW_SHA, + "--maintainer", + "maintainer", + "--reason", + "Reviewed exact control-plane revision", + ]), + ).toEqual({ + mode: "resolve-control-plane", + prNumber: 42, + headSha: HEAD_SHA, + baseSha: BASE_SHA, + workflowSha: WORKFLOW_SHA, + maintainer: "maintainer", + reason: "Reviewed exact control-plane revision", + }); + }); + + it("parses an abandon command", () => { + expect( + parseControllerCommand(["--mode", "abandon", "--check-id", "17", "--run-id", "23"]), + ).toEqual({ mode: "abandon", checkRunId: 17, childRunId: 23 }); + }); + + it("rejects an unsafe pull request number", () => { + expect(() => parseControllerCommand(["--mode", "cancel", "--pr", "9007199254740992"])).toThrow( + /safe integer range/u, + ); + }); + + it("accepts a start command without a pull request number", () => { + withPrivateWorkDir((workDir) => { + expect(parseStartCommand(workDir, "")).toMatchObject({ mode: "start", prNumber: undefined }); + }); + }); + + it("rejects a finish workspace that is not private", () => { + withPrivateWorkDir((workDir) => { + fs.chmodSync(workDir, 0o755); + expect(() => parseControllerCommand(["--mode", "finish", "--work-dir", workDir])).toThrow( + /owned private absolute directory/u, + ); + }); + }); +}); diff --git a/test/pr-e2e-gate-exceptions.test.ts b/test/pr-e2e-gate-exceptions.test.ts index 539e5e05c4..0a3a6565b5 100644 --- a/test/pr-e2e-gate-exceptions.test.ts +++ b/test/pr-e2e-gate-exceptions.test.ts @@ -168,7 +168,7 @@ function startCommand(workDir: string) { function approvalWorkflowRun(overrides: Record = {}) { return { id: APPROVAL_RUN_ID, - name: "E2E / PR Gate", + name: "E2E / PR Gate Controller", path: ".github/workflows/pr-e2e-gate.yaml", event: "workflow_run", head_sha: WORKFLOW_SHA, @@ -309,6 +309,9 @@ describe("PR E2E controller exception safety", () => { }, }); expect(JSON.stringify(completion?.body)).toContain("Review deployments"); + expect(JSON.stringify(completion?.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("e2e-no-secret-exception"); expect(JSON.stringify(completion?.body)).toContain("unprotected environment fails closed"); expect(JSON.stringify(completion?.body)).toContain("manual workflow-dispatch resolver"); @@ -376,6 +379,9 @@ describe("PR E2E controller exception safety", () => { ), }, }); + expect(JSON.stringify(completion?.body)).toContain( + `[E2E / PR Gate Controller run ${GATE_RUN_ID}](https://github.com/NVIDIA/NemoClaw/actions/runs/${GATE_RUN_ID})`, + ); expect(fs.readFileSync(outputPath, "utf8")).toContain( [ "exception_mode=resolve-control-plane", @@ -461,18 +467,35 @@ describe("PR E2E controller exception safety", () => { conclusion: "success", details_url: `https://github.com/NVIDIA/NemoClaw/actions/runs/${APPROVAL_RUN_ID}`, output: { - title: "Fork exception recorded by @maintainer", - summary: expect.stringContaining("Validated environment approval run"), + title: "No E2E run — exception approved by @maintainer", + summary: expect.stringContaining("**Outcome: EXCEPTION — 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); }); + 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(resolveApprovedGate(approvedForkCommand())).rejects.toThrow( + /No protected-environment approval was recorded.*may be missing or lack required reviewers.*trigger fresh PR CI.*typed manual fallback/u, + ); + expect(requests.some((request) => request.method === "PATCH")).toBe(false); + }); + it.each([ - { name: "an empty unconfigured history", approvals: [] }, { name: "a malformed history object", approvals: {} }, { name: "a malformed review", @@ -636,10 +659,14 @@ describe("PR E2E controller exception safety", () => { status: "completed", conclusion: "success", output: { - title: "Fork exception recorded by @maintainer", - summary: expect.stringContaining("Credential-bearing E2E was not run"), + title: "No E2E run — exception approved by @maintainer", + summary: expect.stringContaining("**Outcome: EXCEPTION — credentialed E2E did not run.**"), }, }); + expect(JSON.stringify(completion?.body)).toContain("Selected jobs 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"); }); @@ -697,12 +724,13 @@ describe("PR E2E controller exception safety", () => { status: "completed", conclusion: "success", output: { - title: "Control-plane exception recorded by @maintainer", - summary: expect.stringContaining( - "Credential-bearing E2E was not run because this PR controls E2E execution or evidence", - ), + title: "No E2E run — exception approved by @maintainer", + summary: expect.stringContaining("**Outcome: EXCEPTION — credentialed E2E did not run.**"), }, }); + expect(JSON.stringify(completion?.body)).toContain( + "Selected jobs not run because this PR controls E2E execution or evidence", + ); expect(JSON.stringify(completion?.body)).not.toContain("tests passed"); expect(JSON.stringify(completion?.body)).not.toContain("Supporting trusted run"); }); @@ -1093,7 +1121,7 @@ describe("PR E2E controller exception safety", () => { maintainer: "maintainer", reason: "The exact base must remain current until the exception is recorded.", }), - ).rejects.toThrow(/expected exact head and base/u); + ).rejects.toThrow(/Superseded by PR update/u); expect(pullReads).toBe(3); expect(requests.some((request) => request.method === "PATCH")).toBe(false); }); diff --git a/test/pr-e2e-gate-lifecycle.test.ts b/test/pr-e2e-gate-lifecycle.test.ts index 0a8e8e19d4..326445cce7 100644 --- a/test/pr-e2e-gate-lifecycle.test.ts +++ b/test/pr-e2e-gate-lifecycle.test.ts @@ -174,6 +174,51 @@ function signal( }; } +function writePassingEvidence(evidencePath: string, gate: PrGateState): void { + for (const job of gate.expectedJobs) { + for (const shard of gate.expectedShards[job]!) { + const directory = path.join(evidencePath, `${job}-${shard}`); + fs.mkdirSync(directory, { recursive: true }); + fs.writeFileSync( + path.join(directory, "risk-signal.json"), + `${JSON.stringify(signal(gate, job, shard))}\n`, + ); + } + } +} + +function writeMalformedEvidence(evidencePath: string, _gate: PrGateState): void { + const directory = path.join(evidencePath, "malformed"); + fs.mkdirSync(directory); + fs.writeFileSync(path.join(directory, "risk-signal.json"), "{not-json\n"); +} + +async function expectHandledFinalization( + finalization: Promise, + _expectedSummary: string, +): Promise { + await expect(finalization).resolves.toBeUndefined(); +} + +async function expectControllerFailureFinalization( + finalization: Promise, + expectedSummary: string, +): Promise { + await expect(finalization).rejects.toThrow(expectedSummary); +} + +function expectSelectedRunLink(body: unknown): void { + expect(JSON.stringify(body)).toContain( + `[Selected E2E run 23](https://github.com/NVIDIA/NemoClaw/actions/runs/23)`, + ); +} + +function expectControllerDetailsLink(body: unknown): void { + expect(body).toMatchObject({ + details_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/23", + }); +} + function workflowRun(gate: PrGateState, overrides: Record = {}) { return { id: 23, @@ -283,14 +328,53 @@ describe("PR E2E controller lifecycle", () => { ...pullRequest(), base: { ...pullRequest().base, sha: "c".repeat(40) }, }, - expectedError: /expected exact head and base/u, + writeEvidence: writeMalformedEvidence, + expectedLivePullReads: 1, + expectedTitle: "Superseded by PR update", + expectedSummary: + "moved from head `aaaaaaa` on base `bbbbbbb` to head `aaaaaaa` on base `ccccccc`", }, { label: "the pull request closes after dispatch", currentPull: { ...pullRequest(), state: "closed" }, - expectedError: /invalid pull request state/u, + writeEvidence: writeMalformedEvidence, + expectedLivePullReads: 1, + expectedTitle: "PR closed — gate no longer applies", + expectedSummary: "closed before this gate completed", }, - ])("fails finalization when $label", async ({ currentPull, expectedError }) => { + { + label: "the pull request closes after its fork repository is deleted", + currentPull: { + ...pullRequest(), + state: "closed", + head: { ...pullRequest().head, repo: null }, + }, + writeEvidence: writeMalformedEvidence, + expectedLivePullReads: 1, + expectedTitle: "PR closed — gate no longer applies", + expectedSummary: "closed before this gate completed", + }, + { + label: "the pull request changes while passing evidence is parsed", + firstFinalizationPull: pullRequest(), + currentPull: { + ...pullRequest(), + head: { ...pullRequest().head, sha: "c".repeat(40) }, + }, + writeEvidence: writePassingEvidence, + expectedLivePullReads: 2, + expectedTitle: "Superseded by PR update", + expectedSummary: + "moved from head `aaaaaaa` on base `bbbbbbb` to head `ccccccc` on base `bbbbbbb`", + }, + ])("records an obsolete exact-diff outcome without failing the controller when $label", async ({ + currentPull, + firstFinalizationPull = currentPull, + writeEvidence, + expectedLivePullReads, + expectedTitle, + expectedSummary, + }) => { const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-retarget-")); const outputPath = path.join(workDir, "github-output"); const statePath = path.join(workDir, "controller-state.json"); @@ -300,20 +384,12 @@ describe("PR E2E controller lifecycle", () => { fs.writeFileSync(outputPath, "", { mode: 0o600 }); fs.writeFileSync(statePath, serializedState, { mode: 0o600 }); fs.mkdirSync(evidencePath); - for (const job of gate.expectedJobs) { - for (const shard of gate.expectedShards[job]!) { - const directory = path.join(evidencePath, `${job}-${shard}`); - fs.mkdirSync(directory, { recursive: true }); - fs.writeFileSync( - path.join(directory, "risk-signal.json"), - `${JSON.stringify(signal(gate, job, shard))}\n`, - ); - } - } + writeEvidence(evidencePath, gate); vi.stubEnv("GITHUB_TOKEN", "token"); vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); vi.stubEnv("GITHUB_OUTPUT", outputPath); const requests: RecordedGitHubRequest[] = []; + let livePullReads = 0; vi.spyOn(globalThis, "fetch").mockImplementation( createGitHubFetchRouter( [ @@ -321,9 +397,10 @@ describe("PR E2E controller lifecycle", () => { ({ url, method }) => url.endsWith("/actions/runs/23") && method === "GET", () => githubResponse(workflowRun(gate)), ), + existingPrGateCheckRunsRoute(), githubFetchRoute( ({ url, method }) => url.endsWith("/pulls/42") && method === "GET", - () => githubResponse(currentPull), + () => githubResponse(livePullReads++ === 0 ? firstFinalizationPull : currentPull), ), githubFetchRoute( ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", @@ -342,22 +419,28 @@ describe("PR E2E controller lifecycle", () => { evidencePath, checkRunId: 17, childRunId: 23, + evidenceOutcome: "success", }), - ).rejects.toThrow(expectedError); + ).resolves.toBeUndefined(); expect( requests.some( (request) => request.url.includes("/commits/") && request.url.includes("/check-runs?"), ), - ).toBe(false); + ).toBe(true); const completion = requests.find( (request) => request.url.endsWith("/check-runs/17") && request.method === "PATCH", ); expect(completion?.body).toMatchObject({ status: "completed", - conclusion: "failure", - output: { title: "Evidence could not be verified" }, + conclusion: "cancelled", + details_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/23", + output: { + title: expectedTitle, + summary: expect.stringContaining(expectedSummary), + }, }); expect(fs.readFileSync(outputPath, "utf8")).toContain("finalized=true"); + expect(livePullReads).toBe(expectedLivePullReads); } finally { fs.rmSync(workDir, { recursive: true, force: true }); } @@ -367,22 +450,105 @@ describe("PR E2E controller lifecycle", () => { { label: "missing evidence", status: "completed", + conclusion: "success", + jobs: [], + evidenceOutcome: "success" as const, + assertFinalization: expectHandledFinalization, + assertCompletionLink: expectSelectedRunLink, expectCancellation: false, expectedTitle: "Evidence is missing", - expectedError: /Missing signals: onboard-repair:default, onboard-resume:default/u, + expectedSummary: "Missing signals: onboard-repair:default, onboard-resume:default", }, { label: "an unfinished child", status: "in_progress", + conclusion: "success", + jobs: [], + evidenceOutcome: "success" as const, + assertFinalization: expectHandledFinalization, + assertCompletionLink: expectSelectedRunLink, expectCancellation: true, - expectedTitle: "E2E run did not succeed", - expectedError: /The run concluded unfinished \(in_progress\)/u, + expectedTitle: "Selected E2E did not pass", + expectedSummary: "concluded `unfinished (in_progress)`", }, - ])("closes the check as failure for $label", async ({ + { + label: "a failed child job", + status: "completed", + conclusion: "failure", + jobs: [ + { + id: 77, + name: "Hermes security-posture", + conclusion: "failure", + steps: [{ name: "Run security posture live Vitest test", conclusion: "failure" }], + }, + ], + evidenceOutcome: "success" as const, + assertFinalization: expectHandledFinalization, + assertCompletionLink: expectSelectedRunLink, + expectCancellation: false, + expectedTitle: "Hermes security-posture failed", + expectedSummary: + "[Hermes security-posture](https://github.com/NVIDIA/NemoClaw/actions/runs/23/job/77) — failed step: `Run security posture live Vitest test`", + }, + { + label: "a failed child whose job details are unavailable", + status: "completed", + conclusion: "failure", + jobs: null, + evidenceOutcome: "success" as const, + assertFinalization: expectHandledFinalization, + assertCompletionLink: expectSelectedRunLink, + expectCancellation: false, + expectedTitle: "Selected E2E did not pass", + expectedSummary: "Job details could not be loaded", + }, + { + label: "the evidence download fails after a successful child", + status: "completed", + conclusion: "success", + jobs: [], + evidenceOutcome: "failure" as const, + assertFinalization: expectControllerFailureFinalization, + assertCompletionLink: expectControllerDetailsLink, + expectCancellation: false, + expectedTitle: "Evidence could not be verified", + expectedSummary: "Evidence download did not complete (outcome: failure)", + }, + { + label: "the evidence download is cancelled after a successful child", + status: "completed", + conclusion: "success", + jobs: [], + evidenceOutcome: "cancelled" as const, + assertFinalization: expectControllerFailureFinalization, + assertCompletionLink: expectControllerDetailsLink, + expectCancellation: false, + expectedTitle: "Evidence could not be verified", + expectedSummary: "Evidence download did not complete (outcome: cancelled)", + }, + { + label: "the evidence download is skipped after a successful child", + status: "completed", + conclusion: "success", + jobs: [], + evidenceOutcome: "skipped" as const, + assertFinalization: expectControllerFailureFinalization, + assertCompletionLink: expectControllerDetailsLink, + expectCancellation: false, + expectedTitle: "Evidence could not be verified", + expectedSummary: "Evidence download did not complete (outcome: skipped)", + }, + ])("records the expected check and controller outcomes when $label", async ({ status, + conclusion, + jobs, + evidenceOutcome, + assertFinalization, + assertCompletionLink, expectCancellation, expectedTitle, - expectedError, + expectedSummary, }) => { const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-finish-")); const outputPath = path.join(workDir, "github-output"); @@ -402,12 +568,19 @@ describe("PR E2E controller lifecycle", () => { [ githubFetchRoute( ({ url, method }) => url.endsWith("/actions/runs/23") && method === "GET", - () => githubResponse(workflowRun(gate, { status, conclusion: "success" })), + () => githubResponse(workflowRun(gate, { status, conclusion })), ), githubFetchRoute( ({ url, method }) => url.endsWith("/actions/runs/23/cancel") && method === "POST", () => githubResponse(undefined, 202), ), + githubFetchRoute( + ({ url, method }) => url.includes("/actions/runs/23/jobs?") && method === "GET", + () => + jobs === null + ? githubResponse({ message: "temporary failure" }, 503) + : githubResponse({ total_count: jobs.length, jobs }), + ), githubFetchRoute( ({ url, method }) => url.endsWith("/pulls/42") && method === "GET", () => githubResponse(pullRequest()), @@ -423,15 +596,15 @@ describe("PR E2E controller lifecycle", () => { ); try { - await expect( - finishPrGate({ - statePath, - stateHash: sha256(serializedState), - evidencePath, - checkRunId: 17, - childRunId: 23, - }), - ).rejects.toThrow(expectedError); + const finalization = finishPrGate({ + statePath, + stateHash: sha256(serializedState), + evidencePath, + checkRunId: 17, + childRunId: 23, + evidenceOutcome, + }); + await assertFinalization(finalization, expectedSummary); expect(requests.some((request) => request.url.endsWith("/actions/runs/23/cancel"))).toBe( expectCancellation, ); @@ -439,8 +612,12 @@ describe("PR E2E controller lifecycle", () => { expect(completion?.body).toMatchObject({ status: "completed", conclusion: "failure", - output: { title: expectedTitle }, + output: { + title: expectedTitle, + summary: expect.stringContaining(expectedSummary), + }, }); + assertCompletionLink(completion?.body); expect(fs.readFileSync(outputPath, "utf8")).toContain("finalized=true"); } finally { fs.rmSync(workDir, { recursive: true, force: true }); diff --git a/test/pr-e2e-gate-workflow.test.ts b/test/pr-e2e-gate-workflow.test.ts index 723d2fa9a7..de51552c48 100644 --- a/test/pr-e2e-gate-workflow.test.ts +++ b/test/pr-e2e-gate-workflow.test.ts @@ -317,7 +317,7 @@ describe("PR E2E gate workflow", () => { const approveException = workflow.jobs["approve-exception"]; const resolveException = workflow.jobs["resolve-exception"]; - expect(workflow.name).toBe("E2E / PR Gate"); + expect(workflow.name).toBe("E2E / PR Gate Controller"); expect(workflow.on).toEqual({ workflow_run: { workflows: ["CI / Pull Request"], @@ -356,7 +356,8 @@ describe("PR E2E gate workflow", () => { type: "string", }, evidence_url: { - description: "Optional maintainer-supplied NVIDIA/NemoClaw Actions reference.", + description: + "Optional Actions run URL, for example https://github.com/NVIDIA/NemoClaw/actions/runs/123. Leave blank if none.", required: false, default: "", type: "string", @@ -449,6 +450,8 @@ describe("PR E2E gate workflow", () => { expect(start.env?.GATE_RUN_ID).toBe("${{ github.run_id }}"); expect(start.run).toContain('--ci-display-title "$CI_DISPLAY_TITLE"'); expect(start.run).toContain('--gate-run-id "$GATE_RUN_ID"'); + const finish = step(coordinate, "Verify evidence"); + expect(finish.run).toContain('--evidence-outcome "${{ steps.evidence.outcome }}"'); const approval = step(approveException, "Record approved E2E exception"); expect(approval.env).toEqual({ APPROVAL_RUN_ATTEMPT: "${{ github.run_attempt }}", @@ -508,7 +511,9 @@ describe("PR E2E gate workflow", () => { expect(nodeSetups.every((setup) => setup.with?.["node-version"] === "22")).toBe(true); expect(nodeSetups.every((setup) => !("cache" in (setup.with ?? {})))).toBe(true); expect(installs).toHaveLength(5); - expect(installs.every((install) => install.run === "npm ci --ignore-scripts")).toBe(true); + expect( + installs.every((install) => install.run === "npm ci --ignore-scripts --no-audit --no-fund"), + ).toBe(true); expect( allSteps.some((candidate) => candidate.uses?.startsWith("actions/download-artifact@")), ).toBe(false); @@ -608,13 +613,13 @@ describe("PR E2E gate workflow", () => { ]); }); - it("surfaces a terminal child failure", () => { + it("leaves terminal child failures for finalization to report", () => { const result = runWaitStep("failure"); - expect(result.status).toBe(1); + expect(result.status).toBe(0); expect(result.stdout.match(/status=in_progress/gu)).toHaveLength(1); - expect(result.stderr).toContain("::error title=E2E run failed::"); - expect(result.stderr).toContain("completed with conclusion failure"); + expect(result.stdout).toContain("status=completed conclusion=failure"); + expect(result.stderr).toBe(""); }); it("preserves GitHub CLI errors when status queries fail", () => { @@ -625,12 +630,12 @@ describe("PR E2E gate workflow", () => { expect(result.stderr).toContain("::error title=Run status query failed::"); }); - it("labels only the bounded wait exit as a timeout", () => { + it("leaves bounded wait timeouts for finalization to cancel and report", () => { const result = runWaitStep("timeout"); - expect(result.status).toBe(124); - expect(result.stderr).toContain("::error title=E2E run timed out::"); - expect(result.stderr).toContain("did not complete within 105 minutes"); + expect(result.status).toBe(0); + expect(result.stdout).toContain("did not complete within 105 minutes"); + expect(result.stderr).toBe(""); }); it("rejects an invalid child run ID before querying GitHub", () => { diff --git a/test/pr-e2e-gate.test.ts b/test/pr-e2e-gate.test.ts index 348652a4cd..5cdd4d7fbb 100644 --- a/test/pr-e2e-gate.test.ts +++ b/test/pr-e2e-gate.test.ts @@ -11,11 +11,9 @@ import { buildRiskPlan, riskPlanRequiredJobIds } from "../tools/advisors/risk-pl import { assertCorrelatedWorkflowRun, classifyPrGateEvidence, - controllerErrorAnnotationTitle, dispatchPrGate, expectedSignalShards, finishPrGate, - PrerequisiteCiError, type PrGateState, type PullRequest, parseControllerCommand, @@ -233,146 +231,28 @@ function workflowRun(gate: PrGateState, overrides: Record = {}) } describe("PR E2E controller", () => { - it("parses one lifecycle command set inside a private workspace", () => { - const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-")); - try { - expect( - parseControllerCommand([ - "--mode", - "start", - "--head", - HEAD_SHA, - "--head-repo", - "NVIDIA/NemoClaw", - "--head-branch", - "feature/pr-e2e-gate", - "--workflow-sha", - WORKFLOW_SHA, - "--ci-conclusion", - "success", - "--ci-display-title", - `CI PR #42 head ${HEAD_SHA} base ${BASE_SHA} gate true`, - "--ci-run-attempt", - String(CI_RUN_ATTEMPT), - "--ci-run-id", - String(CI_RUN_ID), - "--gate-run-id", - String(GATE_RUN_ID), - "--pr", - "42", - "--work-dir", - workDir, - ]), - ).toMatchObject({ - mode: "start", - ciRunAttempt: CI_RUN_ATTEMPT, - ciRunId: CI_RUN_ID, - gateRunId: GATE_RUN_ID, - prNumber: 42, - planPath: path.join(workDir, "risk-plan.json"), - statePath: path.join(workDir, "controller-state.json"), - evidencePath: path.join(workDir, "evidence"), - }); - expect(parseControllerCommand(["--mode", "cancel", "--pr", "42"])).toEqual({ - mode: "cancel", - prNumber: 42, - }); - expect( - parseControllerCommand([ - "--mode", - "seed", - "--pr", - "42", - "--head", - HEAD_SHA, - "--base", - BASE_SHA, - ]), - ).toEqual({ - mode: "seed", - prNumber: 42, - headSha: HEAD_SHA, - baseSha: BASE_SHA, - }); - expect( - parseControllerCommand([ - "--mode", - "resolve-fork", - "--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: "resolve-fork", - 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", - }); - expect( - parseControllerCommand([ - "--mode", - "resolve-control-plane", - "--pr", - "42", - "--head", - HEAD_SHA, - "--base", - BASE_SHA, - "--workflow-sha", - WORKFLOW_SHA, - "--maintainer", - "maintainer", - "--reason", - "Reviewed exact control-plane revision", - ]), - ).toEqual({ - mode: "resolve-control-plane", - prNumber: 42, - headSha: HEAD_SHA, - baseSha: BASE_SHA, - workflowSha: WORKFLOW_SHA, - maintainer: "maintainer", - reason: "Reviewed exact control-plane revision", - }); - expect( - parseControllerCommand(["--mode", "abandon", "--check-id", "17", "--run-id", "23"]), - ).toEqual({ mode: "abandon", checkRunId: 17, childRunId: 23 }); - expect(() => - parseControllerCommand(["--mode", "cancel", "--pr", "9007199254740992"]), - ).toThrow(/safe integer range/u); - expect(startCommand(workDir, "").prNumber).toBeUndefined(); - - fs.chmodSync(workDir, 0o755); - expect(() => parseControllerCommand(["--mode", "finish", "--work-dir", workDir])).toThrow( - /owned private absolute directory/u, - ); - } finally { - fs.chmodSync(workDir, 0o700); - fs.rmSync(workDir, { recursive: true, force: true }); - } - }); - - it("distinguishes prerequisite CI failures from controller failures in annotations", () => { - expect(controllerErrorAnnotationTitle(new PrerequisiteCiError("CI failed"))).toBe( - "PR CI did not pass", - ); - expect(controllerErrorAnnotationTitle(new Error("controller failed"))).toBe( - "Controller failed", + it("explains the accepted evidence URL when a manual exception uses another GitHub URL", () => { + expect(() => + parseControllerCommand([ + "--mode", + "resolve-control-plane", + "--pr", + "42", + "--head", + HEAD_SHA, + "--base", + BASE_SHA, + "--workflow-sha", + WORKFLOW_SHA, + "--maintainer", + "maintainer", + "--reason", + "Reviewed exact control-plane 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.", ); }); @@ -914,9 +794,7 @@ describe("PR E2E controller", () => { ), ); - await expect(seedPrGate(42, HEAD_SHA, BASE_SHA)).rejects.toThrow( - /expected exact head and base/u, - ); + await expect(seedPrGate(42, HEAD_SHA, BASE_SHA)).rejects.toThrow(/Superseded by PR update/u); expect(requests).toHaveLength(1); expect(requests.some((request) => request.url.includes("/check-runs"))).toBe(false); }); @@ -965,7 +843,7 @@ describe("PR E2E controller", () => { } }); - it("rejects stale failed-CI evidence before mutating the live base check", async () => { + it("ignores stale failed-CI evidence before mutating the live base check", async () => { const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-stale-ci-")); const outputPath = path.join(workDir, "github-output"); fs.writeFileSync(outputPath, "", { mode: 0o600 }); @@ -989,17 +867,17 @@ describe("PR E2E controller", () => { try { await expect( startPrGate({ ...startCommand(workDir), ciConclusion: "failure" }), - ).rejects.toThrow(/expected exact head and base/u); + ).resolves.toBeUndefined(); expect(requests).toHaveLength(2); expect(requests.filter((request) => request.url.includes("/check-runs?"))).toHaveLength(1); expect(requests.some((request) => request.url.endsWith("/check-runs"))).toBe(false); - expect(fs.readFileSync(outputPath, "utf8")).toBe(""); + expect(fs.readFileSync(outputPath, "utf8")).toBe("dispatched=false\nfinalized=true\n"); } finally { fs.rmSync(workDir, { recursive: true, force: true }); } }); - it("recovers a superseded exact-diff check for the workflow cleanup step", async () => { + it("closes a superseded exact-diff check without failing the controller", async () => { const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-superseded-")); const outputPath = path.join(workDir, "github-output"); fs.writeFileSync(outputPath, "", { mode: 0o600 }); @@ -1015,19 +893,33 @@ describe("PR E2E controller", () => { ...pullRequest(), head: { ...pullRequest().head, sha: "c".repeat(40) }, }), + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", + () => githubResponse({}), + ), ], requests, ), ); try { - await expect(startPrGate(startCommand(workDir))).rejects.toThrow( - /expected exact head and base/u, - ); - expect(requests).toHaveLength(2); + await expect(startPrGate(startCommand(workDir))).resolves.toBeUndefined(); + expect(requests).toHaveLength(3); expect(requests.some((request) => request.url.endsWith("/check-runs"))).toBe(false); - expect(requests.some((request) => request.method === "PATCH")).toBe(false); - expect(fs.readFileSync(outputPath, "utf8")).toBe("check_id=17\n"); + const completion = requests.find((request) => request.method === "PATCH"); + expect(completion?.body).toMatchObject({ + status: "completed", + conclusion: "cancelled", + output: { + title: "Superseded by PR update", + summary: expect.stringContaining( + "moved from head `aaaaaaa` on base `bbbbbbb` to head `ccccccc` on base `bbbbbbb`", + ), + }, + }); + expect(fs.readFileSync(outputPath, "utf8")).toBe( + "check_id=17\ndispatched=false\nfinalized=true\n", + ); } finally { fs.rmSync(workDir, { recursive: true, force: true }); } @@ -1055,13 +947,11 @@ describe("PR E2E controller", () => { ); try { - await expect(startPrGate(startCommand(workDir))).rejects.toThrow( - /expected exact head and base/u, - ); + await expect(startPrGate(startCommand(workDir))).resolves.toBeUndefined(); expect(requests).toHaveLength(2); expect(requests.some((request) => request.url.endsWith("/check-runs"))).toBe(false); expect(requests.some((request) => request.method === "PATCH")).toBe(false); - expect(fs.readFileSync(outputPath, "utf8")).toBe(""); + expect(fs.readFileSync(outputPath, "utf8")).toBe("dispatched=false\nfinalized=true\n"); } finally { fs.rmSync(workDir, { recursive: true, force: true }); } @@ -1086,7 +976,8 @@ describe("PR E2E controller", () => { ), githubFetchRoute( ({ url }) => - url.includes(`/actions/runs/${CI_RUN_ID}/attempts/${CI_RUN_ATTEMPT}/jobs?`), + url.includes(`/actions/runs/${CI_RUN_ID}/attempts/${CI_RUN_ATTEMPT}/jobs?`) && + new URL(url).searchParams.get("page") === "1", () => githubResponse({ total_count: 101, @@ -1124,6 +1015,23 @@ describe("PR E2E controller", () => { ], }), ), + githubFetchRoute( + ({ url }) => + url.includes(`/actions/runs/${CI_RUN_ID}/attempts/${CI_RUN_ATTEMPT}/jobs?`) && + new URL(url).searchParams.get("page") === "2", + () => + githubResponse({ + total_count: 101, + jobs: [ + { + id: 105, + name: "late-failure", + conclusion: "failure", + steps: [{ name: "Run late check", conclusion: "failure" }], + }, + ], + }), + ), githubFetchRoute( ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", () => githubResponse({}), @@ -1134,20 +1042,9 @@ describe("PR E2E controller", () => { ); try { - let rejection: unknown; - try { - await startPrGate({ ...startCommand(workDir), ciConclusion: "failure" }); - } catch (error) { - rejection = error; - } - expect(rejection).toBeInstanceOf(PrerequisiteCiError); - const rejectionMessage = (rejection as Error).message; - expect(rejectionMessage).toContain("PR #42: https://github.com/NVIDIA/NemoClaw/pull/42"); - expect(rejectionMessage).toContain( - `CI run attempt ${CI_RUN_ATTEMPT}: https://github.com/NVIDIA/NemoClaw/actions/runs/${CI_RUN_ID}/attempts/${CI_RUN_ATTEMPT}`, - ); - expect(rejectionMessage).toContain("cli-test-shards (6) (Run CLI coverage shard)"); - expect(rejectionMessage).toContain("job listing truncated"); + await expect( + startPrGate({ ...startCommand(workDir), ciConclusion: "failure" }), + ).resolves.toBeUndefined(); expect(requests.some((request) => request.url.endsWith("/dispatches"))).toBe(false); expect(requests.filter((request) => request.url.endsWith("/pulls/42"))).toHaveLength(1); @@ -1179,11 +1076,19 @@ describe("PR E2E controller", () => { expect(summary).toContain( `[unsafe\\] ::error::<tag>&](https://github.com/NVIDIA/NemoClaw/actions/runs/${CI_RUN_ID}/job/104)`, ); + expect(summary).toContain( + `[late-failure](https://github.com/NVIDIA/NemoClaw/actions/runs/${CI_RUN_ID}/job/105)`, + ); expect(summary).toContain("failed step: `bad' step ::warning::`"); expect(summary).not.toContain("\n::error::"); expect(summary).not.toContain("\n::warning::"); expect(summary).not.toContain("static-checks"); - expect(summary).toContain("The job listing was truncated"); + expect(summary).not.toContain("The job listing was truncated"); + expect( + requests.filter((request) => + request.url.includes(`/actions/runs/${CI_RUN_ID}/attempts/${CI_RUN_ATTEMPT}/jobs?`), + ), + ).toHaveLength(2); const outputs = fs.readFileSync(outputPath, "utf8"); expect(outputs).toContain("dispatched=false"); expect(outputs).toContain("finalized=true"); @@ -1237,12 +1142,7 @@ describe("PR E2E controller", () => { ciConclusion: "failure", prNumber: undefined, }), - ).rejects.toThrow( - new RegExp( - `CI run attempt ${CI_RUN_ATTEMPT}: https://github\\.com/NVIDIA/NemoClaw/actions/runs/${CI_RUN_ID}/attempts/${CI_RUN_ATTEMPT}`, - "u", - ), - ); + ).resolves.toBeUndefined(); expect(requests.filter((request) => request.url.endsWith("/pulls/42"))).toHaveLength(1); expect(requests.some((request) => request.url.includes("/pulls?"))).toBe(false); @@ -1368,11 +1268,14 @@ describe("PR E2E controller", () => { evidencePath: command.evidencePath, checkRunId: Number(outputs.check_id), childRunId: Number(outputs.run_id), + evidenceOutcome: "success", }); expect(gate.expectedJobs).toEqual(BROAD_JOBS); expect(requests.filter((request) => request.url.includes("/pulls?"))).toHaveLength(1); - expect(requests.filter((request) => request.url.endsWith("/pulls/42"))).toHaveLength(3); + // Finalization brackets evidence parsing with exact-diff reads so a PR update cannot + // turn stale evidence into a current-revision result. + expect(requests.filter((request) => request.url.endsWith("/pulls/42"))).toHaveLength(4); const checkCreation = requests.find( (request) => request.url.endsWith("/check-runs") && request.method === "POST", ); diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index 7df21c2e44..4c18b5a934 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -31,6 +31,7 @@ const E2E_WORKFLOW_PATH = `.github/workflows/${E2E_WORKFLOW}`; const PR_GATE_WORKFLOW_PATH = ".github/workflows/pr-e2e-gate.yaml"; const PR_GATE_APPROVAL_ENVIRONMENT = "e2e-no-secret-exception"; const CHECK_NAME = "E2E / PR Gate"; +const WORKFLOW_NAME = "E2E / PR Gate Controller"; const CHECK_EXTERNAL_ID_PREFIX = "nemoclaw-pr-e2e:v2"; const LEGACY_CHECK_EXTERNAL_ID_PREFIX = "nemoclaw-pr-e2e:v1"; const GITHUB_ACTIONS_APP_ID = 15368; @@ -50,7 +51,8 @@ const MAX_CONTROLLER_ERROR_CHARS = 512; const MAX_PR_FILES = 3000; const MAX_COMPATIBILITY_FILES = 300; const MAX_ACTIVE_RUN_PAGES_PER_STATUS = 10; -const MAX_REPORTED_CI_JOBS = 10; +const MAX_WORKFLOW_JOB_PAGES = 10; +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; @@ -75,6 +77,8 @@ type ControllerPaths = { evidencePath: string; }; +type EvidenceStepOutcome = "success" | "failure" | "cancelled" | "skipped"; + type ManualResolutionCommandBase = { prNumber: number; headSha: string; @@ -126,13 +130,14 @@ export type ControllerCommand = checkRunId: number; childRunId: number; stateHash: string; + evidenceOutcome: EvidenceStepOutcome; } & ControllerPaths) | { mode: "abandon"; checkRunId: number; childRunId?: number } | { mode: "cancel"; prNumber: number } | ManualResolutionCommand | ApprovedResolutionCommand; -type CheckConclusion = "success" | "failure"; +type CheckConclusion = "success" | "failure" | "cancelled"; export type PullRequest = { number: number; @@ -216,7 +221,12 @@ export type PrGateVerdict = { summary: string; }; -export class PrerequisiteCiError extends Error {} +class ObsoleteExactDiffError extends Error { + constructor(readonly verdict: PrGateVerdict) { + super(`${verdict.title}: ${verdict.summary}`); + this.name = "ObsoleteExactDiffError"; + } +} function isObjectRecord(value: unknown): value is Record { return !!value && typeof value === "object" && !Array.isArray(value); @@ -240,6 +250,14 @@ function parseHash(value: string | undefined, name: string): string { return parsed; } +function parseEvidenceStepOutcome(value: string | undefined): EvidenceStepOutcome { + const outcome = requiredArgument(value, "evidence-outcome"); + if (!["success", "failure", "cancelled", "skipped"].includes(outcome)) { + throw new Error("--evidence-outcome must be success, failure, cancelled, or skipped"); + } + return outcome as EvidenceStepOutcome; +} + export function parseCiRunIdentity(displayTitle: string): { prNumber: number; headSha: string; @@ -363,6 +381,7 @@ export function parseControllerCommand(argv: string[]): ControllerCommand { checkRunId: parsePositiveId(requiredArgument(args.checkId, "check-id"), "--check-id"), childRunId: parsePositiveId(requiredArgument(args.runId, "run-id"), "--run-id"), stateHash: parseHash(args.stateHash, "state-hash"), + evidenceOutcome: parseEvidenceStepOutcome(args.evidenceOutcome), }; } if (args.mode === "abandon") { @@ -383,7 +402,9 @@ export function parseControllerCommand(argv: string[]): ControllerCommand { 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 name an NVIDIA/NemoClaw Actions run"); + 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, @@ -903,7 +924,10 @@ async function completeFailureAfterControllerError( } } -function validatePullRequestIdentity(value: unknown): PullRequestListItem { +function validatePullRequestIdentity( + value: unknown, + options: { allowClosed?: boolean } = {}, +): PullRequestListItem { if ( !isObjectRecord(value) || !Number.isSafeInteger(value.number) || @@ -911,19 +935,25 @@ function validatePullRequestIdentity(value: unknown): PullRequestListItem { ) { throw new Error("GitHub returned an invalid pull request number"); } - if (value.state !== "open") throw new Error("GitHub returned invalid pull request state"); + if (value.state !== "open" && (!options.allowClosed || value.state !== "closed")) { + throw new Error("GitHub returned invalid pull request state"); + } if (!isObjectRecord(value.head) || !isObjectRecord(value.base)) { throw new Error("GitHub returned invalid pull request refs"); } const head = value.head; const base = value.base; + const validHeadRepository = + isObjectRecord(head.repo) && + typeof head.repo.full_name === "string" && + REPOSITORY_PATTERN.test(head.repo.full_name); + const closedWithDeletedHeadRepository = + options.allowClosed === true && value.state === "closed" && head.repo === null; if ( typeof head.ref !== "string" || typeof head.sha !== "string" || !SHA_PATTERN.test(head.sha) || - !isObjectRecord(head.repo) || - typeof head.repo.full_name !== "string" || - !REPOSITORY_PATTERN.test(head.repo.full_name) || + (!validHeadRepository && !closedWithDeletedHeadRepository) || typeof base.sha !== "string" || !SHA_PATTERN.test(base.sha) || !isObjectRecord(base.repo) || @@ -935,8 +965,8 @@ function validatePullRequestIdentity(value: unknown): PullRequestListItem { return value as PullRequestListItem; } -function validatePullRequest(value: unknown): PullRequest { - const identity = validatePullRequestIdentity(value); +function validatePullRequest(value: unknown, options: { allowClosed?: boolean } = {}): PullRequest { + const identity = validatePullRequestIdentity(value, options); if (!isObjectRecord(value) || !Number.isSafeInteger(value.changed_files)) { throw new Error("GitHub returned an invalid pull request changed-file count"); } @@ -958,16 +988,25 @@ async function requireLiveExactDiff(options: { userAgent: USER_AGENT, }, ), + { allowClosed: true }, ); - if ( - pull.number !== options.prNumber || - pull.state !== "open" || - !pull.head.repo || - pull.head.sha !== options.headSha || - pull.base.sha !== options.baseSha || - pull.base.repo.full_name !== options.repository - ) { - throw new Error("pull request no longer matches the expected exact head and base SHAs"); + if (pull.number !== options.prNumber || pull.base.repo.full_name !== options.repository) { + throw new Error("GitHub returned mismatched pull request identity"); + } + const prUrl = `https://github.com/${options.repository}/pull/${options.prNumber}`; + if (pull.state === "closed") { + throw new ObsoleteExactDiffError({ + conclusion: "cancelled", + title: "PR closed — gate no longer applies", + summary: `[PR #${options.prNumber}](${prUrl}) closed before this gate completed. This check for head \`${options.headSha.slice(0, 7)}\` on base \`${options.baseSha.slice(0, 7)}\` no longer applies.`, + }); + } + if (!pull.head.repo || pull.head.sha !== options.headSha || pull.base.sha !== options.baseSha) { + throw new ObsoleteExactDiffError({ + conclusion: "cancelled", + title: "Superseded by PR update", + summary: `[PR #${options.prNumber}](${prUrl}) moved from head \`${options.headSha.slice(0, 7)}\` on base \`${options.baseSha.slice(0, 7)}\` to head \`${pull.head.sha.slice(0, 7)}\` on base \`${pull.base.sha.slice(0, 7)}\`. No result from this run was accepted; review the gate on the current PR revision.`, + }); } return pull; } @@ -1005,7 +1044,7 @@ export async function resolvePullRequest(options: { ); if (!Array.isArray(response)) throw new Error("GitHub returned an invalid pull request list"); const matches = response - .map(validatePullRequestIdentity) + .map((candidate) => validatePullRequestIdentity(candidate)) .filter( (pull) => pull.head.sha === options.headSha && @@ -1079,27 +1118,41 @@ function validateWorkflowJobsPage(value: unknown): WorkflowJobsPage { }; } -async function listNonPassingCiJobs( +async function listNonPassingWorkflowJobs( repository: string, token: string, - ciRunId: number, - ciRunAttempt: number, + runId: number, + runAttempt?: number, ): Promise<{ jobs: WorkflowJob[]; complete: boolean }> { - const response = validateWorkflowJobsPage( - await githubApi( - `repos/${repository}/actions/runs/${ciRunId}/attempts/${ciRunAttempt}/jobs?per_page=100`, - token, - { userAgent: USER_AGENT }, - ), - ); - if (response.jobs.length > response.totalCount) { - throw new Error("GitHub returned an invalid workflow job count"); + const jobs: WorkflowJob[] = []; + let totalCount: number | undefined; + for (let page = 1; page <= MAX_WORKFLOW_JOB_PAGES; page += 1) { + const runPath = runAttempt ? `runs/${runId}/attempts/${runAttempt}` : `runs/${runId}`; + const response = validateWorkflowJobsPage( + await githubApi( + `repos/${repository}/actions/${runPath}/jobs?per_page=100&page=${page}`, + token, + { userAgent: USER_AGENT }, + ), + ); + totalCount ??= response.totalCount; + if (response.totalCount !== totalCount || jobs.length + response.jobs.length > totalCount) { + throw new Error("GitHub returned an invalid workflow job count"); + } + jobs.push(...response.jobs); + if (jobs.length === totalCount) { + return { + jobs: jobs.filter( + (job) => !["success", "skipped", "neutral"].includes(job.conclusion ?? ""), + ), + complete: true, + }; + } + if (response.jobs.length < 100) break; } return { - jobs: response.jobs.filter( - (job) => !["success", "skipped", "neutral"].includes(job.conclusion ?? ""), - ), - complete: response.jobs.length === response.totalCount, + jobs: jobs.filter((job) => !["success", "skipped", "neutral"].includes(job.conclusion ?? "")), + complete: jobs.length === totalCount, }; } @@ -1126,6 +1179,52 @@ function markdownCode(value: string, fallback: string): string { return `\`${normalizedCiMetadata(value, fallback).replace(/`/gu, "'")}\``; } +function nonPassingJobDetails(options: { + runUrl: string; + runLabel: string; + jobs: readonly WorkflowJob[]; + available: boolean; + complete: boolean; +}): { lines: string[]; reportedJobs: readonly WorkflowJob[] } { + const reportedJobs = options.jobs.slice(0, MAX_REPORTED_WORKFLOW_JOBS); + const lines: string[] = []; + if (reportedJobs.length > 0) { + lines.push("", "Jobs that did not pass:"); + for (const job of reportedJobs) { + const jobUrl = `${options.runUrl}/job/${job.id}`; + const failedSteps = job.steps.filter((step) => step.conclusion === "failure"); + const detail = + failedSteps.length > 0 + ? `${failedSteps.length === 1 ? "failed step" : "failed steps"}: ${failedSteps + .slice(0, 3) + .map((step) => markdownCode(step.name, "unnamed step")) + .join(", ")}${failedSteps.length > 3 ? ` and ${failedSteps.length - 3} more` : ""}` + : `concluded ${markdownCode(job.conclusion ?? "without a result", "without a result")}`; + lines.push(`- [${markdownLinkText(job.name)}](${jobUrl}) — ${detail}.`); + } + if (options.jobs.length > reportedJobs.length) { + lines.push( + `- ${options.jobs.length - reportedJobs.length} more; open the ${options.runLabel} for details.`, + ); + } + if (!options.complete) { + lines.push( + `- The job listing was truncated; open the ${options.runLabel} for the full result.`, + ); + } + } else if (options.available) { + lines.push( + "", + options.complete + ? `GitHub reported no non-passing job. Open the ${options.runLabel} for details.` + : `The job listing was truncated before a non-passing job was found. Open the ${options.runLabel} for details.`, + ); + } else { + lines.push("", `Job details could not be loaded. Open the ${options.runLabel} for details.`); + } + return { lines, reportedJobs }; +} + function ciFailureReport(options: { repository: string; prNumber?: number; @@ -1140,8 +1239,8 @@ function ciFailureReport(options: { ? `https://github.com/${options.repository}/pull/${options.prNumber}` : undefined; const ciRunUrl = `https://github.com/${options.repository}/actions/runs/${options.ciRunId}/attempts/${options.ciRunAttempt}`; + const runUrl = `https://github.com/${options.repository}/actions/runs/${options.ciRunId}`; const conclusion = normalizedCiMetadata(options.ciConclusion, "without a result"); - const reportedJobs = options.jobs.slice(0, MAX_REPORTED_CI_JOBS); const ciLink = `[CI / Pull Request attempt ${options.ciRunAttempt}](${ciRunUrl})`; const summary = options.prNumber ? [ @@ -1150,40 +1249,16 @@ function ciFailureReport(options: { : [ `${ciLink} concluded ${markdownCode(conclusion, "without a result")}, so no E2E run was dispatched. The triggering PR was not present in the workflow event.`, ]; - if (reportedJobs.length > 0) { - summary.push("", "Jobs that did not pass:"); - for (const job of reportedJobs) { - const jobUrl = `https://github.com/${options.repository}/actions/runs/${options.ciRunId}/job/${job.id}`; - const failedSteps = job.steps.filter((step) => step.conclusion === "failure"); - const detail = - failedSteps.length > 0 - ? `${failedSteps.length === 1 ? "failed step" : "failed steps"}: ${failedSteps - .slice(0, 3) - .map((step) => markdownCode(step.name, "unnamed step")) - .join(", ")}${failedSteps.length > 3 ? ` and ${failedSteps.length - 3} more` : ""}` - : `concluded ${markdownCode(job.conclusion ?? "without a result", "without a result")}`; - summary.push(`- [${markdownLinkText(job.name)}](${jobUrl}) — ${detail}.`); - } - if (options.jobs.length > reportedJobs.length) { - summary.push( - `- ${options.jobs.length - reportedJobs.length} more; open the CI run for details.`, - ); - } - if (!options.jobDetailsComplete) { - summary.push("- The job listing was truncated; open the CI run for the full result."); - } - } else if (options.jobDetailsAvailable) { - summary.push( - "", - options.jobDetailsComplete - ? "GitHub reported no non-passing job. Open the CI run for details." - : "The job listing was truncated before a non-passing job was found. Open the CI run for details.", - ); - } else { - summary.push("", "Job details could not be loaded. Open the CI run for details."); - } + const details = nonPassingJobDetails({ + runUrl, + runLabel: "CI run", + jobs: options.jobs, + available: options.jobDetailsAvailable, + complete: options.jobDetailsComplete, + }); + summary.push(...details.lines); - const conciseJobs = reportedJobs.slice(0, 3).map((job) => { + const conciseJobs = details.reportedJobs.slice(0, 3).map((job) => { const failedSteps = job.steps .filter((step) => step.conclusion === "failure") .slice(0, 2) @@ -1203,6 +1278,37 @@ function ciFailureReport(options: { }; } +function e2eFailureReport(options: { + repository: string; + runId: number; + workflowConclusion: string | null; + jobs: readonly WorkflowJob[]; + jobDetailsAvailable: boolean; + jobDetailsComplete: boolean; +}): PrGateVerdict { + const runUrl = `https://github.com/${options.repository}/actions/runs/${options.runId}`; + const conclusion = normalizedCiMetadata( + options.workflowConclusion ?? "without a result", + "without a result", + ); + const summary = [ + `[Selected E2E run ${options.runId}](${runUrl}) concluded ${markdownCode(conclusion, "without a result")}. No passing result was accepted.`, + ]; + const details = nonPassingJobDetails({ + runUrl, + runLabel: "E2E run", + jobs: options.jobs, + available: options.jobDetailsAvailable, + complete: options.jobDetailsComplete, + }); + summary.push(...details.lines); + const title = + details.reportedJobs.length === 1 + ? `${normalizedCiMetadata(details.reportedJobs[0]!.name, "Selected E2E job")} ${details.reportedJobs[0]!.conclusion === "failure" ? "failed" : "did not pass"}` + : "Selected E2E did not pass"; + return { conclusion: "failure", title, summary: summary.join("\n") }; +} + export async function pullChangedFiles( repository: string, pull: PullRequest, @@ -1553,13 +1659,27 @@ export async function startPrGate( const existingCheckRunId = existingChecks[0]?.status === "in_progress" ? existingChecks[0].id : undefined; if (existingCheckRunId) appendOutput("check_id", String(existingCheckRunId)); - const pull = await requireLiveExactDiff({ - repository, - token, - prNumber: ciIdentity.prNumber, - headSha: ciIdentity.headSha, - baseSha: ciIdentity.baseSha, - }); + let pull: PullRequest; + try { + pull = await requireLiveExactDiff({ + repository, + token, + prNumber: ciIdentity.prNumber, + headSha: ciIdentity.headSha, + baseSha: ciIdentity.baseSha, + }); + } catch (error) { + if (!(error instanceof ObsoleteExactDiffError)) throw error; + if (existingCheckRunId) { + await completeCheck({ repository, checkRunId: existingCheckRunId }, token, error.verdict); + } + appendOutput("dispatched", "false"); + appendOutput("finalized", "true"); + console.log( + `Ignored obsolete CI event: pr=${ciIdentity.prNumber} head=${ciIdentity.headSha} base=${ciIdentity.baseSha} reason=${error.verdict.title}`, + ); + return; + } if ( pull.head.repo?.full_name !== command.headRepository || pull.head.ref !== command.headBranch @@ -1589,7 +1709,7 @@ export async function startPrGate( let jobDetailsAvailable = true; let jobDetailsComplete: boolean; try { - const details = await listNonPassingCiJobs( + const details = await listNonPassingWorkflowJobs( repository, token, command.ciRunId, @@ -1625,7 +1745,8 @@ export async function startPrGate( appendOutput("dispatched", "false"); appendOutput("finalized", "true"); finalized = true; - throw new PrerequisiteCiError(report.errorMessage); + console.log(report.errorMessage); + return; } const changedFiles = await pullChangedFiles(repository, pull, token); @@ -1651,6 +1772,7 @@ export async function startPrGate( assertPullUnchanged(pull, currentPull); if (command.headRepository !== repository && jobs.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, @@ -1660,7 +1782,7 @@ export async function startPrGate( summary: [ `This exact fork diff (head ${command.headSha}, base ${ciIdentity.baseSha}) selected credential-bearing E2E jobs: ${jobs.join(", ")}.`, "Fork code was not executed and no repository secret was exposed.", - `Open the linked workflow run, choose Review deployments, and approve the \`${PR_GATE_APPROVAL_ENVIRONMENT}\` environment. GitHub records the reviewer and optional comment; an unprotected environment fails closed. The manual workflow-dispatch resolver remains available as fallback.`, + `Open ${gateRunLink}, choose Review deployments, and approve the \`${PR_GATE_APPROVAL_ENVIRONMENT}\` environment. GitHub records the reviewer and optional comment; an unprotected environment fails closed. The manual workflow-dispatch resolver remains available as fallback.`, ].join("\n\n"), }, gateRunUrl, @@ -1677,6 +1799,7 @@ export async function startPrGate( const controlPlaneFamily = plan.families.find((family) => family.id === "e2e-control-plane"); if (controlPlaneFamily) { const gateRunUrl = `https://github.com/${repository}/actions/runs/${command.gateRunId}`; + const gateRunLink = `[${WORKFLOW_NAME} run ${command.gateRunId}](${gateRunUrl})`; await completeCheck( { repository, checkRunId }, token, @@ -1686,7 +1809,7 @@ export async function startPrGate( summary: [ `This exact internal diff (head ${command.headSha}, base ${ciIdentity.baseSha}) changes trusted E2E execution or evidence code and selected credential-bearing E2E jobs: ${jobs.join(", ")}.`, "No PR-controlled E2E workflow, test, support code, or evidence reporter was executed with repository credentials.", - `Open the linked workflow run, choose Review deployments, and approve the \`${PR_GATE_APPROVAL_ENVIRONMENT}\` environment. GitHub records the reviewer and optional comment; an unprotected environment fails closed. The manual workflow-dispatch resolver remains available as fallback.`, + `Open ${gateRunLink}, choose Review deployments, and approve the \`${PR_GATE_APPROVAL_ENVIRONMENT}\` environment. GitHub records the reviewer and optional comment; an unprotected environment fails closed. The manual workflow-dispatch resolver remains available as fallback.`, `Deterministic plan: ${plan.planHash}.`, ].join("\n\n"), }, @@ -1841,6 +1964,7 @@ export async function finishPrGate(options: { evidencePath: string; checkRunId: number; childRunId: number; + evidenceOutcome: EvidenceStepOutcome; }): Promise { const { token, repository } = tokenAndRepository(); const childRunUrl = `https://github.com/${repository}/actions/runs/${options.childRunId}`; @@ -1875,30 +1999,6 @@ export async function finishPrGate(options: { } const workflowConclusion = child.status === "completed" ? child.conclusion : `unfinished (${child.status})`; - const expectedSignalCount = Object.values(state.expectedShards).reduce( - (total, shards) => total + shards.length, - 0, - ); - const signals = - workflowConclusion === "success" - ? findSignalFiles(options.evidencePath, { - ...EVIDENCE_LIMITS, - maxSignalFiles: expectedSignalCount + 1, - }).map((file) => validateSignal(readRegularJson(file), state)) - : []; - const verdict = classifyPrGateEvidence({ - workflowConclusion, - expectedJobs: state.expectedJobs, - expectedShards: state.expectedShards, - signals, - }); - await requireLiveExactDiff({ - repository, - token, - prNumber: state.prNumber, - headSha: state.commitSha, - baseSha: state.baseSha, - }); const matchingChecks = await matchingPrGateChecks({ repository, token, @@ -1909,15 +2009,83 @@ export async function finishPrGate(options: { if (matchingChecks.length !== 1 || matchingChecks[0]!.id !== options.checkRunId) { throw new Error("controller state does not match the exact PR gate check"); } + const finalizeObsoleteExactDiff = async (): Promise => { + try { + await requireLiveExactDiff({ + repository, + token, + prNumber: state.prNumber, + headSha: state.commitSha, + baseSha: state.baseSha, + }); + return false; + } catch (error) { + if (!(error instanceof ObsoleteExactDiffError)) throw error; + await completeCheck(context, token, error.verdict, childRunUrl); + appendOutput("finalized", "true"); + finalized = true; + console.log( + `Run superseded: run=${options.childRunId} title=${error.verdict.title} url=${childRunUrl}`, + ); + return true; + } + }; + if (await finalizeObsoleteExactDiff()) return; + const expectedSignalCount = Object.values(state.expectedShards).reduce( + (total, shards) => total + shards.length, + 0, + ); + let verdict: PrGateVerdict; + if (workflowConclusion === "success") { + if (options.evidenceOutcome !== "success") { + throw new Error( + `Evidence download did not complete (outcome: ${options.evidenceOutcome}) after selected E2E run ${options.childRunId} succeeded. The controller could not verify its artifacts; inspect the Download evidence step and rerun the gate.`, + ); + } + const signals = findSignalFiles(options.evidencePath, { + ...EVIDENCE_LIMITS, + maxSignalFiles: expectedSignalCount + 1, + }).map((file) => validateSignal(readRegularJson(file), state)); + verdict = classifyPrGateEvidence({ + workflowConclusion, + expectedJobs: state.expectedJobs, + expectedShards: state.expectedShards, + signals, + }); + if (verdict.conclusion === "failure") { + verdict = { + ...verdict, + summary: `[Selected E2E run ${options.childRunId}](${childRunUrl}) completed, but its evidence did not satisfy the gate.\n\n${verdict.summary}`, + }; + } + } else { + let jobs: WorkflowJob[] = []; + let jobDetailsAvailable = true; + let jobDetailsComplete = false; + try { + const details = await listNonPassingWorkflowJobs(repository, token, options.childRunId); + jobs = details.jobs; + jobDetailsComplete = details.complete; + } catch (error) { + jobDetailsAvailable = false; + console.warn(`Could not load E2E job details: ${controllerErrorMessage(error)}`); + } + verdict = e2eFailureReport({ + repository, + runId: options.childRunId, + workflowConclusion, + jobs, + jobDetailsAvailable, + jobDetailsComplete, + }); + } + if (await finalizeObsoleteExactDiff()) return; await completeCheck(context, token, verdict, childRunUrl); appendOutput("finalized", "true"); finalized = true; console.log( `Run completed: run=${options.childRunId} conclusion=${verdict.conclusion} title=${verdict.title} url=${childRunUrl}`, ); - if (verdict.conclusion === "failure") { - throw new Error(`${verdict.title}: ${verdict.summary}`); - } } catch (error) { if (!finalized) { const closed = await completeFailureAfterControllerError( @@ -1967,7 +2135,7 @@ function validateApprovalWorkflowRun( const expectedUrl = `https://github.com/${options.repository}/actions/runs/${options.runId}`; const valid = value.id === options.runId && - value.name === CHECK_NAME && + value.name === WORKFLOW_NAME && value.event === "workflow_run" && value.path === PR_GATE_WORKFLOW_PATH && value.head_branch === "main" && @@ -1984,8 +2152,18 @@ function validateApprovalWorkflowRun( } function validateApprovalReview(value: unknown): { maintainer: string; comment: string | null } { - if (!Array.isArray(value) || value.length < 1 || value.length > MAX_APPROVAL_REVIEWS) { - throw new Error("GitHub returned no bounded environment approval history"); + if (!Array.isArray(value)) { + throw new Error("GitHub returned malformed environment approval history"); + } + if (value.length === 0) { + throw new Error( + `No protected-environment approval was recorded for ${PR_GATE_APPROVAL_ENVIRONMENT}. The environment may be missing or lack required reviewers; configure it, then trigger fresh PR CI, or use the typed manual fallback.`, + ); + } + if (value.length > MAX_APPROVAL_REVIEWS) { + throw new Error( + `GitHub returned more than ${MAX_APPROVAL_REVIEWS} environment approval reviews; refusing ambiguous approval history`, + ); } const reviews = value.map((candidate) => { if ( @@ -2141,19 +2319,16 @@ async function resolveGateException(command: GateResolutionCommand): Promise { return active.size; } -export function controllerErrorAnnotationTitle(error: unknown): string { - return error instanceof PrerequisiteCiError ? "PR CI did not pass" : "Controller failed"; -} - function reportControllerError(error: unknown): void { const message = controllerErrorMessage(error); console.error(message); if (process.env.GITHUB_ACTIONS === "true") { const escaped = message.replace(/%/gu, "%25").replace(/\r/gu, "%0D").replace(/\n/gu, "%0A"); - console.error(`::error title=${controllerErrorAnnotationTitle(error)}::${escaped}`); + console.error(`::error title=Controller failed::${escaped}`); } } @@ -2327,6 +2499,7 @@ async function main(): Promise { evidencePath: command.evidencePath, checkRunId: command.checkRunId, childRunId: command.childRunId, + evidenceOutcome: command.evidenceOutcome, }); return; }