From 9be62e2125fc97bb98fecb4e756c7f560d2968d0 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 27 Aug 2026 10:40:16 -0700 Subject: [PATCH 01/11] Code review: reviewers introduce themselves once reviewing starts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each reviewer definition carries a short introduction in its own voice. After the connect card's start-reviewing succeeds, the route hands the introductions to a new onReviewingStarted port and the hub posts them under each reviewer's own address, in roster order — the first thing a person reads after picking repos is who is reviewing and what for, not a join dump. --- apps/hub/src/index.ts | 30 +++++++ packages/code-review/README.md | 7 ++ packages/code-review/package.json | 3 +- packages/code-review/src/index.ts | 4 + .../code-review/src/introductions.test.ts | 32 ++++++++ packages/code-review/src/introductions.ts | 21 +++++ packages/code-review/src/reviewers.ts | 31 +++++++ .../connect-github-credential-link.test.ts | 2 + .../src/connect-github-routes.test.ts | 81 +++++++++++++++++++ .../src/connect-github-routes.ts | 39 +++++++++ .../src/connect-github-setup.ts | 15 ++-- 11 files changed, 256 insertions(+), 9 deletions(-) create mode 100644 packages/code-review/src/introductions.test.ts create mode 100644 packages/code-review/src/introductions.ts diff --git a/apps/hub/src/index.ts b/apps/hub/src/index.ts index 5529ff05a..aae856550 100644 --- a/apps/hub/src/index.ts +++ b/apps/hub/src/index.ts @@ -94,6 +94,9 @@ import { isWorkbenchHostDefinitionName, listConnectedProviders, listDefaultInferencePreferences, + localPartOf, + parseParticipants, + postRoomMessage, startWorkflowCommand, sendWorkbenchMessage, settleConnectedService, @@ -2266,6 +2269,33 @@ export async function createHub(config: HubConfig) { data: { updatedBy: principalId, settings: row.settings }, }); }, + onReviewingStarted: async ( + tenantId, + workbenchId, + _principalId, + introductions, + ) => { + const row = await chatStore.getWorkbenchSettings(tenantId, workbenchId); + const participants = parseParticipants( + row?.settings["chat/participants"], + ); + for (const introduction of introductions) { + const participant = participants.find( + (candidate) => candidate.handle === introduction.handle, + ); + if (participant === undefined) continue; + await postRoomMessage( + { roomMessages, publish: workbenchSubscribers.publish }, + { + tenantId, + workbenchId, + sender: { name: null, address: participant.address }, + runId: localPartOf(participant.address), + parts: [{ kind: "text", text: introduction.text }], + }, + ); + } + }, }), ); // Template block workflows (CL-6405): the instantiate path's diff --git a/packages/code-review/README.md b/packages/code-review/README.md index 73b9f15bc..6045a1be3 100644 --- a/packages/code-review/README.md +++ b/packages/code-review/README.md @@ -18,6 +18,13 @@ Three reviewers, each a lens rather than a whole opinion: requests, so the roster a review fans out to is the roster a person can see and edit in the workbench. +Each reviewer also carries a canned `introduction` — one or two sentences +in its own voice, naming the job and the repos it will watch. +`reviewerIntroductions(repoNames)` (`src/introductions.ts`, +`@corbits/code-review/introductions`) renders all three in roster order; +a host posts them once a person starts reviewing, so the room shows who +just came online without running any inference. + ## The report contract Every reviewer replies under `REVIEWER_REPORT_CONTRACT` diff --git a/packages/code-review/package.json b/packages/code-review/package.json index 98b3eeedd..d711a9bb0 100644 --- a/packages/code-review/package.json +++ b/packages/code-review/package.json @@ -8,7 +8,8 @@ "exports": { ".": "./src/index.ts", "./reviewers": "./src/reviewers.ts", - "./agent-requests": "./src/agent-requests.ts" + "./agent-requests": "./src/agent-requests.ts", + "./introductions": "./src/introductions.ts" }, "scripts": { "typecheck": "tsc --noEmit", diff --git a/packages/code-review/src/index.ts b/packages/code-review/src/index.ts index f8f16cc86..79b02612c 100644 --- a/packages/code-review/src/index.ts +++ b/packages/code-review/src/index.ts @@ -23,6 +23,10 @@ export { reviewerById, type ReviewerDefinition, } from "./reviewers"; +export { + reviewerIntroductions, + type ReviewerIntroduction, +} from "./introductions"; export { runPullRequestReview, type CodeReviewGitHub, diff --git a/packages/code-review/src/introductions.test.ts b/packages/code-review/src/introductions.test.ts new file mode 100644 index 000000000..2130d94f4 --- /dev/null +++ b/packages/code-review/src/introductions.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, test } from "bun:test"; + +import { CODE_REVIEW_REVIEWERS } from "./reviewers"; +import { reviewerIntroductions } from "./introductions"; + +const REPO_NAMES = ["acme/widgets", "acme/gadgets"]; + +describe("reviewerIntroductions", () => { + test("returns one introduction per reviewer, in roster order", () => { + const introductions = reviewerIntroductions(REPO_NAMES); + expect(introductions.map((introduction) => introduction.handle)).toEqual( + CODE_REVIEW_REVIEWERS.map((reviewer) => reviewer.handle), + ); + }); + + test("every introduction names a picked repo and carries no internal detail", () => { + const introductions = reviewerIntroductions(REPO_NAMES); + for (const { handle, text } of introductions) { + expect(REPO_NAMES.some((name) => text.includes(name))).toBe(true); + expect(text).not.toContain("@"); + expect(text.toLowerCase()).not.toContain("http"); + expect(text).not.toContain(handle); + } + }); + + test("names the single picked repo when only one is selected", () => { + const introductions = reviewerIntroductions(["acme/widgets"]); + for (const { text } of introductions) { + expect(text).toContain("acme/widgets"); + } + }); +}); diff --git a/packages/code-review/src/introductions.ts b/packages/code-review/src/introductions.ts new file mode 100644 index 000000000..134ead986 --- /dev/null +++ b/packages/code-review/src/introductions.ts @@ -0,0 +1,21 @@ +// The canned copy each reviewer posts once repos are picked and it +// starts reviewing — the identity beat of the first minute. Pure +// formatting over `./reviewers.ts`'s own `introduction` templates; no +// inference runs to produce these, exactly like `@corbits/chat`'s +// `postCannedGreeting`. +import { CODE_REVIEW_REVIEWERS } from "./reviewers"; + +export interface ReviewerIntroduction { + readonly handle: string; + readonly text: string; +} + +/** One introduction per reviewer, in `CODE_REVIEW_REVIEWERS` order. */ +export function reviewerIntroductions( + repoNames: readonly string[], +): readonly ReviewerIntroduction[] { + return CODE_REVIEW_REVIEWERS.map((reviewer) => ({ + handle: reviewer.handle, + text: reviewer.introduction(repoNames), + })); +} diff --git a/packages/code-review/src/reviewers.ts b/packages/code-review/src/reviewers.ts index 25d1bc8d3..1acac760e 100644 --- a/packages/code-review/src/reviewers.ts +++ b/packages/code-review/src/reviewers.ts @@ -52,6 +52,24 @@ export interface ReviewerDefinition { readonly displayName: string; readonly description: string; readonly systemPrompt: string; + /** The canned message this reviewer posts, in its own voice, once + * repos are picked and it starts reviewing — who it is and what it + * will do for these repos. Never run through inference; see + * `./introductions.ts`'s `reviewerIntroductions`. */ + readonly introduction: (repoNames: readonly string[]) => string; +} + +/** Renders a list of repo names the way a sentence names them: + * "widgets", "widgets and gadgets", or "widgets, gadgets, and sprockets". */ +function namedRepos(repoNames: readonly string[]): string { + if (repoNames.length === 0) return "these repos"; + const [first, ...rest] = repoNames; + if (first === undefined) return "these repos"; + if (rest.length === 0) return first; + const last = rest[rest.length - 1]; + if (rest.length === 1) return `${first} and ${last}`; + const middle = [first, ...rest.slice(0, -1)].join(", "); + return `${middle}, and ${last}`; } const ARCHITECTURE_REVIEWER: ReviewerDefinition = { @@ -75,6 +93,11 @@ const ARCHITECTURE_REVIEWER: ReviewerDefinition = { "Out of lane: style-only nitpicking, and speculative redesigns of " + "code this change did not touch.\n\n" + REVIEWER_REPORT_CONTRACT, + introduction: (repoNames) => + `I'm the architecture reviewer. I'll read every pull request on ` + + `${namedRepos(repoNames)} for whether the shape holds up: the ` + + `invariants, where a constraint should live, and what it costs to ` + + `maintain later.`, }; const CORRECTNESS_REVIEWER: ReviewerDefinition = { @@ -102,6 +125,10 @@ const CORRECTNESS_REVIEWER: ReviewerDefinition = { "became a promise, a changed parameter order, a return type that " + "narrowed — is blocking.\n\n" + REVIEWER_REPORT_CONTRACT, + introduction: (repoNames) => + `I'm the correctness reviewer. I'll read every pull request opened ` + + `on ${namedRepos(repoNames)} for defects, with the file, the line, ` + + `and the input that trips them.`, }; const RELEASE_RISK_REVIEWER: ReviewerDefinition = { @@ -122,6 +149,10 @@ const RELEASE_RISK_REVIEWER: ReviewerDefinition = { "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, + introduction: (repoNames) => + `I'm the release-risk reviewer. I'll weigh in on pull requests to ` + + `${namedRepos(repoNames)}, saying plainly what actually blocks ` + + `shipping, what ships with a note, and what can wait.`, }; /** The roster a review fans out to, in the order findings are reported. */ diff --git a/packages/workflow-catalog/src/connect-github-credential-link.test.ts b/packages/workflow-catalog/src/connect-github-credential-link.test.ts index 940ca780f..1a4cb8da4 100644 --- a/packages/workflow-catalog/src/connect-github-credential-link.test.ts +++ b/packages/workflow-catalog/src/connect-github-credential-link.test.ts @@ -146,6 +146,7 @@ describe("the room GitHub connect card reads what its own submit writes", () => selectedRepos: [], }), persistSelectedRepos: async () => {}, + onReviewingStarted: async () => {}, listReposFn: async () => [], fetchAuthenticatedLoginFn: async () => "octocat", }); @@ -188,6 +189,7 @@ describe("the room GitHub connect card reads what its own submit writes", () => selectedRepos: [], }), persistSelectedRepos: async () => {}, + onReviewingStarted: async () => {}, listReposFn: async () => [], fetchAuthenticatedLoginFn: async () => "octocat", }); diff --git a/packages/workflow-catalog/src/connect-github-routes.test.ts b/packages/workflow-catalog/src/connect-github-routes.test.ts index b2e8ba34e..690d02acc 100644 --- a/packages/workflow-catalog/src/connect-github-routes.test.ts +++ b/packages/workflow-catalog/src/connect-github-routes.test.ts @@ -62,6 +62,12 @@ function mountAs(routes: Hono): Hono { function buildApp(overrides: Partial = {}) { const grants: { tenantId: string; repo: GitHubRepoSummary }[] = []; const triggers: { tenantId: string; repo: GitHubRepoSummary }[] = []; + const introductionCalls: { + tenantId: string; + workbenchId: string; + principalId: string; + introductions: readonly { handle: string; text: string }[]; + }[] = []; let settings: { pendingConnections: readonly string[]; selectedRepos: readonly string[]; @@ -97,6 +103,19 @@ function buildApp(overrides: Partial = {}) { selectedRepos: patch["template/selectedRepos"], }; }, + onReviewingStarted: async ( + tenantId, + workbenchId, + principalId, + introductions, + ) => { + introductionCalls.push({ + tenantId, + workbenchId, + principalId, + introductions, + }); + }, listReposFn: async () => REPOS, fetchAuthenticatedLoginFn: async () => "octocat", ...overrides, @@ -107,6 +126,7 @@ function buildApp(overrides: Partial = {}) { app: mountAs(routes), grants, triggers, + introductionCalls, settingsNow: () => settings, }; } @@ -172,6 +192,47 @@ describe("POST /:workbenchId/github/start-reviewing", () => { }); }); + test("posts each reviewer's introduction once, naming the selected repos, after success", async () => { + const harness = buildApp(); + const response = await harness.app.request("/wb_1/github/start-reviewing", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ repoIds: ["1", "2"] }), + }); + expect(response.status).toBe(200); + expect(harness.introductionCalls.length).toBe(1); + const [call] = harness.introductionCalls; + if (call === undefined) throw new Error("expected one introduction call"); + expect(call.tenantId).toBe("tnt_1"); + expect(call.workbenchId).toBe("wb_1"); + expect(call.principalId).toBe("prn_alice"); + expect(call.introductions.length).toBe(3); + for (const introduction of call.introductions) { + expect( + ["acme/widgets", "acme/gadgets"].some((name) => + introduction.text.includes(name), + ), + ).toBe(true); + } + }); + + test("a rejecting introduction port still yields 200 and logs the failure", async () => { + const logs: string[] = []; + const harness = buildApp({ + log: (line) => logs.push(line), + onReviewingStarted: async () => { + throw new Error("boom"); + }, + }); + const response = await harness.app.request("/wb_1/github/start-reviewing", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ repoIds: ["1"] }), + }); + expect(response.status).toBe(200); + expect(logs.some((line) => line.includes("boom"))).toBe(true); + }); + test("400s on a malformed body without leaking raw parser text", async () => { const harness = buildApp(); const response = await harness.app.request("/wb_1/github/start-reviewing", { @@ -182,6 +243,7 @@ describe("POST /:workbenchId/github/start-reviewing", () => { expect(response.status).toBe(400); const body = (await response.json()) as { error: { code: string } }; expect(body.error.code).toBe("bad_request"); + expect(harness.introductionCalls).toEqual([]); }); test("409s when the tenant has no github credential yet", async () => { @@ -193,6 +255,24 @@ describe("POST /:workbenchId/github/start-reviewing", () => { }); expect(response.status).toBe(409); expect(harness.grants).toEqual([]); + expect(harness.introductionCalls).toEqual([]); + }); + + test("502s when GitHub cannot be read, without posting introductions", async () => { + const harness = buildApp({ + listReposFn: async () => { + throw new Error( + "GitHub request to /user/repos failed: 401 Bad credentials", + ); + }, + }); + const response = await harness.app.request("/wb_1/github/start-reviewing", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ repoIds: ["1"] }), + }); + expect(response.status).toBe(502); + expect(harness.introductionCalls).toEqual([]); }); test("404s when the code-review workflow isn't deployed in this tenant", async () => { @@ -206,5 +286,6 @@ describe("POST /:workbenchId/github/start-reviewing", () => { }); expect(response.status).toBe(404); expect(harness.grants).toEqual([]); + expect(harness.introductionCalls).toEqual([]); }); }); diff --git a/packages/workflow-catalog/src/connect-github-routes.ts b/packages/workflow-catalog/src/connect-github-routes.ts index 3eb02a59c..60a948a3e 100644 --- a/packages/workflow-catalog/src/connect-github-routes.ts +++ b/packages/workflow-catalog/src/connect-github-routes.ts @@ -33,6 +33,10 @@ import { type GitHubClientConfig, type GitHubRepoSummary, } from "@corbits/github-tools"; +import { + reviewerIntroductions, + type ReviewerIntroduction, +} from "@corbits/code-review/introductions"; import { startReviewingRepos } from "./connect-github-setup"; import { templateReposSettingsPatch } from "./settings"; @@ -110,6 +114,19 @@ export type ConnectGithubRoutesDeps = { principalId: string, patch: ReturnType, ): Promise; + /** Posts each reviewer's canned introduction to the room once + * `startReviewingRepos` succeeds — the identity beat of the first + * minute a person sees after picking repos. Called once, after the + * repos are recorded and before the 200 response; a rejection is + * logged through `deps.log` and never fails the response — the + * webhook triggers already exist, so a person must not see an error + * for a missed introduction. */ + onReviewingStarted( + tenantId: string, + workbenchId: string, + principalId: string, + introductions: readonly ReviewerIntroduction[], + ): Promise; /** Test-only override, defaulting to `@corbits/github-tools`' real * `listRepos` — lets `connect-github-routes.test.ts` stub the GitHub * client without reaching for module mocking or the real network. */ @@ -271,6 +288,28 @@ export function createConnectGithubRoutes( ); }, }); + + const reposById = new Map( + state.repos.map((repo) => [repo.id, repo] as const), + ); + const repoNames = body.repoIds + .map((repoId) => reposById.get(repoId)?.name) + .filter((name): name is string => name !== undefined); + try { + await deps.onReviewingStarted( + tenant.id, + workbenchId, + principal.id, + reviewerIntroductions(repoNames), + ); + } catch (cause) { + const message = + cause instanceof Error ? cause.message : String(cause); + deps.log( + `connect-github: reviewer introductions failed for tenant ${tenant.id}, workbench ${workbenchId}: ${message}`, + ); + } + return c.json( { startedTriggerCount: result.createdTriggerIds.length }, 200, diff --git a/packages/workflow-catalog/src/connect-github-setup.ts b/packages/workflow-catalog/src/connect-github-setup.ts index b32c4d9e8..ad49fda96 100644 --- a/packages/workflow-catalog/src/connect-github-setup.ts +++ b/packages/workflow-catalog/src/connect-github-setup.ts @@ -1,11 +1,10 @@ // What runs once a person has actually picked repos on a room's GitHub -// connect card (CL-6345 — the slice `./instantiate.ts`'s -// `webhookTriggerTodos` named as still open). `instantiateWorkbenchTemplate` +// connect card and starts reviewing. `instantiateWorkbenchTemplate` // resolves a template at create time, before any repo is known; this // module resolves the repo-scoped half once the person answers the -// connect card, mirroring `instantiate.ts`'s own shape exactly: pure -// orchestration over injected async ports, no HTTP, no store, testable -// with plain fakes. +// connect card and creates the live webhook trigger per repo, mirroring +// `instantiate.ts`'s own shape exactly: pure orchestration over injected +// async ports, no HTTP, no store, testable with plain fakes. import type { GitHubRepoSummary } from "@corbits/github-tools"; export interface ConnectGithubSetupPorts { @@ -20,9 +19,9 @@ export interface ConnectGithubSetupPorts { mintRepoGrant(repo: GitHubRepoSummary): Promise; /** * Creates the live `webhook_trigger` row this repo's pull-request-opened - * events fire — the resolution of `instantiate.ts`'s own honest - * `webhookTriggerTodo` for this repo. A host binds this to - * `@corbits/webhook-triggers`' `WebhookTriggerStore.create`. + * events fire — the onboarding card's start-reviewing step is what + * creates this trigger, for each repo the person picked. A host binds + * this to `@corbits/webhook-triggers`' `WebhookTriggerStore.create`. */ createWebhookTrigger( repo: GitHubRepoSummary, From e2c05b341f015c55436a7771b538c4618872c1bf Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 27 Aug 2026 11:17:30 -0700 Subject: [PATCH 02/11] Chat UI: the in-room walkthrough is the first-minute scene A connect-github card posted in the room's own voice renders as a scene, not a member's message: no author row, the job as its title, the promise beneath, and the walkthrough's steps listed with the current one marked in words. Once repos are recorded the card shows what it is reviewing instead of still offering Connect, with a change-repos link back to the pick; every state flips in place on the same row. Consecutive agent-joined rows collapse into one line naming everyone, so a template room opens on the scene and the reviewers' own introductions, never a join dump. --- .../blocks/connect-github-block-container.tsx | 44 +++ .../src/blocks/connect-github-block.tsx | 156 ++++++++++- packages/chat-ui/src/index.ts | 3 + packages/chat-ui/src/strings.ts | 9 +- packages/chat-ui/src/styles.css | 72 +++++ packages/chat-ui/src/timeline.tsx | 137 ++++++++- .../test/connect-github-block.test.tsx | 24 +- .../chat-ui/test/connect-github-flow.test.tsx | 9 +- .../test/onboarding-scene-card.test.tsx | 260 ++++++++++++++++++ .../chat-ui/test/system-join-edge.test.tsx | 53 ++++ 10 files changed, 732 insertions(+), 35 deletions(-) create mode 100644 packages/chat-ui/test/onboarding-scene-card.test.tsx 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 9089cb372..ec1733b6e 100644 --- a/packages/chat-ui/src/blocks/connect-github-block-container.tsx +++ b/packages/chat-ui/src/blocks/connect-github-block-container.tsx @@ -27,8 +27,17 @@ import type { ConnectGithubActions, ConnectGithubQuery, } from "./connect-github-actions"; +import type { OnboardingScene } from "./connect-github-block"; import { ConnectGithubBlockView } from "./connect-github-block"; +/** Positions in the room's three-step walkthrough. Which one is current + * is read off the live connect state, never off the card's own data: + * disconnected means connect, connected with nothing recorded means pick, + * and repos the server actually recorded means reviewing. */ +const STEP_CONNECT = 0; +const STEP_PICK = 1; +const STEP_REVIEWING = 2; + type ConnectedGithubQuery = Extract; /** Survives container remounts so a post-connect loading flash never @@ -60,6 +69,7 @@ function displayQueryOf( } export function ConnectGithubBlockContainer({ + data, messageId, actions, }: { @@ -73,6 +83,10 @@ export function ConnectGithubBlockContainer({ const [selectedRepoIds, setSelectedRepoIds] = useState( () => lastConnectedOf(messageId)?.selectedRepoIds ?? [], ); + /** A person who pressed "change repos" on the done state gets the + * picker back without the server's recorded selection changing — only + * pressing "Start reviewing" again writes anything. */ + const [repickRequested, setRepickRequested] = useState(false); const mountedRef = useRef(true); const applyQuery = useCallback( @@ -123,10 +137,25 @@ export function ConnectGithubBlockContainer({ ); const displayQuery = displayQueryOf(messageId, query); + const recordedRepoIds = + displayQuery.kind === "connected" ? displayQuery.selectedRepoIds : []; + const currentStepIndex = + displayQuery.kind !== "connected" + ? STEP_CONNECT + : recordedRepoIds.length === 0 + ? STEP_PICK + : STEP_REVIEWING; + const scene: OnboardingScene = { + title: data.requiredForTemplate, + currentStepIndex, + ...(data.promise !== undefined ? { promise: data.promise } : {}), + ...(data.steps !== undefined ? { steps: data.steps } : {}), + }; if (actions === undefined || displayQuery.kind !== "connected") { return ( actions?.requestConnect()} onSubmitAccessToken={submitAccessTokenAndRefresh} @@ -134,6 +163,19 @@ export function ConnectGithubBlockContainer({ ); } + if (currentStepIndex === STEP_REVIEWING && !repickRequested) { + return ( + recordedRepoIds.includes(repo.id)) + .map((repo) => repo.name)} + onChangeRepos={() => setRepickRequested(true)} + /> + ); + } + function toggleRepo(repoId: string) { setSelectedRepoIds((current) => current.includes(repoId) @@ -144,6 +186,7 @@ export function ConnectGithubBlockContainer({ return ( { + setRepickRequested(false); void actions.startReviewing(repoIds); }} onSkip={() => { diff --git a/packages/chat-ui/src/blocks/connect-github-block.tsx b/packages/chat-ui/src/blocks/connect-github-block.tsx index a8b25af6c..b790106a1 100644 --- a/packages/chat-ui/src/blocks/connect-github-block.tsx +++ b/packages/chat-ui/src/blocks/connect-github-block.tsx @@ -1,6 +1,7 @@ -// The GitHub connect card renders both states of the first-run connect flow -// (CL-6342 screen 2) inline in the room, using the same `BlockCard` frame -// every other block uses -- there is no settings page or dialog for this. +// The room's first-minute scene card: one `BlockCard` that names the job, +// carries the walkthrough the workbench's own onboarding copy wrote, and +// flips its body between connecting, picking repos, and reviewing -- +// inline in the room, never a settings page or a dialog. // Selection is controlled: like `PollBlockView` never keeps its own tally, // this view never owns which repos are picked -- it renders what it's given // and reports toggles upward. It stays pure and props-driven so it can be @@ -29,7 +30,34 @@ export type ConnectGithubRepo = { readonly openPullRequestCount: number; }; -export type ConnectGithubCardProps = +/** One labelled step of the room's walkthrough, as the workbench's own + * onboarding copy wrote it — the card never holds step text of its own. */ +export type OnboardingSceneStep = { + readonly title: string; + readonly why: string; +}; + +/** The framing every state of this card keeps: the job the room is here + * to do, the one-line promise under it, and the ordered walkthrough. The + * card flips its body between states while this header stays put. */ +export type OnboardingScene = { + readonly title: string; + readonly promise?: string; + readonly steps?: readonly OnboardingSceneStep[]; + /** Which step of `steps` the person is on now, by position. */ + readonly currentStepIndex: number; +}; + +/** The walkthrough this card knows how to mark: connect, pick, review. + * A workbench whose steps read differently still gets its labels + * rendered — just without a "you're here" marker the positions would + * only be guessing at. */ +const MARKABLE_STEP_COUNT = 3; + +/** What the card's body is doing right now, without the scene framing + * every state shares — the shape a host names when it only cares about + * the body variant. */ +export type ConnectGithubCardBody = | { readonly kind: "disconnected"; readonly onConnect: () => void; @@ -55,14 +83,113 @@ export type ConnectGithubCardProps = readonly onChangeConnection: () => void; readonly onStartReviewing: (repoIds: readonly string[]) => void; readonly onSkip: () => void; + } + | { + readonly kind: "reviewing"; + readonly repoNames: readonly string[]; + readonly onChangeRepos: () => void; }; +export type ConnectGithubCardProps = ConnectGithubCardBody & { + readonly scene: OnboardingScene; +}; + function repoMetaLabel(openPullRequestCount: number): string { return openPullRequestCount === 0 ? CHAT_STRINGS.blockConnectGithubNoOpenPulls : CHAT_STRINGS.blockConnectGithubOpenPulls(openPullRequestCount); } +type StepState = "done" | "current" | "upcoming"; + +function stepStateAt(index: number, currentStepIndex: number): StepState { + if (index < currentStepIndex) return "done"; + if (index === currentStepIndex) return "current"; + return "upcoming"; +} + +function stepMarkLabel(state: StepState): string | undefined { + if (state === "done") return CHAT_STRINGS.blockConnectGithubStepDone; + if (state === "current") return CHAT_STRINGS.blockConnectGithubStepCurrent; + return undefined; +} + +/** The card's constant framing: what this room is for, the promise under + * it, and where the person is in the walkthrough. Every label here is the + * workbench's own onboarding copy, never this package's. */ +function SceneHeader({ scene }: { readonly scene: OnboardingScene }) { + const steps = scene.steps; + const marked = steps !== undefined && steps.length === MARKABLE_STEP_COUNT; + const currentWhy = marked ? steps[scene.currentStepIndex]?.why : undefined; + return ( + <> + {scene.promise !== undefined ? ( +

