From e56fa2d50053d43f1dcd4b430f52c049db8ab8d0 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 24 Aug 2026 08:46:34 -0700 Subject: [PATCH 1/3] Settle GitHub connect card and system notice after success Closes CL-6741 --- .../blocks/connect-github-block-container.tsx | 74 +++++++++++--- packages/chat-ui/src/strings.ts | 8 ++ packages/chat-ui/src/styles.css | 6 ++ packages/chat-ui/src/timeline.test.ts | 13 +++ packages/chat-ui/src/timeline.tsx | 35 ++++++- .../connect-github-block-container.test.tsx | 97 +++++++++++++++++++ .../chat-ui/test/system-row-chrome.test.tsx | 27 ++++++ packages/chat/src/connect-pending.ts | 39 +++++++- packages/chat/src/index.ts | 2 + packages/chat/test/connect-pending.test.ts | 70 ++++++++++--- 10 files changed, 341 insertions(+), 30 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 86eb74479..9089cb372 100644 --- a/packages/chat-ui/src/blocks/connect-github-block-container.tsx +++ b/packages/chat-ui/src/blocks/connect-github-block-container.tsx @@ -14,6 +14,12 @@ // `settleConnectedService`, which clears this room's own // `template/pendingConnections` entry — so this card's next mount already // reads connected without needing a push while it sits open. +// +// CL-6741: once a card has read connected, a later loading/error fold +// (or a remount that starts on loading) must keep the last connected +// snapshot — never flash DisconnectedBody / "Connect" again over a +// known-good connection. An explicit `disconnected` result clears the +// snapshot so a real disconnect still shows Connect. import { useCallback, useEffect, useRef, useState } from "react"; import type { ConnectGithubBlockData } from "@corbits/chat/blocks"; @@ -23,6 +29,36 @@ import type { } from "./connect-github-actions"; import { ConnectGithubBlockView } from "./connect-github-block"; +type ConnectedGithubQuery = Extract; + +/** Survives container remounts so a post-connect loading flash never + * resets the card to Connect (CL-6741). Cleared only on an explicit + * disconnected result. */ +const lastConnectedByMessageId = new Map(); + +function rememberConnected(messageId: string, query: ConnectedGithubQuery) { + lastConnectedByMessageId.set(messageId, query); +} + +function forgetConnected(messageId: string) { + lastConnectedByMessageId.delete(messageId); +} + +function lastConnectedOf(messageId: string): ConnectedGithubQuery | undefined { + return lastConnectedByMessageId.get(messageId); +} + +function displayQueryOf( + messageId: string, + query: ConnectGithubQuery, +): ConnectGithubQuery { + if (query.kind === "connected" || query.kind === "disconnected") { + return query; + } + const prior = lastConnectedOf(messageId); + return prior ?? query; +} + export function ConnectGithubBlockContainer({ messageId, actions, @@ -31,15 +67,27 @@ export function ConnectGithubBlockContainer({ readonly messageId: string; readonly actions?: ConnectGithubActions; }) { - const [query, setQuery] = useState({ kind: "loading" }); - const [selectedRepoIds, setSelectedRepoIds] = useState([]); + const [query, setQuery] = useState(() => { + return lastConnectedOf(messageId) ?? { kind: "loading" }; + }); + const [selectedRepoIds, setSelectedRepoIds] = useState( + () => lastConnectedOf(messageId)?.selectedRepoIds ?? [], + ); const mountedRef = useRef(true); - const applyQuery = useCallback((result: ConnectGithubQuery) => { - if (!mountedRef.current) return; - setQuery(result); - if (result.kind === "connected") setSelectedRepoIds(result.selectedRepoIds); - }, []); + const applyQuery = useCallback( + (result: ConnectGithubQuery) => { + if (!mountedRef.current) return; + if (result.kind === "connected") { + rememberConnected(messageId, result); + setSelectedRepoIds(result.selectedRepoIds); + } else if (result.kind === "disconnected") { + forgetConnected(messageId); + } + setQuery(result); + }, + [messageId], + ); useEffect(() => { mountedRef.current = true; @@ -74,7 +122,9 @@ export function ConnectGithubBlockContainer({ [actions, messageId, applyQuery], ); - if (actions === undefined || query.kind !== "connected") { + const displayQuery = displayQueryOf(messageId, query); + + if (actions === undefined || displayQuery.kind !== "connected") { return ( setSelectedRepoIds(query.repos.map((repo) => repo.id))} + onSelectAll={() => + setSelectedRepoIds(displayQuery.repos.map((repo) => repo.id)) + } onChangeConnection={actions.requestConnect} onStartReviewing={(repoIds) => { void actions.startReviewing(repoIds); diff --git a/packages/chat-ui/src/strings.ts b/packages/chat-ui/src/strings.ts index 4b6c2f845..60c2f6e67 100644 --- a/packages/chat-ui/src/strings.ts +++ b/packages/chat-ui/src/strings.ts @@ -91,7 +91,15 @@ export const CHAT_STRINGS = { eventWorkbenchRenamedTo: (to: string): string => `Renamed to "${to}"`, eventBlockResponsePoll: "A vote was recorded", eventBlockResponseForm: "A form was submitted", + /** Plain-text form of the settle notice (CL-6741). EventLine renders + * the same copy with "Plugins" as a `/plugins` link. */ + eventConnectionConnected: (displayName: string): string => + `${displayName} connected successfully. Manage in Plugins`, + eventConnectionConnectedBeforePlugins: (displayName: string): string => + `${displayName} connected successfully. Manage in `, + eventConnectionConnectedPlugins: "Plugins", eventGeneric: (event: string) => event.replace(/[.\-_]+/g, " "), + inviteAgentAction: "Invite agent", workbenchMembersLabel: "Members", teamStackOverflow: (count: number) => diff --git a/packages/chat-ui/src/styles.css b/packages/chat-ui/src/styles.css index c27f4b5d9..4ec3000c9 100644 --- a/packages/chat-ui/src/styles.css +++ b/packages/chat-ui/src/styles.css @@ -1187,6 +1187,12 @@ color: var(--muted-foreground); } +.chat-event-line a { + color: inherit; + text-decoration: underline; + text-underline-offset: 0.12em; +} + .chat-event-time { font-size: 0.6875rem; opacity: 0.85; diff --git a/packages/chat-ui/src/timeline.test.ts b/packages/chat-ui/src/timeline.test.ts index dda3856c0..0888a06c7 100644 --- a/packages/chat-ui/src/timeline.test.ts +++ b/packages/chat-ui/src/timeline.test.ts @@ -46,3 +46,16 @@ describe("friendlyEventText — workbench.agent-joined (CL-6594)", () => { expect(friendlyEventText(part, [])).toBe("An agent joined"); }); }); + +describe("friendlyEventText — connection.connected (CL-6741)", () => { + test("names the connected service and points at Plugins", () => { + const part: Part & { kind: "event" } = { + kind: "event", + event: "connection.connected", + data: { connectorId: "github", displayName: "GitHub" }, + }; + expect(friendlyEventText(part, [])).toBe( + "GitHub connected successfully. Manage in Plugins", + ); + }); +}); diff --git a/packages/chat-ui/src/timeline.tsx b/packages/chat-ui/src/timeline.tsx index baac298bf..231924354 100644 --- a/packages/chat-ui/src/timeline.tsx +++ b/packages/chat-ui/src/timeline.tsx @@ -644,6 +644,15 @@ export function friendlyEventText( ? CHAT_STRINGS.eventBlockResponsePoll : CHAT_STRINGS.eventBlockResponseForm; } + case "connection.connected": { + const displayName = + data !== undefined && typeof data.displayName === "string" + ? data.displayName + : undefined; + return displayName !== undefined + ? CHAT_STRINGS.eventConnectionConnected(displayName) + : CHAT_STRINGS.eventGeneric(part.event); + } default: return CHAT_STRINGS.eventGeneric(part.event); } @@ -658,9 +667,33 @@ function EventLine({ createdAt: string; participants: readonly ParticipantRecord[]; }) { + const data = + typeof part.data === "object" && part.data !== null + ? (part.data as Record) + : undefined; + const connectedDisplayName = + part.event === "connection.connected" && + data !== undefined && + typeof data.displayName === "string" + ? data.displayName + : undefined; + return (
- {friendlyEventText(part, participants)} + + {connectedDisplayName !== undefined ? ( + <> + {CHAT_STRINGS.eventConnectionConnectedBeforePlugins( + connectedDisplayName, + )} + + {CHAT_STRINGS.eventConnectionConnectedPlugins} + + + ) : ( + friendlyEventText(part, participants) + )} + {formatTimestamp(createdAt)}
); 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 ae0e9b98c..c79488870 100644 --- a/packages/chat-ui/test/connect-github-block-container.test.tsx +++ b/packages/chat-ui/test/connect-github-block-container.test.tsx @@ -162,3 +162,100 @@ describe("ConnectGithubBlockContainer post-submit refresh (CL-6463)", () => { expect(tokenField.disabled).toBe(false); }); }); + +describe("ConnectGithubBlockContainer keeps connected across loading (CL-6741)", () => { + test("a loading remount after connected keeps ConnectedBody — never flashes Connect", async () => { + let state: ConnectGithubQuery = { + kind: "connected", + orgName: "octocat", + repos: REPOS, + selectedRepoIds: [], + }; + let subscriber: ((next: ConnectGithubQuery) => void) | undefined; + + const actions: ConnectGithubActions = { + getConnectState: () => Promise.resolve(state), + subscribeConnectState: (_messageId, onUpdate) => { + subscriber = onUpdate; + return () => { + subscriber = undefined; + }; + }, + requestConnect: () => {}, + submitAccessToken: async () => ({ ok: true as const }), + startReviewing: async () => ({ startedTriggerCount: 0 }), + skip: async () => {}, + }; + + const el = await mount(actions); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + expect(el.textContent).toContain("Connected to GitHub as octocat"); + expect(el.textContent).not.toContain("Connect GitHub"); + + await act(async () => { + state = { kind: "loading" }; + subscriber?.({ kind: "loading" }); + }); + expect(el.textContent).toContain("Connected to GitHub as octocat"); + expect(el.textContent).not.toContain("Connect GitHub"); + expect(el.querySelectorAll(".chat-block-connect-repo-row")).toHaveLength( + REPOS.length, + ); + + // Remount while the host still reports loading — the last connected + // snapshot must survive so Connect never flashes. + if (root !== null) { + await act(async () => { + root?.unmount(); + }); + root = null; + } + container?.remove(); + container = null; + + const remounted = await mount(actions); + expect(remounted.textContent).toContain("Connected to GitHub as octocat"); + expect(remounted.textContent).not.toContain("Connect GitHub"); + }); + + test("an explicit disconnected result after connected does show Connect again", async () => { + let state: ConnectGithubQuery = { + kind: "connected", + orgName: "octocat", + repos: REPOS, + selectedRepoIds: [], + }; + let subscriber: ((next: ConnectGithubQuery) => void) | undefined; + + const actions: ConnectGithubActions = { + getConnectState: () => Promise.resolve(state), + subscribeConnectState: (_messageId, onUpdate) => { + subscriber = onUpdate; + return () => { + subscriber = undefined; + }; + }, + requestConnect: () => {}, + submitAccessToken: async () => ({ ok: true as const }), + startReviewing: async () => ({ startedTriggerCount: 0 }), + skip: async () => {}, + }; + + const el = await mount(actions); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + expect(el.textContent).toContain("Connected to GitHub as octocat"); + + await act(async () => { + state = { kind: "disconnected" }; + subscriber?.({ kind: "disconnected" }); + }); + expect(el.textContent).toContain("Connect GitHub"); + expect(el.textContent).not.toContain("Connected to GitHub as"); + }); +}); diff --git a/packages/chat-ui/test/system-row-chrome.test.tsx b/packages/chat-ui/test/system-row-chrome.test.tsx index 92e608922..02c028b7f 100644 --- a/packages/chat-ui/test/system-row-chrome.test.tsx +++ b/packages/chat-ui/test/system-row-chrome.test.tsx @@ -82,6 +82,33 @@ describe("CL-6739: system / error / connect rows hide social chrome", () => { expectNoSocialChrome(el); }); + test("connection.connected settle notice links Plugins to /plugins (CL-6741)", async () => { + const el = await mount([ + { + id: "settle_1", + createdAt: "2026-01-01T00:00:00.000Z", + parts: [ + { + kind: "event", + event: "connection.connected", + data: { connectorId: "github", displayName: "GitHub" }, + }, + ], + sender: { name: null, address: "system@agents.example" }, + } as MessageItem, + ]); + + const line = el.querySelector(".chat-event-line"); + expect(line).not.toBeNull(); + expect(line?.textContent).toContain( + "GitHub connected successfully. Manage in Plugins", + ); + const pluginsLink = line?.querySelector('a[href="/plugins"]'); + expect(pluginsLink).not.toBeNull(); + expect(pluginsLink?.textContent).toBe("Plugins"); + expectNoSocialChrome(el); + }); + test("a generic system event row has no reaction, reply, or overflow", async () => { const el = await mount([ { diff --git a/packages/chat/src/connect-pending.ts b/packages/chat/src/connect-pending.ts index 6dcf9c914..96d24f597 100644 --- a/packages/chat/src/connect-pending.ts +++ b/packages/chat/src/connect-pending.ts @@ -3,9 +3,10 @@ // the room's own settings under `connections/pending`; when the // connection completes in the browser, `settleConnectedService` finds // every room in the tenant still waiting on that connector, clears the -// entry (publishing `chat.settings` so the open card flips), and wakes -// the room's host agent via `dispatchTurn` — never by posting a timeline -// row as the connecting person. +// entry (publishing `chat.settings` so the open card flips), posts an +// event-only system notice onto the room timeline, and wakes the room's +// host agent via `dispatchTurn` — never by posting a human text bubble +// as the connecting person. // // The code-review template's own GitHub connect card registers // under a second, template-owned key (`@corbits/workflow-catalog`'s @@ -21,7 +22,11 @@ import { localPartOf } from "./agent-address"; import { ConnectServiceBlockData } from "./blocks"; import { isAgentAddress } from "./mentions"; import type { Part as PartType } from "./parts"; -import type { RoomMessage, RoomMessageStore } from "./room-messages"; +import { + postRoomMessage, + type RoomMessage, + type RoomMessageStore, +} from "./room-messages"; import type { ChatStore } from "./store"; import { dispatchTurn, @@ -31,6 +36,10 @@ import { participantsOf } from "./workbench-settings"; export const CONNECTIONS_PENDING_KEY = "connections/pending"; +/** Timeline event name for the settle notice `settleConnectedService` + * posts (CL-6741) — event-only, never a human text bubble. */ +export const CONNECTION_CONNECTED_EVENT = "connection.connected"; + /** The code-review template's own pending-connections key * (`@corbits/workflow-catalog`'s `templateSettingsPatch`/ * `templateReposSettingsPatch`) — a room minted from that template @@ -179,6 +188,28 @@ export async function settleConnectedService( }); const agentAddress = hostAgentAddress(updated.settings, input.principalId); + // CL-6741: event-only system row — never a signed-in user text bubble. + // Sender is the host agent when one exists; otherwise a synthetic + // system address so the row never attributes to the connecting person. + await postRoomMessage(deps, { + tenantId: input.tenantId, + workbenchId: row.workbenchId, + sender: { + name: null, + address: agentAddress ?? `system@${row.workbenchId}`, + }, + parts: [ + { + kind: "event", + event: CONNECTION_CONNECTED_EVENT, + data: { + connectorId: input.connectorId, + displayName: input.displayName, + }, + }, + ], + }); + if (agentAddress === undefined) continue; const requestMessageIds = await existingRequestMessageIds( diff --git a/packages/chat/src/index.ts b/packages/chat/src/index.ts index 765d3ac38..f872d4ae6 100644 --- a/packages/chat/src/index.ts +++ b/packages/chat/src/index.ts @@ -23,11 +23,13 @@ export { } from "./blocks"; export type { Block, BlockParseResult } from "./blocks"; export { + CONNECTION_CONNECTED_EVENT, CONNECTIONS_PENDING_KEY, pendingConnectionsOf, connectServiceConnectorIds, settleConnectedService, } from "./connect-pending"; + export type { SettleConnectedServiceDeps, SettleConnectedServiceInput, diff --git a/packages/chat/test/connect-pending.test.ts b/packages/chat/test/connect-pending.test.ts index 590712be4..cc4c2397d 100644 --- a/packages/chat/test/connect-pending.test.ts +++ b/packages/chat/test/connect-pending.test.ts @@ -1,8 +1,9 @@ // Tests for the connect-settling half of the in-room connect flow: a // connection completing in the browser settles every room that was waiting // on it — the pending entry clears, `chat.settings` fires so the card -// flips, and the host agent is woken via `dispatchTurn` / `sendMail` -// without a new timeline row authored as the signed-in user. +// flips, an event-only system notice lands on the timeline (CL-6741), and +// the host agent is woken via `dispatchTurn` / `sendMail` without a new +// timeline row authored as the signed-in user. import { expect, test } from "bun:test"; import { createInMemoryAgentTurnStore } from "../src/agent-turns"; @@ -10,7 +11,10 @@ import { createInMemoryChatStore } from "../src/store"; import { createInMemoryRoomMessageStore } from "../src/room-messages"; import { createInMemoryTurnClaimStore } from "../src/turn-claims"; import { createWorkbenchTurnQueue } from "../src/turn-queue"; -import { settleConnectedService } from "../src/connect-pending"; +import { + CONNECTION_CONNECTED_EVENT, + settleConnectedService, +} from "../src/connect-pending"; import { fakePlatform, TENANT } from "./test-support"; const HUMAN_ADDRESS = "prn_owner@acme.example"; @@ -80,6 +84,31 @@ function buildDeps() { return { store, roomMessages, published, platform, agentTurns, deps }; } +function expectEventOnlySettleNotice( + items: Awaited< + ReturnType["listMessages"]> + >["items"], + displayName: string, +) { + const notices = items.filter( + (item) => + item.parts.length > 0 && + item.parts.every((part) => part.kind === "event"), + ); + expect(notices.length).toBeGreaterThanOrEqual(1); + const notice = notices[0]!; + expect(notice.senderPrincipalId).toBeNull(); + expect(notice.sender.address).not.toBe(HUMAN_ADDRESS); + const part = notice.parts[0]!; + expect(part.kind).toBe("event"); + if (part.kind !== "event") return; + expect(part.event).toBe(CONNECTION_CONNECTED_EVENT); + expect(part.data).toEqual({ + connectorId: expect.any(String), + displayName, + }); +} + test("After connect, no new timeline row is authored as the signed-in user by the product", async () => { const { store, roomMessages, published, platform, agentTurns, deps } = buildDeps(); @@ -103,14 +132,15 @@ test("After connect, no new timeline row is authored as the signed-in user by th ), ).toBe(true); expect(published.some((entry) => entry.event.type === "chat.message")).toBe( - false, + true, ); const listed = await roomMessages.listMessages({ tenantId: TENANT.id, workbenchId: "chan_waiting", }); - expect(listed.items).toHaveLength(0); + expect(listed.items).toHaveLength(1); + expectEventOnlySettleNotice(listed.items, "Gmail"); expect(platform.sentMail).toHaveLength(1); expect(platform.sentMail[0]?.workbenchId).toBe("ins_myra"); @@ -173,23 +203,26 @@ test("Connect card flips in place; agent wakes without a forged user message", a ), ).toBe(true); expect(published.some((entry) => entry.event.type === "chat.message")).toBe( - false, + true, ); const listed = await roomMessages.listMessages({ tenantId: TENANT.id, workbenchId: "chan_waiting", }); - expect(listed.items).toHaveLength(2); - expect(listed.items.map((item) => item.id).toSorted()).toEqual([ - "msg_1", - "msg_2", - ]); + expect(listed.items).toHaveLength(3); + expect(listed.items.map((item) => item.id).toSorted()).toEqual( + expect.arrayContaining(["msg_1", "msg_2"]), + ); expect( listed.items.some((item) => JSON.stringify(item.parts).includes("is connected now"), ), ).toBe(false); + expectEventOnlySettleNotice(listed.items, "Gmail"); + expect( + listed.items.filter((item) => item.senderPrincipalId === "prn_owner"), + ).toHaveLength(1); expect(platform.sentMail).toHaveLength(1); expect(platform.sentMail[0]?.workbenchId).toBe("ins_myra"); @@ -219,7 +252,8 @@ test("matches a pending mcp-prefixed entry when the preset connects under its ba tenantId: TENANT.id, workbenchId: "chan_waiting", }); - expect(listed.items).toHaveLength(0); + expect(listed.items).toHaveLength(1); + expectEventOnlySettleNotice(listed.items, "Notion"); }); test("settles a room whose GitHub card is pending under the code-review template's own key — a credential created out of band (not through that card's own submit) still reaches it", async () => { @@ -245,14 +279,15 @@ test("settles a room whose GitHub card is pending under the code-review template ), ).toBe(true); expect(published.some((entry) => entry.event.type === "chat.message")).toBe( - false, + true, ); const listed = await roomMessages.listMessages({ tenantId: TENANT.id, workbenchId: "chan_template", }); - expect(listed.items).toHaveLength(0); + expect(listed.items).toHaveLength(1); + expectEventOnlySettleNotice(listed.items, "GitHub"); expect(platform.sentMail).toHaveLength(1); expect(platform.sentMail[0]?.workbenchId).toBe("ins_myra"); @@ -282,12 +317,19 @@ test("System / settle notices are not presented as the human's messages", async tenantId: TENANT.id, workbenchId: "chan_waiting", }); + expect(listed.items).toHaveLength(1); + expectEventOnlySettleNotice(listed.items, "GitHub"); expect( listed.items.filter((item) => item.sender.address === HUMAN_ADDRESS), ).toHaveLength(0); expect( listed.items.filter((item) => item.senderPrincipalId === "prn_owner"), ).toHaveLength(0); + expect( + listed.items.some((item) => + item.parts.some((part) => part.kind === "text"), + ), + ).toBe(false); }); test("a connector no room is waiting on settles nothing", async () => { From 2250c318b18b758f7e94d840121979a0dd4e624a Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 24 Aug 2026 09:23:40 -0700 Subject: [PATCH 2/3] Format files changed in this PR --- packages/chat/test/connect-pending.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/chat/test/connect-pending.test.ts b/packages/chat/test/connect-pending.test.ts index cc4c2397d..d7cf3d14a 100644 --- a/packages/chat/test/connect-pending.test.ts +++ b/packages/chat/test/connect-pending.test.ts @@ -86,7 +86,9 @@ function buildDeps() { function expectEventOnlySettleNotice( items: Awaited< - ReturnType["listMessages"]> + ReturnType< + ReturnType["listMessages"] + > >["items"], displayName: string, ) { From 3a95aed9656dbdcf87dda2b36d369b0b50b92459 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 24 Aug 2026 09:48:16 -0700 Subject: [PATCH 3/3] Fix lint on GitHub connect settle --- packages/chat/test/connect-pending.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/chat/test/connect-pending.test.ts b/packages/chat/test/connect-pending.test.ts index d7cf3d14a..7012922a1 100644 --- a/packages/chat/test/connect-pending.test.ts +++ b/packages/chat/test/connect-pending.test.ts @@ -98,10 +98,14 @@ function expectEventOnlySettleNotice( item.parts.every((part) => part.kind === "event"), ); expect(notices.length).toBeGreaterThanOrEqual(1); - const notice = notices[0]!; + const notice = notices[0]; + expect(notice).toBeDefined(); + if (notice === undefined) return; expect(notice.senderPrincipalId).toBeNull(); expect(notice.sender.address).not.toBe(HUMAN_ADDRESS); - const part = notice.parts[0]!; + const part = notice.parts[0]; + expect(part).toBeDefined(); + if (part === undefined) return; expect(part.kind).toBe("event"); if (part.kind !== "event") return; expect(part.event).toBe(CONNECTION_CONNECTED_EVENT);