-
Notifications
You must be signed in to change notification settings - Fork 0
fix: prevent security review from re-posting comments on resolved threads #80
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -42,24 +42,37 @@ agent instructions. | |||||||||||||||||||||||||||||
| `/tmp/gh-aw/cache-memory/security-review-pr-${{ github.event.pull_request.number }}.json`. It can contain information about previous review findings, categories, files reviewed, and timestamps for this PR. | ||||||||||||||||||||||||||||||
| - Identify recurring security patterns in this repository from | ||||||||||||||||||||||||||||||
| `/tmp/gh-aw/cache-memory/security-review-patterns.json`. It can contain information about recurring security issues and patterns in the repository. | ||||||||||||||||||||||||||||||
| - Avoid repeating the same inline comments from previous reviews if the previous comment is not resolved yet nor outdated (e.g., if the same issue is still present in the code or if the code has not changed since the last review). | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| 2. **Fetch the pull request diff.** Read the pull request details and all | ||||||||||||||||||||||||||||||
| 2. **Check existing review threads.** Before posting any new comment, use | ||||||||||||||||||||||||||||||
| the `pull_requests` toolset to fetch all existing review comment threads | ||||||||||||||||||||||||||||||
| 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). | ||||||||||||||||||||||||||||||
| - **Do not re-post a comment** for any finding that already has a | ||||||||||||||||||||||||||||||
| resolved thread on the same file and line. Even if the finding is still | ||||||||||||||||||||||||||||||
| technically present in the code, do not re-open resolved threads by | ||||||||||||||||||||||||||||||
| posting a duplicate comment. | ||||||||||||||||||||||||||||||
| - **Do not re-post a comment** for any finding that already has an | ||||||||||||||||||||||||||||||
| unresolved thread on the same file and line and whose code has not | ||||||||||||||||||||||||||||||
| changed since the last review. | ||||||||||||||||||||||||||||||
|
Comment on lines
+57
to
+58
|
||||||||||||||||||||||||||||||
| 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.