Skip to content

Move Semgrep triage from Claude Code to Pi - #30

Merged
rgarcia merged 3 commits into
mainfrom
hypeship/semgrep-triage-pi
Nov 6, 2026
Merged

rgarcia merged 3 commits into
mainfrom
hypeship/semgrep-triage-pi

Conversation

@rgarcia

@rgarcia rgarcia commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Moves the Semgrep triage agent in semgrep.yml from Claude Code (claude -p) to Pi, using the same setup as the other Pi workflows: Node 22, @earendil-works/pi-coding-agent, PI_PREFERRED_MODEL, and ANTHROPIC_API_KEY / OPENAI_API_KEY.

The agent no longer gets GitHub credentials. It used to run with --dangerously-skip-permissions, a shell, and the caller's GITHUB_TOKEN, and post the review itself. Now the work is split by step, the same way pr-risk.yml does it:

  1. Gather PR comments (runner, GITHUB_TOKEN): scripts/semgrep_triage.py gather moves the Semgrep findings from the merge commit Semgrep scanned onto the PR head, using git diff -U0 between the two, and drops findings on lines only the base branch added. It writes the PR's commentable diff lines and the location and author of every existing inline comment, then checks out the PR head. The agent's files, the line numbers, the diff ranges, and the review commit_id therefore all refer to the same commit.
  2. Triage findings (Pi, no token): read-only tools only (--tools read,grep,find,ls). --no-approve keeps .pi resources in the PR checkout from loading, and --no-context-files keeps the checkout's AGENTS.md and CLAUDE.md out of the instructions. The checkout no longer persists the token in .git/config. Pi's final response is a JSON object with comments (path, line, body) and omitted.
  3. Post review (runner, GITHUB_TOKEN): semgrep_triage.py post drops comments that aren't on a Semgrep finding, fall outside the PR diff, or are on a line that already has a comment. It re-reads existing comments before posting, keeps at most 5, and posts one review.

Output contract

The PR output is unchanged and matches what recent triage runs actually posted:

  • One review per run, event=COMMENT, with inline comments on the flagged lines and commit_id set to the PR head SHA.
  • The review body is empty. It is only set when findings were left out ("N more actionable Semgrep findings not posted (limit: 5 inline comments)").
  • Nothing is posted when there are no actionable findings, and the log prints No actionable security findings.
  • Deduplication is still by file and line against comments from any author. The runner now enforces it too, not just the prompt.
  • The scan and too-many-findings jobs, the findings_count output, the inputs, and the semgrep-results artifact are untouched.

The prompt's triage rules and comment-body requirements are carried over word for word. Only the gh instructions were replaced: the agent now reads the gathered files and returns JSON.

Secrets and callers

  • All 23 callers use @main with secrets: inherit and grant contents: read / pull-requests: write. semgrep.yml declares no workflow_call secrets, so OPENAI_API_KEY reaches it the same way ANTHROPIC_API_KEY already does. No caller changes are needed.
  • Unverified: I couldn't read org secrets or variables. This needs PI_PREFERRED_MODEL (org variable) and the key for that model's provider (org secret) to be visible to every caller repo. If PI_PREFERRED_MODEL is missing, triage fails with Set the PI_PREFERRED_MODEL organization variable instead of silently skipping.
  • The README's required secrets and variables list is updated for semgrep.

Rollout note

The triage job fetches the prompt and scripts/semgrep_triage.py from main, as it already did for the prompt. That means this PR's own self-test can't exercise the new triage path. For a few minutes after merge, raw.githubusercontent.com's cache can serve the old prompt or a 404 for the script. During that window, the triage job fails visibly (::error:: or a curl error) and posts nothing. It is not the required scan / scan check.

Validation

  • actionlint (with shellcheck) on semgrep.yml and semgrep-tests.yml: the only remaining note is the existing SC2086 info in the untouched Count findings step.
  • New scripts/test_semgrep_triage.py (21 tests, including a real git base/PR/merge fixture where the base branch shifts the PR's lines, also run with a file name that needs literal pathspecs), run in CI by the new semgrep-tests.yml. It covers diff-line parsing, result parsing, and comment filtering (finding, diff, and existing-comment checks, the 5-comment cap, the omitted count). It also tests the review payload and the credential boundaries: no token in the agent step, read-only tools, --no-approve, a checkout without persisted credentials, model keys only in the agent step, and step order. Mutation checks confirmed that adding GH_TOKEN to the agent step, adding bash to its tools, or persisting checkout credentials each make the tests fail. Mutation checks also confirmed that removing the merge-to-head line shift, or posting on GITHUB_SHA instead of the head, fails the tests. All scripts/test_*.py suites pass (62 tests).
  • Local end-to-end run of the triage and post steps on a fixture repo, with real Semgrep findings (a Flask command injection plus a finding in a test file), a hostile AGENTS.md, and a .pi/settings.json. Pi (openai/gpt-5.5, --thinking max, the exact flags above) returned valid JSON with one comment on the injection line and skipped the test-file finding. post against a stubbed gh produced the expected single COMMENT review.
  • Ran Semgrep with this workflow's rule sets against main as the baseline: no new findings on this branch, so the self-test's triage job will skip.
  • Not run: a live GitHub Actions triage on a real PR (not possible before merge, see the rollout note), and an Anthropic-model run of the prompt (only an OpenAI key was available locally).

@rgarcia

rgarcia commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Picked ulziibay-kernel to review: touched the changed code in workflows; 23 pending

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread .github/workflows/semgrep.yml Resolved

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 75835da. Configure here.

Comment thread scripts/semgrep_triage.py Resolved
@rgarcia
rgarcia merged commit 6d7bfc5 into main Nov 6, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant