From 1c9a509dc7edc448c5f8423e786d6552497d440c Mon Sep 17 00:00:00 2001 From: Wael AbuSeada Date: Wed, 3 Jun 2026 16:45:34 -0600 Subject: [PATCH] Use intake artifact metadata for PR resolution and improve parse diagnostics - make Copilot PR Review Runner resolve PR details from downloaded intake artifact metadata - remove workflow fallback lookup complexity and keep strict SHA validation - enhance JSON parse diagnostics with candidate index/hash/length and dedicated parse-errors artifact file --- .github/workflows/CopilotPRReviewRunner.yaml | 65 ++++++++++--------- .../scripts/Invoke-CopilotPRReview.ps1 | 26 ++++++-- 2 files changed, 55 insertions(+), 36 deletions(-) diff --git a/.github/workflows/CopilotPRReviewRunner.yaml b/.github/workflows/CopilotPRReviewRunner.yaml index ce5db91496..0f71e240e3 100644 --- a/.github/workflows/CopilotPRReviewRunner.yaml +++ b/.github/workflows/CopilotPRReviewRunner.yaml @@ -29,6 +29,15 @@ jobs: run: shell: pwsh steps: + - name: Download PR metadata artifact + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + github-token: ${{ github.token }} + run-id: ${{ github.event.workflow_run.id }} + pattern: copilot-pr-review-input-* + path: ${{ runner.temp }}/copilot-pr-review-input + merge-multiple: true + - name: Resolve PR details id: pr env: @@ -42,49 +51,43 @@ jobs: $expectedHeadSha = '${{ github.event.workflow_run.head_sha }}' - $prNumber = '${{ github.event.workflow_run.pull_requests[0].number }}' - if (-not $prNumber) { - $headOwner = '${{ github.event.workflow_run.head_repository.owner.login }}' - $headBranch = '${{ github.event.workflow_run.head_branch }}' - $baseBranch = '${{ github.event.workflow_run.pull_requests[0].base.ref }}' - - if (-not $headOwner -or -not $headBranch) { - throw 'No pull request number in workflow_run payload and insufficient head repo/branch metadata for fallback lookup.' - } - - $encodedHead = [System.Uri]::EscapeDataString("${headOwner}:$headBranch") - $query = "state=open&head=$encodedHead" - if ($baseBranch) { - $encodedBase = [System.Uri]::EscapeDataString($baseBranch) - $query = "$query&base=$encodedBase" - } - - $searchUri = "https://api.github.com/repos/${{ github.repository }}/pulls?$query&per_page=30" - $candidates = Invoke-RestMethod -Uri $searchUri -Headers $headers -Method GET - - $matchingCandidates = @($candidates | Where-Object { $_.head.sha -eq $expectedHeadSha }) - if ($matchingCandidates.Count -eq 1) { - $prNumber = [string]$matchingCandidates[0].number - } - elseif ($matchingCandidates.Count -gt 1) { - throw 'Fallback PR lookup returned multiple PRs matching the workflow_run head SHA.' + $prNumber = $null + $resolvedHeadSha = $null + $resolvedBaseRef = $null + + # Primary source: metadata artifact produced by the intake workflow run. + try { + $artifactDir = Join-Path $env:RUNNER_TEMP 'copilot-pr-review-input' + $metadataFile = Get-ChildItem -Path $artifactDir -Filter 'pr-metadata.json' -Recurse -File | Select-Object -First 1 + if ($metadataFile) { + $metadata = Get-Content -Path $metadataFile.FullName -Raw | ConvertFrom-Json + if ($metadata.prNumber) { $prNumber = [string]$metadata.prNumber } + if ($metadata.headSha) { $resolvedHeadSha = [string]$metadata.headSha } + if ($metadata.baseRef) { $resolvedBaseRef = [string]$metadata.baseRef } + Write-Host "Resolved PR metadata from artifact file $($metadataFile.FullName)." } } + catch { + Write-Warning "Failed to resolve PR metadata from artifact: $($_.Exception.Message)" + } if (-not $prNumber) { - throw 'No pull request number found in workflow_run payload or fallback PR lookup.' + throw 'No pull request number found in artifact metadata.' } $uri = "https://api.github.com/repos/${{ github.repository }}/pulls/$prNumber" $pr = Invoke-RestMethod -Uri $uri -Headers $headers -Method GET - if ($expectedHeadSha -and $pr.head.sha -ne $expectedHeadSha) { + if (-not $resolvedHeadSha) { $resolvedHeadSha = [string]$pr.head.sha } + if (-not $resolvedBaseRef) { $resolvedBaseRef = [string]$pr.base.ref } + + if ($expectedHeadSha -and $resolvedHeadSha -ne $expectedHeadSha) { throw 'Resolved PR head SHA does not match workflow_run head SHA.' } - "number=$($pr.number)" >> $env:GITHUB_OUTPUT - "head_sha=$($pr.head.sha)" >> $env:GITHUB_OUTPUT - "base_ref=$($pr.base.ref)" >> $env:GITHUB_OUTPUT + "number=$([string]$pr.number)" >> $env:GITHUB_OUTPUT + "head_sha=$resolvedHeadSha" >> $env:GITHUB_OUTPUT + "base_ref=$resolvedBaseRef" >> $env:GITHUB_OUTPUT - name: Checkout trusted base code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 diff --git a/tools/Code Review/scripts/Invoke-CopilotPRReview.ps1 b/tools/Code Review/scripts/Invoke-CopilotPRReview.ps1 index ebc49bd888..b4d1512631 100644 --- a/tools/Code Review/scripts/Invoke-CopilotPRReview.ps1 +++ b/tools/Code Review/scripts/Invoke-CopilotPRReview.ps1 @@ -521,15 +521,29 @@ function Get-Findings { @($Output.Trim()) } + $candidateIndex = 0 foreach ($candidate in $candidates) { + $candidateIndex++ + if ([string]::IsNullOrWhiteSpace($candidate)) { continue } + $trimmedCandidate = $candidate.Trim() + try { - $parsed = $candidate | ConvertFrom-Json -ErrorAction Stop + $parsed = $trimmedCandidate | ConvertFrom-Json -ErrorAction Stop } catch { $message = $_.Exception.Message if ($message) { - $preview = $candidate.Trim() - if ($preview.Length -gt 160) { $preview = $preview.Substring(0, 160) + '...' } - $parseErrors.Add("$message | candidate: $preview") + $preview = $trimmedCandidate + if ($preview.Length -gt 1200) { $preview = $preview.Substring(0, 1200) + '...' } + $candidateHash = '' + try { + $sha256 = [System.Security.Cryptography.SHA256]::Create() + $bytes = [System.Text.Encoding]::UTF8.GetBytes($trimmedCandidate) + $candidateHash = ([System.BitConverter]::ToString($sha256.ComputeHash($bytes))).Replace('-', '').ToLowerInvariant() + $sha256.Dispose() + } catch { + $candidateHash = 'unavailable' + } + $parseErrors.Add("candidateIndex=$candidateIndex; hash=$candidateHash; length=$($trimmedCandidate.Length); parserError=$message; preview=$preview") } continue } @@ -978,6 +992,7 @@ function Save-ReviewArtifacts { $rawPath = Join-Path $ReviewOutputDir 'al-code-review-raw.txt' $jsonPath = Join-Path $ReviewOutputDir 'al-code-review-findings.json' + $parseErrorsPath = Join-Path $ReviewOutputDir 'al-code-review-parse-errors.txt' Set-Content -Path $rawPath -Value $RawOutput -Encoding UTF8 @@ -992,6 +1007,7 @@ function Save-ReviewArtifacts { parseErrors = @($ParseErrors) } Set-Content -Path $jsonPath -Value ($payload | ConvertTo-Json -Depth 12) -Encoding UTF8 + Set-Content -Path $parseErrorsPath -Value (@($ParseErrors) -join [Environment]::NewLine) -Encoding UTF8 Write-Host "Saved review artifacts to $ReviewOutputDir" } @@ -1043,7 +1059,7 @@ Save-ReviewArtifacts -RawOutput $output -Findings $findings -ParseErrors $script if ($FailOnParseError -and $findings.Count -eq 0 -and $script:LastParsingErrors.Count -gt 0) { $errorPreview = ($script:LastParsingErrors | Select-Object -First 3) -join ' || ' - throw "Copilot output JSON parsing failed; refusing to post an empty review summary. Set COPILOT_REVIEW_FAIL_ON_PARSE_ERROR=false to bypass. Parse errors: $errorPreview" + throw "Copilot output JSON parsing failed; refusing to post an empty review summary. Set COPILOT_REVIEW_FAIL_ON_PARSE_ERROR=false to bypass. Parse errors: $errorPreview. Full parse diagnostics: $ReviewOutputDir/al-code-review-parse-errors.txt" } # Group findings by resolved domain so comment headers and summary use domain labels.