Skip to content

feat(github): show pull request state as the icon of PR links in messages - #1790

Open
matt2e wants to merge 15 commits into
mainfrom
github-links-status-icon
Open

matt2e wants to merge 15 commits into
mainfrom
github-links-status-icon

Conversation

@matt2e

@matt2e matt2e commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Before / after

Before: every GitHub PR link shows the same octocat. After: open (green pull-request glyph), draft (dashed circle), merged (purple merge glyph) and closed (circle-x), in light and dark themes, in received and own bubbles.

Icons keep a fixed 1em box, so lists stay aligned; on the high colour level the glyphs inside your own bubble take the link ink so they remain legible on the solid accent.

Summary

GitHub pull request links in messages now show the pull request's current state (open, draft, merged, closed) as the link's icon, in place of the generic GitHub icon.

  • Rendering: the GitHub plugin registers a conversation link renderer for pull request URLs only (order: -1, so it wins the tie with Links). Repositories, issues and commits stay with Links. LinkLabel takes an optional icon so the renderer reuses the Links label layout.
  • Shared state store (pull-states.ts): one store, keyed by canonical URL, serves every session's Solo sidebar and every message link. A pull request shown in both places costs one read. The store polls only the URLs in view, sends one gh query per poll, and backs off reads that fail.
  • Public reads (no gh): each poll reads at most 10 pull requests. A 403/429 holds all public reads until GitHub's rate-limit reset. When the window becomes visible again, anything due is read immediately. An unchanged state keeps its object, so mounted links and sidebar rows don't re-render.
  • Solo sidebar: solo-pull.ts now uses the shared store instead of running its own polling.
  • Design system: the open, merged and draft/closed glyphs use icon-tier roles (--icon-success, --icon-categorical-purple, new --icon-subtle). Each role has a new -own pair, and .messageBubble[data-own] swaps in the pair. On the high colour level, where the own bubble is the solid accent, the glyphs use the link ink, so they stay legible. The contrast guard now measures these roles on the received and own bubbles at every family and level.
  • Tests: added coverage for the store (metering, rate-limit hold, unchanged states, visibility return), PullLink, LinkLabel icon override and the contrast pairings. pages.integration.test.mjs now expects the GitHub link renderer next to Links.

Validation

  • Pre-push hooks pass: types, related unit tests, design-system checks, biome.
  • Still to do: an in-app check by a human, in both themes and at the high colour level.

🤖 Generated with Claude Code

@matt2e
matt2e marked this pull request as ready for review October 28, 2026 05:19
@matt2e
matt2e requested a review from a team as a code owner October 28, 2026 05:19
@matt2e
matt2e force-pushed the github-links-status-icon branch from 88aad59 to 1088c21 Compare October 28, 2026 05:33
Comment thread src/bundled/github/pull-states.ts
Comment thread src/bundled/github/pull-states.ts
Comment thread src/bundled/github/PullLink.tsx
Comment thread src/bundled/github/PullLink.tsx
matt2e added a commit that referenced this pull request Oct 28, 2026
…t gh, read nothing while composing, name the state icon

Addresses the four P2 findings of the review of 1088c21 on PR #1790.

A displayed link ending in `/pull/2147483648` parsed as a pull request, so
the store admitted it to the shared `gh` query, where `$pN: Int!` cannot hold
it. GitHub rejected the whole request with no data, so every pull request
shown alongside stayed unknown or stale, and the batch failed the same way a
minute later for as long as the link was mounted. `parseGitHubReference` now
requires a number a signed 32-bit integer can hold, which is how GitHub
numbers them, so such a URL is no reference: Links keeps it as a plain GitHub
link, the store never sees it and the panel does not claim it.

The desktop host always supplies `gh`, so without a sign-in every poll throws
`GhUnavailable` and falls to the public API. Once `publicUntil` lapsed,
`readsPublic()` was false at the top of the poll, so the hold guard was
skipped, and the fallback read publicly without looking at `publicHeldUntil`:
a 403 with a reset an hour out produced a refused fetch every five minutes.
The fallback now checks the hold after `gh` throws and reads nothing under
it. `gh` is still tried every five minutes, so a later sign-in is picked up,
and the public API only once GitHub says it resets. The five-minute public
cadence while `gh` is unavailable is kept: that poll is what retries `gh`,
and the hold now bounds what it can spend.

Composer decorations and previews render links through `MessageLink` with
`interactive={false}`, but a renderer component knows only its URL, so typing
`/pull/1790` watched `/pull/1`, `/17`, `/179` and `/1790` in turn, spending a
public read on each prefix, or with `gh` restarting the batch per keystroke.
`LinkInteractiveContext` joins the host's other link contexts in
`LinkLabelContext.ts`; `MessageLink` provides its `interactive` prop through
it, and `PullLink` shows what the store already knows without watching when
it is false. The `LinkRenderer` contract is unchanged.

The state was a `title` on a generic span around an `aria-hidden` glyph, so
accessibility snapshots held only the link's name and URL. The span is now
`role="img"` named by `describePull`, as the status display is, so the tree
shows `img "Pull request, merged"` inside the link, while a labelled link
keeps its authored name from the anchor's `aria-label`.

Tests: a number past Int is no reference, and its neighbours are read in one
query without it; a refusal while `gh` is unavailable holds the public API
until the reset while `gh` is retried, telling no one; a non-interactive
`PullLink` shows cached state without watching, and a composed `MessageLink`
with both renderers spends no read; the state is found by role and name
through every transition while the link keeps its authored name. Each fails
on the previous code: seven refused fetches in 30 minutes, the invalid number
in the query's variables, a watched decoration, and no `img` in the tree.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e
matt2e force-pushed the github-links-status-icon branch from f795f21 to 5a091da Compare October 28, 2026 23:53
matt2e added a commit that referenced this pull request Oct 28, 2026
…t gh, read nothing while composing, name the state icon

