From b95e98be61c8a01e2c1c9bc1a78257f94057738d Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 3 Sep 2026 11:33:26 -0700 Subject: [PATCH 1/3] Add tests for code-review GitHub settle and consumer inference copy (CL-6764) --- .../chat-ui/src/inference-failure.test.ts | 22 ++--- packages/chat-ui/test/components.test.tsx | 3 +- .../chat-ui/test/failed-turn-strip.test.tsx | 3 +- .../chat/src/consumer-inference-text.test.ts | 29 +++++-- packages/chat/src/room-messages.test.ts | 2 +- packages/chat/test/connect-pending.test.ts | 85 +++++++++++++++++++ 6 files changed, 124 insertions(+), 20 deletions(-) diff --git a/packages/chat-ui/src/inference-failure.test.ts b/packages/chat-ui/src/inference-failure.test.ts index 80c9326d6..5ba8784b3 100644 --- a/packages/chat-ui/src/inference-failure.test.ts +++ b/packages/chat-ui/src/inference-failure.test.ts @@ -1,26 +1,23 @@ import { describe, expect, test } from "bun:test"; import { + CONSUMER_INFERENCE_FAILURE_NOTICE, consumerFacingInferenceText, isClassifiedInferenceFailureText, } from "./inference-failure"; describe("consumerFacingInferenceText", () => { - test("keeps the vendor preamble and drops [HTTP …] plus the raw provider message", () => { + test("maps a classified credential failure to one recovery sentence", () => { expect( consumerFacingInferenceText( "This agent could not complete your request due to a credential error [HTTP 401]: API key is invalid", ), - ).toBe( - "This agent could not complete your request due to a credential error", - ); + ).toBe(CONSUMER_INFERENCE_FAILURE_NOTICE); expect( consumerFacingInferenceText( "This agent could not complete your request because the API quota has been exhausted [HTTP 429]: rate limited", ), - ).toBe( - "This agent could not complete your request because the API quota has been exhausted", - ); + ).toBe(CONSUMER_INFERENCE_FAILURE_NOTICE); }); test("a forced [HTTP 401] dump is not consumer copy", () => { @@ -47,12 +44,15 @@ describe("isClassifiedInferenceFailureText", () => { ).toBe(true); }); - test("matches a credential_failure reply after HTTP/raw is stripped", () => { + test("a stripped credential_failure reply is recovery copy, not the classified preamble", () => { + const facing = consumerFacingInferenceText( + "This agent could not complete your request due to a credential error [HTTP 401]: invalid api key", + ); + expect(facing).toBe(CONSUMER_INFERENCE_FAILURE_NOTICE); + expect(facing.toLowerCase()).not.toContain("credential error"); expect( isClassifiedInferenceFailureText( - consumerFacingInferenceText( - "This agent could not complete your request due to a credential error [HTTP 401]: invalid api key", - ), + "This agent could not complete your request due to a credential error [HTTP 401]: invalid api key", ), ).toBe(true); }); diff --git a/packages/chat-ui/test/components.test.tsx b/packages/chat-ui/test/components.test.tsx index 44669c268..483585de6 100644 --- a/packages/chat-ui/test/components.test.tsx +++ b/packages/chat-ui/test/components.test.tsx @@ -508,8 +508,9 @@ describe("WorkbenchTimeline", () => { expect(markup).not.toMatch(/\[HTTP/); expect(markup).not.toContain("401"); expect(markup).not.toContain("invalid api key"); + expect(markup).not.toMatch(/credential error/i); expect(markup).toContain( - "This agent could not complete your request due to a credential error", + "This didn't go through. Try again, or check the connection in Settings.", ); expect(markup).toContain("Fix this connection"); }); diff --git a/packages/chat-ui/test/failed-turn-strip.test.tsx b/packages/chat-ui/test/failed-turn-strip.test.tsx index e6d869ca5..49a9c5674 100644 --- a/packages/chat-ui/test/failed-turn-strip.test.tsx +++ b/packages/chat-ui/test/failed-turn-strip.test.tsx @@ -240,8 +240,9 @@ describe("the failed-turn notice renders through PrFailedTurnStrip", () => { const detail = container.querySelector(".chat-turn-failed-detail"); expect(detail?.textContent).toBe( - "This agent could not complete your request due to a credential error", + "This didn't go through. Try again, or check the connection in Settings.", ); + expect(detail?.textContent).not.toMatch(/credential error/i); expect(detail?.textContent).not.toMatch(/\[HTTP/); expect(detail?.textContent).not.toContain("401"); expect(detail?.textContent).not.toContain("API key is invalid"); diff --git a/packages/chat/src/consumer-inference-text.test.ts b/packages/chat/src/consumer-inference-text.test.ts index d604a0019..4cb7f41fb 100644 --- a/packages/chat/src/consumer-inference-text.test.ts +++ b/packages/chat/src/consumer-inference-text.test.ts @@ -10,14 +10,31 @@ describe("consumerFacingInferenceText", () => { expect(consumerFacingInferenceText("Hello there.")).toBe("Hello there."); }); - test("keeps a classified preamble and drops a trailing HTTP dump", () => { - expect( - consumerFacingInferenceText( - "This agent could not complete your request due to a credential error [HTTP 401]: API key is invalid.", - ), - ).toBe( + test("a classified credential failure is one recovery sentence, not a credential-error bubble", () => { + const text = consumerFacingInferenceText( + "This agent could not complete your request due to a credential error [HTTP 401]: API key is invalid.", + ); + expect(text).toBe(CONSUMER_INFERENCE_FAILURE_NOTICE); + expect(text.toLowerCase()).not.toContain("credential error"); + expect(text).not.toMatch(/\[HTTP/i); + expect(text).not.toMatch(/401/); + expect(text.toLowerCase()).not.toContain("api key is invalid"); + }); + + test("a classified credential preamble without an HTTP dump is still recovery copy", () => { + const text = consumerFacingInferenceText( "This agent could not complete your request due to a credential error", ); + expect(text).toBe(CONSUMER_INFERENCE_FAILURE_NOTICE); + expect(text.toLowerCase()).not.toContain("credential error"); + }); + + test("a classified quota failure is the same recovery sentence", () => { + const text = consumerFacingInferenceText( + "This agent could not complete your request because the API quota has been exhausted [HTTP 429]: rate limited", + ); + expect(text).toBe(CONSUMER_INFERENCE_FAILURE_NOTICE); + expect(text).not.toMatch(/\[HTTP/i); }); test("a forced HTTP dump is not consumer copy", () => { diff --git a/packages/chat/src/room-messages.test.ts b/packages/chat/src/room-messages.test.ts index b80bad1cc..50657bee5 100644 --- a/packages/chat/src/room-messages.test.ts +++ b/packages/chat/src/room-messages.test.ts @@ -82,7 +82,7 @@ describe("postRoomMessage", () => { expect(posted.parts).toEqual([ { kind: "text", - text: "This agent could not complete your request due to a credential error", + text: CONSUMER_INFERENCE_FAILURE_NOTICE, }, ]); const listed = await roomMessages.listMessages({ diff --git a/packages/chat/test/connect-pending.test.ts b/packages/chat/test/connect-pending.test.ts index e05ab140d..060d1cd3d 100644 --- a/packages/chat/test/connect-pending.test.ts +++ b/packages/chat/test/connect-pending.test.ts @@ -309,6 +309,91 @@ test("settles a room whose GitHub card is pending under the code-review template expect(turns).toHaveLength(0); }); +test("a code-review template room never wakes an agent when GitHub connects — even if connections/pending also names it", async () => { + const { store, roomMessages, platform, agentTurns, deps } = buildDeps(); + await store.createWorkbenchSettings({ + tenantId: TENANT.id, + workbenchId: "chan_review", + settings: { + "chat/kind": "workbench", + "chat/participants": [ + { address: HUMAN_ADDRESS, handle: "owner" }, + { + address: "ins_architecture-reviewer@acme.example", + handle: "architecture-reviewer", + }, + { + address: "ins_correctness-reviewer@acme.example", + handle: "correctness-reviewer", + }, + ], + "template/id": "code-review", + "connections/pending": ["github"], + "template/pendingConnections": ["github"], + }, + updatedBy: "prn_owner", + }); + + await settleConnectedService(deps, { + tenantId: TENANT.id, + principalId: "prn_owner", + connectorId: "github", + displayName: "GitHub", + }); + + const settled = await store.getWorkbenchSettings(TENANT.id, "chan_review"); + expect(settled?.settings["connections/pending"]).toEqual([]); + expect(settled?.settings["template/pendingConnections"]).toEqual([]); + + const listed = await roomMessages.listMessages({ + tenantId: TENANT.id, + workbenchId: "chan_review", + }); + expectEventOnlySettleNotice(listed.items, "GitHub"); + expect(listed.items[0]?.sender.address).toBe("system@chan_review"); + expect(platform.sentMail).toHaveLength(0); + const turns = await agentTurns.listTurns({ + tenantId: TENANT.id, + workbenchId: "chan_review", + }); + expect(turns).toHaveLength(0); +}); + +test("a code-review room that only has connections/pending for GitHub still does not dispatch a reviewer", async () => { + const { store, platform, agentTurns, deps } = buildDeps(); + await store.createWorkbenchSettings({ + tenantId: TENANT.id, + workbenchId: "chan_review_pending", + settings: { + "chat/kind": "workbench", + "chat/participants": [ + { address: HUMAN_ADDRESS, handle: "owner" }, + { + address: "ins_architecture-reviewer@acme.example", + handle: "architecture-reviewer", + }, + ], + "template/id": "code-review", + "connections/pending": ["github"], + }, + updatedBy: "prn_owner", + }); + + await settleConnectedService(deps, { + tenantId: TENANT.id, + principalId: "prn_owner", + connectorId: "github", + displayName: "GitHub", + }); + + expect(platform.sentMail).toHaveLength(0); + const turns = await agentTurns.listTurns({ + tenantId: TENANT.id, + workbenchId: "chan_review_pending", + }); + expect(turns).toHaveLength(0); +}); + test("a room pending under both keys still wakes its host agent", async () => { const { store, roomMessages, platform, agentTurns, deps } = buildDeps(); await store.createWorkbenchSettings({ From e6012462028a31678053bb5da041142bfb0170af Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 3 Sep 2026 11:33:41 -0700 Subject: [PATCH 2/3] Do not wake code-review reviewers on GitHub PAT settle (CL-6764) Connecting GitHub on a code-review template room used to dispatch the first reviewer, who then answered with a credential-error bubble even though Connect succeeded. Reviewers introduce themselves from canned copy after start-reviewing. Classified inference failures now render as one recovery sentence, never HTTP status or the credential-error preamble. --- packages/chat-ui/src/inference-failure.ts | 5 ++++- packages/chat-ui/src/timeline.tsx | 10 ++++++++-- packages/chat/src/connect-pending.ts | 18 ++++++++++++------ packages/chat/src/consumer-inference-text.ts | 14 +++++++++++--- packages/chat/src/room-messages.ts | 6 +++++- 5 files changed, 40 insertions(+), 13 deletions(-) diff --git a/packages/chat-ui/src/inference-failure.ts b/packages/chat-ui/src/inference-failure.ts index 0b3bb35a2..65906aab6 100644 --- a/packages/chat-ui/src/inference-failure.ts +++ b/packages/chat-ui/src/inference-failure.ts @@ -29,7 +29,10 @@ export const CLASSIFIED_INFERENCE_FAILURE_PREAMBLES: readonly string[] = [ "This agent could not complete your request because the API quota has been exhausted", ]; -export { consumerFacingInferenceText } from "@corbits/chat/consumer-inference-text"; +export { + CONSUMER_INFERENCE_FAILURE_NOTICE, + consumerFacingInferenceText, +} from "@corbits/chat/consumer-inference-text"; export function isClassifiedInferenceFailureText(text: string): boolean { return CLASSIFIED_INFERENCE_FAILURE_PREAMBLES.some((preamble) => diff --git a/packages/chat-ui/src/timeline.tsx b/packages/chat-ui/src/timeline.tsx index 6b3eadfea..8d21ffd2a 100644 --- a/packages/chat-ui/src/timeline.tsx +++ b/packages/chat-ui/src/timeline.tsx @@ -59,6 +59,7 @@ import type { ConnectGithubActions } from "./blocks/connect-github-actions"; import type { ConnectServiceActions } from "./blocks/connect-service-actions"; import { BlockPartView } from "./blocks/registry"; import { + CONSUMER_INFERENCE_FAILURE_NOTICE, consumerFacingInferenceText, isClassifiedInferenceFailureText, } from "./inference-failure"; @@ -551,7 +552,8 @@ function TextBubble({ {onFixConnection !== undefined && - isClassifiedInferenceFailureText(consumerText) && ( + (isClassifiedInferenceFailureText(text) || + consumerText === CONSUMER_INFERENCE_FAILURE_NOTICE) && (