diff --git a/.github/workflows/gh-aw-fragments/pr-context.md b/.github/workflows/gh-aw-fragments/pr-context.md index 00f3c923..31b8ee07 100644 --- a/.github/workflows/gh-aw-fragments/pr-context.md +++ b/.github/workflows/gh-aw-fragments/pr-context.md @@ -74,6 +74,14 @@ steps: --jq '.data.repository.pullRequest.reviewThreads.nodes' \ | jq -s 'add // []' > /tmp/pr-context/review_comments.json + # Filtered review thread views (pre-computed so agents don't need to parse review_comments.json) + jq '[.[] | select(.isResolved == false)]' /tmp/pr-context/review_comments.json \ + > /tmp/pr-context/unresolved_threads.json + jq '[.[] | select(.isResolved == true)]' /tmp/pr-context/review_comments.json \ + > /tmp/pr-context/resolved_threads.json + jq '[.[] | select(.isOutdated == true)]' /tmp/pr-context/review_comments.json \ + > /tmp/pr-context/outdated_threads.json + # Per-file review threads (mirrors diffs/ structure) jq -c '.[]' /tmp/pr-context/review_comments.json | while IFS= read -r thread; do filepath=$(echo "$thread" | jq -r '.path // empty') @@ -118,6 +126,9 @@ steps: | `file_order_largest.txt` | Changed files sorted by diff size descending (largest first), one filename per line | | `reviews.json` | Prior review submissions — author, state (APPROVED/CHANGES_REQUESTED/COMMENTED), body | | `review_comments.json` | All review threads (GraphQL) — each thread has `id` (node ID for resolving), `isResolved`, `isOutdated`, `path`, `line`, and nested `comments` with `id`, `databaseId` (numeric REST ID for replies), body/author | + | `unresolved_threads.json` | Unresolved review threads — subset of `review_comments.json` where `isResolved` is false | + | `resolved_threads.json` | Resolved review threads — subset of `review_comments.json` where `isResolved` is true | + | `outdated_threads.json` | Outdated review threads — subset of `review_comments.json` where `isOutdated` is true (code changed since comment) | | `threads/.json` | Per-file review threads — one file per changed file with existing threads, mirroring the repo path under `threads/` | | `comments.json` | PR discussion comments (not inline) | | `issue-{N}.json` | Linked issue details (one file per linked issue, if any) | diff --git a/.github/workflows/gh-aw-mention-in-pr-by-id.lock.yml b/.github/workflows/gh-aw-mention-in-pr-by-id.lock.yml index f7beed2d..35726214 100644 --- a/.github/workflows/gh-aw-mention-in-pr-by-id.lock.yml +++ b/.github/workflows/gh-aw-mention-in-pr-by-id.lock.yml @@ -43,7 +43,7 @@ # # inlined-imports: true # -# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"3380718b7637f543b06f1560af5f1a0bc4956ba6ca0c2688835694427410f69a"} +# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"c1596c1d583a1d00154c0d46ffeb93694bd2f8cd2568fa0b0669f1e76848d4b3"} name: "Mention in PR by ID" "on": @@ -690,7 +690,7 @@ jobs: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ github.event.pull_request.number || inputs.target-pr-number || github.event.issue.number }} name: Fetch PR context to disk - run: "set -euo pipefail\nmkdir -p /tmp/pr-context\n\n# PR metadata\ngh pr view \"$PR_NUMBER\" --json title,body,author,baseRefName,headRefName,headRefOid,url \\\n > /tmp/pr-context/pr.json\n\n# Full diff\nif ! gh pr diff \"$PR_NUMBER\" > /tmp/pr-context/pr.diff; then\n echo \"::warning::Failed to fetch full PR diff; per-file diffs from files.json are still available.\"\n : > /tmp/pr-context/pr.diff\nfi\n\n# Changed files list (--paginate may output concatenated arrays; jq -s 'add' merges them)\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/files.json\n\n# Per-file diffs\njq -c '.[]' /tmp/pr-context/files.json | while IFS= read -r entry; do\n filename=$(echo \"$entry\" | jq -r '.filename')\n mkdir -p \"/tmp/pr-context/diffs/$(dirname \"$filename\")\"\n echo \"$entry\" | jq -r '.patch // empty' > \"/tmp/pr-context/diffs/${filename}.diff\"\ndone\n\n# File orderings for sub-agent review (3 strategies)\njq -r '[.[] | .filename] | sort | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_az.txt\njq -r '[.[] | .filename] | sort | reverse | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_za.txt\njq -r '[.[] | {filename, size: ((.additions // 0) + (.deletions // 0))}] | sort_by(-.size) | .[].filename' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_largest.txt\n\n# Existing reviews\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/reviews.json\n\n# Review threads with resolution status (GraphQL — REST lacks isResolved/isOutdated)\ngh api graphql --paginate -f query='\n query($owner: String!, $repo: String!, $number: Int!, $endCursor: String) {\n repository(owner: $owner, name: $repo) {\n pullRequest(number: $number) {\n reviewThreads(first: 100, after: $endCursor) {\n pageInfo { hasNextPage endCursor }\n nodes {\n id\n isResolved\n isOutdated\n isCollapsed\n path\n line\n startLine\n comments(first: 100) {\n nodes {\n id\n databaseId\n body\n author { login }\n createdAt\n }\n }\n }\n }\n }\n }\n }\n' -F owner=\"${GITHUB_REPOSITORY%/*}\" -F repo=\"${GITHUB_REPOSITORY#*/}\" -F \"number=$PR_NUMBER\" \\\n --jq '.data.repository.pullRequest.reviewThreads.nodes' \\\n | jq -s 'add // []' > /tmp/pr-context/review_comments.json\n\n# Per-file review threads (mirrors diffs/ structure)\njq -c '.[]' /tmp/pr-context/review_comments.json | while IFS= read -r thread; do\n filepath=$(echo \"$thread\" | jq -r '.path // empty')\n [ -z \"$filepath\" ] && continue\n mkdir -p \"/tmp/pr-context/threads/$(dirname \"$filepath\")\"\n echo \"$thread\" >> \"/tmp/pr-context/threads/${filepath}.jsonl\"\ndone\n# Convert per-file JSONL to proper JSON arrays\nmkdir -p /tmp/pr-context/threads\nfind /tmp/pr-context/threads -name '*.jsonl' 2>/dev/null | while IFS= read -r jsonl; do\n jq -s '.' \"$jsonl\" > \"${jsonl%.jsonl}.json\"\n rm \"$jsonl\"\ndone\n\n# PR discussion comments\ngh api \"repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/comments.json\n\n# Linked issues\njq -r '.body // \"\"' /tmp/pr-context/pr.json 2>/dev/null \\\n | grep -oiE '(fixes|closes|resolves)\\s+#[0-9]+' \\\n | grep -oE '[0-9]+$' \\\n | sort -u \\\n | while read -r issue; do\n gh api \"repos/$GITHUB_REPOSITORY/issues/$issue\" > \"/tmp/pr-context/issue-${issue}.json\" || true\n done || true\n\n# Write manifest\ncat > /tmp/pr-context/README.md << 'MANIFEST'\n# PR Context\n\nPre-fetched PR data. All files are in `/tmp/pr-context/`.\n\n| File | Description |\n| --- | --- |\n| `pr.json` | PR metadata — title, body, author, base/head branches, head commit SHA (`headRefOid`), URL |\n| `pr.diff` | Full unified diff of all changes |\n| `files.json` | Changed files array — each entry has `filename`, `status`, `additions`, `deletions`, `patch` |\n| `diffs/.diff` | Per-file diffs — one file per changed file, mirroring the repo path under `diffs/` |\n| `file_order_az.txt` | Changed files sorted alphabetically (A→Z), one filename per line |\n| `file_order_za.txt` | Changed files sorted reverse-alphabetically (Z→A), one filename per line |\n| `file_order_largest.txt` | Changed files sorted by diff size descending (largest first), one filename per line |\n| `reviews.json` | Prior review submissions — author, state (APPROVED/CHANGES_REQUESTED/COMMENTED), body |\n| `review_comments.json` | All review threads (GraphQL) — each thread has `id` (node ID for resolving), `isResolved`, `isOutdated`, `path`, `line`, and nested `comments` with `id`, `databaseId` (numeric REST ID for replies), body/author |\n| `threads/.json` | Per-file review threads — one file per changed file with existing threads, mirroring the repo path under `threads/` |\n| `comments.json` | PR discussion comments (not inline) |\n| `issue-{N}.json` | Linked issue details (one file per linked issue, if any) |\n| `agents.md` | Repository conventions from `generate_agents_md` (if written by agent) |\n| `review-instructions.md` | Review instructions, criteria, and calibration examples (if written by review-process fragment) |\nMANIFEST\n\necho \"PR context written to /tmp/pr-context/\"\nls -la /tmp/pr-context/" + run: "set -euo pipefail\nmkdir -p /tmp/pr-context\n\n# PR metadata\ngh pr view \"$PR_NUMBER\" --json title,body,author,baseRefName,headRefName,headRefOid,url \\\n > /tmp/pr-context/pr.json\n\n# Full diff\nif ! gh pr diff \"$PR_NUMBER\" > /tmp/pr-context/pr.diff; then\n echo \"::warning::Failed to fetch full PR diff; per-file diffs from files.json are still available.\"\n : > /tmp/pr-context/pr.diff\nfi\n\n# Changed files list (--paginate may output concatenated arrays; jq -s 'add' merges them)\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/files.json\n\n# Per-file diffs\njq -c '.[]' /tmp/pr-context/files.json | while IFS= read -r entry; do\n filename=$(echo \"$entry\" | jq -r '.filename')\n mkdir -p \"/tmp/pr-context/diffs/$(dirname \"$filename\")\"\n echo \"$entry\" | jq -r '.patch // empty' > \"/tmp/pr-context/diffs/${filename}.diff\"\ndone\n\n# File orderings for sub-agent review (3 strategies)\njq -r '[.[] | .filename] | sort | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_az.txt\njq -r '[.[] | .filename] | sort | reverse | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_za.txt\njq -r '[.[] | {filename, size: ((.additions // 0) + (.deletions // 0))}] | sort_by(-.size) | .[].filename' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_largest.txt\n\n# Existing reviews\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/reviews.json\n\n# Review threads with resolution status (GraphQL — REST lacks isResolved/isOutdated)\ngh api graphql --paginate -f query='\n query($owner: String!, $repo: String!, $number: Int!, $endCursor: String) {\n repository(owner: $owner, name: $repo) {\n pullRequest(number: $number) {\n reviewThreads(first: 100, after: $endCursor) {\n pageInfo { hasNextPage endCursor }\n nodes {\n id\n isResolved\n isOutdated\n isCollapsed\n path\n line\n startLine\n comments(first: 100) {\n nodes {\n id\n databaseId\n body\n author { login }\n createdAt\n }\n }\n }\n }\n }\n }\n }\n' -F owner=\"${GITHUB_REPOSITORY%/*}\" -F repo=\"${GITHUB_REPOSITORY#*/}\" -F \"number=$PR_NUMBER\" \\\n --jq '.data.repository.pullRequest.reviewThreads.nodes' \\\n | jq -s 'add // []' > /tmp/pr-context/review_comments.json\n\n# Filtered review thread views (pre-computed so agents don't need to parse review_comments.json)\njq '[.[] | select(.isResolved == false)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/unresolved_threads.json\njq '[.[] | select(.isResolved == true)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/resolved_threads.json\njq '[.[] | select(.isOutdated == true)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/outdated_threads.json\n\n# Per-file review threads (mirrors diffs/ structure)\njq -c '.[]' /tmp/pr-context/review_comments.json | while IFS= read -r thread; do\n filepath=$(echo \"$thread\" | jq -r '.path // empty')\n [ -z \"$filepath\" ] && continue\n mkdir -p \"/tmp/pr-context/threads/$(dirname \"$filepath\")\"\n echo \"$thread\" >> \"/tmp/pr-context/threads/${filepath}.jsonl\"\ndone\n# Convert per-file JSONL to proper JSON arrays\nmkdir -p /tmp/pr-context/threads\nfind /tmp/pr-context/threads -name '*.jsonl' 2>/dev/null | while IFS= read -r jsonl; do\n jq -s '.' \"$jsonl\" > \"${jsonl%.jsonl}.json\"\n rm \"$jsonl\"\ndone\n\n# PR discussion comments\ngh api \"repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/comments.json\n\n# Linked issues\njq -r '.body // \"\"' /tmp/pr-context/pr.json 2>/dev/null \\\n | grep -oiE '(fixes|closes|resolves)\\s+#[0-9]+' \\\n | grep -oE '[0-9]+$' \\\n | sort -u \\\n | while read -r issue; do\n gh api \"repos/$GITHUB_REPOSITORY/issues/$issue\" > \"/tmp/pr-context/issue-${issue}.json\" || true\n done || true\n\n# Write manifest\ncat > /tmp/pr-context/README.md << 'MANIFEST'\n# PR Context\n\nPre-fetched PR data. All files are in `/tmp/pr-context/`.\n\n| File | Description |\n| --- | --- |\n| `pr.json` | PR metadata — title, body, author, base/head branches, head commit SHA (`headRefOid`), URL |\n| `pr.diff` | Full unified diff of all changes |\n| `files.json` | Changed files array — each entry has `filename`, `status`, `additions`, `deletions`, `patch` |\n| `diffs/.diff` | Per-file diffs — one file per changed file, mirroring the repo path under `diffs/` |\n| `file_order_az.txt` | Changed files sorted alphabetically (A→Z), one filename per line |\n| `file_order_za.txt` | Changed files sorted reverse-alphabetically (Z→A), one filename per line |\n| `file_order_largest.txt` | Changed files sorted by diff size descending (largest first), one filename per line |\n| `reviews.json` | Prior review submissions — author, state (APPROVED/CHANGES_REQUESTED/COMMENTED), body |\n| `review_comments.json` | All review threads (GraphQL) — each thread has `id` (node ID for resolving), `isResolved`, `isOutdated`, `path`, `line`, and nested `comments` with `id`, `databaseId` (numeric REST ID for replies), body/author |\n| `unresolved_threads.json` | Unresolved review threads — subset of `review_comments.json` where `isResolved` is false |\n| `resolved_threads.json` | Resolved review threads — subset of `review_comments.json` where `isResolved` is true |\n| `outdated_threads.json` | Outdated review threads — subset of `review_comments.json` where `isOutdated` is true (code changed since comment) |\n| `threads/.json` | Per-file review threads — one file per changed file with existing threads, mirroring the repo path under `threads/` |\n| `comments.json` | PR discussion comments (not inline) |\n| `issue-{N}.json` | Linked issue details (one file per linked issue, if any) |\n| `agents.md` | Repository conventions from `generate_agents_md` (if written by agent) |\n| `review-instructions.md` | Review instructions, criteria, and calibration examples (if written by review-process fragment) |\nMANIFEST\n\necho \"PR context written to /tmp/pr-context/\"\nls -la /tmp/pr-context/" - name: Write review instructions to disk run: "mkdir -p /tmp/pr-context\ncat > /tmp/pr-context/review-instructions.md << 'REVIEW_EOF'\n# Review Instructions for Sub-agents\n\nYou are a code review sub-agent. Read these instructions, then review the PR files in the order provided in your prompt.\n\n## Context\n\nBefore reviewing files, read these to understand the PR:\n\n1. `/tmp/pr-context/pr.json` — PR title, description, author, and branches. Understand what the PR is trying to accomplish.\n2. `/tmp/pr-context/agents.md` — Repository coding conventions and guidelines (if it exists).\n3. `/tmp/pr-context/review_comments.json` — Existing review threads. Note which files already have threads so you don't duplicate.\n4. `/tmp/pr-context/issue-*.json` — Linked issue details (if any). Understand the motivation and acceptance criteria.\n\n## Process\n\nReview the PR diff file by file in your assigned order. For each changed file:\n\n1. **Read the diff** for this file from `/tmp/pr-context/diffs/.diff` to understand what changed. If the diff is empty or truncated (e.g., binary files or very large changes), fall back to reading the full file from the workspace and comparing against context.\n2. **Read the full file from the workspace.** The PR branch is checked out locally — open the file directly to get complete contents with line numbers.\n3. **Check existing threads** for this file from `/tmp/pr-context/threads/.json` (if it exists). Skip issues that are already under discussion — each thread has `isResolved` and `isOutdated` fields.\n4. **Identify potential issues** matching the review criteria below.\n5. **Quick-check each issue** before including it:\n - What specific code pattern or change triggers this concern?\n - Is there an obvious guard, handler, or mitigation visible in the immediate context?\n - Can you describe a concrete failure scenario (the `evidence` field)? If you cannot articulate what specific input or state triggers the problem, drop the finding.\n - If the issue is clearly handled, skip it. If you're unsure, include it — the parent will verify.\n6. **Add to your findings list.** Do NOT leave inline comments — you don't have that tool. Return findings in this format:\n\n```\n- file: path/to/file\n line: 42\n severity: HIGH\n title: Brief title\n description: What the issue is and why it matters\n evidence: The specific code pattern and failure scenario\n suggestion: corrected code here (optional — only if you can provide a concrete fix)\n```\n\n**Review every file in your assigned order.** Files reviewed earlier get more attention, which is why different sub-agents use different orderings.\n\n**Check existing threads** — per-file threads are at `/tmp/pr-context/threads/.json` (step 3 above). The full list is at `/tmp/pr-context/review_comments.json`. Do not flag issues that are already under discussion (resolved or unresolved). For outdated threads, only re-flag if the issue still applies to the current diff.\n\n**Return your full findings list** when done, or an empty list if no issues were found.\n\n## Review Criteria\n\nFocus on these categories in priority order:\n\n1. Security vulnerabilities (injection, XSS, auth bypass, secrets exposure)\n2. Logic bugs that could cause runtime failures or incorrect behavior\n3. Data integrity issues (race conditions, missing transactions, corruption risk)\n4. Performance bottlenecks (N+1 queries, memory leaks, blocking operations)\n5. Error handling gaps (unhandled exceptions, missing validation)\n6. Breaking changes to public APIs without migration path\n7. Missing or incorrect test coverage for critical paths\n\n## What NOT to Flag\n\nOnly review the diff — do not flag issues in unchanged code, pre-existing problems not introduced by this PR, or style preferences handled by linters or formatters.\n\n**Common false positives** — these patterns look like issues but usually aren't. Before flagging anything in these categories, confirm the problem is real by reading the surrounding code:\n\n- **Security — input already sanitized:** Don't flag injection or XSS risks when inputs are sanitized upstream, parameterized queries are used, or the framework auto-escapes output.\n- **Null/undefined — guarded elsewhere:** Don't flag potential null dereferences if the value is guaranteed by a type guard, assertion, schema validation, or upstream null check.\n- **Error handling — handled at a different layer:** Don't flag missing try/catch if the caller, middleware, or framework catches and handles the error (e.g., Express error middleware, React error boundaries).\n- **Performance — theoretical, not practical:** Don't flag algorithmic complexity (e.g., O(n^2)) unless N is demonstrably large enough to matter in the actual usage context. \"This could be slow\" without evidence is not actionable.\n- **Validation — exists at another layer:** Don't flag missing input validation if it's handled by an API gateway, middleware, schema validator, or type system.\n- **Test coverage — trivial or generated code:** Don't flag missing tests for trivial getters/setters, auto-generated code, or simple delegation methods.\n- **Style or naming — not in coding guidelines:** Don't flag naming conventions or code style unless they violate the repository's documented coding guidelines (from `generate_agents_md` or CONTRIBUTING docs).\n\n**Existing review threads** — check BEFORE flagging any issue:\n\n- **Resolved with reviewer reply** (e.g. \"This is intentional\") — reviewer's decision is final. Do NOT re-flag.\n- **Resolved without reply** — author likely fixed it. Do NOT re-raise unless the fix introduced a new problem.\n- **Unresolved** — already flagged. Do NOT duplicate.\n- **Outdated** — only re-flag if the issue still applies to the current diff.\n\nWhen in doubt, do not duplicate. Redundant comments erode trust.\n\nFinding no issues is a valid and valuable outcome. An empty findings list is better than findings that waste the author's time or erode trust. Do not manufacture findings to justify your review — if the code is sound, return an empty list.\n\n## Severity Classification\n\nDetermine severity AFTER investigating the issue, not before. First identify the problem and trace through the code, then assign a severity based on the evidence you found.\n\n- 🔴 CRITICAL — Must fix before merge (security vulnerabilities, data corruption, production-breaking bugs)\n- 🟠 HIGH — Should fix before merge (logic errors, missing validation, significant performance issues)\n- 🟡 MEDIUM — Address soon, non-blocking (error handling gaps, suboptimal patterns, missing edge cases)\n- ⚪ LOW — Author discretion (minor improvements, documentation gaps)\n- 💬 NITPICK — Truly optional (stylistic preferences, alternative approaches)\n\n## Review Intensity\n\nThe review intensity is `${{ inputs.intensity || 'balanced' }}`.\n\n- **conservative**: High evidence bar. Only flag when you can demonstrate a concrete failure scenario. If you can construct a reasonable counterargument, do not flag. Approval with zero findings is the expected outcome for most PRs.\n- **balanced**: Standard evidence bar. Flag when you can point to specific code that would fail. If the issue is ambiguous, lean toward not flagging.\n- **aggressive**: Lower evidence bar. Flag when evidence exists even if the failure scenario is not fully confirmed. Improvement suggestions welcome but must cite specific code.\n\n## Calibration Examples\n\nUse these examples to calibrate your judgment. Each pair shows a real issue and a similar-looking pattern that is NOT an issue.\n\n### Example 1: Null/Undefined Access\n\n**True positive — flag this:**\n\n```js\n// PR adds this handler\napp.get('/user/:id', async (req, res) => {\n const user = await db.findUser(req.params.id);\n res.json({ name: user.name, email: user.email });\n});\n```\n\nWhy flag: `db.findUser()` can return `null` when no user matches the ID. Accessing `user.name` will throw a TypeError at runtime. No upstream guard exists — the route handler is the entry point.\n\n**False positive — do NOT flag this:**\n\n```ts\n// PR adds this line inside an existing function\nconst settings = user.getSettings();\n```\n\nWhy skip: Reading the full file reveals `user` is typed as `User` (not `User | null`), and the calling function only runs after `authenticateUser()` middleware which guarantees a valid user object. The null case is handled at a different layer.\n\n### Example 2: SQL Injection\n\n**True positive — flag this:**\n\n```python\n# PR adds this query\ncursor.execute(f\"SELECT * FROM orders WHERE customer_id = '{customer_id}'\")\n```\n\nWhy flag: String interpolation in a SQL query with user-controlled input (`customer_id` comes from the request). No parameterization or sanitization anywhere in the call chain.\n\n**False positive — do NOT flag this:**\n\n```python\n# PR adds this query\ncursor.execute(f\"SELECT * FROM orders WHERE status = '{OrderStatus.PENDING.value}'\")\n```\n\nWhy skip: The interpolated value is a hardcoded enum constant (`OrderStatus.PENDING`), not user input. There is no injection vector.\n\n### Example 3: Borderline — Do NOT Flag\n\n```go\n// PR adds this function\nfunc processItems(items []Item) []Result {\n results := make([]Result, 0)\n for _, item := range items {\n for _, tag := range item.Tags {\n results = append(results, process(item, tag))\n }\n }\n return results\n}\n```\n\nThis looks like an O(n*m) performance concern. But without evidence that `items` or `Tags` are large in practice, this is speculative. The function processes a bounded dataset (items from a single user request). Do not flag theoretical performance issues without evidence of real-world impact.\nREVIEW_EOF" - env: diff --git a/.github/workflows/gh-aw-mention-in-pr-no-sandbox.lock.yml b/.github/workflows/gh-aw-mention-in-pr-no-sandbox.lock.yml index 17f93fe9..4cf8e4d8 100644 --- a/.github/workflows/gh-aw-mention-in-pr-no-sandbox.lock.yml +++ b/.github/workflows/gh-aw-mention-in-pr-no-sandbox.lock.yml @@ -44,7 +44,7 @@ # # inlined-imports: true # -# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"b3ef49cde6385f81cbe4d462ee3fea0a8d34622d81d9481eab9554944ee6484a"} +# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"b8f87b805cb8e2d0dabcb69a1a14c5bfe167f537bbf66deda2f051163a3bbb21"} name: "Mention in PR (no sandbox)" "on": @@ -554,14 +554,14 @@ jobs: - **Bot-authored PRs**: If the PR author is `github-actions[bot]`, you can only submit a `COMMENT` review — `APPROVE` and `REQUEST_CHANGES` will fail because GitHub does not allow bot accounts to approve or request changes on their own PRs. Use `COMMENT` and state your verdict in the review body instead. **If asked to fix code or address review feedback:** - - Read `/tmp/pr-context/review_comments.json` to see open review threads and understand what needs to be addressed. + - Read `/tmp/pr-context/unresolved_threads.json` to see open review threads and understand what needs to be addressed. - For each unresolved thread you address: - Make the code changes in the workspace. - If the fix isn't obvious from the code change alone, call `reply_to_pull_request_review_comment` with the comment's numeric ID to briefly explain what you changed. - If you disagree with feedback or it's unclear, call `reply_to_pull_request_review_comment` to explain your reasoning instead of making changes. Do NOT resolve the thread — let the reviewer decide. - Run required repo commands (lint/build/test) from README, CONTRIBUTING, DEVELOPING, Makefile, or CI config relevant to the change and include results. If required commands cannot be run, explain why and do not push changes. - Use `push_to_pull_request_branch` to push your changes. - - After pushing, resolve every review thread that your changes address by calling `resolve_pull_request_review_thread` with the thread's GraphQL node ID (the `id` field, e.g., `PRRT_kwDO...`). This includes threads left by other reviewers AND threads from your own prior reviews. Check `/tmp/pr-context/review_comments.json` for all unresolved threads (`isResolved: false`) — `isOutdated` threads have had the underlying code changed since the comment was made, so check whether your changes address them. Do NOT resolve threads you disagreed with, skipped, or only partially addressed — leave those open for the reviewer. + - After pushing, resolve every review thread that your changes address by calling `resolve_pull_request_review_thread` with the thread's GraphQL node ID (the `id` field, e.g., `PRRT_kwDO...`). This includes threads left by other reviewers AND threads from your own prior reviews. Check `/tmp/pr-context/unresolved_threads.json` for all unresolved threads — also check `/tmp/pr-context/outdated_threads.json` for threads where the underlying code changed since the comment was made and verify whether your changes address them. Do NOT resolve threads you disagreed with, skipped, or only partially addressed — leave those open for the reviewer. - **Fork PRs**: Check via `pull_request_read` with method `get` whether the PR head repo differs from the base repo. If it's a fork, you cannot push — reply explaining that you do not have permission to push to fork branches and suggest that the PR author apply the changes themselves. This is a GitHub security limitation. You can still review code, make local changes, and provide suggestions. **If asked a question about the code:** @@ -756,7 +756,7 @@ jobs: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ github.event.pull_request.number || inputs.target-pr-number || github.event.issue.number }} name: Fetch PR context to disk - run: "set -euo pipefail\nmkdir -p /tmp/pr-context\n\n# PR metadata\ngh pr view \"$PR_NUMBER\" --json title,body,author,baseRefName,headRefName,headRefOid,url \\\n > /tmp/pr-context/pr.json\n\n# Full diff\nif ! gh pr diff \"$PR_NUMBER\" > /tmp/pr-context/pr.diff; then\n echo \"::warning::Failed to fetch full PR diff; per-file diffs from files.json are still available.\"\n : > /tmp/pr-context/pr.diff\nfi\n\n# Changed files list (--paginate may output concatenated arrays; jq -s 'add' merges them)\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/files.json\n\n# Per-file diffs\njq -c '.[]' /tmp/pr-context/files.json | while IFS= read -r entry; do\n filename=$(echo \"$entry\" | jq -r '.filename')\n mkdir -p \"/tmp/pr-context/diffs/$(dirname \"$filename\")\"\n echo \"$entry\" | jq -r '.patch // empty' > \"/tmp/pr-context/diffs/${filename}.diff\"\ndone\n\n# File orderings for sub-agent review (3 strategies)\njq -r '[.[] | .filename] | sort | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_az.txt\njq -r '[.[] | .filename] | sort | reverse | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_za.txt\njq -r '[.[] | {filename, size: ((.additions // 0) + (.deletions // 0))}] | sort_by(-.size) | .[].filename' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_largest.txt\n\n# Existing reviews\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/reviews.json\n\n# Review threads with resolution status (GraphQL — REST lacks isResolved/isOutdated)\ngh api graphql --paginate -f query='\n query($owner: String!, $repo: String!, $number: Int!, $endCursor: String) {\n repository(owner: $owner, name: $repo) {\n pullRequest(number: $number) {\n reviewThreads(first: 100, after: $endCursor) {\n pageInfo { hasNextPage endCursor }\n nodes {\n id\n isResolved\n isOutdated\n isCollapsed\n path\n line\n startLine\n comments(first: 100) {\n nodes {\n id\n databaseId\n body\n author { login }\n createdAt\n }\n }\n }\n }\n }\n }\n }\n' -F owner=\"${GITHUB_REPOSITORY%/*}\" -F repo=\"${GITHUB_REPOSITORY#*/}\" -F \"number=$PR_NUMBER\" \\\n --jq '.data.repository.pullRequest.reviewThreads.nodes' \\\n | jq -s 'add // []' > /tmp/pr-context/review_comments.json\n\n# Per-file review threads (mirrors diffs/ structure)\njq -c '.[]' /tmp/pr-context/review_comments.json | while IFS= read -r thread; do\n filepath=$(echo \"$thread\" | jq -r '.path // empty')\n [ -z \"$filepath\" ] && continue\n mkdir -p \"/tmp/pr-context/threads/$(dirname \"$filepath\")\"\n echo \"$thread\" >> \"/tmp/pr-context/threads/${filepath}.jsonl\"\ndone\n# Convert per-file JSONL to proper JSON arrays\nmkdir -p /tmp/pr-context/threads\nfind /tmp/pr-context/threads -name '*.jsonl' 2>/dev/null | while IFS= read -r jsonl; do\n jq -s '.' \"$jsonl\" > \"${jsonl%.jsonl}.json\"\n rm \"$jsonl\"\ndone\n\n# PR discussion comments\ngh api \"repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/comments.json\n\n# Linked issues\njq -r '.body // \"\"' /tmp/pr-context/pr.json 2>/dev/null \\\n | grep -oiE '(fixes|closes|resolves)\\s+#[0-9]+' \\\n | grep -oE '[0-9]+$' \\\n | sort -u \\\n | while read -r issue; do\n gh api \"repos/$GITHUB_REPOSITORY/issues/$issue\" > \"/tmp/pr-context/issue-${issue}.json\" || true\n done || true\n\n# Write manifest\ncat > /tmp/pr-context/README.md << 'MANIFEST'\n# PR Context\n\nPre-fetched PR data. All files are in `/tmp/pr-context/`.\n\n| File | Description |\n| --- | --- |\n| `pr.json` | PR metadata — title, body, author, base/head branches, head commit SHA (`headRefOid`), URL |\n| `pr.diff` | Full unified diff of all changes |\n| `files.json` | Changed files array — each entry has `filename`, `status`, `additions`, `deletions`, `patch` |\n| `diffs/.diff` | Per-file diffs — one file per changed file, mirroring the repo path under `diffs/` |\n| `file_order_az.txt` | Changed files sorted alphabetically (A→Z), one filename per line |\n| `file_order_za.txt` | Changed files sorted reverse-alphabetically (Z→A), one filename per line |\n| `file_order_largest.txt` | Changed files sorted by diff size descending (largest first), one filename per line |\n| `reviews.json` | Prior review submissions — author, state (APPROVED/CHANGES_REQUESTED/COMMENTED), body |\n| `review_comments.json` | All review threads (GraphQL) — each thread has `id` (node ID for resolving), `isResolved`, `isOutdated`, `path`, `line`, and nested `comments` with `id`, `databaseId` (numeric REST ID for replies), body/author |\n| `threads/.json` | Per-file review threads — one file per changed file with existing threads, mirroring the repo path under `threads/` |\n| `comments.json` | PR discussion comments (not inline) |\n| `issue-{N}.json` | Linked issue details (one file per linked issue, if any) |\n| `agents.md` | Repository conventions from `generate_agents_md` (if written by agent) |\n| `review-instructions.md` | Review instructions, criteria, and calibration examples (if written by review-process fragment) |\nMANIFEST\n\necho \"PR context written to /tmp/pr-context/\"\nls -la /tmp/pr-context/" + run: "set -euo pipefail\nmkdir -p /tmp/pr-context\n\n# PR metadata\ngh pr view \"$PR_NUMBER\" --json title,body,author,baseRefName,headRefName,headRefOid,url \\\n > /tmp/pr-context/pr.json\n\n# Full diff\nif ! gh pr diff \"$PR_NUMBER\" > /tmp/pr-context/pr.diff; then\n echo \"::warning::Failed to fetch full PR diff; per-file diffs from files.json are still available.\"\n : > /tmp/pr-context/pr.diff\nfi\n\n# Changed files list (--paginate may output concatenated arrays; jq -s 'add' merges them)\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/files.json\n\n# Per-file diffs\njq -c '.[]' /tmp/pr-context/files.json | while IFS= read -r entry; do\n filename=$(echo \"$entry\" | jq -r '.filename')\n mkdir -p \"/tmp/pr-context/diffs/$(dirname \"$filename\")\"\n echo \"$entry\" | jq -r '.patch // empty' > \"/tmp/pr-context/diffs/${filename}.diff\"\ndone\n\n# File orderings for sub-agent review (3 strategies)\njq -r '[.[] | .filename] | sort | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_az.txt\njq -r '[.[] | .filename] | sort | reverse | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_za.txt\njq -r '[.[] | {filename, size: ((.additions // 0) + (.deletions // 0))}] | sort_by(-.size) | .[].filename' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_largest.txt\n\n# Existing reviews\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/reviews.json\n\n# Review threads with resolution status (GraphQL — REST lacks isResolved/isOutdated)\ngh api graphql --paginate -f query='\n query($owner: String!, $repo: String!, $number: Int!, $endCursor: String) {\n repository(owner: $owner, name: $repo) {\n pullRequest(number: $number) {\n reviewThreads(first: 100, after: $endCursor) {\n pageInfo { hasNextPage endCursor }\n nodes {\n id\n isResolved\n isOutdated\n isCollapsed\n path\n line\n startLine\n comments(first: 100) {\n nodes {\n id\n databaseId\n body\n author { login }\n createdAt\n }\n }\n }\n }\n }\n }\n }\n' -F owner=\"${GITHUB_REPOSITORY%/*}\" -F repo=\"${GITHUB_REPOSITORY#*/}\" -F \"number=$PR_NUMBER\" \\\n --jq '.data.repository.pullRequest.reviewThreads.nodes' \\\n | jq -s 'add // []' > /tmp/pr-context/review_comments.json\n\n# Filtered review thread views (pre-computed so agents don't need to parse review_comments.json)\njq '[.[] | select(.isResolved == false)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/unresolved_threads.json\njq '[.[] | select(.isResolved == true)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/resolved_threads.json\njq '[.[] | select(.isOutdated == true)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/outdated_threads.json\n\n# Per-file review threads (mirrors diffs/ structure)\njq -c '.[]' /tmp/pr-context/review_comments.json | while IFS= read -r thread; do\n filepath=$(echo \"$thread\" | jq -r '.path // empty')\n [ -z \"$filepath\" ] && continue\n mkdir -p \"/tmp/pr-context/threads/$(dirname \"$filepath\")\"\n echo \"$thread\" >> \"/tmp/pr-context/threads/${filepath}.jsonl\"\ndone\n# Convert per-file JSONL to proper JSON arrays\nmkdir -p /tmp/pr-context/threads\nfind /tmp/pr-context/threads -name '*.jsonl' 2>/dev/null | while IFS= read -r jsonl; do\n jq -s '.' \"$jsonl\" > \"${jsonl%.jsonl}.json\"\n rm \"$jsonl\"\ndone\n\n# PR discussion comments\ngh api \"repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/comments.json\n\n# Linked issues\njq -r '.body // \"\"' /tmp/pr-context/pr.json 2>/dev/null \\\n | grep -oiE '(fixes|closes|resolves)\\s+#[0-9]+' \\\n | grep -oE '[0-9]+$' \\\n | sort -u \\\n | while read -r issue; do\n gh api \"repos/$GITHUB_REPOSITORY/issues/$issue\" > \"/tmp/pr-context/issue-${issue}.json\" || true\n done || true\n\n# Write manifest\ncat > /tmp/pr-context/README.md << 'MANIFEST'\n# PR Context\n\nPre-fetched PR data. All files are in `/tmp/pr-context/`.\n\n| File | Description |\n| --- | --- |\n| `pr.json` | PR metadata — title, body, author, base/head branches, head commit SHA (`headRefOid`), URL |\n| `pr.diff` | Full unified diff of all changes |\n| `files.json` | Changed files array — each entry has `filename`, `status`, `additions`, `deletions`, `patch` |\n| `diffs/.diff` | Per-file diffs — one file per changed file, mirroring the repo path under `diffs/` |\n| `file_order_az.txt` | Changed files sorted alphabetically (A→Z), one filename per line |\n| `file_order_za.txt` | Changed files sorted reverse-alphabetically (Z→A), one filename per line |\n| `file_order_largest.txt` | Changed files sorted by diff size descending (largest first), one filename per line |\n| `reviews.json` | Prior review submissions — author, state (APPROVED/CHANGES_REQUESTED/COMMENTED), body |\n| `review_comments.json` | All review threads (GraphQL) — each thread has `id` (node ID for resolving), `isResolved`, `isOutdated`, `path`, `line`, and nested `comments` with `id`, `databaseId` (numeric REST ID for replies), body/author |\n| `unresolved_threads.json` | Unresolved review threads — subset of `review_comments.json` where `isResolved` is false |\n| `resolved_threads.json` | Resolved review threads — subset of `review_comments.json` where `isResolved` is true |\n| `outdated_threads.json` | Outdated review threads — subset of `review_comments.json` where `isOutdated` is true (code changed since comment) |\n| `threads/.json` | Per-file review threads — one file per changed file with existing threads, mirroring the repo path under `threads/` |\n| `comments.json` | PR discussion comments (not inline) |\n| `issue-{N}.json` | Linked issue details (one file per linked issue, if any) |\n| `agents.md` | Repository conventions from `generate_agents_md` (if written by agent) |\n| `review-instructions.md` | Review instructions, criteria, and calibration examples (if written by review-process fragment) |\nMANIFEST\n\necho \"PR context written to /tmp/pr-context/\"\nls -la /tmp/pr-context/" - name: Write review instructions to disk run: "mkdir -p /tmp/pr-context\ncat > /tmp/pr-context/review-instructions.md << 'REVIEW_EOF'\n# Review Instructions for Sub-agents\n\nYou are a code review sub-agent. Read these instructions, then review the PR files in the order provided in your prompt.\n\n## Context\n\nBefore reviewing files, read these to understand the PR:\n\n1. `/tmp/pr-context/pr.json` — PR title, description, author, and branches. Understand what the PR is trying to accomplish.\n2. `/tmp/pr-context/agents.md` — Repository coding conventions and guidelines (if it exists).\n3. `/tmp/pr-context/review_comments.json` — Existing review threads. Note which files already have threads so you don't duplicate.\n4. `/tmp/pr-context/issue-*.json` — Linked issue details (if any). Understand the motivation and acceptance criteria.\n\n## Process\n\nReview the PR diff file by file in your assigned order. For each changed file:\n\n1. **Read the diff** for this file from `/tmp/pr-context/diffs/.diff` to understand what changed. If the diff is empty or truncated (e.g., binary files or very large changes), fall back to reading the full file from the workspace and comparing against context.\n2. **Read the full file from the workspace.** The PR branch is checked out locally — open the file directly to get complete contents with line numbers.\n3. **Check existing threads** for this file from `/tmp/pr-context/threads/.json` (if it exists). Skip issues that are already under discussion — each thread has `isResolved` and `isOutdated` fields.\n4. **Identify potential issues** matching the review criteria below.\n5. **Quick-check each issue** before including it:\n - What specific code pattern or change triggers this concern?\n - Is there an obvious guard, handler, or mitigation visible in the immediate context?\n - Can you describe a concrete failure scenario (the `evidence` field)? If you cannot articulate what specific input or state triggers the problem, drop the finding.\n - If the issue is clearly handled, skip it. If you're unsure, include it — the parent will verify.\n6. **Add to your findings list.** Do NOT leave inline comments — you don't have that tool. Return findings in this format:\n\n```\n- file: path/to/file\n line: 42\n severity: HIGH\n title: Brief title\n description: What the issue is and why it matters\n evidence: The specific code pattern and failure scenario\n suggestion: corrected code here (optional — only if you can provide a concrete fix)\n```\n\n**Review every file in your assigned order.** Files reviewed earlier get more attention, which is why different sub-agents use different orderings.\n\n**Check existing threads** — per-file threads are at `/tmp/pr-context/threads/.json` (step 3 above). The full list is at `/tmp/pr-context/review_comments.json`. Do not flag issues that are already under discussion (resolved or unresolved). For outdated threads, only re-flag if the issue still applies to the current diff.\n\n**Return your full findings list** when done, or an empty list if no issues were found.\n\n## Review Criteria\n\nFocus on these categories in priority order:\n\n1. Security vulnerabilities (injection, XSS, auth bypass, secrets exposure)\n2. Logic bugs that could cause runtime failures or incorrect behavior\n3. Data integrity issues (race conditions, missing transactions, corruption risk)\n4. Performance bottlenecks (N+1 queries, memory leaks, blocking operations)\n5. Error handling gaps (unhandled exceptions, missing validation)\n6. Breaking changes to public APIs without migration path\n7. Missing or incorrect test coverage for critical paths\n\n## What NOT to Flag\n\nOnly review the diff — do not flag issues in unchanged code, pre-existing problems not introduced by this PR, or style preferences handled by linters or formatters.\n\n**Common false positives** — these patterns look like issues but usually aren't. Before flagging anything in these categories, confirm the problem is real by reading the surrounding code:\n\n- **Security — input already sanitized:** Don't flag injection or XSS risks when inputs are sanitized upstream, parameterized queries are used, or the framework auto-escapes output.\n- **Null/undefined — guarded elsewhere:** Don't flag potential null dereferences if the value is guaranteed by a type guard, assertion, schema validation, or upstream null check.\n- **Error handling — handled at a different layer:** Don't flag missing try/catch if the caller, middleware, or framework catches and handles the error (e.g., Express error middleware, React error boundaries).\n- **Performance — theoretical, not practical:** Don't flag algorithmic complexity (e.g., O(n^2)) unless N is demonstrably large enough to matter in the actual usage context. \"This could be slow\" without evidence is not actionable.\n- **Validation — exists at another layer:** Don't flag missing input validation if it's handled by an API gateway, middleware, schema validator, or type system.\n- **Test coverage — trivial or generated code:** Don't flag missing tests for trivial getters/setters, auto-generated code, or simple delegation methods.\n- **Style or naming — not in coding guidelines:** Don't flag naming conventions or code style unless they violate the repository's documented coding guidelines (from `generate_agents_md` or CONTRIBUTING docs).\n\n**Existing review threads** — check BEFORE flagging any issue:\n\n- **Resolved with reviewer reply** (e.g. \"This is intentional\") — reviewer's decision is final. Do NOT re-flag.\n- **Resolved without reply** — author likely fixed it. Do NOT re-raise unless the fix introduced a new problem.\n- **Unresolved** — already flagged. Do NOT duplicate.\n- **Outdated** — only re-flag if the issue still applies to the current diff.\n\nWhen in doubt, do not duplicate. Redundant comments erode trust.\n\nFinding no issues is a valid and valuable outcome. An empty findings list is better than findings that waste the author's time or erode trust. Do not manufacture findings to justify your review — if the code is sound, return an empty list.\n\n## Severity Classification\n\nDetermine severity AFTER investigating the issue, not before. First identify the problem and trace through the code, then assign a severity based on the evidence you found.\n\n- 🔴 CRITICAL — Must fix before merge (security vulnerabilities, data corruption, production-breaking bugs)\n- 🟠 HIGH — Should fix before merge (logic errors, missing validation, significant performance issues)\n- 🟡 MEDIUM — Address soon, non-blocking (error handling gaps, suboptimal patterns, missing edge cases)\n- ⚪ LOW — Author discretion (minor improvements, documentation gaps)\n- 💬 NITPICK — Truly optional (stylistic preferences, alternative approaches)\n\n## Review Intensity\n\nThe review intensity is `${{ inputs.intensity || 'balanced' }}`.\n\n- **conservative**: High evidence bar. Only flag when you can demonstrate a concrete failure scenario. If you can construct a reasonable counterargument, do not flag. Approval with zero findings is the expected outcome for most PRs.\n- **balanced**: Standard evidence bar. Flag when you can point to specific code that would fail. If the issue is ambiguous, lean toward not flagging.\n- **aggressive**: Lower evidence bar. Flag when evidence exists even if the failure scenario is not fully confirmed. Improvement suggestions welcome but must cite specific code.\n\n## Calibration Examples\n\nUse these examples to calibrate your judgment. Each pair shows a real issue and a similar-looking pattern that is NOT an issue.\n\n### Example 1: Null/Undefined Access\n\n**True positive — flag this:**\n\n```js\n// PR adds this handler\napp.get('/user/:id', async (req, res) => {\n const user = await db.findUser(req.params.id);\n res.json({ name: user.name, email: user.email });\n});\n```\n\nWhy flag: `db.findUser()` can return `null` when no user matches the ID. Accessing `user.name` will throw a TypeError at runtime. No upstream guard exists — the route handler is the entry point.\n\n**False positive — do NOT flag this:**\n\n```ts\n// PR adds this line inside an existing function\nconst settings = user.getSettings();\n```\n\nWhy skip: Reading the full file reveals `user` is typed as `User` (not `User | null`), and the calling function only runs after `authenticateUser()` middleware which guarantees a valid user object. The null case is handled at a different layer.\n\n### Example 2: SQL Injection\n\n**True positive — flag this:**\n\n```python\n# PR adds this query\ncursor.execute(f\"SELECT * FROM orders WHERE customer_id = '{customer_id}'\")\n```\n\nWhy flag: String interpolation in a SQL query with user-controlled input (`customer_id` comes from the request). No parameterization or sanitization anywhere in the call chain.\n\n**False positive — do NOT flag this:**\n\n```python\n# PR adds this query\ncursor.execute(f\"SELECT * FROM orders WHERE status = '{OrderStatus.PENDING.value}'\")\n```\n\nWhy skip: The interpolated value is a hardcoded enum constant (`OrderStatus.PENDING`), not user input. There is no injection vector.\n\n### Example 3: Borderline — Do NOT Flag\n\n```go\n// PR adds this function\nfunc processItems(items []Item) []Result {\n results := make([]Result, 0)\n for _, item := range items {\n for _, tag := range item.Tags {\n results = append(results, process(item, tag))\n }\n }\n return results\n}\n```\n\nThis looks like an O(n*m) performance concern. But without evidence that `items` or `Tags` are large in practice, this is speculative. The function processes a bounded dataset (items from a single user request). Do not flag theoretical performance issues without evidence of real-world impact.\nREVIEW_EOF" - env: diff --git a/.github/workflows/gh-aw-mention-in-pr-no-sandbox.md b/.github/workflows/gh-aw-mention-in-pr-no-sandbox.md index 644037cb..f6129b81 100644 --- a/.github/workflows/gh-aw-mention-in-pr-no-sandbox.md +++ b/.github/workflows/gh-aw-mention-in-pr-no-sandbox.md @@ -156,14 +156,14 @@ Based on what's asked, do the appropriate thing: - **Bot-authored PRs**: If the PR author is `github-actions[bot]`, you can only submit a `COMMENT` review — `APPROVE` and `REQUEST_CHANGES` will fail because GitHub does not allow bot accounts to approve or request changes on their own PRs. Use `COMMENT` and state your verdict in the review body instead. **If asked to fix code or address review feedback:** -- Read `/tmp/pr-context/review_comments.json` to see open review threads and understand what needs to be addressed. +- Read `/tmp/pr-context/unresolved_threads.json` to see open review threads and understand what needs to be addressed. - For each unresolved thread you address: - Make the code changes in the workspace. - If the fix isn't obvious from the code change alone, call `reply_to_pull_request_review_comment` with the comment's numeric ID to briefly explain what you changed. - If you disagree with feedback or it's unclear, call `reply_to_pull_request_review_comment` to explain your reasoning instead of making changes. Do NOT resolve the thread — let the reviewer decide. - Run required repo commands (lint/build/test) from README, CONTRIBUTING, DEVELOPING, Makefile, or CI config relevant to the change and include results. If required commands cannot be run, explain why and do not push changes. - Use `push_to_pull_request_branch` to push your changes. -- After pushing, resolve every review thread that your changes address by calling `resolve_pull_request_review_thread` with the thread's GraphQL node ID (the `id` field, e.g., `PRRT_kwDO...`). This includes threads left by other reviewers AND threads from your own prior reviews. Check `/tmp/pr-context/review_comments.json` for all unresolved threads (`isResolved: false`) — `isOutdated` threads have had the underlying code changed since the comment was made, so check whether your changes address them. Do NOT resolve threads you disagreed with, skipped, or only partially addressed — leave those open for the reviewer. +- After pushing, resolve every review thread that your changes address by calling `resolve_pull_request_review_thread` with the thread's GraphQL node ID (the `id` field, e.g., `PRRT_kwDO...`). This includes threads left by other reviewers AND threads from your own prior reviews. Check `/tmp/pr-context/unresolved_threads.json` for all unresolved threads — also check `/tmp/pr-context/outdated_threads.json` for threads where the underlying code changed since the comment was made and verify whether your changes address them. Do NOT resolve threads you disagreed with, skipped, or only partially addressed — leave those open for the reviewer. - **Fork PRs**: Check via `pull_request_read` with method `get` whether the PR head repo differs from the base repo. If it's a fork, you cannot push — reply explaining that you do not have permission to push to fork branches and suggest that the PR author apply the changes themselves. This is a GitHub security limitation. You can still review code, make local changes, and provide suggestions. **If asked a question about the code:** diff --git a/.github/workflows/gh-aw-mention-in-pr.lock.yml b/.github/workflows/gh-aw-mention-in-pr.lock.yml index fc514788..8397c3e9 100644 --- a/.github/workflows/gh-aw-mention-in-pr.lock.yml +++ b/.github/workflows/gh-aw-mention-in-pr.lock.yml @@ -44,7 +44,7 @@ # # inlined-imports: true # -# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"dcfc3f738b1239dfa710eb6390c0d7232a4cdbb99128101cf3572c308f76a58c"} +# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"93982cd9e65d497aa1db1b9215850ab8773b3d6bcd4c01af401cef6e4059b706"} name: "Mention in PR" "on": @@ -567,14 +567,14 @@ jobs: - **Bot-authored PRs**: If the PR author is `github-actions[bot]`, you can only submit a `COMMENT` review — `APPROVE` and `REQUEST_CHANGES` will fail because GitHub does not allow bot accounts to approve or request changes on their own PRs. Use `COMMENT` and state your verdict in the review body instead. **If asked to fix code or address review feedback:** - - Read `/tmp/pr-context/review_comments.json` to see open review threads and understand what needs to be addressed. + - Read `/tmp/pr-context/unresolved_threads.json` to see open review threads and understand what needs to be addressed. - For each unresolved thread you address: - Make the code changes in the workspace. - If the fix isn't obvious from the code change alone, call `reply_to_pull_request_review_comment` with the comment's numeric ID to briefly explain what you changed. - If you disagree with feedback or it's unclear, call `reply_to_pull_request_review_comment` to explain your reasoning instead of making changes. Do NOT resolve the thread — let the reviewer decide. - Run required repo commands (lint/build/test) from README, CONTRIBUTING, DEVELOPING, Makefile, or CI config relevant to the change and include results. If required commands cannot be run, explain why and do not push changes. - Use `push_to_pull_request_branch` to push your changes. - - After pushing, resolve every review thread that your changes address by calling `resolve_pull_request_review_thread` with the thread's GraphQL node ID (the `id` field, e.g., `PRRT_kwDO...`). This includes threads left by other reviewers AND threads from your own prior reviews. Check `/tmp/pr-context/review_comments.json` for all unresolved threads (`isResolved: false`) — `isOutdated` threads have had the underlying code changed since the comment was made, so check whether your changes address them. Do NOT resolve threads you disagreed with, skipped, or only partially addressed — leave those open for the reviewer. + - After pushing, resolve every review thread that your changes address by calling `resolve_pull_request_review_thread` with the thread's GraphQL node ID (the `id` field, e.g., `PRRT_kwDO...`). This includes threads left by other reviewers AND threads from your own prior reviews. Check `/tmp/pr-context/unresolved_threads.json` for all unresolved threads — also check `/tmp/pr-context/outdated_threads.json` for threads where the underlying code changed since the comment was made and verify whether your changes address them. Do NOT resolve threads you disagreed with, skipped, or only partially addressed — leave those open for the reviewer. - **Fork PRs**: Check via `pull_request_read` with method `get` whether the PR head repo differs from the base repo. If it's a fork, you cannot push — reply explaining that you do not have permission to push to fork branches and suggest that the PR author apply the changes themselves. This is a GitHub security limitation. You can still review code, make local changes, and provide suggestions. **If asked to fix merge conflicts:** @@ -784,7 +784,7 @@ jobs: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ github.event.pull_request.number || inputs.target-pr-number || github.event.issue.number }} name: Fetch PR context to disk - run: "set -euo pipefail\nmkdir -p /tmp/pr-context\n\n# PR metadata\ngh pr view \"$PR_NUMBER\" --json title,body,author,baseRefName,headRefName,headRefOid,url \\\n > /tmp/pr-context/pr.json\n\n# Full diff\nif ! gh pr diff \"$PR_NUMBER\" > /tmp/pr-context/pr.diff; then\n echo \"::warning::Failed to fetch full PR diff; per-file diffs from files.json are still available.\"\n : > /tmp/pr-context/pr.diff\nfi\n\n# Changed files list (--paginate may output concatenated arrays; jq -s 'add' merges them)\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/files.json\n\n# Per-file diffs\njq -c '.[]' /tmp/pr-context/files.json | while IFS= read -r entry; do\n filename=$(echo \"$entry\" | jq -r '.filename')\n mkdir -p \"/tmp/pr-context/diffs/$(dirname \"$filename\")\"\n echo \"$entry\" | jq -r '.patch // empty' > \"/tmp/pr-context/diffs/${filename}.diff\"\ndone\n\n# File orderings for sub-agent review (3 strategies)\njq -r '[.[] | .filename] | sort | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_az.txt\njq -r '[.[] | .filename] | sort | reverse | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_za.txt\njq -r '[.[] | {filename, size: ((.additions // 0) + (.deletions // 0))}] | sort_by(-.size) | .[].filename' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_largest.txt\n\n# Existing reviews\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/reviews.json\n\n# Review threads with resolution status (GraphQL — REST lacks isResolved/isOutdated)\ngh api graphql --paginate -f query='\n query($owner: String!, $repo: String!, $number: Int!, $endCursor: String) {\n repository(owner: $owner, name: $repo) {\n pullRequest(number: $number) {\n reviewThreads(first: 100, after: $endCursor) {\n pageInfo { hasNextPage endCursor }\n nodes {\n id\n isResolved\n isOutdated\n isCollapsed\n path\n line\n startLine\n comments(first: 100) {\n nodes {\n id\n databaseId\n body\n author { login }\n createdAt\n }\n }\n }\n }\n }\n }\n }\n' -F owner=\"${GITHUB_REPOSITORY%/*}\" -F repo=\"${GITHUB_REPOSITORY#*/}\" -F \"number=$PR_NUMBER\" \\\n --jq '.data.repository.pullRequest.reviewThreads.nodes' \\\n | jq -s 'add // []' > /tmp/pr-context/review_comments.json\n\n# Per-file review threads (mirrors diffs/ structure)\njq -c '.[]' /tmp/pr-context/review_comments.json | while IFS= read -r thread; do\n filepath=$(echo \"$thread\" | jq -r '.path // empty')\n [ -z \"$filepath\" ] && continue\n mkdir -p \"/tmp/pr-context/threads/$(dirname \"$filepath\")\"\n echo \"$thread\" >> \"/tmp/pr-context/threads/${filepath}.jsonl\"\ndone\n# Convert per-file JSONL to proper JSON arrays\nmkdir -p /tmp/pr-context/threads\nfind /tmp/pr-context/threads -name '*.jsonl' 2>/dev/null | while IFS= read -r jsonl; do\n jq -s '.' \"$jsonl\" > \"${jsonl%.jsonl}.json\"\n rm \"$jsonl\"\ndone\n\n# PR discussion comments\ngh api \"repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/comments.json\n\n# Linked issues\njq -r '.body // \"\"' /tmp/pr-context/pr.json 2>/dev/null \\\n | grep -oiE '(fixes|closes|resolves)\\s+#[0-9]+' \\\n | grep -oE '[0-9]+$' \\\n | sort -u \\\n | while read -r issue; do\n gh api \"repos/$GITHUB_REPOSITORY/issues/$issue\" > \"/tmp/pr-context/issue-${issue}.json\" || true\n done || true\n\n# Write manifest\ncat > /tmp/pr-context/README.md << 'MANIFEST'\n# PR Context\n\nPre-fetched PR data. All files are in `/tmp/pr-context/`.\n\n| File | Description |\n| --- | --- |\n| `pr.json` | PR metadata — title, body, author, base/head branches, head commit SHA (`headRefOid`), URL |\n| `pr.diff` | Full unified diff of all changes |\n| `files.json` | Changed files array — each entry has `filename`, `status`, `additions`, `deletions`, `patch` |\n| `diffs/.diff` | Per-file diffs — one file per changed file, mirroring the repo path under `diffs/` |\n| `file_order_az.txt` | Changed files sorted alphabetically (A→Z), one filename per line |\n| `file_order_za.txt` | Changed files sorted reverse-alphabetically (Z→A), one filename per line |\n| `file_order_largest.txt` | Changed files sorted by diff size descending (largest first), one filename per line |\n| `reviews.json` | Prior review submissions — author, state (APPROVED/CHANGES_REQUESTED/COMMENTED), body |\n| `review_comments.json` | All review threads (GraphQL) — each thread has `id` (node ID for resolving), `isResolved`, `isOutdated`, `path`, `line`, and nested `comments` with `id`, `databaseId` (numeric REST ID for replies), body/author |\n| `threads/.json` | Per-file review threads — one file per changed file with existing threads, mirroring the repo path under `threads/` |\n| `comments.json` | PR discussion comments (not inline) |\n| `issue-{N}.json` | Linked issue details (one file per linked issue, if any) |\n| `agents.md` | Repository conventions from `generate_agents_md` (if written by agent) |\n| `review-instructions.md` | Review instructions, criteria, and calibration examples (if written by review-process fragment) |\nMANIFEST\n\necho \"PR context written to /tmp/pr-context/\"\nls -la /tmp/pr-context/" + run: "set -euo pipefail\nmkdir -p /tmp/pr-context\n\n# PR metadata\ngh pr view \"$PR_NUMBER\" --json title,body,author,baseRefName,headRefName,headRefOid,url \\\n > /tmp/pr-context/pr.json\n\n# Full diff\nif ! gh pr diff \"$PR_NUMBER\" > /tmp/pr-context/pr.diff; then\n echo \"::warning::Failed to fetch full PR diff; per-file diffs from files.json are still available.\"\n : > /tmp/pr-context/pr.diff\nfi\n\n# Changed files list (--paginate may output concatenated arrays; jq -s 'add' merges them)\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/files.json\n\n# Per-file diffs\njq -c '.[]' /tmp/pr-context/files.json | while IFS= read -r entry; do\n filename=$(echo \"$entry\" | jq -r '.filename')\n mkdir -p \"/tmp/pr-context/diffs/$(dirname \"$filename\")\"\n echo \"$entry\" | jq -r '.patch // empty' > \"/tmp/pr-context/diffs/${filename}.diff\"\ndone\n\n# File orderings for sub-agent review (3 strategies)\njq -r '[.[] | .filename] | sort | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_az.txt\njq -r '[.[] | .filename] | sort | reverse | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_za.txt\njq -r '[.[] | {filename, size: ((.additions // 0) + (.deletions // 0))}] | sort_by(-.size) | .[].filename' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_largest.txt\n\n# Existing reviews\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/reviews.json\n\n# Review threads with resolution status (GraphQL — REST lacks isResolved/isOutdated)\ngh api graphql --paginate -f query='\n query($owner: String!, $repo: String!, $number: Int!, $endCursor: String) {\n repository(owner: $owner, name: $repo) {\n pullRequest(number: $number) {\n reviewThreads(first: 100, after: $endCursor) {\n pageInfo { hasNextPage endCursor }\n nodes {\n id\n isResolved\n isOutdated\n isCollapsed\n path\n line\n startLine\n comments(first: 100) {\n nodes {\n id\n databaseId\n body\n author { login }\n createdAt\n }\n }\n }\n }\n }\n }\n }\n' -F owner=\"${GITHUB_REPOSITORY%/*}\" -F repo=\"${GITHUB_REPOSITORY#*/}\" -F \"number=$PR_NUMBER\" \\\n --jq '.data.repository.pullRequest.reviewThreads.nodes' \\\n | jq -s 'add // []' > /tmp/pr-context/review_comments.json\n\n# Filtered review thread views (pre-computed so agents don't need to parse review_comments.json)\njq '[.[] | select(.isResolved == false)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/unresolved_threads.json\njq '[.[] | select(.isResolved == true)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/resolved_threads.json\njq '[.[] | select(.isOutdated == true)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/outdated_threads.json\n\n# Per-file review threads (mirrors diffs/ structure)\njq -c '.[]' /tmp/pr-context/review_comments.json | while IFS= read -r thread; do\n filepath=$(echo \"$thread\" | jq -r '.path // empty')\n [ -z \"$filepath\" ] && continue\n mkdir -p \"/tmp/pr-context/threads/$(dirname \"$filepath\")\"\n echo \"$thread\" >> \"/tmp/pr-context/threads/${filepath}.jsonl\"\ndone\n# Convert per-file JSONL to proper JSON arrays\nmkdir -p /tmp/pr-context/threads\nfind /tmp/pr-context/threads -name '*.jsonl' 2>/dev/null | while IFS= read -r jsonl; do\n jq -s '.' \"$jsonl\" > \"${jsonl%.jsonl}.json\"\n rm \"$jsonl\"\ndone\n\n# PR discussion comments\ngh api \"repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/comments.json\n\n# Linked issues\njq -r '.body // \"\"' /tmp/pr-context/pr.json 2>/dev/null \\\n | grep -oiE '(fixes|closes|resolves)\\s+#[0-9]+' \\\n | grep -oE '[0-9]+$' \\\n | sort -u \\\n | while read -r issue; do\n gh api \"repos/$GITHUB_REPOSITORY/issues/$issue\" > \"/tmp/pr-context/issue-${issue}.json\" || true\n done || true\n\n# Write manifest\ncat > /tmp/pr-context/README.md << 'MANIFEST'\n# PR Context\n\nPre-fetched PR data. All files are in `/tmp/pr-context/`.\n\n| File | Description |\n| --- | --- |\n| `pr.json` | PR metadata — title, body, author, base/head branches, head commit SHA (`headRefOid`), URL |\n| `pr.diff` | Full unified diff of all changes |\n| `files.json` | Changed files array — each entry has `filename`, `status`, `additions`, `deletions`, `patch` |\n| `diffs/.diff` | Per-file diffs — one file per changed file, mirroring the repo path under `diffs/` |\n| `file_order_az.txt` | Changed files sorted alphabetically (A→Z), one filename per line |\n| `file_order_za.txt` | Changed files sorted reverse-alphabetically (Z→A), one filename per line |\n| `file_order_largest.txt` | Changed files sorted by diff size descending (largest first), one filename per line |\n| `reviews.json` | Prior review submissions — author, state (APPROVED/CHANGES_REQUESTED/COMMENTED), body |\n| `review_comments.json` | All review threads (GraphQL) — each thread has `id` (node ID for resolving), `isResolved`, `isOutdated`, `path`, `line`, and nested `comments` with `id`, `databaseId` (numeric REST ID for replies), body/author |\n| `unresolved_threads.json` | Unresolved review threads — subset of `review_comments.json` where `isResolved` is false |\n| `resolved_threads.json` | Resolved review threads — subset of `review_comments.json` where `isResolved` is true |\n| `outdated_threads.json` | Outdated review threads — subset of `review_comments.json` where `isOutdated` is true (code changed since comment) |\n| `threads/.json` | Per-file review threads — one file per changed file with existing threads, mirroring the repo path under `threads/` |\n| `comments.json` | PR discussion comments (not inline) |\n| `issue-{N}.json` | Linked issue details (one file per linked issue, if any) |\n| `agents.md` | Repository conventions from `generate_agents_md` (if written by agent) |\n| `review-instructions.md` | Review instructions, criteria, and calibration examples (if written by review-process fragment) |\nMANIFEST\n\necho \"PR context written to /tmp/pr-context/\"\nls -la /tmp/pr-context/" - name: Write review instructions to disk run: "mkdir -p /tmp/pr-context\ncat > /tmp/pr-context/review-instructions.md << 'REVIEW_EOF'\n# Review Instructions for Sub-agents\n\nYou are a code review sub-agent. Read these instructions, then review the PR files in the order provided in your prompt.\n\n## Context\n\nBefore reviewing files, read these to understand the PR:\n\n1. `/tmp/pr-context/pr.json` — PR title, description, author, and branches. Understand what the PR is trying to accomplish.\n2. `/tmp/pr-context/agents.md` — Repository coding conventions and guidelines (if it exists).\n3. `/tmp/pr-context/review_comments.json` — Existing review threads. Note which files already have threads so you don't duplicate.\n4. `/tmp/pr-context/issue-*.json` — Linked issue details (if any). Understand the motivation and acceptance criteria.\n\n## Process\n\nReview the PR diff file by file in your assigned order. For each changed file:\n\n1. **Read the diff** for this file from `/tmp/pr-context/diffs/.diff` to understand what changed. If the diff is empty or truncated (e.g., binary files or very large changes), fall back to reading the full file from the workspace and comparing against context.\n2. **Read the full file from the workspace.** The PR branch is checked out locally — open the file directly to get complete contents with line numbers.\n3. **Check existing threads** for this file from `/tmp/pr-context/threads/.json` (if it exists). Skip issues that are already under discussion — each thread has `isResolved` and `isOutdated` fields.\n4. **Identify potential issues** matching the review criteria below.\n5. **Quick-check each issue** before including it:\n - What specific code pattern or change triggers this concern?\n - Is there an obvious guard, handler, or mitigation visible in the immediate context?\n - Can you describe a concrete failure scenario (the `evidence` field)? If you cannot articulate what specific input or state triggers the problem, drop the finding.\n - If the issue is clearly handled, skip it. If you're unsure, include it — the parent will verify.\n6. **Add to your findings list.** Do NOT leave inline comments — you don't have that tool. Return findings in this format:\n\n```\n- file: path/to/file\n line: 42\n severity: HIGH\n title: Brief title\n description: What the issue is and why it matters\n evidence: The specific code pattern and failure scenario\n suggestion: corrected code here (optional — only if you can provide a concrete fix)\n```\n\n**Review every file in your assigned order.** Files reviewed earlier get more attention, which is why different sub-agents use different orderings.\n\n**Check existing threads** — per-file threads are at `/tmp/pr-context/threads/.json` (step 3 above). The full list is at `/tmp/pr-context/review_comments.json`. Do not flag issues that are already under discussion (resolved or unresolved). For outdated threads, only re-flag if the issue still applies to the current diff.\n\n**Return your full findings list** when done, or an empty list if no issues were found.\n\n## Review Criteria\n\nFocus on these categories in priority order:\n\n1. Security vulnerabilities (injection, XSS, auth bypass, secrets exposure)\n2. Logic bugs that could cause runtime failures or incorrect behavior\n3. Data integrity issues (race conditions, missing transactions, corruption risk)\n4. Performance bottlenecks (N+1 queries, memory leaks, blocking operations)\n5. Error handling gaps (unhandled exceptions, missing validation)\n6. Breaking changes to public APIs without migration path\n7. Missing or incorrect test coverage for critical paths\n\n## What NOT to Flag\n\nOnly review the diff — do not flag issues in unchanged code, pre-existing problems not introduced by this PR, or style preferences handled by linters or formatters.\n\n**Common false positives** — these patterns look like issues but usually aren't. Before flagging anything in these categories, confirm the problem is real by reading the surrounding code:\n\n- **Security — input already sanitized:** Don't flag injection or XSS risks when inputs are sanitized upstream, parameterized queries are used, or the framework auto-escapes output.\n- **Null/undefined — guarded elsewhere:** Don't flag potential null dereferences if the value is guaranteed by a type guard, assertion, schema validation, or upstream null check.\n- **Error handling — handled at a different layer:** Don't flag missing try/catch if the caller, middleware, or framework catches and handles the error (e.g., Express error middleware, React error boundaries).\n- **Performance — theoretical, not practical:** Don't flag algorithmic complexity (e.g., O(n^2)) unless N is demonstrably large enough to matter in the actual usage context. \"This could be slow\" without evidence is not actionable.\n- **Validation — exists at another layer:** Don't flag missing input validation if it's handled by an API gateway, middleware, schema validator, or type system.\n- **Test coverage — trivial or generated code:** Don't flag missing tests for trivial getters/setters, auto-generated code, or simple delegation methods.\n- **Style or naming — not in coding guidelines:** Don't flag naming conventions or code style unless they violate the repository's documented coding guidelines (from `generate_agents_md` or CONTRIBUTING docs).\n\n**Existing review threads** — check BEFORE flagging any issue:\n\n- **Resolved with reviewer reply** (e.g. \"This is intentional\") — reviewer's decision is final. Do NOT re-flag.\n- **Resolved without reply** — author likely fixed it. Do NOT re-raise unless the fix introduced a new problem.\n- **Unresolved** — already flagged. Do NOT duplicate.\n- **Outdated** — only re-flag if the issue still applies to the current diff.\n\nWhen in doubt, do not duplicate. Redundant comments erode trust.\n\nFinding no issues is a valid and valuable outcome. An empty findings list is better than findings that waste the author's time or erode trust. Do not manufacture findings to justify your review — if the code is sound, return an empty list.\n\n## Severity Classification\n\nDetermine severity AFTER investigating the issue, not before. First identify the problem and trace through the code, then assign a severity based on the evidence you found.\n\n- 🔴 CRITICAL — Must fix before merge (security vulnerabilities, data corruption, production-breaking bugs)\n- 🟠 HIGH — Should fix before merge (logic errors, missing validation, significant performance issues)\n- 🟡 MEDIUM — Address soon, non-blocking (error handling gaps, suboptimal patterns, missing edge cases)\n- ⚪ LOW — Author discretion (minor improvements, documentation gaps)\n- 💬 NITPICK — Truly optional (stylistic preferences, alternative approaches)\n\n## Review Intensity\n\nThe review intensity is `${{ inputs.intensity || 'balanced' }}`.\n\n- **conservative**: High evidence bar. Only flag when you can demonstrate a concrete failure scenario. If you can construct a reasonable counterargument, do not flag. Approval with zero findings is the expected outcome for most PRs.\n- **balanced**: Standard evidence bar. Flag when you can point to specific code that would fail. If the issue is ambiguous, lean toward not flagging.\n- **aggressive**: Lower evidence bar. Flag when evidence exists even if the failure scenario is not fully confirmed. Improvement suggestions welcome but must cite specific code.\n\n## Calibration Examples\n\nUse these examples to calibrate your judgment. Each pair shows a real issue and a similar-looking pattern that is NOT an issue.\n\n### Example 1: Null/Undefined Access\n\n**True positive — flag this:**\n\n```js\n// PR adds this handler\napp.get('/user/:id', async (req, res) => {\n const user = await db.findUser(req.params.id);\n res.json({ name: user.name, email: user.email });\n});\n```\n\nWhy flag: `db.findUser()` can return `null` when no user matches the ID. Accessing `user.name` will throw a TypeError at runtime. No upstream guard exists — the route handler is the entry point.\n\n**False positive — do NOT flag this:**\n\n```ts\n// PR adds this line inside an existing function\nconst settings = user.getSettings();\n```\n\nWhy skip: Reading the full file reveals `user` is typed as `User` (not `User | null`), and the calling function only runs after `authenticateUser()` middleware which guarantees a valid user object. The null case is handled at a different layer.\n\n### Example 2: SQL Injection\n\n**True positive — flag this:**\n\n```python\n# PR adds this query\ncursor.execute(f\"SELECT * FROM orders WHERE customer_id = '{customer_id}'\")\n```\n\nWhy flag: String interpolation in a SQL query with user-controlled input (`customer_id` comes from the request). No parameterization or sanitization anywhere in the call chain.\n\n**False positive — do NOT flag this:**\n\n```python\n# PR adds this query\ncursor.execute(f\"SELECT * FROM orders WHERE status = '{OrderStatus.PENDING.value}'\")\n```\n\nWhy skip: The interpolated value is a hardcoded enum constant (`OrderStatus.PENDING`), not user input. There is no injection vector.\n\n### Example 3: Borderline — Do NOT Flag\n\n```go\n// PR adds this function\nfunc processItems(items []Item) []Result {\n results := make([]Result, 0)\n for _, item := range items {\n for _, tag := range item.Tags {\n results = append(results, process(item, tag))\n }\n }\n return results\n}\n```\n\nThis looks like an O(n*m) performance concern. But without evidence that `items` or `Tags` are large in practice, this is speculative. The function processes a bounded dataset (items from a single user request). Do not flag theoretical performance issues without evidence of real-world impact.\nREVIEW_EOF" - env: diff --git a/.github/workflows/gh-aw-mention-in-pr.md b/.github/workflows/gh-aw-mention-in-pr.md index 62e0a8d7..70d7f00d 100644 --- a/.github/workflows/gh-aw-mention-in-pr.md +++ b/.github/workflows/gh-aw-mention-in-pr.md @@ -164,14 +164,14 @@ Based on what's asked, do the appropriate thing. **Requests can combine multiple - **Bot-authored PRs**: If the PR author is `github-actions[bot]`, you can only submit a `COMMENT` review — `APPROVE` and `REQUEST_CHANGES` will fail because GitHub does not allow bot accounts to approve or request changes on their own PRs. Use `COMMENT` and state your verdict in the review body instead. **If asked to fix code or address review feedback:** -- Read `/tmp/pr-context/review_comments.json` to see open review threads and understand what needs to be addressed. +- Read `/tmp/pr-context/unresolved_threads.json` to see open review threads and understand what needs to be addressed. - For each unresolved thread you address: - Make the code changes in the workspace. - If the fix isn't obvious from the code change alone, call `reply_to_pull_request_review_comment` with the comment's numeric ID to briefly explain what you changed. - If you disagree with feedback or it's unclear, call `reply_to_pull_request_review_comment` to explain your reasoning instead of making changes. Do NOT resolve the thread — let the reviewer decide. - Run required repo commands (lint/build/test) from README, CONTRIBUTING, DEVELOPING, Makefile, or CI config relevant to the change and include results. If required commands cannot be run, explain why and do not push changes. - Use `push_to_pull_request_branch` to push your changes. -- After pushing, resolve every review thread that your changes address by calling `resolve_pull_request_review_thread` with the thread's GraphQL node ID (the `id` field, e.g., `PRRT_kwDO...`). This includes threads left by other reviewers AND threads from your own prior reviews. Check `/tmp/pr-context/review_comments.json` for all unresolved threads (`isResolved: false`) — `isOutdated` threads have had the underlying code changed since the comment was made, so check whether your changes address them. Do NOT resolve threads you disagreed with, skipped, or only partially addressed — leave those open for the reviewer. +- After pushing, resolve every review thread that your changes address by calling `resolve_pull_request_review_thread` with the thread's GraphQL node ID (the `id` field, e.g., `PRRT_kwDO...`). This includes threads left by other reviewers AND threads from your own prior reviews. Check `/tmp/pr-context/unresolved_threads.json` for all unresolved threads — also check `/tmp/pr-context/outdated_threads.json` for threads where the underlying code changed since the comment was made and verify whether your changes address them. Do NOT resolve threads you disagreed with, skipped, or only partially addressed — leave those open for the reviewer. - **Fork PRs**: Check via `pull_request_read` with method `get` whether the PR head repo differs from the base repo. If it's a fork, you cannot push — reply explaining that you do not have permission to push to fork branches and suggest that the PR author apply the changes themselves. This is a GitHub security limitation. You can still review code, make local changes, and provide suggestions. **If asked to fix merge conflicts:** diff --git a/.github/workflows/gh-aw-pr-review-addresser.lock.yml b/.github/workflows/gh-aw-pr-review-addresser.lock.yml index 454f2539..5a23878b 100644 --- a/.github/workflows/gh-aw-pr-review-addresser.lock.yml +++ b/.github/workflows/gh-aw-pr-review-addresser.lock.yml @@ -40,7 +40,7 @@ # # inlined-imports: true # -# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"cd1dad8eacfc407bb65ea9f5b9f248ea4de5d9c4d0b303d4c43509313459b9e6"} +# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"e14207a7410bee2f30a37e655d590e23de4f5a211b2f562a06622289c3b028b2"} name: "PR Review Addresser" "on": @@ -389,7 +389,7 @@ jobs: 1. Call `generate_agents_md` to get the repository's coding guidelines and conventions. If this fails, continue without it. 2. Read `/tmp/pr-context/pr.json` for PR details (author, description, branches). Check whether this is a fork PR — if the head repo differs from the base repo, you cannot push changes. 3. Read `/tmp/pr-context/issue-*.json` if any exist to understand linked issue motivation and requirements. - 4. Read `/tmp/pr-context/review_comments.json` to get all review threads. Identify which threads are unresolved and need attention. + 4. Read `/tmp/pr-context/unresolved_threads.json` to get unresolved review threads that need attention. For full context including resolved threads, see `review_comments.json`. 5. Read `/tmp/pr-context/diffs/` to understand the current state of changes. ### Step 2: Address Each Review Thread @@ -412,7 +412,7 @@ jobs: Skip this step for fork PRs where you could not push. - After pushing, resolve every review thread that your changes address by calling `resolve_pull_request_review_thread` with the thread's GraphQL node ID (the `id` field, e.g., `PRRT_kwDO...`). This includes threads from any reviewer — external reviewers, bots, and your own prior reviews. Check `/tmp/pr-context/review_comments.json` for all unresolved threads (`isResolved: false`) — `isOutdated` threads have had the underlying code changed since the comment was made, so check whether your changes address them. Do NOT resolve threads you disagreed with, skipped, or only partially addressed — leave those open for the reviewer. Fall back to `pull_request_read` with method `get_review_comments` if the pre-fetched data is unavailable. + After pushing, resolve every review thread that your changes address by calling `resolve_pull_request_review_thread` with the thread's GraphQL node ID (the `id` field, e.g., `PRRT_kwDO...`). This includes threads from any reviewer — external reviewers, bots, and your own prior reviews. Check `/tmp/pr-context/unresolved_threads.json` for all unresolved threads — also check `/tmp/pr-context/outdated_threads.json` for threads where the underlying code changed since the comment was made and verify whether your changes address them. Do NOT resolve threads you disagreed with, skipped, or only partially addressed — leave those open for the reviewer. Fall back to `pull_request_read` with method `get_review_comments` if the pre-fetched data is unavailable. ### Step 5: Respond @@ -599,7 +599,7 @@ jobs: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ github.event.pull_request.number || inputs.target-pr-number || github.event.issue.number }} name: Fetch PR context to disk - run: "set -euo pipefail\nmkdir -p /tmp/pr-context\n\n# PR metadata\ngh pr view \"$PR_NUMBER\" --json title,body,author,baseRefName,headRefName,headRefOid,url \\\n > /tmp/pr-context/pr.json\n\n# Full diff\nif ! gh pr diff \"$PR_NUMBER\" > /tmp/pr-context/pr.diff; then\n echo \"::warning::Failed to fetch full PR diff; per-file diffs from files.json are still available.\"\n : > /tmp/pr-context/pr.diff\nfi\n\n# Changed files list (--paginate may output concatenated arrays; jq -s 'add' merges them)\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/files.json\n\n# Per-file diffs\njq -c '.[]' /tmp/pr-context/files.json | while IFS= read -r entry; do\n filename=$(echo \"$entry\" | jq -r '.filename')\n mkdir -p \"/tmp/pr-context/diffs/$(dirname \"$filename\")\"\n echo \"$entry\" | jq -r '.patch // empty' > \"/tmp/pr-context/diffs/${filename}.diff\"\ndone\n\n# File orderings for sub-agent review (3 strategies)\njq -r '[.[] | .filename] | sort | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_az.txt\njq -r '[.[] | .filename] | sort | reverse | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_za.txt\njq -r '[.[] | {filename, size: ((.additions // 0) + (.deletions // 0))}] | sort_by(-.size) | .[].filename' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_largest.txt\n\n# Existing reviews\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/reviews.json\n\n# Review threads with resolution status (GraphQL — REST lacks isResolved/isOutdated)\ngh api graphql --paginate -f query='\n query($owner: String!, $repo: String!, $number: Int!, $endCursor: String) {\n repository(owner: $owner, name: $repo) {\n pullRequest(number: $number) {\n reviewThreads(first: 100, after: $endCursor) {\n pageInfo { hasNextPage endCursor }\n nodes {\n id\n isResolved\n isOutdated\n isCollapsed\n path\n line\n startLine\n comments(first: 100) {\n nodes {\n id\n databaseId\n body\n author { login }\n createdAt\n }\n }\n }\n }\n }\n }\n }\n' -F owner=\"${GITHUB_REPOSITORY%/*}\" -F repo=\"${GITHUB_REPOSITORY#*/}\" -F \"number=$PR_NUMBER\" \\\n --jq '.data.repository.pullRequest.reviewThreads.nodes' \\\n | jq -s 'add // []' > /tmp/pr-context/review_comments.json\n\n# Per-file review threads (mirrors diffs/ structure)\njq -c '.[]' /tmp/pr-context/review_comments.json | while IFS= read -r thread; do\n filepath=$(echo \"$thread\" | jq -r '.path // empty')\n [ -z \"$filepath\" ] && continue\n mkdir -p \"/tmp/pr-context/threads/$(dirname \"$filepath\")\"\n echo \"$thread\" >> \"/tmp/pr-context/threads/${filepath}.jsonl\"\ndone\n# Convert per-file JSONL to proper JSON arrays\nmkdir -p /tmp/pr-context/threads\nfind /tmp/pr-context/threads -name '*.jsonl' 2>/dev/null | while IFS= read -r jsonl; do\n jq -s '.' \"$jsonl\" > \"${jsonl%.jsonl}.json\"\n rm \"$jsonl\"\ndone\n\n# PR discussion comments\ngh api \"repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/comments.json\n\n# Linked issues\njq -r '.body // \"\"' /tmp/pr-context/pr.json 2>/dev/null \\\n | grep -oiE '(fixes|closes|resolves)\\s+#[0-9]+' \\\n | grep -oE '[0-9]+$' \\\n | sort -u \\\n | while read -r issue; do\n gh api \"repos/$GITHUB_REPOSITORY/issues/$issue\" > \"/tmp/pr-context/issue-${issue}.json\" || true\n done || true\n\n# Write manifest\ncat > /tmp/pr-context/README.md << 'MANIFEST'\n# PR Context\n\nPre-fetched PR data. All files are in `/tmp/pr-context/`.\n\n| File | Description |\n| --- | --- |\n| `pr.json` | PR metadata — title, body, author, base/head branches, head commit SHA (`headRefOid`), URL |\n| `pr.diff` | Full unified diff of all changes |\n| `files.json` | Changed files array — each entry has `filename`, `status`, `additions`, `deletions`, `patch` |\n| `diffs/.diff` | Per-file diffs — one file per changed file, mirroring the repo path under `diffs/` |\n| `file_order_az.txt` | Changed files sorted alphabetically (A→Z), one filename per line |\n| `file_order_za.txt` | Changed files sorted reverse-alphabetically (Z→A), one filename per line |\n| `file_order_largest.txt` | Changed files sorted by diff size descending (largest first), one filename per line |\n| `reviews.json` | Prior review submissions — author, state (APPROVED/CHANGES_REQUESTED/COMMENTED), body |\n| `review_comments.json` | All review threads (GraphQL) — each thread has `id` (node ID for resolving), `isResolved`, `isOutdated`, `path`, `line`, and nested `comments` with `id`, `databaseId` (numeric REST ID for replies), body/author |\n| `threads/.json` | Per-file review threads — one file per changed file with existing threads, mirroring the repo path under `threads/` |\n| `comments.json` | PR discussion comments (not inline) |\n| `issue-{N}.json` | Linked issue details (one file per linked issue, if any) |\n| `agents.md` | Repository conventions from `generate_agents_md` (if written by agent) |\n| `review-instructions.md` | Review instructions, criteria, and calibration examples (if written by review-process fragment) |\nMANIFEST\n\necho \"PR context written to /tmp/pr-context/\"\nls -la /tmp/pr-context/" + run: "set -euo pipefail\nmkdir -p /tmp/pr-context\n\n# PR metadata\ngh pr view \"$PR_NUMBER\" --json title,body,author,baseRefName,headRefName,headRefOid,url \\\n > /tmp/pr-context/pr.json\n\n# Full diff\nif ! gh pr diff \"$PR_NUMBER\" > /tmp/pr-context/pr.diff; then\n echo \"::warning::Failed to fetch full PR diff; per-file diffs from files.json are still available.\"\n : > /tmp/pr-context/pr.diff\nfi\n\n# Changed files list (--paginate may output concatenated arrays; jq -s 'add' merges them)\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/files.json\n\n# Per-file diffs\njq -c '.[]' /tmp/pr-context/files.json | while IFS= read -r entry; do\n filename=$(echo \"$entry\" | jq -r '.filename')\n mkdir -p \"/tmp/pr-context/diffs/$(dirname \"$filename\")\"\n echo \"$entry\" | jq -r '.patch // empty' > \"/tmp/pr-context/diffs/${filename}.diff\"\ndone\n\n# File orderings for sub-agent review (3 strategies)\njq -r '[.[] | .filename] | sort | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_az.txt\njq -r '[.[] | .filename] | sort | reverse | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_za.txt\njq -r '[.[] | {filename, size: ((.additions // 0) + (.deletions // 0))}] | sort_by(-.size) | .[].filename' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_largest.txt\n\n# Existing reviews\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/reviews.json\n\n# Review threads with resolution status (GraphQL — REST lacks isResolved/isOutdated)\ngh api graphql --paginate -f query='\n query($owner: String!, $repo: String!, $number: Int!, $endCursor: String) {\n repository(owner: $owner, name: $repo) {\n pullRequest(number: $number) {\n reviewThreads(first: 100, after: $endCursor) {\n pageInfo { hasNextPage endCursor }\n nodes {\n id\n isResolved\n isOutdated\n isCollapsed\n path\n line\n startLine\n comments(first: 100) {\n nodes {\n id\n databaseId\n body\n author { login }\n createdAt\n }\n }\n }\n }\n }\n }\n }\n' -F owner=\"${GITHUB_REPOSITORY%/*}\" -F repo=\"${GITHUB_REPOSITORY#*/}\" -F \"number=$PR_NUMBER\" \\\n --jq '.data.repository.pullRequest.reviewThreads.nodes' \\\n | jq -s 'add // []' > /tmp/pr-context/review_comments.json\n\n# Filtered review thread views (pre-computed so agents don't need to parse review_comments.json)\njq '[.[] | select(.isResolved == false)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/unresolved_threads.json\njq '[.[] | select(.isResolved == true)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/resolved_threads.json\njq '[.[] | select(.isOutdated == true)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/outdated_threads.json\n\n# Per-file review threads (mirrors diffs/ structure)\njq -c '.[]' /tmp/pr-context/review_comments.json | while IFS= read -r thread; do\n filepath=$(echo \"$thread\" | jq -r '.path // empty')\n [ -z \"$filepath\" ] && continue\n mkdir -p \"/tmp/pr-context/threads/$(dirname \"$filepath\")\"\n echo \"$thread\" >> \"/tmp/pr-context/threads/${filepath}.jsonl\"\ndone\n# Convert per-file JSONL to proper JSON arrays\nmkdir -p /tmp/pr-context/threads\nfind /tmp/pr-context/threads -name '*.jsonl' 2>/dev/null | while IFS= read -r jsonl; do\n jq -s '.' \"$jsonl\" > \"${jsonl%.jsonl}.json\"\n rm \"$jsonl\"\ndone\n\n# PR discussion comments\ngh api \"repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/comments.json\n\n# Linked issues\njq -r '.body // \"\"' /tmp/pr-context/pr.json 2>/dev/null \\\n | grep -oiE '(fixes|closes|resolves)\\s+#[0-9]+' \\\n | grep -oE '[0-9]+$' \\\n | sort -u \\\n | while read -r issue; do\n gh api \"repos/$GITHUB_REPOSITORY/issues/$issue\" > \"/tmp/pr-context/issue-${issue}.json\" || true\n done || true\n\n# Write manifest\ncat > /tmp/pr-context/README.md << 'MANIFEST'\n# PR Context\n\nPre-fetched PR data. All files are in `/tmp/pr-context/`.\n\n| File | Description |\n| --- | --- |\n| `pr.json` | PR metadata — title, body, author, base/head branches, head commit SHA (`headRefOid`), URL |\n| `pr.diff` | Full unified diff of all changes |\n| `files.json` | Changed files array — each entry has `filename`, `status`, `additions`, `deletions`, `patch` |\n| `diffs/.diff` | Per-file diffs — one file per changed file, mirroring the repo path under `diffs/` |\n| `file_order_az.txt` | Changed files sorted alphabetically (A→Z), one filename per line |\n| `file_order_za.txt` | Changed files sorted reverse-alphabetically (Z→A), one filename per line |\n| `file_order_largest.txt` | Changed files sorted by diff size descending (largest first), one filename per line |\n| `reviews.json` | Prior review submissions — author, state (APPROVED/CHANGES_REQUESTED/COMMENTED), body |\n| `review_comments.json` | All review threads (GraphQL) — each thread has `id` (node ID for resolving), `isResolved`, `isOutdated`, `path`, `line`, and nested `comments` with `id`, `databaseId` (numeric REST ID for replies), body/author |\n| `unresolved_threads.json` | Unresolved review threads — subset of `review_comments.json` where `isResolved` is false |\n| `resolved_threads.json` | Resolved review threads — subset of `review_comments.json` where `isResolved` is true |\n| `outdated_threads.json` | Outdated review threads — subset of `review_comments.json` where `isOutdated` is true (code changed since comment) |\n| `threads/.json` | Per-file review threads — one file per changed file with existing threads, mirroring the repo path under `threads/` |\n| `comments.json` | PR discussion comments (not inline) |\n| `issue-{N}.json` | Linked issue details (one file per linked issue, if any) |\n| `agents.md` | Repository conventions from `generate_agents_md` (if written by agent) |\n| `review-instructions.md` | Review instructions, criteria, and calibration examples (if written by review-process fragment) |\nMANIFEST\n\necho \"PR context written to /tmp/pr-context/\"\nls -la /tmp/pr-context/" - env: GITHUB_TOKEN: ${{ github.token }} REPO_NAME: ${{ github.repository }} diff --git a/.github/workflows/gh-aw-pr-review-addresser.md b/.github/workflows/gh-aw-pr-review-addresser.md index 00484b24..8907ed77 100644 --- a/.github/workflows/gh-aw-pr-review-addresser.md +++ b/.github/workflows/gh-aw-pr-review-addresser.md @@ -122,7 +122,7 @@ PR context is pre-fetched to `/tmp/pr-context/`. Read `/tmp/pr-context/README.md 1. Call `generate_agents_md` to get the repository's coding guidelines and conventions. If this fails, continue without it. 2. Read `/tmp/pr-context/pr.json` for PR details (author, description, branches). Check whether this is a fork PR — if the head repo differs from the base repo, you cannot push changes. 3. Read `/tmp/pr-context/issue-*.json` if any exist to understand linked issue motivation and requirements. -4. Read `/tmp/pr-context/review_comments.json` to get all review threads. Identify which threads are unresolved and need attention. +4. Read `/tmp/pr-context/unresolved_threads.json` to get unresolved review threads that need attention. For full context including resolved threads, see `review_comments.json`. 5. Read `/tmp/pr-context/diffs/` to understand the current state of changes. ### Step 2: Address Each Review Thread @@ -145,7 +145,7 @@ For each unresolved review thread: Skip this step for fork PRs where you could not push. -After pushing, resolve every review thread that your changes address by calling `resolve_pull_request_review_thread` with the thread's GraphQL node ID (the `id` field, e.g., `PRRT_kwDO...`). This includes threads from any reviewer — external reviewers, bots, and your own prior reviews. Check `/tmp/pr-context/review_comments.json` for all unresolved threads (`isResolved: false`) — `isOutdated` threads have had the underlying code changed since the comment was made, so check whether your changes address them. Do NOT resolve threads you disagreed with, skipped, or only partially addressed — leave those open for the reviewer. Fall back to `pull_request_read` with method `get_review_comments` if the pre-fetched data is unavailable. +After pushing, resolve every review thread that your changes address by calling `resolve_pull_request_review_thread` with the thread's GraphQL node ID (the `id` field, e.g., `PRRT_kwDO...`). This includes threads from any reviewer — external reviewers, bots, and your own prior reviews. Check `/tmp/pr-context/unresolved_threads.json` for all unresolved threads — also check `/tmp/pr-context/outdated_threads.json` for threads where the underlying code changed since the comment was made and verify whether your changes address them. Do NOT resolve threads you disagreed with, skipped, or only partially addressed — leave those open for the reviewer. Fall back to `pull_request_read` with method `get_review_comments` if the pre-fetched data is unavailable. ### Step 5: Respond diff --git a/.github/workflows/gh-aw-pr-review.lock.yml b/.github/workflows/gh-aw-pr-review.lock.yml index 983e1854..be4ad4d8 100644 --- a/.github/workflows/gh-aw-pr-review.lock.yml +++ b/.github/workflows/gh-aw-pr-review.lock.yml @@ -40,7 +40,7 @@ # # inlined-imports: true # -# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"5e96210a88a0e71fe8a1a2ce86adee6b4b1b1c3a552cac35b1db93dacab00ef5"} +# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"f7adb4454527b9d061ef790aece9abe299347d6fc5caa2d0d62cb5797e7392e9"} name: "PR Review" "on": @@ -710,7 +710,7 @@ jobs: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ github.event.pull_request.number || inputs.target-pr-number || github.event.issue.number }} name: Fetch PR context to disk - run: "set -euo pipefail\nmkdir -p /tmp/pr-context\n\n# PR metadata\ngh pr view \"$PR_NUMBER\" --json title,body,author,baseRefName,headRefName,headRefOid,url \\\n > /tmp/pr-context/pr.json\n\n# Full diff\nif ! gh pr diff \"$PR_NUMBER\" > /tmp/pr-context/pr.diff; then\n echo \"::warning::Failed to fetch full PR diff; per-file diffs from files.json are still available.\"\n : > /tmp/pr-context/pr.diff\nfi\n\n# Changed files list (--paginate may output concatenated arrays; jq -s 'add' merges them)\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/files.json\n\n# Per-file diffs\njq -c '.[]' /tmp/pr-context/files.json | while IFS= read -r entry; do\n filename=$(echo \"$entry\" | jq -r '.filename')\n mkdir -p \"/tmp/pr-context/diffs/$(dirname \"$filename\")\"\n echo \"$entry\" | jq -r '.patch // empty' > \"/tmp/pr-context/diffs/${filename}.diff\"\ndone\n\n# File orderings for sub-agent review (3 strategies)\njq -r '[.[] | .filename] | sort | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_az.txt\njq -r '[.[] | .filename] | sort | reverse | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_za.txt\njq -r '[.[] | {filename, size: ((.additions // 0) + (.deletions // 0))}] | sort_by(-.size) | .[].filename' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_largest.txt\n\n# Existing reviews\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/reviews.json\n\n# Review threads with resolution status (GraphQL — REST lacks isResolved/isOutdated)\ngh api graphql --paginate -f query='\n query($owner: String!, $repo: String!, $number: Int!, $endCursor: String) {\n repository(owner: $owner, name: $repo) {\n pullRequest(number: $number) {\n reviewThreads(first: 100, after: $endCursor) {\n pageInfo { hasNextPage endCursor }\n nodes {\n id\n isResolved\n isOutdated\n isCollapsed\n path\n line\n startLine\n comments(first: 100) {\n nodes {\n id\n databaseId\n body\n author { login }\n createdAt\n }\n }\n }\n }\n }\n }\n }\n' -F owner=\"${GITHUB_REPOSITORY%/*}\" -F repo=\"${GITHUB_REPOSITORY#*/}\" -F \"number=$PR_NUMBER\" \\\n --jq '.data.repository.pullRequest.reviewThreads.nodes' \\\n | jq -s 'add // []' > /tmp/pr-context/review_comments.json\n\n# Per-file review threads (mirrors diffs/ structure)\njq -c '.[]' /tmp/pr-context/review_comments.json | while IFS= read -r thread; do\n filepath=$(echo \"$thread\" | jq -r '.path // empty')\n [ -z \"$filepath\" ] && continue\n mkdir -p \"/tmp/pr-context/threads/$(dirname \"$filepath\")\"\n echo \"$thread\" >> \"/tmp/pr-context/threads/${filepath}.jsonl\"\ndone\n# Convert per-file JSONL to proper JSON arrays\nmkdir -p /tmp/pr-context/threads\nfind /tmp/pr-context/threads -name '*.jsonl' 2>/dev/null | while IFS= read -r jsonl; do\n jq -s '.' \"$jsonl\" > \"${jsonl%.jsonl}.json\"\n rm \"$jsonl\"\ndone\n\n# PR discussion comments\ngh api \"repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/comments.json\n\n# Linked issues\njq -r '.body // \"\"' /tmp/pr-context/pr.json 2>/dev/null \\\n | grep -oiE '(fixes|closes|resolves)\\s+#[0-9]+' \\\n | grep -oE '[0-9]+$' \\\n | sort -u \\\n | while read -r issue; do\n gh api \"repos/$GITHUB_REPOSITORY/issues/$issue\" > \"/tmp/pr-context/issue-${issue}.json\" || true\n done || true\n\n# Write manifest\ncat > /tmp/pr-context/README.md << 'MANIFEST'\n# PR Context\n\nPre-fetched PR data. All files are in `/tmp/pr-context/`.\n\n| File | Description |\n| --- | --- |\n| `pr.json` | PR metadata — title, body, author, base/head branches, head commit SHA (`headRefOid`), URL |\n| `pr.diff` | Full unified diff of all changes |\n| `files.json` | Changed files array — each entry has `filename`, `status`, `additions`, `deletions`, `patch` |\n| `diffs/.diff` | Per-file diffs — one file per changed file, mirroring the repo path under `diffs/` |\n| `file_order_az.txt` | Changed files sorted alphabetically (A→Z), one filename per line |\n| `file_order_za.txt` | Changed files sorted reverse-alphabetically (Z→A), one filename per line |\n| `file_order_largest.txt` | Changed files sorted by diff size descending (largest first), one filename per line |\n| `reviews.json` | Prior review submissions — author, state (APPROVED/CHANGES_REQUESTED/COMMENTED), body |\n| `review_comments.json` | All review threads (GraphQL) — each thread has `id` (node ID for resolving), `isResolved`, `isOutdated`, `path`, `line`, and nested `comments` with `id`, `databaseId` (numeric REST ID for replies), body/author |\n| `threads/.json` | Per-file review threads — one file per changed file with existing threads, mirroring the repo path under `threads/` |\n| `comments.json` | PR discussion comments (not inline) |\n| `issue-{N}.json` | Linked issue details (one file per linked issue, if any) |\n| `agents.md` | Repository conventions from `generate_agents_md` (if written by agent) |\n| `review-instructions.md` | Review instructions, criteria, and calibration examples (if written by review-process fragment) |\nMANIFEST\n\necho \"PR context written to /tmp/pr-context/\"\nls -la /tmp/pr-context/" + run: "set -euo pipefail\nmkdir -p /tmp/pr-context\n\n# PR metadata\ngh pr view \"$PR_NUMBER\" --json title,body,author,baseRefName,headRefName,headRefOid,url \\\n > /tmp/pr-context/pr.json\n\n# Full diff\nif ! gh pr diff \"$PR_NUMBER\" > /tmp/pr-context/pr.diff; then\n echo \"::warning::Failed to fetch full PR diff; per-file diffs from files.json are still available.\"\n : > /tmp/pr-context/pr.diff\nfi\n\n# Changed files list (--paginate may output concatenated arrays; jq -s 'add' merges them)\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/files.json\n\n# Per-file diffs\njq -c '.[]' /tmp/pr-context/files.json | while IFS= read -r entry; do\n filename=$(echo \"$entry\" | jq -r '.filename')\n mkdir -p \"/tmp/pr-context/diffs/$(dirname \"$filename\")\"\n echo \"$entry\" | jq -r '.patch // empty' > \"/tmp/pr-context/diffs/${filename}.diff\"\ndone\n\n# File orderings for sub-agent review (3 strategies)\njq -r '[.[] | .filename] | sort | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_az.txt\njq -r '[.[] | .filename] | sort | reverse | .[]' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_za.txt\njq -r '[.[] | {filename, size: ((.additions // 0) + (.deletions // 0))}] | sort_by(-.size) | .[].filename' /tmp/pr-context/files.json \\\n > /tmp/pr-context/file_order_largest.txt\n\n# Existing reviews\ngh api \"repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/reviews.json\n\n# Review threads with resolution status (GraphQL — REST lacks isResolved/isOutdated)\ngh api graphql --paginate -f query='\n query($owner: String!, $repo: String!, $number: Int!, $endCursor: String) {\n repository(owner: $owner, name: $repo) {\n pullRequest(number: $number) {\n reviewThreads(first: 100, after: $endCursor) {\n pageInfo { hasNextPage endCursor }\n nodes {\n id\n isResolved\n isOutdated\n isCollapsed\n path\n line\n startLine\n comments(first: 100) {\n nodes {\n id\n databaseId\n body\n author { login }\n createdAt\n }\n }\n }\n }\n }\n }\n }\n' -F owner=\"${GITHUB_REPOSITORY%/*}\" -F repo=\"${GITHUB_REPOSITORY#*/}\" -F \"number=$PR_NUMBER\" \\\n --jq '.data.repository.pullRequest.reviewThreads.nodes' \\\n | jq -s 'add // []' > /tmp/pr-context/review_comments.json\n\n# Filtered review thread views (pre-computed so agents don't need to parse review_comments.json)\njq '[.[] | select(.isResolved == false)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/unresolved_threads.json\njq '[.[] | select(.isResolved == true)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/resolved_threads.json\njq '[.[] | select(.isOutdated == true)]' /tmp/pr-context/review_comments.json \\\n > /tmp/pr-context/outdated_threads.json\n\n# Per-file review threads (mirrors diffs/ structure)\njq -c '.[]' /tmp/pr-context/review_comments.json | while IFS= read -r thread; do\n filepath=$(echo \"$thread\" | jq -r '.path // empty')\n [ -z \"$filepath\" ] && continue\n mkdir -p \"/tmp/pr-context/threads/$(dirname \"$filepath\")\"\n echo \"$thread\" >> \"/tmp/pr-context/threads/${filepath}.jsonl\"\ndone\n# Convert per-file JSONL to proper JSON arrays\nmkdir -p /tmp/pr-context/threads\nfind /tmp/pr-context/threads -name '*.jsonl' 2>/dev/null | while IFS= read -r jsonl; do\n jq -s '.' \"$jsonl\" > \"${jsonl%.jsonl}.json\"\n rm \"$jsonl\"\ndone\n\n# PR discussion comments\ngh api \"repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments\" --paginate \\\n | jq -s 'add // []' > /tmp/pr-context/comments.json\n\n# Linked issues\njq -r '.body // \"\"' /tmp/pr-context/pr.json 2>/dev/null \\\n | grep -oiE '(fixes|closes|resolves)\\s+#[0-9]+' \\\n | grep -oE '[0-9]+$' \\\n | sort -u \\\n | while read -r issue; do\n gh api \"repos/$GITHUB_REPOSITORY/issues/$issue\" > \"/tmp/pr-context/issue-${issue}.json\" || true\n done || true\n\n# Write manifest\ncat > /tmp/pr-context/README.md << 'MANIFEST'\n# PR Context\n\nPre-fetched PR data. All files are in `/tmp/pr-context/`.\n\n| File | Description |\n| --- | --- |\n| `pr.json` | PR metadata — title, body, author, base/head branches, head commit SHA (`headRefOid`), URL |\n| `pr.diff` | Full unified diff of all changes |\n| `files.json` | Changed files array — each entry has `filename`, `status`, `additions`, `deletions`, `patch` |\n| `diffs/.diff` | Per-file diffs — one file per changed file, mirroring the repo path under `diffs/` |\n| `file_order_az.txt` | Changed files sorted alphabetically (A→Z), one filename per line |\n| `file_order_za.txt` | Changed files sorted reverse-alphabetically (Z→A), one filename per line |\n| `file_order_largest.txt` | Changed files sorted by diff size descending (largest first), one filename per line |\n| `reviews.json` | Prior review submissions — author, state (APPROVED/CHANGES_REQUESTED/COMMENTED), body |\n| `review_comments.json` | All review threads (GraphQL) — each thread has `id` (node ID for resolving), `isResolved`, `isOutdated`, `path`, `line`, and nested `comments` with `id`, `databaseId` (numeric REST ID for replies), body/author |\n| `unresolved_threads.json` | Unresolved review threads — subset of `review_comments.json` where `isResolved` is false |\n| `resolved_threads.json` | Resolved review threads — subset of `review_comments.json` where `isResolved` is true |\n| `outdated_threads.json` | Outdated review threads — subset of `review_comments.json` where `isOutdated` is true (code changed since comment) |\n| `threads/.json` | Per-file review threads — one file per changed file with existing threads, mirroring the repo path under `threads/` |\n| `comments.json` | PR discussion comments (not inline) |\n| `issue-{N}.json` | Linked issue details (one file per linked issue, if any) |\n| `agents.md` | Repository conventions from `generate_agents_md` (if written by agent) |\n| `review-instructions.md` | Review instructions, criteria, and calibration examples (if written by review-process fragment) |\nMANIFEST\n\necho \"PR context written to /tmp/pr-context/\"\nls -la /tmp/pr-context/" - name: Write review instructions to disk run: "mkdir -p /tmp/pr-context\ncat > /tmp/pr-context/review-instructions.md << 'REVIEW_EOF'\n# Review Instructions for Sub-agents\n\nYou are a code review sub-agent. Read these instructions, then review the PR files in the order provided in your prompt.\n\n## Context\n\nBefore reviewing files, read these to understand the PR:\n\n1. `/tmp/pr-context/pr.json` — PR title, description, author, and branches. Understand what the PR is trying to accomplish.\n2. `/tmp/pr-context/agents.md` — Repository coding conventions and guidelines (if it exists).\n3. `/tmp/pr-context/review_comments.json` — Existing review threads. Note which files already have threads so you don't duplicate.\n4. `/tmp/pr-context/issue-*.json` — Linked issue details (if any). Understand the motivation and acceptance criteria.\n\n## Process\n\nReview the PR diff file by file in your assigned order. For each changed file:\n\n1. **Read the diff** for this file from `/tmp/pr-context/diffs/.diff` to understand what changed. If the diff is empty or truncated (e.g., binary files or very large changes), fall back to reading the full file from the workspace and comparing against context.\n2. **Read the full file from the workspace.** The PR branch is checked out locally — open the file directly to get complete contents with line numbers.\n3. **Check existing threads** for this file from `/tmp/pr-context/threads/.json` (if it exists). Skip issues that are already under discussion — each thread has `isResolved` and `isOutdated` fields.\n4. **Identify potential issues** matching the review criteria below.\n5. **Quick-check each issue** before including it:\n - What specific code pattern or change triggers this concern?\n - Is there an obvious guard, handler, or mitigation visible in the immediate context?\n - Can you describe a concrete failure scenario (the `evidence` field)? If you cannot articulate what specific input or state triggers the problem, drop the finding.\n - If the issue is clearly handled, skip it. If you're unsure, include it — the parent will verify.\n6. **Add to your findings list.** Do NOT leave inline comments — you don't have that tool. Return findings in this format:\n\n```\n- file: path/to/file\n line: 42\n severity: HIGH\n title: Brief title\n description: What the issue is and why it matters\n evidence: The specific code pattern and failure scenario\n suggestion: corrected code here (optional — only if you can provide a concrete fix)\n```\n\n**Review every file in your assigned order.** Files reviewed earlier get more attention, which is why different sub-agents use different orderings.\n\n**Check existing threads** — per-file threads are at `/tmp/pr-context/threads/.json` (step 3 above). The full list is at `/tmp/pr-context/review_comments.json`. Do not flag issues that are already under discussion (resolved or unresolved). For outdated threads, only re-flag if the issue still applies to the current diff.\n\n**Return your full findings list** when done, or an empty list if no issues were found.\n\n## Review Criteria\n\nFocus on these categories in priority order:\n\n1. Security vulnerabilities (injection, XSS, auth bypass, secrets exposure)\n2. Logic bugs that could cause runtime failures or incorrect behavior\n3. Data integrity issues (race conditions, missing transactions, corruption risk)\n4. Performance bottlenecks (N+1 queries, memory leaks, blocking operations)\n5. Error handling gaps (unhandled exceptions, missing validation)\n6. Breaking changes to public APIs without migration path\n7. Missing or incorrect test coverage for critical paths\n\n## What NOT to Flag\n\nOnly review the diff — do not flag issues in unchanged code, pre-existing problems not introduced by this PR, or style preferences handled by linters or formatters.\n\n**Common false positives** — these patterns look like issues but usually aren't. Before flagging anything in these categories, confirm the problem is real by reading the surrounding code:\n\n- **Security — input already sanitized:** Don't flag injection or XSS risks when inputs are sanitized upstream, parameterized queries are used, or the framework auto-escapes output.\n- **Null/undefined — guarded elsewhere:** Don't flag potential null dereferences if the value is guaranteed by a type guard, assertion, schema validation, or upstream null check.\n- **Error handling — handled at a different layer:** Don't flag missing try/catch if the caller, middleware, or framework catches and handles the error (e.g., Express error middleware, React error boundaries).\n- **Performance — theoretical, not practical:** Don't flag algorithmic complexity (e.g., O(n^2)) unless N is demonstrably large enough to matter in the actual usage context. \"This could be slow\" without evidence is not actionable.\n- **Validation — exists at another layer:** Don't flag missing input validation if it's handled by an API gateway, middleware, schema validator, or type system.\n- **Test coverage — trivial or generated code:** Don't flag missing tests for trivial getters/setters, auto-generated code, or simple delegation methods.\n- **Style or naming — not in coding guidelines:** Don't flag naming conventions or code style unless they violate the repository's documented coding guidelines (from `generate_agents_md` or CONTRIBUTING docs).\n\n**Existing review threads** — check BEFORE flagging any issue:\n\n- **Resolved with reviewer reply** (e.g. \"This is intentional\") — reviewer's decision is final. Do NOT re-flag.\n- **Resolved without reply** — author likely fixed it. Do NOT re-raise unless the fix introduced a new problem.\n- **Unresolved** — already flagged. Do NOT duplicate.\n- **Outdated** — only re-flag if the issue still applies to the current diff.\n\nWhen in doubt, do not duplicate. Redundant comments erode trust.\n\nFinding no issues is a valid and valuable outcome. An empty findings list is better than findings that waste the author's time or erode trust. Do not manufacture findings to justify your review — if the code is sound, return an empty list.\n\n## Severity Classification\n\nDetermine severity AFTER investigating the issue, not before. First identify the problem and trace through the code, then assign a severity based on the evidence you found.\n\n- 🔴 CRITICAL — Must fix before merge (security vulnerabilities, data corruption, production-breaking bugs)\n- 🟠 HIGH — Should fix before merge (logic errors, missing validation, significant performance issues)\n- 🟡 MEDIUM — Address soon, non-blocking (error handling gaps, suboptimal patterns, missing edge cases)\n- ⚪ LOW — Author discretion (minor improvements, documentation gaps)\n- 💬 NITPICK — Truly optional (stylistic preferences, alternative approaches)\n\n## Review Intensity\n\nThe review intensity is `${{ inputs.intensity || 'balanced' }}`.\n\n- **conservative**: High evidence bar. Only flag when you can demonstrate a concrete failure scenario. If you can construct a reasonable counterargument, do not flag. Approval with zero findings is the expected outcome for most PRs.\n- **balanced**: Standard evidence bar. Flag when you can point to specific code that would fail. If the issue is ambiguous, lean toward not flagging.\n- **aggressive**: Lower evidence bar. Flag when evidence exists even if the failure scenario is not fully confirmed. Improvement suggestions welcome but must cite specific code.\n\n## Calibration Examples\n\nUse these examples to calibrate your judgment. Each pair shows a real issue and a similar-looking pattern that is NOT an issue.\n\n### Example 1: Null/Undefined Access\n\n**True positive — flag this:**\n\n```js\n// PR adds this handler\napp.get('/user/:id', async (req, res) => {\n const user = await db.findUser(req.params.id);\n res.json({ name: user.name, email: user.email });\n});\n```\n\nWhy flag: `db.findUser()` can return `null` when no user matches the ID. Accessing `user.name` will throw a TypeError at runtime. No upstream guard exists — the route handler is the entry point.\n\n**False positive — do NOT flag this:**\n\n```ts\n// PR adds this line inside an existing function\nconst settings = user.getSettings();\n```\n\nWhy skip: Reading the full file reveals `user` is typed as `User` (not `User | null`), and the calling function only runs after `authenticateUser()` middleware which guarantees a valid user object. The null case is handled at a different layer.\n\n### Example 2: SQL Injection\n\n**True positive — flag this:**\n\n```python\n# PR adds this query\ncursor.execute(f\"SELECT * FROM orders WHERE customer_id = '{customer_id}'\")\n```\n\nWhy flag: String interpolation in a SQL query with user-controlled input (`customer_id` comes from the request). No parameterization or sanitization anywhere in the call chain.\n\n**False positive — do NOT flag this:**\n\n```python\n# PR adds this query\ncursor.execute(f\"SELECT * FROM orders WHERE status = '{OrderStatus.PENDING.value}'\")\n```\n\nWhy skip: The interpolated value is a hardcoded enum constant (`OrderStatus.PENDING`), not user input. There is no injection vector.\n\n### Example 3: Borderline — Do NOT Flag\n\n```go\n// PR adds this function\nfunc processItems(items []Item) []Result {\n results := make([]Result, 0)\n for _, item := range items {\n for _, tag := range item.Tags {\n results = append(results, process(item, tag))\n }\n }\n return results\n}\n```\n\nThis looks like an O(n*m) performance concern. But without evidence that `items` or `Tags` are large in practice, this is speculative. The function processes a bounded dataset (items from a single user request). Do not flag theoretical performance issues without evidence of real-world impact.\nREVIEW_EOF" - env: