WH-086: Set up review hero support

[edmofro]

Summary

Review hero is a system that reviews PRs and posts comments.

The way review hero is triggered is by checking a tickbox on the PR. We need a way to set that off. That part would be called "run review hero", and be a button on the workhorse side that mapped to the checkbox on the PR side. When it was checked on the PR side, the button should be disabled. When it is unchecked, including not present, (and we can poll for that) it should be enabled. Clicking it should edit the PR description to check the button.

Review Hero

• Run Review Hero
• Auto-fix review suggestions
• Auto-fix CI failures

[review-hero]

🦸 Review Hero Summary
12 agents reviewed this PR | 0 critical | 4 suggestions | 1 nitpick | Filtering: consensus 3 voters, 5 below threshold

Below consensus threshold (5 unique issues not confirmed by majority)

Location	Agent	Severity	Comment
src/app/api/trigger-review-hero/route.ts:8	Bugs & Correctness	suggestion	CHECKBOX_CHECKED only matches lowercase [x], but GitHub's markdown spec also accepts [X] as a checked checkbox (and users can write it when editing raw markdown). If the box is checked with upperca...
src/components/card/PrSection.tsx:178	Frontend Snappiness	suggestion	ReviewHeroRow is guarded only by !s.isMerged, so it renders for cards that have no PR yet (where isMerged is false but prNumber is null). Clicking 'Run Review Hero' in that state fires a POST tha...
src/lib/auth/session.ts:98	Security	suggestion	requireCardAccess receives userId but the Prisma query ignores it — it looks up the card by cardId alone with no user filter. The only authz enforcement is the downstream hasProjectAccess c...
src/lib/git/githubClient.ts:444	Security	suggestion	The owner and repo values are interpolated directly into the GitHub API URL without encodeURIComponent(). If either field contains /, @, or %-encoded characters (e.g. from a malformed p...
src/lib/hooks/usePrSection.ts:213	Frontend Snappiness	suggestion	reviewHeroChecked defaults to false before status loads (because status?.reviewHeroChecked ?? false). If the checkbox is already checked on the server, the button flashes from enabled → dis...

Nitpicks

File	Line	Agent	Comment
src/app/api/card-branch-status/route.ts	155	Design & Architecture	The regex /- \[x\] .+/ is written inline here and also defined as CHECKBOX_CHECKED in trigger-review-hero/route.ts. Both routes parsing the same checkbox format means the pattern is split across two files with no shared source. Extract it (and the marker string AI_REVIEW_MARKER...

Local fix prompt (copy to your coding agent)

Fix these issues identified on the pull request. One commit per issue fixed.

---

src/lib/git/githubClient.ts:416: getPrBody calls GET /repos/{owner}/{repo}/pulls/{prNumber} — the exact same endpoint getPullRequest already fetches, which now returns body. This adds a second fetch path for identical data. The trigger-review-hero route could call getPullRequest instead and getPrBody could be removed. If a lighter call is preferred long-term, that's a future optimisation; right now it's only called from one place and adds unnecessary surface area.

---

src/lib/hooks/usePrSection.ts:220: reviewHeroOptimistic is set to true on trigger and never cleared in the success path — the useEffect only resets it when the server returns false. Once the server confirms checked=true, reviewHeroOptimistic is permanently true dead state until unmount. A simpler model: drop the optimistic flag entirely and just use reviewHeroTriggering to disable the button during the in-flight request, then rely on the invalidateBranchStatus() poll for the confirmed state. The current setup adds a two-variable state machine to solve a problem one variable already handles.

---

src/app/api/card-branch-status/route.ts:155: The regex /- [x] .+<!-- #ai-review -->/ is written inline here and also defined as CHECKBOX_CHECKED in trigger-review-hero/route.ts. Both routes parsing the same checkbox format means the pattern is split across two files with no shared source. Extract it (and the marker string AI_REVIEW_MARKER) to githubClient.ts or a small shared module so a format change only needs one edit.

---

src/lib/hooks/usePrSection.ts:243: This useEffect resets reviewHeroOptimistic whenever the server returns reviewHeroChecked: false. There is a race window after success: setReviewHeroTriggering(false) fires in finally before the invalidateBranchStatus() refetch resolves. If a background polling interval returns stale false data during that gap, reviewHeroOptimistic gets cleared and the button briefly re-enables — a visible flash. Fix: guard the reset with && !reviewHeroTriggering (add it to the effect deps too), so the effect only clears the optimistic state when no mutation is in-flight.

---

src/components/card/PrSection.tsx:354: The button shows 'Review Hero' (disabled, greyed out) both when triggering=true AND when checked=true, making them visually identical. During the async fetch the user has no feedback that anything is happening — the button looks already-done rather than in-progress. Replace the static Shield icon with Loader2 (already imported at the top of this file) when triggering=true, and use a distinct label like 'Running…' to distinguish the loading state from the settled state. Fix: {triggering ? <Loader2 size={11} className="animate-spin" /> : <Shield size={11} />} {triggering ? 'Running…' : checked ? 'Review Hero' : 'Run Review Hero'}

[review-hero]

[Bugs & Correctness] suggestion

After a successful trigger, invalidateBranchStatus() fires a background refetch (line 293), then finally immediately sets setReviewHeroTriggering(false) (line 297). Between those two events there's a render where reviewHeroTriggering is false but status?.reviewHeroChecked hasn't updated yet, so reviewHeroChecked computes to false and the button briefly re-enables. A fast double-click could send a second POST. Fix: add a separate reviewHeroLocallyTriggered boolean that's set to true on success and never reset, so the button stays disabled until status.reviewHeroChecked confirms it: const reviewHeroChecked = reviewHeroLocallyTriggered || reviewHeroTriggering || (status?.reviewHeroChecked ?? false).

[review-hero]

[Frontend Snappiness] suggestion

No loading indicator while the trigger is in-flight. When triggering=true the button shows the same static disabled appearance as when checked=true — there is zero visual feedback that the click registered. Loader2 is already imported in this file but unused in ReviewHeroRow. Add a conditional: triggering ? <Loader2 size={11} className='animate-spin' /> : <Shield size={11} /> and change the label to e.g. 'Triggering…' so users know something is happening rather than wondering if their click failed.

[review-hero]

[Security] suggestion

REVIEW_HERO_CHECKED and REVIEW_HERO_UNCHECKED lack line anchors and the multiline flag. They match anywhere in the PR body, so anyone with write access to the PR description could add a spoofed '- [x] fake ' line anywhere (e.g., in a code block or comment) to make reviewHeroChecked return true and hide the Run Review Hero button. Fix: use /^- [x] [^\n]+/m (and same for UNCHECKED) so the pattern only matches at an actual line boundary.

[review-hero]

🦸 Review Hero Summary
9 agents reviewed this PR | 0 critical | 3 suggestions | 0 nitpicks | Filtering: consensus 3 voters, 1 below threshold

Below consensus threshold (1 unique issue not confirmed by majority)

Location	Agent	Severity	Comment
src/components/card/PrSection.tsx:178	Frontend Snappiness	suggestion	ReviewHeroRow renders during initial status load with checked=false (the ?? false fallback in usePrSection line 278). If the PR already has Review Hero triggered, users briefly see an active 'Run R...

Local fix prompt (copy to your coding agent)

Fix these issues identified on the pull request. One commit per issue fixed.

---

src/lib/hooks/usePrSection.ts:297: After a successful trigger, invalidateBranchStatus() fires a background refetch (line 293), then finally immediately sets setReviewHeroTriggering(false) (line 297). Between those two events there's a render where reviewHeroTriggering is false but status?.reviewHeroChecked hasn't updated yet, so reviewHeroChecked computes to false and the button briefly re-enables. A fast double-click could send a second POST. Fix: add a separate reviewHeroLocallyTriggered boolean that's set to true on success and never reset, so the button stays disabled until status.reviewHeroChecked confirms it: const reviewHeroChecked = reviewHeroLocallyTriggered || reviewHeroTriggering || (status?.reviewHeroChecked ?? false).

---

src/components/card/PrSection.tsx:356: No loading indicator while the trigger is in-flight. When triggering=true the button shows the same static disabled appearance as when checked=true — there is zero visual feedback that the click registered. Loader2 is already imported in this file but unused in ReviewHeroRow. Add a conditional: triggering ? <Loader2 size={11} className='animate-spin' /> : <Shield size={11} /> and change the label to e.g. 'Triggering…' so users know something is happening rather than wondering if their click failed.

---

src/lib/git/githubClient.ts:416: REVIEW_HERO_CHECKED and REVIEW_HERO_UNCHECKED lack line anchors and the multiline flag. They match anywhere in the PR body, so anyone with write access to the PR description could add a spoofed '- [x] fake <!-- #ai-review -->' line anywhere (e.g., in a code block or comment) to make reviewHeroChecked return true and hide the Run Review Hero button. Fix: use /^- [x] [^\n]+<!-- #ai-review -->/m (and same for UNCHECKED) so the pattern only matches at an actual line boundary.

[edmofro]

src/lib/hooks/usePrSection.ts:297: After a successful trigger, invalidateBranchStatus() fires a background refetch (line 293), then finally immediately sets setReviewHeroTriggering(false) (line 297). Between those two events there's a render where reviewHeroTriggering is false but status?.reviewHeroChecked hasn't updated yet, so reviewHeroChecked computes to false and the button briefly re-enables. A fast double-click could send a second POST. Fix: add a separate reviewHeroLocallyTriggered boolean that's set to true on success and never reset, so the button stays disabled until status.reviewHeroChecked confirms it: const reviewHeroChecked = reviewHeroLocallyTriggered || reviewHeroTriggering || (status?.reviewHeroChecked ?? false).

src/components/card/PrSection.tsx:356: No loading indicator while the trigger is in-flight. When triggering=true the button shows the same static disabled appearance as when checked=true — there is zero visual feedback that the click registered. Loader2 is already imported in this file but unused in ReviewHeroRow. Add a conditional: triggering ? <Loader2 size={11} className='animate-spin' /> : <Shield size={11} /> and change the label to e.g. 'Triggering…' so users know something is happening rather than wondering if their click failed.

[edmofro]

REVIEW_HERO_CHECKED and REVIEW_HERO_UNCHECKED lack line anchors and the multiline flag. They match anywhere in the PR body, so anyone with write access to the PR description could add a spoofed '- [x] fake ' line anywhere (e.g., in a code block or comment) to make reviewHeroChecked return true and hide the Run Review Hero button. Fix: use /^- [x] [^\n]+/m (and same for UNCHECKED) so the pattern only matches at an actual line boundary.