From 8ec27906dbeb0bee8809889adb2122c83aa8c9df Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 27 Aug 2026 16:47:50 -0700 Subject: [PATCH] Approvals: read the native pending list and resolve names from the run view The needs-you read surface duplicated what Interchange already serves. The web now reads the native pending-approvals list and single-approval routes through one composer that adds the agent's display name from the run view, the headline from the approvals package's browser-safe subpath, and the bench name from membership. The chat approve card's status poll keeps its ready / forbidden / not-found / error shape, and the e2e harnesses page the native list and fail loud on a non-200 instead of silently skipping. The approvals package keeps its grant-allowance gate and headline helper; the needs-you routes, view model and their tests are gone, as is the hub mount. The vendored hub-api approval routes return to upstream's plain /:approvalId patterns, leaving that vendored copy with two deltas. --- ARCHITECTURE.md | 9 +- VENDORED.md | 22 +- apps/hub/src/index.ts | 15 - apps/hub/src/memory-mount.test.ts | 2 +- .../hub/test/artifact-doc-persistence.test.ts | 2 +- apps/web/package.json | 1 + apps/web/src/api.ts | 83 ----- apps/web/src/approval-actions.ts | 26 +- apps/web/src/pages/mission-control-page.tsx | 51 ++- apps/web/src/pages/workbench-timeline.tsx | 14 +- apps/web/src/pending-approvals.ts | 186 +++++++++++ apps/web/src/query-client.ts | 14 +- apps/web/src/shell/app-shell.tsx | 14 +- apps/web/src/shell/workbench-list.tsx | 8 +- apps/web/src/workbench-timeline-merge.test.ts | 6 +- apps/web/src/workbench-timeline-merge.ts | 6 +- apps/web/test/mission-control-page.test.tsx | 8 +- apps/web/test/pending-approvals.test.tsx | 246 ++++++++++++++ apps/web/test/query-client.test.ts | 6 +- apps/web/test/sidebar.test.tsx | 12 +- apps/web/test/workbench-list.test.tsx | 37 +- bun.lock | 15 +- docs/needs-you.md | 133 ++++---- packages/approvals/README.md | 45 ++- packages/approvals/package.json | 5 +- .../{view-model.test.ts => headline.test.ts} | 2 +- packages/approvals/src/headline.ts | 38 +++ packages/approvals/src/index.ts | 4 +- packages/approvals/src/routes.ts | 138 -------- packages/approvals/src/view-model.ts | 121 ------- packages/approvals/test/needs-you.test.ts | 315 ------------------ .../src/blocks/approve-card-state.test.ts | 14 +- .../chat-ui/src/blocks/approve-card-state.ts | 15 +- scripts/checks/browser-safe-subpaths.ts | 4 + scripts/checks/kill-dates.txt | 2 +- scripts/e2e/cl-6324-launch-proof.ts | 65 +++- scripts/e2e/play.ts | 74 ++-- vendor/intx/hub-api/VENDORED-FROM | 2 +- .../workflow-run-deploy-auth.test.ts | 4 +- vendor/intx/hub-api/src/routes/approvals.ts | 6 +- workflows/pain-point-collateral/README.md | 2 +- 41 files changed, 813 insertions(+), 959 deletions(-) create mode 100644 apps/web/src/pending-approvals.ts create mode 100644 apps/web/test/pending-approvals.test.tsx rename packages/approvals/src/{view-model.test.ts => headline.test.ts} (97%) create mode 100644 packages/approvals/src/headline.ts delete mode 100644 packages/approvals/src/routes.ts delete mode 100644 packages/approvals/src/view-model.ts delete mode 100644 packages/approvals/test/needs-you.test.ts diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index c6a3ca4ef3..8517cbcb5c 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -120,10 +120,11 @@ installed globally rather than per bench or workbench (CL-6272.2). Every external side effect — anything leaving the platform's own boundary — sits behind a human approval gate. Approval itself is native Interchange state (an `approval` row backed by a -`signal_correlation` row); workbench's own `@corbits/approvals` package -only resolves the names on top of it (which agent, which bench) so a -person can read what they're being asked to approve — see -[docs/needs-you.md](docs/needs-you.md). +`signal_correlation` row); the web client composes the names on top of it +(which agent, which bench) from the native pending-approvals list and run +view, and `@corbits/approvals` supplies the grant-allowance gate and the +human-readable headline, so a person can read what they're being asked to +approve — see [docs/needs-you.md](docs/needs-you.md). ## Sidecar allocation and the provisioner model diff --git a/VENDORED.md b/VENDORED.md index da39c7977e..fff12c6bdf 100644 --- a/VENDORED.md +++ b/VENDORED.md @@ -22,15 +22,15 @@ never a convenience. ## Ledger -| Vendored path | What was copied | Upstream repo @ commit | Why not a published package | Owner | Kill date | Kill-date test | -| ----------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ---------- | ----------------- | -| `apps/sidecar` | Derived from upstream's own `apps/sidecar`: 11 shared modules, of which `signing-keypair.ts` is near-verbatim and the rest (`index.ts`, `config.ts`, `tool-materialization.ts`, `workflow-run-pack-client.ts`, …) are substantially rewritten, plus workbench-only modules. A living fork, not a frozen copy, so this row carries no tree hash. | [faremeter/interchange](https://github.com/faremeter/interchange) @ `b5580a02` (v0.3.0) | An app is never npm-published, so no publish can cover the execution host; retired by consuming an upstream-published host, or by renewing this row deliberately | sawyer | 2026-09-19 | `check:killdates` | -| `vendor/intx/db` | `@intx/db` source (`src/`, `migrations/`, drizzle config, manifest, tsconfigs) | [faremeter/interchange](https://github.com/faremeter/interchange) @ `b5580a02` (v0.3.0) | npm 0.3.0 covers the base package but not the `wire_projection` column/loader delta (CL-6324) or the `workflow_definition.origin` column separating a definition from the per-run record of one folded run's deploy (CL-6452); retired when upstream absorbs the deltas | sawyer | 2026-09-19 | `check:killdates` | -| `vendor/intx/hub-api` | `@intx/hub-api` source (`src/`, manifest, tsconfig) | [faremeter/interchange](https://github.com/faremeter/interchange) @ `b5580a02` (v0.3.0) | npm 0.3.0 covers the base package but not the `needs-you` approval-route reservation, the exported null-principal `resolveApproval` (CL-6345), or the bearer-authenticated workflow-deploy mirror (`middleware/workflow-run-deploy-auth.ts`, CL-workflow-deploy-bearer); retired when upstream absorbs the deltas | sawyer | 2026-09-19 | `check:killdates` | -| `vendor/intx/hub-sessions` | `@intx/hub-sessions` source (`src/`, manifest, tsconfig) | [faremeter/interchange](https://github.com/faremeter/interchange) @ `b5580a02` (v0.3.0) | npm 0.3.0 covers the base package but not the usage forward (CL-5879), pack-acceptance fixes, adopted deploy front, wire-projection writer, event-collector serialization, anchor ordering, or malformed tool-call-name sanitization (CL-6478) | sawyer | 2026-09-19 | `check:killdates` | -| `vendor/intx/workflow` | `@intx/workflow` source (`src/`, manifest, tsconfig) | [faremeter/interchange](https://github.com/faremeter/interchange) @ `b5580a02` (v0.3.0) | npm 0.3.0 covers the base package but not the `onBodyFailure` trigger policy and its projection (CL-6326, CL-6324); retired when upstream absorbs the delta | sawyer | 2026-09-19 | `check:killdates` | -| `vendor/intx/workflow-deploy` | `@intx/workflow-deploy` source (`src/`, manifest, tsconfig) | [faremeter/interchange](https://github.com/faremeter/interchange) @ `b5580a02` (v0.3.0) | Carries no delta of its own, but must bind against the vendored `@intx/workflow` (whose `onBodyFailure` field flows through the projection it hashes); retired with the workflow delta | sawyer | 2026-09-19 | `check:killdates` | -| `vendor/intx/workflow-host` | `@intx/workflow-host` source (`src/`, manifest, tsconfig) | [faremeter/interchange](https://github.com/faremeter/interchange) @ `b5580a02` (v0.3.0) | npm 0.3.0 covers the base package but not the empty-mail drop (CL-6164), the action/loop runtime bind (CL-6325; its adapters live in `packages/workflow-host-actions` since CL-6435), or the body-spawn authorize/credential threading (CL-6448); retired when upstream absorbs the deltas | sawyer | 2026-09-19 | `check:killdates` | +| Vendored path | What was copied | Upstream repo @ commit | Why not a published package | Owner | Kill date | Kill-date test | +| ----------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------ | ---------- | ----------------- | +| `apps/sidecar` | Derived from upstream's own `apps/sidecar`: 11 shared modules, of which `signing-keypair.ts` is near-verbatim and the rest (`index.ts`, `config.ts`, `tool-materialization.ts`, `workflow-run-pack-client.ts`, …) are substantially rewritten, plus workbench-only modules. A living fork, not a frozen copy, so this row carries no tree hash. | [faremeter/interchange](https://github.com/faremeter/interchange) @ `b5580a02` (v0.3.0) | An app is never npm-published, so no publish can cover the execution host; retired by consuming an upstream-published host, or by renewing this row deliberately | sawyer | 2026-09-19 | `check:killdates` | +| `vendor/intx/db` | `@intx/db` source (`src/`, `migrations/`, drizzle config, manifest, tsconfigs) | [faremeter/interchange](https://github.com/faremeter/interchange) @ `b5580a02` (v0.3.0) | npm 0.3.0 covers the base package but not the `wire_projection` column/loader delta (CL-6324) or the `workflow_definition.origin` column separating a definition from the per-run record of one folded run's deploy (CL-6452); retired when upstream absorbs the deltas | sawyer | 2026-09-19 | `check:killdates` | +| `vendor/intx/hub-api` | `@intx/hub-api` source (`src/`, manifest, tsconfig) | [faremeter/interchange](https://github.com/faremeter/interchange) @ `b5580a02` (v0.3.0) | npm 0.3.0 covers the base package but not the exported null-principal `resolveApproval` (CL-6345) or the bearer-authenticated workflow-deploy mirror (`middleware/workflow-run-deploy-auth.ts`, CL-workflow-deploy-bearer); retired when upstream absorbs the deltas | sawyer | 2026-09-19 | `check:killdates` | +| `vendor/intx/hub-sessions` | `@intx/hub-sessions` source (`src/`, manifest, tsconfig) | [faremeter/interchange](https://github.com/faremeter/interchange) @ `b5580a02` (v0.3.0) | npm 0.3.0 covers the base package but not the usage forward (CL-5879), pack-acceptance fixes, adopted deploy front, wire-projection writer, event-collector serialization, anchor ordering, or malformed tool-call-name sanitization (CL-6478) | sawyer | 2026-09-19 | `check:killdates` | +| `vendor/intx/workflow` | `@intx/workflow` source (`src/`, manifest, tsconfig) | [faremeter/interchange](https://github.com/faremeter/interchange) @ `b5580a02` (v0.3.0) | npm 0.3.0 covers the base package but not the `onBodyFailure` trigger policy and its projection (CL-6326, CL-6324); retired when upstream absorbs the delta | sawyer | 2026-09-19 | `check:killdates` | +| `vendor/intx/workflow-deploy` | `@intx/workflow-deploy` source (`src/`, manifest, tsconfig) | [faremeter/interchange](https://github.com/faremeter/interchange) @ `b5580a02` (v0.3.0) | Carries no delta of its own, but must bind against the vendored `@intx/workflow` (whose `onBodyFailure` field flows through the projection it hashes); retired with the workflow delta | sawyer | 2026-09-19 | `check:killdates` | +| `vendor/intx/workflow-host` | `@intx/workflow-host` source (`src/`, manifest, tsconfig) | [faremeter/interchange](https://github.com/faremeter/interchange) @ `b5580a02` (v0.3.0) | npm 0.3.0 covers the base package but not the empty-mail drop (CL-6164), the action/loop runtime bind (CL-6325; its adapters live in `packages/workflow-host-actions` since CL-6435), or the body-spawn authorize/credential threading (CL-6448); retired when upstream absorbs the deltas | sawyer | 2026-09-19 | `check:killdates` | The pinned commit `b5580a02` is upstream's `v0.3.0` release tag, 16 commits past the previous pin `4ed8baf4`: a workflow-host supervisor @@ -60,9 +60,7 @@ direct TypeScript source resolution (`types`/`default` → `./src/...`), with removed — workbench forbids custom resolve conditions — and each tsconfig carries `types: ["bun"]`; `vendor/intx/hub-api` adds a `@types/ssri` devDependency that bun's isolated linker does not hoist -from tool-packaging the way upstream's install does, and its approval param -routes exclude the reserved segment `needs-you` so hosts can mount a sibling -`/approvals/needs-you` list without `/:approvalId` capturing it. +from tool-packaging the way upstream's install does. `vendor/intx/hub-api` (CL-workflow-deploy-bearer) also adds `middleware/workflow-run-deploy-auth.ts`, an optional bearer-authenticated mirror onto the SAME session-cookie `POST/GET .../workflows/deployments` diff --git a/apps/hub/src/index.ts b/apps/hub/src/index.ts index 5529ff05ae..a2eaa61be8 100644 --- a/apps/hub/src/index.ts +++ b/apps/hub/src/index.ts @@ -234,7 +234,6 @@ import { wireMailRedelivery } from "./mail-redelivery"; import { getLogger, setup } from "@intx/log"; import { hexEncode } from "@intx/types"; import { - createNeedsYouRoutes, createToolAllowanceRegistry, withGrantAllowance, } from "@corbits/approvals"; @@ -1128,20 +1127,6 @@ export async function createHub(config: HubConfig) { let artifactSeedOnJoin: ((key: PresenceRoomKey, principalId: string) => Promise) | undefined; - // The "needs you" list: the same `approval:*`/"resolve" grant Interchange's - // own approve/reject routes require, layered with the agent/bench names - // this tenant's approvals don't carry on their own. Approving and - // rejecting still go straight to Interchange's native routes below -- - // this route only ever reads. - app.route( - `${TENANT_PREFIX}/approvals/needs-you`, - createNeedsYouRoutes({ - db, - grantStore: createGrantStore(db), - conditionRegistry: { time_window: timeWindowEvaluator }, - }), - ); - // Chat's own grant store/condition registry, built the same way // `createApp` builds its default when none is supplied (see // `@intx/hub-api`'s `mountHubRoutes`): a db-backed grant store and diff --git a/apps/hub/src/memory-mount.test.ts b/apps/hub/src/memory-mount.test.ts index 5152db799c..57023c1a42 100644 --- a/apps/hub/src/memory-mount.test.ts +++ b/apps/hub/src/memory-mount.test.ts @@ -119,7 +119,7 @@ describe("mountMemory", () => { // DB-gated: skipped when DATABASE_URL is unreachable, matching this repo's // existing convention for tests that talk to a real Postgres (see -// packages/approvals/test/needs-you.test.ts). Proves the actual cutover: +// apps/hub/test/artifact-doc-persistence.test.ts). Proves the actual cutover: // mounting with only DATABASE_URL (no second memory-plane URL) lands the // memory engine's tables in its own `memory` schema, never `public`. const databaseUrl = process.env["DATABASE_URL"]; diff --git a/apps/hub/test/artifact-doc-persistence.test.ts b/apps/hub/test/artifact-doc-persistence.test.ts index 945ae83f01..d948ff1a03 100644 --- a/apps/hub/test/artifact-doc-persistence.test.ts +++ b/apps/hub/test/artifact-doc-persistence.test.ts @@ -1,6 +1,6 @@ // DB-gated: skipped when DATABASE_URL is unreachable, matching this repo's // existing convention for tests that talk to a real Postgres (see -// packages/approvals/test/needs-you.test.ts). Proves the exact wiring +// apps/hub/src/memory-mount.test.ts). Proves the exact wiring // `src/index.ts` composes between `@corbits/presence`'s // `createArtifactDocPersistence` and `@corbits/artifacts`' own // `getArtifact`/`writeArtifactVersion` seam actually lands a new artifact diff --git a/apps/web/package.json b/apps/web/package.json index dcfd638adf..4646db72ac 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -19,6 +19,7 @@ "dependencies": { "@corbits/agent-directory": "workspace:*", "@corbits/api-query": "workspace:*", + "@corbits/approvals": "workspace:*", "@corbits/artifact-ui": "workspace:*", "@corbits/bench-ui": "workspace:*", "@corbits/chat": "workspace:*", diff --git a/apps/web/src/api.ts b/apps/web/src/api.ts index 3a2231a7b9..effff10a6e 100644 --- a/apps/web/src/api.ts +++ b/apps/web/src/api.ts @@ -72,36 +72,6 @@ export const ArtifactCountsSchema = type({ routine: "number", }); -// `@corbits/approvals`'s "needs you" read: the same pending approvals as -// `TenantApprovalsSchema`, but with the agent and bench names already -// resolved server-side, so nothing here ever needs a raw id to render. -export const NeedsYouSchema = type({ - items: type({ - id: "string", - agentName: "string", - benchName: "string", - headline: "string", - arguments: "object", - status: '"pending"', - createdAt: "string.date.iso", - }).array(), -}); - -// The single-approval sibling of `NeedsYouSchema` (`@corbits/approvals`'s -// `GET .../approvals/needs-you/:approvalId`): the same display-safe -// hydration, for one id, in any status -- not just pending. This is the -// chat approve card's live status read; see `approval-actions.ts`. -export const NeedsYouDetailSchema = type({ - id: "string", - agentName: "string", - benchName: "string", - headline: "string", - arguments: "object", - status: "'pending' | 'approved' | 'rejected' | 'timeout' | 'expired'", - createdAt: "string.date.iso", -}); -export type NeedsYouDetail = typeof NeedsYouDetailSchema.infer; - export type Profile = typeof UserProfile.infer; export type Principal = typeof PrincipalSummary.infer; export type WorkflowRun = typeof WorkflowRunSummary.infer; @@ -111,23 +81,6 @@ export type ArtifactListItem = typeof ArtifactListItemSchema.infer; export type ArtifactListPage = typeof ArtifactListPageSchema.infer; export type ArtifactDetail = typeof ArtifactDetailSchema.infer; export type ArtifactCounts = typeof ArtifactCountsSchema.infer; -export type NeedsYou = typeof NeedsYouSchema.infer; -export type NeedsYouItem = NeedsYou["items"][number]; - -/** - * How many things need this bench's attention right now — the count the - * second column's "Approvals" row badges. `null` while unknown (no bench - * selected yet, or the read hasn't resolved), so a caller never mistakes - * "still loading" for "zero pending." - */ -export function useNeedsYouCount(tenantId: string | null): number | null { - const query = useAPIQuery( - tenantId === null ? "" : `/api/tenants/${tenantId}/approvals/needs-you`, - NeedsYouSchema, - ); - return query.kind === "ready" ? query.data.items.length : null; -} - /** * The envelope paginatedSchema validates, stated structurally: the generic * schema's inferred type carries an arktype inference artifact that rejects @@ -296,39 +249,3 @@ export async function fetchArtifactDetail( } return parsed; } - -export type NeedsYouDetailResult = - | { readonly kind: "ready"; readonly item: NeedsYouDetail } - | { readonly kind: "forbidden" } - | { readonly kind: "not-found" } - | { readonly kind: "error"; readonly message: string }; - -/** - * The chat approve card's live status read: `@corbits/approvals`'s - * single-approval "needs you" detail, in any status. A 403 here means the - * tenant-wide read grant was refused -- deliberately coarser than the - * native approve/reject routes' per-deployment grant, so it is not proof - * the viewer cannot resolve this approval (see `approval-actions.ts`). - */ -export async function getApprovalNeedsYou( - tenantId: string, - approvalId: string, -): Promise { - const response = await fetch( - `/api/tenants/${tenantId}/approvals/needs-you/${approvalId}`, - { headers: { accept: "application/json" } }, - ); - if (response.status === 403) return { kind: "forbidden" }; - if (response.status === 404) return { kind: "not-found" }; - if (!response.ok) { - return { - kind: "error", - message: `The server answered ${response.status} for this approval.`, - }; - } - const parsed = NeedsYouDetailSchema(await response.json()); - if (parsed instanceof type.errors) { - return { kind: "error", message: parsed.summary }; - } - return { kind: "ready", item: parsed }; -} diff --git a/apps/web/src/approval-actions.ts b/apps/web/src/approval-actions.ts index 7ac3583af5..1737f38ed3 100644 --- a/apps/web/src/approval-actions.ts +++ b/apps/web/src/approval-actions.ts @@ -1,9 +1,9 @@ // Builds the `ApprovalActions` port `ChatWorkspace` (`@corbits/chat-ui`) -// calls for its in-chat approve card. Reuses the exact functions the -// Activity band already uses -- `getApprovalNeedsYou`, `approveApproval`, -// `rejectApproval` in `api.ts` -- and invalidates the same query keys it -// invalidates, so approving in chat updates the Activity band's count -// live, and vice versa. This is the one place that logic belongs: chat-ui +// calls for its in-chat approve card. Reuses the exact reads and writes +// every other approval surface uses -- `getApprovalDetail` in +// `pending-approvals.ts`, `approveApproval`/`rejectApproval` in `api.ts` -- +// and invalidates the same query keys they invalidate, so approving in chat +// updates the shell's pending count live, and vice versa. This is the one place that logic belongs: chat-ui // owns no `QueryClient`, so the invalidation the design calls for can only // run here. @@ -12,8 +12,9 @@ import type { ApprovalActions, ApprovalDecisionResult } from "@corbits/chat-ui"; import { CHAT_STRINGS } from "@corbits/chat-ui"; import { ApiQueryError } from "@corbits/api-query"; -import { approveApproval, getApprovalNeedsYou, rejectApproval } from "./api"; +import { approveApproval, rejectApproval } from "./api"; import type { Approval } from "./api"; +import { getApprovalDetail } from "./pending-approvals"; import { tenantKeys } from "./query-client"; export function createChatApprovalActions( @@ -22,7 +23,7 @@ export function createChatApprovalActions( ): ApprovalActions { function invalidate() { void queryClient.invalidateQueries({ - queryKey: tenantKeys.needsYou(tenantId), + queryKey: tenantKeys.pendingApprovals(tenantId), }); // Any cached query keyed under this tenant's inbox API paths (list, // detail, counts) — the tasks backend still owns these routes even @@ -80,13 +81,12 @@ export function createChatApprovalActions( return { async getStatus(approvalId) { - const result = await getApprovalNeedsYou(tenantId, approvalId); + const result = await getApprovalDetail(tenantId, approvalId); switch (result.kind) { case "ready": - // A successful read against the tenant-wide `approval:*` grant - // implies the native per-deployment grant too (the pattern the - // wider grant is checked against subsumes it) -- pending here is - // always actionable. See `packages/approvals/src/routes.ts`. + // This read is gated on the very grant approve and reject are + // gated on, so a pending approval that could be read is one this + // viewer can act on. return { kind: "ready", status: result.item.status, @@ -94,7 +94,7 @@ export function createChatApprovalActions( detail: { agentName: result.item.agentName, headline: result.item.headline, - arguments: result.item.arguments as Record, + arguments: result.item.arguments, }, }; case "forbidden": diff --git a/apps/web/src/pages/mission-control-page.tsx b/apps/web/src/pages/mission-control-page.tsx index 473154c37b..5c79090406 100644 --- a/apps/web/src/pages/mission-control-page.tsx +++ b/apps/web/src/pages/mission-control-page.tsx @@ -2,7 +2,7 @@ // decision, what's running, and a way back into recent context. A new // top-level route (`/mission-control`), never `/` — `/` stays the Myra // land-hop redirect (see routes.tsx's header comment). Every panel here is -// backed by a query already used elsewhere in this app (needs-you +// backed by a query already used elsewhere in this app (pending // approvals, top-level runs, insights activity); nothing on this page is // invented. A panel with no honest data source renders an empty state // naming what's missing instead of a fabricated number. @@ -33,16 +33,14 @@ import { useQueryClient } from "@tanstack/react-query"; import { formatUsd } from "@corbits/insights/client"; import { CHAT_STRINGS, type Workbench } from "@corbits/chat-ui"; -import { - approveApproval, - NeedsYouSchema, - rejectApproval, - useAPIQuery, - type NeedsYouItem, -} from "../api"; +import { approveApproval, rejectApproval, useAPIQuery } from "../api"; import { useBench } from "../bench-context"; import { ActivityResponseSchema, insightsActivityPath } from "../insights-api"; import { Link } from "../navigation"; +import { + usePendingApprovals, + type PendingApproval, +} from "../pending-approvals"; import { tenantKeys } from "../query-client"; import { NEW_WORKBENCH_PATH } from "../routes"; import { useBenchActivity } from "../shell/bench-activity"; @@ -137,7 +135,7 @@ function ApprovalRow({ item, tenantId, }: { - readonly item: NeedsYouItem; + readonly item: PendingApproval; readonly tenantId: string; }) { const queryClient = useQueryClient(); @@ -149,7 +147,7 @@ function ApprovalRow({ if (action === "approve") await approveApproval(tenantId, item.id); else await rejectApproval(tenantId, item.id); await queryClient.invalidateQueries({ - queryKey: tenantKeys.needsYou(tenantId), + queryKey: tenantKeys.pendingApprovals(tenantId), }); } catch (cause) { toast( @@ -201,10 +199,7 @@ export function MissionControlRoute({ readonly navigate: (to: string) => void; }) { const { selectedTenantId: tenantId } = useBench(); - const needsYouQuery = useAPIQuery( - tenantId === null ? "" : `/api/tenants/${tenantId}/approvals/needs-you`, - NeedsYouSchema, - ); + const approvalsQuery = usePendingApprovals(tenantId); const activity = useBenchActivity(tenantId); const activityRange = useMemo( () => ({ @@ -218,11 +213,11 @@ export function MissionControlRoute({ ActivityResponseSchema, ); - const needsYouItems = - needsYouQuery.kind === "ready" ? needsYouQuery.data.items : null; + const pendingApprovals = + approvalsQuery.kind === "ready" ? approvalsQuery.data : null; const oldestWaitingAt = - needsYouItems !== null && needsYouItems.length > 0 - ? needsYouItems.reduce((oldest, item) => + pendingApprovals !== null && pendingApprovals.length > 0 + ? pendingApprovals.reduce((oldest, item) => Date.parse(item.createdAt) < Date.parse(oldest.createdAt) ? item : oldest, @@ -297,8 +292,10 @@ export function MissionControlRoute({ /> 0} + value={dash(pendingApprovals?.length ?? null)} + danger={ + pendingApprovals !== null && pendingApprovals.length > 0 + } sub={ oldestWaitingAt !== null ? `oldest ${formatRelativeTime(oldestWaitingAt)}` @@ -328,28 +325,28 @@ export function MissionControlRoute({ approvals block agents until you act - {needsYouQuery.kind === "loading" ? ( + {approvalsQuery.kind === "loading" ? ( ) : null} - {needsYouQuery.kind === "error" ? ( + {approvalsQuery.kind === "error" ? ( ) : null} - {needsYouQuery.kind === "unauthenticated" ? ( + {approvalsQuery.kind === "unauthenticated" ? ( ) : null} - {needsYouItems !== null && needsYouItems.length === 0 ? ( + {pendingApprovals !== null && pendingApprovals.length === 0 ? ( ) : null} - {needsYouItems !== null && needsYouItems.length > 0 ? ( + {pendingApprovals !== null && pendingApprovals.length > 0 ? ( @@ -365,7 +362,7 @@ export function MissionControlRoute({ {tenantId !== null && - needsYouItems.map((item) => ( + pendingApprovals.map((item) => ( ; + readonly status: "pending" | "approved" | "rejected" | "timeout" | "expired"; + readonly createdAt: string; +}; + +/** A pending approval, named for where it landed. The name is absent, not + * invented, when the account's membership list hasn't resolved. */ +export type PendingApproval = ApprovalDisplay & { + readonly benchName?: string; +}; + +type ApprovalRow = typeof ApprovalResponse.infer; + +// Copy for the rare case where the naming read cannot answer: an approval is +// blocking an agent right now, so it is always shown — an unnamed asker is +// far better than a hidden decision. +const UNNAMED_AGENT = "An agent"; + +export function pendingApprovalsPath(tenantId: string): string { + return `/api/tenants/${tenantId}/approvals`; +} + +function runViewPath(tenantId: string, runId: string): string { + return `/api/tenants/${tenantId}/runs/${runId}`; +} + +/** + * The display name of the agent behind a run. Answers `UNNAMED_AGENT` rather + * than failing when the run view is unreadable — a per-deployment approver + * can hold the grant to resolve an approval without holding the read grant + * on its run. + */ +async function fetchAgentName( + tenantId: string, + runId: string, +): Promise { + const response = await fetch(runViewPath(tenantId, runId), { + headers: { accept: "application/json" }, + }); + if (!response.ok) return UNNAMED_AGENT; + const parsed = WorkflowRunResponse(await response.json()); + if (parsed instanceof type.errors) return UNNAMED_AGENT; + return parsed.definitionName; +} + +function composeApproval(row: ApprovalRow, agentName: string): ApprovalDisplay { + return { + id: row.id, + agentName, + headline: headlineFor(row.toolDefinition, row.toolArguments), + arguments: row.toolArguments, + status: row.status, + createdAt: row.createdAt, + }; +} + +/** + * Every approval pending on this bench, newest-first as the platform lists + * them. One naming read per distinct run, cached by react-query, so a run + * with several pending asks is named once. + */ +export function usePendingApprovals( + tenantId: string | null, +): APIQuery { + const { memberships } = useBench(); + const list = useAPIQuery( + tenantId === null ? "" : pendingApprovalsPath(tenantId), + TenantApprovalsSchema, + ); + const rows = list.kind === "ready" ? list.data.data : []; + const runIds = [...new Set(rows.map((row) => row.runId))]; + const agentNames = useQueries({ + queries: + tenantId === null + ? [] + : runIds.map((runId) => ({ + queryKey: tenantKeys.runView(tenantId, runId), + queryFn: () => fetchAgentName(tenantId, runId), + })), + }); + + if (list.kind !== "ready") return list; + // A half-named list would render the placeholder and then swap in real + // names a tick later; wait instead. + if (agentNames.some((name) => name.isPending)) return { kind: "loading" }; + + const agentNameByRunId = new Map( + runIds.map((runId, index) => [ + runId, + agentNames[index]?.data ?? UNNAMED_AGENT, + ]), + ); + const membership = + memberships.kind === "ready" + ? memberships.data.data.find((row) => row.tenantId === tenantId) + : undefined; + const benchName = membership?.tenantName; + + return { + kind: "ready", + data: rows.map((row) => ({ + ...composeApproval(row, agentNameByRunId.get(row.runId) ?? UNNAMED_AGENT), + ...(benchName !== undefined ? { benchName } : {}), + })), + }; +} + +/** + * How many things need this bench's attention right now — the count the + * shell's chip and the second column's signal render. `null` while unknown + * (no bench selected yet, or the read hasn't resolved), so a caller never + * mistakes "still loading" for "zero pending." + */ +export function usePendingApprovalCount( + tenantId: string | null, +): number | null { + const list = useAPIQuery( + tenantId === null ? "" : pendingApprovalsPath(tenantId), + TenantApprovalsSchema, + ); + return list.kind === "ready" ? list.data.data.length : null; +} + +export type ApprovalDetailResult = + | { readonly kind: "ready"; readonly item: ApprovalDisplay } + | { readonly kind: "forbidden" } + | { readonly kind: "not-found" } + | { readonly kind: "error"; readonly message: string }; + +/** + * The chat approve card's live status read: one approval, in any status, + * through the same composer the list uses. The grant this read is refused + * on (403) is the same per-deployment grant approve and reject are gated + * on, so a refusal here is proof the viewer cannot act on it. + */ +export async function getApprovalDetail( + tenantId: string, + approvalId: string, +): Promise { + const response = await fetch( + `/api/tenants/${tenantId}/approvals/${approvalId}`, + { headers: { accept: "application/json" } }, + ); + if (response.status === 403) return { kind: "forbidden" }; + if (response.status === 404) return { kind: "not-found" }; + if (!response.ok) { + return { + kind: "error", + message: `The server answered ${response.status} for this approval.`, + }; + } + const parsed = ApprovalResponse(await response.json()); + if (parsed instanceof type.errors) { + return { kind: "error", message: parsed.summary }; + } + return { + kind: "ready", + item: composeApproval(parsed, await fetchAgentName(tenantId, parsed.runId)), + }; +} diff --git a/apps/web/src/query-client.ts b/apps/web/src/query-client.ts index a9bdf9293a..e7bc333b9e 100644 --- a/apps/web/src/query-client.ts +++ b/apps/web/src/query-client.ts @@ -84,8 +84,12 @@ export const meKeys = { /** Tenant-scoped keys — removed wholesale when the user leaves a bench. */ export const tenantKeys = { all: (tenantId: string) => ["tenant", tenantId] as const, - needsYou: (tenantId: string) => - ["tenant", tenantId, "approvals", "needs-you"] as const, + pendingApprovals: (tenantId: string) => + ["tenant", tenantId, "approvals"] as const, + /** One agent-name read per run, shared by every approval that run raised + * (see `pending-approvals.ts`). */ + runView: (tenantId: string, runId: string) => + ["tenant", tenantId, "runs", runId] as const, routines: (tenantId: string) => ["tenant", tenantId, "routines"] as const, skills: (tenantId: string) => ["tenant", tenantId, "skills"] as const, mcpServers: (tenantId: string) => @@ -163,8 +167,10 @@ export const tenantKeys = { export function pathToQueryKey(path: string): readonly unknown[] { if (path === "/api/me") return meKeys.profile; if (path === "/api/me/principals") return meKeys.principals; - const needsYou = /^\/api\/tenants\/([^/]+)\/approvals\/needs-you$/.exec(path); - if (needsYou?.[1] !== undefined) return tenantKeys.needsYou(needsYou[1]); + const approvals = /^\/api\/tenants\/([^/]+)\/approvals$/.exec(path); + if (approvals?.[1] !== undefined) { + return tenantKeys.pendingApprovals(approvals[1]); + } const assets = /^\/api\/tenants\/([^/]+)\/assets$/.exec(path); if (assets?.[1] !== undefined) return tenantKeys.assets(assets[1]); const artifactCounts = /^\/api\/tenants\/([^/]+)\/artifacts\/counts$/.exec( diff --git a/apps/web/src/shell/app-shell.tsx b/apps/web/src/shell/app-shell.tsx index fe7398433a..7401589ae5 100644 --- a/apps/web/src/shell/app-shell.tsx +++ b/apps/web/src/shell/app-shell.tsx @@ -24,7 +24,7 @@ import type { ArtifactSaveState } from "@corbits/artifact-ui"; import { WorkbenchLoadingState } from "@corbits/chat-ui"; -import { useNeedsYouCount } from "../api"; +import { usePendingApprovalCount } from "../pending-approvals"; import { useBench } from "../bench-context"; import { useNavigate } from "../navigation"; import { usePresenceRoom } from "../presence/use-presence-room"; @@ -150,14 +150,14 @@ export function AppShell({ const mainRef = useRef(null); // Route changes must not inherit the previous page's scroll position. useScrollReset(mainRef, path); - const needsYouCount = useNeedsYouCount(tenantId); - const needsYouChip = - needsYouCount === null + const pendingCount = usePendingApprovalCount(tenantId); + const pendingChip = + pendingCount === null ? undefined - : needsYouCount > 0 + : pendingCount > 0 ? { tone: "needs-you" as const, - label: `${String(needsYouCount)} waiting on you`, + label: `${String(pendingCount)} waiting on you`, } : { tone: "ok" as const, label: "All caught up" }; @@ -175,7 +175,7 @@ export function AppShell({ {routeHasNoStageTopBar(path) ? ( ) : null} diff --git a/apps/web/src/workbench-timeline-merge.test.ts b/apps/web/src/workbench-timeline-merge.test.ts index 5e7de5ec5e..269a9e7db9 100644 --- a/apps/web/src/workbench-timeline-merge.test.ts +++ b/apps/web/src/workbench-timeline-merge.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from "bun:test"; import type { WorkbenchThread, MessageItem } from "@corbits/chat-ui"; -import type { NeedsYouItem } from "./api"; +import type { PendingApproval } from "./pending-approvals"; import type { Routine, RoutineRun } from "./routines-api"; import { computeTimelineDayKpis, @@ -73,7 +73,7 @@ function routineRun(overrides: Partial = {}): RoutineRun { } as RoutineRun; } -function approval(overrides: Partial = {}): NeedsYouItem { +function approval(overrides: Partial = {}): PendingApproval { return { id: "approval-1", agentName: "Researcher", @@ -83,7 +83,7 @@ function approval(overrides: Partial = {}): NeedsYouItem { status: "pending", createdAt: "2026-08-17T12:00:00.000Z", ...overrides, - } as NeedsYouItem; + } as PendingApproval; } describe("toMessageEvents", () => { diff --git a/apps/web/src/workbench-timeline-merge.ts b/apps/web/src/workbench-timeline-merge.ts index 5365842271..a3ef7e943e 100644 --- a/apps/web/src/workbench-timeline-merge.ts +++ b/apps/web/src/workbench-timeline-merge.ts @@ -8,7 +8,7 @@ import { isAgentAddress } from "@corbits/chat/mentions"; import { localPartOf } from "@corbits/chat/agent-address"; import type { WorkbenchThread, MessageItem } from "@corbits/chat-ui"; -import type { NeedsYouItem } from "./api"; +import type { PendingApproval } from "./pending-approvals"; import type { Routine, RoutineRun } from "./routines-api"; export type TimelineMessageEvent = { @@ -165,12 +165,12 @@ export function toRoutineRunEvents( } /** - * `needs-you` carries no workbench/workbench id (a known v1 gap — see + * An approval carries no workbench id (a known v1 gap — see * workbench-timeline.tsx), so every pending approval for the owning bench * shows up here, not just this workbench's own. */ export function toApprovalEvents( - items: readonly NeedsYouItem[], + items: readonly PendingApproval[], ): readonly TimelineApprovalEvent[] { return items.map((item) => ({ kind: "approval", diff --git a/apps/web/test/mission-control-page.test.tsx b/apps/web/test/mission-control-page.test.tsx index c62e8184e4..8b95c66e18 100644 --- a/apps/web/test/mission-control-page.test.tsx +++ b/apps/web/test/mission-control-page.test.tsx @@ -120,9 +120,9 @@ const benchState: BenchState = { function stubEmptyBenchFetch(): void { globalThis.fetch = ((input: RequestInfo | URL) => { const url = typeof input === "string" ? input : input.toString(); - if (url.includes("/approvals/needs-you")) { + if (url.includes("/approvals")) { return Promise.resolve( - new Response(JSON.stringify({ items: [] }), { + new Response(JSON.stringify({ data: [], nextCursor: null }), { status: 200, headers: { "content-type": "application/json" }, }), @@ -163,9 +163,9 @@ function stubBenchFetchWithActivity( ): void { globalThis.fetch = ((input: RequestInfo | URL) => { const url = typeof input === "string" ? input : input.toString(); - if (url.includes("/approvals/needs-you")) { + if (url.includes("/approvals")) { return Promise.resolve( - new Response(JSON.stringify({ items: [] }), { + new Response(JSON.stringify({ data: [], nextCursor: null }), { status: 200, headers: { "content-type": "application/json" }, }), diff --git a/apps/web/test/pending-approvals.test.tsx b/apps/web/test/pending-approvals.test.tsx new file mode 100644 index 0000000000..2e25ec1243 --- /dev/null +++ b/apps/web/test/pending-approvals.test.tsx @@ -0,0 +1,246 @@ +// The client-side composer behind every approval surface: the platform's +// pending list, named through the run view and the account's own +// membership. What matters here is that a run is named once no matter how +// many approvals it raised, that a refused list read is shown as a failure +// rather than an empty queue, and that the chat card's status read speaks +// the same display model. + +import { afterEach, describe, expect, test } from "bun:test"; +import { QueryClient } from "@tanstack/react-query"; +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; + +import { createChatApprovalActions } from "../src/approval-actions"; +import { BenchContext, type BenchState } from "../src/bench-context"; +import { NavigationProvider } from "../src/navigation"; +import { MissionControlRoute } from "../src/pages/mission-control-page"; +import { usePendingApprovals } from "../src/pending-approvals"; +import { TestQueryProvider } from "./test-query-provider"; + +const realFetch = globalThis.fetch; +const TENANT_ID = "tnt_1"; + +const benchState: BenchState = { + memberships: { + kind: "ready", + data: { + data: [ + { + principalId: "prn_1", + tenantId: TENANT_ID, + tenantName: "Growth Team Bench", + tenantSlug: "growth-team-bench", + kind: "user", + status: "active", + roles: [], + }, + ], + nextCursor: null, + }, + }, + selectedTenantId: TENANT_ID, + selectedPrincipalId: "prn_1", + selectTenant: () => undefined, + onBenchCreated: () => undefined, +}; + +function approvalRow(overrides: Record = {}) { + return { + id: "apr_1", + tenantId: TENANT_ID, + anchorRunId: "run_1", + runId: "run_1", + agentAddress: "researcher@growth", + correlationId: "cor_1", + toolDefinition: { + name: "send_email", + description: "Sends an email on the tenant's behalf", + }, + toolArguments: { title: "Welcome Acme" }, + scope: null, + status: "pending", + timeoutAt: null, + resolvedAt: null, + createdAt: "2026-08-20T09:00:00.000Z", + updatedAt: "2026-08-20T09:00:00.000Z", + ...overrides, + }; +} + +function json(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { "content-type": "application/json" }, + }); +} + +/** Every path the stub was asked for, in order — the evidence that a run is + * named once per run and not once per approval. */ +let requested: string[] = []; + +function stubFetch( + handler: (path: string) => Response | undefined, + fallback: Response = json({ items: [], data: [], nextCursor: null }), +): void { + requested = []; + globalThis.fetch = ((input: RequestInfo | URL) => { + const path = typeof input === "string" ? input : String(input); + requested.push(path); + return Promise.resolve(handler(path) ?? fallback.clone()); + }) as typeof fetch; +} + +afterEach(() => { + globalThis.fetch = realFetch; +}); + +/** Renders exactly what the composer produced, so the assertions read the + * display model the way a card does. */ +function ApprovalNames() { + const approvals = usePendingApprovals(TENANT_ID); + if (approvals.kind !== "ready") return

