From 15dfe14c77213744157c7b0746dd168f9950a5e8 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 29 Aug 2026 23:19:07 -0700 Subject: [PATCH 1/5] Stop the repo picker from bursting GitHub's search API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Listing repos cost one `/search/issues` call per repo, fired concurrently, purely to decorate each row with an exact open-PR count. GitHub allows 30 search requests a minute and secondary-rate-limits bursts, so any account past a handful of repos got a 403, `listRepos` rejected, and the connect card fell into its error state — every time, so the connection looked permanently broken. The picker now reads from the one list call it was already making. The open-PR count is gone; the push timestamp that explains the ordering rides along on that same response for free. --- packages/chat-ui/src/api.ts | 2 +- packages/chat/src/blocks.test.ts | 2 +- packages/github-tools/package.json | 2 +- packages/github-tools/src/repos.test.ts | 38 ++++++---- packages/github-tools/src/repos.ts | 69 +++++++------------ .../src/connect-github-routes.test.ts | 4 +- .../src/connect-github-setup.test.ts | 7 +- workflows/code-review/src/index.ts | 2 +- workflows/last-30-days-research/src/index.ts | 2 +- .../test/definition.test.ts | 2 +- 10 files changed, 61 insertions(+), 69 deletions(-) diff --git a/packages/chat-ui/src/api.ts b/packages/chat-ui/src/api.ts index 6a0f31415..2a6e39f62 100644 --- a/packages/chat-ui/src/api.ts +++ b/packages/chat-ui/src/api.ts @@ -1236,7 +1236,7 @@ export function patchWorkbenchSettings( const ConnectGithubRepoResponse = type({ id: "string", name: "string", - openPullRequestCount: "number", + "lastPushedAt?": "string", }); const ConnectGithubStateResponse = type({ kind: "'disconnected'" }) diff --git a/packages/chat/src/blocks.test.ts b/packages/chat/src/blocks.test.ts index 4f2bef521..d3e3844e1 100644 --- a/packages/chat/src/blocks.test.ts +++ b/packages/chat/src/blocks.test.ts @@ -414,7 +414,7 @@ describe("parseBlock connect-github", () => { requiredForTemplate: "github", state: "connected", orgName: "octocat", - repos: [{ id: "1", name: "acme/widgets", openPullRequestCount: 99 }], + repos: [{ id: "1", name: "acme/widgets" }], }, }); expect(result.ok).toBe(true); diff --git a/packages/github-tools/package.json b/packages/github-tools/package.json index 9d270daea..de8d8a1e5 100644 --- a/packages/github-tools/package.json +++ b/packages/github-tools/package.json @@ -2,7 +2,7 @@ "name": "@corbits/github-tools", "private": true, "description": "GitHub search integration: a minimal REST client and an @intx/agent tool bundle exposing github_activity", - "version": "0.0.8", + "version": "0.0.9", "license": "LGPL-2.1-or-later", "type": "module", "exports": { diff --git a/packages/github-tools/src/repos.test.ts b/packages/github-tools/src/repos.test.ts index d652119ff..94e52c843 100644 --- a/packages/github-tools/src/repos.test.ts +++ b/packages/github-tools/src/repos.test.ts @@ -13,34 +13,37 @@ function fakeFetch(handler: (url: string) => Promise): typeof fetch { handler(String(input))) as unknown as typeof fetch; } -test("listRepos maps each repo and fills in its own open-PR count", async () => { +test("listRepos reads the whole picker from one list call, never a per-repo search", async () => { const requested: string[] = []; const fetchImpl = fakeFetch((url) => { requested.push(url); if (url.includes("/user/repos")) { return Promise.resolve( jsonResponse([ - { id: 1, full_name: "acme/widgets" }, - { id: 2, full_name: "acme/gadgets" }, + { + id: 1, + full_name: "acme/widgets", + pushed_at: "2026-08-29T12:00:00Z", + }, + { id: 2, full_name: "acme/gadgets", pushed_at: null }, ]), ); } - if (url.includes("repo%3Aacme%2Fwidgets")) { - return Promise.resolve(jsonResponse({ total_count: 3 })); - } - if (url.includes("repo%3Aacme%2Fgadgets")) { - return Promise.resolve(jsonResponse({ total_count: 0 })); - } throw new Error(`unstubbed request: ${url}`); }); const repos = await listRepos({ apiKey: "tok", baseUrl: BASE, fetchImpl }); expect(repos).toEqual([ - { id: "1", name: "acme/widgets", openPullRequestCount: 3 }, - { id: "2", name: "acme/gadgets", openPullRequestCount: 0 }, + { + id: "1", + name: "acme/widgets", + lastPushedAt: "2026-08-29T12:00:00Z", + }, + { id: "2", name: "acme/gadgets" }, ]); - expect(requested.some((url) => url.includes("/user/repos"))).toBe(true); + expect(requested).toHaveLength(1); + expect(requested.some((url) => url.includes("/search/"))).toBe(false); }); test("listRepos throws when the repos response doesn't match the expected shape", async () => { @@ -52,6 +55,17 @@ test("listRepos throws when the repos response doesn't match the expected shape" ); }); +test("listRepos names the status when GitHub rejects the list call", async () => { + const fetchImpl = fakeFetch(() => + Promise.resolve( + new Response("nope", { status: 403, statusText: "Forbidden" }), + ), + ); + await expect(listRepos({ baseUrl: BASE, fetchImpl })).rejects.toThrow( + /403 Forbidden/, + ); +}); + test("fetchAuthenticatedLogin reads the PAT's own login", async () => { const fetchImpl = fakeFetch((url) => { expect(url).toContain("/user"); diff --git a/packages/github-tools/src/repos.ts b/packages/github-tools/src/repos.ts index bafe0d58d..c0401f525 100644 --- a/packages/github-tools/src/repos.ts +++ b/packages/github-tools/src/repos.ts @@ -1,13 +1,13 @@ -// The connect-github card's repo picker (CL-6345): the authenticated -// user's own repositories, plus one honestly-limited open-PR count per -// repo. GitHub's `/user/repos` list has no open-PR count on the row -// itself, so getting one costs a second call per repo — the search API's -// `total_count`, which is exact and free of pagination, unlike walking -// `/pulls` pages. That is an N+1 fetch (one list call, one search call -// per repo returned); acceptable for the picker's own repo count (a -// handful of repos, not thousands), but a caller listing an org with -// hundreds of repos will feel it. A cheaper batched count is future work, -// not something this module fakes today. +// The connect-github card's repo picker (CL-6345): the repositories the +// connected token can reach, most recently pushed first. +// +// One call, on purpose. This used to decorate every row with an exact +// open-PR count from the search API, which cost one `/search/issues` call +// per repo fired concurrently — and GitHub's search API allows 30 requests +// a minute and secondary-rate-limits bursts, so any account past a handful +// of repos got a 403 and the whole picker failed to load (CL-7189). The +// count was decoration on a checkbox row; the push timestamp that explains +// the ordering rides along on the list response for free. import { type } from "arktype"; import type { GitHubClientConfig } from "./client"; @@ -15,18 +15,19 @@ import type { GitHubClientConfig } from "./client"; const GitHubUserRepo = type({ id: "number", full_name: "string", + "pushed_at?": "string | null", }); const GitHubUserReposResponse = GitHubUserRepo.array(); -const GitHubSearchTotalCount = type({ total_count: "number" }); - const DEFAULT_BASE_URL = "https://api.github.com"; const PER_PAGE = 100; export interface GitHubRepoSummary { readonly id: string; readonly name: string; - readonly openPullRequestCount: number; + /** When GitHub last saw a push, ISO-8601 — absent on a repo that has + * never been pushed to. */ + readonly lastPushedAt?: string; } function headers(apiKey: string | undefined): Record { @@ -55,29 +56,11 @@ async function fetchJSON( return response.json(); } -async function openPullRequestCountOf( - config: GitHubClientConfig, - base: string, - fullName: string, -): Promise { - const url = new URL(`${base}/search/issues`); - url.searchParams.set("q", `repo:${fullName} is:pr is:open`); - url.searchParams.set("per_page", "1"); - const raw = await fetchJSON(config, url); - const parsed = GitHubSearchTotalCount(raw); - if (parsed instanceof type.errors) { - throw new Error( - `GitHub open-PR count response did not match the expected shape: ${parsed.summary}`, - ); - } - return parsed.total_count; -} - /** * Lists the repositories the connected credential can see, most - * recently pushed first, with each repo's own open-PR count filled in. - * Throws on any transport, HTTP, or shape failure; the connect-github - * card's own host catches at its render boundary. + * recently pushed first. Throws on any transport, HTTP, or shape + * failure; the connect-github card's own host catches at its render + * boundary. */ export async function listRepos( config: GitHubClientConfig, @@ -95,17 +78,13 @@ export async function listRepos( ); } - return Promise.all( - repos.map(async (repo) => ({ - id: String(repo.id), - name: repo.full_name, - openPullRequestCount: await openPullRequestCountOf( - config, - base, - repo.full_name, - ), - })), - ); + return repos.map((repo) => ({ + id: String(repo.id), + name: repo.full_name, + ...(typeof repo.pushed_at === "string" + ? { lastPushedAt: repo.pushed_at } + : {}), + })); } const GitHubAuthenticatedUser = type({ login: "string" }); diff --git a/packages/workflow-catalog/src/connect-github-routes.test.ts b/packages/workflow-catalog/src/connect-github-routes.test.ts index 1074a8c6a..1677dd00a 100644 --- a/packages/workflow-catalog/src/connect-github-routes.test.ts +++ b/packages/workflow-catalog/src/connect-github-routes.test.ts @@ -44,8 +44,8 @@ const allowAll: RequireGrant = () => async (_c, next) => { }; const REPOS: readonly GitHubRepoSummary[] = [ - { id: "1", name: "acme/widgets", openPullRequestCount: 2 }, - { id: "2", name: "acme/gadgets", openPullRequestCount: 0 }, + { id: "1", name: "acme/widgets" }, + { id: "2", name: "acme/gadgets" }, ]; function mountAs(routes: Hono): Hono { diff --git a/packages/workflow-catalog/src/connect-github-setup.test.ts b/packages/workflow-catalog/src/connect-github-setup.test.ts index f7aee456d..275a8d620 100644 --- a/packages/workflow-catalog/src/connect-github-setup.test.ts +++ b/packages/workflow-catalog/src/connect-github-setup.test.ts @@ -8,9 +8,9 @@ import { import type { GitHubRepoSummary } from "@corbits/github-tools"; const REPOS: readonly GitHubRepoSummary[] = [ - { id: "1", name: "acme/widgets", openPullRequestCount: 2 }, - { id: "2", name: "acme/gadgets", openPullRequestCount: 0 }, - { id: "3", name: "acme/sprockets", openPullRequestCount: 5 }, + { id: "1", name: "acme/widgets" }, + { id: "2", name: "acme/gadgets" }, + { id: "3", name: "acme/sprockets" }, ]; function fakePorts() { @@ -196,7 +196,6 @@ describe("webhookTriggerName", () => { const repo: GitHubRepoSummary = { id: "1", name: "acme/widgets", - openPullRequestCount: 0, }; expect(webhookTriggerName(repo)).toBe("acme/widgets pull-request-opened"); }); diff --git a/workflows/code-review/src/index.ts b/workflows/code-review/src/index.ts index b0a6b4e92..77df3f416 100644 --- a/workflows/code-review/src/index.ts +++ b/workflows/code-review/src/index.ts @@ -43,7 +43,7 @@ export const CODE_REVIEW_STEP_ID = "code-review"; /** The tool packages this definition pins: GitHub reach, nothing else. */ export const CODE_REVIEW_TOOL_PACKAGE_PINS: readonly ToolPackagePin[] = [ - { name: "@corbits/github-tools", version: "0.0.8" }, + { name: "@corbits/github-tools", version: "0.0.9" }, ]; /** Binds the pinned package's "github" handle to the tenant's connection. */ diff --git a/workflows/last-30-days-research/src/index.ts b/workflows/last-30-days-research/src/index.ts index 20df4ca38..226ee7c66 100644 --- a/workflows/last-30-days-research/src/index.ts +++ b/workflows/last-30-days-research/src/index.ts @@ -145,7 +145,7 @@ export const LAST_30_DAYS_RESEARCH_PENDING_SOURCES = [ export const LAST_30_DAYS_RESEARCH_TOOL_PACKAGE_PINS: readonly ToolPackagePin[] = [ { name: "@corbits/web-search-tools", version: "0.0.3" }, - { name: "@corbits/github-tools", version: "0.0.8" }, + { name: "@corbits/github-tools", version: "0.0.9" }, ]; const SYSTEM_PROMPT = [ diff --git a/workflows/last-30-days-research/test/definition.test.ts b/workflows/last-30-days-research/test/definition.test.ts index 424c44ce6..38be28cc1 100644 --- a/workflows/last-30-days-research/test/definition.test.ts +++ b/workflows/last-30-days-research/test/definition.test.ts @@ -58,7 +58,7 @@ test("the step carries an explicit per-turn timeout, no inline tools, and the tw expect(only.agent.capabilities).toEqual([]); expect(LAST_30_DAYS_RESEARCH_TOOL_PACKAGE_PINS).toEqual([ { name: "@corbits/web-search-tools", version: "0.0.3" }, - { name: "@corbits/github-tools", version: "0.0.8" }, + { name: "@corbits/github-tools", version: "0.0.9" }, ]); expect(only.agent.toolPackagePins).toEqual( LAST_30_DAYS_RESEARCH_TOOL_PACKAGE_PINS, From 3660c08d90cc873b82f9f6ff94e1ce40d7da7765 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 29 Aug 2026 23:19:07 -0700 Subject: [PATCH 2/5] Say what a personal access token actually scopes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The walkthrough read Connect GitHub, then Pick your repos, and the picker's helper called each repo "its own permission". That describes a GitHub App, where repo selection happens after install. We ship a personal access token: repo access is decided while the token is being created, and picking a repo afterwards grants nothing. The connect step now walks a person through a fine-grained token scoped to the repositories they want reviewed, and the picker is honestly a watch-list over what that token already reaches — "Choose what gets reviewed", with narrowing pointed back at GitHub where it really happens. --- .../src/blocks/connect-github-block.tsx | 21 ++++--- packages/chat-ui/src/relative-time.ts | 17 ++++++ packages/chat-ui/src/strings.ts | 27 +++++---- packages/chat-ui/src/timeline.tsx | 15 +---- .../test/connect-github-block.test.tsx | 58 ++++++++++--------- .../chat-ui/test/connect-github-flow.test.tsx | 10 ++-- .../test/onboarding-scene-card.test.tsx | 32 +++++----- packages/workflow-catalog/src/templates.ts | 6 +- 8 files changed, 106 insertions(+), 80 deletions(-) create mode 100644 packages/chat-ui/src/relative-time.ts diff --git a/packages/chat-ui/src/blocks/connect-github-block.tsx b/packages/chat-ui/src/blocks/connect-github-block.tsx index 786fcc440..c7575bc4a 100644 --- a/packages/chat-ui/src/blocks/connect-github-block.tsx +++ b/packages/chat-ui/src/blocks/connect-github-block.tsx @@ -14,20 +14,23 @@ // mode) rather than a hand-rolled input -- AGENTS.md puts generic controls // upstream in react-ui, not here. Its corner radius is react-ui's rounded // default, a visual delta from the mock's flat radius-0 system; the row -// layout around it (name left, open-PR count right) is workbench-specific +// layout around it (name left, last-push time right) is workbench-specific // composition and stays local. import { useEffect, useRef, useState } from "react"; import { Button, Checkbox, Input } from "@corbits/react-ui"; import { Check } from "@corbits/icons"; +import { formatRelativeActivity } from "../relative-time"; import { CHAT_STRINGS } from "../strings"; import { BlockCard } from "./block-card"; export type ConnectGithubRepo = { readonly id: string; readonly name: string; - readonly openPullRequestCount: number; + /** When GitHub last saw a push, ISO-8601 — absent on a repo with no + * commits yet. It is what the picker's ordering is explaining. */ + readonly lastPushedAt?: string; }; /** One labelled step of the room's walkthrough, as the workbench's own @@ -113,10 +116,14 @@ export type ConnectGithubCardProps = ConnectGithubCardBody & { readonly scene: OnboardingScene; }; -function repoMetaLabel(openPullRequestCount: number): string { - return openPullRequestCount === 0 - ? CHAT_STRINGS.blockConnectGithubNoOpenPulls - : CHAT_STRINGS.blockConnectGithubOpenPulls(openPullRequestCount); +function repoMetaLabel(lastPushedAt: string | undefined): string { + if (lastPushedAt === undefined) { + return CHAT_STRINGS.blockConnectGithubRepoNeverPushed; + } + const relative = formatRelativeActivity(lastPushedAt); + return relative === "" + ? CHAT_STRINGS.blockConnectGithubRepoNeverPushed + : CHAT_STRINGS.blockConnectGithubRepoUpdated(relative); } type StepState = "done" | "current" | "upcoming"; @@ -425,7 +432,7 @@ function ConnectedBody({ {repo.name} - {repoMetaLabel(repo.openPullRequestCount)} + {repoMetaLabel(repo.lastPushedAt)} diff --git a/packages/chat-ui/src/relative-time.ts b/packages/chat-ui/src/relative-time.ts new file mode 100644 index 000000000..d0096efdd --- /dev/null +++ b/packages/chat-ui/src/relative-time.ts @@ -0,0 +1,17 @@ +/** How long ago a timestamp was, in the compact form the chat surfaces + * use for activity: "just now", "12m ago", "3h ago", "5d ago". An + * absent or unparseable timestamp renders as nothing rather than a + * placeholder date. */ +export function formatRelativeActivity(iso: string | null): string { + if (iso === null) return ""; + const date = new Date(iso); + if (Number.isNaN(date.getTime())) return ""; + const deltaMs = Date.now() - date.getTime(); + const minutes = Math.round(deltaMs / 60_000); + if (minutes < 1) return "just now"; + if (minutes < 60) return `${minutes}m ago`; + const hours = Math.round(minutes / 60); + if (hours < 24) return `${hours}h ago`; + const days = Math.round(hours / 24); + return `${days}d ago`; +} diff --git a/packages/chat-ui/src/strings.ts b/packages/chat-ui/src/strings.ts index 4af67fb48..0e9e0a8d9 100644 --- a/packages/chat-ui/src/strings.ts +++ b/packages/chat-ui/src/strings.ts @@ -215,7 +215,7 @@ export const CHAT_STRINGS = { blockQuestionSubmitting: "Sending…", blockQuestionAnswerError: "Couldn't send your answer — try again.", blockQuestionAnsweredLabel: "Your answer", - blockConnectGithubPickHeadline: "Pick your repos", + blockConnectGithubPickHeadline: "Choose what gets reviewed", blockConnectGithubStepDone: "Done", blockConnectGithubStepCurrent: "You're here", blockConnectGithubReviewingHeadline: "Reviewing", @@ -223,36 +223,39 @@ export const CHAT_STRINGS = { "Every new pull request in these gets a review posted right here.", blockConnectGithubChangeRepos: "change repos", blockConnectGithubIntro: - "Connect GitHub with a personal access token — three quick steps, about a minute.", + "Connect GitHub with a personal access token. You choose which repositories the token can reach while you're creating it — that's exactly what the reviewers will be able to read.", blockConnectGithubAction: "Connect GitHub", blockConnectGithubReconnect: "Reconnect", blockConnectGithubTokenSteps: [ - "Open github.com/settings/tokens and generate a new token.", - "Give it the repo scope — that lets agents read code, issues, and pull requests.", + "Open GitHub's fine-grained token page and generate a new token.", + "Under Repository access, select the repositories you want reviewed — nothing outside that list is ever reachable with this token.", + "Under Repository permissions, set Contents, Issues, and Pull requests to Read and write, so the reviewers can read your diffs and post back on them.", "Paste it here. It's stored encrypted, only your agents use it, and you can remove it any time.", ] as readonly string[], - blockConnectGithubTokenSettingsUrl: "https://github.com/settings/tokens", - blockConnectGithubTokenSettingsLink: "Open github.com/settings/tokens", + blockConnectGithubTokenSettingsUrl: + "https://github.com/settings/personal-access-tokens/new", + blockConnectGithubTokenSettingsLink: "Open GitHub's token page", blockConnectGithubTokenHelper: "Your token is stored encrypted, only your agents use it, and you can remove it any time.", blockConnectGithubConnectedAs: (org: string) => `Connected to GitHub as ${org}`, blockConnectGithubChange: "change", blockConnectGithubRepoCount: (found: number, picked: number) => - `${found} repos found · ${picked} picked`, + `${found} your token can reach · ${picked} picked`, blockConnectGithubSelectAll: "Select all", - blockConnectGithubNoOpenPulls: "no open pull requests", - blockConnectGithubOpenPulls: (count: number) => - count === 1 ? "1 open pull request" : `${count} open pull requests`, + blockConnectGithubRepoUpdated: (relative: string) => `updated ${relative}`, + blockConnectGithubRepoNeverPushed: "no commits yet", blockConnectGithubPermissionHelper: - "Picking a repo lets the reviewers post reviews to it — each repo is its own permission, and you can turn any off later.", + "These are the repositories your token can reach. Pick the ones you want reviewed — you can change the list any time, and narrowing what the token itself can reach is done back on GitHub.", blockConnectGithubStartReviewing: (count: number) => `Start reviewing ${count} repo${count === 1 ? "" : "s"}`, blockConnectGithubSkip: "skip for now", blockConnectGithubStartReviewingError: "Couldn't start reviewing — try again.", + blockConnectGithubStateUnreadable: + "Couldn't reach GitHub with your token just now — try connecting again.", blockConnectGithubTokenFieldLabel: "Personal access token", - blockConnectGithubTokenFieldPlaceholder: "ghp_...", + blockConnectGithubTokenFieldPlaceholder: "github_pat_...", blockConnectGithubTokenSubmit: "Connect", blockConnectGithubTokenSubmitting: "Connecting…", blockConnectGithubTokenCancel: "cancel", diff --git a/packages/chat-ui/src/timeline.tsx b/packages/chat-ui/src/timeline.tsx index 31caa8b1d..85cbf6c3e 100644 --- a/packages/chat-ui/src/timeline.tsx +++ b/packages/chat-ui/src/timeline.tsx @@ -66,6 +66,7 @@ import { WorkbenchLoadingState } from "./loading-state"; import { Markdown } from "./markdown"; import type { ProfileSubject } from "./profile-subject"; import { profileSubjectFromParticipant } from "./profile-subject"; +import { formatRelativeActivity } from "./relative-time"; import { CHAT_STRINGS } from "./strings"; /** @@ -1801,20 +1802,6 @@ export type ThreadAffordanceMeta = { readonly participantAddresses: readonly string[]; }; -function formatRelativeActivity(iso: string | null): string { - if (iso === null) return ""; - const date = new Date(iso); - if (Number.isNaN(date.getTime())) return ""; - const deltaMs = Date.now() - date.getTime(); - const minutes = Math.round(deltaMs / 60_000); - if (minutes < 1) return "just now"; - if (minutes < 60) return `${minutes}m ago`; - const hours = Math.round(minutes / 60); - if (hours < 24) return `${hours}h ago`; - const days = Math.round(hours / 24); - return `${days}d ago`; -} - function ThreadAffordance({ messageId, meta, diff --git a/packages/chat-ui/test/connect-github-block.test.tsx b/packages/chat-ui/test/connect-github-block.test.tsx index 1e020a023..477cce109 100644 --- a/packages/chat-ui/test/connect-github-block.test.tsx +++ b/packages/chat-ui/test/connect-github-block.test.tsx @@ -23,13 +23,15 @@ import type { } from "../src/blocks/connect-github-block"; import { ConnectGithubBlockView } from "../src/blocks/connect-github-block"; +const AN_HOUR_AGO = new Date(Date.now() - 60 * 60_000).toISOString(); + const REPOS: readonly ConnectGithubRepo[] = [ - { id: "checkout", name: "acme/checkout", openPullRequestCount: 4 }, - { id: "billing-api", name: "acme/billing-api", openPullRequestCount: 2 }, - { id: "web", name: "acme/web", openPullRequestCount: 7 }, - { id: "mobile", name: "acme/mobile", openPullRequestCount: 1 }, - { id: "design-tokens", name: "acme/design-tokens", openPullRequestCount: 0 }, - { id: "handbook", name: "acme/handbook", openPullRequestCount: 0 }, + { id: "checkout", name: "acme/checkout", lastPushedAt: AN_HOUR_AGO }, + { id: "billing-api", name: "acme/billing-api" }, + { id: "web", name: "acme/web" }, + { id: "mobile", name: "acme/mobile" }, + { id: "design-tokens", name: "acme/design-tokens" }, + { id: "handbook", name: "acme/handbook" }, ]; /** The framing the room's onboarding card always carries. Individual @@ -40,7 +42,10 @@ const SCENE: OnboardingScene = { promise: "Every new pull request gets reviewed before you merge it.", steps: [ { title: "Connect GitHub", why: "Reviewers need to read your code." }, - { title: "Pick your repos", why: "Only the repos you pick are watched." }, + { + title: "Choose what gets reviewed", + why: "Of the repos your token reaches, these get watched.", + }, { title: "Start reviewing", why: "Reviews land right here in this room." }, ], currentStepIndex: 0, @@ -129,7 +134,7 @@ describe("connect GitHub card — 2a disconnected", () => { // Honest PAT-first framing — there is no hosted GitHub sign-in in // this card, so it never claims an app install it can't do. expect(el.textContent).toContain( - "Connect GitHub with a personal access token — three quick steps, about a minute.", + "You choose which repositories the token can reach while you're creating it", ); expect(el.textContent).toContain( "stored encrypted, only your agents use it, and you can remove it any time", @@ -166,14 +171,16 @@ describe("connect GitHub card — 2a disconnected", () => { const steps = [...el.querySelectorAll(".chat-block-connect-steps li")].map( (item) => item.textContent, ); - expect(steps).toHaveLength(3); + expect(steps).toHaveLength(4); expect(steps[0]).toContain( - "Open github.com/settings/tokens and generate a new token.", + "Open GitHub's fine-grained token page and generate a new token.", ); - expect(steps[1]).toContain("repo scope"); - expect(steps[2]).toContain("Paste it here"); + expect(steps[1]).toContain("Repository access"); + expect(steps[3]).toContain("Paste it here"); expect( - el.querySelector('a[href="https://github.com/settings/tokens"]'), + el.querySelector( + 'a[href="https://github.com/settings/personal-access-tokens/new"]', + ), ).not.toBeNull(); const field = el.querySelector("#connect-github-token"); @@ -271,17 +278,16 @@ describe("connect GitHub card — 2b pick your repos", () => { }); expect(el.textContent).toContain("Connected to GitHub as acme"); - expect(el.textContent).toContain("6 repos found · 3 picked"); + expect(el.textContent).toContain("6 your token can reach · 3 picked"); const rows = el.querySelectorAll(".chat-block-connect-repo-row"); expect(rows).toHaveLength(6); expect(el.textContent).toContain("acme/checkout"); - expect(el.textContent).toContain("4 open pull requests"); - expect(el.textContent).toContain("1 open pull request"); - expect(el.textContent).toContain("no open pull requests"); + expect(el.textContent).toContain("updated 1h ago"); + expect(el.textContent).toContain("no commits yet"); expect(el.textContent).toContain( - "Picking a repo lets the reviewers post reviews to it — each repo is its own permission, and you can turn any off later.", + "These are the repositories your token can reach. Pick the ones you want reviewed", ); }); @@ -321,7 +327,7 @@ describe("connect GitHub card — 2b pick your repos", () => { />, ); - expect(el.textContent).toContain("6 repos found · 3 picked"); + expect(el.textContent).toContain("6 your token can reach · 3 picked"); const start = [...el.querySelectorAll("button")].find((button) => button.textContent?.startsWith("Start reviewing"), ) as HTMLButtonElement; @@ -334,7 +340,7 @@ describe("connect GitHub card — 2b pick your repos", () => { mobileCheckbox?.click(); }); - expect(el.textContent).toContain("6 repos found · 4 picked"); + expect(el.textContent).toContain("6 your token can reach · 4 picked"); const startAfter = [...el.querySelectorAll("button")].find((button) => button.textContent?.startsWith("Start reviewing"), ) as HTMLButtonElement; @@ -354,7 +360,7 @@ describe("connect GitHub card — 2b pick your repos", () => { />, ); - expect(el.textContent).toContain("6 repos found · 0 picked"); + expect(el.textContent).toContain("6 your token can reach · 0 picked"); const start = [...el.querySelectorAll("button")].find((button) => button.textContent?.startsWith("Start reviewing"), ) as HTMLButtonElement; @@ -372,7 +378,7 @@ describe("connect GitHub card — 2b pick your repos", () => { selectAll.click(); }); - expect(el.textContent).toContain("6 repos found · 6 picked"); + expect(el.textContent).toContain("6 your token can reach · 6 picked"); const startAfter = [...el.querySelectorAll("button")].find((button) => button.textContent?.startsWith("Start reviewing"), ) as HTMLButtonElement; @@ -434,7 +440,7 @@ describe("connect GitHub card — accessibility", () => { const group = el.querySelector('[role="group"]'); expect( el.querySelector(".chat-block-scene-pick-heading")?.textContent, - ).toBe("Pick your repos"); + ).toBe("Choose what gets reviewed"); expect(group?.getAttribute("aria-labelledby")).toBe( "connect-github-pick-heading", ); @@ -534,7 +540,7 @@ describe("connect GitHub card — accessibility", () => { const status = el.querySelector(".chat-block-scene-status"); const alert = el.querySelector('[role="alert"]'); - expect(status?.textContent).toBe("Pick your repos"); + expect(status?.textContent).toBe("Choose what gets reviewed"); expect(alert?.textContent).toContain("Couldn't start reviewing"); expect(status?.contains(alert)).toBe(false); expect( @@ -562,7 +568,7 @@ describe("connect GitHub card — accessibility", () => { }); expect(status?.textContent).toBe("Connect GitHub"); - expect(el.textContent).toContain("6 repos found · 2 picked"); + expect(el.textContent).toContain("6 your token can reach · 2 picked"); }); test("autoFocus on the pick-repos scene moves focus onto the pick heading", async () => { @@ -580,7 +586,7 @@ describe("connect GitHub card — accessibility", () => { }); const heading = el.querySelector(".chat-block-scene-pick-heading"); - expect(heading?.textContent).toBe("Pick your repos"); + expect(heading?.textContent).toBe("Choose what gets reviewed"); expect(heading?.getAttribute("tabindex")).toBe("-1"); expect(document.activeElement).toBe(heading); }); diff --git a/packages/chat-ui/test/connect-github-flow.test.tsx b/packages/chat-ui/test/connect-github-flow.test.tsx index b7678342a..9bebeea59 100644 --- a/packages/chat-ui/test/connect-github-flow.test.tsx +++ b/packages/chat-ui/test/connect-github-flow.test.tsx @@ -29,10 +29,10 @@ import type { import { WorkbenchTimeline } from "../src/timeline"; const REPOS: readonly ConnectGithubRepo[] = [ - { id: "1", name: "acme/checkout", openPullRequestCount: 4 }, - { id: "2", name: "acme/billing-api", openPullRequestCount: 0 }, - { id: "3", name: "acme/web", openPullRequestCount: 7 }, - { id: "4", name: "acme/mobile", openPullRequestCount: 1 }, + { id: "1", name: "acme/checkout" }, + { id: "2", name: "acme/billing-api" }, + { id: "3", name: "acme/web" }, + { id: "4", name: "acme/mobile" }, ]; function messageWithConnectGithubBlock(): MessageItem[] { @@ -226,7 +226,7 @@ describe("connect-github round trip (CL-6345)", () => { checkboxes[1]?.click(); checkboxes[2]?.click(); }); - expect(el.textContent).toContain("4 repos found · 3 picked"); + expect(el.textContent).toContain("4 your token can reach · 3 picked"); const getConnectStateCallCountBeforeStart = harness.getConnectStateCallCount(); diff --git a/packages/chat-ui/test/onboarding-scene-card.test.tsx b/packages/chat-ui/test/onboarding-scene-card.test.tsx index f0c8fc5a3..13f4b1cf4 100644 --- a/packages/chat-ui/test/onboarding-scene-card.test.tsx +++ b/packages/chat-ui/test/onboarding-scene-card.test.tsx @@ -22,15 +22,18 @@ import { WorkbenchTimeline } from "../src/timeline"; const STEPS: { title: string; why: string }[] = [ { title: "Connect GitHub", why: "Reviewers need to read your code." }, - { title: "Pick your repos", why: "Only the repos you pick are watched." }, + { + title: "Choose what gets reviewed", + why: "Of the repos your token reaches, these get watched.", + }, { title: "Start reviewing", why: "Reviews land right here in this room." }, ]; const PROMISE = "Every new pull request gets reviewed before you merge it."; const REPOS: readonly ConnectGithubRepo[] = [ - { id: "1", name: "acme/checkout", openPullRequestCount: 2 }, - { id: "2", name: "acme/web", openPullRequestCount: 0 }, + { id: "1", name: "acme/checkout" }, + { id: "2", name: "acme/web" }, ]; let container: HTMLDivElement | null = null; @@ -151,7 +154,7 @@ describe("the room's onboarding card is a scene, not a member's message", () => ); expect(stepTitles(el)).toEqual([ "Connect GitHub", - "Pick your repos", + "Choose what gets reviewed", "Start reviewing", ]); }); @@ -201,7 +204,10 @@ describe("the room's onboarding card is a scene, not a member's message", () => })} />, ); - expect(stepTitles(el)).toEqual(["Connect GitHub", "Pick your repos"]); + expect(stepTitles(el)).toEqual([ + "Connect GitHub", + "Choose what gets reviewed", + ]); expect(currentStepTitle(el)).toBeUndefined(); expect(el.querySelector(".chat-block-scene-why")).toBeNull(); }); @@ -250,8 +256,8 @@ describe("the walkthrough marker follows the live connect state", () => { repos: REPOS, selectedRepoIds: [], }); - expect(currentStepTitle(el)).toBe("Pick your repos"); - expect(currentStepAria(el)).toBe("Pick your repos"); + expect(currentStepTitle(el)).toBe("Choose what gets reviewed"); + expect(currentStepAria(el)).toBe("Choose what gets reviewed"); expect(el.querySelector(".chat-block-scene-why")?.textContent).toBe( STEPS[1]?.why, ); @@ -292,11 +298,11 @@ describe("the walkthrough marker follows the live connect state", () => { await act(async () => { changeRepos?.click(); }); - expect(el.textContent).toContain("2 repos found · 1 picked"); + expect(el.textContent).toContain("2 your token can reach · 1 picked"); expect(el.querySelector(".chat-block-title")?.textContent).toBe( "Code review", ); - expect(currentStepTitle(el)).toBe("Pick your repos"); + expect(currentStepTitle(el)).toBe("Choose what gets reviewed"); expect(el.querySelector(".chat-block-scene-why")?.textContent).toBe( STEPS[1]?.why, ); @@ -326,7 +332,7 @@ describe("the walkthrough marker follows the live connect state", () => { await act(async () => { changeRepos?.click(); }); - expect(el.textContent).toContain("2 repos found · 1 picked"); + expect(el.textContent).toContain("2 your token can reach · 1 picked"); const start = [...el.querySelectorAll("button")].find((button) => button.textContent?.startsWith("Start reviewing"), @@ -338,13 +344,13 @@ describe("the walkthrough marker follows the live connect state", () => { }); expect(el.querySelector(".chat-block-scene-reviewing")).toBeNull(); - expect(el.textContent).toContain("2 repos found · 1 picked"); - expect(currentStepTitle(el)).toBe("Pick your repos"); + expect(el.textContent).toContain("2 your token can reach · 1 picked"); + expect(currentStepTitle(el)).toBe("Choose what gets reviewed"); const alert = el.querySelector('[role="alert"]'); expect(alert).not.toBeNull(); expect(alert?.textContent).toContain("Couldn't start reviewing"); const status = el.querySelector(".chat-block-scene-status"); - expect(status?.textContent).toBe("Pick your repos"); + expect(status?.textContent).toBe("Choose what gets reviewed"); expect(status?.contains(alert)).toBe(false); expect(report).toHaveBeenCalled(); expect(report.mock.calls[0]?.[1]).toMatchObject({ diff --git a/packages/workflow-catalog/src/templates.ts b/packages/workflow-catalog/src/templates.ts index ac0636819..2600f11d1 100644 --- a/packages/workflow-catalog/src/templates.ts +++ b/packages/workflow-catalog/src/templates.ts @@ -348,12 +348,12 @@ export const CODE_REVIEW_TEMPLATE: WorkbenchDefinition = { kind: "connect-plugin", connectorId: "github", title: "Connect GitHub", - why: "The reviewers need it to read your diffs and post what they'd change.", + why: "Create a token scoped to the repositories you want reviewed — that is what the reviewers will be able to read.", }, { kind: "pick-github-repos", - title: "Pick your repos", - why: "Only the repositories you choose get reviewed.", + title: "Choose what gets reviewed", + why: "Of the repositories your token reaches, these are the ones a new pull request starts a review in.", }, { kind: "start-webhook-trigger", From 94e251609f003a2bd8d23520409d2a4da18b6c14 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 29 Aug 2026 23:19:17 -0700 Subject: [PATCH 3/5] Report a failed connect read instead of showing Connect GitHub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The card's mount effect read its state with no rejection path, so a read that threw left the query on `loading` — which renders the disconnected body. A person with a working GitHub connection was told they had never connected one, with a Connect button that could only lead them through the same failure again. A rejected read now becomes the card's own error state, reported through the error sink with a refId, and says plainly that GitHub could not be reached with this token. --- .../blocks/connect-github-block-container.tsx | 23 ++++++++++-- .../connect-github-block-container.test.tsx | 35 ++++++++++++++++--- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/packages/chat-ui/src/blocks/connect-github-block-container.tsx b/packages/chat-ui/src/blocks/connect-github-block-container.tsx index 177a309e3..8bef95f41 100644 --- a/packages/chat-ui/src/blocks/connect-github-block-container.tsx +++ b/packages/chat-ui/src/blocks/connect-github-block-container.tsx @@ -60,6 +60,25 @@ function lastConnectedOf(messageId: string): ConnectedGithubQuery | undefined { return lastConnectedByMessageId.get(messageId); } +/** A state read that fails is an error the card says out loud, never a + * silent fall-through to "Connect GitHub" over a connection that + * exists (CL-7189): an unhandled rejection used to leave the query on + * `loading`, which renders the disconnected body. */ +async function readConnectState( + actions: ConnectGithubActions, + messageId: string, +): Promise { + try { + return await actions.getConnectState(messageId); + } catch (cause) { + reportError(cause, { operation: "connect-github.getConnectState" }); + return { + kind: "error", + message: CHAT_STRINGS.blockConnectGithubStateUnreadable, + }; + } +} + function displayQueryOf( messageId: string, query: ConnectGithubQuery, @@ -120,7 +139,7 @@ export function ConnectGithubBlockContainer({ useEffect(() => { if (actions === undefined) return; - actions.getConnectState(messageId).then(applyQuery); + void readConnectState(actions, messageId).then(applyQuery); const unsubscribe = actions.subscribeConnectState(messageId, applyQuery); return unsubscribe; }, [actions, messageId, applyQuery]); @@ -137,7 +156,7 @@ export function ConnectGithubBlockContainer({ } const result = await actions.submitAccessToken(token); if (result.ok) { - applyQuery(await actions.getConnectState(messageId)); + applyQuery(await readConnectState(actions, messageId)); } return result; }, diff --git a/packages/chat-ui/test/connect-github-block-container.test.tsx b/packages/chat-ui/test/connect-github-block-container.test.tsx index 796bf9d71..27d57d37c 100644 --- a/packages/chat-ui/test/connect-github-block-container.test.tsx +++ b/packages/chat-ui/test/connect-github-block-container.test.tsx @@ -25,9 +25,7 @@ const DATA: ConnectGithubBlockData = { state: "disconnected", }; -const REPOS: readonly ConnectGithubRepo[] = [ - { id: "1", name: "acme/widgets", openPullRequestCount: 2 }, -]; +const REPOS: readonly ConnectGithubRepo[] = [{ id: "1", name: "acme/widgets" }]; let container: HTMLDivElement | null = null; let root: Root | null = null; @@ -169,7 +167,7 @@ describe("ConnectGithubBlockContainer post-submit refresh (CL-6463)", () => { }); const heading = el.querySelector(".chat-block-scene-pick-heading"); - expect(heading?.textContent).toBe("Pick your repos"); + expect(heading?.textContent).toBe("Choose what gets reviewed"); expect(heading?.getAttribute("tabindex")).toBe("-1"); expect(document.activeElement).toBe(heading); expect(el.querySelector("#connect-github-token")).toBeNull(); @@ -322,4 +320,33 @@ describe("ConnectGithubBlockContainer names a kind:error state (PR 422)", () => ); expect(connectPrimary).toBeUndefined(); }); + + // CL-7189: a rejected read used to leave the query on `loading`, which + // renders the disconnected body — the card told a person with a working + // connection that they had never connected. + test("a state read that rejects becomes a spoken error, never a silent Connect GitHub", async () => { + const actions: ConnectGithubActions = { + getConnectState: () => Promise.reject(new Error("boom")), + subscribeConnectState: () => () => {}, + requestConnect: () => {}, + submitAccessToken: async () => ({ ok: true as const }), + startReviewing: async () => ({ startedTriggerCount: 0 }), + skip: async () => {}, + }; + + const el = await mount(actions, "m_state_rejected"); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + + const alert = el.querySelector('[role="alert"]'); + expect(alert?.textContent).toBe( + "Couldn't reach GitHub with your token just now — try connecting again.", + ); + const connectPrimary = [...el.querySelectorAll("button")].find( + (button) => button.textContent === "Connect GitHub", + ); + expect(connectPrimary).toBeUndefined(); + }); }); From b295b2cc70055f8ad94fa5d5e8d52557cba18e96 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 29 Aug 2026 23:19:17 -0700 Subject: [PATCH 4/5] Keep the JSON report contract off the reviewers' chat prompt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each reviewer's system prompt ended with "Reply with JSON and nothing else", and those same prompts install the reviewers as ordinary chat agents. Asking one of them anything in a room got a raw `{"summary": ..., "findings": []}` back. The same contract also sat inside the review workflow's own prompt, where it contradicted the instruction to write one prose review and post it through the GitHub tool. A reviewer definition now carries its lens alone. `reviewerReportPrompt` appends the contract on the one path that parses JSON back — the review run's pass turns, which now receive the system prompt to use rather than reaching for the definition's. --- .../code-review/src/agent-requests.test.ts | 16 +++++++++++ packages/code-review/src/index.ts | 1 + packages/code-review/src/review-run.ts | 19 +++++++++++-- packages/code-review/src/reviewers.ts | 28 ++++++++++++++----- 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/packages/code-review/src/agent-requests.test.ts b/packages/code-review/src/agent-requests.test.ts index 2713dbe78..8be2855e2 100644 --- a/packages/code-review/src/agent-requests.test.ts +++ b/packages/code-review/src/agent-requests.test.ts @@ -3,6 +3,7 @@ import { type } from "arktype"; import { CreateAgentDefinitionInput } from "@corbits/agent-directory"; import { codeReviewAgentRequests } from "./agent-requests"; +import { CODE_REVIEW_REVIEWERS, reviewerReportPrompt } from "./reviewers"; test("every reviewer installs through the agent create path unchanged", () => { const requests = codeReviewAgentRequests(); @@ -19,3 +20,18 @@ test("each reviewer gets its own handle", () => { const handles = codeReviewAgentRequests().map((request) => request.handle); expect(new Set(handles).size).toBe(handles.length); }); + +test("an installed reviewer answers a person in prose — the JSON report contract is never in its system prompt", () => { + for (const request of codeReviewAgentRequests()) { + expect(request.systemPrompt).not.toContain("Reply with JSON"); + expect(request.systemPrompt).not.toContain('"findings"'); + } +}); + +test("the report contract rides along only on the path that parses JSON back", () => { + for (const reviewer of CODE_REVIEW_REVIEWERS) { + const prompt = reviewerReportPrompt(reviewer); + expect(prompt).toContain(reviewer.systemPrompt); + expect(prompt).toContain("Reply with JSON"); + } +}); diff --git a/packages/code-review/src/index.ts b/packages/code-review/src/index.ts index 79b02612c..f00b9c365 100644 --- a/packages/code-review/src/index.ts +++ b/packages/code-review/src/index.ts @@ -20,6 +20,7 @@ export { export { CODE_REVIEW_REVIEWERS, REVIEWER_REPORT_CONTRACT, + reviewerReportPrompt, reviewerById, type ReviewerDefinition, } from "./reviewers"; diff --git a/packages/code-review/src/review-run.ts b/packages/code-review/src/review-run.ts index c995b427a..6a5d9ca4d 100644 --- a/packages/code-review/src/review-run.ts +++ b/packages/code-review/src/review-run.ts @@ -29,7 +29,11 @@ import { aggregateReview, type ReviewerPass } from "./aggregate"; import { isBotAuthor } from "./bot-guard"; import { fingerprintsIn } from "./fingerprint"; import { renderReviewPrompt } from "./prompt"; -import { CODE_REVIEW_REVIEWERS, type ReviewerDefinition } from "./reviewers"; +import { + CODE_REVIEW_REVIEWERS, + reviewerReportPrompt, + type ReviewerDefinition, +} from "./reviewers"; /** The GitHub reach a review run needs, under the connection's credential. */ export interface CodeReviewGitHub { @@ -45,9 +49,14 @@ export interface CodeReviewGitHub { ): Promise; } -/** Runs one reviewer's turn and returns its raw reply. */ +/** Runs one reviewer's turn and returns its raw reply. `systemPrompt` + * is the reviewer's lens *plus* the JSON report contract this run parses + * back — a host must use it rather than `reviewer.systemPrompt`, which + * carries the lens alone so the same reviewer can answer a person in + * prose (CL-7189). */ export type ReviewerTurn = (input: { readonly reviewer: ReviewerDefinition; + readonly systemPrompt: string; readonly prompt: string; }) => Promise; @@ -85,7 +94,11 @@ async function runOne( prompt: string, ): Promise { try { - const reply = await deps.runReviewerTurn({ reviewer, prompt }); + const reply = await deps.runReviewerTurn({ + reviewer, + systemPrompt: reviewerReportPrompt(reviewer), + prompt, + }); return { reviewer, ok: true, reply }; } catch (err) { return { reviewer, ok: false, reason: reasonOf(err) }; diff --git a/packages/code-review/src/reviewers.ts b/packages/code-review/src/reviewers.ts index 1acac760e..f92964011 100644 --- a/packages/code-review/src/reviewers.ts +++ b/packages/code-review/src/reviewers.ts @@ -10,7 +10,17 @@ // takes, so the same three definitions can be installed as agents in a // workbench and driven by this package's own review run. -/** The output contract every reviewer replies under. */ +/** + * The JSON contract a reviewer replies under **when a machine parses the + * reply back** — `./review-run.ts`'s pass runner, and nothing else. + * + * It is deliberately not part of `ReviewerDefinition.systemPrompt`. The + * same prompts install these reviewers as ordinary chat agents + * (`./agent-requests.ts`), and a reviewer carrying this contract answers + * a person in the room with `{"summary": ..., "findings": []}` instead of + * a sentence (CL-7189). Append it with `reviewerReportPrompt` on the one + * path that reads JSON back. + */ export const REVIEWER_REPORT_CONTRACT = "Reply with JSON and nothing else — no prose before or after, no code " + 'fence. Shape: {"summary": string, "findings": [{"severity": ' + @@ -91,8 +101,7 @@ const ARCHITECTURE_REVIEWER: ReviewerDefinition = { "duplication that should have been a refactor or an extension of an " + "existing interface.\n\n" + "Out of lane: style-only nitpicking, and speculative redesigns of " + - "code this change did not touch.\n\n" + - REVIEWER_REPORT_CONTRACT, + "code this change did not touch.", introduction: (repoNames) => `I'm the architecture reviewer. I'll read every pull request on ` + `${namedRepos(repoNames)} for whether the shape holds up: the ` + @@ -123,8 +132,7 @@ const CORRECTNESS_REVIEWER: ReviewerDefinition = { "that cannot happen, or tests for inputs that cannot occur. A " + "signature that drifted from what callers expect — a value that " + "became a promise, a changed parameter order, a return type that " + - "narrowed — is blocking.\n\n" + - REVIEWER_REPORT_CONTRACT, + "narrowed — is blocking.", introduction: (repoNames) => `I'm the correctness reviewer. I'll read every pull request opened ` + `on ${namedRepos(repoNames)} for defects, with the file, the line, ` + @@ -147,8 +155,7 @@ const RELEASE_RISK_REVIEWER: ReviewerDefinition = { "Say what the team is most likely getting wrong that nobody else " + 'would raise. Say "do not ship" plainly when you mean it — an early ' + "no is worth more than a late surprise. Sequencing, rollout order, " + - "and what has to be true before this lands are yours to raise.\n\n" + - REVIEWER_REPORT_CONTRACT, + "and what has to be true before this lands are yours to raise.", introduction: (repoNames) => `I'm the release-risk reviewer. I'll weigh in on pull requests to ` + `${namedRepos(repoNames)}, saying plainly what actually blocks ` + @@ -162,6 +169,13 @@ export const CODE_REVIEW_REVIEWERS: readonly ReviewerDefinition[] = [ RELEASE_RISK_REVIEWER, ]; +/** This reviewer's lens plus the JSON contract — the system prompt for + * a turn whose reply is parsed, never the one an installed chat agent + * answers a person under. */ +export function reviewerReportPrompt(reviewer: ReviewerDefinition): string { + return `${reviewer.systemPrompt}\n\n${REVIEWER_REPORT_CONTRACT}`; +} + /** Looks a reviewer up by id; an unknown id is a named error. */ export function reviewerById(id: string): ReviewerDefinition { const found = CODE_REVIEW_REVIEWERS.find((reviewer) => reviewer.id === id); From b26fc128eaf55af8bb2f08769dedce3510a66ee4 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 29 Aug 2026 23:21:59 -0700 Subject: [PATCH 5/5] Name the noun in the repo counter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "6 your token can reach · 3 picked" was missing its subject. --- packages/chat-ui/src/strings.ts | 2 +- packages/chat-ui/test/connect-github-block.test.tsx | 12 ++++++------ packages/chat-ui/test/connect-github-flow.test.tsx | 2 +- packages/chat-ui/test/onboarding-scene-card.test.tsx | 6 +++--- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/chat-ui/src/strings.ts b/packages/chat-ui/src/strings.ts index 0e9e0a8d9..0c1fe979a 100644 --- a/packages/chat-ui/src/strings.ts +++ b/packages/chat-ui/src/strings.ts @@ -241,7 +241,7 @@ export const CHAT_STRINGS = { `Connected to GitHub as ${org}`, blockConnectGithubChange: "change", blockConnectGithubRepoCount: (found: number, picked: number) => - `${found} your token can reach · ${picked} picked`, + `${found} repo${found === 1 ? "" : "s"} your token can reach · ${picked} picked`, blockConnectGithubSelectAll: "Select all", blockConnectGithubRepoUpdated: (relative: string) => `updated ${relative}`, blockConnectGithubRepoNeverPushed: "no commits yet", diff --git a/packages/chat-ui/test/connect-github-block.test.tsx b/packages/chat-ui/test/connect-github-block.test.tsx index 477cce109..55421954b 100644 --- a/packages/chat-ui/test/connect-github-block.test.tsx +++ b/packages/chat-ui/test/connect-github-block.test.tsx @@ -278,7 +278,7 @@ describe("connect GitHub card — 2b pick your repos", () => { }); expect(el.textContent).toContain("Connected to GitHub as acme"); - expect(el.textContent).toContain("6 your token can reach · 3 picked"); + expect(el.textContent).toContain("6 repos your token can reach · 3 picked"); const rows = el.querySelectorAll(".chat-block-connect-repo-row"); expect(rows).toHaveLength(6); @@ -327,7 +327,7 @@ describe("connect GitHub card — 2b pick your repos", () => { />, ); - expect(el.textContent).toContain("6 your token can reach · 3 picked"); + expect(el.textContent).toContain("6 repos your token can reach · 3 picked"); const start = [...el.querySelectorAll("button")].find((button) => button.textContent?.startsWith("Start reviewing"), ) as HTMLButtonElement; @@ -340,7 +340,7 @@ describe("connect GitHub card — 2b pick your repos", () => { mobileCheckbox?.click(); }); - expect(el.textContent).toContain("6 your token can reach · 4 picked"); + expect(el.textContent).toContain("6 repos your token can reach · 4 picked"); const startAfter = [...el.querySelectorAll("button")].find((button) => button.textContent?.startsWith("Start reviewing"), ) as HTMLButtonElement; @@ -360,7 +360,7 @@ describe("connect GitHub card — 2b pick your repos", () => { />, ); - expect(el.textContent).toContain("6 your token can reach · 0 picked"); + expect(el.textContent).toContain("6 repos your token can reach · 0 picked"); const start = [...el.querySelectorAll("button")].find((button) => button.textContent?.startsWith("Start reviewing"), ) as HTMLButtonElement; @@ -378,7 +378,7 @@ describe("connect GitHub card — 2b pick your repos", () => { selectAll.click(); }); - expect(el.textContent).toContain("6 your token can reach · 6 picked"); + expect(el.textContent).toContain("6 repos your token can reach · 6 picked"); const startAfter = [...el.querySelectorAll("button")].find((button) => button.textContent?.startsWith("Start reviewing"), ) as HTMLButtonElement; @@ -568,7 +568,7 @@ describe("connect GitHub card — accessibility", () => { }); expect(status?.textContent).toBe("Connect GitHub"); - expect(el.textContent).toContain("6 your token can reach · 2 picked"); + expect(el.textContent).toContain("6 repos your token can reach · 2 picked"); }); test("autoFocus on the pick-repos scene moves focus onto the pick heading", async () => { diff --git a/packages/chat-ui/test/connect-github-flow.test.tsx b/packages/chat-ui/test/connect-github-flow.test.tsx index 9bebeea59..e14809ace 100644 --- a/packages/chat-ui/test/connect-github-flow.test.tsx +++ b/packages/chat-ui/test/connect-github-flow.test.tsx @@ -226,7 +226,7 @@ describe("connect-github round trip (CL-6345)", () => { checkboxes[1]?.click(); checkboxes[2]?.click(); }); - expect(el.textContent).toContain("4 your token can reach · 3 picked"); + expect(el.textContent).toContain("4 repos your token can reach · 3 picked"); const getConnectStateCallCountBeforeStart = harness.getConnectStateCallCount(); diff --git a/packages/chat-ui/test/onboarding-scene-card.test.tsx b/packages/chat-ui/test/onboarding-scene-card.test.tsx index 13f4b1cf4..52e935c22 100644 --- a/packages/chat-ui/test/onboarding-scene-card.test.tsx +++ b/packages/chat-ui/test/onboarding-scene-card.test.tsx @@ -298,7 +298,7 @@ describe("the walkthrough marker follows the live connect state", () => { await act(async () => { changeRepos?.click(); }); - expect(el.textContent).toContain("2 your token can reach · 1 picked"); + expect(el.textContent).toContain("2 repos your token can reach · 1 picked"); expect(el.querySelector(".chat-block-title")?.textContent).toBe( "Code review", ); @@ -332,7 +332,7 @@ describe("the walkthrough marker follows the live connect state", () => { await act(async () => { changeRepos?.click(); }); - expect(el.textContent).toContain("2 your token can reach · 1 picked"); + expect(el.textContent).toContain("2 repos your token can reach · 1 picked"); const start = [...el.querySelectorAll("button")].find((button) => button.textContent?.startsWith("Start reviewing"), @@ -344,7 +344,7 @@ describe("the walkthrough marker follows the live connect state", () => { }); expect(el.querySelector(".chat-block-scene-reviewing")).toBeNull(); - expect(el.textContent).toContain("2 your token can reach · 1 picked"); + expect(el.textContent).toContain("2 repos your token can reach · 1 picked"); expect(currentStepTitle(el)).toBe("Choose what gets reviewed"); const alert = el.querySelector('[role="alert"]'); expect(alert).not.toBeNull();