feat(github): linked-PR chips on the Issues list (#816)

[sheeerth]

Refs #816
Tracking plan: .ai/runs/2026-08-14-issue-linked-pr-chip.md
Source doc: .ai/specs/2026-08-09-issue-linked-pr-chip.md
Status: complete

🎯 Goal

• Each row of the cockpit's GitHub Issues list now carries a clickable ↗ PR #123 chip for every pull request linked to that issue, tinted by the PR's state — green for open, violet for merged, danger for closed-unmerged, and a muted variant for a draft. A triager can see that an issue is already being worked before handing it to an agent, instead of spending a run to find out.
• The links come from GitHub's authoritative issue↔PR relationships (timeline ConnectedEvent / CrossReferencedEvent), hydrated lazily for the on-screen row window through a new GET /api/v1/github/issue-prs endpoint — an additive sibling of the lazy checks-glyph endpoint (GitHub tab: incremental/paginated issue & PR loading instead of the 30→1000 two-shot fetch #664), so the fast one-shot list fetch is untouched.
• Implements .ai/specs/2026-08-09-issue-linked-pr-chip.md (Phase 1, steps 1–8) landed by the design-only spec PR docs(specs): Linked-PR chips on the GitHub Issues list #816.

What Changed

• packages/contract/src/github.ts — new linkedPrSchema ({number, url, state: 'open'|'merged'|'closed', isDraft?}) and githubIssuePrsDataSchema, a discriminated union on available mirroring githubChecksDataSchema. The §2-protected githubItemSchema and the list payload are unchanged.
• packages/cezar/src/server/forge/github.ts — new fetchGithubIssuePrs(repoRoot, numbers, refresh?) plus the exported, injection-testable fetchIssuePrLinks. One aliased GraphQL query (i0: issue(number:), …) covers a whole window, so 100 issues cost one subprocess. Per issue it keeps only PullRequest subjects/sources, dedupes by PR number (a PR seen as both connected and cross-referenced collapses to one chip), subtracts any PR carried by a DisconnectedEvent so a deliberately-unlinked PR loses its chip, and orders open → merged → closed then by descending number. Backed by a bounded 60 s per-issue issuePrsCache with a refresh bypass, a __clearIssuePrsCacheForTests() seam, and a CEZ_DRY_RUN=1 mock so the offline demo and the e2e suite paint real chips. It resolves the handle through the memoized resolveRepoHandle rather than fetchGithubChecks's inline gh repo view — one fewer subprocess per window.
• packages/cezar/src/server/server.ts — the new route registered as a chained .get('/github/issue-prs', …) link in the githubRoutes family (a loose app.get would vanish from AppType), with the same CSV validator shape and 400 wording as /github/checks plus the optional refresh=1 flag. Re-exported through the server/github.ts barrel.
• BACKWARD_COMPATIBILITY.md — the route added to the §2 inventory with its full contract, including why an issue with no links is absent from the map rather than present-and-empty.
• packages/web/src/api/client.ts / queries.ts — getGithubIssuePrs, queryKeys.githubIssuePrs and useGithubIssuePrs, mirroring the checks sibling. packages/api-client is deliberately untouched — its export * from '@open-mercato/cezar-contract' already re-exports the new types.
• packages/web/src/routes/github/github.tsx — derives the on-screen issue window (pinning the URL-selected issue so a deep link still hydrates), gates the hook to view === 'issues', and renders LinkedPrChip / LinkedPrOverflowChip on the row's meta line: in-cockpit /github/prs/:n when the PR is in the loaded open set, the PR's GitHub URL otherwise (isHttpUrl-guarded), and a +N collapse past three chips. The header's Refresh re-fetches the window with refresh=1, because a bare invalidate would be handed the same ≤60 s server cache and the flagship case is "an agent just opened a PR for this issue".
• The row is now a stretched link. GithubRow used to be one <Link> wrapping everything, which a chip cannot live inside: an <a> within an <a> is invalid HTML and a WCAG 4.1.2 (nested interactive) failure that this SSR-less SPA would emit silently. The <li> is now the positioning context, the row link an absolute inset-0 overlay carrying the navigation, the drag payload and an explicit aria-label, and the chips are its siblings — one interactive element per region, and no stopPropagation needed.

🧪 Tests

• npm run typecheck — clean. This is where the contract guard lives: the new GithubIssuePrs200 mutual-assignability assertion in contract-parity.github.test.ts fails compilation if the schema and the handler ever drift in either direction.
• npm test — 6091 passed / 324 files. New: 20 forge cases (forge/github.test.ts) covering alias mapping, the three state words plus the unknown-state fallback, isDraft omitted rather than sent as undefined, dedupe, disconnected-subtraction, the last:-window ordering, chunking with a failing chunk, the empty-list and total-failure degrades, the cache, its refresh bypass and the dry-run mock; 9 route cases (github-issue-prs-api.test.ts) covering the happy payload, absent-vs-empty, refresh=1, both 400 wordings, the cap, padded/negative numbers and the project-scoped alias; 3 client cases; and 9 cockpit cases (github.test.tsx) covering chip text, tint, the draft variant, both link targets, that clicking a chip opens the PR and not the row's issue, the no-nested-anchor structural invariant, the +N overflow, the unavailable and PRs-view cases, selected-issue pinning, and the refresh=1 on the header refresh.
• npm run test:unit — 36 passed. npm run build + check:pack — ok (475 files). npm run test:package — 15 passed.
• packages/web/e2e/github.e2e.ts gains a case that drives the real cockpit under CEZ_DRY_RUN=1 and screenshots the chipped row.

💥 Breaking Changes

• None. The slice is purely additive — a new schema, a new chained route, a new query hook and a new chip; no §2-protected shape changes and no existing route's payload is touched. The one structural edit, the stretched-link row, preserves every data-slot, the href, aria-current, the drag payload and the hover/active styling; two existing test selectors were rescoped from the row link to the row <li> (gh-row-item) because the meta glyphs are now its siblings.
• Rollback is deleting the slice: the row falls back to exactly today's meta line. No migration, no persisted state.

📋 Progress

See the Progress section in the tracking plan — all 8 steps checked.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ sheeerth
❌ BananaGawron
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[sheeerth]

🔍 Code Review

🎯 Summary

PR #885 implements .ai/specs/2026-08-09-issue-linked-pr-chip.md — the linked-PR chips on the GitHub Issues list — as one additive vertical slice modelled on the lazy checks-glyph slice (#664): a contract schema, a forge fetcher, a chained route, a typed client, a query hook and a row chip. The diff against the PR's base is 15 files, +1354/−10, and better than half of it is tests. Nothing existing is reshaped: no §2-protected shape changes, the one-shot GET /api/v1/github list payload is untouched, and rollback is deleting the slice.

The change carries exactly one structural edit — GithubRow becomes a stretched link so the chips can be anchors without nesting inside the row's anchor — and that is where the regression risk lives. I reviewed it specifically for drag, hover, focus, truncation and keyboard order, and found no functional regression; the details are under 🧩 Findings.

Verdict: approve. No blockers and no majors. One minor and three nits are listed below for the author to take or leave; none of them blocks the merge.

One process note that is not a code finding: this run could not apply labels. The authoring account has pull-only rights on open-mercato/cezar, so every label mutation answers HTTP 403. The label set this PR should carry — review, feature, needs-qa, priority-medium, risk-medium — needs a maintainer with write access, and until needs-qa is on the PR the qaGate cannot do its job.

🧪 Validation Gate

Every configured command was re-run in order, on the reviewed head, after the last edit to it:

Command	Result	Evidence
npm run typecheck	✅ pass	Clean across api-client, server and web. This is where the contract guard lives — the new GithubIssuePrs200 mutual-assignability assertion fails compilation if schema and handler drift in either direction.
npm test	✅ pass	6091 passed / 324 files.
npm run test:unit	✅ pass	36 passed, 0 failed.
npm run build	✅ pass	tsc → dist/, vite → web/dist/, then check:pack ok — 475 files, 85 under web/dist.
npm run test:package	✅ pass	15 passed, 0 failed — the tarball installs and the built CLI runs.

Beyond the gate: the GraphQL document was verified against real GitHub

The gate cannot catch the one failure mode this slice is most exposed to. fetchIssuePrLinks injects runGraphql, so every unit test drives a mocked forge — and the fetcher deliberately swallows a failed chunk (catch {}, degrading to absent chips). A malformed GraphQL document would therefore ship green and simply render no chips forever, which is precisely the "degradation path hiding a real failure" case CODE_REVIEW.md ranks second.

So I ran the generated document against the live API (gh api graphql, open-mercato/cezar, issues #765 and #730) and then fed the real response through fetchIssuePrLinks unmodified:

• The document is accepted: the per-issue aliasing (i0:, i1:), the itemTypes: [CONNECTED_EVENT, DISCONNECTED_EVENT, CROSS_REFERENCED_EVENT] enum, and the field-merging of subject/source across the three inline fragments are all valid.
• The real payload contains exactly the shapes the parser is written for, including source: { "__typename": "Issue" } entries with no other fields — the issue↔issue references that must be dropped.
• The parser's output on that payload is correct and correctly ordered: issue #765 → [#873 open, #766 closed], issue #730 → [#732 open].

That is the strongest evidence available short of a live integration test, and it is what moves the GraphQL builder from "plausible" to "verified" in this review.

🧩 Findings

Minor

1. The refresh handler writes fresh chips under the pre-refresh window key. packages/web/src/routes/github/github.tsx:227-241

The refresh mutation's onSuccess closes over issuePrNumbers as computed from the list before the refresh, and writes the re-fetched links to queryKeys.githubIssuePrs(issuePrNumbers). If the refresh changed the open-issue set, the component re-renders with a different window key; that key is cold, so React Query refetches it — without refresh=1 — and the server may answer from the ≤60 s cache the explicit refresh was meant to bust.

Why it does not block: in the scenario the flag exists for — an agent just opened a PR against an existing open issue — the issue list does not change, so the key is stable and the refresh lands. And when a PR closes its issue, the issue leaves the list entirely and has no row to chip. The sibling checks invalidation two lines above (invalidateQueries({ queryKey: queryKeys.githubChecks(checkPrNumbers) })) has the identical shape, so this is consistent with the established pattern rather than a new hazard.

Suggested fix if you want it airtight: derive the window from the data the mutation just received rather than from the closed-over value, so the key written matches the key the next render will read.

Nit

2. Row text is no longer mouse-selectable. packages/web/src/routes/github/github.tsx:624

The content wrapper is pointer-events-none so that hovering and clicking anywhere on the row still reaches the overlay link. A side effect is that the title and meta text can no longer be selected by dragging or double-clicking. In practice the content previously lived inside a draggable anchor, where selection was already awkward, so this is close to a no-op — and re-enabling selection would reintroduce exactly the hit-testing problem the structure solves. I checked that nothing interactive is caught by it: LabelChip and CommentCount are plain <span>s, and the chips opt back in with pointer-events-auto. Worth accepting knowingly rather than fixing.

3. The overflow chip's non-HTTP fallback drops its tooltip. packages/web/src/routes/github/github.tsx:744-748

The anchor branch carries both title and aria-label; the <span> fallback carries only aria-label, so a sighted mouse user loses the "N more linked pull requests" hover text in the (rare) case the issue URL is malformed. LinkedPrChip's own fallback keeps both. One attribute for symmetry.

4. Keep the two last: 30 tests together — they are only meaningful as a pair. packages/cezar/src/server/forge/github.test.ts

"still finds the PR behind 29 issue cross-references" feeds a reply that is already in last: order, so on its own it proves the parser scans the whole window rather than proving the window is the newest one. What actually pins the ordering is its neighbour, which asserts the emitted document contains timelineItems(last: 30 and not timelineItems(first:. Together they lock the behaviour; separately, either one is deletable as "redundant". A one-line comment on the first pointing at the second would protect them from a future cleanup.

💥 Breaking Changes

• No exported API, route, event name, CLI flag, DB schema or config format was removed, renamed or retyped. Every addition is new: linkedPrSchema, githubIssuePrsDataSchema, LinkedPr, GithubIssuePrsData, GH_ISSUE_PRS_MAX, fetchGithubIssuePrs, fetchIssuePrLinks, __clearIssuePrsCacheForTests, getGithubIssuePrs, useGithubIssuePrs, queryKeys.githubIssuePrs.
• The §2-protected surfaces are intact. githubItemSchema and the GET /api/v1/github list payload are byte-identical; bc-route-inventory.test.ts, route-parity.test.ts, versioned-surface.test.ts and typed-bodies.test.ts all pass.
• The new route is inventoried in BACKWARD_COMPATIBILITY.md §2 with its full contract, including the rule that an issue with no links is absent from the map rather than present-and-empty, and which future changes would be additive versus breaking.
• The route is registered by chaining into the githubRoutes family, not as a loose app.get(…) — so it reaches AppType and the typed client, which is the failure mode AGENTS.md § The HTTP API calls out and #694 shipped eleven times.
• Input is validated as route middleware (queryZodValidator), not parsed inside the handler, so the validated shape is recorded in the route type.
• Degradation paths are preserved, not narrowed. gh missing, no remote, offline and an unknown handle all answer 200 { available: false, reason }; a failed GraphQL chunk costs only its own issues; CEZ_DRY_RUN=1 returns a mock catalog so the offline demo and the e2e suite still paint chips. Nothing in this diff turns an expected absence into a throw or a 5xx.
• Bounds are explicit. The window is capped at GH_ISSUE_PRS_MAX = 100 at the route and again in the fetcher; the per-issue cache is bounded at 500 entries with LRU eviction; the timeline window is capped at 30 items per issue.
• No new dependencies — server-runtime budget untouched, no browser package added.

🧪 Test Coverage

Coverage is proportionate and, importantly, not vacuous — I checked the new assertions against the code they claim to pin.

• Forge (20 cases, forge/github.test.ts). Alias→issue mapping; all three state words plus the unknown-state fallback to closed; isDraft omitted rather than emitted as undefined (the exact JSON.stringify-drops-undefined trap AGENTS.md names, and the reason the contract's optional field survives parity); dedupe of a PR seen as both connected and cross-referenced; DisconnectedEvent subtraction; issue↔issue references dropped; the last:-window pair discussed in finding 4; chunking with one failing chunk; the empty-list and total-failure degrades; the cache, its refresh bypass, the "no links is cached too" case, and the CEZ_DRY_RUN mock.
• Route (9 cases, github-issue-prs-api.test.ts). The happy payload, absent-vs-empty, refresh=1, both 400 wordings kept distinct (missing issues query for ?issues= versus invalid issues query for a malformed list), the >100 cap, zero/negative/zero-padded numbers, and the project-scoped alias.
• Contract. GithubIssuePrs200 mutual assignability — a compile-time guard, so it is npm run typecheck that enforces it, exactly as the sibling GithubChecks200 does.
• Client (3 cases) and cockpit (9 cases). Chip text, the three tints, the draft variant, both link targets, the +N overflow, the unavailable payload, the PRs view never paying for the window, selected-issue pinning, and refresh=1 on the header refresh.
• The structural invariant has its own test, which is the right call for a change like this: github.test.tsx walks every anchor under [data-slot="gh-rows"] and asserts none has an anchor ancestor. That is what stops a future edit from quietly putting the chips back inside the row link — the failure this restructure exists to prevent, and one no type or lint rule in this repo would catch.
• E2E. packages/web/e2e/github.e2e.ts drives the real cockpit under CEZ_DRY_RUN=1, waits for the hydrated chips on issue #142 and screenshots the row. Enabled by the dry-run mock added in the same change, so the e2e layer is genuinely exercised rather than skipped.
• Regression risk on the restructure is covered behaviourally, not just structurally: the existing drag, hover-prefetch, aria-current, deep-link and row-click tests all still pass against the new DOM, and two selectors were rescoped from the row link to the row <li> because the meta glyphs are now its siblings — a rescope, not a weakened assertion.

Gaps I looked for and did not find a reason to file: the detail pane is explicitly out of scope per the spec's Non-goals, and the Phase 2 runs-store instant-paint is deferred by the same section.

✅ Verdict

Approve. No blockers, no majors; the full validation gate is green on the reviewed head, the GraphQL path is verified against the live forge, and the one structural edit is covered by a test that pins the invariant it exists to protect. The minor and the three nits are the author's call and do not block.

Two things that need a human with write access rather than a code change: applying the label set (403 for this account), and signing the CLA so license/cla can settle — it is the only check on this PR and is currently pending, unsigned.

[sheeerth]

🤖 om-auto-implement-spec → om-auto-create-pr — run summary

Status: complete. The spec at .ai/specs/2026-08-09-issue-linked-pr-chip.md is implemented, reviewed, and verified in a real browser. All 8 plan steps in .ai/runs/2026-08-14-issue-linked-pr-chip.md are checked, the full validation gate is green, and the code review found no blockers and no majors.

Engine: om-auto-create-pr (steps: 8, --loop: no)

The spec's Implementation Plan drafted to 8 Steps, comfortably under the configured 20-Step loop threshold and with no --loop flag passed, so the plain engine owned the run: one worktree, one commit per step or pair of steps, the validation gate, and a single review pass.

📋 What was built

Six of the eight steps are the vertical slice the spec designed, mirroring the lazy checks-glyph slice (#664) layer for layer:

1. Contract — linkedPrSchema and githubIssuePrsDataSchema in packages/contract/src/github.ts, a discriminated union on available. No §2-protected shape changed.
2. Forge — fetchGithubIssuePrs / fetchIssuePrLinks: one aliased GraphQL query per window over timelineItems(last: 30, itemTypes: [CONNECTED_EVENT, DISCONNECTED_EVENT, CROSS_REFERENCED_EVENT]), deduped by PR number, disconnections subtracted, ordered open → merged → closed. Bounded 60 s per-issue cache with a refresh bypass, a test seam, and a CEZ_DRY_RUN=1 mock.
3. Route — GET /api/v1/github/issue-prs, chained into the githubRoutes family (a loose app.get would vanish from AppType), inventoried in BACKWARD_COMPATIBILITY.md §2.
   4–5. Client + query hook — getGithubIssuePrs, queryKeys.githubIssuePrs, useGithubIssuePrs. packages/api-client deliberately untouched.
4. Row chip — the window derivation with selected-issue pinning, LinkedPrChip / LinkedPrOverflowChip, and the header refresh re-fetching with refresh=1.

The one structural edit: GithubRow becomes a stretched link. The row used to be a single <Link> wrapping everything, which a chip cannot live inside — an <a> within an <a> is invalid HTML and a WCAG 4.1.2 failure that this SSR-less SPA would emit silently. The <li> is now the positioning context, the row link an absolute inset-0 overlay carrying the navigation, drag payload and accessible name, and the chips are its siblings. No nested anchors, no stopPropagation.

🧪 Verification phases completed

Validation gate — every configured command, in order, on the reviewed head:

Command	Result
npm run typecheck	✅ clean (this is what enforces the contract-parity assertion)
npm test	✅ 6091 passed / 324 files
npm run test:unit	✅ 36 passed
npm run build	✅ + check:pack ok — 475 files
npm run test:package	✅ 15 passed

Code review — verdict approve, no blockers, no majors. The full report is in the review on this PR. Two verifications in it are worth repeating here because the gate could not have produced them:

• The GraphQL document was run against live GitHub, not just the mock. Every unit test injects runGraphql, and the fetcher deliberately swallows a failed chunk — so a malformed document would ship green and silently render no chips forever. The generated query was executed against open-mercato/cezar (issues Fix: collapse dense run metadata in the session header #765, Fix: GitHub tab search cannot find closed/merged issues & PRs — only the open set is ever fetched #730) and the real response fed back through fetchIssuePrLinks: it returned #765 → [873 open, 766 closed] and #730 → [732 open], correctly dropping the {"__typename":"Issue"} cross-references. The document, the aliasing, the itemTypes enum and the three-fragment field merging are all confirmed valid against the real schema.
• Findings were 1 minor and 3 nits, none blocking; one review follow-up commit landed (a comment whose stated rationale was backwards).

📸 UI verification — real Chrome against the built app. npm run test:e2e reported TEST_E2E_STATUS=skipped on this machine (the provider could not launch Chrome under the container's disabled user namespaces), which is explicitly not a pass — so the verification was performed directly instead: the production build was booted under CEZ_DRY_RUN=1 and driven in real Chrome with --no-sandbox.

What was confirmed in the live DOM, not in jsdom:

• Chips render with the right tint, text and target. Issue fix: surface tool-use loop exhaustion as a failure (#28) #142 → ↗ PR #128 (text-success, open) linking in-cockpit to /p/<project>/github/prs/128 because it is in the loaded open set, and ↗ PR #91 (text-violet, merged) linking out to GitHub with target="_blank" rel="noopener noreferrer". Issue fix: push thrown-step record into runRecords (#32) #135 → ↗ PR #77 (text-danger, closed) out to GitHub. Issue fix: persist assignees on issue sync in upsertIssue (#30) #139 → no chip at all.
• The nested-anchor invariant holds in the browser: 6 anchors under [data-slot="gh-rows"], 0 with an anchor ancestor.
• The stretched link behaves. The overlay measures exactly the row box (343×78), elementFromPoint over the title and meta returns the overlay (content pointer-events: none passes the hit through), and the chips read pointer-events: auto.
• Both click targets are correct. Clicking the chip navigated to /github/prs/128 and the detail pane rendered "Fix flaky auth test in CI"; clicking the row body navigated to /github/issues/139 and rendered "Add --json flag to cez CLI output". The chip wins its own click without stopPropagation, and the row is still clickable everywhere else.
• Title truncation and both themes survived the restructure (text-overflow: ellipsis intact; chips legible in dark and light).
• The route answers correctly on the real server: {available:true, links:{...}}, 400 {"error":"missing issues query"} for ?issues=, 400 {"error":"invalid issues query"} for a malformed list, and refresh=1 accepted.

Not covered: a mobile-width pass. The browser tool's viewport command failed here, so the 390 px layout was not exercised — the row structure is unchanged apart from one inline chip, but it is honestly unverified.

⚠️ Two things this run could not do — they need a maintainer

1. Labels were not applied. The authoring account has pull-only rights on this repository, so every label mutation answers HTTP 403. The set this PR should carry is review, feature, needs-qa, priority-medium, risk-medium — needs-qa because the change is user-facing, and risk-medium because the spec's own Risks section rates the implementation diff that way (the spec PR was risk-low because it shipped only a document). Until needs-qa is on the PR, the qaGate cannot hold the merge for QA sign-off. No qa-approved or qa-self-verified was added, and none should be — the screenshots above are evidence, not a QA sign-off.
2. The CLA is unsigned. license/cla is the only check on this PR and is pending because the author has not signed. That is a legal act this automation must not perform on anyone's behalf.

💥 Breaking changes

None. The slice is additive throughout, no §2-protected surface changed, and rollback is deleting it — the row falls back to exactly today's meta line, with no migration and no persisted state.

[sheeerth]

🤖 om-auto-implement-spec — 📸 mobile verification + ✅ CLA resolved

Two follow-ups since the run summary above. Both close gaps that summary was honest about leaving open.

📸 The 390 px layout is now verified

The earlier summary flagged mobile as unverified — the browser tool's viewport command was failing. The command was simply wrong (set viewport, not viewport); re-run at 390×844 in real Chrome against the same CEZ_DRY_RUN=1 build, the row holds up:

Check	Result
Viewport	innerWidth: 390
Chips stay inside the row	✅ chipsInsideRow: true for every row
Horizontal scroll introduced	✅ none (scrollWidth <= innerWidth)
Meta line wraps to a second line	✅ no — 78 px row height, unchanged from desktop
Overlay still covers the row exactly	✅ overlayExact: true on all three rows
Hit test over the title	✅ returns gh-row — the row is still clickable through the content

So the stretched-link restructure behaves identically at phone width, and the chips cost the row no extra height.

One observation, offered as a note rather than a finding: the chips render 16 px tall, below the 44 px touch-target guideline. That is not a regression this PR introduces — the whole existing meta line (comment count, checks glyph, the queued-run flag) is built at the same scale, and the chip is a secondary affordance beside a row that is itself one large tap target. Worth a separate pass over that line if the project wants to raise it; out of scope here.

✅ license/cla now passes — it was a commit-authorship problem

The summary listed the unsigned CLA as something needing a human. It turned out to be a defect in this run, not a missing signature.

Every commit was authored sheeerth <bartosz.gawronski@sherth.pl>, and GitHub maps that address to a different account (BananaGawron) — so although sheeerth opened the PR, the commits were credited elsewhere and the CLA bot correctly reported an unsigned contributor. The merged spec PR #816 had used Bartosz Gawroński <bggrafik66@gmail.com>, which is the identity tied to sheeerth.

With the author's approval the branch was rewritten to the correct identity and force-pushed. Verified before pushing: the resulting tree hash is byte-identical to the pre-rewrite tree (326e3016…), the diff against the old head is empty, and all author dates and commit messages are preserved — only the author and committer fields changed. Afterwards GitHub attributes all commits to sheeerth and license/cla reports SUCCESS.

The final commit refreshes the tracking plan, whose Progress section still pointed at the pre-rewrite shas.

🏷️ Still outstanding for a maintainer

Only one item remains, unchanged: labels cannot be applied from this account (pull-only rights → HTTP 403). The PR should carry review, feature, needs-qa, priority-medium, risk-medium. needs-qa matters most — with qaGate on, it is what holds the merge for QA sign-off. No qa-approved or qa-self-verified has been added and neither should be; the screenshots here are evidence, not a sign-off.

[sheeerth]

🔍 Code Review — re-review (supersedes the earlier approve)

🎯 Summary

A second, adversarial pass over this branch found two major correctness defects that the first review missed, and my earlier approve on this PR was therefore wrong. Both are now fixed in 35464f2c; this review records what was wrong, how it was proven, and what changed.

The first review's headline verification — running the GraphQL document against live GitHub — is what should have caught this. It ran the query over two valid issue numbers, got a clean result, and declared the path verified. It never tried an invalid one. That is the lesson worth keeping: a happy-path probe against a real service proves the document parses, not that the code survives what the service does when it is unhappy.

Verdict on the branch as it now stands: approve. Full gate green (6097 tests, up from 6091), both defects fixed with regression tests proven red without the fix.

🧩 What was wrong

Major 1 — one unresolvable number blanked every chip in the window

issue(number: N) does not answer null for a number that is not an issue. GitHub returns a NOT_FOUND entry in an errors array, and gh api graphql exits non-zero the moment that array is present — even when data carries a perfectly good answer for every other alias. runGraphql rejected, the per-chunk catch swallowed it, and the entire chunk was discarded.

Proven live against this repository — one batch, a real issue and a PR number:

gh exit code: 1
gh: Could not resolve to an Issue with the number of 885.
{"data":{"repository":{"i0":{"timelineItems":{"nodes":[…4 nodes…]}},"i1":null}},
 "errors":[{"type":"NOT_FOUND","path":["repository","i1"], …}]}

i0 came back complete and was thrown away.

Reachable in one click: the on-screen window pinned the URL-selected number, so /github/issues/885 — a PR number pasted or bookmarked into the issues route — cost every row its chips.

This is a trap the repository had already hit and documented. refStatusQuery carries the explanation verbatim: "asking issue(number:) about a pull request is a question with no answer, and GitHub says so with a NOT_FOUND error rather than a null… one mis-guessed kind used to fail the whole batch." This slice reintroduced the class its own sibling had cured.

Major 2 — a failed query was reported as a definitive "no linked PRs"

fetchIssuePrLinks swallowed every chunk failure internally, so it never threw and fetchGithubIssuePrs could not distinguish "we could not ask" from "there is nothing here". A rate limit, a network blip, or major 1 produced:

return { available: true, links: {} }   // …then cached for 60 s

{available: true, links: {}} is the shape BACKWARD_COMPATIBILITY.md §2 defines as "we asked, and none of these issues has a linked pull request" — the opposite of what happened — and the 60 s cache kept repeating it. The documented failure shape was unreachable for the case it exists for. CODE_REVIEW.md ranks graceful degradation second among review priorities; this was the inverse failure, an error dressed as a successful answer.

🔧 What changed in 35464f2c

• issueOrPullRequest(number:) with the timeline selected ... on Issue, matching refStatusQuery. A number that is a PR now resolves to a bare {__typename: 'PullRequest'} with no timeline and is skipped like any other absent answer. Verified live: the same mixed batch that used to exit 1 now returns gh exit=0, no errors array, every real issue answered.
• The window no longer pins a :n the list does not hold. That removes the remaining vector — a number GitHub never issued still errors the batch — at source, and costs nothing: with no row on screen there was never a chip to hydrate.
• Chunk failures propagate. fetchIssuePrLinks returns {links, failed, reason} (mirroring fetchRefStatuses, which keeps a failed list for exactly this reason). Failed numbers are not returned, not cached, and when the whole window fails the route answers {available: false, reason}. Recovery does not depend on the user pressing Refresh.
• Removed an unreachable /ENOENT/ branch whose actionable "install gh" hint could never fire, because resolveRepoHandle swallows the cause first. The handle failure now names the possibilities, and the ENOENT mapping moved to the one path where the message genuinely arrives.
• §2 updated to state that a failed query answers available: false and must never be reported as an empty map.

End-to-end live check of the fixed path, with a PR number sitting in a window of real issues:

gh exit=0
{ "links": { "730": [#732 open], "765": [#885 open, #873 open, #766 closed] }, "failed": [] }

Before the fix that same window returned nothing at all.

🧪 Test coverage — the gap that let this through

Every forge test injected runGraphql, so the failure semantics of the real transport were never exercised. The mock's failure modes are "the promise rejects" and "the alias is null"; gh's is "exit non-zero for the whole batch because one alias errored". Nothing in the suite could tell them apart. Worse, the unknown-issue test encoded i0: null — a reply GitHub never sends — so it passed against a fiction.

Added, each verified red without the fix via git stash push -- <source files> and green with it:

• a PR number in the window is skipped without costing its neighbours their chips;
• the emitted document uses the union and never issue(number:;
• a failed chunk is reported in failed with its reason, while surviving chunks keep their answers;
• total failure answers {available: false, reason}, not an empty map;
• a failed number is not cached — the next call re-asks, with no refresh needed;
• the window never pins a :n absent from the list.

🧪 Validation Gate

Command	Result
npm run typecheck	✅ pass
npm test	✅ 6097 passed / 324 files
npm run test:unit	✅ 36 passed
npm run build	✅ + check:pack ok — 475 files
npm run test:package	✅ 15 passed

💥 Breaking Changes

None. Still purely additive; no §2-protected surface changed. The §2 entry for this route gained precision about the failure shape — the route now behaves as documented, where before the documentation was aspirational.

✅ Verdict

Approve as of 35464f2c. The remaining open items are unchanged and unrelated to correctness: the earlier minor about the refresh handler's window key, two cosmetic nits, and the label set this account cannot apply (review, feature, needs-qa, priority-medium, risk-medium — HTTP 403 on pull-only rights).

[sheeerth]

🤖 om-auto-qa-pr — QA evidence (linked-PR chips)

Verdict: functionally PASS. Verified end-to-end against a live cockpit booted under CEZ_DRY_RUN=1 (base merged to current main, worktree at c1ff4799). A live-app screenshot could not be captured in this sandbox — see the environment note at the end — but the feature is exercised through its real HTTP surface, its browser-DOM component tests, and the passing gate.

🔌 Live endpoint (running server, GET /api/v1/github/issue-prs)

• ?issues=142 → {"142":[{"number":128,…,"state":"open"},{"number":91,…,"state":"merged"}]} — multi-chip, open + merged.
• ?issues=142,139,135 → adds "135":[{"number":77,"state":"closed"}]; fix: persist assignees on issue sync in upsertIssue (#30) #139 is absent (no linked PR → no chip). All three states + the "absent means no chip" rule confirmed.
• ?issues= → 400 (missing issues query); ?issues=<101 numbers> → 400 (invalid issues query). Cap + validation confirmed.

🧪 Automated evidence (this branch, merged to main)

• 403/403 feature tests pass: contract/src/github.ts, forge/github.test.ts (dedup, state map, DisconnectedEvent subtraction, last-window, degrade), github-issue-prs-api.test.ts (400s, happy, refresh), web/src/api/client.test.ts, and routes/github/github.test.tsx — which asserts chip text, tint (incl. draft), href/route target, that clicking a chip navigates to the PR and not the row's issue, and no chip when empty/unavailable.
• Typecheck green, including contract-parity.github.test.ts (GithubIssuePrs200 schema↔handler exactness).
• The purpose-built browser spec web/e2e/github.e2e.ts ("an issue with a linked PR paints its chip on the row (docs(specs): Linked-PR chips on the GitHub Issues list #816)") exists and drives a real browser; it skipped here only because the browser provider could not be provisioned (below).

🎨 Visual

The proposed-UI mockup (state-tinted chips, +N overflow, no-PR row) is attached on the design PR #816; it matches the shipped stretched-link-overlay implementation.

⚠️ Environment note

This sandbox cannot launch a fresh headless Chrome (Chrome exited early without writing DevToolsActivePort, even with --no-sandbox) — the e2e provider gate marks the browser unavailable and skips (non-blocking by the runner's contract), and a manual agent-browser drive failed the same way across 4 retries. So no live screenshot was captured here; re-running om-auto-qa-pr on a browser-capable machine will produce github-issue-linked-pr-chip.png. Nothing about the feature failed — only the capture environment.

Note: needs-qa would apply (user-facing change) but I'm pull-only on this repo and cannot set labels.