{approvals.kind}

; + return ( +
    + {approvals.data.map((approval) => ( +
  • + {approval.agentName} in {approval.benchName}: {approval.headline} +
    • + ))} +
+ ); +} + +describe("usePendingApprovals", () => { + let container: HTMLDivElement | null = null; + let root: Root | null = null; + + afterEach(() => { + if (root !== null) { + act(() => root?.unmount()); + root = null; + } + container?.remove(); + container = null; + }); + + async function mount(children: React.ReactNode) { + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + await act(async () => { + root?.render( + + undefined}> + + {children} + + + , + ); + }); + for (let tick = 0; tick < 10; tick += 1) { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + } + return container; + } + + test("names both of one run's approvals from a single run-view read", async () => { + stubFetch((path) => { + if (path === `/api/tenants/${TENANT_ID}/approvals`) { + return json({ + data: [approvalRow(), approvalRow({ id: "apr_2" })], + nextCursor: null, + }); + } + if (path === `/api/tenants/${TENANT_ID}/runs/run_1`) { + return json({ + id: "run_1", + definitionId: "def_1", + definitionName: "Research Analyst", + tenantId: TENANT_ID, + address: "researcher@growth", + status: "running", + createdAt: "2026-08-20T08:00:00.000Z", + updatedAt: "2026-08-20T08:00:00.000Z", + }); + } + return undefined; + }); + + const el = await mount(); + + const items = [...el.querySelectorAll("li")].map((li) => li.textContent); + expect(items).toEqual([ + 'Research Analyst in Growth Team Bench: Sends an email on the tenant\'s behalf: "Welcome Acme"', + 'Research Analyst in Growth Team Bench: Sends an email on the tenant\'s behalf: "Welcome Acme"', + ]); + expect( + requested.filter((path) => path.includes("/runs/run_1")), + ).toHaveLength(1); + }); + + test("a refused list read reads as a failure, never as an empty queue", async () => { + stubFetch((path) => { + if (path === `/api/tenants/${TENANT_ID}/approvals`) { + return json({ error: { code: "forbidden", message: "no" } }, 403); + } + if (path.includes("/insights/activity")) return json({ days: [] }); + return undefined; + }); + + const el = await mount( undefined} />); + + expect(el.textContent).toContain("Couldn't load approvals"); + expect(el.textContent).not.toContain("Nothing waiting on you"); + }); +}); + +describe("chat approve card status read", () => { + function actions() { + return createChatApprovalActions(TENANT_ID, new QueryClient()); + } + + test("a pending approval is actionable, named, and headlined", async () => { + stubFetch((path) => { + if (path === `/api/tenants/${TENANT_ID}/approvals/apr_1`) { + return json(approvalRow()); + } + if (path === `/api/tenants/${TENANT_ID}/runs/run_1`) { + return json({ + id: "run_1", + definitionId: "def_1", + definitionName: "Research Analyst", + tenantId: TENANT_ID, + address: "researcher@growth", + status: "running", + createdAt: "2026-08-20T08:00:00.000Z", + updatedAt: "2026-08-20T08:00:00.000Z", + }); + } + return undefined; + }); + + expect(await actions().getStatus("apr_1")).toEqual({ + kind: "ready", + status: "pending", + canAct: true, + detail: { + agentName: "Research Analyst", + headline: 'Sends an email on the tenant\'s behalf: "Welcome Acme"', + arguments: { title: "Welcome Acme" }, + }, + }); + }); + + test("a refused read is forbidden", async () => { + stubFetch(() => json({ error: { code: "forbidden" } }, 403)); + expect(await actions().getStatus("apr_1")).toEqual({ kind: "forbidden" }); + }); + + test("an unknown approval is not-found", async () => { + stubFetch(() => json({ error: { code: "not_found" } }, 404)); + expect(await actions().getStatus("apr_1")).toEqual({ kind: "not-found" }); + }); +}); diff --git a/apps/web/test/query-client.test.ts b/apps/web/test/query-client.test.ts index f5179df412..6118ebe69e 100644 --- a/apps/web/test/query-client.test.ts +++ b/apps/web/test/query-client.test.ts @@ -12,9 +12,9 @@ describe("pathToQueryKey", () => { expect(pathToQueryKey("/api/me/principals")).toEqual(meKeys.principals); }); - test("maps needs-you onto a tenant-scoped key", () => { - expect(pathToQueryKey("/api/tenants/tnt_1/approvals/needs-you")).toEqual( - tenantKeys.needsYou("tnt_1"), + test("maps the pending-approvals list onto a tenant-scoped key", () => { + expect(pathToQueryKey("/api/tenants/tnt_1/approvals")).toEqual( + tenantKeys.pendingApprovals("tnt_1"), ); }); diff --git a/apps/web/test/sidebar.test.tsx b/apps/web/test/sidebar.test.tsx index 413dc7af2d..fcb5c837f5 100644 --- a/apps/web/test/sidebar.test.tsx +++ b/apps/web/test/sidebar.test.tsx @@ -369,8 +369,8 @@ describe("Sidebar", () => { return Promise.resolve(json({ items: [workbench] })); if (path.includes("/chat/workbenches?kind=chat")) return Promise.resolve(json({ items: [] })); - if (path.includes("/approvals/needs-you")) - return Promise.resolve(json({ items: [] })); + if (path.includes("/approvals")) + return Promise.resolve(json({ data: [], nextCursor: null })); if (path.includes("/top-level-runs")) return Promise.resolve(json({ data: [], nextCursor: null })); if (path.includes("/agent-definitions/visible")) @@ -488,8 +488,8 @@ describe("Sidebar", () => { return Promise.resolve( json({ definitions: [ownAgent, inheritedAgent] }), ); - if (path.includes("/approvals/needs-you")) - return Promise.resolve(json({ items: [] })); + if (path.includes("/approvals")) + return Promise.resolve(json({ data: [], nextCursor: null })); if (path.includes("/top-level-runs")) return Promise.resolve(json({ data: [], nextCursor: null })); return Promise.resolve(json({ items: [] })); @@ -572,8 +572,8 @@ describe("Sidebar", () => { return Promise.resolve(json({ items: [channel] })); if (path.includes("/chat/workbenches?kind=chat")) return Promise.resolve(json({ items: [dm] })); - if (path.includes("/approvals/needs-you")) - return Promise.resolve(json({ items: [] })); + if (path.includes("/approvals")) + return Promise.resolve(json({ data: [], nextCursor: null })); if (path.includes("/top-level-runs")) return Promise.resolve(json({ data: [], nextCursor: null })); if (path.includes("/agent-definitions/visible")) diff --git a/apps/web/test/workbench-list.test.tsx b/apps/web/test/workbench-list.test.tsx index b53e778abc..0dbbe8d05c 100644 --- a/apps/web/test/workbench-list.test.tsx +++ b/apps/web/test/workbench-list.test.tsx @@ -48,7 +48,7 @@ function json(body: unknown): Response { function stubFetch( data: { - readonly needsYou?: readonly unknown[]; + readonly pendingApprovals?: readonly unknown[]; readonly workbenches?: readonly unknown[]; readonly chats?: readonly unknown[]; } = {}, @@ -59,8 +59,10 @@ function stubFetch( return Promise.resolve(json(membership)); if (path.includes("/top-level-runs")) return Promise.resolve(json({ data: [], nextCursor: null })); - if (path.includes("/approvals/needs-you")) - return Promise.resolve(json({ items: data.needsYou ?? [] })); + if (path.includes("/approvals")) + return Promise.resolve( + json({ data: data.pendingApprovals ?? [], nextCursor: null }), + ); if (path.includes("/agent-definitions/visible")) return Promise.resolve(json({ definitions: [] })); if (path.includes("/chat/workbenches?kind=workbench")) @@ -71,15 +73,22 @@ function stubFetch( }) as typeof fetch; } -function needsYouItem(overrides: Record = {}) { +function pendingApproval(overrides: Record = {}) { return { id: "apr_1", - agentName: "Myra", - benchName: "Corbits Bench", - headline: "Merge the checkout fix", - arguments: {}, + tenantId: "tnt_1", + anchorRunId: "run_1", + runId: "run_1", + agentAddress: "agent@bench", + correlationId: "cor_1", + toolDefinition: { name: "merge_pr", description: "Merge the checkout fix" }, + toolArguments: {}, + scope: null, status: "pending", + timeoutAt: null, + resolvedAt: null, createdAt: "2026-08-14T00:00:00.000Z", + updatedAt: "2026-08-14T00:00:00.000Z", ...overrides, }; } @@ -102,15 +111,15 @@ async function mount(onNavigate: (to: string) => void = () => undefined) { await new Promise((resolve) => setTimeout(resolve, 0)); }); // Prefer `shell-ch-row-wrap` over bare `shell-ch-row`: a loading - // skeleton can paint before the parallel needs-you query settles. + // skeleton can paint before the parallel approvals query settles. if ( container.innerHTML.includes("shell-ch-row-wrap") || container.innerHTML.includes("waiting on you") ) { break; } - // Empty mixed list: keep spinning until needs-you resolves (chip text) - // or enough ticks have passed for an empty needs-you to settle as null. + // Empty mixed list: keep spinning until the approvals read resolves + // (chip text) or enough ticks have passed for an empty list to settle. if ( container.innerHTML.includes(SIDEBAR_EMPTY_COPY) && !container.innerHTML.includes("shell-activity-skeleton") && @@ -122,16 +131,16 @@ async function mount(onNavigate: (to: string) => void = () => undefined) { return container; } -describe("WorkbenchList — needs-you signal", () => { +describe("WorkbenchList — pending-approvals signal", () => { test("hides the signal when nothing is pending", async () => { - stubFetch({ needsYou: [] }); + stubFetch({ pendingApprovals: [] }); const el = await mount(); expect(el.textContent).not.toContain("waiting on you"); }); test("shows a filled needs-you chip with the real pending count", async () => { stubFetch({ - needsYou: [needsYouItem(), needsYouItem({ id: "apr_2" })], + pendingApprovals: [pendingApproval(), pendingApproval({ id: "apr_2" })], }); const el = await mount(); expect(el.textContent).toContain("2 waiting on you"); diff --git a/bun.lock b/bun.lock index 9bc93fec96..09ffcdbb6a 100644 --- a/bun.lock +++ b/bun.lock @@ -140,6 +140,7 @@ "dependencies": { "@corbits/agent-directory": "workspace:*", "@corbits/api-query": "workspace:*", + "@corbits/approvals": "workspace:*", "@corbits/artifact-ui": "workspace:*", "@corbits/bench-ui": "workspace:*", "@corbits/chat": "workspace:*", @@ -3438,15 +3439,7 @@ "@babel/helper-compilation-targets/semver": ["semver@6.3.1", "", { "bin": { "semver": "bin/semver.js" } }, "sha512-BR7VvDCVHO+q2xBEWskxS6DJE1qRnb7DxzUrogb71CWoSficBxYsiAGd+Kl0mmq/MprG9yArRkyrQxTO6XjMzA=="], - "@corbits/artifact-ui/@corbits/react-ui": ["@corbits/react-ui@github:corbitsdev/react-ui#3b12281", { "dependencies": { "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "tailwind-merge": "^3.3.1" }, "peerDependencies": { "@radix-ui/react-dialog": "^1.1.15", "@radix-ui/react-dropdown-menu": "^2.1.16", "@radix-ui/react-slot": "^1.2.3", "@radix-ui/react-tooltip": "^1.2.8", "@tanstack/react-query": "^5.90.2", "lucide-react": "^0.545.0 || ^1.0.0", "react": "^18.2.0 || ^19.0.0", "react-dom": "^18.2.0 || ^19.0.0", "sonner": "^2.0.7" }, "optionalPeers": ["@tanstack/react-query"] }, "corbitsdev-react-ui-3b12281", "sha512-Abvm/DO0Gqg0ITHGT9355ZxyKRPMVJLSSQSjpd3a8qt4JPrSMOLIOS4sX8ZMNNaArIbnY9F+VKrOWkUJUyO4Nw=="], - - "@corbits/chat-ui/@corbits/react-ui": ["@corbits/react-ui@github:corbitsdev/react-ui#3b12281", { "dependencies": { "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "tailwind-merge": "^3.3.1" }, "peerDependencies": { "@radix-ui/react-dialog": "^1.1.15", "@radix-ui/react-dropdown-menu": "^2.1.16", "@radix-ui/react-slot": "^1.2.3", "@radix-ui/react-tooltip": "^1.2.8", "@tanstack/react-query": "^5.90.2", "lucide-react": "^0.545.0 || ^1.0.0", "react": "^18.2.0 || ^19.0.0", "react-dom": "^18.2.0 || ^19.0.0", "sonner": "^2.0.7" }, "optionalPeers": ["@tanstack/react-query"] }, "corbitsdev-react-ui-3b12281", "sha512-Abvm/DO0Gqg0ITHGT9355ZxyKRPMVJLSSQSjpd3a8qt4JPrSMOLIOS4sX8ZMNNaArIbnY9F+VKrOWkUJUyO4Nw=="], - - "@corbits/context-menu/@corbits/react-ui": ["@corbits/react-ui@github:corbitsdev/react-ui#3b12281", { "dependencies": { "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "tailwind-merge": "^3.3.1" }, "peerDependencies": { "@radix-ui/react-dialog": "^1.1.15", "@radix-ui/react-dropdown-menu": "^2.1.16", "@radix-ui/react-slot": "^1.2.3", "@radix-ui/react-tooltip": "^1.2.8", "@tanstack/react-query": "^5.90.2", "lucide-react": "^0.545.0 || ^1.0.0", "react": "^18.2.0 || ^19.0.0", "react-dom": "^18.2.0 || ^19.0.0", "sonner": "^2.0.7" }, "optionalPeers": ["@tanstack/react-query"] }, "corbitsdev-react-ui-3b12281", "sha512-Abvm/DO0Gqg0ITHGT9355ZxyKRPMVJLSSQSjpd3a8qt4JPrSMOLIOS4sX8ZMNNaArIbnY9F+VKrOWkUJUyO4Nw=="], - - "@corbits/plugins-ui/@corbits/react-ui": ["@corbits/react-ui@github:corbitsdev/react-ui#3b12281", { "dependencies": { "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "tailwind-merge": "^3.3.1" }, "peerDependencies": { "@radix-ui/react-dialog": "^1.1.15", "@radix-ui/react-dropdown-menu": "^2.1.16", "@radix-ui/react-slot": "^1.2.3", "@radix-ui/react-tooltip": "^1.2.8", "@tanstack/react-query": "^5.90.2", "lucide-react": "^0.545.0 || ^1.0.0", "react": "^18.2.0 || ^19.0.0", "react-dom": "^18.2.0 || ^19.0.0", "sonner": "^2.0.7" }, "optionalPeers": ["@tanstack/react-query"] }, "corbitsdev-react-ui-3b12281", "sha512-Abvm/DO0Gqg0ITHGT9355ZxyKRPMVJLSSQSjpd3a8qt4JPrSMOLIOS4sX8ZMNNaArIbnY9F+VKrOWkUJUyO4Nw=="], - - "@corbits/settings-ui/@corbits/react-ui": ["@corbits/react-ui@github:corbitsdev/react-ui#3b12281", { "dependencies": { "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "tailwind-merge": "^3.3.1" }, "peerDependencies": { "@radix-ui/react-dialog": "^1.1.15", "@radix-ui/react-dropdown-menu": "^2.1.16", "@radix-ui/react-slot": "^1.2.3", "@radix-ui/react-tooltip": "^1.2.8", "@tanstack/react-query": "^5.90.2", "lucide-react": "^0.545.0 || ^1.0.0", "react": "^18.2.0 || ^19.0.0", "react-dom": "^18.2.0 || ^19.0.0", "sonner": "^2.0.7" }, "optionalPeers": ["@tanstack/react-query"] }, "corbitsdev-react-ui-3b12281", "sha512-Abvm/DO0Gqg0ITHGT9355ZxyKRPMVJLSSQSjpd3a8qt4JPrSMOLIOS4sX8ZMNNaArIbnY9F+VKrOWkUJUyO4Nw=="], + "@corbits/memory-hub/@corbits/memory": ["@corbits/memory@github:corbitsdev/corbits-memory#9e6f213", { "dependencies": { "@intx/agent": "0.2.2", "@intx/authz": "0.2.2", "@intx/hub-api": "0.2.2", "@intx/log": "0.2.2", "@intx/workflow": "0.2.2", "arktype": "^2.1.29", "drizzle-orm": "^0.45.1", "hono": "^4.9.0", "hono-openapi": "^1.3.1", "postgres": "^3.4.7" } }, "corbitsdev-corbits-memory-9e6f213", "sha512-utnM4ZT2zmslcPXYWAAqxlDNLcpGsXFiTOtj8h7+OXnhCP0Eaw8yl25+yCTyHpvt3jcdeG4h5uFsSj7ou0BZCA=="], "@esbuild-kit/core-utils/esbuild": ["esbuild@0.18.20", "", { "optionalDependencies": { "@esbuild/android-arm": "0.18.20", "@esbuild/android-arm64": "0.18.20", "@esbuild/android-x64": "0.18.20", "@esbuild/darwin-arm64": "0.18.20", "@esbuild/darwin-x64": "0.18.20", "@esbuild/freebsd-arm64": "0.18.20", "@esbuild/freebsd-x64": "0.18.20", "@esbuild/linux-arm": "0.18.20", "@esbuild/linux-arm64": "0.18.20", "@esbuild/linux-ia32": "0.18.20", "@esbuild/linux-loong64": "0.18.20", "@esbuild/linux-mips64el": "0.18.20", "@esbuild/linux-ppc64": "0.18.20", "@esbuild/linux-riscv64": "0.18.20", "@esbuild/linux-s390x": "0.18.20", "@esbuild/linux-x64": "0.18.20", "@esbuild/netbsd-x64": "0.18.20", "@esbuild/openbsd-x64": "0.18.20", "@esbuild/sunos-x64": "0.18.20", "@esbuild/win32-arm64": "0.18.20", "@esbuild/win32-ia32": "0.18.20", "@esbuild/win32-x64": "0.18.20" }, "bin": { "esbuild": "bin/esbuild" } }, "sha512-ceqxoedUrcayh7Y7ZX6NdbbDzGROiyVBgC4PriJThBKSVPWnnFHZAkfI1lJT8QFkOwH4qOS2SJkS4wvpGl8BpA=="], @@ -3470,7 +3463,9 @@ "@typescript-eslint/eslint-plugin/ignore": ["ignore@7.0.6", "", {}, "sha512-BAg6QkE8W+TuQLrrw0Ugr7HegXduRuuj8/ti2kSOc+jz1dmx8/WNcjr6XGnq5YpDWxFwwaavqD0+jIUOKelTsw=="], - "@workbench/web/@corbits/react-ui": ["@corbits/react-ui@github:corbitsdev/react-ui#3b12281", { "dependencies": { "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "tailwind-merge": "^3.3.1" }, "peerDependencies": { "@radix-ui/react-dialog": "^1.1.15", "@radix-ui/react-dropdown-menu": "^2.1.16", "@radix-ui/react-slot": "^1.2.3", "@radix-ui/react-tooltip": "^1.2.8", "@tanstack/react-query": "^5.90.2", "lucide-react": "^0.545.0 || ^1.0.0", "react": "^18.2.0 || ^19.0.0", "react-dom": "^18.2.0 || ^19.0.0", "sonner": "^2.0.7" }, "optionalPeers": ["@tanstack/react-query"] }, "corbitsdev-react-ui-3b12281", "sha512-Abvm/DO0Gqg0ITHGT9355ZxyKRPMVJLSSQSjpd3a8qt4JPrSMOLIOS4sX8ZMNNaArIbnY9F+VKrOWkUJUyO4Nw=="], + "@workbench/hub/@corbits/mailbox": ["@corbits/mailbox@github:corbitsdev/corbits-mailbox#caa5214", { "dependencies": { "@hono/standard-validator": "0.2.3", "@standard-community/standard-json": "0.3.5", "@standard-community/standard-openapi": "0.2.9", "arktype": "2.1.29", "hono-openapi": "1.3.1" }, "peerDependencies": { "@intx/log": "^0.2.2", "@intx/mime": "^0.2.2", "@intx/types": "^0.2.2", "drizzle-orm": "^0.45.2", "hono": "^4.12.0", "postgres": "^3.4.0" } }, "corbitsdev-corbits-mailbox-caa5214", "sha512-z8DRBFgA4ukM8p29COeaMjfKZYe5jAUF4OBMiaIQFuW592+DGD/y6Ws6SjGlXmR9azkHNWh8oTzjlWlRP24vsQ=="], + + "@workbench/hub/@corbits/memory": ["@corbits/memory@github:corbitsdev/corbits-memory#9e6f213", { "dependencies": { "@intx/agent": "0.2.2", "@intx/authz": "0.2.2", "@intx/hub-api": "0.2.2", "@intx/log": "0.2.2", "@intx/workflow": "0.2.2", "arktype": "^2.1.29", "drizzle-orm": "^0.45.1", "hono": "^4.9.0", "hono-openapi": "^1.3.1", "postgres": "^3.4.7" } }, "corbitsdev-corbits-memory-9e6f213", "sha512-utnM4ZT2zmslcPXYWAAqxlDNLcpGsXFiTOtj8h7+OXnhCP0Eaw8yl25+yCTyHpvt3jcdeG4h5uFsSj7ou0BZCA=="], "ajv-formats/ajv": ["ajv@8.20.0", "", { "dependencies": { "fast-deep-equal": "^3.1.3", "fast-uri": "^3.0.1", "json-schema-traverse": "^1.0.0", "require-from-string": "^2.0.2" } }, "sha512-Thbli+OlOj+iMPYFBVBfJ3OmCAnaSyNn4M1vz9T6Gka5Jt9ba/HIR56joy65tY6kx/FCF5VXNB819Y7/GUrBGA=="], diff --git a/docs/needs-you.md b/docs/needs-you.md index a6e04c2ff2..f803af6ba2 100644 --- a/docs/needs-you.md +++ b/docs/needs-you.md @@ -3,73 +3,86 @@ Approval is "needs you." When a running workflow parks on a human decision, that pause is Interchange's own native state — an `approval` row backed by a `signal_correlation` row, produced by `AwaitSignalPrimitive` — not a second -concept this repo invents alongside it. This document describes the one -piece Interchange's approval machinery doesn't carry on its own: turning a -pending approval into something a person can actually read. +concept this repo invents alongside it. This document describes how a +pending approval becomes something a person can read and act on, entirely +through the platform's own routes. ## What's native, unchanged -Everything about deciding an approval is Interchange's, consumed as-is: - -- `GET /api/tenants/:tenantId/approvals` lists a tenant's pending approvals. -- `POST /api/tenants/:tenantId/approvals/:id/approve` and `.../reject` - resolve one. Both run inside a single transaction that claims the - approval's `signal_correlation` row under a `resolved_at IS NULL` guard - and only then flips the approval's status — so two concurrent decisions - (two clicks, two people) can never both land: the second finds nothing - left to claim and is told the approval is already resolved. -- Every resolve is authorized against the same grant an approver has always - needed: `approval:` (or the tenant-wide `approval:*`), - checked server-side before the transaction runs, never left to the - interface to hide a button. - -None of that changed. `@corbits/approvals` never creates, claims, or -resolves an approval — it only reads. - -## What's new: resolving names, not ids - -An `approval` row only carries what it needs to authorize and resume: a -`deploymentId`, a `runId`, an `agentAddress` with an instance id baked into -it. None of that is something a person should have to read. `GET -/api/tenants/:tenantId/approvals/needs-you` (`@corbits/approvals`, -`packages/approvals`) reads the same pending rows the native list route -does and resolves each one's real name before it ever reaches a client: - -- `runId -> workflow_run.definitionId -> workflow_definition.name` for - which agent is asking. -- `tenantId -> tenant.name` for which bench the ask is in. - -The result is a small, display-only view model — an agent name, a bench -name, the tool's headline, its arguments, nothing else — gated by the exact -same `approval:*` / `resolve` grant the native list route requires. A -principal without that grant gets a `403`, not an empty list dressed up as -"nothing pending." +Everything about reading and deciding an approval is Interchange's, +consumed as-is: + +- `GET /api/tenants/:tenantId/approvals` lists a tenant's pending + approvals; each row carries the tool snapshot, its arguments, the run it + came from, and a status. +- `GET /api/tenants/:tenantId/approvals/:approvalId` reads one, in any + status, so a card can render a terminal state after the fact. It is + gated on the same grant approve and reject require, so a refusal here is + proof the viewer cannot resolve it; a foreign id reads as `404`, never a + `403` that would confirm it exists. +- `POST /api/tenants/:tenantId/approvals/:approvalId/approve` and + `.../reject` resolve one. Both run inside a single transaction that + claims the approval's `signal_correlation` row under a `resolved_at IS +NULL` guard and only then flips the approval's status — so two + concurrent decisions (two clicks, two people) can never both land: the + second finds nothing left to claim and is told the approval is already + resolved. +- `GET /api/tenants/:tenantId/runs/:runId` returns the run view, whose + `definitionName` is the display name of the agent that is asking. + +There is no sibling read on the hub. Nothing in this repo creates, claims, +or resolves an approval. + +## Names, not ids + +An `approval` row carries only what it needs to authorize and resume: a +`runId`, an `agentAddress` with an instance id baked into it. None of that +is something a person should have to read. `apps/web/src/pending-approvals.ts` +is the one composer that turns those rows into the display model every +approval surface renders: + +- the run view's `definitionName`, read once per distinct run and cached, is + which agent is asking; +- the account's own membership (`GET /api/me/principals` → `tenantName`) is + which bench the ask is in; +- `headlineFor` (`@corbits/approvals/headline`) turns the tool snapshot into + the line a person reads — the tool's own description, plus the call's + `title` argument when it carries one. + +Not one field on that model holds an identifier. When the naming read is +refused — a per-deployment approver can resolve an approval without holding +the read grant on its run — the approval is still shown, with the asker +unnamed: a blocking decision must never be hidden because its label was +unavailable. ## Where it shows up -There is no "Approvals" nav row and no dedicated Approvals page. Needs-you -approvals surface through the Activity band (`ActivityBand`, -`apps/web/src/shell/activity-band.tsx`) — a permanent section of the -contextual panel shown on every page, the same slot pins uses, not something -scoped to one route. It reads this same `needs-you` endpoint, renders each -pending request as "`` in ``" through -`ApprovalCard` (never a raw agent address or run id), and posts approve/ -reject straight to Interchange's native -`/api/tenants/:tenantId/approvals/:id/{approve,reject}` routes — this -package only ever supplies the names. Approve only offers scope "once" (the -hub rejects "always" with a 400); reject collects an optional message -through a confirm dialog. - -The band's heading carries a live count as a `Badge` next to "Activity" when -there is anything pending, and the whole band hides — no hollow empty state -— once it resolves to zero. It stays mounted while loading, or once items -arrive, so a pending approval can be resolved without leaving whatever page -the user is on. +There is no "Approvals" nav row and no dedicated Approvals page. Pending +approvals surface in three places, all reading the composer above: + +- the shell's top bar chip ("N waiting on you", or "All caught up") and the + second column's signal above the conversation list; +- Mission Control's "Needs you" panel, which approves or denies inline and + invalidates the same query key the chat card does, so both update at once; +- a workbench's Timeline, where an approval takes its place on the + wall-clock spine. + +The in-chat approve card (`@corbits/chat-ui`) reads one approval's live +status through the same composer and posts approve/reject straight to the +native routes. Approve only offers scope "once" (the hub rejects "always" +with a 400); reject collects an optional message through a confirm dialog. + +A refused list read is shown as a failure to load, never as an empty +"nothing pending" — a person who cannot see the queue is told so. ## What this deliberately does not add No new "gate" table, no parallel resolution path, no reimplementation of -`claimTerminal` or the approve/reject transaction. Mail-based delivery -(notifying a human's inbox the moment an approval is created, rather than -this page polling for it) and reply-to-resume are real, valuable follow-ups -that build on this same `approval` row — neither is part of this change. +`claimTerminal` or the approve/reject transaction, and no read route +mirroring one the platform already serves. `@corbits/approvals` keeps only +what the platform has no reason to own: the grant-allowance gate that +auto-approves a call an existing grant already covers, and the pure +headline builder. Mail-based delivery (notifying a human's inbox the moment +an approval is created, rather than a page polling for it) and +reply-to-resume are real, valuable follow-ups that build on this same +`approval` row — neither is part of this change. diff --git a/packages/approvals/README.md b/packages/approvals/README.md index 114a3350e1..bb9796df45 100644 --- a/packages/approvals/README.md +++ b/packages/approvals/README.md @@ -1,34 +1,30 @@ # @corbits/approvals -Resolves a tenant's pending approvals — Interchange's own "needs you" -state — into a display-safe view model, and exposes it as a Hono route -factory the hub mounts alongside Interchange's native approve/reject -routes. This package never creates, resolves, or claims anything: -approving and rejecting stay on Interchange's own -`/api/tenants/:tenantId/approvals/:approvalId/{approve,reject}` routes, -whose authorize + claimTerminal + resolve transaction is already -exactly-once and grant-scoped. The one net-new concept here is naming — -turning raw approval rows into something safe to render. +Decides when a tenant's standing grants already authorize a tool call, so +an agent is only stopped for a decision a human has not already made — and +names an approval in words a person can act on. This package never creates, +resolves, or claims anything: listing, reading, approving and rejecting all +live on Interchange's own +`/api/tenants/:tenantId/approvals` routes, whose authorize + +claimTerminal + resolve transaction is already exactly-once and +grant-scoped. ## Composition over Interchange -- Reads through `@intx/db`'s `schema` and `parseApprovalRow`, and - authorizes with `@intx/authz`'s `authorize` against the same grant and - action (`approval:*` / `resolve`) the native list/resolve routes - require, so the two surfaces never drift apart. -- Adds no parallel approval store or state machine — every identifier on - an approval row is resolved into a name before `hydrateNeedsYou` (the - view model's only producer) ever constructs a result, so nothing - downstream can render a raw id even by accident. +- The allowance gate evaluates the tenant's own grant rules through + `@intx/authz`'s `evaluateGrants` — no parallel policy engine, and no + approval store or state machine of its own. +- Names and statuses are read straight off Interchange's native approval + and run views by whoever renders them; nothing here mirrors those rows. ## Key modules -- `routes.ts` — `createNeedsYouRoutes`: the tenant-scoped "needs you" - list/detail surface, grant-checked before it queries. -- `view-model.ts` — `hydrateNeedsYou`, `headlineFor`, and the - `NeedsYouItem` shape approvals hydrate into; carries the full - `approval.status` union so a detail read can render - approved/rejected/etc., not just pending. +- `allowance.ts` — `createToolAllowanceRegistry`, `evaluateToolAllowance`, + `withGrantAllowance`: the gate that auto-approves a call an existing + grant already covers. +- `headline.ts` — `headlineFor`: the pure headline builder, exported at + `@corbits/approvals/headline` for browser callers (the root entry reaches + server-only dependencies). - `index.ts` — package entry point. ## Tests @@ -36,6 +32,3 @@ turning raw approval rows into something safe to render. ``` cd packages/approvals && bun test ``` - -`test/needs-you.test.ts` needs a real database: -`DATABASE_URL=postgres://localhost:5432/workbench_e2e bun test`. diff --git a/packages/approvals/package.json b/packages/approvals/package.json index 22cc0c2d3f..060cc50a09 100644 --- a/packages/approvals/package.json +++ b/packages/approvals/package.json @@ -1,12 +1,13 @@ { "name": "@corbits/approvals", "private": true, - "description": "Resolves the tenant's pending approvals (Interchange's own \"needs you\" state) into a display-safe view model, and exposes it as a Hono route factory the hub mounts alongside Interchange's native approve/reject routes", + "description": "The grant-allowance gate that decides when a tenant's standing grants already authorize a tool call, plus the pure headline builder that names an approval for a human", "version": "0.0.1", "license": "LGPL-2.1-or-later", "type": "module", "exports": { - ".": "./src/index.ts" + ".": "./src/index.ts", + "./headline": "./src/headline.ts" }, "scripts": { "typecheck": "tsc --noEmit", diff --git a/packages/approvals/src/view-model.test.ts b/packages/approvals/src/headline.test.ts similarity index 97% rename from packages/approvals/src/view-model.test.ts rename to packages/approvals/src/headline.test.ts index c6e00d61d1..6a7aebd913 100644 --- a/packages/approvals/src/view-model.test.ts +++ b/packages/approvals/src/headline.test.ts @@ -1,6 +1,6 @@ import { expect, test } from "bun:test"; -import { headlineFor } from "./view-model"; +import { headlineFor } from "./headline"; test("falls back to a generic label when the tool definition carries neither", () => { expect(headlineFor({}, {})).toBe("Run a tool"); diff --git a/packages/approvals/src/headline.ts b/packages/approvals/src/headline.ts new file mode 100644 index 0000000000..48d0cc3e8d --- /dev/null +++ b/packages/approvals/src/headline.ts @@ -0,0 +1,38 @@ +// The one display concern this package owns: the human-readable line an +// approval is described by. Pure string work over the tool snapshot an +// approval already carries, with no database or server dependency, so the +// browser can compose the same headline the hub-side tools do — hence its +// own module and `./headline` export rather than living beside the +// grant-allowance gate. + +function stringField(source: object, field: string): string | undefined { + if (!(field in source)) return undefined; + const value = (source as Record)[field]; + return typeof value === "string" && value.trim() !== "" ? value : undefined; +} + +/** + * Builds the headline for an approval. Prefers the tool's own + * `description` — written by the tool's author to be human-readable — + * over its bare `name`, which is a machine identifier. When the live + * call's arguments carry a `title` (a tool author's own convention for + * per-invocation context, e.g. "finalize this piece of collateral titled + * X"), it is appended so the headline reflects what THIS approval is + * actually about, not just which tool is asking. + */ +export function headlineFor( + toolDefinition: unknown, + toolArguments: unknown, +): string { + const base = + typeof toolDefinition === "object" && toolDefinition !== null + ? (stringField(toolDefinition, "description") ?? + stringField(toolDefinition, "name")) + : undefined; + const headline = base ?? "Run a tool"; + const title = + typeof toolArguments === "object" && toolArguments !== null + ? stringField(toolArguments, "title") + : undefined; + return title === undefined ? headline : `${headline}: "${title}"`; +} diff --git a/packages/approvals/src/index.ts b/packages/approvals/src/index.ts index ba830a0846..6dd924f486 100644 --- a/packages/approvals/src/index.ts +++ b/packages/approvals/src/index.ts @@ -11,6 +11,4 @@ export type { ToolAllowance, ToolAllowanceRegistry, } from "./allowance"; -export { createNeedsYouRoutes } from "./routes"; -export type { CreateNeedsYouRoutesDeps } from "./routes"; -export { NeedsYouItem, hydrateNeedsYou, headlineFor } from "./view-model"; +export { headlineFor } from "./headline"; diff --git a/packages/approvals/src/routes.ts b/packages/approvals/src/routes.ts deleted file mode 100644 index 8958501c18..0000000000 --- a/packages/approvals/src/routes.ts +++ /dev/null @@ -1,138 +0,0 @@ -import { and, eq } from "drizzle-orm"; -import { Hono } from "hono"; - -import { authorize } from "@intx/authz"; -import type { DB } from "@intx/db"; -import { schema, parseApprovalRow } from "@intx/db"; -import type { ConditionRegistry, GrantStore } from "@intx/types/authz"; -import type { TenantEnv } from "@intx/hub-api"; - -import { hydrateNeedsYou } from "./view-model"; - -export type CreateNeedsYouRoutesDeps = { - db: DB["db"]; - grantStore: GrantStore; - conditionRegistry: ConditionRegistry; -}; - -/** - * The one net-new domain concept this package adds: a display-ready read of - * "what needs this tenant's attention right now." It never creates, resolves, - * or claims anything -- approving and rejecting stay on Interchange's own - * `/api/tenants/:tenantId/approvals/:approvalId/{approve,reject}` routes, - * whose authorize + claimTerminal + approvalStore.resolve transaction is - * already exactly-once and already grant-scoped. Reimplementing that - * machinery here would be the parallel gate concept the design explicitly - * rejects; this route only adds the naming layer that machinery has no - * reason to own. - */ -export function createNeedsYouRoutes( - deps: CreateNeedsYouRoutesDeps, -): Hono { - const app = new Hono(); - - app.get("/", async (c) => { - const tenant = c.get("tenant"); - const principal = c.get("principal"); - - // Same grant and action the native list/resolve routes require -- - // whoever can resolve a tenant's approvals is who "needs you" is for. - // Reusing this string keeps the two surfaces from drifting apart. - const authz = await authorize( - deps.grantStore, - principal.id, - tenant.id, - "approval:*", - "resolve", - deps.conditionRegistry, - ); - if (authz.effect !== "allow") { - return c.json( - { - error: { - code: "forbidden", - message: "You do not have permission to see this bench's approvals", - }, - }, - 403, - ); - } - - const rows = await deps.db.query.approval.findMany({ - where: and( - eq(schema.approval.tenantId, tenant.id), - eq(schema.approval.status, "pending"), - ), - orderBy: (row, { asc }) => [asc(row.createdAt)], - }); - - const items = await hydrateNeedsYou(deps.db, rows.map(parseApprovalRow)); - return c.json({ items }); - }); - - // A single approval's display-safe status, in any status (not just - // pending) -- an in-chat approve card's live read. Gated by the same - // tenant-wide `approval:*`/"resolve" grant as the list above, which is - // deliberately coarser than the native routes' per-deployment - // `approval:`/"resolve" check: a principal scoped to one - // deployment's approvals can still resolve them there even after a 403 - // here. Callers must treat that 403 as "could not determine," never as - // "cannot act" -- see `packages/chat-ui/src/blocks/approve-card-state.ts`. - // - // Authorization runs before the existence lookup -- deliberately unlike - // the native detail route's not-found-masks-cross-tenant precedent (that - // route's grant is per-approval, so it must load the row first to know - // which deployment to check). This grant is tenant-wide and independent - // of the target id, so checking it first means an unauthorized caller - // always sees 403, never learning from a 404-vs-403 split whether some - // id exists in this tenant. - app.get("/:approvalId", async (c) => { - const tenant = c.get("tenant"); - const principal = c.get("principal"); - const approvalId = c.req.param("approvalId"); - - const authz = await authorize( - deps.grantStore, - principal.id, - tenant.id, - "approval:*", - "resolve", - deps.conditionRegistry, - ); - if (authz.effect !== "allow") { - return c.json( - { - error: { - code: "forbidden", - message: "You do not have permission to see this approval", - }, - }, - 403, - ); - } - - const row = await deps.db.query.approval.findFirst({ - where: and( - eq(schema.approval.id, approvalId), - eq(schema.approval.tenantId, tenant.id), - ), - }); - if (row === undefined) { - return c.json( - { error: { code: "not_found", message: "Approval not found" } }, - 404, - ); - } - - const [item] = await hydrateNeedsYou(deps.db, [parseApprovalRow(row)]); - if (item === undefined) { - return c.json( - { error: { code: "not_found", message: "Approval not found" } }, - 404, - ); - } - return c.json(item); - }); - - return app; -} diff --git a/packages/approvals/src/view-model.ts b/packages/approvals/src/view-model.ts deleted file mode 100644 index 2d161fe5c9..0000000000 --- a/packages/approvals/src/view-model.ts +++ /dev/null @@ -1,121 +0,0 @@ -import { inArray } from "drizzle-orm"; -import { type } from "arktype"; - -import type { DBExecutor } from "@intx/db"; -import { schema, parseApprovalRow } from "@intx/db"; - -type ApprovalRow = ReturnType; - -// The shape a human decides against. Every identifier the underlying -// `approval` row carries (agentAddress, deploymentId, tenantId) is resolved -// here into a name before this type's only two producers -- `hydrateNeedsYou` -// and its tests -- ever construct one, so nothing downstream can render a raw -// id even by accident: there is no field on this type that holds one. -// -// `status` carries the full `approval.status` union (not just `"pending"`): -// the list route (`GET /`) only ever queries pending rows, so its items are -// always `"pending"` in practice, but the single-item detail route -// (`GET /:approvalId`) hydrates a row in any status so a caller that already -// knows the id -- an in-chat approve card re-reading after resolve -- can -// render "approved"/"rejected"/etc. through the same display-safe shape. -export const NeedsYouItem = type({ - id: "string", - agentName: "string", - benchName: "string", - headline: "string", - arguments: "object", - status: "'pending' | 'approved' | 'rejected' | 'timeout' | 'expired'", - createdAt: "string.date.iso", -}); -export type NeedsYouItem = typeof NeedsYouItem.infer; - -function stringField(source: object, field: string): string | undefined { - if (!(field in source)) return undefined; - const value = (source as Record)[field]; - return typeof value === "string" && value.trim() !== "" ? value : undefined; -} - -/** - * Builds the inbox headline for a pending approval. Prefers the tool's - * own `description` — written by the tool's author to be human-readable - * — over its bare `name`, which is a machine identifier. When the live - * call's arguments carry a `title` (a tool author's own convention for - * per-invocation context, e.g. "finalize this piece of collateral - * titled X"), it is appended so the headline reflects what THIS - * approval is actually about, not just which tool is asking. - */ -export function headlineFor( - toolDefinition: unknown, - toolArguments: unknown, -): string { - const base = - typeof toolDefinition === "object" && toolDefinition !== null - ? (stringField(toolDefinition, "description") ?? - stringField(toolDefinition, "name")) - : undefined; - const headline = base ?? "Run a tool"; - const title = - typeof toolArguments === "object" && toolArguments !== null - ? stringField(toolArguments, "title") - : undefined; - return title === undefined ? headline : `${headline}: "${title}"`; -} - -/** - * Resolves a page of pending `approval` rows into the display-safe view - * model a human approves or rejects. Every name is looked up through the - * definition/tenant the approval's own foreign keys already point at -- - * `workflow_run.definitionId -> workflow_definition.name` for "which agent - * is asking" and `approval.tenantId -> tenant.name` for "in which bench" -- - * so no new naming concept is introduced, only a read of names that already - * exist on the rows the approval is anchored to. - */ -export async function hydrateNeedsYou( - db: DBExecutor, - approvals: readonly ApprovalRow[], -): Promise { - if (approvals.length === 0) return []; - - const tenantIds = [...new Set(approvals.map((row) => row.tenantId))]; - const runIds = [...new Set(approvals.map((row) => row.runId))]; - - const [tenants, runs] = await Promise.all([ - db.query.tenant.findMany({ where: inArray(schema.tenant.id, tenantIds) }), - db.query.workflowRun.findMany({ - where: inArray(schema.workflowRun.id, runIds), - }), - ]); - - const tenantNameById = new Map(tenants.map((row) => [row.id, row.name])); - const definitionIdByRunId = new Map( - runs.map((row) => [row.id, row.definitionId]), - ); - const definitionIds = [...new Set(definitionIdByRunId.values())]; - const definitions = - definitionIds.length === 0 - ? [] - : await db.query.workflowDefinition.findMany({ - where: inArray(schema.workflowDefinition.id, definitionIds), - }); - const definitionNameById = new Map( - definitions.map((row) => [row.id, row.name]), - ); - - return approvals.map((row) => { - const definitionId = definitionIdByRunId.get(row.runId); - const agentName = - (definitionId !== undefined - ? definitionNameById.get(definitionId) - : undefined) ?? "An agent"; - const benchName = tenantNameById.get(row.tenantId) ?? "A bench"; - return { - id: row.id, - agentName, - benchName, - headline: headlineFor(row.toolDefinition, row.toolArguments), - arguments: row.toolArguments as object, - status: row.status, - createdAt: row.createdAt.toISOString(), - }; - }); -} diff --git a/packages/approvals/test/needs-you.test.ts b/packages/approvals/test/needs-you.test.ts deleted file mode 100644 index 1e1b0efdbf..0000000000 --- a/packages/approvals/test/needs-you.test.ts +++ /dev/null @@ -1,315 +0,0 @@ -// DB-gated: skipped when DATABASE_URL is unreachable, matching this repo's -// existing convention for tests that talk to a real Postgres (see -// packages/chat/test/migrations.test.ts). Runs against the caller's own -// database and deletes every row it wrote in afterAll, since the tables it -// touches (tenant, workflow_definition, workflow_run, approval) are shared -// platform tables this suite must not leave dirty for anything else that -// reads them. -import { afterAll, beforeAll, describe, expect, test } from "bun:test"; -import { eq } from "drizzle-orm"; -import { Hono } from "hono"; -import type { Context, Next } from "hono"; - -import { createDB, schema, parseApprovalRow, type DB } from "@intx/db"; -import { createInMemoryGrantStore } from "@intx/authz"; -import type { TenantEnv, TenantRow, PrincipalRow } from "@intx/hub-api"; -import { generateId } from "@intx/hub-common"; - -import { createNeedsYouRoutes } from "../src/routes"; -import { hydrateNeedsYou } from "../src/view-model"; - -// Parse DATABASE_URL the same way the hub does (apps/hub/src/index.ts): -// an empty user falls through to the postgres client's OS-username default. -// Do not default to "postgres" — that role often does not exist on laptop -// installs that use peer auth as $USER. -function dbConfigFromUrl(databaseUrl: string) { - const url = new URL(databaseUrl); - return { - host: url.hostname, - port: url.port === "" ? 5432 : Number(url.port), - user: decodeURIComponent(url.username), - password: decodeURIComponent(url.password), - database: url.pathname.replace(/^\//, ""), - }; -} - -const databaseUrl = process.env["DATABASE_URL"]; -const describeIfDb = databaseUrl === undefined ? describe.skip : describe; - -describeIfDb("needs-you: resolving pending approvals to display labels", () => { - let db: DB; - - const tenantId = generateId("tenant"); - const otherTenantId = generateId("tenant"); - const definitionId = `def_${generateId("workflowRun")}`; - const runId = `run_${generateId("workflowRun")}`; - const approvalId = generateId("approval"); - const resolvedApprovalId = generateId("approval"); - const otherTenantApprovalId = generateId("approval"); - const approverPrincipalId = generateId("principal"); - const strangerPrincipalId = generateId("principal"); - - const tenantRow: TenantRow = { - id: tenantId, - name: "Growth Team Bench", - slug: `growth-${tenantId}`, - domain: `growth-${tenantId}.localhost`, - parentId: null, - config: null, - createdAt: new Date(), - updatedAt: new Date(), - }; - - beforeAll(async () => { - db = createDB(dbConfigFromUrl(databaseUrl as string)); - - await db.db.insert(schema.tenant).values({ - id: tenantId, - name: "Growth Team Bench", - slug: `growth-${tenantId}`, - domain: `growth-${tenantId}.localhost`, - }); - await db.db.insert(schema.tenant).values({ - id: otherTenantId, - name: "Unrelated Bench", - slug: `unrelated-${otherTenantId}`, - domain: `unrelated-${otherTenantId}.localhost`, - }); - await db.db.insert(schema.workflowDefinition).values({ - id: definitionId, - tenantId, - name: "Outreach Composer", - status: "deployed", - }); - await db.db.insert(schema.workflowRun).values({ - id: runId, - definitionId, - tenantId, - status: "running", - }); - await db.db.insert(schema.approval).values({ - id: approvalId, - tenantId, - anchorRunId: runId, - runId, - agentAddress: `instance_abc123@growth-${tenantId}.localhost`, - correlationId: `cor_${generateId("signal")}`, - toolDefinition: { name: "send_email" }, - toolArguments: { to: "customer@example.com" }, - status: "pending", - }); - await db.db.insert(schema.approval).values({ - id: resolvedApprovalId, - tenantId, - anchorRunId: runId, - runId, - agentAddress: `instance_abc123@growth-${tenantId}.localhost`, - correlationId: `cor_${generateId("signal")}`, - toolDefinition: { name: "send_email" }, - toolArguments: { to: "customer@example.com" }, - status: "approved", - resolvedAt: new Date(), - }); - await db.db.insert(schema.approval).values({ - id: otherTenantApprovalId, - tenantId: otherTenantId, - anchorRunId: runId, - runId, - agentAddress: `instance_abc123@growth-${tenantId}.localhost`, - correlationId: `cor_${generateId("signal")}`, - toolDefinition: { name: "send_email" }, - toolArguments: {}, - status: "pending", - }); - }); - - afterAll(async () => { - await db.db - .delete(schema.approval) - .where(eq(schema.approval.id, approvalId)); - await db.db - .delete(schema.approval) - .where(eq(schema.approval.id, resolvedApprovalId)); - await db.db - .delete(schema.approval) - .where(eq(schema.approval.id, otherTenantApprovalId)); - await db.db - .delete(schema.workflowRun) - .where(eq(schema.workflowRun.id, runId)); - await db.db - .delete(schema.workflowDefinition) - .where(eq(schema.workflowDefinition.id, definitionId)); - await db.db.delete(schema.tenant).where(eq(schema.tenant.id, tenantId)); - await db.db - .delete(schema.tenant) - .where(eq(schema.tenant.id, otherTenantId)); - await db.close(); - }); - - test("hydrateNeedsYou resolves the agent and bench names off the approval's own foreign keys", async () => { - const row = await db.db.query.approval.findFirst({ - where: eq(schema.approval.id, approvalId), - }); - if (row === undefined) throw new Error("fixture approval missing"); - - const [item] = await hydrateNeedsYou(db.db, [parseApprovalRow(row)]); - - expect(item).toBeDefined(); - expect(item?.agentName).toBe("Outreach Composer"); - expect(item?.benchName).toBe("Growth Team Bench"); - expect(item?.headline).toBe("send_email"); - // The UI floor: nothing on the view model may be the raw id a person - // would have to decode to understand what they're looking at. - expect(JSON.stringify(item)).not.toContain(runId); - expect(JSON.stringify(item)).not.toContain(tenantId); - expect(JSON.stringify(item)).not.toContain(definitionId); - }); - - function mountedApp(principal: PrincipalRow) { - const app = new Hono(); - app.use("*", async (c: Context, next: Next) => { - c.set("tenant", tenantRow); - c.set("principal", principal); - c.set("user", null); - c.set("session", null); - await next(); - }); - app.route( - "/", - createNeedsYouRoutes({ - db: db.db, - grantStore: createInMemoryGrantStore([ - { - id: generateId("grant"), - resource: "approval:*", - action: "resolve", - effect: "allow", - origin: "system", - conditions: null, - expiresAt: null, - roleId: null, - principalId: approverPrincipalId, - }, - ]), - conditionRegistry: {}, - }), - ); - return app; - } - - test("a principal holding the approval:* grant sees the pending item with resolved names", async () => { - const app = mountedApp({ - id: approverPrincipalId, - tenantId, - kind: "user", - refId: "usr_approver", - status: "active", - createdAt: new Date(), - updatedAt: new Date(), - }); - - const response = await app.request("/"); - expect(response.status).toBe(200); - const body = (await response.json()) as { items: unknown[] }; - expect(body.items).toHaveLength(1); - expect(body.items[0]).toMatchObject({ - agentName: "Outreach Composer", - benchName: "Growth Team Bench", - headline: "send_email", - }); - }); - - test("a principal with no approval grant is refused server-side, not just hidden in the UI", async () => { - const app = mountedApp({ - id: strangerPrincipalId, - tenantId, - kind: "user", - refId: "usr_stranger", - status: "active", - createdAt: new Date(), - updatedAt: new Date(), - }); - - const response = await app.request("/"); - expect(response.status).toBe(403); - const body = (await response.json()) as { error: { code: string } }; - expect(body.error.code).toBe("forbidden"); - }); - - describe("single-approval detail read", () => { - function approverApp() { - return mountedApp({ - id: approverPrincipalId, - tenantId, - kind: "user", - refId: "usr_approver", - status: "active", - createdAt: new Date(), - updatedAt: new Date(), - }); - } - - function strangerApp() { - return mountedApp({ - id: strangerPrincipalId, - tenantId, - kind: "user", - refId: "usr_stranger", - status: "active", - createdAt: new Date(), - updatedAt: new Date(), - }); - } - - test("an approver reads a pending approval's status", async () => { - const response = await approverApp().request(`/${approvalId}`); - expect(response.status).toBe(200); - const body = (await response.json()) as { status: string }; - expect(body).toMatchObject({ - id: approvalId, - status: "pending", - agentName: "Outreach Composer", - benchName: "Growth Team Bench", - }); - }); - - test("a resolved approval is still fetchable, in its terminal status", async () => { - const response = await approverApp().request(`/${resolvedApprovalId}`); - expect(response.status).toBe(200); - const body = (await response.json()) as { status: string }; - expect(body.status).toBe("approved"); - }); - - // The tenant-wide approval:* grant this read checks is deliberately - // coarser than the native routes' per-deployment approval: check -- - // a 403 here means "could not determine," never "cannot resolve." See - // `packages/chat-ui/src/blocks/approve-card-state.ts`. - test("a principal without the tenant-wide grant is refused, not shown a guessed status", async () => { - const response = await strangerApp().request(`/${approvalId}`); - expect(response.status).toBe(403); - const body = (await response.json()) as { error: { code: string } }; - expect(body.error.code).toBe("forbidden"); - }); - - // Authorization is checked before the row is loaded (the grant is - // tenant-wide, not keyed to the target id), so an unauthorized caller - // gets the same 403 whether the id exists or not -- a 404-vs-403 split - // here would let a stranger enumerate which ids exist in the tenant. - test("an unauthorized principal sees 403 even for an id that doesn't exist, never a leaking 404", async () => { - const response = await strangerApp().request("/apv_does_not_exist"); - expect(response.status).toBe(403); - const body = (await response.json()) as { error: { code: string } }; - expect(body.error.code).toBe("forbidden"); - }); - - test("an unknown approval id 404s", async () => { - const response = await approverApp().request("/apv_does_not_exist"); - expect(response.status).toBe(404); - }); - - test("cross-tenant existence is masked as 404, not 403", async () => { - const response = await approverApp().request(`/${otherTenantApprovalId}`); - expect(response.status).toBe(404); - }); - }); -}); diff --git a/packages/chat-ui/src/blocks/approve-card-state.test.ts b/packages/chat-ui/src/blocks/approve-card-state.test.ts index 8d20b6fb41..596487bfac 100644 --- a/packages/chat-ui/src/blocks/approve-card-state.test.ts +++ b/packages/chat-ui/src/blocks/approve-card-state.test.ts @@ -44,7 +44,7 @@ describe("deriveApproveCardView", () => { }); test("pending + canAct (host maps a successful pending read to canAct) is actionable, never spectator", () => { - // Host contract: a successful needs-you GET that returns pending always + // Host contract: a successful status read that returns pending always // sets canAct true (`apps/web/src/approval-actions.ts`). The card must // then show live buttons -- not demote that to spectator. const view = deriveApproveCardView({ @@ -136,13 +136,11 @@ describe("deriveApproveCardView", () => { }); }); - test("needs-you 403 is could-not-determine, never cannot-act/spectator", () => { - // GET /:approvalId is gated by tenant-wide `approval:*`/"resolve". A - // principal scoped only to `approval:` still gets 403 on - // that read even though the native approve/reject route may allow them. - // Callers must treat that 403 as "could not determine" (`forbidden` → - // `undetermined`, buttons stay), never as "cannot act" (`spectator`). - // See `packages/approvals/src/routes.ts`. + test("a forbidden status read keeps the buttons, never demotes to spectator", () => { + // A refused status read leaves the card with no platform detail to + // show. It still renders buttons: the refusal a person can act on is + // the one their own decision returns, surfaced inline, not a silently + // disabled card (`forbidden` → `undetermined`, never `spectator`). const view = deriveApproveCardView({ wired: true, live: { kind: "forbidden" }, diff --git a/packages/chat-ui/src/blocks/approve-card-state.ts b/packages/chat-ui/src/blocks/approve-card-state.ts index 287acdc869..7b490b50eb 100644 --- a/packages/chat-ui/src/blocks/approve-card-state.ts +++ b/packages/chat-ui/src/blocks/approve-card-state.ts @@ -1,7 +1,7 @@ // Pure state mapping for the approve card: turns the live status read plus // any in-flight decision into exactly what the view renders. Kept free of // React so the branch logic (actable vs. spectator vs. resolved vs. the -// undeterminable-403 fallback) is unit-testable without a DOM. +// forbidden-read branch) is unit-testable without a DOM. import type { ApprovalLiveStatus, @@ -29,12 +29,13 @@ export type ApproveCardView = readonly status: ApprovalLiveStatus; readonly detail: PlatformApprovalDetail; } - /** The read itself was forbidden: actionability -- and the platform's own - * detail -- could not be determined. Buttons render anyway; a real 403 - * from approve/reject surfaces inline instead of being guessed in - * advance. There is deliberately no `detail` here: this is the one case - * where buttons show without it, so the view must never let the block's - * agent-authored `body`/`title` stand in as if it were that detail. */ + /** The read itself was forbidden, so the platform's own detail never + * arrived. Buttons render anyway: the refusal a person needs to see is + * the one their own decision earns, surfaced inline from approve/reject, + * not a disabled card they cannot ask about. There is deliberately no + * `detail` here: this is the one case where buttons show without it, so + * the view must never let the block's agent-authored `body`/`title` + * stand in as if it were that detail. */ | { readonly kind: "undetermined"; readonly deciding: DecisionInFlight; diff --git a/scripts/checks/browser-safe-subpaths.ts b/scripts/checks/browser-safe-subpaths.ts index f7c6f21988..95c4e557d5 100644 --- a/scripts/checks/browser-safe-subpaths.ts +++ b/scripts/checks/browser-safe-subpaths.ts @@ -66,6 +66,10 @@ export const ENTRIES: readonly BrowserSafeEntry[] = [ // subpaths precisely so a browser package never pulls that in. { package: "@corbits/presence", subpath: "./client" }, { package: "@corbits/presence", subpath: "./color" }, + // @corbits/approvals' root reaches @intx/authz for the grant-allowance + // gate; `headlineFor` is pure string work over a tool snapshot, so the + // browser composes an approval's headline through this subpath. + { package: "@corbits/approvals", subpath: "./headline" }, ]; const DENYLIST_PATTERNS: readonly RegExp[] = [ diff --git a/scripts/checks/kill-dates.txt b/scripts/checks/kill-dates.txt index afbf368608..f260a5558a 100644 --- a/scripts/checks/kill-dates.txt +++ b/scripts/checks/kill-dates.txt @@ -15,7 +15,7 @@ # package's VENDORED-FROM delta line in the same change. apps/sidecar | sawyer | 2026-09-19 vendor/intx/db | sawyer | 2026-09-19 | 642b29735a7e36a1d3decdf8af26c33fa91e0989720323addb4c2f786ae88a18 -vendor/intx/hub-api | sawyer | 2026-09-19 | 8449bf150e0253ca5d5eb26450c3ce2e5ff81116a02cdf2ba049b26453cfa201 +vendor/intx/hub-api | sawyer | 2026-09-19 | f93a383cb5d6acdf50a461b43e4c8991dbdf7e13d34a4e5598e556eaee66308b vendor/intx/hub-sessions | sawyer | 2026-09-19 | df5f1d275e197668851c53f58c51182d8bc455f3585e2ed65784d3687a856001 vendor/intx/workflow | sawyer | 2026-09-19 | 34628e7bbd0587f131a07e3a206141983881106963a20ab607e68aeed1135593 vendor/intx/workflow-deploy | sawyer | 2026-09-19 | 95711adf282180852b0daec1cac39d00a4dc24aff15f9a515e07eb3d2ca749f9 diff --git a/scripts/e2e/cl-6324-launch-proof.ts b/scripts/e2e/cl-6324-launch-proof.ts index 355b83c86e..a79fdc127a 100644 --- a/scripts/e2e/cl-6324-launch-proof.ts +++ b/scripts/e2e/cl-6324-launch-proof.ts @@ -21,11 +21,14 @@ // DATABASE_URL=postgres://localhost:5432/wb6324proof_e2e \ // bun run scripts/e2e/cl-6324-launch-proof.ts import { expect } from "bun:test"; +import { ApprovalResponse, paginatedSchema } from "@intx/types"; +import { type } from "arktype"; import { mkdtemp } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join as pathJoin } from "node:path"; +import { headlineFor } from "../../packages/approvals/src/headline.ts"; import { resetSchema, setupDatabase } from "../db-setup.ts"; import { createGitWorkflowPusher, @@ -934,28 +937,54 @@ async function main(): Promise { ); // ---- proof 3: a real message gets a real reply -------------------- + // Pages through the platform's native `GET .../approvals` pending list + // and approves every pending row with scope "once", printing what it + // approved. Without this, a turn that needs an approval-gated tool + // never finishes and this proof times out waiting for a reply that + // can never come. + const TenantApprovalsSchema = paginatedSchema(ApprovalResponse); async function autoApproveAll(): Promise { - const res = await api( - hub.baseUrl, - "GET", - `/api/tenants/${tenant.tenantId}/approvals/needs-you`, - undefined, - user.cookies, - ); - if (res.status !== 200) return; - const items = arrayField(res.data, "items", "needs-you") as { - id: string; - agentName: string; - headline: string; - }[]; - for (const item of items) { - await api( + let cursor: string | undefined; + for (;;) { + const query = cursor ? `?cursor=${encodeURIComponent(cursor)}` : ""; + const res = await api( hub.baseUrl, - "POST", - `/api/tenants/${tenant.tenantId}/approvals/${item.id}/approve`, - { scope: "once" }, + "GET", + `/api/tenants/${tenant.tenantId}/approvals${query}`, + undefined, user.cookies, ); + if (res.status !== 200) { + throw new Error( + `auto-approve: listing pending approvals failed with HTTP ` + + `${String(res.status)}: ${JSON.stringify(res.data)}`, + ); + } + const page = TenantApprovalsSchema(res.data); + if (page instanceof type.errors) { + throw new Error( + `auto-approve: malformed pending approvals list: ${page.summary}`, + ); + } + for (const item of page.data) { + const approved = await api( + hub.baseUrl, + "POST", + `/api/tenants/${tenant.tenantId}/approvals/${item.id}/approve`, + { scope: "once" }, + user.cookies, + ); + const headline = headlineFor(item.toolDefinition, item.toolArguments); + if (approved.status === 200) { + console.log(` [auto-approved] ${headline}`); + } else { + console.log( + ` [auto-approve FAILED ${String(approved.status)}] ${headline}`, + ); + } + } + if (page.nextCursor === null) break; + cursor = page.nextCursor; } } diff --git a/scripts/e2e/play.ts b/scripts/e2e/play.ts index 4c0c975961..1334757794 100644 --- a/scripts/e2e/play.ts +++ b/scripts/e2e/play.ts @@ -10,7 +10,10 @@ // // PLAY_DUMP=1 also prints filtered sidecar output at the end. import { expect } from "bun:test"; +import { ApprovalResponse, paginatedSchema } from "@intx/types"; +import { type } from "arktype"; +import { headlineFor } from "../../packages/approvals/src/headline.ts"; import { resetSchema, setupDatabase } from "../db-setup.ts"; import { createGitWorkflowPusher, @@ -513,40 +516,55 @@ async function main(): Promise { // `postApproveBlock`) that a real bench user would click. An // `approval: "ask"` tool (every pinned mutating tool -- routine // create/update/run-now, task dispatch, request_capability, ...) - // parks on the tenant's `GET .../approvals/needs-you` inbox instead - // of ever completing, so this polls that inbox and approves every - // pending item with scope "once", printing what it approved. Without - // this, a turn that needs an approval-gated tool never finishes and - // the play loop times out waiting for a reply that can never come. + // parks on the platform's native `GET .../approvals` pending list + // instead of ever completing, so this pages through that list and + // approves every pending row with scope "once", printing what it + // approved. Without this, a turn that needs an approval-gated tool + // never finishes and the play loop times out waiting for a reply that + // can never come. + const TenantApprovalsSchema = paginatedSchema(ApprovalResponse); async function autoApproveAll(): Promise { - const res = await api( - hub.baseUrl, - "GET", - `/api/tenants/${tenant.tenantId}/approvals/needs-you`, - undefined, - user.cookies, - ); - if (res.status !== 200) return; - const items = arrayField(res.data, "items", "needs-you") as { - id: string; - agentName: string; - headline: string; - }[]; - for (const item of items) { - const approved = await api( + let cursor: string | undefined; + for (;;) { + const query = cursor ? `?cursor=${encodeURIComponent(cursor)}` : ""; + const res = await api( hub.baseUrl, - "POST", - `/api/tenants/${tenant.tenantId}/approvals/${item.id}/approve`, - { scope: "once" }, + "GET", + `/api/tenants/${tenant.tenantId}/approvals${query}`, + undefined, user.cookies, ); - if (approved.status === 200) { - console.log(` [auto-approved] ${item.agentName}: ${item.headline}`); - } else { - console.log( - ` [auto-approve FAILED ${String(approved.status)}] ${item.agentName}: ${item.headline}`, + if (res.status !== 200) { + throw new Error( + `auto-approve: listing pending approvals failed with HTTP ` + + `${String(res.status)}: ${JSON.stringify(res.data)}`, ); } + const page = TenantApprovalsSchema(res.data); + if (page instanceof type.errors) { + throw new Error( + `auto-approve: malformed pending approvals list: ${page.summary}`, + ); + } + for (const item of page.data) { + const approved = await api( + hub.baseUrl, + "POST", + `/api/tenants/${tenant.tenantId}/approvals/${item.id}/approve`, + { scope: "once" }, + user.cookies, + ); + const headline = headlineFor(item.toolDefinition, item.toolArguments); + if (approved.status === 200) { + console.log(` [auto-approved] ${headline}`); + } else { + console.log( + ` [auto-approve FAILED ${String(approved.status)}] ${headline}`, + ); + } + } + if (page.nextCursor === null) break; + cursor = page.nextCursor; } } diff --git a/vendor/intx/hub-api/VENDORED-FROM b/vendor/intx/hub-api/VENDORED-FROM index 48588c9343..e073e89a50 100644 --- a/vendor/intx/hub-api/VENDORED-FROM +++ b/vendor/intx/hub-api/VENDORED-FROM @@ -1,4 +1,4 @@ Source: https://github.com/faremeter/interchange (packages/hub-api) Commit: b5580a02fb918eebccc33ded7727ffee781ffbd1 (tag v0.3.0) License: LGPL-2.1-only (see vendor/intx/LICENSE) -Local modifications: exports map repointed from the upstream intx-src condition to direct TypeScript source resolution (types/default -> ./src/...); dist references removed. adds a @types/ssri devDependency that bun's isolated linker does not hoist from tool-packaging the way upstream's install does; approval param routes exclude the reserved segment `needs-you` so hosts can mount a sibling /approvals/needs-you list without /:approvalId capturing it. resolveApproval is exported (with its args/result types and readDurableWorkflowRunLifecycles) and accepts principalId: null for policy-resolved (grant-allowance) decisions, which skip the per-principal approval:/resolve gate the HTTP routes still enforce (CL-6345). +Local modifications: exports map repointed from the upstream intx-src condition to direct TypeScript source resolution (types/default -> ./src/...); dist references removed. adds a @types/ssri devDependency that bun's isolated linker does not hoist from tool-packaging the way upstream's install does. resolveApproval is exported (with its args/result types and readDurableWorkflowRunLifecycles) and accepts principalId: null for policy-resolved (grant-allowance) decisions, which skip the per-principal approval:/resolve gate the HTTP routes still enforce (CL-6345). workflow-run-deploy-auth.test.ts's header comment no longer cites the deleted packages/approvals/test/needs-you.test.ts. diff --git a/vendor/intx/hub-api/src/middleware/workflow-run-deploy-auth.test.ts b/vendor/intx/hub-api/src/middleware/workflow-run-deploy-auth.test.ts index 5fc7413960..d88630252d 100644 --- a/vendor/intx/hub-api/src/middleware/workflow-run-deploy-auth.test.ts +++ b/vendor/intx/hub-api/src/middleware/workflow-run-deploy-auth.test.ts @@ -1,7 +1,7 @@ // DB-gated: skipped when DATABASE_URL is unreachable, matching this repo's // existing convention for tests that talk to a real Postgres (see -// `packages/approvals/test/needs-you.test.ts`). Runs against the caller's -// own database and deletes every row it wrote in `afterAll`. +// `apps/hub/src/memory-mount.test.ts`). Runs against the caller's own +// database and deletes every row it wrote in `afterAll`. // // This suite proves the bearer mirror (`createWorkflowRunDeployAuth`) reaches // the EXACT SAME `createWorkflowRoutes` deploy handler a human session does diff --git a/vendor/intx/hub-api/src/routes/approvals.ts b/vendor/intx/hub-api/src/routes/approvals.ts index 83d0f65f1e..63ef3ec861 100644 --- a/vendor/intx/hub-api/src/routes/approvals.ts +++ b/vendor/intx/hub-api/src/routes/approvals.ts @@ -404,7 +404,7 @@ export function createApprovalRoutes( ); app.get( - "/:approvalId{^(?!needs-you$)[^/]+}", + "/:approvalId", describeRoute({ tags: ["Approvals"], summary: "Get approval details", @@ -475,7 +475,7 @@ export function createApprovalRoutes( ); app.post( - "/:approvalId{^(?!needs-you$)[^/]+}/approve", + "/:approvalId/approve", describeRoute({ tags: ["Approvals"], summary: "Approve an action", @@ -555,7 +555,7 @@ export function createApprovalRoutes( ); app.post( - "/:approvalId{^(?!needs-you$)[^/]+}/reject", + "/:approvalId/reject", describeRoute({ tags: ["Approvals"], summary: "Reject an action", diff --git a/workflows/pain-point-collateral/README.md b/workflows/pain-point-collateral/README.md index b1c8862c41..1c9f83f870 100644 --- a/workflows/pain-point-collateral/README.md +++ b/workflows/pain-point-collateral/README.md @@ -72,7 +72,7 @@ means for deploys today. gate — not a workflow-DSL `awaitSignal`, and not anything invented for this port. Calling it suspends the run: the sidecar co-writes a real `approval` row (`vendor/intx/db/src/schema/approvals.ts`), visible in -the inbox via `@corbits/approvals`' "needs-you" list, and the run parks +the native pending-approvals list the web client reads, and the run parks until a human approves or rejects it (`@intx/inference/src/reactor.ts`). On approval the parked call re-dispatches and the tool actually runs, exactly once. On rejection the