From 100b364264c225085fb6633f11bde002f9afde19 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 30 Oct 2026 22:02:58 -0500 Subject: [PATCH] Preserve selected Google services when setup resumes (#18048) 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. > - Connections control which external services an agent can use. > - Google Workspace setup lets a person choose services and read or write access. > - A draft lost that choice after a declined or expired OAuth handoff. > - Finish setup could then show all services with write access instead of the saved narrow choice. > - This pull request saves the exact draft choice without treating it as granted access. > - The benefit is safe setup recovery that preserves the person's choice and existing account limits. ## Linked Issues or Issue Description Refs #15545. The duplicate search found no open PR for Workspace draft consent recovery. Related #18000 concerns satisfied gateway access requests, not saved OAuth setup intent. **What happened?** A cold Workspace draft resumed with the new-account service defaults. The selected request lived only in short-lived OAuth state. Denial consumed that state. The picker ignored draft connections. **Expected behavior** Finish setup restores the same person's saved service choice. It does not use another person's choice, a broad profile hint, or a credential grant as draft intent. An old draft with no reliable choice asks the person to choose services. **Steps to reproduce** 1. Start a new Google Workspace connection. Select Gmail read-only. Turn off Drive and Calendar. 2. Continue to Google, then decline authorization. 3. Close the setup page. Return through Finish setup. 4. Open the service picker. Before this fix, all three services can be enabled with write access. **Paperclip version or commit** Reproduced in tests on master `820c458465fa40137c81fe510f3e6dc573b4f895`. **Deployment mode** Authenticated and local source builds. The source regression uses real PostgreSQL fixtures and the production React setup components. Live provider acceptance of this repair is still pending deployment. ## What Changed - Save validated, non-secret draft intent before managed or customer-owned OAuth. Bind it to company, connection, actor, grant kind, and subject. - Restore that intent after denial, state expiry, and cold Finish setup. Keep active reconnect bound to its saved grant. Do not change new-account defaults. - Preserve agent-to-human setup handoffs. Consume only the addressed person's reviewed proposal. Keep other people's proposals. - Lock and merge the current draft before saving intent. Return 409 if its credential identity or method changed during setup. - Preserve the latest saved choice across overlapping customer-owned starts. Merge both current configuration records under short locks. Reject stale client-binding changes. Keep each OAuth state's own requested scopes. - Add shared, service, handoff, picker, and cold-page regression tests. Update the connector contract and existing feature plan. - Route the default ungrouped test/release paths through six sequential exact-cover chat partitions. Preserve remaining server/native coverage, explicit grouped CI, selection-first execution and shared-database paths. - Correct the retention test driver so pending owned real I/O does not advance logical deadlines. Pump zero-duration connection timers and add a held-I/O regression; keep production scheduling and original timing assertions unchanged. The shared helper is needed by the server and picker to validate the same narrow binding. It does not resolve credentials or grant access. No production schema migration, new route, provider scope or deployment configuration is added. The test-harness changes do not change production timers. ## Verification ### Current candidate: source qualification passed; live acceptance pending - Current published candidate: `82d13d4d698a5579378b5d506cb7a1f9cb64b5c8`, tree `78d6a821ce53f481fe3e9991edce56f0acffb22f`. It combines the Workspace repair with launcher commit `bf29c9` and retention-test commit `e25987`, then integrates master `493e229d93ba7798e80152f4549e2e9177ce50ae`. The integration brings 40 upstream commits across 713 files. One launcher conflict keeps upstream validated CI selection and its selection-first path, then applies six default chat partitions only without a selection. The authored PR diff has 16 files, 833 additions and 100 deletions; it adds no authored lockfile or `.github` changes. The public head is confirmed at this revision. Fresh CI and review passed as recorded below. Source qualification is complete using exact-tree full CI and the local results below. The separate local whole command was interrupted by host disk protection and is not called a pass. Live repaired-draft acceptance remains pending deployment. - Exact clean `82d13d4` passed one Node 24.19.0 builtin-only run: 41/41 cases (37 launcher and four CI-workflow cases), zero failures/skips. HEAD, tree and relevant file hashes stayed unchanged and clean before, at start and after. This proves launcher fixtures, not real chat execution, current typecheck/build, the dependency-backed selection graph, or full feature acceptance. - The new default ungrouped whole command uses six sequential exact-cover chat partitions, then the remaining server suites, workspaces and serialized suites: 28 expected invocations. It preserves all native suites, including upstream usage coverage. The plain ungrouped release launch path also changes. Explicit grouped CI, validated selection and shared-database execution stay unchanged. Launcher hard RED had three expected body failures and two unchanged passes; the repaired focused five and final 37 passed. A later 41-case staged-merge pass is retained separately; the clean 82 run above is the frozen proof. - Fresh [CI run 41153113944](https://github.com/paperclipai/paperclip/actions/runs/41153113944) completed successfully on October 31 at 01:14:53 UTC. Current head has 52 successful check runs, four conditional skips and no pending/failing checks; legacy Snyk also passed. The skips are test-file selection, the optional paired benchmark and two Storybook checks. The full server/chat/workspace/serialized matrix, all eight e2e shards, normal build, build-gap typecheck, Runner checks and final aggregate gates passed. Build and all three chat job logs identify checkout `e5e831fd3a8534d0d0f1e892118c0f8fbe12dd92`, merging this head into `493e229d93`. Its Git tree is exactly `78d6a821ce53f481fe3e9991edce56f0acffb22f`, identical to the clean candidate. The six real chat partitions passed 314, 358, 273, 239, 321 and 311 cases: 1,816 total. Other cases shown as skipped in each partition were selection exclusions, not extra passed cases. This is current tree-equivalent CI proof, not a pass of the separate default ungrouped local whole command. - Fresh [Greptile summary](https://github.com/paperclipai/paperclip/pull/18048#issuecomment-6473856992), edited October 31 at 01:05:04 UTC, explicitly reviews exact head `82d13d4` at 5/5 with no actionable code issue. Its exact-head check passed. The prior overlapping-start discussion is resolved. This review is source-only, not provider or live acceptance. - Exact clean `82d13d4` passed full recursive typecheck and normal `pnpm build` on the independent darwin/arm64 snapshot. Typecheck finished on October 31 at 01:27:33 UTC; build finished at 01:28:33 UTC. Both exited 0. Normal native preparation and code signing passed. Source HEAD, tree and tracked/untracked state stayed unchanged and clean. `pnpm check:token-gates` also passed all token, palette and ClipLab checks on this candidate. The earlier forced-clone `ENOSYS` preflight remains recorded as setup failure, not an assertion result. - The unchanged default `pnpm test:run` started at 01:51:33.796 UTC on clean exact `82d13d4`. All six real chat partitions passed: 358, 314, 273, 239, 311 and 321 cases, or 1,816 actual cases. The original FIFO and ambiguous-retry cases both passed under their unchanged 15-second limits. General-server then reported 37 completed file rows with 5,438 passed cases and five default remote-continuity skips, including all 498 tool-access service cases and all 83 connection-route cases. At 02:39:44 UTC, the unchanged 5-GiB host-disk guard terminated the owned run. The child ended with SIGTERM and the container/harness exited 143, with no OOM or recorded assertion failure. Only seven of 28 RUN invocations started. Retention had no completed local row. Later invocations did not run. This whole local command is interrupted/incomplete, not passed. - A coverage check found no default suite missing from current full PR CI. The maintained launcher tests prove exact coverage for all 12 PR server shards, chat and five native suites; all configured workspace projects; and all nine serialized shards. The final Runner CI shard executes the five native server suites. The 41 local launcher/workflow cases cover six sequential chat children, per-child isolation, native retention, fail-fast propagation and unchanged explicit selection/shared-database paths. The actual six-chat run above exercises that changed default path. The remaining unqualified local difference is uninterrupted sequential whole-command composition and resource accumulation, not omitted test selection. Current exact-tree full CI supplies broad source-test qualification; no local whole-pass claim or provider acceptance is inferred. - A task-owned remote runner was considered to complete that separate composition check. Its provider rejected the requested allocation with per-sandbox limits of 8 GiB RAM and 10 GiB disk. No sandbox was admitted and no remote tests ran. No tests, timeouts, assertions, database synchronization, source or provider settings were changed to avoid the capacity limit. Historical a0 failures remain below. No deployment or live repaired-draft acceptance has run. ### Bounded follow-up on the prior source - On a0, one finite phase-traced chat diagnostic executed exactly the two failed cases. Both passed (1,374 and 1,767 ms); 1,814 other cases were filtered, not passed. This did not reproduce a blocked await or establish the original monolithic timeout cause. No assertion or 15-second limit changed. - The retention diagnosis preserved an initial zero-body selector failure, then corrected only the selector: one original case passed in 783 ms, with 42 filtered. Trace showed the old settling helper advancing logical time while real I/O was active. A new held-I/O regression then failed hard because the old helper advanced 1,000 ms. This proves a test-driver defect, not the precise cause of the old 5,340 ms full-run gap. - The first manual-clock repair failed the complete 44-case file: 43 passed, one body hit 15 seconds and cleanup hit 30 seconds. Its planned mutant did not run. Source then identified a fresh PostgreSQL connection's zero-duration timer. The corrected driver pumps zero-duration timers while initial readiness or tracked I/O is pending, without advancing logical Date. That pre-integration file passed 44/44 with no skips. A separate scratch-only 5,000 ms normal-continuation mutant failed the original hard `5000 < 3000` assertion; its expected failure is not a test pass. The original page-count, fairness, expiry and quiet assertions stayed intact. The authored repair does not change the production worker. The merged master changes an older retention case and dependencies; these pre-integration results are not a complete 82 retention or whole-suite pass. ### Historical qualification: SHA-bound, not 82 acceptance - Historical a0 candidate: `a0f9089fc1ff466bf4e1eeddc5b1a910d09e380b`, tree `b9d73713319dcd52b9eaa6053008b5190e43fdf1`. It adds the overlapping-start repair to parent `6c18`, which already merged pinned master `1511c92965325249ed6b52dd5be52eb6d736fbfa`. That candidate changed 12 files. Its CI and review passed; its local full qualification remained failed/incomplete. - Greptile's previous `6c18` review found P1 [Latest service choice gets lost](https://github.com/paperclipai/paperclip/pull/18048#discussion_r4425965828), confidence 4/5. Two hard-assert regressions reproduced it on untouched `6c18`: the already-bound path overwrote the later choice, and real first-use client stamping also restored the old choice. Existing OAuth states/grants were not widened, but a later setup could restore an older request. The new repair covers both write boundaries without holding locks over external work or copying another attempt's scopes. - The complete affected service file passed 498 tests with no failures or skips. All 31 required controls ran, including both unchanged RED cases, 14 new client-drift cases, seven earlier identity guards, reconnect/cold-resume cases and upstream transaction tests. The tested three-file patch is byte-identical to this commit; the connector contract update is separate. Independent scoped review found no additional actionable issue. Fresh Greptile reviewed exact `a0f` at 5/5 with no actionable findings. The previous P1 thread is resolved. - On exact `a0f`, the five shared/UI files passed 312 tests. An initial complete connection-management run then failed: 74 passed, eight timed out at the unchanged 15-second limit, and one AI-account verification returned 422 instead of 201. The Google addressed-user handoff passed. The container exited 1 without OOM and stopped before chat/token/whole tests. This material failure remains retained and its cause unclassified; the later controlled recovery below is not a causal explanation. - One diagnostic executed the first failed gateway case and the two AI cases with temporary phase-only tracing. All three passed at 8.516, 5.511 and 3.523 seconds; 80 other cases were filtered. Original hard assertions and timeouts stayed unchanged. No timeout or suspected late provider cleanup reproduced, so the complete-file failure remains unresolved. Vitest exited 0; the harness exited 1 because its trace parser missed ANSI prefixes. All 203 events were recovered from retained logs without rerunning tests. Instrumentation was removed. This is not acceptance. - Historical a0 CI [41097596364](https://github.com/paperclipai/paperclip/actions/runs/41097596364) completed successfully: 52 successful checks, two expected Storybook skips and no failed/pending checks. Legacy Snyk passed. All 48 CI jobs passed, including all 1,816 chat cases and explicit passing lines for the original 20 local failures. [Server 11/12](https://github.com/paperclipai/paperclip/actions/runs/41097596364/job/124207469276) explicitly passed the complete 83-case connection file in 129.49 seconds, including all nine local failure titles. The actual checkout was synthetic merge `06851da6eb969d63e1c7dc9f7c95f08d09fa4ff1`, combining `a0f` with `aa58fa107389e8b9ae5949f7ad815943dfd94292`. This is not standalone local `a0f` qualification and does not erase the nine local route failures. - Linux admission uses Node 24.22.2, pnpm 9.15.4 and Rust 1.97.1. Frozen offline install, exact native hashes and real migrated PostgreSQL 18.1 passed. Native/protocol/build/lock inputs are identical between `6c18` and `a0f`; validated cache reuse retains the actual `6c18` native-build provenance. The selected packaged release binary is not called a fresh `a0f` build. Database synchronization settings stayed on. This admission alone does not explain the route failures. - A fresh Mac snapshot of exact `a0f` passed frozen install, workspace prerequisites, full sequential recursive typecheck and normal `pnpm build`, both on their first attempt. Typecheck completed at 17:43:05 UTC, build at 17:44:43 UTC; both exited 0 with clean tracked/untracked source. Pinned Node 24.19, pnpm 9.15.4, Rust 1.97.1, package concurrency one, Cargo jobs one and a 12 GiB V8 heap ceiling were retained. Largest-process maximum RSS was 7,668,154,368 bytes for typecheck and 10,038,837,248 bytes for build, not aggregate tree peaks. The first build correctly withheld its freshness stamp because 125 ignored, pinned Copilot assets appeared during preparation; exact path/hash analysis explained this. The normal build then recorded a genuine stamp and reused Cargo outputs in 1.18 seconds. No manual stamp, source workaround or gate retry. This is darwin/arm64 static/build proof, not Linux packaging/runtime or full tests. - The historical a0 candidate's token checker passed all four gates across 1,410 files, run directly with pinned Node 24 against the script behind `pnpm check:token-gates`. - The single controlled complete-file comparison passed: baseline `6c18` 83/83 in 287.07 seconds, then candidate `a0f` 83/83 in 293.03 seconds, with zero skips. All nine earlier failure titles passed on both. Both child/container exits were 0, no OOM/interruption; source and native/resolver/PostgreSQL guards matched. This ran only after all owned Mac compilation ended, with fresh processes/databases and identical four-CPU-quota/6-GiB/overlay inputs. No filtering, instrumentation, assertion/timeout/source changes or retry loop. This establishes standalone complete-file recovery, not the cause of the earlier eight timeouts/AI 422. - The first unchanged full `pnpm test:run` on exact clean `a0f` ran from October 30 at 18:01:43 UTC to 21:06:19 UTC. It failed naturally with test and container exit 1, without OOM. General-server reported 1,257 passed, one failed and seven skipped files; 28,633 passed, one failed and 103 skipped tests. Only the first of 22 invocations ran. The later four groups and 21 invocations did not run. The sole failure was the unchanged chat-shard CLI meta-test: its nested projectless pnpm lookup hit the offline network guard. This was a failed full gate, not Google provider acceptance. - A finite environment pair reproduced that one meta-test failure, then passed the same unchanged case after supported `corepack prepare pnpm@9.15.4 --activate` as the existing non-root node user. The cached pnpm payload was identical. Only the missing node-user last-known-good record changed. Core source, dependencies, lockfile, fixtures, assertions, timers and offline guards stayed unchanged. This isolates the nested collection defect; it does not explain the earlier route failures or complete the full gate. - The corrected unchanged full command ran from October 30 at 21:29:03.500 UTC to October 31 at 00:02:44.207 UTC on the same exact clean `a0f`. It failed naturally with exit 1, signal null and no OOM. Source stayed clean before, at start and after. It retained two visible CPUs/quota, 6 GiB, private IPC, normal overlay, matching native inputs, synchronized real PostgreSQL 18.1, and the default five groups/22 invocations. Only the first general-server invocation ran; the other 21 did not run. General-server reported 1,265 files: 1,256 passed, two failed and seven skipped. It reported 28,737 tests: 28,631 passed, three failed and 103 skipped. The two chat cases emitted `Test timed out in 15000ms` at `chat-channels.integration.test.ts:77310` and `:81790`, with durations of 15,045 and 15,058 ms. These were the durable pre-run FIFO notice and ambiguous GitHub text restart/retry cases. The retention case emitted `expected 5340 to be less than 3000` at `skill-version-retention.test.ts:593`. Its real duration was 1,727 ms; 5,340 ms was the recorded fake-time directory-page gap, not a deadline timeout. The timeout stacks do not identify the blocked internal await. The cause of these failures remains unclassified. Inside this same failed run, the complete 83-case connection file and 498-case service file passed. The former Corepack meta-case also passed in 6,396 ms. These component passes do not waive the failed/incomplete full gate. No rerun, cancellation, timeout/assertion change or full-pass claim. A later fresh two-case phase trace passed both chat cases; it did not reproduce or establish the original timeout cause. At that checkpoint, the PR stayed draft. - Prior `6c18` targeted checks passed 930 tests across eight files and token gates. Its selected queue group passed all 53 cases, including the original 20 failures and 12 new named controls. Backlog cases took 10.49 and 8.45 seconds under the unchanged 15-second limit. Another 1,763 chat cases were intentionally filtered. This is historical targeted recovery, not a full pass or proof of the new repair. No timeout or assertion was relaxed. - Prior `6c18` full local qualification failed during serial typecheck: server TypeScript was killed with code 137. Docker confirmed OOM in the 6 GiB container; full tests and build never started. The failure is retained. The new Mac resource plan avoids repeating that undersized run and does not change the shared Docker VM. - Prior CI [41089793558](https://github.com/paperclipai/paperclip/actions/runs/41089793558) completed successfully for synthetic merge `a73bffcfa3ede051961bcabcfa576c703d10c932` (`6c18` plus `abc270ee4c03e13b4d6eaad156a41b4ebed4bb0a`). All 48 CI jobs passed; all 1,816 chat cases passed, including explicit log lines for the original 20. Check runs were 51 success, two expected skips and one Greptile failure; legacy Snyk passed. These results do not qualify `a0f` or erase its local route failures. - Red proof on the unmodified master baseline: cold picker and Finish setup restore broad defaults; managed and customer-owned drafts lack saved intent. The stale setup tests also fail before the lock guard. - Live staging baseline `b25f3e2af8e77d882cd22d36ca55640e6468e46f` also reproduces the ordinary failure: select Gmail read-only only, cancel at Google, reload the connector catalog, then Finish setup. All three groups return enabled at read/write. Authorization was declined; no new Google access was granted. This is baseline failure evidence, not a deployed-fix pass. - Historical affected suites: 800 tests passed before the final lock guard. The final focused service and handoff cases passed 126 tests after that guard. These are not results for the integrated candidate. - Historical `pnpm -r typecheck` and final lock-guard server typecheck passed. The historical a0 candidate passed the full recursive gate separately, as recorded above. - `pnpm build`: passed again on the final lock-guard source in commit `1c31cde0ed199fbf4e41b7649590554f7886dfc2`. - Historical `pnpm check:token-gates` passed; the historical a0 candidate's direct checker pass is recorded above. - `pnpm test:run` on macOS failed on the frozen commit. It exited 1 after 9,432.41 seconds. General server results: 1,195 files passed, 10 failed, eight skipped; 26,706 tests passed, one timed out, 157 skipped. Nine setup hooks timed out while starting embedded PostgreSQL. The retained log records host-wide semaphore exhaustion (`semget`), not disk exhaustion. One unchanged publication-text performance case also timed out. Its cause is not established. Later workspace and serialized groups did not run. This is a failed and incomplete local gate. - The independent full-source Linux run also failed and was incomplete on the same frozen commit. Native and real-database admission passed. Chat integration reported 20 failures in 1,593 tests. Tool-access passed all 479 tests. Native-session-executor passed all 965 tests. Docker interrupted the run after about 48 minutes on October 29. The container exited 143 and was not OOM-killed. The run produced no final group totals or detailed assertion summary. Shell trap metadata reports zero, but this is not a valid test-pass result. Source and evidence remain unchanged. The focused diagnosis below retains those failures; their cause is not assumed from the interruption. Neither historical local full run passed. - A fresh focused replay on the frozen source then executed exactly those 20 case bodies. All 20 failed; this was not a setup-hook failure. The two backlog cases reached the unchanged 15-second timeout. The 18 FIFO cases failed on missing notices or receipt rows. All 18 FIFO cases passed when run alone in a fresh process and database. This supports an order interaction, not a complete cause or baseline conclusion. A separate two-backlog control used RAM-backed temporary storage and real PostgreSQL with synchronization settings on. One backlog case passed in 10.65 seconds; the other still timed out at 15 seconds. No source, assertion or timeout was changed. The full local gate remains open. - Earlier mixed-revision run: it began before the final guard and imported older source before collecting later regression cases. It failed those new cases with that older cached module. This run is retained, not counted as candidate acceptance. The fresh focused run passed all seven stale-setup cases. No case was skipped to hide a failure. - Historical remote gates for `1c31` were terminal and clean: 53 successful contexts and two expected Storybook skips. Main CI [40720536514](https://github.com/paperclipai/paperclip/actions/runs/40720536514) passed. Greptile reviewed exact head `1c31cde0ed199fbf4e41b7649590554f7886dfc2` at 5/5 with no actionable findings; the separate review job also passed, and there were no inline review comments. These results do not qualify `6c18`. - CI source qualification: the chat jobs checked out synthetic merge `ef36746b3d2ec52ba5727dcbe4da265a03421fe4`, which merges the PR into `7d75ae0569e41786640cf09647ae8849411d2f22`. All 20 locally failing case names have explicit passing CI log lines. These passes do not establish standalone local acceptance of `1c31`. The merge adds two upstream Teams tests, so it collects 1,595 chat cases instead of 1,593. Neither backlog/FIFO assertions nor their timeouts changed. - Independent source review found an earlier credential-identity race. The guard and seven deterministic cases resolve that finding. A later integration review missed the separate overlapping-start race. Both RED failures and that review miss are retained. The narrow follow-up repair passed 498 service cases, independent source review and fresh remote CI/5-of-5 review; full local qualification remains incomplete. - Source qualification is ready for merge. Live repaired-draft recovery, delivery and Google approval are not established by these results. The local whole command remains explicitly incomplete. ## Risks - Historical drafts without a matching intent now need an explicit service choice. This is deliberate and prevents silent broadening. - A concurrent credential or method edit now returns 409. Reloading lets the person review the changed setup. - Overlapping starts keep separate requested scope sets. Updating draft intent does not relabel an earlier OAuth state's consent request. - Saved intent is not consent. Provider-reported scopes and current action policies still control discovery and dispatch. - No migration is needed. No existing token or grant is rewritten by saving draft intent. - Default ungrouped launch paths now execute six chat children in sequence and 28 whole-command invocations. Exact coverage, local launcher contracts and all six real default chat partitions passed. A complete uninterrupted local whole-command run remains unqualified because of host capacity. - The retention test-clock repair depends on owned-I/O and zero-duration connection-timer progress. The controlled regression, sensitivity mutant and current exact-tree CI protect the original assertion. It did not reach a completed row in the interrupted local whole run. ## Model Used OpenAI Codex, described by the runtime as GPT-5 based. The exact runtime model ID and context-window size are not exposed to this task. Used reasoning, code execution, browser verification, and independent delegated review. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used, including the runtime metadata limit - [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 described the issue in-PR following the bug report template - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal ticket or instance identifier - [x] I have run tests locally and they pass — exact clean 82 passed 41 launcher/workflow cases and all 1,816 real default chat cases, full recursive typecheck, normal build and token gates. Full exact-tree PR CI passed. The separate local whole `pnpm test:run` was interrupted by disk protection and remains incomplete; it is not claimed as a local whole 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 risks above - [x] All Paperclip CI gates are green — current exact-head checks passed; the verified CI checkout tree equals clean 82. The separate interrupted local whole command is disclosed above. - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups — fresh exact-82 summary and check passed; the earlier finding is resolved. This is source review, not acceptance. - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/DEVELOPING.md | 11 +- doc/connections/GOOGLE-WORKSPACE.md | 19 ++ ...0-08-unified-google-workspace-connector.md | 18 ++ .../src/google-workspace-connectors.test.ts | 15 + .../shared/src/google-workspace-connectors.ts | 25 ++ .../run-vitest-stable-shard.test.mjs | 132 +++++++- scripts/run-vitest-stable.mjs | 23 +- .../connection-management.integration.test.ts | 8 + .../__tests__/skill-version-retention.test.ts | 64 +++- .../src/__tests__/tool-access-service.test.ts | 296 +++++++++++++++++- server/src/services/tool-access.ts | 3 + .../tool-access/oauth/authorization-start.ts | 118 ++++--- .../tool-access/oauth/client-registration.ts | 121 ++++--- .../GoogleWorkspaceServices.test.tsx | 31 ++ .../connections/GoogleWorkspaceServices.tsx | 19 +- ui/src/pages/apps/AppsConnect.test.tsx | 30 ++ 16 files changed, 833 insertions(+), 100 deletions(-) diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 0204ccbebb6..b4bec17b31c 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -991,7 +991,16 @@ Use the cheap local default unless you are specifically working on browser flows pnpm test ``` -`pnpm test` runs the Vitest suite only. For interactive Vitest watch mode use: +`pnpm test` runs the Vitest suite only. Its default chat integration coverage runs +first in six sequential, exact-cover child processes, each with its own home, +config and embedded PostgreSQL. The remaining server invocation excludes that +chat file once and retains the native Runner suites. This bounds shared fixture +and process lifetime without increasing concurrency or changing case assertions +or timeouts. An explicit `PAPERCLIP_TEST_DATABASE_URL` retains the original +single, serial database path. Explicit `--group` invocations and CI shard routing +are unchanged. + +For interactive Vitest watch mode use: ```sh pnpm test:watch diff --git a/doc/connections/GOOGLE-WORKSPACE.md b/doc/connections/GOOGLE-WORKSPACE.md index 993a8cf31b7..d810dcc2437 100644 --- a/doc/connections/GOOGLE-WORKSPACE.md +++ b/doc/connections/GOOGLE-WORKSPACE.md @@ -14,6 +14,25 @@ permissions; discovery skips services without grants and excludes unsupported actions. The gateway checks actual grant scopes again before each call, including after refresh. Requested scopes alone never authorize an action. +Before managed or customer-owned OAuth opens, Paperclip saves the validated +service choice as non-secret setup intent on the draft. It is bound to the +company, connection, initiating actor, grant kind, and credential subject. +Declined or expired authorization and a cold **Finish setup** return restore +that choice, not the new-account write defaults. Historical drafts without a +matching saved intent require an explicit choice. Reconnecting an established +account without a new choice keeps its exact grant request. + +Overlapping customer-owned starts preserve the latest saved draft choice through +client binding and authorization metadata updates. Each OAuth state and consent +URL keeps that attempt's own requested scopes. These updates preserve unrelated +current settings and other users' pending proposals in both configuration records. + +An agent can hand setup to its responsible human without choosing services or +creating provider OAuth state. If the agent proposes services, the human can +review and change them in the picker. Saving the human's choice consumes only +that person's pending proposal, so an old proposal cannot replace their choice +after a declined sign-in. Neither draft intent nor a proposal is consent. + Discovery isolates service outages. Healthy services remain usable while the connection's health message identifies unavailable services and recommends a catalog refresh. Failed services' cached actions are disabled until rediscovered; diff --git a/doc/plans/2026-10-08-unified-google-workspace-connector.md b/doc/plans/2026-10-08-unified-google-workspace-connector.md index dd951ea77c0..481f217c17c 100644 --- a/doc/plans/2026-10-08-unified-google-workspace-connector.md +++ b/doc/plans/2026-10-08-unified-google-workspace-connector.md @@ -37,3 +37,21 @@ validation and token gates pass. The production setup components were checked in a background browser using simulated Storybook data: one profile, personal access and OAuth method switching. Full regression checks and PR review are in progress. No live Google proof yet. + +## Draft resume follow-up — 2026-10-28 + +The setup draft now retains its validated service request before either managed +or customer-owned OAuth handoff. This non-secret intent is bound to the company, +connection, initiating actor, grant kind, and credential subject. Denial, expired +OAuth state, and a cold Finish setup return restore that exact request rather +than the new-account write defaults. Historical or differently owned drafts +require an explicit service choice; existing reconnects keep their grant request. + +Agent handoffs still open the addressed human's picker without issuing provider +state or inventing service access. A human's reviewed choice consumes only their +own pending service proposal, so an older proposal cannot override that choice +on the next draft resume. Other users' proposals remain unchanged. + +This follow-up is local to a fresh checkout of master at `820c458465fa`. +Provider-fixture/database and production-component tests cover the changed +journey; they do not establish live Google consent, merge, or deployment. diff --git a/packages/shared/src/google-workspace-connectors.test.ts b/packages/shared/src/google-workspace-connectors.test.ts index 2a3eae7217d..56ce31604a1 100644 --- a/packages/shared/src/google-workspace-connectors.test.ts +++ b/packages/shared/src/google-workspace-connectors.test.ts @@ -5,11 +5,26 @@ import { googleWorkspaceGrantedScopes, googleWorkspaceToolTarget, isGoogleWorkspaceToolGranted, googleWorkspaceServiceGranted, isGoogleWorkspaceScopeGrant, googleWorkspaceSelectedScopes, googleWorkspaceEffectiveScopes, GOOGLE_WORKSPACE_DEFAULT_SELECTION, + googleWorkspaceDraftRequestedScopes, } from "./google-workspace-connectors.js"; const scope = (value: string) => `https://www.googleapis.com/auth/${value}`; describe("combined Google Workspace consent", () => { + it("restores draft intent only for its exact identity and current service scopes", () => { + const binding = { companyId: "company", connectionId: "connection", actorType: "user", actorId: "owner", + grantKind: "user" as const, subjectId: "owner" }; + const draft = { version: 1, ...binding, requestedScopes: [scope("gmail.readonly")] }; + expect(googleWorkspaceDraftRequestedScopes(draft, binding)).toEqual([scope("gmail.readonly")]); + for (const field of ["companyId", "connectionId", "actorType", "actorId", "grantKind", "subjectId"] as const) { + expect(googleWorkspaceDraftRequestedScopes({ ...draft, [field]: "different" }, binding)).toBeNull(); + } + for (const requestedScopes of [[], [scope("chat.messages.readonly")], [scope("contacts.readonly")], [scope("gmail.send")], "gmail.readonly"]) { + expect(googleWorkspaceDraftRequestedScopes({ ...draft, requestedScopes }, binding)).toBeNull(); + } + expect(googleWorkspaceDraftRequestedScopes(undefined, binding)).toBeNull(); + }); + it("defaults to Gmail, combined Drive, and Calendar write scopes without Chat or People", () => { const selected = googleWorkspaceSelectedScopes(GOOGLE_WORKSPACE_DEFAULT_SELECTION); expect(selected).toHaveLength(9); diff --git a/packages/shared/src/google-workspace-connectors.ts b/packages/shared/src/google-workspace-connectors.ts index f2890ae7d6c..212243767bb 100644 --- a/packages/shared/src/google-workspace-connectors.ts +++ b/packages/shared/src/google-workspace-connectors.ts @@ -48,6 +48,31 @@ export function googleWorkspaceSelectionForScopes(scopes: readonly string[]): Go return selection; } +export interface GoogleWorkspaceDraftBinding { + companyId: string; + connectionId: string; + actorType: string | null; + actorId: string | null; + grantKind: "user" | "agent" | "organization"; + subjectId: string | null; +} + +export function googleWorkspaceDraftRequestedScopes( + value: unknown, + binding: GoogleWorkspaceDraftBinding, +): string[] | null { + if (!value || typeof value !== "object" || Array.isArray(value)) return null; + const draft = value as Record; + if (!binding.actorType || !binding.actorId || draft.version !== 1 + || Object.entries(binding).some(([key, expected]) => draft[key] !== expected)) return null; + const scopes = draft.requestedScopes; + const allowed = new Set(GOOGLE_WORKSPACE_SERVICE_GROUPS.flatMap(group => + [...group.read, ...group.write].flatMap(profile => GOOGLE_WORKSPACE_CONNECTOR_PROFILES[profile].scopes))); + if (!Array.isArray(scopes) || scopes.length === 0 || scopes.length > allowed.size + || scopes.some(scope => typeof scope !== "string" || !allowed.has(scope))) return null; + return [...new Set(scopes)].sort(); +} + const auth = (scope: string) => `https://www.googleapis.com/auth/${scope}`; const SERVICE_PROFILES: Readonly, { diff --git a/scripts/__tests__/run-vitest-stable-shard.test.mjs b/scripts/__tests__/run-vitest-stable-shard.test.mjs index 52209944424..4ca308b1f33 100644 --- a/scripts/__tests__/run-vitest-stable-shard.test.mjs +++ b/scripts/__tests__/run-vitest-stable-shard.test.mjs @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; -import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; @@ -553,6 +553,136 @@ test("shared-database and unsharded chat runs retain one serial partition", () = { PAPERCLIP_TEST_DATABASE_URL: "" }).chatVitestShards, [{ index: 0, count: 1 }]); }); +test("default chat isolation covers six partitions without dropping native suites", () => { + for (const args of [[], ["--mode", "general"]]) { + for (const workflow of ["PR", "Release", ""]) { + const env = { GITHUB_WORKFLOW: workflow, PAPERCLIP_TEST_DATABASE_URL: "" }; + const plan = dryRunJson(args, env); + const full = dryRunJson(["--mode", "general", "--group", "general-server", + "--shard-index", "0", "--shard-count", "1"], env); + assert.deepEqual(plan.chatVitestShards, Array.from({ length: 6 }, (_, index) => ({ index, count: 6 }))); + assert.ok(!plan.selectedGeneralServerSuites.includes(chatSuitePath)); + assert.equal(new Set(plan.selectedGeneralServerSuites).size, plan.selectedGeneralServerSuites.length); + assert.deepEqual([...plan.selectedGeneralServerSuites, chatSuitePath].sort(), full.selectedGeneralServerSuites.sort()); + } + } +}); + +test("default chat isolation leaves explicit groups and shared databases unchanged", () => { + for (const args of [[], ["--mode", "general"]]) { + const shared = dryRunJson(args, { PAPERCLIP_TEST_DATABASE_URL: "postgresql://shared/test", GITHUB_WORKFLOW: "PR" }); + assert.equal(shared.chatVitestShards, null); + assert.equal(shared.selectedGeneralServerSuites, null); + } + const full = dryRunJson(["--mode", "general", "--group", "general-server", + "--shard-index", "0", "--shard-count", "1"], { PAPERCLIP_TEST_DATABASE_URL: "", GITHUB_WORKFLOW: "PR" }); + assert.equal(full.chatVitestShards, null); + assert.ok(full.selectedGeneralServerSuites.includes(chatSuitePath)); + assert.ok(full.selectedGeneralServerSuites.includes(nativeRunnerSuitePath)); + assert.deepEqual(dryRunJson(["--mode", "general", "--group", "general-chat"], + { PAPERCLIP_TEST_DATABASE_URL: "" }).chatVitestShards, [{ index: 0, count: 1 }]); + assert.equal(dryRunJson(["--mode", "serialized"], { PAPERCLIP_TEST_DATABASE_URL: "" }).chatVitestShards, null); +}); + +function defaultLauncher({ shared = false, failureIndex = null } = {}) { + const directory = mkdtempSync(path.join(os.tmpdir(), "default-chat-launch-")); + const file = path.join(repoRoot, chatSuitePath); + const lines = readFileSync(file, "utf8").split("\n") + .flatMap((line, index) => /^\s*it\(/.test(line) ? [index + 1] : []).slice(0, 6); + assert.equal(lines.length, 6); + const cases = lines.map(line => ({ file, projectName: "@paperclipai/server", + name: `default launcher fixture ${line}`, location: { line } })); + const receipt = path.join(directory, "events.ndjson"); + const active = path.join(directory, "active"); + const fakePnpm = path.join(directory, "pnpm"); + try { + writeFileSync(fakePnpm, `#!${process.execPath} + const fs = require('node:fs'); + const args = process.argv.slice(2); + const tests = ${JSON.stringify(cases)}; + const selected = args.flatMap(arg => { + const match = arg.match(/chat-channels\\.integration\\.test\\.ts:(\\d+)$/); + return match ? [Number(match[1])] : []; + }); + if (args.includes('list')) { + const output = args.find(arg => arg.startsWith('--json=')).slice(7); + fs.writeFileSync(output, JSON.stringify(selected.length + ? tests.filter(test => selected.includes(test.location.line)) : tests)); + } else { + const entry = { args, selected, home: process.env.PAPERCLIP_HOME, + config: process.env.PAPERCLIP_CONFIG, instance: process.env.PAPERCLIP_INSTANCE_ID, + tmpdir: process.env.TMPDIR }; + const record = edge => fs.appendFileSync(${JSON.stringify(receipt)}, JSON.stringify({ ...entry, edge }) + '\\n'); + record('begin'); + if (fs.existsSync(${JSON.stringify(active)})) process.exit(91); + fs.writeFileSync(${JSON.stringify(active)}, 'running'); + setTimeout(() => { + record('end'); + fs.unlinkSync(${JSON.stringify(active)}); + process.exit(selected.includes(${JSON.stringify(failureIndex === null ? null : lines[failureIndex])}) ? 7 : 0); + }, selected.length ? 200 : 0); + } + `); + chmodSync(fakePnpm, 0o755); + const result = spawnSync(process.execPath, [script, "--mode", "general"], { + cwd: repoRoot, encoding: "utf8", timeout: 30_000, + env: workflowEnv({ GITHUB_WORKFLOW: "PR", PATH: `${directory}${path.delimiter}${process.env.PATH}`, + PAPERCLIP_TEST_DATABASE_URL: shared ? "postgresql://shared/test" : "", PAPERCLIP_CI_PROFILE: "false" }), + }); + const events = existsSync(receipt) ? readFileSync(receipt, "utf8").trim().split("\n").map(line => JSON.parse(line)) : []; + return { result, events, lines }; + } finally { + rmSync(directory, { recursive: true, force: true }); + } +} + +test("default chat isolation executes every partition sequentially before other groups", { skip: process.platform === "win32" }, () => { + const { result, events, lines } = defaultLauncher(); + assert.equal(result.status, 0, result.stdout + result.stderr); + const chat = events.filter(event => event.selected.length > 0); + assert.deepEqual(chat.map(event => event.edge), Array.from({ length: 6 }, () => ["begin", "end"]).flat()); + const starts = chat.filter(event => event.edge === "begin"); + assert.deepEqual(starts.flatMap(event => event.selected).sort((a, b) => a - b), lines); + for (const event of starts) { + assert.ok(event.args.includes("--no-file-parallelism")); + assert.deepEqual(event.args.filter(arg => arg.startsWith("--maxWorkers")), ["--maxWorkers=1"]); + } + for (const field of ["home", "config", "instance", "tmpdir"]) { + assert.equal(new Set(starts.map(event => event[field])).size, 6, `${field} must be isolated per child`); + } + assert.equal(events.findIndex(event => event.selected.length === 0), 12); + const server = events[12]; + assert.ok(server.args.includes("@paperclipai/server")); + const excluded = server.args.flatMap((arg, index) => arg === "--exclude" ? [server.args[index + 1]] : []); + assert.equal(excluded.filter(file => file === chatSuitePath.replace(/^server\//, "")).length, 1); + for (const file of [nativeRunnerSuitePath, usageDeliverySuitePath, dotRunnerSuitePath, restartRecoverySuitePath, + "server/src/services/native-runtime/plugin-runner-probe.test.ts"]) { + assert.ok(!excluded.includes(file.replace(/^server\//, "")), `default must retain ${file}`); + } +}); + +test("default chat isolation propagates failure without starting another partition or group", { skip: process.platform === "win32" }, () => { + const { result, events } = defaultLauncher({ failureIndex: 2 }); + assert.equal(result.status, 1, result.stdout + result.stderr); + assert.match(result.stderr, /failed \(7\)/); + assert.equal(events.length, 6); + assert.ok(events.every(event => event.selected.length === 1)); + assert.deepEqual(events.map(event => event.edge), ["begin", "end", "begin", "end", "begin", "end"]); +}); + +test("default chat isolation keeps shared-database execution monolithic and serial", { skip: process.platform === "win32" }, () => { + const { result, events } = defaultLauncher({ shared: true }); + assert.equal(result.status, 0, result.stdout + result.stderr); + assert.ok(events.every(event => event.selected.length === 0)); + assert.ok(events[0].args.includes("@paperclipai/server")); + assert.ok(events[0].args.includes("--no-file-parallelism")); + assert.deepEqual(events[0].args.filter(arg => arg.startsWith("--maxWorkers")), ["--maxWorkers=1"]); + const excluded = events[0].args.flatMap((arg, index) => arg === "--exclude" ? [events[0].args[index + 1]] : []); + assert.ok(!excluded.includes(chatSuitePath.replace(/^server\//, ""))); + assert.ok(!excluded.includes(nativeRunnerSuitePath.replace(/^server\//, ""))); + assert.deepEqual(events.map(event => event.edge), Array.from({ length: events.length / 2 }, () => ["begin", "end"]).flat()); +}); + test("a failed chat child fails the job after its sibling finishes", { skip: process.platform === "win32" }, () => { const directory = mkdtempSync(path.join(os.tmpdir(), "chat-launch-proof-")); diff --git a/scripts/run-vitest-stable.mjs b/scripts/run-vitest-stable.mjs index fb48274670f..e7b4037bfdb 100644 --- a/scripts/run-vitest-stable.mjs +++ b/scripts/run-vitest-stable.mjs @@ -457,9 +457,13 @@ async function runVitest(args, label, testShard = null, exactFiles = false) { }); } -async function runGeneralSuites(routeTests) { +async function runGeneralSuites(routeTests, chatShards) { + for (const shard of chatShards ?? []) { + await runVitest(["--project", "@paperclipai/server", ...serializedServerVitestArgs, chatSuite], + "chat integration test shard", shard); + } for (const groupName of generalGroupNames) { - await runGeneralGroup(routeTests, groupName); + await runGeneralGroup(routeTests, groupName, null, null, chatShards ? [chatSuite] : []); } } @@ -484,7 +488,7 @@ function chatShardWorkers(index, count) { })); } -async function runGeneralGroup(routeTests, groupName, shardIndex = null, shardCount = null) { +async function runGeneralGroup(routeTests, groupName, shardIndex = null, shardCount = null, additionalExcludedSuites = []) { if (groupName === generalChatGroupName) { // Each invocation gets its own temporary home and embedded PostgreSQL. // Wait for both children even on failure so no sibling is abandoned. @@ -504,7 +508,7 @@ async function runGeneralGroup(routeTests, groupName, shardIndex = null, shardCo return; } if (generalServerGroups.includes(groupName)) { - const excludedSuites = excludedServerSuites(groupName); + const excludedSuites = [...excludedServerSuites(groupName), ...additionalExcludedSuites]; const files = generalServerTestFiles.filter((file) => !excludedSuites.includes(file)); if (shardCount !== null && shardCount > 1) { const shardFiles = selectGeneralServerShard( @@ -641,6 +645,9 @@ if (selection && options.dryRun) { writeFileSync(1, JSON.stringify({ selection })); process.exit(0); } +const defaultChatShards = !selection && options.group === null && options.mode !== serializedModeName && !process.env.PAPERCLIP_TEST_DATABASE_URL + ? Array.from({ length: 6 }, (_, index) => ({ index, count: 6 })) + : null; if (options.dryRun) { const serializedSuites = options.mode === serializedModeName @@ -659,7 +666,9 @@ if (options.dryRun) { selectedSerializedSuites: serializedSuites.map((routeTest) => routeTest.repoPath), generalServerSuiteCount: generalServerTestFiles.length, selectedGeneralServerSuites: - options.mode === generalModeName && options.group === generalServerNativeRunnerGroupName + defaultChatShards + ? generalServerTestFiles.filter(file => file !== chatSuite) + : options.mode === generalModeName && options.group === generalServerNativeRunnerGroupName ? nativeRunnerSuites : options.mode === generalModeName && generalServerGroups.includes(options.group) && @@ -672,7 +681,7 @@ if (options.dryRun) { ) : null, chatVitestShards: options.group === generalChatGroupName - ? chatShardWorkers(options.shardIndex, options.shardCount) : null, + ? chatShardWorkers(options.shardIndex, options.shardCount) : defaultChatShards, workspaceProjects: options.group === generalWorkspacesAGroupName ? generalWorkspacesAProjects @@ -709,7 +718,7 @@ try { if (options.group) { await runGeneralGroup(routeTests, options.group, options.shardIndex, options.shardCount); } else { - await runGeneralSuites(routeTests); + await runGeneralSuites(routeTests, defaultChatShards); } } diff --git a/server/src/__tests__/connection-management.integration.test.ts b/server/src/__tests__/connection-management.integration.test.ts index bbf43adfc18..103e81dd964 100644 --- a/server/src/__tests__/connection-management.integration.test.ts +++ b/server/src/__tests__/connection-management.integration.test.ts @@ -785,6 +785,10 @@ describe("connection management with production default agents", () => { const created = await f.request("POST", `/companies/${f.companyId}/tools/connections`, { name: "Selected Google services", transport: "mcp_remote", authKind: "oauth", credentialPolicy: "per_user", status: "draft", config: { url: "https://1.1.1.1/mcp", sourceTemplateKey: "google-workspace", connectionMethodKey: "paperclip-workspace" } }); expect(created, JSON.stringify(created)).toMatchObject({ status: 201 }); + const handoff = await f.request("POST", `/tools/oauth/${created.body.id}/start`, { asCurrentUser: true }); + expect(handoff, JSON.stringify(handoff)).toMatchObject({ status: 200, body: { expiresAt: null, requiredUserAction: { userId: f.userId } } }); + const [unselected] = await server.db.select().from(toolConnections).where(eq(toolConnections.id, created.body.id)); + expect(unselected.config.workspaceSetup).toBeUndefined(); const services = { gmail: "read" as const }; const started = await f.request("POST", `/tools/oauth/${created.body.id}/start`, { asCurrentUser: true, workspaceServices: services }); expect(started, JSON.stringify(started)).toMatchObject({ status: 200, body: { expiresAt: null, requiredUserAction: { userId: f.userId } } }); @@ -797,6 +801,10 @@ describe("connection management with production default agents", () => { [f.userId]: { requestedScopes: googleWorkspaceSelectedScopes(services) }, } } } } }); expect(await server.db.select().from(toolOauthStates).where(eq(toolOauthStates.connectionId, saved.id))).toHaveLength(0); + expect(await f.request("POST", `/tools/oauth/${saved.id}/start`, { asCurrentUser: true })).toMatchObject({ status: 200, body: { authorizationUrl: started.body.authorizationUrl } }); + const [handedOff] = await server.db.select().from(toolConnections).where(eq(toolConnections.id, saved.id)); + expect(handedOff.config.workspaceSetup).toBeUndefined(); + expect(handedOff.config.oauth).toEqual(saved.config.oauth); }); it("discovers and invokes connection APIs through default production native bindings", async () => { diff --git a/server/src/__tests__/skill-version-retention.test.ts b/server/src/__tests__/skill-version-retention.test.ts index bb757c7b6a6..a84adff3042 100644 --- a/server/src/__tests__/skill-version-retention.test.ts +++ b/server/src/__tests__/skill-version-retention.test.ts @@ -10,7 +10,7 @@ import { registerSkillVersionRetentionWork } from "../services/skill-version-ret import { notifySkillVersionRetention } from "../services/skill-version-retention-notifications.js"; import { notifyInstructionCollection } from "../services/instruction-collection-notifications.js"; import { notifyNativeEnvironmentSettlement } from "../services/native-environment-settlement-notifications.js"; -import { idleWorkSnapshot } from "../services/task-admission.js"; +import { idleWorkSnapshot, trackIdleWork } from "../services/task-admission.js"; import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js"; import { applyPendingMigrations } from "@paperclipai/db"; import { touchSkillVersionCache, lastUsedAt, skillVersionCachePaths } from "../services/skill-version-cache.js"; @@ -157,13 +157,18 @@ describeEmbeddedPostgres("skill version snapshot retention", () => { return dir; } - async function startRetention(graceMs = 20_000, service = skillVersionRetentionService(db, { graceMs })) { + async function startRetention( + graceMs = 20_000, + service = skillVersionRetentionService(db, { graceMs }), + advanceAutomatically = true, + ) { await Promise.all(Array.from({ length: 6 }, () => db.execute(sql`select 1`))); - vi.useFakeTimers({ toFake: ["Date", "setTimeout", "clearTimeout", "setInterval", "clearInterval"], shouldAdvanceTime: true }); + vi.useFakeTimers({ toFake: ["Date", "setTimeout", "clearTimeout", "setInterval", "clearInterval"], shouldAdvanceTime: advanceAutomatically }); coordinator = createDeliveryWorkCoordinator({ owner: db, canRun: () => admitted, canReconcile: () => true, onError: error => { throw error; } }); worker = registerSkillVersionRetentionWork(coordinator, service, () => admitted); - await worker.ready; + if (advanceAutomatically) await worker.ready; + else await waitForIo(worker.ready); return service; } async function settled(deadline: number | null = null) { @@ -173,6 +178,31 @@ describeEmbeddedPostgres("skill version snapshot retention", () => { expect(coordinator!.nextWakeAt()).toBe(deadline); }, { timeout: 10_000 }); } + async function waitForIo(operation: Promise) { + let complete = false; + void operation.then(() => { complete = true; }, () => { complete = true; }); + const deadline = process.hrtime.bigint() + 10_000_000_000n; + while (!complete) { + if (process.hrtime.bigint() >= deadline) throw new Error("Retention I/O did not settle within 10000ms"); + await vi.advanceTimersByTimeAsync(0); + await realDelay(0); + } + return operation; + } + async function settledAtDeadlines() { + const deadline = process.hrtime.bigint() + 10_000_000_000n; + for (;;) { + if (process.hrtime.bigint() >= deadline) throw new Error("Retention work did not settle within 10000ms"); + if (idleWorkSnapshot().active > 0) { + await vi.advanceTimersByTimeAsync(0); + await realDelay(0); + continue; + } + const next = coordinator!.nextWakeAt(); + if (next === null) return; + await vi.advanceTimersByTimeAsync(Math.max(0, next - Date.now())); + } + } async function quiet(ms = 3 * HOUR_MS) { const reads = vi.spyOn(db, "select"), transactions = vi.spyOn(db, "transaction"), directories = vi.spyOn(fs, "opendir"); await vi.advanceTimersByTimeAsync(ms); @@ -574,6 +604,30 @@ describeEmbeddedPostgres("skill version snapshot retention", () => { expect(await exists(directory)).toBe(false); await quiet(); }); + it("does not advance retention deadlines while owned directory I/O is pending", async () => { + await startRetention(20_000, undefined, false); + const entered = Promise.withResolvers(), release = Promise.withResolvers(); + const reading = trackIdleWork((async () => { + const directory = await fs.opendir(paperclipHome!); + try { entered.resolve(); await release.promise; } + finally { await directory.close(); } + })()); + const advance = vi.spyOn(vi, "advanceTimersByTimeAsync"); + let settling: Promise | undefined; + try { + await Promise.race([entered.promise, reading]); + const heldAt = Date.now(); + settling = settledAtDeadlines(); + await realDelay(0); + expect(advance.mock.calls.every(([ms]) => ms === 0)).toBe(true); + expect(Date.now()).toBe(heldAt); + } finally { + release.resolve(); + try { await reading; await settling; } + finally { advance.mockRestore(); } + } + }); + it("keeps directory pages moving while another company's grace deadline is due", async () => { const dueCompany = await createCompany(), pagedCompany = await createCompany(); const dueSkill = await createSkill(dueCompany, 2), pagedSkill = await createSkill(pagedCompany, 1); @@ -588,7 +642,7 @@ describeEmbeddedPostgres("skill version snapshot retention", () => { if (id === pagedCompany) pageTimes.push(Date.now()); return prune(id, options); }; - await startRetention(5000, service); await settled(); + await startRetention(5000, service, false); await settledAtDeadlines(); expect(pageTimes.length).toBeGreaterThan(5); expect(Math.max(...pageTimes.slice(1).map((at, index) => at - pageTimes[index]!))).toBeLessThan(3000); expect(await exists(expiring)).toBe(false); await quiet(); diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index 00ba194591d..aa5afc03b1a 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -83,6 +83,10 @@ import { } from "../services/tool-access.js"; import { accessService } from "../services/access.js"; import { instanceSettingsService } from "../services/instance-settings.js"; +import { createOAuthAuthorizationStart } from "../services/tool-access/oauth/authorization-start.js"; +import { createOAuthClientRegistration } from "../services/tool-access/oauth/client-registration.js"; +import { createPersonalConnectionIdentity } from "../services/tool-access/connections/personal-identity.js"; +import { createConnectionSetupPolicy } from "../services/tool-access/connections/setup-policy.js"; import { toolAccessPolicyService } from "../services/tool-access-policy.js"; import { secretService } from "../services/secrets.js"; import { @@ -2620,6 +2624,224 @@ describeEmbeddedPostgres("tool access service", () => { expect(reconnected.connection.agentInstructions).toEqual(settings); }); + it.each(["managed", "customer"] as const)("preserves a %s Workspace draft's selection after declined and expired authorization", async (mode) => { + const company = await createCompany(db); + const userId = `workspace-draft-${randomUUID()}`; + await grantBoardUser(db, company.id, userId, [], "owner"); + const actor = { actorType: "user" as const, actorId: userId }; + const connector = fakeGoogleWorkspaceConnector(company.id, userId, "workspace.all"); + const service = createTestToolAccessService(db, { paperclipCloudConnector: connector }); + const connected = await service.connectGalleryApp(company.id, { + galleryKey: "google-workspace", grantKind: "user", + connectionMethodKey: mode === "managed" ? "paperclip-workspace" : "customer-workspace-oauth", + ...(mode === "customer" ? { oauthClient: { clientId: "fixture-client", clientSecret: "fixture-secret" } } : {}), + }, actor); + const redirectUri = "https://paperclip.example.test/api/tools/oauth/callback"; + const [unstarted] = await db.select().from(toolConnections).where(eq(toolConnections.id, connected.connectionId)); + const configWithProposals = { ...unstarted.config, oauth: { ...(unstarted.config.oauth as Record), pendingWorkspaceServiceRequests: { + [userId]: { requestedScopes: googleWorkspaceSelectedScopes({ drive: "write" }), requestedAt: "2026-10-26T10:00:00Z" }, + other: { requestedScopes: googleWorkspaceSelectedScopes({ calendar: "write" }), requestedAt: "2026-10-26T10:00:00Z" }, + } } }; + await db.update(toolConnections).set({ config: configWithProposals, transportConfig: configWithProposals }).where(eq(toolConnections.id, connected.connectionId)); + const started = await service.startOAuth(company.id, connected.connectionId, { + redirectUri, actor, workspaceServices: { gmail: "read" }, + }); + const state = new URL(started.authorizationUrl).searchParams.get("state")!; + if (mode === "managed") { + await expect(service.completePaperclipCloudConnectorCallback({ state, error: "access_denied", actor })).rejects.toMatchObject({ status: 400 }); + } else { + await expect(service.completeOAuthCallback({ state, error: "access_denied", redirectUri, actor })).rejects.toMatchObject({ status: 400 }); + } + const draft = await service.getConnection(connected.connectionId); + expect(draft.status).toBe("draft"); + expect(draft.config.workspaceSetup).toEqual({ + version: 1, companyId: company.id, connectionId: draft.id, actorType: "user", actorId: userId, + grantKind: "user", subjectId: userId, + requestedScopes: ["https://www.googleapis.com/auth/gmail.readonly"], + }); + expect((draft.config.oauth as typeof configWithProposals.oauth).pendingWorkspaceServiceRequests).toEqual({ other: configWithProposals.oauth.pendingWorkspaceServiceRequests.other }); + const retained = await service.connectGalleryApp(company.id, { + galleryKey: "google-workspace", resumeConnectionId: draft.id, + connectionMethodKey: mode === "managed" ? "paperclip-workspace" : "customer-workspace-oauth", + ...(mode === "customer" ? { oauthClient: { clientId: "fixture-client" } } : {}), + }, actor); + expect(retained.connectionId).toBe(draft.id); + const resumed = await service.startOAuth(company.id, retained.connectionId, { redirectUri, actor }); + const [resumedState] = await db.select().from(toolOauthStates).where(eq(toolOauthStates.state, new URL(resumed.authorizationUrl).searchParams.get("state")!)); + expect(resumedState.requestedScopes).toEqual(["https://www.googleapis.com/auth/gmail.readonly"]); + await db.update(toolOauthStates).set({ expiresAt: new Date(Date.now() - 1000) }).where(eq(toolOauthStates.state, resumedState.state)); + if (mode === "managed") { + await expect(service.completePaperclipCloudConnectorCallback({ state: resumedState.state, claimId: "expired", actor })).rejects.toMatchObject({ status: 400 }); + } else { + await expect(service.completeOAuthCallback({ state: resumedState.state, code: "expired", redirectUri, actor })).rejects.toMatchObject({ status: 400 }); + } + const coldService = createTestToolAccessService(db, { paperclipCloudConnector: connector }); + const coldResume = await coldService.startOAuth(company.id, draft.id, { redirectUri, actor }); + const [coldState] = await db.select().from(toolOauthStates).where(eq(toolOauthStates.state, new URL(coldResume.authorizationUrl).searchParams.get("state")!)); + expect(coldState.requestedScopes).toEqual(["https://www.googleapis.com/auth/gmail.readonly"]); + }); + + it.each(["bound", "first-use"] as const)("preserves the latest Workspace draft choice across overlapping %s customer starts", async (bindingMode) => { + const company = await createCompany(db); + const userId = `workspace-overlap-${randomUUID()}`; + await grantBoardUser(db, company.id, userId, [], "owner"); + const actor = { actorType: "user" as const, actorId: userId }; + const service = createTestToolAccessService(db); + const connected = await service.connectGalleryApp(company.id, { + galleryKey: "google-workspace", connectionMethodKey: "customer-workspace-oauth", grantKind: "user", + oauthClient: { clientId: "fixture-client", clientSecret: "fixture-secret" }, + }, actor); + const redirectUri = "https://paperclip.example.test/api/tools/oauth/callback"; + const endpoints = { + provider: "google", authorizationUrl: "https://accounts.google.com/o/oauth2/v2/auth", + tokenUrl: "https://oauth2.googleapis.com/token", registrationUrl: null, + scopes: googleWorkspaceSelectedScopes(GOOGLE_WORKSPACE_DEFAULT_SELECTION), + offlineAccessSupported: true, issuer: "https://accounts.google.com", resource: null, + }; + const getConnectionRow = async () => { + const [row] = await db.select().from(toolConnections).where(eq(toolConnections.id, connected.connectionId)); + return row; + }; + const initial = await getConnectionRow(); + if (bindingMode === "bound") { + const config = { ...initial.config, oauth: { ...(initial.config.oauth as Record), + clientRedirectUri: redirectUri, clientIssuer: endpoints.issuer, clientResource: null, clientCompanyId: company.id, + } }; + await db.update(toolConnections).set({ config, transportConfig: config }).where(eq(toolConnections.id, initial.id)); + } else { + expect((initial.config.oauth as Record).clientRedirectUri).toBeUndefined(); + } + const observedClientRows: typeof initial[] = []; + const ensureOAuthClient = createOAuthClientRegistration(db, { + now: () => new Date(), options: {}, getConnectionRow, + requestOAuthClientRegistration: vi.fn(), + store: { + createOrRotateOAuthSecret: vi.fn(), syncCredentialBindings: vi.fn(), + oauthClientForConnection: async (row) => { + observedClientRows.push(row); + return { clientId: "fixture-client", clientSecret: "fixture-secret", clientIdEnv: null, clientSecretEnv: null }; + }, + }, + }); + let releaseA!: () => void; + let reachedA!: () => void; + const pausedA = new Promise(resolve => { reachedA = resolve; }); + const resumeA = new Promise(resolve => { releaseA = resolve; }); + let discoveries = 0; + const makeStart = () => createOAuthAuthorizationStart(db, { + now: () => new Date(), getConnectionRow, ensureOAuthClient, + setupPolicy: createConnectionSetupPolicy(db), fixedPersonalIdentityForReconnect: createPersonalConnectionIdentity(db), + currentCloudConnector: () => null, vercelConnect: null, + oauthEndpointsForConnection: async () => { + if (++discoveries === 1) { reachedA(); await resumeA; } + return endpoints; + }, + assertOAuthEndpointUrl: (_kind, url) => String(url), + }); + const start = makeStart(); + const first = start(company.id, initial.id, { redirectUri, actor, subjectUserId: userId, workspaceServices: { gmail: "write" } }); + let startedA; + let startedB; + let afterB; + try { + await pausedA; + startedB = await start(company.id, initial.id, { redirectUri, actor, subjectUserId: userId, workspaceServices: { gmail: "read" } }); + afterB = await getConnectionRow(); + expect((afterB.config.oauth as Record).clientRedirectUri).toBe(redirectUri); + const pending = { anotherUser: { requestedScopes: googleWorkspaceSelectedScopes({ calendar: "read" }), requestedAt: "2026-10-30T16:00:00Z" } }; + await db.update(toolConnections).set({ + config: { ...afterB.config, concurrentSetting: "config-b", oauth: { ...(afterB.config.oauth as Record), pendingWorkspaceServiceRequests: pending } }, + transportConfig: { ...afterB.transportConfig, transportOnlySetting: "transport-b", oauth: { ...(afterB.transportConfig.oauth as Record), pendingWorkspaceServiceRequests: pending } }, + }).where(eq(toolConnections.id, initial.id)); + } finally { + releaseA(); + startedA = await first; + } + const writeScopes = googleWorkspaceSelectedScopes({ gmail: "write" }); + const readScopes = googleWorkspaceSelectedScopes({ gmail: "read" }); + const expectedBinding = { version: 1, companyId: company.id, connectionId: initial.id, actorType: "user", actorId: userId, + grantKind: "user", subjectId: userId, requestedScopes: readScopes }; + if (bindingMode === "first-use") { + expect(observedClientRows).toHaveLength(2); + expect((observedClientRows[1].config.oauth as Record).clientRedirectUri).toBe(redirectUri); + expect(observedClientRows[1].config.workspaceSetup).toEqual(expectedBinding); + } + const latest = await getConnectionRow(); + expect(afterB!.config.workspaceSetup).toEqual(expectedBinding); + expect(latest.config.workspaceSetup).toEqual(expectedBinding); + expect(latest.transportConfig.workspaceSetup).toEqual(expectedBinding); + expect(latest.config.concurrentSetting).toBe("config-b"); + expect(latest.transportConfig.transportOnlySetting).toBe("transport-b"); + for (const config of [latest.config, latest.transportConfig]) { + expect((config.oauth as Record).pendingWorkspaceServiceRequests).toEqual({ anotherUser: { + requestedScopes: googleWorkspaceSelectedScopes({ calendar: "read" }), requestedAt: "2026-10-30T16:00:00Z", + } }); + } + for (const [started, expected] of [[startedA!, writeScopes], [startedB!, readScopes]] as const) { + const url = new URL(started.authorizationUrl); + expect(url.searchParams.get("scope")?.split(" ")).toEqual(expected); + const [state] = await db.select().from(toolOauthStates).where(eq(toolOauthStates.state, url.searchParams.get("state")!)); + expect(state.requestedScopes).toEqual(expected); + } + const cold = await makeStart()(company.id, initial.id, { redirectUri, actor, subjectUserId: userId }); + const coldUrl = new URL(cold.authorizationUrl); + expect(coldUrl.searchParams.get("scope")?.split(" ")).toEqual(readScopes); + const [coldState] = await db.select().from(toolOauthStates).where(eq(toolOauthStates.state, coldUrl.searchParams.get("state")!)); + expect(coldState.requestedScopes).toEqual(readScopes); + }); + + it.each((["clientId", "clientSecretRef", "clientIssuer", "clientResource", "clientRedirectUri", "issuer", "resource"] as const) + .flatMap(field => (["discovery", "client-resolution"] as const).map(stage => [field, stage] as const)))( + "rejects a concurrent %s change during Workspace %s", async (field, stage) => { + const company = await createCompany(db); + const actor = { actorType: "user" as const, actorId: "board" }; + const service = createTestToolAccessService(db); + const connected = await service.connectGalleryApp(company.id, { + galleryKey: "google-workspace", connectionMethodKey: "customer-workspace-oauth", grantKind: "organization", + oauthClient: { clientId: "fixture-client", clientSecret: "fixture-secret" }, + }, actor); + const getConnectionRow = async () => { + const [row] = await db.select().from(toolConnections).where(eq(toolConnections.id, connected.connectionId)); + return row; + }; + const endpoints = { provider: "google", authorizationUrl: "https://accounts.google.com/o/oauth2/v2/auth", + tokenUrl: "https://oauth2.googleapis.com/token", scopes: googleWorkspaceSelectedScopes({ gmail: "read" }), + issuer: "https://accounts.google.com", resource: null }; + const ensureClient = createOAuthClientRegistration(db, { + now: () => new Date(), options: {}, getConnectionRow, requestOAuthClientRegistration: vi.fn(), + store: { createOrRotateOAuthSecret: vi.fn(), syncCredentialBindings: vi.fn(), + oauthClientForConnection: async () => ({ clientId: "fixture-client", clientSecret: "fixture-secret", clientIdEnv: null, clientSecretEnv: null }) }, + }); + let changed: Awaited>; + const mutateClient = async () => { + const row = await getConnectionRow(); + const updates = field === "clientSecretRef" + ? { credentialSecretRefs: row.credentialSecretRefs.map(ref => ref.configPath === "oauth.client_secret" ? { ...ref, versionSelector: 1 } : ref) } + : { + config: { ...row.config, oauth: { ...(row.config.oauth as Record), [field]: `changed-${field}` } }, + transportConfig: { ...row.transportConfig, oauth: { ...(row.transportConfig.oauth as Record), [field]: `changed-${field}` } }, + }; + if (field === "clientSecretRef") expect(updates.credentialSecretRefs).not.toEqual(row.credentialSecretRefs); + [changed] = await db.update(toolConnections).set(updates).where(eq(toolConnections.id, row.id)).returning(); + }; + const start = createOAuthAuthorizationStart(db, { + now: () => new Date(), getConnectionRow, + setupPolicy: createConnectionSetupPolicy(db), fixedPersonalIdentityForReconnect: createPersonalConnectionIdentity(db), + currentCloudConnector: () => null, vercelConnect: null, + oauthEndpointsForConnection: async () => { if (stage === "discovery") await mutateClient(); return endpoints; }, + ensureOAuthClient: async input => { const resolved = await ensureClient(input); if (stage === "client-resolution") await mutateClient(); return resolved; }, + assertOAuthEndpointUrl: (_kind, url) => String(url), + }); + await expect(start(company.id, connected.connectionId, { + redirectUri: "https://paperclip.example.test/api/tools/oauth/callback", actor, workspaceServices: { gmail: "read" }, + })).rejects.toMatchObject({ status: 409 }); + const retained = await getConnectionRow(); + expect(retained.config).toEqual(changed!.config); + expect(retained.transportConfig).toEqual(changed!.transportConfig); + expect(retained.credentialSecretRefs).toEqual(changed!.credentialSecretRefs); + }, + ); + it.each(["user", "organization"] as const)("preserves the %s Workspace request when account-page reconnect omits a selection", async (kind) => { const company = await createCompany(db); const service = createTestToolAccessService(db); @@ -2653,6 +2875,69 @@ describeEmbeddedPostgres("tool access service", () => { expect(new URL(selected.authorizationUrl).searchParams.get("scope")?.split(" ")).toEqual(requestedScopes); }); + it("requires explicit selection for historical Workspace drafts and a different setup owner", async () => { + const company = await createCompany(db); + const userId = `workspace-owner-${randomUUID()}`; + const otherUserId = `workspace-owner-${randomUUID()}`; + await grantBoardUser(db, company.id, userId, [], "owner"); + await grantBoardUser(db, company.id, otherUserId, [], "owner"); + const actor = { actorType: "user" as const, actorId: userId }; + const connector = fakeGoogleWorkspaceConnector(company.id, userId, "workspace.all"); + const service = createTestToolAccessService(db, { paperclipCloudConnector: connector }); + const connected = await service.connectGalleryApp(company.id, { + galleryKey: "google-workspace", connectionMethodKey: "paperclip-workspace", grantKind: "organization", + }, actor); + const redirectUri = "https://paperclip.example.test/api/tools/oauth/callback"; + await expect(service.startOAuth(company.id, connected.connectionId, { redirectUri, actor })).rejects.toMatchObject({ status: 400 }); + await service.startOAuth(company.id, connected.connectionId, { redirectUri, actor, workspaceServices: { gmail: "read" } }); + await expect(service.startOAuth(company.id, connected.connectionId, { + redirectUri, actor: { actorType: "user", actorId: otherUserId }, + })).rejects.toMatchObject({ status: 400 }); + expect((await service.getConnection(connected.connectionId)).config.workspaceSetup).toMatchObject({ actorId: userId }); + }); + + it.each(["credentialPolicy", "credentialSource", "authKind", "transport", "createdByUserId", "sourceTemplateKey", "connectionMethodKey"] as const)("rejects a Workspace draft whose %s changes during authorization", async (field) => { + const company = await createCompany(db); + const actor = { actorType: "user" as const, actorId: "board" }; + const service = createTestToolAccessService(db); + const connected = await service.connectGalleryApp(company.id, { + galleryKey: "google-workspace", connectionMethodKey: "customer-workspace-oauth", grantKind: "organization", + oauthClient: { clientId: "fixture-client", clientSecret: "fixture-secret" }, + }, actor); + const [original] = await db.select().from(toolConnections).where(eq(toolConnections.id, connected.connectionId)); + const changed: Partial = {}; + if (field === "credentialPolicy") changed.credentialPolicy = "per_user"; + else if (field === "credentialSource") { + changed.credentialSource = "vercel_connect"; + changed.credentialRefs = []; + changed.credentialSecretRefs = []; + changed.externalCredential = { provider: "vercel_connect", connectorId: "fixture", connectorUid: "fixture", service: "google", principalMode: "user" }; + } + else if (field === "authKind") changed.authKind = "none"; + else if (field === "transport") changed.transport = "rest_api"; + else if (field === "createdByUserId") changed.createdByUserId = "another-owner"; + else changed.config = { ...original.config, [field]: "different" }; + const start = createOAuthAuthorizationStart(db, { + now: () => new Date(), + getConnectionRow: async () => { + await db.update(toolConnections).set(changed).where(eq(toolConnections.id, original.id)); + return original; + }, + setupPolicy: createConnectionSetupPolicy(db), + fixedPersonalIdentityForReconnect: createPersonalConnectionIdentity(db), + currentCloudConnector: () => null, + vercelConnect: null, + ensureOAuthClient: vi.fn(), + oauthEndpointsForConnection: vi.fn(), + assertOAuthEndpointUrl: vi.fn(), + }); + await expect(start(company.id, original.id, { + redirectUri: "https://paperclip.example.test/api/tools/oauth/callback", actor, workspaceServices: { gmail: "read" }, + })).rejects.toMatchObject({ status: 409 }); + expect((await service.getConnection(original.id)).config.workspaceSetup).toBeUndefined(); + expect(await db.select().from(toolOauthStates).where(eq(toolOauthStates.connectionId, original.id))).toHaveLength(0); + }); + it("binds selected Workspace service scopes to OAuth state and rejects other providers", async () => { const company = await createCompany(db); const service = createTestToolAccessService(db); @@ -2686,7 +2971,7 @@ describeEmbeddedPostgres("tool access service", () => { oauthClient: { clientId: "test-workspace-client", clientSecret: "test-workspace-secret" }, }, actor); const redirectUri = "https://paperclip.example.test/api/tools/oauth/callback"; - const started = await service.startOAuth(company.id, connected.connectionId, { redirectUri, actor }); + const started = await service.startOAuth(company.id, connected.connectionId, { redirectUri, actor, workspaceServices: GOOGLE_WORKSPACE_DEFAULT_SELECTION }); const authorization = new URL(started.authorizationUrl); expect(authorization.searchParams.get("scope")?.split(" ")).toEqual(googleWorkspaceSelectedScopes(GOOGLE_WORKSPACE_DEFAULT_SELECTION)); expect(authorization.searchParams.get("enable_granular_consent")).toBe("true"); @@ -2742,7 +3027,9 @@ describeEmbeddedPostgres("tool access service", () => { const redirectUri = `https://paperclip.example/api/tools/oauth/${mode === "managed" ? "cloud-connector/" : ""}callback`; for (const reconnect of [false, true]) { if (reconnect) await service.connectGalleryApp(company.id, { ...input, reconnectConnectionId: connected.connectionId }, actor); - const started = await service.startOAuth(company.id, connected.connectionId, { redirectUri, actor }); + const started = await service.startOAuth(company.id, connected.connectionId, { redirectUri, actor, + ...(!reconnect ? { workspaceServices: GOOGLE_WORKSPACE_DEFAULT_SELECTION } : {}), + }); const state = new URL(started.authorizationUrl).searchParams.get("state")!; const completed = mode === "managed" ? await service.completePaperclipCloudConnectorCallback({ state, claimId: "workspace-no-agents", actor }) @@ -7629,6 +7916,7 @@ describeEmbeddedPostgres("tool access service", () => { expect(workspaceApp.methods.map((method: { key: string }) => method.key)).toContain(methodKey); const connected = await request(app).post(`/api/companies/${company.id}/tools/apps/connect`).send({ galleryKey: slug, connectionMethodKey: methodKey, grantKind: "user", name: `Personal ${slug}`, + ...(profile === "workspace.all" ? { workspaceServices: GOOGLE_WORKSPACE_DEFAULT_SELECTION } : {}), }); expect(connected.status).toBe(201); expect(connected.body.connection).toMatchObject({ credentialPolicy: "per_user", ownership: "platform_shared" }); @@ -7691,6 +7979,7 @@ describeEmbeddedPostgres("tool access service", () => { .map((method: { key: string }) => method.key)).toContain(methodKey); const result = await request(app).post(`/api/companies/${company.id}/tools/apps/connect`).send({ galleryKey: slug, connectionMethodKey: methodKey, grantKind: "user", name: `Cloud ${slug}`, + ...(profile === "workspace.all" ? { workspaceServices: GOOGLE_WORKSPACE_DEFAULT_SELECTION } : {}), }); expect(result.status, JSON.stringify(result.body)).toBe(201); expect(result.body.auth.startUrl).toBe("https://my.paperclip.app/connections/confirm?id=test-workspace-session"); @@ -8297,6 +8586,7 @@ describeEmbeddedPostgres("tool access service", () => { }, actor); const started = await service.startOAuth(company.id, connected.connectionId, { redirectUri: "https://paperclip.example/api/tools/oauth/cloud-connector/callback", actor, + workspaceServices: GOOGLE_WORKSPACE_DEFAULT_SELECTION, }); const completed = await service.completePaperclipCloudConnectorCallback({ state: new URL(started.authorizationUrl).searchParams.get("state")!, claimId: "workspace-claim", actor, @@ -8364,7 +8654,7 @@ describeEmbeddedPostgres("tool access service", () => { ...(mode === "customer" ? { oauthClient: { clientId: "workspace-client", clientSecret: "workspace-client-secret" } } : {}), }, actor); const redirectUri = mode === "managed" ? "https://paperclip.example/api/tools/oauth/cloud-connector/callback" : "https://paperclip.example/api/tools/oauth/callback"; - const started = await service.startOAuth(company.id, connected.connectionId, { redirectUri, actor }); + const started = await service.startOAuth(company.id, connected.connectionId, { redirectUri, actor, workspaceServices: GOOGLE_WORKSPACE_DEFAULT_SELECTION }); const state = new URL(started.authorizationUrl).searchParams.get("state")!; const completed = mode === "managed" ? await service.completePaperclipCloudConnectorCallback({ state, claimId: "fixture", actor }) diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index 81b6f646c3c..0b1cc80f3bf 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -2112,6 +2112,9 @@ export function toolAccessService( sourceTemplateKey: galleryEntry.slug, connectionMethodKey: method?.key, methodConfig: normalizedMethodConfig?.values ?? {}, + ...(galleryEntry.slug === "google-workspace" && retainedConnection?.status === "draft" + ? { workspaceSetup: retainedConnection.config.workspaceSetup } + : {}), ...(method?.defaults?.mcpSessionRequired === true ? { mcpSessionRequired: true } : {}), diff --git a/server/src/services/tool-access/oauth/authorization-start.ts b/server/src/services/tool-access/oauth/authorization-start.ts index 81ce6161f86..00c472c1381 100644 --- a/server/src/services/tool-access/oauth/authorization-start.ts +++ b/server/src/services/tool-access/oauth/authorization-start.ts @@ -1,13 +1,13 @@ import { notifyIssueReviewState } from "../../issue-review-state-notifications.js"; import { vercelConnectHttpError, vercelCredentialFor, vercelConnectResourcesFor } from "../vercel-credentials.js"; import { credentialScope, managedConnectorProfile } from "./client-settings.js"; -import { createOAuthClientRegistration } from "./client-registration.js"; +import { assertOAuthClientBindingUnchanged, createOAuthClientRegistration } from "./client-registration.js"; import type { ActorInfo } from "../types.js"; import { createOAuthEndpointPolicy } from "./endpoint-policy.js"; import { createOAuthDiscovery } from "./discovery.js"; -import { oauthConfig, normalizeOauthScopes, originOf } from "./metadata.js"; +import { asRecord, oauthConfig, normalizeOauthScopes, originOf } from "./metadata.js"; import { connectionMethodForConnection } from "../connection-methods.js"; -import { GOOGLE_WORKSPACE_DEFAULT_SELECTION, googleWorkspaceSelectedScopes, type GoogleWorkspaceServiceSelection, isGoogleWorkspaceScopeGrant } from "@paperclipai/shared"; +import { googleWorkspaceDraftRequestedScopes, googleWorkspaceSelectedScopes, type GoogleWorkspaceServiceSelection, isGoogleWorkspaceScopeGrant } from "@paperclipai/shared"; import { and, eq, lt } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { companies, agents, connectionGrants, issueThreadInteractions, toolConnections, toolOauthStates } from "@paperclipai/db"; @@ -72,17 +72,37 @@ export function createOAuthAuthorizationStart(db: Db, dependencies: Dependencies const galleryMethod = galleryEntry ? connectionMethodForConnection(galleryEntry, connection) : null; + const starterBinding = actorBinding(input.actor); + const fixedPersonalIdentity = await fixedPersonalIdentityForReconnect( + connection, + input.subjectUserId, + input.actor, + ); + const authorizationSubjectUserId = + fixedPersonalIdentity?.subjectUserId ?? input.subjectUserId; + const authorizationSubjectAgentId = input.subjectAgentId; + const workspaceDraftBinding = { + companyId, connectionId: connection.id, + actorType: starterBinding.actorType, actorId: starterBinding.actorId, + grantKind: connection.credentialPolicy === "per_user" ? "user" as const + : connection.credentialPolicy === "per_agent" ? "agent" as const : "organization" as const, + subjectId: connection.credentialPolicy === "per_user" ? authorizationSubjectUserId ?? null + : connection.credentialPolicy === "per_agent" ? authorizationSubjectAgentId ?? null : null, + }; const requestedScopes = await (async () => { if (input.workspaceServices && sourceTemplateKey !== "google-workspace") throw badRequest("Service selection requires Google Workspace"); if (sourceTemplateKey === "google-workspace") { if (input.workspaceServices && input.scopes) throw badRequest("Choose services or scopes, not both"); const allowed = normalizeOauthScopes(galleryMethod?.defaults?.scopesHint); let scopes = input.scopes ?? (input.workspaceServices ? googleWorkspaceSelectedScopes(input.workspaceServices) : undefined); - if (!scopes && connection.status !== "draft") { + if (!scopes && connection.status === "draft") { + scopes = googleWorkspaceDraftRequestedScopes(connection.config.workspaceSetup, workspaceDraftBinding) ?? undefined; + } + if (!scopes && input.actor.managementContext?.actor.type === "agent") return null; + if (!scopes) { // Account-page reconnect has no new service choice. Never replace its // subject's saved request with the new-account write defaults. - const binding = actorBinding(input.actor); - const subjectUserId = input.subjectUserId ?? (binding.actorType === "user" ? binding.actorId : null); + const subjectUserId = authorizationSubjectUserId ?? (starterBinding.actorType === "user" ? starterBinding.actorId : null); const kind = connection.credentialPolicy === "per_user" ? "user" : connection.credentialPolicy === "per_agent" ? "agent" : "organization"; const [grant] = await db.select().from(connectionGrants).where(and( @@ -94,11 +114,12 @@ export function createOAuthAuthorizationStart(db: Db, dependencies: Dependencies : eq(connectionGrants.isDefault, true), )).limit(1); const saved = grant?.providerTenant?.oauth; - scopes = normalizeOauthScopes(saved?.requestedScopes ?? saved?.scopes) + scopes = normalizeOauthScopes(saved?.requestedScopes ?? (connection.status !== "draft" ? saved?.scopes : undefined)) .filter(scope => allowed.includes(scope)); - if (scopes.length === 0) throw badRequest("Choose your Google Workspace services before reconnecting"); + if (scopes.length === 0) throw badRequest(connection.status === "draft" + ? "Choose your Google Workspace services before continuing setup" + : "Choose your Google Workspace services before reconnecting"); } - scopes ??= googleWorkspaceSelectedScopes(GOOGLE_WORKSPACE_DEFAULT_SELECTION); if (!isGoogleWorkspaceScopeGrant(scopes) || scopes.some(scope => !allowed.includes(scope))) throw badRequest("Select at least one supported Google service"); return scopes; } @@ -118,15 +139,6 @@ export function createOAuthAuthorizationStart(db: Db, dependencies: Dependencies } return requested; })(); - const starterBinding = actorBinding(input.actor); - const fixedPersonalIdentity = await fixedPersonalIdentityForReconnect( - connection, - input.subjectUserId, - input.actor, - ); - const authorizationSubjectUserId = - fixedPersonalIdentity?.subjectUserId ?? input.subjectUserId; - const authorizationSubjectAgentId = input.subjectAgentId; if (authorizationSubjectAgentId) { const [subjectAgent] = await db .select({ id: agents.id }) @@ -209,6 +221,37 @@ export function createOAuthAuthorizationStart(db: Db, dependencies: Dependencies return { connectionId: connection.id, provider: sourceTemplateKey ?? "oauth", authorizationUrl: destination.toString(), expiresAt: null }; } + if (sourceTemplateKey === "google-workspace" && connection.status === "draft") { + const workspaceSetup = { version: 1, ...workspaceDraftBinding, requestedScopes }; + const updated = await db.transaction(async tx => { + const [saved] = await tx.select().from(toolConnections).where(and( + eq(toolConnections.id, connection.id), eq(toolConnections.companyId, companyId), eq(toolConnections.status, "draft"), + )).for("no key update"); + if (!saved || (["credentialPolicy", "credentialSource", "authKind", "transport", "createdByUserId"] as const) + .some(key => saved[key] !== connection[key]) + || ["sourceTemplateKey", "connectionMethodKey"].some(key => saved.config[key] !== connection.config[key] + || saved.transportConfig[key] !== connection.transportConfig[key])) { + throw conflict("Connection setup changed. Reload before continuing"); + } + const config: Record = { ...saved.config, workspaceSetup }; + const transportConfig: Record = { ...saved.transportConfig, workspaceSetup }; + if (starterBinding.actorType === "user" && starterBinding.actorId) { + for (const target of [config, transportConfig]) { + const oauth = asRecord(target.oauth); + const pending = { ...asRecord(oauth.pendingWorkspaceServiceRequests) }; + if (Object.hasOwn(pending, starterBinding.actorId)) { + delete pending[starterBinding.actorId]; + target.oauth = { ...oauth, pendingWorkspaceServiceRequests: pending }; + } + } + } + const [row] = await tx.update(toolConnections).set({ config, transportConfig, updatedAt: now() }) + .where(and(eq(toolConnections.id, saved.id), eq(toolConnections.companyId, companyId), eq(toolConnections.status, "draft"))).returning(); + return row; + }); + if (!updated) throw conflict("Connection setup changed. Reload before continuing"); + connection = updated; + } if (connection.credentialSource === "vercel_connect") { if (!vercelConnect) throw vercelConnectHttpError( @@ -629,10 +672,13 @@ export function createOAuthAuthorizationStart(db: Db, dependencies: Dependencies }); } - const nextConfig = { - ...connection.config, - oauth: { - ...oauthConfig(connection), + await db.transaction(async tx => { + const [saved] = await tx.select().from(toolConnections).where(and( + eq(toolConnections.id, connection.id), eq(toolConnections.companyId, companyId), + )).for("no key update"); + if (!saved) throw notFound("Tool connection not found"); + assertOAuthClientBindingUnchanged(connection, saved, endpoints, input.redirectUri); + const oauth = { provider: endpoints.provider, authorizationUrl: endpoints.authorizationUrl, tokenUrl: endpoints.tokenUrl, @@ -655,27 +701,29 @@ export function createOAuthAuthorizationStart(db: Db, dependencies: Dependencies // Persist the issuer and resource this authorization run is bound to. // `iss` on the callback is validated against `expectedIssuer`, and // refresh/reconnect/revoke reuse the same pair rather than re-deriving it. - issuer: endpoints.issuer ?? oauthConfig(connection).issuer ?? null, + issuer: endpoints.issuer ?? oauthConfig(saved).issuer ?? null, expectedIssuer: endpoints.issuer ?? null, resource: - endpoints.resource ?? oauthConfig(connection).resource ?? null, + endpoints.resource ?? oauthConfig(saved).resource ?? null, clientIdMetadataDocumentSupported: endpoints.clientIdMetadataDocumentSupported === true, - }, - }; - await db - .update(toolConnections) - .set({ + }; + const nextConfig = { ...saved.config, oauth: { ...oauthConfig(saved), ...oauth } }; + const nextTransportConfig = { ...saved.transportConfig, oauth: { ...asRecord(saved.transportConfig.oauth), ...oauth } }; + await tx + .update(toolConnections) + .set({ // A generic URL connection starts life as `authKind: none`. Once it has // completed OAuth discovery and client resolution it is an OAuth // connection, and refresh, reconnect, revoke and diagnostics must all // treat it as one. - authKind: "oauth", - config: nextConfig, - transportConfig: nextConfig, - updatedAt: new Date(), - }) - .where(eq(toolConnections.id, connection.id)); + authKind: "oauth", + config: nextConfig, + transportConfig: nextTransportConfig, + updatedAt: new Date(), + }) + .where(and(eq(toolConnections.id, connection.id), eq(toolConnections.companyId, companyId))); + }); return { connectionId: connection.id, diff --git a/server/src/services/tool-access/oauth/client-registration.ts b/server/src/services/tool-access/oauth/client-registration.ts index 50841892b10..490ff6319e7 100644 --- a/server/src/services/tool-access/oauth/client-registration.ts +++ b/server/src/services/tool-access/oauth/client-registration.ts @@ -1,6 +1,6 @@ import type { ActorInfo } from "../types.js"; import { createOAuthClientProtocol } from "./client.js"; -import { oauthConfig, sameOAuthIssuer } from "./metadata.js"; +import { asRecord, oauthConfig, sameOAuthIssuer } from "./metadata.js"; import { resolveOAuthClientIdMetadataDocumentUrl } from "./client-metadata.js"; import type { OAuthProviderEndpoints, OAuthClientRegistrationSource } from "./types.js"; import { connectionMethodForConnection } from "../connection-methods.js"; @@ -8,7 +8,7 @@ import { and, eq } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { toolConnections } from "@paperclipai/db"; import type { AppDefinition } from "@paperclipai/shared"; -import { notFound, unprocessable } from "../../../errors.js"; +import { conflict, notFound, unprocessable } from "../../../errors.js"; import { type RemoteHttpEndpointLookup } from "../../remote-http-endpoint-guard.js"; import type { OAuthCredentialStore } from "./credential-store.js"; @@ -24,6 +24,34 @@ type RegistrationDependencies = { requestOAuthClientRegistration: ReturnType["requestOAuthClientRegistration"]; }; +export function assertOAuthClientBindingUnchanged( + expected: typeof toolConnections.$inferSelect, + current: typeof toolConnections.$inferSelect, + endpoints: OAuthProviderEndpoints, + redirectUri: string, +) { + const changedSetup = expected.config.sourceTemplateKey === "google-workspace" && expected.status === "draft" + && (current.status !== "draft" || (["credentialPolicy", "credentialSource", "authKind", "transport", "createdByUserId"] as const) + .some(key => current[key] !== expected[key]) + || ["sourceTemplateKey", "connectionMethodKey"].some(key => current.config[key] !== expected.config[key] + || current.transportConfig[key] !== expected.transportConfig[key])); + const bindings: Record = { + clientRedirectUri: redirectUri, clientIssuer: endpoints.issuer ?? null, + clientResource: endpoints.resource ?? null, clientCompanyId: expected.companyId, + issuer: endpoints.issuer ?? null, resource: endpoints.resource ?? null, + }; + const changedClient = expected.ownership !== current.ownership + || JSON.stringify(oauthSecretRef(expected, "oauth.client_secret")) !== JSON.stringify(oauthSecretRef(current, "oauth.client_secret")) + || (["config", "transportConfig"] as const).some(target => { + const before = asRecord(expected[target].oauth); + const after = asRecord(current[target].oauth); + return ["clientId", "clientRegistrationSource", "clientTokenEndpointAuthMethod"].some(key => before[key] !== after[key]) + || Object.entries(bindings).some(([key, value]) => before[key] !== after[key] + && !(before[key] == null && (after[key] == null || after[key] === value))); + }); + if (changedSetup || changedClient) throw conflict("Connection sign-in settings changed. Reload before continuing"); +} + export function createOAuthClientRegistration(db: Db, { now, options, store, getConnectionRow, requestOAuthClientRegistration }: RegistrationDependencies) { const { createOrRotateOAuthSecret, oauthClientForConnection, syncCredentialBindings } = store; async function registerOAuthClient(input: { @@ -300,46 +328,55 @@ async function stampOAuthClientBinding( endpoints: OAuthProviderEndpoints, redirectUri: string, ): Promise { - const originalOAuth = oauthConfig(connection); - const oauth = reviewedOAuthClientBinding(connection, endpoints); - const nextBinding = { - clientRedirectUri: redirectUri, - clientIssuer: - typeof oauth.clientIssuer === "string" && oauth.clientIssuer - ? oauth.clientIssuer - : (endpoints.issuer ?? null), - clientResource: - typeof oauth.clientResource === "string" && oauth.clientResource - ? oauth.clientResource - : (endpoints.resource ?? null), - clientCompanyId: - typeof oauth.clientCompanyId === "string" && oauth.clientCompanyId - ? oauth.clientCompanyId - : connection.companyId, - }; - const unchanged = Object.entries(nextBinding).every( - ([key, value]) => originalOAuth[key] === value, - ); - if (unchanged) return connection; - const nextConfig = { - ...connection.config, - oauth: { ...oauth, ...nextBinding }, - }; - const [updated] = await db - .update(toolConnections) - .set({ - config: nextConfig, - transportConfig: nextConfig, - updatedAt: now(), - }) - .where( - and( - eq(toolConnections.id, connection.id), - eq(toolConnections.companyId, connection.companyId), - ), - ) - .returning(); - return updated ?? connection; + return db.transaction(async tx => { + const [saved] = await tx.select().from(toolConnections).where(and( + eq(toolConnections.id, connection.id), eq(toolConnections.companyId, connection.companyId), + )).for("no key update"); + if (!saved) throw notFound("Tool connection not found"); + assertOAuthClientBindingUnchanged(connection, saved, endpoints, redirectUri); + const originalOAuth = oauthConfig(saved); + const oauth = reviewedOAuthClientBinding(saved, endpoints); + const nextBinding = { + clientRedirectUri: redirectUri, + clientIssuer: + typeof oauth.clientIssuer === "string" && oauth.clientIssuer + ? oauth.clientIssuer + : (endpoints.issuer ?? null), + clientResource: + typeof oauth.clientResource === "string" && oauth.clientResource + ? oauth.clientResource + : (endpoints.resource ?? null), + clientCompanyId: + typeof oauth.clientCompanyId === "string" && oauth.clientCompanyId + ? oauth.clientCompanyId + : saved.companyId, + }; + const unchanged = Object.entries(nextBinding).every( + ([key, value]) => originalOAuth[key] === value, + ); + if (unchanged) return saved; + const nextConfig = { + ...saved.config, + oauth: { ...oauth, ...nextBinding }, + }; + const nextTransportConfig = { ...saved.transportConfig, oauth: { ...asRecord(saved.transportConfig.oauth), ...nextBinding } }; + const [updated] = await tx + .update(toolConnections) + .set({ + config: nextConfig, + transportConfig: nextTransportConfig, + updatedAt: now(), + }) + .where( + and( + eq(toolConnections.id, connection.id), + eq(toolConnections.companyId, connection.companyId), + ), + ) + .returning(); + if (!updated) throw notFound("Tool connection not found"); + return updated; + }); } /** diff --git a/ui/src/features/connections/GoogleWorkspaceServices.test.tsx b/ui/src/features/connections/GoogleWorkspaceServices.test.tsx index 86865bd28be..79023e59337 100644 --- a/ui/src/features/connections/GoogleWorkspaceServices.test.tsx +++ b/ui/src/features/connections/GoogleWorkspaceServices.test.tsx @@ -32,6 +32,7 @@ describe("Workspace service consent", () => { } beforeEach(() => { vi.clearAllMocks(); + listGrants.mockResolvedValue({ currentUserId: "me", grants: [] }); container = document.createElement("div"); document.body.append(container); root = createRoot(container); @@ -68,6 +69,36 @@ describe("Workspace service consent", () => { expect(onContinue).toHaveBeenCalledWith({ gmail: "read" }); }); + it("restores a cold draft's selected services after declined OAuth", async () => { + await render({ ...existing, companyId: "company-1", status: "draft", config: { + workspaceSetup: { + version: 1, companyId: "company-1", connectionId: existing.id, + actorType: "user", actorId: "me", grantKind: "user", subjectId: "me", + requestedScopes: ["https://www.googleapis.com/auth/gmail.readonly"], + }, + oauth: { scopes: ["https://www.googleapis.com/auth/documents"] }, + } }); + expect([...document.querySelectorAll('[role="switch"]')].map(item => item.getAttribute("aria-checked"))).toEqual(["true", "false", "false"]); + expect(document.querySelector("select")?.value).toBe("read"); + await click("Continue"); + expect(onContinue).toHaveBeenCalledWith({ gmail: "read" }); + }); + + it.each([undefined, "another-user"])("requires a deliberate choice for an unbound draft (%s)", async (actorId) => { + await render({ ...existing, companyId: "company-1", status: "draft", config: { + ...(actorId ? { workspaceSetup: { + version: 1, companyId: "company-1", connectionId: existing.id, + actorType: "user", actorId, grantKind: "user", subjectId: "me", + requestedScopes: ["https://www.googleapis.com/auth/gmail.readonly"], + } } : {}), + oauth: { scopes: ["https://www.googleapis.com/auth/documents"] }, + } }); + expect([...document.querySelectorAll('[role="switch"]')].map(item => item.getAttribute("aria-checked"))).toEqual(["false", "false", "false"]); + const proceed = [...document.querySelectorAll("button")].find(item => item.textContent === "Continue")!; + expect(proceed.disabled).toBe(true); + expect(onContinue).not.toHaveBeenCalled(); + }); + it("blocks on grant-load failure and retries without broadening the saved request", async () => { listGrants.mockRejectedValueOnce(new Error("unavailable")); await render(existing); diff --git a/ui/src/features/connections/GoogleWorkspaceServices.tsx b/ui/src/features/connections/GoogleWorkspaceServices.tsx index b0a9c21e2ba..0b05da2595a 100644 --- a/ui/src/features/connections/GoogleWorkspaceServices.tsx +++ b/ui/src/features/connections/GoogleWorkspaceServices.tsx @@ -1,6 +1,6 @@ import { useState } from "react"; import { useQuery } from "@tanstack/react-query"; -import { GOOGLE_WORKSPACE_DEFAULT_SELECTION, GOOGLE_WORKSPACE_SERVICE_GROUPS, googleWorkspaceSelectionForScopes, type GoogleWorkspaceServiceSelection, type ToolConnection } from "@paperclipai/shared"; +import { GOOGLE_WORKSPACE_DEFAULT_SELECTION, GOOGLE_WORKSPACE_SERVICE_GROUPS, googleWorkspaceDraftRequestedScopes, googleWorkspaceSelectionForScopes, type GoogleWorkspaceServiceSelection, type ToolConnection } from "@paperclipai/shared"; import { toolsApi } from "@/api/tools"; import { Button } from "@/components/ui/button"; import { Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle } from "@/components/ui/dialog"; @@ -43,12 +43,11 @@ type DialogProps = { }; export function GoogleWorkspaceServicesDialog(props: DialogProps) { - const existing = props.connection && props.connection.status !== "draft" ? props.connection : null; + const existing = props.connection ?? null; const oauth = props.connection?.config?.oauth as { pendingWorkspaceServiceRequests?: Record } | undefined; const pendingRequests = oauth?.pendingWorkspaceServiceRequests; - const loadConnection = existing ?? (pendingRequests && Object.keys(pendingRequests).length ? props.connection : null); - const grants = useQuery({ queryKey: ["workspace-service-consent", loadConnection?.id], queryFn: () => toolsApi.listConnectionGrants(loadConnection!.id), enabled: Boolean(loadConnection) }); - if (loadConnection && !grants.data) return { if (!open) props.onCancel(); }}> + const grants = useQuery({ queryKey: ["workspace-service-consent", existing?.id], queryFn: () => toolsApi.listConnectionGrants(existing!.id), enabled: Boolean(existing) }); + if (existing && !grants.data) return { if (!open) props.onCancel(); }}> Connect Google Workspace{grants.isError ? "Could not load your saved service access. Your connection has not changed." : "Loading your saved service access…"} ; @@ -57,7 +56,15 @@ export function GoogleWorkspaceServicesDialog(props: DialogProps) { : existing?.credentialPolicy === "per_agent" ? grant.kind === "agent" && grant.subjectAgentId === (existing.config?.oauth as { connectorSubjectAgentId?: string } | undefined)?.connectorSubjectAgentId : grant.kind === "organization"); - const savedScopes = grant?.providerTenant?.oauth?.requestedScopes ?? grant?.providerTenant?.oauth?.scopes ?? []; + const savedScopes = existing?.status === "draft" + ? googleWorkspaceDraftRequestedScopes(existing.config?.workspaceSetup, { + companyId: existing.companyId, connectionId: existing.id, + actorType: "user", actorId: grants.data?.currentUserId ?? null, + grantKind: existing.credentialPolicy === "per_user" ? "user" : existing.credentialPolicy === "per_agent" ? "agent" : "organization", + subjectId: existing.credentialPolicy === "per_user" ? grants.data?.currentUserId ?? null + : existing.credentialPolicy === "per_agent" ? (existing.config?.oauth as { connectorSubjectAgentId?: string } | undefined)?.connectorSubjectAgentId ?? null : null, + }) ?? grant?.providerTenant?.oauth?.requestedScopes ?? [] + : grant?.providerTenant?.oauth?.requestedScopes ?? grant?.providerTenant?.oauth?.scopes ?? []; const pending = grants.data?.currentUserId ? pendingRequests?.[grants.data.currentUserId] : undefined; const pendingTime = Date.parse(pending?.requestedAt ?? ""); // Refresh and lease writes update the grant row without changing consent. diff --git a/ui/src/pages/apps/AppsConnect.test.tsx b/ui/src/pages/apps/AppsConnect.test.tsx index 23273b9550e..327b3f36eb2 100644 --- a/ui/src/pages/apps/AppsConnect.test.tsx +++ b/ui/src/pages/apps/AppsConnect.test.tsx @@ -24,6 +24,7 @@ vi.mock("@/api/instanceSettings", () => ({ instanceSettingsApi: { } })); const listApplicationsMock = vi.hoisted(() => vi.fn()); const listConnectionsMock = vi.hoisted(() => vi.fn()); +const listConnectionGrantsMock = vi.hoisted(() => vi.fn()); const getConnectionMock = vi.hoisted(() => vi.fn()); const getConnectionInstallsMock = vi.hoisted(() => vi.fn()); const connectAppMock = vi.hoisted(() => vi.fn()); @@ -82,6 +83,7 @@ vi.mock("@/api/tools", () => ({ listGallery: (companyId: string) => listGalleryMock(companyId), listApplications: (companyId: string) => listApplicationsMock(companyId), listConnections: (companyId: string) => listConnectionsMock(companyId), + listConnectionGrants: (connectionId: string) => listConnectionGrantsMock(connectionId), getConnection: (id: string) => getConnectionMock(id), getConnectionInstalls: (id: string) => getConnectionInstallsMock(id), connectApp: (companyId: string, input: unknown) => connectAppMock(companyId, input), @@ -2894,6 +2896,34 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { ); }); + it("keeps Gmail read-only selected when Finish setup resumes a declined Workspace draft", async () => { + const connectionId = "22222222-2222-4222-8222-222222222222"; + const workspace = getAppStoreDefinition("google-workspace")!; + mockSearch.value = `source=google-workspace&resume=${connectionId}&oauth=denied`; + listGalleryMock.mockResolvedValue({ apps: [{ ...workspace, ownershipAvailability: { ...workspace.ownershipAvailability, platform_shared: true } }] }); + listApplicationsMock.mockResolvedValue({ applications: [{ id: "app-workspace", companyId: "company-1", status: "draft", metadata: { sourceTemplateKey: "google-workspace" } }] }); + listConnectionsMock.mockResolvedValue({ connections: [{ + ...notionConnection(connectionId, "per_user", "draft"), companyId: "company-1", applicationId: "app-workspace", + config: { sourceTemplateKey: "google-workspace", connectionMethodKey: "paperclip-workspace", workspaceSetup: { + version: 1, companyId: "company-1", connectionId, actorType: "user", actorId: "me", grantKind: "user", subjectId: "me", + requestedScopes: ["https://www.googleapis.com/auth/gmail.readonly"], + } }, + }] }); + listConnectionGrantsMock.mockResolvedValue({ currentUserId: "me", grants: [] }); + await render(); + await vi.waitFor(() => expect(container.textContent).toContain("Try again")); + await act(async () => buttonByText("Try again")!.click()); + await flushReact(); + await vi.waitFor(() => expect(document.querySelector('[role="dialog"] select')).not.toBeNull()); + const dialog = document.querySelector('[role="dialog"]')!; + expect([...dialog.querySelectorAll('[role="switch"]')].map(item => item.getAttribute("aria-checked"))).toEqual(["true", "false", "false"]); + expect(dialog.querySelector("select")?.value).toBe("read"); + await act(async () => buttonByText("Continue")!.click()); + await flushReact(); + expect(connectAppMock).not.toHaveBeenCalled(); + expect(startOAuthMock).toHaveBeenCalledWith(connectionId, { asCurrentUser: true, workspaceServices: { gmail: "read" } }); + }); + it("resumes a hidden provider draft only after matching its company-visible application", async () => { const connectionId = "22222222-2222-4222-8222-222222222222"; mockSearch.value = `source=clickup&resume=${connectionId}`;