From 0b73ebb86c9bc97675b789c7d9221e6da57ff66e Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 31 Aug 2026 20:40:12 -0500 Subject: [PATCH] Fix managed OAuth catalog activation (#12623) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Apps system gives agents governed access to external services > - The managed OAuth callback discovers provider tools before it activates a grant > - Fresh managed connections kept every discovered tool in quarantine > - The Apps page also counted disabled tools as available actions > - This pull request makes setup activation atomic and keeps later catalog changes quarantined > - The benefit is a usable catalog after consent without weakening reauthorization safeguards ## Linked Issues or Issue Description N/A — no public GitHub issue exists for this follow-up. Related merged work: [#12619](https://github.com/paperclipai/paperclip/pull/12619) and [paperclip-cloud #319](https://github.com/paperclipai/paperclip-cloud/pull/319). **What happened?** A fresh Paperclip-managed OAuth connection discovered the correct Google Workspace tools, but it left every allowed tool in quarantine. The Apps page then reported zero actions for write profiles and counted disabled actions for read profiles. **Expected behavior** A fresh or revived managed connection must remain disabled until Paperclip stores credentials, discovers the catalog, reviews the profile allowlist, installs default policies, and activates the connection. A later reauthorization must preserve user choices. A later catalog change must quarantine new or changed tools. **Steps to reproduce** 1. Connect a managed Google Workspace write profile. 2. Complete provider consent and return through the instance callback. 3. Open the connection in Apps. 4. Observe that the connection is active but the allowed actions remain quarantined. **Paperclip version or commit** Commit `c7ebc089c` from merged pull request #12619. **Deployment mode** Self-hosted local development through Tailscale HTTPS. The same callback logic applies to Cloud-hosted instances. **Installation method** Built from source with pnpm. **Agent adapter(s) involved** Not adapter-specific. This change affects the core Apps and tool-access paths. ## What Changed - Kept fresh and revived managed connections in the draft state until catalog finalization succeeds. - Added a managed-draft refresh option that quarantines discovery results without changing generic draft behavior. - Activated reviewed profile tools, created bindings, and installed ask-first policies in the existing finalization transaction. - Preserved custom profiles, bindings, archived state, and policies during ordinary reauthorization. - Kept new or changed tools quarantined after activation and kept out-of-profile tools disabled. - Counted only active catalog entries as available actions in the Apps page. - Added retry, revival, reauthorization, policy, profile, and UI regression coverage. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/tool-access-service.test.ts` — passed, 208 tests. - `pnpm --filter @paperclipai/ui exec vitest run src/pages/apps/AppDetail.test.tsx` — passed, 52 tests. - Server and UI typechecks passed. - `pnpm build` — passed on the final tree. - `pnpm check:token-gates` — passed. - `git diff --check` — passed. - Browser walkthrough — passed for all 16 enabled Google Workspace profiles through the staging Cloud broker and a self-hosted Tailscale HTTPS instance. Every final connection became active and exposed at least one allowed action. - `pnpm test:run` — the changed suites passed. The shared live-QA environment caused unrelated workspace-runtime concurrency and cleanup failures, so hosted CI is the clean-environment authority for the full suite. ## Risks - The change affects managed OAuth only. Customer-owned OAuth setup keeps its current behavior. - A failed initial finalization now leaves a safe draft that the callback can retry. - An ordinary active reauthorization does not rebuild defaults, so existing user policy remains intact. - There are no database migrations and no public API changes. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex, GPT-5.6, with reasoning, repository tools, browser control, code execution, test execution, and parallel subagent review. The effective context window was managed by the Codex task runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and the changed suites pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- .../src/__tests__/tool-access-service.test.ts | 410 +++++++++++++++++- server/src/services/tool-access.ts | 91 +++- ui/src/pages/apps/AppDetail.test.tsx | 17 + ui/src/pages/apps/AppDetail.tsx | 2 +- 4 files changed, 495 insertions(+), 25 deletions(-) diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index 2a897f1dbe..2c9050a219 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -4999,6 +4999,7 @@ describeEmbeddedPostgres("tool access service", () => { healthStatus: "ok", config: { sourceTemplateKey: "google-drive", + quarantineNewEntries: true, oauth: { strategy: "paperclip_cloud_connector", provider: "google-drive", @@ -5009,9 +5010,26 @@ describeEmbeddedPostgres("tool access service", () => { }, }); expect(callback.body.catalog).toEqual(expect.arrayContaining([ - expect.objectContaining({ toolName: "search_files", status: "quarantined", riskLevel: "read" }), + expect.objectContaining({ toolName: "search_files", status: "active", riskLevel: "read" }), expect.objectContaining({ toolName: "create_file", status: "disabled", riskLevel: "write" }), ])); + const [profileRow] = await db.select().from(toolProfiles).where(eq( + toolProfiles.profileKey, + `app:${connected.connectionId}`, + )); + const profileEntries = await db.select().from(toolProfileEntries).where(eq( + toolProfileEntries.profileId, + profileRow!.id, + )); + expect(profileEntries).toHaveLength(1); + expect(profileEntries[0]).toMatchObject({ + catalogEntryId: callback.body.catalog.find((entry: { toolName: string }) => entry.toolName === "search_files").id, + effect: "include", + }); + await expect(db.select().from(toolConnectionInstalls).where(and( + eq(toolConnectionInstalls.connectionId, connected.connectionId), + eq(toolConnectionInstalls.targetType, "company"), + ))).resolves.toHaveLength(1); const [grant] = await db.select().from(connectionGrants).where(and( eq(connectionGrants.connectionId, connected.connectionId), @@ -5065,6 +5083,396 @@ describeEmbeddedPostgres("tool access service", () => { } }); + it("activates allowed Drive write actions with recommended approval defaults after a managed callback", async () => { + const company = await createCompany(db); + const userId = `drive-write-member-${randomUUID()}`; + await grantBoardUser(db, company.id, userId, [], "owner"); + const profile = "drive.write" as const; + const connector = fakeGoogleWorkspaceConnector(company.id, userId, profile); + const service = createTestToolAccessService(db, { paperclipCloudConnector: connector }); + const actor = { actorType: "user" as const, actorId: userId }; + const driveDefinition = getConnectableAppDefinition("google-drive")!; + const previousOwnershipAvailability = driveDefinition.ownershipAvailability; + driveDefinition.ownershipAvailability = { ...previousOwnershipAvailability, platform_shared: true }; + mockToolsList([ + { name: "search_files", annotations: { readOnlyHint: true } }, + { name: "create_file", annotations: { readOnlyHint: false } }, + { name: "delete_file", annotations: { destructiveHint: true } }, + ]); + + try { + const connected = await service.connectGalleryApp(company.id, { + galleryKey: "google-drive", + connectionMethodKey: "paperclip-write", + grantKind: "user", + name: "Drive managed write", + }, actor); + const started = await service.startOAuth(company.id, connected.connectionId, { + redirectUri: "https://paperclip.example/api/tools/oauth/cloud-connector/callback", + actor, + }); + const state = new URL(started.authorizationUrl).searchParams.get("state")!; + const app = createRouteApp( + db, + boardSessionActor(company.id, "owner", userId), + undefined, + { paperclipCloudConnector: connector }, + ); + + const callback = await request(app) + .get("/api/tools/oauth/cloud-connector/callback") + .query({ state, claim_id: "drive-write-claim" }) + .set("accept", "application/json"); + + expect(callback.status).toBe(200); + expect(callback.body.connection.config.quarantineNewEntries).toBe(true); + expect(callback.body.catalog).toEqual(expect.arrayContaining([ + expect.objectContaining({ toolName: "search_files", status: "active", riskLevel: "read" }), + expect.objectContaining({ toolName: "create_file", status: "active", riskLevel: "write" }), + expect.objectContaining({ toolName: "delete_file", status: "disabled", riskLevel: "destructive" }), + ])); + const [profileRow] = await db.select().from(toolProfiles).where(eq( + toolProfiles.profileKey, + `app:${connected.connectionId}`, + )); + const profileEntries = await db.select().from(toolProfileEntries).where(eq( + toolProfileEntries.profileId, + profileRow!.id, + )); + expect(profileEntries.map((entry) => entry.catalogEntryId).sort()).toEqual( + callback.body.catalog + .filter((entry: { status: string }) => entry.status === "active") + .map((entry: { id: string }) => entry.id) + .sort(), + ); + const searchEntry = callback.body.catalog.find((entry: { toolName: string }) => entry.toolName === "search_files"); + const createEntry = callback.body.catalog.find((entry: { toolName: string }) => entry.toolName === "create_file"); + const [approvalPolicy] = await db.select().from(toolPolicies).where(and( + eq(toolPolicies.companyId, company.id), + eq(toolPolicies.enabled, true), + )); + expect(approvalPolicy).toMatchObject({ + policyType: "require_approval", + selectors: expect.objectContaining({ catalogEntryId: createEntry.id }), + }); + await expect(db.select().from(toolConnectionInstalls).where(and( + eq(toolConnectionInstalls.connectionId, connected.connectionId), + eq(toolConnectionInstalls.targetType, "company"), + ))).resolves.toHaveLength(1); + + const agent = await createAgent(db, company.id); + await db.delete(toolProfileEntries).where(and( + eq(toolProfileEntries.profileId, profileRow!.id), + eq(toolProfileEntries.catalogEntryId, searchEntry.id), + )); + await db.delete(toolProfileBindings).where(eq(toolProfileBindings.profileId, profileRow!.id)); + await db.insert(toolProfileBindings).values({ + companyId: company.id, + profileId: profileRow!.id, + targetType: "agent", + targetId: agent.id, + }); + await db.update(toolProfiles).set({ status: "archived" }).where(eq(toolProfiles.id, profileRow!.id)); + await db.update(toolPolicies).set({ enabled: false }).where(eq(toolPolicies.id, approvalPolicy!.id)); + + mockToolsList([ + { name: "search_files", description: "Search files with a changed contract.", annotations: { readOnlyHint: true } }, + { name: "create_file", annotations: { readOnlyHint: false } }, + { name: "copy_file", annotations: { readOnlyHint: false } }, + { name: "delete_file", annotations: { destructiveHint: true } }, + ]); + const reconnect = await service.startOAuth(company.id, connected.connectionId, { + redirectUri: "https://paperclip.example/api/tools/oauth/cloud-connector/callback", + actor, + }); + const reconnectState = new URL(reconnect.authorizationUrl).searchParams.get("state")!; + const reconnected = await request(app) + .get("/api/tools/oauth/cloud-connector/callback") + .query({ state: reconnectState, claim_id: "drive-write-reconnect-claim" }) + .set("accept", "application/json"); + + expect(reconnected.status).toBe(200); + expect(reconnected.body.connection.config.quarantineNewEntries).toBe(true); + expect(reconnected.body.catalog).toEqual(expect.arrayContaining([ + expect.objectContaining({ toolName: "search_files", status: "quarantined" }), + expect.objectContaining({ toolName: "create_file", status: "active" }), + expect.objectContaining({ toolName: "copy_file", status: "quarantined" }), + expect.objectContaining({ toolName: "delete_file", status: "disabled" }), + ])); + await expect(db.select().from(toolProfiles).where(eq(toolProfiles.id, profileRow!.id))) + .resolves.toEqual([expect.objectContaining({ status: "archived" })]); + await expect(db.select().from(toolProfileEntries).where(eq( + toolProfileEntries.profileId, + profileRow!.id, + ))).resolves.toEqual([ + expect.objectContaining({ catalogEntryId: createEntry.id, effect: "include" }), + ]); + await expect(db.select().from(toolProfileBindings).where(eq( + toolProfileBindings.profileId, + profileRow!.id, + ))).resolves.toEqual([ + expect.objectContaining({ targetType: "agent", targetId: agent.id }), + ]); + await expect(db.select().from(toolPolicies).where(eq(toolPolicies.id, approvalPolicy!.id))) + .resolves.toEqual([expect.objectContaining({ enabled: false })]); + } finally { + driveDefinition.ownershipAvailability = previousOwnershipAvailability; + } + }); + + it("keeps a managed draft retryable when recommended-default finalization fails", async () => { + const company = await createCompany(db); + const userId = `drive-finalize-failure-${randomUUID()}`; + await grantBoardUser(db, company.id, userId, [], "owner"); + const connector = fakeGoogleWorkspaceConnector(company.id, userId, "drive.write"); + const service = createTestToolAccessService(db, { paperclipCloudConnector: connector }); + const actor = { actorType: "user" as const, actorId: userId }; + const driveDefinition = getConnectableAppDefinition("google-drive")!; + const previousOwnershipAvailability = driveDefinition.ownershipAvailability; + driveDefinition.ownershipAvailability = { ...previousOwnershipAvailability, platform_shared: true }; + mockToolsList([ + { name: "search_files", annotations: { readOnlyHint: true } }, + { name: "create_file", annotations: { readOnlyHint: false } }, + { name: "delete_file", annotations: { destructiveHint: true } }, + ]); + + try { + const connected = await service.connectGalleryApp(company.id, { + galleryKey: "google-drive", + connectionMethodKey: "paperclip-write", + grantKind: "user", + name: "Drive managed finalize failure", + }, actor); + const started = await service.startOAuth(company.id, connected.connectionId, { + redirectUri: "https://paperclip.example/api/tools/oauth/cloud-connector/callback", + actor, + }); + const state = new URL(started.authorizationUrl).searchParams.get("state")!; + const originalTransaction = db.transaction.bind(db); + let transactionCount = 0; + const transactionSpy = vi.spyOn(db, "transaction").mockImplementation((async (operation, config) => { + transactionCount += 1; + if (transactionCount === 2) throw new Error("recommended defaults failed"); + return originalTransaction(operation, config); + }) as typeof db.transaction); + + await expect(service.completePaperclipCloudConnectorCallback({ + state, + claimId: "drive-finalize-failure-claim", + actor, + })).rejects.toThrow("recommended defaults failed"); + + await expect(db.select().from(toolCatalogEntries).where(eq( + toolCatalogEntries.connectionId, + connected.connectionId, + ))).resolves.toEqual(expect.arrayContaining([ + expect.objectContaining({ toolName: "search_files", status: "quarantined" }), + expect.objectContaining({ toolName: "create_file", status: "quarantined" }), + expect.objectContaining({ toolName: "delete_file", status: "disabled" }), + ])); + await expect(db.select().from(toolProfiles).where(eq( + toolProfiles.profileKey, + `app:${connected.connectionId}`, + ))).resolves.toHaveLength(0); + await expect(service.getConnection(connected.connectionId)).resolves.toMatchObject({ + status: "draft", + enabled: false, + config: { quarantineNewEntries: true }, + }); + await expect(db.select().from(toolApplications).where(eq( + toolApplications.id, + connected.application.id, + ))).resolves.toEqual([expect.objectContaining({ status: "draft" })]); + + transactionSpy.mockRestore(); + const retry = await service.startOAuth(company.id, connected.connectionId, { + redirectUri: "https://paperclip.example/api/tools/oauth/cloud-connector/callback", + actor, + }); + const completed = await service.completePaperclipCloudConnectorCallback({ + state: new URL(retry.authorizationUrl).searchParams.get("state")!, + claimId: "drive-finalize-retry-claim", + actor, + }); + + expect(completed.connection).toMatchObject({ + status: "active", + enabled: true, + config: { quarantineNewEntries: true }, + }); + expect(completed.catalog).toEqual(expect.arrayContaining([ + expect.objectContaining({ toolName: "search_files", status: "active" }), + expect.objectContaining({ toolName: "create_file", status: "active" }), + expect.objectContaining({ toolName: "delete_file", status: "disabled" }), + ])); + const [profileRow] = await db.select().from(toolProfiles).where(eq( + toolProfiles.profileKey, + `app:${connected.connectionId}`, + )); + const activeEntryIds = completed.catalog + .filter((entry) => entry.status === "active") + .map((entry) => entry.id) + .sort(); + await expect(db.select().from(toolProfileEntries).where(eq( + toolProfileEntries.profileId, + profileRow!.id, + )).then((rows) => rows.map((entry) => entry.catalogEntryId).sort())) + .resolves.toEqual(activeEntryIds); + await expect(db.select().from(toolProfileBindings).where(eq( + toolProfileBindings.profileId, + profileRow!.id, + ))).resolves.toEqual([ + expect.objectContaining({ targetType: "company", targetId: company.id }), + ]); + const createEntry = completed.catalog.find((entry) => entry.toolName === "create_file")!; + await expect(db.select().from(toolPolicies).where(and( + eq(toolPolicies.companyId, company.id), + eq(toolPolicies.enabled, true), + ))).resolves.toEqual([ + expect.objectContaining({ + policyType: "require_approval", + selectors: expect.objectContaining({ catalogEntryId: createEntry.id }), + }), + ]); + await expect(db.select().from(toolConnectionInstalls).where(and( + eq(toolConnectionInstalls.connectionId, connected.connectionId), + eq(toolConnectionInstalls.targetType, "company"), + ))).resolves.toHaveLength(1); + await expect(db.select().from(toolApplications).where(eq( + toolApplications.id, + connected.application.id, + ))).resolves.toEqual([expect.objectContaining({ status: "active" })]); + + mockToolsList([ + { name: "search_files", description: "Changed after retry.", annotations: { readOnlyHint: true } }, + { name: "create_file", annotations: { readOnlyHint: false } }, + { name: "copy_file", annotations: { readOnlyHint: false } }, + { name: "delete_file", annotations: { destructiveHint: true } }, + ]); + const futureRefresh = await service.refreshCatalog(connected.connectionId, actor); + expect(futureRefresh.catalog).toEqual(expect.arrayContaining([ + expect.objectContaining({ toolName: "search_files", status: "quarantined" }), + expect.objectContaining({ toolName: "create_file", status: "active" }), + expect.objectContaining({ toolName: "copy_file", status: "quarantined" }), + expect.objectContaining({ toolName: "delete_file", status: "disabled" }), + ])); + expect(futureRefresh.connection.config.quarantineNewEntries).toBe(true); + } finally { + driveDefinition.ownershipAvailability = previousOwnershipAvailability; + } + }); + + it("rebuilds managed defaults when a removed connection is revived with its archived profile retained", async () => { + const company = await createCompany(db); + const userId = `drive-revival-${randomUUID()}`; + await grantBoardUser(db, company.id, userId, [], "owner"); + const connector = fakeGoogleWorkspaceConnector(company.id, userId, "drive.write"); + const service = createTestToolAccessService(db, { paperclipCloudConnector: connector }); + const actor = { actorType: "user" as const, actorId: userId }; + const driveDefinition = getConnectableAppDefinition("google-drive")!; + const previousOwnershipAvailability = driveDefinition.ownershipAvailability; + driveDefinition.ownershipAvailability = { ...previousOwnershipAvailability, platform_shared: true }; + mockToolsList([ + { name: "search_files", annotations: { readOnlyHint: true } }, + { name: "create_file", annotations: { readOnlyHint: false } }, + { name: "delete_file", annotations: { destructiveHint: true } }, + ]); + + try { + const first = await service.connectGalleryApp(company.id, { + galleryKey: "google-drive", + connectionMethodKey: "paperclip-write", + grantKind: "user", + name: "Drive managed revival", + }, actor); + const firstStart = await service.startOAuth(company.id, first.connectionId, { + redirectUri: "https://paperclip.example/api/tools/oauth/cloud-connector/callback", + actor, + }); + await service.completePaperclipCloudConnectorCallback({ + state: new URL(firstStart.authorizationUrl).searchParams.get("state")!, + claimId: "drive-revival-first-claim", + actor, + }); + const [retainedProfile] = await db.select().from(toolProfiles).where(eq( + toolProfiles.profileKey, + `app:${first.connectionId}`, + )); + await db.insert(toolMcpGateways).values({ + companyId: company.id, + name: `Retained managed gateway ${randomUUID()}`, + slug: `retained-managed-${randomUUID()}`, + profileId: retainedProfile!.id, + status: "active", + }); + await service.archiveConnection(first.connectionId, company.id, actor); + await expect(db.select().from(toolProfiles).where(eq(toolProfiles.id, retainedProfile!.id))) + .resolves.toEqual([expect.objectContaining({ status: "archived" })]); + + const revived = await service.connectGalleryApp(company.id, { + galleryKey: "google-drive", + connectionMethodKey: "paperclip-write", + grantKind: "user", + name: "Drive managed revival", + }, actor); + expect(revived.connectionId).toBe(first.connectionId); + expect(revived.connection).toMatchObject({ status: "draft", enabled: false }); + await expect(db.select().from(toolProfiles).where(eq(toolProfiles.id, retainedProfile!.id))) + .resolves.toEqual([expect.objectContaining({ status: "archived" })]); + + const revivedStart = await service.startOAuth(company.id, revived.connectionId, { + redirectUri: "https://paperclip.example/api/tools/oauth/cloud-connector/callback", + actor, + }); + const completed = await service.completePaperclipCloudConnectorCallback({ + state: new URL(revivedStart.authorizationUrl).searchParams.get("state")!, + claimId: "drive-revival-second-claim", + actor, + }); + + expect(completed.connection).toMatchObject({ + status: "active", + enabled: true, + config: { quarantineNewEntries: true }, + }); + expect(completed.catalog).toEqual(expect.arrayContaining([ + expect.objectContaining({ toolName: "search_files", status: "active" }), + expect.objectContaining({ toolName: "create_file", status: "active" }), + expect.objectContaining({ toolName: "delete_file", status: "disabled" }), + ])); + await expect(db.select().from(toolProfiles).where(eq(toolProfiles.id, retainedProfile!.id))) + .resolves.toEqual([expect.objectContaining({ status: "active" })]); + const revivedEntries = await db.select().from(toolProfileEntries).where(eq( + toolProfileEntries.profileId, + retainedProfile!.id, + )); + expect(revivedEntries.map((entry) => entry.catalogEntryId).sort()).toEqual( + completed.catalog + .filter((entry) => entry.status === "active") + .map((entry) => entry.id) + .sort(), + ); + await expect(db.select().from(toolProfileBindings).where(eq( + toolProfileBindings.profileId, + retainedProfile!.id, + ))).resolves.toEqual([ + expect.objectContaining({ targetType: "company", targetId: company.id }), + ]); + const createEntry = completed.catalog.find((entry) => entry.toolName === "create_file")!; + await expect(db.select().from(toolPolicies).where(and( + eq(toolPolicies.companyId, company.id), + eq(toolPolicies.enabled, true), + ))).resolves.toEqual([ + expect.objectContaining({ + policyType: "require_approval", + selectors: expect.objectContaining({ catalogEntryId: createEntry.id }), + }), + ]); + } finally { + driveDefinition.ownershipAvailability = previousOwnershipAvailability; + } + }); + it("keeps brokered OAuth state retryable until credentials are durably stored", async () => { const company = await createCompany(db); const userId = `gmail-retry-${randomUUID()}`; diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index 60cab5d615..e94facb478 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -5358,6 +5358,9 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} refreshOptions: { enableAllByDefault?: boolean; restoreDraftDefaults?: boolean; + /** Keep first managed OAuth discovery quarantined without changing generic draft refresh semantics. */ + quarantineManagedOAuthDraft?: boolean; + skipDefaultProfileSync?: boolean; credentialHeaders?: Record; } = {}, ): Promise { @@ -5405,7 +5408,11 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} : null; const quarantineOnRefresh = !refreshOptions.enableAllByDefault && shouldQuarantineNewEntries(connection) - && (connection.status === "active" || sourceTemplateKey === "posthog"); + && ( + connection.status === "active" + || sourceTemplateKey === "posthog" + || refreshOptions.quarantineManagedOAuthDraft === true + ); const safeDefault = asRecord(connection.config).safeDefault === true; for (const descriptor of descriptors) { const riskLevel = classifyRisk(descriptor, sourceTemplateKey); @@ -5521,20 +5528,22 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} } const activeEntries = updatedEntries.filter((entry) => entry.status === "active"); - await enableCatalogEntriesByDefault({ - connection: updatedConnection, - newCatalogEntryIds: refreshOptions.enableAllByDefault - ? activeEntries.map((entry) => entry.id) - : activeEntries - .filter((entry) => { - const previous = existingByName.get(entry.toolName); - return !previous || previous.status === "quarantined"; - }) - .map((entry) => entry.id), - activeCatalogEntryIds: activeEntries.map((entry) => entry.id), - restoreDraftDefaults: refreshOptions.restoreDraftDefaults, - actor, - }); + if (!refreshOptions.skipDefaultProfileSync) { + await enableCatalogEntriesByDefault({ + connection: updatedConnection, + newCatalogEntryIds: refreshOptions.enableAllByDefault + ? activeEntries.map((entry) => entry.id) + : activeEntries + .filter((entry) => { + const previous = existingByName.get(entry.toolName); + return !previous || previous.status === "quarantined"; + }) + .map((entry) => entry.id), + activeCatalogEntryIds: activeEntries.map((entry) => entry.id), + restoreDraftDefaults: refreshOptions.restoreDraftDefaults, + actor, + }); + } await audit({ companyId: connection.companyId, connectionId: connection.id, @@ -10040,6 +10049,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} connection: typeof toolConnections.$inferSelect; catalog: ToolCatalogEntry[]; suggestedDefaults: ConnectToolAppResult["suggestedDefaults"]; + activateQuarantined?: boolean; actor?: ActorInfo; }) { const installs = await db.select().from(toolConnectionInstalls).where(and( @@ -10060,12 +10070,17 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} ) : [], ); - const enabledCatalog = input.catalog.filter((entry) => entry.status === "active"); + const enabledCatalog = input.catalog.filter((entry) => + entry.status === "active" || (input.activateQuarantined === true && entry.status === "quarantined") + ); const finished = await finishGalleryAppConnection(input.connection.companyId, input.connection.id, { enabledCatalogEntryIds: enabledCatalog.map((entry) => entry.id), askFirstCatalogEntryIds: enabledCatalog .filter((entry) => askFirstRiskLevels.has(entry.riskLevel)) .map((entry) => entry.id), + reviewedCatalogEntryIds: input.activateQuarantined === true + ? enabledCatalog.filter((entry) => entry.status === "quarantined").map((entry) => entry.id) + : undefined, access, }, input.actor); if (installs.length === 0) { @@ -10092,6 +10107,13 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} // transient broker, database, or secret-store failure can retry safely. const stateRow = await validateOAuthState(input.state, input.actor); let connection = await getConnectionRow(stateRow.connectionId, stateRow.companyId); + // The connection lifecycle, not the incidental presence of its app profile, + // distinguishes setup from reauthorization. New connections and connections + // revived after removal are drafts until this callback completes. A profile + // may legitimately survive removal because an MCP gateway retains it, or be + // intentionally archived on an otherwise active connection; neither case + // should invert whether recommended defaults are rebuilt. + const shouldFinalizeManagedDefaults = connection.status === "draft"; const sourceTemplateKey = typeof connection.config.sourceTemplateKey === "string" ? connection.config.sourceTemplateKey : null; const galleryEntry = sourceTemplateKey ? getConnectableAppDefinition(sourceTemplateKey) : null; const method = galleryEntry ? connectionMethodForConnection(galleryEntry, connection) : null; @@ -10231,14 +10253,17 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} }, }; [connection] = await tx.update(toolConnections).set({ - status: "active", - enabled: true, + status: shouldFinalizeManagedDefaults ? "draft" : "active", + enabled: shouldFinalizeManagedDefaults ? false : true, authKind: "oauth", config: nextConfig, transportConfig: nextConfig, updatedAt: now(), }).where(eq(toolConnections.id, connection.id)).returning(); - await tx.update(toolApplications).set({ status: "active", updatedAt: now() }).where(eq(toolApplications.id, connection.applicationId)); + await tx.update(toolApplications).set({ + status: shouldFinalizeManagedDefaults ? "draft" : "active", + updatedAt: now(), + }).where(eq(toolApplications.id, connection.applicationId)); await syncCredentialBindings(connection, credentialSecretRefs, tx); const linkedInteractionKind = stateRow.interactionId ? await tx @@ -10260,16 +10285,36 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} }); const refresh = await refreshCatalog(connection.id, input.actor, { enableAllByDefault: false, + quarantineManagedOAuthDraft: shouldFinalizeManagedDefaults, + skipDefaultProfileSync: true, credentialHeaders: { Authorization: `Bearer ${credentials.accessToken}` }, }); + const suggestedDefaults = recommendedDefaultsForApp(galleryEntry!, method.key); + const finished = shouldFinalizeManagedDefaults + ? await finishOAuthCatalogWithRecommendedDefaults({ + connection, + catalog: refresh.catalog, + suggestedDefaults, + activateQuarantined: true, + actor: input.actor, + }) + : null; + const activatedCatalogEntryIds = new Set( + shouldFinalizeManagedDefaults + ? refresh.catalog.filter((entry) => entry.status === "quarantined").map((entry) => entry.id) + : [], + ); + const catalog = refresh.catalog.map((entry) => + activatedCatalogEntryIds.has(entry.id) ? { ...entry, status: "active" as const } : entry + ); const [application] = await db.select().from(toolApplications).where(eq(toolApplications.id, connection.applicationId)); return { connectionId: connection.id, application: toApplication(application), - connection: refresh.connection, - catalog: refresh.catalog, - actions: groupedActions(refresh.catalog), - suggestedDefaults: recommendedDefaultsForApp(galleryEntry!, method.key), + connection: finished?.connection ?? refresh.connection, + catalog, + actions: groupedActions(catalog), + suggestedDefaults, auth: null, }; } diff --git a/ui/src/pages/apps/AppDetail.test.tsx b/ui/src/pages/apps/AppDetail.test.tsx index 89d8fc0c61..62881e1df7 100644 --- a/ui/src/pages/apps/AppDetail.test.tsx +++ b/ui/src/pages/apps/AppDetail.test.tsx @@ -516,6 +516,23 @@ describe("AppDetail", () => { expect(container.querySelector("section.bg-card")).toBeNull(); }); + it("counts only active catalog entries as available actions", async () => { + mockParams.tab = "permissions"; + listCatalogMock.mockResolvedValue({ + catalog: [ + catalogEntry(), + catalogEntry({ id: "catalog-disabled", toolName: "disabled_action", status: "disabled" }), + catalogEntry({ id: "catalog-quarantined", toolName: "pending_action", status: "quarantined" }), + catalogEntry({ id: "catalog-removed", toolName: "removed_action", status: "removed" }), + ], + }); + + await renderAppDetail(); + + expect(container.textContent).toContain("1 action available"); + expect(container.textContent).not.toContain("2 actions available"); + }); + it("redirects the legacy Advanced route to Setup", async () => { mockParams.tab = "advanced"; diff --git a/ui/src/pages/apps/AppDetail.tsx b/ui/src/pages/apps/AppDetail.tsx index 8b2480b30a..12dd02d512 100644 --- a/ui/src/pages/apps/AppDetail.tsx +++ b/ui/src/pages/apps/AppDetail.tsx @@ -586,7 +586,7 @@ export function AppDetail() { const status = statusFor(connection); const needsReconnect = status.tone === "attention" && connection.healthStatus !== "unknown"; const quarantined = catalog.filter((e) => e.status === "quarantined"); - const active = catalog.filter((e) => e.status !== "quarantined" && e.status !== "removed"); + const active = catalog.filter((e) => e.status === "active"); const readOnly = active.filter((e) => e.isReadOnly); const canChange = active.filter((e) => !e.isReadOnly); const actionCount = catalogQuery.data ? active.length : null;