-
Notifications
You must be signed in to change notification settings - Fork 293
Add autofix review loop for Copilot PRs #8181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| --- | ||
| name: "Address Review Comments" | ||
| description: >- | ||
| Automatically addresses code review feedback on Copilot-created PRs. | ||
| Reads review comments, applies fixes, verifies the build, and pushes changes. | ||
| Includes a circuit breaker (max 3 iterations) to prevent infinite loops. | ||
|
|
||
| on: | ||
| pull_request_review: | ||
| types: [submitted] | ||
| reaction: "eyes" | ||
|
|
||
| # Only run when: | ||
| # 1. The review has actionable feedback (not an approval) | ||
| # 2. The PR is from the same repo (not a fork — we need push access) | ||
| # 3. The PR was created by Copilot OR has the 'copilot-autofix' label | ||
| if: >- | ||
| github.event.review.state != 'approved' | ||
| && github.event.pull_request.head.repo.id == github.repository_id | ||
| && ( | ||
| github.event.pull_request.user.login == 'copilot-swe-agent[bot]' | ||
| || contains(github.event.pull_request.labels.*.name, 'copilot-autofix') | ||
| ) | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: read | ||
| actions: read | ||
|
|
||
| imports: | ||
| - shared/address-review-shared.md | ||
|
|
||
| safe-outputs: | ||
| noop: | ||
| report-as-issue: false | ||
|
|
||
| timeout-minutes: 60 | ||
| --- | ||
|
|
||
| <!-- Body provided by shared/address-review-shared.md --> | ||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| --- | ||
| name: "Address Review Comments (command)" | ||
| description: >- | ||
| Addresses code review feedback when a maintainer comments /autofix on a PR. | ||
| Same behavior as the automatic address-review workflow but manually triggered. | ||
|
|
||
| on: | ||
| slash_command: | ||
| name: autofix | ||
| events: [pull_request_comment] | ||
| roles: [admin, maintainer, write] | ||
| reaction: "eyes" | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: read | ||
| actions: read | ||
|
|
||
| imports: | ||
| - shared/address-review-shared.md | ||
|
|
||
| safe-outputs: | ||
| noop: | ||
| report-as-issue: false | ||
|
|
||
| timeout-minutes: 60 | ||
| --- | ||
|
|
||
| <!-- Body provided by shared/address-review-shared.md --> |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| --- | ||
| name: "Re-review After Autofix" | ||
| description: >- | ||
| Re-runs the expert code review after new commits are pushed to a | ||
| Copilot-created PR. This closes the autofix loop: after the address-review | ||
| workflow pushes fixes, this workflow verifies them with the expert-reviewer. | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [synchronize] | ||
|
|
||
| # Only re-review when: | ||
| # 1. The PR is not a draft | ||
| # 2. The PR was created by Copilot OR has the 'copilot-autofix' label | ||
| if: >- | ||
| github.event.pull_request.draft == false | ||
| && ( | ||
| github.event.pull_request.user.login == 'copilot-swe-agent[bot]' | ||
| || contains(github.event.pull_request.labels.*.name, 'copilot-autofix') | ||
| ) | ||
|
|
||
| checkout: false | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: read | ||
|
|
||
| imports: | ||
| - shared/review-shared.md | ||
|
|
||
| safe-outputs: | ||
| noop: | ||
| report-as-issue: false | ||
|
|
||
| timeout-minutes: 60 | ||
| --- | ||
|
|
||
| <!-- Body provided by shared/review-shared.md --> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,187 @@ | ||
| --- | ||
| # Shared configuration and body for the address-review workflows. | ||
| # | ||
| # Imported by address-review.agent.md (automatic trigger) and | ||
| # autofix.agent.md (slash command). Keeps the fix logic in one place. | ||
|
|
||
| description: "Shared configuration for address-review workflows" | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: read | ||
| actions: read | ||
|
|
||
| network: | ||
| allowed: | ||
| - defaults | ||
| - dotnet | ||
|
|
||
| imports: | ||
| - shared/repo-build-setup.md | ||
|
|
||
| tools: | ||
| github: | ||
| lockdown: true | ||
| toolsets: [pull_requests, repos] | ||
| min-integrity: none | ||
| edit: | ||
| bash: ["dotnet", "git", "find", "ls", "cat", "grep", "head", "tail", "wc", "mkdir", "sed"] | ||
|
|
||
| safe-outputs: | ||
| push-to-pull-request-branch: | ||
| max: 3 | ||
| add-comment: | ||
| max: 3 | ||
| hide-older-comments: true | ||
| noop: | ||
| report-as-issue: false | ||
| --- | ||
|
|
||
| # Address Review Comments | ||
|
|
||
| You are an expert .NET developer working on the MSTest testing framework and | ||
| Microsoft.Testing.Platform (MTP) repository. A pull request has received a code | ||
| review with feedback. Your job is to read every review comment, assess whether it | ||
| should be addressed, apply fixes where appropriate, explain your reasoning when | ||
| skipping a suggestion, verify the build, and push the changes. | ||
|
|
||
| ## Context | ||
|
|
||
| - **PR**: #${{ github.event.pull_request.number || github.event.issue.number }} | ||
| - **Repository**: ${{ github.repository }} | ||
|
|
||
| ## Circuit Breaker — Check First | ||
|
|
||
| Before making any code changes, determine how many autofix iterations have | ||
| already run on this PR: | ||
|
|
||
| 1. List all comments on the PR. | ||
| 2. Count comments that contain the **exact** marker `<!-- autofix-iteration -->`. | ||
| 3. **If the count is 3 or more**, post the following comment and then **stop immediately** — do not edit any files: | ||
|
|
||
| > ⚠️ **Autofix loop limit reached** (3 iterations). This PR needs human attention. | ||
| > <!-- autofix-iteration --> | ||
|
Evangelink marked this conversation as resolved.
Outdated
|
||
|
|
||
| Then exit. | ||
|
|
||
| ## Step 1 — Understand the Review Feedback | ||
|
|
||
| 1. Fetch the full diff for the pull request. | ||
| 2. Fetch **all** review comments (inline and top-level) from the most recent | ||
| review. | ||
| 3. Read each comment carefully. Classify it as one of: | ||
| - **Blocking** — the reviewer explicitly requested a change (`REQUEST_CHANGES` | ||
| review, or language like "must", "should", "fix", "change"). | ||
| - **Suggestion** — a non-blocking improvement idea ("consider", "nit", | ||
| "optional", "could", or posted under a `COMMENT` review). | ||
| - **Informational** — praise, acknowledgement, or context with no action asked. | ||
| 4. Identify the specific file and line(s) for each comment. | ||
| 5. Group related comments together (e.g., the same pattern flagged in multiple | ||
| files). | ||
|
|
||
| If there are **no blocking or suggestion comments** (only informational or | ||
| praise), post a short comment explaining there is nothing to address and exit. | ||
|
|
||
| ## Step 2 — Read Project Conventions | ||
|
|
||
| Before editing code, read the following files to understand project conventions: | ||
|
|
||
| - `.github/copilot-instructions.md` — coding standards, localization rules, | ||
| public API policy, and testing guidelines. | ||
| - `.editorconfig` — formatting and naming rules. | ||
|
|
||
| Apply these conventions to every change you make. | ||
|
|
||
| ## Step 3 — Assess and Apply Fixes | ||
|
|
||
| For each review comment (or group of related comments), apply the following | ||
| decision process: | ||
|
|
||
| ### Blocking comments → always fix | ||
|
|
||
| 1. Read the relevant source file(s) to understand context. | ||
| 2. Apply the requested change precisely. | ||
| 3. If the review flags a **pattern** (e.g., missing `ConfigureAwait(false)`, | ||
| wrong naming convention), search for the same pattern in **other files | ||
| touched by the PR** and fix those occurrences too. | ||
| 4. Keep changes minimal — only fix what was flagged. Do not refactor unrelated | ||
| code. | ||
|
|
||
| ### Suggestion comments → assess and decide | ||
|
|
||
| For each suggestion: | ||
|
|
||
| 1. Evaluate whether the suggestion improves correctness, readability, | ||
| performance, or consistency with project conventions. | ||
| 2. **If beneficial** — apply the change (same rules as blocking comments). | ||
| 3. **If not applicable or would cause issues** — do NOT apply the change. | ||
| Instead, note the comment and your reasoning in the summary (Step 6). | ||
| Provide a brief, respectful explanation of why the suggestion was skipped | ||
| (e.g., "Skipped: this pattern is intentional for backward compatibility"). | ||
|
|
||
| ### Informational comments → no action needed | ||
|
|
||
| Acknowledge in the summary if relevant, but do not make code changes. | ||
|
|
||
| ### Common fix categories | ||
|
|
||
| | Review finding | What to do | | ||
| |---|---| | ||
| | Missing `ConfigureAwait(false)` | Add it to every `await` in library code | | ||
| | `init` accessor on new public API | Change to `set` or use constructor | | ||
| | Missing `PublicAPI.Unshipped.txt` entry | Add the API signature | | ||
| | Style / formatting violation | Fix to match `.editorconfig` | | ||
| | Missing null check at boundary | Add guard clause | | ||
| | Thread-safety issue | Apply appropriate synchronization | | ||
| | Missing XML doc comment | Add concise doc comment | | ||
| | Localization issue | Add/update `.resx` entry; do NOT edit `.xlf` files | | ||
|
|
||
| ## Step 4 — Verify the Build | ||
|
|
||
| After applying all fixes: | ||
|
|
||
| 1. Run `dotnet build` on the affected project(s) or the solution if unsure which | ||
| projects are impacted. | ||
| 2. If there are **build errors**, fix them. Repeat until the build succeeds. | ||
| 3. Do **not** run the full test suite (it takes too long). Compilation | ||
| verification is sufficient. | ||
|
|
||
| ## Step 5 — Commit and Push | ||
|
|
||
| 1. Stage all changed files. | ||
| 2. Commit with a message in this format: | ||
|
|
||
| ``` | ||
|
Check failure on line 154 in .github/workflows/shared/address-review-shared.md
|
||
| Address review feedback | ||
|
|
||
| Applied fixes for N review comments from the expert-reviewer. | ||
| ``` | ||
|
|
||
| 3. Push to the PR branch. | ||
|
Evangelink marked this conversation as resolved.
Outdated
|
||
|
|
||
| ## Step 6 — Post Summary | ||
|
|
||
| Post a comment on the PR summarizing what was done. Use this format: | ||
|
|
||
| > 🔧 **Autofix applied** — assessed N review comment(s). | ||
| > | ||
| > **Applied (M):** | ||
| > - `path/to/file.cs`: Brief description of change | ||
|
Check failure on line 169 in .github/workflows/shared/address-review-shared.md
|
||
| > - `path/to/other.cs`: Brief description of change | ||
| > | ||
| > **Skipped with explanation (K):** | ||
| > - *"Consider using Span<T> here"* — Skipped: not applicable on netstandard2.0 TFM without polyfill. | ||
|
Check failure on line 173 in .github/workflows/shared/address-review-shared.md
|
||
| > - *"Nit: rename variable"* — Skipped: name is consistent with surrounding code. | ||
| > | ||
| > **No action needed (J):** | ||
| > - Informational / praise comments acknowledged. | ||
|
Check failure on line 177 in .github/workflows/shared/address-review-shared.md
|
||
| > | ||
| > A re-review will be triggered automatically if code was pushed. | ||
| > <!-- autofix-iteration --> | ||
|
|
||
| Omit the **Skipped** or **No action needed** sections if they are empty. | ||
|
|
||
| If **no code changes** were needed (all comments were informational or | ||
| intentionally skipped), still post the summary but do **not** push a commit. | ||
|
|
||
| The `<!-- autofix-iteration -->` marker is **required** for the circuit breaker. | ||
Uh oh!
There was an error while loading. Please reload this page.