Skip to content

Commit 67d19f7

Browse files
ghackettclaude
andauthored
The composer doesn't offer a verdict on your own pull request (#307)
GitHub refuses a review of a pull request you opened yourself, so the PR page's **Approve** and **Request changes** buttons could only ever come back as an error on your own PR — a button whose one possible answer is a refusal. They now sit out a PR the signed-in account authored, the same way they already sit out a merged or closed one. **Comment** — the half that is always yours to do — stays, and so does the Claude button beside it. The PR context menus are untouched: they never offered a verdict in the first place. ## Your own pull request | Before | After | | --- | --- | | <img src="/web-api/iframe-proxy?url=https://github.comhttps://media.githubusercontent.com/media/episode6/screenshots/main/collins/no-verdicts-on-your-own-pr/composer-before-20260813-224600.png" width="380" alt="Composer with Address comments, Request changes, Approve and Comment" /> | <img src="/web-api/iframe-proxy?url=https://github.comhttps://media.githubusercontent.com/media/episode6/screenshots/main/collins/no-verdicts-on-your-own-pr/composer-after-20260813-224600.png" width="380" alt="Composer with Address comments and Comment only" /> | ## Somebody else's pull request, unchanged The gate is authorship, not a blanket removal — the verdicts are still there on a PR you didn't open (anthropics/claude-code#86537, fetched live): <img src="/web-api/iframe-proxy?url=https://github.comhttps://media.githubusercontent.com/media/episode6/screenshots/main/collins/no-verdicts-on-your-own-pr/composer-someone-elses-pr-20260813-224600.png" width="380" alt="Composer on someone else's PR, still offering Request changes and Approve" /> ## How it knows - `prstatus.viewer_login()` asks `gh api user` for the login gh is signed in as, the first time a PR page loads, and remembers it for the run — the account doesn't change under a running app. A failure isn't remembered: offline or signed out is a state that gets better. - `prdetail` compares that login with the PR's author (case-insensitively, since gh spells an author back the way the account was registered) and carries the answer as `PullRequestDetail.viewer_is_author`; `parse_detail` takes the login as an argument, so it stays pure and fixture-testable. - The composer's `sync` draws the two verdicts only for a live PR that somebody else opened. - Every unanswerable case — no gh, offline, an author gh didn't name — reads as somebody else's PR, which leaves the page exactly as it was before anyone asked. ## Full window | Before | After | | --- | --- | | <img src="/web-api/iframe-proxy?url=https://github.comhttps://media.githubusercontent.com/media/episode6/screenshots/main/collins/no-verdicts-on-your-own-pr/window-before-20260813-224600.png" width="440" alt="Collins window, PR panel with all four composer buttons" /> | <img src="/web-api/iframe-proxy?url=https://github.comhttps://media.githubusercontent.com/media/episode6/screenshots/main/collins/no-verdicts-on-your-own-pr/window-after-20260813-224600.png" width="440" alt="Collins window, PR panel without the verdict buttons" /> | ## Testing - `pytest` — 1749 passed (7 new: the authorship compare and its unanswerable cases, the fetch handing the login through and surviving without it, and `viewer_login`'s ask-once/don't-remember-a-failure/no-gh behavior). - `ruff check collins tests` — clean. - Captured headlessly against real GitHub data: PR 305 (mine, open) before and after, and a live PR from another author to confirm the verdicts survive there. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01EfA4T1qTZ8MV7SiJW99FTf --------- Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
1 parent 4767390 commit 67d19f7

5 files changed

Lines changed: 180 additions & 21 deletions

File tree

