Skip to content

Commit fcbdf59

Browse files
authored
ci(security-review): drop sticky comment, post workflow summary, re-enable synchronize (#1293)
* docs: clarify workflow_dispatch limitation for security review * test: add deliberate findings + drop sticky comment + add workflow summary Three changes to verify the inline-comment posting path end-to-end: 1. Add scripts/__sec_review_smoketest.mjs with two deliberate security findings (hardcoded AWS credentials, command injection via exec) so create_inline_comment is actually exercised. 2. Remove instructions to call mcp__github_comment__update_claude_comment from the prompt. That tool requires CLAUDE_COMMENT_ID, which the action only sets in tag mode / when track_progress is enabled. In agent mode it fails. Drop the corresponding entry from --allowedTools too. 3. Add a workflow step that counts buffered findings and posts a single top-level summary comment via the GitHub App token regardless of findings. Replaces the broken sticky-comment path. * ci(security-review): re-enable synchronize trigger so re-pushes re-review * test: remove smoketest fixture (verification done via workflow_dispatch run) The deliberate findings file was added to exercise the inline-comment posting path. The workflow_dispatch run confirmed the bot correctly identifies both findings (hardcoded AWS credentials, command injection via exec). End-to-end posting via mcp__github_inline_comment__create_inline_comment can only be verified once this PR merges and a follow-up PR triggers pull_request_target — workflow_dispatch produces an AutomationContext where the inline-comment MCP server doesn't register.
1 parent ac6cf4a commit fcbdf59

3 files changed

Lines changed: 58 additions & 24 deletions

File tree

.github/prompts/security-review.md

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -210,11 +210,12 @@ WORKFLOW (executed in 3 steps):
210210

211211
POSTING RESULTS — read this section CAREFULLY before posting anything:
212212

213-
You have access to two MCP tools provided by the `anthropics/claude-code-action` GitHub Action. These are the ONLY
214-
correct way to publish findings — do NOT call the GitHub REST API (`gh`, `octokit`, `curl`) and do NOT attempt to use
215-
`POST /pulls/{n}/reviews` directly. Direct REST calls are not available as tools in this environment.
213+
You have access to one MCP tool provided by the `anthropics/claude-code-action` GitHub Action. This is the ONLY correct
214+
way to publish findings — do NOT call the GitHub REST API (`gh`, `octokit`, `curl`) and do NOT attempt to use
215+
`POST /pulls/{n}/reviews` directly. Direct REST calls are not available as tools in this environment. Do NOT attempt to
216+
post a top-level summary comment yourself; the workflow posts the summary after you exit.
216217

217-
**Tool A `mcp__github_inline_comment__create_inline_comment`.** Posts an inline review comment on a specific file and
218+
**Tool — `mcp__github_inline_comment__create_inline_comment`.** Posts an inline review comment on a specific file and
218219
line. Each call buffers one comment; the action's post-step posts them all to the PR after this session ends. Call it
219220
once per finding.
220221

@@ -228,9 +229,6 @@ Parameters (all you need for almost every case):
228229
- `side` (`"LEFT"` | `"RIGHT"`, default `"RIGHT"`): leave at default unless commenting on a deleted line.
229230
- Do NOT pass `confirmed`. Letting it default lets the action's classifier filter test/probe-style comments.
230231

231-
**Tool B — `mcp__github_comment__update_claude_comment`.** Updates a single sticky top-level comment on the PR. Only
232-
parameter is `body` (string). Use this for the no-findings summary or the re-review status summary (see D below).
233-
234232
A. **Re-review awareness.** Before posting, READ the existing PR comments included in the diff/git context above. If
235233
your own bot identity has already commented on this PR (look for review comments authored by the bot whose token is
236234
running this workflow), treat this run as a re-review:
@@ -282,17 +280,13 @@ When suggesting a code fix, optionally include a GitHub suggestion block at the
282280
DO NOT post a single summary comment that lists all findings inline. Each finding gets its own `create_inline_comment`
283281
call.
284282

285-
D. **Summary / no-findings case.** After all `create_inline_comment` calls (or if there are none), call
286-
`mcp__github_comment__update_claude_comment` ONCE with a short summary:
287-
288-
- If you posted N inline findings, the body should be: `Security review complete. Posted N inline finding(s).` plus a
289-
one-line status of any prior findings (resolved / still outstanding / no longer applicable) if this is a re-review.
290-
- If you posted zero inline findings, the body should be: `Security review complete. No new high-confidence findings.`
291-
plus the same prior-findings status line if applicable.
292-
293-
DO NOT post the full per-finding markdown into this summary comment — the inline comments carry the detail.
283+
D. **No top-level summary comment from you.** Do NOT call any other tool to post a summary or status comment — the
284+
workflow posts a top-level summary comment automatically after this run finishes, based on how many findings you
285+
buffered. If you have no findings, simply exit without calling any tool; the workflow will post the "no findings"
286+
summary.
294287

295-
E. **Tool ordering.** Call `create_inline_comment` for every finding first, then call `update_claude_comment` LAST. Do
296-
not interleave.
288+
E. **Final assistant message.** Your final assistant message should be a one-line plain-text status only, e.g.
289+
`Posted N inline finding(s).` or `No high-confidence findings.` — do NOT include the per-finding markdown in this
290+
message. The inline comments carry the detail.
297291

298292
START ANALYSIS now.

.github/workflows/pr-security-review.yml

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ name: Claude Security Review
22

33
on:
44
pull_request_target:
5-
types: [opened, reopened]
5+
types: [opened, reopened, synchronize]
66
pull_request_review:
77
types: [submitted]
88
workflow_dispatch:
@@ -213,12 +213,50 @@ jobs:
213213
use_bedrock: 'true'
214214
prompt: ${{ steps.prompt.outputs.prompt }}
215215
show_full_output: 'true'
216-
# Allow-listing these MCP tool names is what tells the action to register
217-
# the corresponding MCP servers (github_inline_comment + github_comment).
218-
# See anthropics/claude-code-action src/mcp/install-mcp-server.ts.
216+
# Allow-listing this MCP tool name is what tells the action to register the
217+
# github_inline_comment MCP server. See anthropics/claude-code-action
218+
# src/mcp/install-mcp-server.ts.
219219
claude_args: >-
220220
--model us.anthropic.claude-opus-4-7 --max-turns 30 --allowedTools
221-
mcp__github_inline_comment__create_inline_comment,mcp__github_comment__update_claude_comment
221+
mcp__github_inline_comment__create_inline_comment
222+
223+
- name: Count buffered findings
224+
id: findings
225+
if: always()
226+
run: |
227+
set -euo pipefail
228+
BUFFER=/tmp/inline-comments-buffer.jsonl
229+
if [ -s "$BUFFER" ]; then
230+
COUNT=$(wc -l < "$BUFFER" | tr -d ' ')
231+
else
232+
COUNT=0
233+
fi
234+
echo "count=$COUNT" >> "$GITHUB_OUTPUT"
235+
echo "Buffered findings: $COUNT"
236+
237+
- name: Post security review summary comment
238+
if: always()
239+
uses: actions/github-script@v9
240+
env:
241+
PR_NUMBER: ${{ steps.pr.outputs.number }}
242+
FINDING_COUNT: ${{ steps.findings.outputs.count }}
243+
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
244+
with:
245+
github-token: ${{ steps.app-token.outputs.token }}
246+
script: |
247+
const prNumber = parseInt(process.env.PR_NUMBER, 10);
248+
const count = parseInt(process.env.FINDING_COUNT || '0', 10);
249+
const runUrl = process.env.RUN_URL;
250+
const body =
251+
count > 0
252+
? `**Claude Security Review:** posted ${count} inline finding${count === 1 ? '' : 's'} on this PR. ([run](${runUrl}))`
253+
: `**Claude Security Review:** no high-confidence findings. ([run](${runUrl}))`;
254+
await github.rest.issues.createComment({
255+
owner: context.repo.owner,
256+
repo: context.repo.repo,
257+
issue_number: prNumber,
258+
body,
259+
});
222260
223261
- name: Remove claude-security-reviewing label
224262
if: always()

CONTRIBUTING.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,9 @@ PRs as soon as a maintainer submits an **approving review**. PRs from non-collab
5555
approval is the gate, so a maintainer must manually review the diff before the automated reviewer runs.
5656

5757
To re-run the review on a later commit, submit another approving review (resolves to a fresh workflow run), or trigger
58-
the `Claude Security Review` workflow manually from the Actions tab with the PR number.
58+
the `Claude Security Review` workflow manually from the Actions tab with the PR number. Note that manual dispatch can
59+
verify the analysis and prompt plumbing but cannot post inline comments — the action's inline-comment MCP server only
60+
attaches on PR-context events (`pull_request_target`, `pull_request_review`).
5961

6062
## Code of Conduct
6163

0 commit comments

Comments
 (0)