diff --git a/README.md b/README.md index b4e1b4b..4cb1acf 100644 --- a/README.md +++ b/README.md @@ -116,12 +116,14 @@ Scripts live in [`bin/`](bin) and are added to `PATH` via `zsh/zshrc.symlink`. #### Automated PR review -These orchestrate Claude Code reviews of pull requests. They power the `review-all-prs` LaunchAgent. +These scripts run pull request reviews through Claude Code or Codex. The `review-all-prs` LaunchAgent uses Codex, reviews only PRs from `team-feature-flags`, starts one PR per hourly run, and allows two attempts per calendar day. Failed reviews count toward the daily limit. + +Before starting or reinstalling the LaunchAgent, install the Codex CLI and run `codex login`. Existing service installations that only configured Claude will stop at the runner's authentication check until Codex is available. | Script | Purpose | | ------ | ------- | -| [`review-all-prs.sh`](bin/review-all-prs.sh) | Find PRs awaiting your review in a GitHub org using the GraphQL API. Filters out PRs you've already reviewed and sorts by priority: PRs authored by `--priority-team` members, then flags-scoped titles, then the rest. | -| [`run-pr-reviews.sh`](bin/run-pr-reviews.sh) | Take a list of PRs and run `/review-code` against each one in priority order, with per-review timeouts and Claude usage-limit detection. | +| [`review-all-prs.sh`](bin/review-all-prs.sh) | Find PRs awaiting your review in a GitHub org using the GraphQL API. `--author-team` limits every result source to current team members. The script filters out settled reviews and sorts by priority: `--priority-team` authors, flags-scoped titles, then the rest. | +| [`run-pr-reviews.sh`](bin/run-pr-reviews.sh) | Take a list of PRs and run the `review-code` skill through `--engine claude` or `--engine codex`. It supports per-run and daily attempt limits, review timeouts, and engine usage-limit detection. | | [`review-all-prs-service.sh`](bin/review-all-prs-service.sh) | Manage the `review-all-prs` macOS LaunchAgent (install, start, stop, logs, run). | | [`recent-reviews.sh`](bin/recent-reviews.sh) | Show recent PR review activity from session state files. | | [`seed-pr-failures.sh`](bin/seed-pr-failures.sh) | Rebuild the persistent PR-failure ledger from session history. | diff --git a/bin/lib/test-review-search-queries.sh b/bin/lib/test-review-search-queries.sh index f0e330f..428eeaf 100755 --- a/bin/lib/test-review-search-queries.sh +++ b/bin/lib/test-review-search-queries.sh @@ -63,9 +63,12 @@ if [[ "${1-}" == "api" && "${2-}" == "graphql" ]]; then fi done # PR_FIXTURE_QUERY names a substring; searches matching it return a page of - # two PRs, every other search an empty page. That reproduces a draft only some + # four PRs, every other search an empty page. That reproduces a draft only some # qualifiers can see. 99001 carries an unsubmitted draft review by "me"; - # 99002 carries no review at all, so it is what pending mode has to drop. + # 99002 carries no review at all, so it is what pending mode has to drop; + # 99003 is an outside-team author with a pending review, so strict author-team + # filtering has to remove it even when it came from the pending sweep; 99004 + # belongs to a second team for repeatable --author-team coverage. if [[ -n "${PR_FIXTURE_QUERY-}" && "$query" == *"$PR_FIXTURE_QUERY"* ]]; then cat <<'NODE' {"data":{"search":{"pageInfo":{"hasNextPage":false,"endCursor":null},"edges":[{"node":{ @@ -86,6 +89,24 @@ if [[ "${1-}" == "api" && "${2-}" == "graphql" ]]; then "updatedAt":"2026-08-24T12:00:00Z", "reviews":{"nodes":[]}, "commits":{"nodes":[{"commit":{"committedDate":"2026-08-24T11:00:00Z"}}]} +}}, {"node":{ +"number":99003, +"title":"fix(flags): fixture PR from outside the team", +"url":"https://github.com/PostHog/posthog/pull/99003", +"repository":{"nameWithOwner":"PostHog/posthog"}, +"author":{"login":"outside-dev"}, +"updatedAt":"2026-08-24T12:00:00Z", +"reviews":{"nodes":[{"author":{"login":"me"},"state":"PENDING","submittedAt":null}]}, +"commits":{"nodes":[{"commit":{"committedDate":"2026-08-24T11:00:00Z"}}]} +}}, {"node":{ +"number":99004, +"title":"feat(flags): fixture PR from a second allowed team", +"url":"https://github.com/PostHog/posthog/pull/99004", +"repository":{"nameWithOwner":"PostHog/posthog"}, +"author":{"login":"growth-dev"}, +"updatedAt":"2026-08-24T12:00:00Z", +"reviews":{"nodes":[]}, +"commits":{"nodes":[{"commit":{"committedDate":"2026-08-24T11:00:00Z"}}]} }}]}}} NODE exit 0 @@ -95,12 +116,16 @@ NODE fi case "${2-}" in user) echo "me" ;; + orgs/*/teams/growth/members*) + if [[ "$*" == *"[.[].login]"* ]]; then echo '["growth-dev"]' + else echo 'growth-dev'; fi + ;; orgs/*/teams/*/members*) # '[.[].login]' asks for a JSON array, '.[].login' for bare lines. if [[ "$*" == *"[.[].login]"* ]]; then echo '["dev-one","dev-two"]' else printf 'dev-one\ndev-two\n'; fi ;; - orgs/*/members*) echo '["dev-one","dev-two"]' ;; + orgs/*/members*) echo '["dev-one","dev-two","growth-dev"]' ;; *) echo '[]' ;; esac SHIM @@ -177,6 +202,12 @@ assert "--all folds the priority team into team-review-requested" \ ran_query "team-review-requested:PostHog/flags" assert "--all sweeps involves:@me for pending drafts" ran_query "involves:@me" +run_queries --all --author-team flags +assert "--all uses --author-team members for its author search" \ + ran_query "author:dev-one author:dev-two" +assert_not "--author-team does not add a team review-request query" \ + ran_query "team-review-requested:PostHog/flags" + run_queries --org acme --draft assert "--draft scopes its queries to --org" ran_query "org:acme" @@ -197,6 +228,34 @@ out=$(run_with_fixture "involves:@me" --draft --json) assert "--draft still reports a draft found only through the involves sweep" \ grep -q '"number": 99001' <<< "$out" +# --author-team filters every merged result, unlike --team, which only expands discovery. +out=$(run_with_fixture "review-requested:@me" --author-team flags --json) +assert "--author-team keeps a direct review request authored by a team member" \ + grep -q '"number": 99002' <<< "$out" +assert_not "--author-team excludes a direct review request from outside the team" \ + grep -q '"number": 99003' <<< "$out" +assert_not "one --author-team excludes authors from a different team" \ + grep -q '"number": 99004' <<< "$out" + +out=$(run_with_fixture "review-requested:@me" \ + --author-team flags --author-team growth --json) +assert "repeated --author-team keeps authors from the first team" \ + grep -q '"number": 99002' <<< "$out" +assert "repeated --author-team keeps authors from the second team" \ + grep -q '"number": 99004' <<< "$out" +assert_not "repeated --author-team still excludes authors from other teams" \ + grep -q '"number": 99003' <<< "$out" + +out=$(run_with_fixture "involves:@me" --author-team flags --json) +assert "--author-team keeps a team member's pending review from the involves sweep" \ + grep -q '"number": 99001' <<< "$out" +assert_not "--author-team excludes an outside author's pending review from the merged sweep" \ + grep -q '"number": 99003' <<< "$out" + +out=$(run_with_fixture "review-requested:@me" --team flags --json) +assert "--team keeps its discovery-only semantics for outside authors" \ + grep -q '"number": 99003' <<< "$out" + # The hidden count measures the new-commits gate, which pending mode's extra # PENDING filter makes meaningless, so pending mode must not report one. out=$(run_with_fixture "review-requested:@me" --draft) diff --git a/bin/lib/test-run-pr-reviews.sh b/bin/lib/test-run-pr-reviews.sh new file mode 100755 index 0000000..f77bfbe --- /dev/null +++ b/bin/lib/test-run-pr-reviews.sh @@ -0,0 +1,312 @@ +#!/bin/bash +# Offline tests for the engine boundary and overnight safety limits; shims prevent network requests. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +# shellcheck source=bin/lib/test-helpers.sh +source "$SCRIPT_DIR/test-helpers.sh" + +BIN="$(cd "$SCRIPT_DIR/.." && pwd)/run-pr-reviews.sh" +ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" +PLIST="$ROOT/macos/LaunchAgents/com.haacked.review-all-prs.plist" +SERVICE="$ROOT/bin/review-all-prs-service.sh" + +TESTTMP="$(mktemp -d)" +TESTTMP="$(cd "$TESTTMP" && pwd -P)" +trap 'rm -rf "$TESTTMP"' EXIT + +FAKE_HOME="$TESTTMP/home" +SHIM_BIN="$TESTTMP/bin" +CODEX_LOG="$TESTTMP/codex.log" +GH_LOG="$TESTTMP/gh.log" +mkdir -p "$FAKE_HOME" "$SHIM_BIN" + +BASH4="" +for candidate in "$BASH" /opt/homebrew/bin/bash /usr/local/bin/bash "$(command -v bash)"; do + [[ -x "$candidate" ]] || continue + # shellcheck disable=SC2016 # BASH_VERSINFO must expand in the candidate. + if [[ "$("$candidate" -c 'echo ${BASH_VERSINFO[0]}')" -ge 4 ]]; then + BASH4="$candidate" + break + fi +done +if [[ -z "$BASH4" ]]; then + echo "No bash 4+ found; run-pr-reviews.sh cannot run. Install bash via Homebrew." >&2 + exit 1 +fi +SHIM_PATH="$SHIM_BIN:$(dirname "$BASH4"):/usr/bin:/bin" + +cat > "$SHIM_BIN/gh" <<'SHIM' +#!/bin/bash +printf '%s\n' "$*" >> "$GH_LOG" + +if [[ "${1-}" == "auth" && "${2-}" == "status" ]]; then + exit 0 +fi +if [[ "${1-}" != "api" ]]; then + echo '[]' + exit 0 +fi + +case "${2-}" in + user) + echo 'me' + ;; + graphql) + if [[ "${GH_AUTO_FIXTURE:-false}" == "true" ]]; then + cat <<'JSON' +{"data":{"search":{"pageInfo":{"hasNextPage":false,"endCursor":null},"edges":[{"node":{"number":99021,"title":"feat(flags): team fixture","url":"https://github.com/PostHog/posthog/pull/99021","repository":{"nameWithOwner":"PostHog/posthog"},"author":{"login":"team-dev"},"updatedAt":"2026-08-30T12:00:00Z","reviews":{"nodes":[]},"commits":{"nodes":[{"commit":{"committedDate":"2026-08-30T11:00:00Z"}}]}}},{"node":{"number":99022,"title":"feat(flags): outside fixture","url":"https://github.com/PostHog/posthog/pull/99022","repository":{"nameWithOwner":"PostHog/posthog"},"author":{"login":"outside-dev"},"updatedAt":"2026-08-30T12:00:00Z","reviews":{"nodes":[]},"commits":{"nodes":[{"commit":{"committedDate":"2026-08-30T11:00:00Z"}}]}}}]}}} +JSON + else + echo '{"data":{"search":{"pageInfo":{"hasNextPage":false,"endCursor":null},"edges":[]}}}' + fi + ;; + orgs/*/teams/*/members*) + if [[ "$*" == *"[.[].login]"* ]]; then + echo '["team-dev"]' + else + echo 'team-dev' + fi + ;; + orgs/*/members*) + echo '[]' + ;; + repos/*/pulls/*/reviews) + echo '[]' + ;; + repos/*/pulls/*/comments) + echo '[{"user":{"login":"me"},"created_at":"2099-01-01T00:00:00Z"}]' + ;; + *) + echo '[]' + ;; +esac +SHIM + +cat > "$SHIM_BIN/codex" <<'SHIM' +#!/bin/bash +if [[ "${1-}" == "login" && "${2-}" == "status" ]]; then + exit 0 +fi + +printf '<%s>\n' "$@" >> "$CODEX_LOG" +call_number=$(grep -c '^$' "$CODEX_LOG") +if [[ "${CODEX_FAIL_FIRST:-false}" == "true" && "$call_number" -eq 1 ]]; then + echo '{"type":"error","message":"fixture engine failure"}' + exit 7 +fi +if [[ "${CODEX_RATE_LIMIT:-false}" == "true" ]]; then + cat <<'JSON' +{"type":"error","message":"You've hit your usage limit."} +JSON + exit 1 +fi +if [[ "${CODEX_AUTH_FAIL:-false}" == "true" ]]; then + echo '{"type":"error","message":"status 401: unauthorized"}' + exit 1 +fi +echo '{"type":"item.completed","item":{"id":"item_0","type":"agent_message","text":"Codex fixture review result\n\nSecond fixture line"}}' +echo '{"type":"turn.completed","usage":{"input_tokens":101,"cached_input_tokens":17,"output_tokens":23}}' +SHIM + +cat > "$SHIM_BIN/claude" <<'SHIM' +#!/bin/bash +if [[ "${CLAUDE_RATE_LIMIT:-false}" == "true" ]]; then + echo '{"type":"error","error":{"type":"rate_limit_error"}}' + exit 1 +fi +if [[ "${CLAUDE_RATE_LIMIT_TEXT:-false}" == "true" ]]; then + echo '{"type":"result","subtype":"success","result":"Claude fixture review covers rate limit handling"}' +else + echo '{"type":"result","subtype":"success","result":"Claude fixture review result"}' +fi +SHIM + +cat > "$SHIM_BIN/caffeinate" <<'SHIM' +#!/bin/bash +[[ "${1-}" == "-i" ]] && shift +exec "$@" +SHIM + +cat > "$SHIM_BIN/timeout" <<'SHIM' +#!/bin/bash +while [[ "${1-}" == --* ]]; do shift; done +shift +exec "$@" +SHIM + +chmod +x "$SHIM_BIN/gh" "$SHIM_BIN/codex" "$SHIM_BIN/claude" \ + "$SHIM_BIN/caffeinate" "$SHIM_BIN/timeout" + +PR_ONE='[{"number":99011,"title":"feat(flags): first fixture","url":"https://github.com/PostHog/posthog/pull/99011","repo":"PostHog/posthog","author":"team-dev","user_review_state":null}]' +PR_THREE='[ + {"number":99011,"title":"feat(flags): first fixture","url":"https://github.com/PostHog/posthog/pull/99011","repo":"PostHog/posthog","author":"team-dev","user_review_state":null}, + {"number":99012,"title":"feat(flags): second fixture","url":"https://github.com/PostHog/posthog/pull/99012","repo":"PostHog/posthog","author":"team-dev","user_review_state":null}, + {"number":99013,"title":"feat(flags): third fixture","url":"https://github.com/PostHog/posthog/pull/99013","repo":"PostHog/posthog","author":"team-dev","user_review_state":null} +]' + +case_number=0 +STATE_CASE="" +start_case() { + case_number=$((case_number + 1)) + STATE_CASE="$TESTTMP/state-${case_number}" + mkdir -p "$STATE_CASE" + : > "$CODEX_LOG" + : > "$GH_LOG" +} + +run_runner() { + local state_dir="$1" input="$2" + shift 2 + printf '%s\n' "$input" | run_bounded 20 env \ + HOME="$FAKE_HOME" PATH="$SHIM_PATH" GH_LOG="$GH_LOG" \ + GH_AUTO_FIXTURE="${GH_AUTO_FIXTURE:-false}" \ + CODEX_LOG="$CODEX_LOG" CODEX_FAIL_FIRST="${CODEX_FAIL_FIRST:-false}" \ + CODEX_RATE_LIMIT="${CODEX_RATE_LIMIT:-false}" \ + CODEX_AUTH_FAIL="${CODEX_AUTH_FAIL:-false}" \ + CLAUDE_RATE_LIMIT="${CLAUDE_RATE_LIMIT:-false}" \ + CLAUDE_RATE_LIMIT_TEXT="${CLAUDE_RATE_LIMIT_TEXT:-false}" \ + RUN_PR_REVIEWS_STATE_DIR="$state_dir" "$BASH4" "$BIN" "$@" +} + +codex_log_has_pair() { + local option="$1" value="$2" + awk -v option="<$option>" -v value="<$value>" ' + $0 == option { + if ((getline next_line) > 0 && next_line == value) found = 1 + } + END { exit !found } + ' "$CODEX_LOG" +} + +start_case +run_runner "$STATE_CASE" "$PR_ONE" --engine codex --max-prs 1 --delay 0 >/dev/null +assert "Codex uses the non-interactive exec command" grep -qx '' "$CODEX_LOG" +assert "Codex streams machine-readable JSONL" grep -qx '<--json>' "$CODEX_LOG" +assert "Codex does not persist an unattended task" grep -qx '<--ephemeral>' "$CODEX_LOG" +assert "Codex uses the workspace-write sandbox" grep -qx '' "$CODEX_LOG" +assert "Codex routes any approval request through automatic safety review" \ + grep -qx '<--approve-for-me>' "$CODEX_LOG" +assert "Codex accepts an isolated non-repository working directory" \ + grep -qx '<--skip-git-repo-check>' "$CODEX_LOG" +assert "Codex permits the review skill's session state to be written" \ + codex_log_has_pair --add-dir "$FAKE_HOME/.agents/skills/review-code/.sessions" +assert "Codex permits review worktrees to be created" \ + codex_log_has_pair --add-dir "$FAKE_HOME/.agents/skills/review-code/.worktrees" +assert "Codex permits persistent review files to be written" \ + codex_log_has_pair --add-dir "$FAKE_HOME/.agents/skills/review-code/.reviews" +assert "Codex permits the current review output to be written" \ + codex_log_has_pair --add-dir "$FAKE_HOME/dev/ai/reviews/PostHog-posthog" +assert "Codex runs from the isolated review output directory" \ + codex_log_has_pair -C "$FAKE_HOME/dev/ai/reviews/PostHog-posthog" +assert_not "Codex cannot write the whole installed review skill" \ + grep -qxF "<$FAKE_HOME/.agents/skills/review-code>" "$CODEX_LOG" +assert_not "Codex cannot write the live dotfiles checkout" \ + grep -qxF "<$FAKE_HOME/.dotfiles>" "$CODEX_LOG" +# shellcheck disable=SC2016 # Codex skill prompts intentionally start with a literal dollar sign. +assert "Codex receives the review-code skill prompt" \ + grep -qxF '<$review-code https://github.com/PostHog/posthog/pull/99011 --force --draft>' "$CODEX_LOG" + +review_file="$FAKE_HOME/dev/ai/reviews/PostHog-posthog/pr-99011-$(date +%Y%m%d).md" +assert "the Codex agent message is extracted from JSONL into the review artifact" \ + grep -qF 'Codex fixture review result' "$review_file" +assert "the complete multiline Codex message is preserved in the review artifact" \ + grep -qF 'Second fixture line' "$review_file" +assert "the Codex review artifact preserves input-token usage" \ + grep -Eqi 'input[^0-9]*101' "$review_file" +assert "the Codex review artifact preserves output-token usage" \ + grep -Eqi 'output[^0-9]*23' "$review_file" + +start_case +CODEX_FAIL_FIRST=true run_runner "$STATE_CASE" "$PR_THREE" \ + --engine codex --max-prs 1 --daily-max-prs 2 --delay 0 >/dev/null || true +run_runner "$STATE_CASE" "$PR_THREE" \ + --engine codex --max-prs 1 --daily-max-prs 2 --delay 0 >/dev/null +run_runner "$STATE_CASE" "$PR_THREE" \ + --engine codex --max-prs 1 --daily-max-prs 2 --delay 0 >/dev/null +assert "--daily-max-prs stops after two attempted Codex reviews" \ + test "$(grep -c '^$' "$CODEX_LOG")" -eq 2 +assert "a failed review consumes the daily safety budget" \ + jq -e '.failed | map(.url) | index("https://github.com/PostHog/posthog/pull/99011") != null' \ + "$STATE_CASE/session-$(date +%Y-%m-%d).json" >/dev/null +assert_not "the daily cap leaves the third PR for a later day" \ + grep -qF 'https://github.com/PostHog/posthog/pull/99013' "$CODEX_LOG" + +start_case +CODEX_RATE_LIMIT=true run_runner "$STATE_CASE" "$PR_THREE" \ + --engine codex --delay 0 >/dev/null || true +assert "Codex's usage-limit wording stops the session after one attempt" \ + test "$(grep -c '^$' "$CODEX_LOG")" -eq 1 +assert "a Codex usage limit is recorded as an engine failure" \ + jq -e '.failed[0].reason == "rate_limited"' \ + "$STATE_CASE/session-$(date +%Y-%m-%d).json" >/dev/null +assert_not "a Codex usage limit does not count against the PR failure ledger" \ + jq -e '.prs | has("https://github.com/PostHog/posthog/pull/99011")' \ + "$STATE_CASE/pr-failures.json" >/dev/null + +start_case +CODEX_AUTH_FAIL=true run_runner "$STATE_CASE" "$PR_ONE" \ + --engine codex --delay 0 >/dev/null || true +assert "a Codex authentication failure is recorded as an engine failure" \ + jq -e '.failed[0].reason == "auth_failed"' \ + "$STATE_CASE/session-$(date +%Y-%m-%d).json" >/dev/null +assert_not "a Codex authentication failure does not count against the PR failure ledger" \ + jq -e '.prs | has("https://github.com/PostHog/posthog/pull/99011")' \ + "$STATE_CASE/pr-failures.json" >/dev/null + +start_case +out=$(GH_AUTO_FIXTURE=true run_runner "$STATE_CASE" '[]' --auto --engine codex \ + --author-team team-feature-flags --dry-run) +assert "--auto keeps a PR authored by the requested team" \ + grep -qF 'https://github.com/PostHog/posthog/pull/99021' <<< "$out" +assert_not "--auto excludes an outside-team PR at the runner boundary" \ + grep -qF 'https://github.com/PostHog/posthog/pull/99022' <<< "$out" + +start_case +out=$(run_runner "$STATE_CASE" "$PR_ONE" --dry-run --max-prs 1 --delay 0) +assert "Claude remains the default engine for manual compatibility" \ + grep -qF 'Would run: claude -p "/review-code https://github.com/PostHog/posthog/pull/99011 --force --draft"' <<< "$out" + +start_case +CLAUDE_RATE_LIMIT_TEXT=true run_runner "$STATE_CASE" "$PR_ONE" \ + --engine claude --max-prs 1 --delay 0 >/dev/null +assert "successful Claude prose about rate limits remains successful" \ + jq -e '.reviewed | map(.url) | index("https://github.com/PostHog/posthog/pull/99011") != null' \ + "$STATE_CASE/session-$(date +%Y-%m-%d).json" >/dev/null + +start_case +CLAUDE_RATE_LIMIT=true run_runner "$STATE_CASE" "$PR_ONE" \ + --engine claude --max-prs 1 --delay 0 >/dev/null || true +assert "Claude's shipped rate-limit signal still stops the session" \ + jq -e '.failed[0].reason == "rate_limited"' \ + "$STATE_CASE/session-$(date +%Y-%m-%d).json" >/dev/null + +plist_has_pair() { + local option="$1" value="$2" + awk -v option="$option" -v value="$value" ' + index($0, "" option "") { + if ((getline next_line) > 0 && index(next_line, "" value "")) found = 1 + } + END { exit !found } + ' "$PLIST" +} + +assert "the LaunchAgent selects the Codex engine" plist_has_pair --engine codex +assert "the LaunchAgent limits review authors to team-feature-flags" \ + plist_has_pair --author-team team-feature-flags +assert "the LaunchAgent starts at most one PR per hourly tick" \ + plist_has_pair --max-prs 1 +assert "the LaunchAgent spends at most two attempts per day" \ + plist_has_pair --daily-max-prs 2 +assert "the service wrapper selects the Codex engine" \ + grep -Eq -- 'WORKER_ARGS=.*--engine codex' "$SERVICE" +assert "the service wrapper limits review authors to team-feature-flags" \ + grep -Eq -- 'WORKER_ARGS=.*--author-team team-feature-flags' "$SERVICE" +assert "the service wrapper starts at most one PR per run" \ + grep -Eq -- 'WORKER_ARGS=.*--max-prs 1' "$SERVICE" +assert "the service wrapper spends at most two attempts per day" \ + grep -Eq -- 'WORKER_ARGS=.*--daily-max-prs 2' "$SERVICE" + +print_results diff --git a/bin/review-all-prs-service.sh b/bin/review-all-prs-service.sh index 9b9a2df..2fe9c3d 100755 --- a/bin/review-all-prs-service.sh +++ b/bin/review-all-prs-service.sh @@ -19,7 +19,7 @@ source "${SCRIPT_DIR}/lib/launchd-service.sh" SERVICE_NAME="review-all-prs" WORKER="${SCRIPT_DIR}/run-pr-reviews.sh" # Keep in sync with the args in macos/LaunchAgents/com.haacked.review-all-prs.plist. -WORKER_ARGS=(--auto --team team-feature-flags --priority-team team-feature-flags --all) +WORKER_ARGS=(--auto --engine codex --team team-feature-flags --author-team team-feature-flags --priority-team team-feature-flags --all --max-prs 1 --daily-max-prs 2) SCHEDULE_DESC="hourly 6 PM-2 AM on weekdays, around the clock on weekends" # Today's review session counts, appended to `status`. diff --git a/bin/review-all-prs.sh b/bin/review-all-prs.sh index d8bc38c..c18cc74 100755 --- a/bin/review-all-prs.sh +++ b/bin/review-all-prs.sh @@ -12,6 +12,7 @@ # --limit N Maximum number of PRs to return (default: 50) # --json Output raw JSON (default: formatted table) # --team TEAM Also find PRs requested from this team (repeatable) +# --author-team TEAM Only return PRs authored by this team (repeatable) # --priority-team TEAM PRs authored by members of this team sort first # --sort KEY[:DIR] Sort order: priority|repo|status|number, optional :asc # or :desc (default: priority). priority groups by tier; @@ -23,7 +24,8 @@ # -h, --help Show this help message # # --all widens the default reviewer-requested list to the team's review queue. -# With no --priority-team or --team it defaults to team-feature-flags, then +# With no --priority-team, --team, or --author-team it defaults to +# team-feature-flags, then # folds in: PRs requested from the team, and all open non-draft PRs authored # by team members. This is a deliberate superset for triage, not a mirror of # any project board. It does NOT imply --include-reviewed: PRs you've already @@ -88,8 +90,9 @@ INCLUDE_REVIEWED=false PENDING_ONLY=false ALL=false TEAMS=() +AUTHOR_TEAMS=() PRIORITY_TEAM="" -# Team to fall back on when --all is given with no --priority-team or --team. +# Team to fall back on when --all has no team option. DEFAULT_TEAM="team-feature-flags" # Sort spec: a JSON array of {key, dir} pairs in precedence order. Empty by # default — the filter always appends the priority tier and recency — and @@ -107,6 +110,10 @@ Options: --limit N Maximum number of PRs to return (default: 50) --json Output raw JSON (default: formatted table) --team TEAM Also find PRs requested from this team (repeatable) + --author-team TEAM Only return PRs authored by members of this team. + Repeat to allow authors from more than one team. This is + a strict final filter across review requests and pending + draft reviews; unlike --team, it does not widen discovery. --priority-team TEAM PRs authored by members of this team sort first --sort SPEC Sort order as a comma-separated list of KEY[:DIR]. KEY is one of priority, repo, status, number; DIR is @@ -117,12 +124,12 @@ Options: --include-reviewed Include PRs you've already reviewed. PRs where you have a pending (unsubmitted) draft review are always shown, with or without this flag. - --all Widen the list to the team's whole review queue. Defaults - to ${DEFAULT_TEAM} when no --priority-team or --team is - given. Adds, for those teams: PRs requested from the team - and all open non-draft PRs authored by team members. Does - NOT imply --include-reviewed; pass that too to also show - PRs you've already reviewed with no new commits since. + --all Widen the list to the selected teams' whole review queue. + Defaults to ${DEFAULT_TEAM} when no team option is given. + Adds team requests for --team and --priority-team, plus + all open non-draft PRs authored by members of any selected + team. Pass --include-reviewed to show PRs you've already + reviewed with no new commits since. Paginates so a busy team's queue isn't truncated at --limit. A triage superset, not a mirror of any project board. @@ -140,6 +147,7 @@ Examples: $(basename "$0") --json # Output as JSON for scripting $(basename "$0") --limit 10 # Limit to 10 PRs $(basename "$0") --team my-team # Include team review requests + $(basename "$0") --author-team my-team # Only show PRs by team members $(basename "$0") --pending # List your pending draft reviews $(basename "$0") --all --priority-team my-team # Whole team review queue $(basename "$0") --sort repo # Sort the whole list by repository name @@ -212,6 +220,14 @@ while [[ $# -gt 0 ]]; do TEAMS+=("$2") shift 2 ;; + --author-team) + if [[ -z "${2:-}" || "${2:-}" == --* ]]; then + echo "--author-team requires a team name" >&2 + exit 1 + fi + AUTHOR_TEAMS+=("$2") + shift 2 + ;; --priority-team) if [[ -z "${2:-}" || "${2:-}" == --* ]]; then echo "--priority-team requires a team name" >&2 @@ -258,7 +274,7 @@ if [[ "$ALL" == "true" ]]; then echo "--all and --pending/--draft cannot be combined" >&2 exit 1 fi - if [[ -z "$PRIORITY_TEAM" && ${#TEAMS[@]} -eq 0 ]]; then + if [[ -z "$PRIORITY_TEAM" && ${#TEAMS[@]} -eq 0 && ${#AUTHOR_TEAMS[@]} -eq 0 ]]; then PRIORITY_TEAM="$DEFAULT_TEAM" echo "--all with no team specified; defaulting to ${ORG}/${DEFAULT_TEAM}" >&2 fi @@ -266,15 +282,37 @@ fi GITHUB_USER=$(get_github_user) +get_team_members_json() { + local team="$1" + gh api "orgs/${ORG}/teams/${team}/members?per_page=100" --paginate --jq '[.[].login]' | jq -s 'add // []' +} + # Logins whose PRs get priority 1, as a JSON array for the jq filter. TEAM_MEMBERS="[]" if [[ -n "$PRIORITY_TEAM" ]]; then - TEAM_MEMBERS=$(gh api "orgs/${ORG}/teams/${PRIORITY_TEAM}/members?per_page=100" --paginate --jq '[.[].login]' | jq -s 'add') || { + TEAM_MEMBERS=$(get_team_members_json "$PRIORITY_TEAM") || { echo "Could not list members of ${ORG}/${PRIORITY_TEAM}" >&2 exit 1 } fi +# Restrict results after merging discovery sources so direct requests and pending reviews cannot bypass the filter. +AUTHOR_TEAM_MEMBERS="[]" +for team in "${AUTHOR_TEAMS[@]}"; do + if [[ -n "$PRIORITY_TEAM" && "$team" == "$PRIORITY_TEAM" ]]; then + author_team_members_for_team="$TEAM_MEMBERS" + else + author_team_members_for_team=$(get_team_members_json "$team") || { + echo "Could not list members of ${ORG}/${team}" >&2 + exit 1 + } + fi + AUTHOR_TEAM_MEMBERS=$(jq -cn \ + --argjson existing "$AUTHOR_TEAM_MEMBERS" \ + --argjson additions "$author_team_members_for_team" \ + '$existing + $additions | unique') +done + # Teams whose review requests count as yours, deduplicated because --team and # --priority-team can name the same team. --all folds in the priority team, # since it widens to that team's whole queue rather than just --team. This one @@ -300,9 +338,9 @@ if [[ "$ALL" == "true" ]]; then for team in "${REQUEST_TEAMS[@]}"; do # The priority team's roster is already in TEAM_MEMBERS as a JSON array. if [[ -n "$PRIORITY_TEAM" && "$team" == "$PRIORITY_TEAM" ]]; then - members=$(jq -r '.[]' <<< "$TEAM_MEMBERS") + team_members_json="$TEAM_MEMBERS" else - members=$(gh api "orgs/${ORG}/teams/${team}/members?per_page=100" --paginate --jq '.[].login') || { + team_members_json=$(get_team_members_json "$team") || { echo "Could not list members of ${ORG}/${team}" >&2 exit 1 } @@ -311,8 +349,13 @@ if [[ "$ALL" == "true" ]]; then [[ -z "$login" || -n "${seen_member[$login]:-}" ]] && continue seen_member[$login]=1 AUTHOR_MEMBERS+=("$login") - done <<< "$members" + done < <(jq -r '.[]' <<< "$team_members_json") done + while IFS= read -r login; do + [[ -z "$login" || -n "${seen_member[$login]:-}" ]] && continue + seen_member[$login]=1 + AUTHOR_MEMBERS+=("$login") + done < <(jq -r '.[]' <<< "$AUTHOR_TEAM_MEMBERS") fi # Org members, used to flag PR authors who belong to the org with a hedgehog @@ -471,6 +514,11 @@ ALL_RESULTS=$(merge_results "$PENDING_NODES" "$ALL_RESULTS") # Deduplicate by PR URL ALL_RESULTS=$(echo "$ALL_RESULTS" | jq 'unique_by(.url)') +if [[ ${#AUTHOR_TEAMS[@]} -gt 0 ]]; then + ALL_RESULTS=$(echo "$ALL_RESULTS" | jq --argjson allowed "$AUTHOR_TEAM_MEMBERS" \ + '[.[] | select(.author.login as $author | ($allowed | index($author)) != null)]') +fi + PROCESSED=$(echo "$ALL_RESULTS" | jq \ --arg user "$GITHUB_USER" \ --argjson include_reviewed "$INCLUDE_REVIEWED" \ diff --git a/bin/run-pr-reviews.sh b/bin/run-pr-reviews.sh index 2235e0d..aedee16 100755 --- a/bin/run-pr-reviews.sh +++ b/bin/run-pr-reviews.sh @@ -1,8 +1,8 @@ #!/usr/bin/env bash -# run-pr-reviews.sh - Orchestrate PR reviews using Claude Code +# run-pr-reviews.sh - Orchestrate PR reviews using Claude Code or Codex # # Takes a list of PRs (from review-all-prs.sh or directly) and runs -# /review-code on each one sequentially. +# review-code on each one sequentially. # # Usage: # run-pr-reviews.sh [OPTIONS] @@ -10,12 +10,16 @@ # # Options: # --auto Run in automatic mode (calls review-all-prs.sh) +# --engine ENGINE Review harness: claude or codex (default: claude) # --max-prs N Maximum PRs to review; 0 means no cap, the session -# time budget and Claude usage limits govern (default: 0) +# time budget and engine usage limits govern (default: 0) +# --daily-max-prs N Maximum review attempts per calendar day; 0 means no +# daily cap (default: 0) # --delay SECONDS Delay between reviews in seconds (default: 30) # --dry-run Show what would be reviewed without running # --org ORG GitHub org for --auto mode (default: PostHog) # --team TEAM Also find PRs requested from this team (repeatable) +# --author-team TEAM Only review PRs authored by this team (repeatable) # --priority-team TEAM PRs authored by members of this team review first # --all Widen discovery to the team's whole review queue (see # review-all-prs.sh --all). Does not re-review PRs you've @@ -26,7 +30,7 @@ # first, then flags-scoped titles, then the rest. PRs where you have an # unsubmitted draft review are skipped: a pending draft means you're # mid-review, and GitHub rejects starting another review while one is -# pending. If a review fails because the Claude usage limit was hit, the +# pending. If a review fails because the engine usage limit was hit, the # session stops; the next scheduled tick picks up where it left off once # the limit window resets. # @@ -45,19 +49,20 @@ source "${SCRIPT_DIR}/lib/fs.sh" # Configuration. STATE_DIR is overridable for tests; everything else is fixed. STATE_DIR="${RUN_PR_REVIEWS_STATE_DIR:-${HOME}/.local/state/review-all-prs}" REVIEWS_DIR="${HOME}/dev/ai/reviews" -# 0 = no per-tick cap; the session time budget and Claude usage limits decide +# 0 = no per-tick cap; the session time budget and engine usage limits decide # when to stop. MAX_PRS=0 +DAILY_MAX_PRS=0 # How many candidates to fetch from GitHub per search query during discovery. # Deliberately larger than any realistic per-tick throughput so that skips # (already reviewed, quarantined) never starve the queue. DISCOVERY_LIMIT=50 DELAY_SECONDS=30 # Generous enough for a full multi-agent review; the session time budget and -# Claude usage limits bound total spend, not a per-review dollar cap. +# engine usage limits bound total spend, not a per-review dollar cap. REVIEW_TIMEOUT_SECONDS=3600 # Force SIGKILL this many seconds after the initial SIGTERM if the review -# process ignores it. Without this, claude can ignore the timeout and run for +# process ignores it. Without this, an engine can ignore the timeout and run for # hours of wall-clock time (especially on a sleeping laptop). REVIEW_KILL_AFTER_SECONDS=60 # Quarantine a PR after this many consecutive failures across sessions, so a @@ -73,18 +78,19 @@ DRY_RUN=false AUTO_MODE=false ORG="PostHog" TEAMS=() +AUTHOR_TEAMS=() PRIORITY_TEAM="" ALL=false -# Set when a review fails because Claude itself is out of quota. Further +ENGINE="claude" +# Set when a review fails because the engine itself is out of quota. Further # reviews in this session would fail the same way, so the main loop stops. RATE_LIMITED=false -# Set when a review fails because Claude's OAuth session expired. Further +# Set when a review fails because the engine's session expired. Further # reviews in this session would fail the same way, so the main loop stops. AUTH_FAILED=false # Authenticated GitHub username, set once in main() and read by run_review() # to verify a review actually landed on GitHub. GITHUB_USER="" -REVIEW_FILE_PATH_SCRIPT="${HOME}/.claude/skills/review-code/scripts/review-file-path.sh" FAILURES_FILE="${STATE_DIR}/pr-failures.json" SESSION_START_TIME=0 LEDGER='{"version": 1, "prs": {}}' @@ -93,15 +99,20 @@ usage() { cat < /dev/null 2>&1 } +daily_attempt_count() { + echo "$SESSION" | jq '(.reviewed | length) + (.failed | length)' +} + +daily_limit_reached() { + [[ "$DRY_RUN" != "true" && "$DAILY_MAX_PRS" -gt 0 ]] || return 1 + [[ "$(daily_attempt_count)" -ge "$DAILY_MAX_PRS" ]] +} + # Check if a review file already exists for a PR # Uses the review-code skill's path resolution script review_exists() { @@ -376,6 +433,9 @@ get_pr_list() { for team in "${TEAMS[@]}"; do team_args+=(--team "$team") done + for team in "${AUTHOR_TEAMS[@]}"; do + team_args+=(--author-team "$team") + done if [[ -n "$PRIORITY_TEAM" ]]; then team_args+=(--priority-team "$PRIORITY_TEAM") fi @@ -399,10 +459,15 @@ get_pr_list() { # Check prerequisites. Failures are recorded in the session so that # recent-reviews.sh can show why a session produced no reviews. check_prerequisites() { - # Check for claude CLI - if ! command -v claude &> /dev/null; then - log_error "Claude CLI not found. Please install it first." - mark_error "Claude CLI not found" + if ! command -v "$ENGINE" &> /dev/null; then + log_error "${ENGINE} CLI not found. Please install it first." + mark_error "${ENGINE} CLI not found" + exit 1 + fi + + if [[ "$ENGINE" == "codex" ]] && ! codex login status &> /dev/null; then + log_error "Not authenticated with Codex. Run 'codex login' first." + mark_error "Codex authentication failed" exit 1 fi @@ -428,24 +493,33 @@ check_prerequisites() { fi } -# claude is invoked with --output-format stream-json, which emits one JSON -# object per turn/tool-call as it happens instead of buffering the whole run -# until completion. That's what lets a timed-out review still show what -# Claude was doing when it got killed. Lines are parsed with `fromjson? // -# empty` throughout so a line truncated mid-write by SIGKILL is silently -# skipped rather than aborting the pipeline. - -# Extracts the final assistant answer from the transcript (the last "result" -# event's .result field) to store in the review file, matching what -# --output-format text would have printed on its own. extract_result_text() { local output_file="$1" - jq -R -r 'fromjson? // empty | select(.type == "result") | .result // empty' "$output_file" | tail -n 1 + local result_json + if [[ "$ENGINE" == "codex" ]]; then + result_json=$(jq -R -c ' + fromjson? // empty + | select(.type == "item.completed" and .item.type == "agent_message") + | .item.text // empty + ' "$output_file" | tail -n 1) + else + result_json=$(jq -R -c \ + 'fromjson? // empty | select(.type == "result") | .result // empty' \ + "$output_file" | tail -n 1) + fi + [[ -z "$result_json" ]] || jq -r '.' <<< "$result_json" } -# Reduces the transcript to one line per tool call/result/assistant message, -# so a timeout's diagnostics show what Claude was doing without dumping raw -# JSON (system init alone is a multi-KB line). +extract_codex_usage() { + local output_file="$1" + jq -R -c ' + fromjson? // empty + | select(.type == "turn.completed" and (.usage | type) == "object") + | .usage + ' "$output_file" | tail -n 1 +} + +# Summarize transcript events so timeout diagnostics do not include multi-KB JSON lines. summarize_transcript() { local output_file="$1" jq -R -r ' @@ -462,14 +536,17 @@ summarize_transcript() { else empty end elif .type == "system" and .subtype == "init" then "system: session started" elif .type == "result" then "result(" + (.subtype // "") + "): " + ((.result // "") | tostring | .[0:500]) + elif (.type == "item.started" or .type == "item.completed") and .item.type == "command_execution" then + "command(" + (.item.status // "") + "): " + ((.item.command // "") | tostring | .[0:300]) + elif .type == "item.completed" and .item.type == "agent_message" then + "assistant: " + ((.item.text // "") | .[0:500]) + elif .type == "turn.failed" or .type == "error" then + "error: " + ((.error.message // .message // .error // "") | tostring | .[0:500]) else empty end ' "$output_file" } -# Append a summarized tail of the agent transcript to the review file when a -# review times out, so we can see what claude was doing when it got stuck. -# Keeps the snippet bounded so we don't double the size of long transcripts. append_timeout_diagnostics() { local review_file="$1" local output_file="$2" @@ -508,7 +585,7 @@ count_user_reviews() { # comments appended to an already-existing one on a --append re-review, # which reuses the same review id), or, for the rare comment-free review # (e.g. a plain approval body), a first-ever review appearing where none -# existed before. Claude can exit 0 without posting anything, e.g. if it +# existed before. An engine can exit 0 without posting anything, e.g. if it # stalls mid-synthesis waiting on the Copilot meta-review step, so this # guards succeed_review against marking that as done. A stale review from a # prior day must not by itself count as success on a re-review, which is why @@ -524,7 +601,7 @@ review_posted() { local new_comments if ! new_comments=$(gh api "repos/${pr_repo}/pulls/${pr_number}/comments" --paginate 2>/dev/null | jq -s \ "[.[][] | select(.user.login == \"${GITHUB_USER}\" and .created_at > \"${since_iso}\")] | length"); then - log_warn "Could not verify GitHub review state for PR #${pr_number}; trusting Claude's exit code" + log_warn "Could not verify GitHub review state for PR #${pr_number}; trusting the engine exit code" return 0 fi if [[ "$new_comments" -gt 0 ]]; then @@ -533,7 +610,7 @@ review_posted() { local review_count if ! review_count=$(count_user_reviews "$pr_repo" "$pr_number"); then - log_warn "Could not verify GitHub review state for PR #${pr_number}; trusting Claude's exit code" + log_warn "Could not verify GitHub review state for PR #${pr_number}; trusting the engine exit code" return 0 fi [[ "$had_prior_review" == "false" && "$review_count" -gt 0 ]] @@ -559,19 +636,24 @@ run_review() { local repo_name repo_name=$(echo "$pr_repo" | tr '/' '-') local review_dir="${REVIEWS_DIR}/${repo_name}" - local review_file="${review_dir}/pr-${pr_number}-$(date +%Y%m%d).md" + local review_file + review_file="${review_dir}/pr-${pr_number}-$(date +%Y%m%d).md" mkdir -p "$review_dir" # --append targets the skill's persistent per-PR file, which is separate # from the date-stamped file this orchestrator writes. - local prompt="/review-code ${pr_url} --force --draft" + local prompt="${SKILL_COMMAND} ${pr_url} --force --draft" if review_exists "$pr_number" "$pr_repo"; then prompt+=" --append" log_info "Existing review file detected, will append" fi if [[ "$DRY_RUN" == "true" ]]; then - log_info "[DRY RUN] Would run: claude -p \"${prompt}\"" + if [[ "$ENGINE" == "codex" ]]; then + log_info "[DRY RUN] Would run: codex exec \"${prompt}\"" + else + log_info "[DRY RUN] Would run: claude -p \"${prompt}\"" + fi log_info "[DRY RUN] Output would be saved to: ${review_file}" return 0 fi @@ -600,20 +682,38 @@ run_review() { prior_review_count=$(count_user_reviews "$pr_repo" "$pr_number") || prior_review_count=0 [[ "${prior_review_count:-0}" -gt 0 ]] && had_prior_review="true" - # caffeinate -i prevents idle sleep so the timeout measures real wall-clock - # time. timeout --kill-after fires SIGKILL if claude ignores SIGTERM, so a - # stuck review cannot run for hours. --output-format stream-json (requires - # --verbose) streams each turn to output_file as it happens, instead of - # buffering everything until the process exits, so a killed run still has - # something on disk to diagnose (see summarize_transcript above). + # caffeinate prevents idle sleep so timeout measures real wall-clock time. + # Both engines stream JSONL to output_file for timeout diagnostics. local exit_code=0 local output_file output_file=$(mktemp) - start_heartbeat 30 "Claude reviewing PR #${pr_number}" + local review_command=() + if [[ "$ENGINE" == "codex" ]]; then + local review_skill_dir="${HOME}/.agents/skills/review-code" + mkdir -p "$review_skill_dir/.sessions" "$review_skill_dir/.worktrees" \ + "$review_skill_dir/.reviews" + review_command=( + env -u CLAUDECODE -u CLAUDE_CONFIG_DIR codex exec + --json + --ephemeral + --sandbox workspace-write + --approve-for-me + --skip-git-repo-check + --add-dir "$review_skill_dir/.sessions" + --add-dir "$review_skill_dir/.worktrees" + --add-dir "$review_skill_dir/.reviews" + --add-dir "$review_dir" + -C "$review_dir" + "$prompt" + ) + else + review_command=(claude -p "$prompt" --output-format stream-json --verbose) + fi + start_heartbeat 30 "${ENGINE_LABEL} reviewing PR #${pr_number}" set +o pipefail caffeinate -i timeout --kill-after="$REVIEW_KILL_AFTER_SECONDS" \ "$REVIEW_TIMEOUT_SECONDS" \ - claude -p "$prompt" --output-format stream-json --verbose 2>&1 | tee "$output_file" || true + "${review_command[@]}" 2>&1 | tee "$output_file" || true exit_code=${PIPESTATUS[0]} set -o pipefail stop_heartbeat @@ -622,35 +722,29 @@ run_review() { end_time=$(date +%s) local duration=$((end_time - start_time)) - # Append Claude's final answer (matching what --output-format text used to - # print directly) to the review file. local result_text result_text=$(extract_result_text "$output_file") if [[ -n "$result_text" ]]; then echo "$result_text" >> "$review_file" fi - # Check whether Claude itself ran out of quota. Subscription billing prints - # "Claude AI usage limit reached|"; API billing surfaces - # rate_limit_error / 429. Either way the session should stop: every further - # review would fail identically until the limit window resets. + # Stop the session when the engine is out of quota. local rate_limited=false - if grep -qiE "usage limit reached|rate_limit_error|overloaded_error" "$output_file"; then + if [[ "$ENGINE" == "codex" ]]; then + if [[ $exit_code -ne 0 ]] && \ + grep -qiE "usage limit reached|hit (your )?usage limit|rate[ _-]?limit|overloaded_error|too many requests|status[^0-9]*429" "$output_file"; then + rate_limited=true + fi + elif grep -qiE "usage limit reached|rate_limit_error|overloaded_error" "$output_file"; then rate_limited=true fi - # Check whether claude itself failed to authenticate (OAuth session expired - # and refresh failed). Every further review would fail identically until - # the session is refreshed, so treat this like rate limiting rather than a - # PR-specific problem. Gated on a non-zero exit code so a successful review - # that merely discusses authentication in the PR's diff or commentary can't - # be misread as a failure. + # Require a nonzero exit so review prose about authentication is not misread. local auth_failed=false - if [[ $exit_code -ne 0 ]] && grep -qiE "OAuth session expired|failed to authenticate" "$output_file"; then + if [[ $exit_code -ne 0 ]] && grep -qiE "OAuth session expired|failed to authenticate|authentication failed|login required|not logged in|unauthorized|status[^0-9]*401" "$output_file"; then auth_failed=true fi - # Append status footer { echo "" echo "---" @@ -661,70 +755,73 @@ run_review() { echo "- **Exit code:** ${exit_code}" } >> "$review_file" + if [[ "$ENGINE" == "codex" ]]; then + local usage_json + usage_json=$(extract_codex_usage "$output_file") + if [[ -n "$usage_json" ]]; then + local usage_summary + usage_summary=$(jq -r '"input \(.input_tokens // 0), cached input \(.cached_input_tokens // 0), output \(.output_tokens // 0)"' <<< "$usage_json") + echo "- **Codex orchestrator usage:** ${usage_summary}" >> "$review_file" + fi + fi + + local review_result=1 if [[ "$rate_limited" == "true" ]]; then { - echo "- **Status:** ⚠️ INCOMPLETE — Claude usage limit reached" + echo "- **Status:** ⚠️ INCOMPLETE — ${ENGINE_LABEL} usage limit reached" echo "" - echo "> **Note:** This review failed because Claude itself was rate limited, not because of the PR. It will be retried on a later run." + echo "> **Note:** This review failed because ${ENGINE_LABEL} was rate limited, not because of the PR. It will be retried on a later run." } >> "$review_file" - log_warn "Claude usage limit reached during PR #${pr_number}; stopping session" - log_info "Review saved to: ${review_file}" + log_warn "${ENGINE_LABEL} usage limit reached during PR #${pr_number}; stopping session" # Not the PR's fault: record the failure in the session for visibility, # but skip the persistent ledger so the PR isn't quarantined. mark_failed "$pr_url" "rate_limited" RATE_LIMITED=true - rm -f "$output_file" - return 1 elif [[ "$auth_failed" == "true" ]]; then { - echo "- **Status:** ⚠️ INCOMPLETE — Claude authentication failed" + echo "- **Status:** ⚠️ INCOMPLETE — ${ENGINE_LABEL} authentication failed" echo "" - echo "> **Note:** This review failed because Claude's OAuth session expired, not because of the PR. It will be retried once authentication is restored." + echo "> **Note:** This review failed because ${ENGINE_LABEL} authentication expired, not because of the PR. It will be retried once authentication is restored." } >> "$review_file" - log_warn "Claude authentication failed during PR #${pr_number}; stopping session" - log_info "Review saved to: ${review_file}" + log_warn "${ENGINE_LABEL} authentication failed during PR #${pr_number}; stopping session" # Not the PR's fault: record the failure in the session for visibility, # but skip the persistent ledger so the PR isn't quarantined. mark_failed "$pr_url" "auth_failed" AUTH_FAILED=true - rm -f "$output_file" - return 1 elif [[ $exit_code -eq 0 ]] && review_posted "$pr_repo" "$pr_number" "$start_iso" "$had_prior_review"; then echo "- **Status:** ✅ Complete" >> "$review_file" log_success "Review complete for PR #${pr_number} (${duration}s)" - log_info "Review saved to: ${review_file}" succeed_review "$pr_url" "$review_file" - rm -f "$output_file" - return 0 + review_result=0 elif [[ $exit_code -eq 0 ]]; then - echo "- **Status:** ⚠️ INCOMPLETE — Claude exited cleanly but posted nothing to GitHub" >> "$review_file" + echo "- **Status:** ⚠️ INCOMPLETE — ${ENGINE_LABEL} exited cleanly but posted nothing to GitHub" >> "$review_file" log_error "Review for PR #${pr_number} exited 0 but left no review on GitHub" - log_info "Review saved to: ${review_file}" fail_review "$pr_url" "no_review_posted" - rm -f "$output_file" - return 1 elif [[ $exit_code -eq 124 || $exit_code -eq 137 ]]; then # 124 = SIGTERM-and-exit; 137 = SIGKILL via --kill-after. append_timeout_diagnostics "$review_file" "$output_file" "$exit_code" log_error "Review timed out for PR #${pr_number}" - log_info "Review saved to: ${review_file}" fail_review "$pr_url" "timeout" - rm -f "$output_file" - return 1 else echo "- **Status:** ❌ Failed (exit code: ${exit_code})" >> "$review_file" log_error "Review failed for PR #${pr_number} (exit code: ${exit_code})" - log_info "Review saved to: ${review_file}" fail_review "$pr_url" "exit_code_${exit_code}" - rm -f "$output_file" - return 1 fi + + log_info "Review saved to: ${review_file}" + rm -f "$output_file" + return "$review_result" } # Main execution main() { trap 'stop_heartbeat; save_session; save_failures' EXIT + if daily_limit_reached; then + log_warn "Daily review-attempt limit reached ($(daily_attempt_count)/${DAILY_MAX_PRS}); stopping" + exit 0 + fi + check_prerequisites SESSION_START_TIME=$(date +%s) @@ -732,6 +829,9 @@ main() { local max_prs_desc="${MAX_PRS}" [[ "$MAX_PRS" -eq 0 ]] && max_prs_desc="no cap (session budget governs)" log_info "Max PRs: ${max_prs_desc}" + if [[ "$DAILY_MAX_PRS" -gt 0 ]]; then + log_info "Daily max attempts: ${DAILY_MAX_PRS} ($(daily_attempt_count) used)" + fi if [[ "$DRY_RUN" == "true" ]]; then log_warn "Running in DRY RUN mode - no reviews will be executed" @@ -822,14 +922,19 @@ main() { ((started++)) || true if [[ "$RATE_LIMITED" == "true" ]]; then - log_warn "Stopping session: Claude usage limit reached" - mark_error "claude usage limit reached" + log_warn "Stopping session: ${ENGINE_LABEL} usage limit reached" + mark_error "${ENGINE} usage limit reached" break fi if [[ "$AUTH_FAILED" == "true" ]]; then - log_warn "Stopping session: Claude authentication failed" - mark_error "claude authentication failed" + log_warn "Stopping session: ${ENGINE_LABEL} authentication failed" + mark_error "${ENGINE} authentication failed" + break + fi + + if daily_limit_reached; then + log_info "Reached --daily-max-prs limit of ${DAILY_MAX_PRS}; stopping" break fi diff --git a/macos/LaunchAgents/com.haacked.review-all-prs.plist b/macos/LaunchAgents/com.haacked.review-all-prs.plist index 1f59f1e..de95b2c 100644 --- a/macos/LaunchAgents/com.haacked.review-all-prs.plist +++ b/macos/LaunchAgents/com.haacked.review-all-prs.plist @@ -18,29 +18,40 @@ log_dir="$HOME/.local/state/review-all-prs" mkdir -p "$log_dir" -# Run the review script with logging. No --max-prs cap: the session time -# budget and Claude usage-limit detection decide when each tick stops. -# --all widens discovery beyond review-requested PRs to every open PR -# authored by a team-feature-flags member; it does not re-review PRs -# already reviewed with no new commits since. -exec "$HOME/.dotfiles/bin/run-pr-reviews.sh" --auto \ - --team team-feature-flags --priority-team team-feature-flags --all \ - >> "$log_dir/launchd.log" 2>&1 +# Run the review script with logging. Arguments remain separate ProgramArguments +# below so the service wrapper and this job can be compared directly. +exec "$HOME/.dotfiles/bin/run-pr-reviews.sh" "$@" >> "$log_dir/launchd.log" 2>&1 ]]> + review-all-prs + --auto + --engine + codex + --team + team-feature-flags + --author-team + team-feature-flags + --priority-team + team-feature-flags + --all + --max-prs + 1 + --daily-max-prs + 2