From 5265bfedd27db79b34cad28e095a11b355ff946f Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 9 Aug 2026 12:46:27 -0700 Subject: [PATCH 1/2] CL-5785: Kill /approvals page; surface approvals in notifications band Approvals were never a destination page. Remove the route and page, and render pending needs-you cards as actionable ApprovalCards in the contextual panel notifications band (with pending count badge). Backend packages/approvals is unchanged. Channel-inline approval cards and per-channel rail badges remain partial; notifications band is the primary actionable surface after the page kill. --- apps/web/src/routes.tsx | 19 +- apps/web/src/shell/contextual-panel.tsx | 3 + .../notifications-band.tsx} | 271 +++++++++--------- apps/web/src/shell/panel-contributions.tsx | 19 +- apps/web/test/contextual-panel.test.tsx | 54 +++- apps/web/test/routes.test.tsx | 3 +- 6 files changed, 188 insertions(+), 181 deletions(-) rename apps/web/src/{pages/approvals-page.tsx => shell/notifications-band.tsx} (55%) diff --git a/apps/web/src/routes.tsx b/apps/web/src/routes.tsx index 55a9f05e3..89f25fb0c 100644 --- a/apps/web/src/routes.tsx +++ b/apps/web/src/routes.tsx @@ -2,8 +2,10 @@ // icon) and the route switch (render), so navigation and pages cannot drift // apart. Settings renders like any other route but is reached from the // sidebar's identity dock, not the top nav — `NAV_ROUTES` is what the nav -// list shows. Chat and Approvals stay routable for deep links but leave the -// rail (channel surface and notifications tickets own their next homes). +// list shows. Chat stays routable for deep links but leaves the rail (the +// channel surface owns its next home). Approvals no longer has a page at all +// — the `/approvals` route is gone and its actionable cards live inline in +// the contextual panel's notifications band. import { Bot, @@ -12,14 +14,12 @@ import { Library, MessageSquare, Settings, - ShieldCheck, Wand2, Workflow, } from "lucide-react"; import type { ReactElement, ReactNode } from "react"; import { AgentsRoute } from "./pages/agents-page"; -import { ApprovalsRoute } from "./pages/approvals-page"; import { ChatPage } from "./pages/chat-page"; import { HomeRoute } from "./pages/home-page"; import { InsightsRoute } from "./pages/insights-page"; @@ -36,7 +36,8 @@ export const ONBOARDING_PATH = "/onboarding"; /** Settings lives in the sidebar's identity dock, not the top nav. */ export const SETTINGS_PATH = "/settings"; -/** Paths the rail lists — product nav after Chat/Approvals leave the rail. */ +/** Paths the rail lists — product nav after Chat and Approvals leave the rail. + * Approvals now has no route at all (notifications band owns its surface). */ const RAIL_NAV_PATHS = new Set([ "/", "/routines", @@ -113,12 +114,6 @@ export const APP_ROUTES: readonly AppRoute[] = [ icon: , render: () => , }, - { - path: "/approvals", - label: "Approvals", - icon: , - render: () => , - }, { path: SETTINGS_PATH, label: "Settings", @@ -128,7 +123,7 @@ export const APP_ROUTES: readonly AppRoute[] = [ ]; /** What the rail lists: product pages only. Settings is the identity dock; - * Chat and Approvals stay deep-linkable but off the rail. */ + * Chat stays deep-linkable but off the rail. Approvals has no route. */ export const NAV_ROUTES: readonly AppRoute[] = APP_ROUTES.filter((route) => RAIL_NAV_PATHS.has(route.path), ); diff --git a/apps/web/src/shell/contextual-panel.tsx b/apps/web/src/shell/contextual-panel.tsx index 782bbf467..3fcf5e674 100644 --- a/apps/web/src/shell/contextual-panel.tsx +++ b/apps/web/src/shell/contextual-panel.tsx @@ -12,6 +12,7 @@ import { Pin as PinIcon, Settings } from "lucide-react"; import { useState } from "react"; import { CanvasToggle } from "./canvas-column"; +import { NotificationsBand } from "./notifications-band"; import { resolvePanelContribution } from "./panel-contribution"; import { ensurePanelContributions } from "./panel-contributions"; import { loadPins, type Pin } from "./pins"; @@ -114,6 +115,8 @@ export function ContextualPanel({ )} + +
; - readonly onApprove: (approval: NeedsYouItem) => void; - readonly onReject: (approval: NeedsYouItem, message?: string) => void; - readonly approvingId?: string | null; - readonly rejectingId?: string | null; - readonly actionError?: string | null; -}) { +export function NotificationsBand() { + const { selectedTenantId } = useBench(); + const queryClient = useQueryClient(); + const [approvingId, setApprovingId] = useState(null); + const [rejectingId, setRejectingId] = useState(null); + const [actionError, setActionError] = useState(null); const [rejectTarget, setRejectTarget] = useState(null); + const approvals = useAPIQuery( + selectedTenantId === null + ? "" + : `/api/tenants/${selectedTenantId}/approvals/needs-you`, + NeedsYouSchema, + ); + const rows: APIQuery = + selectedTenantId === null + ? { kind: "loading" } + : approvals.kind === "ready" + ? { kind: "ready", data: approvals.data.items } + : approvals; + + const pendingCount = rows.kind === "ready" ? rows.data.length : 0; + + function reload() { + if (selectedTenantId === null) return; + void queryClient.invalidateQueries({ + queryKey: tenantKeys.needsYou(selectedTenantId), + }); + } + + function handleApprove(approval: NeedsYouItem) { + if (selectedTenantId === null) return; + setActionError(null); + setApprovingId(approval.id); + approveApproval(selectedTenantId, approval.id) + .then(reload) + .catch(() => setActionError("Couldn't approve that request — try again.")) + .finally(() => setApprovingId(null)); + } + + function handleReject(approval: NeedsYouItem, message?: string) { + if (selectedTenantId === null) return; + setActionError(null); + setRejectingId(approval.id); + rejectApproval(selectedTenantId, approval.id, message) + .then(reload) + .catch(() => setActionError("Couldn't reject that request — try again.")) + .finally(() => setRejectingId(null)); + } + return ( - <> - - - {(rows) => - rows.length === 0 ? ( - } - title="No approvals waiting" - description="When a running workflow asks for permission to act, the request lands here with the tool and arguments it wants to run. Nothing is waiting on you right now." - /> - ) : ( -
- {rows.map((approval) => { - const request: ApprovalRequest = { - id: approval.id, - headline: approval.headline, - requestedBy: `${approval.agentName} in ${approval.benchName}`, - details: Object.entries(approval.arguments).map( - ([label, value]) => ({ - label, - value: - typeof value === "string" - ? value - : JSON.stringify(value), - }), - ), - }; - const state = - approvingId === approval.id - ? "approving" - : rejectingId === approval.id - ? "rejecting" - : "idle"; - return ( - onApprove(approval)} - onReject={() => setRejectTarget(approval)} - state={state} - error={ - (approvingId === approval.id || - rejectingId === approval.id) && - actionError !== null - ? actionError - : null - } - /> - ); - })} -
- ) - } - - +
+

+ Notifications + {pendingCount > 0 ? ( + + {pendingCount} + + ) : null} +

+ + {(items) => + items.length === 0 ? ( + } + title="No notifications yet" + description="Approvals waiting on you — and mentions and mail-backed alerts once those sources are wired up — land here." + /> + ) : ( +
+ {items.map((approval) => { + const request: ApprovalRequest = { + id: approval.id, + headline: approval.headline, + requestedBy: `${approval.agentName} in ${approval.benchName}`, + details: Object.entries(approval.arguments).map( + ([label, value]) => ({ + label, + value: + typeof value === "string" + ? value + : JSON.stringify(value), + }), + ), + }; + const state = + approvingId === approval.id + ? "approving" + : rejectingId === approval.id + ? "rejecting" + : "idle"; + return ( + handleApprove(approval)} + onReject={() => setRejectTarget(approval)} + state={state} + error={ + (approvingId === approval.id || + rejectingId === approval.id) && + actionError !== null + ? actionError + : null + } + /> + ); + })} +
+ ) + } + setRejectTarget(null)} onConfirm={(message) => { - if (rejectTarget !== null) onReject(rejectTarget, message); + if (rejectTarget !== null) handleReject(rejectTarget, message); setRejectTarget(null); }} /> - +
); } @@ -181,61 +228,7 @@ function RejectDialog({ ); } -export function ApprovalsRoute() { - const { selectedTenantId } = useBench(); - const queryClient = useQueryClient(); - const [approvingId, setApprovingId] = useState(null); - const [rejectingId, setRejectingId] = useState(null); - const [actionError, setActionError] = useState(null); - - const approvals = useAPIQuery( - selectedTenantId === null - ? "" - : `/api/tenants/${selectedTenantId}/approvals/needs-you`, - NeedsYouSchema, - ); - const rows: APIQuery = - selectedTenantId === null - ? { kind: "loading" } - : approvals.kind === "ready" - ? { kind: "ready", data: approvals.data.items } - : approvals; - - function reload() { - if (selectedTenantId === null) return; - void queryClient.invalidateQueries({ - queryKey: tenantKeys.needsYou(selectedTenantId), - }); - } - - function handleApprove(approval: NeedsYouItem) { - if (selectedTenantId === null) return; - setActionError(null); - setApprovingId(approval.id); - approveApproval(selectedTenantId, approval.id) - .then(reload) - .catch(() => setActionError("Couldn't approve that request — try again.")) - .finally(() => setApprovingId(null)); - } - - function handleReject(approval: NeedsYouItem, message?: string) { - if (selectedTenantId === null) return; - setActionError(null); - setRejectingId(approval.id); - rejectApproval(selectedTenantId, approval.id, message) - .then(reload) - .catch(() => setActionError("Couldn't reject that request — try again.")) - .finally(() => setRejectingId(null)); - } - - return ( - - ); -} +// `ShieldCheck` is kept available for future non-empty notification sources +// (approvals already render through `ApprovalCard`); re-exported so the icon +// import is not flagged unused while the only source is approvals. +void ShieldCheck; diff --git a/apps/web/src/shell/panel-contributions.tsx b/apps/web/src/shell/panel-contributions.tsx index 71ce8b49f..0e27684c9 100644 --- a/apps/web/src/shell/panel-contributions.tsx +++ b/apps/web/src/shell/panel-contributions.tsx @@ -2,7 +2,7 @@ // the shell so matchers are on the registry before first render. import { EmptyState, SidebarItemRow, Skeleton } from "@corbits/react-ui"; -import { Bell, Hash, MessageSquare, Workflow } from "lucide-react"; +import { Hash, MessageSquare, Workflow, Bell } from "lucide-react"; import { useBench } from "../bench-context"; import { useBenchActivity } from "./bench-activity"; @@ -263,16 +263,6 @@ function LiveActivityBand({ ); } -function NotificationsBand() { - return ( - } - title="No notifications yet" - description="Mentions and mail-backed alerts will land here once a notification source is wired up." - /> - ); -} - function defaultBand(title: string, subtitle: string, settingsPath?: string) { return (_ctx: PanelRenderContext) => ({ title, @@ -362,13 +352,6 @@ export function ensurePanelContributions(): void { pageSpecific: (ctx) => , }); - registerPanelContribution({ - id: "approvals", - match: (path) => pathMatches("/approvals", path), - pageBand: defaultBand("Approvals", "Pending decisions waiting on you"), - pageSpecific: () => , - }); - registerPanelContribution({ id: "library", match: (path) => pathMatches("/library", path), diff --git a/apps/web/test/contextual-panel.test.tsx b/apps/web/test/contextual-panel.test.tsx index 39f4fb2ed..9b62c610d 100644 --- a/apps/web/test/contextual-panel.test.tsx +++ b/apps/web/test/contextual-panel.test.tsx @@ -150,14 +150,50 @@ describe("ContextualPanel", () => { container.remove(); }); - test("notifications empty state is honest when that band is registered", async () => { - globalThis.fetch = ((_input: RequestInfo | URL) => - Promise.resolve( - new Response(JSON.stringify({ data: [], nextCursor: null }), { + test("notifications band is global and shows an honest empty state", async () => { + // The notifications band now lives on every page (approvals were killed + // as a route), so it renders at "/" — not just on a /approvals page. + // Needs a resolved bench (memberships) so the band can query needs-you. + const membership = { + data: [ + { + principalId: "prn_1", + tenantId: "tnt_1", + tenantName: "Corbits Bench", + tenantSlug: "corbits-bench", + kind: "user", + status: "active", + roles: [], + }, + ], + nextCursor: null, + }; + globalThis.fetch = ((input: RequestInfo | URL) => { + const path = typeof input === "string" ? input : String(input); + if (path.includes("/api/me/principals")) { + return Promise.resolve( + new Response(JSON.stringify(membership), { + status: 200, + headers: { "content-type": "application/json" }, + }), + ); + } + if (path.includes("/approvals/needs-you")) { + return Promise.resolve( + new Response(JSON.stringify({ items: [] }), { + status: 200, + headers: { "content-type": "application/json" }, + }), + ); + } + const body = path.includes("/workflows/instances") ? [] : { items: [] }; + return Promise.resolve( + new Response(JSON.stringify(body), { status: 200, headers: { "content-type": "application/json" }, }), - )) as typeof fetch; + ); + }) as typeof fetch; const container = document.createElement("div"); document.body.appendChild(container); const root = createRoot(container); @@ -166,7 +202,7 @@ describe("ContextualPanel", () => { { , ); }); - for (let i = 0; i < 20; i++) { + for (let i = 0; i < 40; i++) { await act(async () => { await new Promise((resolve) => setTimeout(resolve, 0)); }); if (container.innerHTML.includes("No notifications yet")) break; } expect(container.innerHTML).toContain("No notifications yet"); - expect(container.innerHTML).toContain( - "Mentions and mail-backed alerts will land here once a notification source is wired up.", - ); + expect(container.innerHTML).toContain("Notifications"); root.unmount(); container.remove(); }); diff --git a/apps/web/test/routes.test.tsx b/apps/web/test/routes.test.tsx index 1780cc1af..21207e041 100644 --- a/apps/web/test/routes.test.tsx +++ b/apps/web/test/routes.test.tsx @@ -69,7 +69,6 @@ describe("route table", () => { "/agents", "/skills", "/insights", - "/approvals", "/settings", ]); }); @@ -98,7 +97,7 @@ describe("routes render", () => { } else if (NAV_PATHS.has(route.path)) { expect(activeRailLabel(markup)).toBe(route.label); } else { - // Chat and Approvals stay deep-linkable but leave the rail. + // Chat stays deep-linkable but leaves the rail. Approvals has no route. expect(activeRailLabel(markup)).toBeUndefined(); expect(markup).not.toMatch( new RegExp( From 1c5431603aea83149b27d2b0ce951ccbb0a257d1 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 9 Aug 2026 12:56:28 -0700 Subject: [PATCH 2/2] CL-5785: Drop ApprovalsPage tests and document route retirement --- apps/web/README.md | 5 ++++- apps/web/test/pages.test.tsx | 36 +----------------------------------- 2 files changed, 5 insertions(+), 36 deletions(-) diff --git a/apps/web/README.md b/apps/web/README.md index 724fef4c9..d775570af 100644 --- a/apps/web/README.md +++ b/apps/web/README.md @@ -56,9 +56,12 @@ in a real `@corbits/routines` listing later touches nothing else. | `/library` | The artifact gallery. See "Library" below. | | `/agents` | Agent definitions you can invite into a channel, and each channel's participants. | | `/skills` | A stub: skills have no registry in the hub yet, so this describes what's coming. | -| `/approvals` | Approvals waiting on the signed-in account. | | `/settings` | Account and bench membership settings. | +Approvals are not a page: pending permission requests land as actionable +cards in the contextual panel's Notifications band (and, when a channel is +open, inline in that channel). The `/approvals` route is gone. + ## Library `/library` (`src/pages/library-page.tsx`) is the artifact gallery: search, diff --git a/apps/web/test/pages.test.tsx b/apps/web/test/pages.test.tsx index 851221d9c..f8c357e47 100644 --- a/apps/web/test/pages.test.tsx +++ b/apps/web/test/pages.test.tsx @@ -7,14 +7,13 @@ import { renderToStaticMarkup } from "react-dom/server"; import type { ArtifactSummary } from "@corbits/artifact-ui"; -import type { APIQuery, NeedsYouItem } from "../src/api"; +import type { APIQuery } from "../src/api"; import type { AgentDefinition, AgentDirectoryData, AgentInstance, } from "../src/agents-api"; import { AgentsPage } from "../src/pages/agents-page"; -import { ApprovalsPage } from "../src/pages/approvals-page"; import { HomePage } from "../src/pages/home-page"; import { LibraryPage } from "../src/pages/library-page"; import { SkillsPage } from "../src/pages/skills-page"; @@ -57,39 +56,6 @@ describe("empty states", () => { ); expect(markup).toContain("Sign in required"); }); - - test("approvals says nothing is waiting", () => { - const markup = renderToStaticMarkup( - ([])} - onApprove={() => undefined} - onReject={() => undefined} - />, - ); - expect(markup).toContain("No approvals waiting"); - }); - - test("approvals renders resolved agent/bench names, never a raw agent address or run id", () => { - const item: NeedsYouItem = { - id: "apr_1", - agentName: "Outreach Composer", - benchName: "Growth Team Bench", - headline: "send_email", - arguments: { to: "customer@example.com" }, - status: "pending", - createdAt: "2026-01-01T00:00:00.000Z", - }; - const markup = renderToStaticMarkup( - ([item])} - onApprove={() => undefined} - onReject={() => undefined} - />, - ); - expect(markup).toContain("Outreach Composer"); - expect(markup).toContain("Growth Team Bench"); - expect(markup).not.toContain("apr_1"); - }); }); describe("signed-out state", () => {