feat(api,web): server-resolved PR/issue titles for the connected-work rail#282
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds GitHub App authentication and KV caching for issue/PR titles, exposes a member-scoped batch API, and asynchronously repaints connected-work rail labels with resolved titles while preserving expansion state and ignoring stale responses. ChangesGitHub title resolution
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-api | e5566ad | Commit Preview URL Branch Preview URL |
Jul 20 2026, 02:25 PM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-web | f483dea | Commit Preview URL Branch Preview URL |
Jul 20 2026, 01:44 PM |
Zach Dunn (zachdunn)
added
the
coderabbit:review
…re (#267) - share one lazily-minted home-installation token across a resolveTitles batch (kills the cold-cache N-way JWT/token-mint stampede) - reuse secrets.ts b64url codec in github-app.ts instead of a second copy - export GITHUB_TITLES_MAX_REFS from api-client; rail imports it instead of redeclaring the cap - hoist the shared GitHub App test env fixture
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
apps/api/src/routes/me.ts (1)
539-568: 🚀 Performance & Scalability | 🔵 TrivialConsider throttling the batch title endpoint against the shared home-installation quota.
Up to 20 refs per request, each capable of issuing 2 outbound GitHub calls on a cold cache, all draw from the single shared home-installation token/rate-limit bucket used across every workspace. There's no per-user/workspace throttle here beyond membership — worth considering if this becomes a hot path, though it's consistent with other read routes in this file that also skip rate limiting.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/routes/me.ts` around lines 539 - 568, Consider adding throttling for the github-titles route around resolveTitles, using the existing shared home-installation quota mechanism so requests are limited per user or workspace before issuing outbound GitHub calls. Preserve the current membership validation, batch-size validation, and per-reference null behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/github-app.ts`:
- Around line 78-132: The GitHub API calls in installationForRepo and
installationToken lack request timeouts, allowing resolveTitles to hang on slow
responses. Add the same AbortController/timeout mechanism used by fetchIssue to
both fetchImpl calls, pass the abort signal, and preserve the existing
null-return behavior when requests time out or fail.
- Line 1: Add a shared AbortController-based timeout helper and use it for the
fetchImpl calls in installationForRepo, installationToken, and fetchIssue,
applying the same fail-fast duration to each outbound GitHub request. Ensure the
timeout signal is passed to fetchImpl and abort resources are cleaned up after
completion, while preserving existing response and error handling.
In `@apps/api/src/github-titles.ts`:
- Around line 33-46: Add timeout-based cancellation to the fetch request in
fetchIssue, using an AbortController and passing its signal to fetchImpl. Ensure
the controller aborts after the configured timeout and is cleaned up when the
request completes, while preserving the existing error outcome behavior.
- Around line 86-97: Update the fallback logic in the surrounding fetch flow so
a failed home installationToken attempt still proceeds to installationForRepo
and retries fetchIssue with the repo-specific token. Track the home-token
failure distinctly from the initial error outcome, while preserving the existing
no-access fallback and negative-cache behavior for genuine terminal errors.
In `@apps/web/src/lib/api-client.ts`:
- Around line 550-552: Strengthen the response validation before returning from
the refs-parsing flow: require body.refs to be a non-array record, and validate
every entry as either null or an object with string title/state fields and a
valid kind value. Return null for any malformed entry, including arrays or
non-string titles, and only cast to GithubTitleMap after all entries pass
validation.
In `@apps/web/src/lib/workspace-rail.ts`:
- Around line 147-154: Remove the HTMLButtonElement generic from the
querySelector call in the rail expansion listener. Use the project’s Worker-safe
selector abstraction or structural typing for the selected control, while
preserving the existing click handler and once-only listener behavior.
---
Nitpick comments:
In `@apps/api/src/routes/me.ts`:
- Around line 539-568: Consider adding throttling for the github-titles route
around resolveTitles, using the existing shared home-installation quota
mechanism so requests are limited per user or workspace before issuing outbound
GitHub calls. Preserve the current membership validation, batch-size validation,
and per-reference null behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6edbcece-f81e-42f6-8e8e-d3181755d511
📒 Files selected for processing (14)
apps/api/src/env.d.tsapps/api/src/github-app.test.tsapps/api/src/github-app.tsapps/api/src/github-titles.test.tsapps/api/src/github-titles.tsapps/api/src/routes/github-titles-route.test.tsapps/api/src/routes/me.tsapps/api/test/fake-kv.tsapps/api/wrangler.jsoncapps/web/src/lib/api-client.tsapps/web/src/lib/gh-context.test.tsapps/web/src/lib/gh-context.tsapps/web/src/lib/workspace-rail.test.tsapps/web/src/lib/workspace-rail.ts
| @@ -0,0 +1,132 @@ | |||
| /** | |||
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
No timeout on outbound GitHub API calls in the request path. installationForRepo, installationToken (github-app.ts), and fetchIssue (github-titles.ts) all call fetchImpl with no AbortController/timeout. All three run synchronously inside resolveTitles, which the new /workspaces/:name/github-titles route awaits before responding — a slow or hanging GitHub response ties up the request with no fail-fast guard, and there's no shared coalescing to bound worst-case latency for a 20-ref batch.
apps/api/src/github-app.ts#L78-132: wrap thefetchImplcalls ininstallationForRepoandinstallationTokenwith anAbortController-based timeout (e.g., 5-10s).apps/api/src/github-titles.ts#L33-46: apply the same timeout tofetchIssue'sfetchImplcall, ideally via a shared helper so all three sites stay consistent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/github-app.ts` at line 1, Add a shared AbortController-based
timeout helper and use it for the fetchImpl calls in installationForRepo,
installationToken, and fetchIssue, applying the same fail-fast duration to each
outbound GitHub request. Ensure the timeout signal is passed to fetchImpl and
abort resources are cleaned up after completion, while preserving existing
response and error handling.
| export async function installationForRepo( | ||
| env: Env, | ||
| cfg: GithubAppConfig, | ||
| repo: string, | ||
| fetchImpl: typeof fetch = fetch, | ||
| ): Promise<number | null> { | ||
| const key = `ghinst:${repo}`; | ||
| const cached = (await env.GITHUB_CACHE.get(key)) as string | null; | ||
| if (cached !== null) return cached === "none" ? null : Number(cached); | ||
| try { | ||
| const res = await fetchImpl(`https://api.github.com/repos/${repo}/installation`, { | ||
| headers: githubHeaders(await appJwt(cfg)), | ||
| }); | ||
| if (res.status === 404) { | ||
| await env.GITHUB_CACHE.put(key, "none", { expirationTtl: INSTALL_TTL }); | ||
| return null; | ||
| } | ||
| if (!res.ok) return null; | ||
| const body = (await res.json()) as { id?: number }; | ||
| if (typeof body.id !== "number") return null; | ||
| await env.GITHUB_CACHE.put(key, String(body.id), { expirationTtl: INSTALL_TTL }); | ||
| return body.id; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Short-lived installation token, KV-cached below its 60-minute life. Tokens | ||
| * are read-only (App permissions) and expire server-side, so KV storage is an | ||
| * accepted trade-off (spec §Components). | ||
| */ | ||
| export async function installationToken( | ||
| env: Env, | ||
| cfg: GithubAppConfig, | ||
| installationId: number, | ||
| fetchImpl: typeof fetch = fetch, | ||
| ): Promise<string | null> { | ||
| const key = `ghtok:${installationId}`; | ||
| const cached = (await env.GITHUB_CACHE.get(key)) as string | null; | ||
| if (cached !== null) return cached; | ||
| try { | ||
| const res = await fetchImpl( | ||
| `https://api.github.com/app/installations/${installationId}/access_tokens`, | ||
| { method: "POST", headers: githubHeaders(await appJwt(cfg)) }, | ||
| ); | ||
| if (res.status !== 201) return null; | ||
| const body = (await res.json()) as { token?: string }; | ||
| if (typeof body.token !== "string") return null; | ||
| await env.GITHUB_CACHE.put(key, body.token, { expirationTtl: TOKEN_TTL }); | ||
| return body.token; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
No timeout on GitHub API calls used in the request path.
installationForRepo and installationToken call fetchImpl with no AbortController/timeout. Both run synchronously inside resolveTitles, which the new /workspaces/:name/github-titles route awaits before responding — a slow/hanging GitHub response ties up the request with no fail-fast guard. Same root cause as fetchIssue in apps/api/src/github-titles.ts; see the consolidated comment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/github-app.ts` around lines 78 - 132, The GitHub API calls in
installationForRepo and installationToken lack request timeouts, allowing
resolveTitles to hang on slow responses. Add the same AbortController/timeout
mechanism used by fetchIssue to both fetchImpl calls, pass the abort signal, and
preserve the existing null-return behavior when requests time out or fail.
| async function fetchIssue( | ||
| repo: string, | ||
| num: string, | ||
| token: string, | ||
| fetchImpl: typeof fetch, | ||
| ): Promise<FetchOutcome> { | ||
| let res: Response; | ||
| try { | ||
| res = await fetchImpl(`https://api.github.com/repos/${repo}/issues/${num}`, { | ||
| headers: githubHeaders(token), | ||
| }); | ||
| } catch { | ||
| return { kind: "error" }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
No timeout on the GitHub issue fetch.
fetchIssue calls fetchImpl with no AbortController/timeout, same gap as installationForRepo/installationToken in apps/api/src/github-app.ts. All three run synchronously in the /workspaces/:name/github-titles request path. See consolidated comment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/github-titles.ts` around lines 33 - 46, Add timeout-based
cancellation to the fetch request in fetchIssue, using an AbortController and
passing its signal to fetchImpl. Ensure the controller aborts after the
configured timeout and is cleaned up when the request completes, while
preserving the existing error outcome behavior.
| let negativeTtl = NEGATIVE_TTL; | ||
| let outcome: FetchOutcome = { kind: "error" }; | ||
| const homeToken = await installationToken(env, cfg, Number(cfg.homeInstallationId), fetchImpl); | ||
| if (homeToken) outcome = await fetchIssue(repo, num, homeToken, fetchImpl); | ||
|
|
||
| if (outcome.kind === "no-access") { | ||
| const installId = await installationForRepo(env, cfg, repo, fetchImpl); | ||
| if (installId !== null) { | ||
| const instToken = await installationToken(env, cfg, installId, fetchImpl); | ||
| if (instToken) outcome = await fetchIssue(repo, num, instToken, fetchImpl); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fallback ladder skips repo-installation retry when the home token mint itself fails.
outcome only reaches "no-access" when fetchIssue actually ran with the home token. If installationToken(...) for the home installation returns null (transient mint failure — network error, non-201), outcome stays { kind: "error" } from its initial value, and the outcome.kind === "no-access" check on line 91 never triggers — so the repo-specific installation is never tried, even for refs with a legitimate, cached install. The ref then gets negative-cached for up to an hour on a purely transient blip, for both public and private repos.
🐛 Proposed fix
- if (outcome.kind === "no-access") {
+ if (outcome.kind === "no-access" || !homeToken) {
const installId = await installationForRepo(env, cfg, repo, fetchImpl);Want me to add a test case for "home token mint fails, repo install succeeds" alongside the fix?
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let negativeTtl = NEGATIVE_TTL; | |
| let outcome: FetchOutcome = { kind: "error" }; | |
| const homeToken = await installationToken(env, cfg, Number(cfg.homeInstallationId), fetchImpl); | |
| if (homeToken) outcome = await fetchIssue(repo, num, homeToken, fetchImpl); | |
| if (outcome.kind === "no-access") { | |
| const installId = await installationForRepo(env, cfg, repo, fetchImpl); | |
| if (installId !== null) { | |
| const instToken = await installationToken(env, cfg, installId, fetchImpl); | |
| if (instToken) outcome = await fetchIssue(repo, num, instToken, fetchImpl); | |
| } | |
| } | |
| let negativeTtl = NEGATIVE_TTL; | |
| let outcome: FetchOutcome = { kind: "error" }; | |
| const homeToken = await installationToken(env, cfg, Number(cfg.homeInstallationId), fetchImpl); | |
| if (homeToken) outcome = await fetchIssue(repo, num, homeToken, fetchImpl); | |
| if (outcome.kind === "no-access" || !homeToken) { | |
| const installId = await installationForRepo(env, cfg, repo, fetchImpl); | |
| if (installId !== null) { | |
| const instToken = await installationToken(env, cfg, installId, fetchImpl); | |
| if (instToken) outcome = await fetchIssue(repo, num, instToken, fetchImpl); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/github-titles.ts` around lines 86 - 97, Update the fallback
logic in the surrounding fetch flow so a failed home installationToken attempt
still proceeds to installationForRepo and retries fetchIssue with the
repo-specific token. Track the home-token failure distinctly from the initial
error outcome, while preserving the existing no-access fallback and
negative-cache behavior for genuine terminal errors.
| const body = (await result.response.json().catch(() => null)) as { refs?: unknown } | null; | ||
| if (!body || typeof body !== "object" || !body.refs || typeof body.refs !== "object") return null; | ||
| return body.refs as GithubTitleMap; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate each refs entry before casting.
Arrays and entries such as { title: 1 } pass this check, despite the documented malformed-response fallback. That can propagate a non-string label into rail rendering. Require a non-array record whose entries are null or have string title/state and a valid kind; otherwise return null.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/lib/api-client.ts` around lines 550 - 552, Strengthen the
response validation before returning from the refs-parsing flow: require
body.refs to be a non-array record, and validate every entry as either null or
an object with string title/state fields and a valid kind value. Return null for
any malformed entry, including arrays or non-string titles, and only cast to
GithubTitleMap after all entries pass validation.
| list.querySelector<HTMLButtonElement>("[data-rail-more]")?.addEventListener( | ||
| "click", | ||
| () => { | ||
| expanded = true; | ||
| paint(items.length); | ||
| }, | ||
| { once: true }, | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Avoid the newly introduced DOM-library generic.
Line 147 adds HTMLButtonElement, which conflicts with the Workers TypeScript rule. Use the project’s Worker-safe abstraction or structural typing for the selected control instead.
As per coding guidelines, “do not rely on DOM library types in this Workers codebase.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/lib/workspace-rail.ts` around lines 147 - 154, Remove the
HTMLButtonElement generic from the querySelector call in the rail expansion
listener. Use the project’s Worker-safe selector abstraction or structural
typing for the selected control, while preserving the existing click handler and
once-only listener behavior.
Source: Coding guidelines
- 8s AbortSignal deadline on every outbound GitHub call (githubFetch helper) - retry via the repo installation when the home token fails to mint, so a transient mint blip can't negative-cache a resolvable ref for an hour - per-entry shape validation of the github-titles response in the web client
|
Addressed the review in f483dea:
Skipped — The batch-throttling nitpick is covered by the home-token memoization added in c965f42 (one lazily-minted token per |
The exact-PR-match banner rendered the bare gh.ref for files without an attach-time gh.title, never calling the title resolver that PR #282 wired into the right rail. Resolve titles once per listing in WorkspaceFileTable and share the ref->title map across both the banner and the rail, dropping the rail's duplicate fetch. Falls back to the bare ref on resolver miss. Closes #285
1 participant
Implements phase 1 of the GitHub App integration (design + plan in
.context/267-*, session-local).ghinst:/ghtok:in a newGITHUB_CACHEnamespace); ref→title resolution over a token ladder (home installation for public repos → the repo's own installation for private), with TTL'd positive/negative caching and rate-limit-aware backoff; member-gated batch endpointGET /me/workspaces/:name/github-titles(≤20 refs, per-ref nulls never fail the batch).gh.title/ref), then swaps in server-resolved titles via a generation-guarded repaint that preserves show-more expansion. Every failure path degrades to today's labels.gh.titlestamping remains the offline seed. No changeset (api/web are changeset-ignored).Verified in the local portless stack: with no App secrets the rail is byte-identical to today (endpoint returns nulls); with a cached title the rail swaps the label in place.
Operator steps before this is live in prod (all code degrades safely until then): register the GitHub App (
issues:read,pull_requests:read, webhooks off), install onbuildinternet,wrangler kv namespace create GITHUB_CACHE+ fill the id inapps/api/wrangler.jsonc(currentlyTBD_BY_OPERATOR), setGITHUB_APP_PRIVATE_KEY(PKCS#8) /GITHUB_APP_ID/GITHUB_APP_HOME_INSTALLATION_ID.Closes #267
Before / after (local stack,
dev-demofiles tab)Summary by CodeRabbit
New Features
Bug Fixes
Tests