Addresses the four P2 findings of the review of 1088c21 on PR #1790.

A displayed link ending in `/pull/2147483648` parsed as a pull request, so
the store admitted it to the shared `gh` query, where `$pN: Int!` cannot hold
it. GitHub rejected the whole request with no data, so every pull request
shown alongside stayed unknown or stale, and the batch failed the same way a
minute later for as long as the link was mounted. `parseGitHubReference` now
requires a number a signed 32-bit integer can hold, which is how GitHub
numbers them, so such a URL is no reference: Links keeps it as a plain GitHub
link, the store never sees it and the panel does not claim it.

The desktop host always supplies `gh`, so without a sign-in every poll throws
`GhUnavailable` and falls to the public API. Once `publicUntil` lapsed,
`readsPublic()` was false at the top of the poll, so the hold guard was
skipped, and the fallback read publicly without looking at `publicHeldUntil`:
a 403 with a reset an hour out produced a refused fetch every five minutes.
The fallback now checks the hold after `gh` throws and reads nothing under
it. `gh` is still tried every five minutes, so a later sign-in is picked up,
and the public API only once GitHub says it resets. The five-minute public
cadence while `gh` is unavailable is kept: that poll is what retries `gh`,
and the hold now bounds what it can spend.

Composer decorations and previews render links through `MessageLink` with
`interactive={false}`, but a renderer component knows only its URL, so typing
`/pull/1790` watched `/pull/1`, `/17`, `/179` and `/1790` in turn, spending a
public read on each prefix, or with `gh` restarting the batch per keystroke.
`LinkInteractiveContext` joins the host's other link contexts in
`LinkLabelContext.ts`; `MessageLink` provides its `interactive` prop through
it, and `PullLink` shows what the store already knows without watching when
it is false. The `LinkRenderer` contract is unchanged.

The state was a `title` on a generic span around an `aria-hidden` glyph, so
accessibility snapshots held only the link's name and URL. The span is now
`role="img"` named by `describePull`, as the status display is, so the tree
shows `img "Pull request, merged"` inside the link, while a labelled link
keeps its authored name from the anchor's `aria-label`.

Tests: a number past Int is no reference, and its neighbours are read in one
query without it; a refusal while `gh` is unavailable holds the public API
until the reset while `gh` is retried, telling no one; a non-interactive
`PullLink` shows cached state without watching, and a composed `MessageLink`
with both renderers spends no read; the state is found by role and name
through every transition while the link keeps its authored name. Each fails
on the previous code: seven refused fetches in 30 minutes, the invalid number
in the query's variables, a watched decoration, and no `img` in the tree.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
matt2e added a commit that referenced this pull request Oct 29, 2026
…nd expect the inline read

Addresses the two branch defects in the CI notes on #1790, which failed ten
browser journeys on both engines in every run.

5a091da gave the state glyph `role="img"` named by `describePull`, inside
the host's anchor. A link shown as its URL has no `aria-label`, so its name is
computed from its contents and became "Pull request, openhttps://…": no
separator, and a name that changed from "Pull request" to "…open…" to
"…merged…" as the state was read. Screen readers announced it run together,
and `layout.spec.mjs`'s exact-name lookup no longer found the link.

The state now describes the link instead. `LinkDescriptionContext` joins the
host's link contexts in `LinkLabelContext.ts`. `MessageLink` provides a setter
for its live anchor's `aria-description`, the attribute it already sets for an
unavailable chat. `PullLink` sets the state's words while shown and clears
them on unmount. The glyph's span is `aria-hidden` again, keeping its `title`
tooltip, so the name stays the authored label or the URL whatever the state.
The `LinkRenderer` contract is unchanged, and a composer decoration has no
anchor to describe.

The inline link's state read made the panel's GitHub journeys count one more
`pulls/N` request than they expected. In the API-error journey, that read hit
the fixture's 403 and logged an unexpected console error. These journeys now
expect it. Before opening the panel, each waits for the inline read: the two
checklist journeys on the link's description "Pull request, open", the error
journey on the refused response. Then they expect `pulls/N` twice, or allow
the 403 twice. After the refusal, the store holds public reads until GitHub's
reset, so the retry adds no third read.

Tests: the unit test of the states expects each one's words as the
description and none once unmounted; a labelled link keeps its name with the
state as its description; and a link shown as its URL is named by the URL
alone before and after it is read as merged. All three fail on the previous
`PullLink`. `layout.spec.mjs`, `external-links.spec.mjs` and
`github-checklist.spec.mjs` pass on Chromium and WebKit (52 cases).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Carl, an automated reviewer, commenting via Wes’s GitHub account.

Thanks for this. It reads cleanly. The URL→gh boundary is sound: exact github.com host, bounded Int32 numbers, and owner/repo/number passed as GraphQL variables. Kalvin's four earlier findings hold at 2a4d563. CI is green.

Recommend fixing before merge (comment only; humans decide):

  1. Without gh, pull request states stop updating after the first read (pull-states.ts:177)
  2. The selected Solo row's icon loses its selected-row colour (GitHub.module.css:404)
  3. At Buzzin', state icons vanish inside neutral insets in your own bubble (Messages.module.css:3042)

Should fix: without gh, links that keep failing can permanently starve one that has never been read (pull-states.ts:269).
Optional: a doc mismatch on renderer failure, and the glyph shown for failed.

