Reviews

Pull request review

Review one pull request, post inline comments, and re-review incrementally.

sh-pr-review reviews a single pull request rather than a whole codebase, posts the result as inline comments on the PR itself, and sets a security/pr-review commit status that branch protection can enforce.

Run it from your own machine, on any PR you can read. Install the plugin and ask:

review https://github.com/acme/api/pull/128
review PR 42
security review this PR

Paste a PR link and it reviews that PR in that repository, cloning it to a temporary directory first, because the hunters read files and not just the patch. Nothing is written into the repo you are working in.

Pass a bare number and it resolves against the repo you are currently in, the one git remote points at. Pass nothing and it takes the open PR for your current branch.

Phase 7 prints the findings and the verdict and asks before posting anything - a decline is a normal outcome, and the payload stays on disk for you to post later.

Before spending any analysis it checks whether you can actually write to the target repo, so reviewing someone else's project tells you up front that posting will 403 rather than discovering it ten minutes in.

Three properties make it usable as a merge gate rather than noise:

• Only what the PR is responsible for fails it. Every finding carries a pr_impact of introduced, aggravated, or pre_existing. The first two block; pre_existing is reported and never blocks. Blocking a merge over code the author never wrote is how a required check gets deleted, so when a hunter is unsure between aggravated and pre_existing, it must pick pre_existing.
• Depth follows risk. Triage runs first, in the orchestrator, with no subagents. It maps changed paths and added-line sink tokens onto the same class slugs the sh-kb-* bases use, then picks a tier. Tier 0 (no security-relevant change) launches nothing at all and still sets the status. Tier 3 runs the full pipeline.
• Re-pushes do not spam. Each comment carries a hidden fingerprint computed without line numbers, so a re-review adds only what is new and lists what was fixed as "Resolved since the last review".

warn reports success on purpose: a warning that blocks a merge is a failure with extra steps, and teams respond by removing the check. error is kept distinct from failure so a broken run never looks like a vulnerability it did not find.

The review event is always COMMENT, never REQUEST_CHANGES or APPROVE. The commit status is the enforcement mechanism, and it is the one branch protection reads.

Scope: the skill writes to the pull request and the commit status, and nowhere else. It opens no issues and creates nothing in any external tracker.

Re-reviews are incremental

A PR gets reviewed once per push, so the second review has to be cheaper than the first or the tool becomes something people turn off.

Dedup happens before the spending, not before the posting. The fingerprints already on the PR are read in Phase 1 and handed to the hunters and the verifier. Finding a duplicate at the end would mean the most expensive model in the pipeline had already re-confirmed a conclusion that was written on the PR the whole time. This needs no cache: the state lives in the PR, so it works on a cold machine and in CI.

A local cache makes the rest incremental. indra review cache stores each run's file hashes, findings and verdicts under your OS cache directory (never in the repo, since a cross-repo review runs in a temp clone that gets deleted). The next review re-hunts only files whose content actually changed, reuses verdicts for findings that are unchanged, and reuses the recon map if nothing it covers moved.

A base-branch merge costs nothing. Merging main into a PR branch changes the head SHA and nothing the author wrote, but a commit status is pinned to a SHA, so the required check silently disappears from the new head. When the PR's own files are byte-identical and the base delta touches nothing the findings depend on, the previous verdict is re-stamped onto the new SHA with no agents launched at all. That last condition is what makes it safe: a base merge that deletes a sanitizer leaves every PR file unchanged while turning a safe line into an exploitable one.

Invalidation is deliberately conservative, because a stale entry in a security tool does not make it slow, it makes it wrong. The cache key hashes every sh-kb-* knowledge base, so a KB update invalidates every cached finding - a cached "clean" must never suppress the finding that update was written to catch. Model identity, skill version, file content and a 7-day TTL all invalidate too, and an unspecified model is treated as a miss.

--no-cache disables it, --refresh-cache re-baselines, and run.md records per phase what was launched, reused and skipped, so a cache that quietly stops hitting is visible rather than assumed.

Posting, and where things are written

The review and the commit status are posted by default. A review that was computed and never delivered helped nobody. --confirm restores a prompt before posting, --dry-run sends nothing, --no-status posts the review but leaves the commit status alone.

Posting goes through indra review post rather than hand-built API calls, because it is a multi-step operation with a mandatory tail: review, then status, then receipts, with a 422 recovered by moving the comment rather than shifting a line number. It never exits leaving the status at pending - if the review cannot be posted it still sets error, saying the tooling failed rather than accusing the PR.

Inline comments are reserved for findings at medium or above with confidence >= 80. Low-severity findings go in the collapsed body section, so a low-priority finding showing up with zero inline comments is the policy working, not a failure.

Local run metrics

Every run records what it cost, so "the cache is working" and "reviews got slower" stop being matters of opinion.

indra review metrics path      # where records live
indra review metrics report    # aggregate, by model
indra review metrics purge --older-than-days 30

Two append-only JSONL files - runs.jsonl (repo, PR, tier, verdict, totals, the flags you passed) and events.jsonl (one line per phase or agent: model, tokens, duration, outcome, whether it was reused from cache). JSONL so a crashed run still leaves valid lines above the crash.

Linux follows the XDG Base Directory spec, so both honour XDG_DATA_HOME and XDG_CACHE_HOME when set and fall back to ~/.local/share and ~/.cache when they are not. Override either directly with SH_METRICS_DIR and SH_REVIEW_CACHE_DIR.

One binary, no interpreter. The cache, the metrics, posting, the memory recorder and the publish planner are all indra subcommands, so a review needs no Python on any platform; install indra once (see the indra CLI).

Strictly local. Neither the cache nor the metrics contain any network code or reporting endpoint. Anything token-shaped is redacted before it is written, because local files get pasted into issues.

Running it unattended

Optional, and a separate decision from using the skill. Run it by hand on your own PRs for a while first, so you know what it says about your codebase before it says it in front of your team.

When you are ready, "Enforcing the check on a repository" in references/pr-review-mapping.md has a copy-paste workflow for your repo, plus the fail-safe that stops a dead job leaving a required check stuck on pending.

The threshold stays at the default medium. Pre-existing findings never block a merge, so an unscanned codebase does not produce a wall of red on day one - only what a PR actually introduces or aggravates can fail it.