{scene.promise}

+ ) : null} + {steps !== undefined ? ( +
    + {steps.map((step, index) => { + const state = stepStateAt(index, scene.currentStepIndex); + const mark = marked ? stepMarkLabel(state) : undefined; + return ( +
  1. + + {step.title} + + {mark !== undefined ? ( + {mark} + ) : null} +
    2. + ); + })} +
+ ) : null} + {currentWhy !== undefined ? ( +

{currentWhy}

+ ) : null} + + ); +} + +/** The walkthrough's done state: the repos under review, what happens in + * them now, and one quiet way back to the picker. Deliberately says + * nothing about connecting — this card is past that. */ +function ReviewingBody({ + repoNames, + onChangeRepos, +}: { + readonly repoNames: readonly string[]; + readonly onChangeRepos: () => void; +}) { + return ( +
+

+ + {CHAT_STRINGS.blockConnectGithubReviewingHeadline} +

+
    + {repoNames.map((name) => ( +
  • {name}
    • + ))} +
+

+ {CHAT_STRINGS.blockConnectGithubReviewingLine} +

+
+ +
+
+ ); +} + function DisconnectedBody({ onConnect, onSubmitAccessToken, @@ -265,19 +392,22 @@ function ConnectedBody({ } export function ConnectGithubBlockView(props: ConnectGithubCardProps) { - if (props.kind === "disconnected") { - return ( - + return ( + + + {props.kind === "disconnected" ? ( - - ); - } - return ( - - + ) : null} + {props.kind === "connected" ? : null} + {props.kind === "reviewing" ? ( + + ) : null} ); } diff --git a/packages/chat-ui/src/index.ts b/packages/chat-ui/src/index.ts index 1c8b4c5a2..50e1aff40 100644 --- a/packages/chat-ui/src/index.ts +++ b/packages/chat-ui/src/index.ts @@ -95,8 +95,11 @@ export { BlockCard } from "./blocks/block-card"; export { ConnectGithubBlockView } from "./blocks/connect-github-block"; export type { + ConnectGithubCardBody, ConnectGithubCardProps, ConnectGithubRepo, + OnboardingScene, + OnboardingSceneStep, } from "./blocks/connect-github-block"; export type { ApprovalActions, diff --git a/packages/chat-ui/src/strings.ts b/packages/chat-ui/src/strings.ts index a2a42543a..de2ab30b6 100644 --- a/packages/chat-ui/src/strings.ts +++ b/packages/chat-ui/src/strings.ts @@ -91,6 +91,8 @@ export const CHAT_STRINGS = { legacyBadgeLabel: "Legacy", eventAgentJoined: (displayName: string) => `${displayName} joined`, eventAgentJoinedUnknown: "An agent joined", + eventAgentsJoined: (displayNames: readonly string[]) => + `${joinWithAnd(displayNames)} joined`, eventMembershipChanged: "Membership updated", eventSettingsChanged: "Settings updated", eventWorkbenchRenamed: (from: string, to: string): string => @@ -212,8 +214,13 @@ export const CHAT_STRINGS = { blockQuestionSubmitting: "Sending…", blockQuestionAnswerError: "Couldn't send your answer — try again.", blockQuestionAnsweredLabel: "Your answer", - blockConnectGithubHeadline: "Connect GitHub", blockConnectGithubPickHeadline: "Pick your repos", + blockConnectGithubStepDone: "Done", + blockConnectGithubStepCurrent: "You're here", + blockConnectGithubReviewingHeadline: "Reviewing", + blockConnectGithubReviewingLine: + "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.", blockConnectGithubAction: "Connect GitHub", diff --git a/packages/chat-ui/src/styles.css b/packages/chat-ui/src/styles.css index c42bb8b84..99d3199a7 100644 --- a/packages/chat-ui/src/styles.css +++ b/packages/chat-ui/src/styles.css @@ -1703,6 +1703,78 @@ color: var(--destructive); } +/* The room's onboarding scene card: one constant header (what this room + is for, the promise under it, where you are in the walkthrough) above a + body that flips between connect, pick, and reviewing. Step state is + carried in words inside each row, never by color alone — DESIGN.md + State Pills. */ +.chat-block-scene-promise { + margin: 0 0 0.6rem; + font-size: 0.9rem; + line-height: 1.45; + color: var(--foreground); +} + +.chat-block-scene-steps { + margin: 0 0 0.55rem; + padding: 0; + list-style: none; + border: 1px solid var(--border); + background: var(--background); +} + +.chat-block-scene-step { + display: flex; + align-items: baseline; + gap: 0.5rem; + padding: 0.4rem 0.7rem; + border-bottom: 1px solid var(--border); + font-size: 0.8125rem; + color: var(--muted-foreground); +} + +.chat-block-scene-step:last-child { + border-bottom: 0; +} + +.chat-block-scene-step[data-state="current"] { + color: var(--foreground); + font-weight: 650; +} + +.chat-block-scene-step-mark { + margin-left: auto; + font-size: 0.6875rem; + font-weight: 650; + letter-spacing: 0.02em; + text-transform: uppercase; + color: var(--muted-foreground); +} + +.chat-block-scene-step[data-state="current"] .chat-block-scene-step-mark { + color: var(--chat-agent-accent, var(--primary)); +} + +.chat-block-scene-step[data-state="done"] .chat-block-scene-step-mark { + color: var(--ok, #37904a); +} + +.chat-block-scene-why { + margin: 0 0 0.65rem; +} + +.chat-block-scene-repo-names { + margin: 0 0 0.55rem; + padding: 0; + list-style: none; + display: flex; + flex-wrap: wrap; + gap: 0.35rem 0.6rem; + font-size: 0.875rem; + font-weight: 600; + color: var(--foreground); +} + /* GitHub connect card (CL-6342 screen 2). The row's checkbox control is `@corbits/react-ui`'s own `Checkbox`; only the row layout around it (name left, open-PR count right) is workbench-specific. */ diff --git a/packages/chat-ui/src/timeline.tsx b/packages/chat-ui/src/timeline.tsx index e5dc43866..e06703b59 100644 --- a/packages/chat-ui/src/timeline.tsx +++ b/packages/chat-ui/src/timeline.tsx @@ -578,10 +578,17 @@ function TextBubble({ * anything else falls back to the event name with its separators turned * into spaces. */ -export function friendlyEventText( +/** + * The display name a "workbench.agent-joined" event carries, resolved the + * friendly way: the roster's own handle for that address when the roster + * knows it, else the address's own local part — never the raw address, and + * never the generic "An agent joined", which hides a name the event + * already carries. Undefined only when the event names nobody. + */ +function joinedAgentName( part: Part & { kind: "event" }, participants: readonly ParticipantRecord[], -): string { +): string | undefined { const data = typeof part.data === "object" && part.data !== null ? (part.data as Record) @@ -590,22 +597,28 @@ export function friendlyEventText( data !== undefined && typeof data.address === "string" ? data.address : undefined; - // The participant record's own handle is the friendly, settings-held - // name (see `packages/chat/src/participants.ts`); when the roster - // hasn't caught up with this address yet, the address's own local - // part (CL-6594) is still a real identifier — never the generic "An - // agent joined", which hides a name the event already carries. + if (address === undefined) return undefined; const handle = - address !== undefined - ? (participants.find((participant) => participant.address === address) - ?.handle ?? localPartOf(address)) - : undefined; + participants.find((participant) => participant.address === address) + ?.handle ?? localPartOf(address); + return displayNameFromHandle(handle); +} +export function friendlyEventText( + part: Part & { kind: "event" }, + participants: readonly ParticipantRecord[], +): string { + const data = + typeof part.data === "object" && part.data !== null + ? (part.data as Record) + : undefined; switch (part.event) { - case "workbench.agent-joined": - return handle !== undefined - ? CHAT_STRINGS.eventAgentJoined(displayNameFromHandle(handle)) + case "workbench.agent-joined": { + const name = joinedAgentName(part, participants); + return name !== undefined + ? CHAT_STRINGS.eventAgentJoined(name) : CHAT_STRINGS.eventAgentJoinedUnknown; + } case "workbench.membership-changed": return CHAT_STRINGS.eventMembershipChanged; case "workbench.settings-changed": { @@ -658,10 +671,14 @@ function EventLine({ part, createdAt, participants, + collapsedText, }: { part: Part & { kind: "event" }; createdAt: string; participants: readonly ParticipantRecord[]; + /** The one line that stands in for a whole run of consecutive joins — + * see `collapseAgentJoinRuns`. Undefined on every other event row. */ + collapsedText?: string; }) { const data = typeof part.data === "object" && part.data !== null @@ -677,7 +694,9 @@ function EventLine({ return (
- {connectedDisplayName !== undefined ? ( + {collapsedText !== undefined ? ( + collapsedText + ) : connectedDisplayName !== undefined ? ( <> {CHAT_STRINGS.eventConnectionConnectedBeforePlugins( connectedDisplayName, @@ -965,6 +984,81 @@ export function isSystemNoticeItem(item: MessageItem): boolean { ); } +/** + * A message posted in the room's own voice rather than by any member — + * the `system@` sender the room's onboarding card arrives under. Such a + * row is never "own" for any viewer and never carries author chrome: it + * is the room talking, not a person. + */ +export function isSystemSenderItem(item: MessageItem): boolean { + return ( + item.sender !== undefined && localPartOf(item.sender.address) === "system" + ); +} + +/** The display name a lone agent-joined row names, or undefined when the + * item is anything else. */ +function agentJoinName( + item: TimelineMessageItem, + participants: readonly ParticipantRecord[], +): string | undefined { + if (item.parts.length !== 1) return undefined; + const part = item.parts[0]; + if ( + part === undefined || + part.kind !== "event" || + part.event !== "workbench.agent-joined" + ) { + return undefined; + } + return joinedAgentName(part, participants); +} + +/** + * A room whose whole team arrives at once used to open on a stack of + * "X joined / Y joined / Z joined" — the first thing a person read was a + * membership log. Consecutive joins with nothing between them collapse + * into a single line naming everyone; joins separated by a real message + * stay their own rows, because there the sequence is the point. + * + * Returns the line to render on each run's first item, and the ids of the + * items that line already accounts for. + */ +export function collapseAgentJoinRuns( + items: readonly TimelineMessageItem[], + participants: readonly ParticipantRecord[], +): { + readonly textByLeadId: ReadonlyMap; + readonly absorbedIds: ReadonlySet; +} { + const textByLeadId = new Map(); + const absorbedIds = new Set(); + let index = 0; + while (index < items.length) { + const names: string[] = []; + let end = index; + while (end < items.length) { + const item = items[end]; + if (item === undefined) break; + const name = agentJoinName(item, participants); + if (name === undefined) break; + names.push(name); + end += 1; + } + if (names.length > 1) { + const lead = items[index]; + if (lead !== undefined) { + textByLeadId.set(lead.id, CHAT_STRINGS.eventAgentsJoined(names)); + for (const absorbed of items.slice(index + 1, end)) { + absorbedIds.add(absorbed.id); + } + } + } + index = end > index ? end : index + 1; + } + return { textByLeadId, absorbedIds }; +} + /** * A failed send's inline recovery row (CL-6251/CL-5879): appended below * the bubble text of the exact same message group a confirmed message @@ -1339,6 +1433,7 @@ function MessagePartsInner({ currentUser, showDayDivider, showHeader, + collapsedJoinText, threadMeta, threadAffordanceMode = "reply", onOpenThread, @@ -1367,6 +1462,9 @@ function MessagePartsInner({ /** `false` when this message continues an unbroken run from the same * author as the item directly above it — see `isGroupedWithPrevious`. */ readonly showHeader: boolean; + /** Set only on the first item of a collapsed run of consecutive agent + * joins — see `collapseAgentJoinRuns`. */ + readonly collapsedJoinText?: string; readonly threadMeta?: ThreadAffordanceMeta | undefined; readonly threadAffordanceMode?: ThreadAffordanceMode; readonly onOpenThread?: (messageId: string) => void; @@ -1421,6 +1519,7 @@ function MessagePartsInner({ currentUser !== undefined && item.sender !== undefined && !isSystemNoticeItem(item) && + !isSystemSenderItem(item) && localPartOf(item.sender.address) === currentUser.principalId; const contextMenu = useContextMenuState(); const menu = offersSocialChrome @@ -1512,6 +1611,9 @@ function MessagePartsInner({ part={part} createdAt={item.createdAt} participants={participants} + {...(collapsedJoinText !== undefined + ? { collapsedText: collapsedJoinText } + : {})} /> ); } @@ -1919,6 +2021,8 @@ export function WorkbenchTimeline({ return () => observer.disconnect(); }, []); + const joinRuns = collapseAgentJoinRuns(items, participants); + if (items.length === 0) { // A freshly created agent chat answers before it finishes setting up // in the background: the agent participant streams in seconds later. @@ -1965,6 +2069,7 @@ export function WorkbenchTimeline({ return (
{items.map((item, index) => { + if (joinRuns.absorbedIds.has(item.id)) return null; const previous = index > 0 ? items[index - 1] : undefined; const showDayDivider = previous === undefined || @@ -1996,6 +2101,7 @@ export function WorkbenchTimeline({ previous, showDayDivider, ); + const collapsedJoinText = joinRuns.textByLeadId.get(item.id); return ( ); +async function mount(props: ConnectGithubCardBody) { + return mountElement(); } function typeInto(element: HTMLInputElement, text: string) { @@ -80,6 +95,7 @@ function PickReposHarness({ useState(initiallySelected); return ( { connect?.click(); }); - const steps = [...el.querySelectorAll("ol li")].map( + const steps = [...el.querySelectorAll(".chat-block-connect-steps li")].map( (item) => item.textContent, ); expect(steps).toHaveLength(3); diff --git a/packages/chat-ui/test/connect-github-flow.test.tsx b/packages/chat-ui/test/connect-github-flow.test.tsx index 14175c4a8..665e539ab 100644 --- a/packages/chat-ui/test/connect-github-flow.test.tsx +++ b/packages/chat-ui/test/connect-github-flow.test.tsx @@ -250,7 +250,12 @@ describe("connect-github round trip (CL-6345)", () => { expect(harness.getConnectStateCallCount()).toBe( getConnectStateCallCountBeforeStart, ); - expect(el.textContent).toContain("Connected to GitHub as octocat"); - expect(el.textContent).toContain("4 repos found · 3 picked"); + // Reviewing, not "pick your repos" again and never "Connect": the + // server recorded the selection, so the same card now names what is + // under review. + expect(el.textContent).toContain("Reviewing"); + expect(el.textContent).toContain("acme/checkout"); + expect(el.textContent).toContain("acme/web"); + expect(el.textContent).not.toContain("Connect"); }); }); diff --git a/packages/chat-ui/test/onboarding-scene-card.test.tsx b/packages/chat-ui/test/onboarding-scene-card.test.tsx new file mode 100644 index 000000000..51212ad3c --- /dev/null +++ b/packages/chat-ui/test/onboarding-scene-card.test.tsx @@ -0,0 +1,260 @@ +// The room's first minute: one designed card posted in the room's own +// voice that names the job, walks the person through connect → pick → +// reviewing, and lands on a real done state. Covers the card's chrome (no +// author row, no "Member", no avatar), the header the card keeps across +// every state, and the walkthrough marker each live connect state puts on +// it. + +import { afterEach, describe, expect, test } from "bun:test"; +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import type { Root } from "react-dom/client"; + +import type { MessageItem } from "../src/api"; +import type { + ConnectGithubActions, + ConnectGithubQuery, + ConnectGithubRepo, +} from "../src/blocks/connect-github-actions"; +import { ConnectGithubBlockContainer } from "../src/blocks/connect-github-block-container"; +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: "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 }, +]; + +let container: HTMLDivElement | null = null; +let root: Root | null = null; + +afterEach(() => { + if (root !== null) act(() => root?.unmount()); + container?.remove(); + container = null; + root = null; +}); + +async function mount(element: React.ReactElement) { + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + await act(async () => { + root?.render(element); + }); + return container; +} + +function onboardingCardItem(data: unknown): readonly MessageItem[] { + return [ + { + id: "m_onboarding", + createdAt: "2026-01-01T00:00:00.000Z", + sender: { name: null, address: "system@wb_1" }, + parts: [{ kind: "block", block: { type: "connect-github", data } }], + }, + ]; +} + +function stepRows(el: HTMLElement) { + return [...el.querySelectorAll(".chat-block-scene-step")]; +} + +function stepTitles(el: HTMLElement) { + return stepRows(el).map( + (row) => + row.querySelector(".chat-block-scene-step-title")?.textContent ?? "", + ); +} + +function currentStepTitle(el: HTMLElement): string | undefined { + const current = stepRows(el).find( + (row) => row.getAttribute("data-state") === "current", + ); + return ( + current?.querySelector(".chat-block-scene-step-title")?.textContent ?? + undefined + ); +} + +/** A host whose live state is whatever the test says it is, pushed once + * on mount — the same read-then-subscribe contract the real + * `createChatConnectGithubActions` binds. */ +function fixedStateActions(state: ConnectGithubQuery): ConnectGithubActions { + return { + getConnectState: () => Promise.resolve(state), + subscribeConnectState: () => () => undefined, + requestConnect: () => undefined, + submitAccessToken: () => Promise.resolve({ ok: true as const }), + startReviewing: () => Promise.resolve({ startedTriggerCount: 0 }), + skip: () => Promise.resolve(), + }; +} + +describe("the room's onboarding card is a scene, not a member's message", () => { + test("a card posted in the room's own voice carries no author row, no avatar, and no Member label", async () => { + const el = await mount( + , + ); + expect(el.querySelector(".chat-block")).not.toBeNull(); + expect(el.querySelector(".chat-sender-avatar-wrap")).toBeNull(); + expect(el.querySelector(".chat-bubble-row")).toBeNull(); + expect(el.textContent).not.toContain("Member"); + expect(el.textContent).not.toContain("system"); + expect( + el.querySelector(".chat-message-group")?.getAttribute("data-own"), + ).toBe("false"); + }); + + test("the card names the job, promises the outcome, and lists the room's own step labels in order", async () => { + const el = await mount( + , + ); + expect(el.querySelector(".chat-block-title")?.textContent).toBe( + "Code review", + ); + expect(el.querySelector(".chat-block-scene-promise")?.textContent).toBe( + PROMISE, + ); + expect(stepTitles(el)).toEqual([ + "Connect GitHub", + "Pick your repos", + "Start reviewing", + ]); + }); + + test("a card persisted without a promise or steps renders the card without either — never invented copy", async () => { + const el = await mount( + , + ); + expect(el.querySelector(".chat-block-title")?.textContent).toBe( + "Code review", + ); + expect(el.querySelector(".chat-block-scene-promise")).toBeNull(); + expect(el.querySelector(".chat-block-scene-steps")).toBeNull(); + expect(el.querySelector(".chat-block-scene-why")).toBeNull(); + }); + + test("a walkthrough of some other length still shows its labels, but marks no step", async () => { + const el = await mount( + , + ); + expect(stepTitles(el)).toEqual(["Connect GitHub", "Pick your repos"]); + expect(currentStepTitle(el)).toBeUndefined(); + expect(el.querySelector(".chat-block-scene-why")).toBeNull(); + }); +}); + +describe("the walkthrough marker follows the live connect state", () => { + const DATA = { + requiredForTemplate: "Code review", + state: "disconnected" as const, + promise: PROMISE, + steps: STEPS, + }; + + async function mountAt(messageId: string, state: ConnectGithubQuery) { + return mount( + , + ); + } + + test("nothing connected yet marks Connect GitHub and explains why that step matters", async () => { + const el = await mountAt("m_step1", { kind: "disconnected" }); + expect(currentStepTitle(el)).toBe("Connect GitHub"); + expect(el.querySelector(".chat-block-scene-why")?.textContent).toBe( + STEPS[0]?.why, + ); + }); + + test("connected with nothing recorded yet marks Pick your repos", async () => { + const el = await mountAt("m_step2", { + kind: "connected", + orgName: "acme", + repos: REPOS, + selectedRepoIds: [], + }); + expect(currentStepTitle(el)).toBe("Pick your repos"); + expect(el.querySelector(".chat-block-scene-why")?.textContent).toBe( + STEPS[1]?.why, + ); + expect(el.textContent).toContain("acme/checkout"); + }); + + test("repos the server actually recorded mark Start reviewing and show the done state, never Connect", async () => { + const el = await mountAt("m_step3", { + kind: "connected", + orgName: "acme", + repos: REPOS, + selectedRepoIds: ["1", "2"], + }); + expect(currentStepTitle(el)).toBe("Start reviewing"); + const done = el.querySelector(".chat-block-scene-reviewing"); + expect(done?.textContent).toContain("Reviewing"); + const names = [ + ...el.querySelectorAll(".chat-block-scene-repo-names li"), + ].map((item) => item.textContent); + expect(names).toEqual(["acme/checkout", "acme/web"]); + expect(done?.textContent).toContain("change repos"); + // The done state never re-offers connecting — the card is past it. + expect(done?.textContent).not.toContain("Connect"); + }); + + test("change repos hands the picker back without touching what the server recorded", async () => { + const el = await mountAt("m_step4", { + kind: "connected", + orgName: "acme", + repos: REPOS, + selectedRepoIds: ["1"], + }); + const changeRepos = [...el.querySelectorAll("button")].find( + (button) => button.textContent === "change repos", + ); + expect(changeRepos).not.toBeUndefined(); + await act(async () => { + changeRepos?.click(); + }); + expect(el.textContent).toContain("2 repos found · 1 picked"); + expect(el.querySelector(".chat-block-title")?.textContent).toBe( + "Code review", + ); + }); +}); diff --git a/packages/chat-ui/test/system-join-edge.test.tsx b/packages/chat-ui/test/system-join-edge.test.tsx index 6281d1689..c61ec6da7 100644 --- a/packages/chat-ui/test/system-join-edge.test.tsx +++ b/packages/chat-ui/test/system-join-edge.test.tsx @@ -50,6 +50,30 @@ async function mount(items: readonly MessageItem[], currentUser?: CurrentUser) { return container; } +function joinItemFor(id: string, agentAddress: string): MessageItem { + return { + id, + createdAt: "2026-01-01T00:00:00.000Z", + sender: { name: null, address: "sawyer@agents.example" }, + parts: [ + { + kind: "event", + event: "workbench.agent-joined", + data: { address: agentAddress }, + }, + ], + }; +} + +function textItem(id: string): MessageItem { + return { + id, + createdAt: "2026-01-01T00:00:00.000Z", + sender: { name: "Sawyer", address: "sawyer@agents.example" }, + parts: [{ kind: "text", text: "Morning" }], + }; +} + function joinItem(senderAddress: string): MessageItem { return { id: "join_1", @@ -101,3 +125,32 @@ describe("CL-6772: join / system notices stay on the left edge", () => { expect(block?.[0]).toMatch(/2\.9rem/); }); }); + +describe("a whole team arriving reads as one line, not a join dump", () => { + test("three consecutive joins collapse into one line naming everyone", async () => { + const el = await mount([ + joinItemFor("j1", "correctness-reviewer@agents.example"), + joinItemFor("j2", "architecture-reviewer@agents.example"), + joinItemFor("j3", "release-risk-reviewer@agents.example"), + ]); + const lines = [...el.querySelectorAll(".chat-event-line")]; + expect(lines).toHaveLength(1); + const text = lines[0]?.textContent ?? ""; + expect(text).toContain("Correctness Reviewer"); + expect(text).toContain("Architecture Reviewer"); + expect(text).toContain("Release Risk Reviewer"); + expect(text.match(/joined/g)).toHaveLength(1); + }); + + test("joins separated by a real message stay their own rows", async () => { + const el = await mount([ + joinItemFor("j1", "correctness-reviewer@agents.example"), + textItem("t1"), + joinItemFor("j2", "architecture-reviewer@agents.example"), + ]); + const lines = [...el.querySelectorAll(".chat-event-line")]; + expect(lines).toHaveLength(2); + expect(lines[0]?.textContent).toContain("Correctness Reviewer joined"); + expect(lines[1]?.textContent).toContain("Architecture Reviewer joined"); + }); +}); From 634afa3054e91b60d2d4f6d6247609f9e0c47b5e Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 27 Aug 2026 11:28:57 -0700 Subject: [PATCH 03/11] Update docs: Workbench Definition drives template create MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DESIGN.md, PRODUCT.md, the connect-cards doc, and the glossary now describe what shipped: a template is a Workbench Definition — default agents, routines, tools, plugins, ordered onboarding steps — that mints an empty channel with no host and runs its walkthrough as an in-room scene card; Code review's roster is three reviewers who introduce themselves once reviewing starts. Inviting teammates is a later slice. --- DESIGN.md | 24 +++++++++++++++++++----- PRODUCT.md | 9 +++++++-- docs/connect-cards.md | 10 +++++++--- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index cee28133a..0dbb6d538 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -63,11 +63,16 @@ real, never a 404. not home. The primary act is a prompt: say what the channel should do, or pick a named-template shortcut underneath. Blank `+` / prompt mint an empty channel and invite nobody. Named templates mint that same -empty channel, then invite existing agents (including Myra as a -participant, never as the mint host). The sidebar `+` opens this -route. First-run after credential does not: `/` hops to Myra's one -DM (`openAgentDm` / find-or-reopen). There is no parallel Myra home -route and no Describe door. +empty channel with no host, then instantiate the picked Workbench +Definition — the agents, block workflows, and pending plugins it names +(Myra joins only when the definition names her; Code review's three +reviewers do not) — and run its ordered onboarding walkthrough as an +in-room card the room itself posts, never a side effect of hosting an +agent. The card reads live connection state and flips straight to the +repo pick, so there is one walkthrough, not a separate already-connected +dialog. The sidebar `+` opens this route. First-run after credential +does not: `/` hops to Myra's one DM (`openAgentDm` / find-or-reopen). +There is no parallel Myra home route and no Describe door. **`/inbox` is gone as a page** (CL-6151). The path stays as a redirect home so old links still resolve; it is not a live groups inbox. @@ -283,6 +288,15 @@ personal-access-token paste, then the same card flips to pick repositories. A GitHub App / hosted OAuth Connect as the welcome mat is CL-6343 (out of scope), not the shipped card. +The room's own onboarding card renders as a scene, not a member's +message: no author row, the job as its title with the promise beneath, +and the walkthrough's steps listed with the current one marked in +words. Once repos are recorded the card shows the Reviewing state — +what it's reviewing now — with a change-repos link back to the picker, +never still offering Connect. Consecutive agent-joined rows collapse +into one line naming everyone, so a template room opens on the scene +and the reviewers' own introductions, never a join dump. + ## State Pills Status indicators (ok / warn / error / running) use semantic colors that diff --git a/PRODUCT.md b/PRODUCT.md index 3f27815a0..86eb1429e 100644 --- a/PRODUCT.md +++ b/PRODUCT.md @@ -95,8 +95,13 @@ its Workbench Definition names three reviewers and no Myra. The room itself posts the onboarding card: Connect GitHub with a personal access token (the shipped path today). The same in-room card reads live connection state and flips in place to pick repositories — already -connected GitHub is that card, not a `/new` dialog. A GitHub App / -hosted OAuth welcome mat is future work (CL-6343), not current product. +connected GitHub is that card, not a `/new` dialog — then Start +reviewing. Once reviewing starts, each reviewer posts its own canned +introduction under its own address, in roster order — the first thing +a person reads is who is reviewing and what for, never a join dump. A +GitHub App / hosted OAuth welcome mat is future work (CL-6343), not +current product. Inviting teammates into the room is a later slice, +not part of this first minute. Settle for a template-key-only wait posts from a system sender and does not wake an agent. Generic `connections/pending` still wakes the diff --git a/docs/connect-cards.md b/docs/connect-cards.md index 4713539b1..9a57743d2 100644 --- a/docs/connect-cards.md +++ b/docs/connect-cards.md @@ -54,9 +54,13 @@ the walkthrough. Connect opens a guided personal-access-token paste (create a token with the `repo` scope, paste it, store encrypted). After Connect succeeds — or when GitHub is already connected — the same in-room card flips in place to pick repositories; there is no -`/new` already-connected dialog. Settling a credential a template room -is waiting on posts the connected notice from the system address and -never wakes an agent (`packages/chat/src/connect-pending.ts`). +`/new` already-connected dialog. Code review needs a repo pick before +reviewers are watching, then Start reviewing. Settling a credential a +template room is waiting on posts the connected notice from the system +address and never wakes an agent (`packages/chat/src/connect-pending.ts`) +— a reviewer roster has no host to answer. Once reviewing starts, each +reviewer posts its own canned introduction under its own address, in +roster order (`packages/code-review/src/introductions.ts`). A GitHub App / hosted OAuth Connect as the welcome mat is CL-6343, out of scope for the shipped card — do not document OAuth-first GitHub connect From baf60b2b350b3e00d62cabd938bfdf0713580a27 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 27 Aug 2026 15:51:37 -0700 Subject: [PATCH 04/11] Add tests for first-minute onboarding polish Covers the walkthrough staying on pick-repos after change repos, an empty steps list drawing no rail, start-reviewing rejections keeping the picker and reporting through the error sink, introduction failures reporting with tenant and room, and a second start-reviewing not re-posting intros. --- .../test/onboarding-scene-card.test.tsx | 69 ++++++++++++++++++- .../src/connect-github-routes.test.ts | 36 ++++++++-- 2 files changed, 99 insertions(+), 6 deletions(-) diff --git a/packages/chat-ui/test/onboarding-scene-card.test.tsx b/packages/chat-ui/test/onboarding-scene-card.test.tsx index 51212ad3c..330eb9f13 100644 --- a/packages/chat-ui/test/onboarding-scene-card.test.tsx +++ b/packages/chat-ui/test/onboarding-scene-card.test.tsx @@ -5,10 +5,11 @@ // every state, and the walkthrough marker each live connect state puts on // it. -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, mock, spyOn, test } from "bun:test"; import { act } from "react"; import { createRoot } from "react-dom/client"; import type { Root } from "react-dom/client"; +import * as errorSink from "@corbits/error-sink"; import type { MessageItem } from "../src/api"; import type { @@ -40,6 +41,7 @@ afterEach(() => { container?.remove(); container = null; root = null; + mock.restore(); }); async function mount(element: React.ReactElement) { @@ -162,6 +164,23 @@ describe("the room's onboarding card is a scene, not a member's message", () => expect(el.querySelector(".chat-block-scene-why")).toBeNull(); }); + test("an empty steps list does not render the step list", async () => { + const el = await mount( + , + ); + expect(el.querySelector(".chat-block-scene-promise")?.textContent).toBe( + PROMISE, + ); + expect(el.querySelector(".chat-block-scene-steps")).toBeNull(); + }); + test("a walkthrough of some other length still shows its labels, but marks no step", async () => { const el = await mount( { expect(el.querySelector(".chat-block-title")?.textContent).toBe( "Code review", ); + expect(currentStepTitle(el)).toBe("Pick your repos"); + expect(el.querySelector(".chat-block-scene-why")?.textContent).toBe( + STEPS[1]?.why, + ); + }); + + test("a rejected start reviewing after change repos keeps the picker and reports the error", async () => { + const report = spyOn(errorSink, "reportError").mockReturnValue("ref_test"); + const actions: ConnectGithubActions = { + ...fixedStateActions({ + kind: "connected", + orgName: "acme", + repos: REPOS, + selectedRepoIds: ["1"], + }), + startReviewing: () => Promise.reject(new Error("could not start")), + }; + const el = await mount( + , + ); + const changeRepos = [...el.querySelectorAll("button")].find( + (button) => button.textContent === "change repos", + ); + await act(async () => { + changeRepos?.click(); + }); + expect(el.textContent).toContain("2 repos found · 1 picked"); + + const start = [...el.querySelectorAll("button")].find((button) => + button.textContent?.startsWith("Start reviewing"), + ); + await act(async () => { + start?.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + + 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(report).toHaveBeenCalled(); + expect(report.mock.calls[0]?.[1]).toMatchObject({ + operation: "connect-github.startReviewing", + }); }); }); diff --git a/packages/workflow-catalog/src/connect-github-routes.test.ts b/packages/workflow-catalog/src/connect-github-routes.test.ts index 690d02acc..255e0e73c 100644 --- a/packages/workflow-catalog/src/connect-github-routes.test.ts +++ b/packages/workflow-catalog/src/connect-github-routes.test.ts @@ -3,10 +3,11 @@ // mounted the same way `packages/connections/src/routes.test.ts` mounts // its own routes: a bare `Hono` with a tenant-injecting middleware, no // real network, no real database — every port is a plain fake. -import { describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, mock, spyOn, test } from "bun:test"; import { Hono } from "hono"; import type { MiddlewareHandler } from "hono"; import type { RequireGrant, TenantEnv } from "@intx/hub-api"; +import * as errorSink from "@corbits/error-sink"; import type { GitHubClientConfig, GitHubRepoSummary, @@ -131,6 +132,10 @@ function buildApp(overrides: Partial = {}) { }; } +afterEach(() => { + mock.restore(); +}); + describe("GET /:workbenchId/github/state", () => { test("reports disconnected with no github credential", async () => { const { app } = buildApp({ resolveGithubConfig: async () => undefined }); @@ -216,10 +221,9 @@ describe("POST /:workbenchId/github/start-reviewing", () => { } }); - test("a rejecting introduction port still yields 200 and logs the failure", async () => { - const logs: string[] = []; + test("a rejecting introduction port still yields 200 and reports the failure", async () => { + const report = spyOn(errorSink, "reportError").mockReturnValue("ref_test"); const harness = buildApp({ - log: (line) => logs.push(line), onReviewingStarted: async () => { throw new Error("boom"); }, @@ -230,7 +234,29 @@ describe("POST /:workbenchId/github/start-reviewing", () => { body: JSON.stringify({ repoIds: ["1"] }), }); expect(response.status).toBe(200); - expect(logs.some((line) => line.includes("boom"))).toBe(true); + expect(report).toHaveBeenCalled(); + expect(report.mock.calls[0]?.[0]).toBeInstanceOf(Error); + expect(report.mock.calls[0]?.[1]).toMatchObject({ + operation: "connect-github.reviewerIntroductions", + tenantId: "tnt_1", + roomId: "wb_1", + }); + }); + + test("a second start-reviewing does not re-post introductions already landed for the room", async () => { + const harness = buildApp(); + const startReviewing = (repoIds: readonly string[]) => + harness.app.request("/wb_1/github/start-reviewing", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ repoIds }), + }); + const first = await startReviewing(["1"]); + expect(first.status).toBe(200); + expect(harness.introductionCalls).toHaveLength(1); + const second = await startReviewing(["1", "2"]); + expect(second.status).toBe(200); + expect(harness.introductionCalls).toHaveLength(1); }); test("400s on a malformed body without leaking raw parser text", async () => { From 3666c0a8a23feda3d8e64b34e9972e4e8eee4a34 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 27 Aug 2026 15:51:47 -0700 Subject: [PATCH 05/11] Keep pick-repos current while changing repos A change-repos click puts the walkthrough back on pick-repos instead of still marking Start reviewing. Empty steps no longer draw a blank rail. Start reviewing waits to settle before leaving the picker, and a rejection stays on the picker and reports through the error sink. Reviewer introductions post once per room; a later start-reviewing does not repeat them, and an introduction failure reports with tenant and room without failing the 200. --- bun.lock | 1 + .../blocks/connect-github-block-container.tsx | 28 ++++++++---- .../src/blocks/connect-github-block.tsx | 2 +- packages/workflow-catalog/package.json | 1 + .../src/connect-github-routes.ts | 44 ++++++++++++------- 5 files changed, 50 insertions(+), 26 deletions(-) diff --git a/bun.lock b/bun.lock index 9bc93fec9..4e0948eb1 100644 --- a/bun.lock +++ b/bun.lock @@ -1455,6 +1455,7 @@ "dependencies": { "@corbits/code-review": "workspace:*", "@corbits/code-review-workflow": "workspace:*", + "@corbits/error-sink": "workspace:*", "@corbits/github-tools": "workspace:*", "@corbits/jimmy-agent": "workspace:*", "@corbits/scout-agent": "workspace:*", 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 ec1733b6e..f29ceafd0 100644 --- a/packages/chat-ui/src/blocks/connect-github-block-container.tsx +++ b/packages/chat-ui/src/blocks/connect-github-block-container.tsx @@ -22,6 +22,7 @@ // snapshot so a real disconnect still shows Connect. import { useCallback, useEffect, useRef, useState } from "react"; import type { ConnectGithubBlockData } from "@corbits/chat/blocks"; +import { reportError } from "@corbits/error-sink"; import type { ConnectGithubActions, @@ -32,8 +33,9 @@ import { ConnectGithubBlockView } from "./connect-github-block"; /** Positions in the room's three-step walkthrough. Which one is current * is read off the live connect state, never off the card's own data: - * disconnected means connect, connected with nothing recorded means pick, - * and repos the server actually recorded means reviewing. */ + * disconnected means connect, connected with nothing recorded (or a + * person who pressed "change repos") means pick, and repos the server + * actually recorded means reviewing. */ const STEP_CONNECT = 0; const STEP_PICK = 1; const STEP_REVIEWING = 2; @@ -142,7 +144,7 @@ export function ConnectGithubBlockContainer({ const currentStepIndex = displayQuery.kind !== "connected" ? STEP_CONNECT - : recordedRepoIds.length === 0 + : recordedRepoIds.length === 0 || repickRequested ? STEP_PICK : STEP_REVIEWING; const scene: OnboardingScene = { @@ -163,7 +165,7 @@ export function ConnectGithubBlockContainer({ ); } - if (currentStepIndex === STEP_REVIEWING && !repickRequested) { + if (currentStepIndex === STEP_REVIEWING) { return ( current.includes(repoId) @@ -184,6 +188,15 @@ export function ConnectGithubBlockContainer({ ); } + async function startReviewing(repoIds: readonly string[]) { + try { + await connectedActions.startReviewing(repoIds); + if (mountedRef.current) setRepickRequested(false); + } catch (cause) { + reportError(cause, { operation: "connect-github.startReviewing" }); + } + } + return ( setSelectedRepoIds(displayQuery.repos.map((repo) => repo.id)) } - onChangeConnection={actions.requestConnect} + onChangeConnection={connectedActions.requestConnect} onStartReviewing={(repoIds) => { - setRepickRequested(false); - void actions.startReviewing(repoIds); + void startReviewing(repoIds); }} onSkip={() => { - void actions.skip(); + void connectedActions.skip(); }} /> ); diff --git a/packages/chat-ui/src/blocks/connect-github-block.tsx b/packages/chat-ui/src/blocks/connect-github-block.tsx index b790106a1..59d92b616 100644 --- a/packages/chat-ui/src/blocks/connect-github-block.tsx +++ b/packages/chat-ui/src/blocks/connect-github-block.tsx @@ -126,7 +126,7 @@ function SceneHeader({ scene }: { readonly scene: OnboardingScene }) { {scene.promise !== undefined ? (

{scene.promise}

) : null} - {steps !== undefined ? ( + {steps !== undefined && steps.length > 0 ? (
    {steps.map((step, index) => { const state = stepStateAt(index, scene.currentStepIndex); diff --git a/packages/workflow-catalog/package.json b/packages/workflow-catalog/package.json index 5a5b36974..46331677f 100644 --- a/packages/workflow-catalog/package.json +++ b/packages/workflow-catalog/package.json @@ -17,6 +17,7 @@ "dependencies": { "@corbits/code-review": "workspace:*", "@corbits/code-review-workflow": "workspace:*", + "@corbits/error-sink": "workspace:*", "@corbits/github-tools": "workspace:*", "@corbits/jimmy-agent": "workspace:*", "@corbits/scout-agent": "workspace:*", diff --git a/packages/workflow-catalog/src/connect-github-routes.ts b/packages/workflow-catalog/src/connect-github-routes.ts index 60a948a3e..d62e04a0d 100644 --- a/packages/workflow-catalog/src/connect-github-routes.ts +++ b/packages/workflow-catalog/src/connect-github-routes.ts @@ -26,6 +26,7 @@ import { Hono } from "hono"; import { type } from "arktype"; import type { RequireGrant, TenantEnv } from "@intx/hub-api"; import { makeErrorEnvelope } from "@workbench/hub-client"; +import { reportError } from "@corbits/error-sink"; import { fetchAuthenticatedLogin, @@ -117,10 +118,11 @@ export type ConnectGithubRoutesDeps = { /** Posts each reviewer's canned introduction to the room once * `startReviewingRepos` succeeds — the identity beat of the first * minute a person sees after picking repos. Called once, after the - * repos are recorded and before the 200 response; a rejection is - * logged through `deps.log` and never fails the response — the - * webhook triggers already exist, so a person must not see an error - * for a missed introduction. */ + * repos are recorded and before the 200 response; a later + * `start-reviewing` (change repos) does not call this again. A + * rejection is reported through `reportError` and never fails the + * response — the webhook triggers already exist, so a person must + * not see an error for a missed introduction. */ onReviewingStarted( tenantId: string, workbenchId: string, @@ -263,6 +265,12 @@ export function createConnectGithubRoutes( } try { + const settingsBefore = await deps.getTemplateSettings( + tenant.id, + workbenchId, + ); + const introductionsAlreadyPosted = + settingsBefore.selectedRepos.length > 0; const result = await startReviewingRepos(body.repoIds, state.repos, { mintRepoGrant: (repo) => deps.mintRepoGrant(tenant.id, repo), createWebhookTrigger: (repo) => @@ -295,19 +303,21 @@ export function createConnectGithubRoutes( const repoNames = body.repoIds .map((repoId) => reposById.get(repoId)?.name) .filter((name): name is string => name !== undefined); - try { - await deps.onReviewingStarted( - tenant.id, - workbenchId, - principal.id, - reviewerIntroductions(repoNames), - ); - } catch (cause) { - const message = - cause instanceof Error ? cause.message : String(cause); - deps.log( - `connect-github: reviewer introductions failed for tenant ${tenant.id}, workbench ${workbenchId}: ${message}`, - ); + if (!introductionsAlreadyPosted) { + try { + await deps.onReviewingStarted( + tenant.id, + workbenchId, + principal.id, + reviewerIntroductions(repoNames), + ); + } catch (cause) { + reportError(cause, { + operation: "connect-github.reviewerIntroductions", + tenantId: tenant.id, + roomId: workbenchId, + }); + } } return c.json( From ebafbe2e325780c363ba87335fedb3f0b37882b1 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 27 Aug 2026 16:17:00 -0700 Subject: [PATCH 06/11] Add tests for first-minute onboarding a11y --- .../test/connect-github-block.test.tsx | 31 +++++++++++ .../test/onboarding-scene-card.test.tsx | 53 +++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/packages/chat-ui/test/connect-github-block.test.tsx b/packages/chat-ui/test/connect-github-block.test.tsx index 246262945..b2e5d1cb2 100644 --- a/packages/chat-ui/test/connect-github-block.test.tsx +++ b/packages/chat-ui/test/connect-github-block.test.tsx @@ -249,6 +249,10 @@ describe("connect GitHub card — 2a disconnected", () => { expect(el.textContent).toContain("Bad token."); expect(el.querySelector("#connect-github-token")).not.toBeNull(); + expect( + el.querySelector(".chat-block-connect-token-error")?.getAttribute("role"), + ).toBe("alert"); + expect(field.getAttribute("aria-invalid")).toBe("true"); }); }); @@ -355,6 +359,11 @@ describe("connect GitHub card — 2b pick your repos", () => { button.textContent?.startsWith("Start reviewing"), ) as HTMLButtonElement; expect(start.textContent).toBe("Start reviewing 0 repos"); + expect(start.disabled).toBe(true); + const skip = [...el.querySelectorAll("button")].find( + (button) => button.textContent === "skip for now", + ) as HTMLButtonElement; + expect(skip.disabled).toBe(false); const selectAll = [...el.querySelectorAll("button")].find( (button) => button.textContent === "Select all", @@ -368,6 +377,7 @@ describe("connect GitHub card — 2b pick your repos", () => { button.textContent?.startsWith("Start reviewing"), ) as HTMLButtonElement; expect(startAfter.textContent).toBe("Start reviewing 6 repos"); + expect(startAfter.disabled).toBe(false); }); test("skip fires its own quiet callback, distinct from starting", async () => { @@ -432,6 +442,27 @@ describe("connect GitHub card — accessibility", () => { expect(document.activeElement).toBe(firstCheckbox); }); + test("the current walkthrough step is marked aria-current=step and the scene body is a live region", async () => { + const el = await mount({ + kind: "disconnected", + onConnect: () => undefined, + onSubmitAccessToken: () => Promise.resolve({ ok: true }), + }); + + const current = [...el.querySelectorAll(".chat-block-scene-step")].find( + (row) => row.getAttribute("data-state") === "current", + ); + expect(current?.getAttribute("aria-current")).toBe("step"); + expect( + [...el.querySelectorAll(".chat-block-scene-step")].filter( + (row) => row.getAttribute("aria-current") === "step", + ), + ).toHaveLength(1); + expect( + el.querySelector(".chat-block-scene-body")?.getAttribute("aria-live"), + ).toBe("polite"); + }); + test("the disconnected state's actions are real buttons, not divs", async () => { const el = await mount({ kind: "disconnected", diff --git a/packages/chat-ui/test/onboarding-scene-card.test.tsx b/packages/chat-ui/test/onboarding-scene-card.test.tsx index 330eb9f13..bde784e1d 100644 --- a/packages/chat-ui/test/onboarding-scene-card.test.tsx +++ b/packages/chat-ui/test/onboarding-scene-card.test.tsx @@ -86,6 +86,15 @@ function currentStepTitle(el: HTMLElement): string | undefined { ); } +function currentStepAria(el: HTMLElement): string | null { + const current = stepRows(el).find( + (row) => row.getAttribute("aria-current") === "step", + ); + return ( + current?.querySelector(".chat-block-scene-step-title")?.textContent ?? null + ); +} + /** A host whose live state is whatever the test says it is, pushed once * on mount — the same read-then-subscribe contract the real * `createChatConnectGithubActions` binds. */ @@ -219,9 +228,13 @@ describe("the walkthrough marker follows the live connect state", () => { test("nothing connected yet marks Connect GitHub and explains why that step matters", async () => { const el = await mountAt("m_step1", { kind: "disconnected" }); expect(currentStepTitle(el)).toBe("Connect GitHub"); + expect(currentStepAria(el)).toBe("Connect GitHub"); expect(el.querySelector(".chat-block-scene-why")?.textContent).toBe( STEPS[0]?.why, ); + expect( + el.querySelector(".chat-block-scene-body")?.getAttribute("aria-live"), + ).toBe("polite"); }); test("connected with nothing recorded yet marks Pick your repos", async () => { @@ -232,6 +245,7 @@ describe("the walkthrough marker follows the live connect state", () => { selectedRepoIds: [], }); expect(currentStepTitle(el)).toBe("Pick your repos"); + expect(currentStepAria(el)).toBe("Pick your repos"); expect(el.querySelector(".chat-block-scene-why")?.textContent).toBe( STEPS[1]?.why, ); @@ -246,6 +260,7 @@ describe("the walkthrough marker follows the live connect state", () => { selectedRepoIds: ["1", "2"], }); expect(currentStepTitle(el)).toBe("Start reviewing"); + expect(currentStepAria(el)).toBe("Start reviewing"); const done = el.querySelector(".chat-block-scene-reviewing"); expect(done?.textContent).toContain("Reviewing"); const names = [ @@ -319,9 +334,47 @@ 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"); + const alert = el.querySelector('[role="alert"]'); + expect(alert).not.toBeNull(); + expect(alert?.textContent).toContain("Couldn't start reviewing"); expect(report).toHaveBeenCalled(); expect(report.mock.calls[0]?.[1]).toMatchObject({ operation: "connect-github.startReviewing", }); }); + + test("a successful start reviewing after change repos moves focus onto the reviewing scene", async () => { + const el = await mount( + , + ); + const changeRepos = [...el.querySelectorAll("button")].find( + (button) => button.textContent === "change repos", + ); + await act(async () => { + changeRepos?.click(); + }); + const start = [...el.querySelectorAll("button")].find((button) => + button.textContent?.startsWith("Start reviewing"), + ); + await act(async () => { + start?.focus(); + start?.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + + const reviewing = el.querySelector(".chat-block-scene-reviewing"); + expect(reviewing).not.toBeNull(); + expect(currentStepAria(el)).toBe("Start reviewing"); + expect(reviewing?.contains(document.activeElement)).toBe(true); + }); }); From 785d64fd5a7fcc4148d64c0e3a61eec960af558f Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 27 Aug 2026 16:17:04 -0700 Subject: [PATCH 07/11] Chat UI: announce steps and keep start-reviewing failures on the card --- .../blocks/connect-github-block-container.tsx | 16 +++++ .../src/blocks/connect-github-block.tsx | 66 ++++++++++++++----- packages/chat-ui/src/strings.ts | 2 + packages/chat-ui/src/styles.css | 7 +- 4 files changed, 74 insertions(+), 17 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 f29ceafd0..7e3ae70ed 100644 --- a/packages/chat-ui/src/blocks/connect-github-block-container.tsx +++ b/packages/chat-ui/src/blocks/connect-github-block-container.tsx @@ -24,6 +24,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import type { ConnectGithubBlockData } from "@corbits/chat/blocks"; import { reportError } from "@corbits/error-sink"; +import { CHAT_STRINGS } from "../strings"; import type { ConnectGithubActions, ConnectGithubQuery, @@ -89,7 +90,11 @@ export function ConnectGithubBlockContainer({ * picker back without the server's recorded selection changing — only * pressing "Start reviewing" again writes anything. */ const [repickRequested, setRepickRequested] = useState(false); + const [startReviewingError, setStartReviewingError] = useState< + string | undefined + >(undefined); const mountedRef = useRef(true); + const hadPickerRef = useRef(false); const applyQuery = useCallback( (result: ConnectGithubQuery) => { @@ -147,6 +152,7 @@ export function ConnectGithubBlockContainer({ : recordedRepoIds.length === 0 || repickRequested ? STEP_PICK : STEP_REVIEWING; + if (currentStepIndex === STEP_PICK) hadPickerRef.current = true; const scene: OnboardingScene = { title: data.requiredForTemplate, currentStepIndex, @@ -174,6 +180,7 @@ export function ConnectGithubBlockContainer({ .filter((repo) => recordedRepoIds.includes(repo.id)) .map((repo) => repo.name)} onChangeRepos={() => setRepickRequested(true)} + {...(hadPickerRef.current ? { autoFocus: true } : {})} /> ); } @@ -190,10 +197,16 @@ export function ConnectGithubBlockContainer({ async function startReviewing(repoIds: readonly string[]) { try { + setStartReviewingError(undefined); await connectedActions.startReviewing(repoIds); if (mountedRef.current) setRepickRequested(false); } catch (cause) { reportError(cause, { operation: "connect-github.startReviewing" }); + if (mountedRef.current) { + setStartReviewingError( + CHAT_STRINGS.blockConnectGithubStartReviewingError, + ); + } } } @@ -215,6 +228,9 @@ export function ConnectGithubBlockContainer({ onSkip={() => { void connectedActions.skip(); }} + {...(startReviewingError !== undefined + ? { error: startReviewingError } + : {})} /> ); } diff --git a/packages/chat-ui/src/blocks/connect-github-block.tsx b/packages/chat-ui/src/blocks/connect-github-block.tsx index 59d92b616..7060dfaf1 100644 --- a/packages/chat-ui/src/blocks/connect-github-block.tsx +++ b/packages/chat-ui/src/blocks/connect-github-block.tsx @@ -17,7 +17,7 @@ // layout around it (name left, open-PR count right) is workbench-specific // composition and stays local. -import { useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { Button, Checkbox, Input } from "@corbits/react-ui"; import { Check } from "@corbits/icons"; @@ -83,11 +83,17 @@ export type ConnectGithubCardBody = readonly onChangeConnection: () => void; readonly onStartReviewing: (repoIds: readonly string[]) => void; readonly onSkip: () => void; + /** Set when `onStartReviewing` rejected — the picker stays up and + * this is the card's own alert, never a toast-only failure. */ + readonly error?: string; } | { readonly kind: "reviewing"; readonly repoNames: readonly string[]; readonly onChangeRepos: () => void; + /** Move focus onto the reviewing scene after Start reviewing + * succeeded, so it does not stay on a control that unmounted. */ + readonly autoFocus?: boolean; }; export type ConnectGithubCardProps = ConnectGithubCardBody & { @@ -136,6 +142,9 @@ function SceneHeader({ scene }: { readonly scene: OnboardingScene }) { key={step.title} className="chat-block-scene-step" {...(marked ? { "data-state": state } : {})} + aria-current={ + marked && state === "current" ? "step" : undefined + } > {step.title} @@ -161,12 +170,22 @@ function SceneHeader({ scene }: { readonly scene: OnboardingScene }) { function ReviewingBody({ repoNames, onChangeRepos, + autoFocus, }: { readonly repoNames: readonly string[]; readonly onChangeRepos: () => void; + readonly autoFocus?: boolean; }) { + const sceneRef = useRef(null); + useEffect(() => { + if (autoFocus === true) sceneRef.current?.focus(); + }, [autoFocus]); return ( -
    +

    +

    {error}

    ) : null} @@ -317,6 +340,7 @@ function ConnectedBody({ onChangeConnection, onStartReviewing, onSkip, + error, }: Extract) { const pickedCount = selectedRepoIds.length; return ( @@ -375,11 +399,18 @@ function ConnectedBody({ {CHAT_STRINGS.blockConnectGithubPermissionHelper}

    + {error !== undefined ? ( +

    + {error} +

    + ) : null} +
    @@ -395,19 +426,22 @@ export function ConnectGithubBlockView(props: ConnectGithubCardProps) { return ( - {props.kind === "disconnected" ? ( - - ) : null} - {props.kind === "connected" ? : null} - {props.kind === "reviewing" ? ( - - ) : null} +
    + {props.kind === "disconnected" ? ( + + ) : null} + {props.kind === "connected" ? : null} + {props.kind === "reviewing" ? ( + + ) : null} +
    ); } diff --git a/packages/chat-ui/src/strings.ts b/packages/chat-ui/src/strings.ts index de2ab30b6..29da1b96f 100644 --- a/packages/chat-ui/src/strings.ts +++ b/packages/chat-ui/src/strings.ts @@ -247,6 +247,8 @@ export const CHAT_STRINGS = { blockConnectGithubStartReviewing: (count: number) => `Start reviewing ${count} repo${count === 1 ? "" : "s"}`, blockConnectGithubSkip: "skip for now", + blockConnectGithubStartReviewingError: + "Couldn't start reviewing — try again.", blockConnectGithubTokenFieldLabel: "Personal access token", blockConnectGithubTokenFieldPlaceholder: "ghp_...", blockConnectGithubTokenSubmit: "Connect", diff --git a/packages/chat-ui/src/styles.css b/packages/chat-ui/src/styles.css index 99d3199a7..f6224fa7e 100644 --- a/packages/chat-ui/src/styles.css +++ b/packages/chat-ui/src/styles.css @@ -1247,7 +1247,12 @@ height: 0.4rem; border-radius: 0; background: currentColor; - animation: chat-block-pulse 1.6s ease infinite; +} + +@media (prefers-reduced-motion: no-preference) { + .chat-block-pulse { + animation: chat-block-pulse 1.6s ease infinite; + } } @keyframes chat-block-pulse { From db92032790e60e0746437f4b87591992cc5ab84d Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 27 Aug 2026 16:17:54 -0700 Subject: [PATCH 08/11] Update docs: first-minute onboarding scene --- ARCHITECTURE.md | 12 ++++++++++++ IMPLEMENTATION.md | 17 +++++++++++++++++ PRODUCT.md | 27 +++++++++++++++++---------- 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 518e41b4f..4ededf72c 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -88,6 +88,18 @@ settings") are composition on top of this tenant and its settings rows, not a separate object; the surface lives in `packages/chat-ui`'s `workbench-settings`. +**In-room onboarding scene.** A named template's first-minute walkthrough +is one timeline card posted in the room's own voice, not a member's +message and not a side effect of hosting an agent. The card keeps a +stable header — the job, the promise, the ordered steps — and flips its +body in place through connect, pick-repos, and reviewing. Change-repos +returns the body to the picker without rewriting what the room already +recorded. An empty step list is omitted, not rendered as an empty rail. +The current step is named in words; colour is additive, never the only +signal. Consecutive agent-joined events collapse into one line so the +scene and the reviewers' own introductions are what a person reads +first. + **Streaming a reply.** An agent's live reply reaches the timeline through one path, deltas to pixels: diff --git a/IMPLEMENTATION.md b/IMPLEMENTATION.md index e485dee54..1d3abd7a8 100644 --- a/IMPLEMENTATION.md +++ b/IMPLEMENTATION.md @@ -139,6 +139,23 @@ the current product path — do not document OAuth as the first Connect step, and do not treat a PAT paste as a defect against an OAuth-first welcome mat that has not shipped. +The room posts that card from `system@` +(`POST /workbenches/:id/onboarding` in `packages/chat/src/routes.ts`). +`packages/chat-ui` renders a system-sender `connect-github` block as a +scene: no author row, no avatar, no "Member" label; the job title and +optional promise stay put while the body flips. Step labels come from +the block's `steps` array — an empty or omitted array does not draw a +step list. The current-step marker ("You're here") is applied only when +there are exactly three steps (connect / pick / review); `data-state` on +the step is colour, never the only signal. After the server has recorded +repos, the body is the reviewing state (repo names plus a `change repos` +control). Clicking `change repos` is client-local in +`connect-github-block-container` and does not mutate the recorded +selection. `onReviewingStarted` posts canned introductions from +`packages/code-review/src/introductions.ts` under each reviewer's own +address in roster order. Consecutive `workbench.agent-joined` rows +collapse to one line in `packages/chat-ui/src/timeline.tsx`. + Optional `GITHUB_APP_CLIENT_ID` / `GITHUB_APP_CLIENT_SECRET` exist for that future hosted path; leaving them unset is normal. See `docs/connect-cards.md` and PRODUCT.md's Code review first minute. diff --git a/PRODUCT.md b/PRODUCT.md index 86eb1429e..5e33328ec 100644 --- a/PRODUCT.md +++ b/PRODUCT.md @@ -92,16 +92,23 @@ put there. Code review is the product scene for the definition-driven path: minting the Code review workbench opens an empty room with no host — its Workbench Definition names three reviewers and no Myra. The room -itself posts the onboarding card: Connect GitHub with a personal access -token (the shipped path today). The same in-room card reads live -connection state and flips in place to pick repositories — already -connected GitHub is that card, not a `/new` dialog — then Start -reviewing. Once reviewing starts, each reviewer posts its own canned -introduction under its own address, in roster order — the first thing -a person reads is who is reviewing and what for, never a join dump. A -GitHub App / hosted OAuth welcome mat is future work (CL-6343), not -current product. Inviting teammates into the room is a later slice, -not part of this first minute. +itself posts the onboarding card as a scene, not a member's message: +no author row, the job as its title, the promise beneath, and the +walkthrough's steps listed with the current one marked in words. A +walkthrough with no steps hides the step list rather than drawing an +empty one. Connect GitHub with a personal access token (the shipped +path today). The same in-room card reads live connection state and +flips in place to pick repositories — already connected GitHub is +that card, not a `/new` dialog — then Start reviewing. Once repos are +recorded the same card shows what it is reviewing, with a change-repos +link back to the picker — a reviewing card never still says Connect. +Once reviewing starts, each reviewer posts its own canned introduction +under its own address, in roster order — the first thing a person +reads is who is reviewing and what for, never a join dump. Consecutive +agent-joined rows collapse into one line naming everyone. A GitHub App +/ hosted OAuth welcome mat is future work (CL-6343), not current +product. Inviting teammates into the room is a later slice, not part +of this first minute. Settle for a template-key-only wait posts from a system sender and does not wake an agent. Generic `connections/pending` still wakes the From 21e234c37b8b57bb81c0ed7db82c6e0330607e85 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 27 Aug 2026 16:36:04 -0700 Subject: [PATCH 09/11] Chat UI: announce onboarding steps beside the card, not around it --- .../blocks/connect-github-block-container.tsx | 3 + .../src/blocks/connect-github-block.tsx | 29 +++- packages/chat-ui/src/styles.css | 19 +++ .../connect-github-block-container.test.tsx | 39 +++++- .../test/connect-github-block.test.tsx | 130 +++++++++++++++++- .../test/onboarding-scene-card.test.tsx | 11 +- 6 files changed, 222 insertions(+), 9 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 7e3ae70ed..7c9467796 100644 --- a/packages/chat-ui/src/blocks/connect-github-block-container.tsx +++ b/packages/chat-ui/src/blocks/connect-github-block-container.tsx @@ -95,6 +95,7 @@ export function ConnectGithubBlockContainer({ >(undefined); const mountedRef = useRef(true); const hadPickerRef = useRef(false); + const hadConnectRef = useRef(false); const applyQuery = useCallback( (result: ConnectGithubQuery) => { @@ -152,6 +153,7 @@ export function ConnectGithubBlockContainer({ : recordedRepoIds.length === 0 || repickRequested ? STEP_PICK : STEP_REVIEWING; + if (currentStepIndex === STEP_CONNECT) hadConnectRef.current = true; if (currentStepIndex === STEP_PICK) hadPickerRef.current = true; const scene: OnboardingScene = { title: data.requiredForTemplate, @@ -231,6 +233,7 @@ export function ConnectGithubBlockContainer({ {...(startReviewingError !== undefined ? { error: startReviewingError } : {})} + {...(hadConnectRef.current ? { autoFocus: true } : {})} /> ); } diff --git a/packages/chat-ui/src/blocks/connect-github-block.tsx b/packages/chat-ui/src/blocks/connect-github-block.tsx index 7060dfaf1..7f74ea000 100644 --- a/packages/chat-ui/src/blocks/connect-github-block.tsx +++ b/packages/chat-ui/src/blocks/connect-github-block.tsx @@ -86,6 +86,9 @@ export type ConnectGithubCardBody = /** Set when `onStartReviewing` rejected — the picker stays up and * this is the card's own alert, never a toast-only failure. */ readonly error?: string; + /** Move focus onto the pick-repos heading after Connect succeeded, + * so it does not stay on a control that unmounted. */ + readonly autoFocus?: boolean; } | { readonly kind: "reviewing"; @@ -341,10 +344,23 @@ function ConnectedBody({ onStartReviewing, onSkip, error, + autoFocus, }: Extract) { + const headingRef = useRef(null); + useEffect(() => { + if (autoFocus === true) headingRef.current?.focus(); + }, [autoFocus]); const pickedCount = selectedRepoIds.length; return ( <> +

    + {CHAT_STRINGS.blockConnectGithubPickHeadline} +

    {repos.map((repo) => { const selected = selectedRepoIds.includes(repo.id); @@ -423,10 +439,19 @@ function ConnectedBody({ } export function ConnectGithubBlockView(props: ConnectGithubCardProps) { + const currentStepTitle = + props.scene.steps?.[props.scene.currentStepIndex]?.title; return ( -
    +
    + {currentStepTitle ?? null} +
    +
    {props.kind === "disconnected" ? ( { root = null; }); -async function mount(actions: ConnectGithubActions) { +async function mount(actions: ConnectGithubActions, messageId = "m1") { container = document.createElement("div"); document.body.appendChild(container); root = createRoot(container); @@ -47,7 +47,7 @@ async function mount(actions: ConnectGithubActions) { root?.render( , ); @@ -140,6 +140,41 @@ describe("ConnectGithubBlockContainer post-submit refresh (CL-6463)", () => { ); }); + test("a successful PAT submit moves focus onto the pick-repos heading", async () => { + const harness = buildNeverNotifiesHarness(); + const el = await mount(harness.actions, "m_pat_focus"); + + const connectButton = [...el.querySelectorAll("button")].find( + (button) => button.textContent === "Connect GitHub", + ) as HTMLButtonElement; + await act(async () => { + connectButton.click(); + }); + const tokenField = el.querySelector( + "#connect-github-token", + ) as HTMLInputElement; + await act(async () => { + typeInto(tokenField, "ghp_test123"); + }); + const submitButton = [...el.querySelectorAll("button")].find( + (button) => button.textContent === "Connect", + ) as HTMLButtonElement; + await act(async () => { + submitButton.focus(); + submitButton.click(); + }); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + + const heading = el.querySelector(".chat-block-scene-pick-heading"); + expect(heading?.textContent).toBe("Pick your repos"); + expect(heading?.getAttribute("tabindex")).toBe("-1"); + expect(document.activeElement).toBe(heading); + expect(el.querySelector("#connect-github-token")).toBeNull(); + }); + test("a rejected token shows what went wrong and leaves a working submit button, never a dead card", async () => { const harness = buildNeverNotifiesHarness({ submitResult: { ok: false, message: "That token looks expired." }, diff --git a/packages/chat-ui/test/connect-github-block.test.tsx b/packages/chat-ui/test/connect-github-block.test.tsx index b2e5d1cb2..1e020a023 100644 --- a/packages/chat-ui/test/connect-github-block.test.tsx +++ b/packages/chat-ui/test/connect-github-block.test.tsx @@ -432,7 +432,12 @@ describe("connect GitHub card — accessibility", () => { } const group = el.querySelector('[role="group"]'); - expect(group?.getAttribute("aria-label")).toBe("Pick your repos"); + expect( + el.querySelector(".chat-block-scene-pick-heading")?.textContent, + ).toBe("Pick your repos"); + expect(group?.getAttribute("aria-labelledby")).toBe( + "connect-github-pick-heading", + ); const firstCheckbox = checkboxes[0]; if (firstCheckbox === undefined) { @@ -442,7 +447,7 @@ describe("connect GitHub card — accessibility", () => { expect(document.activeElement).toBe(firstCheckbox); }); - test("the current walkthrough step is marked aria-current=step and the scene body is a live region", async () => { + test("the current walkthrough step is marked aria-current=step and a sibling status live region names it", async () => { const el = await mount({ kind: "disconnected", onConnect: () => undefined, @@ -458,9 +463,126 @@ describe("connect GitHub card — accessibility", () => { (row) => row.getAttribute("aria-current") === "step", ), ).toHaveLength(1); + const status = el.querySelector(".chat-block-scene-status"); + const body = el.querySelector(".chat-block-scene-body"); + expect(body?.getAttribute("aria-live")).toBeNull(); + expect(status?.getAttribute("aria-live")).toBe("polite"); + expect(status?.getAttribute("aria-atomic")).toBe("true"); + expect(status?.textContent).toBe("Connect GitHub"); + expect(body?.contains(status)).toBe(false); + expect( + el.querySelector("#connect-github-token") ?? + el.querySelector('input[type="password"]'), + ).toBeNull(); + }); + + test("a token error is an alert beside the status live region, not nested inside it", async () => { + const el = await mount({ + kind: "disconnected", + onConnect: () => undefined, + onSubmitAccessToken: () => + Promise.resolve({ ok: false, message: "Bad token." }), + }); + + const openLink = [...el.querySelectorAll("button")].find( + (button) => button.textContent === "Connect GitHub", + ) as HTMLButtonElement; + await act(async () => { + openLink.click(); + }); + + const field = el.querySelector("#connect-github-token") as HTMLInputElement; + await act(async () => { + typeInto(field, "ghp_bad"); + }); + const submit = [...el.querySelectorAll("button")].find( + (button) => button.textContent === "Connect", + ) as HTMLButtonElement; + await act(async () => { + submit.click(); + }); + await act(async () => { + await Promise.resolve(); + }); + + const status = el.querySelector(".chat-block-scene-status"); + const alert = el.querySelector('[role="alert"]'); + expect(alert?.textContent).toContain("Bad token."); + expect(status?.contains(alert)).toBe(false); + expect(status?.textContent).toBe("Connect GitHub"); + expect(el.querySelector(".chat-block-scene-body")?.contains(field)).toBe( + true, + ); + }); + + test("a start-reviewing error is an alert beside the status live region, not nested inside it", async () => { + const el = await mountElement( + undefined} + onSelectAll={() => undefined} + onChangeConnection={() => undefined} + onStartReviewing={() => undefined} + onSkip={() => undefined} + error="Couldn't start reviewing — try again." + />, + ); + + const status = el.querySelector(".chat-block-scene-status"); + const alert = el.querySelector('[role="alert"]'); + expect(status?.textContent).toBe("Pick your repos"); + expect(alert?.textContent).toContain("Couldn't start reviewing"); + expect(status?.contains(alert)).toBe(false); expect( - el.querySelector(".chat-block-scene-body")?.getAttribute("aria-live"), - ).toBe("polite"); + el + .querySelector(".chat-block-scene-body") + ?.contains(el.querySelector('input[type="checkbox"]')), + ).toBe(true); + }); + + test("toggling a repo checkbox does not change the status live region", async () => { + const el = await mountElement( + undefined} + />, + ); + const status = el.querySelector(".chat-block-scene-status"); + expect(status?.textContent).toBe("Connect GitHub"); + + const mobileCheckbox = [ + ...el.querySelectorAll('input[type="checkbox"]'), + ][3]; + await act(async () => { + mobileCheckbox?.click(); + }); + + expect(status?.textContent).toBe("Connect GitHub"); + expect(el.textContent).toContain("6 repos found · 2 picked"); + }); + + test("autoFocus on the pick-repos scene moves focus onto the pick heading", async () => { + const el = await mount({ + kind: "connected", + orgName: "acme", + repos: REPOS, + selectedRepoIds: [], + onToggleRepo: () => undefined, + onSelectAll: () => undefined, + onChangeConnection: () => undefined, + onStartReviewing: () => undefined, + onSkip: () => undefined, + autoFocus: true, + }); + + const heading = el.querySelector(".chat-block-scene-pick-heading"); + expect(heading?.textContent).toBe("Pick your repos"); + expect(heading?.getAttribute("tabindex")).toBe("-1"); + expect(document.activeElement).toBe(heading); }); test("the disconnected state's actions are real buttons, not divs", async () => { diff --git a/packages/chat-ui/test/onboarding-scene-card.test.tsx b/packages/chat-ui/test/onboarding-scene-card.test.tsx index bde784e1d..f0c8fc5a3 100644 --- a/packages/chat-ui/test/onboarding-scene-card.test.tsx +++ b/packages/chat-ui/test/onboarding-scene-card.test.tsx @@ -232,9 +232,15 @@ describe("the walkthrough marker follows the live connect state", () => { expect(el.querySelector(".chat-block-scene-why")?.textContent).toBe( STEPS[0]?.why, ); + const status = el.querySelector(".chat-block-scene-status"); + expect(status?.getAttribute("aria-live")).toBe("polite"); + expect(status?.textContent).toBe("Connect GitHub"); expect( el.querySelector(".chat-block-scene-body")?.getAttribute("aria-live"), - ).toBe("polite"); + ).toBeNull(); + expect(el.querySelector(".chat-block-scene-body")?.contains(status)).toBe( + false, + ); }); test("connected with nothing recorded yet marks Pick your repos", async () => { @@ -337,6 +343,9 @@ describe("the walkthrough marker follows the live connect state", () => { 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?.contains(alert)).toBe(false); expect(report).toHaveBeenCalled(); expect(report.mock.calls[0]?.[1]).toMatchObject({ operation: "connect-github.startReviewing", From dc06d0fd56ae5465a1edfae79e90f5c2487c317d Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 27 Aug 2026 16:56:35 -0700 Subject: [PATCH 10/11] Chat UI: show GitHub repo-read errors on the card A successful PAT can still leave github/state as kind error. The card treated every non-connected query as disconnected, so that failure remounted as a silent Connect GitHub CTA. The error message now renders as an alert with Reconnect. --- .../blocks/connect-github-block-container.tsx | 12 +++++++ .../src/blocks/connect-github-block.tsx | 36 +++++++++++++++++-- packages/chat-ui/src/strings.ts | 1 + .../connect-github-block-container.test.tsx | 31 ++++++++++++++++ 4 files changed, 77 insertions(+), 3 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 7c9467796..177a309e3 100644 --- a/packages/chat-ui/src/blocks/connect-github-block-container.tsx +++ b/packages/chat-ui/src/blocks/connect-github-block-container.tsx @@ -162,6 +162,18 @@ export function ConnectGithubBlockContainer({ ...(data.steps !== undefined ? { steps: data.steps } : {}), }; + if (displayQuery.kind === "error") { + return ( + actions?.requestConnect()} + onSubmitAccessToken={submitAccessTokenAndRefresh} + /> + ); + } + if (actions === undefined || displayQuery.kind !== "connected") { return ( ; } + | { + readonly kind: "error"; + readonly message: string; + readonly onConnect: () => void; + readonly onSubmitAccessToken: ( + token: string, + ) => Promise< + { readonly ok: true } | { readonly ok: false; readonly message: string } + >; + } | { readonly kind: "connected"; readonly orgName: string; @@ -215,6 +225,8 @@ function ReviewingBody({ function DisconnectedBody({ onConnect, onSubmitAccessToken, + error: stateError, + actionLabel, }: { readonly onConnect: () => void; readonly onSubmitAccessToken: ( @@ -222,11 +234,13 @@ function DisconnectedBody({ ) => Promise< { readonly ok: true } | { readonly ok: false; readonly message: string } >; + readonly error?: string; + readonly actionLabel?: string; }) { const [fieldOpen, setFieldOpen] = useState(false); const [token, setToken] = useState(""); const [submitting, setSubmitting] = useState(false); - const [error, setError] = useState(undefined); + const [error, setError] = useState(stateError); function openField() { onConnect(); @@ -309,7 +323,7 @@ function DisconnectedBody({ onClick={() => { setFieldOpen(false); setToken(""); - setError(undefined); + setError(stateError); }} > {CHAT_STRINGS.blockConnectGithubTokenCancel} @@ -321,10 +335,18 @@ function DisconnectedBody({ return ( <> + {error !== undefined ? ( +

    + {error} +

    + ) : null}

    {CHAT_STRINGS.blockConnectGithubIntro}

    @@ -458,6 +480,14 @@ export function ConnectGithubBlockView(props: ConnectGithubCardProps) { onSubmitAccessToken={props.onSubmitAccessToken} /> ) : null} + {props.kind === "error" ? ( + + ) : null} {props.kind === "connected" ? : null} {props.kind === "reviewing" ? ( { + test("query kind error shows the state message as an alert, not a silent Connect GitHub primary", async () => { + const message = + "Couldn't read your GitHub repositories. Try reconnecting."; + const actions: ConnectGithubActions = { + getConnectState: () => + Promise.resolve({ kind: "error", message }), + subscribeConnectState: () => () => {}, + requestConnect: () => {}, + submitAccessToken: async () => ({ ok: true as const }), + startReviewing: async () => ({ startedTriggerCount: 0 }), + skip: async () => {}, + }; + + const el = await mount(actions, "m_state_error"); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + + const alert = el.querySelector('[role="alert"]'); + expect(alert).not.toBeNull(); + expect(alert?.textContent).toBe(message); + + const connectPrimary = [...el.querySelectorAll("button")].find( + (button) => button.textContent === "Connect GitHub", + ); + expect(connectPrimary).toBeUndefined(); + }); +}); From dc6fb7ceba1ca9406cb9c35a43f3a96046a3dc2a Mon Sep 17 00:00:00 2001 From: x Date: Fri, 28 Aug 2026 12:13:13 -0700 Subject: [PATCH 11/11] Format connect-github-block-container test --- .../chat-ui/test/connect-github-block-container.test.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) 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 224bcb12a..796bf9d71 100644 --- a/packages/chat-ui/test/connect-github-block-container.test.tsx +++ b/packages/chat-ui/test/connect-github-block-container.test.tsx @@ -297,11 +297,9 @@ describe("ConnectGithubBlockContainer keeps connected across loading (CL-6741)", describe("ConnectGithubBlockContainer names a kind:error state (PR 422)", () => { test("query kind error shows the state message as an alert, not a silent Connect GitHub primary", async () => { - const message = - "Couldn't read your GitHub repositories. Try reconnecting."; + const message = "Couldn't read your GitHub repositories. Try reconnecting."; const actions: ConnectGithubActions = { - getConnectState: () => - Promise.resolve({ kind: "error", message }), + getConnectState: () => Promise.resolve({ kind: "error", message }), subscribeConnectState: () => () => {}, requestConnect: () => {}, submitAccessToken: async () => ({ ok: true as const }),