Attach a node to a GitHub issue or pull request (#462 phase 3a)

[Temikus]

Phase 3a of #462: a node can be attached to a GitHub issue or pull request, explicitly.

The rule this is built on: a link exists only because the user made it. Nothing is inferred from terminal output, a transcript, or a branch name — so there is no path here that can put a wrong chip on someone's canvas.

What to review

• Storage is node data. CanvasNodeState.github?: GitHubLink[] (kind + number + a title snapshot). The repository is the project's and is never stored per link, so the same committed project.json resolves correctly in any clone. It is git-shared content, so sanitizeNodeGitHubLinks runs on every crossing in both directions, exactly like sanitizeNodeTriggers — plus at the canvas-sync inbound hook.
• lookup reports refusals as VALUES, not throws. Electron invoke and the ws-bridge's RpcErr both flatten a typed code, and the picker has to render not-approved as a disabled row rather than as a failure.
• search is a method, not a columnId sentinel on query. A sentinel would collide with a real column id, leak into the counts keys and the relay jail's shape, and force every existing query caller to handle a third state. query and search share the new column-blind candidates, so query's output is unchanged — its existing tests pass untouched.
• getIssue still refuses a pull request. moveIssue relies on that guard; getIssueOrPull is an opt-in sibling for the one caller that wants either kind.
• One write funnel. setNodeGitHubLinks in Canvas does the node write, the dirty mark and the board-log event together; every surface (node menu, group frame, board card, card modal, metadata strip) reaches it through a module bridge, the same indirection setWorktreeActionHandler uses.

Canvas chips deliberately hold no host subscription and do not refresh while mounted: a chip looks its item up when painted (reusing an answer under 5 minutes old), and a board open re-seeds them. Host-side, lookup single-flights per repository+number and memoises API answers for a minute. Documented; a later phase can hold a subscription while a chip is visible.

Surfaces: Desktop and Server Edition both work (the handlers are registered by registerGitHubIntegration, which both shells boot). The Omni (all-projects) board works too: each lane's chips, card modal and card menu act on that lane's project. Relay guests are jailed to the shared project: lookup/search have rows in relay-project-scope.ts, added in the same commit as the channels. Mobile: N/A — the agent-status mirror carries no node data. cc @eneskirca for the mobile call.

Tests

New: shared/github-link.test.ts, core/workspace-files.github-links.test.ts, renderer/state/workspace.github-links.test.ts, renderer/lib/githubLinks.test.ts, renderer/state/githubLinks.test.ts, GitHubLinkChip.test.tsx, GitHubLinkPicker.test.tsx, CardMetaBar.test.tsx.

Extended: node-exec, client, service, handlers, relay-host, relay-project-scope, KanbanView.accountMenu, toKanbanSession, board-log-panel, ui-visibility.

npm run typecheck clean. Failures in this checkout are environmental only (node-pty not built, jsdom localStorage, BSD mv) and in files this PR does not touch.

Not in this PR

A reverse indicator on the GitHub card, and an agent-facing canvas-control attach verb — both land on this same write funnel later. Device verification of the approved-repository flow is still owed.

[eneskirca]

Review: CHANGES-NEEDED — short list, and the design is right

I merged this into current main in a scratch worktree. Nine files conflicted (Canvas.tsx, KanbanView.tsx, CardModal.tsx, CardMetaBar.tsx, SessionCard.tsx, workspace-files.ts, styles.css, ui-visibility.test.ts, CONTRIBUTING.md) — 16 hunks, all additive except workspace-files.ts, where main grew a sessionId string re-check beside your sanitizeNodeGitHubLinks. After resolving, both tsc --noEmit projects are clean and everything in the area is green: your eight new suites (58 tests), plus core/github, relay-host, workspace-files, state/workspace, toKanbanSession, ui-visibility — 317 more, all passing.

One integration gap the merge surfaced, not a defect in what you wrote: main has since added GlobalKanbanView.tsx (the omni swimlane board), which renders CardModal and therefore needs the two new props. That is a compile error on rebase; I stubbed it locally to run the suites.

Overlap with what shipped: none. This extends the same service.

I checked specifically for a second link concept, a second cache, or a second client, and there is none of the three. search lands on a new private candidates() that query is itself refactored onto — same snapshot, same mapping, same fold rules, so query's output is genuinely unchanged and its existing tests pass untouched. lookup reads the existing GitHubIssueCache first and otherwise goes through the existing GitHubIssuesClient / readWithEpoch / GitHubRequestCoordinator. getIssueOrPull is a sibling of getIssue, whose pull refusal (load-bearing for moveIssue) is untouched. The kanban card's issue association is a different relation — which column is this issue in — and you did not touch it.

Three things I want to name because they are the best decisions in the PR:

• The shared file carries content, not identity. No repository slug, no URL, no project id in the link — just {kind, number, title?}, resolved against the reading project's own kanban.github.repository. A committed project.json is byte-identical on every machine and a clone resolves against its own repo. That is exactly the rule, applied without being asked.
• Links live on the node, so there is no dangling-link problem and no prune to write — strictly better than copying the pruneAssignments / cardMeta shape.
• Sanitisation is applied in all four crossings (projectToFile, fileToProject, sanitizeLoadedClosedSessions, sanitizeInboundNode), rebuilding the object from three known keys so a smuggled field cannot ride in, with bad kind/number dropping the entry and a bad title dropping only the title. The sanitizeNodeTriggers precedent, matched.

Claims I checked against the code and can confirm: query output unchanged; getIssue still refuses a pull request; refusals reach the picker as values and render as a disabled row (with a test); the single setNodeGitHubLinks funnel doing node write + markDirty + board-log event together; and the relay jail extended in the same commit, with relay-host.test.ts proving a cross-project call is refused before dispatch. Writing that jail rule into CONTRIBUTING.md as well is the right instinct.

Authentication is unchanged and correct: everything goes through GitHubCredentialResolver (the gh CLI or a PAT in the platform secret store), resolved host-side, and no new call touches a token — they take captured.client, which already carries the header. Nothing in argv, nothing logged.

---

Required before merge

1. Rebase. mergeable_state is dirty; main is 304 commits ahead and 23 of your files have moved. Re-run typecheck and the suite afterwards, and close the GlobalKanbanView gap above.

2. PLAN-462-phase3.md (342 lines) is committed at the repo root. Worktree frames suggest their open pull request (#462 phase 3b) #625 removes it, but Attach a node to a GitHub issue or pull request (#462 phase 3a) #624 merges first, so it would land on main. Drop it here.

3. Bound lookup. It is relay-reachable and it reaches the network, and it has nothing equivalent to REFRESH_MIN_INTERVAL_MS — the floor whose comment 20 lines above it in the same file says why it exists ("reachable from the renderer AND — for a shared project — from a relay guest, and nothing else bounds it"). Only 404s are memoised, so a number that exists but is not in the snapshot (an evicted pull under pullsTruncated, or anything past the caps) costs one API call per call, forever; and with no host-side single-flight, N chips on the same uncached number are N calls. The coordinator's 4-way cap turns this into queuing rather than a 429 storm, which is real mitigation, but it still spends the account's hourly budget at whatever rate a renderer asks. A per-(repo, number) in-flight map plus a short positive memo closes it.

4. Record a back-off in ensureCard for failed / configuration-changed. The store records gate for not-approved / not-authenticated and missing for not-found, but nothing for those two and nothing in the catch — so while offline every chip remount re-issues its lookup with no delay.

Worth fixing while you are in there

5. A kind-mismatched link retries forever. ensureCard caches under linkKey(the item's own kind), which is correct, but the guard at the top reads cards[projectId][linkKey(the requested kind)]. If a persisted link says issue#7 and GitHub says refactor(usage): move the usage service into core so Server Edition serves it #7 is a pull, nothing is ever written under the asked key and no missing entry is recorded, so every mount re-asks and the dot stays unknown. Only reachable from a hand-edited project.json — but it is the one unbounded-retry hole in an otherwise carefully bounded store.
6. invalidate() does not re-trigger a fetch. ChipBody's effect deps are [api, projectId, links], which invalidate does not change, so the chip renders unknown until remount. It self-heals in practice because invalidate only fires while the board is open and the board's reload re-seeds via seedFromPages — worth a comment saying so, since that is not obvious. pending is not cleared either.
7. rememberLookupMiss is fire-and-forget async, re-resolving cacheContext in a detached promise, so two concurrent chips on the same missing number both reach the API before the memo lands.
8. The canvas detach submenu always shows the stale title — linkToBoardTitle(link) is called without a card in the node and group menu builders, while the chip menu passes cardFor(link). Same label, two freshness stories.
9. docs/github-issues-kanban.md overclaims chip freshness — "refreshed when it is first painted and then at most every 5 minutes" reads as a 5-minute refresh. There is no refresh while mounted; the TTL only gates the next ensureCard call. Your PR body states this correctly, so it is the doc that should move.

Not verified here: the approved-repository flow on device, which your description already lists as owed.

This is the cleanest external contribution in this area I have reviewed — it extends the shipped service rather than shadowing it, obeys the explicit-link rule from #462 exactly, and its tests are load-bearing rather than decorative. The list above is small and none of it touches the design.

[Temikus]

@eneskirca Thanks for the review! Addressed all of it, PTAL:

1. Rebased onto main. Omni board now wires the new CardModal props for real: lanes scope their chips via a GitHubLinkProject context, and the funnel takes a projectId (active project goes to React Flow, others to the store). lookup/search are in relay-project-scope.ts.
2. PLAN-462-phase3.md is gone from history.
3. lookup single-flights per (repo, number) and memoises API answers for 60s.
4. ensureCard backs off 15s on failed / configuration-changed / a rejected call.
5. Kind mismatch records a miss under the asked key.
6. invalidate() clears pending.
7. The miss memo is written inside the flight.
8. Detach rows use the cached card title.
9. Doc sentence fixed.