Validation gaps: Mongo's two store findings were reproduced with focused Vitest probes (fake timers, mocked fetch). The CSS findings were traced from source; no in-app check in either theme at Buzzin'. The PR's own human in-app check is still pending.

Comment thread src/bundled/github/pull-states.ts Outdated
Comment thread src/bundled/github/pull-states.ts Outdated
Comment thread src/bundled/github/GitHub.module.css Outdated
Comment thread src/features/messages/Messages.module.css Outdated
Comment thread docs/plugin-architecture.md Outdated
Comment thread src/bundled/github/PullLink.tsx
matt2e added a commit that referenced this pull request Oct 29, 2026
…t unread first, and keep state icons out of insets

Addresses the review of 2a4d563 on PR #1790.

Without `gh`, a successful public read set every state's next read to
never, so an open, draft or closed pull request kept the state it was first
read in for the session: neither polling, returning to the window nor a
remount read it again. On `main`, the Solo sidebar read non-merged pull
requests publicly every five minutes. Only merged is now settled. Every
other public state is read again at `PULL_SLOW_REFRESH_MS`, which is what
the store already did while `gh` was unavailable. The 403/429 hold bounds
what that spends.

A public poll read the first ten due URLs in the order they were first
watched. URLs that kept failing, such as private repositories read without
`gh`, came round every five minutes and took the slots back. A URL never
read was left over each time and moved a minute on, so fifty failing links
kept the fifty-first unread indefinitely. Ordering by due time, as the review
suggested, does not help, because that minute moves a left-over URL level
with or behind the failures it lost to; the new test fails that way too. The
store now records when each URL was last read and takes the longest unread
first, never read before all. With `gh` the batch is read whole, so only the
aliases' order changes.

The Solo sidebar's quiet icon read `--icon-subtle`. That role is
`var(--text-subtle)` computed at `:root`, so `.navigation-item`'s hover,
highlight and selected rebinds of `--text-subtle` no longer reached it, and a
selected row showed root grey on the selected fill. `.soloPull` reads
`--text-subtle` again. Only a message link's `.pullState` uses
`--icon-subtle`.

The own bubble rebound the shared icon roles for everything inside it,
including neutral insets. At Buzzin', a pull request glyph inside a table or
revealed spoiler in your own message took the accent ink on
`--surface-panel`: `#fff` on `#fff` in light magenta, iris, cobalt and mono,
and `#000` on `#1a1a1a` in dark bee, citrus and mono. This now follows the
`--mention-*` pattern. The bubble sets `--message-icon-subtle`,
`--message-icon-success` and `--message-icon-categorical-purple`. Both neutral
inset rules reset them to `initial`, as they reset mentions. `.pullState`
reads each one with the shared role as its fallback.

Docs: `plugin-architecture.md` describes the two fallbacks separately:
Links when the plugin is disabled, a plain link when the renderer fails. The
plugin README makes no such claim. `design-system.md` names the bubble's
variables and the inset reset.

An unavailable pull request still shows the plain pull request glyph,
greyed. Tabler, the only allowed source, has no unavailable variant: its pull
request family is open, closed, draft and conflict. The neutral glyphs
already exported each mean something else here or elsewhere: circle-dashed
is draft, x-circle is closed, and help-circle means help. A new glyph meaning
is a designer call under AGENTS.md. The state stays in the link's
`aria-description` and its `title`.

Tests: the store reads a public pull request again every five minutes through
draft and closed, then never once merged. Twelve public links are read again
ten and then two at a time. A never-read link behind fifty failing ones is
read within five minutes. All three fail on the previous store, and the last
also fails with only the first fix or with due-time ordering.
`GitHub.module.test.ts` reads the stylesheets, since jsdom does not resolve
`var()`. It checks that the sidebar's quiet colour reads only roles that
`.navigation-item` rebinds. It also checks that each neutral inset resets
every icon variable the own bubble hands down, and that `.pullState` reads
each with its shared role as fallback. Both fail on the previous CSS, and the
second still fails with only the sidebar fix. The contrast guard already
measures the base roles on `--surface-panel`, which is what the inset now
shows. It reads only `tokens.css`, so it cannot see the module cascade.

A Chromium and WebKit probe of the real stylesheets confirmed the computed
colours. At high/magenta/light and high/bee/dark, an own bubble's table and
revealed-spoiler icons take the shared green, grey and purple, where they
were `#fff` and `#000` before. A selected Solo row's quiet icon takes the
row's selected ink, where it was the root grey `rgb(82, 82, 82)` before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>

@kalvinnchau kalvinnchau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 One public polling quota issue remains.

Comment thread src/bundled/github/pull-states.ts
matt2e added a commit that referenced this pull request Oct 29, 2026
…s hourly limit

Addresses the review of 128bd1c on PR #1790.

Without `gh`, 128bd1c read every non-merged pull request from the public
API every five minutes. Six shown pull requests cost 72 reads an hour and ten
cost 120, against the 60 GitHub allows an address, before the panel read
anything. The 403/429 hold only stopped reads once the limit was spent. Then
the links stopped updating until the reset, and an opened panel reported the
limit.

Public reads now draw on a budget the store keeps: `PUBLIC_READS_PER_POLL`
(10) can be spent at once, as on a channel of links first shown, and it
refills at `PUBLIC_READS_PER_HOUR` (30). However many pull requests are
watched, first reads included, the store spends at most 40 public reads in
any hour, so the panel keeps at least 20. The pull requests a poll cannot
afford wait a minute, as left-overs already did. They still go longest
unread first, so the budget rotates through every shown pull request. Each
is still read at most every five minutes, and `gh` reads are unchanged.

