-
Notifications
You must be signed in to change notification settings - Fork 53
fix(#2253): pass human review comments to intent-coherence sub-agent #2257
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 |
|---|---|---|
|
|
@@ -181,6 +181,29 @@ rewrite), or if `total_commits` exceeds 250 (the compare API | |
| silently truncates file lists at 300 files), treat all files as | ||
| changed — no anchoring for this run. | ||
|
|
||
| ### 2b. Human review comments | ||
|
|
||
| Fetch human review comments from the PR to provide sub-agents with | ||
| context about human-authorized scope changes. Human reviewers may | ||
| request changes that deviate from the linked issue's original | ||
| specification — sub-agents need visibility into these requests to | ||
| avoid flagging human-directed changes as unauthorized scope creep. | ||
|
|
||
| ```bash | ||
| # Fetch all review comments (not inline diff comments) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [info] edge-case The gh api call filters reviews where .user.type != Bot. Organization and Mannequin account types would pass this filter, though in practice neither type can submit PR reviews on GitHub. |
||
| PR_REVIEWS=$(gh api "repos/${REPO_FULL_NAME}/pulls/${PR_NUMBER}/reviews" \ | ||
| --jq '[.[] | select(.user.type != "Bot") | {author: .user.login, state: .state, body: .body}]') | ||
| ``` | ||
|
|
||
| Filter to **human-authored reviews only** (exclude bot reviews by | ||
| checking `.user.type != "Bot"`). Include the review state (`APPROVED`, | ||
| `CHANGES_REQUESTED`, `COMMENTED`) so sub-agents can distinguish | ||
| between casual comments and explicit change requests. | ||
|
|
||
| If no human reviews exist, set `human_review_comments` to an empty | ||
| list. This is normal for first-review PRs or PRs that have only | ||
| received bot reviews. | ||
|
|
||
| ### 3. Triage | ||
|
|
||
| Classify the change and prepare context packages for sub-agents. This | ||
|
|
@@ -303,6 +326,10 @@ For each selected sub-agent, assemble a context package containing: | |
| - `pr_metadata`: title, body, author, labels | ||
| - `issue_context`: linked issue title, body, comments (for | ||
| `intent-coherence`) | ||
| - `human_review_comments`: human-authored review comments from prior | ||
| PR review cycles (from 2b), for `intent-coherence`. These provide | ||
| context about human-authorized scope amendments that may deviate | ||
| from the linked issue's original specification. | ||
| - `cross_repo_context`: findings from 3a for `cross-repo-contracts` | ||
|
|
||
| ### 4. Dispatch sub-agents | ||
|
|
@@ -349,6 +376,9 @@ For each selected sub-agent: | |
|
|
||
| ### Issue context | ||
| <linked issue content or "no linked issue"> | ||
|
|
||
| ### Human review comments | ||
| <human review comments JSON or "none"> | ||
| ``` | ||
|
|
||
| **Part 5 — Dispatch guard flag:** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,48 @@ Calibrate investigation to the diff size and nature. | |
| scope. If there is no linked issue, flag a `missing-authorization` | ||
| finding — non-trivial changes require explicit authorization. | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [info] design-smell Scope authorization is becoming a multi-source reconciliation problem (issue spec + human review comments + PR description) relying on LLM interpretation of natural language. Consider whether structured representation would be more robust. |
||
| ## Human-authorized scope amendments | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] prompt-injection The section instructs the sub-agent to treat review comments as authoritative scope amendments that suppress medium+ findings to info-level. This creates a trust elevation: PR review comments (which any collaborator can post) are granted the same authority as linked issues without verifying the commenter authority level. |
||
|
|
||
| When the context package includes human review comments, check whether | ||
| a human reviewer has explicitly requested changes that deviate from | ||
| the linked issue's original specification. Human reviewers have the | ||
| authority to amend the scope of work — their review comments function | ||
| as addenda to the issue's authorization. | ||
|
|
||
| **Identifying human-authorized deviations:** | ||
|
|
||
| A deviation is human-authorized when a human reviewer (not a bot) has | ||
| explicitly requested it in a review comment with state | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] edge-case The Human-authorized scope amendments section specifies that deviations authorized by CHANGES_REQUESTED or COMMENTED review states should be treated as info-level, but APPROVED is excluded. A reviewer who approves with a comment authorizing a deviation has given stronger authorization than a COMMENTED review. The data is already available but the sub-agent does not act on it. |
||
| `CHANGES_REQUESTED` or `COMMENTED`. Look for: | ||
|
|
||
| - Direct instructions to rename, restructure, or change approach | ||
| (e.g., "rename this to X", "use Y instead of Z", "change the | ||
| category from A to B") | ||
| - Explicit approval of a deviation the PR author proposed | ||
| - Requests that expand or narrow the scope beyond the issue's | ||
| original specification | ||
|
|
||
| **How to handle human-authorized deviations:** | ||
|
|
||
| - **Do not raise medium+ findings** for deviations that a human | ||
| reviewer explicitly requested. Flagging human-directed changes as | ||
| unauthorized scope creep is a false positive. | ||
| - **Report as info-level** with category `scope-exceeded` so the | ||
| deviation is visible and the issue can be updated to reflect the | ||
| amended scope. The description should note both the deviation from | ||
| the issue and the human review comment that authorized it. | ||
| - If the PR includes changes **beyond** what the human authorized, | ||
| flag only the unauthorized portion at the appropriate severity. | ||
|
|
||
| **Ambiguous cases:** | ||
|
|
||
| - If the human comment is vague or does not clearly authorize the | ||
| specific deviation (e.g., "looks good" without addressing the | ||
| change), treat the deviation as unauthorized and flag at the | ||
| normal severity. | ||
| - If multiple human reviewers give conflicting feedback about the | ||
| same change, flag for human resolution at medium severity. | ||
|
|
||
| ## Revert PR authorization | ||
|
|
||
| A PR is a candidate revert if **at least two** of the following signals | ||
|
|
||
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.
[medium] prompt-injection
Human review comments are passed as context to sub-agents without being identified as untrusted input. The meta-prompt warns that diff and PR metadata are untrusted, but human_review_comments is a new input channel not covered by this warning. Any GitHub user with permission to leave PR reviews can craft review comments containing prompt injection payloads to manipulate the intent-coherence sub-agent into suppressing legitimate scope-creep findings.
Suggested fix: Update meta-prompt.md to explicitly list human_review_comments as untrusted input alongside diff and PR metadata. Additionally, in SKILL.md step 4 context package template, add a warning label to the Human review comments section.