From 58308a3e6343bb4ef71fa7f87899d730dab2e312 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 2 Nov 2026 17:12:22 -0600 Subject: [PATCH] perf(apps): scope sidebar review counts to one connection (#18688) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip helps people manage AI agents and their governed app access. > - The Apps sidebar displays a Review badge for one connection. > - It polls the full company attention service every 30 seconds to get two counts. > - That service also loads every profile, rule and active catalog entry. > - This pull request adds an authorized connection read for only those counts. > - The same badge now avoids unrelated profile evaluation and database result decoding. ## Linked Issues or Issue Description **Current behavior** The contextual sidebar polls company-wide attention. It needs only one connection's signed pending action count and whether any catalog entries are quarantined. **Proposed behavior** Both sidebar variants poll a connection-scoped count endpoint with existing connection read authority. The full attention API stays unchanged. **Reason and benefit** The retained 200-profile fixture performs seven service queries and returns 80,115 rows for this badge. The new reader performs one query and returns one row. This reduces local service CPU and avoids unnecessary decoded profile/rule data. Related merged evaluator fixes are already in the baseline: Refs #18673, Refs #18681. A search found no existing PR for this scoped count reader. ROADMAP.md contains no duplicate planned work. ## What Changed - Add the narrow shared DTO, typed client, guarded REST/OpenAPI route and catalog-owned reader. - Count independent aggregates with both company boundaries and exact signed-argument semantics. Keep archived counts at zero. - Update both contextual sidebar variants. Keep the exact badge, 30 second interval and existing attention-prefix invalidation. - Add PostgreSQL differential tests, authenticated HTTP tests, sidebar tests, a repeatable benchmark and access documentation. ## Verification - Baseline: `3f8ebfff8aa9febd0733c6f65b07485dcadfd0e2`, including both merged evaluator fixes. Service treatment: `0827f7d0b3`; route/fixture refinement: `230c42ee15`; final local existence gate: `390a218254`. - Passed: focused PostgreSQL differential test; assembled HTTP test with board session and agent JWT; 14 sidebar component tests; token gates; map checks; diff check. - Fixture: 200 profiles, 76,000 rules, 1,900 active tools, 695 removed references, 12 agents, a sibling connection and 2,000 unrelated historical invocations. Complete normalized two-count results are equal on both paths. - Quiet local measurement: seven alternating warm pairs after two warmup pairs, Node 24.22.2, macOS, real disposable PostgreSQL. Legacy/new service CPU median: 213.891/1.680 ms; ranges: 209.329–228.552/0.515–6.137 ms. Wall median: 156.867/1.248 ms; ranges: 151.739–162.289/1.115–1.386 ms. Queries: 7/1. Returned rows: 80,115/1. - Run `cd server && node --expose-gc --import ./node_modules/tsx/dist/loader.mjs scripts/benchmark-connection-review-counts.ts`. See `doc/connection-review-counts.md` for scope and failure history. Byte-volume accounting runs outside timing; no HTTP payload reduction is claimed. - Initial raw scalar SQL failed with ambiguous columns. Typed subqueries fixed it and passed real PostgreSQL equality. Initial fixture agents lacked permissions and correctly received 403. The final fixture grants explicit authority and tests revocation. An initial benchmark included byte serialization in timing; the reported run excludes it. - Passed required local `pnpm -r typecheck` and `pnpm build`. The initial sandbox typecheck could not open the tsx IPC socket; the escalated rerun passed. Full monolithic `pnpm test:run` was not run locally: repeating a 15–25 minute suite serially across both performance PRs adds no distinct evidence. Broad suite acceptance uses final-head CI, which runs the same general/serialized test partitions and the native Runner lane. Final-head CI is in progress. - Passed built-static Chromium journey on an owned local-trusted server and disposable PostgreSQL: Review 3 appears, only scoped counts are requested by this sidebar, resolving requests/clearing quarantines then reloading removes the badge, and archived counts are zero. The first ad hoc launcher omitted required decision-service options and failed at startup; using the normal no-op wake callback fixed that harness. - Greptile identified an invalid resolved-request fixture label. Corrected `denied` to the declared `rejected`, then reran the real PostgreSQL differential test. The count SQL and non-pending exclusion are unchanged. - The first final-head CI failed the static cross-tenant existence-oracle guard because the new explicit resource company assertion had no nearby `hasCompanyAccess` gate. Existing management admission already rejects foreign resources with 404. The route now also follows the standard local two-step gate. The unchanged guard plus authenticated HTTP negative cases passed (3 tests). No allowlist or test was weakened. - These local service measurements do not prove hosted performance. No deployment is included. ## Risks Low risk. New read admission reuses existing current-authority checks. The old attention API is unchanged. Counts still require database work over relevant pending/catalog rows. No cache, schema migration or telemetry is added. Roll back this PR if the badge differs or scoped reads deny previously permitted users. ## Model Used OpenAI `gpt-6.1-sol` (Sol 6.1), with reasoning, tool use and code execution. The host does not expose the configured context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/connection-review-counts.md | 101 ++++++++++++++++++ maps/agent-access-map/app-permissions.md | 9 ++ maps/feature-map/app-permissions.md | 6 ++ packages/shared/src/index.ts | 1 + packages/shared/src/types/index.ts | 1 + packages/shared/src/types/tool-access.ts | 5 + .../benchmark-connection-review-counts.ts | 48 +++++++++ .../tool-connection-review-counts-fixture.ts | 17 +++ ...tool-connection-review-counts-http.test.ts | 56 ++++++++++ .../tool-connection-review-counts.test.ts | 45 ++++++++ server/src/routes/openapi.ts | 9 ++ server/src/routes/tool-access.ts | 8 ++ server/src/services/tool-access.ts | 2 + .../tool-access/catalog/review-counts.ts | 33 ++++++ ui/src/api/tools.ts | 3 + .../AppConnectionSidebar.production.tsx | 20 ++-- .../components/AppConnectionSidebar.test.tsx | 44 ++++++++ ui/src/components/AppConnectionSidebar.tsx | 20 ++-- ui/src/lib/queryKeys.ts | 2 + 19 files changed, 406 insertions(+), 24 deletions(-) create mode 100644 doc/connection-review-counts.md create mode 100644 server/scripts/benchmark-connection-review-counts.ts create mode 100644 server/src/__tests__/helpers/tool-connection-review-counts-fixture.ts create mode 100644 server/src/__tests__/tool-connection-review-counts-http.test.ts create mode 100644 server/src/__tests__/tool-connection-review-counts.test.ts create mode 100644 server/src/services/tool-access/catalog/review-counts.ts diff --git a/doc/connection-review-counts.md b/doc/connection-review-counts.md new file mode 100644 index 00000000000..603e73d6192 --- /dev/null +++ b/doc/connection-review-counts.md @@ -0,0 +1,101 @@ +# Connection review counts + +The contextual App sidebar uses `GET /api/tool-connections/:connectionId/review-counts` +for its Review badge. The response contains `pendingActionRequestCount` and +`quarantinedCatalogEntryCount`. The badge adds the pending count and one when the +quarantine count is positive. Both sidebar variants keep the 30 second cadence. +The key is a child of the existing company attention prefix so current mutations +and events still invalidate it. + +The route uses existing connection read admission. It resolves UUIDs and UIDs, +checks company and current actor/responsible-user authority, and enforces personal +account restrictions before reading counts. An authorized archived connection +returns zero counts, as its absent legacy attention item did. Missing and hidden +connections retain normal admission errors. This read adds no execution authority. + +The catalog owner reads two independent scalar aggregates in one SQL statement. +Pending counts join requests to invocations by invocation ID and constrain both +company IDs. SQL NULL signed arguments are excluded; empty signed text counts. +Multiple requests for one invocation count separately. No profile, rule or active +catalog projection runs for this read. The company-wide attention API is unchanged. + +## Local performance proof (2026-11-02) + +Baseline: `3f8ebfff8aa9febd0733c6f65b07485dcadfd0e2` on current master, including +both prior evaluator fixes (#18673 and #18681). Treatment is the count reader in +this change. Node v24.22.2, macOS, disposable real embedded PostgreSQL, one database +connection, no cross-request cache. The comparison calls the actual legacy +attention service and selects one connection's two counts, then calls the new +reader. All normalized results equal `{pendingActionRequestCount: 2, +quarantinedCatalogEntryCount: 3}`. + +Fixture: 200 profiles, 76,000 rules, 1,900 active tools, 695 removed references, +12 agents, one sibling connection, six quarantine entries, signed/unsigned/resolved +requests and 2,000 unrelated historical invocations. Reproduction: + +```sh +cd server +node --expose-gc --import ./node_modules/tsx/dist/loader.mjs scripts/benchmark-connection-review-counts.ts +``` + +After two warmup pairs, seven alternating measured pairs in a coordinated quiet +window produced: + +| Service measurement | Legacy attention → selected counts | Scoped counts | +| --- | --- | --- | +| Queries | 7 | 1 | +| Returned application rows | 80,115 | 1 | +| Process CPU median (range), ms | 213.891 (209.329–228.552) | 1.680 (0.515–6.137) | +| Wall median (range), ms | 156.867 (151.739–162.289) | 1.248 (1.115–1.386) | + +Separate work-volume instrumentation serialized database result values to JSON: +32,978,153 versus 66 bytes. This measures decoded application values, not network +bytes or HTTP response size. The legacy HTTP response never contains the rule +arrays. Work-volume serialization is disabled during every timed sample. + +These are local source-service measurements, not deployed latency or capacity +claims. They exclude HTTP admission, network transport and serialization. They do +not measure PostgreSQL CPU. Scoped SQL still scans relevant pending/catalog data +using existing indexes; no migration or new cache was needed. + +## Correctness and preserved failures + +PostgreSQL differential tests cover sibling/company boundaries, multiple requests, +NULL and empty signed text, unsigned/resolved requests, null invocation connections, +cross-company historical references, unhealthy-only connections, archive/restore, +and changed rows between reads. Assembled HTTP tests use actual human sessions +and agent JWT authentication with explicit permissions. They exercise UID lookup, +missing/foreign resources, personal grant revocation and membership revocation. +Both sidebar variants have badge, disabled-before-identity and prefix invalidation +component tests. + +The first SQL attempt embedded column references directly into scalar raw SQL. +Real PostgreSQL rejected the resulting ambiguous unqualified columns. Typed +Drizzle subqueries fixed that failure; the passing differential test exercises the +corrected query. An initial benchmark serialized result values inside its timed +instrumentation and inflated CPU. That run was discarded; the results above come +from a fresh alternating run with byte accounting outside timing. Initial HTTP +fixtures lacked agent membership/management grants and correctly received 403; +the accepted fixture supplies both and retains revocation denial checks. + +The built static UI was also exercised in Chromium against an owned local-trusted +server and disposable PostgreSQL. The connection Permissions page showed Review 3, +issued the scoped count request, and made no company attention request. After +resolving requests and clearing quarantines, reload removed the badge. An archived +connection returned zero counts. This browser path uses board authority; separate +authenticated HTTP fixtures prove delegated agent authority. The first ad hoc +browser launcher omitted required decision-service options and failed at startup; +adding the standard no-op wake callback allowed the same journey to pass. No +product behavior was changed to recover that harness setup failure. + +Required local recursive typecheck and full build passed. The first sandboxed +typecheck could not open the tsx IPC socket; the escalated rerun passed. Greptile +identified an invalid resolved-request fixture label (`denied`); it was corrected +to the declared `rejected` status. This preserves the non-pending exclusion that +the differential test verifies. + +The first final-head CI run flagged the new explicit resource company assertion +because the route lacked a local `hasCompanyAccess` existence gate. Management +admission already rejects foreign resources with 404, but the route now also +follows the repository's two-step gate pattern. The static existence-oracle guard +and actual authenticated HTTP negative cases verify that pattern together. diff --git a/maps/agent-access-map/app-permissions.md b/maps/agent-access-map/app-permissions.md index 8bef4add8b2..22675ec3973 100644 --- a/maps/agent-access-map/app-permissions.md +++ b/maps/agent-access-map/app-permissions.md @@ -404,6 +404,15 @@ Evidence: `source-only` — [route](../../server/src/routes/tool-access.ts); rev Verification: `not-run` — individual operation not executed in this audit; shared authorization and representative family flows are exercised by the authenticated integration suite. Follow the route schema, inspect persistence/audit, and assert owner, scope and revoked-authority denial. Gaps: This individual operation has source review but no recorded HTTP verification. Provider availability and live setup remain unqualified. +### `get-tool-connections-review-counts` + +API: `present` — `GET /api/tool-connections/{connectionId}/review-counts`; JSON REST. +Agent access: `available` — agent REST and default native `search_api` / `call_api`. +Policy: `tools:manage_connections`; existing connection read admission checks current company membership, resolved UUID/UID, responsible-user authority and personal account access. Authorized archived connections return zero counts. +Evidence: `verified` — [PostgreSQL and authenticated HTTP fixtures](../../server/src/__tests__/tool-connection-review-counts-http.test.ts) exercise the assembled server and actual board session and agent JWT middleware. +Verification: `passed` — deterministic local fixture with UID resolution, archive, foreign/missing connection, personal grant revocation and membership revocation. +Gaps: Local fixtures do not prove deployed performance or provider availability. + ### `get-tool-connections-usage` API: `present` — `GET /api/tool-connections/{connectionId}/usage`; JSON REST. diff --git a/maps/feature-map/app-permissions.md b/maps/feature-map/app-permissions.md index c725de0d1f7..921a493d5c9 100644 --- a/maps/feature-map/app-permissions.md +++ b/maps/feature-map/app-permissions.md @@ -162,3 +162,9 @@ Sources: Source: [PermissionsPanel.tsx](../../ui/src/pages/apps/app-detail/Permi ### `delegated-configuration` See the canonical [delegated configuration guidance and acceptance evidence](connection-setup.md#delegated-configuration). + +The contextual connection sidebar reads only the selected connection's pending +signed action request and quarantined action counts. Its Review badge is the +pending request count plus one when any action is quarantined. It refreshes every +30 seconds and after existing attention invalidations. Archived connections show +no badge. The full Apps attention view keeps its existing company-wide read. diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 676ca8ed6b0..81150730214 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -1466,6 +1466,7 @@ export type { ToolAppAttentionItem, ToolAppAttentionReason, ToolAppsAttentionResponse, + ToolConnectionReviewCounts, ToolActionRequestListItem, ToolActionRequestsResponse, ToolConnectionActivityResponse, diff --git a/packages/shared/src/types/index.ts b/packages/shared/src/types/index.ts index a00d768be26..14b31713338 100644 --- a/packages/shared/src/types/index.ts +++ b/packages/shared/src/types/index.ts @@ -507,6 +507,7 @@ export type { ToolAppAttentionItem, ToolAppAttentionReason, ToolAppsAttentionResponse, + ToolConnectionReviewCounts, ToolActionRequestListItem, ToolActionRequestsResponse, ToolConnectionActivityResponse, diff --git a/packages/shared/src/types/tool-access.ts b/packages/shared/src/types/tool-access.ts index 961a97a8f99..39e7e92d3ad 100644 --- a/packages/shared/src/types/tool-access.ts +++ b/packages/shared/src/types/tool-access.ts @@ -1020,6 +1020,11 @@ export interface ToolAppAttentionItem { reasons: ToolAppAttentionReason[]; } +export interface ToolConnectionReviewCounts { + pendingActionRequestCount: number; + quarantinedCatalogEntryCount: number; +} + export interface ToolAppsAttentionResponse { generatedAt: Date | string; apps: ToolAppAttentionItem[]; diff --git a/server/scripts/benchmark-connection-review-counts.ts b/server/scripts/benchmark-connection-review-counts.ts new file mode 100644 index 00000000000..ac425f55aac --- /dev/null +++ b/server/scripts/benchmark-connection-review-counts.ts @@ -0,0 +1,48 @@ +import assert from "node:assert/strict"; +import { createDb, startEmbeddedPostgresTestDatabase, type Db } from "@paperclipai/db"; +import { createConnectionReviewCountsFixture } from "../src/__tests__/helpers/tool-connection-review-counts-fixture.js"; +import { createToolAppAttention } from "../src/services/tool-access/catalog/attention.js"; +import { getToolConnectionReviewCounts } from "../src/services/tool-access/catalog/review-counts.js"; +const database = await startEmbeddedPostgresTestDatabase("connection-review-counts-benchmark-"); +const db = createDb(database.connectionString, { maxConnections: 1 }); +let queries = 0, rows = 0, returnedValueBytes = 0, collectBytes = false; +const measuredDb = new Proxy(db, { get(target, key) { + if (key !== "select") return Reflect.get(target, key); + return (...fields: Parameters) => { + const builder = target.select(...fields), from = builder.from.bind(builder); + builder.from = ((...args: Parameters) => { + const selection = from(...args), execute = selection.execute.bind(selection); + selection.execute = async () => { const result = await execute(); queries++; rows += result.length; if (collectBytes) returnedValueBytes += Buffer.byteLength(JSON.stringify(result)); return result; }; + return selection; + }) as typeof builder.from; + return builder; + }; +} }); +try { + const f = await createConnectionReviewCountsFixture(db, true); + const legacy = createToolAppAttention(measuredDb, () => new Date("2026-02-01T00:00:00Z")); + const invoke = async (label: "before" | "after") => { + if (label === "after") return getToolConnectionReviewCounts(measuredDb, f.company.id, f.connection.id); + const item = (await legacy(f.company.id)).apps.find(app => app.connection.id === f.connection.id); + return { pendingActionRequestCount: item?.pendingActionRequestCount ?? 0, quarantinedCatalogEntryCount: item?.quarantinedCatalogEntryCount ?? 0 }; + }; + const expected = await invoke("before"); + assert.deepEqual(await invoke("after"), expected); + console.log(JSON.stringify({ fixture: { profiles: 200, rules: 76000, activeTools: 1900, removedTools: 695, agents: 12, siblingConnections: 1, extraInvocationHistory: 2000 }, equality: true, expected, node: process.version, platform: process.platform, baseline: "3f8ebfff8a", scope: "service only; excludes HTTP admission and serialization; returnedValueBytes is application JSON value size, not wire bytes" })); + for (const label of ["before", "after"] as const) { + queries = 0; rows = 0; returnedValueBytes = 0; collectBytes = true; + assert.deepEqual(await invoke(label), expected); + console.log(JSON.stringify({ label, kind: "work-counts", queries, rows, returnedValueBytes })); + } + collectBytes = false; + for (let round = -2; round < 7; round++) { + for (const label of round % 2 ? ["after", "before"] as const : ["before", "after"] as const) { + global.gc?.(); queries = 0; rows = 0; returnedValueBytes = 0; + const cpu = process.cpuUsage(), start = performance.now(); + const result = await invoke(label); + const elapsedMs = performance.now() - start, used = process.cpuUsage(cpu); + assert.deepEqual(result, expected); + if (round >= 0) console.log(JSON.stringify({ round, label, elapsedMs, cpuMs: (used.user + used.system) / 1000, queries, rows, forcedGc: Boolean(global.gc) })); + } + } +} finally { await db.$client.end({ timeout: 1 }); await database.cleanup(); } diff --git a/server/src/__tests__/helpers/tool-connection-review-counts-fixture.ts b/server/src/__tests__/helpers/tool-connection-review-counts-fixture.ts new file mode 100644 index 00000000000..ec00fdca602 --- /dev/null +++ b/server/src/__tests__/helpers/tool-connection-review-counts-fixture.ts @@ -0,0 +1,17 @@ +import { randomUUID } from "node:crypto"; +import { toolActionRequests, toolCatalogEntries, toolConnections, toolInvocations, type Db } from "@paperclipai/db"; +import { createProfileProjectionFixture } from "./tool-profile-projection-fixture.js"; + +export async function createConnectionReviewCountsFixture(db: Db, large = false) { + const f = await createProfileProjectionFixture(db, large ? {} : { profiles: 4, tools: 12, rules: 4 }); + const [sibling] = await db.insert(toolConnections).values({ companyId: f.company.id, applicationId: f.application.id, name: "Sibling", uid: randomUUID(), transport: "mcp_remote", status: "active", healthStatus: "error" }).returning(); + await db.insert(toolCatalogEntries).values([f.connection, sibling].flatMap(connection => Array.from({ length: 3 }, (_, i) => ({ companyId: f.company.id, connectionId: connection.id, name: `quarantine_${i}`, toolName: `quarantine_${i}`, versionHash: "v1", status: "quarantined" as const })))); + const [invocation, siblingInvocation, nullInvocation] = await db.insert(toolInvocations).values([f.connection.id, sibling.id, null].map(connectionId => ({ companyId: f.company.id, connectionId, toolName: "review_me" }))).returning(); + const request = (invocationId: string, signedArguments: string | null, status: "pending" | "rejected" = "pending") => ({ companyId: f.company.id, invocationId, signedArguments, status, canonicalArgumentsHash: "hash", canonicalArgumentsSummary: { summary: "Synthetic review request" } }); + const requests = await db.insert(toolActionRequests).values([ + request(invocation.id, "signed"), request(invocation.id, ""), request(invocation.id, null), request(invocation.id, "signed", "rejected"), + request(siblingInvocation.id, "signed"), request(nullInvocation.id, "signed"), + ]).returning(); + if (large) await db.insert(toolInvocations).values(Array.from({ length: 2000 }, () => ({ companyId: f.company.id, connectionId: sibling.id, toolName: "history", status: "succeeded" as const }))); + return { ...f, sibling, invocation, requests }; +} diff --git a/server/src/__tests__/tool-connection-review-counts-http.test.ts b/server/src/__tests__/tool-connection-review-counts-http.test.ts new file mode 100644 index 00000000000..01d7dd223ef --- /dev/null +++ b/server/src/__tests__/tool-connection-review-counts-http.test.ts @@ -0,0 +1,56 @@ +import { randomUUID } from "node:crypto"; +import { eq } from "drizzle-orm"; +import { beforeAll, afterAll, describe, expect, it, vi } from "vitest"; +import { companyMemberships, toolApplications, toolConnections, toolCatalogEntries, connectionGrants, principalPermissionGrants } from "@paperclipai/db"; +import { startRunnerApiTestServer } from "./helpers/runner-api-server.js"; +import { accessService } from "../services/access.js"; +import { createLocalAgentJwt } from "../agent-auth-jwt.js"; + +describe("review-counts through assembled authenticated HTTP routes", () => { + let server: Awaited>; + beforeAll(async () => { + vi.stubEnv("PAPERCLIP_AGENT_JWT_SECRET", randomUUID()); + vi.stubEnv("PAPERCLIP_RESPONSIBLE_USER_AUTHZ_MODE", "shadow"); + vi.stubEnv("PAPERCLIP_AUTH_RATE_LIMIT_ENABLED", "false"); + server = await startRunnerApiTestServer({ authPublicBaseUrl: "https://paperclip.fixture.invalid", humanSessions: true }); + }, 90_000); + afterAll(async () => { await server?.close(); vi.unstubAllEnvs(); }); + it("uses current company, personal and private authority for board and actual agent identities", async () => { + const f = await server.fixture({ connectionScenario: "fresh" }); + const access = accessService(server.db); + await access.ensureMembership(f.companyId, "agent", f.agentId, "member", "active"); + await access.ensureMembership(f.companyId, "user", f.responsibleUserId!, "operator", "active"); + await access.ensureRoleDefaultGrants(f.companyId, f.responsibleUserId!, "operator", f.responsibleUserId!); + await server.db.insert(principalPermissionGrants).values({ companyId: f.companyId, principalType: "agent", principalId: f.agentId, permissionKey: "tools:manage_connections", scope: null }); + const cookie = await server.humanSession(f.responsibleUserId!); + const token = createLocalAgentJwt(f.agentId, f.companyId, "paperclip_runner", f.runId, f.responsibleUserId!); + const [application] = await server.db.insert(toolApplications).values({ companyId: f.companyId, name: "Scoped counts", type: "mcp_http" }).returning(); + const [connection] = await server.db.insert(toolConnections).values({ companyId: f.companyId, applicationId: application.id, name: "Scoped counts", uid: `counts-${randomUUID()}`, transport: "mcp_remote", status: "active", createdByUserId: f.responsibleUserId }).returning(); + await server.db.insert(toolCatalogEntries).values({ companyId: f.companyId, connectionId: connection.id, name: "Review", toolName: "review", versionHash: "v1", status: "quarantined" }); + const read = async (id: string, actor: "board" | "agent" = "agent") => { + const response = await fetch(`${server.apiUrl}/api/tool-connections/${id}/review-counts`, { headers: actor === "board" ? { cookie } : { Authorization: `Bearer ${token}` } }); + return { status: response.status, body: await response.json() }; + }; + for (const actor of ["board", "agent"] as const) { + expect(await read(connection.id, actor)).toEqual({ status: 200, body: { pendingActionRequestCount: 0, quarantinedCatalogEntryCount: 1 } }); + expect(await read(connection.uid, actor)).toEqual(await read(connection.id, actor)); + expect((await read(randomUUID(), actor)).status).toBe(404); + } + await server.db.update(toolConnections).set({ status: "archived" }).where(eq(toolConnections.id, connection.id)); + expect(await read(connection.id)).toEqual({ status: 200, body: { pendingActionRequestCount: 0, quarantinedCatalogEntryCount: 0 } }); + await server.db.update(toolConnections).set({ status: "active", credentialPolicy: "per_user", createdByUserId: "another-user" }).where(eq(toolConnections.id, connection.id)); + expect((await read(connection.id)).status).toBe(403); + const [grant] = await server.db.insert(connectionGrants).values({ companyId: f.companyId, connectionId: connection.id, kind: "user", subjectUserId: f.responsibleUserId!, status: "active" }).returning(); + expect((await read(connection.id)).status).toBe(200); + await server.db.update(connectionGrants).set({ status: "revoked" }).where(eq(connectionGrants.id, grant.id)); + expect((await read(connection.id)).status).toBe(403); + await server.db.update(toolConnections).set({ credentialPolicy: "shared", createdByUserId: f.responsibleUserId }).where(eq(toolConnections.id, connection.id)); + const [foreignApplication] = await server.db.insert(toolApplications).values({ companyId: f.foreignCompanyId, name: "Foreign", type: "mcp_http" }).returning(); + const [foreign] = await server.db.insert(toolConnections).values({ applicationId: foreignApplication.id, companyId: f.foreignCompanyId, name: "Foreign", uid: randomUUID(), transport: "mcp_remote" }).returning(); + expect([403, 404]).toContain((await read(foreign.id)).status); + await server.db.delete(principalPermissionGrants).where(eq(principalPermissionGrants.principalId, f.responsibleUserId!)); + await server.db.update(companyMemberships).set({ status: "suspended" }).where(eq(companyMemberships.principalId, f.responsibleUserId!)); + expect([403, 404]).toContain((await read(connection.id)).status); + expect([403, 404]).toContain((await read(connection.id, "board")).status); + }, 30_000); +}); diff --git a/server/src/__tests__/tool-connection-review-counts.test.ts b/server/src/__tests__/tool-connection-review-counts.test.ts new file mode 100644 index 00000000000..4e701cbb0b8 --- /dev/null +++ b/server/src/__tests__/tool-connection-review-counts.test.ts @@ -0,0 +1,45 @@ +import { and, eq } from "drizzle-orm"; +import { beforeAll, afterAll, describe, expect, it } from "vitest"; +import { createDb, companies, toolActionRequests, toolCatalogEntries, toolConnections, toolInvocations } from "@paperclipai/db"; +import { startRecoveryPostgres } from "./helpers/recovery-postgres.js"; +import { createConnectionReviewCountsFixture } from "./helpers/tool-connection-review-counts-fixture.js"; +import { createToolAppAttention } from "../services/tool-access/catalog/attention.js"; +import { getToolConnectionReviewCounts } from "../services/tool-access/catalog/review-counts.js"; + +const zero = { pendingActionRequestCount: 0, quarantinedCatalogEntryCount: 0 }; +describe("connection review counts on PostgreSQL", () => { + let database: Awaited>; + let db: ReturnType; + beforeAll(async () => { database = await startRecoveryPostgres("connection-review-counts-"); db = createDb(database.connectionString); }, 90_000); + afterAll(async () => { await db?.$client.end(); await database?.cleanup(); }); + it("preserves the complete legacy badge projection without fanout and recomputes after changes", async () => { + const f = await createConnectionReviewCountsFixture(db); + const legacy = createToolAppAttention(db, () => new Date()); + const compare = async (id: string) => { + const item = (await legacy(f.company.id)).apps.find(app => app.connection.id === id); + const expected = item ? { pendingActionRequestCount: item.pendingActionRequestCount, quarantinedCatalogEntryCount: item.quarantinedCatalogEntryCount } : zero; + const result = await getToolConnectionReviewCounts(db, f.company.id, id); + expect(result).toEqual(expected); + return result; + }; + expect(await compare(f.connection.id)).toEqual({ pendingActionRequestCount: 2, quarantinedCatalogEntryCount: 3 }); + expect(await compare(f.sibling.id)).toEqual({ pendingActionRequestCount: 1, quarantinedCatalogEntryCount: 3 }); + const [foreign] = await db.insert(companies).values({ name: "Foreign review counts", issuePrefix: "FRC" }).returning(); + // Historical schema permits cross-company references: neither side can contribute. + const [invocation] = await db.insert(toolInvocations).values({ companyId: foreign.id, connectionId: f.connection.id, toolName: "foreign" }).returning(); + await db.insert(toolActionRequests).values([ + { companyId: f.company.id, invocationId: invocation.id, signedArguments: "signed", canonicalArgumentsHash: "hash", canonicalArgumentsSummary: { summary: "Synthetic review request" } }, + { companyId: foreign.id, invocationId: f.invocation.id, signedArguments: "signed", canonicalArgumentsHash: "hash", canonicalArgumentsSummary: { summary: "Synthetic review request" } }, + ]); + await db.insert(toolCatalogEntries).values({ companyId: foreign.id, connectionId: f.connection.id, name: "foreign", toolName: "foreign", versionHash: "v1", status: "quarantined" }); + expect(await compare(f.connection.id)).toEqual({ pendingActionRequestCount: 2, quarantinedCatalogEntryCount: 3 }); + await db.update(toolConnections).set({ status: "archived" }).where(eq(toolConnections.id, f.connection.id)); + expect(await compare(f.connection.id)).toEqual(zero); + await db.update(toolConnections).set({ status: "active" }).where(eq(toolConnections.id, f.connection.id)); + expect(await compare(f.connection.id)).toEqual({ pendingActionRequestCount: 2, quarantinedCatalogEntryCount: 3 }); + await db.update(toolActionRequests).set({ status: "rejected" }).where(eq(toolActionRequests.companyId, f.company.id)); + await db.update(toolCatalogEntries).set({ status: "active" }).where(and(eq(toolCatalogEntries.companyId, f.company.id), eq(toolCatalogEntries.status, "quarantined"))); + expect(await compare(f.connection.id)).toEqual(zero); + expect(await compare(f.sibling.id)).toEqual(zero); // unhealthy-only remains a zero badge + }); +}); diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index 772b4706a19..e19d5035a83 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -11769,6 +11769,15 @@ registerCurrentRoute({ summary: "Revoke a tool connection grant", }); +registerCurrentRoute({ + "x-paperclip-transport": "rest", + "x-paperclip-connection-management": "connections:read", + method: "get", + path: "/api/tool-connections/{connectionId}/review-counts", + tags: ["tool-access"], + summary: "Get pending action request and quarantined catalog entry counts for one connection", +}); + registerCurrentRoute({ "x-paperclip-transport": "rest", "x-paperclip-connection-management": "connections:read", diff --git a/server/src/routes/tool-access.ts b/server/src/routes/tool-access.ts index 72209d84746..889c9f11733 100644 --- a/server/src/routes/tool-access.ts +++ b/server/src/routes/tool-access.ts @@ -1921,6 +1921,14 @@ function connectorEnrollmentPrincipal(req: Request): string { res.json(delegation); }); + router.get("/tool-connections/:connectionId/review-counts", managementGuard("connections:read", "GET /tool-connections/:connectionId/review-counts", { personal: true }), async (req, res) => { + assertBoardOrAgent(req); + const { resource } = connectionRequestContext(req)!; + if (!hasCompanyAccess(req, resource.companyId)) throw notFound("Tool connection not found"); + assertCompanyAccess(req, resource.companyId); + res.json(await svc.getConnectionReviewCounts(resource.companyId, resource.connectionId!)); + }); + router.get("/tool-connections/:connectionId/usage", managementGuard("connections:read", "GET /tool-connections/:connectionId/usage", { personal: true }), async (req, res) => { assertBoardOrAgent(req); const connection = await getAccessibleResource(req, res, svc.getConnection(req.params.connectionId as string), "Tool connection not found"); diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index ee3717e231d..3e3f6292565 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -18,6 +18,7 @@ import { isOAuthEndpointRejection } from "./tool-access/catalog/failures.js"; import { createCatalogDefaultAccess, toPolicy } from "./tool-access/catalog/default-access.js"; import { createToolCatalogDiscovery } from "./tool-access/catalog/discovery.js"; import { createConnectionHealth } from "./tool-access/catalog/health.js"; +import { getToolConnectionReviewCounts } from "./tool-access/catalog/review-counts.js"; import { createToolAppAttention } from "./tool-access/catalog/attention.js"; import { createToolCatalogPublication } from "./tool-access/catalog/publication.js"; import { createToolCatalogReader } from "./tool-access/catalog/reader.js"; @@ -4737,6 +4738,7 @@ export function toolAccessService( }, listAppsNeedingAttention, + getConnectionReviewCounts: (companyId: string, connectionId: string) => getToolConnectionReviewCounts(db, companyId, connectionId), sweepConnectionHealth, scheduledHealth: connectionHealthSchedule(db, checkConnectionHealth), diff --git a/server/src/services/tool-access/catalog/review-counts.ts b/server/src/services/tool-access/catalog/review-counts.ts new file mode 100644 index 00000000000..dc287b7f163 --- /dev/null +++ b/server/src/services/tool-access/catalog/review-counts.ts @@ -0,0 +1,33 @@ +import { and, count, eq, isNotNull, sql } from "drizzle-orm"; +import { toolActionRequests, toolCatalogEntries, toolConnections, toolInvocations, type Db } from "@paperclipai/db"; +import type { ToolConnectionReviewCounts } from "@paperclipai/shared"; + +/** Counts for one already-authorized connection. Independent aggregates avoid catalog/request fanout. */ +export async function getToolConnectionReviewCounts( + db: Db, + companyId: string, + connectionId: string, +): Promise { + const pending = db.select({ count: count() }).from(toolActionRequests) + .innerJoin(toolInvocations, eq(toolInvocations.id, toolActionRequests.invocationId)) + .where(and( + eq(toolActionRequests.companyId, companyId), + eq(toolInvocations.companyId, companyId), + eq(toolInvocations.connectionId, connectionId), + eq(toolActionRequests.status, "pending"), + isNotNull(toolActionRequests.signedArguments), + )); + const quarantined = db.select({ count: count() }).from(toolCatalogEntries).where(and( + eq(toolCatalogEntries.companyId, companyId), + eq(toolCatalogEntries.connectionId, connectionId), + eq(toolCatalogEntries.status, "quarantined"), + )); + const [counts] = await db.select({ + pendingActionRequestCount: sql`case when ${toolConnections.status} = 'archived' then 0 else (${pending}) end`.mapWith(Number), + quarantinedCatalogEntryCount: sql`case when ${toolConnections.status} = 'archived' then 0 else (${quarantined}) end`.mapWith(Number), + }).from(toolConnections).where(and( + eq(toolConnections.companyId, companyId), eq(toolConnections.id, connectionId), + )); + // Missing/hidden resources are rejected by route admission before this reader. + return counts ?? { pendingActionRequestCount: 0, quarantinedCatalogEntryCount: 0 }; +} diff --git a/ui/src/api/tools.ts b/ui/src/api/tools.ts index 1087548dfff..8ab5b4ed9e1 100644 --- a/ui/src/api/tools.ts +++ b/ui/src/api/tools.ts @@ -43,6 +43,7 @@ import type { ReorderToolPolicies, AppDefinition, ToolAppsAttentionResponse, + ToolConnectionReviewCounts, ToolConnectionActivityResponse, ToolConnectionLifecycleEventType, ToolConnectionTestAgentsResponse, @@ -334,6 +335,8 @@ export const toolsApi = { `/companies/${companyId}/tools/apps/${connectionId}/finish`, input, ), + getConnectionReviewCounts: (connectionId: string) => + api.get(`/tool-connections/${connectionId}/review-counts`), listAppsAttention: (companyId: string) => api.get(`/companies/${companyId}/tools/apps/attention`), listApplications: (companyId: string) => diff --git a/ui/src/components/AppConnectionSidebar.production.tsx b/ui/src/components/AppConnectionSidebar.production.tsx index 8eb84c326da..7cec3dab4cc 100644 --- a/ui/src/components/AppConnectionSidebar.production.tsx +++ b/ui/src/components/AppConnectionSidebar.production.tsx @@ -50,13 +50,6 @@ export function AppDetailSidebar(props: AppDetailSidebarProps) { queryFn: () => toolsApi.listGallery(selectedCompanyId!), enabled: !!selectedCompanyId, }); - const attentionQuery = useQuery({ - queryKey: queryKeys.apps.attention(selectedCompanyId ?? "__none__"), - queryFn: () => toolsApi.listAppsAttention(selectedCompanyId!), - enabled: !!selectedCompanyId, - refetchInterval: 30_000, - }); - const connection = connectionQuery.data; const application = props.kind === "application" ? (applicationsQuery.data?.applications ?? []).find((app) => app.id === props.applicationId) @@ -72,12 +65,15 @@ export function AppDetailSidebar(props: AppDetailSidebarProps) { application ?? undefined, ); const reviewConnectionId = connection?.id ?? previousConnection?.id ?? null; - const attentionItem = reviewConnectionId - ? attentionQuery.data?.apps.find((app) => app.connection.id === reviewConnectionId) - : null; + const reviewCountsQuery = useQuery({ + queryKey: queryKeys.apps.connectionReviewCounts(selectedCompanyId ?? "__none__", reviewConnectionId ?? "__none__"), + queryFn: () => toolsApi.getConnectionReviewCounts(reviewConnectionId!), + enabled: !!selectedCompanyId && !!reviewConnectionId, + refetchInterval: 30_000, + }); const reviewCount = - (attentionItem?.pendingActionRequestCount ?? 0) + - ((attentionItem?.quarantinedCatalogEntryCount ?? 0) > 0 ? 1 : 0); + (reviewCountsQuery.data?.pendingActionRequestCount ?? 0) + + ((reviewCountsQuery.data?.quarantinedCatalogEntryCount ?? 0) > 0 ? 1 : 0); return (