Tests: twelve public pull requests watched for two hours cost ten reads at
once, then 30 an hour, and the last twelve reads are one of each. Under
128bd1c the first hour cost about 140. The never-read-first test now
watches thirteen failing links, not fifty. That is enough for some to be
unread when the first come round again. It shows a readable link is read
before any failing one is tried again. Fifty would take over an hour to read
within the budget. Watch-order selection fails it. Not run locally at the
author's request; left to CI.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Star Lord’s automated source follow-up via Wes’s account: no further changes requested. The prior polling, fairness and icon-color findings are addressed, and the public-read budget is bounded; Rocket, Mantis and Groot independently reviewed the follow-up, with no fix-introduced defects found. Reviewed head c7e8bf3691f38abd3227cc003b91de91a1caad28 against base 49fac0341e6ffeb1b9cae841d5ed8d96a6734aad, following the prior review. Source-only: tests and app flows were not run, CI was not assessed, and human visual acceptance remains pending.

@kalvinnchau kalvinnchau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 One new quota-accounting defect remains after the steady-state polling fix.

Comment thread src/bundled/github/pull-states.ts Outdated
matt2e added a commit that referenced this pull request Oct 29, 2026
…a clock set back

Addresses the review of c7e8bf3 on PR #1790.

The public read budget refilled by the time since it was last counted, as a
fraction of a read. With the budget spent, a clock corrected backwards made
that fraction negative, and `Math.floor(-0.5)` is -1, so `slice(0, -1)` read
every due pull request but the last: seventy links shown after a one-minute
correction cost 69 reads at once.

Reads are now earned whole, one for every `PUBLIC_READ_EARNED_MS` (two
minutes) since the budget was last earned to, and that time moves on by the
whole steps earned. The count stays an integer, so no fraction drifts across
an hour of polls. A clock set back earns nothing. Its time is earned again
once the clock passes where it was. The cadence is unchanged: one read every
two minutes once the first ten are spent.

Tests: with the first ten spent and the clock a minute back, three links
shown read nothing, and are read, with the two left over, one every two
minutes once the clock is past where it was. On c7e8bf3 they cost two
reads at once.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Star Lord’s automated source follow-up via Wes’s account: no further changes requested. This fixes the backward-clock over-read missed in my previous review; Rocket, Mantis and Groot independently reviewed the follow-up, with no fix-introduced defects found. Reviewed head 046db51b05c94e073acc4cd93ffff820d79be3c0 against base 49fac0341e6ffeb1b9cae841d5ed8d96a6734aad, following the prior review. Source-only: tests and app flows were not run, CI was not assessed, and human visual acceptance remains pending.

@kalvinnchau kalvinnchau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 One remaining public polling fairness defect; the clock-rollback quota burst is fixed.

Comment thread src/bundled/github/pull-states.ts Outdated
matt2e and others added 13 commits October 29, 2026 17:17
…ages

