Problem Statement
Edit 7 of the review process deviated significantly into speculative concerns about XSS/SSRF vulnerabilities related to image URL handling. These concerns were entirely unrelated to the actual goal of the pull request (PR), which was to constrain image height on mobile. The deviation also involved code that was not meaningfully altered in the context of the PR.
Goal
Ensure that PR reviews remain focused on their stated intent and prevent topic drift into unrelated concerns, especially speculative security issues, unless they are directly tied to new untrusted input paths introduced by the PR.
Non-Goals
None.
Proposed Approach
- Pass the PR's stated goal or description into the review prompt dynamically to ensure reviewers are clearly informed of the PR's intended scope.
- Constrain the review process to only flag:
- Security issues if the diff introduces a new untrusted input path (e.g., new user-controlled data flowing to a sensitive area), or
- Genuine, evidence-backed regressions caused by the changes in the PR.
- Exclude speculative or unrelated review topics unless directly linked to the PR's stated goal or identified regressions.
Example of the updated review prompt:
This PR's stated goal: "{pr_description_or_plan_summary}"
Flag security issues only if this diff introduces a NEW untrusted input
path (e.g. new user-controlled data flowing somewhere it wasn't before).
Do not introduce review topics unrelated to the PR's stated goal unless
you find a genuine, evidence-backed regression caused by this diff.
Alternatives Considered
- Manual Review Protocol Updates: Train reviewers to focus on scope and avoid topic drift through guidelines and best practices.
- Reason Rejected: Less enforceable and prone to human error compared to automated prompts.
- Static Analysis Enforcement: Use tools to automatically detect out-of-scope concerns during reviews.
- Reason Rejected: Overkill for this specific issue and would require significant development effort.
Architectural Impact
Minimal architectural impact, as the necessary changes are limited to updating the review prompt. This has already been partially implemented in PR #2568, which dynamically fetches the PR goal from the GitHub API and incorporates it into the prompt.
Scope
- Ensure the PR's stated goal is dynamically fetched from the GitHub API and included in the review prompt.
- Update the review prompt to include specific instructions for identifying valid security issues and excluding unrelated or speculative concerns.
- No additional changes to the review or CI/CD systems are required.
UNDERSTAND THE ISSUE
The primary issue is that PR reviews can sometimes drift into unrelated or speculative concerns, as evidenced by Edit 7's deviation into XSS/SSRF considerations in the context of PR #2548. This undermines the efficiency and focus of the review process.
DETERMINE APPROACH
The approach involves enhancing the review prompt to explicitly define the PR's stated goal and constrain out-of-scope findings. This has already been partially implemented in PR #2568, where the PR goal is dynamically fetched and incorporated into the review process.
SPECIFY SCOPE
- Dynamically fetch and inject the PR's stated goal into the review prompt using the GitHub API.
- Update the review prompt to constrain reviewers to focus on:
- Security issues tied to new untrusted input paths introduced by the PR.
- Evidence-backed regressions caused by the PR's changes.
- Ensure the prompt discourages unrelated speculative topics unless directly tied to the PR's scope or evidence-backed regressions.
DEFINITION OF DONE
- The review prompt includes the dynamically fetched PR goal from the GitHub API.
- The updated review prompt explicitly directs reviewers to focus only on the PR's stated goal and avoid unrelated speculative concerns.
- Reviews are constrained to flag only:
- Security issues associated with new untrusted input paths introduced by the PR.
- Genuine, evidence-backed regressions caused by the PR's changes.
- The updated prompt has been tested and verified to prevent topic drift during reviews.
Problem Statement
Edit 7 of the review process deviated significantly into speculative concerns about XSS/SSRF vulnerabilities related to image URL handling. These concerns were entirely unrelated to the actual goal of the pull request (PR), which was to constrain image height on mobile. The deviation also involved code that was not meaningfully altered in the context of the PR.
Goal
Ensure that PR reviews remain focused on their stated intent and prevent topic drift into unrelated concerns, especially speculative security issues, unless they are directly tied to new untrusted input paths introduced by the PR.
Non-Goals
None.
Proposed Approach
Example of the updated review prompt:
Alternatives Considered
Architectural Impact
Minimal architectural impact, as the necessary changes are limited to updating the review prompt. This has already been partially implemented in PR #2568, which dynamically fetches the PR goal from the GitHub API and incorporates it into the prompt.
Scope
UNDERSTAND THE ISSUE
The primary issue is that PR reviews can sometimes drift into unrelated or speculative concerns, as evidenced by Edit 7's deviation into XSS/SSRF considerations in the context of PR #2548. This undermines the efficiency and focus of the review process.
DETERMINE APPROACH
The approach involves enhancing the review prompt to explicitly define the PR's stated goal and constrain out-of-scope findings. This has already been partially implemented in PR #2568, where the PR goal is dynamically fetched and incorporated into the review process.
SPECIFY SCOPE
DEFINITION OF DONE