feat: delete previous review before posting new one (#6) #22

Merged
aweiker merged 9 commits from feat/6-update-existing-review into main 2026-05-02 04:50:16 +00:00

9 Commits

Author SHA1 Message Date
Rodin 55391c66d8 refactor: validate reviewer-name early (fail fast before LLM call)
CI / test (pull_request) Successful in 13s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 24s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m0s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m40s
Moved validateReviewerName check to right after flag parsing. Previously
it ran after the LLM request completed — wasting an expensive API call
if the name was invalid.

Sonnet review finding #1.
2026-05-01 21:42:49 -07:00
Rodin 2287a8238c feat: add role title as H1 header for visual differentiation
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 19s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m27s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m42s
When reviewer-name is set, prepend "# Security Review" / "# Sonnet Review"
etc. as a top-level header. Makes it immediately obvious which role each
review represents in the Gitea UI, especially when multiple reviews come
from the same bot account.
2026-05-01 21:36:32 -07:00
Rodin 436e6a8824 fix: symlink traversal + worst-wins pre-check + user scoping
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 20s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m11s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m16s
Security (MAJOR):
- Add filepath.EvalSymlinks after Clean for system-prompt-file
- Re-validate resolved path is still within workspace
- Prevents symlink → /etc/shadow exfiltration via malicious repo

Worst-wins:
- Check BEFORE posting (not after) — no delete+repost dance
- Identify sibling bots by <!-- review-bot: prefix in body
- Only escalates for bot reviews, not human REQUEST_CHANGES
- If sibling bot has REQUEST_CHANGES and we would APPROVE → post
  REQUEST_CHANGES instead

Addresses security review finding #1 (MAJOR) and sonnet finding #1.
2026-05-01 21:31:17 -07:00
Rodin 687005d982 feat: worst-wins reconciliation for shared-token review types
CI / test (pull_request) Successful in 13s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 24s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m9s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m18s
When multiple review types share a Gitea bot account, Gitea uses the
latest review to determine the user's approval state. This creates a
race: if security finds issues but code-quality finishes last with
APPROVE, the PR appears approved.

Now before posting, each job checks if any sibling review from the same
user has REQUEST_CHANGES. If so and we would post APPROVE, we downgrade
to COMMENT instead — the review is still visible but won't override
the blocking state.

Documented in README under "Shared Token: Worst-Wins."
2026-05-01 21:12:34 -07:00
Rodin 6a3c813279 fix: address review findings (path restriction, login cross-check, README)
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 19s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m15s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m20s
- system-prompt-file: reject absolute paths and paths containing ".."
  Prevents reading arbitrary files outside the workspace on shared runners.
- Cleanup: cross-check r.User.Login == posted.User.Login before deletion
  Defense-in-depth: only attempt to delete reviews from same author.
  Flagged by both sonnet and security reviewers.
- README: fix wording (cleanup happens after posting, not before)

Issues filed for deferred work:
- #24: Consistent url.PathEscape across all client endpoints
- #25: Binary signature verification for supply-chain hardening
2026-05-01 21:05:18 -07:00
Rodin b8af8306a6 docs: comprehensive README with action usage, cleanup behavior, custom prompts
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 23s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m5s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m37s
- Quick start example with composite action + matrix strategy
- Full action inputs table with descriptions
- How sentinel-based cleanup works (explains the reviewer-name concept)
- Custom prompt file usage with security review example
- CLI usage with all flags
- Environment variables table
- Token scopes documentation
- Setup guide for new repos
2026-05-01 20:59:34 -07:00
Rodin 69e0a459c3 feat: sentinel-based review cleanup + system prompt file + security review
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 23s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 58s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m35s
Sentinel-based cleanup:
- Reviews embed <!-- review-bot:NAME --> in body (hidden HTML comment)
- Cleanup matches by sentinel, not token identity
- Each reviewer-name is a logical identity (sonnet, gpt, security)
- Same token can run multiple review types without conflict
- No extra API scopes needed

System prompt file (--system-prompt-file / SYSTEM_PROMPT_FILE):
- Loads a local file with additional review instructions
- Appended to system base as "Additional Review Instructions"
- Enables specialized reviews (security, performance, etc.)
- Partially addresses #5

Security review:
- SECURITY_REVIEW.md prompt focused on vulnerabilities
- 3rd CI matrix entry using same token, different prompt
- Focus: injection, auth, secrets, input validation, crypto, races

CI changes:
- REVIEWER_NAME passed from matrix.name
- SYSTEM_PROMPT_FILE passed from matrix (empty for standard reviews)
- 3 reviewers: sonnet (general), gpt (general), security (focused)
2026-05-01 20:55:09 -07:00
Rodin 41c670b44b fix: post-then-cleanup flow, remove dead code, pagination
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 31s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m22s
- PostReview now returns *Review (id + user login from response)
- Delete flow: post first, then delete stale reviews by same user
- No read:user scope needed (identity from POST response)
- Removed GetAuthenticatedUser (requires scope we lack)
- ListReviews: full pagination (loops until partial page)
- envOrDefaultBool: case-insensitive, whitespace-trimmed
- action.yml: document accepted boolean values
- Tests updated for new PostReview signature
2026-05-01 20:38:21 -07:00
Rodin 0d417e068e feat: delete previous review before posting new one (#6)
CI / test (pull_request) Successful in 13s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 21s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m20s
Before posting a review, the bot now:
1. Calls GET /api/v1/user to identify its own login
2. Lists all reviews on the PR
3. Deletes any existing reviews from itself
4. Posts the fresh review

This keeps PR threads clean — one review per bot at any time.

New Gitea client methods:
- GetAuthenticatedUser() — token self-identification
- ListReviews() — fetch reviews on a PR
- DeleteReview() — delete a review by ID

Flag: --update-existing / UPDATE_EXISTING (default true)
Set to false to preserve old behavior (stack reviews).

All delete failures are non-fatal (logged as warnings).

Closes #6
2026-05-01 20:17:01 -07:00