‎collins/prdetail.py‎

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
timeline, checks, per-file diffs — fetched only when a view asks for it, never
99
polled, and never persisted (a diff must not end up in state.json).
1010
11-
A load is three `gh` calls, all through prstatus's transport so the URL gate,
11+
A load is three `gh` calls (four on the first load of a run, which also asks
12+
who the signed-in user is), all through prstatus's transport so the URL gate,
1213
timeouts, argv-only policy and missing-gh latch stay in one place: one
1314
``gh pr view --json`` with the full field list, one paginated ``gh api
1415
graphql`` for the review threads (the CLI's --json surface has no thread
@@ -200,6 +201,13 @@ class PullRequestDetail:
200201
timeline: tuple[PrComment | PrReview | PrThread, ...]
201202
files: tuple[PrFile, ...]
202203
threads: tuple[PrThread, ...] = ()
204+
# Whether the account gh is signed in as is the one who opened this PR.
205+
# What takes the review verdicts off the page (see prview's composer):
206+
# GitHub refuses an approval of your own pull request, so a button
207+
# offering one could only ever come back as an error. False whenever the
208+
# question can't be answered — no gh, offline, an author gh didn't name —
209+
# which leaves the page exactly as it was before anyone asked.
210+
viewer_is_author: bool = False
203211

204212

205213
def fetch(url: str) -> PullRequestDetail | None:
@@ -209,10 +217,11 @@ def fetch(url: str) -> PullRequestDetail | None:
209217
gh, offline — and the caller keeps showing what it has (stale beats blank
210218
here too). A failed or over-cap diff is *not* a failure (the files arrive
211219
stat-only, patches None), and neither is a failed thread fetch (the
212-
conversation arrives threadless). The reply is folded into the summary
213-
cache on the way through (`prstatus.absorb`), so the chip and mark update
214-
with the view. Never call on the main thread — this waits on gh three
215-
times.
220+
conversation arrives threadless) or an unanswerable "who am I" (the PR
221+
reads as somebody else's, which is what the page already assumed). The
222+
reply is folded into the summary cache on the way through
223+
(`prstatus.absorb`), so the chip and mark update with the view. Never call
224+
on the main thread — this waits on gh three times.
216225
"""
217226
if prstatus.repository_for(url) is None:
218227
return None
@@ -227,7 +236,11 @@ def fetch(url: str) -> PullRequestDetail | None:
227236
prstatus.absorb(url, data)
228237
threads = fetch_threads(url)
229238
diff = prstatus.gh_text(["pr", "diff", url], max_bytes=MAX_DIFF_BYTES)
230-
return parse_detail(url, data, diff, threads)
239+
# Who "you" are, so the page knows whether this PR is the user's own. Asked
240+
# once for the whole run (prstatus.viewer_login caches it), so only the
241+
# first load of a session pays for it, and "" — the unanswerable case —
242+
# simply means no PR reads as authored here.
243+
return parse_detail(url, data, diff, threads, viewer=prstatus.viewer_login())
231244

232245

233246
def fetch_threads(url: str) -> tuple[PrThread, ...]:
@@ -279,23 +292,30 @@ def fetch_threads(url: str) -> tuple[PrThread, ...]:
279292

280293

281294
def parse_detail(
282-
url: str, data: dict, diff: str | None, threads: tuple[PrThread, ...] = ()
295+
url: str,
296+
data: dict,
297+
diff: str | None,
298+
threads: tuple[PrThread, ...] = (),
299+
viewer: str = "",
283300
) -> PullRequestDetail | None:
284301
"""One gh view reply (with its diff and threads, if any) as the record the
285302
view renders.
286303
287304
Pure — module state is never touched — so recorded gh output drives it
288-
straight in tests. None only when *url*/*data* can't even identify a PR
289-
(`prstatus.summarize`'s answer).
305+
straight in tests; *viewer* is the signed-in login the caller looked up
306+
(`prstatus.viewer_login`), passed in rather than asked for here so this
307+
stays a function of its arguments. None only when *url*/*data* can't even
308+
identify a PR (`prstatus.summarize`'s answer).
290309
"""
291310
summary = prstatus.summarize(url, data)
292311
if summary is None:
293312
return None
294313
patches = dict(split_unified_diff(diff)) if diff else {}
314+
author = _author(data.get("author"))
295315
return PullRequestDetail(
296316
summary=summary,
297317
body=_text(data.get("body")),
298-
author=_author(data.get("author")),
318+
author=author,
299319
created_at=_line(data.get("createdAt")),
300320
base_ref=_line(data.get("baseRefName")),
301321
head_ref=_line(data.get("headRefName")),
@@ -307,6 +327,9 @@ def parse_detail(
307327
timeline=_timeline(data.get("comments"), data.get("reviews"), threads),
308328
files=_files(data.get("files"), patches),
309329
threads=threads,
330+
# Logins are case-insensitive on GitHub, and gh spells one back the
331+
# way the account was registered rather than the way it was asked for.
332+
viewer_is_author=bool(author) and author.casefold() == viewer.casefold(),
310333
)
311334

312335

‎collins/prstatus.py‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,9 @@
146146
_SWEEP_WORKERS = 8
147147
# How many PRs a row's tooltip spells out before it starts counting them.
148148
_MAX_TOOLTIP_PRS = 8
149+
# The longest a GitHub login can plausibly be: 39 characters for an account,
150+
# plus room for the ``[bot]`` an app posts under. What `viewer_login` keeps.
151+
_MAX_LOGIN = 50
149152

150153
# Only fetch for URLs shaped like a PR page. The URL comes out of a transcript
151154
# — repo content, i.e. untrusted — and lands in an argv, so this also keeps a
@@ -168,6 +171,7 @@
168171
_statuses: dict[str, tuple[float, dict | None]] = {}
169172
_inflight: set[str] = set()
170173
_gh_missing = False # gh isn't on PATH; nothing to retry against this run
174+
_viewer = "" # the signed-in login, once asked for; "" until then (viewer_login)
171175

172176
# What a PR's badge — the small status mark riding its base icon — can say.
173177
# Pure names rather than icon names: which icon and color each one gets is the
@@ -783,6 +787,40 @@ def gh_text(args: list[str], max_bytes: int | None = None) -> str | None:
783787
return result.stdout
784788

785789

790+
def viewer_login() -> str:
791+
"""The GitHub login gh is signed in as — "" when it can't be had.
792+
793+
One ``gh api user`` the first time somebody asks, then that answer for the
794+
rest of the run: the signed-in account doesn't change under a running app,
795+
and the question is asked on every PR page load (whether the PR is the
796+
user's own decides what the page offers to do about it — see
797+
prdetail.PullRequestDetail.viewer_is_author). A failure isn't remembered:
798+
no gh, offline or signed out is a state that gets better, so the next
799+
caller asks again. Never call on the main thread.
800+
801+
The lock is held around the answer, never around the call: two pages
802+
loading cold at the same moment may both ask gh, which costs one spare
803+
subprocess and writes the same login twice. Holding it across a
804+
subprocess to save that would park every other thread that wants a
805+
status behind a network round trip — the worse of the two.
806+
"""
807+
global _viewer
808+
with _lock:
809+
if _viewer:
810+
return _viewer
811+
if _gh_missing:
812+
return ""
813+
data = gh_json(["api", "user"])
814+
login = data.get("login") if isinstance(data, dict) else None
815+
# GitHub's own logins are short; anything else isn't one, and this is the
816+
# value every PR's author is compared against.
817+
if not isinstance(login, str) or not login or len(login) > _MAX_LOGIN:
818+
return ""
819+
with _lock:
820+
_viewer = login
821+
return login
822+
823+
786824
def _entry(data: dict) -> dict:
787825
"""A gh reply reduced to the CLI cache's `{state, checks}` shape, plus the
788826
title and mergeability — which that cache has no room for and the chips'

‎collins/prview.py‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@
2929
types the COMMENTS prompt into the owning session ("Address comments", while
3030
someone is waiting on a reply) or asks the repository's workflow for a review
3131
("Request review") — the composer is for answering a reviewer yourself, the
32-
button for making the agent do it.
32+
button for making the agent do it. The verdicts sit out a pull request the
33+
signed-in account opened, which GitHub won't let anyone review their own of.
3334
3435
Review threads render as their own cards (`_ThreadCard`): anchored in the
3536
Conversation timeline by when they started, and again under their file's
@@ -597,7 +598,7 @@ def _rebuild(self) -> None:
597598
empty = Gtk.Label(label=_("No comments yet."), xalign=0.0)
598599
empty.add_css_class("dim-label")
599600
self._content.append(empty)
600-
self._composer.sync(self._pr)
601+
self._composer.sync(self._pr, detail.viewer_is_author)
601602
self._content.append(self._composer)
602603

603604
def _acted(self) -> None:
@@ -1064,6 +1065,12 @@ class _Composer(Gtk.Box):
10641065
stands alone. A failure comes back as gh's own sentence in a dialog, the
10651066
text kept where it was typed; success clears the box and re-reads the PR.
10661067
1068+
The two verdicts are only there to be pressed on somebody else's pull
1069+
request: GitHub won't take a review of your own, so on a PR the signed-in
1070+
account opened (see prdetail's `viewer_is_author`) they aren't drawn at
1071+
all — a button whose only possible answer is a refusal is worse than no
1072+
button. Commenting is the half that is always yours to do.
1073+
10671074
The Claude button beside them is the complement, not a competitor, and
10681075
which complement depends on who is waiting: "Address comments" while
10691076
somebody's word is unanswered, typing the COMMENTS prompt into the owning
@@ -1163,14 +1170,16 @@ def __init__(
11631170
self.append(row)
11641171
self._sync_buttons()
11651172

1166-
def sync(self, pr: PullRequest) -> None:
1173+
def sync(self, pr: PullRequest, viewer_is_author: bool = False) -> None:
11671174
"""Point the composer at *pr* as freshly fetched, and re-read the
1168-
session behind it. Verdicts only show for a live PR — GitHub refuses
1169-
a review on a merged or closed one, commenting stays open forever."""
1175+
session behind it. Verdicts only show for a live PR that somebody else
1176+
opened — GitHub refuses a review on a merged or closed one, and
1177+
refuses your own pull request's approval whatever state it is in.
1178+
Commenting stays open in every case."""
11701179
self._pr = pr
1171-
live = pr.state in practions.LIVE
1172-
self._approve_btn.set_visible(live)
1173-
self._request_btn.set_visible(live)
1180+
verdicts = pr.state in practions.LIVE and not viewer_is_author
1181+
self._approve_btn.set_visible(verdicts)
1182+
self._request_btn.set_visible(verdicts)
11741183
self._comment_btn.set_tooltip_text(_("Comment on {slug}").format(slug=pr.slug))
11751184
self._approve_btn.set_tooltip_text(_("Approve {slug}").format(slug=pr.slug))
11761185
self._request_btn.set_tooltip_text(

‎tests/test_prdetail.py‎

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,15 @@
2525

2626
@pytest.fixture(autouse=True)
2727
def clean_status_cache():
28-
"""fetch() absorbs into prstatus's module cache; keep tests independent."""
28+
"""fetch() absorbs into prstatus's module cache and remembers the signed-in
29+
login there for the run; keep tests independent of both."""
2930
prstatus._statuses.clear()
3031
prstatus._inflight.clear()
32+
prstatus._viewer = ""
3133
yield
3234
prstatus._statuses.clear()
3335
prstatus._inflight.clear()
36+
prstatus._viewer = ""
3437

3538

3639
# Shaped like real `gh pr view --json` output (recorded off episode6/collins
@@ -150,6 +153,25 @@ def test_the_header_fields_arrive_whole():
150153
assert detail.labels == ("enhancement",)
151154

152155

156+
def test_a_pr_is_the_viewers_own_when_the_logins_match():
157+
"""What takes the review verdicts off the page: GitHub refuses a review of
158+
your own pull request, and logins compare case-insensitively — gh spells
159+
an author back the way the account was registered."""
160+
assert parse_detail(URL, _reply(), DIFF, viewer="ghackett").viewer_is_author
161+
assert parse_detail(URL, _reply(), DIFF, viewer="GHackett").viewer_is_author
162+
assert not parse_detail(URL, _reply(), DIFF, viewer="someone-else").viewer_is_author
163+
164+
165+
def test_an_unanswerable_author_reads_as_somebody_elses():
166+
"""No gh, offline, signed out, or a reply that never named an author: all
167+
of them leave the page as it was before anyone asked."""
168+
assert not parse_detail(URL, _reply(), DIFF).viewer_is_author # no viewer
169+
assert not parse_detail(URL, _reply(author=None), DIFF, viewer="").viewer_is_author
170+
assert not parse_detail(
171+
URL, _reply(author=None), DIFF, viewer="ghackett"
172+
).viewer_is_author
173+
174+
153175
def test_checks_cover_both_of_ghs_shapes():
154176
checks = parse_detail(URL, _reply(), DIFF).checks
155177
assert [(c.name, c.state) for c in checks] == [
@@ -588,6 +610,36 @@ def gh_json(args, cwd=None, timeout=None):
588610
assert any(isinstance(entry, PrThread) for entry in detail.timeline)
589611

590612

613+
def test_fetch_asks_who_is_signed_in_and_hands_it_to_the_detail(monkeypatch):
614+
"""The fourth call, and the only one whose answer outlives the load: the
615+
login is remembered for the run, so a second page doesn't ask again."""
616+
calls = []
617+
618+
def gh_json(args, cwd=None, timeout=None):
619+
calls.append(args)
620+
if args[:2] == ["api", "user"]:
621+
return {"login": "ghackett"}
622+
return None if args[0] == "api" else _reply()
623+
624+
monkeypatch.setattr(prstatus, "gh_json", gh_json)
625+
monkeypatch.setattr(prstatus, "gh_text", lambda args, max_bytes=None: DIFF)
626+
assert fetch(URL).viewer_is_author is True
627+
assert ["api", "user"] in calls
628+
calls.clear()
629+
assert fetch(URL).viewer_is_author is True
630+
assert ["api", "user"] not in calls
631+
632+
633+
def test_fetch_survives_not_knowing_who_is_signed_in(gh):
634+
"""The gh fixture answers nothing to every `api` call, this one included:
635+
the load lands whole, the PR simply reads as somebody else's."""
636+
set_view, _set_diff, _calls = gh
637+
set_view(_reply())
638+
detail = fetch(URL)
639+
assert detail is not None
640+
assert detail.viewer_is_author is False
641+
642+
591643
def test_fetch_absorbs_into_the_summary_cache(gh):
592644
"""Opening the view updates the chip and mark for free: the reply lands in
593645
prstatus as a fetch of our own."""
@@ -625,7 +677,8 @@ def test_fetch_survives_a_dead_or_oversized_diff(gh):
625677
def test_fetch_runs_every_call_on_the_action_budget(monkeypatch):
626678
"""All the way down to subprocess.run: a load someone is waiting on gets
627679
the action timeout for the heavy view reply, the thread query and the
628-
diff alike, not the poll's short one."""
680+
diff alike, not the poll's short one. The who-am-I call rides along on the
681+
poll budget — it is one line of reply, and a load survives losing it."""
629682
seen = []
630683

631684
def run(argv, **kwargs):
@@ -636,5 +689,5 @@ def run(argv, **kwargs):
636689
monkeypatch.setattr(prstatus.shutil, "which", lambda _: "/usr/bin/gh")
637690
monkeypatch.setattr(prstatus.subprocess, "run", run)
638691
assert fetch(URL) is not None
639-
assert [kwargs["timeout"] for _argv, kwargs in seen] \
692+
assert [kwargs["timeout"] for _argv, kwargs in seen][:3] \
640693
== [prstatus._GH_ACTION_TIMEOUT_S] * 3

‎tests/test_prstatus.py‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ def scheduled(monkeypatch):
9595
urls: list[str] = []
9696
monkeypatch.setattr(prstatus, "_schedule", urls.append)
9797
monkeypatch.setattr(prstatus, "_gh_missing", False)
98+
monkeypatch.setattr(prstatus, "_viewer", "") # the run's remembered login
9899
prstatus._statuses.clear()
99100
prstatus._inflight.clear()
100101
yield urls
@@ -796,6 +797,41 @@ def test_gh_run_feeds_stdin_rather_than_argv(monkeypatch):
796797
assert "A comment body.\n" not in argv
797798

798799

800+
# -- who is signed in (what tells a PR page it is looking at your own PR) -----
801+
802+
803+
def test_viewer_login_is_asked_once_and_kept(monkeypatch):
804+
calls = []
805+
monkeypatch.setattr(
806+
prstatus, "gh_json",
807+
lambda args, cwd=None, timeout=None: calls.append(args) or {"login": "ghackett"},
808+
)
809+
assert prstatus.viewer_login() == "ghackett"
810+
assert prstatus.viewer_login() == "ghackett"
811+
assert calls == [["api", "user"]] # the second answer came out of the run's memory
812+
813+
814+
def test_viewer_login_doesnt_remember_a_failure(monkeypatch):
815+
"""Offline, signed out, or a reply that isn't a user: all of them are
816+
states that get better, so the next caller asks again."""
817+
replies = [None, {"login": ""}, {"login": 7}, {"login": "x" * 200}, {"login": "gh"}]
818+
monkeypatch.setattr(
819+
prstatus, "gh_json", lambda args, cwd=None, timeout=None: replies.pop(0)
820+
)
821+
assert [prstatus.viewer_login() for _ in range(4)] == ["", "", "", ""]
822+
assert prstatus.viewer_login() == "gh"
823+
824+
825+
def test_viewer_login_stays_off_a_machine_without_gh(monkeypatch):
826+
calls = []
827+
monkeypatch.setattr(prstatus, "_gh_missing", True)
828+
monkeypatch.setattr(
829+
prstatus, "gh_json", lambda args, cwd=None, timeout=None: calls.append(args)
830+
)
831+
assert prstatus.viewer_login() == ""
832+
assert calls == []
833+
834+
799835
# -- absorbing a detail fetch -------------------------------------------------
800836

801837
# Shaped like the detail view's `gh pr view --json` reply (prdetail): a strict

0 commit comments

Comments
 (0)