Skip to content
Merged
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
14 changes: 14 additions & 0 deletions .github/workflows/claude-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,29 @@ jobs:
# serializes the double-fire; the marker probe catches the second run.
# (The marker, not just any claude[bot] activity: the fix agent posts
# claude[bot] thread replies too, which must not count as "reviewed".)
#
# EXCEPTION — deep-review `labeled` events skip the probe: a triage+
# human re-applying the label (verified live by the perm step above)
# is an explicit "run a fresh review" request, e.g. after new commits
# or a prompt upgrade. Usage stays human-metered: one opus session per
# deliberate label application. The ai-loop path and label-at-creation
# double-fires keep the one-review invariant.
- name: Skip if already reviewed
id: dedupe
if: steps.perm.outputs.allowed == 'true'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
REPO: ${{ github.repository }}
PR: ${{ github.event.pull_request.number }}
ACTION: ${{ github.event.action }}
LABEL: ${{ github.event.label.name }}
run: |
set -euo pipefail
if [ "$ACTION" = "labeled" ] && [ "$LABEL" = "deep-review" ]; then
echo "skip=false" >> "$GITHUB_OUTPUT"
echo "deep-review label applied by a verified triage+ user — running a fresh review"
exit 0
fi
Comment on lines +98 to +120

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the trigger/concurrency config and the (unchanged) header comment
# referenced elsewhere as lines 10-17, to confirm the double-fire premise.
sed -n '1,45p' .github/workflows/claude-review.yml

Repository: allxsmith/bestax

Length of output: 2068


🏁 Script executed:

#!/bin/bash
set -euo pipefail
nl -ba .github/workflows/claude-review.yml | sed -n '45,170p'

Repository: allxsmith/bestax

Length of output: 194


🏁 Script executed:

#!/bin/bash
set -euo pipefail
awk 'NR>=45 && NR<=170 { printf "%4d %s\n", NR, $0 }' .github/workflows/claude-review.yml

Repository: allxsmith/bestax

Length of output: 6701


deep-review relabel bypass breaks the one-review invariant

  • The ACTION == 'labeled' && LABEL == 'deep-review' fast path skips the marker probe, so a PR created with deep-review already attached still fires both opened and labeled and runs Claude twice. Keep the probe here, or gate this bypass on a signal that distinguishes a deliberate re-label from the initial label-at-creation event.
  • The surrounding comment about the marker capping reviews at one per PR is now inaccurate and should be updated too.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/claude-review.yml around lines 98 - 120, The deduplication
bypass in the “Skip if already reviewed” step allows initial deep-review label
events to evade the marker probe and run duplicate reviews. Update the labeled
deep-review handling to distinguish deliberate re-labels from label-at-creation
events, preserving the probe for initial labels, and revise the surrounding
one-review-invariant comment to match the corrected behavior.

Source: Learnings

MARKER=$(gh api "repos/$REPO/pulls/$PR/reviews?per_page=100" --jq \
'[.[] | select((.user.login | test("^claude(\\[bot\\])?$"))
and (.body | contains("<!-- claude-deep-review -->")))] | length')
Expand Down
4 changes: 2 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ Issues labeled `claude-fix` (requires triage+ access, verified live) are impleme
fix/verify rounds (cap 4) until CI is green and every AI review thread is resolved. Labels:
`ai-loop` (in the loop), `needs-human-review` (converged or contested — owner reviews and
squash-merges manually; the loop never merges), `ai-loop-paused` (cap/guard hit). A triage+
user can also apply the opt-in `deep-review` label to any PR to run the one-shot Claude
deep review on it (once ever per PR). Kill switches: remove `ai-loop` (per PR) or set repo
user can also apply the opt-in `deep-review` label to any PR to run the Claude deep
review on it (re-applying the label re-runs it). Kill switches: remove `ai-loop` (per PR) or set repo
variable `AI_LOOP_ENABLED=false` (whole system). The `<!-- ai-loop-state … -->` PR comment
is machine-managed — never reformat its first line. The loop refuses PRs that touch
`.github/**` or the jest/commitlint/release/pnpm-workspace configs.
14 changes: 7 additions & 7 deletions docs/docs/guides/getting-started/ai-development.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ the review-time requirements (Storybook story for UI changes, docs page for API

PRs authored by the loop move through a small label lifecycle:

| Label | Where | Meaning |
| -------------------- | ------ | ----------------------------------------------------------------------------------------------------- |
| `claude-fix` | issues | Maintainer-approved: Claude implements this issue and opens a PR |
| `ai-loop` | PRs | The PR is inside the autonomous review/fix loop |
| `needs-human-review` | PRs | The loop converged (or hit a disagreement) — awaiting maintainer review/merge |
| `ai-loop-paused` | PRs | The loop hit its iteration cap or a guardrail — a maintainer must intervene |
| `deep-review` | PRs | Opt-in: a triage+ user applies it to run the one-shot Claude deep review on any PR (once ever per PR) |
| Label | Where | Meaning |
| -------------------- | ------ | ---------------------------------------------------------------------------------------------- |
| `claude-fix` | issues | Maintainer-approved: Claude implements this issue and opens a PR |
| `ai-loop` | PRs | The PR is inside the autonomous review/fix loop |
| `needs-human-review` | PRs | The loop converged (or hit a disagreement) — awaiting maintainer review/merge |
| `ai-loop-paused` | PRs | The loop hit its iteration cap or a guardrail — a maintainer must intervene |
| `deep-review` | PRs | Opt-in: a triage+ user applies it to run the Claude deep review on any PR (re-apply to re-run) |

The loop itself: Claude implements the issue and opens the PR → CodeRabbit reviews it and a
second, independent Claude review (a stronger model than the implementer) does a deep pass →
Expand Down
Loading