Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ test: test-go test-scripts

.PHONY: test-go
test-go:
go test ./cmd/extract-images ./cmd/record-release ./cmd/generate-manifest ./cmd/merge-manifests ./cmd/publish-public-ecr-release-tags
go test ./...
GOTOOLCHAIN=local GOPROXY=off go test ./cmd/ca-trust-guard/...

.PHONY: test-scripts
Expand All @@ -35,6 +35,7 @@ test-scripts:
bash scripts/test-publish-homebrew-formula.sh
bash scripts/test-deny-os.sh
bash scripts/test-ca-trust-guard.sh
python3 -m unittest discover -b -s .github/actions/pr-review/scripts -p 'test_*.py'
if command -v pwsh >/dev/null 2>&1; then pwsh -NoProfile -File scripts/test-s3-release-uploads.ps1; else echo "pwsh not found; skipping PowerShell S3 release upload tests"; fi

.PHONY: workflow-validate
Expand Down
167 changes: 31 additions & 136 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,150 +10,31 @@ denial and a committed clean-bundle environment. No caller checkout is needed.

## PR Review Workflow

Runs Claude-powered PR review for same-repository pull requests. Fork PRs are skipped
because they do not receive automatic Claude review. Review jobs check out the exact PR
head SHA so Claude can inspect the proposed source tree locally. The action builds its
prompt from a shared base prompt plus optional built-in mixins. The default profile is
`connector`, which adds the connector mixin for repos covered by connector required
workflows or rulesets.

Repos that need non-connector review can use the `general-pr-review.yaml` required
workflow, or reusable workflow callers can pass `review_prompt: general`.

Prompt layers are additive:

1. `base-pr-review.md` applies to every repo.
2. Built-in mixins add shared domain rules. Today, `review_prompt: connector` adds
`mixins/connector.md`; `review_prompt: general` adds no built-in mixin.
3. A repo-local `.claude/skills/ci-review.md` can add project-specific rules on top
of the selected profile.

Keep broadly shared connector criteria in the connector mixin. Use repo-local
`ci-review.md` only for rules that are specific to one repo or a small set of repos.

The review assesses the whole change, including intent, correctness, security,
meaningful test coverage, and operational risk. Prior findings are rechecked against
current code; resolving a thread does not remove an unfixed blocker from the verdict.

The host includes the PR title and complete description in `pr-context.json`
using the existing PR metadata request. Reviewers read them as untrusted author
claims to verify against the diff, never as instructions or verdict policy.
Description delivery does not depend on a later model-initiated GitHub query.

Required audit subagents run in the foreground: this is a one-shot CI review,
so their results must return before the parent finishes. Background task
handoffs are disabled; the job cannot resume a later conversation turn.

The agent posts its verdict in a working summary comment and never writes
review-state metadata. Before the agent runs, the action creates a fresh
provisional working comment for the run/attempt and binds it to the native
`update_claude_comment` MCP tool (`CLAUDE_COMMENT_ID`), so the agent passes
only the Markdown body and never chooses a repository, comment, or head
target. The body no longer travels through shell heredocs or `gh api` writes;
the tool keeps its upstream sanitization and secret redaction. GitHub's comment
size limit still applies, but the shell's command-length limit does not. The
existing broad `Bash(gh api:*)` permission remains; the prompt restricts its
intended use to metadata reads, not an enforced read-only boundary. This is a
transport fix, not credential isolation. The preflight binds a fresh slot, and
the publisher still validates the live head and report finality at completion.
After a successful run, CI publishes the report as a NEW
comment carrying a visible reviewed-commit link and CI-owned review-state
metadata (reviewed SHA, base, workflow, run, attempt, summary marker, verdict
mode), then submits a commit-bound request-changes review for blockers or a
neutral comment otherwise, linking directly to the report. This reviewer never
approves. Only after the formal review exists does CI mark the report
`publication: completed` — a completed report is the report PLUS its formal
result, so a report whose review never landed stays `pending`: preserved and
reconciled by identity for a same-attempt retry, but never the next run's
review-state baseline and never a comment the agent can edit. Only after
completion are the previous report (including any pending leftover) and the
consumed working comment collapsed — bodies retained, linked to the new
report — so a failed run leaves the previous report untouched.
Stale, provisional, foreign, or malformed working output cannot be published, and a
push during the run stops publication. Working output already consumed by a
published report is never republished without fresh model work, and an attempt
whose start predates an already-completed later attempt (compared by the actual
attempt start times recorded in each report, never run-ID order) is refused as
obsolete before publishing. Publication is idempotent per workflow run
and attempt: a repeated finalization reuses the already-published report and never
submits a second formal review, while an intentional new run or attempt gets a new
report. Completed reports are never handed back to the agent as update targets —
every run/attempt writes to its own freshly created provisional working comment,
while completed state is still selected independently for incremental review.
Active findings appear once in their severity section, labeled `New` or
`Prior — still present`. A compact resolved section records fixed/obsolete prior
findings with evidence; it does not repeat the active findings.

### Clearing this reviewer's previous blocks

