diff --git a/.github/scripts/pr-validation.Tests.ps1 b/.github/scripts/pr-validation.Tests.ps1 index 7418d9c9304..13b95a29bfb 100644 --- a/.github/scripts/pr-validation.Tests.ps1 +++ b/.github/scripts/pr-validation.Tests.ps1 @@ -7,12 +7,12 @@ function Assert($Condition, $Message) { if (!$Condition) { throw $Message } } -function Assert-Snapshot($Request) { +function Assert-Snapshot($Request, $ExpectedSha = ('a' * 40)) { Assert ($Request.resources.repositories.self.refName -ceq 'refs/pull/42/head') 'Wrong YAML ref' - Assert ($Request.resources.repositories.self.version -ceq ('a' * 40)) 'Wrong YAML revision' + Assert ($Request.resources.repositories.self.version -ceq $ExpectedSha) 'Wrong YAML revision' Assert ($Request.templateParameters.prNumber -is [string] -and $Request.templateParameters.prNumber -ceq '42') 'PR number must be a string' - Assert ($Request.templateParameters.headSha -ceq ('a' * 40)) 'Wrong head snapshot' - Assert ($Request.templateParameters.baseSha -ceq ('b' * 40)) 'Wrong base snapshot' + Assert ($Request.templateParameters.headSha -ceq $ExpectedSha) 'Wrong reviewed snapshot' + Assert ($Request.templateParameters.PSObject.Properties.Name -cnotcontains 'baseSha') 'Base parameter must not be queued' } function Invoke-WebRequest { @@ -44,18 +44,21 @@ function Test-Case($Name, [scriptblock]$Change = {}, $ExpectedError = '', $Mode base = @{ sha = 'b' * 40; ref = 'main'; repo = @{ full_name = 'dotnet/fsharp' } } } $permission = @{ permission = 'write' } + $expectedSha = 'a' * 40 $request = @{ resources = @{ repositories = @{ self = @{ refName = 'refs/pull/42/head'; version = 'a' * 40 } } } - templateParameters = @{ prNumber = '42'; headSha = 'a' * 40; baseSha = 'b' * 40 } + templateParameters = @{ prNumber = '42'; headSha = 'a' * 40 } } $permissionUri = 'https://api.github.com/repos/dotnet/fsharp/collaborators/maintainer/permission' $pullUri = 'https://api.github.com/repos/dotnet/fsharp/pulls/42' + $commitsUri = "$pullUri/commits?per_page=100&page=1" $memberUri = 'https://api.github.com/orgs/microsoft/members/maintainer' $queueUri = 'https://dev.azure.com/devdiv/DevDiv/_apis/pipelines/123/runs?api-version=7.1' $reportUri = 'https://api.github.com/repos/dotnet/fsharp/issues/42/comments' $responses = @{ $permissionUri = @{ Method = 'GET'; Status = 200; Body = $permission } $pullUri = @{ Method = 'GET'; Status = 200; Body = $pr } + $commitsUri = @{ Method = 'GET'; Status = 200; Body = @(@{ sha = 'a' * 40 }) } $memberUri = @{ Method = 'GET'; Status = 204 } $queueUri = @{ Method = 'POST'; Status = 200; Body = @{ id = 456 } } $reportUri = @{ Method = 'POST'; Status = 201; Body = @{ id = 789 } } @@ -65,10 +68,10 @@ function Test-Case($Name, [scriptblock]$Change = {}, $ExpectedError = '', $Mode $env:GITHUB_OUTPUT = Join-Path $root 'output' $env:GITHUB_REPOSITORY = 'dotnet/fsharp'; $env:GITHUB_ACTOR = 'rerunning-admin' $env:GITHUB_SERVER_URL = 'https://github.com'; $env:GITHUB_RUN_ID = '987' - $env:GITHUB_TOKEN = if ($Mode -eq 'Membership') { 'membership-token' } else { 'github-token' } + $env:GITHUB_TOKEN = 'github-token' $env:PIPELINE_ID = '123'; $env:AZDO_TOKEN = 'test-token' $env:REQUEST_BODY = ConvertTo-Json -InputObject $request -Depth 10 -Compress - $env:RUN_URL = ''; $env:HEAD_SHA = 'a' * 40; $env:BASE_SHA = 'b' * 40 + $env:RUN_URL = ''; $env:HEAD_SHA = 'a' * 40 $expectedToken = if ($Mode -eq 'Queue') { $env:AZDO_TOKEN } else { $env:GITHUB_TOKEN } . $Change ConvertTo-Json -InputObject $event -Depth 10 | Set-Content -LiteralPath $env:GITHUB_EVENT_PATH -Encoding utf8 @@ -79,7 +82,7 @@ function Test-Case($Name, [scriptblock]$Change = {}, $ExpectedError = '', $Mode if ($ExpectedError) { Assert ($failure -and $failure -match $ExpectedError) "$Name failed for wrong reason: $failure" Assert (!(Test-Path -LiteralPath $env:GITHUB_OUTPUT)) "$Name wrote outputs on failure" - Assert ($failure -notmatch 'test-token|github-token|membership-token') 'Leaked token' + Assert ($failure -notmatch 'test-token|github-token') 'Leaked token' } else { Assert (!$failure) "$Name failed: $failure" @@ -87,8 +90,8 @@ function Test-Case($Name, [scriptblock]$Change = {}, $ExpectedError = '', $Mode switch ($Mode) { 'Prepare' { $lines = @(Get-Content -LiteralPath $env:GITHUB_OUTPUT) - Assert ($lines.Count -eq 3 -and $lines[1] -ceq "head=$('a' * 40)" -and $lines[2] -ceq "base=$('b' * 40)") 'Wrong snapshot outputs' - Assert-Snapshot (ConvertFrom-Json $lines[0].Substring(5)) + Assert ($lines.Count -eq 2 -and $lines[1] -ceq "head=$expectedSha") 'Wrong snapshot outputs' + Assert-Snapshot (ConvertFrom-Json $lines[0].Substring(5)) $expectedSha Assert ($calls[0].Uri -ceq $permissionUri -and $calls[1].Uri -ceq $pullUri) 'Wrong requester or PR' } 'Queue' { @@ -101,10 +104,13 @@ function Test-Case($Name, [scriptblock]$Change = {}, $ExpectedError = '', $Mode else { 'F# Apex request failed. [Workflow details](https://github.com/dotnet/fsharp/actions/runs/987).' } Assert ($body.StartsWith($expected)) 'Wrong PR feedback' Assert ($body.Contains('`/dart `') -and $body.Contains('`/pr-val `')) 'Missing retry guidance' - if ($env:RUN_URL) { Assert ($body.Contains("Reviewed PR head and pipeline YAML: ``$env:HEAD_SHA``") -and $body.Contains("Base: ``$env:BASE_SHA``")) 'Missing snapshot identifiers' } + if ($env:RUN_URL) { + Assert ($body.Contains("Reviewed commit and pipeline YAML: ``$env:HEAD_SHA``") -and !$body.Contains('Base:')) 'Wrong snapshot identifiers' + Assert ($body.Contains('otherwise it tests this commit directly')) 'Missing execution mode guidance' + } } } - if ($CallCount -lt 0) { $CallCount = if ($Mode -eq 'Prepare') { 2 } else { 1 } } + if ($CallCount -lt 0) { $CallCount = if ($Mode -eq 'Prepare') { 3 } else { 1 } } } if ($CallCount -ge 0) { Assert ($calls.Count -eq $CallCount) "$Name made unexpected HTTP calls" } $script:count++ @@ -121,10 +127,34 @@ try { Test-Case "reject command '$command'" { $event.comment.body = $command } 'standalone' -CallCount 0 } foreach ($alias in 'dart', 'pr-val') { - Test-Case "$alias rejects base SHA" { $event.comment.body = "/$alias $('b' * 40)" } 'current PR head' -CallCount 2 - Test-Case "$alias rejects stale reviewed SHA" { $event.comment.body = "/$alias $('c' * 40)" } 'current PR head' -CallCount 2 - Test-Case "$alias rejects updated PR head" { $event.comment.body = "/$alias $('a' * 40)"; $pr.head.sha = 'c' * 40 } 'current PR head' -CallCount 2 + Test-Case "$alias rejects unrelated base SHA" { $event.comment.body = "/$alias $('b' * 40)" } 'not found in the PR commit list' -CallCount 3 + Test-Case "$alias rejects unlisted SHA" { $event.comment.body = "/$alias $('c' * 40)" } 'not found in the PR commit list' -CallCount 3 + Test-Case "$alias keeps reviewed SHA after updated PR head" { $event.comment.body = "/$alias $('a' * 40)"; $pr.head.sha = 'c' * 40 } + Test-Case "$alias accepts older reviewed SHA" { + $expectedSha = 'c' * 40 + $event.comment.body = "/$alias $expectedSha" + $responses[$commitsUri].Body = @(@{ sha = $expectedSha }, @{ sha = 'a' * 40 }) + } + } + foreach ($page in 2, 3) { + Test-Case "reviewed commit on page $page" { + for ($i = 1; $i -lt $page; $i++) { + $responses["$pullUri/commits?per_page=100&page=$i"] = @{ Method = 'GET'; Status = 200; Body = @(1..100 | ForEach-Object { @{ sha = 'c' * 40 } }) } + } + $responses["$pullUri/commits?per_page=100&page=$page"] = @{ Method = 'GET'; Status = 200; Body = @(@{ sha = 'a' * 40 }) } + } -CallCount (2 + $page) } + Test-Case 'unlisted commit across all 250 available commits' { + foreach ($page in 1, 2, 3) { + $countOnPage = if ($page -eq 3) { 50 } else { 100 } + $responses["$pullUri/commits?per_page=100&page=$page"] = @{ Method = 'GET'; Status = 200; Body = @(1..$countOnPage | ForEach-Object { @{ sha = 'c' * 40 } }) } + } + } 'not found in the PR commit list' -CallCount 5 + foreach ($json in '{}', 'null', '{"sha":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}', '[{}]', '[{"sha":123}]', '[{"sha":"abcdef0"}]') { + Test-Case "invalid commit list $json" { $responses[$commitsUri].Raw = $json } 'invalid PR commit' -CallCount 3 + } + Test-Case 'empty commit list' { $responses[$commitsUri].Body = @() } 'not found in the PR commit list' -CallCount 3 + Test-Case 'commit list exceeds page size' { $responses[$commitsUri].Body = @(1..101 | ForEach-Object { @{ sha = 'a' * 40 } }) } 'invalid PR commit list' -CallCount 3 foreach ($access in 'write', 'maintain', 'admin') { Test-Case "permission $access" { $permission.permission = $access } } foreach ($access in 'read', 'triage', 'WRITE', '', $null, @('write')) { Test-Case "reject permission '$access'" { $permission.permission = $access } 'write access' -CallCount 1 } Test-Case 'bot' { $event.comment.user.type = 'Bot' } 'user comments' -CallCount 0 @@ -147,16 +177,15 @@ try { Test-Case 'array branch' { $pr.base.ref = @('main') } 'main' foreach ($branch in 'release/test', 'Main') { Test-Case "non-main $branch" { $pr.base.ref = $branch } 'main' } Test-Case 'foreign target' { $pr.base.repo.full_name = 'other/fsharp' } 'main' - foreach ($side in 'head', 'base') { - foreach ($sha in 'abcdef0', $null, ('A' * 40), (('a' * 40) + "`n"), 123, @('a' * 40)) { - Test-Case "bad $side SHA '$sha'" { $pr[$side].sha = $sha } 'full head/base' - } + foreach ($sha in 'abcdef0', $null, ('A' * 40), (('a' * 40) + "`n"), 123, @('a' * 40)) { + Test-Case "bad head SHA '$sha'" { $pr.head.sha = $sha } 'full PR head' } - Test-Case 'membership original commenter' -Mode Membership + Test-Case 'main advancing does not affect request' { $pr.base.sha = 'c' * 40 } + Test-Case 'membership original commenter with repository token' -Mode Membership foreach ($status in 200, 201, 301, 302, 307, 308, 401, 403, 404, 429, 500) { Test-Case "membership HTTP $status" { $responses[$memberUri].Status = $status } "HTTP $status" Membership } - foreach ($endpoint in 'permissionUri', 'pullUri', 'memberUri', 'queueUri', 'reportUri') { + foreach ($endpoint in 'permissionUri', 'pullUri', 'commitsUri', 'memberUri', 'queueUri', 'reportUri') { $mode = switch ($endpoint) { memberUri { 'Membership' }; queueUri { 'Queue' }; reportUri { 'Report' }; default { 'Prepare' } } Test-Case "$endpoint network failure" { $responses[(Get-Variable $endpoint -ValueOnly)].Failure = $true } 'HTTP request failed' $mode Test-Case "$endpoint API failure" { $responses[(Get-Variable $endpoint -ValueOnly)].Status = 500 } 'HTTP 500' $mode diff --git a/.github/scripts/pr-validation.ps1 b/.github/scripts/pr-validation.ps1 index 14fe7270aa8..37a5820e61d 100644 --- a/.github/scripts/pr-validation.ps1 +++ b/.github/scripts/pr-validation.ps1 @@ -36,7 +36,7 @@ switch ($Mode) { 'Prepare' { if ($event.comment.user.type -isnot [string] -or $event.comment.user.type -cne 'User') { throw 'Only user comments can request validation.' } if ($event.comment.body -isnot [string] -or $event.comment.body.Trim() -cnotmatch '\A/(dart|pr-val) ([0-9a-f]{40})\z') { - throw 'Use a standalone /dart or /pr-val with the full 40-character current PR head SHA.' + throw 'Use a standalone /dart or /pr-val with the full 40-character reviewed PR commit SHA.' } $requestedSha = $Matches[2] $permission = Invoke-Api "$repoApi/collaborators/$login/permission" $env:GITHUB_TOKEN @@ -48,19 +48,30 @@ switch ($Mode) { $pr.base.ref -isnot [string] -or $pr.base.ref -cne 'main') { throw 'Only open PRs targeting dotnet/fsharp main are supported.' } - $headSha, $baseSha = $pr.head.sha, $pr.base.sha - foreach ($sha in $headSha, $baseSha) { - if ($sha -isnot [string] -or $sha -cnotmatch '\A[0-9a-f]{40}\z') { throw 'GitHub did not return full head/base commit SHAs.' } + if ($pr.head.sha -isnot [string] -or $pr.head.sha -cnotmatch '\A[0-9a-f]{40}\z') { + throw 'GitHub did not return a full PR head commit SHA.' } - if ($requestedSha -cne $headSha) { - throw 'The requested SHA does not match the current PR head. Review the current commit and request a new run.' + $found = $false + for ($page = 1; $page -le 3; $page++) { + $commits = Invoke-Api "$repoApi/pulls/$prNumber/commits?per_page=100&page=$page" $env:GITHUB_TOKEN + if ($commits -isnot [array] -or $commits.Count -gt 100) { throw 'GitHub returned an invalid PR commit list.' } + foreach ($commit in $commits) { + if ($commit -isnot [System.Management.Automation.PSCustomObject] -or + $commit.PSObject.Properties.Name -cnotcontains 'sha' -or + $commit.sha -isnot [string] -or $commit.sha -cnotmatch '\A[0-9a-f]{40}\z') { throw 'GitHub returned an invalid PR commit SHA.' } + if ($commit.sha -ceq $requestedSha) { $found = $true } + } + if ($found -or $commits.Count -lt 100) { break } + } + if (!$found) { + throw 'The requested SHA was not found in the PR commit list (GitHub exposes at most 250 commits). Review a listed commit and request a new run.' } $request = @{ resources = @{ repositories = @{ self = @{ refName = "refs/pull/$prNumber/head"; version = $requestedSha } } } - templateParameters = @{ prNumber = "$prNumber"; headSha = $headSha; baseSha = $baseSha } + templateParameters = @{ prNumber = "$prNumber"; headSha = $requestedSha } } $json = ConvertTo-Json -InputObject $request -Depth 10 -Compress - Add-Content -LiteralPath $env:GITHUB_OUTPUT -Encoding utf8 -Value "body=$json", "head=$headSha", "base=$baseSha" + Add-Content -LiteralPath $env:GITHUB_OUTPUT -Encoding utf8 -Value "body=$json", "head=$requestedSha" } 'Membership' { $null = Invoke-Api "https://api.github.com/orgs/microsoft/members/$login" $env:GITHUB_TOKEN -ExpectedStatus 204 @@ -79,10 +90,10 @@ switch ($Mode) { 'Report' { $workflowUrl = "$env:GITHUB_SERVER_URL/dotnet/fsharp/actions/runs/$env:GITHUB_RUN_ID" $body = if ($env:RUN_URL) { - "[F# Apex run]($env:RUN_URL) requested by @$($event.comment.user.login).`n`nReviewed PR head and pipeline YAML: ``$env:HEAD_SHA```nBase: ``$env:BASE_SHA```n`nThe run verifies the PR merge against this snapshot before building. Changes require review and a new ``/dart `` or ``/pr-val `` comment." + "[F# Apex run]($env:RUN_URL) requested by @$($event.comment.user.login).`n`nReviewed commit and pipeline YAML: ``$env:HEAD_SHA```n`nThe run tests a PR merge containing this commit if it is still the PR head; otherwise it tests this commit directly. The setup log records the selected mode and tested SHA. New commits require review and a new ``/dart `` or ``/pr-val `` comment." } else { - "F# Apex request failed. [Workflow details]($workflowUrl). Use a standalone ``/dart `` or ``/pr-val `` with the full current PR head SHA after reviewing code and pipeline YAML. The PR must target ``main`` and the requester needs repository write access and Microsoft-org membership." + "F# Apex request failed. [Workflow details]($workflowUrl). Use a standalone ``/dart `` or ``/pr-val `` with a full commit SHA listed in the PR after reviewing code and pipeline YAML. The PR must target ``main`` and the requester needs repository write access and Microsoft-org membership." } $null = Invoke-Api "$repoApi/issues/$prNumber/comments" $env:GITHUB_TOKEN -Method POST -Body @{ body = $body } -ExpectedStatus 201 } diff --git a/.github/workflows/pr-validation.yml b/.github/workflows/pr-validation.yml index a1ad5ad5f21..bc8cd8c872b 100644 --- a/.github/workflows/pr-validation.yml +++ b/.github/workflows/pr-validation.yml @@ -33,18 +33,9 @@ jobs: GITHUB_TOKEN: ${{ github.token }} run: .\.github\scripts\pr-validation.ps1 -Mode Prepare - - name: Get Microsoft membership lookup token - id: membership-token - uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2 - with: - app-id: ${{ secrets.MICROSOFT_MEMBERS_APP_ID }} - private-key: ${{ secrets.MICROSOFT_MEMBERS_APP_PRIVATE_KEY }} - owner: microsoft - permission-members: read - - name: Verify Microsoft organization membership env: - GITHUB_TOKEN: ${{ steps.membership-token.outputs.token }} + GITHUB_TOKEN: ${{ github.token }} run: .\.github\scripts\pr-validation.ps1 -Mode Membership - name: Azure Login with OpenID Connect @@ -72,5 +63,4 @@ jobs: GITHUB_TOKEN: ${{ github.token }} RUN_URL: ${{ steps.queue.outputs.url }} HEAD_SHA: ${{ steps.request.outputs.head }} - BASE_SHA: ${{ steps.request.outputs.base }} run: .\.github\scripts\pr-validation.ps1 -Mode Report diff --git a/azure-pipelines-integration-dartlab.yml b/azure-pipelines-integration-dartlab.yml index 76e7c66b39b..31729846536 100644 --- a/azure-pipelines-integration-dartlab.yml +++ b/azure-pipelines-integration-dartlab.yml @@ -22,8 +22,6 @@ parameters: type: string - name: headSha type: string -- name: baseSha - type: string - name: visualStudioBootstrapperURI type: string default: '(default)' @@ -50,7 +48,6 @@ extends: parameters: prNumber: ${{ parameters.prNumber }} headSha: ${{ parameters.headSha }} - baseSha: ${{ parameters.baseSha }} visualStudioBootstrapperURI: ${{ parameters.visualStudioBootstrapperURI }} testMachineLab: ${{ parameters.testMachineLab }} testMachineHoldStrategy: ${{ parameters.testMachineHoldStrategy }} diff --git a/eng/pipelines/apex-integration/README.md b/eng/pipelines/apex-integration/README.md index 7c5a141dd1d..c6c1e0ed230 100644 --- a/eng/pipelines/apex-integration/README.md +++ b/eng/pipelines/apex-integration/README.md @@ -5,39 +5,84 @@ CloudTest machine, builds and deploys the PR's F# VSIX, then runs the Apex tests It avoids the dependency skew of older preinstalled CI images. The chosen VS drop must still support the PR's package pins; a recent drop is not a compatibility guarantee. +Before checking out source, the bootstrap uses `DartLab.Modules.Git`'s `Install-Git` +to copy the Git distribution bundled with the selected DartLab Production package +to `C:\Test\Git`. No separate installer or latest-version download is used. +Bootstrap and `run-apex.ps1` each prepend `C:\Test\Git\cmd` to their process PATH; +the Apex build and test children inherit it. CloudTest operations run in independent +processes, so setup does not rely on a previous operation's PATH changes. +Provisioning failures, an unusable Git executable, or a missing installation stop +the run rather than falling back to an unverified machine installation. + +The runner also sets process-local `AGENT_WORKFOLDER` to the repository root. +CloudTest supplies `BUILD_BUILDID` without an Azure Pipelines agent workspace; +MicroBuild's `AddSBOM` task needs that workspace path when packaging VS manifests. +The build children inherit the value; the caller's environment is unchanged. + +After setup/build execution, the runner copies the contents of +`artifacts\TestResults\` into CloudTest's required `LoggingDirectory`, +preserving attachment subdirectories. Handoff also runs on failure without replacing +the original build error; copy errors are reported separately. A successful build +without TRX results fails explicitly. No placeholder results are generated. + ## Requesting a run Post **`/dart `** or **`/pr-val `** as the entire comment on an open `dotnet/fsharp` PR targeting `main`. Replace `` with the full 40-character -lowercase SHA of the current PR head after reviewing that revision, including its -pipeline YAML, templates and scripts. Short, stale or non-head SHAs are rejected. +lowercase SHA of a commit listed in the PR after reviewing that revision, including +its pipeline YAML, templates and scripts. Older listed commits are supported; +short, unlisted or force-pushed-away SHAs are rejected. GitHub exposes at most 250 +PR commits, so a revision absent from that list cannot be authorized by this flow. Both commands run the same pipeline. Fork PRs are supported. The commenter must have repository write, maintain or admin access **and** be a member of the `microsoft` GitHub organization. Supplying the SHA is the requester's explicit authorization of that revision, not an automatic check of GitHub review status. -The [GitHub workflow](../../../.github/workflows/pr-validation.yml) captures the PR -head and base SHAs and queues Azure DevOps over OIDC. It posts a run link and those -revisions to the PR. New pushes, PR events and VS builds do **not** request runs. +The [GitHub workflow](../../../.github/workflows/pr-validation.yml) validates the +requested commit and queues Azure DevOps over OIDC. It posts a run link and the +reviewed revision to the PR. New pushes, PR events and VS builds do **not** request runs. Administrators can still queue runs manually, subject to pipeline permissions. The [pipeline](../../../azure-pipelines-integration-dartlab.yml) is registered directly against GitHub `dotnet/fsharp`; its `self` is **not** the dnceng mirror. The Actions workflow and authorization helper run from the default branch. The workflow pins pipeline YAML and local templates to `refs/pull//head` -at the explicitly requested head SHA, including for fork PRs. On the test machine, -[`setup-pr-validation.ps1`](../../setup-pr-validation.ps1) fetches the public PR -merge ref from a checkout of that reviewed head, verifies both parents against -the captured head/base SHAs, checks the PR is still open and observes current -head/base refs before checking out that exact merge. - -A conflict, missing/stale merge, changed head/base, or API/Git failure stops the run -before building PR code in the provided pipeline. Review the updated head and -post a new command with its SHA after resolving the problem. A push -after verification does not alter the detached commit being tested. The setup and -test logs record the actual merge SHA; Azure DevOps `Build.SourceVersion` identifies -the reviewed PR head/YAML revision, **not** the tested PR merge. Results apply only -to the recorded snapshot, not automatically to later PR revisions. +at the explicitly requested commit SHA, including for fork PRs. On the test machine, +[`setup-pr-validation.ps1`](../../setup-pr-validation.ps1) verifies the initial +checkout and checks that the PR is still open and targeting `main`. + +If the requested commit is still the current PR head, setup fetches GitHub's +generated PR merge and verifies that its second parent is the reviewed commit. +The merge's base parent is logged, not compared with a captured or live `main` +revision. Advancing `main` does not invalidate the run. A missing or stale merge +while the requested commit is still current fails explicitly rather than silently +dropping integration coverage. + +If the PR head has advanced, setup verifies the requested commit is still listed +in the PR and tests that exact commit directly, without requiring a merge ref. +This also handles a push during merge fetching: no merge containing a newer, +unreviewed head is executed. API/Git failures and invalid PR membership remain +errors. A push after selection does not alter the detached commit being tested. + +Immediately before checkout in either mode, setup refreshes the PR metadata and +requires the PR to remain open and target `dotnet/fsharp`'s `main`. Observed +retargeting or closure during selection is rejected without reintroducing a +base-SHA freshness check. GitHub's API and Git refs are not an atomic snapshot; +this recheck cannot prove that a fetched merge matches a concurrently changing +target when GitHub serves stale observations. + +Setup logs record `mode=merge` or `mode=commit`, the actual tested SHA +(`FSharp.TestedCommitSha`), and the mode (`FSharp.TestedCommitMode`). Azure DevOps +`Build.SourceVersion` identifies the reviewed YAML revision, which equals the +tested SHA only in direct-commit mode. Results apply to that recorded snapshot, +not automatically to newer PR revisions. Direct-commit mode does not validate +integration with newer `main`; normal PR CI provides that coverage. + +For manual runs, set `prNumber` and `headSha`; the latter means the requested +reviewed commit, even if it is older than the current PR head. Pin the pipeline +source Commit to the same SHA. There is no `baseSha` input. Historical revisions +whose own YAML/scripts still require `baseSha` do not acquire the new contract +retroactively; older-commit runs require a revision containing this updated flow. ## Registration and authorization @@ -61,12 +106,11 @@ Provision these before enabling requests: federated to `repo:dotnet/fsharp:environment:fsharp_pr_validation`. Grant it permission to queue this pipeline, not edit pipeline definitions. This flow does not require an Azure subscription. -5. Provide **`MICROSOFT_MEMBERS_APP_ID`** and **`MICROSOFT_MEMBERS_APP_PRIVATE_KEY`** - environment secrets for an approved GitHub App installed in `microsoft`, with - organization **Members: read** permission. The workflow requests a short-lived - membership-only token. The dotnet/fsharp `GITHUB_TOKEN` alone cannot be assumed - to see private memberships in another organization. Verify lookup with a private - member before rollout. Lookup errors fail closed; they do not prove nonmembership. +5. Microsoft-org membership lookup uses the repository **`GITHUB_TOKEN`**, matching + Roslyn's comment-command flow. No separate GitHub App or membership secrets are + required. Private memberships may not be visible to this token, so a legitimate + member with hidden membership may be rejected. Verify lookup for the intended + requesters before rollout. Lookup errors fail closed; they do not prove nonmembership. 6. Have the lab owners approve execution of fork PR code and verify effective credential/network isolation. This is a manually queued build that executes reviewed PR pipeline YAML: **do not assume automatic fork-build secret @@ -74,12 +118,12 @@ Provision these before enabling requests: provided source-verification script runs, so the SHA gate is not a sandbox. Review job tokens, feed credentials, service connections and internal network access. Git fetches use the public repository, and the Actions OIDC and - membership tokens are not sent to the test machine, but these measures alone do + GitHub tokens are not sent to the test machine, but these measures alone do not sandbox a build. Do not enable this pipeline if lab policy cannot support it. -These are three separate identities: the Azure Pipelines GitHub source connection, -the Entra queue identity, and the Microsoft-org membership lookup App. None replaces -the others. Protect `main`, restrict who can edit/queue this privileged pipeline, +The Azure Pipelines GitHub source connection and the Entra queue identity are +separate; neither replaces the other or the Actions `GITHUB_TOKEN`. +Protect `main`, restrict who can edit/queue this privileged pipeline, keep `testMachineHoldStrategy: None` as the default, and authorize no signing/publishing credentials. Use `Failure` or `Completion` only for a bounded investigation; CloudTest held machines are accessed through the generated Bastion link. @@ -96,30 +140,38 @@ Local regression commands (no product build or DartLab access required): ```powershell pwsh -NoProfile -File .github\scripts\pr-validation.Tests.ps1 powershell -NoProfile -ExecutionPolicy Bypass -File eng\tests\SetupPrValidation.Tests.ps1 +powershell -NoProfile -ExecutionPolicy Bypass -File eng\tests\ApexGit.Tests.ps1 +powershell -NoProfile -ExecutionPolicy Bypass -File eng\tests\ApexResults.Tests.ps1 ``` The script tests use temporary local Git repositories and a mocked GitHub PR response. -They cover merge identity, ref races, closed/changed PRs and failure before checkout. +They cover merge identity, advancing main, current/older reviewed commits, head +races, closed PRs and failure before checkout. +The Git provisioning tests run offline with the installer boundary mocked. They +cover missing initial PATH, independent Apex process setup, build/test child PATH +inheritance, and provisioning failures without changing machine-wide PATH. +Result handoff tests cover copied TRX/attachments, missing results, and preserving +the primary setup/build error when copying also fails. The PowerShell 7 tests run the Actions helper offline with a fake HTTP boundary. They cover command syntax, authorization of the original commenter (also on reruns), -private membership lookup with the separate App token, the queued snapshot and PR feedback. +membership lookup with the repository token, the queued snapshot and PR feedback. After provisioning, use the Runs API's `previewRun` with the same repository -ref/version, snapshot parameters, `DD-CB-ReleaseVS` resource pin and confirmed +ref/version, reviewed-commit parameters, `DD-CB-ReleaseVS` resource pin and confirmed `testMachineLab` to verify actual private-template expansion. Fix every template, parameter, expression, service-connection and operation-expansion error before allocating a CloudTest machine. Verify GitHub `self.version` pinning against the -requested head SHA for same-repository and fork PRs, and the test-machine source +requested commit SHA for same-repository and fork PRs, and the test-machine source bootstrap; local YAML parsing cannot establish either. Land the Actions workflow/authorization helper on main before activation, and configure the environment and real pipeline ID together. Explicitly request same-repository and fork PR smoke runs. Confirm the exact merge -SHA, installed VS, VSIX load, Azure DevOps test counts, Apex results and CloudTest -TRX/build/setup log artifacts. Exercise changed head/base rejection and an -unauthorized request that queues no run. Exercise missing, short, mismatched and -stale command SHAs. Verify the compiled YAML/local templates come from the -explicitly requested PR head rather than main or a newer push, and no UI overrides, +SHA and mode, installed VS, VSIX load, Azure DevOps test counts, Apex results and +CloudTest TRX/build/setup log artifacts. Exercise an advancing main, a newer PR +push and an unauthorized request that queues no run. Exercise missing, short and +unlisted command SHAs. Verify the compiled YAML/local templates come from the +explicitly requested commit rather than main or a newer push, and no UI overrides, schedules or resource triggers introduce automatic runs. Completion is not reported back as a new required GitHub check; the PR comment links to DevDiv results. diff --git a/eng/pipelines/apex-integration/run-apex.ps1 b/eng/pipelines/apex-integration/run-apex.ps1 index 65515b73669..fd1d97a6ffc 100644 --- a/eng/pipelines/apex-integration/run-apex.ps1 +++ b/eng/pipelines/apex-integration/run-apex.ps1 @@ -7,18 +7,58 @@ param( Set-StrictMode -Version Latest $ErrorActionPreference = 'Stop' +$gitDirectory = 'C:\Test\Git\cmd' +if (-not (Test-Path -LiteralPath (Join-Path $gitDirectory 'git.exe') -PathType Leaf)) { + throw 'DartLab Git was not provisioned before running Apex.' +} +$env:PATH = "$gitDirectory;$env:PATH" + $repositoryRoot = Resolve-Path (Join-Path $PSScriptRoot '..\..\..') Set-Location $repositoryRoot +$env:AGENT_WORKFOLDER = $repositoryRoot.Path + +$testedSha = (& git rev-parse HEAD).Trim() +if ($LASTEXITCODE -ne 0 -or $testedSha -notmatch '^[0-9a-f]{40}$') { + throw 'Unable to determine the selected PR test commit SHA.' +} +Write-Host "Running Apex against selected PR test commit $testedSha." -$mergeSha = (& git rev-parse HEAD).Trim() -if ($LASTEXITCODE -ne 0 -or $mergeSha -notmatch '^[0-9a-f]{40}$') { - throw 'Unable to determine the verified PR merge SHA.' +$resultsDirectory = Join-Path $repositoryRoot.Path "artifacts\TestResults\$Configuration" +$destination = $env:LoggingDirectory +if ([string]::IsNullOrWhiteSpace($destination)) { + throw 'CloudTest LoggingDirectory is required to publish Apex test results.' } -Write-Host "Running Apex against verified PR merge $mergeSha." $env:NativeToolsOnMachine = 'true' -& .\eng\SetupVSHive.ps1 -& .\eng\CIBuildNoPublish.cmd -configuration $Configuration -deployExtensions -testApex -if ($LASTEXITCODE -ne 0) { - throw "Apex build and tests failed with exit code $LASTEXITCODE." +$buildFailure = $null +try { + & .\eng\SetupVSHive.ps1 + & .\eng\CIBuildNoPublish.cmd -configuration $Configuration -deployExtensions -testApex + if ($LASTEXITCODE -ne 0) { + throw "Apex build and tests failed with exit code $LASTEXITCODE." + } +} +catch { + $buildFailure = $_ + throw +} +finally { + try { + $trxFiles = @() + if (Test-Path -LiteralPath $resultsDirectory -PathType Container) { + $trxFiles = @(Get-ChildItem -LiteralPath $resultsDirectory -Filter '*.trx' -File -Recurse) + New-Item -ItemType Directory -Path $destination -Force | Out-Null + Get-ChildItem -LiteralPath $resultsDirectory -Force | + Copy-Item -Destination $destination -Recurse -Force + } + Write-Host "Apex result handoff: $($trxFiles.Count) TRX file(s) from '$resultsDirectory' to '$destination'." + if (!$buildFailure -and $trxFiles.Count -eq 0) { + throw "Apex build succeeded but no TRX results were produced in '$resultsDirectory'." + } + } + catch { + $message = "Failed to hand off Apex results from '$resultsDirectory' to '$destination': $_" + if (!$buildFailure) { throw $message } + Write-Error -Message $message -ErrorAction Continue + } } diff --git a/eng/pipelines/apex-integration/stage.yml b/eng/pipelines/apex-integration/stage.yml index 1b8e701bd55..97b786fba4b 100644 --- a/eng/pipelines/apex-integration/stage.yml +++ b/eng/pipelines/apex-integration/stage.yml @@ -3,8 +3,6 @@ parameters: type: string - name: headSha type: string -- name: baseSha - type: string - name: visualStudioBootstrapperURI type: string default: '(default)' @@ -127,6 +125,11 @@ stages: displayName: Verify and check out PR snapshot scriptBlock: | $ErrorActionPreference = 'Stop' + Import-Module DartLab.Modules.Git -ErrorAction Stop + Install-Git -InstallationDirectory 'C:\Test\Git' -Scope Process + $env:PATH = "C:\Test\Git\cmd;$env:PATH" + & 'C:\Test\Git\cmd\git.exe' --version + if ($LASTEXITCODE -ne 0) { throw 'The provisioned DartLab Git is not usable.' } $sourceRoot = 'C:\Test\FSharp' if (Test-Path -LiteralPath $sourceRoot) { throw "$sourceRoot already exists; refusing to reuse an unverified checkout." @@ -136,7 +139,7 @@ stages: git -C $sourceRoot remote add origin https://github.com/dotnet/fsharp.git if ($LASTEXITCODE -ne 0) { throw 'git remote add failed.' } if ($env:PIPELINE_SOURCE_SHA -cne $env:HEAD_SHA) { - throw 'The pipeline YAML revision does not match the reviewed PR head.' + throw 'The pipeline YAML revision does not match the reviewed PR commit.' } git -C $sourceRoot fetch --no-tags --depth=1 origin $env:PIPELINE_SOURCE_SHA if ($LASTEXITCODE -ne 0) { throw 'Fetching the reviewed pipeline source revision failed.' } @@ -147,7 +150,7 @@ stages: } Push-Location $sourceRoot try { - & .\eng\setup-pr-validation.ps1 -prNumber $env:PR_NUMBER -headSha $env:HEAD_SHA -baseSha $env:BASE_SHA + & .\eng\setup-pr-validation.ps1 -prNumber $env:PR_NUMBER -headSha $env:HEAD_SHA } finally { Pop-Location @@ -156,5 +159,4 @@ stages: PIPELINE_SOURCE_SHA: $(Build.SourceVersion) PR_NUMBER: ${{ parameters.prNumber }} HEAD_SHA: ${{ parameters.headSha }} - BASE_SHA: ${{ parameters.baseSha }} timeout: 0:0:10:0 diff --git a/eng/setup-pr-validation.ps1 b/eng/setup-pr-validation.ps1 index 75276e9c54f..18b02f795df 100644 --- a/eng/setup-pr-validation.ps1 +++ b/eng/setup-pr-validation.ps1 @@ -5,10 +5,7 @@ param ( [string]$prNumber, [Parameter(Mandatory=$true)] [ValidatePattern('^[0-9a-f]{40}$')] - [string]$headSha, - [Parameter(Mandatory=$true)] - [ValidatePattern('^[0-9a-f]{40}$')] - [string]$baseSha + [string]$headSha ) Set-StrictMode -Version Latest @@ -24,34 +21,86 @@ function Invoke-Git { $output | ForEach-Object { "$_" } } -if ((Invoke-Git rev-parse HEAD) -ne $headSha) { - throw 'Expected the reviewed PR checkout at the captured head SHA.' +function Get-PrHeadSha { + $refName = "refs/pull/$prNumber/head" + $refs = @(Invoke-Git ls-remote $repository $refName) + Write-Host 'Advertised PR head ref (tabs shown as ):' + foreach ($ref in $refs) { + Write-Host " $($ref.Replace("`t", ''))" + } + if ($refs.Count -ne 1 -or $refs[0] -cnotmatch "\A([0-9a-f]{40})`t$refName\z") { + throw 'Git did not advertise exactly one valid PR head ref.' + } + $Matches[1] } -$repository = 'https://github.com/dotnet/fsharp.git' -Invoke-Git fetch --no-tags --depth=2 $repository "refs/pull/$prNumber/merge" | Out-Null -$mergeSha = Invoke-Git rev-parse FETCH_HEAD -$parents = (Invoke-Git show -s --format=%P $mergeSha) -split ' ' -if ($parents.Count -ne 2 -or $parents[0] -ne $baseSha -or $parents[1] -ne $headSha) { - throw 'The fetched PR merge does not match the captured base/head. Request a new /dart or /pr-val run.' +function Get-ValidatedPr { + $pr = Invoke-RestMethod -Uri "https://api.github.com/repos/dotnet/fsharp/pulls/$prNumber" -Headers @{ + Accept = 'application/vnd.github+json' + 'User-Agent' = 'FSharp-Apex-Validation' + 'X-GitHub-Api-Version' = '2022-11-28' + } + Write-Host "GitHub PR: state=$($pr.state) target=$($pr.base.repo.full_name):$($pr.base.ref) head=$($pr.head.sha) base=$($pr.base.sha)" + if ($pr.state -cne 'open' -or $pr.base.repo.full_name -cne 'dotnet/fsharp' -or + $pr.base.ref -cne 'main') { + throw 'The PR is closed or no longer targets dotnet/fsharp main.' + } + if ($pr.head.sha -isnot [string] -or $pr.head.sha -cnotmatch '\A[0-9a-f]{40}\z') { + throw 'GitHub did not return a full PR head commit SHA.' + } + $pr } -$pr = Invoke-RestMethod -Uri "https://api.github.com/repos/dotnet/fsharp/pulls/$prNumber" -Headers @{ - Accept = 'application/vnd.github+json' - 'User-Agent' = 'FSharp-Apex-Validation' - 'X-GitHub-Api-Version' = '2022-11-28' +Write-Host "Requested snapshot: PR #$prNumber reviewed=$headSha" +$checkoutSha = Invoke-Git rev-parse HEAD +Write-Host "Initial checkout: $checkoutSha" +if ($checkoutSha -ne $headSha) { + throw 'Expected the reviewed PR checkout at the captured head SHA.' } -if ($pr.state -ne 'open' -or $pr.base.repo.full_name -ne 'dotnet/fsharp' -or - $pr.base.ref -ne 'main' -or $pr.head.sha -ne $headSha -or $pr.base.sha -ne $baseSha) { - throw 'The PR is closed or its head/base changed. Request a new /dart or /pr-val run.' + +$repository = 'https://github.com/dotnet/fsharp.git' +$pr = Get-ValidatedPr +$currentHead = Get-PrHeadSha +$testedSha = $headSha +$mode = 'commit' +if ($currentHead -ceq $headSha) { + Invoke-Git fetch --no-tags --depth=2 $repository "refs/pull/$prNumber/merge" | Out-Null + $mergeSha = Invoke-Git rev-parse FETCH_HEAD + $parents = (Invoke-Git show -s --format=%P $mergeSha) -split ' ' + Write-Host "Fetched merge: $mergeSha parents=$($parents -join ' ')" + $currentHead = Get-PrHeadSha + if ($currentHead -ceq $headSha) { + if ($parents.Count -ne 2 -or $parents[1] -cne $headSha) { + throw 'The fetched PR merge does not contain the reviewed head as its second parent.' + } + $testedSha = $mergeSha + $mode = 'merge' + } } -$refs = Invoke-Git ls-remote $repository refs/heads/main "refs/pull/$prNumber/head" -foreach ($expected in @("$baseSha`trefs/heads/main", "$headSha`trefs/pull/$prNumber/head")) { - if ($refs -cnotcontains $expected) { - throw 'The current head/base refs do not match the snapshot. Request a new /dart or /pr-val run.' + +if ($mode -eq 'commit') { + $found = $false + for ($page = 1; $page -le 3; $page++) { + $commits = Invoke-RestMethod -Uri "https://api.github.com/repos/dotnet/fsharp/pulls/$prNumber/commits?per_page=100&page=$page" -Headers @{ + Accept = 'application/vnd.github+json' + 'User-Agent' = 'FSharp-Apex-Validation' + 'X-GitHub-Api-Version' = '2022-11-28' + } + if ($commits -isnot [array] -or $commits.Count -gt 100) { throw 'GitHub returned an invalid PR commit list.' } + foreach ($commit in $commits) { + if ($commit -isnot [System.Management.Automation.PSCustomObject] -or + $commit.PSObject.Properties.Name -cnotcontains 'sha' -or + $commit.sha -isnot [string] -or $commit.sha -cnotmatch '\A[0-9a-f]{40}\z') { throw 'GitHub returned an invalid PR commit SHA.' } + if ($commit.sha -ceq $headSha) { $found = $true } + } + if ($found -or $commits.Count -lt 100) { break } } + if (!$found) { throw 'The reviewed SHA was not found in the PR commit list (GitHub exposes at most 250 commits).' } + Write-Host "PR head observations: API=$($pr.head.sha) Git=$currentHead; testing reviewed commit directly." } -Invoke-Git checkout --detach $mergeSha | Out-Null -Write-Host "PR #$prNumber`: head=$headSha base=$baseSha merge=$mergeSha" -Write-Host "##vso[task.setvariable variable=FSharp.PrMergeSha]$mergeSha" +$pr = Get-ValidatedPr +Invoke-Git checkout --detach $testedSha | Out-Null +Write-Host "PR #$prNumber`: reviewed=$headSha mode=$mode tested=$testedSha" +Write-Host "##vso[task.setvariable variable=FSharp.TestedCommitSha]$testedSha" +Write-Host "##vso[task.setvariable variable=FSharp.TestedCommitMode]$mode" diff --git a/eng/tests/ApexGit.Tests.ps1 b/eng/tests/ApexGit.Tests.ps1 new file mode 100644 index 00000000000..cd827c7fc06 --- /dev/null +++ b/eng/tests/ApexGit.Tests.ps1 @@ -0,0 +1,124 @@ +$ErrorActionPreference = 'Stop' +$apex = Join-Path $PSScriptRoot '..\pipelines\apex-integration' +$root = Join-Path $PSScriptRoot ("apex-git-fixture-" + [Guid]::NewGuid()) +$installation = Join-Path $root 'Git' +$git = (Get-Command git.exe -CommandType Application | Select-Object -First 1).Source +$gitRoot = Split-Path (Split-Path $git -Parent) -Parent +$previousPath = $env:PATH +$previousWorkFolder = $env:AGENT_WORKFOLDER +$previousLoggingDirectory = $env:LoggingDirectory +$systemPath = "$env:SystemRoot\System32" +$powershell = "$env:SystemRoot\System32\WindowsPowerShell\v1.0\powershell.exe" +$yaml = Get-Content (Join-Path $apex 'stage.yml') -Raw +$bootstrap = [regex]::Match($yaml, '(?ms)displayName: Verify and check out PR snapshot\r?\n scriptBlock: \|\r?\n(?