From ec772ca0e55a3f1e992b25cfa44fc3dba252783d Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 28 Aug 2026 05:35:40 -0700 Subject: [PATCH 1/3] Add tests for people-section reportError on mutation failure A failing suspend mutation should call reportError with the action's operation and tenant id, not just set the generic message. Covers this before people-section.tsx wires reportError in. --- .../settings-ui/test/people-section.test.tsx | 60 ++++++++++++++++++- 1 file changed, 58 insertions(+), 2 deletions(-) diff --git a/packages/settings-ui/test/people-section.test.tsx b/packages/settings-ui/test/people-section.test.tsx index 64f7ae73a..bea09a986 100644 --- a/packages/settings-ui/test/people-section.test.tsx +++ b/packages/settings-ui/test/people-section.test.tsx @@ -8,12 +8,23 @@ // native role-assignment routes, the last owner can't be demoted, and a // pending invite can be cancelled. -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, mock, test } from "bun:test"; import { act } from "react"; import { createRoot } from "react-dom/client"; import type { Root } from "react-dom/client"; -import { PeopleSection } from "../src/people-section"; +const reportErrorCalls: { + error: unknown; + context: Record; +}[] = []; +mock.module("@corbits/error-sink", () => ({ + reportError: (error: unknown, context: Record) => { + reportErrorCalls.push({ error, context }); + return "ref_test"; + }, +})); + +const { PeopleSection } = await import("../src/people-section"); const realFetch = globalThis.fetch; afterEach(() => { @@ -416,4 +427,49 @@ describe("PeopleSection", () => { container.remove(); } }); + + test("a failing suspend reports the error with its operation and tenant", async () => { + const calls: FetchCall[] = []; + reportErrorCalls.length = 0; + mockFetch( + { + "/api/tenants/tnt_1/principals": { + data: [humanPrincipal()], + nextCursor: null, + }, + "/api/tenants/tnt_1/roles": rolesPage, + "/api/tenants/tnt_1/access-policy/pending-invites": noInvites, + "PATCH /api/tenants/tnt_1/principals/prn_human_1": () => + json(500, { error: "boom" }), + }, + calls, + ); + + const { container, root } = mount(); + try { + await settle(); + + const suspendButton = Array.from( + container.querySelectorAll("button"), + ).find((b) => b.textContent === "Suspend"); + expect(suspendButton).toBeDefined(); + act(() => + suspendButton?.dispatchEvent( + new MouseEvent("click", { bubbles: true }), + ), + ); + await settle(); + + expect( + reportErrorCalls.some( + (call) => + call.context.operation === "settings.people.updateStatus" && + call.context.tenantId === "tnt_1", + ), + ).toBe(true); + } finally { + act(() => root.unmount()); + container.remove(); + } + }); }); From 708ada4537d07a6a0aec364d3fb2c692190def57 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 28 Aug 2026 05:35:52 -0700 Subject: [PATCH 2/3] People section: report mutation failures via reportError Invite, cancel-invite, suspend/reactivate, remove, and role-change catches silently discarded the cause and never gave support a refId to trace. Route each through reportError with its operation and tenant id before setting the existing user-facing message. Fixes CL-7139. --- bun.lock | 1 + packages/settings-ui/package.json | 1 + packages/settings-ui/src/people-section.tsx | 35 ++++++++++++++++++--- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/bun.lock b/bun.lock index 61acacdc1..27bd147ae 100644 --- a/bun.lock +++ b/bun.lock @@ -1251,6 +1251,7 @@ "@corbits/api-query": "workspace:*", "@corbits/bench-ui": "workspace:*", "@corbits/chat-ui": "workspace:*", + "@corbits/error-sink": "workspace:*", "@corbits/icons": "workspace:*", "@corbits/inference-settings": "workspace:*", "@corbits/react-ui": "github:corbitsdev/react-ui#3b122812a307ccb35be31386f7696020c5a84635", diff --git a/packages/settings-ui/package.json b/packages/settings-ui/package.json index 53073b9cd..195fc707c 100644 --- a/packages/settings-ui/package.json +++ b/packages/settings-ui/package.json @@ -20,6 +20,7 @@ "@corbits/api-query": "workspace:*", "@corbits/bench-ui": "workspace:*", "@corbits/chat-ui": "workspace:*", + "@corbits/error-sink": "workspace:*", "@corbits/inference-settings": "workspace:*", "@corbits/react-ui": "github:corbitsdev/react-ui#3b122812a307ccb35be31386f7696020c5a84635", "@corbits/workflow-catalog": "workspace:*", diff --git a/packages/settings-ui/src/people-section.tsx b/packages/settings-ui/src/people-section.tsx index 3c96d0dd3..58d3799de 100644 --- a/packages/settings-ui/src/people-section.tsx +++ b/packages/settings-ui/src/people-section.tsx @@ -36,6 +36,7 @@ import { UnauthenticatedError, describeQueryError, } from "@corbits/api-query"; +import { reportError } from "@corbits/error-sink"; import { PRINCIPAL_KIND_LABEL, principalLabel } from "./identity"; import { AccessPolicyBlock } from "./access-policy"; import { @@ -171,7 +172,10 @@ export function PeopleSection({ setInviteOpen(false); reload(); }) - .catch(() => setInviteError(SETTINGS_STRINGS.peopleInviteError)) + .catch((cause: unknown) => { + reportError(cause, { operation: "settings.people.invite", tenantId }); + setInviteError(SETTINGS_STRINGS.peopleInviteError); + }) .finally(() => setInviting(false)); } @@ -180,7 +184,13 @@ export function PeopleSection({ setRowError(null); deletePendingInvite(tenantId, invite.id) .then(reload) - .catch(() => setRowError(SETTINGS_STRINGS.pendingInviteCancelError)); + .catch((cause: unknown) => { + reportError(cause, { + operation: "settings.people.cancelInvite", + tenantId, + }); + setRowError(SETTINGS_STRINGS.pendingInviteCancelError); + }); } function handleStatusChange( @@ -191,7 +201,13 @@ export function PeopleSection({ setRowError(null); updatePrincipalStatus(tenantId, principal.id, status) .then(reload) - .catch(() => setRowError(SETTINGS_STRINGS.peopleStatusUpdateError)); + .catch((cause: unknown) => { + reportError(cause, { + operation: "settings.people.updateStatus", + tenantId, + }); + setRowError(SETTINGS_STRINGS.peopleStatusUpdateError); + }); } function handleRemove(principal: Principal) { @@ -199,7 +215,10 @@ export function PeopleSection({ setRowError(null); removePrincipal(tenantId, principal.id) .then(reload) - .catch(() => setRowError(SETTINGS_STRINGS.peopleRemoveError)); + .catch((cause: unknown) => { + reportError(cause, { operation: "settings.people.remove", tenantId }); + setRowError(SETTINGS_STRINGS.peopleRemoveError); + }); } function handleRoleChange( @@ -232,7 +251,13 @@ export function PeopleSection({ ) .then(() => assignRole(tenantId, principal.id, newRoleId)) .then(reload) - .catch(() => setRowError(SETTINGS_STRINGS.peopleRoleChangeError)); + .catch((cause: unknown) => { + reportError(cause, { + operation: "settings.people.changeRole", + tenantId, + }); + setRowError(SETTINGS_STRINGS.peopleRoleChangeError); + }); } return ( From f215a6b3d3d5ba79b22ae9cf27e8f5e2c6afe28a Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 28 Aug 2026 05:50:38 -0700 Subject: [PATCH 3/3] Add tests for reportError coverage on every people-section mutation The prior test only covered the suspend catch. Parametrize it over invite, cancel-invite, remove, and role-change too, each asserting its own settings.people. operation. --- .../settings-ui/test/people-section.test.tsx | 237 +++++++++++++++--- 1 file changed, 198 insertions(+), 39 deletions(-) diff --git a/packages/settings-ui/test/people-section.test.tsx b/packages/settings-ui/test/people-section.test.tsx index bea09a986..56715fa79 100644 --- a/packages/settings-ui/test/people-section.test.tsx +++ b/packages/settings-ui/test/people-section.test.tsx @@ -428,48 +428,207 @@ describe("PeopleSection", () => { } }); - test("a failing suspend reports the error with its operation and tenant", async () => { - const calls: FetchCall[] = []; - reportErrorCalls.length = 0; - mockFetch( - { - "/api/tenants/tnt_1/principals": { - data: [humanPrincipal()], - nextCursor: null, - }, - "/api/tenants/tnt_1/roles": rolesPage, - "/api/tenants/tnt_1/access-policy/pending-invites": noInvites, + // CL-7139: every mutation catch must report the failure through + // reportError with its own operation, not just set the generic message. + const REPORT_ERROR_CASES: { + readonly name: string; + readonly operation: string; + readonly principals: unknown[]; + readonly invites: { readonly data: unknown[] }; + readonly failingHandler: Record; + readonly trigger: (container: HTMLDivElement) => Promise; + }[] = [ + { + name: "invite", + operation: "settings.people.invite", + principals: [humanPrincipal()], + invites: noInvites, + failingHandler: { + "POST /api/tenants/tnt_1/access-policy/pending-invites": () => + json(500, { error: "boom" }), + }, + trigger: async (container) => { + const inviteButton = Array.from( + container.querySelectorAll("button"), + ).find((b) => b.textContent === "Invite someone"); + act(() => + inviteButton?.dispatchEvent( + new MouseEvent("click", { bubbles: true }), + ), + ); + await settle(); + const emailInput = document.querySelector( + 'input[type="email"]', + ) as HTMLInputElement; + act(() => setNativeValue(emailInput, "bob@example.com")); + await settle(); + const form = document.getElementById( + "invite-person-form", + ) as HTMLFormElement; + act(() => { + form.dispatchEvent( + new Event("submit", { bubbles: true, cancelable: true }), + ); + }); + await settle(); + }, + }, + { + name: "cancelInvite", + operation: "settings.people.cancelInvite", + principals: [humanPrincipal()], + invites: { + data: [ + { + id: "pinv_1", + tenantId: "tnt_1", + matchType: "email", + value: "carol@example.com", + roleId: "role_member", + createdAt: timestamps.createdAt, + }, + ], + }, + failingHandler: { + "DELETE /api/tenants/tnt_1/access-policy/pending-invites/pinv_1": () => + json(500, { error: "boom" }), + }, + trigger: async (container) => { + const cancelButton = Array.from( + container.querySelectorAll("button"), + ).find((b) => b.textContent === "Cancel"); + act(() => + cancelButton?.dispatchEvent( + new MouseEvent("click", { bubbles: true }), + ), + ); + await settle(); + const confirmButton = Array.from( + container.querySelectorAll("button"), + ).find((b) => b.textContent?.includes("Cancel this invite")); + act(() => + confirmButton?.dispatchEvent( + new MouseEvent("click", { bubbles: true }), + ), + ); + await settle(); + }, + }, + { + name: "updateStatus", + operation: "settings.people.updateStatus", + principals: [humanPrincipal()], + invites: noInvites, + failingHandler: { "PATCH /api/tenants/tnt_1/principals/prn_human_1": () => json(500, { error: "boom" }), }, - calls, - ); - - const { container, root } = mount(); - try { - await settle(); - - const suspendButton = Array.from( - container.querySelectorAll("button"), - ).find((b) => b.textContent === "Suspend"); - expect(suspendButton).toBeDefined(); - act(() => - suspendButton?.dispatchEvent( - new MouseEvent("click", { bubbles: true }), - ), + trigger: async (container) => { + const suspendButton = Array.from( + container.querySelectorAll("button"), + ).find((b) => b.textContent === "Suspend"); + act(() => + suspendButton?.dispatchEvent( + new MouseEvent("click", { bubbles: true }), + ), + ); + await settle(); + }, + }, + { + name: "remove", + operation: "settings.people.remove", + principals: [humanPrincipal()], + invites: noInvites, + failingHandler: { + "DELETE /api/tenants/tnt_1/principals/prn_human_1": () => + json(500, { error: "boom" }), + }, + trigger: async (container) => { + const removeButton = Array.from( + container.querySelectorAll("button"), + ).find((b) => b.textContent === "Remove"); + act(() => + removeButton?.dispatchEvent( + new MouseEvent("click", { bubbles: true }), + ), + ); + await settle(); + const confirmButton = Array.from( + container.querySelectorAll("button"), + ).find((b) => b.textContent === "Remove for good?"); + act(() => + confirmButton?.dispatchEvent( + new MouseEvent("click", { bubbles: true }), + ), + ); + await settle(); + }, + }, + { + name: "changeRole", + operation: "settings.people.changeRole", + principals: [ + humanPrincipal(), + humanPrincipal({ + id: "prn_human_2", + displayName: "Bob Baker", + refId: "user_2", + roles: [{ id: MEMBER_ROLE.id, name: MEMBER_ROLE.name }], + }), + ], + invites: noInvites, + failingHandler: { + "DELETE /api/tenants/tnt_1/principals/prn_human_2/roles/role_member": + () => json(500, { error: "boom" }), + }, + trigger: async (container) => { + const selects = container.querySelectorAll("tbody select"); + const bobSelect = Array.from(selects).find( + (s) => (s as HTMLSelectElement).value === "role_member", + ) as HTMLSelectElement; + act(() => { + bobSelect.value = "role_owner"; + bobSelect.dispatchEvent(new Event("change", { bubbles: true })); + }); + await settle(); + }, + }, + ]; + + for (const testCase of REPORT_ERROR_CASES) { + test(`a failing ${testCase.name} reports the error with its operation and tenant`, async () => { + const calls: FetchCall[] = []; + reportErrorCalls.length = 0; + mockFetch( + { + "/api/tenants/tnt_1/principals": { + data: testCase.principals, + nextCursor: null, + }, + "/api/tenants/tnt_1/roles": rolesPage, + "/api/tenants/tnt_1/access-policy/pending-invites": testCase.invites, + ...testCase.failingHandler, + }, + calls, ); - await settle(); - expect( - reportErrorCalls.some( - (call) => - call.context.operation === "settings.people.updateStatus" && - call.context.tenantId === "tnt_1", - ), - ).toBe(true); - } finally { - act(() => root.unmount()); - container.remove(); - } - }); + const { container, root } = mount(); + try { + await settle(); + await testCase.trigger(container); + + expect( + reportErrorCalls.some( + (call) => + call.context.operation === testCase.operation && + call.context.tenantId === "tnt_1", + ), + ).toBe(true); + } finally { + act(() => root.unmount()); + container.remove(); + } + }); + } });