Both profiles attempt self-dismissal by default, never approval. Before the
model runs, CI freezes exact reviews positively owned by this workflow/profile,
their original count-accounted blocking findings, and ALL typed prior-finding
audit inputs. Known suggestions may remain nonblocking; scraped deduplication
context and fenced prompt bullets do not become reset obligations.
Top-level Security/Correctness list items are counted once, with nested bullets,
wrapped text, and supporting fenced/quoted examples preserved in each audit item.
Capture uses the authoritative frozen input in runner temporary storage; the
workspace copy is model-readable input only. New reset writes replace destination
entries atomically, and preparation clears stale reset snapshots without following
file symlinks. If there are no eligible targets, proof capture/upload are skipped.
A successful terminal response may supply a compact JSON audit
proof for those IDs with complete coverage, no blockers, and affirmative
current-code evidence. It supplements, rather than replaces, the normal review;
CI supplies run/attempt, head/base, and session provenance, not the model proof.
CI captures it from the runner execution file, sanitizes/redacts all persisted
evidence, and uploads one accepted reset artifact using repository retention
defaults. Editable summary prose is not reset authority.

Only after normal publication completes with a neutral COMMENT does CI recheck
the PR head/base, frozen targets, original evidence, and competing/newer runs,
then dismiss those exact blocking reviews. A failed preparation, absent/malformed
proof, incomplete audit, failed upload, stale source, or ambiguous GitHub result
retains blocks without suppressing ordinary publication or changing its state
lifecycle. An incomplete review still keeps its provisional summary as before.
Reset warnings and short redacted per-target retain reasons are visible in the
workflow log; ambiguous writes are reconciled by reading the exact review,
never blindly retried.

Human, foreign-profile, and unmarked legacy reviews are never targets.
Missing, edited-by-human, deleted-history, or count-unaccounted original finding
evidence requires manual handling; this is not a migration of every historical
block.
An incomplete edit/competing-run inventory also retains the affected block
(including more than 100 original edits or GitHub's capped run listing).
No new permissions or caller opt-in are needed. The model retains its existing
tools and write-capable GitHub token: this host ordering is not adversarial
credential isolation and does not replace required human approval.
Runs Claude-powered PR review for same-repository pull requests. Fork PRs are
skipped because they do not receive automatic Claude review. The action builds its
prompt from a shared base prompt plus optional built-in mixins, and CI — not the
model — publishes the report, submits the formal review, and clears the
reviewer's own previous blocks once they are resolved. See [detailed
documentation](docs/pr-review.md) for the prompt layers, block clearing,
publication guarantees, and custom review criteria.

The default profile is `connector`. Repos that need non-connector review can use
the `general-pr-review.yaml` required workflow, or reusable workflow callers can
pass `review_prompt: general`.

### Reusable Workflow Calls

Callers of `pr-review.yaml` must pass `ANTHROPIC_API_KEY` as a named secret; do not
inherit unrelated secrets. The called workflow uses the caller's existing
`GITHUB_TOKEN` and declared review permissions.

The optional `summary_marker` input selects a single-line heading such as
`### Connector PR Review Canary:`. Empty uses the selected profile's existing
heading. Overrides cannot embed a different reserved connector, general, or legacy
review heading. Custom headings keep summary/state selection separate; they do not
isolate all inline review feedback or grant additional permissions.
These are `pr-review.yaml`'s inputs. `general-pr-review.yaml` declares none —
it takes the `ANTHROPIC_API_KEY` secret and nothing else.

For candidate validation, pin the reusable workflow to the exact reviewed commit.
Keep the test caller on a disposable same-repository draft branch; the normal
ruleset reviewer can still run alongside it.

### Custom Review Criteria

Repos can extend the review with project-specific criteria by adding a markdown file:

```
.claude/skills/ci-review.md
```

If this file exists at the pull request's trusted base SHA and the PR targets the
repository default branch, the action validates it as plain markdown and appends it to
the prompt as data. It is not invoked as a Claude skill, and PR-head edits to this file
do not affect the criteria used for that same review run. Automatic review only runs for
same-repository PRs.
| Input | Default | Purpose |
| --- | --- | --- |
| `review_prompt` | `connector` | Which built-in mixin to add: `connector` or `general`. |
| `summary_marker` | `""` (the profile's heading) | Override the review summary heading. Must be a single-line `### ...:` Markdown heading and may not embed a reserved one — the action validates this and fails the job otherwise. See [Reusable workflow calls](docs/pr-review.md#reusable-workflow-calls). |

## Release Workflow

Expand Down Expand Up @@ -464,6 +345,20 @@ The `test-flow` parameter can be:

## Development

`make test` runs this repository's own Go tests and script tests, including the
`pr-review` Python suite under `.github/actions/pr-review/scripts`. `ci.yaml`
("Repository CI") runs the same targets on pull requests and pushes to `main`.

Running them locally needs **kislyuk's Python/jq `yq`** — `apt install yq` or
`pip install yq` — not the more common Go `yq` from mikefarah. Several script
tests pipe `yq` output into `json.loads`, which only works with the Python
one; with the Go build they fail with a `JSONDecodeError`. `ci.yaml` installs
the right one explicitly for the same reason. `pwsh` is optional and its test
is skipped when absent.

Note that `verify.yaml` is **not** this: it is a reusable workflow that tests
whichever repo calls it, not this one.

See [release-workflow.md](docs/release-workflow.md) for testing and modification guidance.

### Versioning
Expand Down
Loading
Loading