From 6244e4e17c9cd527584d07d384eeddb589de1470 Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Sat, 24 Oct 2026 20:53:27 +0200 Subject: [PATCH 1/2] bugfix-687-fixworkflows-isolate-pr-concurrency-by-pull-request-identity: scope review concurrency to pull request identity --- .github/workflows/copilot_pull_request.yml | 2 +- .../copilot_pull_request_review_state.yml | 2 +- docs/features.mdx | 4 +- docs/pull-requests/examples.mdx | 4 +- docs/pull-requests/workflow-setup.mdx | 13 +++++- scripts/validate-workflow-contract.cjs | 6 +-- setup/workflows/copilot_pull_request.yml | 2 +- .../copilot_pull_request_review_state.yml | 2 +- specs/CATALOG.md | 8 ++-- specs/bugbot-review-state-reconciliation.md | 41 +++++++++++++++++-- specs/catalog.json | 10 +++-- .../validate_workflow_contract.test.ts | 38 +++++++++++++++-- 12 files changed, 105 insertions(+), 27 deletions(-) diff --git a/.github/workflows/copilot_pull_request.yml b/.github/workflows/copilot_pull_request.yml index 928453a51..24fb417db 100644 --- a/.github/workflows/copilot_pull_request.yml +++ b/.github/workflows/copilot_pull_request.yml @@ -16,7 +16,7 @@ jobs: runs-on: [self-hosted, codex, Linux] timeout-minutes: 120 concurrency: - group: copilot-pr-${{ github.repository }}-${{ github.event.pull_request.head.ref || github.ref_name }}-analysis + group: copilot-pr-${{ github.repository }}-${{ github.event.pull_request.number }}-analysis cancel-in-progress: true permissions: checks: write diff --git a/.github/workflows/copilot_pull_request_review_state.yml b/.github/workflows/copilot_pull_request_review_state.yml index eb5879069..63cedc3b4 100644 --- a/.github/workflows/copilot_pull_request_review_state.yml +++ b/.github/workflows/copilot_pull_request_review_state.yml @@ -12,7 +12,7 @@ jobs: runs-on: [self-hosted, codex, Linux] timeout-minutes: 120 concurrency: - group: copilot-pr-${{ github.repository }}-${{ github.event.pull_request.head.ref || github.ref_name }}-review-state + group: copilot-pr-${{ github.repository }}-${{ github.event.pull_request.number }}-review-state cancel-in-progress: true permissions: checks: write diff --git a/docs/features.mdx b/docs/features.mdx index e66982ccc..8339fd373 100644 --- a/docs/features.mdx +++ b/docs/features.mdx @@ -169,7 +169,7 @@ decorative media or expose configuration for it. **Coordination policies:** issue and comment mutation workflows preserve admitted events in a workflow-local queue. Deployment mutations use a shared issue-scoped native queue. Commit and Pull Request each have a separate repository/branch latest-revision lane, so paired events cannot cancel one another. -GitHub's native [concurrency](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#concurrency) can cancel in-progress runs when a newer one starts. Its default `queue: single` retains only one pending run; `queue: max` allows up to 100 pending jobs or workflow runs when `cancel-in-progress` is `false` or omitted. The replaceable branch jobs therefore use event-specific keys: Commit uses `copilot-push-${{ github.repository }}-${{ github.ref_name }}`; Pull Request code/lifecycle analysis uses the branch key with an `analysis` suffix; and the dedicated review-state workflow uses the same branch key with a `review-state` suffix. Each lane cancels only its own obsolete revision. Commit still synchronizes issue progress, then Bugbot performs a read-only exact-head lookup because a `push` payload does not contain PR identity. A validated open same-repository PR makes the push stop before review-context loading or agent invocation; the PR `synchronize` event exclusively owns review for that head. This prevents duplicate reviews without letting a push or review-state event cancel useful PR analysis. Branches without an open PR retain push-time Bugbot. Metadata-only `pull_request: edited` events are not subscribed because Copilot changes the PR body itself; excluding them prevents self-generated run cascades and preserves human metadata edits. Application-level head guards prevent an older run from updating a newer PR, and the next code/lifecycle run repairs any durable half-transition discovered after cancellation. Fixed workflow/job names expose analysis and review-state checks without leaking an unevaluated expression when GitHub skips bot-authored events. A separate merge-group workflow avoids a skipped duplicate on normal PRs while deliberately preserving the exact `Copilot - Pull Request` required-check context for branch-protection compatibility. +GitHub's native [concurrency](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#concurrency) can cancel in-progress runs when a newer one starts. Its default `queue: single` retains only one pending run; `queue: max` allows up to 100 pending jobs or workflow runs when `cancel-in-progress` is `false` or omitted. The replaceable event jobs therefore use event-specific keys: Commit uses `copilot-push-${{ github.repository }}-${{ github.ref_name }}`; Pull Request code/lifecycle analysis uses the repository and PR number with an `analysis` suffix; and the dedicated review-state workflow uses the same PR-number key with a `review-state` suffix. Different PRs sharing a head branch cannot cancel one another. Each PR lane cancels only its own obsolete revision. Commit still synchronizes issue progress, then Bugbot performs a read-only exact-head lookup because a `push` payload does not contain PR identity. A validated open same-repository PR makes the push stop before review-context loading or agent invocation; the PR `synchronize` event exclusively owns review for that head. This prevents duplicate reviews without letting a push or review-state event cancel useful PR analysis. Branches without an open PR retain push-time Bugbot. Metadata-only `pull_request: edited` events are not subscribed because Copilot changes the PR body itself; excluding them prevents self-generated run cascades and preserves human metadata edits. Application-level head guards prevent an older run from updating a newer PR, and the next code/lifecycle run repairs any durable half-transition discovered after cancellation. Fixed workflow/job names expose analysis and review-state checks without leaking an unevaluated expression when GitHub skips bot-authored events. A separate merge-group workflow avoids a skipped duplicate on normal PRs while deliberately preserving the exact `Copilot - Pull Request` required-check context for branch-protection compatibility. For non-replaceable issue and comment mutations, Copilot adds an application-level queue per workflow: every started run waits for earlier active runs of that same workflow, so intermediate events are not discarded by a native concurrency group. Runs triggered by the PAT owner that would only re-trigger the normal pipeline complete before entering this queue. @@ -221,7 +221,7 @@ jobs: project-ids: '2,3' ``` -Issue and comment mutation workflows use the application-level queue above to preserve intermediate events. Release, hotfix, and managed-PR deployment mutations use native `concurrency` with `queue: max` and `cancel-in-progress: false`, which preserves pending deployment runs and is valid GitHub Actions syntax. Without `queue: max`, native concurrency retains only one pending run and can discard intermediate mutations. See [deployment concurrency validation](/issues/deployment-orchestration#validating-queue-max) before changing generated workflows in response to a linter error. Commit and Pull Request use separate native groups with cancellation for their own replaceable branch revisions, so they cannot cancel one another. Native concurrency also remains appropriate for replaceable read-only workflows such as CI and the optional, manually dispatched RepoWise analysis. +Issue and comment mutation workflows use the application-level queue above to preserve intermediate events. Release, hotfix, and managed-PR deployment mutations use native `concurrency` with `queue: max` and `cancel-in-progress: false`, which preserves pending deployment runs and is valid GitHub Actions syntax. Without `queue: max`, native concurrency retains only one pending run and can discard intermediate mutations. See [deployment concurrency validation](/issues/deployment-orchestration#validating-queue-max) before changing generated workflows in response to a linter error. Commit and Pull Request use separate native groups with cancellation for their own replaceable branch or PR revisions, so they cannot cancel one another. Native concurrency also remains appropriate for replaceable read-only workflows such as CI and the optional, manually dispatched RepoWise analysis. --- diff --git a/docs/pull-requests/examples.mdx b/docs/pull-requests/examples.mdx index ead600b0b..f0cc68a92 100644 --- a/docs/pull-requests/examples.mdx +++ b/docs/pull-requests/examples.mdx @@ -24,7 +24,7 @@ jobs: name: Copilot - Pull Request runs-on: ubuntu-latest concurrency: - group: copilot-pr-${{ github.repository }}-${{ github.event.pull_request.head.ref || github.ref_name }}-analysis + group: copilot-pr-${{ github.repository }}-${{ github.event.pull_request.number }}-analysis cancel-in-progress: true steps: - name: Checkout Repository @@ -78,7 +78,7 @@ jobs: name: Copilot - Pull Request Review State runs-on: ubuntu-latest concurrency: - group: copilot-pr-${{ github.repository }}-${{ github.event.pull_request.head.ref || github.ref_name }}-review-state + group: copilot-pr-${{ github.repository }}-${{ github.event.pull_request.number }}-review-state cancel-in-progress: true steps: - uses: actions/checkout@v5 diff --git a/docs/pull-requests/workflow-setup.mdx b/docs/pull-requests/workflow-setup.mdx index b8dd08688..0ef51e1ee 100644 --- a/docs/pull-requests/workflow-setup.mdx +++ b/docs/pull-requests/workflow-setup.mdx @@ -91,7 +91,7 @@ jobs: name: Copilot - Pull Request runs-on: ubuntu-latest concurrency: - group: copilot-pr-${{ github.repository }}-${{ github.event.pull_request.head.ref || github.ref_name }}-analysis + group: copilot-pr-${{ github.repository }}-${{ github.event.pull_request.number }}-analysis cancel-in-progress: true steps: - name: Checkout Repository @@ -120,11 +120,20 @@ jobs: Install the shipped `copilot_pull_request_review_state.yml` alongside it. That workflow listens only to `pull_request_review`, uses the fixed `Copilot - Pull Request Review State` workflow/job name, and uses the same -branch key with a `review-state` suffix. +PR-number key with a `review-state` suffix. + +Existing installations retain their workflow files when the `@v3` action moves. +To adopt PR-number groups, use a released CLI containing this fix and review +`copilot setup --web --update-workflows`, or apply the two group expressions in a +reviewed maintenance PR. Update analysis and review-state together and retain +project-specific setup/release customizations. No new Variable, Secret or +permission is required. Current-head checks and review remain the merge gate. - **`token`** is required (use a fine-grained PAT with repo and project permissions; see [Authentication](/authentication)). - **`project-ids`** is optional but needed if you want PRs linked to GitHub Project boards and moved to columns (e.g. "In Progress"). - The conditional fetch downloads only the two commit objects needed by the incremental reviewer. Keep it when `synchronize` is enabled; a full-history checkout is not required. +- Different PR numbers use independent groups even when they share a head branch + and target different bases. Repeated events retain the same PR/lane identity. - Keep both PR-specific concurrency groups and cancellation policies exactly as shown. A newer code/lifecycle event cancels only obsolete analysis; a newer review event cancels only obsolete review-state observation. The Commit diff --git a/scripts/validate-workflow-contract.cjs b/scripts/validate-workflow-contract.cjs index 8bcae2918..caabb3230 100644 --- a/scripts/validate-workflow-contract.cjs +++ b/scripts/validate-workflow-contract.cjs @@ -31,8 +31,8 @@ const ISOLATED_REPOWISE_VENV_NAME = 'repowise-venv-${{ github.run_id }}-${{ gith const ISOLATED_REPOWISE_REPORT_NAME = 'repowise-report-${{ github.run_id }}-${{ github.run_attempt }}-code-health'; const CRLF_WHITESPACE = 'core.whitespace=blank-at-eol,blank-at-eof,space-before-tab,cr-at-eol'; const PUSH_BRANCH_CONCURRENCY_GROUP = 'copilot-push-${{ github.repository }}-${{ github.ref_name }}'; -const PULL_REQUEST_ANALYSIS_CONCURRENCY_GROUP = 'copilot-pr-${{ github.repository }}-${{ github.event.pull_request.head.ref || github.ref_name }}-analysis'; -const PULL_REQUEST_REVIEW_STATE_CONCURRENCY_GROUP = 'copilot-pr-${{ github.repository }}-${{ github.event.pull_request.head.ref || github.ref_name }}-review-state'; +const PULL_REQUEST_ANALYSIS_CONCURRENCY_GROUP = 'copilot-pr-${{ github.repository }}-${{ github.event.pull_request.number }}-analysis'; +const PULL_REQUEST_REVIEW_STATE_CONCURRENCY_GROUP = 'copilot-pr-${{ github.repository }}-${{ github.event.pull_request.number }}-review-state'; const BUGBOT_CONCURRENCY_JOBS = Object.freeze({ 'copilot_commit.yml': Object.freeze({ jobId: 'copilot-commits', @@ -703,7 +703,7 @@ function assertReviewConcurrency(relativeFile, workflow) { } if (job.concurrency?.group !== group || job.concurrency?.['cancel-in-progress'] !== cancelInProgress) { - throw new Error(`${relativeFile} job ${jobId} must use its workflow- and event-specific branch group, avoid cross-canceling another event owner, and cancel superseded runs within its lane.`); + throw new Error(`${relativeFile} job ${jobId} must use its workflow- and event-specific group (PR number for pull requests), avoid cross-canceling another event owner, and cancel superseded runs within its lane.`); } } } diff --git a/setup/workflows/copilot_pull_request.yml b/setup/workflows/copilot_pull_request.yml index 1127ec283..f4115b643 100644 --- a/setup/workflows/copilot_pull_request.yml +++ b/setup/workflows/copilot_pull_request.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 120 concurrency: - group: copilot-pr-${{ github.repository }}-${{ github.event.pull_request.head.ref || github.ref_name }}-analysis + group: copilot-pr-${{ github.repository }}-${{ github.event.pull_request.number }}-analysis cancel-in-progress: true permissions: checks: write diff --git a/setup/workflows/copilot_pull_request_review_state.yml b/setup/workflows/copilot_pull_request_review_state.yml index 8942d1d7f..b6c2fda4f 100644 --- a/setup/workflows/copilot_pull_request_review_state.yml +++ b/setup/workflows/copilot_pull_request_review_state.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 120 concurrency: - group: copilot-pr-${{ github.repository }}-${{ github.event.pull_request.head.ref || github.ref_name }}-review-state + group: copilot-pr-${{ github.repository }}-${{ github.event.pull_request.number }}-review-state cancel-in-progress: true permissions: checks: write diff --git a/specs/CATALOG.md b/specs/CATALOG.md index a143eb019..a0d046df3 100644 --- a/specs/CATALOG.md +++ b/specs/CATALOG.md @@ -13,7 +13,7 @@ debt or convert unknown historic intent into a design decision. | `github-communication-experience` | Implemented | English-default, localized, semantic, bounded, and idempotent product messages across GitHub and repository-aware operator surfaces | [Semantic GitHub communication and repository localization](./semantic-github-publication-and-notification.md) + 3 companion | 277 paths · 2026-10-20 | | `release-orchestration` | Implemented | Release and hotfix promotion, publication, bounded merge readiness recovery, and receipt-verified reconciliation without republication | [Configurable production-first release orchestration](./configurable-release-orchestration.md) + 2 companion | 59 paths · 2026-10-16 | | `merge-queue-readiness` | Implemented | Fail-closed validation of required checks and merge-group workflow support; optional manual-only RepoWise analysis with independent CI and verified required-check migration; cached RepoWise exports and bounded full-history secret processing with truthful completeness and resource evidence | [Merge queue readiness and effective target rules](./merge-queue-readiness.md) | 29 paths · 2026-10-21 | -| `bugbot-review-state-reconciliation` | Implemented | Reconcile review snapshots, findings, threads, comments, and check conclusions | [Bugbot review-state reconciliation](./bugbot-review-state-reconciliation.md) | 70 paths · 2026-10-21 | +| `bugbot-review-state-reconciliation` | Implemented | Reconcile review snapshots, findings, threads, comments, and check conclusions | [Bugbot review-state reconciliation](./bugbot-review-state-reconciliation.md) | 74 paths · 2026-10-24 | | `execution-lifecycle` | Implemented | Shared GitHub Action lifecycle from event admission through durable user-facing results | [Execution admission, queueing, routing, and result publication](./execution-admission-queue-and-publication.md) + 3 companion | 105 paths · 2026-10-21 | | `architecture-quality-hardening` | Implemented | Close verified concurrency, error-contract, context-coupling, fan-out, setup/doctor, and provider-policy risks in dependency order | [Architecture quality and scalability hardening](./architecture-quality-and-scalability-hardening.md) + 1 companion | 72 paths · 2026-10-13 | | `setup-and-doctor` | Implemented | Plan, validate, provision, and audit a repository installation without exposing credentials | [Setup, configuration, credentials, and doctor](./setup-configuration-credentials-and-doctor.md) + 6 companion | 332 paths · 2026-10-24 | @@ -70,11 +70,11 @@ debt or convert unknown historic intent into a design decision. ### `bugbot-review-state-reconciliation` — Bugbot review-state reconciliation - Owner: Copilot maintainers -- Last verified: 2026-10-21 +- Last verified: 2026-10-24 - Specifications: [`specs/bugbot-review-state-reconciliation.md`](./bugbot-review-state-reconciliation.md) -- Workflows: [`.github/workflows/copilot_commit.yml`](../.github/workflows/copilot_commit.yml) · [`.github/workflows/copilot_pull_request.yml`](../.github/workflows/copilot_pull_request.yml) · [`.github/workflows/copilot_pull_request_comment.yml`](../.github/workflows/copilot_pull_request_comment.yml) +- Workflows: [`.github/workflows/copilot_commit.yml`](../.github/workflows/copilot_commit.yml) · [`.github/workflows/copilot_pull_request.yml`](../.github/workflows/copilot_pull_request.yml) · [`.github/workflows/copilot_pull_request_comment.yml`](../.github/workflows/copilot_pull_request_comment.yml) · [`.github/workflows/copilot_pull_request_review_state.yml`](../.github/workflows/copilot_pull_request_review_state.yml) · [`setup/workflows/copilot_pull_request.yml`](../setup/workflows/copilot_pull_request.yml) · [`setup/workflows/copilot_pull_request_review_state.yml`](../setup/workflows/copilot_pull_request_review_state.yml) - Entrypoints: [`src/application/usecases/steps/commit/detect_potential_problems_workflow.ts`](../src/application/usecases/steps/commit/detect_potential_problems_workflow.ts) · [`src/application/usecases/steps/commit/bugbot/reconcile_bugbot_review_state_use_case.ts`](../src/application/usecases/steps/commit/bugbot/reconcile_bugbot_review_state_use_case.ts) · [`src/actions/github_action_completion.ts`](../src/actions/github_action_completion.ts) · [`src/api.ts`](../src/api.ts) -- Core code: [`scripts/coverage-budgets.json`](../scripts/coverage-budgets.json) · [`src/domain/bugbot/review_state.ts`](../src/domain/bugbot/review_state.ts) · [`src/domain/bugbot/review_projection.ts`](../src/domain/bugbot/review_projection.ts) · [`src/application/policies/bugbot_reconciliation_policy.ts`](../src/application/policies/bugbot_reconciliation_policy.ts) · [`src/application/policies/bugbot_event_ownership_policy.ts`](../src/application/policies/bugbot_event_ownership_policy.ts) · [`src/application/policies/bugbot_result_finding_state_projection_policy.ts`](../src/application/policies/bugbot_result_finding_state_projection_policy.ts) · [`src/application/policies/bugbot_telemetry_projection_policy.ts`](../src/application/policies/bugbot_telemetry_projection_policy.ts) · [`src/application/policies/bugbot_message_catalog.ts`](../src/application/policies/bugbot_message_catalog.ts) · [`src/application/policies/bugbot_finding_marker_policy.ts`](../src/application/policies/bugbot_finding_marker_policy.ts) · [`src/application/policies/bugbot_review_presentation_policy.ts`](../src/application/policies/bugbot_review_presentation_policy.ts) · [`src/application/policies/action_summary_policy.ts`](../src/application/policies/action_summary_policy.ts) · [`src/application/policies/copilot_evidence_policy.ts`](../src/application/policies/copilot_evidence_policy.ts) · [`src/application/policies/lifecycle_state_policy.ts`](../src/application/policies/lifecycle_state_policy.ts) · [`src/application/policies/status_command_policy.ts`](../src/application/policies/status_command_policy.ts) · [`src/application/usecases/steps/common/publish_resume_workflow.ts`](../src/application/usecases/steps/common/publish_resume_workflow.ts) · [`src/application/usecases/steps/commit/bugbot/load_bugbot_context_use_case.ts`](../src/application/usecases/steps/commit/bugbot/load_bugbot_context_use_case.ts) · [`src/application/usecases/steps/commit/bugbot/schema.ts`](../src/application/usecases/steps/commit/bugbot/schema.ts) · [`src/application/usecases/steps/commit/bugbot/prepare_bugbot_findings_policy.ts`](../src/application/usecases/steps/commit/bugbot/prepare_bugbot_findings_policy.ts) · [`src/application/usecases/steps/commit/bugbot/publish_pr_review_comments.ts`](../src/application/usecases/steps/commit/bugbot/publish_pr_review_comments.ts) · [`src/application/usecases/steps/commit/bugbot/synchronize_bugbot_review_presentation_use_case.ts`](../src/application/usecases/steps/commit/bugbot/synchronize_bugbot_review_presentation_use_case.ts) · [`src/application/policies/bugbot_status_count_marker_policy.ts`](../src/application/policies/bugbot_status_count_marker_policy.ts) · [`src/application/errors/bugbot_mutation_error.ts`](../src/application/errors/bugbot_mutation_error.ts) · [`src/data/repository/pull_request/pull_request_review_error_metadata.ts`](../src/data/repository/pull_request/pull_request_review_error_metadata.ts) · [`src/domain/bugbot/finding.ts`](../src/domain/bugbot/finding.ts) · [`src/application/usecases/steps/commit/bugbot/analyze_bugbot_revision_use_case.ts`](../src/application/usecases/steps/commit/bugbot/analyze_bugbot_revision_use_case.ts) · [`src/application/usecases/steps/commit/bugbot/publish_findings_use_case.ts`](../src/application/usecases/steps/commit/bugbot/publish_findings_use_case.ts) +- Core code: [`scripts/coverage-budgets.json`](../scripts/coverage-budgets.json) · [`src/domain/bugbot/review_state.ts`](../src/domain/bugbot/review_state.ts) · [`src/domain/bugbot/review_projection.ts`](../src/domain/bugbot/review_projection.ts) · [`src/application/policies/bugbot_reconciliation_policy.ts`](../src/application/policies/bugbot_reconciliation_policy.ts) · [`src/application/policies/bugbot_event_ownership_policy.ts`](../src/application/policies/bugbot_event_ownership_policy.ts) · [`src/application/policies/bugbot_result_finding_state_projection_policy.ts`](../src/application/policies/bugbot_result_finding_state_projection_policy.ts) · [`src/application/policies/bugbot_telemetry_projection_policy.ts`](../src/application/policies/bugbot_telemetry_projection_policy.ts) · [`src/application/policies/bugbot_message_catalog.ts`](../src/application/policies/bugbot_message_catalog.ts) · [`src/application/policies/bugbot_finding_marker_policy.ts`](../src/application/policies/bugbot_finding_marker_policy.ts) · [`src/application/policies/bugbot_review_presentation_policy.ts`](../src/application/policies/bugbot_review_presentation_policy.ts) · [`src/application/policies/action_summary_policy.ts`](../src/application/policies/action_summary_policy.ts) · [`src/application/policies/copilot_evidence_policy.ts`](../src/application/policies/copilot_evidence_policy.ts) · [`src/application/policies/lifecycle_state_policy.ts`](../src/application/policies/lifecycle_state_policy.ts) · [`src/application/policies/status_command_policy.ts`](../src/application/policies/status_command_policy.ts) · [`src/application/usecases/steps/common/publish_resume_workflow.ts`](../src/application/usecases/steps/common/publish_resume_workflow.ts) · [`src/application/usecases/steps/commit/bugbot/load_bugbot_context_use_case.ts`](../src/application/usecases/steps/commit/bugbot/load_bugbot_context_use_case.ts) · [`src/application/usecases/steps/commit/bugbot/schema.ts`](../src/application/usecases/steps/commit/bugbot/schema.ts) · [`src/application/usecases/steps/commit/bugbot/prepare_bugbot_findings_policy.ts`](../src/application/usecases/steps/commit/bugbot/prepare_bugbot_findings_policy.ts) · [`src/application/usecases/steps/commit/bugbot/publish_pr_review_comments.ts`](../src/application/usecases/steps/commit/bugbot/publish_pr_review_comments.ts) · [`src/application/usecases/steps/commit/bugbot/synchronize_bugbot_review_presentation_use_case.ts`](../src/application/usecases/steps/commit/bugbot/synchronize_bugbot_review_presentation_use_case.ts) · [`src/application/policies/bugbot_status_count_marker_policy.ts`](../src/application/policies/bugbot_status_count_marker_policy.ts) · [`src/application/errors/bugbot_mutation_error.ts`](../src/application/errors/bugbot_mutation_error.ts) · [`src/data/repository/pull_request/pull_request_review_error_metadata.ts`](../src/data/repository/pull_request/pull_request_review_error_metadata.ts) · [`src/domain/bugbot/finding.ts`](../src/domain/bugbot/finding.ts) · [`src/application/usecases/steps/commit/bugbot/analyze_bugbot_revision_use_case.ts`](../src/application/usecases/steps/commit/bugbot/analyze_bugbot_revision_use_case.ts) · [`src/application/usecases/steps/commit/bugbot/publish_findings_use_case.ts`](../src/application/usecases/steps/commit/bugbot/publish_findings_use_case.ts) · [`scripts/validate-workflow-contract.cjs`](../scripts/validate-workflow-contract.cjs) - Tests: [`src/domain/bugbot/__tests__/review_state.test.ts`](../src/domain/bugbot/__tests__/review_state.test.ts) · [`src/domain/bugbot/__tests__/review_projection.test.ts`](../src/domain/bugbot/__tests__/review_projection.test.ts) · [`src/application/usecases/steps/commit/bugbot/__tests__/schema.test.ts`](../src/application/usecases/steps/commit/bugbot/__tests__/schema.test.ts) · [`src/application/usecases/steps/commit/bugbot/__tests__/prepare_bugbot_findings.test.ts`](../src/application/usecases/steps/commit/bugbot/__tests__/prepare_bugbot_findings.test.ts) · [`src/application/usecases/steps/commit/bugbot/__tests__/reconcile_bugbot_review_state_integration.test.ts`](../src/application/usecases/steps/commit/bugbot/__tests__/reconcile_bugbot_review_state_integration.test.ts) · [`src/application/usecases/steps/commit/bugbot/__tests__/synchronize_bugbot_review_presentation_use_case.test.ts`](../src/application/usecases/steps/commit/bugbot/__tests__/synchronize_bugbot_review_presentation_use_case.test.ts) · [`src/application/policies/__tests__/bugbot_result_finding_state_projection_policy.test.ts`](../src/application/policies/__tests__/bugbot_result_finding_state_projection_policy.test.ts) · [`src/application/policies/__tests__/bugbot_event_ownership_policy.test.ts`](../src/application/policies/__tests__/bugbot_event_ownership_policy.test.ts) · [`src/application/policies/__tests__/bugbot_telemetry_projection_policy.test.ts`](../src/application/policies/__tests__/bugbot_telemetry_projection_policy.test.ts) · [`src/application/policies/__tests__/bugbot_message_catalog.test.ts`](../src/application/policies/__tests__/bugbot_message_catalog.test.ts) · [`src/application/policies/__tests__/bugbot_finding_marker_policy.test.ts`](../src/application/policies/__tests__/bugbot_finding_marker_policy.test.ts) · [`src/application/policies/__tests__/bugbot_review_presentation_policy.test.ts`](../src/application/policies/__tests__/bugbot_review_presentation_policy.test.ts) · [`src/application/policies/__tests__/action_summary_policy.test.ts`](../src/application/policies/__tests__/action_summary_policy.test.ts) · [`src/application/policies/__tests__/copilot_evidence_policy.test.ts`](../src/application/policies/__tests__/copilot_evidence_policy.test.ts) · [`src/application/policies/__tests__/lifecycle_state_policy.test.ts`](../src/application/policies/__tests__/lifecycle_state_policy.test.ts) · [`src/application/policies/__tests__/status_command_policy.test.ts`](../src/application/policies/__tests__/status_command_policy.test.ts) · [`src/actions/__tests__/github_action_completion.test.ts`](../src/actions/__tests__/github_action_completion.test.ts) · [`src/application/usecases/steps/common/__tests__/publish_resume_use_case.test.ts`](../src/application/usecases/steps/common/__tests__/publish_resume_use_case.test.ts) · [`src/application/usecases/actions/__tests__/synchronize_lifecycle_state_use_case.test.ts`](../src/application/usecases/actions/__tests__/synchronize_lifecycle_state_use_case.test.ts) · [`src/application/usecases/steps/commit/__tests__/detect_potential_problems_use_case.test.ts`](../src/application/usecases/steps/commit/__tests__/detect_potential_problems_use_case.test.ts) · [`src/infrastructure/composition/__tests__/pull_request_use_case_composition_root.test.ts`](../src/infrastructure/composition/__tests__/pull_request_use_case_composition_root.test.ts) · [`src/tooling/__tests__/validate_workflow_contract.test.ts`](../src/tooling/__tests__/validate_workflow_contract.test.ts) · [`src/application/errors/__tests__/bugbot_mutation_error.test.ts`](../src/application/errors/__tests__/bugbot_mutation_error.test.ts) · [`src/application/usecases/steps/commit/bugbot/__tests__/finding_mutation_recovery.test.ts`](../src/application/usecases/steps/commit/bugbot/__tests__/finding_mutation_recovery.test.ts) · [`src/domain/bugbot/__tests__/finding.test.ts`](../src/domain/bugbot/__tests__/finding.test.ts) · [`src/application/usecases/steps/commit/bugbot/__tests__/finding_identity_replay.test.ts`](../src/application/usecases/steps/commit/bugbot/__tests__/finding_identity_replay.test.ts) · [`src/application/usecases/steps/commit/bugbot/__tests__/analyze_bugbot_revision_use_case.test.ts`](../src/application/usecases/steps/commit/bugbot/__tests__/analyze_bugbot_revision_use_case.test.ts) · [`src/application/usecases/steps/commit/bugbot/__tests__/publish_findings_use_case.test.ts`](../src/application/usecases/steps/commit/bugbot/__tests__/publish_findings_use_case.test.ts) - User documentation: [`docs/bugbot/detection.mdx`](../docs/bugbot/detection.mdx) · [`docs/bugbot/finding-publication.mdx`](../docs/bugbot/finding-publication.mdx) · [`docs/bugbot/quality-observability.mdx`](../docs/bugbot/quality-observability.mdx) · [`docs/bugbot/failure-scenarios.mdx`](../docs/bugbot/failure-scenarios.mdx) · [`docs/bugbot/configuration.mdx`](../docs/bugbot/configuration.mdx) · [`docs/bugbot/how-it-works.mdx`](../docs/bugbot/how-it-works.mdx) · [`docs/bugbot/programmatic-api.mdx`](../docs/bugbot/programmatic-api.mdx) · [`docs/pull-requests/workflow-setup.mdx`](../docs/pull-requests/workflow-setup.mdx) · [`docs/issues/comment-commands.mdx`](../docs/issues/comment-commands.mdx) diff --git a/specs/bugbot-review-state-reconciliation.md b/specs/bugbot-review-state-reconciliation.md index 8cee3fc89..cb5a70d05 100644 --- a/specs/bugbot-review-state-reconciliation.md +++ b/specs/bugbot-review-state-reconciliation.md @@ -488,13 +488,48 @@ readiness. `unknown` is a system failure and fails the review regardless of ### 6.5 Duplicate, stale, and out-of-order behavior +#### PR identity isolation — issue #687 + +The former head-branch key collided for simultaneous PRs to different bases. +PR number is now the fixed, non-configurable owner within each PR lane; check +names, permissions, event filters and application head/base guards stay in force. +No new domain service, persisted state or user setting is needed: workflow +adapters declare the identity and the executable workflow contract enforces it. + +| Illustrative events | Native groups | Visible effect | +|---|---|---| +| PR 21 and PR 22, both from master | Separate analysis and observer groups per PR | Each PR retains its own checks | +| Two revisions of PR 21 | Same PR/lane group | Only its superseded run may be canceled | +| PR 21 analysis and PR 21 review event | Distinct lane suffixes | Observation cannot cancel analysis | + +Text equivalent: one branch may own multiple PRs; each PR owns independent +analysis and review-state lanes. A later revision replaces only work within the +same PR and lane. Existing pending, complete, failed and superseded check copy +and fixed check names remain the user UI; this change adds no comment surface. +On rollout an old branch-key run can overlap a new PR-key run, so freshness and +idempotency guards remain required. Canceled old runs never certify readiness; +inspect the exact current-head result or rerun only the affected PR workflow. +Existing consumers update both workflow keys together through reviewed PRs. +Rollback restores the old templates but does not undo findings or reviews. + +The additional numeric budget is **6 distinct contract cases**: two behavioral +replays (shipped/setup) exercise shared-head separation, stable PR identity and +lane separation; four mutants (two workflows × two directories) reject the old +branch key. All cases execute locally without models, timers or provider calls. +Existing coverage and freshness/security budgets remain unchanged. Acceptance +requires all six cases, workflow validation, specification validation and docs +checks, plus independent current-head review with no pending findings. + + - A duplicate run for the same head and same provider state produces the same projection digest. Bodies that already contain that digest MUST NOT be updated again. - A run whose analyzed head is no longer the PR head MUST return superseded and MUST NOT mutate findings or current-state projections. - Shipped Commit, Pull Request analysis, and Pull Request review-state workflows - MUST use distinct branch-scoped concurrency groups. Pull Request code/lifecycle + MUST use distinct owner-scoped concurrency groups. Commit uses its repository + and branch; both PR workflows use their repository and PR number, so two PRs + sharing one head branch MUST NOT cancel each other. Pull Request code/lifecycle and review-state events MUST use separate lane suffixes and dedicated workflow files with fixed job names. Each uses cancel-in-progress semantics only for its own replaceable revisions, so review-state observation cannot cancel @@ -1307,7 +1342,7 @@ examples should reuse the same fixtures as presentation tests where practical. superseded and does not update current-state surfaces. 16. Given cancellation after one durable mutation, then the last verified card remains truthful and the next run repairs the discovered drift. -17. Given duplicate same-head workflows, then branch-scoped workflow concurrency and +17. Given duplicate same-PR workflows, then PR-scoped workflow concurrency and application idempotency prevent duplicate reviews/comments. 18. Given a maintainer, external automation, or Copilot body update emits `pull_request: edited`, then the supplied PR workflow starts no run and the @@ -1396,7 +1431,7 @@ examples should reuse the same fixtures as presentation tests where practical. | Safe resolution/reopen order | reconciliation plan/apply use case | every partial mutation boundary | How it works, Failure scenarios | | Human dismissal precedence | lifecycle policy + resolver adapter | bot/human/unknown resolver matrix | Concepts, Detection | | Freshness and concurrency | head guards + workflow contract | stale, duplicate, canceled, race cases | Workflow setup | -| Review ownership and metadata non-preemption | distinct push/PR branch groups + exact-head push preflight + conditional PR cancellation | active/setup workflow parser, cross-group negative fixtures, ownership policy matrix, push-shaped detection integration, PR #363/#367 live evidence | Workflow setup, Configuration, How it works | +| Review ownership and metadata non-preemption | distinct push-branch/PR-number lane groups + exact-head push preflight + conditional PR cancellation | active/setup workflow parser, cross-group negative fixtures, ownership policy matrix, push-shaped detection integration, PR #363/#367 live evidence | Workflow setup, Configuration, How it works | | Metadata/review publication ownership | result-publication mode + telemetry/evidence policies | metadata generic-comment negative, outcome matrix, completion integration, PR #363 latest-by-name/noise replay | Detection, Workflow setup, How it works, Troubleshooting | | Canonical Result evidence | discriminated telemetry-set and finding-state projections + completion/lifecycle/status/summary/Check policies | complete aggregation, malformed/duplicate telemetry siblings, required-outcome absence, missing/extra key, numeric limits, overflow, cross-surface fail-closed cases | Detection, Observability, Comment commands, Failure scenarios | | Coherent final snapshot | snapshot loader + explicit surface completeness | before/after head, head-change, missing-head, per-surface failure, shared issue/PR read cases | How it works, Failure scenarios | diff --git a/specs/catalog.json b/specs/catalog.json index f645accf8..57b8cf4ab 100644 --- a/specs/catalog.json +++ b/specs/catalog.json @@ -440,14 +440,17 @@ "status": "implemented", "scope": "Reconcile review snapshots, findings, threads, comments, and check conclusions", "owner": "Copilot maintainers", - "lastVerified": "2026-10-21", + "lastVerified": "2026-10-24", "specs": [ "specs/bugbot-review-state-reconciliation.md" ], "workflows": [ ".github/workflows/copilot_commit.yml", ".github/workflows/copilot_pull_request.yml", - ".github/workflows/copilot_pull_request_comment.yml" + ".github/workflows/copilot_pull_request_comment.yml", + ".github/workflows/copilot_pull_request_review_state.yml", + "setup/workflows/copilot_pull_request.yml", + "setup/workflows/copilot_pull_request_review_state.yml" ], "entrypoints": [ "src/application/usecases/steps/commit/detect_potential_problems_workflow.ts", @@ -481,7 +484,8 @@ "src/data/repository/pull_request/pull_request_review_error_metadata.ts", "src/domain/bugbot/finding.ts", "src/application/usecases/steps/commit/bugbot/analyze_bugbot_revision_use_case.ts", - "src/application/usecases/steps/commit/bugbot/publish_findings_use_case.ts" + "src/application/usecases/steps/commit/bugbot/publish_findings_use_case.ts", + "scripts/validate-workflow-contract.cjs" ], "tests": [ "src/domain/bugbot/__tests__/review_state.test.ts", diff --git a/src/tooling/__tests__/validate_workflow_contract.test.ts b/src/tooling/__tests__/validate_workflow_contract.test.ts index b58e0a72d..8bd78c119 100644 --- a/src/tooling/__tests__/validate_workflow_contract.test.ts +++ b/src/tooling/__tests__/validate_workflow_contract.test.ts @@ -234,7 +234,7 @@ describe('workflow contract validator', () => { expect(() => assertQueueBudget(90, 90)).not.toThrow(); }); - it('serializes each branch event owner without cross-canceling the other workflow', () => { + it('scopes replaceable event lanes to their branch or PR owner', () => { for (const directory of ['.github/workflows', 'setup/workflows']) { for (const manifest of QUEUE_WORKFLOW_MANIFEST) { const file = path.join(process.cwd(), directory, manifest.file); @@ -245,7 +245,7 @@ describe('workflow contract validator', () => { if (['copilot_commit.yml', 'copilot_pull_request.yml', 'copilot_pull_request_review_state.yml'].includes(manifest.file)) { const expectedGroup = manifest.file === 'copilot_commit.yml' ? 'copilot-push-${{ github.repository }}-${{ github.ref_name }}' - : `copilot-pr-${'${{ github.repository }}'}-${'${{ github.event.pull_request.head.ref || github.ref_name }}'}-${manifest.file === 'copilot_pull_request.yml' ? 'analysis' : 'review-state'}`; + : `copilot-pr-${'${{ github.repository }}'}-${'${{ github.event.pull_request.number }}'}-${manifest.file === 'copilot_pull_request.yml' ? 'analysis' : 'review-state'}`; expect(workflow.jobs[manifest.jobId].concurrency).toEqual({ group: expectedGroup, 'cancel-in-progress': true, @@ -258,6 +258,36 @@ describe('workflow contract validator', () => { } }); + it.each(['.github/workflows', 'setup/workflows'])( + 'isolates two PRs on the same head branch while retaining stable independent lanes in %s', + directory => { + const render = (file: string, jobId: string, pullNumber: number, headRef = 'master') => { + const workflow = yaml.load(readFileSync(path.join(process.cwd(), directory, file), 'utf8')) as MutationWorkflow; + return workflow.jobs[jobId].concurrency.group + .replace('${{ github.repository }}', 'example/repository') + .replace('${{ github.event.pull_request.number }}', String(pullNumber)) + .replace('${{ github.event.pull_request.head.ref || github.ref_name }}', headRef); + }; + const analysis = (number: number, headRef?: string) => render('copilot_pull_request.yml', 'copilot-pull-requests', number, headRef); + const observer = (number: number, headRef?: string) => render('copilot_pull_request_review_state.yml', 'copilot-pull-request-review-state', number, headRef); + expect(analysis(21)).not.toBe(analysis(22)); + expect(observer(21)).not.toBe(observer(22)); + expect(analysis(21)).toBe(analysis(21, 'renamed-head')); + expect(observer(21)).toBe(observer(21, 'renamed-head')); + expect(analysis(21)).not.toBe(observer(21)); + }, + ); + + it.each(['.github/workflows', 'setup/workflows'].flatMap(directory => [ + [directory, 'copilot_pull_request.yml', 'copilot-pull-requests', 'analysis'], + [directory, 'copilot_pull_request_review_state.yml', 'copilot-pull-request-review-state', 'review-state'], + ]))('rejects cross-PR branch collisions in %s/%s', (directory, fileName, jobId, lane) => { + const file = path.join(process.cwd(), directory, fileName); + const workflow = yaml.load(readFileSync(file, 'utf8')) as MutationWorkflow; + workflow.jobs[jobId].concurrency.group = `copilot-pr-${'${{ github.repository }}'}-${'${{ github.event.pull_request.head.ref || github.ref_name }}'}-${lane}`; + expect(() => validateWorkflow(file, workflow)).toThrow('PR number'); + }); + it.each(['.github/workflows', 'setup/workflows'].flatMap(directory => [ [directory, 'copilot_pull_request.yml', 'copilot-pull-requests'], [directory, 'copilot_pull_request_review_state.yml', 'copilot-pull-request-review-state'], @@ -275,7 +305,7 @@ describe('workflow contract validator', () => { it.each([ ['copilot_commit.yml', 'copilot-commits', 'copilot-pr-${{ github.repository }}-${{ github.ref_name }}'], ['copilot_pull_request.yml', 'copilot-pull-requests', 'copilot-push-${{ github.repository }}-${{ github.ref_name }}'], - ['copilot_pull_request_review_state.yml', 'copilot-pull-request-review-state', 'copilot-pr-${{ github.repository }}-${{ github.event.pull_request.head.ref || github.ref_name }}-analysis'], + ['copilot_pull_request_review_state.yml', 'copilot-pull-request-review-state', 'copilot-pr-${{ github.repository }}-${{ github.event.pull_request.number }}-analysis'], ])( 'rejects a cross-workflow concurrency group in %s', (workflowFile, jobId, sharedGroup) => { @@ -293,7 +323,7 @@ describe('workflow contract validator', () => { const file = path.join(process.cwd(), directory, 'copilot_pull_request.yml'); const workflow = yaml.load(readFileSync(file, 'utf8')) as MutationWorkflow; workflow.jobs['copilot-pull-requests'].concurrency.group = - 'copilot-pr-${{ github.repository }}-${{ github.event.pull_request.head.ref || github.ref_name }}'; + 'copilot-pr-${{ github.repository }}-${{ github.event.pull_request.number }}'; expect(() => validateWorkflow(file, workflow)).toThrow('avoid cross-canceling another event owner'); }, From 0097c0ee314e67b9b05369ec06d13cf8c63837d8 Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Sun, 25 Oct 2026 00:56:14 +0200 Subject: [PATCH 2/2] chore(release): prepare Copilot 3.10.4 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c52b3f9f0..3fbd08712 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@vypdev/copilot", - "version": "3.10.3", + "version": "3.10.4", "packageManager": "pnpm@10.12.4", "pnpm": { "overrides": {