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}`;