fix: prevent security review from re-posting comments on resolved threads#80
fix: prevent security review from re-posting comments on resolved threads#80
Conversation
…eads Add an explicit step 2 in the security-review workflow prompt instructing the agent to use the pull_requests toolset to fetch all existing review comment threads, check their resolution status, and skip re-posting comments for findings that already have a resolved (or unchanged unresolved) thread on the same file and line. Renumber subsequent steps accordingly. Co-authored-by: pmalarme <686568+pmalarme@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where the security review agent was re-posting inline comments on already-resolved PR threads. The fix adds explicit instructions to check GitHub review thread resolution status before posting comments, rather than relying solely on cache memory.
Changes:
- Added new step 2 to explicitly fetch and check existing review threads via the GitHub API
- Removed vague "avoid repeating" instruction from step 1 that wasn't working reliably
- Renumbered subsequent steps 3-7 to maintain consistency
| for PR #${{ github.event.pull_request.number }}. For each thread: | ||
| - Record the file path, line number, and body of the original comment. | ||
| - Record whether the thread is **resolved** (i.e., marked as resolved by | ||
| a reviewer or via the `/create-issue` command). |
There was a problem hiding this comment.
The instruction states that threads can be "marked as resolved by a reviewer or via the /create-issue command", but it's unclear whether the /create-issue command actually resolves threads in the GitHub API. Based on the create-issue-command.md workflow, the /create-issue command creates an issue and replies to the comment, but there's no indication it marks the thread as resolved. This could lead to confusion for the AI agent executing this workflow. Consider clarifying this by either: (1) verifying that /create-issue does resolve threads and keeping this language, (2) removing the reference to /create-issue if it doesn't actually resolve threads, or (3) instructing the agent to also check if a thread has an associated issue created via /create-issue as an alternative indicator that it's been addressed.
| a reviewer or via the `/create-issue` command). | |
| a reviewer). |
| unresolved thread on the same file and line and whose code has not | ||
| changed since the last review. |
There was a problem hiding this comment.
The instruction to skip re-posting for unresolved threads "whose code has not changed since the last review" is ambiguous and may be difficult for the AI agent to execute reliably. The workflow doesn't specify how the agent should determine whether the code at a specific line has changed between reviews. Consider providing more explicit guidance, such as: (1) instructing the agent to compare the current line's content with what was flagged in the previous review (stored in cache memory), (2) checking the commit history for that specific line, or (3) comparing the diff hunk context from the previous review comment with the current state. Without clearer implementation guidance, this instruction may not prevent duplicate comments as intended.
| unresolved thread on the same file and line and whose code has not | |
| changed since the last review. | |
| unresolved thread on the same file and line **if the underlying code is | |
| unchanged since the last review**. To determine whether the code has | |
| changed: | |
| - Compare the current line content and its immediate diff hunk context | |
| (for example, the line plus approximately three surrounding lines) with | |
| what was previously recorded for that thread in cache memory at | |
| `/tmp/gh-aw/cache-memory/security-review-pr-${{ github.event.pull_request.number }}.json`. | |
| - If the cached content for that file path and line (including its diff | |
| hunk context) matches the current diff hunk, treat the code as | |
| unchanged and skip posting a new comment on that thread. | |
| - If no prior cache entry exists, or if the content or context differs, | |
| treat the code as changed and apply your normal commenting logic. |
The security review agent was re-posting inline comments on already-resolved PR threads because deduplication relied solely on cache memory, which doesn't reflect actual GitHub thread resolution state.
Changes
.github/workflows/security-review.md: Added step 2 to explicitly use thepull_requeststoolset to fetch all existing review threads, check their resolution status via the GitHub API, and skip posting for:No frontmatter changed; no
gh aw compilerecompilation required.Testing
Checklist
Additional context
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.