From c7d444f757a86487566e1fab6d779228a0617bb5 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Fri, 2 Oct 2026 23:36:11 -0700 Subject: [PATCH 1/3] Add tests for grant preview sentence and expiry presets --- apps/web/src/settings/grant-preview.test.ts | 76 +++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 apps/web/src/settings/grant-preview.test.ts diff --git a/apps/web/src/settings/grant-preview.test.ts b/apps/web/src/settings/grant-preview.test.ts new file mode 100644 index 000000000..58fdb8aff --- /dev/null +++ b/apps/web/src/settings/grant-preview.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, test } from "bun:test"; + +import { + expiryIsoFromPreset, + expiryLabelFromPreset, + grantPreviewSentence, +} from "./grant-preview"; + +describe("grantPreviewSentence", () => { + test("an allow reads as permission for the named target", () => { + expect( + grantPreviewSentence({ + targetLabel: "Billing", + resource: "credential", + action: "use", + effect: "allow", + expiresLabel: null, + }), + ).toBe("Billing may use on credential."); + }); + + test("a deny reads as refusal, and an ask as a checkpoint", () => { + expect( + grantPreviewSentence({ + targetLabel: "Deploy bot", + resource: "workflow-run", + action: "create", + effect: "deny", + expiresLabel: null, + }), + ).toBe("Deploy bot must not create on workflow-run."); + expect( + grantPreviewSentence({ + targetLabel: "Deploy bot", + resource: "workflow-run", + action: "create", + effect: "ask", + expiresLabel: null, + }), + ).toBe("Deploy bot must ask before they can create on workflow-run."); + }); + + test("a missing target falls back to Someone, and an expiry extends the sentence", () => { + expect( + grantPreviewSentence({ + targetLabel: null, + resource: "credential", + action: "use", + effect: "allow", + expiresLabel: null, + }), + ).toBe("Someone may use on credential."); + expect( + grantPreviewSentence({ + targetLabel: " ", + resource: "credential", + action: "use", + effect: "allow", + expiresLabel: "in 7 days", + }), + ).toBe("Someone may use on credential, until in 7 days."); + }); +}); + +describe("expiry presets", () => { + test("never means no expiry in both projections", () => { + expect(expiryIsoFromPreset("never")).toBeNull(); + expect(expiryLabelFromPreset("never")).toBeNull(); + }); + + test("a 7d preset lands exactly seven days after now", () => { + const now = new Date("2026-02-10T00:00:00.000Z"); + expect(expiryIsoFromPreset("7d", now)).toBe("2026-02-17T00:00:00.000Z"); + expect(expiryLabelFromPreset("7d")).toBe("in 7 days"); + }); +}); From 40e1de246bb696279e2073c9a7ea0d2f3059cd0f Mon Sep 17 00:00:00 2001 From: Sawyer Date: Fri, 2 Oct 2026 23:36:13 -0700 Subject: [PATCH 2/3] Roles and grants Settings sections report failures through reportError --- apps/web/src/settings/grant-preview.test.ts | 6 +---- apps/web/src/settings/grants-section.tsx | 11 +++++++-- apps/web/src/settings/roles-section.tsx | 26 +++++++++++++++++---- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/apps/web/src/settings/grant-preview.test.ts b/apps/web/src/settings/grant-preview.test.ts index 58fdb8aff..61870f7b8 100644 --- a/apps/web/src/settings/grant-preview.test.ts +++ b/apps/web/src/settings/grant-preview.test.ts @@ -1,10 +1,6 @@ import { describe, expect, test } from "bun:test"; -import { - expiryIsoFromPreset, - expiryLabelFromPreset, - grantPreviewSentence, -} from "./grant-preview"; +import { expiryIsoFromPreset, expiryLabelFromPreset, grantPreviewSentence } from "./grant-preview"; describe("grantPreviewSentence", () => { test("an allow reads as permission for the named target", () => { diff --git a/apps/web/src/settings/grants-section.tsx b/apps/web/src/settings/grants-section.tsx index d2c478c20..72406f3cb 100644 --- a/apps/web/src/settings/grants-section.tsx +++ b/apps/web/src/settings/grants-section.tsx @@ -31,6 +31,7 @@ import { useMemo, useState } from "react"; import { QueryView, toAPIQuery } from "@/lib/api-query"; import { tenantKeys } from "@/query-client"; +import { reportError } from "@corbits/error-sink"; import { PRINCIPAL_KIND_LABEL, PRINCIPAL_KIND_ORDER, principalLabel } from "./identity"; import { expiryIsoFromPreset, expiryLabelFromPreset, grantPreviewSentence } from "./grant-preview"; import { KindCards } from "./kind-cards"; @@ -157,7 +158,10 @@ export function GrantsSection({ tenantId }: { readonly tenantId: string | null } reload(); toast(SETTINGS_STRINGS.grantCreatedToast); }) - .catch(() => setCreateError(SETTINGS_STRINGS.grantsCreateError)) + .catch((cause: unknown) => { + reportError(cause, { operation: "settings.grants.create", tenantId }); + setCreateError(SETTINGS_STRINGS.grantsCreateError); + }) .finally(() => setCreating(false)); } @@ -169,7 +173,10 @@ export function GrantsSection({ tenantId }: { readonly tenantId: string | null } reload(); toast(SETTINGS_STRINGS.grantRevokedToast); }) - .catch(() => setRowError(SETTINGS_STRINGS.grantsRevokeError)); + .catch((cause: unknown) => { + reportError(cause, { operation: "settings.grants.revoke", tenantId }); + setRowError(SETTINGS_STRINGS.grantsRevokeError); + }); } return ( diff --git a/apps/web/src/settings/roles-section.tsx b/apps/web/src/settings/roles-section.tsx index ae1031b98..bdbcc895d 100644 --- a/apps/web/src/settings/roles-section.tsx +++ b/apps/web/src/settings/roles-section.tsx @@ -25,6 +25,7 @@ import { useState } from "react"; import { QueryView, toAPIQuery } from "@/lib/api-query"; import { tenantKeys } from "@/query-client"; +import { reportError } from "@corbits/error-sink"; import { principalLabel } from "./identity"; import { SETTINGS_STRINGS } from "./strings"; import { @@ -94,7 +95,10 @@ export function RolesSection({ tenantId }: { readonly tenantId: string | null }) setCreateOpen(false); reload(); }) - .catch(() => setCreateError(SETTINGS_STRINGS.rolesCreateError)) + .catch((cause: unknown) => { + reportError(cause, { operation: "settings.roles.create", tenantId }); + setCreateError(SETTINGS_STRINGS.rolesCreateError); + }) .finally(() => setCreating(false)); } @@ -103,7 +107,10 @@ export function RolesSection({ tenantId }: { readonly tenantId: string | null }) setRowError(null); deleteRole(tenantId, role.id) .then(reload) - .catch(() => setRowError(SETTINGS_STRINGS.rolesDeleteError)); + .catch((cause: unknown) => { + reportError(cause, { operation: "settings.roles.delete", tenantId }); + setRowError(SETTINGS_STRINGS.rolesDeleteError); + }); } function handleRename(role: Role, name: string) { @@ -111,7 +118,10 @@ export function RolesSection({ tenantId }: { readonly tenantId: string | null }) setRowError(null); renameRole(tenantId, role.id, { name }) .then(reload) - .catch(() => setRowError(SETTINGS_STRINGS.rolesRenameError)); + .catch((cause: unknown) => { + reportError(cause, { operation: "settings.roles.rename", tenantId }); + setRowError(SETTINGS_STRINGS.rolesRenameError); + }); } function handleAssign(principalId: string, roleId: string) { @@ -119,7 +129,10 @@ export function RolesSection({ tenantId }: { readonly tenantId: string | null }) setRowError(null); assignRole(tenantId, principalId, roleId) .then(reload) - .catch(() => setRowError(SETTINGS_STRINGS.rolesAssignError)); + .catch((cause: unknown) => { + reportError(cause, { operation: "settings.roles.assign", tenantId }); + setRowError(SETTINGS_STRINGS.rolesAssignError); + }); } function handleUnassign(principalId: string, roleId: string) { @@ -127,7 +140,10 @@ export function RolesSection({ tenantId }: { readonly tenantId: string | null }) setRowError(null); unassignRole(tenantId, principalId, roleId) .then(reload) - .catch(() => setRowError(SETTINGS_STRINGS.rolesUnassignError)); + .catch((cause: unknown) => { + reportError(cause, { operation: "settings.roles.unassign", tenantId }); + setRowError(SETTINGS_STRINGS.rolesUnassignError); + }); } return ( From 5dbcb11fc5af2a72aa1dec0f75eda348814c35a3 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Sat, 3 Oct 2026 09:39:27 -0700 Subject: [PATCH 3/3] Settings role assignment covers agent/workflow principals with first-run guidance Roles assignment picker and table list every principal kind (users first, machine principals kind-qualified); a 404 on assign explains first-run minting instead of reporting a transient failure. E2E walks create-role, grant, assign (person and agent), evaluate over stock routes. --- apps/web/src/settings/role-assignment.test.ts | 77 ++++++ apps/web/src/settings/role-assignment.ts | 35 +++ apps/web/src/settings/roles-section.tsx | 28 ++- apps/web/src/settings/strings.ts | 6 +- apps/web/src/settings/tenancy-api.ts | 11 + e2e/grants/roles-assign-evaluate.test.ts | 221 ++++++++++++++++++ 6 files changed, 365 insertions(+), 13 deletions(-) create mode 100644 apps/web/src/settings/role-assignment.test.ts create mode 100644 apps/web/src/settings/role-assignment.ts create mode 100644 e2e/grants/roles-assign-evaluate.test.ts diff --git a/apps/web/src/settings/role-assignment.test.ts b/apps/web/src/settings/role-assignment.test.ts new file mode 100644 index 000000000..7a9e03b2c --- /dev/null +++ b/apps/web/src/settings/role-assignment.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, test } from "bun:test"; + +import { + assignFailureMessage, + assignmentPrincipalLabel, + compareAssignmentPrincipals, +} from "./role-assignment"; +import { SETTINGS_STRINGS } from "./strings"; +import { TenancyApiError, type Principal } from "./tenancy-api"; + +function principal(overrides: Partial & Pick): Principal { + return { + tenantId: "tenant-1", + refId: overrides.id, + displayName: "Alice Anderson", + status: "active", + roles: [], + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + ...overrides, + }; +} + +describe("assignFailureMessage", () => { + test("a missing principal earns first-run guidance, not the transient error", () => { + expect(assignFailureMessage(new TenancyApiError("The hub answered 404 for /roles.", 404))).toBe( + SETTINGS_STRINGS.rolesAssignMissingPrincipalError, + ); + expect(SETTINGS_STRINGS.rolesAssignMissingPrincipalError).not.toBe( + SETTINGS_STRINGS.rolesAssignError, + ); + }); + + test("server and network failures stay on the transient error", () => { + expect(assignFailureMessage(new TenancyApiError("The hub answered 500 for /roles.", 500))).toBe( + SETTINGS_STRINGS.rolesAssignError, + ); + expect(assignFailureMessage(new TenancyApiError("fetch failed"))).toBe( + SETTINGS_STRINGS.rolesAssignError, + ); + expect(assignFailureMessage(new Error("boom"))).toBe(SETTINGS_STRINGS.rolesAssignError); + }); +}); + +describe("assignmentPrincipalLabel", () => { + test("a person keeps their bare name", () => { + expect(assignmentPrincipalLabel(principal({ id: "prn-1", kind: "user" }))).toBe( + "Alice Anderson", + ); + }); + + test("agent and workflow principals carry their kind", () => { + expect( + assignmentPrincipalLabel( + principal({ id: "prn-2", kind: "agent", displayName: "Deploy Bot" }), + ), + ).toBe(`Deploy Bot — ${SETTINGS_STRINGS.peopleKindAgent}`); + expect( + assignmentPrincipalLabel( + principal({ id: "prn-3", kind: "workflow", displayName: "Nightly" }), + ), + ).toBe(`Nightly — ${SETTINGS_STRINGS.peopleKindWorkflow}`); + }); +}); + +describe("compareAssignmentPrincipals", () => { + test("people sort before agents before workflows", () => { + const workflow = principal({ id: "prn-3", kind: "workflow", displayName: "Aaron" }); + const agent = principal({ id: "prn-2", kind: "agent", displayName: "Zed" }); + const person = principal({ id: "prn-1", kind: "user", displayName: "Zoe" }); + expect([workflow, agent, person].sort(compareAssignmentPrincipals).map((p) => p.id)).toEqual([ + "prn-1", + "prn-2", + "prn-3", + ]); + }); +}); diff --git a/apps/web/src/settings/role-assignment.ts b/apps/web/src/settings/role-assignment.ts new file mode 100644 index 000000000..3d5fe0905 --- /dev/null +++ b/apps/web/src/settings/role-assignment.ts @@ -0,0 +1,35 @@ +// Pure logic behind the Roles assignment UI: kind-qualified labels so +// machine principals never read as people, users-first ordering for the +// picker, and the failure message that distinguishes a missing (never +// minted) principal from a transient error. + +import { PRINCIPAL_KIND_LABEL, PRINCIPAL_KIND_ORDER, principalLabel } from "./identity"; +import { SETTINGS_STRINGS } from "./strings"; +import { isNotFoundError, type Principal } from "./tenancy-api"; + +/** + * A 404 from the assign endpoint means the hub has no such principal (or + * role) in this tenant -- for machine principals that usually means the + * account has not been minted yet, because agent and workflow principals are + * created on their first run rather than at deploy time. Any other failure + * is reported as transient. Still fail-closed: both branches only choose the + * message shown next to the untouched assignment table. + */ +export function assignFailureMessage(cause: unknown): string { + if (isNotFoundError(cause)) return SETTINGS_STRINGS.rolesAssignMissingPrincipalError; + return SETTINGS_STRINGS.rolesAssignError; +} + +/** "Deploy Bot — Agent": machine principals are never shown as bare people, + * matching the grants picker's kind-qualified labels. */ +export function assignmentPrincipalLabel(principal: Principal): string { + const { label } = principalLabel(principal.displayName); + if (principal.kind === "user") return label; + return `${label} — ${PRINCIPAL_KIND_LABEL[principal.kind]}`; +} + +export function compareAssignmentPrincipals(a: Principal, b: Principal): number { + const kindOrder = PRINCIPAL_KIND_ORDER.indexOf(a.kind) - PRINCIPAL_KIND_ORDER.indexOf(b.kind); + if (kindOrder !== 0) return kindOrder; + return assignmentPrincipalLabel(a).localeCompare(assignmentPrincipalLabel(b)); +} diff --git a/apps/web/src/settings/roles-section.tsx b/apps/web/src/settings/roles-section.tsx index bdbcc895d..ec765e118 100644 --- a/apps/web/src/settings/roles-section.tsx +++ b/apps/web/src/settings/roles-section.tsx @@ -26,7 +26,11 @@ import { useState } from "react"; import { QueryView, toAPIQuery } from "@/lib/api-query"; import { tenantKeys } from "@/query-client"; import { reportError } from "@corbits/error-sink"; -import { principalLabel } from "./identity"; +import { + assignFailureMessage, + assignmentPrincipalLabel, + compareAssignmentPrincipals, +} from "./role-assignment"; import { SETTINGS_STRINGS } from "./strings"; import { assignRole, @@ -131,7 +135,7 @@ export function RolesSection({ tenantId }: { readonly tenantId: string | null }) .then(reload) .catch((cause: unknown) => { reportError(cause, { operation: "settings.roles.assign", tenantId }); - setRowError(SETTINGS_STRINGS.rolesAssignError); + setRowError(assignFailureMessage(cause)); }); } @@ -297,11 +301,13 @@ export function RoleAssignments({ const [principalId, setPrincipalId] = useState(""); const [roleId, setRoleId] = useState(""); - // The "Person" picker should match People's member roster, not the - // full tenant-wide principal list. - const people = principals.filter((p) => p.kind === "user"); + // `assignRole` accepts any principal id, so the picker lists every kind: + // agents and workflows hold roles exactly like people do, and their + // principals only exist after their first run, which is also why a 404 + // here earns its own guidance (see `assignFailureMessage`). + const assignable = [...principals].sort(compareAssignmentPrincipals); - const assignments = people.flatMap((principal) => + const assignments = assignable.flatMap((principal) => principal.roles.map((role) => ({ principal, role })), ); @@ -310,16 +316,16 @@ export function RoleAssignments({

{SETTINGS_STRINGS.rolesAssignSectionTitle}