From 00fea0da528d5abdd2229f9f731e5e21c71a6b76 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 2 Nov 2026 16:50:58 -0600 Subject: [PATCH 01/57] refactor(perf): reuse database test setup for profile benchmark (#18683) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Tool profiles govern the tools that agents can use. > - Profile listing and app attention shared nested selector scans that consumed the server CPU. > - #18681 and #18673 replaced those scans with indexes for each computation. > - Their performance benchmarks need consistent setup and reproducible per-service measurements. > - This pull request uses the shared helper entry point and retains the exact measurement harness and results. > - Reviewers can inspect both the CPU savings and the limits of the evidence. ## Linked Issues or Issue Description Refs #18681 and #18673. **What existing behavior does this improve?** The consistency of benchmark setup and the reproducibility of evidence for the two merged CPU fixes. **Current behavior** The profile-review benchmark imports the database starter directly. The matching benchmark uses the shared server test-helper entry point. On the current base, that helper re-exports the database starter, which already supports `PAPERCLIP_TEST_POSTGRES_ADMIN_URL`. The previous committed benchmark measures both services together; the per-service measurements and separate HTTP/Inspector runs lacked a committed harness. **Proposed behavior** Use the same test-helper entry point in both benchmarks. Retain the compile, per-service timing, Inspector, and HTTP-probe programs with their exact commands and raw samples. Preserve the existing embedded and admin-server database behavior. **Reason and benefit** A shared entry point keeps benchmark setup consistent. The committed harness lets reviewers reproduce and compare each real owning service before and after the merged fixes. The failure-path checks verify that result-write errors still release the database and probe resources. **Breaking changes** None. This change affects benchmark setup and documentation. ## What Changed - Import the database starter from the repository test helper in `benchmark-profile-review-projection.ts`. - Record separate CPU and elapsed measurements for profile listing and app attention. - Retain the actual compile, per-service measurement, Inspector capture, and separate-process HTTP probe harness. - Always tear down the test database, HTTP server, and probe even when result writing fails. - Retain all timing and responsiveness samples as sanitized JSON. - State the runtime, fixture, source revisions, and limits of the local comparison. ## Verification - 24 focused tests passed across selector matching, pending projection, review-service freshness, and effective assignments. - Four selected real database-backed service/route tests passed for summaries, review changes, auto-allow behavior, and company isolation. The other 494 tests in that file were not selected. - Injected an `EISDIR` result-write failure into both profiling programs on an owned local admin server. Each child exited with the expected error and removed its disposable database. The responsiveness child also terminated its probe/server handles. - The changed benchmark completed with a configured local PostgreSQL admin URL. Its full responses matched the baseline, and query/row counts matched. - The exact documented compile, benchmark, and responsiveness commands completed from the committed harness at `ca458baddc6461fb6c888984246e329e2bb4bdfe`. The JSON records that build revision and the four owner source hashes. - Five alternating samples per service and implementation used Node 24.22.2 with compiled ES modules. No runtime TypeScript loader ran in those measurements. - Median process CPU from the committed harness: profile listing 3,164 ms to 385 ms; app attention 2,466 ms to 221 ms. Both versions returned identical ordered results and made seven queries per service. - Separate Inspector profiles confirmed that the repeated scope and summary scans were removed. Three separate-process HTTP probe trials per service showed less event-loop blocking. These are local synthetic service results, not hosted page-load acceptance. - Initial local setup failed on missing embedded PostgreSQL library aliases and an unbuilt plugin SDK. Restoring local aliases and building the SDK resolved those setup failures. The original failed setup logs were retained; the 28 selected tests then passed. - The merged summary fix CI run [41535304968](https://github.com/paperclipai/paperclip/actions/runs/41535304968) hit two 15-second timeouts in the unchanged attention-retention history cases. The server shard passed unchanged on one failed-jobs-only retry. Original and retry logs are retained. The compared runs used different runners and synthetic merge trees, so the original timeout cause is not established. No timeout or assertion was changed. - Full repository typecheck, test matrix, and build passed in CI on final head `2f484ed4284f3fa632d8e73d9be084827d1c987e`: 54 successful check runs, three expected skips, one non-blocking Paperclip review, and a successful Snyk status. No failures or pending checks remain. Those broad commands were not rerun locally for this benchmark-tooling follow-up; the 28 selected local regressions and the actual profiling/failure-path programs ran locally. - Fresh Greptile is 5/5 on that exact head. Both review P2 threads are resolved. The Paperclip review's current-base documentation note is corrected in this description; its optional tooling suggestions remain outside this bounded change. ## Risks - The test helper creates a unique disposable database on the configured admin server and removes it afterward. The operator must use a database server where those test operations are appropriate. - Synthetic data and shared-host activity affect absolute timings. The measurements do not establish a production latency percentage or capacity limit. - Production behavior is covered by the two linked merged fixes. This follow-up changes only benchmark setup and documentation. ## Model Used OpenAI GPT-6 through Codex, with code execution and repository tools. The exact runtime model revision and context window were not exposed in this session. The original linked performance fixes record their own model details. ## 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 - [x] All Paperclip CI gates are green - [x] 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 --- .../2026-11-02-profile-projection.json | 532 ++++++++++++++++++ doc/mcp-discovery-performance.md | 83 +++ .../benchmark-profile-review-projection.ts | 3 +- .../profile-tool-projection/benchmark.mjs | 95 ++++ .../profile-tool-projection/compile.mjs | 56 ++ .../scripts/profile-tool-projection/probe.mjs | 20 + .../profile-tool-projection/register.mjs | 13 + .../responsiveness.mjs | 74 +++ 8 files changed, 875 insertions(+), 1 deletion(-) create mode 100644 doc/benchmarks/2026-11-02-profile-projection.json create mode 100644 server/scripts/profile-tool-projection/benchmark.mjs create mode 100644 server/scripts/profile-tool-projection/compile.mjs create mode 100644 server/scripts/profile-tool-projection/probe.mjs create mode 100644 server/scripts/profile-tool-projection/register.mjs create mode 100644 server/scripts/profile-tool-projection/responsiveness.mjs diff --git a/doc/benchmarks/2026-11-02-profile-projection.json b/doc/benchmarks/2026-11-02-profile-projection.json new file mode 100644 index 00000000000..428f514b0ae --- /dev/null +++ b/doc/benchmarks/2026-11-02-profile-projection.json @@ -0,0 +1,532 @@ +{ + "measurements": [ + { + "kind": "environment", + "node": "v24.22.2", + "platform": "darwin", + "arch": "arm64", + "build": "esbuild transpiled ES modules, node24 target; no runtime TypeScript loader", + "baseline": "1f8f2e195f4897cd1c237bdc068de95780ab0c3c", + "pending": "fd59c0183ce64c44696f79604516f4d51ca5662b", + "treatmentHead": "ca458baddc6461fb6c888984246e329e2bb4bdfe", + "sourceSha256": { + "profiles/projection": "fdb8b2cd220ce71a1ef322afef8d25e684beff88b73fedb1a5b73122b95e9a8d", + "profiles/service": "d56f83ec975bf4d2955cb9a31e9b4ebeb4f3872f28d930436bd37b6004ce1824", + "profiles/resolution": "cb621da4864ee143922c94644487b8bf53a185848a419e9b05945ed6b0237a14", + "catalog/attention": "c3fdc44e01f39b59cec047584c75d99df192f4e67809a6078ac75dfbb3c2b005" + }, + "forcedGc": true, + "fixture": { + "profiles": 200, + "activeTools": 1900, + "removedTools": 695, + "rules": 76000, + "agents": 12 + }, + "repeats": 5, + "caveat": "Local synthetic service workload, shared developer host; not hosted page-load acceptance" + }, + { + "kind": "correctness", + "owner": "profiles", + "completeOutputEquality": true + }, + { + "kind": "correctness", + "owner": "attention", + "completeOutputEquality": true + }, + { + "kind": "timing", + "round": 0, + "owner": "profiles", + "label": "baseline", + "wallMs": 3044.6725000000006, + "cpuMs": 3086.254, + "queries": 7, + "rows": 78114, + "resultCount": 200 + }, + { + "kind": "timing", + "round": 0, + "owner": "profiles", + "label": "pending", + "wallMs": 1331.632625000002, + "cpuMs": 1475.759, + "queries": 7, + "rows": 78114, + "resultCount": 200 + }, + { + "kind": "timing", + "round": 0, + "owner": "profiles", + "label": "combined", + "wallMs": 214.5939580000013, + "cpuMs": 384.783, + "queries": 7, + "rows": 78114, + "resultCount": 200 + }, + { + "kind": "timing", + "round": 0, + "owner": "attention", + "label": "baseline", + "wallMs": 2378.09475, + "cpuMs": 2466.076, + "queries": 7, + "rows": 78101 + }, + { + "kind": "timing", + "round": 0, + "owner": "attention", + "label": "pending", + "wallMs": 174.1545839999999, + "cpuMs": 319.323, + "queries": 7, + "rows": 78101 + }, + { + "kind": "timing", + "round": 0, + "owner": "attention", + "label": "combined", + "wallMs": 159.26837499999965, + "cpuMs": 219.179, + "queries": 7, + "rows": 78101 + }, + { + "kind": "timing", + "round": 1, + "owner": "profiles", + "label": "combined", + "wallMs": 181.5132080000003, + "cpuMs": 224.232, + "queries": 7, + "rows": 78114, + "resultCount": 200 + }, + { + "kind": "timing", + "round": 1, + "owner": "profiles", + "label": "pending", + "wallMs": 1997.141208000001, + "cpuMs": 1830.433, + "queries": 7, + "rows": 78114, + "resultCount": 200 + }, + { + "kind": "timing", + "round": 1, + "owner": "profiles", + "label": "baseline", + "wallMs": 4942.23775, + "cpuMs": 4553.281, + "queries": 7, + "rows": 78114, + "resultCount": 200 + }, + { + "kind": "timing", + "round": 1, + "owner": "attention", + "label": "combined", + "wallMs": 318.5823750000018, + "cpuMs": 466.636, + "queries": 7, + "rows": 78101 + }, + { + "kind": "timing", + "round": 1, + "owner": "attention", + "label": "pending", + "wallMs": 192.32737500000076, + "cpuMs": 254.286, + "queries": 7, + "rows": 78101 + }, + { + "kind": "timing", + "round": 1, + "owner": "attention", + "label": "baseline", + "wallMs": 2694.246791999998, + "cpuMs": 2761.568, + "queries": 7, + "rows": 78101 + }, + { + "kind": "timing", + "round": 2, + "owner": "profiles", + "label": "baseline", + "wallMs": 4083.2392089999994, + "cpuMs": 3864.952, + "queries": 7, + "rows": 78114, + "resultCount": 200 + }, + { + "kind": "timing", + "round": 2, + "owner": "profiles", + "label": "pending", + "wallMs": 1423.3007910000015, + "cpuMs": 1581.88, + "queries": 7, + "rows": 78114, + "resultCount": 200 + }, + { + "kind": "timing", + "round": 2, + "owner": "profiles", + "label": "combined", + "wallMs": 223.0350000000035, + "cpuMs": 414.014, + "queries": 7, + "rows": 78114, + "resultCount": 200 + }, + { + "kind": "timing", + "round": 2, + "owner": "attention", + "label": "baseline", + "wallMs": 1864.8400000000038, + "cpuMs": 1928.275, + "queries": 7, + "rows": 78101 + }, + { + "kind": "timing", + "round": 2, + "owner": "attention", + "label": "pending", + "wallMs": 175.5971669999999, + "cpuMs": 339.946, + "queries": 7, + "rows": 78101 + }, + { + "kind": "timing", + "round": 2, + "owner": "attention", + "label": "combined", + "wallMs": 147.17741699999897, + "cpuMs": 202.889, + "queries": 7, + "rows": 78101 + }, + { + "kind": "timing", + "round": 3, + "owner": "profiles", + "label": "combined", + "wallMs": 182.54275000000052, + "cpuMs": 218.628, + "queries": 7, + "rows": 78114, + "resultCount": 200 + }, + { + "kind": "timing", + "round": 3, + "owner": "profiles", + "label": "pending", + "wallMs": 1203.7079159999994, + "cpuMs": 1258.248, + "queries": 7, + "rows": 78114, + "resultCount": 200 + }, + { + "kind": "timing", + "round": 3, + "owner": "profiles", + "label": "baseline", + "wallMs": 2998.833458000001, + "cpuMs": 3164.455, + "queries": 7, + "rows": 78114, + "resultCount": 200 + }, + { + "kind": "timing", + "round": 3, + "owner": "attention", + "label": "combined", + "wallMs": 174.7018339999995, + "cpuMs": 353.529, + "queries": 7, + "rows": 78101 + }, + { + "kind": "timing", + "round": 3, + "owner": "attention", + "label": "pending", + "wallMs": 147.98641700000007, + "cpuMs": 204.736, + "queries": 7, + "rows": 78101 + }, + { + "kind": "timing", + "round": 3, + "owner": "attention", + "label": "baseline", + "wallMs": 2482.0454580000005, + "cpuMs": 2538.083, + "queries": 7, + "rows": 78101 + }, + { + "kind": "timing", + "round": 4, + "owner": "profiles", + "label": "baseline", + "wallMs": 2980.427125000002, + "cpuMs": 3091.659, + "queries": 7, + "rows": 78114, + "resultCount": 200 + }, + { + "kind": "timing", + "round": 4, + "owner": "profiles", + "label": "pending", + "wallMs": 1334.6162909999985, + "cpuMs": 1494.868, + "queries": 7, + "rows": 78114, + "resultCount": 200 + }, + { + "kind": "timing", + "round": 4, + "owner": "profiles", + "label": "combined", + "wallMs": 233.16475000000355, + "cpuMs": 439.851, + "queries": 7, + "rows": 78114, + "resultCount": 200 + }, + { + "kind": "timing", + "round": 4, + "owner": "attention", + "label": "baseline", + "wallMs": 2031.9052500000034, + "cpuMs": 2090.426, + "queries": 7, + "rows": 78101 + }, + { + "kind": "timing", + "round": 4, + "owner": "attention", + "label": "pending", + "wallMs": 173.21041600000171, + "cpuMs": 316.039, + "queries": 7, + "rows": 78101 + }, + { + "kind": "timing", + "round": 4, + "owner": "attention", + "label": "combined", + "wallMs": 158.0989999999947, + "cpuMs": 221.091, + "queries": 7, + "rows": 78101 + }, + { + "kind": "profile", + "owner": "profiles", + "label": "baseline", + "durationMs": 2994.209, + "samples": 2345 + }, + { + "kind": "profile", + "owner": "profiles", + "label": "pending", + "durationMs": 1304.667, + "samples": 999 + }, + { + "kind": "profile", + "owner": "profiles", + "label": "combined", + "durationMs": 271.542, + "samples": 173 + }, + { + "kind": "profile", + "owner": "attention", + "label": "baseline", + "durationMs": 2502.958, + "samples": 1958 + }, + { + "kind": "profile", + "owner": "attention", + "label": "pending", + "durationMs": 248.75, + "samples": 149 + }, + { + "kind": "profile", + "owner": "attention", + "label": "combined", + "durationMs": 216.375, + "samples": 129 + } + ], + "responsiveness": [ + { + "kind": "responsiveness", + "round": 0, + "owner": "profiles", + "label": "baseline", + "operationMs": 3020.712625, + "completedHealthRequests": 4, + "maxHealthMs": 2885.9768750000003, + "errors": [], + "scope": "Separate-process HTTP probe to a minimal liveness handler in the same Node event loop as the real service; not the full application health route" + }, + { + "kind": "responsiveness", + "round": 0, + "owner": "profiles", + "label": "combined", + "operationMs": 216.8745829999989, + "completedHealthRequests": 5, + "maxHealthMs": 78.3608750000003, + "errors": [], + "scope": "Separate-process HTTP probe to a minimal liveness handler in the same Node event loop as the real service; not the full application health route" + }, + { + "kind": "responsiveness", + "round": 0, + "owner": "attention", + "label": "baseline", + "operationMs": 2220.037542, + "completedHealthRequests": 4, + "maxHealthMs": 2103.2218329999996, + "errors": [], + "scope": "Separate-process HTTP probe to a minimal liveness handler in the same Node event loop as the real service; not the full application health route" + }, + { + "kind": "responsiveness", + "round": 0, + "owner": "attention", + "label": "combined", + "operationMs": 178.0747500000016, + "completedHealthRequests": 4, + "maxHealthMs": 55.81975000000057, + "errors": [], + "scope": "Separate-process HTTP probe to a minimal liveness handler in the same Node event loop as the real service; not the full application health route" + }, + { + "kind": "responsiveness", + "round": 1, + "owner": "profiles", + "label": "combined", + "operationMs": 180.5625, + "completedHealthRequests": 4, + "maxHealthMs": 63.572291000000405, + "errors": [], + "scope": "Separate-process HTTP probe to a minimal liveness handler in the same Node event loop as the real service; not the full application health route" + }, + { + "kind": "responsiveness", + "round": 1, + "owner": "profiles", + "label": "baseline", + "operationMs": 3155.7248339999987, + "completedHealthRequests": 5, + "maxHealthMs": 3027.892249999999, + "errors": [], + "scope": "Separate-process HTTP probe to a minimal liveness handler in the same Node event loop as the real service; not the full application health route" + }, + { + "kind": "responsiveness", + "round": 1, + "owner": "attention", + "label": "combined", + "operationMs": 183.60120800000004, + "completedHealthRequests": 4, + "maxHealthMs": 71.33616699999766, + "errors": [], + "scope": "Separate-process HTTP probe to a minimal liveness handler in the same Node event loop as the real service; not the full application health route" + }, + { + "kind": "responsiveness", + "round": 1, + "owner": "attention", + "label": "baseline", + "operationMs": 2413.9667499999996, + "completedHealthRequests": 3, + "maxHealthMs": 2292.3195000000014, + "errors": [], + "scope": "Separate-process HTTP probe to a minimal liveness handler in the same Node event loop as the real service; not the full application health route" + }, + { + "kind": "responsiveness", + "round": 2, + "owner": "profiles", + "label": "baseline", + "operationMs": 3146.798458000001, + "completedHealthRequests": 4, + "maxHealthMs": 3008.1585419999974, + "errors": [], + "scope": "Separate-process HTTP probe to a minimal liveness handler in the same Node event loop as the real service; not the full application health route" + }, + { + "kind": "responsiveness", + "round": 2, + "owner": "profiles", + "label": "combined", + "operationMs": 230.38120900000285, + "completedHealthRequests": 4, + "maxHealthMs": 83.7714589999996, + "errors": [], + "scope": "Separate-process HTTP probe to a minimal liveness handler in the same Node event loop as the real service; not the full application health route" + }, + { + "kind": "responsiveness", + "round": 2, + "owner": "attention", + "label": "baseline", + "operationMs": 1886.0740839999999, + "completedHealthRequests": 4, + "maxHealthMs": 1770.5739170000015, + "errors": [], + "scope": "Separate-process HTTP probe to a minimal liveness handler in the same Node event loop as the real service; not the full application health route" + }, + { + "kind": "responsiveness", + "round": 2, + "owner": "attention", + "label": "combined", + "operationMs": 180.26520900000105, + "completedHealthRequests": 3, + "maxHealthMs": 64.22629199999938, + "errors": [], + "scope": "Separate-process HTTP probe to a minimal liveness handler in the same Node event loop as the real service; not the full application health route" + } + ], + "responsiveness_limit": "Minimal local HTTP liveness handler on the same event loop as real database-backed service operations, not the assembled application health route." +} diff --git a/doc/mcp-discovery-performance.md b/doc/mcp-discovery-performance.md index ac696b01187..bed7a912efd 100644 --- a/doc/mcp-discovery-performance.md +++ b/doc/mcp-discovery-performance.md @@ -138,3 +138,86 @@ seven samples of ten resolutions. It reports actual query and returned-row counts, process CPU, and elapsed time. The installation dependency returns an empty list for this fixture, which has no installed connections. It does not measure provider work, whole-run latency, or hosted performance. + + +## Local profile comparison on 2026-11-02 + +The profile and app-attention paths were re-profiled together after +[#18681](https://github.com/paperclipai/paperclip/pull/18681) and +[#18673](https://github.com/paperclipai/paperclip/pull/18673). Both fixes replace +nested selector scans with indexes that live for one computation. + +The comparison used Node 24.22.2 on macOS arm64. Service modules were compiled +as ES modules for Node 24 before measurement. No TypeScript loader ran during +measurement. All variants used the same database, fixture, and database client. +Only the service and projection implementation changed between variants. +The fixture had 200 profiles, 1,900 active tools, 695 removed tools, 76,000 +rules, and 12 agents. Complete ordered responses matched before timing. + +Baseline: `1f8f2e195f4897cd1c237bdc068de95780ab0c3c`. The pending-review fix was +`fd59c0183ce64c44696f79604516f4d51ca5662b`. The summary fix was +`e20165b6eb871c29e504a3837e8fe92a17ef969f`; its later benchmark helper change +does not change production code. Five unprofiled samples alternated order, +with forced GC before each operation. Values below are medians. +[All samples](benchmarks/2026-11-02-profile-projection.json) retain each measurement +and the pending-review-only treatment. + +| Service | Baseline CPU | Combined CPU | Baseline elapsed | Combined elapsed | +| --- | ---: | ---: | ---: | ---: | +| `listProfiles` | 3,164 ms | 385 ms | 3,045 ms | 215 ms | +| `listAppsNeedingAttention` | 2,466 ms | 221 ms | 2,378 ms | 159 ms | + +Each version made seven queries per service. Profile listing returned 78,114 +database rows; app attention returned 78,101. These counts did not change. +The gains came from less JavaScript work after the reads. + +Separate Inspector CPU captures used a 1 ms sampling interval. The baseline +showed `profileCoversCatalogScope` and `summarizeProfile` as major CPU costs. +Their repeated scans were absent from the combined capture. Residual work +included pending projection, row conversion, and garbage collection. +Profile capture time was excluded from the timing samples above. + +The committed harness is in `server/scripts/profile-tool-projection/`. +Use Node 24.22.2, install workspace dependencies, and run these commands from +the repository root in a disposable worktree. The compile step writes local +`dist/` output. It is not the full application build. The Git history must +contain the baseline and pending-fix commits named above. + +```sh +git fetch https://github.com/paperclipai/paperclip.git refs/pull/18681/head +pnpm install --frozen-lockfile +node server/scripts/profile-tool-projection/compile.mjs +node --expose-gc --import ./server/scripts/profile-tool-projection/register.mjs server/scripts/profile-tool-projection/benchmark.mjs --output-dir /tmp/paperclip-tool-projection --repeat 5 +node --expose-gc --import ./server/scripts/profile-tool-projection/register.mjs server/scripts/profile-tool-projection/responsiveness.mjs --output-dir /tmp/paperclip-tool-projection +``` + +The compile step writes the baseline and pending-only owner modules beside +the current compiled modules. A build manifest records the current Git HEAD +and the SHA-256 hashes of the four current owner source files. The benchmark +records this manifest, validates complete output equality, then measures +`profiles` and `attention` separately for `baseline`, `pending`, and `combined`. +It writes `measurements.json` and six separate `.cpuprofile` files to the +output directory. The HTTP probe writes `responsiveness.json` there. Keep +CPU profiles private because they contain local source paths. Rebuild and +rerun if the owner files change. Serialize these commands with other heavy +builds and benchmarks on the same host. + +A separate process also sent simple HTTP requests every 20 ms while each +real service operation ran. The local liveness handler shared the service's +Node event loop. There was at most one outstanding probe request. Across +three alternating trials, the median largest probe delay fell from 3,008 ms +to 78 ms during profile listing, and from 2,103 ms to 64 ms during app +attention. All 48 probe requests completed without error. + +These are synthetic service measurements on a shared developer machine. +They are not a full application build, a browser test, or hosted acceptance. +The liveness handler is a minimal test handler, not the application's health +route. Workload, JIT warmup, and host activity affect absolute timings. The +comparison does not establish a page-load percentage or production capacity. +Use a matched deployment window to confirm the effect on user requests. + +Both benchmark scripts use the test database helper. Set +`PAPERCLIP_TEST_POSTGRES_ADMIN_URL` to use a local PostgreSQL server; otherwise +they create and remove an embedded test database. The helper creates a unique +disposable database and removes it afterward. Benchmark setup and fixture +insertion are outside the timing samples. diff --git a/server/scripts/benchmark-profile-review-projection.ts b/server/scripts/benchmark-profile-review-projection.ts index 694f3a25704..1c1265ec049 100644 --- a/server/scripts/benchmark-profile-review-projection.ts +++ b/server/scripts/benchmark-profile-review-projection.ts @@ -1,7 +1,8 @@ import assert from "node:assert/strict"; import { isDeepStrictEqual } from "node:util"; import { pathToFileURL } from "node:url"; -import { createDb, startEmbeddedPostgresTestDatabase, type Db } from "@paperclipai/db"; +import { createDb, type Db } from "@paperclipai/db"; +import { startEmbeddedPostgresTestDatabase } from "../src/__tests__/helpers/embedded-postgres.js"; import { createToolProfileService } from "../src/services/tool-access/profiles/service.js"; import { createToolAppAttention } from "../src/services/tool-access/catalog/attention.js"; import { createProfileProjectionFixture } from "../src/__tests__/helpers/tool-profile-projection-fixture.js"; diff --git a/server/scripts/profile-tool-projection/benchmark.mjs b/server/scripts/profile-tool-projection/benchmark.mjs new file mode 100644 index 00000000000..f4a0ed3b176 --- /dev/null +++ b/server/scripts/profile-tool-projection/benchmark.mjs @@ -0,0 +1,95 @@ +import assert from 'node:assert/strict'; +import { writeFile, mkdir, readFile } from 'node:fs/promises'; +import path from 'node:path'; +import { Session } from 'node:inspector'; +import { createDb } from '@paperclipai/db'; +import { startEmbeddedPostgresTestDatabase } from '../../dist/__tests__/helpers/embedded-postgres.js'; +import { createProfileProjectionFixture } from '../../dist/__tests__/helpers/tool-profile-projection-fixture.js'; +const option = (name, fallback) => { const index = process.argv.indexOf(name); return index < 0 ? fallback : process.argv[index + 1] ?? fallback; }; +const output = path.resolve(option('--output-dir', '/tmp/paperclip-tool-projection')); +const repeats = Number(option('--repeat', '5')); +assert.ok(Number.isInteger(repeats) && repeats > 0, '--repeat must be a positive integer'); +assert.equal(typeof global.gc, 'function', 'Run Node with --expose-gc'); +const build = JSON.parse(await readFile(new URL('../../dist/.profile-projection-build.json', import.meta.url), 'utf8')); +await mkdir(output, { recursive: true }); +const variants = {}; +for (const label of ['baseline', 'pending', 'combined']) { + const p = label === 'combined' ? './service.js' : `./.${label}-service.js`; + const a = label === 'combined' ? './attention.js' : `./.${label}-attention.js`; + variants[label] = { + profileFactory: (await import(new URL(`../../dist/services/tool-access/profiles/${p}`, import.meta.url))).createToolProfileService, + attentionFactory: (await import(new URL(`../../dist/services/tool-access/catalog/${a}`, import.meta.url))).createToolAppAttention, + }; +} +const database = await startEmbeddedPostgresTestDatabase('paperclip-hotpaths-owned-'); +const db = createDb(database.connectionString, { maxConnections: 1 }); +let queries = 0, rows = 0; +const measuredDb = new Proxy(db, { get(target, key) { + if (key !== 'select') return Reflect.get(target, key); + return (...fields) => { + const builder = target.select(...fields), from = builder.from.bind(builder); + builder.from = (...args) => { + const selection = from(...args), execute = selection.execute.bind(selection); + selection.execute = async () => { const result = await execute(); queries++; rows += result.length; return result; }; + return selection; + }; + return builder; + }; +} }); +const records = []; +const emit = record => { records.push(record); console.log(JSON.stringify(record)); }; +try { + const fixture = await createProfileProjectionFixture(db); + const now = () => new Date('2026-02-01T00:00:00Z'); + for (const variant of Object.values(variants)) { + const profiles = variant.profileFactory(measuredDb, now); + const attention = variant.attentionFactory(measuredDb, now); + variant.profiles = () => profiles.listProfiles(fixture.company.id); + variant.attention = () => attention(fixture.company.id); + } + emit({ kind: 'environment', node: process.version, platform: process.platform, arch: process.arch, + build: 'esbuild transpiled ES modules, node24 target; no runtime TypeScript loader', + ...build, forcedGc: true, + fixture: { profiles: 200, activeTools: 1900, removedTools: 695, rules: 76000, agents: 12 }, repeats, + caveat: 'Local synthetic service workload, shared developer host; not hosted page-load acceptance' }); + for (const owner of ['profiles', 'attention']) { + const expected = await variants.baseline[owner](); + for (const label of ['pending', 'combined']) assert.deepEqual(await variants[label][owner](), expected); + emit({ kind: 'correctness', owner, completeOutputEquality: true }); + } + for (let round = 0; round < repeats; round++) { + for (const owner of ['profiles', 'attention']) { + const order = round % 2 ? ['combined', 'pending', 'baseline'] : ['baseline', 'pending', 'combined']; + for (const label of order) { + global.gc?.(); queries = 0; rows = 0; + const started = performance.now(), cpu = process.cpuUsage(); + const result = await variants[label][owner](); + const used = process.cpuUsage(cpu); + emit({ kind: 'timing', round, owner, label, wallMs: performance.now() - started, cpuMs: (used.user + used.system) / 1000, + queries, rows, resultCount: owner === 'profiles' ? result.length : result.items?.length }); + } + } + } + for (const owner of ['profiles', 'attention']) { + for (const label of ['baseline', 'pending', 'combined']) { + global.gc?.(); + const session = new Session(); session.connect(); + const post = (method, args = {}) => new Promise((resolve, reject) => session.post(method, args, (error, value) => error ? reject(error) : resolve(value))); + try { + await post('Profiler.enable'); await post('Profiler.setSamplingInterval', { interval: 1000 }); + await post('Profiler.start'); + await variants[label][owner](); + const { profile } = await post('Profiler.stop'); + await writeFile(`${output}/${owner}-${label}.cpuprofile`, JSON.stringify(profile)); + emit({ kind: 'profile', owner, label, durationMs: (profile.endTime - profile.startTime) / 1000, samples: profile.samples?.length }); + } finally { session.disconnect(); } + } + } +} finally { + try { + await writeFile(`${output}/measurements.json`, JSON.stringify(records, null, 2)); + } finally { + try { await db.$client.end({ timeout: 1 }); } + finally { await database.cleanup(); } + } +} diff --git a/server/scripts/profile-tool-projection/compile.mjs b/server/scripts/profile-tool-projection/compile.mjs new file mode 100644 index 00000000000..c82b2b8adf1 --- /dev/null +++ b/server/scripts/profile-tool-projection/compile.mjs @@ -0,0 +1,56 @@ +import { build, transform } from 'esbuild'; +import { readdir, mkdir, writeFile, cp, readFile } from 'node:fs/promises'; +import { fileURLToPath } from 'node:url'; +import { createHash } from 'node:crypto'; +import { execFileSync } from 'node:child_process'; +import path from 'node:path'; +const root = fileURLToPath(new URL("../../..", import.meta.url)); +process.chdir(root); +async function sources(dir) { + const out = []; + for (const entry of await readdir(dir, { withFileTypes: true })) { + const file = path.join(dir, entry.name); + if (entry.isDirectory() && entry.name !== '__tests__') out.push(...await sources(file)); + else if (entry.isFile() && /\.tsx?$/.test(entry.name) && !/\.d\.ts$|\.test\.tsx?$/.test(entry.name)) out.push(file); + } + return out; +} +async function assets(dir, out) { + for (const entry of await readdir(dir, { withFileTypes: true })) { + const file = path.join(dir, entry.name), target = path.join(out, entry.name); + if (entry.isDirectory() && entry.name !== '__tests__') await assets(file, target); + else if (entry.isFile() && entry.name.endsWith('.json')) { await mkdir(out, { recursive: true }); await cp(file, target); } + } +} +for (const pkg of ['packages/shared', 'packages/db', 'server']) { + const src = path.join(root, pkg, 'src'); + await build({ entryPoints: await sources(src), outbase: src, outdir: path.join(root, pkg, 'dist'), + format: 'esm', platform: 'node', target: 'node24', sourcemap: true, logLevel: 'warning' }); + await assets(src, path.join(root, pkg, 'dist')); +} +await cp('packages/db/src/migrations', 'packages/db/dist/migrations', { recursive: true }); +await cp('packages/shared/src/app-definitions', 'packages/shared/dist/app-definitions', { recursive: true }); +await mkdir('server/dist/__tests__/helpers', { recursive: true }); +await build({ entryPoints: ['server/src/__tests__/helpers/tool-profile-projection-fixture.ts', 'server/src/__tests__/helpers/embedded-postgres.ts'], + outbase: 'server/src/__tests__/helpers', outdir: 'server/dist/__tests__/helpers', format: 'esm', platform: 'node', target: 'node24', sourcemap: true }); +for (const [label, sha] of [['baseline', '1f8f2e195f4897cd1c237bdc068de95780ab0c3c'], ['pending', 'fd59c0183ce64c44696f79604516f4d51ca5662b']]) { + for (const [name, file] of [['projection', 'profiles/projection'], ['service', 'profiles/service'], ['attention', 'catalog/attention']]) { + const relative = `server/src/services/tool-access/${file}.ts`; + let code = execFileSync('git', ['show', `${sha}:${relative}`], { encoding: 'utf8', maxBuffer: 5e6 }); + if (name === 'service') code = code.replace('from "./projection.js"', `from "./.${label}-projection.js"`); + if (name === 'attention') code = code.replace('from "../profiles/projection.js"', `from "../profiles/.${label}-projection.js"`); + const outfile = `server/dist/services/tool-access/${path.dirname(file)}/.${label}-${name}.js`; + const result = await transform(code, { loader: 'ts', format: 'esm', platform: 'node', target: 'node24', sourcefile: relative, sourcemap: 'inline' }); + await writeFile(outfile, result.code); + } +} +const owners = ['profiles/projection', 'profiles/service', 'profiles/resolution', 'catalog/attention']; +const manifest = { + baseline: '1f8f2e195f4897cd1c237bdc068de95780ab0c3c', + pending: 'fd59c0183ce64c44696f79604516f4d51ca5662b', + treatmentHead: execFileSync('git', ['rev-parse', 'HEAD'], { encoding: 'utf8' }).trim(), + sourceSha256: Object.fromEntries(await Promise.all(owners.map(async file => [file, + createHash('sha256').update(await readFile(`server/src/services/tool-access/${file}.ts`)).digest('hex')]))), +}; +await writeFile('server/dist/.profile-projection-build.json', JSON.stringify(manifest, null, 2)); +console.log('Compiled baseline, pending-only, and current owning service modules; no runtime TypeScript loader.'); diff --git a/server/scripts/profile-tool-projection/probe.mjs b/server/scripts/profile-tool-projection/probe.mjs new file mode 100644 index 00000000000..41e43cbd9dd --- /dev/null +++ b/server/scripts/profile-tool-projection/probe.mjs @@ -0,0 +1,20 @@ +import http from 'node:http'; +const agent = new http.Agent({ keepAlive: true }); +let active = false, pending, times = [], port; +function ping() { + if (!active || pending) return; + pending = new Promise(resolve => { + const start = performance.now(); + const request = http.get({ host: '127.0.0.1', port, path: '/api/health', agent }, response => { + response.resume(); response.on('end', () => { times.push(performance.now() - start); resolve(); }); + }); + request.setTimeout(20000, () => request.destroy(new Error('Probe timeout'))); + request.on('error', error => { times.push({ error: error.message }); resolve(); }); + }).finally(() => { pending = null; }); +} +const interval = setInterval(ping, 20); +process.on('message', async message => { + if (message.kind === 'arm') { port = message.port; times = []; active = true; process.send({ kind: 'armed' }); } + if (message.kind === 'stop') { active = false; await pending; process.send({ kind: 'stopped', times }); } + if (message.kind === 'close') { clearInterval(interval); agent.destroy(); process.disconnect(); } +}); diff --git a/server/scripts/profile-tool-projection/register.mjs b/server/scripts/profile-tool-projection/register.mjs new file mode 100644 index 00000000000..3053c27df24 --- /dev/null +++ b/server/scripts/profile-tool-projection/register.mjs @@ -0,0 +1,13 @@ +import { registerHooks } from 'node:module'; +import { pathToFileURL, fileURLToPath } from 'node:url'; +import path from 'node:path'; +const root = fileURLToPath(new URL("../../..", import.meta.url)); +registerHooks({ resolve(specifier, context, next) { + for (const [name, dir] of [['@paperclipai/db', 'packages/db'], ['@paperclipai/shared', 'packages/shared']]) { + if (specifier === name || specifier.startsWith(name + '/')) { + const file = specifier === name ? 'index' : specifier.slice(name.length + 1); + return next(pathToFileURL(path.join(root, dir, 'dist', `${file}.js`)).href, context); + } + } + return next(specifier, context); +} }); diff --git a/server/scripts/profile-tool-projection/responsiveness.mjs b/server/scripts/profile-tool-projection/responsiveness.mjs new file mode 100644 index 00000000000..d38d4b754e5 --- /dev/null +++ b/server/scripts/profile-tool-projection/responsiveness.mjs @@ -0,0 +1,74 @@ +import assert from 'node:assert/strict'; +import path from 'node:path'; +import { createServer } from 'node:http'; +import { fork } from 'node:child_process'; +import { once } from 'node:events'; +import { writeFile, mkdir } from 'node:fs/promises'; +import { createDb } from '@paperclipai/db'; +import { startEmbeddedPostgresTestDatabase } from '../../dist/__tests__/helpers/embedded-postgres.js'; +import { createProfileProjectionFixture } from '../../dist/__tests__/helpers/tool-profile-projection-fixture.js'; +const index = process.argv.indexOf('--output-dir'); +const output = path.resolve(index < 0 ? '/tmp/paperclip-tool-projection' : process.argv[index + 1]); +assert.equal(typeof global.gc, 'function', 'Run Node with --expose-gc'); +await mkdir(output, { recursive: true }); +const database = await startEmbeddedPostgresTestDatabase('paperclip-hotpaths-http-owned-'); +const db = createDb(database.connectionString, { maxConnections: 1 }); +const server = createServer((_request, response) => { response.writeHead(200, { 'Content-Type': 'application/json' }); response.end('{"status":"ok"}'); }); +let probe; +const control = async message => { + const reply = once(probe, 'message', { signal: AbortSignal.timeout(30000) }); + probe.send(message); + return (await reply)[0]; +}; +const records = []; +try { + server.listen(0, '127.0.0.1'); await once(server, 'listening'); + probe = fork(new URL('./probe.mjs', import.meta.url), { execArgv: [], stdio: ['ignore', 'inherit', 'inherit', 'ipc'] }); + const fixture = await createProfileProjectionFixture(db); + const variants = {}, now = () => new Date('2026-02-01T00:00:00Z'); + for (const label of ['baseline', 'combined']) { + const p = label === 'combined' ? 'service' : '.baseline-service'; + const a = label === 'combined' ? 'attention' : '.baseline-attention'; + const profiles = (await import(new URL(`../../dist/services/tool-access/profiles/${p}.js`, import.meta.url))).createToolProfileService(db, now); + const attention = (await import(new URL(`../../dist/services/tool-access/catalog/${a}.js`, import.meta.url))).createToolAppAttention(db, now); + variants[label] = { profiles: () => profiles.listProfiles(fixture.company.id), attention: () => attention(fixture.company.id) }; + } + for (const variant of Object.values(variants)) for (const owner of ['profiles', 'attention']) await variant[owner](); + for (let round = 0; round < 3; round++) { + for (const owner of ['profiles', 'attention']) { + for (const label of round % 2 ? ['combined', 'baseline'] : ['baseline', 'combined']) { + global.gc?.(); + await control({ kind: 'arm', port: server.address().port }); + const start = performance.now(); + await variants[label][owner](); + const operationMs = performance.now() - start; + const result = await control({ kind: 'stop' }); + const good = result.times.filter(value => typeof value === 'number'); + const record = { kind: 'responsiveness', round, owner, label, operationMs, + completedHealthRequests: good.length, maxHealthMs: good.length ? Math.max(...good) : null, + errors: result.times.filter(value => typeof value !== 'number'), + scope: 'Separate-process HTTP probe to a minimal liveness handler in the same Node event loop as the real service; not the full application health route' }; + records.push(record); console.log(JSON.stringify(record)); + } + } + } +} finally { + try { + await writeFile(path.join(output, 'responsiveness.json'), JSON.stringify(records, null, 2)); + } finally { + try { + if (probe && probe.exitCode === null && probe.signalCode === null) { + const exited = once(probe, 'exit'); + if (probe.connected) probe.send({ kind: 'close' }); + else probe.kill(); + await exited; + } + } finally { + try { await new Promise(resolve => server.close(resolve)); } + finally { + try { await db.$client.end({ timeout: 1 }); } + finally { await database.cleanup(); } + } + } + } +} From 554a770c6f8e48e327e19f5208b575a5ec5bce30 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 2 Nov 2026 16:55:56 -0600 Subject: [PATCH 02/57] perf(apps): scope connection profile configuration reads (#18687) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The Apps Permissions page reads saved access profiles for one connection. > - The page fetched every company profile, rule, and binding. > - It used only the app profile and eligible task-granted profiles for that connection. > - This pull request adds a saved configuration read owned by the profile service. > - The route keeps current connection and per-profile management checks. > - The benefit is fewer database rows, less Node work, and a smaller response for the same displayed configuration. ## Linked Issues or Issue Description **Subsystem affected** Server profile configuration reads and the Apps Permissions and Review data caller. **Problem or motivation** One connection page loaded and serialized unrelated company profiles. In the local fixture, this produced a 32.35 MB response with 76,001 rule rows. **Proposed solution** Select the displayed connection's app profile and exact eligible connection-intent profiles before loading their entries and bindings. Return an explicit configuration DTO with complete selected arrays. Keep summary and pending-review work on the existing full API. **Alternatives considered** Filtering the full response in the browser retains the database and Node cost. Truncating the full API would break other clients. Filtering entries to the connection would lose selectors and bindings needed by saves. **Roadmap alignment** This improves the existing governed Apps capability. It does not add a new access policy. Related merged work: #18673 and #18681. Related harness cleanup: #18683. Public PR searches found no duplicate connection configuration projection. ## What Changed - Add `ToolProfileConfiguration` and `GET /api/tool-connections/{connectionId}/profile-configuration`. - Select `app:` at any status and exact active, correctly bound `connection_intent` profiles. Run the existing per-profile visibility filter before configuration queries. - Preserve complete ordered entries and bindings. Keep all save behavior, empty-binding install fallback, and the existing full profile API. - Switch AppDetail and AgentConnectionAccess to the configuration DTO. Use a distinct cache key below the existing profiles prefix so mutation and reconnect invalidations still refresh it. - Add real PostgreSQL differential and authenticated HTTP tests, UI caller coverage, a reproducible benchmark, and access-map documentation. ## Verification Baseline: `3f8ebfff8aa9febd0733c6f65b07485dcadfd0e2`, after both #18673 and #18681. Final head: `b40c1ed7c0833d403e20ee8a5ff9d09f1bad6d22`. Current implementation and reproduction details are in [the performance note](https://github.com/paperclipai/paperclip/blob/b40c1ed7c0833d403e20ee8a5ff9d09f1bad6d22/doc/performance/2026-11-02-connection-profile-configuration.md). - AppDetail: 79 tests passed. Existing access/action saves, install fallback, grant removal, errors, and refresh cases use the new caller. - Real PostgreSQL differential: passed. Full ordered selected configuration equals the old full-list selection. Cases cover extra bindings, sibling selectors, wrong source/key/status/binding, null and non-string agent metadata, fresh mutations, empty app bindings, visibility filtering, and company scope. - Production HTTP authentication: API key and run JWT cases passed. Hidden personal accounts omit spanning profiles. Private connection reads fail. Restored ownership restores visibility. Revoked management grants deny fresh calls. Final UID canonicalization and foreign-company 404 assertions passed on the current head. - Map structural check, token gate, and `git diff --check`: passed. - Required local `pnpm -r typecheck` and `pnpm build`: passed on the current head. `pnpm test:run` was started once. Its first chat shard passed (337 tests, 151.87 seconds); the second shard was intentionally interrupted after the complete final-head CI suite passed. This partial local attempt is not a full local test pass. The same stable-runner general and serialized partitions and Runner lanes passed in [final-head CI](https://github.com/paperclipai/paperclip/actions/runs/41538805231), including aggregate `ci / verify`. No local assertion failures were observed before interruption. - Final-head focused acceptance: four files, 85 tests passed. Full token gates, maps, and module boundaries passed. - Actual browser acceptance passed on the current checkout with isolated real PostgreSQL and a local trusted board. An action saved as Off persisted after reload. Both app agent bindings stayed intact. Removing an agent-only task grant persisted after reload. Requests used only the narrow profile endpoint. No browser exceptions occurred. No responses were mocked. No provider calls or hosted changes were made. The helper stopped the owned runtime and database after the check. - Final exact-head checks: 54 success, one clean neutral Paperclip Review, three skipped, zero pending or failures. Legacy Snyk status: success. No inline review comments. - Greptile reviewed exact head `b40c1ed7c0833d403e20ee8a5ff9d09f1bad6d22`: 5/5, no findings, completed successful check. Five alternating unprofiled pairs ran with real PostgreSQL on macOS/arm64, Node v24.22.2, and forced GC. The fixture has 210 profiles (200 heavy profiles plus ten eligibility cases), 76,001 rules, 1,900 active tools, 695 removed tools, and 12 agents. The selected configuration has two profiles and 1,391 rules. Each pair asserts complete ordered selected DTO equality. | Measurement | Full list | Configuration read | | --- | --- | --- | | Service wall ms, median [min, max] | 200.927 [193.319, 213.390] | 6.414 [5.508, 6.973] | | Node CPU ms, median [min, max] | 262.751 [246.727, 271.573] | 27.525 [22.576, 29.308] | | Select queries / returned rows | 7 / 78,139 | 3 / 1,397 | | Actual response JSON bytes | 32,353,534 | 592,233 | | Serialization wall ms, median [min, max] | 90.714 [88.715, 99.304] | 1.644 [1.637, 1.745] | | Serialization CPU ms, median [min, max] | 90.644 [88.569, 96.778] | 1.654 [1.637, 1.807] | Reproduce: `node --expose-gc --import ./server/node_modules/tsx/dist/loader.mjs server/scripts/benchmark-connection-profile-configuration.ts 5`. The benchmark measures the service and actual response JSON separately. It excludes HTTP and authorization work through an identity visibility callback. Node CPU does not include PostgreSQL CPU. No hosted or foreground-under-load improvement is claimed. Preserved failures: offline install lacked a locked tarball; frozen online install passed. Sandbox localhost and tsx IPC restrictions blocked initial DB/typecheck attempts; permitted retries use the same checks. The first HTTP suite needed the unbuilt plugin SDK; its owning build helper fixed setup. Shared typecheck caught a missing barrel export, now fixed. A broad typecheck inherited the host's unpinned nested pnpm executable; the owned stalled process was stopped and restarted with pinned pnpm 9. The complete current-head CI suite and aggregate verify passed. The separate Paperclip agent review published 5/5 with no findings at this head and completed with a clean neutral conclusion. An added foreign-company assertion expected 403; the existing guard correctly returned 404. Only the test expectation changed. The first real browser attempt saved and reloaded the action correctly, but grant deletion returned 422: the stress fixture had a company binding, which makes that profile the company default. The UI showed the existing deletion error. The second browser seed used the production agent-only intent shape and passed removal and reload. This does not change the stress fixture or product deletion gates. Initial external browser script attempts also needed an ESM extension and the correct dependency path. Logs, first-failure screenshots, traces, and result files are retained in the local evidence directory. ## Risks - SQL selection must stay aligned with connection-intent eligibility. Differential fixtures cover the exact current predicate. - A selected connection can still have many rules. This change preserves all selected configuration. - Per-profile authorization still reads current selected resources and can add work. Connection visibility does not bypass that check. - No migration, default-allow, effective-access, policy-review, provider, or deployment behavior changes. The old endpoint remains available. Rollback restores the old AppDetail caller and removes the additive endpoint. ## Model Used OpenAI Codex, `gpt-6.1-sol`, with code execution and repository tools. The host does not expose the 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 - [x] All Paperclip CI gates are green - [x] 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 --- ...-11-02-connection-profile-configuration.md | 81 +++++++++++++++++++ maps/agent-access-map/app-permissions.md | 9 +++ maps/feature-map/app-permissions.md | 2 + packages/shared/src/index.ts | 1 + packages/shared/src/types/index.ts | 1 + packages/shared/src/types/tool-access.ts | 6 ++ ...chmark-connection-profile-configuration.ts | 57 +++++++++++++ .../connection-management.integration.test.ts | 49 ++++++++++- .../connection-profile-configuration.test.ts | 44 ++++++++++ ...onnection-profile-configuration-fixture.ts | 51 ++++++++++++ server/src/routes/openapi.ts | 9 +++ server/src/routes/tool-access.ts | 9 +++ .../profiles/connection-configuration.ts | 64 +++++++++++++++ .../services/tool-access/profiles/service.ts | 3 + ui/src/api/tools.ts | 3 + ui/src/lib/queryKeys.ts | 2 + ui/src/pages/apps/AppDetail.test.tsx | 25 ++++-- ui/src/pages/apps/AppDetail.tsx | 12 +-- .../apps/app-detail/AgentConnectionAccess.tsx | 4 +- 19 files changed, 415 insertions(+), 17 deletions(-) create mode 100644 doc/performance/2026-11-02-connection-profile-configuration.md create mode 100644 server/scripts/benchmark-connection-profile-configuration.ts create mode 100644 server/src/__tests__/connection-profile-configuration.test.ts create mode 100644 server/src/__tests__/helpers/connection-profile-configuration-fixture.ts create mode 100644 server/src/services/tool-access/profiles/connection-configuration.ts diff --git a/doc/performance/2026-11-02-connection-profile-configuration.md b/doc/performance/2026-11-02-connection-profile-configuration.md new file mode 100644 index 00000000000..bf2d7c51fab --- /dev/null +++ b/doc/performance/2026-11-02-connection-profile-configuration.md @@ -0,0 +1,81 @@ +# Connection Permissions configuration read + +Baseline: `3f8ebfff8aa9febd0733c6f65b07485dcadfd0e2`, which includes #18673 +and #18681. This change removes unrelated profile reads from AppDetail. The +profile service owns selection and projection. The route retains the connection +management guard and the full-list route's per-profile visibility checks. The +legacy full profile endpoint remains unchanged. + +The new configuration DTO contains saved profile fields and complete ordered +entries and bindings. It has no summary or pending-review counts. Selection +matches the old UI: `app:` at any status, plus active +`connection_intent` profiles with exact metadata/key and a matching agent binding. +This is a presentation view, not effective access. Complete bindings preserve +save behavior and the empty-binding install fallback. + +## Local experiment + +Run with Node 24, pnpm 9 and installed dependencies: + +```sh +node --expose-gc --import ./server/node_modules/tsx/dist/loader.mjs \ + server/scripts/benchmark-connection-profile-configuration.ts 5 +``` + +The benchmark uses the existing real PostgreSQL profile fixture: 200 heavy +profiles, 76,000 rules, 1,900 active tools, 695 removed tools, and 12 agents. It +adds ten eligibility cases and one cross-connection rule. The selected connection +has two profiles and 1,391 rules. Each sample asserts equality of the complete +ordered selected configuration against the old full-list selection. Selected +rules are not truncated, even when they refer to another connection. + +Five alternating unprofiled pairs ran in a coordinated quiet window on +macOS/arm64, Node v24.22.2, with forced GC before samples. Times below are +median [minimum, maximum] milliseconds. CPU measures Node process CPU, not +PostgreSQL CPU. The benchmark uses an identity visibility callback and excludes +HTTP, middleware, and authorization work. It separately serializes the actual +old full response and new configuration response. + +| Measurement | Full list | Connection configuration | +| --- | --- | --- | +| Service wall | 200.927 [193.319, 213.390] | 6.414 [5.508, 6.973] | +| Node CPU | 262.751 [246.727, 271.573] | 27.525 [22.576, 29.308] | +| Select queries | 7 | 3 | +| Returned database rows | 78,139 | 1,397 | +| Actual response JSON bytes | 32,353,534 | 592,233 | +| JSON serialization wall | 90.714 [88.715, 99.304] | 1.644 [1.637, 1.745] | +| JSON serialization CPU | 90.644 [88.569, 96.778] | 1.654 [1.637, 1.807] | + +This establishes local service work and payload reduction. It does not establish +hosted latency, foreground responsiveness under load, or live provider behavior. +Cost still grows with the selected profiles' complete configuration. Current +agent authorization can separately read selected profile entries and bindings; +those reads are excluded from this benchmark and remain in the request path. + +## Correctness and acceptance + +- The PostgreSQL differential test covers exact eligibility, null/non-string + agent metadata, inactive intent profiles, wrong key/source/bindings, sibling + and unmanaged profiles, complete binding ordering, fresh entry changes, empty + app bindings, denied profile selection, and company isolation. +- The assembled HTTP integration fixture exercises standard agent keys and run + JWTs. It checks private account visibility, missing connections, and fresh + permission revocation. A profile spanning a hidden account is omitted while + the displayed connection stays readable. +- AppDetail's component suite covers saved access and action toggles, complete + access restatement, install fallback, removing grants, errors, and refresh. + The configuration key is a distinct child of the existing profiles prefix, + so existing mutation and reconnect invalidations still refresh it. + +Preserved setup failures: the offline install lacked a locked package tarball; +a frozen online install passed. The first database attempt failed on sandbox +localhost binding restrictions; the escalated real PostgreSQL run passed. The +first HTTP suite could not resolve the unbuilt plugin SDK; the owning +`ensure-build-deps` command built it and the retry passed. A missing shared +barrel export failed the first shared typecheck and was fixed. The first broad +typecheck inherited an unpinned nested pnpm executable; its stalled owned +processes were stopped and the command was restarted with a pinned pnpm wrapper. + +The added foreign-company HTTP assertion first expected 403. The existing guard +returned 404 to hide the resource, as designed. The test expectation was corrected; +production authorization did not change. diff --git a/maps/agent-access-map/app-permissions.md b/maps/agent-access-map/app-permissions.md index 78877ca1bf1..8bef4add8b2 100644 --- a/maps/agent-access-map/app-permissions.md +++ b/maps/agent-access-map/app-permissions.md @@ -83,6 +83,15 @@ Evidence: `verified` — [authenticated integration fixtures](../../server/src/_ Verification: `passed` — deterministic local fixture; no live provider qualification. Gaps: Provider fixtures do not qualify live provider setup or availability. +### `get-tool-connections-profile-configuration` + +API: `present` — `GET /api/tool-connections/{connectionId}/profile-configuration`; JSON REST. Returns saved app and eligible connection-intent profiles, including complete ordered entries and bindings. Omits summary and pending-review counts. The company-wide full profile list stays available. +Agent access: `available` — agent REST and default native `search_api` / `call_api`. +Policy: `tools:manage_profiles`; the displayed connection and every selected profile use current management-resource authorization. Profiles that span a hidden personal account are omitted. Connection visibility alone does not grant profile visibility. +Evidence: `verified` — [authenticated integration fixture](../../server/src/__tests__/connection-management.integration.test.ts) covers standard agent keys and run JWTs, private accounts, missing resources, and revoked authority on local PostgreSQL. +Verification: `passed` — deterministic local fixture. [Configuration differential test](../../server/src/__tests__/connection-profile-configuration.test.ts) compares complete selected entries and bindings with the full list, including mutations and empty bindings. +Gaps: These fixtures do not qualify live providers or hosted performance. + ### `get-tool-connections-installs` API: `present` — `GET /api/tool-connections/{connectionId}/installs`; JSON REST. diff --git a/maps/feature-map/app-permissions.md b/maps/feature-map/app-permissions.md index 0fd4b55a1df..c725de0d1f7 100644 --- a/maps/feature-map/app-permissions.md +++ b/maps/feature-map/app-permissions.md @@ -49,6 +49,8 @@ Preconditions: follow the [baseline](./README.md#before-driving-a-journey). Prep ### `permissions` +The page loads saved profiles for the displayed connection through the configuration read. It preserves complete app bindings for saves and the install fallback when bindings are empty. It does not request profile summaries or pending-review counts. + Automated: [app detail](../../ui/src/pages/apps/AppDetail.test.tsx) covers agent/action controls, quarantined actions, and navigation using mocked requests. [Installation permission regressions](../../server/src/__tests__/tool-connection-installs.test.ts) diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index da43d193d30..676ca8ed6b0 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -1526,6 +1526,7 @@ export type { ToolProfileSummary, ToolProfileStatus, ToolProfileWithDetails, + ToolProfileConfiguration, ToolRateLimitCounter, ToolRateLimitRule, ToolRateLimitWindowKind, diff --git a/packages/shared/src/types/index.ts b/packages/shared/src/types/index.ts index 2f4c4e616cc..a00d768be26 100644 --- a/packages/shared/src/types/index.ts +++ b/packages/shared/src/types/index.ts @@ -600,6 +600,7 @@ export type { ToolProfileSummary, ToolProfileStatus, ToolProfileWithDetails, + ToolProfileConfiguration, ToolRateLimitCounter, ToolRateLimitRule, ToolRateLimitWindowKind, diff --git a/packages/shared/src/types/tool-access.ts b/packages/shared/src/types/tool-access.ts index 5627ca6a174..961a97a8f99 100644 --- a/packages/shared/src/types/tool-access.ts +++ b/packages/shared/src/types/tool-access.ts @@ -785,6 +785,12 @@ export interface ToolProfileSummary { isCompanyDefault: boolean; } +/** Saved configuration only; no effective-access summary or pending-review calculation. */ +export interface ToolProfileConfiguration extends Omit { + entries: ToolProfileEntry[]; + bindings: ToolProfileBinding[]; +} + export interface ToolProfileWithDetails extends ToolProfile { entries: ToolProfileEntry[]; bindings: ToolProfileBinding[]; diff --git a/server/scripts/benchmark-connection-profile-configuration.ts b/server/scripts/benchmark-connection-profile-configuration.ts new file mode 100644 index 00000000000..aad1bc74cbf --- /dev/null +++ b/server/scripts/benchmark-connection-profile-configuration.ts @@ -0,0 +1,57 @@ +import assert from "node:assert/strict"; +import { createDb, type Db } from "@paperclipai/db"; +import { startEmbeddedPostgresTestDatabase } from "../src/__tests__/helpers/embedded-postgres.js"; +import { createToolProfileService } from "../src/services/tool-access/profiles/service.js"; +import { createConnectionProfileConfigurationFixture, legacyConnectionConfiguration } from "../src/__tests__/helpers/connection-profile-configuration-fixture.js"; + +const repeat = Number(process.argv[2] ?? 5); +if (!Number.isInteger(repeat) || repeat < 1) throw new Error("Positive repeat count required"); +const database = await startEmbeddedPostgresTestDatabase("connection-profile-config-benchmark-"); +const db = createDb(database.connectionString, { maxConnections: 1 }); +let queries = 0, rows = 0; +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; return result; }; + return selection; + }) as typeof builder.from; + return builder; + }; +} }); +try { + const f = await createConnectionProfileConfigurationFixture(db); + const service = createToolProfileService(measuredDb, () => new Date("2026-02-01T00:00:00Z")); + const invoke = async (label: "before" | "after") => { + if (label === "before") { + const profiles = await service.listProfiles(f.company.id); + return { profiles, selected: legacyConnectionConfiguration(profiles, f.connection.id) }; + } + const profiles = await service.listConnectionProfileConfiguration(f.company.id, f.connection.id, async rows => rows); + return { profiles, selected: profiles }; + }; + const baseline = await invoke("before"), treatment = await invoke("after"); + assert.deepStrictEqual(treatment.selected, baseline.selected); + console.log(JSON.stringify({ node: process.version, platform: `${process.platform}/${process.arch}`, fixture: { + profiles: baseline.profiles.length, rules: baseline.profiles.reduce((n, p) => n + p.entries.length, 0), + activeTools: 1900, removedTools: 695, agents: 12, selectedProfiles: treatment.profiles.length, + selectedRules: treatment.profiles.reduce((n, p) => n + p.entries.length, 0), + }, equality: "full ordered selected configuration", scope: "real PostgreSQL service + actual response JSON; identity filter, excludes HTTP/auth" })); + for (let round = 0; round < repeat; round++) { + for (const label of round % 2 ? ["after", "before"] as const : ["before", "after"] as const) { + global.gc?.(); queries = 0; rows = 0; + const start = performance.now(), cpu = process.cpuUsage(); + const result = await invoke(label); + const serviceMs = performance.now() - start, used = process.cpuUsage(cpu); + const serializeStart = performance.now(), serializeCpu = process.cpuUsage(); + const json = JSON.stringify({ profiles: result.profiles }); + const serializationMs = performance.now() - serializeStart, serializeUsed = process.cpuUsage(serializeCpu); + assert.deepStrictEqual(result.selected, baseline.selected); + console.log(JSON.stringify({ round, label, serviceMs, cpuMs: (used.user + used.system) / 1000, queries, rows, + jsonBytes: Buffer.byteLength(json), serializationMs, serializationCpuMs: (serializeUsed.user + serializeUsed.system) / 1000, + forcedGc: Boolean(global.gc) })); + } + } +} finally { await db.$client.end({ timeout: 1 }); await database.cleanup(); } diff --git a/server/src/__tests__/connection-management.integration.test.ts b/server/src/__tests__/connection-management.integration.test.ts index a1f3f3e301d..4ddbe7d4021 100644 --- a/server/src/__tests__/connection-management.integration.test.ts +++ b/server/src/__tests__/connection-management.integration.test.ts @@ -6,7 +6,7 @@ import { join } from "node:path"; import { createServer } from "node:http"; import { and, eq, sql } from "drizzle-orm"; import { beforeAll, afterAll, describe, expect, it, vi } from "vitest"; -import { createDb, authUsers, activityLog, agentApiKeys, companies, companySecrets, companySecretVersions, chatConversations, chatDeliveries, chatEndpoints, chatExternalPrincipals, chatIdentityLinks, toolPolicies, toolRateLimitCounters, connectionGrants, connectionGrantMembers, connectionGrantDelegations, toolAccessAuditEvents, toolOauthStates, issueThreadInteractions, connectionIntentDeliveries, agents, companyMemberships, heartbeatRuns, issues, principalPermissionGrants, projects, toolApplications, toolConnections, toolConnectionInstalls, aiConnectionPools, toolMcpGateways, toolMcpGatewayTokens, toolActionRequests, toolCatalogEntries, toolInvocations, toolCallEvents, toolProfiles, toolProfileBindings, plugins, adapterAuthSessions, environments, routines, chatGitHubReviews } from "@paperclipai/db"; +import { createDb, authUsers, activityLog, agentApiKeys, companies, companySecrets, companySecretVersions, chatConversations, chatDeliveries, chatEndpoints, chatExternalPrincipals, chatIdentityLinks, toolPolicies, toolRateLimitCounters, connectionGrants, connectionGrantMembers, connectionGrantDelegations, toolAccessAuditEvents, toolOauthStates, issueThreadInteractions, connectionIntentDeliveries, agents, companyMemberships, heartbeatRuns, issues, principalPermissionGrants, projects, toolApplications, toolConnections, toolConnectionInstalls, aiConnectionPools, toolMcpGateways, toolMcpGatewayTokens, toolActionRequests, toolCatalogEntries, toolInvocations, toolCallEvents, toolProfileEntries, toolProfiles, toolProfileBindings, plugins, adapterAuthSessions, environments, routines, chatGitHubReviews } from "@paperclipai/db"; import { findSatisfiedToolConnection, satisfiedConnectionIntentService } from "../services/satisfied-connection-intents.js"; import { connectionManagementService, continuingConnectionManagement } from "../services/connection-management.js"; import { aiConnectionService } from "../services/ai-connections.js"; @@ -408,6 +408,53 @@ describe("connection management with production default agents", () => { for (const id of profileIds) expect(await f.request("GET", `/tool-profiles/${id}/new-tools?connectionId=${first.body.id}`)).toMatchObject({ status: 200 }); }); + it("keeps connection profile configuration scoped and rechecks each profile against current personal access", async () => { + const f = await fixture(); + const create = async (name: string) => { + const response = await f.request("POST", `/companies/${f.companyId}/tools/connections`, { + name, transport: "mcp_remote", authKind: "none", config: { url: "https://1.1.1.1/mcp" }, + }); + expect(response).toMatchObject({ status: 201 }); + return response.body; + }; + const owned = await create("Configuration owned"), privateConnection = await create("Configuration private"); + const [app, intent] = await server.db.insert(toolProfiles).values([ + { companyId: f.companyId, profileKey: `app:${owned.id}`, name: "App" }, + { companyId: f.companyId, profileKey: `connection-intent:${owned.id}:${f.agentId}`, name: "Intent", + metadata: { source: "connection_intent", connectionId: owned.id, agentId: f.agentId } }, + ]).returning(); + await server.db.insert(toolProfileBindings).values({ companyId: f.companyId, profileId: intent.id, targetType: "agent", targetId: f.agentId }); + await server.db.insert(toolProfileEntries).values([ + { companyId: f.companyId, profileId: app.id, selectorType: "connection", connectionId: privateConnection.id }, + { companyId: f.companyId, profileId: intent.id, selectorType: "connection", connectionId: owned.id }, + ]); + const path = `/tool-connections/${owned.id}/profile-configuration`; + const uidResult = await f.request("GET", `/tool-connections/${encodeURIComponent(owned.uid)}/profile-configuration`); + expect(uidResult).toMatchObject({ status: 200 }); + expect(uidResult.body.profiles.map((p: { id: string }) => p.id).sort()).toEqual([app.id, intent.id].sort()); + const foreign = await fixture(); + expect(await foreign.request("GET", `/tool-connections/${owned.id}/profile-configuration`)).toMatchObject({ status: 404 }); + for (const token of [f.key.token, f.jwt]) { + const result = await f.request("GET", path, undefined, token); + expect(result).toMatchObject({ status: 200 }); + expect(result.body.profiles.map((p: { id: string }) => p.id).sort()).toEqual([app.id, intent.id].sort()); + expect(result.body.profiles.every((p: object) => !("summary" in p) && !("newToolsPendingCount" in p))).toBe(true); + } + // Seeing the displayed connection must not expose a profile spanning a + // second account after that account becomes private to another person. + await server.db.update(toolConnections).set({ credentialPolicy: "per_user", createdByUserId: randomUUID() }).where(eq(toolConnections.id, privateConnection.id)); + for (const token of [f.key.token, f.jwt]) { + expect(await f.request("GET", path, undefined, token)).toMatchObject({ status: 200, body: { profiles: [expect.objectContaining({ id: intent.id })] } }); + expect(await f.request("GET", `/tool-connections/${privateConnection.id}/profile-configuration`, undefined, token)).toMatchObject({ status: 403 }); + expect(await f.request("GET", `/tool-connections/${randomUUID()}/profile-configuration`, undefined, token)).toMatchObject({ status: 404 }); + } + await server.db.update(toolConnections).set({ createdByUserId: f.userId }).where(eq(toolConnections.id, privateConnection.id)); + expect((await f.request("GET", path)).body.profiles).toHaveLength(2); + await server.db.delete(principalPermissionGrants).where(and(eq(principalPermissionGrants.principalId, f.agentId), eq(principalPermissionGrants.permissionKey, "tools:manage_profiles"))); + expect(await f.request("GET", path)).toMatchObject({ status: 403 }); + expect(await f.request("GET", path, undefined, f.jwt)).toMatchObject({ status: 403 }); + }); + it("rechecks authority after remote endpoint validation before committing a connection update", async () => { const f = await fixture(); const created = await f.request("POST", `/companies/${f.companyId}/tools/connections`, { name: "Before validation", transport: "mcp_remote", authKind: "none", transportConfig: { url: "https://1.1.1.1/mcp" } }); diff --git a/server/src/__tests__/connection-profile-configuration.test.ts b/server/src/__tests__/connection-profile-configuration.test.ts new file mode 100644 index 00000000000..526596d3454 --- /dev/null +++ b/server/src/__tests__/connection-profile-configuration.test.ts @@ -0,0 +1,44 @@ +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { eq } from "drizzle-orm"; +import { createDb, toolProfileBindings, toolProfileEntries } from "@paperclipai/db"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; +import { createConnectionProfileConfigurationFixture, legacyConnectionConfiguration } from "./helpers/connection-profile-configuration-fixture.js"; +import { createToolProfileService } from "../services/tool-access/profiles/service.js"; +const support = await getEmbeddedPostgresTestSupport(); +const describeDatabase = support.supported ? describe : describe.skip; +describeDatabase("connection Permissions profile configuration", () => { + let db: ReturnType, database: Awaited>; + beforeAll(async () => { database = await startEmbeddedPostgresTestDatabase("connection-profile-configuration-"); db = createDb(database.connectionString); }); + afterAll(async () => { await db?.$client.end({ timeout: 1 }); await database?.cleanup(); }); + it("matches the old selected view with full ordered entries and bindings, before and after changes", async () => { + const f = await createConnectionProfileConfigurationFixture(db, { profiles: 8, tools: 12, rules: 4 }); + const service = createToolProfileService(db, () => new Date("2026-02-01T00:00:00Z")); + const visible = async (rows: T[]) => rows; + const compare = async () => { + const expected = legacyConnectionConfiguration(await service.listProfiles(f.company.id), f.connection.id); + const actual = await service.listConnectionProfileConfiguration(f.company.id, f.connection.id, visible); + expect(actual).toEqual(expected); + expect(actual.map(p => p.id).sort()).toEqual([...f.selectedIds].sort()); + expect(actual.every(p => !("summary" in p) && !("newToolsPendingCount" in p))).toBe(true); + return actual; + }; + let result = await compare(); + expect(result.find(p => p.id === f.selectedIds[0])!.bindings.map(b => b.priority)).toEqual([1, 2]); + expect(result.find(p => p.id === f.selectedIds[1])!.bindings).toHaveLength(2); + await db.insert(toolProfileEntries).values({ companyId: f.company.id, profileId: f.selectedIds[1], selectorType: "tool_name", toolName: "new selection" }); + await compare(); + // An unbound app profile must remain present for the install fallback. + await db.delete(toolProfileBindings).where(eq(toolProfileBindings.profileId, f.selectedIds[0])); + result = await compare(); + expect(result.find(p => p.id === f.selectedIds[0])!.bindings).toEqual([]); + const seen: string[][] = []; + const filtered = await service.listConnectionProfileConfiguration(f.company.id, f.connection.id, async rows => { + seen.push(rows.map(p => p.id)); return rows.filter(p => p.id !== f.selectedIds[1]); + }); + expect(seen[0].sort()).toEqual([...f.selectedIds].sort()); + expect(filtered.map(p => p.id)).toEqual([f.selectedIds[0]]); + expect(await service.listConnectionProfileConfiguration(f.company.id, f.connection.id, async () => [])).toEqual([]); + const foreign = await createConnectionProfileConfigurationFixture(db, { profiles: 2, tools: 3, rules: 2 }); + expect(await service.listConnectionProfileConfiguration(foreign.company.id, f.connection.id, visible)).toEqual([]); + }); +}); diff --git a/server/src/__tests__/helpers/connection-profile-configuration-fixture.ts b/server/src/__tests__/helpers/connection-profile-configuration-fixture.ts new file mode 100644 index 00000000000..1ea46603bfa --- /dev/null +++ b/server/src/__tests__/helpers/connection-profile-configuration-fixture.ts @@ -0,0 +1,51 @@ +import { randomUUID } from "node:crypto"; +import { eq } from "drizzle-orm"; +import { agents, toolConnections, toolProfileBindings, toolProfileEntries, toolProfiles, type Db } from "@paperclipai/db"; +import type { ToolProfileConfiguration, ToolProfileWithDetails } from "@paperclipai/shared"; +import { createProfileProjectionFixture } from "./tool-profile-projection-fixture.js"; + +export async function createConnectionProfileConfigurationFixture(db: Db, options: Parameters[1] = {}) { + const f = await createProfileProjectionFixture(db, options); + const companyAgents = await db.select().from(agents).where(eq(agents.companyId, f.company.id)); + const agentId = companyAgents[0]!.id; + const [sibling] = await db.insert(toolConnections).values({ companyId: f.company.id, applicationId: f.application.id, + name: "Sibling account", uid: randomUUID(), transport: "mcp_remote", status: "active" }).returning(); + await db.update(toolProfiles).set({ profileKey: `app:${f.connection.id}`, status: "disabled" }).where(eq(toolProfiles.id, f.profiles[0]!.id)); + const metadata = { source: "connection_intent", connectionId: f.connection.id, agentId }; + await db.update(toolProfiles).set({ profileKey: `connection-intent:${f.connection.id}:${agentId}`, metadata }).where(eq(toolProfiles.id, f.profiles[1]!.id)); + const rejected = await db.insert(toolProfiles).values([ + { profileKey: `app:${sibling.id}`, name: "Sibling app" }, + { profileKey: `connection-intent:${sibling.id}:${agentId}`, name: "Sibling intent", metadata: { ...metadata, connectionId: sibling.id } }, + { profileKey: `connection-intent:${f.connection.id}:wrong-key`, name: "Wrong key", metadata }, + { profileKey: `connection-intent:${f.connection.id}:${companyAgents[1]!.id}`, name: "Wrong binding", metadata: { ...metadata, agentId: companyAgents[1]!.id } }, + { profileKey: `connection-intent:${f.connection.id}:${companyAgents[2]!.id}`, name: "Inactive intent", status: "disabled" as const, metadata: { ...metadata, agentId: companyAgents[2]!.id } }, + { profileKey: `connection-intent:${f.connection.id}:123`, name: "Non-string agent", metadata: { ...metadata, agentId: 123 } }, + { profileKey: `connection-intent:${f.connection.id}:null`, name: "Null agent", metadata: { ...metadata, agentId: null } }, + { profileKey: "unmanaged", name: "Unmanaged", metadata }, + { profileKey: `connection-intent:${f.connection.id}:${companyAgents[3]!.id}`, name: "Wrong source", metadata: { ...metadata, agentId: companyAgents[3]!.id, source: "other" } }, + { profileKey: `connection-intent:${f.connection.id}:${companyAgents[4]!.id}`, name: "No binding", metadata: { ...metadata, agentId: companyAgents[4]!.id } }, + ].map(profile => ({ companyId: f.company.id, ...profile }))).returning(); + const bindingAt = new Date("2026-01-01T00:00:00Z"); + await db.insert(toolProfileBindings).values([ + { profileId: f.profiles[0]!.id, targetType: "agent" as const, targetId: agentId, priority: 2 }, + { profileId: f.profiles[0]!.id, targetType: "agent" as const, targetId: companyAgents[1]!.id, priority: 1 }, + { profileId: f.profiles[1]!.id, targetType: "agent" as const, targetId: agentId, priority: 0 }, + { profileId: f.profiles[1]!.id, targetType: "company" as const, targetId: f.company.id, priority: 3 }, + ...rejected.filter(p => p.name !== "No binding").map(p => ({ profileId: p.id, targetType: "agent" as const, targetId: agentId, priority: 0 })), + ].map(binding => ({ companyId: f.company.id, ...binding, createdAt: bindingAt }))); + // Selected profiles can carry selectors outside this account. Keep all rows. + await db.insert(toolProfileEntries).values({ companyId: f.company.id, profileId: f.profiles[0]!.id, + selectorType: "connection", connectionId: sibling.id, effect: "exclude" }); + return { ...f, sibling, agentId, selectedIds: [f.profiles[0]!.id, f.profiles[1]!.id], rejected }; +} + +/** The old AppDetail/AgentConnectionAccess selection, with only unused derived fields removed. */ +export function legacyConnectionConfiguration(profiles: ToolProfileWithDetails[], connectionId: string): ToolProfileConfiguration[] { + return profiles.filter(profile => { + const agentId = profile.metadata?.agentId; + return profile.profileKey === `app:${connectionId}` || (profile.status === "active" + && profile.metadata?.source === "connection_intent" && profile.metadata.connectionId === connectionId + && typeof agentId === "string" && profile.profileKey === `connection-intent:${connectionId}:${agentId}` + && profile.bindings.some(binding => binding.targetType === "agent" && binding.targetId === agentId)); + }).map(({ summary: _summary, newToolsPendingCount: _pending, ...configuration }) => configuration); +} diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index c766d6c4836..772b4706a19 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -12068,6 +12068,15 @@ registerCurrentRoute({ summary: "Handle a managed Vercel Connect OAuth callback", }); +registerCurrentRoute({ + "x-paperclip-transport": "rest", + "x-paperclip-connection-management": "connections:profiles", + method: "get", + path: "/api/tool-connections/{connectionId}/profile-configuration", + tags: ["tool-access"], + summary: "Read connection Permissions profile configuration without summaries or review counts", +}); + registerCurrentRoute({ "x-paperclip-transport": "rest", "x-paperclip-connection-management": "connections:profiles", diff --git a/server/src/routes/tool-access.ts b/server/src/routes/tool-access.ts index 3f3bb760e17..72209d84746 100644 --- a/server/src/routes/tool-access.ts +++ b/server/src/routes/tool-access.ts @@ -2327,6 +2327,15 @@ function connectorEnrollmentPrincipal(req: Request): string { res.json({ profiles: await filterConnectionManagementRows(db, req, "connections:profiles", await svc.listProfiles(companyId), row => ({ profileId: row.id })) }); }); + router.get("/tool-connections/:connectionId/profile-configuration", managementGuard("connections:profiles", "GET /tool-connections/:connectionId/profile-configuration", { personal: true }), async (req, res) => { + assertBoardOrAgent(req); + const existing = await getAccessibleResource(req, res, svc.getConnection(req.params.connectionId as string), "Tool connection not found"); + if (!existing) return; + const profiles = await svc.listConnectionProfileConfiguration(existing.companyId, existing.id, + rows => filterConnectionManagementRows(db, req, "connections:profiles", rows, row => ({ profileId: row.id }))); + res.json({ profiles }); + }); + router.get("/tool-profiles/:profileId/new-tools", managementGuard("connections:profiles", "GET /tool-profiles/:profileId/new-tools", { personal: true }), async (req, res) => { assertBoardOrAgent(req); const existing = await getAccessibleResource(req, res, svc.getProfile(req.params.profileId as string), "Tool profile not found"); diff --git a/server/src/services/tool-access/profiles/connection-configuration.ts b/server/src/services/tool-access/profiles/connection-configuration.ts new file mode 100644 index 00000000000..0538929ebaf --- /dev/null +++ b/server/src/services/tool-access/profiles/connection-configuration.ts @@ -0,0 +1,64 @@ +import { and, asc, desc, eq, inArray, or, sql } from "drizzle-orm"; +import { toolProfileBindings, toolProfileEntries, toolProfiles, type Db } from "@paperclipai/db"; +import type { ToolProfileConfiguration } from "@paperclipai/shared"; +import { toProfile, toProfileBinding, toProfileEntry } from "./projection.js"; + +type ProfileRow = typeof toolProfiles.$inferSelect; + +/** Saved profiles shown by connection Permissions; this is not effective access. */ +export function createConnectionProfileConfigurationReader(db: Db) { + return async ( + companyId: string, + connectionId: string, + visibleProfiles: (profiles: ProfileRow[]) => Promise, + ): Promise => { + const candidates = await db.select().from(toolProfiles).where(and( + eq(toolProfiles.companyId, companyId), + or( + eq(toolProfiles.profileKey, `app:${connectionId}`), + and( + eq(toolProfiles.status, "active"), + sql`${toolProfiles.metadata}->>'source' = 'connection_intent'`, + sql`${toolProfiles.metadata}->>'connectionId' = ${connectionId}`, + sql`jsonb_typeof(${toolProfiles.metadata}->'agentId') = 'string'`, + sql`${toolProfiles.profileKey} = 'connection-intent:' || ${connectionId} || ':' || (${toolProfiles.metadata}->>'agentId')`, + sql`exists (select 1 from ${toolProfileBindings} where + ${toolProfileBindings.companyId} = ${toolProfiles.companyId} + and ${toolProfileBindings.profileId} = ${toolProfiles.id} + and ${toolProfileBindings.targetType} = 'agent' + and ${toolProfileBindings.targetId} = ${toolProfiles.metadata}->>'agentId')`, + ), + ), + )).orderBy(desc(toolProfiles.updatedAt)); + // Preserve the full-list route's per-profile current-authority checks before + // loading configuration. Connection access alone does not grant profile access. + const profiles = await visibleProfiles(candidates); + if (profiles.length === 0) return []; + const ids = profiles.map(profile => profile.id); + const [entries, bindings] = await Promise.all([ + db.select().from(toolProfileEntries).where(and( + eq(toolProfileEntries.companyId, companyId), inArray(toolProfileEntries.profileId, ids), + )).orderBy(asc(toolProfileEntries.createdAt), asc(toolProfileEntries.id)), + db.select().from(toolProfileBindings).where(and( + eq(toolProfileBindings.companyId, companyId), inArray(toolProfileBindings.profileId, ids), + )).orderBy(asc(toolProfileBindings.priority), asc(toolProfileBindings.createdAt)), + ]); + const entriesByProfile = new Map(); + const bindingsByProfile = new Map(); + for (const entry of entries) { + const group = entriesByProfile.get(entry.profileId) ?? []; + group.push(toProfileEntry(entry)); + entriesByProfile.set(entry.profileId, group); + } + for (const binding of bindings) { + const group = bindingsByProfile.get(binding.profileId) ?? []; + group.push(toProfileBinding(binding)); + bindingsByProfile.set(binding.profileId, group); + } + return profiles.map(profile => ({ + ...toProfile(profile), + entries: entriesByProfile.get(profile.id) ?? [], + bindings: bindingsByProfile.get(profile.id) ?? [], + })); + }; +} diff --git a/server/src/services/tool-access/profiles/service.ts b/server/src/services/tool-access/profiles/service.ts index b0d9f6e8774..223ca9a9169 100644 --- a/server/src/services/tool-access/profiles/service.ts +++ b/server/src/services/tool-access/profiles/service.ts @@ -1,4 +1,5 @@ import type { ConnectionManagementContext } from "../../connection-management.js"; +import { createConnectionProfileConfigurationReader } from "./connection-configuration.js"; import { connectionActorUserId } from "../actor.js"; import { assertProfileEntryInputs } from "../../tool-profile-entry-validation.js"; import { randomUUID } from "node:crypto"; @@ -360,6 +361,8 @@ export function createToolProfileService(db: Db, now: () => Date) { } } return { + listConnectionProfileConfiguration: createConnectionProfileConfigurationReader(db), + listProfiles: async ( companyId: string, ): Promise => { diff --git a/ui/src/api/tools.ts b/ui/src/api/tools.ts index 4bf3021dfc4..1087548dfff 100644 --- a/ui/src/api/tools.ts +++ b/ui/src/api/tools.ts @@ -34,6 +34,7 @@ import type { ToolProfileEntrySelectorType, ToolProfileStatus, ToolProfileWithDetails, + ToolProfileConfiguration, ToolProfileNewToolReviewDecision, ToolProfileNewToolsReview, ToolProfileNewToolsReviewResult, @@ -480,6 +481,8 @@ export const toolsApi = { ), // --- Profiles --- + listConnectionProfileConfiguration: (connectionId: string) => + api.get<{ profiles: ToolProfileConfiguration[] }>(`/tool-connections/${connectionId}/profile-configuration`), listProfiles: (companyId: string) => api.get(`/companies/${companyId}/tools/profiles`), getProfileNewTools: (profileId: string) => diff --git a/ui/src/lib/queryKeys.ts b/ui/src/lib/queryKeys.ts index 13b265befa2..e11a10395e3 100644 --- a/ui/src/lib/queryKeys.ts +++ b/ui/src/lib/queryKeys.ts @@ -88,6 +88,8 @@ export const queryKeys = { actionRequests: (companyId: string, status: string) => ["tools", companyId, "action-requests", status] as const, gateways: (companyId: string) => ["tools", "gateways", companyId] as const, + connectionProfileConfiguration: (companyId: string, connectionId: string) => + ["tools", companyId, "profiles", "connection-configuration", connectionId] as const, profiles: (companyId: string) => ["tools", companyId, "profiles"] as const, profileNewTools: (profileId: string) => ["tools", "profiles", profileId, "new-tools"] as const, diff --git a/ui/src/pages/apps/AppDetail.test.tsx b/ui/src/pages/apps/AppDetail.test.tsx index 3650f68d88d..c227ff9ccbc 100644 --- a/ui/src/pages/apps/AppDetail.test.tsx +++ b/ui/src/pages/apps/AppDetail.test.tsx @@ -19,7 +19,7 @@ const listApplicationsMock = vi.hoisted(() => vi.fn()); const listGalleryMock = vi.hoisted(() => vi.fn()); const listConnectionsMock = vi.hoisted(() => vi.fn()); const listCatalogMock = vi.hoisted(() => vi.fn()); -const listProfilesMock = vi.hoisted(() => vi.fn()); +const listConnectionProfileConfigurationMock = vi.hoisted(() => vi.fn()); const listPoliciesMock = vi.hoisted(() => vi.fn()); const listConnectionActivityMock = vi.hoisted(() => vi.fn()); const listActionRequestsMock = vi.hoisted(() => vi.fn()); @@ -60,7 +60,7 @@ vi.mock("@/api/tools", () => ({ listGallery: (companyId: string) => listGalleryMock(companyId), listConnections: (companyId: string) => listConnectionsMock(companyId), listCatalog: (connectionId: string) => listCatalogMock(connectionId), - listProfiles: (companyId: string) => listProfilesMock(companyId), + listConnectionProfileConfiguration: (connectionId: string) => listConnectionProfileConfigurationMock(connectionId), deleteProfile: (profileId: string) => deleteProfileMock(profileId), listPolicies: (companyId: string) => listPoliciesMock(companyId), listConnectionActivity: (connectionId: string, limit: number) => @@ -409,7 +409,7 @@ describe("AppDetail", () => { }), ], }); - listProfilesMock.mockResolvedValue({ + listConnectionProfileConfigurationMock.mockResolvedValue({ profiles: [ { profileKey: "app:conn-1", @@ -1034,6 +1034,15 @@ describe("AppDetail", () => { }); }); + it("fetches the connection configuration under a distinct child of the profiles cache", async () => { + await renderAppDetail(); + expect(listConnectionProfileConfigurationMock).toHaveBeenCalledWith("conn-1"); + expect(queryKeys.tools.connectionProfileConfiguration("company-1", "conn-1").slice(0, 3)) + .toEqual(queryKeys.tools.profiles("company-1")); + expect(queryKeys.tools.connectionProfileConfiguration("company-1", "conn-1")) + .not.toEqual(queryKeys.tools.connectionProfileConfiguration("company-1", "conn-2")); + }); + it("shows and removes task-granted access without changing the connection's default permissions", async () => { const grant = { id: "task-grant", status: "active", profileKey: "connection-intent:conn-1:agent-1", @@ -1041,7 +1050,7 @@ describe("AppDetail", () => { bindings: [{ targetType: "agent", targetId: "agent-1" }], entries: [{ effect: "include", catalogEntryId: "catalog-write" }], }; - listProfilesMock.mockResolvedValue({ profiles: [grant] }); + listConnectionProfileConfigurationMock.mockResolvedValue({ profiles: [grant] }); listPoliciesMock.mockResolvedValue({ policies: [{ enabled: true, policyType: "require_approval", selectors: { catalogEntryId: "catalog-write" }, @@ -1051,7 +1060,7 @@ describe("AppDetail", () => { const section = Array.from(container.querySelectorAll("section")).find(section => section.textContent?.includes("Additional agent access"))!; expect(section.textContent).toContain("Write issue"); expect(section.textContent).toContain("Ask first"); - listProfilesMock.mockResolvedValue({ profiles: [] }); + listConnectionProfileConfigurationMock.mockResolvedValue({ profiles: [] }); await act(async () => section.querySelector("button")!.click()); await flushReact(); expect(deleteProfileMock).toHaveBeenCalledWith("task-grant"); @@ -1061,7 +1070,7 @@ describe("AppDetail", () => { }); it("keeps the task grant visible when removal fails", async () => { - listProfilesMock.mockResolvedValue({ profiles: [{ + listConnectionProfileConfigurationMock.mockResolvedValue({ profiles: [{ id: "task-grant", status: "active", profileKey: "connection-intent:conn-1:agent-1", metadata: { source: "connection_intent", connectionId: "conn-1", agentId: "agent-1" }, bindings: [{ targetType: "agent", targetId: "agent-1" }], @@ -1078,7 +1087,7 @@ describe("AppDetail", () => { it("persists agent access independently from always-installed agents", async () => { mockParams.tab = "permissions"; - listProfilesMock.mockResolvedValue({ + listConnectionProfileConfigurationMock.mockResolvedValue({ profiles: [{ profileKey: "app:conn-1", entries: [ @@ -1144,7 +1153,7 @@ describe("AppDetail", () => { it("uses the setup-style agent access question without install terminology", async () => { mockParams.tab = "permissions"; - listProfilesMock.mockResolvedValue({ + listConnectionProfileConfigurationMock.mockResolvedValue({ profiles: [{ profileKey: "app:conn-1", entries: [ diff --git a/ui/src/pages/apps/AppDetail.tsx b/ui/src/pages/apps/AppDetail.tsx index c57fa4806c0..bffed20f0fd 100644 --- a/ui/src/pages/apps/AppDetail.tsx +++ b/ui/src/pages/apps/AppDetail.tsx @@ -15,7 +15,7 @@ import { Check, Loader2, Pencil } from "lucide-react"; import type { ToolConnection, ToolPolicy, - ToolProfileWithDetails, + ToolProfileConfiguration, } from "@paperclipai/shared"; import { connectionDisplaySecondaryHint, @@ -133,9 +133,9 @@ function StandardAppDetail({ renderActions, renderAgentSettings, renderConnectio enabled: !!connectionId && needsCatalog, }); const profilesQuery = useQuery({ - queryKey: queryKeys.tools.profiles(selectedCompanyId ?? "__none__"), - queryFn: () => toolsApi.listProfiles(selectedCompanyId!), - enabled: !!selectedCompanyId && (activeTab === "review" || activeTab === "permissions"), + queryKey: queryKeys.tools.connectionProfileConfiguration(selectedCompanyId ?? "__none__", connectionId), + queryFn: () => toolsApi.listConnectionProfileConfiguration(connectionId), + enabled: !!selectedCompanyId && !!connectionId && (activeTab === "review" || activeTab === "permissions"), }); const policiesQuery = useQuery({ queryKey: queryKeys.tools.policies(selectedCompanyId ?? "__none__"), @@ -923,7 +923,7 @@ function StatusBadge({ status }: { status: StatusInfo }) { ); } -function enabledCatalogIds(profile: ToolProfileWithDetails | undefined): Set { +function enabledCatalogIds(profile: ToolProfileConfiguration | undefined): Set { const ids = new Set(); for (const entry of profile?.entries ?? []) { if (entry.effect === "include" && entry.catalogEntryId) ids.add(entry.catalogEntryId); @@ -959,7 +959,7 @@ function askFirstCatalogIds(policies: ToolPolicy[], connectionId: string): Set Date: Mon, 2 Nov 2026 15:01:52 -0800 Subject: [PATCH 03/57] fix: Compare native finaliser lease expiry at millisecond precision when releasing an expired owner (#17214) > [!NOTE] > Opened by Prospector's automated issue-fix pipeline. A maintainer reviews and decides; nothing here merges on its own. ## Thinking Path - Issue #16715 reports a defect in paperclipai/paperclip. - Root cause, as the fixing agent read it: Lease renewal writes 'lease_expires_at' from the database clock ('now() + interval'), so the stored value has microseconds. 'agentExecutionsHaveStopped' reads that value into a millisecond JS 'Date' and then releases the row with an exact 'eq' on it. The update never matches, so the stop check returns false on every tick and the agent stays in 'pausing'. - This pull request: Compare native finaliser lease expiry at millisecond precision when releasing an expired owner ## What Changed - `server/src/services/agent-execution-stop.ts`: The release update now truncates the stored lease expiry to milliseconds before comparing it with the value it read, using the same 'date_trunc('milliseconds', ...) = ...::timestamptz' pattern as 'chat-channels/activity.ts'. The other guards stay the same: owner, generation, phase and the null result. This was the only exact-match comparison on a lease expiry in 'server/src'. ## Verification - Reproducing test(s), added in this PR: `server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts` - They fail on the base commit and pass with the fix, each twice in a fresh sandbox. - 3 independent attempts wrote 3 reproductions; this fix passes every one of them. - The repository's compile command passes with the fix applied. - The full test suite passes except for the 10 file(s) that already fail on the base commit. ## Risks - Written by an automated pipeline, not a person: review it as you would an outside contribution. - Risk tier of the touched paths: 2. - The scope reviewer found it also changes, beyond the report: agentExecutionsHaveStopped for an expired finaliser row whose lease_expires_at is already millisecond-precise (written from a JS Date) - The scope reviewer found it also changes, beyond the report: Finaliser row whose lease was renewed or taken over between the select and the update - The scope reviewer found it also changes, beyond the report: Rows with null leaseExpiresAt ## Model Used - codex:gpt-6.1-sol, opus, sonnet (Anthropic Claude, via Claude Code; OpenAI, via the Codex CLI), writing and checking the change in an isolated sandbox. - opus (Anthropic Claude, via Claude Code), reviewing it independently. ## Linked Issues Fixes #16715 Other pull requests that name #16715: - #17204 (open): fix(heartbeat): stop loading full run rows and stale pids when pausing an agent - #16745 (open): fix(agent-lifecycle): release an expired native finalizer lease at millisecond precision ## 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) - [ ] 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) - [ ] 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 - [ ] 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 - [ ] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: commitperclip[bot] <284812584+commitperclip[bot]@users.noreply.github.com> --- .../agent-lifecycle/__tests__/lifecycle.test.ts | 15 +++++++++++++++ server/src/services/agent-execution-stop.ts | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts b/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts index 404e76d6b30..ab4e53d0839 100644 --- a/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts +++ b/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts @@ -414,6 +414,21 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu expect((await db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, run.id)))[0]).toMatchObject({ leaseOwner: null, leaseExpiresAt: null }); }); + it("releases an expired native owner whose renewed lease expiry has microsecond precision", async () => { + const agent = await hire(); + const [issue] = await db.insert(issues).values({ companyId, title: "Renewed native run" }).returning(); + const [run] = await db.insert(heartbeatRuns).values({ companyId, agentId: agent.id, nativeIssueId: issue.id, + status: "cancelled", invocationSource: "on_demand", runtimeMode: "native", + startedAt: new Date(), finishedAt: new Date(), processPid: 2_000_000_000 }).returning(); + await db.insert(nativeRunFinalizations).values({ companyId, issueId: issue.id, runId: run.id, + phase: "executing", leaseOwner: "stopped-server", leaseExpiresAt: new Date(0) }); + await db.update(nativeRunFinalizations) + .set({ leaseExpiresAt: sql`'2020-01-01T00:00:00.123456Z'::timestamptz` }) + .where(eq(nativeRunFinalizations.runId, run.id)); + expect(await agentExecutionsHaveStopped(db, [agent.id])).toBe(true); + expect((await db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, run.id)))[0]).toMatchObject({ leaseOwner: null, leaseExpiresAt: null }); + }); + it("does not advance termination while a cancelled process still runs", async () => { const agent = await hire(); const [run] = await db.insert(heartbeatRuns).values({ companyId, agentId: agent.id, diff --git a/server/src/services/agent-execution-stop.ts b/server/src/services/agent-execution-stop.ts index 3fad14e89b1..92e9e5964bb 100644 --- a/server/src/services/agent-execution-stop.ts +++ b/server/src/services/agent-execution-stop.ts @@ -64,7 +64,7 @@ export async function agentExecutionsHaveStopped(db: Db, agentIds: string[]) { const [cleared] = await db.update(nativeRunFinalizations).set({ leaseOwner: null, leaseExpiresAt: null, updatedAt: now }) .where(and(eq(nativeRunFinalizations.runId, coordinator.runId), eq(nativeRunFinalizations.leaseOwner, coordinator.leaseOwner!), - eq(nativeRunFinalizations.leaseExpiresAt, coordinator.leaseExpiresAt!), + sql`date_trunc('milliseconds', ${nativeRunFinalizations.leaseExpiresAt}) = ${coordinator.leaseExpiresAt!.toISOString()}::timestamptz`, eq(nativeRunFinalizations.controllerGeneration, coordinator.controllerGeneration), eq(nativeRunFinalizations.phase, coordinator.phase), isNull(nativeRunFinalizations.resultId))) .returning({ runId: nativeRunFinalizations.runId }); 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 04/57] 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 (