diff --git a/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md b/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md index 99a01ea86e..ccc5e06251 100644 --- a/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md +++ b/.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md @@ -30,10 +30,10 @@ The product-scope gate is a human decision and is not represented by `allPass`. 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 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. -- **Fork no-secret exception:** Use this only when an exact-head gate has failed with `Maintainer fork exception required`. In Actions, open `E2E / PR Gate`, choose **Run workflow** on `main`, select `resolve-fork`, and provide the fork PR number, its current 40-character head SHA, a specific 10–500-character reason, and optionally an `NVIDIA/NemoClaw` Actions run URL as supporting evidence. Re-read the PR head immediately before dispatch. The controller rejects a changed head, an internal PR, an empty E2E plan, a non-failed gate, a stale controller commit that is no longer `main`, or an actor below maintainer role. It 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. A successful dispatch records that credential-bearing E2E was not run; it is an audited no-secret exception, not passing E2E evidence. -- **E2E control-plane no-secret exception:** Use this only when an internal exact-head 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 run `E2E / PR Gate` on `main` with `resolve-control-plane`, the PR number, current exact SHA, a specific reason, and optionally a same-repository Actions run URL. The controller rejects forks, stale heads, plans without that family, a mismatched failed gate, a stale controller commit that is no longer `main`, or an actor below maintainer role. It does not query an approval record or inspect optional run evidence; the privileged dispatch and reason are the maintainer's auditable assertion that the review occurred. The recorded success states that the credential-bearing jobs were waived; it never claims they passed. -- **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 (approving a first-time fork run when necessary), then verify that `E2E / PR Gate` is attached to that same head SHA. 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. +- **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. +- **Fork no-secret exception:** Use this only when an exact-diff gate has failed with `Maintainer fork exception required`. In Actions, open `E2E / PR Gate`, 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 as supporting evidence. Re-read both PR SHAs immediately before dispatch. 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 an actor 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. 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. It re-reads the open PR's exact head and base immediately before writing success. A successful dispatch records that credential-bearing E2E was not run; it is an audited no-secret exception, not passing E2E evidence. +- **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 run `E2E / PR Gate` on `main` with `resolve-control-plane`, the PR number, current `expected_head_sha`, current `expected_base_sha`, a specific reason, and optionally a same-repository Actions run URL. 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 an actor 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 does not query an approval record or inspect optional run evidence; the privileged dispatch and reason are the maintainer's auditable assertion that the review occurred. It re-reads the open PR's exact head and base immediately before writing success. The recorded success states that the credential-bearing jobs were waived; it never claims they passed. +- **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/e2e.yaml b/.github/workflows/e2e.yaml index 56fb617592..6c21643b34 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -39,6 +39,16 @@ on: required: false default: "" type: string + base_sha: + description: PR base commit selected by the controller. + required: false + default: "" + type: string + workflow_sha: + description: Trusted E2E workflow commit selected by the controller. + required: false + default: "" + type: string plan_hash: description: SHA-256 of the selected E2E plan. required: false @@ -82,7 +92,9 @@ jobs: - name: Validate controller dispatch if: ${{ inputs.checkout_sha != '' }} env: + BASE_SHA: ${{ inputs.base_sha }} CHECKOUT_SHA: ${{ inputs.checkout_sha }} + EXPECTED_WORKFLOW_SHA: ${{ inputs.workflow_sha }} GITHUB_TOKEN: ${{ github.token }} JOBS: ${{ inputs.jobs }} PLAN_HASH: ${{ inputs.plan_hash }} @@ -91,10 +103,13 @@ jobs: TARGETS: ${{ inputs.targets }} WORKFLOW_EVENT: ${{ github.event_name }} WORKFLOW_REF: ${{ github.ref }} + WORKFLOW_SHA: ${{ github.workflow_sha }} run: | set -euo pipefail [[ "$WORKFLOW_EVENT" == "workflow_dispatch" && "$WORKFLOW_REF" == "refs/heads/main" ]] || { echo "::error::PR E2E runs must be dispatched from main"; exit 1; } [[ "$CHECKOUT_SHA" =~ ^[a-f0-9]{40}$ ]] || { echo "::error::checkout_sha must be a lowercase 40-character SHA"; exit 1; } + [[ "$BASE_SHA" =~ ^[a-f0-9]{40}$ ]] || { echo "::error::base_sha must be a lowercase 40-character SHA"; exit 1; } + [[ "$EXPECTED_WORKFLOW_SHA" =~ ^[a-f0-9]{40}$ && "$WORKFLOW_SHA" == "$EXPECTED_WORKFLOW_SHA" ]] || { echo "::error::workflow_sha must match the trusted workflow commit"; exit 1; } [[ "$(git rev-parse --verify HEAD)" == "$CHECKOUT_SHA" ]] || { echo "::error::checked-out commit does not match checkout_sha"; exit 1; } [[ "$PLAN_HASH" =~ ^[a-f0-9]{64}$ ]] || { echo "::error::plan_hash must be a lowercase SHA-256"; exit 1; } [[ "$CORRELATION_ID" =~ ^[a-f0-9]{8}-[a-f0-9]{4}-4[a-f0-9]{3}-[89ab][a-f0-9]{3}-[a-f0-9]{12}$ ]] || { echo "::error::correlation_id must be a lowercase UUIDv4"; exit 1; } @@ -109,6 +124,7 @@ jobs: [[ "$(jq -r '.state' <<< "$pull_json")" == "open" ]] || { echo "::error::pull request must still be open"; exit 1; } [[ "$(jq -r '.head.repo.full_name // ""' <<< "$pull_json")" == "$GITHUB_REPOSITORY" ]] || { echo "::error::pull request must originate from this repository"; exit 1; } [[ "$(jq -r '.head.sha' <<< "$pull_json")" == "$CHECKOUT_SHA" ]] || { echo "::error::checkout_sha must match the PR head commit"; exit 1; } + [[ "$(jq -r '.base.sha' <<< "$pull_json")" == "$BASE_SHA" ]] || { echo "::error::base_sha must match the PR base commit"; exit 1; } - name: Prepare E2E workspace uses: NVIDIA/NemoClaw/.github/actions/prepare-e2e@50281ee84c4a6fc759da95ea28fc0b7d9c378a28 diff --git a/.github/workflows/pr-e2e-gate.yaml b/.github/workflows/pr-e2e-gate.yaml index e2d24b2616..1eafe4968c 100644 --- a/.github/workflows/pr-e2e-gate.yaml +++ b/.github/workflows/pr-e2e-gate.yaml @@ -8,11 +8,11 @@ on: workflows: ["CI / Pull Request"] types: [completed] pull_request_target: - types: [opened, synchronize, reopened, ready_for_review, closed] + types: [opened, synchronize, reopened, ready_for_review, edited, closed] workflow_dispatch: inputs: operation: - description: Exact-head exception type to record. + description: Exact-diff exception type to record. required: true default: resolve-fork type: choice @@ -27,6 +27,10 @@ on: description: Exact 40-character PR head SHA reviewed by the maintainer. required: true type: string + expected_base_sha: + description: Exact 40-character PR base SHA reviewed by the maintainer. + required: true + type: string waiver_reason: description: Why credentialed E2E cannot be run safely for this revision. required: true @@ -41,14 +45,15 @@ permissions: {} jobs: initialize: - if: ${{ github.event_name == 'pull_request_target' && github.repository == 'NVIDIA/NemoClaw' && github.event.action != 'closed' }} + if: ${{ github.event_name == 'pull_request_target' && github.repository == 'NVIDIA/NemoClaw' && github.event.action != 'closed' && (github.event.action != 'edited' || github.event.changes.base != null) }} runs-on: ubuntu-latest timeout-minutes: 10 permissions: checks: write contents: read + pull-requests: read concurrency: - group: pr-e2e-gate-seed-${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }} + group: pr-e2e-gate-${{ github.event.pull_request.head.repo.full_name }}-${{ github.event.pull_request.head.ref }} cancel-in-progress: false steps: - name: Checkout controller @@ -65,8 +70,9 @@ jobs: - name: Install controller dependencies run: npm ci --ignore-scripts - - name: Reserve exact-head gate + - name: Reserve exact-diff gate env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} GITHUB_TOKEN: ${{ github.token }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} PR_NUMBER: ${{ github.event.pull_request.number }} @@ -75,9 +81,10 @@ jobs: --mode seed --pr "$PR_NUMBER" --head "$HEAD_SHA" + --base "$BASE_SHA" cancel-superseded: - if: ${{ github.event_name == 'pull_request_target' && github.repository == 'NVIDIA/NemoClaw' && github.event.pull_request.head.repo.full_name == github.repository }} + if: ${{ github.event_name == 'pull_request_target' && github.repository == 'NVIDIA/NemoClaw' && github.event.pull_request.head.repo.full_name == github.repository && (github.event.action != 'edited' || github.event.changes.base != null) }} runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -108,7 +115,7 @@ jobs: --pr "$PR_NUMBER" coordinate: - if: ${{ github.event_name == 'workflow_run' && github.repository == 'NVIDIA/NemoClaw' && github.event.workflow_run.event == 'pull_request' }} + if: ${{ github.event_name == 'workflow_run' && github.repository == 'NVIDIA/NemoClaw' && github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.path == '.github/workflows/pr.yaml' && endsWith(github.event.workflow_run.display_title, ' gate true') }} runs-on: ubuntu-latest timeout-minutes: 180 permissions: @@ -148,6 +155,7 @@ jobs: name: Start evaluation env: CI_CONCLUSION: ${{ github.event.workflow_run.conclusion }} + CI_DISPLAY_TITLE: ${{ github.event.workflow_run.display_title }} CI_RUN_ATTEMPT: ${{ github.event.workflow_run.run_attempt }} CI_RUN_ID: ${{ github.event.workflow_run.id }} GITHUB_TOKEN: ${{ github.token }} @@ -165,6 +173,7 @@ jobs: --head-branch "$HEAD_BRANCH" --workflow-sha "$WORKFLOW_SHA" --ci-conclusion "$CI_CONCLUSION" + --ci-display-title "$CI_DISPLAY_TITLE" --ci-run-attempt "$CI_RUN_ATTEMPT" --ci-run-id "$CI_RUN_ID" --pr "$PR_NUMBER" @@ -324,6 +333,7 @@ jobs: - name: Record E2E exception env: EVIDENCE_URL: ${{ inputs.evidence_url }} + EXPECTED_BASE_SHA: ${{ inputs.expected_base_sha }} EXPECTED_HEAD_SHA: ${{ inputs.expected_head_sha }} GITHUB_TOKEN: ${{ github.token }} MAINTAINER: ${{ github.triggering_actor }} @@ -336,6 +346,7 @@ jobs: --mode "$OPERATION" --pr "$PR_NUMBER" --head "$EXPECTED_HEAD_SHA" + --base "$EXPECTED_BASE_SHA" --workflow-sha "$WORKFLOW_SHA" --maintainer "$MAINTAINER" --reason "$WAIVER_REASON" diff --git a/.github/workflows/pr-review-advisor.yaml b/.github/workflows/pr-review-advisor.yaml index 7d519127e5..b60bd33739 100644 --- a/.github/workflows/pr-review-advisor.yaml +++ b/.github/workflows/pr-review-advisor.yaml @@ -8,7 +8,7 @@ on: # advisor secrets available for both internal and fork PRs. The analysis job # never executes PR-controlled content and has no write permission. pull_request_target: - types: [opened, synchronize, reopened, ready_for_review] + types: [opened, synchronize, reopened, ready_for_review, edited] workflow_dispatch: inputs: base_ref: @@ -46,13 +46,13 @@ on: permissions: {} concurrency: - group: pr-review-advisor-${{ github.event_name }}-${{ github.event.pull_request.number || github.ref }}-${{ inputs.target_repo || github.repository }}-${{ inputs.target_pr || '' }} + group: pr-review-advisor-${{ github.event_name }}-${{ github.event.pull_request.number || github.ref }}-${{ inputs.target_repo || github.repository }}-${{ inputs.target_pr || '' }}-${{ github.event_name != 'pull_request_target' || github.event.action != 'edited' || github.event.changes.base != null }} cancel-in-progress: true jobs: review: name: PR review advisor (${{ matrix.advisor.label }}) - if: ${{ github.repository == 'NVIDIA/NemoClaw' }} + if: ${{ github.repository == 'NVIDIA/NemoClaw' && (github.event_name != 'pull_request_target' || github.event.action != 'edited' || github.event.changes.base != null) }} permissions: actions: read checks: read @@ -369,7 +369,7 @@ jobs: publish: name: Publish PR review advisor needs: review - if: ${{ always() && github.event_name == 'pull_request_target' && needs.review.result != 'cancelled' }} + if: ${{ always() && github.event_name == 'pull_request_target' && (github.event.action != 'edited' || github.event.changes.base != null) && needs.review.result != 'cancelled' }} # Publication is best-effort and must never hide the primary analysis # lane's required outcome in the review matrix above. continue-on-error: true diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index b12ada24d6..67439b169a 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -3,15 +3,17 @@ name: CI / Pull Request +run-name: "CI PR #${{ github.event.pull_request.number }} head ${{ github.event.pull_request.head.sha }} base ${{ github.event.pull_request.base.sha }} gate ${{ github.event.action != 'edited' || github.event.changes.base != null }}" + on: pull_request: - types: [opened, synchronize, reopened] + types: [opened, synchronize, reopened, edited] permissions: contents: read concurrency: - group: ${{ github.workflow }}-${{ github.ref }} + group: ${{ github.workflow }}-${{ github.ref }}-${{ github.event.action != 'edited' || github.event.changes.base != null }} cancel-in-progress: true jobs: diff --git a/test/e2e/README.md b/test/e2e/README.md index 3673e9d99d..869ab22674 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -84,51 +84,81 @@ map to this consolidated `e2e-artifacts/live/` registry-target artifact layout. ## PR E2E check -On open, synchronization, reopen, or transition out of draft, -`.github/workflows/pr-e2e-gate.yaml` reserves `E2E / PR Gate` on the exact PR -head commit, including fork heads. The trusted controller reads all changed -files after `CI / Pull Request` completes and builds the deterministic risk -plan. Runtime families and changes to workflow-wired live tests select +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 +gate in that head's lineage before reserving the new exact-diff gate. The +`CI / Pull Request` run name binds its PR number, head SHA, base SHA, and gate +eligibility so the trusted controller can authenticate the completed run even +when a fork `workflow_run` payload omits pull-request metadata. The controller +also requires the completed run's workflow path to be +`.github/workflows/pr.yaml`. Metadata-only edits are marked ineligible and are +ignored by the controller and PR Review Advisor; base edits are eligible. PR CI +and advisor concurrency groups include that eligibility, so an ignored +metadata-edit run cannot cancel an eligible run for the same PR. The trusted +controller reads all changed files after eligible PR CI completes and builds +the deterministic risk plan. +Runtime families and changes to workflow-wired live tests select canonical selectors from the trusted `e2e.yaml` inventory independently of advisor output. Ordinary internal changes execute those focused selections. +Gate initialization 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. Control-plane selections remain hash-bound in the recorded plan, but their -credentialed execution is waived only through the exact-head approval below. Shared -sandbox-boundary changes have a floor of `full-e2e`, `hermes-e2e`, and +credentialed execution is waived only through the exact-diff approval below. +Shared sandbox-boundary changes have a floor of `full-e2e`, `hermes-e2e`, and `security-posture`. E2E control-plane changes select `cloud-onboard`, `credential-sanitization`, and `security-posture`, but the controller does not run those jobs with credentials. This is a conservative path boundary that includes non-documentation files under `tools/e2e/` and `test/e2e/`, plus the E2E and PR-CI workflows, risk policy, dependency and test configuration, and -preparation and upload actions. It does not attempt to classify an individual matching diff as -harmless. Instead, the exact-head check fails until a maintainer records the -control-plane exception described below. If no job is selected, the check -passes without an E2E run. - -Before dispatch, the controller verifies that the PR is unchanged and that -`main` still points to its workflow commit. It accepts only an E2E run using -that commit. Each selected job checks out `checkout_sha`. Before preparation or -secret-bearing jobs can run, `e2e.yaml` verifies that the PR remains open, -belongs to `NVIDIA/NemoClaw`, and still has that head commit. 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. +preparation and upload actions. It does not attempt to classify an individual +matching diff as harmless. Instead, the exact-diff check fails until a +maintainer records the control-plane exception described below. If no job is +selected, the check passes without an E2E run. + +Before dispatch, the controller verifies that the live PR still matches the CI +run's exact head and base. It uses its own workflow commit when that commit is +still `main`. If `main` advanced, the controller accepts the current commit +only when GitHub reports it as a descendant whose merge base is the workflow +commit, the comparison contains fewer than 300 fully enumerated files, neither +side of a rename enters the `e2e-control-plane` risk family, and a second read +confirms that `main` did not move again. Any divergence, incomplete comparison, +control-plane change, or second advance fails closed. The accepted `main` +commit is recorded as the workflow SHA and passed as `workflow_sha`. Before +matrix or secret-bearing jobs can run, `e2e.yaml` requires +`github.workflow_sha` to match that accepted commit. Each selected job checks +out `checkout_sha`. The same validation verifies that the PR remains open, +belongs to `NVIDIA/NemoClaw`, and still has both the dispatched head and base +commits. The dispatch includes selected jobs 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. 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-head +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. A maintainer or administrator can resolve that exact revision only through the workflow's typed manual dispatch on `main`, choosing `resolve-fork` or -`resolve-control-plane`. -The controller revalidates the open PR, exact head SHA, repository origin, -maintainer role, deterministic plan, matching failed gate, and that the -checked-out controller commit is still `main`. The result records an explicit no-secret exception with a bounded reason and optional +`resolve-control-plane`. The maintainer must provide both `expected_head_sha` +and `expected_base_sha`. +The controller revalidates the open PR, exact head and base SHAs, repository +origin, maintainer role, deterministic plan, matching failed gate, and that the +checked-out controller commit is either still `main` or has only a compatible +safe descendant as described above. The result records an explicit no-secret +exception with a bounded reason and optional `NVIDIA/NemoClaw` Actions run URL; it does not claim the waived jobs passed. The URL's shape is validated, but its run contents are not. The privileged dispatch and reason are an auditable maintainer assertion; the controller does -not query a separate approval record. Any new commit receives a different gate -and requires a new decision. +not query a separate approval record. Immediately before recording success, +the controller reads the live PR again and requires the same exact head and +base. 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. diff --git a/test/e2e/support/e2e-operations-workflow-boundary.test.ts b/test/e2e/support/e2e-operations-workflow-boundary.test.ts index c86e537e8f..6d15856811 100644 --- a/test/e2e/support/e2e-operations-workflow-boundary.test.ts +++ b/test/e2e/support/e2e-operations-workflow-boundary.test.ts @@ -66,12 +66,16 @@ describe("E2E operations workflow boundary", () => { it("rejects controller protocol and PR validation drift", () => { const workflow = readE2eOperationsWorkflow(); + delete workflow.on?.workflow_dispatch?.inputs?.base_sha; + delete workflow.on?.workflow_dispatch?.inputs?.workflow_sha; delete workflow.on?.workflow_dispatch?.inputs?.plan_hash; workflow.env!.NEMOCLAW_E2E_PLAN_HASH = "${{ inputs.checkout_sha }}"; workflow.concurrency!["cancel-in-progress"] = false; const validation = workflow.jobs["generate-matrix"].steps!.find( (step) => step.name === "Validate controller dispatch", )!; + delete validation.env?.BASE_SHA; + delete validation.env?.EXPECTED_WORKFLOW_SHA; validation.if = "${{ inputs.plan_hash != '' }}"; validation.run = "echo unchecked"; const checkout = workflow.jobs["generate-matrix"].steps!.find((step) => @@ -81,10 +85,17 @@ describe("E2E operations workflow boundary", () => { expect(validateE2eOperationsWorkflow(workflow)).toEqual( expect.arrayContaining([ + "workflow_dispatch base_sha must be an optional string with an empty default", + "workflow_dispatch workflow_sha must be an optional string with an empty default", "workflow_dispatch plan_hash must be an optional string with an empty default", "E2E workflow must bind NEMOCLAW_E2E_PLAN_HASH to controller metadata", "PR E2E concurrency must cancel obsolete runs", "Controller validation must be activated only by checkout_sha", + "Controller validation must bind BASE_SHA", + "Controller validation must bind EXPECTED_WORKFLOW_SHA", + 'Controller validation must retain "$BASE_SHA" =~ ^[a-f0-9]{40}$', + 'Controller validation must retain "$WORKFLOW_SHA" == "$EXPECTED_WORKFLOW_SHA"', + 'Controller validation must retain [[ "$(jq -r \'.base.sha\' <<< "$pull_json")" == "$BASE_SHA" ]]', 'Controller validation must retain "$PR_NUMBER" =~ ^[1-9][0-9]*$', "generate-matrix checkout must use the selected PR commit", ]), diff --git a/test/pr-e2e-gate-exceptions.test.ts b/test/pr-e2e-gate-exceptions.test.ts index 2c313c5431..b221499764 100644 --- a/test/pr-e2e-gate-exceptions.test.ts +++ b/test/pr-e2e-gate-exceptions.test.ts @@ -23,6 +23,7 @@ import { const HEAD_SHA = "a".repeat(40); const BASE_SHA = "b".repeat(40); const WORKFLOW_SHA = "d".repeat(40); +const ADVANCED_WORKFLOW_SHA = "e".repeat(40); const CI_RUN_ID = 99; const CI_RUN_ATTEMPT = 3; @@ -52,7 +53,7 @@ function exactPrGateCheck(overrides: Record = {}) { id: 17, name: "E2E / PR Gate", head_sha: HEAD_SHA, - external_id: prGateExternalId(42, HEAD_SHA), + external_id: prGateExternalId(42, HEAD_SHA, BASE_SHA), status: "in_progress", conclusion: null, app: { id: 15368 }, @@ -78,6 +79,25 @@ function mainWorkflowRefRoute(sha = WORKFLOW_SHA) { ); } +function compatibleMainComparisonRoute( + files: Array<{ filename: string; previous_filename?: string }>, + mainSha = ADVANCED_WORKFLOW_SHA, +) { + return githubFetchRoute( + ({ url }) => url.includes(`/compare/${WORKFLOW_SHA}...${mainSha}`), + () => + githubResponse({ + status: "ahead", + ahead_by: 1, + behind_by: 0, + base_commit: { sha: WORKFLOW_SHA }, + merge_base_commit: { sha: WORKFLOW_SHA }, + head_commit: { sha: mainSha }, + files, + }), + ); +} + function pullRequest(changedFiles = 1): PullRequest { return { number: 42, @@ -125,6 +145,8 @@ function startCommand(workDir: string) { 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", @@ -296,7 +318,7 @@ describe("PR E2E controller exception safety", () => { } }); - it("records an authorized exact-SHA fork exception without claiming tests passed", async () => { + it("records an authorized exact-head/base fork exception after a compatible main advance", async () => { vi.stubEnv("GITHUB_TOKEN", "token"); vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); const requests: RecordedGitHubRequest[] = []; @@ -325,7 +347,8 @@ describe("PR E2E controller exception safety", () => { conclusion: "failure", output: { title: "Maintainer fork exception required" }, }), - mainWorkflowRefRoute(), + mainWorkflowRefRoute(ADVANCED_WORKFLOW_SHA), + compatibleMainComparisonRoute([{ filename: "docs/get-started/quickstart.mdx" }]), githubFetchRoute( ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", () => githubResponse({}), @@ -339,6 +362,7 @@ describe("PR E2E controller exception safety", () => { mode: "resolve-fork", prNumber: 42, headSha: HEAD_SHA, + baseSha: BASE_SHA, workflowSha: WORKFLOW_SHA, maintainer: "maintainer", reason: "The fork cannot safely receive credential-bearing test secrets.", @@ -399,6 +423,7 @@ describe("PR E2E controller exception safety", () => { mode: "resolve-control-plane", prNumber: 42, headSha: HEAD_SHA, + baseSha: BASE_SHA, workflowSha: WORKFLOW_SHA, maintainer: "maintainer", reason: "The control-plane change received independent non-secret validation.", @@ -445,6 +470,7 @@ describe("PR E2E controller exception safety", () => { mode: "resolve-control-plane", prNumber: 42, headSha: HEAD_SHA, + baseSha: BASE_SHA, workflowSha: WORKFLOW_SHA, maintainer: "contributor", reason: "A write-role collaborator tried to record an exception.", @@ -489,6 +515,7 @@ describe("PR E2E controller exception safety", () => { const common = { prNumber: 42, headSha: HEAD_SHA, + baseSha: BASE_SHA, workflowSha: WORKFLOW_SHA, maintainer: "maintainer", reason: "The resolver operation must match the pull request origin.", @@ -530,6 +557,7 @@ describe("PR E2E controller exception safety", () => { mode: "resolve-control-plane", prNumber: 42, headSha: HEAD_SHA, + baseSha: BASE_SHA, workflowSha: WORKFLOW_SHA, maintainer: "maintainer", reason: "The ordinary change does not qualify for this exception.", @@ -572,6 +600,7 @@ describe("PR E2E controller exception safety", () => { mode: "resolve-control-plane", prNumber: 42, headSha: HEAD_SHA, + baseSha: BASE_SHA, workflowSha: WORKFLOW_SHA, maintainer: "maintainer", reason: "The failed gate must match the requested exception type.", @@ -580,7 +609,7 @@ describe("PR E2E controller exception safety", () => { expect(requests.some((request) => request.method === "PATCH")).toBe(false); }); - it("rejects a manual exception after main advances past the controller commit", async () => { + it("rejects a manual exception after main advances through the E2E control plane", async () => { vi.stubEnv("GITHUB_TOKEN", "token"); vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); const requests: RecordedGitHubRequest[] = []; @@ -604,7 +633,8 @@ describe("PR E2E controller exception safety", () => { conclusion: "failure", output: { title: "Maintainer control-plane exception required" }, }), - mainWorkflowRefRoute(BASE_SHA), + mainWorkflowRefRoute(ADVANCED_WORKFLOW_SHA), + compatibleMainComparisonRoute([{ filename: ".github/workflows/e2e.yaml" }]), ], requests, ), @@ -615,11 +645,12 @@ describe("PR E2E controller exception safety", () => { mode: "resolve-control-plane", prNumber: 42, headSha: HEAD_SHA, + baseSha: BASE_SHA, workflowSha: WORKFLOW_SHA, maintainer: "maintainer", - reason: "The controller commit must still be the current main revision.", + reason: "The controller commit must not be followed by control-plane changes.", }), - ).rejects.toThrow(/main no longer points to workflow commit/u); + ).rejects.toThrow(/main advanced through trusted E2E control-plane changes/u); expect(requests.some((request) => request.method === "PATCH")).toBe(false); }); @@ -663,6 +694,7 @@ describe("PR E2E controller exception safety", () => { mode: "resolve-control-plane", prNumber: 42, headSha: HEAD_SHA, + baseSha: BASE_SHA, workflowSha: WORKFLOW_SHA, maintainer: "maintainer", reason: "The internal head changed while the review was being recorded.", @@ -700,6 +732,7 @@ describe("PR E2E controller exception safety", () => { mode: "resolve-fork", prNumber: 42, headSha: HEAD_SHA, + baseSha: BASE_SHA, workflowSha: WORKFLOW_SHA, maintainer: "maintainer", reason: "The reviewed revision has since changed upstream.", @@ -707,4 +740,98 @@ describe("PR E2E controller exception safety", () => { ).rejects.toThrow(/no longer matches/u); expect(requests.some((request) => request.method === "PATCH")).toBe(false); }); + + it("rejects a fork exception after the pull request is retargeted", async () => { + vi.stubEnv("GITHUB_TOKEN", "token"); + vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url }) => url.endsWith("/collaborators/maintainer/permission"), + () => githubResponse({ role_name: "maintain", user: { login: "maintainer" } }), + ), + githubFetchRoute( + ({ url }) => url.endsWith("/pulls/42"), + () => + githubResponse({ + ...forkPullRequest(), + base: { ...forkPullRequest().base, sha: "f".repeat(40) }, + }), + ), + ], + requests, + ), + ); + + await expect( + resolveForkGate({ + mode: "resolve-fork", + prNumber: 42, + headSha: HEAD_SHA, + baseSha: BASE_SHA, + workflowSha: WORKFLOW_SHA, + maintainer: "maintainer", + reason: "The reviewed base revision has since changed upstream.", + }), + ).rejects.toThrow(/no longer matches the reviewed exact head and base SHAs/u); + expect(requests.some((request) => request.method === "PATCH")).toBe(false); + }); + + it("rejects a manual exception when the base changes immediately before completion", async () => { + vi.stubEnv("GITHUB_TOKEN", "token"); + vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + const requests: RecordedGitHubRequest[] = []; + let pullReads = 0; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url }) => url.endsWith("/collaborators/maintainer/permission"), + () => githubResponse({ role_name: "maintain", user: { login: "maintainer" } }), + ), + githubFetchRoute( + ({ url }) => url.endsWith("/pulls/42"), + () => { + pullReads += 1; + return githubResponse( + pullReads < 3 + ? pullRequest() + : { + ...pullRequest(), + base: { ...pullRequest().base, sha: "f".repeat(40) }, + }, + ); + }, + ), + githubFetchRoute( + ({ url }) => url.includes("/pulls/42/files?"), + () => githubResponse([{ filename: "tools/e2e/pr-e2e-gate.mts" }]), + ), + existingPrGateCheckRunsRoute({ + status: "completed", + conclusion: "failure", + output: { title: "Maintainer control-plane exception required" }, + }), + mainWorkflowRefRoute(), + ], + requests, + ), + ); + + await expect( + resolveControlPlaneGate({ + mode: "resolve-control-plane", + prNumber: 42, + headSha: HEAD_SHA, + baseSha: BASE_SHA, + workflowSha: WORKFLOW_SHA, + maintainer: "maintainer", + reason: "The exact base must remain current until the exception is recorded.", + }), + ).rejects.toThrow(/expected exact head and base/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 new file mode 100644 index 0000000000..7d73321d59 --- /dev/null +++ b/test/pr-e2e-gate-lifecycle.test.ts @@ -0,0 +1,655 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { createHash } from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, describe, expect, it, vi } from "vitest"; +import { buildRiskPlan } from "../tools/advisors/risk-plan.mts"; +import { + abandonPrGate, + cancelPrGate, + findSignalFiles, + finishPrGate, + type PrGateState, + type PullRequest, + parseControllerCommand, + prGateExternalId, + startPrGate, +} from "../tools/e2e/pr-e2e-gate.mts"; +import type { E2eRiskSignal } from "../tools/e2e/risk-signal.ts"; +import { + createGitHubFetchRouter, + githubFetchRoute, + type RecordedGitHubRequest, +} from "./support/github-fetch-router.ts"; + +const HEAD_SHA = "a".repeat(40); +const BASE_SHA = "b".repeat(40); +const WORKFLOW_SHA = "d".repeat(40); +const CI_RUN_ID = 99; +const CI_RUN_ATTEMPT = 3; +const CORRELATION_ID = "12345678-1234-4123-8123-123456789abc"; + +afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllEnvs(); +}); + +function githubResponse(value?: unknown, status = 200): Response { + return { + ok: status >= 200 && status < 300, + status, + json: async () => value, + text: async () => (value === undefined ? "" : JSON.stringify(value)), + } as Response; +} + +function emptyPrGateCheckRunsRoute() { + return githubFetchRoute( + ({ url, method }) => url.includes(`/commits/${HEAD_SHA}/check-runs?`) && method === "GET", + () => githubResponse({ total_count: 0, check_runs: [] }), + ); +} + +function exactPrGateCheck(overrides: Record = {}) { + return { + id: 17, + name: "E2E / PR Gate", + head_sha: HEAD_SHA, + external_id: prGateExternalId(42, HEAD_SHA, BASE_SHA), + status: "in_progress", + conclusion: null, + app: { id: 15368 }, + ...overrides, + }; +} + +function existingPrGateCheckRunsRoute(overrides: Record = {}) { + return githubFetchRoute( + ({ url, method }) => url.includes(`/commits/${HEAD_SHA}/check-runs?`) && method === "GET", + () => githubResponse({ total_count: 1, check_runs: [exactPrGateCheck(overrides)] }), + ); +} + +function sha256(value: string): string { + return createHash("sha256").update(value).digest("hex"); +} + +function pullRequest(changedFiles = 1): PullRequest { + return { + number: 42, + state: "open", + changed_files: changedFiles, + head: { + ref: "feature/pr-e2e-gate", + sha: HEAD_SHA, + repo: { full_name: "NVIDIA/NemoClaw" }, + }, + base: { + sha: BASE_SHA, + repo: { full_name: "NVIDIA/NemoClaw" }, + }, + }; +} + +function pullRequestListItem(pull = pullRequest()): Omit { + const { changed_files: _changedFiles, ...item } = pull; + return item; +} + +function state(): PrGateState { + const plan = buildRiskPlan({ headSha: HEAD_SHA, changedFiles: ["src/lib/onboard.ts"] }); + return { + version: 2, + commitSha: HEAD_SHA, + baseSha: BASE_SHA, + workflowSha: WORKFLOW_SHA, + planHash: plan.planHash, + correlationId: CORRELATION_ID, + prNumber: 42, + expectedJobs: ["onboard-repair", "onboard-resume"], + expectedShards: { + "onboard-repair": ["default"], + "onboard-resume": ["default"], + }, + }; +} + +function startCommand(workDir: string, prNumber = "42") { + const command = 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), + "--pr", + prNumber, + "--work-dir", + workDir, + ]); + expect(command.mode).toBe("start"); + return command as Extract, { mode: "start" }>; +} + +function signal( + gate: PrGateState, + jobId: string, + shardId = "default", + overrides: Partial = {}, +): E2eRiskSignal { + return { + version: 1, + jobId, + shardId, + expectedSha: gate.commitSha, + testedSha: gate.commitSha, + planHash: gate.planHash, + correlationId: gate.correlationId, + passed: 1, + failed: 0, + skipped: 0, + pending: 0, + unhandledErrors: 0, + runReason: "passed", + ...overrides, + }; +} + +function workflowRun(gate: PrGateState, overrides: Record = {}) { + return { + id: 23, + name: "E2E", + path: ".github/workflows/e2e.yaml", + workflow_id: 304268429, + event: "workflow_dispatch", + head_sha: gate.workflowSha, + status: "completed", + conclusion: "success", + display_title: `E2E PR #${gate.prNumber} (${gate.correlationId})`, + html_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/23", + ...overrides, + }; +} + +describe("PR E2E controller lifecycle", () => { + it("cancels the child and closes the check when startup fails after dispatch", async () => { + const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-start-")); + const outputPath = path.join(workDir, "github-output"); + fs.writeFileSync(outputPath, "", { mode: 0o600 }); + vi.stubEnv("GITHUB_TOKEN", "token"); + vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + vi.stubEnv("GITHUB_OUTPUT", outputPath); + const requests: RecordedGitHubRequest[] = []; + let checkPatches = 0; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + emptyPrGateCheckRunsRoute(), + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs") && method === "POST", + () => githubResponse({ id: 17 }), + ), + githubFetchRoute( + ({ url }) => url.includes("/pulls?state=open&head="), + () => githubResponse([pullRequestListItem()]), + ), + githubFetchRoute( + ({ url }) => url.includes("/pulls/42/files?"), + () => githubResponse([{ filename: "src/lib/onboard.ts" }]), + ), + githubFetchRoute( + ({ url }) => url.endsWith("/pulls/42"), + () => githubResponse(pullRequest()), + ), + githubFetchRoute( + ({ url }) => url.endsWith("/git/ref/heads/main"), + () => + githubResponse({ + ref: "refs/heads/main", + object: { type: "commit", sha: WORKFLOW_SHA }, + }), + ), + githubFetchRoute( + ({ url }) => url.endsWith("/actions/workflows/e2e.yaml/dispatches"), + () => + githubResponse({ + workflow_run_id: 23, + run_url: "https://api.github.com/repos/NVIDIA/NemoClaw/actions/runs/23", + html_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/23", + }), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/actions/runs/23/cancel") && method === "POST", + () => githubResponse(undefined, 202), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", + () => { + checkPatches += 1; + return checkPatches === 2 + ? githubResponse({ message: "simulated update failure" }, 500) + : githubResponse({}); + }, + ), + ], + requests, + ), + ); + + try { + await expect(startPrGate(startCommand(workDir))).rejects.toThrow(/simulated update failure/u); + expect(requests.some((request) => request.url.endsWith("/actions/runs/23/cancel"))).toBe( + true, + ); + const checkUpdates = requests.filter((request) => request.url.endsWith("/check-runs/17")); + expect(checkUpdates).toHaveLength(3); + expect(checkUpdates[2]?.body).toMatchObject({ + status: "completed", + conclusion: "failure", + output: { + title: "Run could not start", + summary: expect.stringContaining("The controller could not complete the check."), + }, + }); + expect(fs.readFileSync(outputPath, "utf8")).toContain("finalized=true"); + } finally { + fs.rmSync(workDir, { recursive: true, force: true }); + } + }); + + it.each([ + { + label: "the pull request base changes after dispatch", + currentPull: { + ...pullRequest(), + base: { ...pullRequest().base, sha: "c".repeat(40) }, + }, + expectedError: /expected exact head and base/u, + }, + { + label: "the pull request closes after dispatch", + currentPull: { ...pullRequest(), state: "closed" }, + expectedError: /invalid pull request state/u, + }, + ])("fails finalization when $label", async ({ currentPull, expectedError }) => { + 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"); + const evidencePath = path.join(workDir, "evidence"); + const gate = state(); + const serializedState = `${JSON.stringify(gate, null, 2)}\n`; + 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`, + ); + } + } + vi.stubEnv("GITHUB_TOKEN", "token"); + vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + vi.stubEnv("GITHUB_OUTPUT", outputPath); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url, method }) => url.endsWith("/actions/runs/23") && method === "GET", + () => githubResponse(workflowRun(gate)), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/pulls/42") && method === "GET", + () => githubResponse(currentPull), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", + () => githubResponse({}), + ), + ], + requests, + ), + ); + + try { + await expect( + finishPrGate({ + statePath, + stateHash: sha256(serializedState), + evidencePath, + checkRunId: 17, + childRunId: 23, + }), + ).rejects.toThrow(expectedError); + expect( + requests.some( + (request) => request.url.includes("/commits/") && request.url.includes("/check-runs?"), + ), + ).toBe(false); + 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" }, + }); + expect(fs.readFileSync(outputPath, "utf8")).toContain("finalized=true"); + } finally { + fs.rmSync(workDir, { recursive: true, force: true }); + } + }); + + it.each([ + { + label: "missing evidence", + status: "completed", + expectCancellation: false, + expectedTitle: "Evidence is missing", + expectedError: /Missing signals: onboard-repair:default, onboard-resume:default/u, + }, + { + label: "an unfinished child", + status: "in_progress", + expectCancellation: true, + expectedTitle: "E2E run did not succeed", + expectedError: /The run concluded unfinished \(in_progress\)/u, + }, + ])("closes the check as failure for $label", async ({ + status, + expectCancellation, + expectedTitle, + expectedError, + }) => { + const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-finish-")); + const outputPath = path.join(workDir, "github-output"); + const statePath = path.join(workDir, "controller-state.json"); + const evidencePath = path.join(workDir, "evidence"); + const gate = state(); + const serializedState = `${JSON.stringify(gate, null, 2)}\n`; + fs.writeFileSync(outputPath, "", { mode: 0o600 }); + fs.writeFileSync(statePath, serializedState, { mode: 0o600 }); + fs.mkdirSync(evidencePath); + vi.stubEnv("GITHUB_TOKEN", "token"); + vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + vi.stubEnv("GITHUB_OUTPUT", outputPath); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url, method }) => url.endsWith("/actions/runs/23") && method === "GET", + () => githubResponse(workflowRun(gate, { status, conclusion: "success" })), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/actions/runs/23/cancel") && method === "POST", + () => githubResponse(undefined, 202), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/pulls/42") && method === "GET", + () => githubResponse(pullRequest()), + ), + existingPrGateCheckRunsRoute(), + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", + () => githubResponse({}), + ), + ], + requests, + ), + ); + + try { + await expect( + finishPrGate({ + statePath, + stateHash: sha256(serializedState), + evidencePath, + checkRunId: 17, + childRunId: 23, + }), + ).rejects.toThrow(expectedError); + expect(requests.some((request) => request.url.endsWith("/actions/runs/23/cancel"))).toBe( + expectCancellation, + ); + const completion = requests.find((request) => request.url.endsWith("/check-runs/17")); + expect(completion?.body).toMatchObject({ + status: "completed", + conclusion: "failure", + output: { title: expectedTitle }, + }); + expect(fs.readFileSync(outputPath, "utf8")).toContain("finalized=true"); + } finally { + fs.rmSync(workDir, { recursive: true, force: true }); + } + }); + + it("queries active statuses without traversing completed run history", async () => { + vi.stubEnv("GITHUB_TOKEN", "token"); + vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + const gate = state(); + const requests: RecordedGitHubRequest[] = []; + const fullCompletedPage = Array.from({ length: 100 }, (_, index) => + workflowRun(gate, { id: 1_000 + index }), + ); + const fullUnrelatedQueuedPage = Array.from({ length: 100 }, (_, index) => + workflowRun( + { ...gate, prNumber: 420 }, + { id: 2_000 + index, status: "queued", conclusion: null }, + ), + ); + const runsByQuery = new Map([ + ["missing:1", fullCompletedPage], + ["queued:1", fullUnrelatedQueuedPage], + [ + "queued:2", + [ + workflowRun(gate, { status: "queued", conclusion: null }), + workflowRun(gate, { id: 24, status: "completed" }), + workflowRun(gate, { + id: 25, + status: "queued", + conclusion: null, + display_title: "E2E manual", + }), + workflowRun({ ...gate, prNumber: 420 }, { id: 26, status: "queued", conclusion: null }), + ], + ], + ]); + const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url }) => url.includes("/actions/workflows/e2e.yaml/runs?"), + ({ url }) => { + const query = new URL(url); + const status = query.searchParams.get("status"); + const page = query.searchParams.get("page"); + return githubResponse({ + workflow_runs: runsByQuery.get(`${status ?? "missing"}:${page}`) ?? [], + }); + }, + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/actions/runs/23/cancel") && method === "POST", + () => githubResponse(undefined, 202), + ), + ], + requests, + ), + ); + + await expect(cancelPrGate(42)).resolves.toBe(1); + const listQueries = requests + .filter((request) => request.url.includes("/actions/workflows/e2e.yaml/runs?")) + .map((request) => { + const query = new URL(request.url); + return `${query.searchParams.get("status")}:${query.searchParams.get("page")}`; + }); + expect(listQueries).toEqual([ + "requested:1", + "waiting:1", + "pending:1", + "queued:1", + "queued:2", + "in_progress:1", + ]); + expect( + fetchMock.mock.calls.filter(([input]) => String(input).endsWith("/cancel")), + ).toHaveLength(1); + expect(fetchMock.mock.calls.some(([input]) => String(input).endsWith("/26/cancel"))).toBe( + false, + ); + }); + + it("cancels a run once as it advances between active-status responses", async () => { + vi.stubEnv("GITHUB_TOKEN", "token"); + vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + const gate = state(); + const runsByStatus = new Map([ + ["requested", [workflowRun(gate, { status: "queued", conclusion: null })]], + ["queued", [workflowRun(gate, { status: "in_progress", conclusion: null })]], + ]); + const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter([ + githubFetchRoute( + ({ url }) => url.includes("/actions/workflows/e2e.yaml/runs?"), + ({ url }) => + githubResponse({ + workflow_runs: runsByStatus.get(new URL(url).searchParams.get("status") ?? "") ?? [], + }), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/actions/runs/23/cancel") && method === "POST", + () => githubResponse(undefined, 202), + ), + ]), + ); + + await expect(cancelPrGate(42)).resolves.toBe(1); + expect( + fetchMock.mock.calls.filter(([input]) => String(input).endsWith("/actions/runs/23/cancel")), + ).toHaveLength(1); + }); + + it("fails before cancellation when an active-status search reaches its result limit", async () => { + vi.stubEnv("GITHUB_TOKEN", "token"); + vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + const gate = state(); + const requests: RecordedGitHubRequest[] = []; + const fullActivePage = Array.from({ length: 100 }, (_, index) => + workflowRun(gate, { id: 3_000 + index, status: "in_progress", conclusion: null }), + ); + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url }) => url.includes("/actions/workflows/e2e.yaml/runs?"), + ({ url }) => + githubResponse({ + workflow_runs: + new URL(url).searchParams.get("status") === "in_progress" ? fullActivePage : [], + }), + ), + ], + requests, + ), + ); + + await expect(cancelPrGate(42)).rejects.toThrow( + "in_progress run listing exceeded its page limit", + ); + expect(requests.some((request) => request.url.endsWith("/cancel"))).toBe(false); + }); + + it("cancels a known child and closes an abandoned check as failure", async () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-abandon-")); + const outputPath = path.join(directory, "github-output"); + fs.writeFileSync(outputPath, "", { mode: 0o600 }); + vi.stubEnv("GITHUB_TOKEN", "token"); + vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + vi.stubEnv("GITHUB_OUTPUT", outputPath); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url, method }) => url.endsWith("/actions/runs/23/cancel") && method === "POST", + () => githubResponse(undefined, 202), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", + () => githubResponse(undefined), + ), + ], + requests, + ), + ); + + try { + await abandonPrGate(17, 23); + expect(requests.map((request) => request.url)).toEqual([ + "https://api.github.com/repos/NVIDIA/NemoClaw/actions/runs/23/cancel", + "https://api.github.com/repos/NVIDIA/NemoClaw/check-runs/17", + ]); + expect(requests[1]?.body).toMatchObject({ + status: "completed", + conclusion: "failure", + output: { + title: "Controller stopped early", + summary: "The controller stopped before it could complete the check.", + }, + }); + expect(fs.readFileSync(outputPath, "utf8")).toContain("finalized=true"); + } finally { + fs.rmSync(directory, { recursive: true, force: true }); + } + }); + + it("bounds recursive signal discovery and rejects symlinks", () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-evidence-")); + try { + const first = path.join(directory, "first"); + fs.mkdirSync(first); + fs.writeFileSync(path.join(first, "risk-signal.json"), "{}\n"); + expect(findSignalFiles(directory, { maxDepth: 2, maxEntries: 3, maxSignalFiles: 1 })).toEqual( + [path.join(first, "risk-signal.json")], + ); + + const second = path.join(directory, "second"); + fs.mkdirSync(second); + fs.writeFileSync(path.join(second, "risk-signal.json"), "{}\n"); + expect(() => + findSignalFiles(directory, { maxDepth: 2, maxEntries: 8, maxSignalFiles: 1 }), + ).toThrow(/signal-file limit/u); + + fs.rmSync(second, { recursive: true }); + fs.symlinkSync(first, path.join(directory, "linked")); + expect(() => + findSignalFiles(directory, { maxDepth: 2, maxEntries: 8, maxSignalFiles: 2 }), + ).toThrow(/symlinks/u); + } finally { + fs.rmSync(directory, { recursive: true, force: true }); + } + }); +}); diff --git a/test/pr-e2e-gate-workflow.test.ts b/test/pr-e2e-gate-workflow.test.ts index dcf1f26499..39b67cef42 100644 --- a/test/pr-e2e-gate-workflow.test.ts +++ b/test/pr-e2e-gate-workflow.test.ts @@ -16,6 +16,9 @@ import { const PR_GATE_PATH = ".github/workflows/pr-e2e-gate.yaml"; const E2E_PATH = ".github/workflows/e2e.yaml"; +const HEAD_SHA = "a".repeat(40); +const BASE_SHA = "b".repeat(40); +const WORKFLOW_SHA = "d".repeat(40); type CoordinatorJob = WorkflowJob & { concurrency?: { group: string; "cancel-in-progress": boolean }; @@ -141,13 +144,14 @@ function runStartStep(headBranch: string, prNumber = "42") { env: { ...process.env, 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", FAKE_NODE_ARGUMENTS: argumentsPath, GITHUB_TOKEN: "token", HEAD_BRANCH: headBranch, HEAD_REPOSITORY: "NVIDIA/NemoClaw", - HEAD_SHA: "a".repeat(40), + HEAD_SHA, PATH: `${binDir}:${process.env.PATH ?? ""}`, PR_NUMBER: prNumber, WORKFLOW_SHA: "d".repeat(40), @@ -198,7 +202,11 @@ function runCancelStep(prNumber: string) { } } -function runChildValidation(currentPullSha: string) { +function runChildValidation( + currentPullSha: string, + currentPullBase = BASE_SHA, + currentWorkflowSha = WORKFLOW_SHA, +) { const workflow = readYaml(E2E_PATH); const validation = step(workflow.jobs["generate-matrix"], "Validate controller dispatch"); const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-child-")); @@ -222,6 +230,7 @@ case "\${2:-}" in .state) printf 'open\\n' ;; .head.repo.full_name*) printf 'NVIDIA/NemoClaw\\n' ;; .head.sha) printf '%s\\n' "$FAKE_PR_SHA" ;; + .base.sha) printf '%s\\n' "$FAKE_PR_BASE_SHA" ;; *) exit 2 ;; esac `, @@ -233,9 +242,12 @@ esac encoding: "utf8", env: { ...process.env, - CHECKOUT_SHA: "a".repeat(40), + BASE_SHA, + CHECKOUT_SHA: HEAD_SHA, CORRELATION_ID: "12345678-1234-4123-8123-123456789abc", - FAKE_CHECKOUT_SHA: "a".repeat(40), + EXPECTED_WORKFLOW_SHA: WORKFLOW_SHA, + FAKE_CHECKOUT_SHA: HEAD_SHA, + FAKE_PR_BASE_SHA: currentPullBase, FAKE_PR_SHA: currentPullSha, GITHUB_REPOSITORY: "NVIDIA/NemoClaw", GITHUB_TOKEN: "token", @@ -246,6 +258,7 @@ esac TARGETS: "", WORKFLOW_EVENT: "workflow_dispatch", WORKFLOW_REF: "refs/heads/main", + WORKFLOW_SHA: currentWorkflowSha, }, }); } finally { @@ -269,12 +282,12 @@ describe("PR E2E gate workflow", () => { types: ["completed"], }, pull_request_target: { - types: ["opened", "synchronize", "reopened", "ready_for_review", "closed"], + types: ["opened", "synchronize", "reopened", "ready_for_review", "edited", "closed"], }, workflow_dispatch: { inputs: { operation: { - description: "Exact-head exception type to record.", + description: "Exact-diff exception type to record.", required: true, default: "resolve-fork", type: "choice", @@ -290,6 +303,11 @@ describe("PR E2E gate workflow", () => { required: true, type: "string", }, + expected_base_sha: { + description: "Exact 40-character PR base SHA reviewed by the maintainer.", + required: true, + type: "string", + }, waiver_reason: { description: "Why credentialed E2E cannot be run safely for this revision.", required: true, @@ -307,14 +325,31 @@ describe("PR E2E gate workflow", () => { expect(workflow.permissions).toEqual({}); expect(initialize.if).toContain("github.event_name == 'pull_request_target'"); expect(initialize.if).toContain("github.event.action != 'closed'"); - expect(initialize.permissions).toEqual({ checks: "write", contents: "read" }); + expect(initialize.if).toContain("github.event.action != 'edited'"); + expect(initialize.if).toContain("github.event.changes.base != null"); + expect(initialize.permissions).toEqual({ + checks: "write", + contents: "read", + "pull-requests": "read", + }); + expect(initialize.concurrency?.group).toBe( + "pr-e2e-gate-${{ github.event.pull_request.head.repo.full_name }}-${{ github.event.pull_request.head.ref }}", + ); expect(cancel.if).toContain("github.event_name == 'pull_request_target'"); expect(cancel.if).toContain( "github.event.pull_request.head.repo.full_name == github.repository", ); + expect(cancel.if).toContain("github.event.action != 'edited'"); + expect(cancel.if).toContain("github.event.changes.base != null"); expect(cancel.permissions).toEqual({ actions: "write", contents: "read" }); expect(coordinate.if).toContain("github.event_name == 'workflow_run'"); expect(coordinate.if).toContain("github.event.workflow_run.event == 'pull_request'"); + expect(coordinate.if).toContain( + "github.event.workflow_run.path == '.github/workflows/pr.yaml'", + ); + expect(coordinate.if).toContain( + "endsWith(github.event.workflow_run.display_title, ' gate true')", + ); expect(coordinate.if).not.toContain("head_repository.full_name == github.repository"); expect(coordinate.permissions).toEqual({ actions: "write", @@ -322,6 +357,9 @@ describe("PR E2E gate workflow", () => { contents: "read", "pull-requests": "read", }); + expect(coordinate.concurrency?.group).toBe( + "pr-e2e-gate-${{ github.event.workflow_run.head_repository.full_name }}-${{ github.event.workflow_run.head_branch }}", + ); expect(resolveException.if).toContain("github.event_name == 'workflow_dispatch'"); expect(resolveException.if).toContain("github.ref == 'refs/heads/main'"); expect(resolveException.permissions).toEqual({ @@ -333,13 +371,22 @@ describe("PR E2E gate workflow", () => { expect( collectStrings(resolveException).some((value) => value.includes('--mode "$OPERATION"')), ).toBe(true); - expect(step(initialize, "Reserve exact-head gate").run).toContain('--head "$HEAD_SHA"'); + expect(step(initialize, "Reserve exact-diff gate").run).toContain('--head "$HEAD_SHA"'); + expect(step(initialize, "Reserve exact-diff gate").env?.BASE_SHA).toBe( + "${{ github.event.pull_request.base.sha }}", + ); + expect(step(initialize, "Reserve exact-diff gate").run).toContain('--base "$BASE_SHA"'); + const start = step(coordinate, "Start evaluation"); + expect(start.env?.CI_DISPLAY_TITLE).toBe("${{ github.event.workflow_run.display_title }}"); + expect(start.run).toContain('--ci-display-title "$CI_DISPLAY_TITLE"'); const resolution = step(resolveException, "Record E2E exception"); expect(resolution.env?.OPERATION).toBe("${{ inputs.operation }}"); expect(resolution.env?.WORKFLOW_SHA).toBe("${{ github.workflow_sha }}"); expect(resolution.env?.MAINTAINER).toBe("${{ github.triggering_actor }}"); expect(resolution.env?.MAINTAINER).not.toBe("${{ github.actor }}"); + expect(resolution.env?.EXPECTED_BASE_SHA).toBe("${{ inputs.expected_base_sha }}"); expect(resolution.run).toContain('--head "$EXPECTED_HEAD_SHA"'); + expect(resolution.run).toContain('--base "$EXPECTED_BASE_SHA"'); expect(resolution.run).toContain('--workflow-sha "$WORKFLOW_SHA"'); expect(resolution.run).toContain('--reason "$WAIVER_REASON"'); expect(resolution.run).toContain('--evidence-url "$EVIDENCE_URL"'); @@ -418,13 +465,19 @@ describe("PR E2E gate workflow", () => { expect(execution.arguments[prFlag + 1]).toBe(""); }); - it("validates the E2E run against the PR head commit", () => { - const current = runChildValidation("a".repeat(40)); + it("validates the E2E run against the PR head, base, and trusted workflow commits", () => { + const current = runChildValidation(HEAD_SHA); const stale = runChildValidation("c".repeat(40)); + const retargeted = runChildValidation(HEAD_SHA, "d".repeat(40)); + const racedWorkflow = runChildValidation(HEAD_SHA, BASE_SHA, "e".repeat(40)); expect(current.status).toBe(0); expect(stale.status).toBe(1); expect(stale.stdout).toContain("checkout_sha must match the PR head commit"); + expect(retargeted.status).toBe(1); + expect(retargeted.stdout).toContain("base_sha must match the PR base commit"); + expect(racedWorkflow.status).toBe(1); + expect(racedWorkflow.stdout).toContain("workflow_sha must match the trusted workflow commit"); }); it("logs each child state once and exits after success", () => { diff --git a/test/pr-e2e-gate.test.ts b/test/pr-e2e-gate.test.ts index fc3f452fe5..83413aa942 100644 --- a/test/pr-e2e-gate.test.ts +++ b/test/pr-e2e-gate.test.ts @@ -9,14 +9,11 @@ import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import { buildRiskPlan, riskPlanRequiredJobIds } from "../tools/advisors/risk-plan.mts"; import { - abandonPrGate, assertCorrelatedWorkflowRun, - cancelPrGate, classifyPrGateEvidence, controllerErrorAnnotationTitle, dispatchPrGate, expectedSignalShards, - findSignalFiles, finishPrGate, PrerequisiteCiError, type PrGateState, @@ -42,6 +39,7 @@ import { const HEAD_SHA = "a".repeat(40); const BASE_SHA = "b".repeat(40); const WORKFLOW_SHA = "d".repeat(40); +const ADVANCED_WORKFLOW_SHA = "e".repeat(40); const CI_RUN_ID = 99; const CI_RUN_ATTEMPT = 3; const CORRELATION_ID = "12345678-1234-4123-8123-123456789abc"; @@ -96,7 +94,7 @@ function exactPrGateCheck(overrides: Record = {}) { id: 17, name: "E2E / PR Gate", head_sha: HEAD_SHA, - external_id: prGateExternalId(42, HEAD_SHA), + external_id: prGateExternalId(42, HEAD_SHA, BASE_SHA), status: "in_progress", conclusion: null, app: { id: 15368 }, @@ -132,6 +130,13 @@ function pullRequest(changedFiles = 1): PullRequest { }; } +function pullRequestDetailRoute(pull = pullRequest()) { + return githubFetchRoute( + ({ url, method }) => url.endsWith("/pulls/42") && method === "GET", + () => githubResponse(pull), + ); +} + function pullRequestListItem(pull = pullRequest()): Omit { const { changed_files: _changedFiles, ...item } = pull; return item; @@ -140,8 +145,9 @@ function pullRequestListItem(pull = pullRequest()): Omit { 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", @@ -261,10 +271,22 @@ describe("PR E2E controller", () => { mode: "cancel", prNumber: 42, }); - expect(parseControllerCommand(["--mode", "seed", "--pr", "42", "--head", HEAD_SHA])).toEqual({ + 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([ @@ -274,6 +296,8 @@ describe("PR E2E controller", () => { "42", "--head", HEAD_SHA, + "--base", + BASE_SHA, "--workflow-sha", WORKFLOW_SHA, "--maintainer", @@ -287,6 +311,7 @@ describe("PR E2E controller", () => { mode: "resolve-fork", prNumber: 42, headSha: HEAD_SHA, + baseSha: BASE_SHA, workflowSha: WORKFLOW_SHA, maintainer: "maintainer", reason: "Reviewed exact fork revision", @@ -300,6 +325,8 @@ describe("PR E2E controller", () => { "42", "--head", HEAD_SHA, + "--base", + BASE_SHA, "--workflow-sha", WORKFLOW_SHA, "--maintainer", @@ -311,6 +338,7 @@ describe("PR E2E controller", () => { mode: "resolve-control-plane", prNumber: 42, headSha: HEAD_SHA, + baseSha: BASE_SHA, workflowSha: WORKFLOW_SHA, maintainer: "maintainer", reason: "Reviewed exact control-plane revision", @@ -465,7 +493,7 @@ describe("PR E2E controller", () => { expect(() => expectedSignalShards(["not-a-workflow-job"])).toThrow(/does not define/u); }); - it("dispatches every selected job through the five-field child protocol", async () => { + it("dispatches every selected job with the exact base and accepted workflow SHA", async () => { const jobs = ["onboard-repair", "onboard-resume", "full-e2e", "hermes-e2e"]; const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation( createGitHubFetchRouter([ @@ -496,11 +524,12 @@ describe("PR E2E controller", () => { jobs, prNumber: 42, commitSha: HEAD_SHA, + baseSha: BASE_SHA, workflowSha: WORKFLOW_SHA, planHash: "c".repeat(64), correlationId: CORRELATION_ID, }), - ).resolves.toBe(23); + ).resolves.toEqual({ runId: 23, workflowSha: WORKFLOW_SHA }); expect(String(fetchMock.mock.calls[0]?.[0])).toContain("git/ref/heads/main"); const request = fetchMock.mock.calls[1]!; expect(String(request[0])).toContain("actions/workflows/e2e.yaml/dispatches"); @@ -510,6 +539,8 @@ describe("PR E2E controller", () => { jobs: jobs.join(","), pr_number: "42", checkout_sha: HEAD_SHA, + base_sha: BASE_SHA, + workflow_sha: WORKFLOW_SHA, plan_hash: "c".repeat(64), correlation_id: CORRELATION_ID, }, @@ -527,12 +558,232 @@ describe("PR E2E controller", () => { ).toThrow(/mismatched workflow dispatch URLs/u); }); - it("refuses dispatch after main advances", async () => { - const fetchMock = vi.spyOn(globalThis, "fetch").mockResolvedValue( - githubResponse({ - ref: "refs/heads/main", - object: { type: "commit", sha: BASE_SHA }, + it("dispatches from a safe descendant of the triggering workflow commit", async () => { + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url }) => url.endsWith("/git/ref/heads/main"), + () => + githubResponse({ + ref: "refs/heads/main", + object: { type: "commit", sha: ADVANCED_WORKFLOW_SHA }, + }), + ), + githubFetchRoute( + ({ url }) => url.includes(`/compare/${WORKFLOW_SHA}...${ADVANCED_WORKFLOW_SHA}`), + () => + githubResponse({ + status: "ahead", + ahead_by: 1, + behind_by: 0, + base_commit: { sha: WORKFLOW_SHA }, + merge_base_commit: { sha: WORKFLOW_SHA }, + head_commit: { sha: ADVANCED_WORKFLOW_SHA }, + files: [{ filename: "docs/quickstart.mdx" }], + }), + ), + githubFetchRoute( + ({ url }) => url.endsWith("/actions/workflows/e2e.yaml/dispatches"), + () => + githubResponse({ + workflow_run_id: 23, + run_url: "https://api.github.com/repos/NVIDIA/NemoClaw/actions/runs/23", + html_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/23", + }), + ), + ], + requests, + ), + ); + + await expect( + dispatchPrGate({ + repository: "NVIDIA/NemoClaw", + token: "token", + jobs: ["onboard-repair"], + prNumber: 42, + commitSha: HEAD_SHA, + baseSha: BASE_SHA, + workflowSha: WORKFLOW_SHA, + planHash: "c".repeat(64), + correlationId: CORRELATION_ID, + }), + ).resolves.toEqual({ runId: 23, workflowSha: ADVANCED_WORKFLOW_SHA }); + const dispatch = requests.find((request) => request.url.endsWith("/dispatches")); + expect(dispatch?.body).toMatchObject({ + inputs: { workflow_sha: ADVANCED_WORKFLOW_SHA }, + }); + expect(requests.filter((request) => request.url.endsWith("/git/ref/heads/main"))).toHaveLength( + 2, + ); + }); + + it.each([ + { + label: "a current control-plane path", + files: [{ filename: ".github/workflows/e2e.yaml" }], + }, + { + label: "a renamed control-plane path", + files: [ + { + filename: "docs/pr-gate-controller.mdx", + previous_filename: "tools/e2e/pr-e2e-gate.mts", + }, + ], + }, + ])("refuses a main advance through $label", async ({ files }) => { + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url }) => url.endsWith("/git/ref/heads/main"), + () => + githubResponse({ + ref: "refs/heads/main", + object: { type: "commit", sha: ADVANCED_WORKFLOW_SHA }, + }), + ), + githubFetchRoute( + ({ url }) => url.includes(`/compare/${WORKFLOW_SHA}...${ADVANCED_WORKFLOW_SHA}`), + () => + githubResponse({ + status: "ahead", + ahead_by: 1, + behind_by: 0, + base_commit: { sha: WORKFLOW_SHA }, + merge_base_commit: { sha: WORKFLOW_SHA }, + head_commit: { sha: ADVANCED_WORKFLOW_SHA }, + files, + }), + ), + ], + requests, + ), + ); + + await expect( + dispatchPrGate({ + repository: "NVIDIA/NemoClaw", + token: "token", + jobs: ["onboard-repair"], + prNumber: 42, + commitSha: HEAD_SHA, + baseSha: BASE_SHA, + workflowSha: WORKFLOW_SHA, + planHash: "c".repeat(64), + correlationId: CORRELATION_ID, + }), + ).rejects.toThrow(/trusted E2E control-plane changes/u); + expect(requests.some((request) => request.url.endsWith("/dispatches"))).toBe(false); + }); + + it.each([ + { + label: "diverged history", + comparison: { + status: "diverged", + ahead_by: 1, + behind_by: 1, + base_commit: { sha: WORKFLOW_SHA }, + merge_base_commit: { sha: WORKFLOW_SHA }, + head_commit: { sha: ADVANCED_WORKFLOW_SHA }, + files: [{ filename: "docs/quickstart.mdx" }], + }, + error: /not a validated descendant/u, + }, + { + label: "a comparison at the 300-file response limit", + comparison: { + status: "ahead", + ahead_by: 1, + behind_by: 0, + base_commit: { sha: WORKFLOW_SHA }, + merge_base_commit: { sha: WORKFLOW_SHA }, + head_commit: { sha: ADVANCED_WORKFLOW_SHA }, + files: Array.from({ length: 300 }, (_, index) => ({ + filename: `docs/generated-${index}.mdx`, + })), + }, + error: /too many files to validate completely/u, + }, + ])("fails closed for $label", async ({ comparison, error }) => { + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url }) => url.endsWith("/git/ref/heads/main"), + () => + githubResponse({ + ref: "refs/heads/main", + object: { type: "commit", sha: ADVANCED_WORKFLOW_SHA }, + }), + ), + githubFetchRoute( + ({ url }) => url.includes(`/compare/${WORKFLOW_SHA}...${ADVANCED_WORKFLOW_SHA}`), + () => githubResponse(comparison), + ), + ], + requests, + ), + ); + + await expect( + dispatchPrGate({ + repository: "NVIDIA/NemoClaw", + token: "token", + jobs: ["onboard-repair"], + prNumber: 42, + commitSha: HEAD_SHA, + baseSha: BASE_SHA, + workflowSha: WORKFLOW_SHA, + planHash: "c".repeat(64), + correlationId: CORRELATION_ID, }), + ).rejects.toThrow(error); + expect(requests.some((request) => request.url.endsWith("/dispatches"))).toBe(false); + }); + + it("fails closed when main changes again during compatibility validation", async () => { + const secondAdvance = "f".repeat(40); + let mainReads = 0; + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + githubFetchRoute( + ({ url }) => url.endsWith("/git/ref/heads/main"), + () => { + mainReads += 1; + return githubResponse({ + ref: "refs/heads/main", + object: { + type: "commit", + sha: mainReads === 1 ? ADVANCED_WORKFLOW_SHA : secondAdvance, + }, + }); + }, + ), + githubFetchRoute( + ({ url }) => url.includes(`/compare/${WORKFLOW_SHA}...${ADVANCED_WORKFLOW_SHA}`), + () => + githubResponse({ + status: "ahead", + ahead_by: 1, + behind_by: 0, + base_commit: { sha: WORKFLOW_SHA }, + merge_base_commit: { sha: WORKFLOW_SHA }, + head_commit: { sha: ADVANCED_WORKFLOW_SHA }, + files: [{ filename: "docs/quickstart.mdx" }], + }), + ), + ], + requests, + ), ); await expect( @@ -542,12 +793,13 @@ describe("PR E2E controller", () => { jobs: ["onboard-repair"], prNumber: 42, commitSha: HEAD_SHA, + baseSha: BASE_SHA, workflowSha: WORKFLOW_SHA, planHash: "c".repeat(64), correlationId: CORRELATION_ID, }), - ).rejects.toThrow(/main no longer points/u); - expect(fetchMock).toHaveBeenCalledOnce(); + ).rejects.toThrow(/main changed again/u); + expect(requests.some((request) => request.url.endsWith("/dispatches"))).toBe(false); }); it("uses one child title for dispatch correlation and verification", () => { @@ -567,31 +819,103 @@ describe("PR E2E controller", () => { ).toThrow(/display_title/u); }); - it("seeds one idempotent exact-head gate", async () => { + it("seeds one idempotent exact-diff gate", async () => { vi.stubEnv("GITHUB_TOKEN", "token"); vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); const requests: RecordedGitHubRequest[] = []; vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter([existingPrGateCheckRunsRoute()], requests), + createGitHubFetchRouter([pullRequestDetailRoute(), existingPrGateCheckRunsRoute()], requests), ); - await expect(seedPrGate(42, HEAD_SHA)).resolves.toBe(17); - expect(requests).toHaveLength(1); - expect(requests[0]?.method).toBe("GET"); - expect(requests[0]?.url).toContain(`/commits/${HEAD_SHA}/check-runs?`); + await expect(seedPrGate(42, HEAD_SHA, BASE_SHA)).resolves.toBe(17); + expect(requests).toHaveLength(2); + expect(requests[1]?.method).toBe("GET"); + expect(requests[1]?.url).toContain(`/commits/${HEAD_SHA}/check-runs?`); + }); + + it("closes a stale retarget check before reusing the original base check", async () => { + vi.stubEnv("GITHUB_TOKEN", "token"); + vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + const otherBaseSha = "c".repeat(40); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + pullRequestDetailRoute(), + githubFetchRoute( + ({ url, method }) => + url.includes(`/commits/${HEAD_SHA}/check-runs?`) && method === "GET", + () => + githubResponse({ + total_count: 2, + check_runs: [ + exactPrGateCheck(), + exactPrGateCheck({ + id: 18, + external_id: prGateExternalId(42, HEAD_SHA, otherBaseSha), + }), + ], + }), + ), + githubFetchRoute( + ({ url, method }) => url.endsWith("/check-runs/18") && method === "PATCH", + () => githubResponse({}), + ), + ], + requests, + ), + ); + + await expect(seedPrGate(42, HEAD_SHA, BASE_SHA)).resolves.toBe(17); + expect(requests).toHaveLength(3); + expect(requests[2]).toMatchObject({ + method: "PATCH", + body: { + status: "completed", + conclusion: "failure", + output: { title: "PR base changed" }, + }, + }); + expect(requests.some((request) => request.url.endsWith("/check-runs"))).toBe(false); }); it("rejects a seeded identity claimed by another GitHub App", async () => { vi.stubEnv("GITHUB_TOKEN", "token"); vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter([existingPrGateCheckRunsRoute({ app: { id: 999 } })]), + createGitHubFetchRouter([ + pullRequestDetailRoute(), + existingPrGateCheckRunsRoute({ app: { id: 999 } }), + ]), ); - await expect(seedPrGate(42, HEAD_SHA)).rejects.toThrow(/unexpected GitHub App/u); + await expect(seedPrGate(42, HEAD_SHA, BASE_SHA)).rejects.toThrow(/unexpected GitHub App/u); + }); + + it("rejects an out-of-order seed before mutating the live base check", async () => { + vi.stubEnv("GITHUB_TOKEN", "token"); + vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + pullRequestDetailRoute({ + ...pullRequest(), + base: { ...pullRequest().base, sha: "c".repeat(40) }, + }), + ], + requests, + ), + ); + + await expect(seedPrGate(42, HEAD_SHA, BASE_SHA)).rejects.toThrow( + /expected exact head and base/u, + ); + expect(requests).toHaveLength(1); + expect(requests.some((request) => request.url.includes("/check-runs"))).toBe(false); }); - it("rejects a pull request that does not match the triggering workflow run", async () => { + it("rejects an event PR number that does not match the trusted CI run title", async () => { const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-pr-identity-")); const outputPath = path.join(workDir, "github-output"); fs.writeFileSync(outputPath, "", { mode: 0o600 }); @@ -626,15 +950,42 @@ describe("PR E2E controller", () => { try { await expect(startPrGate(startCommand(workDir, "43"))).rejects.toThrow( - /PR identity does not match the triggering workflow run/u, + /CI run identity does not match the triggering workflow run/u, ); - expect(requests.some((request) => request.url.includes("/files?"))).toBe(false); - expect(requests.some((request) => request.url.endsWith("/dispatches"))).toBe(false); - const finalUpdate = requests - .filter((request) => request.url.endsWith("/check-runs/17") && request.method === "PATCH") - .at(-1); - expect(finalUpdate?.body).toMatchObject({ status: "completed", conclusion: "failure" }); - expect(fs.readFileSync(outputPath, "utf8")).toContain("finalized=true"); + expect(requests).toHaveLength(0); + expect(fs.readFileSync(outputPath, "utf8")).toBe(""); + } finally { + fs.rmSync(workDir, { recursive: true, force: true }); + } + }); + + it("rejects 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 }); + vi.stubEnv("GITHUB_TOKEN", "token"); + vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); + vi.stubEnv("GITHUB_OUTPUT", outputPath); + const requests: RecordedGitHubRequest[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation( + createGitHubFetchRouter( + [ + pullRequestDetailRoute({ + ...pullRequest(), + base: { ...pullRequest().base, sha: "c".repeat(40) }, + }), + ], + requests, + ), + ); + + try { + await expect( + startPrGate({ ...startCommand(workDir), ciConclusion: "failure" }), + ).rejects.toThrow(/expected exact head and base/u); + expect(requests).toHaveLength(1); + expect(requests.some((request) => request.url.includes("/check-runs"))).toBe(false); + expect(fs.readFileSync(outputPath, "utf8")).toBe(""); } finally { fs.rmSync(workDir, { recursive: true, force: true }); } @@ -651,6 +1002,7 @@ describe("PR E2E controller", () => { vi.spyOn(globalThis, "fetch").mockImplementation( createGitHubFetchRouter( [ + pullRequestDetailRoute(), emptyPrGateCheckRunsRoute(), githubFetchRoute( ({ url, method }) => url.endsWith("/check-runs") && method === "POST", @@ -722,7 +1074,8 @@ describe("PR E2E controller", () => { expect(rejectionMessage).toContain("job listing truncated"); expect(requests.some((request) => request.url.endsWith("/dispatches"))).toBe(false); - expect(requests.some((request) => request.url.includes("/pulls"))).toBe(false); + expect(requests.filter((request) => request.url.endsWith("/pulls/42"))).toHaveLength(1); + expect(requests.some((request) => request.url.includes("/pulls?"))).toBe(false); const finalUpdate = requests .filter((request) => request.url.endsWith("/check-runs/17") && request.method === "PATCH") .at(-1); @@ -815,7 +1168,8 @@ describe("PR E2E controller", () => { ), ); - expect(requests.some((request) => request.url.includes("/pulls"))).toBe(true); + expect(requests.filter((request) => request.url.endsWith("/pulls/42"))).toHaveLength(1); + expect(requests.some((request) => request.url.includes("/pulls?"))).toBe(false); const finalUpdate = requests .filter((request) => request.url.endsWith("/check-runs/17") && request.method === "PATCH") .at(-1); @@ -824,14 +1178,13 @@ describe("PR E2E controller", () => { conclusion: "failure", details_url: `https://github.com/NVIDIA/NemoClaw/actions/runs/${CI_RUN_ID}/attempts/${CI_RUN_ATTEMPT}`, output: { - title: "PR CI did not pass", + title: "PR #42 CI did not pass", summary: expect.stringContaining("Job details could not be loaded"), }, }); const summary = (finalUpdate?.body as { output?: { summary?: string } } | undefined)?.output ?.summary; - expect(summary).toContain("The triggering PR was not present in the workflow event"); - expect(summary).not.toContain("/pull/"); + expect(summary).toContain("[PR #42](https://github.com/NVIDIA/NemoClaw/pull/42)"); const outputs = fs.readFileSync(outputPath, "utf8"); expect(outputs).toContain("dispatched=false"); expect(outputs).toContain("finalized=true"); @@ -849,10 +1202,20 @@ describe("PR E2E controller", () => { vi.stubEnv("GITHUB_OUTPUT", outputPath); const requests: RecordedGitHubRequest[] = []; let gate: PrGateState | undefined; + let checkListCalls = 0; vi.spyOn(globalThis, "fetch").mockImplementation( createGitHubFetchRouter( [ - emptyPrGateCheckRunsRoute(), + githubFetchRoute( + ({ url, method }) => + url.includes(`/commits/${HEAD_SHA}/check-runs?`) && method === "GET", + () => { + checkListCalls += 1; + return checkListCalls === 1 + ? githubResponse({ total_count: 0, check_runs: [] }) + : githubResponse({ total_count: 1, check_runs: [exactPrGateCheck()] }); + }, + ), githubFetchRoute( ({ url, method }) => url.endsWith("/check-runs") && method === "POST", () => githubResponse({ id: 17 }), @@ -932,19 +1295,19 @@ describe("PR E2E controller", () => { }); expect(gate.expectedJobs).toEqual(BROAD_JOBS); - expect(requests.filter((request) => request.url.includes("/pulls?"))).toHaveLength(2); - expect(requests.filter((request) => request.url.endsWith("/pulls/42"))).toHaveLength(2); + expect(requests.filter((request) => request.url.includes("/pulls?"))).toHaveLength(1); + expect(requests.filter((request) => request.url.endsWith("/pulls/42"))).toHaveLength(3); const checkCreation = requests.find( (request) => request.url.endsWith("/check-runs") && request.method === "POST", ); expect(checkCreation?.body).toMatchObject({ name: "E2E / PR Gate", head_sha: HEAD_SHA, - external_id: prGateExternalId(42, HEAD_SHA), + external_id: prGateExternalId(42, HEAD_SHA, BASE_SHA), status: "in_progress", output: { title: "Waiting for PR CI", - summary: expect.stringContaining("exact PR revision"), + summary: expect.stringContaining("exact PR head and base revision"), }, }); const dispatch = requests.find((request) => request.url.endsWith("/dispatches")); @@ -953,6 +1316,7 @@ describe("PR E2E controller", () => { jobs: BROAD_JOBS.join(","), pr_number: "42", checkout_sha: HEAD_SHA, + base_sha: BASE_SHA, plan_hash: gate.planHash, correlation_id: gate.correlationId, }, @@ -997,7 +1361,6 @@ describe("PR E2E controller", () => { vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); vi.stubEnv("GITHUB_OUTPUT", outputPath); const requests: RecordedGitHubRequest[] = []; - let listCalls = 0; let detailCalls = 0; const updatedPull = { ...pullRequest(), @@ -1013,12 +1376,7 @@ describe("PR E2E controller", () => { ), githubFetchRoute( ({ url }) => url.includes("/pulls?state=open&head="), - () => { - listCalls += 1; - return githubResponse([ - pullRequestListItem(listCalls === 1 ? pullRequest() : updatedPull), - ]); - }, + () => githubResponse([pullRequestListItem(updatedPull)]), ), githubFetchRoute( ({ url }) => url.includes("/pulls/42/files?"), @@ -1055,377 +1413,4 @@ describe("PR E2E controller", () => { fs.rmSync(workDir, { recursive: true, force: true }); } }); - - it("cancels the child and closes the check when startup fails after dispatch", async () => { - const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-start-")); - const outputPath = path.join(workDir, "github-output"); - fs.writeFileSync(outputPath, "", { mode: 0o600 }); - vi.stubEnv("GITHUB_TOKEN", "token"); - vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); - vi.stubEnv("GITHUB_OUTPUT", outputPath); - const requests: RecordedGitHubRequest[] = []; - let checkPatches = 0; - vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter( - [ - emptyPrGateCheckRunsRoute(), - githubFetchRoute( - ({ url, method }) => url.endsWith("/check-runs") && method === "POST", - () => githubResponse({ id: 17 }), - ), - githubFetchRoute( - ({ url }) => url.includes("/pulls?state=open&head="), - () => githubResponse([pullRequestListItem()]), - ), - githubFetchRoute( - ({ url }) => url.includes("/pulls/42/files?"), - () => githubResponse([{ filename: "src/lib/onboard.ts" }]), - ), - githubFetchRoute( - ({ url }) => url.endsWith("/pulls/42"), - () => githubResponse(pullRequest()), - ), - githubFetchRoute( - ({ url }) => url.endsWith("/git/ref/heads/main"), - () => - githubResponse({ - ref: "refs/heads/main", - object: { type: "commit", sha: WORKFLOW_SHA }, - }), - ), - githubFetchRoute( - ({ url }) => url.endsWith("/actions/workflows/e2e.yaml/dispatches"), - () => - githubResponse({ - workflow_run_id: 23, - run_url: "https://api.github.com/repos/NVIDIA/NemoClaw/actions/runs/23", - html_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/23", - }), - ), - githubFetchRoute( - ({ url, method }) => url.endsWith("/actions/runs/23/cancel") && method === "POST", - () => githubResponse(undefined, 202), - ), - githubFetchRoute( - ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", - () => { - checkPatches += 1; - return checkPatches === 2 - ? githubResponse({ message: "simulated update failure" }, 500) - : githubResponse({}); - }, - ), - ], - requests, - ), - ); - - try { - await expect(startPrGate(startCommand(workDir))).rejects.toThrow(/simulated update failure/u); - expect(requests.some((request) => request.url.endsWith("/actions/runs/23/cancel"))).toBe( - true, - ); - const checkUpdates = requests.filter((request) => request.url.endsWith("/check-runs/17")); - expect(checkUpdates).toHaveLength(3); - expect(checkUpdates[2]?.body).toMatchObject({ - status: "completed", - conclusion: "failure", - output: { - title: "Run could not start", - summary: expect.stringContaining("The controller could not complete the check."), - }, - }); - expect(fs.readFileSync(outputPath, "utf8")).toContain("finalized=true"); - } finally { - fs.rmSync(workDir, { recursive: true, force: true }); - } - }); - - it.each([ - { - label: "missing evidence", - status: "completed", - expectCancellation: false, - expectedTitle: "Evidence is missing", - expectedError: /Missing signals: onboard-repair:default, onboard-resume:default/u, - }, - { - label: "an unfinished child", - status: "in_progress", - expectCancellation: true, - expectedTitle: "E2E run did not succeed", - expectedError: /The run concluded unfinished \(in_progress\)/u, - }, - ])("closes the check as failure for $label", async ({ - status, - expectCancellation, - expectedTitle, - expectedError, - }) => { - const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-finish-")); - const outputPath = path.join(workDir, "github-output"); - const statePath = path.join(workDir, "controller-state.json"); - const evidencePath = path.join(workDir, "evidence"); - const gate = state(); - const serializedState = `${JSON.stringify(gate, null, 2)}\n`; - fs.writeFileSync(outputPath, "", { mode: 0o600 }); - fs.writeFileSync(statePath, serializedState, { mode: 0o600 }); - fs.mkdirSync(evidencePath); - vi.stubEnv("GITHUB_TOKEN", "token"); - vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); - vi.stubEnv("GITHUB_OUTPUT", outputPath); - const requests: RecordedGitHubRequest[] = []; - vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter( - [ - githubFetchRoute( - ({ url, method }) => url.endsWith("/actions/runs/23") && method === "GET", - () => githubResponse(workflowRun(gate, { status, conclusion: "success" })), - ), - githubFetchRoute( - ({ url, method }) => url.endsWith("/actions/runs/23/cancel") && method === "POST", - () => githubResponse(undefined, 202), - ), - githubFetchRoute( - ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", - () => githubResponse({}), - ), - ], - requests, - ), - ); - - try { - await expect( - finishPrGate({ - statePath, - stateHash: sha256(serializedState), - evidencePath, - checkRunId: 17, - childRunId: 23, - }), - ).rejects.toThrow(expectedError); - expect(requests.some((request) => request.url.endsWith("/actions/runs/23/cancel"))).toBe( - expectCancellation, - ); - const completion = requests.find((request) => request.url.endsWith("/check-runs/17")); - expect(completion?.body).toMatchObject({ - status: "completed", - conclusion: "failure", - output: { title: expectedTitle }, - }); - expect(fs.readFileSync(outputPath, "utf8")).toContain("finalized=true"); - } finally { - fs.rmSync(workDir, { recursive: true, force: true }); - } - }); - - it("queries active statuses without traversing completed run history", async () => { - vi.stubEnv("GITHUB_TOKEN", "token"); - vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); - const gate = state(); - const requests: RecordedGitHubRequest[] = []; - const fullCompletedPage = Array.from({ length: 100 }, (_, index) => - workflowRun(gate, { id: 1_000 + index }), - ); - const fullUnrelatedQueuedPage = Array.from({ length: 100 }, (_, index) => - workflowRun( - { ...gate, prNumber: 420 }, - { id: 2_000 + index, status: "queued", conclusion: null }, - ), - ); - const runsByQuery = new Map([ - ["missing:1", fullCompletedPage], - ["queued:1", fullUnrelatedQueuedPage], - [ - "queued:2", - [ - workflowRun(gate, { status: "queued", conclusion: null }), - workflowRun(gate, { id: 24, status: "completed" }), - workflowRun(gate, { - id: 25, - status: "queued", - conclusion: null, - display_title: "E2E manual", - }), - workflowRun({ ...gate, prNumber: 420 }, { id: 26, status: "queued", conclusion: null }), - ], - ], - ]); - const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter( - [ - githubFetchRoute( - ({ url }) => url.includes("/actions/workflows/e2e.yaml/runs?"), - ({ url }) => { - const query = new URL(url); - const status = query.searchParams.get("status"); - const page = query.searchParams.get("page"); - return githubResponse({ - workflow_runs: runsByQuery.get(`${status ?? "missing"}:${page}`) ?? [], - }); - }, - ), - githubFetchRoute( - ({ url, method }) => url.endsWith("/actions/runs/23/cancel") && method === "POST", - () => githubResponse(undefined, 202), - ), - ], - requests, - ), - ); - - await expect(cancelPrGate(42)).resolves.toBe(1); - const listQueries = requests - .filter((request) => request.url.includes("/actions/workflows/e2e.yaml/runs?")) - .map((request) => { - const query = new URL(request.url); - return `${query.searchParams.get("status")}:${query.searchParams.get("page")}`; - }); - expect(listQueries).toEqual([ - "requested:1", - "waiting:1", - "pending:1", - "queued:1", - "queued:2", - "in_progress:1", - ]); - expect( - fetchMock.mock.calls.filter(([input]) => String(input).endsWith("/cancel")), - ).toHaveLength(1); - expect(fetchMock.mock.calls.some(([input]) => String(input).endsWith("/26/cancel"))).toBe( - false, - ); - }); - - it("cancels a run once as it advances between active-status responses", async () => { - vi.stubEnv("GITHUB_TOKEN", "token"); - vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); - const gate = state(); - const runsByStatus = new Map([ - ["requested", [workflowRun(gate, { status: "queued", conclusion: null })]], - ["queued", [workflowRun(gate, { status: "in_progress", conclusion: null })]], - ]); - const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter([ - githubFetchRoute( - ({ url }) => url.includes("/actions/workflows/e2e.yaml/runs?"), - ({ url }) => - githubResponse({ - workflow_runs: runsByStatus.get(new URL(url).searchParams.get("status") ?? "") ?? [], - }), - ), - githubFetchRoute( - ({ url, method }) => url.endsWith("/actions/runs/23/cancel") && method === "POST", - () => githubResponse(undefined, 202), - ), - ]), - ); - - await expect(cancelPrGate(42)).resolves.toBe(1); - expect( - fetchMock.mock.calls.filter(([input]) => String(input).endsWith("/actions/runs/23/cancel")), - ).toHaveLength(1); - }); - - it("fails before cancellation when an active-status search reaches its result limit", async () => { - vi.stubEnv("GITHUB_TOKEN", "token"); - vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); - const gate = state(); - const requests: RecordedGitHubRequest[] = []; - const fullActivePage = Array.from({ length: 100 }, (_, index) => - workflowRun(gate, { id: 3_000 + index, status: "in_progress", conclusion: null }), - ); - vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter( - [ - githubFetchRoute( - ({ url }) => url.includes("/actions/workflows/e2e.yaml/runs?"), - ({ url }) => - githubResponse({ - workflow_runs: - new URL(url).searchParams.get("status") === "in_progress" ? fullActivePage : [], - }), - ), - ], - requests, - ), - ); - - await expect(cancelPrGate(42)).rejects.toThrow( - "in_progress run listing exceeded its page limit", - ); - expect(requests.some((request) => request.url.endsWith("/cancel"))).toBe(false); - }); - - it("cancels a known child and closes an abandoned check as failure", async () => { - const directory = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-abandon-")); - const outputPath = path.join(directory, "github-output"); - fs.writeFileSync(outputPath, "", { mode: 0o600 }); - vi.stubEnv("GITHUB_TOKEN", "token"); - vi.stubEnv("GITHUB_REPOSITORY", "NVIDIA/NemoClaw"); - vi.stubEnv("GITHUB_OUTPUT", outputPath); - const requests: RecordedGitHubRequest[] = []; - vi.spyOn(globalThis, "fetch").mockImplementation( - createGitHubFetchRouter( - [ - githubFetchRoute( - ({ url, method }) => url.endsWith("/actions/runs/23/cancel") && method === "POST", - () => githubResponse(undefined, 202), - ), - githubFetchRoute( - ({ url, method }) => url.endsWith("/check-runs/17") && method === "PATCH", - () => githubResponse(undefined), - ), - ], - requests, - ), - ); - - try { - await abandonPrGate(17, 23); - expect(requests.map((request) => request.url)).toEqual([ - "https://api.github.com/repos/NVIDIA/NemoClaw/actions/runs/23/cancel", - "https://api.github.com/repos/NVIDIA/NemoClaw/check-runs/17", - ]); - expect(requests[1]?.body).toMatchObject({ - status: "completed", - conclusion: "failure", - output: { - title: "Controller stopped early", - summary: "The controller stopped before it could complete the check.", - }, - }); - expect(fs.readFileSync(outputPath, "utf8")).toContain("finalized=true"); - } finally { - fs.rmSync(directory, { recursive: true, force: true }); - } - }); - - it("bounds recursive signal discovery and rejects symlinks", () => { - const directory = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-pr-e2e-gate-evidence-")); - try { - const first = path.join(directory, "first"); - fs.mkdirSync(first); - fs.writeFileSync(path.join(first, "risk-signal.json"), "{}\n"); - expect(findSignalFiles(directory, { maxDepth: 2, maxEntries: 3, maxSignalFiles: 1 })).toEqual( - [path.join(first, "risk-signal.json")], - ); - - const second = path.join(directory, "second"); - fs.mkdirSync(second); - fs.writeFileSync(path.join(second, "risk-signal.json"), "{}\n"); - expect(() => - findSignalFiles(directory, { maxDepth: 2, maxEntries: 8, maxSignalFiles: 1 }), - ).toThrow(/signal-file limit/u); - - fs.rmSync(second, { recursive: true }); - fs.symlinkSync(first, path.join(directory, "linked")); - expect(() => - findSignalFiles(directory, { maxDepth: 2, maxEntries: 8, maxSignalFiles: 2 }), - ).toThrow(/symlinks/u); - } finally { - fs.rmSync(directory, { recursive: true, force: true }); - } - }); }); diff --git a/test/pr-review-advisor-workflow-boundary.test.ts b/test/pr-review-advisor-workflow-boundary.test.ts index a57bdcbc9f..3f7b0a1610 100644 --- a/test/pr-review-advisor-workflow-boundary.test.ts +++ b/test/pr-review-advisor-workflow-boundary.test.ts @@ -16,7 +16,9 @@ const HEAD_SHA = "b".repeat(40); const BASE_SHA = "a".repeat(40); type Workflow = { - jobs?: Record }>; + on?: { pull_request_target?: { types?: string[] } }; + concurrency?: { group?: string; "cancel-in-progress"?: boolean }; + jobs?: Record }>; }; function workflowSource(): string { @@ -229,6 +231,7 @@ describe("PR review advisor workflow boundary", () => { // source-shape-contract: security -- Exactly one advisor lane may write PR comments and neither privilege domain may gain other GitHub capabilities it("requires one advisor lane to publish the PR comment", () => { const source = fs.readFileSync(WORKFLOW_PATH, "utf8"); + const workflow = YAML.parse(source) as Workflow; const noPrimary = validateMutation((workflow) => workflow.replace("publish_comment: true", "publish_comment: false"), ); @@ -249,6 +252,15 @@ describe("PR review advisor workflow boundary", () => { ); expect(source).toContain("publish_comment: true"); + expect(workflow.on?.pull_request_target?.types).toContain("edited"); + expect(workflow.concurrency?.group).toContain( + "github.event_name != 'pull_request_target' || github.event.action != 'edited' || github.event.changes.base != null", + ); + expect(workflow.concurrency?.["cancel-in-progress"]).toBe(true); + for (const jobName of ["review", "publish"]) { + expect(workflow.jobs?.[jobName]?.if, jobName).toContain("github.event.action != 'edited'"); + expect(workflow.jobs?.[jobName]?.if, jobName).toContain("github.event.changes.base != null"); + } expect(noPrimary).toContain("advisor matrix must identify exactly one primary artifact lane"); expect(twoPrimaries).toContain( "advisor matrix must identify exactly one primary artifact lane", diff --git a/test/pr-workflow-contract.test.ts b/test/pr-workflow-contract.test.ts index 4759853df0..8e7bdf7b67 100644 --- a/test/pr-workflow-contract.test.ts +++ b/test/pr-workflow-contract.test.ts @@ -15,7 +15,9 @@ import { } from "./helpers/e2e-workflow-contract"; type CiWorkflow = { - on?: { pull_request?: { paths?: string[] } }; + "run-name"?: string; + on?: { pull_request?: { paths?: string[]; types?: string[] } }; + concurrency?: { group?: string; "cancel-in-progress"?: boolean }; permissions?: Record; jobs: Record; }; @@ -705,6 +707,20 @@ describe("pull request and main workflow contracts", () => { // source-shape-contract: security -- Pull requests must execute base-trusted actions while main uses reviewed repository actions it("reuses the same shared CI actions in PR and main workflows", () => { + expect(prWorkflow.on?.pull_request?.types).toEqual([ + "opened", + "synchronize", + "reopened", + "edited", + ]); + expect(prWorkflow["run-name"]).toBe( + "CI PR #${{ github.event.pull_request.number }} head ${{ github.event.pull_request.head.sha }} base ${{ github.event.pull_request.base.sha }} gate ${{ github.event.action != 'edited' || github.event.changes.base != null }}", + ); + expect(prWorkflow.concurrency).toEqual({ + group: + "${{ github.workflow }}-${{ github.ref }}-${{ github.event.action != 'edited' || github.event.changes.base != null }}", + "cancel-in-progress": true, + }); for (const [jobName, stepName, trustedActionPath, mainActionPath] of [ [ "static-checks", diff --git a/tools/e2e/operations-workflow-boundary.mts b/tools/e2e/operations-workflow-boundary.mts index 527d9f62ff..1a4ce645f1 100644 --- a/tools/e2e/operations-workflow-boundary.mts +++ b/tools/e2e/operations-workflow-boundary.mts @@ -112,7 +112,15 @@ function requireNode24GithubScript(errors: string[], step: WorkflowStep, owner: function validatePrGateDispatch(errors: string[], workflow: OperationsWorkflow): void { const inputs = workflow.on?.workflow_dispatch?.inputs ?? {}; - for (const name of ["jobs", "pr_number", "checkout_sha", "plan_hash", "correlation_id"]) { + for (const name of [ + "jobs", + "pr_number", + "checkout_sha", + "base_sha", + "workflow_sha", + "plan_hash", + "correlation_id", + ]) { const input = inputs[name]; if (input?.type !== "string" || input.default !== "") { errors.push(`workflow_dispatch ${name} must be an optional string with an empty default`); @@ -156,12 +164,15 @@ function validatePrGateDispatch(errors: string[], workflow: OperationsWorkflow): errors.push("Controller validation must run before workspace preparation"); } const expectedStepEnvironment = { + BASE_SHA: "${{ inputs.base_sha }}", CHECKOUT_SHA: "${{ inputs.checkout_sha }}", + EXPECTED_WORKFLOW_SHA: "${{ inputs.workflow_sha }}", JOBS: "${{ inputs.jobs }}", PLAN_HASH: "${{ inputs.plan_hash }}", PR_NUMBER: "${{ inputs.pr_number }}", CORRELATION_ID: "${{ inputs.correlation_id }}", TARGETS: "${{ inputs.targets }}", + WORKFLOW_SHA: "${{ github.workflow_sha }}", }; for (const [name, value] of Object.entries(expectedStepEnvironment)) { if (validation.env?.[name] !== value) { @@ -172,6 +183,8 @@ function validatePrGateDispatch(errors: string[], workflow: OperationsWorkflow): for (const fragment of [ '"$WORKFLOW_EVENT" == "workflow_dispatch"', '"$WORKFLOW_REF" == "refs/heads/main"', + '"$BASE_SHA" =~ ^[a-f0-9]{40}$', + '"$WORKFLOW_SHA" == "$EXPECTED_WORKFLOW_SHA"', '"$(git rev-parse --verify HEAD)" == "$CHECKOUT_SHA"', '"$PR_NUMBER" =~ ^[1-9][0-9]*$', '[[ -n "$JOBS" && -z "$TARGETS" ]]', @@ -179,6 +192,7 @@ function validatePrGateDispatch(errors: string[], workflow: OperationsWorkflow): "'.state'", "'.head.repo.full_name // \"\"'", "'.head.sha'", + `[[ "$(jq -r '.base.sha' <<< "$pull_json")" == "$BASE_SHA" ]]`, ]) { if (!validationScript.includes(fragment)) { errors.push(`Controller validation must retain ${fragment}`); diff --git a/tools/e2e/pr-e2e-gate.mts b/tools/e2e/pr-e2e-gate.mts index dcba1fdeb6..7b10d326f6 100755 --- a/tools/e2e/pr-e2e-gate.mts +++ b/tools/e2e/pr-e2e-gate.mts @@ -29,11 +29,14 @@ import { const E2E_WORKFLOW = "e2e.yaml"; const E2E_WORKFLOW_PATH = `.github/workflows/${E2E_WORKFLOW}`; const CHECK_NAME = "E2E / PR Gate"; -const CHECK_EXTERNAL_ID_PREFIX = "nemoclaw-pr-e2e:v1"; +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; const USER_AGENT = "nemoclaw-pr-e2e-gate"; const SHA_PATTERN = /^[a-f0-9]{40}$/u; const HASH_PATTERN = /^[a-f0-9]{64}$/u; +const CI_DISPLAY_TITLE_PATTERN = + /^CI PR #([1-9][0-9]*) head ([a-f0-9]{40}) base ([a-f0-9]{40}) gate true$/u; const REPOSITORY_PATTERN = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/u; const JOB_PATTERN = /^[A-Za-z0-9][A-Za-z0-9_-]*$/u; const SHARD_PATTERN = /^(?:default|[A-Za-z0-9][A-Za-z0-9_-]*)$/u; @@ -43,6 +46,7 @@ const RUN_REASONS = new Set(["passed", "failed", "interrupted"]); const MAX_PLAN_BYTES = 1024 * 1024; 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_WAIVER_REASON_CHARS = 500; @@ -71,6 +75,7 @@ type ControllerPaths = { type ManualResolutionCommandBase = { prNumber: number; headSha: string; + baseSha: string; workflowSha: string; maintainer: string; reason: string; @@ -81,7 +86,7 @@ type ManualResolutionCommand = ManualResolutionCommandBase & ({ mode: "resolve-fork" } | { mode: "resolve-control-plane" }); export type ControllerCommand = - | { mode: "seed"; prNumber: number; headSha: string } + | { mode: "seed"; prNumber: number; headSha: string; baseSha: string } | ({ mode: "start"; headSha: string; @@ -89,6 +94,7 @@ export type ControllerCommand = headBranch: string; workflowSha: string; ciConclusion: string; + ciDisplayTitle: string; ciRunId: number; ciRunAttempt: number; prNumber?: number; @@ -149,7 +155,6 @@ type CheckRun = { app?: { id?: number } | null; }; type CheckRunsResponse = { total_count: number; check_runs: CheckRun[] }; -type GitReference = { ref: string; object: { type: string; sha: string } }; type CollaboratorPermission = { role_name?: string; permission?: string; @@ -171,8 +176,9 @@ type WorkflowRunIdentity = { }; export type PrGateState = { - version: 1; + version: 2; commitSha: string; + baseSha: string; workflowSha: string; planHash: string; correlationId: string; @@ -211,6 +217,20 @@ function parseHash(value: string | undefined, name: string): string { return parsed; } +export function parseCiRunIdentity(displayTitle: string): { + prNumber: number; + headSha: string; + baseSha: string; +} { + const match = CI_DISPLAY_TITLE_PATTERN.exec(displayTitle); + if (!match) throw new Error("CI run title does not contain a valid PR and base identity"); + return { + prNumber: parsePositiveId(match[1]!, "CI run PR number"), + headSha: match[2]!, + baseSha: match[3]!, + }; +} + function normalizedWaiverReason(value: string): string { const normalized = value .replace(/[\u0000-\u001f\u007f]+/gu, " ") @@ -291,6 +311,7 @@ export function parseControllerCommand(argv: string[]): ControllerCommand { mode: "seed", prNumber: parsePositiveId(requiredArgument(args.pr, "pr"), "--pr"), headSha: requiredArgument(args.head, "head"), + baseSha: requiredArgument(args.base, "base"), }; } if (args.mode === "start") { @@ -301,6 +322,7 @@ export function parseControllerCommand(argv: string[]): ControllerCommand { headBranch: requiredArgument(args.headBranch, "head-branch"), workflowSha: requiredArgument(args.workflowSha, "workflow-sha"), ciConclusion: requiredArgument(args.ciConclusion, "ci-conclusion"), + ciDisplayTitle: requiredArgument(args.ciDisplayTitle, "ci-display-title"), ciRunId: parsePositiveId(requiredArgument(args.ciRunId, "ci-run-id"), "--ci-run-id"), ciRunAttempt: parsePositiveId( requiredArgument(args.ciRunAttempt, "ci-run-attempt"), @@ -343,6 +365,7 @@ export function parseControllerCommand(argv: string[]): ControllerCommand { mode: args.mode, prNumber: parsePositiveId(requiredArgument(args.pr, "pr"), "--pr"), headSha: requiredArgument(args.head, "head"), + baseSha: requiredArgument(args.base, "base"), workflowSha: requiredArgument(args.workflowSha, "workflow-sha"), maintainer, reason: normalizedWaiverReason(requiredArgument(args.reason, "reason")), @@ -359,12 +382,15 @@ function readRegularJson(file: string, maxBytes = MAX_PLAN_BYTES): unknown { } export function validatePrGateState(value: unknown): PrGateState { - if (!isObjectRecord(value) || value.version !== 1) { + if (!isObjectRecord(value) || value.version !== 2) { throw new Error("State version is invalid"); } if (typeof value.commitSha !== "string" || !SHA_PATTERN.test(value.commitSha)) { throw new Error("State commit SHA is invalid"); } + if (typeof value.baseSha !== "string" || !SHA_PATTERN.test(value.baseSha)) { + throw new Error("State base SHA is invalid"); + } if (typeof value.workflowSha !== "string" || !SHA_PATTERN.test(value.workflowSha)) { throw new Error("State workflow SHA is invalid"); } @@ -573,11 +599,16 @@ function appendOutput(name: string, value: string): void { } } -export function prGateExternalId(prNumber: number, headSha: string): string { - if (!Number.isSafeInteger(prNumber) || prNumber < 1 || !SHA_PATTERN.test(headSha)) { +export function prGateExternalId(prNumber: number, headSha: string, baseSha: string): string { + if ( + !Number.isSafeInteger(prNumber) || + prNumber < 1 || + !SHA_PATTERN.test(headSha) || + !SHA_PATTERN.test(baseSha) + ) { throw new Error("PR gate check identity is invalid"); } - return `${CHECK_EXTERNAL_ID_PREFIX}:${prNumber}:${headSha}`; + return `${CHECK_EXTERNAL_ID_PREFIX}:${prNumber}:${headSha}:${baseSha}`; } function validateCheckRunsResponse(value: unknown): CheckRunsResponse { @@ -601,13 +632,11 @@ function validateCheckRunsResponse(value: unknown): CheckRunsResponse { return { total_count: value.total_count as number, check_runs: checkRuns }; } -async function matchingPrGateChecks(options: { +async function listPrGateChecks(options: { repository: string; token: string; headSha: string; - prNumber: number; }): Promise { - const externalId = prGateExternalId(options.prNumber, options.headSha); const response = validateCheckRunsResponse( await githubApi( `repos/${options.repository}/commits/${options.headSha}/check-runs?check_name=${encodeURIComponent(CHECK_NAME)}&filter=all&per_page=100`, @@ -615,11 +644,30 @@ async function matchingPrGateChecks(options: { { userAgent: USER_AGENT }, ), ); - const sameIdentity = response.check_runs.filter( - (check) => - check.name === CHECK_NAME && - check.head_sha === options.headSha && - check.external_id === externalId, + return response.check_runs.filter( + (check) => check.name === CHECK_NAME && check.head_sha === options.headSha, + ); +} + +function isPrGateLineage(check: CheckRun, prNumber: number, headSha: string): boolean { + const externalId = check.external_id; + return ( + externalId === `${LEGACY_CHECK_EXTERNAL_ID_PREFIX}:${prNumber}:${headSha}` || + (typeof externalId === "string" && + externalId.startsWith(`${CHECK_EXTERNAL_ID_PREFIX}:${prNumber}:${headSha}:`)) + ); +} + +async function matchingPrGateChecks(options: { + repository: string; + token: string; + headSha: string; + baseSha: string; + prNumber: number; +}): Promise { + const externalId = prGateExternalId(options.prNumber, options.headSha, options.baseSha); + const sameIdentity = (await listPrGateChecks(options)).filter( + (check) => check.external_id === externalId, ); if (sameIdentity.some((check) => check.app?.id !== GITHUB_ACTIONS_APP_ID)) { throw new Error("PR gate check identity was claimed by an unexpected GitHub App"); @@ -631,10 +679,27 @@ async function ensurePrGateCheck(options: { repository: string; token: string; headSha: string; + baseSha: string; prNumber: number; }): Promise { - const existing = await matchingPrGateChecks(options); - if (existing.length > 1) throw new Error("Multiple exact-head PR gate checks already exist"); + const checks = await listPrGateChecks(options); + const lineage = checks.filter((check) => + isPrGateLineage(check, options.prNumber, options.headSha), + ); + if (lineage.some((check) => check.app?.id !== GITHUB_ACTIONS_APP_ID)) { + throw new Error("PR gate check identity was claimed by an unexpected GitHub App"); + } + const externalId = prGateExternalId(options.prNumber, options.headSha, options.baseSha); + const existing = lineage.filter((check) => check.external_id === externalId); + if (existing.length > 1) throw new Error("Multiple exact-diff PR gate checks already exist"); + for (const stale of lineage.filter((check) => check.external_id !== externalId)) { + await completeCheck({ repository: options.repository, checkRunId: stale.id }, options.token, { + conclusion: "failure", + title: "PR base changed", + summary: + "This check was computed for an earlier PR base and cannot authorize the current diff.", + }); + } if (existing[0]) return existing[0].id; const check = await githubApi(`repos/${options.repository}/check-runs`, options.token, { @@ -642,12 +707,12 @@ async function ensurePrGateCheck(options: { body: { name: CHECK_NAME, head_sha: options.headSha, - external_id: prGateExternalId(options.prNumber, options.headSha), + external_id: externalId, status: "in_progress", output: { title: "Waiting for PR CI", summary: - "This exact PR revision is reserved for deterministic E2E planning after CI completes.", + "This exact PR head and base revision is reserved for deterministic E2E planning after CI completes.", }, }, userAgent: USER_AGENT, @@ -658,11 +723,25 @@ async function ensurePrGateCheck(options: { return check.id; } -export async function seedPrGate(prNumber: number, headSha: string): Promise { +export async function seedPrGate( + prNumber: number, + headSha: string, + baseSha: string, +): Promise { const { token, repository } = tokenAndRepository(); if (!SHA_PATTERN.test(headSha)) throw new Error("PR head SHA is invalid"); - const checkRunId = await ensurePrGateCheck({ repository, token, headSha, prNumber }); - console.log(`Exact-head gate reserved: pr=${prNumber} sha=${headSha} check=${checkRunId}`); + if (!SHA_PATTERN.test(baseSha)) throw new Error("PR base SHA is invalid"); + await requireLiveExactDiff({ repository, token, prNumber, headSha, baseSha }); + const checkRunId = await ensurePrGateCheck({ + repository, + token, + headSha, + baseSha, + prNumber, + }); + console.log( + `Exact-diff gate reserved: pr=${prNumber} head=${headSha} base=${baseSha} check=${checkRunId}`, + ); return checkRunId; } @@ -794,6 +873,35 @@ function validatePullRequest(value: unknown): PullRequest { return { ...identity, changed_files: value.changed_files as number }; } +async function requireLiveExactDiff(options: { + repository: string; + token: string; + prNumber: number; + headSha: string; + baseSha: string; +}): Promise { + const pull = validatePullRequest( + await githubApi( + `repos/${options.repository}/pulls/${options.prNumber}`, + options.token, + { + userAgent: USER_AGENT, + }, + ), + ); + 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"); + } + return pull; +} + function pullIdentity(pull: PullRequestListItem): Record { return { number: pull.number, @@ -1148,6 +1256,93 @@ export function validateWorkflowDispatchDetails( return value as WorkflowDispatchDetails; } +function validateMainReference(value: unknown): string { + if ( + !isObjectRecord(value) || + value.ref !== "refs/heads/main" || + !isObjectRecord(value.object) || + value.object.type !== "commit" || + typeof value.object.sha !== "string" || + !SHA_PATTERN.test(value.object.sha) + ) { + throw new Error("GitHub returned an invalid main branch reference"); + } + return value.object.sha; +} + +function validateCompatibleMainComparison( + value: unknown, + workflowSha: string, + mainSha: string, +): void { + if ( + !isObjectRecord(value) || + value.status !== "ahead" || + !Number.isSafeInteger(value.ahead_by) || + (value.ahead_by as number) < 1 || + value.behind_by !== 0 || + !isObjectRecord(value.base_commit) || + value.base_commit.sha !== workflowSha || + !isObjectRecord(value.merge_base_commit) || + value.merge_base_commit.sha !== workflowSha || + !isObjectRecord(value.head_commit) || + value.head_commit.sha !== mainSha || + !Array.isArray(value.files) + ) { + throw new Error(`main is not a validated descendant of workflow commit ${workflowSha}`); + } + if (value.files.length >= MAX_COMPATIBILITY_FILES) { + throw new Error("main advance changed too many files to validate completely"); + } + const changedFiles = new Set(); + for (const entry of value.files) { + if ( + !isObjectRecord(entry) || + typeof entry.filename !== "string" || + (entry.previous_filename !== undefined && typeof entry.previous_filename !== "string") + ) { + throw new Error("GitHub returned an invalid main comparison file"); + } + for (const file of [entry.previous_filename, entry.filename]) { + if (typeof file !== "string") continue; + assertRepositoryPath(file); + changedFiles.add(file); + } + } + const plan = buildRiskPlan({ headSha: mainSha, changedFiles: [...changedFiles] }); + if (plan.families.some((family) => family.id === "e2e-control-plane")) { + throw new Error(`main advanced through trusted E2E control-plane changes after ${workflowSha}`); + } +} + +async function readMainWorkflowCommit(repository: string, token: string): Promise { + return validateMainReference( + await githubApi(`repos/${repository}/git/ref/heads/main`, token, { + userAgent: USER_AGENT, + }), + ); +} + +async function compatibleMainWorkflowCommit( + repository: string, + token: string, + workflowSha: string, +): Promise { + const mainSha = await readMainWorkflowCommit(repository, token); + if (mainSha === workflowSha) return mainSha; + const comparison = await githubApi( + `repos/${repository}/compare/${workflowSha}...${mainSha}`, + token, + { userAgent: USER_AGENT }, + ); + validateCompatibleMainComparison(comparison, workflowSha, mainSha); + const confirmedMainSha = await readMainWorkflowCommit(repository, token); + if (confirmedMainSha !== mainSha) { + throw new Error(`main changed again while validating workflow commit ${workflowSha}`); + } + return mainSha; +} + function diagnosticValue(value: unknown): string { const serialized = JSON.stringify(value) ?? String(value); return serialized.length > 256 ? `${serialized.slice(0, 253)}...` : serialized; @@ -1194,10 +1389,11 @@ export async function dispatchPrGate(options: { jobs: readonly string[]; prNumber: number; commitSha: string; + baseSha: string; workflowSha: string; planHash: string; correlationId: string; -}): Promise { +}): Promise<{ runId: number; workflowSha: string }> { assertRepository(options.repository, "repository"); if ( !options.token || @@ -1207,13 +1403,18 @@ export async function dispatchPrGate(options: { !Number.isSafeInteger(options.prNumber) || options.prNumber < 1 || !SHA_PATTERN.test(options.commitSha) || + !SHA_PATTERN.test(options.baseSha) || !SHA_PATTERN.test(options.workflowSha) || !HASH_PATTERN.test(options.planHash) || !CORRELATION_PATTERN.test(options.correlationId) ) { throw new Error("Controller dispatch inputs are invalid"); } - await assertMainWorkflowCommit(options.repository, options.token, options.workflowSha); + const workflowSha = await compatibleMainWorkflowCommit( + options.repository, + options.token, + options.workflowSha, + ); const details = await githubApi( `repos/${options.repository}/actions/workflows/${E2E_WORKFLOW}/dispatches`, options.token, @@ -1225,6 +1426,8 @@ export async function dispatchPrGate(options: { jobs: options.jobs.join(","), pr_number: String(options.prNumber), checkout_sha: options.commitSha, + base_sha: options.baseSha, + workflow_sha: workflowSha, plan_hash: options.planHash, correlation_id: options.correlationId, }, @@ -1233,25 +1436,8 @@ export async function dispatchPrGate(options: { userAgent: USER_AGENT, }, ); - return validateWorkflowDispatchDetails(details, options.repository).workflow_run_id; -} - -async function assertMainWorkflowCommit( - repository: string, - token: string, - workflowSha: string, -): Promise { - const main = await githubApi(`repos/${repository}/git/ref/heads/main`, token, { - userAgent: USER_AGENT, - }); - if ( - !main || - main.ref !== "refs/heads/main" || - main.object?.type !== "commit" || - main.object.sha !== workflowSha - ) { - throw new Error(`main no longer points to workflow commit ${workflowSha}`); - } + const runId = validateWorkflowDispatchDetails(details, options.repository).workflow_run_id; + return { runId, workflowSha }; } async function cancelChildRun(repository: string, token: string, runId: number): Promise { @@ -1274,23 +1460,32 @@ export async function startPrGate( if (!SHA_PATTERN.test(command.workflowSha)) throw new Error("workflow SHA is invalid"); assertRepository(command.headRepository, "PR head repository"); assertBranch(command.headBranch); - let pull: PullRequest | undefined; - let gatePrNumber = command.prNumber; - if (gatePrNumber === undefined) { - pull = await resolvePullRequest({ - repository, - token, - headSha: command.headSha, - headRepository: command.headRepository, - headBranch: command.headBranch, - }); - gatePrNumber = pull.number; + const ciIdentity = parseCiRunIdentity(command.ciDisplayTitle); + if ( + ciIdentity.headSha !== command.headSha || + (command.prNumber !== undefined && command.prNumber !== ciIdentity.prNumber) + ) { + throw new Error("CI run identity does not match the triggering workflow run"); + } + const pull = await requireLiveExactDiff({ + repository, + token, + prNumber: ciIdentity.prNumber, + headSha: ciIdentity.headSha, + baseSha: ciIdentity.baseSha, + }); + if ( + pull.head.repo?.full_name !== command.headRepository || + pull.head.ref !== command.headBranch + ) { + throw new Error("PR repository or branch does not match the triggering CI run"); } const checkRunId = await ensurePrGateCheck({ repository, token, headSha: command.headSha, - prNumber: gatePrNumber, + baseSha: ciIdentity.baseSha, + prNumber: ciIdentity.prNumber, }); appendOutput("check_id", String(checkRunId)); await markCheckInProgress( @@ -1323,7 +1518,7 @@ export async function startPrGate( } const report = ciFailureReport({ repository, - prNumber: command.prNumber, + prNumber: ciIdentity.prNumber, ciRunId: command.ciRunId, ciRunAttempt: command.ciRunAttempt, ciConclusion: command.ciConclusion, @@ -1336,9 +1531,7 @@ export async function startPrGate( token, { conclusion: "failure", - title: command.prNumber - ? `PR #${command.prNumber} CI did not pass` - : "PR CI did not pass", + title: `PR #${ciIdentity.prNumber} CI did not pass`, summary: report.summary, }, report.ciRunUrl, @@ -1349,20 +1542,6 @@ export async function startPrGate( throw new PrerequisiteCiError(report.errorMessage); } - pull ??= await resolvePullRequest({ - repository, - token, - headSha: command.headSha, - headRepository: command.headRepository, - headBranch: command.headBranch, - }); - if ( - pull.head.repo?.full_name !== command.headRepository || - (command.prNumber !== undefined && pull.number !== command.prNumber) - ) { - throw new Error("PR identity does not match the triggering workflow run"); - } - const changedFiles = await pullChangedFiles(repository, pull, token); const inventory = readFreeStandingJobsInventory(); const allowedJobs = new Set(inventory.allowedJobs); @@ -1392,9 +1571,9 @@ export async function startPrGate( conclusion: "failure", title: "Maintainer fork exception required", summary: [ - `This exact fork revision (${command.headSha}) selected credential-bearing E2E jobs: ${jobs.join(", ")}.`, + `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.", - "A maintainer must use the E2E / PR Gate workflow on main to record an explicit no-secret exception for this exact SHA, with a reason and any trusted supporting run.", + "A maintainer must use the E2E / PR Gate workflow on main to record an explicit no-secret exception for these exact head and base SHAs, with a reason and any trusted supporting run.", ].join("\n\n"), }, `https://github.com/${repository}/pull/${pull.number}`, @@ -1416,9 +1595,9 @@ export async function startPrGate( conclusion: "failure", title: "Maintainer control-plane exception required", summary: [ - `This exact internal revision (${command.headSha}) changes trusted E2E execution or evidence code and selected credential-bearing E2E jobs: ${jobs.join(", ")}.`, + `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.", - "A maintainer must use the E2E / PR Gate workflow on main with the resolve-control-plane operation to record an explicit no-secret exception for this exact SHA and explain the independent review performed.", + "A maintainer must use the E2E / PR Gate workflow on main with the resolve-control-plane operation to record an explicit no-secret exception for these exact head and base SHAs and explain the independent review performed.", `Deterministic plan: ${plan.planHash}.`, ].join("\n\n"), }, @@ -1450,21 +1629,24 @@ export async function startPrGate( if (!CORRELATION_PATTERN.test(correlationId)) { throw new Error("generated correlation ID is invalid"); } - childRunId = await dispatchPrGate({ + const dispatch = await dispatchPrGate({ repository, token, jobs, prNumber: pull.number, commitSha: command.headSha, + baseSha: ciIdentity.baseSha, workflowSha: command.workflowSha, planHash: plan.planHash, correlationId, }); + childRunId = dispatch.runId; appendOutput("run_id", String(childRunId)); const state: PrGateState = { - version: 1, + version: 2, commitSha: command.headSha, - workflowSha: command.workflowSha, + baseSha: ciIdentity.baseSha, + workflowSha: dispatch.workflowSha, planHash: plan.planHash, correlationId, prNumber: pull.number, @@ -1615,6 +1797,23 @@ export async function finishPrGate(options: { expectedShards: state.expectedShards, signals, }); + await requireLiveExactDiff({ + repository, + token, + prNumber: state.prNumber, + headSha: state.commitSha, + baseSha: state.baseSha, + }); + const matchingChecks = await matchingPrGateChecks({ + repository, + token, + headSha: state.commitSha, + baseSha: state.baseSha, + prNumber: state.prNumber, + }); + if (matchingChecks.length !== 1 || matchingChecks[0]!.id !== options.checkRunId) { + throw new Error("controller state does not match the exact PR gate check"); + } await completeCheck(context, token, verdict, childRunUrl); appendOutput("finalized", "true"); finalized = true; @@ -1663,6 +1862,7 @@ export async function abandonPrGate(checkRunId: number, childRunId?: number): Pr async function resolveGateException(command: ManualResolutionCommand): Promise { const { token, repository } = tokenAndRepository(); if (!SHA_PATTERN.test(command.headSha)) throw new Error("PR head SHA is invalid"); + if (!SHA_PATTERN.test(command.baseSha)) throw new Error("PR base SHA is invalid"); if (!SHA_PATTERN.test(command.workflowSha)) throw new Error("workflow SHA is invalid"); if (!MAINTAINER_PATTERN.test(command.maintainer)) throw new Error("maintainer login is invalid"); const reason = normalizedWaiverReason(command.reason); @@ -1692,9 +1892,10 @@ async function resolveGateException(command: ManualResolutionCommand): Promise { const command = parseControllerCommand(process.argv.slice(2)); if (command.mode === "seed") { - await seedPrGate(command.prNumber, command.headSha); + await seedPrGate(command.prNumber, command.headSha, command.baseSha); return; } if (command.mode === "start") {