GitHub pull request links in messages rendered the Links plugin's static
octocat. The GitHub plugin now contributes a `LinkRenderer` (order -1, below
Links' default band) for `kind === "pull"` URLs that reuses Links' `LinkLabel`
with the pull request's state glyph: open (success), draft and closed
(subtle), merged (categorical purple). The plain pull request glyph shows
until the state is read, and when it cannot be, so the icon never swaps.
Repositories, issues and commits stay with Links, which also stands in when
the plugin is disabled or the renderer fails.

The batched, visibility-aware state poller moves out of `createSoloPulls`
into a URL-keyed `createPullStates(gh)` store shared by every session's
sidebar and every inline link, so one pull request shown in both costs one
read. Merged stays terminal, closed is read at the slow five-minute cadence,
and without `gh` each pull request is read from the public API once. The
state icons, words and tone move to `pull-state-view.ts` so the sidebar,
panel and inline link agree; closed stays grey on all three.

`LinkLabel` gains an optional `icon` override. Docs and the plugin README
note the renderer and that Links remains the fallback.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…r poll

Addresses the review of 4a1f445f3 on the shared pull request state store.

A shared `gh` read that threw with no usable data (rate limited, a non-zero
exit, the 30 s deadline) recorded nothing, so every URL in the batch stayed
due and `finally → later()` armed a 0 ms timer: a spawn-and-fail loop while
GitHub was already refusing. The catch now moves every due URL's next read
out a minute, keeping what is shown.

Without `gh`, every outcome was read once, so a public read that failed for
a passing reason (offline at launch, a shared NAT's 403) marked the link
unavailable for good. A successful public read is still read once; a failed
one is tried again at the slow five-minute cadence until it is read.

`refresh` aborted any read in flight, and inline links mount per virtualised
row, so scrolling through a channel of PR links restarted the batch on every
newly mounted one and spent a rate-limit point on each discarded query. A
read in flight is now left to finish; its `finally → later()` picks up what
became due meanwhile at once. Aborting is reserved for the last unsubscribe.

Pull requests GitHub could not read were retried as one `gh` process each,
concurrently, every five minutes. They now stay in the one aliased query
with the rest: `readStates` already takes the NOT_FOUND aliases from the
error's partial data, so the retry costs no extra process. Only the cadence
distinguishes them.

Tests cover each: a thrown shared read waits a minute and does not spin, a
failed public read is retried on the slow cadence then read once, watching a
second URL mid-read neither aborts the first nor delays the second, two
unreadable URLs retry inside the shared query, and a hidden window re-arms
without reading. Each behaviour test fails on the previous store.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…watched sidebar views

Addresses the review of a659ce7a8 on the shared pull request state store.

The store keyed each entry on the URL it was given and read the number off
its last path segment, but `parseGitHubReference` accepts a query string and
a trailing slash, which inline links pass through as pasted. `…/pull/123?x`
gave `Number("123?x")`, `NaN`, serialised as `null` for `$pN: Int!`; GitHub
rejected the whole query with no data, so every due URL in the batch backed
off together and failed the same way a minute later, leaving every PR link
in the channel on the plain glyph while that one was shown. `…/pull/123/`
gave number 0, NOT_FOUND, and an unavailable icon for a pull request that
exists. `watch` and `get` now key on the pull request's canonical URL from
`parseGitHubReference`, so the variants of one pull request, and an inline
link and the sidebar's normalised link to it, share one entry and one read,
and `readStates` only ever sees canonical URLs. A URL that is not a pull
request's is not watched and reads as `"failed"` rather than throwing.

Re-watching a chat whose link was already known called `follow` but kept the
cached `views` entry. The shared store can advance a URL while that chat is
unwatched, kept current by another chat or an inline link, and notifies only
its current subscribers, so on remount `useSyncExternalStore` read the stale
view. Pollable states healed within a minute, but merged is terminal and
never re-read, so the sidebar showed open for a merged pull request until a
re-read on returning to the window. `follow` now drops the cached view, so
`get` reads the store afresh.

Tests: a query-string and a trailing-slash variant resolve, cost one query
with the canonical numbers alongside a plain URL, and share one state object;
a non-pull URL is neither read nor thrown on; a chat re-watched after the
store moved to merged shows merged. Each fails on the previous code.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Addresses the optional finding of the review of 7aedd9123.

`notify()` fired every listener of every watched URL after each poll, not
just those whose state the poll recorded. Inline `PullLink`s were unaffected,
since `get` returns a stable reference and React bails out, but each Solo
sidebar listener drops its cached view on notify, so every shown Solo row
with a link rebuilt its view and re-rendered once a minute, including rows
whose pull request is merged and was never in the batch. `notify` now takes
the `due` URLs and tells only their listeners. The thrown-batch back-off
records no state and so, as before, notifies nobody; the public fallback
reads the same `due` URLs and notifies them.

The first store test, which already polls only the open pull request on its
second tick, now checks that the open URL's two listeners hear of it and the
merged URL's does not. It fails on the previous store.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…roles

The open glyph of a pull request link in a message was painted `--text-success`,
a step-12 ink chosen for reading text on fills. Inside a bubble whose body and
link inks are also step 12 it clears the bubble at 10:1 but the neighbouring
ink at only 1.1–1.5:1, so the eye reads a dark mark, not green; merged read
only because purple's hue is far from olive, which does not hold for every
accent. The cascade was correct; the role was the wrong tier.

Two icon-tier roles join `tokens.css` and the registry, next to
`--icon-status-complete` and `--text-categorical-purple`: `--icon-success`
(green-11 in both modes) and `--icon-categorical-purple` (purple-11 in both
modes). The sidebar's and the inline link's open and merged icons move to them;
the panel's chip keeps the text-tier roles, since there green and purple text
sits on a tinted fill and needs 4.5:1. Draft and closed stay `--text-subtle`.

Measured against the own bubble at the tinted levels across all six accents:
green-11 3.89–3.99:1 light and 7.48–7.62:1 dark (Citrus 3.99 / 7.48), purple-11
5.01–5.15:1 light and 6.67–6.79:1 dark (Citrus 5.15 / 6.67); received bubble
4.25 / 7.76 and 5.49 / 6.91; page surfaces 4.33–4.72 / 7.86–11.2 and
5.58–6.08 / 7.01–9.98. Dark green-10 would also clear (4.92–5.27) but green-11
matches `--status-online` and keeps one step in both modes; dark purple-10
clears by only 3.01–3.22, so purple stays at 11.

The contrast guard now measures both roles against the page surfaces and, per
accent family, against both message bubbles at 3:1. On the high colour level
the own bubble is the solid accent step and no single hue clears 3:1 on every
family, a gap the previous text roles shared; the guard names it and skips that
one pairing rather than hiding it in an exception. Tests move each role one
step thinner (green-10, purple-9): both still clear the page, so only the new
bubble pairings fail them.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…ink ink

Addresses the review of d077059d4 on the pull request state icons.

On the high colour level the own bubble is the solid accent step, and no
green or purple clears it on every family: `--icon-success` and
`--icon-categorical-purple` measured 1.05–1.69:1 there, so a pull request's
state glyph was effectively invisible in your own messages. The guard knew
and skipped that one pairing rather than enforcing it.

The high level is expressed in `tokens.css` as a global block that swaps the
`-own` roles, not an own-bubble-scoped block, so redefining the icon roles
there would also repaint received bubbles and the Solo sidebar. The fix
follows the existing `-own` pair pattern instead. `--icon-success-own` and
`--icon-categorical-purple-own` join the role layer and the registry,
pointing at their base role on the neutral and touch levels and at
`--text-message-link-own` in both high blocks. `.messageBubble[data-own]`
hands the pair to whatever sits inside it by shadowing the two roles, as it
already does for links, mentions and selection, so `GitHub.module.css` keeps
no host knowledge and the pair covers any state icon rendered in an own
bubble. On the high own bubble both roles now measure the link ink's floor:
magenta and cobalt 4.50:1, iris 5.04:1, bee 12.88:1, citrus 13.27:1 and
mono 17.40:1 light / 16.83:1 dark, with every other family identical in
both modes since the accent-9 fill and its ink do not change with the mode.

The contrast guard drops the `SOLID_OWN_BUBBLE_LEVEL` skip and measures each
base role on the received bubble and its `-own` pair on the own bubble at
every level. The test's framing is corrected: purple-9 is two steps thinner,
since purple-10 passes the whole guard at 3.01–3.22:1 on the dark bubbles,
and purple-9 also fails the dark popover, which the pinned received-bubble
pairing does not depend on. A third case points the high-level `-own` pair
back at the base role and expects the own-bubble pairing to fail on the high
level, so the previously skipped pairing is shown to be guarded.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…h the link ink

Addresses item 1 of the rendered check of de5abe4a2 on the pull request
state icons.

Draft and closed pull request glyphs were painted `--text-subtle`, a text
role the own bubble does not pair, so on the high colour level, where that
bubble is the solid accent step, the grey measured 1.04–2.98:1 on most
families in both modes, 1.07:1 on citrus in dark: the glyph of a draft or
closed pull request in your own message was effectively invisible while the
open and merged glyphs beside it had already moved to their `-own` pair in
de5abe4a2. The guard did not measure these two states at all.

The fix is the same pair, one tier down. `--icon-subtle`, pointing at
`--text-subtle`, and `--icon-subtle-own` join the role layer and the registry
next to the two existing icon pairs; the `-own` role is its base on the
neutral and touch levels and `--text-message-link-own` in both high blocks.
`.messageBubble[data-own]` shadows it alongside the other two, so
`GitHub.module.css` only switches the draft/closed/unknown default of
`.soloPull` and `.pullState` to `--icon-subtle`; the panel chip's text and
every other supporting text keep `--text-subtle`, which is not shadowed for
the bubble as a whole. On the high own bubble the role now measures the link
ink's floor: magenta and cobalt 4.50:1, iris 5.04:1, bee 12.88:1, citrus
13.27:1 and mono 17.40:1 light / 16.83:1 dark, identical in both modes
elsewhere. On the tinted levels it measures 6.44 light / 8.03 dark at neutral
and 6.44–6.61 light / 8.30–8.46 dark at touch across the six families, and
the base role measures 7.05 light / 8.61 dark on the received bubble.

The contrast guard adds `--icon-subtle` to the message icon roles, so it is
measured on the page surfaces and the received bubble and its `-own` pair on
the own bubble at every family and level. A fourth test case points the
high-level pair back at the base role and expects the dark own-bubble pairing
to fail; the previous guard passes that stylesheet.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…read on window return

Addresses the three ranked concerns of the refresh timing and performance
assessment of 05be7a7 on the shared pull request state store.

Without `gh`, every due pull request was read from the public API at once,
one `fetch` each with no cap, against the 60 reads an hour GitHub allows an
address. Thirty links in view spent thirty at once; a second channel ran out
the hour, after which each failed URL was tried again every five minutes
and refused again, starving the panel's public reads too. A public poll now
reads at most `PUBLIC_READS_PER_POLL` (10) of the due URLs, in the order
they were shown, and moves the rest a minute on rather than leaving them
due. `loadGitHubDetails` throws a typed `GitHubRateLimited` for a 403 or
429, with the same message the panel shows, carrying GitHub's
`x-ratelimit-reset` or an hour on without it; the store catches it, records
nothing for the batch, and holds every public read until that time, as it
already holds a thrown `gh` batch for a minute. A pull request shown during
the hold waits for it too. With `gh`, the hold ends at the earlier of the
reset and `gh`'s next try, so a signed-in `gh` is not kept waiting an hour
behind a public refusal.

Each poll stored a fresh state object even when nothing changed, so every
mounted open or draft link re-rendered once a minute and each Solo sidebar
row with a pollable link dropped and rebuilt its view. An unchanged state,
with the same `state`, `checks` and `conflicts`, now keeps its object, and
only the URLs whose state changed are notified, so `useSyncExternalStore`
bails out and the sidebar keeps its view.

Returning to the window after a long hidden stretch waited for the hidden
tick's re-arm, up to a minute, before reading what had become due. The store
now listens for `visibilitychange` while anything is watched and runs the
scheduler on return: anything due is read at once, and otherwise the poll
keeps its time. The hidden window still looks again later rather than
reading.

Tests: twelve public links cost ten reads, then the other two a minute later,
each once; a refusal with and without a reset header holds every public read,
including a link shown meanwhile, until the reset or an hour, telling no one
and showing nothing as unavailable; a refusal does not keep `gh` waiting past
its retry; a poll reading the same state keeps the object and tells no one,
and the sidebar row's view and listener are left alone; coming back after two
hidden minutes reads at once, after half a minute with nothing due it does
not; the loader's limit error names the reset. The first store test now
changes the checks between polls, since an unchanged poll is no longer heard,
and the failed-public-read test uses a 502, since a 403 is now a hold. Each
new behaviour test fails on the previous store or loader, except the `gh`
retry one, which pins the new hold's bound.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…t gh, read nothing while composing, name the state icon

Addresses the four P2 findings of the review of 1088c21 on PR #1790.

A displayed link ending in `/pull/2147483648` parsed as a pull request, so
the store admitted it to the shared `gh` query, where `$pN: Int!` cannot hold
it. GitHub rejected the whole request with no data, so every pull request
shown alongside stayed unknown or stale, and the batch failed the same way a
minute later for as long as the link was mounted. `parseGitHubReference` now
requires a number a signed 32-bit integer can hold, which is how GitHub
numbers them, so such a URL is no reference: Links keeps it as a plain GitHub
link, the store never sees it and the panel does not claim it.

The desktop host always supplies `gh`, so without a sign-in every poll throws
`GhUnavailable` and falls to the public API. Once `publicUntil` lapsed,
`readsPublic()` was false at the top of the poll, so the hold guard was
skipped, and the fallback read publicly without looking at `publicHeldUntil`:
a 403 with a reset an hour out produced a refused fetch every five minutes.
The fallback now checks the hold after `gh` throws and reads nothing under
it. `gh` is still tried every five minutes, so a later sign-in is picked up,
and the public API only once GitHub says it resets. The five-minute public
cadence while `gh` is unavailable is kept: that poll is what retries `gh`,
and the hold now bounds what it can spend.

Composer decorations and previews render links through `MessageLink` with
`interactive={false}`, but a renderer component knows only its URL, so typing
`/pull/1790` watched `/pull/1`, `/17`, `/179` and `/1790` in turn, spending a
public read on each prefix, or with `gh` restarting the batch per keystroke.
`LinkInteractiveContext` joins the host's other link contexts in
`LinkLabelContext.ts`; `MessageLink` provides its `interactive` prop through
it, and `PullLink` shows what the store already knows without watching when
it is false. The `LinkRenderer` contract is unchanged.

The state was a `title` on a generic span around an `aria-hidden` glyph, so
accessibility snapshots held only the link's name and URL. The span is now
`role="img"` named by `describePull`, as the status display is, so the tree
shows `img "Pull request, merged"` inside the link, while a labelled link
keeps its authored name from the anchor's `aria-label`.

Tests: a number past Int is no reference, and its neighbours are read in one
query without it; a refusal while `gh` is unavailable holds the public API
until the reset while `gh` is retried, telling no one; a non-interactive
`PullLink` shows cached state without watching, and a composed `MessageLink`
with both renderers spends no read; the state is found by role and name
through every transition while the link keeps its authored name. Each fails
on the previous code: seven refused fetches in 30 minutes, the invalid number
in the query's variables, a watched decoration, and no `img` in the tree.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…nd expect the inline read

Addresses the two branch defects in the CI notes on #1790, which failed ten
browser journeys on both engines in every run.

5a091da gave the state glyph `role="img"` named by `describePull`, inside
the host's anchor. A link shown as its URL has no `aria-label`, so its name is
computed from its contents and became "Pull request, openhttps://…": no
separator, and a name that changed from "Pull request" to "…open…" to
"…merged…" as the state was read. Screen readers announced it run together,
and `layout.spec.mjs`'s exact-name lookup no longer found the link.

The state now describes the link instead. `LinkDescriptionContext` joins the
host's link contexts in `LinkLabelContext.ts`. `MessageLink` provides a setter
for its live anchor's `aria-description`, the attribute it already sets for an
unavailable chat. `PullLink` sets the state's words while shown and clears
them on unmount. The glyph's span is `aria-hidden` again, keeping its `title`
tooltip, so the name stays the authored label or the URL whatever the state.
The `LinkRenderer` contract is unchanged, and a composer decoration has no
anchor to describe.

The inline link's state read made the panel's GitHub journeys count one more
`pulls/N` request than they expected. In the API-error journey, that read hit
the fixture's 403 and logged an unexpected console error. These journeys now
expect it. Before opening the panel, each waits for the inline read: the two
checklist journeys on the link's description "Pull request, open", the error
journey on the refused response. Then they expect `pulls/N` twice, or allow
the 403 twice. After the refusal, the store holds public reads until GitHub's
reset, so the retry adds no third read.

Tests: the unit test of the states expects each one's words as the
description and none once unmounted; a labelled link keeps its name with the
state as its description; and a link shown as its URL is named by the URL
alone before and after it is read as merged. All three fail on the previous
`PullLink`. `layout.spec.mjs`, `external-links.spec.mjs` and
`github-checklist.spec.mjs` pass on Chromium and WebKit (52 cases).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…t unread first, and keep state icons out of insets

Addresses the review of 2a4d563 on PR #1790.

Without `gh`, a successful public read set every state's next read to
never, so an open, draft or closed pull request kept the state it was first
read in for the session: neither polling, returning to the window nor a
remount read it again. On `main`, the Solo sidebar read non-merged pull
requests publicly every five minutes. Only merged is now settled. Every
other public state is read again at `PULL_SLOW_REFRESH_MS`, which is what
the store already did while `gh` was unavailable. The 403/429 hold bounds
what that spends.

A public poll read the first ten due URLs in the order they were first
watched. URLs that kept failing, such as private repositories read without
`gh`, came round every five minutes and took the slots back. A URL never
read was left over each time and moved a minute on, so fifty failing links
kept the fifty-first unread indefinitely. Ordering by due time, as the review
suggested, does not help, because that minute moves a left-over URL level
with or behind the failures it lost to; the new test fails that way too. The
store now records when each URL was last read and takes the longest unread
first, never read before all. With `gh` the batch is read whole, so only the
aliases' order changes.

The Solo sidebar's quiet icon read `--icon-subtle`. That role is
`var(--text-subtle)` computed at `:root`, so `.navigation-item`'s hover,
highlight and selected rebinds of `--text-subtle` no longer reached it, and a
selected row showed root grey on the selected fill. `.soloPull` reads
`--text-subtle` again. Only a message link's `.pullState` uses
`--icon-subtle`.

The own bubble rebound the shared icon roles for everything inside it,
including neutral insets. At Buzzin', a pull request glyph inside a table or
revealed spoiler in your own message took the accent ink on
`--surface-panel`: `#fff` on `#fff` in light magenta, iris, cobalt and mono,
and `#000` on `#1a1a1a` in dark bee, citrus and mono. This now follows the
`--mention-*` pattern. The bubble sets `--message-icon-subtle`,
`--message-icon-success` and `--message-icon-categorical-purple`. Both neutral
inset rules reset them to `initial`, as they reset mentions. `.pullState`
reads each one with the shared role as its fallback.

Docs: `plugin-architecture.md` describes the two fallbacks separately:
Links when the plugin is disabled, a plain link when the renderer fails. The
plugin README makes no such claim. `design-system.md` names the bubble's
variables and the inset reset.

An unavailable pull request still shows the plain pull request glyph,
greyed. Tabler, the only allowed source, has no unavailable variant: its pull
request family is open, closed, draft and conflict. The neutral glyphs
already exported each mean something else here or elsewhere: circle-dashed
is draft, x-circle is closed, and help-circle means help. A new glyph meaning
is a designer call under AGENTS.md. The state stays in the link's
`aria-description` and its `title`.

Tests: the store reads a public pull request again every five minutes through
draft and closed, then never once merged. Twelve public links are read again
ten and then two at a time. A never-read link behind fifty failing ones is
read within five minutes. All three fail on the previous store, and the last
also fails with only the first fix or with due-time ordering.
`GitHub.module.test.ts` reads the stylesheets, since jsdom does not resolve
`var()`. It checks that the sidebar's quiet colour reads only roles that
`.navigation-item` rebinds. It also checks that each neutral inset resets
every icon variable the own bubble hands down, and that `.pullState` reads
each with its shared role as fallback. Both fail on the previous CSS, and the
second still fails with only the sidebar fix. The contrast guard already
measures the base roles on `--surface-panel`, which is what the inset now
shows. It reads only `tokens.css`, so it cannot see the module cascade.

A Chromium and WebKit probe of the real stylesheets confirmed the computed
colours. At high/magenta/light and high/bee/dark, an own bubble's table and
revealed-spoiler icons take the shared green, grey and purple, where they
were `#fff` and `#000` before. A selected Solo row's quiet icon takes the
row's selected ink, where it was the root grey `rgb(82, 82, 82)` before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…s hourly limit

Addresses the review of 128bd1c on PR #1790.

Without `gh`, 128bd1c read every non-merged pull request from the public
API every five minutes. Six shown pull requests cost 72 reads an hour and ten
cost 120, against the 60 GitHub allows an address, before the panel read
anything. The 403/429 hold only stopped reads once the limit was spent. Then
the links stopped updating until the reset, and an opened panel reported the
limit.

Public reads now draw on a budget the store keeps: `PUBLIC_READS_PER_POLL`
(10) can be spent at once, as on a channel of links first shown, and it
refills at `PUBLIC_READS_PER_HOUR` (30). However many pull requests are
watched, first reads included, the store spends at most 40 public reads in
any hour, so the panel keeps at least 20. The pull requests a poll cannot
afford wait a minute, as left-overs already did. They still go longest
unread first, so the budget rotates through every shown pull request. Each
is still read at most every five minutes, and `gh` reads are unchanged.

Tests: twelve public pull requests watched for two hours cost ten reads at
once, then 30 an hour, and the last twelve reads are one of each. Under
128bd1c the first hour cost about 140. The never-read-first test now
watches thirteen failing links, not fifty. That is enough for some to be
unread when the first come round again. It shows a readable link is read
before any failing one is tried again. Fifty would take over an hour to read
within the budget. Watch-order selection fails it. Not run locally at the
author's request; left to CI.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
matt2e and others added 2 commits October 29, 2026 17:18
…a clock set back

Addresses the review of c7e8bf3 on PR #1790.

The public read budget refilled by the time since it was last counted, as a
fraction of a read. With the budget spent, a clock corrected backwards made
that fraction negative, and `Math.floor(-0.5)` is -1, so `slice(0, -1)` read
every due pull request but the last: seventy links shown after a one-minute
correction cost 69 reads at once.

Reads are now earned whole, one for every `PUBLIC_READ_EARNED_MS` (two
minutes) since the budget was last earned to, and that time moves on by the
whole steps earned. The count stays an integer, so no fraction drifts across
an hour of polls. A clock set back earns nothing. Its time is earned again
once the clock passes where it was. The cadence is unchanged: one read every
two minutes once the first ten are spent.

Tests: with the first ten spent and the clock a minute back, three links
shown read nothing, and are read, with the two left over, one every two
minutes once the clock is past where it was. On c7e8bf3 they cost two
reads at once.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…, together

Addresses the review of 046db51 on PR #1790.

A public poll that could not afford a pull request moved its next read a
minute on from that poll. Reads are earned every two minutes, so pull
requests shown at different times kept separate one-minute phases, and
whichever group polled first after each read was earned took it every time.
Longest-unread ordering only compares the URLs of a single poll, so it never
saw the other group. Twelve links shown, then three more half a minute
later, read the first group over and over and never the second. In the
clock-rollback test, it left a later link unread, failing in CI.

Those a public poll cannot afford now wait for the next read earned. Every
one waiting is in that poll, and the longest unread of them takes it,
whenever it was shown. GitHub's hold and the hidden window's floor still
apply through the scheduler. Each pull request is still read at most every
five minutes, within the same 40-an-hour budget.

Tests: twelve links shown, then three more half a minute later: within ten
minutes all fifteen are read, using fifteen reads. On 046db51 the later
three are never read. The clock-rollback test passes again. Both fail on
the previous store.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e
matt2e force-pushed the github-links-status-icon branch from 046db51 to 72ec63f Compare October 29, 2026 06:22

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Star Lord’s automated source follow-up via Wes’s account: no further changes requested. The staggered-link starvation missed in my previous review is addressed by scheduling budget-blocked reads at the same credit boundary; Rocket, Mantis and Groot independently checked the follow-up, with no fix-introduced defects found. Reviewed head 72ec63f7364d53aae0d6c090a0b22f6a2c19f65c against base 8385e11f72b2671cf30f3007635320553760514a, following the fairness finding. Source-only: tests, app flows and CI were not verified; human visual acceptance remains pending.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants