diff --git a/.github/workflows/agent-mention-noema-dispatch.yml b/.github/workflows/agent-mention-noema-dispatch.yml index 5bed3e8963..ad8abc7b25 100644 --- a/.github/workflows/agent-mention-noema-dispatch.yml +++ b/.github/workflows/agent-mention-noema-dispatch.yml @@ -8,15 +8,26 @@ on: repository_dispatch: types: [agent-mention-noema] +concurrency: + # Workflow-level admission, for the same reason strix.yml, noema-review.yml, + # opencode-review.yml and opencode-review-dispatch.yml carry theirs at this level: + # a job-level group is never evaluated while the whole run waits behind the + # organization job ceiling, so a superseded mention keeps its queue slot until a + # runner frees up and only then cancels. At workflow level the older run is + # coalesced while both are still queued, which is where the slot is actually held. + # This workflow has a single job, so the group lives here and nowhere else -- + # every workflow in this repository that carries a group at both levels + # (strix.yml, opencode-review-dispatch.yml) gives the two levels DIFFERENT names, + # because a job requesting the group its own run already holds would wait on itself. + group: agent-mention-noema-${{ github.event.client_payload.target_repository }}-${{ github.event.client_payload.pr_number || github.run_id }} + cancel-in-progress: true + permissions: contents: read jobs: validate-and-forward: if: github.repository == 'ContextualWisdomLab/.github' - concurrency: - group: agent-mention-noema-${{ github.event.client_payload.target_repository }}-${{ github.event.client_payload.pr_number || github.run_id }} - cancel-in-progress: true runs-on: ubuntu-24.04 timeout-minutes: 5 permissions: diff --git a/.github/workflows/agent-mention-opencode-dispatch.yml b/.github/workflows/agent-mention-opencode-dispatch.yml index b27062ae37..05461c9551 100644 --- a/.github/workflows/agent-mention-opencode-dispatch.yml +++ b/.github/workflows/agent-mention-opencode-dispatch.yml @@ -8,15 +8,26 @@ on: repository_dispatch: types: [agent-mention-opencode] +concurrency: + # Workflow-level admission, for the same reason strix.yml, noema-review.yml, + # opencode-review.yml and opencode-review-dispatch.yml carry theirs at this level: + # a job-level group is never evaluated while the whole run waits behind the + # organization job ceiling, so a superseded mention keeps its queue slot until a + # runner frees up and only then cancels. At workflow level the older run is + # coalesced while both are still queued, which is where the slot is actually held. + # This workflow has a single job, so the group lives here and nowhere else -- + # every workflow in this repository that carries a group at both levels + # (strix.yml, opencode-review-dispatch.yml) gives the two levels DIFFERENT names, + # because a job requesting the group its own run already holds would wait on itself. + group: agent-mention-opencode-${{ github.event.client_payload.target_repository }}-${{ github.event.client_payload.pr_number || github.run_id }} + cancel-in-progress: true + permissions: contents: read jobs: validate-and-forward: if: github.repository == 'ContextualWisdomLab/.github' - concurrency: - group: agent-mention-opencode-${{ github.event.client_payload.target_repository }}-${{ github.event.client_payload.pr_number || github.run_id }} - cancel-in-progress: true runs-on: ubuntu-24.04 timeout-minutes: 5 permissions: diff --git a/.github/workflows/audit-central-ruleset.yml b/.github/workflows/audit-central-ruleset.yml index bf24e36c7c..2b72f21aab 100644 --- a/.github/workflows/audit-central-ruleset.yml +++ b/.github/workflows/audit-central-ruleset.yml @@ -105,6 +105,18 @@ jobs: python3 scripts/ci/audit_central_required_workflows.py --stacked "$stacked_ruleset_json" - name: Audit organization CodeQL coverage + # Runs even when the ruleset step above failed. Those two audits share a + # job but not a subject: the ruleset step exits 1 on owner-configured + # governance drift, and on 2026-09-06 it did exactly that ("exactly two + # approving reviews are not required", "last-push approval protection is + # disabled"), which silently took this CodeQL coverage detector down with + # it -- every run since 2026-09-04 failed there and never reached this + # step. This step builds its own repository list into its own temp file + # and the step above exports nothing to GITHUB_ENV or GITHUB_OUTPUT, so + # it has no data dependency to lose. The job still fails overall; what + # changes is that a coverage gap is reported instead of hidden behind an + # unrelated failure. + if: always() env: ORG_LOGIN: ContextualWisdomLab ORG_WIDE_CREDENTIAL_AVAILABLE: ${{ secrets.PR_REVIEW_MERGE_TOKEN != '' || secrets.OPENCODE_APPROVE_TOKEN != '' }} @@ -165,13 +177,21 @@ jobs: printf '[]\n' >"$coverage_json" while IFS=$'\t' read -r repository archived; do default_setup_state=null + # `state` alone is not coverage: a repository can report + # "configured" with an empty `languages` list, which scans nothing + # and produces no analyses (measured 2026-09-07 on life-os, aFIPC + # and inkspan). Collect both fields so the audit can tell those + # apart from a setup that actually covers a language. + default_setup_languages=null if [ "$archived" != "true" ]; then - default_setup_state_json="$RUNNER_TEMP/codeql-default-setup-${repository//[^A-Za-z0-9_.-]/_}.json" - if gh api "repos/${ORG_LOGIN}/${repository}/code-scanning/default-setup" --jq .state \ - >"$default_setup_state_json" 2>/dev/null; then - default_setup_state=$(jq -R '.' "$default_setup_state_json") + default_setup_json="$RUNNER_TEMP/codeql-default-setup-${repository//[^A-Za-z0-9_.-]/_}.json" + if gh api "repos/${ORG_LOGIN}/${repository}/code-scanning/default-setup" \ + >"$default_setup_json" 2>/dev/null; then + default_setup_state=$(jq '.state // null' "$default_setup_json") + default_setup_languages=$(jq '.languages // []' "$default_setup_json") else default_setup_state=null + default_setup_languages=null fi fi @@ -187,12 +207,13 @@ jobs: fi fi - echo "CODEQL_COVERAGE repository=${repository} archived=${archived} default_setup_state=${default_setup_state} latest_codeql_analysis=${latest_codeql_analysis}" + echo "CODEQL_COVERAGE repository=${repository} archived=${archived} default_setup_state=${default_setup_state} default_setup_languages=${default_setup_languages} latest_codeql_analysis=${latest_codeql_analysis}" jq --arg name "$repository" \ --argjson archived "$archived" \ --argjson default_setup_state "$default_setup_state" \ + --argjson default_setup_languages "$default_setup_languages" \ --argjson latest_codeql_analysis "$latest_codeql_analysis" \ - '. + [{name: $name, archived: $archived, default_setup_state: $default_setup_state, latest_codeql_analysis: $latest_codeql_analysis}]' \ + '. + [{name: $name, archived: $archived, default_setup_state: $default_setup_state, default_setup_languages: $default_setup_languages, latest_codeql_analysis: $latest_codeql_analysis}]' \ "$coverage_json" >"${coverage_json}.next" mv "${coverage_json}.next" "$coverage_json" done < <(jq -r '.[] | [.name, (.archived | tostring)] | @tsv' "$repositories_json") diff --git a/.github/workflows/codeql-pr.yml b/.github/workflows/codeql-pr.yml index cb07ad2fab..182eac1989 100644 --- a/.github/workflows/codeql-pr.yml +++ b/.github/workflows/codeql-pr.yml @@ -115,7 +115,9 @@ jobs: break fi changed="" - sleep $((attempt * 3)) + if [ "$attempt" -lt 3 ]; then + sleep $((attempt * 3)) + fi done # GitHub caps /pulls/N/files at 3000 entries; a short list would hide # source files behind a doc-only verdict, so require an exact count. diff --git a/.github/workflows/sast-semgrep.yml b/.github/workflows/sast-semgrep.yml index 12b7013da3..273d8f84f6 100644 --- a/.github/workflows/sast-semgrep.yml +++ b/.github/workflows/sast-semgrep.yml @@ -76,7 +76,9 @@ jobs: break fi changed="" - sleep $((attempt * 3)) + if [ "$attempt" -lt 3 ]; then + sleep $((attempt * 3)) + fi done # GitHub caps /pulls/N/files at 3000 entries; a short list would hide # source files behind a doc-only verdict, so require an exact count. diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 500e22b4ab..6703801ec1 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -99,7 +99,9 @@ jobs: break fi changed="" - sleep $((attempt * 3)) + if [ "$attempt" -lt 3 ]; then + sleep $((attempt * 3)) + fi done # GitHub caps /pulls/N/files at 3000 entries; a short list would hide # source files behind a doc-only verdict, so require an exact count. diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index 58ed3dab8d..efa2f7f1fc 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -44,12 +44,9 @@ on: # them, so the same doc/image-only decision is enforced by the # changed-scope job below. The run-name # includes the PR number and head SHA for status grouping, while the - # concurrency group is scoped per repository and event class to prevent - # shared-provider key rate-limit storms. Strix runs intentionally do not - # cancel in progress because a pre-job cancellation leaves no scanner log to - # review. GitHub keeps one active and one pending run per group; the merge - # scheduler re-dispatches exact-head evidence when a pending run is - # superseded. For PRs the merge scheduler manages, same-head Strix evidence + # concurrency group is scoped per workflow, repository, and PR. New runs + # supersede older runs only within that group; other PRs remain independent. + # For PRs the merge scheduler manages, same-head Strix evidence # is still forced at merge time via repository_dispatch (which paths-ignore # does not affect), so merged code never loses evidence. paths-ignore: @@ -94,7 +91,14 @@ permissions: jobs: changed-scope: - name: Detect changed scope + # Deliberately keeps the `changed-scope` job id that CLAUDE.md names as the + # required-workflow skip pattern, but this workflow's copy also carries the + # current-head admission that used to live in a separate + # `admit-current-head` job. The display name says so because the check list + # is where the ambiguity bites: sast-semgrep.yml and security-scan.yml both + # publish a job displayed as "Detect changed scope" that does NOT admit, and + # telling them apart from a check list alone cost real time on 2026-09-06. + name: Detect changed scope and admit the current head # The org ruleset IGNORES every `on:` filter (paths, branches, types) when it # runs this workflow in another repository, and a trigger-level skip would # leave `.github`'s classic required contexts Pending forever. Both @@ -104,74 +108,20 @@ jobs: # Fails OPEN: an unreadable, empty, or truncated file list scans everything. if: github.event_name != 'pull_request_target' || (github.event.action != 'closed' && github.event.action != 'converted_to_draft') runs-on: ubuntu-24.04 - timeout-minutes: 5 + timeout-minutes: 10 permissions: contents: read pull-requests: read outputs: code: ${{ steps.scope.outputs.code }} deps: ${{ steps.scope.outputs.deps }} - steps: - - name: Classify changed paths - id: scope - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.event.pull_request.base.repo.full_name || github.repository }} - PR: ${{ github.event.pull_request.number }} - EXPECTED_FILES: ${{ github.event.pull_request.changed_files }} - shell: bash - run: | - set -uo pipefail - code=true - deps=true - if [ -n "${PR}" ] && [ -n "${EXPECTED_FILES}" ]; then - changed="" - for attempt in 1 2 3; do - if changed="$(gh api --paginate "repos/${REPO}/pulls/${PR}/files?per_page=100" --jq '.[].filename')" && [ -n "$changed" ]; then - break - fi - changed="" - sleep $((attempt * 3)) - done - # GitHub caps /pulls/N/files at 3000 entries; a short list would hide - # source files behind a doc-only verdict, so require an exact count. - if [ -n "$changed" ] && [ "$(printf '%s\n' "$changed" | wc -l | tr -d ' ')" = "${EXPECTED_FILES}" ]; then - code=false - deps=false - while IFS= read -r changed_path; do - case "$changed_path" in - *.md|*.markdown|*.rst|*.png|*.jpg|*.jpeg|*.gif|*.webp|*.bmp|*.ico|LICENSE|LICENSE.txt|COPYING|COPYING.txt|NOTICE|NOTICE.txt|.github/ISSUE_TEMPLATE/*) ;; - *) code=true ;; - esac - case "$changed_path" in - requirements*.txt|*/requirements*.txt|pyproject.toml|*/pyproject.toml|uv.lock|*/uv.lock|pylock.*.toml|*/pylock.*.toml|package.json|*/package.json|package-lock.json|*/package-lock.json|pnpm-lock.yaml|*/pnpm-lock.yaml|yarn.lock|*/yarn.lock|Cargo.toml|*/Cargo.toml|Cargo.lock|*/Cargo.lock|go.mod|*/go.mod|go.sum|*/go.sum|pom.xml|*/pom.xml|build.gradle|*/build.gradle|build.gradle.kts|*/build.gradle.kts|DESCRIPTION|*/DESCRIPTION) deps=true ;; - esac - done <<<"$changed" - else - echo "::notice::changed-scope could not read a complete PR file list; scanning everything." - fi - fi - echo "code=${code}" >> "$GITHUB_OUTPUT" - echo "deps=${deps}" >> "$GITHUB_OUTPUT" - echo "changed-scope code=${code} deps=${deps}" - - admit-current-head: - name: Admit current pull request head - if: >- - github.event_name != 'pull_request_target' || - (github.event.action != 'closed' && github.event.action != 'converted_to_draft') - runs-on: ubuntu-24.04 - timeout-minutes: 5 - permissions: - contents: read - pull-requests: read - outputs: admitted: ${{ steps.admission.outputs.admitted }} target_repository: ${{ steps.admission.outputs.target_repository }} pr_number: ${{ steps.admission.outputs.pr_number }} steps: - name: Verify event metadata against the live pull request id: admission + timeout-minutes: 5 env: GH_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || github.token }} EVENT_NAME: ${{ github.event_name }} @@ -201,6 +151,11 @@ jobs: exit 1 fi pull_request_json="$(gh api "repos/${TARGET_REPOSITORY}/pulls/${TARGET_PR_NUMBER}")" + live_draft="$(jq -er '.draft | if type == "boolean" then tostring else error("Strix PR draft state is unavailable") end' <<<"$pull_request_json")" + if [ "$live_draft" = "true" ]; then + echo "::notice::Strix model work is unnecessary while the pull request is Draft." + exit 0 + fi live_tuple="$(jq -r '[.state // "", .base.repo.full_name // "", .base.ref // "", .base.sha // "", .head.repo.full_name // "", .head.sha // ""] | @tsv' <<<"$pull_request_json")" expected_tuple="$(printf 'open\t%s\t%s\t%s\t%s\t%s' "$TARGET_REPOSITORY" "$EXPECTED_BASE_REF" "$EXPECTED_BASE_SHA" "$EXPECTED_HEAD_REPOSITORY" "$EXPECTED_HEAD_SHA")" if [ "$live_tuple" != "$expected_tuple" ]; then @@ -213,6 +168,54 @@ jobs: echo "pr_number=${TARGET_PR_NUMBER}" } >> "$GITHUB_OUTPUT" + + - name: Classify changed paths + id: scope + if: steps.admission.outputs.admitted == 'true' + timeout-minutes: 5 + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.event.pull_request.base.repo.full_name || github.repository }} + PR: ${{ github.event.pull_request.number }} + EXPECTED_FILES: ${{ github.event.pull_request.changed_files }} + shell: bash + run: | + set -uo pipefail + code=true + deps=true + if [ -n "${PR}" ] && [ -n "${EXPECTED_FILES}" ]; then + changed="" + for attempt in 1 2 3; do + if changed="$(gh api --paginate "repos/${REPO}/pulls/${PR}/files?per_page=100" --jq '.[].filename')" && [ -n "$changed" ]; then + break + fi + changed="" + if [ "$attempt" -lt 3 ]; then + sleep $((attempt * 3)) + fi + done + # GitHub caps /pulls/N/files at 3000 entries; a short list would hide + # source files behind a doc-only verdict, so require an exact count. + if [ -n "$changed" ] && [ "$(printf '%s\n' "$changed" | wc -l | tr -d ' ')" = "${EXPECTED_FILES}" ]; then + code=false + deps=false + while IFS= read -r changed_path; do + case "$changed_path" in + *.md|*.markdown|*.rst|*.png|*.jpg|*.jpeg|*.gif|*.webp|*.bmp|*.ico|LICENSE|LICENSE.txt|COPYING|COPYING.txt|NOTICE|NOTICE.txt|.github/ISSUE_TEMPLATE/*) ;; + *) code=true ;; + esac + case "$changed_path" in + requirements*.txt|*/requirements*.txt|pyproject.toml|*/pyproject.toml|uv.lock|*/uv.lock|pylock.*.toml|*/pylock.*.toml|package.json|*/package.json|package-lock.json|*/package-lock.json|pnpm-lock.yaml|*/pnpm-lock.yaml|yarn.lock|*/yarn.lock|Cargo.toml|*/Cargo.toml|Cargo.lock|*/Cargo.lock|go.mod|*/go.mod|go.sum|*/go.sum|pom.xml|*/pom.xml|build.gradle|*/build.gradle|build.gradle.kts|*/build.gradle.kts|DESCRIPTION|*/DESCRIPTION) deps=true ;; + esac + done <<<"$changed" + else + echo "::notice::changed-scope could not read a complete PR file list; scanning everything." + fi + fi + echo "code=${code}" >> "$GITHUB_OUTPUT" + echo "deps=${deps}" >> "$GITHUB_OUTPUT" + echo "changed-scope code=${code} deps=${deps}" + cancel-superseded-pr-runs: if: >- github.event_name == 'pull_request_target' && @@ -340,8 +343,8 @@ jobs: done strix: - needs: [changed-scope, admit-current-head] - if: needs.changed-scope.outputs.code == 'true' && needs.admit-current-head.outputs.admitted == 'true' + needs: [changed-scope] + if: needs.changed-scope.outputs.code == 'true' && needs.changed-scope.outputs.admitted == 'true' # Large, actively-growing repositories (e.g. contextual-orchestrator) can # legitimately require well over two hours to scan -- this org's own # standing operating directive accepts that central OpenCode/Strix/Noema diff --git a/AGENTS.md b/AGENTS.md index e955f8b36a..8e5301c8a5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -171,6 +171,12 @@ them alone proves succession. ## Test-gate regressions and stale-PR merges +- Changed-file API retries must sleep only before another attempt, never after + the final failure. Preserve complete-list validation and full scanning on + unreadable or incomplete lists. Run the actual workflow shell with fake API + and sleep commands; assert attempt counts, waits, and scan outputs for each + success attempt and exhaustion. Keep the shared classifier bodies aligned in + `tests/test_docs_only_pr_runner_admission.py`. - A red `tests`, coverage, or `interrogate` gate on your pull request is not proof that your diff caused it. Full-suite execution on a push to `main` is not guaranteed: the workflows that run `pytest tests` on push are `paths:`-filtered, so a pairing broken outside their diff --git a/CHANGELOG.md b/CHANGELOG.md index bf192f6a9e..debce108bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,8 @@ - Raised `hourly-review-repair.yml`'s discovery ceiling from 50 to 200 while rotating deterministic 50-PR deep-inspection windows by hourly run number. The scheduler hydrates only the selected window and stops immediately after its single dispatch, preserving access to newer PRs without quadrupling expensive review/check/comment work. See `docs/doctoring/hourly-review-repair-single-file-consolidation.md`'s 2026-09-03 follow-up. ## [Unreleased] +- Consolidate Strix live-head admission and changed-scope classification into + one bounded read-only metadata job before the security scan. - Include merge-scheduler entrypoint, core, and regression-test changes in the existing runtime-quality workflow's trigger and suite selector. Scheduler workflow edits retain queue checks and also select the full review-repair diff --git a/docs/doctoring/strix-metadata-admission-consolidation.md b/docs/doctoring/strix-metadata-admission-consolidation.md new file mode 100644 index 0000000000..515d3f247f --- /dev/null +++ b/docs/doctoring/strix-metadata-admission-consolidation.md @@ -0,0 +1,102 @@ +# Strix metadata admission consolidation + +Status: **Proposed and locally verified**, based on central `.github` commit +`5ea1cc47ec040fa4f6417136f059be637666c2a2`. This is not hosted-run evidence. + +## Change and metric + +The required Strix workflow previously allocated two read-only metadata jobs: +`changed-scope` and `admit-current-head`. A valid code-changing pull request then +allocated three jobs before completion: those two metadata jobs plus `strix`. +This change moves the byte-identical admission shell ahead of the byte-identical +path classifier in `changed-scope`, publishes `code`, `deps`, `admitted`, +`target_repository`, and `pr_number` from that one job, and makes `strix` depend +only on `changed-scope` while requiring both `code == 'true'` and +`admitted == 'true'`. + +The verified source-structure metric is therefore metadata jobs **2 -> 1**. +For `opened`, `reopened`, and `ready_for_review` code-changing PR runs, where +cleanup does not run, total jobs fall **3 -> 2**. A `synchronize` run includes +cleanup and falls **4 -> 3**; an exact dispatch includes the status publisher +and also falls **4 -> 3**. These counts do not claim a hosted queue-time +improvement. A +current exact PR is admitted before its file list is classified; a stale PR +ends successfully with `admitted=false` and never runs the classifier. Direct +push and schedule events make no PR API call and retain fail-open `code=true`. +Exact dispatch validates the live PR but has no native PR changed-file fields, +so the classifier makes no files API call and retains `code=true`. Malformed or +unreadable admission fails the job; empty, unreadable, or count-mismatched file +lists retain the existing fail-open full-scan result. + +GitHub documents that a failed or skipped dependency normally propagates to +dependent jobs, which is why admission and classification remain in the same +successful metadata job and the scan reads both outputs. GitHub also requires +step `timeout-minutes` to be a positive number. The metadata job is bounded at +10 minutes and each of its two steps at 5 minutes. See [workflow syntax for +`jobs..needs`](https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#jobsjob_idneeds) +and [step `timeout-minutes`](https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#jobsjob_idstepstimeout-minutes). + +## Permission and latency boundary + +Both former jobs had only `contents: read` and `pull-requests: read`; the merged +job keeps exactly that job permission set. The stronger admission credential is +still scoped to the admission step's environment and the classifier receives +only `github.token`. The `actions: write` cleanup job and the scan job's +`id-token: write`/`statuses: write` permissions remain separate. Moazen, +Ahmadian, and Balliu's *Granite: Granular Runtime Enforcement for GitHub Actions +Permissions* explains the general risk of steps sharing job permissions; it +supports keeping unlike privileged work in separate jobs, but it does **not** +establish this change's queue metric. The redistributed v1 PDF is preserved +unchanged at [`docs/papers/granite-granular-runtime-enforcement-github-actions-permissions-v1.pdf`](../papers/granite-granular-runtime-enforcement-github-actions-permissions-v1.pdf), +SHA-256 `5d1dd7b26176de6d5347885867156759928641c8e7415a09869cbfb00cbeb07d`, +under [CC BY 4.0](https://creativecommons.org/licenses/by/4.0/). Citation: +Moazen, M., Ahmadian, A. M., & Balliu, M. (2025). *Granite: Granular Runtime +Enforcement for GitHub Actions Permissions* (v1). arXiv. +https://doi.org/10.48550/arXiv.2512.11602. + +The tradeoff is serial latency: admission must finish before classification, +where the old metadata jobs could run in parallel. That delay is bounded by the +5-minute admission step and prevents stale events from spending a second runner. +The original consolidation did not change model, scan, cleanup, or concurrency +behavior. The following admission repair is a separate follow-up. + +## Live Draft admission repair (2026-09-07) + +GitHub timeline evidence shows PR #1706 converted to Draft at +2026-09-06T22:59:11Z before automatic Strix run `34068478185` started at +2026-09-07T00:01:03Z. PR #1150 converted at 23:46:51Z before run +`34067942252` started at 23:49:09Z. Both were first-attempt +`pull_request_target` runs with current-head identities, not manual scans. +The live admission tuple checked state/base/head but omitted Draft status. + +Reuse the existing PR response: require a boolean `draft`, stop before path +classification and scanner admission when true, and fail closed on unavailable +or malformed values. A live false value follows the existing exact-head path, +including `ready_for_review`. No extra API call, model timer, permission change, +or automatic cancellation is introduced. This prevents future Draft admission; +it does not prove that existing jobs have stopped or that organization-wide +capacity has improved. + +Six executable shell cases reproduced the defect on owner head +`402aca1392c829f28ac975d27b68c49ad0495c24`: native PR and repository dispatch +each incorrectly admitted true, null, and string-false values. After the guard, +93 admission/queue tests passed in 20.70 seconds; actionlint and diff checks +passed. Existing false-Draft, stale-head, malformed-event, API-failure and +non-PR cases remain covered. These are local source results, not hosted checks +or protected integration. + +## Evidence boundary and follow-up + +Local regression shells and actionlint verify source wiring and syntax only; +they do not prove hosted output propagation, required-check publication, or +queue latency. Live reads found no separate admission context among the 12 +required contexts on `.github` `main` or the 17 required contexts on Naruon +`develop`; organization ruleset +`18156473` could not be read without `admin:org`, so no organization-wide claim +is made. Project linkage is also unverified in this turn because the project was +unavailable and the Mac UI was locked. + +The pre-existing workflow-header statement that Strix does not cancel in +progress conflicts with the current `cancel-in-progress: true` expression, but +that text belongs to the independent #1938 concurrency hunk and is intentionally +not mixed into this repair. Track it as a follow-up documentation correction. diff --git a/docs/papers/granite-granular-runtime-enforcement-github-actions-permissions-v1.pdf b/docs/papers/granite-granular-runtime-enforcement-github-actions-permissions-v1.pdf new file mode 100644 index 0000000000..27e22e7f1c Binary files /dev/null and b/docs/papers/granite-granular-runtime-enforcement-github-actions-permissions-v1.pdf differ diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 1cc9e20313..fe29c7ffa3 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -3353,3 +3353,17 @@ queries the check-runs API at its own time, order-independently. The implementin their change was safe because they had scoped it narrowly, not because they had checked for the name collision — which is the more useful lesson: **a job name is unique only within one workflow file, and the same name in another file can carry the opposite safety property.** + +### Proposed: consolidate Strix read-only metadata admission + +- **G-03 / objective 16:** the Strix required workflow can preserve its scanner, + cleanup, and fail-closed boundaries while folding the separate live-head + admission runner into the changed-scope runner. The source-verified structural + metric is metadata jobs 2 -> 1; total jobs are 3 -> 2 for code-changing + opened/reopened/ready-for-review PR runs, and 4 -> 3 for synchronize or exact + dispatch runs. No old queued-run snapshot is reused as live-effect evidence. +- **G-04 / objective 17:** this is one bounded workflow consolidation, not an + organization-wide completion claim. Hosted queue effect and Project linkage + remain unverified. Design, permission, latency, test, and evidence boundaries + are recorded in + [`docs/doctoring/strix-metadata-admission-consolidation.md`](doctoring/strix-metadata-admission-consolidation.md). diff --git a/scripts/ci/audit_org_codeql_coverage.py b/scripts/ci/audit_org_codeql_coverage.py index f9fb2eaf17..bdbc835491 100644 --- a/scripts/ci/audit_org_codeql_coverage.py +++ b/scripts/ci/audit_org_codeql_coverage.py @@ -66,6 +66,43 @@ def _is_analysis_fresh_and_successful( return parsed >= now - timedelta(days=CODEQL_ANALYSIS_FRESHNESS_DAYS) +def _default_setup_scans_a_language(repository: dict[str, Any]) -> bool: + """Return True when default-setup is configured AND has languages enabled. + + ``state == "configured"`` alone is not coverage. Measured 2026-09-07: + ``life-os``, ``aFIPC`` and ``inkspan`` all report ``configured`` with an + **empty** ``languages`` list and no ``schedule``; ``life-os`` has zero CodeQL + analyses of any language as a result, while still satisfying the + configured-state check this function replaces. A default setup with nothing + enabled is a commitment to scan nothing. + + A missing ``default_setup_languages`` key fails closed rather than falling + back to the state alone, which would silently restore that gap. The audit + workflow collects the field in the same change that introduced this check, + so the key is absent only when the payload predates them both. + """ + if repository.get("default_setup_state") != "configured": + return False + languages = repository.get("default_setup_languages") + return isinstance(languages, list) and bool(languages) + + +def auditable_repositories( + repositories: list[dict[str, Any]], +) -> list[dict[str, Any]]: + """Return the repositories this audit actually examines. + + Archived repositories are excluded: they cannot run workflows or code + scanning, so a lack of coverage there is not a product gap. Counting them + as examined is what let ``main`` report success over an empty subject set. + """ + return [ + repository + for repository in repositories + if not repository.get("archived") + ] + + def repositories_without_codeql( repositories: list[dict[str, Any]], now: datetime | None = None ) -> list[dict[str, Any]]: @@ -81,15 +118,15 @@ def repositories_without_codeql( """ current = now or datetime.now(timezone.utc) uncovered: list[dict[str, Any]] = [] - for repository in repositories: - if repository.get("archived"): - continue + for repository in auditable_repositories(repositories): # "configured" is GitHub's own forward-looking commitment to run # CodeQL going forward (like a scheduled cron guarantee), not a # one-time historical scan that can go stale -- so it does not need # the same freshness check as latest_codeql_analysis below. Do not - # "fix" this into requiring a completed scan. - has_default_setup = repository.get("default_setup_state") == "configured" + # "fix" this into requiring a completed scan. It does need the + # commitment to cover at least one language: see + # _default_setup_scans_a_language. + has_default_setup = _default_setup_scans_a_language(repository) has_fresh_analysis = _is_analysis_fresh_and_successful( repository.get("latest_codeql_analysis"), current ) @@ -98,13 +135,30 @@ def repositories_without_codeql( return uncovered +def _coverage_gap_reason(repository: dict[str, Any]) -> str: + """Return the gap description that tells the operator what to change. + + "Default setup is on but scans nothing" and "there is no coverage at all" + need different fixes -- enable languages on the existing setup, versus set + coverage up -- so they are reported as different sentences. + """ + if repository.get("default_setup_state") == "configured": + return ( + f"{repository.get('name')} has CodeQL default-setup configured with no " + "languages enabled, so it scans nothing and produces no analyses" + ) + return ( + f"{repository.get('name')} has no CodeQL coverage from any source " + "(no default-setup, no recent analysis)" + ) + + def audit_codeql_coverage( repositories: list[dict[str, Any]], now: datetime | None = None ) -> list[str]: """Return one human-readable error per repository with zero CodeQL coverage.""" return [ - f"{repository.get('name')} has no CodeQL coverage from any source " - "(no default-setup, no recent analysis)" + _coverage_gap_reason(repository) for repository in repositories_without_codeql(repositories, now) ] @@ -137,6 +191,24 @@ def main(argv: list[str] | None = None) -> int: print(f"ERROR: unable to load repository JSON: {exc}", file=sys.stderr) return 2 + audited = auditable_repositories(repositories) + if not audited: + # An audit that examined nothing is not a clean organization, and + # "PASS: all 0 repositories have real CodeQL coverage" reads as + # success. The count that matters is what was examined, not what was + # supplied: an empty payload and a payload of nothing but archived + # repositories both reach zero subjects, and only the first was caught + # when this guard counted `repositories`. The calling workflow refuses + # an enumeration missing its known-private sentinel repositories, but + # the script is directly runnable against a JSON path or stdin, so the + # guard has to live here too. + print( + f"ERROR: this run audited nothing " + f"(0 of {len(repositories)} repositories were eligible)", + file=sys.stderr, + ) + return 2 + errors = audit_codeql_coverage(repositories) if errors: for error in errors: @@ -147,7 +219,7 @@ def main(argv: list[str] | None = None) -> int: ) return 1 - print(f"PASS: all {len(repositories)} repositories have real CodeQL coverage") + print(f"PASS: all {len(audited)} repositories have real CodeQL coverage") return 0 diff --git a/scripts/ci/pr_review_merge_scheduler_core.py b/scripts/ci/pr_review_merge_scheduler_core.py index c4e9d28ebd..4df4dac3de 100644 --- a/scripts/ci/pr_review_merge_scheduler_core.py +++ b/scripts/ci/pr_review_merge_scheduler_core.py @@ -3252,7 +3252,24 @@ def active_review_run_refs( for run_repo in (dispatch_repo,): for run_data in active_workflow_runs(run_repo, statuses): run_name = str(run_data.get("name") or "") - if run_name != workflow and run_name not in workflow_aliases: + # GitHub reports the *rendered* ``run-name:`` in a run's ``name``, + # not the workflow name, and eight workflows here define one -- + # every workflow whose runs this matcher looks for + # (``opencode-review.yml`` = "Required OpenCode Review", + # ``opencode-review-dispatch.yml`` = "OpenCode Review Dispatch", + # ``strix.yml`` = "Strix Security Scan") is among them. Exact + # matching therefore dropped every production dispatch run here, + # before the ``repository_dispatch`` branch below that exists to + # handle it: ``already_running`` never suppressed a same-head + # repeat and ``stale`` never populated, so .github#1529 took 27 + # dispatches on one unchanged head and older-head central runs were + # never cancelled. Sampled 2026-09-07: 100 of 100 + # opencode-review-dispatch runs carry the rendered form, 0 bare. + # Accept it -- the workflow name, then a space, then the suffix. + if not any( + run_name == candidate or run_name.startswith(f"{candidate} ") + for candidate in (workflow, *workflow_aliases) + ): continue run_id = run_data.get("id") if not run_id: diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index b9b1c43de3..e38c31537a 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -199,10 +199,11 @@ assert_strix_workflow_pr_trigger_hardened() { assert_file_contains "$workflow_file" "branches: [main, develop, master]" "strix workflow scans GitHub Flow and Git Flow protected branches" assert_file_contains "$workflow_file" "pull_request_target:" "strix workflow uses trusted PR trigger" - assert_file_contains "$workflow_file" "admit-current-head:" "strix workflow admits the live pull request head before provider execution" - assert_file_contains "$workflow_file" "needs: [changed-scope, admit-current-head]" "strix provider queue waits for live-head admission" + assert_file_not_contains "$workflow_file" " admit-current-head:" "strix workflow consolidates metadata admission into changed-scope" + assert_file_contains "$workflow_file" "needs: [changed-scope]" "strix provider queue waits for consolidated live-head admission" + assert_file_contains "$workflow_file" "needs.changed-scope.outputs.admitted == 'true'" "strix provider scan requires the consolidated admission output" assert_file_contains "$workflow_file" 'strix-security-scan-${{' "strix workflow coalesces by repository and PR before job admission" - assert_file_not_contains "$workflow_file" 'strix-security-scan-${{ needs.admit-current-head.outputs.target_repository }}-${{' "strix concurrency is not delayed until job admission" + assert_file_not_contains "$workflow_file" 'strix-security-scan-${{ needs.changed-scope.outputs.target_repository }}-${{' "strix concurrency is not delayed until job admission" assert_file_contains "$workflow_file" "cancel-superseded-pr-runs:" "strix workflow runs superseded-head cleanup outside the provider scan queue" assert_file_not_contains "$workflow_file" "format('closed-pr-{0}-{1}'" "strix cleanup does not need a second concurrency queue" assert_file_contains "$workflow_file" 'echo "pr_number=${GITHUB_RUN_ID}"' "strix workflow preserves independent push and schedule evidence" diff --git a/tests/test_agent_mention_downstream_idempotency.py b/tests/test_agent_mention_downstream_idempotency.py index c9b6ab86ea..cb3c31763e 100644 --- a/tests/test_agent_mention_downstream_idempotency.py +++ b/tests/test_agent_mention_downstream_idempotency.py @@ -1,5 +1,8 @@ """Static contracts for downstream review-agent invocation idempotency.""" +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) from pathlib import Path ROOT = Path(__file__).resolve().parents[1] @@ -33,10 +36,19 @@ def test_downstream_workflows_claim_artifacts_and_coalesce_by_pull_request() -> ): header = text.split("\npermissions:\n", 1)[0] job = text.split(" validate-and-forward:\n", 1)[1] - concurrency = job.split(" concurrency:\n", 1)[1].split( - "\n runs-on:", 1 - )[0] - assert "concurrency:" not in header + concurrency = header.split("\nconcurrency:\n", 1)[1] + # 109d79b7 ("replace unsupported queue concurrency") deleted a + # workflow-level block that used ``queue: max``, a key GitHub Actions + # does not support, and parked the group on the job while it was at it. + # What that commit pins is the absence of ``queue:``, not the level: the + # group is back at workflow level because a job-level group is never + # evaluated while the run waits behind the organization job ceiling, so a + # superseded mention held its queue slot until a runner freed up. Every + # other queue-bearing workflow here (strix.yml, noema-review.yml, + # opencode-review.yml, codeql-scan-dispatch.yml, + # opencode-review-dispatch.yml) keys its group at workflow level too. + assert "queue:" not in header + assert " concurrency:" not in job assert "github.event.client_payload.agent_invocation_key" in text assert "cwl-agent-invocation:" in text assert "source_comment_id" in text @@ -45,7 +57,7 @@ def test_downstream_workflows_claim_artifacts_and_coalesce_by_pull_request() -> f"group: {workflow_name}-${{{{ github.event.client_payload.target_repository }}}}-${{{{ github.event.client_payload.pr_number || github.run_id }}}}" in concurrency ) - assert "cancel-in-progress: true" in concurrency + assert workflow_level_cancels_in_progress(text) assert "queue: max" not in text assert "^[0-9a-f]{64}$" in text assert "^[1-9][0-9]*$" in text diff --git a/tests/test_agent_mention_queue_isolation.py b/tests/test_agent_mention_queue_isolation.py index e93ae61aed..d751076661 100644 --- a/tests/test_agent_mention_queue_isolation.py +++ b/tests/test_agent_mention_queue_isolation.py @@ -2,6 +2,9 @@ from __future__ import annotations +import re + + from pathlib import Path ROOT = Path(__file__).resolve().parents[1] @@ -68,4 +71,7 @@ def test_interactive_queue_retires_older_requests_for_only_the_same_pr() -> None concurrency = _concurrency_block(local_job) assert "github.event.issue.number || github.run_id" in concurrency - assert "cancel-in-progress: true" in concurrency + # Anchored on the JOB block, not the workflow-level helper: this router + # declares no workflow-level concurrency, so the sibling helper would raise + # rather than read the block this test is about. + assert re.search(r"(?m)^[ \t]+cancel-in-progress:[ \t]+true[ \t]*$", concurrency) diff --git a/tests/test_agent_review_runtime_quality_consolidation.py b/tests/test_agent_review_runtime_quality_consolidation.py index b0c90eb707..4592cfd166 100644 --- a/tests/test_agent_review_runtime_quality_consolidation.py +++ b/tests/test_agent_review_runtime_quality_consolidation.py @@ -2,6 +2,10 @@ from __future__ import annotations +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) + import re import subprocess from pathlib import Path @@ -56,7 +60,7 @@ def test_pr_concurrency_cancels_only_the_same_workflow_repository_and_pr() -> No "${{ github.repository }}-${{ github.event.pull_request.number }}" in concurrency_contract ) - assert "cancel-in-progress: true" in concurrency_contract + assert workflow_level_cancels_in_progress(workflow) assert "github.sha" not in concurrency_contract assert "head.sha" not in concurrency_contract assert "github.ref" not in concurrency_contract diff --git a/tests/test_audit_org_codeql_coverage.py b/tests/test_audit_org_codeql_coverage.py index ccd2cd9c42..0ffc8fa749 100644 --- a/tests/test_audit_org_codeql_coverage.py +++ b/tests/test_audit_org_codeql_coverage.py @@ -14,6 +14,23 @@ def covered_by_default_setup(name: str) -> dict: "name": name, "archived": False, "default_setup_state": "configured", + "default_setup_languages": ["actions", "python"], + "latest_codeql_analysis": None, + } + + +def default_setup_scanning_nothing(name: str) -> dict: + """Return a repository whose default-setup is on but has no languages enabled. + + The live shape measured on 2026-09-07 for ``life-os``, ``aFIPC`` and + ``inkspan``: ``state`` is ``configured``, ``languages`` is empty and + ``schedule`` is null. ``life-os`` had zero CodeQL analyses of any language. + """ + return { + "name": name, + "archived": False, + "default_setup_state": "configured", + "default_setup_languages": [], "latest_codeql_analysis": None, } @@ -89,6 +106,60 @@ def test_default_setup_alone_counts_as_coverage() -> None: assert audit.audit_codeql_coverage(repositories, now=NOW) == [] +def test_default_setup_with_no_languages_enabled_is_not_coverage() -> None: + """A setup that scans nothing must not satisfy the configured-state check. + + Measured 2026-09-07: ``life-os`` reports ``configured`` with an empty + ``languages`` list and has zero CodeQL analyses of any language, while + ``codeql-pr.yml`` still runs on every pull request head. Before this check + the audit passed it on the state alone. + """ + repositories = [default_setup_scanning_nothing("life-os")] + + assert audit.audit_codeql_coverage(repositories, now=NOW) == [ + "life-os has CodeQL default-setup configured with no languages enabled, " + "so it scans nothing and produces no analyses" + ] + + +def test_default_setup_scanning_nothing_still_passes_on_a_fresh_analysis() -> None: + """The empty-language setup is only a gap when nothing else covers the repo. + + ``aFIPC`` and ``inkspan`` both report the empty-language shape yet receive + analyses from a repository-local ``codeql.yml``, so flagging them would be a + false alarm. + """ + repository = default_setup_scanning_nothing("aFIPC") + repository["latest_codeql_analysis"] = covered_by_recent_analysis("aFIPC")[ + "latest_codeql_analysis" + ] + + assert audit.audit_codeql_coverage([repository], now=NOW) == [] + + +def test_payload_without_the_languages_key_fails_closed() -> None: + """A payload predating the workflow change must not pass on state alone. + + Falling back to ``default_setup_state`` when the key is missing would + silently restore the gap this check exists to close. + """ + repository = covered_by_default_setup("PolicyWeave") + del repository["default_setup_languages"] + + assert audit.audit_codeql_coverage([repository], now=NOW) == [ + "PolicyWeave has CodeQL default-setup configured with no languages " + "enabled, so it scans nothing and produces no analyses" + ] + + +def test_non_list_languages_value_fails_closed() -> None: + """A malformed ``languages`` value is not evidence that anything is scanned.""" + repository = covered_by_default_setup("PolicyWeave") + repository["default_setup_languages"] = "python" + + assert len(audit.audit_codeql_coverage([repository], now=NOW)) == 1 + + def test_recent_analysis_alone_counts_as_coverage() -> None: repositories = [covered_by_recent_analysis("TEPP")] @@ -269,3 +340,60 @@ def test_parse_args_defaults_to_none() -> None: args = audit.parse_args([]) assert args.repositories_json is None + + +def test_main_refuses_an_empty_payload_instead_of_passing_vacuously( + monkeypatch, capsys +) -> None: + """An audit that examined nothing must not print PASS. + + ``audit_codeql_coverage([])`` returning no gaps is correct -- there are no + repositories to have gaps. What is wrong is ``main`` turning that into + "PASS: all 0 repositories have real CodeQL coverage" and exiting 0, which + is the same vacuous-pass shape as a default setup that is configured with + no languages enabled. + """ + monkeypatch.setattr("sys.stdin", StringIO("[]")) + + assert audit.main([]) == 2 + captured = capsys.readouterr() + assert "audited nothing" in captured.err + assert "PASS" not in captured.out + + +def test_main_refuses_a_payload_of_only_archived_repositories(monkeypatch, capsys) -> None: + """Counting what was supplied, not what was examined, left the hole open. + + An archived-only payload is non-empty, so it passed the first version of + this guard, and archived repositories are then legitimately skipped -- the + run reported "PASS: all 1 repositories have real CodeQL coverage" having + examined none of them. Found in review, one layer out from the empty-payload + case it replaces. + """ + monkeypatch.setattr( + "sys.stdin", StringIO(json.dumps([uncovered("trivy-sarif-repro", archived=True)])) + ) + + assert audit.main([]) == 2 + captured = capsys.readouterr() + assert "0 of 1 repositories were eligible" in captured.err + assert "PASS" not in captured.out + + +def test_pass_line_counts_examined_repositories_not_supplied_ones( + tmp_path, capsys +) -> None: + """The PASS line must not credit archived repositories it never examined.""" + payload = tmp_path / "repositories.json" + payload.write_text( + json.dumps( + [ + covered_by_default_setup("PolicyWeave"), + uncovered("trivy-sarif-repro", archived=True), + ] + ), + encoding="utf-8", + ) + + assert audit.main([str(payload)]) == 0 + assert "PASS: all 1 repositories" in capsys.readouterr().out diff --git a/tests/test_bootstrap_codeql_pull_requests.py b/tests/test_bootstrap_codeql_pull_requests.py index eb20c3d1e0..12fd1d8b52 100644 --- a/tests/test_bootstrap_codeql_pull_requests.py +++ b/tests/test_bootstrap_codeql_pull_requests.py @@ -2,6 +2,10 @@ from __future__ import annotations +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) + from io import StringIO import json import subprocess @@ -67,7 +71,7 @@ def test_rendered_workflow_redetects_stacks_and_pins_every_action() -> None: assert "pull_request:" not in workflow assert "github.event.pull_request" not in workflow assert "github.event_name == 'push' && github.ref || github.event_name" in workflow - assert "cancel-in-progress: true" in workflow + assert workflow_level_cancels_in_progress(workflow) assert workflow.count("@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9") == 2 assert "actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0" in workflow diff --git a/tests/test_central_required_workflow_ruleset_audit.py b/tests/test_central_required_workflow_ruleset_audit.py index 77bbf53305..cec0d2aead 100644 --- a/tests/test_central_required_workflow_ruleset_audit.py +++ b/tests/test_central_required_workflow_ruleset_audit.py @@ -517,13 +517,21 @@ def test_audit_organization_codeql_coverage_step_has_freshness_and_credential_gu " exit 1\n" " fi" ) in workflow + # This pinned `--jq .state` until 2026-09-07. What it protects is that the + # audit reads default-setup per repository, not that it reads only the + # state: `state == "configured"` with an empty `languages` list scans + # nothing and produces no analyses (live on life-os, aFIPC and inkspan), + # so the step now fetches the whole object and extracts both fields. + assert 'repos/${ORG_LOGIN}/${repository}/code-scanning/default-setup"' in workflow + assert """default_setup_state=$(jq '.state // null' "$default_setup_json")""" in workflow assert ( - 'repos/${ORG_LOGIN}/${repository}/code-scanning/default-setup" --jq .state' + """default_setup_languages=$(jq '.languages // []' "$default_setup_json")""" in workflow ) + assert "default_setup_languages: $default_setup_languages" in workflow assert ( 'if [ "$archived" != "true" ]; then\n' - ' default_setup_state_json="$RUNNER_TEMP/codeql-default-setup-' + ' default_setup_json="$RUNNER_TEMP/codeql-default-setup-' '${repository//[^A-Za-z0-9_.-]/_}.json"' ) in workflow assert ( @@ -540,6 +548,33 @@ def test_audit_organization_codeql_coverage_step_has_freshness_and_credential_gu assert "python3 scripts/ci/audit_org_codeql_coverage.py" in workflow +def test_codeql_coverage_audit_survives_a_ruleset_drift_failure() -> None: + """An owner-configured ruleset drift must not disable the coverage detector. + + Both audits live in one job, and the ruleset step exits 1 on governance + drift. It did on 2026-09-06 ("exactly two approving reviews are not + required", "last-push approval protection is disabled"), so every run since + 2026-09-04 failed before reaching the CodeQL coverage step. The subjects are + unrelated and the coverage step has no data dependency on the one above it, + so it is guarded by ``if: always()``. + + The bootstrap steps below it are deliberately *not* given the same guard: + they open pull requests, and running a mutation after an unexplained + upstream failure is a different decision from running a read-only detector. + """ + workflow = (REPO_ROOT / ".github/workflows/audit-central-ruleset.yml").read_text( + encoding="utf-8" + ) + coverage_step = workflow.split("- name: Audit organization CodeQL coverage\n", 1)[1] + before_next_step = coverage_step.split(" - name: ", 1)[0] + + assert "\n if: always()\n" in before_next_step + bootstrap_step = workflow.split( + "- name: Create missing CodeQL setup pull requests\n", 1 + )[1].split(" - name: ", 1)[0] + assert "if: always()" not in bootstrap_step + + def test_codeql_gap_bootstrap_uses_trusted_opencode_identity_without_pr_head_execution() -> None: """Backlog item 38 stays on trusted main and treats installation tokens as opaque.""" workflow = (REPO_ROOT / ".github/workflows/audit-central-ruleset.yml").read_text( diff --git a/tests/test_close_empty_pr_queue_pressure.py b/tests/test_close_empty_pr_queue_pressure.py index 331a604631..6da88f63f1 100644 --- a/tests/test_close_empty_pr_queue_pressure.py +++ b/tests/test_close_empty_pr_queue_pressure.py @@ -1,5 +1,6 @@ """Regression contracts for close-event runner admission pressure.""" +import re from pathlib import Path import pytest @@ -29,7 +30,7 @@ def test_closed_pull_request_does_not_allocate_a_noop_runner( assert "closed" in workflow assert "github.event.pull_request.number" in concurrency assert "github.event.pull_request.head.sha" not in concurrency - assert "cancel-in-progress:" in concurrency + assert re.search(r"(?m)^[ \t]+cancel-in-progress:[ \t]+\S", concurrency) assert "cancel-closed-pr-runs:" not in workflow assert "github.event.action != 'closed'" in workflow assert evidence_job in workflow diff --git a/tests/test_codeql_scan_dispatch_workflow_contract.py b/tests/test_codeql_scan_dispatch_workflow_contract.py index bad19b54aa..71fa43541f 100644 --- a/tests/test_codeql_scan_dispatch_workflow_contract.py +++ b/tests/test_codeql_scan_dispatch_workflow_contract.py @@ -19,6 +19,10 @@ from scripts.ci import audit_central_required_workflows as ruleset_audit from tests.test_opencode_workflow_shell_syntax import _extract_run_block +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, + workflow_level_concurrency_group, +) REPO_ROOT = Path(__file__).resolve().parents[1] WORKFLOW_PATH = REPO_ROOT / ".github/workflows/codeql-scan-dispatch.yml" @@ -93,12 +97,17 @@ def test_codeql_scan_dispatch_workflow_structure(): def test_codeql_scan_dispatch_keeps_current_head_language_shards_independent(): """A current-head language scan cannot cancel its sibling language scans.""" workflow = WORKFLOW_PATH.read_text(encoding="utf-8") - concurrency = workflow.split("concurrency:\n", 1)[1].split("\n\npermissions:", 1)[0] - - assert "github.event.client_payload.target_repository" in concurrency - assert "github.event.client_payload.pr_number" in concurrency - assert "github.event.client_payload.required_language" in concurrency - assert "cancel-in-progress: true" in concurrency + group_value = workflow_level_concurrency_group(workflow) + + # The language segment is what keeps sibling language shards in separate groups, so it is + # asserted on the group's own value: a comment naming it would otherwise satisfy the check + # while the key had lost it, silently letting one language's scan cancel another's. + assert "github.event.client_payload.target_repository" in group_value + assert "github.event.client_payload.pr_number" in group_value + assert "github.event.client_payload.required_language" in group_value + # Same reasoning as the group above, applied to the flag: the substring form + # is satisfied by a comment quoting it while the key beside it reads false. + assert workflow_level_cancels_in_progress(workflow) def _run_validate_step(tmp_path: Path, env_overrides: dict[str, str], pull_request: dict) -> subprocess.CompletedProcess[str]: diff --git a/tests/test_docs_only_pr_runner_admission.py b/tests/test_docs_only_pr_runner_admission.py index 8b2e6e8ee2..f9816854b2 100644 --- a/tests/test_docs_only_pr_runner_admission.py +++ b/tests/test_docs_only_pr_runner_admission.py @@ -25,6 +25,11 @@ from pathlib import Path import re +import shutil +import subprocess +import textwrap + +import pytest REPO_ROOT = Path(__file__).resolve().parents[1] @@ -85,20 +90,292 @@ def _on_block(workflow: str) -> str: return match.group(1) -def test_gate_job_is_byte_identical_across_the_five_workflows_apart_from_if(): - """The `changed-scope` block must not drift between its five copies.""" - normalized_blocks = set() +def _step_shell(workflow: str, name: str) -> str: + """Return one named step's dedented production shell body.""" + start = workflow.index(f" - name: {name}\n") + try: + end = workflow.index("\n - name:", start + 1) + except ValueError: + end = len(workflow) + return textwrap.dedent(workflow[start:end].split(" run: |\n", 1)[1]) + + +def _read_outputs(path: Path) -> dict[str, str]: + """Read simple key-value outputs written by the tested workflow steps.""" + if not path.exists(): + return {} + return dict(line.split("=", 1) for line in path.read_text().splitlines()) + + +def _run_strix_metadata_steps( + tmp_path: Path, + *, + event_name: str, + scenario: str, + expected_files: str = "1", + draft_json: str = "false", +) -> tuple[ + subprocess.CompletedProcess[str], + subprocess.CompletedProcess[str] | None, + list[str], + dict[str, str], +]: + """Execute the production metadata shells with a deterministic fake GitHub API.""" + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + calls = tmp_path / "gh-calls" + fake_gh = fake_bin / "gh" + fake_gh.write_text( + """#!/bin/bash +set -eu +printf '%s\\n' "$*" >> "$CALLS_FILE" +case "$SCENARIO:$*" in + api-error:*) exit 23 ;; + file-api-error:*/files*) exit 24 ;; + *:*/files*) + case "$SCENARIO" in + docs|dispatch) printf 'docs/readme.md\\n' ;; + code) printf 'backend/app.py\\n' ;; + empty) exit 0 ;; + mismatch) printf 'docs/a.md\\n' ;; + esac + ;; + stale:*) + printf '{"state":"open","draft":%s,"base":{"repo":{"full_name":"owner/repo"},"ref":"main","sha":"%s"},"head":{"repo":{"full_name":"owner/repo"},"sha":"%s"}}\\n' "$DRAFT_JSON" "$BASE_SHA" "$STALE_SHA" + ;; + *) + printf '{"state":"open","draft":%s,"base":{"repo":{"full_name":"owner/repo"},"ref":"main","sha":"%s"},"head":{"repo":{"full_name":"owner/repo"},"sha":"%s"}}\\n' "$DRAFT_JSON" "$BASE_SHA" "$HEAD_SHA" + ;; +esac +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + fake_sleep = fake_bin / "sleep" + fake_sleep.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + fake_sleep.chmod(0o755) + + workflow = _read("strix.yml") + changed_scope = _top_level_job_block(workflow, "changed-scope") + admission_output = tmp_path / "admission-output" + base_sha = "a" * 40 + head_sha = "b" * 40 + env = { + "PATH": f"{fake_bin}:/opt/homebrew/bin:/usr/bin:/bin", + "HOME": str(tmp_path), + "GITHUB_OUTPUT": str(admission_output), + "GITHUB_RUN_ID": "9001", + "EVENT_NAME": event_name, + "TARGET_REPOSITORY": "owner/repo", + "TARGET_PR_NUMBER": "17" if scenario != "malformed" else "bad", + "EXPECTED_BASE_REF": "main", + "EXPECTED_BASE_SHA": base_sha, + "EXPECTED_HEAD_REPOSITORY": "owner/repo", + "EXPECTED_HEAD_SHA": head_sha, + "CALLS_FILE": str(calls), + "SCENARIO": scenario, + "DRAFT_JSON": draft_json, + "BASE_SHA": base_sha, + "HEAD_SHA": head_sha, + "STALE_SHA": "c" * 40, + } + admission = subprocess.run( + [shutil.which("bash") or "/bin/bash", "-c", _step_shell(changed_scope, "Verify event metadata against the live pull request")], + env=env, + capture_output=True, + text=True, + check=False, + ) + outputs = _read_outputs(admission_output) + classifier = None + if admission.returncode == 0 and outputs.get("admitted") == "true": + classifier_output = tmp_path / "classifier-output" + classifier = subprocess.run( + [shutil.which("bash") or "/bin/bash", "-c", _step_shell(changed_scope, "Classify changed paths")], + env={ + **env, + "GITHUB_OUTPUT": str(classifier_output), + "REPO": "owner/repo", + "PR": "17" if event_name == "pull_request_target" else "", + "EXPECTED_FILES": expected_files if event_name == "pull_request_target" else "", + }, + capture_output=True, + text=True, + check=False, + ) + outputs.update(_read_outputs(classifier_output)) + return admission, classifier, calls.read_text().splitlines() if calls.exists() else [], outputs + + +@pytest.mark.parametrize("event_name", ("pull_request_target", "repository_dispatch")) +@pytest.mark.parametrize("draft_json", ("true", "null", '"false"')) +def test_strix_live_draft_state_blocks_admission(tmp_path, event_name, draft_json): + """Draft or unverifiable readiness never reaches file classification or scanning.""" + admission, classifier, calls, outputs = _run_strix_metadata_steps( + tmp_path, event_name=event_name, scenario="code", draft_json=draft_json, + ) + assert outputs["admitted"] == "false" + assert classifier is None + assert len(calls) == 1 + assert admission.returncode == 0 if draft_json == "true" else admission.returncode != 0 + + +@pytest.mark.parametrize("event_name", ("push", "schedule")) +def test_strix_non_pr_events_skip_api_and_scan_by_default( + tmp_path: Path, event_name: str +) -> None: + """Direct pushes and schedules admit without consulting pull-request APIs.""" + admission, classifier, calls, outputs = _run_strix_metadata_steps( + tmp_path, event_name=event_name, scenario="direct" + ) + assert admission.returncode == 0 + assert classifier is not None and classifier.returncode == 0 + assert calls == [] + assert outputs | {"admitted": "true", "code": "true", "deps": "true"} == outputs + + +def test_strix_exact_pr_metadata_admits_then_classifies_docs_only( + tmp_path: Path, +) -> None: + """A current docs-only PR is admitted before its scan is suppressed.""" + admission, classifier, calls, outputs = _run_strix_metadata_steps( + tmp_path, event_name="pull_request_target", scenario="docs" + ) + assert admission.returncode == 0 + assert classifier is not None and classifier.returncode == 0 + assert len(calls) == 2 + assert outputs["admitted"] == "true" + assert outputs["code"] == "false" + assert outputs["deps"] == "false" + + +def test_strix_exact_dispatch_admits_without_pr_file_api(tmp_path: Path) -> None: + """Exact dispatch validates its PR but has no native changed-file fields.""" + admission, classifier, calls, outputs = _run_strix_metadata_steps( + tmp_path, event_name="repository_dispatch", scenario="dispatch" + ) + assert admission.returncode == 0 + assert classifier is not None and classifier.returncode == 0 + assert len(calls) == 1 + assert "/files" not in calls[0] + assert outputs | {"admitted": "true", "code": "true", "deps": "true"} == outputs + + +@pytest.mark.parametrize("event_name", ("pull_request_target", "repository_dispatch")) +def test_strix_stale_pr_stops_before_classifier( + tmp_path: Path, event_name: str +) -> None: + """Native and dispatched stale events stop before changed-path lookup.""" + admission, classifier, calls, outputs = _run_strix_metadata_steps( + tmp_path, event_name=event_name, scenario="stale" + ) + assert admission.returncode == 0 + assert classifier is None + assert len(calls) == 1 + assert outputs == {"admitted": "false"} + + +@pytest.mark.parametrize("scenario", ("empty", "mismatch", "file-api-error")) +def test_strix_incomplete_file_list_fails_open_to_scan( + tmp_path: Path, scenario: str +) -> None: + """Incomplete or unreadable changed-file evidence keeps full scan coverage.""" + admission, classifier, _calls, outputs = _run_strix_metadata_steps( + tmp_path, + event_name="pull_request_target", + scenario=scenario, + expected_files="2" if scenario == "mismatch" else "1", + ) + assert admission.returncode == 0 + assert classifier is not None and classifier.returncode == 0 + assert outputs["admitted"] == "true" + assert outputs["code"] == "true" + assert outputs["deps"] == "true" + + +def test_strix_code_file_keeps_the_scan_admitted(tmp_path: Path) -> None: + """A complete code-changing PR file list must keep the Strix scan enabled.""" + admission, classifier, _calls, outputs = _run_strix_metadata_steps( + tmp_path, event_name="pull_request_target", scenario="code" + ) + assert admission.returncode == 0 + assert classifier is not None and classifier.returncode == 0 + assert outputs["admitted"] == "true" + assert outputs["code"] == "true" + + +@pytest.mark.parametrize("scenario", ("malformed", "api-error")) +def test_strix_invalid_or_unreadable_admission_fails_the_metadata_job( + tmp_path: Path, scenario: str +) -> None: + """Malformed metadata and admission API errors remain hard failures.""" + admission, classifier, calls, outputs = _run_strix_metadata_steps( + tmp_path, event_name="repository_dispatch", scenario=scenario + ) + assert admission.returncode != 0 + assert classifier is None + assert outputs == {"admitted": "false"} + assert len(calls) == (1 if scenario == "api-error" else 0) + + +def test_gate_classifier_shell_is_byte_identical_across_the_workflows(): + """The shared changed-path classifier shell must not drift.""" + classifier_bodies = set() for filename in GATE_WORKFLOWS: workflow = _read(filename) block = _top_level_job_block(workflow, "changed-scope") - normalized = "\n".join( - line for line in block.splitlines() if not line.strip().startswith("if:") - ) - normalized_blocks.add(normalized) - assert len(normalized_blocks) == 1, ( - "changed-scope gate copies drifted; keep them byte-identical apart " - "from the single 'if:' line" + classifier = block.split(" - name: Classify changed paths\n", 1)[1] + run_body = classifier.split(" run: |\n", 1)[1] + classifier_bodies.add(run_body) + assert len(classifier_bodies) == 1, ( + "changed-scope classifier shell bodies drifted; keep the run blocks " + "byte-identical" + ) + + +@pytest.mark.parametrize("filename", (*GATE_WORKFLOWS, "codeql-pr.yml")) +@pytest.mark.parametrize("success_attempt", (0, 1, 2, 3)) +def test_classifier_sleeps_only_before_another_attempt( + tmp_path: Path, filename: str, success_attempt: int +) -> None: + """Exhaustion preserves full scanning without a final, unused backoff.""" + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + for command_name, script_body in { + "gh": '#!/bin/bash\necho call >> "$CALLS_FILE"\n' + 'attempt=$(wc -l < "$CALLS_FILE")\n' + '[ "$attempt" -eq "$SUCCESS_ATTEMPT" ] || exit 23\n' + "printf 'docs/readme.md\\n'\n", + "sleep": '#!/bin/sh\nprintf "%s\\n" "$1" >> "$SLEEPS_FILE"\n', + }.items(): + executable = fake_bin / command_name + executable.write_text(script_body, encoding="utf-8") + executable.chmod(0o755) + calls_file = tmp_path / "calls" + sleeps_file = tmp_path / "sleeps" + output_file = tmp_path / "outputs" + job_name = "detect-languages" if filename == "codeql-pr.yml" else "changed-scope" + result = subprocess.run( + [shutil.which("bash") or "/bin/bash", "-c", _step_shell( + _top_level_job_block(_read(filename), job_name), "Classify changed paths" + )], + env={ + "PATH": f"{fake_bin}:/usr/bin:/bin", + "CALLS_FILE": str(calls_file), "SLEEPS_FILE": str(sleeps_file), + "SUCCESS_ATTEMPT": str(success_attempt), + "GITHUB_OUTPUT": str(output_file), + "REPO": "owner/repo", "PR": "17", "EXPECTED_FILES": "1", + }, + capture_output=True, text=True, check=False, ) + assert result.returncode == 0, result.stderr + assert len(calls_file.read_text().splitlines()) == (success_attempt or 3) + actual_sleeps = sleeps_file.read_text().splitlines() if sleeps_file.exists() else [] + assert actual_sleeps == ["3", "6"][:(success_attempt or 3) - 1] + outputs = _read_outputs(output_file) + assert outputs["code"] == ("false" if success_attempt else "true") + if filename != "codeql-pr.yml": + assert outputs["deps"] == ("false" if success_attempt else "true") def test_gate_job_and_codeql_scope_step_share_one_doc_pattern_line(): @@ -163,7 +440,7 @@ def test_gated_jobs_keep_the_close_guard_and_add_an_output_dependent_condition() for job_name in job_names: block = _top_level_job_block(workflow, job_name) close_guard_block = ( - _top_level_job_block(workflow, "admit-current-head") + _top_level_job_block(workflow, "changed-scope") if filename == "strix.yml" else block ) @@ -177,6 +454,24 @@ def test_gated_jobs_keep_the_close_guard_and_add_an_output_dependent_condition() ) +def test_strix_uses_one_bounded_metadata_job_before_scan_admission(): + """Strix must admit the live head before classifying paths in one job.""" + workflow = _read("strix.yml") + changed_scope = _top_level_job_block(workflow, "changed-scope") + strix = _top_level_job_block(workflow, "strix") + + assert "\n admit-current-head:\n" not in workflow + assert "timeout-minutes: 10" in changed_scope + assert changed_scope.count("timeout-minutes: 5") == 2 + assert changed_scope.index("id: admission") < changed_scope.index("id: scope") + assert "if: steps.admission.outputs.admitted == 'true'" in changed_scope + for output in ("code", "deps", "admitted", "target_repository", "pr_number"): + assert re.search(rf"(?m)^ {output}:", changed_scope) + assert "needs: [changed-scope]" in strix + assert "needs.changed-scope.outputs.code == 'true'" in strix + assert "needs.changed-scope.outputs.admitted == 'true'" in strix + + def test_codeql_pr_gates_analyze_head_at_step_level_not_job_level(): """`analyze-head` must gate its steps, not the whole job. diff --git a/tests/test_exact_artifact_quality_single_runner.py b/tests/test_exact_artifact_quality_single_runner.py index b8711ab7a5..9378e2232c 100644 --- a/tests/test_exact_artifact_quality_single_runner.py +++ b/tests/test_exact_artifact_quality_single_runner.py @@ -2,6 +2,10 @@ from __future__ import annotations +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) + import re from pathlib import Path @@ -68,7 +72,7 @@ def test_pr_concurrency_uses_workflow_repository_and_pr_identity() -> None: "${{ github.event.pull_request.number }}" in concurrency ) - assert "cancel-in-progress: true" in concurrency + assert workflow_level_cancels_in_progress(workflow) assert "github.sha" not in concurrency assert "pull_request.head.sha" not in concurrency diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 5fa23dec53..e8a0dd6f59 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -1,3 +1,7 @@ + +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) import base64 import hashlib import http.client @@ -60,7 +64,7 @@ def test_noema_concurrency_and_live_head_cleanup_preserve_current_review(): workflow = Path(".github/workflows/noema-review.yml").read_text(encoding="utf-8") concurrency = workflow.split("concurrency:", 1)[1].split("permissions:", 1)[0] assert "github.event.workflow_run" not in concurrency - assert "cancel-in-progress: true" in concurrency + assert workflow_level_cancels_in_progress(workflow) admission = workflow.split("\n admit-current-head:\n", 1)[1].split( "\n cancel-closed-pr-runs:", 1 )[0] diff --git a/tests/test_noema_token_lifetime_stale_run_contract.py b/tests/test_noema_token_lifetime_stale_run_contract.py index 77a64cabdb..108647907a 100644 --- a/tests/test_noema_token_lifetime_stale_run_contract.py +++ b/tests/test_noema_token_lifetime_stale_run_contract.py @@ -1,5 +1,8 @@ """Regression contract for consolidated Noema quality-run retirement.""" +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) from pathlib import Path @@ -26,4 +29,4 @@ def test_noema_token_lifetime_quality_ci_retires_superseded_pr_runs() -> None: assert "github.event.pull_request.head.sha" not in concurrency_contract assert "github.sha" not in concurrency_contract assert "github.ref" not in concurrency_contract - assert "cancel-in-progress: true" in concurrency_contract + assert workflow_level_cancels_in_progress(workflow) diff --git a/tests/test_opencode_required_rerun_capacity.py b/tests/test_opencode_required_rerun_capacity.py index 431d3a8bc2..c85bc24e3c 100644 --- a/tests/test_opencode_required_rerun_capacity.py +++ b/tests/test_opencode_required_rerun_capacity.py @@ -1,5 +1,8 @@ """Capacity contract for Required OpenCode dispatch and exact-run wakeup.""" +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) import json import os from pathlib import Path @@ -48,7 +51,7 @@ def test_native_cancellation_runs_before_runner_admission() -> None: assert "required-opencode-review-${{" in concurrency assert "github.event.pull_request.number || github.run_id" in concurrency - assert "cancel-in-progress: true" in concurrency + assert workflow_level_cancels_in_progress(required) assert "live_head_matches()" in required diff --git a/tests/test_opencode_required_verdict_regression.py b/tests/test_opencode_required_verdict_regression.py index f29b97a663..5c5325d1aa 100644 --- a/tests/test_opencode_required_verdict_regression.py +++ b/tests/test_opencode_required_verdict_regression.py @@ -2,6 +2,10 @@ from __future__ import annotations +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) + import json import os import re @@ -86,7 +90,7 @@ def test_opencode_dispatch_uses_the_same_target_repo_pr_group() -> None: assert "opencode-review-${{" in dispatched assert "needs.validate-pr-metadata.outputs.target_repository" in dispatched assert "needs.validate-pr-metadata.outputs.pr_number || github.run_id" in dispatched - assert "cancel-in-progress: true" in dispatched + assert workflow_level_cancels_in_progress(dispatched) assert dispatched.index("validate-pr-metadata:") < dispatched.index(" concurrency:") @@ -621,7 +625,7 @@ def test_opencode_review_trigger_reacts_to_draft_conversion() -> None: "types: [opened, synchronize, reopened, ready_for_review, " "converted_to_draft, closed]" ) in trigger_block - assert "cancel-in-progress: true" in workflow.split("\npermissions:\n", 1)[0] + assert workflow_level_cancels_in_progress(workflow) def test_opencode_review_concurrency_group_is_workflow_level_repo_and_pr() -> None: @@ -637,7 +641,7 @@ def test_opencode_review_concurrency_group_is_workflow_level_repo_and_pr() -> No assert "required-opencode-review-${{" in concurrency_block assert "github.event.pull_request.head.sha || github.run_id" not in concurrency_block assert "github.event.pull_request.number || github.run_id" in concurrency_block - assert "cancel-in-progress: true" in concurrency_block + assert workflow_level_cancels_in_progress(workflow) assert " concurrency:" not in target_job.split(" permissions:", 1)[0] admission = workflow.split("\n admit-current-head:\n", 1)[1].split( "\n coverage-source-tree:", 1 diff --git a/tests/test_pr_review_fix_scheduler_source_pin.py b/tests/test_pr_review_fix_scheduler_source_pin.py index 7958ba5163..0f9e0adb1c 100644 --- a/tests/test_pr_review_fix_scheduler_source_pin.py +++ b/tests/test_pr_review_fix_scheduler_source_pin.py @@ -2,6 +2,10 @@ from __future__ import annotations +from tests.test_required_workflow_queue_contract import ( + workflow_level_cancels_in_progress, +) + from pathlib import Path @@ -88,7 +92,7 @@ def test_reusable_scheduler_retains_least_privilege_and_bounded_dispatch() -> No assert "pull-requests: write" not in workflow assert "MAX_DISPATCHES:" in workflow assert "RETRY_HOURS:" in workflow - assert "cancel-in-progress: true" in workflow + assert workflow_level_cancels_in_progress(workflow) def test_reusable_scheduler_bounds_both_oidc_exchange_requests() -> None: diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 2cbda7f85b..ba47b89c8d 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -6109,6 +6109,101 @@ def test_dispatch_strix_waits_for_active_target_repository_run(monkeypatch, caps assert "target repository already has active run(s) ContextualWisdomLab/.github@9350" in capsys.readouterr().out +def test_central_run_filter_accepts_the_run_name_github_actually_sends(monkeypatch): + """A ``run-name:`` workflow reports the rendered title in ``name``. + + ``opencode-review-dispatch.yml``, ``strix.yml`` and ``noema-review.yml`` all + define ``run-name:``, so GitHub sets each run's ``name`` to the rendered + string, identical to ``display_title`` -- sampled 2026-09-07, 100 of 100 + opencode-review-dispatch runs carry that form and none carries the bare + workflow name. Matching ``name`` exactly against the aliases dropped every + one of them before the ``repository_dispatch`` branch that exists to read + them, so ``already_running`` never suppressed a same-head repeat and + ``stale`` never populated: .github#1529 took 27 dispatches on one unchanged + head, and older-head central runs were never cancelled. + + The neighbouring fixture below sets a bare ``name`` alongside a rendered + ``display_title``, which is why 100% coverage of that branch never showed + that production could not reach it. + """ + head_sha = "a" * 40 + stale_sha = "b" * 40 + current_title = f"Required OpenCode Review owner/repo#1@{head_sha}" + stale_title = f"Required OpenCode Review owner/repo#1@{stale_sha}" + central_runs = [ + { + "id": 9500, + "name": current_title, + "display_title": current_title, + "event": "repository_dispatch", + }, + { + "id": 9501, + "name": stale_title, + "display_title": stale_title, + "event": "repository_dispatch", + }, + ] + + def fake_active_runs(repo, statuses=("queued", "in_progress")): + del statuses + return central_runs if repo == "ContextualWisdomLab/.github" else [] + + monkeypatch.setattr(sched, "active_workflow_runs", fake_active_runs) + monkeypatch.setenv( + "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", + "ContextualWisdomLab/.github", + ) + + assert sched.active_opencode_run_refs( + "owner/repo", + "OpenCode Review", + make_pr(headRefOid=head_sha), + ) == ( + [("ContextualWisdomLab/.github", "9500")], + [("ContextualWisdomLab/.github", "9501")], + ) + + +def test_central_run_filter_reads_the_rendered_strix_run_name_too(monkeypatch): + """Strix shares the matcher, and ``strix.yml`` also defines ``run-name:``. + + ``active_review_run_refs`` has exactly two call sites -- OpenCode's and + ``dispatch_strix_evidence``'s -- so the exact-``name`` match blinded both. + Pinning the Strix side here keeps a later narrowing of the fix to the + OpenCode aliases from silently reopening the Strix half. + """ + head_sha = "c" * 40 + current_title = f"Strix Security Scan owner/repo#1@{head_sha}" + + def fake_active_runs(repo, statuses=("queued", "in_progress")): + del statuses + if repo != "ContextualWisdomLab/.github": + return [] + return [ + { + "id": 9600, + "name": current_title, + "display_title": current_title, + "event": "repository_dispatch", + } + ] + + monkeypatch.setattr(sched, "active_workflow_runs", fake_active_runs) + monkeypatch.setenv( + "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", + "ContextualWisdomLab/.github", + ) + + assert sched.active_review_run_refs( + "owner/repo", + "Strix Security Scan", + make_pr(headRefOid=head_sha), + run_title="Strix Security Scan", + workflow_aliases=frozenset({"Strix Security Scan"}), + ) == ([("ContextualWisdomLab/.github", "9600")], []) + + def test_central_run_filter_ignores_malformed_and_non_dispatch_titles(monkeypatch): head_sha = "a" * 40 central_runs = [ diff --git a/tests/test_repository_metadata_workflow_pages.py b/tests/test_repository_metadata_workflow_pages.py index 82aa4462f7..5c05dfbe3d 100644 --- a/tests/test_repository_metadata_workflow_pages.py +++ b/tests/test_repository_metadata_workflow_pages.py @@ -2,6 +2,8 @@ from __future__ import annotations +import re + import importlib.util import json from pathlib import Path @@ -40,7 +42,11 @@ def test_metadata_pr_validation_cancels_superseded_head_runs() -> None: concurrency = workflow.split("concurrency:", 1)[1].split("jobs:", 1)[0] assert "group: repository-metadata-reconcile-${{ github.ref }}" in concurrency - assert "cancel-in-progress: ${{ github.event_name == 'pull_request' }}" in concurrency + assert re.search( + r"(?m)^[ \t]+cancel-in-progress:[ \t]+\$\{\{ github\.event_name == 'pull_request' \}\}" + r"[ \t]*$", + concurrency, + ) assert "github.event.pull_request.head.sha" not in concurrency diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 48035c2c40..50946661e4 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -21,6 +21,90 @@ def workflow_text(name: str) -> str: return (REPO_ROOT / ".github" / "workflows" / name).read_text(encoding="utf-8") +# The workflow-level block is the one whose key starts at column zero; job-level +# blocks are indented under ``jobs:``. Anchoring there instead of slicing the text +# before ``permissions:`` makes the search independent of key order, which two +# workflows already need: javascript-coverage-quality-ci.yml and +# repository-metadata-reconcile.yml declare ``permissions:`` above ``concurrency:``, +# and the older slice returned nothing for them and raised IndexError rather than +# reading the block that is plainly there. +WORKFLOW_LEVEL_CONCURRENCY_BLOCK = re.compile( + r"(?m)^concurrency:[ \t]*\n(?P(?:[ \t]+[^\n]*\n)+)" +) + + +def workflow_level_concurrency_group(workflow: str) -> str: + """Return only the workflow-level ``concurrency.group`` value, comments removed. + + Asserting that an expression "appears in the concurrency block" is satisfied by + a comment that merely documents the key while the key itself says something + else, because the block's raw text carries its comments. That is not + hypothetical: the block above this workflow's group explains the key in prose, + so a maintainer quoting the expressions there while another change collapsed + the group to the repository alone would leave every pull request in one group, + cancelling each other, with the contract still green. Slice to the group's own + value so the assertion tests the key rather than the documentation beside it. + """ + block_match = WORKFLOW_LEVEL_CONCURRENCY_BLOCK.search(workflow) + if block_match is None: + raise AssertionError("workflow declares no workflow-level concurrency block") + value: list[str] = [] + collecting = False + for line in block_match.group("body").splitlines(): + if line.strip().startswith("#"): + continue + if not collecting: + if re.match(r"^\s*group:", line): + collecting = True + value.append(line.split("group:", 1)[1]) + continue + if re.match(r"^\s*[A-Za-z][\w-]*:", line): + break + value.append(line) + if not collecting: + raise AssertionError("workflow-level concurrency block declares no group") + head = value[0].strip() + if head.startswith("|"): + # Not represented here, and on 2026-09-07 no workflow uses one: a literal + # block keeps its newlines, so folding it would return a value YAML never + # produces. Refusing is better than returning a plausible wrong string. + raise AssertionError("literal block scalars are not supported for the group key") + if head.startswith(">"): + # Nine of the twenty-nine workflow-level keys are folded, including every + # required review workflow, so this is the majority shape rather than an + # edge case. YAML joins a folded scalar's lines with single spaces, so + # returning the indicator and the raw newlines would make the helper + # disagree with the file's own meaning. Blank lines and more-deeply + # indented lines inside a fold keep their newlines in YAML and are not + # handled here; neither shape occurs in this tree. + return " ".join(part.strip() for part in value[1:] if part.strip()) + return "\n".join(value).strip() + + +def workflow_level_cancels_in_progress(workflow: str) -> bool: + """Return whether the workflow-level block really sets ``cancel-in-progress: true``. + + Anchored to the start of a block line, so a commented-out setting cannot + satisfy it. Substring assertions could: commenting the real line out and + adding ``cancel-in-progress: false`` beside it leaves the searched text in + the file while YAML reads the opposite, and on 2026-09-06 that mutation + passed the whole suite (2958 passed, 0 failed) against ``noema-review.yml``. + A required review workflow that stops cancelling superseded runs keeps every + earlier review alive on each push, which is the queue behaviour this + repository has been trying to remove. + + Kept separate from the group helper on purpose: ``cancel-in-progress`` is a + sibling of ``group``, so it lies outside the value that helper returns and + cannot be covered by moving assertions onto it. + """ + block_match = WORKFLOW_LEVEL_CONCURRENCY_BLOCK.search(workflow) + if block_match is None: + raise AssertionError("workflow declares no workflow-level concurrency block") + return bool( + re.search(r"(?m)^[ \t]+cancel-in-progress:[ \t]+true[ \t]*$", block_match.group("body")) + ) + + def workflow_step(workflow: str, name: str) -> str: """Extract one named workflow step without parsing YAML dynamically.""" step = f" - name: {name}\n" @@ -112,7 +196,10 @@ def test_merge_scheduler_uses_native_auto_merge_after_required_checks() -> None: assert "github.event_name == 'repository_dispatch' && github.run_id" not in ( concurrency_contract ) - assert "cancel-in-progress: ${{" in concurrency_contract + # Anchored, not a substring: this workflow's value is an expression rather + # than a constant, so it cannot use the boolean helper, but a commented-out + # setting must not satisfy it either. + assert re.search(r"(?m)^[ \t]+cancel-in-progress:[ \t]+\$\{\{", concurrency_contract) assert "github.event_name == 'repository_dispatch'" in concurrency_contract @@ -234,22 +321,219 @@ def test_privileged_review_dispatch_coalesces_superseded_runs_before_admission() workflow = workflow_text("opencode-review-dispatch.yml") header = workflow.split("permissions:", 1)[0] concurrency_contract = header.split("concurrency:", 1)[1] + group_value = workflow_level_concurrency_group(workflow) assert re.search(r"(?m)^concurrency:", header) - assert "opencode-review-dispatch-" in concurrency_contract + assert "opencode-review-dispatch-" in group_value assert ( "github.event.client_payload.target_repository || github.repository" - in concurrency_contract + in group_value ) - assert ( - "github.event.client_payload.pr_number || github.run_id" - in concurrency_contract - ) - assert "cancel-in-progress: true" in concurrency_contract + assert "github.event.client_payload.pr_number || github.run_id" in group_value + assert workflow_level_cancels_in_progress(workflow) assert "github.event.client_payload.pr_head_sha" not in concurrency_contract assert re.search(r"(?m)^ concurrency:", workflow) +@pytest.mark.parametrize( + ("workflow_name", "group_prefix"), + ( + ("agent-mention-opencode-dispatch.yml", "agent-mention-opencode-"), + ("agent-mention-noema-dispatch.yml", "agent-mention-noema-"), + ), +) +def test_agent_mention_dispatch_coalesces_while_queued( + workflow_name: str, group_prefix: str +) -> None: + """A superseded agent mention must be discarded before it holds a queue slot. + + Both mention dispatchers carried the same defect + ``opencode-review-dispatch.yml`` carried before #1958: the group sat on the + single ``validate-and-forward`` job, and a job-level group is not evaluated + while the run waits behind the organization job ceiling. Measured on the + review dispatcher over the 39.7 hours ending 2026-09-06T12:41Z, 23 pairs of + runs for one pull request overlapped -- the older run was still open when its + successor arrived -- and none was coalesced; the five that ended + ``cancelled`` were cancelled between 0.7 and 2.9 hours after the newer run + was created, which is a sweep, not concurrency. + + The group moves to workflow level and is not duplicated on the job. Every + workflow here that keys a group at both levels (``strix.yml``, + ``opencode-review-dispatch.yml``) gives the two levels different names, + because a job that requests the group its own run already holds waits on + itself. + """ + workflow = workflow_text(workflow_name) + header = workflow.split("permissions:", 1)[0] + group = workflow_level_concurrency_group(workflow) + + assert re.search(r"(?m)^concurrency:", header) + # Read the group's value, not the block: the comment above these keys quotes + # the very expressions asserted here, so a raw-block assertion would survive + # the key being collapsed. That is the hole #1970 closed. + assert group.strip().startswith(group_prefix) + assert "github.event.client_payload.target_repository" in group + assert "github.event.client_payload.pr_number || github.run_id" in group + # ``cancel-in-progress`` is a sibling key, so it is outside the group value. + # Anchor it to its own line at the block's indent; a comment starts with + # ``#`` and cannot satisfy this. + assert re.search(r"(?m)^ cancel-in-progress: true$", header) + # ``\s`` also matches the newline before a column-0 key, so anchor the + # job-level search on horizontal whitespace only. + assert not re.search(r"(?m)^[ \t]+concurrency:", workflow) + + +def test_agent_mention_router_keeps_its_two_distinct_job_groups() -> None: + """The router must not be hoisted: its two jobs need different groups. + + ``agent-mention-router.yml`` runs a per-issue local route that supersedes + itself and an organization-wide sweep that must never be cancelled midway. + A workflow carries at most one workflow-level group, so hoisting either one + would silently give the sweep the route's ``cancel-in-progress: true`` and + let a later comment kill a sweep that is part way through the organization. + """ + workflow = workflow_text("agent-mention-router.yml") + + assert not re.search(r"(?m)^concurrency:", workflow) + assert ( + "group: review-agent-mention-router-local-${{ github.repository }}" + in workflow + ) + assert "group: review-agent-mention-router-sweep-${{ github.repository }}" in workflow + + sweep = workflow.split("sweep-organization-agent-mentions:", 1)[1] + # Anchored on the sweep JOB block: this router declares no workflow-level + # concurrency, so the sibling helper would raise rather than read it. + assert re.search( + r"(?m)^[ \t]+cancel-in-progress:[ \t]+false[ \t]*$", + sweep.split("steps:", 1)[0], + ) + +def test_concurrency_group_slice_ignores_the_comment_that_documents_it() -> None: + """A comment quoting the key must not satisfy an assertion about the key. + + This is the negative control for ``workflow_level_concurrency_group``. The + synthetic workflow below is exactly the shape that defeated the previous + contract: the real group is collapsed to the repository alone, so every pull + request in that repository shares one group and they cancel each other, while + a comment directly above still quotes both expressions the contract looks for. + Reading the raw block finds them; reading the group's value does not. + """ + defeated = textwrap.dedent( + """\ + name: Example + on: + repository_dispatch: + concurrency: + # Key: github.event.client_payload.target_repository || github.repository + # with github.event.client_payload.pr_number || github.run_id + group: opencode-review-dispatch-${{ github.repository }} + cancel-in-progress: true + permissions: + contents: read + """ + ) + raw_block = defeated.split("permissions:", 1)[0].split("concurrency:", 1)[1] + group_value = workflow_level_concurrency_group(defeated) + + assert "github.event.client_payload.pr_number || github.run_id" in raw_block + assert "github.event.client_payload.pr_number || github.run_id" not in group_value + assert "github.event.client_payload.target_repository" not in group_value + assert "opencode-review-dispatch-${{ github.repository }}" in group_value + + +def test_concurrency_group_slice_reads_a_folded_multi_line_key() -> None: + """The real key is a folded block, so the slice must join its continuation lines.""" + folded = textwrap.dedent( + """\ + concurrency: + group: >- + opencode-review-dispatch-${{ + github.event.client_payload.target_repository || github.repository }}-${{ + github.event.client_payload.pr_number || github.run_id }} + cancel-in-progress: true + permissions: + contents: read + """ + ) + group_value = workflow_level_concurrency_group(folded) + + assert ( + "github.event.client_payload.target_repository || github.repository" + in group_value + ) + assert "github.event.client_payload.pr_number || github.run_id" in group_value + assert "cancel-in-progress" not in group_value + + +def test_concurrency_helpers_read_the_block_when_permissions_comes_first() -> None: + """Key order must not decide whether the contract can see the block. + + The earlier helper sliced the text before ``permissions:`` and then split on + ``concurrency:``. That works only when ``concurrency:`` is declared first. Two + workflows in this repository declare ``permissions:`` above it -- + javascript-coverage-quality-ci.yml and repository-metadata-reconcile.yml -- + and for those the slice was empty, so the helper raised ``IndexError`` instead + of reading the block that is plainly there. Anchoring at column zero makes the + order irrelevant. + """ + permissions_first = textwrap.dedent( + """\ + name: Example + permissions: + contents: read + concurrency: + group: example-${{ github.repository }}-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: + build: + runs-on: ubuntu-latest + """ + ) + + assert ( + workflow_level_concurrency_group(permissions_first) + == "example-${{ github.repository }}-${{ github.event.pull_request.number }}" + ) + assert workflow_level_cancels_in_progress(permissions_first) + + +def test_concurrency_helpers_name_a_missing_block_instead_of_index_error() -> None: + """A workflow with no top-level block must fail with a sentence, not ``IndexError``. + + ``IndexError: list index out of range`` names neither the workflow nor the + contract it broke, so a reader has to reconstruct both from the traceback. + """ + no_block = "name: Example\njobs:\n build:\n runs-on: ubuntu-latest\n" + + for helper in (workflow_level_concurrency_group, workflow_level_cancels_in_progress): + with pytest.raises(AssertionError, match="no workflow-level concurrency block"): + helper(no_block) + + +def test_cancel_in_progress_assertion_rejects_a_commented_out_setting() -> None: + """The negative control for ``workflow_level_cancels_in_progress``. + + A substring test for ``cancel-in-progress: true`` is satisfied by a comment + that quotes it. On 2026-09-06 that exact mutation -- comment out the real line + in noema-review.yml, add ``cancel-in-progress: false`` beneath it -- passed the + whole suite (2958 passed, 0 failed) while every push to a pull request stopped + cancelling its own superseded run. Anchoring to the start of a block line is + what closes it. + """ + quoted_but_disabled = textwrap.dedent( + """\ + concurrency: + group: example-${{ github.repository }}-${{ github.event.pull_request.number }} + # cancel-in-progress: true + cancel-in-progress: false + """ + ) + + assert "cancel-in-progress: true" in quoted_but_disabled + assert not workflow_level_cancels_in_progress(quoted_but_disabled) + + def test_required_opencode_dispatch_does_not_wait_on_merge_scheduler() -> None: """Dispatch review execution directly so polling cannot starve its producer.""" workflow = workflow_text("opencode-review.yml") @@ -292,33 +576,32 @@ def test_required_pull_request_workflows_cancel_superseded_runs() -> None: concurrency_contract = workflow.split("concurrency:", 1)[1].split( "permissions:", 1 )[0] + group_value = workflow_level_concurrency_group(workflow) assert "concurrency:" in workflow - assert "github.event.pull_request.base.repo.full_name" in concurrency_contract - assert "github.repository" in concurrency_contract + assert "github.event.pull_request.base.repo.full_name" in group_value + assert "github.repository" in group_value assert "github.event.pull_request.number" in workflow assert re.search(r"(?m)^concurrency:", workflow) - assert "cancel-in-progress: true" in concurrency_contract + assert workflow_level_cancels_in_progress(workflow) if filename == "security-scan.yml": assert ( - "github.event_name == 'pull_request_target'" in concurrency_contract - or ("github.event_name == 'pull_request'" in concurrency_contract) + "github.event_name == 'pull_request_target'" in group_value + or ("github.event_name == 'pull_request'" in group_value) ) elif filename == "opencode-review.yml": - assert "required-opencode-review-${{" in concurrency_contract + assert "required-opencode-review-${{" in group_value assert "outputs.admitted == 'true'" in workflow elif filename == "noema-review.yml": assert not re.search(r"(?m)^ concurrency:", workflow) assert "github.event.workflow_run" not in concurrency_contract - assert "required-noema-review-${{" in concurrency_contract + assert "required-noema-review-${{" in group_value assert "outputs.admitted == 'true'" in workflow else: if filename == "codeql-pr.yml": - assert "github.event_name == 'pull_request'" in concurrency_contract + assert "github.event_name == 'pull_request'" in group_value else: - assert ( - "github.event_name == 'pull_request_target'" in concurrency_contract - ) + assert "github.event_name == 'pull_request_target'" in group_value assert "github.event.pull_request.head.sha" not in concurrency_contract assert "format('pr-{0}-{1}'" not in concurrency_contract @@ -342,12 +625,17 @@ def test_pr_quality_workflows_isolate_concurrency_by_repository_and_pr() -> None "${{ github.event.pull_request.number || github.ref }}" ) in concurrency if filename == "cloudflare-dns.yml": - assert ( - "cancel-in-progress: ${{ github.event_name == 'pull_request' }}" - in concurrency + # Anchored like the ``true`` contracts below: a commented-out setting + # must not satisfy this either, and this workflow deliberately cancels + # only for pull requests, so its value is an expression rather than a + # constant. + assert re.search( + r"(?m)^[ \t]+cancel-in-progress:[ \t]+\$\{\{ github\.event_name ==" + r" 'pull_request' \}\}[ \t]*$", + concurrency, ) else: - assert "cancel-in-progress: true" in concurrency + assert workflow_level_cancels_in_progress(workflow) def test_central_semgrep_logs_every_finding_and_distinguishes_engine_failure() -> None: @@ -425,21 +713,26 @@ def test_strix_serializes_provider_evidence_per_repository_and_pr() -> None: )[0] strix_job = workflow.split("\n strix:\n", 1)[1] + group_value = workflow_level_concurrency_group(workflow) + assert re.search(r"(?m)^concurrency:", workflow) - assert "needs: [changed-scope, admit-current-head]" in strix_job - assert "needs.admit-current-head.outputs.admitted == 'true'" in strix_job - assert "strix-security-scan-${{" in concurrency_contract - assert "github.event.pull_request.base.repo.full_name" in concurrency_contract - assert "github.event.client_payload.target_repository" in concurrency_contract - assert "github.event.pull_request.number" in concurrency_contract - assert "github.event.client_payload.pr_number" in concurrency_contract - assert "github.run_id" in concurrency_contract + assert "needs: [changed-scope]" in strix_job + assert "needs.changed-scope.outputs.admitted == 'true'" in strix_job + assert "strix-security-scan-${{" in group_value + assert "github.event.pull_request.base.repo.full_name" in group_value + assert "github.event.client_payload.target_repository" in group_value + assert "github.event.pull_request.number" in group_value + assert "github.event.client_payload.pr_number" in group_value + assert "github.run_id" in group_value assert "github.event.pull_request.head.sha" not in concurrency_contract assert "github.event.client_payload.pr_head_sha" not in concurrency_contract - assert "cancel-in-progress: true" in concurrency_contract + assert workflow_level_cancels_in_progress(workflow) assert " concurrency:" not in strix_job.split(" permissions:", 1)[0] assert "queue: max" not in workflow - assert workflow.index("admit-current-head:") < workflow.index("\n strix:\n") + changed_scope = workflow.split("\n changed-scope:\n", 1)[1].split( + "\n cancel-superseded-pr-runs:", 1 + )[0] + assert changed_scope.index("id: admission") < changed_scope.index("id: scope") cleanup_job = workflow.split(" cancel-superseded-pr-runs:", 1)[1].split( " strix:", 1 )[0] @@ -615,6 +908,32 @@ def test_strix_draft_transition_cancels_current_scan(tmp_path: Path) -> None: assert "/actions/runs/100/cancel" in calls +def test_pr_keyed_scan_workflows_pin_cancellation_as_a_value() -> None: + """Pin `cancel-in-progress` for the two PR-keyed scans that only had presence. + + Both appear in ``test_pull_request_close_events_cancel_superseded_runs_without_heavy_jobs``, + but in the branch that asserts the key is *present* rather than what it says. + That branch is shaped by ``pr-review-merge-scheduler.yml``, whose value is + deliberately an expression over ``github.event_name``, so the loop cannot + assert a constant for everyone in it. Nothing else read the flag: flipping + either to ``false`` left the whole suite green (2968 passed, 0 failed, + measured 2026-09-06). + + Kept out of ``test_required_pull_request_workflows_cancel_superseded_runs`` + because that loop ends by requiring a ``github.event_name`` discriminator in + the group, and these two key on + ``pull_request.number || github.ref`` with no event-name term. Adding them + there would need a branch that asserts nothing. + """ + for filename in ("python-security.yml", "sast-semgrep.yml"): + workflow = workflow_text(filename) + group_value = workflow_level_concurrency_group(workflow) + + assert workflow_level_cancels_in_progress(workflow) + assert "github.event.pull_request.number" in group_value + assert "github.event_name" not in group_value + + def test_pull_request_close_events_cancel_superseded_runs_without_heavy_jobs() -> None: """Close events should cancel old runs without starting expensive jobs.""" workflows = ( @@ -675,7 +994,9 @@ def test_pull_request_close_events_cancel_superseded_runs_without_heavy_jobs() - )[0] assert "github.event.pull_request.number" in concurrency_contract assert "github.event.pull_request.head.sha" not in concurrency_contract - assert "cancel-in-progress:" in concurrency_contract + assert re.search( + r"(?m)^[ \t]+cancel-in-progress:[ \t]+\S", concurrency_contract + ) else: raise AssertionError(f"unclassified close-event workflow: {filename}") assert "github.event.action != 'closed'" in workflow @@ -690,11 +1011,12 @@ def test_pull_request_close_events_cancel_superseded_runs_without_heavy_jobs() - assert "${{ secrets." not in opencode_bootstrap strix_workflow = workflow_text("strix.yml") - # Strix admits the live head before same-PR cancellation while cleanup stays - # outside that queue so synchronize and close events can retire old work. - assert "admit-current-head:" in strix_workflow + # Strix admits the live head inside changed-scope while cleanup stays outside + # that queue so synchronize and close events can retire old work. + assert "\n admit-current-head:\n" not in strix_workflow + assert "id: admission" in strix_workflow assert "skipping stale evidence" in strix_workflow - assert "cancel-in-progress: true" in strix_workflow + assert workflow_level_cancels_in_progress(strix_workflow) def test_merge_scheduler_owns_empty_pr_cleanup_without_checkout() -> None: @@ -756,7 +1078,7 @@ def test_noema_triggers_preserve_standalone_pull_request_review() -> None: assert "github.event_name" not in concurrency_contract.split( "cancel-in-progress:", 1 )[0] - assert "cancel-in-progress: true" in concurrency_contract + assert workflow_level_cancels_in_progress(workflow) assert re.search(r"(?m)^concurrency:", workflow) assert not re.search(r"(?m)^ concurrency:", workflow) assert "needs.admit-current-head.outputs.admitted == 'true'" in noema_job @@ -1073,7 +1395,7 @@ def test_fix_scheduler_cancels_superseded_cron_runs() -> None: workflow = workflow_text("pr-review-fix-scheduler.yml") assert "central-pr-review-fix-scheduler-" in workflow - assert "cancel-in-progress: true" in workflow + assert workflow_level_cancels_in_progress(workflow) def test_security_scan_fails_closed_when_dependency_review_is_unavailable() -> None: