diff --git a/.github/workflows/evaluation-fork-pr.yml b/.github/workflows/evaluation-fork-pr.yml index 133307992e..fff3216c7d 100644 --- a/.github/workflows/evaluation-fork-pr.yml +++ b/.github/workflows/evaluation-fork-pr.yml @@ -1,10 +1,9 @@ -# Evaluation workflow for fork PRs. +# Evaluation workflow for pull requests. # -# Fork PRs cannot access repository secrets via the `pull_request` trigger. -# This workflow uses `issue_comment` so that a maintainer can trigger evaluation -# by posting "/evaluate" on a fork PR. +# All PRs require an explicit `/evaluate` command to trigger evaluation. +# This avoids automatic evaluation runs on every push, reducing token consumption. # -# The `pull_request_target` trigger handles initial status posting for fork PRs: +# The `pull_request_target` trigger handles initial status posting for all PRs: # it runs discovery (from the base branch, safe) and posts a commit status so # the required "evaluation-status" check is never stuck as "Expected". # @@ -12,17 +11,18 @@ # - Workflow YAML: always from the base branch (enforced by issue_comment # and pull_request_target triggers) # - Validator binary: built from the base branch checkout (trusted) -# - Skill/test content: checked out from the fork PR (untrusted data, read-only) +# - Skill/test content: checked out from the PR head (untrusted for forks, read-only) # - Secret access: only users with write+ permission can trigger evaluation -name: evaluation (fork PRs) +name: evaluation (PRs) on: issue_comment: types: [created] pull_request_target: + types: [opened, synchronize, reopened] concurrency: - group: ${{ github.event_name == 'issue_comment' && (startsWith(github.event.comment.body, '/evaluate') && format('eval-fork-{0}', github.event.issue.number) || format('eval-fork-{0}-noop-{1}', github.event.issue.number, github.event.comment.id)) || format('eval-fork-status-{0}', github.event.pull_request.number) }} + group: ${{ github.event_name == 'issue_comment' && (startsWith(github.event.comment.body, '/evaluate') && format('eval-pr-{0}', github.event.issue.number) || format('eval-pr-{0}-noop-{1}', github.event.issue.number, github.event.comment.id)) || format('eval-pr-status-{0}', github.event.pull_request.number) }} cancel-in-progress: true # pull-requests: write is required at the workflow level because the reusable @@ -35,23 +35,24 @@ permissions: statuses: write jobs: - # Post initial evaluation commit status for fork PRs so the required check + # Post initial evaluation commit status for all PRs so the required check # is never stuck as "Expected". Posts success (no skills) or pending (needs /evaluate). # Uses pull_request_target so it runs with base repo permissions. - fork-status: + pr-status: if: >- - github.event_name == 'pull_request_target' && - github.event.pull_request.head.repo.full_name != github.repository + github.event_name == 'pull_request_target' runs-on: ubuntu-latest permissions: contents: read + pull-requests: write statuses: write steps: - name: Checkout base branch - uses: actions/checkout@v6 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 with: ref: ${{ github.event.pull_request.base.sha }} fetch-depth: 0 + persist-credentials: false - name: Fetch PR head (metadata only) run: git fetch origin +refs/pull/${{ github.event.pull_request.number }}/head @@ -66,7 +67,7 @@ jobs: $changedFiles = git diff --name-only --diff-filter=ACMR $mergeBase $head $hasSkillChanges = $changedFiles | - Where-Object { $_ -match '^(plugins/[^/]+/skills|tests/[^/]+)/[^/]+/' } | + Where-Object { $_ -match '^(plugins/[^/]+/skills|tests/[^/]+)/[^/]+/|^(\.github/workflows/evaluation(-run|-fork-pr)?\.yml$|eng/skill-validator/)' } | Select-Object -First 1 if ($hasSkillChanges) { @@ -81,7 +82,7 @@ jobs: run: | if [[ "${{ steps.discover.outputs.has_skills }}" == "true" ]]; then STATE="pending" - DESC="Fork PR — post /evaluate to trigger evaluation" + DESC="Post /evaluate to trigger evaluation" else STATE="success" DESC="No skills to evaluate" @@ -93,7 +94,16 @@ jobs: -f description="$DESC" \ -f target_url="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" - # Validate the trigger: must be a /evaluate comment on a fork PR from a user + - name: Post /evaluate reminder comment + if: github.event.action == 'opened' && steps.discover.outputs.has_skills == 'true' + env: + GH_TOKEN: ${{ github.token }} + run: | + gh pr comment "${{ github.event.pull_request.number }}" \ + --repo "${{ github.repository }}" \ + --body "This PR changes skills, tests, or evaluation infrastructure. A maintainer can post \`/evaluate\` to trigger evaluation." + + # Validate the trigger: must be a /evaluate comment on a PR from a user # with write+ permissions. gate: if: >- @@ -129,13 +139,7 @@ jobs: PR_NUMBER=${{ github.event.issue.number }} PR_DATA=$(gh api "repos/${{ github.repository }}/pulls/${PR_NUMBER}") HEAD_SHA=$(echo "$PR_DATA" | jq -r '.head.sha') - HEAD_REPO=$(echo "$PR_DATA" | jq -r '.head.repo.full_name') - BASE_REPO=$(echo "$PR_DATA" | jq -r '.base.repo.full_name') BASE_SHA=$(echo "$PR_DATA" | jq -r '.base.sha') - if [[ "$HEAD_REPO" == "$BASE_REPO" ]]; then - echo "::error::This command is only for fork PRs. Same-repo PRs are evaluated automatically." - exit 1 - fi echo "PR #${PR_NUMBER}: head=${HEAD_SHA} base=${BASE_SHA}" echo "head_sha=${HEAD_SHA}" >> $GITHUB_OUTPUT echo "base_sha=${BASE_SHA}" >> $GITHUB_OUTPUT @@ -184,33 +188,49 @@ jobs: $head = (git rev-parse origin/pr-head) $changedFiles = git diff --name-only --diff-filter=ACMR $base $head + # Check if any changed files are in infrastructure paths + $hasInfraChanges = $changedFiles | + Where-Object { $_ -match '^(\.github/workflows/evaluation(-run|-fork-pr)?\.yml$|eng/skill-validator/)' } | + Select-Object -First 1 + # We need the PR content to check for SKILL.md files — use a worktree git worktree add /tmp/pr-content origin/pr-head 2>$null - # Extract unique plugin/skill pairs from changed files - $changedPairs = @($changedFiles | - Where-Object { $_ -match '^(?:plugins/([^/]+)/skills|tests/([^/]+))/([^/]+)/' } | - ForEach-Object { - $p = if ($Matches[1]) { $Matches[1] } else { $Matches[2] } - "$p/$($Matches[3])" - } | - Sort-Object -Unique) + if ($hasInfraChanges) { + # Infra changes can affect any evaluation — evaluate all plugins + Write-Host "Infrastructure changes detected, evaluating all plugins" + $plugins = @(Get-ChildItem -Path "/tmp/pr-content/plugins" -Directory | + Where-Object { (Test-Path (Join-Path $_.FullName "skills")) -and (Test-Path (Join-Path "/tmp/pr-content/tests" $_.Name)) } | + Select-Object -ExpandProperty Name) + $entries = @($plugins | ForEach-Object { + @{ name = $_; plugin = $_; skills_path = "plugins/$_/skills" } + }) + } else { + # Extract unique plugin/skill pairs from changed files + $changedPairs = @($changedFiles | + Where-Object { $_ -match '^(?:plugins/([^/]+)/skills|tests/([^/]+))/([^/]+)/' } | + ForEach-Object { + $p = if ($Matches[1]) { $Matches[1] } else { $Matches[2] } + "$p/$($Matches[3])" + } | + Sort-Object -Unique) - # Filter to skills that have a SKILL.md and a tests directory (check in PR content) - $entries = @($changedPairs | ForEach-Object { - $parts = $_ -split '/' - $plugin = $parts[0] - $skill = $parts[1] - $skillMd = Join-Path "/tmp/pr-content" "plugins" $plugin "skills" $skill "SKILL.md" - $testsDir = Join-Path "/tmp/pr-content" "tests" $plugin - if ((Test-Path $skillMd) -and (Test-Path $testsDir)) { - @{ - name = "$plugin--$skill" - plugin = $plugin - skills_path = "plugins/$plugin/skills/$skill" + # Filter to skills that have a SKILL.md and a tests directory (check in PR content) + $entries = @($changedPairs | ForEach-Object { + $parts = $_ -split '/' + $plugin = $parts[0] + $skill = $parts[1] + $skillMd = Join-Path "/tmp/pr-content" "plugins" $plugin "skills" $skill "SKILL.md" + $testsDir = Join-Path "/tmp/pr-content" "tests" $plugin + if ((Test-Path $skillMd) -and (Test-Path $testsDir)) { + @{ + name = "$plugin--$skill" + plugin = $plugin + skills_path = "plugins/$plugin/skills/$skill" + } } - } - } | Where-Object { $_ }) + } | Where-Object { $_ }) + } git worktree remove /tmp/pr-content --force 2>$null diff --git a/.github/workflows/evaluation.yml b/.github/workflows/evaluation.yml index 8dc154e489..29347beaec 100644 --- a/.github/workflows/evaluation.yml +++ b/.github/workflows/evaluation.yml @@ -1,13 +1,11 @@ name: evaluation on: - pull_request: - schedule: - cron: '0 */3 * * *' concurrency: - group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.run_id }} + group: ${{ github.workflow }}-schedule cancel-in-progress: true env: @@ -15,22 +13,16 @@ env: permissions: contents: write - pull-requests: write jobs: discover: - # Skip fork PRs (handled by evaluation-fork-pr.yml) and scheduled runs - # outside the canonical repo. The schedule guard uses an explicit repo - # name (not just the fork flag) because evaluation requires - # COPILOT_GITHUB_TOKEN* secrets that only exist in dotnet/skills. - if: >- - (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && - (github.event_name != 'schedule' || github.repository == 'dotnet/skills') + # Only run scheduled evaluations in the canonical repo (requires + # COPILOT_GITHUB_TOKEN* secrets that only exist in dotnet/skills). + if: github.repository == 'dotnet/skills' runs-on: ubuntu-latest outputs: entries: ${{ steps.find.outputs.entries }} has_entries: ${{ steps.find.outputs.has_entries }} - is_infra: ${{ steps.find.outputs.is_infra }} plugins: ${{ steps.find.outputs.plugins }} has_plugins: ${{ steps.find.outputs.has_plugins }} steps: @@ -43,68 +35,13 @@ jobs: - name: Find skills to evaluate id: find run: | - $entries = @() - $plugins = @() - - if ("${{ github.event_name }}" -eq "pull_request") { - # PR: detect individual changed skills - $base = "${{ github.event.pull_request.base.sha }}" - $head = "${{ github.event.pull_request.head.sha }}" - $mergeBase = git merge-base $base $head - $changedFiles = git diff --name-only --diff-filter=ACMR $mergeBase $head - - # Check if any changed files are in infrastructure paths (evaluation workflows or eng/skill-validator/) - $hasInfraChanges = $changedFiles | - Where-Object { $_ -match '^(\.github/workflows/evaluation(-run|-fork-pr)?\.yml$|eng/skill-validator/)' } | - Select-Object -First 1 - - if ($hasInfraChanges) { - # Infra changes can affect any evaluation — evaluate all plugins - Write-Host "Infrastructure changes detected, evaluating all plugins" - echo "is_infra=true" >> $env:GITHUB_OUTPUT - $plugins = @(Get-ChildItem -Path "plugins" -Directory | - Where-Object { (Test-Path (Join-Path $_.FullName "skills")) -and (Test-Path (Join-Path "tests" $_.Name)) } | - Select-Object -ExpandProperty Name) - $entries = @($plugins | ForEach-Object { - @{ name = $_; plugin = $_; skills_path = "plugins/$_/skills" } - }) - } else { - # Extract unique plugin/skill pairs from changed files under plugins/*/skills/*/ or tests/*/*/ - $changedPairs = @($changedFiles | - Where-Object { $_ -match '^(?:plugins/([^/]+)/skills|tests/([^/]+))/([^/]+)/' } | - ForEach-Object { - $p = if ($Matches[1]) { $Matches[1] } else { $Matches[2] } - "$p/$($Matches[3])" - } | - Sort-Object -Unique) - - # Filter to skills that have a SKILL.md and a tests directory - $entries = @($changedPairs | ForEach-Object { - $parts = $_ -split '/' - $plugin = $parts[0] - $skill = $parts[1] - $skillMd = Join-Path "plugins" $plugin "skills" $skill "SKILL.md" - $testsDir = Join-Path "tests" $plugin - if ((Test-Path $skillMd) -and (Test-Path $testsDir)) { - @{ - name = "$plugin--$skill" - plugin = $plugin - skills_path = "plugins/$plugin/skills/$skill" - } - } - } | Where-Object { $_ }) - - $plugins = @($entries | ForEach-Object { $_.plugin } | Sort-Object -Unique) - } - } else { - # Schedule: evaluate all plugins with skills and tests - $plugins = @(Get-ChildItem -Path "plugins" -Directory | - Where-Object { (Test-Path (Join-Path $_.FullName "skills")) -and (Test-Path (Join-Path "tests" $_.Name)) } | - Select-Object -ExpandProperty Name) - $entries = @($plugins | ForEach-Object { - @{ name = $_; plugin = $_; skills_path = "plugins/$_/skills" } - }) - } + # Schedule: evaluate all plugins with skills and tests + $plugins = @(Get-ChildItem -Path "plugins" -Directory | + Where-Object { (Test-Path (Join-Path $_.FullName "skills")) -and (Test-Path (Join-Path "tests" $_.Name)) } | + Select-Object -ExpandProperty Name) + $entries = @($plugins | ForEach-Object { + @{ name = $_; plugin = $_; skills_path = "plugins/$_/skills" } + }) # Output entries for evaluate matrix if (-not $entries -or $entries.Count -eq 0) { @@ -135,13 +72,12 @@ jobs: uses: ./.github/workflows/evaluation-run.yml with: entries: ${{ needs.discover.outputs.entries }} - runs: ${{ needs.discover.outputs.is_infra == 'true' && '1' || (github.ref == 'refs/heads/main' && '5' || '3') }} - # Infra changes and scheduled runs evaluate all plugins in parallel — reduce - # per-job concurrency to avoid API rate limits and timeouts from contention. - parallel-skills: ${{ (needs.discover.outputs.is_infra == 'true' || github.event_name == 'schedule') && '2' || '5' }} - parallel-scenarios: ${{ (needs.discover.outputs.is_infra == 'true' || github.event_name == 'schedule') && '3' || '5' }} - parallel-runs: ${{ (needs.discover.outputs.is_infra == 'true' || github.event_name == 'schedule') && '3' || '5' }} - pr-number: ${{ github.event.pull_request.number || '' }} + runs: '5' + # Scheduled runs evaluate all plugins in parallel — reduce per-job + # concurrency to avoid API rate limits and timeouts from contention. + parallel-skills: '2' + parallel-scenarios: '3' + parallel-runs: '3' secrets: COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN }} COPILOT_GITHUB_TOKEN_2: ${{ secrets.COPILOT_GITHUB_TOKEN_2 }} @@ -152,33 +88,6 @@ jobs: COPILOT_GITHUB_TOKEN_7: ${{ secrets.COPILOT_GITHUB_TOKEN_7 }} COPILOT_GITHUB_TOKEN_8: ${{ secrets.COPILOT_GITHUB_TOKEN_8 }} - # Plain check-run job that can be required in the GitHub ruleset. - # For fork PRs, discover is skipped (fork guard) so this job is also skipped; - # the fork-status job in evaluation-fork-pr.yml posts a commit status instead. - evaluation-status: - needs: [discover, run-evaluation] - if: always() && github.event_name == 'pull_request' && needs.discover.result != 'skipped' - runs-on: ubuntu-latest - permissions: {} - steps: - - name: Check evaluation result - run: | - DISCOVER_RESULT="${{ needs.discover.result }}" - HAS_ENTRIES="${{ needs.discover.outputs.has_entries }}" - EVAL_RESULT="${{ needs.run-evaluation.result }}" - - if [[ "$DISCOVER_RESULT" != "success" ]]; then - echo "::error::Discovery failed ($DISCOVER_RESULT)" - exit 1 - elif [[ "$HAS_ENTRIES" != "true" ]]; then - echo "No skills to evaluate" - elif [[ "$EVAL_RESULT" == "success" ]]; then - echo "Evaluation passed" - else - echo "::error::Evaluation did not pass ($EVAL_RESULT)" - exit 1 - fi - publish-benchmark: needs: [discover, run-evaluation] if: github.ref == 'refs/heads/main' && needs.discover.outputs.has_plugins == 'true'