Make the quick-approve button PR-state aware (v1.2.0) - #8
Conversation
…ed, bump 1.2.0 The button no longer shows uniformly on every PR: - merged or closed PR: no widget at all - already approved by me: a passive dismissible 'Already approved' indicator instead of the button, preventing accidental double approves - open and unreviewed: the button, unchanged State detection is layered and fails open: approvals made through the button (persisted in localStorage across sessions), then the live page's state badge and 'approved these changes' sidebar/timeline entries, then one cached fetch of the conversation page when on other tabs. Detection logs one 'pr state' line for tuning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JL2YxajmCPuDRWyejSpCJM
…ump 1.3.0 Inspected a real pr page through the session proxy and matched the detection to what github actually renders: - merged/closed via [data-component=StateLabel][data-status] (react header markup), with the legacy class/title/text checks as fallback - approved-by-me via the reviewers sidebar entry <a id=review-status-<login>> containing the octicon-check icon, then the reviewer tooltips / timeline 'approved these changes' text - approvals are no longer persisted in localStorage: the in-memory set covers the current session and the page/fetch detection recognizes the approval on later visits Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JL2YxajmCPuDRWyejSpCJM
The text layer matched '<me> ... approved these changes' across the combined text of sidebar/timeline containers, so a pr listing me as a requested reviewer next to someone else's approval read as approved by me. Detection now trusts only per-element signals: my own review-status sidebar entry with the check icon, and tooltips/timeline entries whose own short text STARTS with '<me> approved these changes' - another reviewer's approval can no longer cross-match. The pr-state log now also names the exact signal (icon or matched text) for one-paste diagnosis. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JL2YxajmCPuDRWyejSpCJM
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JL2YxajmCPuDRWyejSpCJM
There was a problem hiding this comment.
🟡 Changes recommended
The new “Already approved” indicator is implemented as a clickable <div> (keyboard accessibility issue) and there’s an inaccurate comment about fetch tracking that should be corrected.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates github-pr-approve-helper.user.js to make the floating “✅ Quick approve” UI PR-state aware, hiding it on merged/closed PRs and replacing it with a dismissible “👍 Already approved” indicator when the current user has already approved.
Changes:
- Adds PR-state detection (merged/closed/approved/open) using live DOM signals plus a cached conversation-page fetch fallback.
- Introduces an “Already approved” indicator and hands over to it after a successful quick-approve.
- Updates versioning to v1.2.0 and adjusts PR URL building helpers.
File summaries
| File | Description |
|---|---|
| github-pr-approve-helper.user.js | Adds PR-state detection + new “Already approved” indicator behavior for the quick-approve widget |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Github forbids approving your own pull request, so the button was misleading there. The state detection now recognizes the pr author - the rel=author header link on classic pages (verified: exactly one per page) or the react header's '<author> wants to merge' line - and renders no widget on own prs, in the live page and the fetched conversation page alike. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JL2YxajmCPuDRWyejSpCJM
…comment Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JL2YxajmCPuDRWyejSpCJM
There was a problem hiding this comment.
🟡 Changes recommended
Caching the 'open' state can make the UI state sticky and prevent later detection of approvals/merges within the same session, enabling accidental double-approves.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
github-pr-approve-helper.user.js:681
- The conversation-page fetch path caches
'open'(including in the.catch) the same way as the live-path does. This can freeze the display state as "open" for the rest of the session after a transient fetch/parse miss, preventing later live detection of an approval/merge and allowing accidental double-approves.
const state = detection?.state ?? 'open';
prStateCache.set(key, state);
console.log(
`[GitHub PR Approve Helper] pr state: ${state} (conversation page${
detection ? `, via ${detection.via}` : ''
})`,
);
scheduleScan();
})
.catch(() => prStateCache.set(key, 'open'));
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
| // the conversation tab shows every signal - nothing found means open | ||
| if (location.pathname === prPagePath(pr)) { | ||
| prStateCache.set(key, 'open'); | ||
| return 'open'; | ||
| } |
| // per-pr dismissal of the indicator, so a click hides it until the next | ||
| // navigation to a different pr | ||
| let indicatorDismissedFor = null; |
What
The floating
✅ Quick approvebutton ingithub-pr-approve-helper.user.js(v1.1.0 → v1.2.0) no longer shows uniformly on every linear-b PR:After a quick-approve, the
🎉 Approvedfeedback hands over to the indicator within the session.How — state detection without tokens or storage
Layered, cheapest first, cached per PR, always failing open (never hides the button on a PR that isn't confirmed approved/merged):
localStorage— deliberately dropped)[data-component="StateLabel"][data-status="pullMerged|pullClosed"], with legacy.Stateclass/title/text fallbacks<a id="review-status-<login>">containing theocticon-checkverdict icon (username frommeta[name="user-login"])<login> approved these changes— anchored per element, so another reviewer's approval can never cross-match (fixes a false "already approved" seen on a PR that requested my review while someone else had approved)Every detection logs one
pr state: <state> (<source>, via <signal>)console line for one-paste diagnosis of any future markup drift.Testing
Playwright harness (real userscript in a Chromium isolated world, React replica + fake backend): 40/40 checks — all 29 pre-existing quick-approve/dialog behaviors, plus: merged/approved via live markup, both fetch-layer states, indicator dismissal, success-to-indicator handover, and a regression test reproducing the review-requested false positive. Also verified manually on real linear-b PRs.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JL2YxajmCPuDRWyejSpCJM
Generated by Claude Code