From c67909cb0f1fa973e0991ba068d9c8b2f8f64924 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 9 Nov 2026 13:46:28 -0600 Subject: [PATCH] fix: keep tool approvals valid through OAuth refresh and delayed review (#20340) ## Thinking Path > - Paperclip lets people manage agent work and approve app actions. > - An approval must bind the action that the person reviewed. > - OAuth access tokens can expire while that person is away. > - The old target snapshot treated each token refresh as a new authorization. It also imposed a one-hour review deadline. > - This pull request separates token renewal from authorization identity and keeps the human decision durable. > - Current access, the reviewed arguments, meaningful target changes, task closure, and execution receipts still control dispatch. > - People can approve later without repeating a decision after routine connection maintenance. ## Linked Issues or Issue Description **What happened?** A pending app action returned `approved_tool_target_changed` after its OAuth tokens refreshed. The tool and connection settings were unchanged. The same card also expired after one hour. A sign-in problem could discard an accepted decision before provider dispatch. **Expected behavior** The original card remains reviewable after routine OAuth refresh and a server restart. A later approval runs the exact reviewed action once. A connection repair preserves the decision while Paperclip verifies the same authorization. Meaningful changes require a new review. **Steps to reproduce** 1. Connect an OAuth MCP app and configure an action to require approval. 2. Request the action, then refresh its access and refresh tokens before approving. 3. Approve the original card. Before this change, it returns HTTP 409 even when the authorization is unchanged. 4. Wait more than one hour. Before this change, the card expires without a task-specific deadline. **Paperclip version or commit** Reproduced from master at `76254ead2229`. The final branch is rebased onto current master. ## What Changed - Add versioned approval target comparison. New snapshots bind credential identity epochs, consent, permissions, ownership, connection configuration, tool contracts, and credential references. Authenticated OAuth refresh preserves identity. Manual credential replacement does not. - Remove the implicit one-hour deadline from ordinary app action reviews. Keep explicit deadlines and terminal-task expiry. Synchronize the nullable interaction deadline in shared contracts and UI. - Keep accepted, undispatched actions when sign-in is required. Show a saved-approval repair state. Resume on committed connection changes after target and access revalidation, without an idle retry loop. - Preserve the existing execution claim, result receipt, and uncertain-outcome no-replay rules. - Explain meaningful target changes without inviting the person to retry the stale card. - Add a production task-thread Storybook example and document compatibility and recovery behavior. ## Verification - Red: production-refresher invalid-grant handling and formal-approval withdrawal during a sign-in wait also reproduced as failing regressions before their fixes. - Red: before implementation, the local OAuth HTTP fixture failed for both public and confidential clients with `approved_tool_target_changed` after token refresh. - Green: the extended fixture passes for public and confidential clients with personal and shared grants. It covers actual local HTTP token exchanges, multiple refreshes, a rebuilt gateway, an ended originating run, approval after 25 hours, duplicate approval, preserved sharing arguments, sign-in waiting, and event-driven repair. The gateways use the production OAuth refresher; an invalid refresh preserves the accepted decision before provider dispatch. Omitted-scope responses cover equivalent scope-reporting changes. The provider is an isolated local fixture, not a deployed Notebook service. - Negative controls refuse changed consent, scopes, tool definition, destination, and manual credential replacement before provider execution. - Existing gateway service, connected-tool acceptance, secret owner, secret service, unsigned-review queue, and connection-health suites passed. Explicit-deadline fixtures still prove expiry behavior, including deadlines on accepted sign-in waits. A withdrawn formal approval cancels a waiting action before connection recovery. - Approval card, task-chat, error-copy and target-comparison tests passed. Server and UI typechecks passed. Token gates and Storybook catalog checks passed. - Inspected the new saved-approval repair card in local Storybook using the production task thread. It shows a reconnect link and no second approval button or false running state. - `pnpm verify --explain` returned exit 2 and requested broad CI coverage. [Full CI](https://github.com/paperclipai/paperclip/actions/runs/42677855507) passed on `eb4f1c7caa9ee6e802c314ffc5eadec475a5de0c`, including build, typecheck/release registry, general and serialized tests, and all eight browser shards. - Greptile reviewed the same final head at 5/5. Both findings from the first review are fixed and their threads resolved; no open follow-ups remain. ## Risks - No database migration is needed. The new comparison reuses the vault identity epoch. Only authenticated OAuth refresh code can preserve that epoch; HTTP credential-write schemas cannot select that behavior. - Earlier signed requests keep their original exact-version checks and saved deadlines. Already-expired cards are not revived. A historical card whose credentials already changed needs a fresh review. - A new consent, account, permission set, destination, or manually replaced credential must not inherit the old authorization. Reconnection continues automatically only when revalidation proves the same saved authorization. - The interaction enrichment now permits a null deadline. Existing string deadlines remain valid. The web client and shared validator change together. - This PR does not deploy to staging or perform a live Notebook publication. ## Model Used OpenAI Codex, GPT-6 family, with repository tools, code execution, and browser inspection. The runtime does not expose the exact model ID, context-window limit, or reasoning-mode label; those values are not asserted. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/PRODUCT.md | 2 + doc/SPEC-implementation.md | 2 + doc/execution-semantics.md | 26 ++ .../2026-11-09-durable-tool-approvals.md | 40 +++ packages/shared/src/types/issue.ts | 3 +- packages/shared/src/validators/issue.ts | 2 +- ...-access-oauth-protocol.integration.test.ts | 181 ++++++++++++- .../__tests__/tool-gateway-service.test.ts | 6 +- server/src/__tests__/tool-gateway.test.ts | 4 +- server/src/services/connection-credentials.ts | 7 +- server/src/services/secrets.ts | 15 +- .../tool-access/oauth/credential-store.ts | 2 + .../tool-access/oauth/grant-refresh.ts | 6 +- .../src/services/tool-approval-connection.ts | 40 +++ .../src/services/tool-approval-execution.ts | 23 +- .../src/services/tool-approval-target.test.ts | 105 ++++++++ server/src/services/tool-approval-target.ts | 109 ++++++++ .../tool-connection-health-notifications.ts | 6 +- server/src/services/tool-gateway.ts | 241 +++++++++++------- server/src/services/tool-review-expiry.ts | 45 +++- server/src/services/tool-review-projection.ts | 13 +- server/src/services/tool-review-recovery.ts | 5 + .../IssueThreadInteractionCard.test.tsx | 15 ++ .../components/IssueThreadInteractionCard.tsx | 18 +- .../TaskChatCompactInteractionCard.tsx | 8 +- .../lib/interaction-resolution-error.test.ts | 5 + ui/src/lib/interaction-resolution-error.ts | 2 + .../connection-reviews.stories.tsx | 20 +- 28 files changed, 800 insertions(+), 151 deletions(-) create mode 100644 doc/plans/2026-11-09-durable-tool-approvals.md create mode 100644 server/src/services/tool-approval-connection.ts create mode 100644 server/src/services/tool-approval-target.test.ts create mode 100644 server/src/services/tool-approval-target.ts diff --git a/doc/PRODUCT.md b/doc/PRODUCT.md index 5527cb5751f..1090293c15a 100644 --- a/doc/PRODUCT.md +++ b/doc/PRODUCT.md @@ -499,6 +499,8 @@ The agent performs authorized setup, reuses saved accounts and drafts, and hands Authorized policy administration includes user-requested changes that loosen the acting agent’s own tool controls. These writes remain bounded by management grants, resource scope and the responsible user’s current authority; they do not need an additional approval solely because the agent is a target. Subsequent tool calls obey the resulting policy. Agents cannot approve their own governed action requests or edit their own principal permission grants. +Ordinary tool approvals remain reviewable without an implicit one-hour deadline. Authenticated OAuth refresh preserves a pending approval for the same saved action and authorization. Sign-in repair retains an accepted decision and resumes through connection-change events after revalidation; changed consent, permissions, credentials or action targets require a new review. Current access, explicit deadlines, task closure and execution receipts remain enforced. See [Delayed tool approvals](execution-semantics.md#delayed-tool-approvals). + The implementation, operation inventory, permission matrix and fixture qualification are recorded in [Agent API parity for connection configuration](plans/2026-10-13-connection-api-parity.md). Provider fixtures establish API behavior; they do not constitute live provider setup or an empirical agent-success claim. ## Budget control behavior diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index ba8faa2826b..cf0a49b1094 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -2987,4 +2987,6 @@ The agent performs authorized setup, reuses saved accounts and drafts, and hands Authorized policy administration includes user-requested changes that loosen the acting agent’s own tool controls. These writes remain bounded by management grants, resource scope and the responsible user’s current authority; they do not need an additional approval solely because the agent is a target. Subsequent tool calls obey the resulting policy. Agents cannot approve their own governed action requests or edit their own principal permission grants. +Ordinary tool approvals remain reviewable without an implicit one-hour deadline. Authenticated OAuth refresh preserves a pending approval for the same saved action and authorization. Sign-in repair retains an accepted decision and resumes through connection-change events after revalidation; changed consent, permissions, credentials or action targets require a new review. Current access, explicit deadlines, task closure and execution receipts remain enforced. See [Delayed tool approvals](execution-semantics.md#delayed-tool-approvals). + The implementation, operation inventory, permission matrix and fixture qualification are recorded in [Agent API parity for connection configuration](plans/2026-10-13-connection-api-parity.md). Provider fixtures establish API behavior; they do not constitute live provider setup or an empirical agent-success claim. diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index fc2058fd367..a4608f08d47 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -244,6 +244,32 @@ writer and finalization holds remain until ordinary snapshot publication and cleanup complete. This fallback does not remove source files, rerun provider work, or solve capacity failure while creating the workspace archive itself. +### Delayed tool approvals + +Ordinary tool reviews have no implicit one-hour deadline. A person may approve +the saved action after the requesting run ends or the server restarts. Explicit +action deadlines and terminal-task expiry still apply. The shared interaction +payload represents the absence of a deadline with `toolAction.expiresAt = null`. + +New connected-tool snapshots bind the reviewed arguments, tool contract, +connection settings, credential owner, consent and permissions. Authenticated +OAuth refresh preserves the vault's credential identity epoch; manual credential +replacement and new consent do not. Token versions and refresh timestamps may +change without invalidating the action. Fixed credential versions and non-OAuth +credentials retain their exact-version checks. Earlier signed snapshots retain +their original strict comparison and saved deadlines; the server does not infer +continuity for historical rotations or revive already-expired requests. + +If sign-in is required before dispatch, the accepted decision remains saved. +The card shows that approval is waiting for the app connection and links to its +repair surface. Recovery waits without a recurring timer and wakes on committed +connection changes or startup. It checks the exact saved action and current +authority again. Repair under the same authorization can continue automatically; +new consent, changed permissions or changed credentials require a new review. +The existing execution claim and result receipt prevent repeated approval from +dispatching the action twice. Uncertain provider outcomes retain the existing +no-replay recovery contract. + ### Answered Slack conversations An accepted scheduled-monitor yield remains unfinished task work. Its committed diff --git a/doc/plans/2026-11-09-durable-tool-approvals.md b/doc/plans/2026-11-09-durable-tool-approvals.md new file mode 100644 index 00000000000..43885e024e6 --- /dev/null +++ b/doc/plans/2026-11-09-durable-tool-approvals.md @@ -0,0 +1,40 @@ +# Durable tool approvals + +Outcome: a person can approve the saved action later, after ordinary OAuth refresh +and a server restart. Paperclip executes its reviewed arguments once and returns +the result to the task. Delivery ends at a reviewed PR with passing checks. + +The approval binds the action, task, company, selected authorization, tool contract, +destination, and audience. OAuth token lifetime is separate from that decision. +Authenticated refresh may preserve credential identity; manual replacement and +new consent may not. Current access and task state still gate provider dispatch. + +| Acceptance | Evidence | +| --- | --- | +| Original failure reproduced | Real local OAuth HTTP exchange and two refreshes; original approval fails with `approved_tool_target_changed` before the fix | +| Approve later | Same HTTP fixture, ended requesting run, rebuilt gateway, approval after 25 hours | +| Execute once | Repeated approval returns the receipt without another provider call | +| Preserve boundaries | Changed consent, scope, identity, tool, destination, arguments, closed task, manual rotation and explicit deadlines refuse execution | +| Connection repair | Keep an accepted decision for a provably unchanged authorization during a pre-dispatch authentication wait; show repair in context and resume on connection changes | +| Upgrade compatibility | Old signatures retain strict version checks; no inferred continuity for already-rotated historical credentials | + +Owners: OAuth credential storage owns authenticated refresh continuity. The vault +owns credential identity epochs. Tool approval target validation owns the saved +contract. Existing approval execution, recovery and delivery own claims, results +and continuation. Shared interaction cards own user-facing repair and deadlines. + +Progress: reproduced the original 409 for both OAuth client modes before the +repair. The local OAuth HTTP journey now passes for personal and shared grants, +public and confidential clients, delayed review, ended runs, reconstructed gateway, +duplicate approval, changed scopes/consent/tool/destination, manual token replacement, +and event-driven authorization repair. The journey injects the production OAuth +refresher and proves that an invalid refresh preserves the decision before action +dispatch. It also covers equivalent scope-reporting changes, withdrawn formal +approvals during sign-in waits, and explicit deadlines on saved decisions. +Gateway service regressions, +connected-tool acceptance and vault suites passed after making their intentional +expiry fixtures explicit. UI tests, server/UI typechecks, token gates and catalog +checks passed. The production task-thread repair state was inspected in local +Storybook. Providers are simulated by the local HTTP fixture; no staging rollout +or real Notebook publication is claimed. Verification planning returned exit 2 +and requires broad CI coverage. PR and review checks are next. diff --git a/packages/shared/src/types/issue.ts b/packages/shared/src/types/issue.ts index d053454552a..4fbb6a974dc 100644 --- a/packages/shared/src/types/issue.ts +++ b/packages/shared/src/types/issue.ts @@ -1501,7 +1501,8 @@ export interface RequestConfirmationToolActionPayload { previewMarkdown: string; argumentsSummaryJson: string; argumentsHash: string; - expiresAt: string; + /** Null for ordinary human reviews; explicit action deadlines still apply. */ + expiresAt: string | null; } export interface RequestConfirmationSecretProposalPayload { diff --git a/packages/shared/src/validators/issue.ts b/packages/shared/src/validators/issue.ts index 3fbdc7cac53..54db17c6fc1 100644 --- a/packages/shared/src/validators/issue.ts +++ b/packages/shared/src/validators/issue.ts @@ -1683,7 +1683,7 @@ export const requestConfirmationToolActionPayloadSchema = z.object({ previewMarkdown: z.string().trim().min(1).max(20000), argumentsSummaryJson: z.string().max(20000), argumentsHash: z.string().trim().min(1).max(255), - expiresAt: z.string().datetime({ offset: true }), + expiresAt: z.string().datetime({ offset: true }).nullable(), }); export const requestConfirmationSecretProposalPayloadSchema = z.object({ diff --git a/server/src/__tests__/tool-access-oauth-protocol.integration.test.ts b/server/src/__tests__/tool-access-oauth-protocol.integration.test.ts index daa783228e6..484d3dad919 100644 --- a/server/src/__tests__/tool-access-oauth-protocol.integration.test.ts +++ b/server/src/__tests__/tool-access-oauth-protocol.integration.test.ts @@ -7,10 +7,17 @@ import { join } from "node:path"; import express from "express"; import request from "supertest"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; -import { companies, companyMemberships, createDb, connectionGrants, toolCatalogEntries, toolConnections, toolOauthStates } from "@paperclipai/db"; -import { eq } from "drizzle-orm"; +import { agents, approvals, companies, companyMemberships, createDb, connectionGrants, heartbeatRuns, issues, toolActionRequests, toolCatalogEntries, toolConnections, toolOauthStates, toolPolicies } from "@paperclipai/db"; +import { and, eq } from "drizzle-orm"; import { toolAccessService } from "../services/tool-access.js"; import { toolAccessRoutes } from "../routes/tool-access.js"; +import { createToolGatewayService } from "../services/tool-gateway.js"; +import { initializeRunIdentity } from "../services/run-identity.js"; +import { secretService } from "../services/secrets.js"; +import { createDeliveryWorkCoordinator } from "../services/delivery-work-coordinator.js"; +import { registerToolReviewRecoveryWork } from "../services/tool-review-recovery-work.js"; +import { writeToolConnectionHealthChange } from "../services/tool-connection-health-notifications.js"; +import { readSignedToolArgumentsPayload } from "../services/tool-content-guards.js"; import { errorHandler } from "../middleware/index.js"; import { startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; @@ -35,7 +42,12 @@ describe("tool OAuth over local HTTP", () => { if (keyDir) await rm(keyDir, { recursive: true, force: true }); }); - it.each(["none", "client_secret_basic"] as const)("registers, exchanges, and activates a %s client through the routes", async (authMethod) => { + it.each([ + { authMethod: "none", grantKind: "organization", scopeMode: "omitted_initial" }, + { authMethod: "client_secret_basic", grantKind: "organization", scopeMode: "omitted_refresh" }, + { authMethod: "none", grantKind: "user", scopeMode: "omitted_initial" }, + { authMethod: "client_secret_basic", grantKind: "user", scopeMode: "explicit" }, + ] as const)("approves a $grantKind $authMethod OAuth action after refresh and delayed review ($scopeMode)", async ({ authMethod, grantKind, scopeMode }) => { const userId = `oauth-user-${randomUUID()}`; const [company] = await db.insert(companies).values({ name: "OAuth protocol fixture", issuePrefix: `O${randomUUID().slice(0, 5)}`, @@ -48,6 +60,7 @@ describe("tool OAuth over local HTTP", () => { let origin: string; let currentAccessToken = "fixture-access"; let currentRefreshToken = "fixture-refresh"; + let rejectRefresh = false; const provider = createServer(async (req, res) => { let body = ""; for await (const chunk of req) body += chunk; @@ -66,7 +79,7 @@ describe("tool OAuth over local HTTP", () => { token_endpoint: `${origin}/token`, registration_endpoint: `${origin}/register`, code_challenge_methods_supported: ["S256"], token_endpoint_auth_methods_supported: [authMethod], - scopes_supported: ["tools.read", "offline_access"], + scopes_supported: ["tools.write", "offline_access"], grant_types_supported: ["authorization_code", "refresh_token"], }); } @@ -78,6 +91,7 @@ describe("tool OAuth over local HTTP", () => { if (path === "/token") { const refreshing = new URLSearchParams(body).get("grant_type") === "refresh_token"; if (refreshing) { + if (rejectRefresh) return json({ error: "invalid_grant" }, 400); if (new URLSearchParams(body).get("refresh_token") !== currentRefreshToken) { return json({ error: "invalid_grant" }, 400); } @@ -85,7 +99,9 @@ describe("tool OAuth over local HTTP", () => { currentRefreshToken = "fixture-refresh-rotated"; } return json({ access_token: currentAccessToken, refresh_token: currentRefreshToken, - expires_in: 3600, token_type: "Bearer", scope: "tools.read offline_access" }); + expires_in: 3600, token_type: "Bearer", + ...((scopeMode === "omitted_initial" && !refreshing) || (scopeMode === "omitted_refresh" && refreshing) + ? {} : { scope: "tools.write offline_access" }) }); } if (path === "/mcp") { if (req.headers.authorization !== `Bearer ${currentAccessToken}`) { @@ -97,9 +113,13 @@ describe("tool OAuth over local HTTP", () => { res.writeHead(202).end(); return; } + if (rpc.method === "tools/call") { + return json({ jsonrpc: "2.0", id: rpc.id, + result: { content: [{ type: "text", text: "Reviewed action completed" }] } }); + } return json({ jsonrpc: "2.0", id: rpc.id, result: rpc.method === "initialize" ? { protocolVersion: "2025-06-18", capabilities: { tools: {} }, serverInfo: { name: "fixture", version: "1" } } - : { tools: [{ name: "read_fixture", description: "Read fixture state", inputSchema: { type: "object", properties: {} }, annotations: { readOnlyHint: true } }] }, + : { tools: [{ name: "create_fixture", description: "Create fixture page", inputSchema: { type: "object", properties: {} }, annotations: { readOnlyHint: false } }] }, }); } json({}, 404); @@ -117,7 +137,7 @@ describe("tool OAuth over local HTTP", () => { app.use(errorHandler); const connected = await request(app).post(`/api/companies/${company.id}/tools/apps/connect`) - .send({ link: `${origin}/mcp`, name: "Local OAuth" }).expect(201); + .send({ link: `${origin}/mcp`, name: "Local OAuth", grantKind }).expect(201); const authorization = new URL(connected.body.auth.startUrl); expect(authorization.origin).toBe(origin); expect(authorization.searchParams.get("client_id")).toBe("fixture-client"); @@ -129,7 +149,11 @@ describe("tool OAuth over local HTTP", () => { const completed = await request(app).get("/api/tools/oauth/callback") .query({ state, code: "fixture-code", iss: origin }).expect(200); - expect(completed.body.catalog).toEqual([expect.objectContaining({ toolName: "read_fixture", riskLevel: "read" })]); + expect(completed.body.catalog).toEqual([expect.objectContaining({ toolName: "create_fixture", riskLevel: "write" })]); + if (grantKind === "user") { + await request(app).post(`/api/companies/${company.id}/tools/apps/${connected.body.connectionId}/finalize-oauth-access`) + .send({ grantKind }).expect(200); + } await request(app).post(`/api/companies/${company.id}/tools/apps/${connected.body.connectionId}/finish`) .send({ enabledCatalogEntryIds: completed.body.catalog.map((entry: { id: string }) => entry.id), askFirstCatalogEntryIds: [], access: "all_agents" }).expect(200); @@ -137,9 +161,10 @@ describe("tool OAuth over local HTTP", () => { expect(connection).toMatchObject({ companyId: company.id, status: "active", enabled: true, ownership: "dcr" }); expect(connection.config.oauth).toMatchObject({ clientId: "fixture-client", clientTokenEndpointAuthMethod: authMethod, clientIssuer: origin }); expect(JSON.stringify(connection.config)).not.toMatch(/fixture-(access|refresh|secret)/); - expect(connection.credentialSecretRefs.map((ref) => ref.configPath)).toEqual(expect.arrayContaining(["oauth.access_token", "oauth.refresh_token"])); + if (grantKind === "organization") expect(connection.credentialSecretRefs.map((ref) => ref.configPath)) + .toEqual(expect.arrayContaining(["oauth.access_token", "oauth.refresh_token"])); expect(await db.select().from(toolCatalogEntries).where(eq(toolCatalogEntries.connectionId, connection.id))) - .toEqual([expect.objectContaining({ toolName: "read_fixture", status: "active", reviewedByUserId: userId })]); + .toEqual([expect.objectContaining({ toolName: "create_fixture", status: "active", reviewedByUserId: userId })]); const tokenCall = calls.find((call) => call.path === "/token")!; const tokenBody = new URLSearchParams(tokenCall.body); expect(tokenBody.get("code")).toBe("fixture-code"); @@ -151,17 +176,47 @@ describe("tool OAuth over local HTTP", () => { expect(calls.some((call) => call.path === "/mcp" && call.authorization === "Bearer fixture-access")).toBe(true); expect(calls.filter((call) => call.path === "/register")).toHaveLength(1); - const [grant] = await db.select().from(connectionGrants).where(eq(connectionGrants.connectionId, connection.id)); + const [grant] = await db.select().from(connectionGrants).where(and( + eq(connectionGrants.connectionId, connection.id), eq(connectionGrants.kind, grantKind))); const consentedAt = grant.providerTenant?.oauth?.consentedAt; expect(consentedAt).toEqual(expect.any(String)); expect(Number.isFinite(Date.parse(consentedAt!))).toBe(true); + const [agent] = await db.insert(agents).values({ companyId: company.id, name: "OAuth approval agent", + role: "engineer", adapterType: "process" }).returning(); + const [issue] = await db.insert(issues).values({ companyId: company.id, title: "Delayed OAuth approval", + status: "in_progress", assigneeAgentId: agent.id }).returning(); + const [run] = await db.insert(heartbeatRuns).values({ companyId: company.id, agentId: agent.id, + invocationSource: "assignment", status: "running", contextSnapshot: { issueId: issue.id } }).returning(); + await initializeRunIdentity(db, { companyId: company.id, runId: run.id, issueId: issue.id, + responsibleUserId: userId, cause: "instruction" }); + await db.insert(toolPolicies).values({ companyId: company.id, name: "Review OAuth actions", + policyType: "require_approval", selectors: { connectionId: connection.id } }); + const options = { deploymentMode: "local_trusted" as const, deploymentExposure: "private" as const, paperclipCloudConnector: null }; + const first = toolAccessService(db, options); + const second = toolAccessService(db, options); + const signingSecret = "oauth-protocol-approval-signing-secret"; + const gatewayOptions = { toolActionSigningSecret: signingSecret, + oauthGrantRefresher: first.refreshOAuthGrantCredentials }; + const gateway = createToolGatewayService(db, gatewayOptions); + const session = await gateway.createSession({ companyId: company.id, agentId: agent.id, runId: run.id }); + const tool = (await gateway.listToolsForSession(session.token)).find(entry => entry.connectionId === connection.id)!; + expect(tool).toBeDefined(); + const reviewActions = ["after refresh", "tomorrow", "connection repair", "changed authorization", "withdrawn formal approval", "expired authorization"]; + for (const action of reviewActions) { + await expect(gateway.executeTool({ sessionToken: session.token, tool: tool.name, + parameters: { action, sharingPolicy: { mode: "domain", domain: "paperclip.ing" } } })).rejects.toMatchObject({ reasonCode: "approval_required" }); + } + const savedActions = await db.select().from(toolActionRequests).where(eq(toolActionRequests.issueId, issue.id)); + expect(savedActions).toHaveLength(reviewActions.length); + const pendingActions = reviewActions.map(action => savedActions.find(saved => { + const payload = readSignedToolArgumentsPayload({ signedArguments: saved.signedArguments, + invocationId: saved.invocationId, toolName: tool.name, signingSecret }); + return (payload?.arguments as { action: string })?.action === action; + })!); await db.update(connectionGrants).set({ providerTenant: { ...grant.providerTenant, oauth: { ...grant.providerTenant?.oauth, accessTokenExpiresAt: new Date(0).toISOString() }, } }).where(eq(connectionGrants.id, grant.id)); - const options = { deploymentMode: "local_trusted" as const, deploymentExposure: "private" as const, paperclipCloudConnector: null }; - const first = toolAccessService(db, options); - const second = toolAccessService(db, options); const refreshInput = { companyId: company.id, connectionId: connection.id, grantId: grant.id, actor: { actorType: "user" as const, actorId: userId } }; const refreshed = await Promise.all([ first.refreshOAuthGrantCredentials(refreshInput), @@ -191,6 +246,104 @@ describe("tool OAuth over local HTTP", () => { expect(allExchanges).toHaveLength(3); expect(new URLSearchParams(allExchanges[2].body).get("refresh_token")).toBe("fixture-refresh-rotated"); expect(calls.filter(call => call.path === "/register")).toHaveLength(1); + // Rebuild the gateway after the requesting run ends. The durable request, + // rather than the original session or its old tokens, owns execution. + await db.update(heartbeatRuns).set({ status: "succeeded" }).where(eq(heartbeatRuns.id, run.id)); + const restarted = createToolGatewayService(db, gatewayOptions); + const approve = (id: string) => restarted.approveActionRequest({ companyId: company.id, + actionRequestId: id, actor: { userId } }); + const [currentGrant] = await db.select().from(connectionGrants).where(eq(connectionGrants.id, grant.id)); + const catalog = completed.body.catalog[0]; + for (const change of ["tool", "destination", "scopes", "consent"]) { + try { + if (change === "tool") await db.update(toolCatalogEntries).set({ versionHash: "new tool definition" }) + .where(eq(toolCatalogEntries.id, catalog.id)); + else if (change === "destination") await db.update(toolConnections).set({ + config: { ...connection.config, url: `${origin}/different-target` }, + }).where(eq(toolConnections.id, connection.id)); + else await db.update(connectionGrants).set({ providerTenant: { ...currentGrant.providerTenant, + oauth: { ...currentGrant.providerTenant?.oauth, + ...(change === "scopes" ? { scopes: ["tools.admin"] } : { consentedAt: "2026-11-10T00:00:00Z" }) }, + } }).where(eq(connectionGrants.id, grant.id)); + await expect(approve(pendingActions[3].id)).rejects.toMatchObject({ reasonCode: "approved_tool_target_changed" }); + expect(calls.filter(call => call.path === "/mcp" && JSON.parse(call.body).method === "tools/call")).toHaveLength(0); + } finally { + await db.update(toolCatalogEntries).set({ versionHash: catalog.versionHash }).where(eq(toolCatalogEntries.id, catalog.id)); + await db.update(toolConnections).set({ config: connection.config }).where(eq(toolConnections.id, connection.id)); + await db.update(connectionGrants).set({ providerTenant: currentGrant.providerTenant }).where(eq(connectionGrants.id, grant.id)); + } + } + await expect(approve(pendingActions[0].id)).resolves.toMatchObject({ status: "executed" }); + await expect(approve(pendingActions[0].id)).resolves.toMatchObject({ status: "executed" }); + vi.useFakeTimers({ toFake: ["Date"] }); + try { + vi.setSystemTime(Date.now() + 25 * 60 * 60_000); + await expect(approve(pendingActions[1].id)).resolves.toMatchObject({ status: "executed" }); + } finally { vi.useRealTimers(); } + await db.update(connectionGrants).set({ status: "needs_reauthorization", updatedAt: new Date() }) + .where(eq(connectionGrants.id, grant.id)); + await expect(approve(pendingActions[2].id)).resolves.toMatchObject({ status: "approved" }); + const [formal] = await db.insert(approvals).values({ companyId: company.id, + type: "request_board_approval", status: "approved", payload: {} }).returning(); + await db.update(toolActionRequests).set({ approvalId: formal.id }).where(eq(toolActionRequests.id, pendingActions[4].id)); + await expect(approve(pendingActions[4].id)).resolves.toMatchObject({ status: "approved" }); + await db.update(approvals).set({ status: "rejected" }).where(eq(approvals.id, formal.id)); + await restarted.recoverApprovedActionReviews(company.id); + expect((await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, pendingActions[4].id)))[0].status) + .toBe("cancelled"); + const coordinator = createDeliveryWorkCoordinator({ owner: db, canRun: () => true, + canReconcile: () => true, onError: error => { throw error; } }); + try { + await registerToolReviewRecoveryWork(coordinator, restarted, () => true).ready; + await vi.waitFor(() => expect(coordinator.nextWakeAt()).toBeNull(), { timeout: 5000 }); + await expect(restarted.recoverApprovedActionReviews(company.id)).resolves.toMatchObject({ retry: false }); + // The supported connection writer wakes recovery after commit. No + // second approval or agent run is needed for the same authorization. + await writeToolConnectionHealthChange(db, { companyId: company.id, id: connection.id }, async tx => { + await tx.update(connectionGrants).set({ status: "active", updatedAt: new Date(Date.now() + 1) }) + .where(eq(connectionGrants.id, grant.id)); + }); + await vi.waitFor(async () => { + expect((await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, pendingActions[2].id)))[0].status) + .toBe("executed"); + }, { timeout: 5000, interval: 25 }); + } finally { await coordinator.stop(); } + // A production refresh failure is a pre-dispatch sign-in wait, even + // when the grant looked active when the person clicked Approve. + const [beforeReauthConnection] = await db.select().from(toolConnections).where(eq(toolConnections.id, connection.id)); + const [beforeReauthGrant] = await db.select().from(connectionGrants).where(eq(connectionGrants.id, grant.id)); + rejectRefresh = true; + await db.update(connectionGrants).set({ providerTenant: { ...beforeReauthGrant.providerTenant, + oauth: { ...beforeReauthGrant.providerTenant?.oauth, accessTokenExpiresAt: new Date(0).toISOString() }, + } }).where(eq(connectionGrants.id, grant.id)); + await expect(approve(pendingActions[5].id)).resolves.toMatchObject({ status: "approved" }); + await restarted.recoverApprovedActionReviews(company.id); + expect((await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, pendingActions[5].id)))[0].status) + .toBe("approved"); + const deadline = new Date(Date.now() - 1); + await db.update(toolActionRequests).set({ expiresAt: deadline }).where(eq(toolActionRequests.id, pendingActions[5].id)); + expect(await restarted.nextActionReviewExpiryAt()).toBe(deadline.getTime()); + await restarted.sweepExpiredActionReviews(); + expect((await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, pendingActions[5].id)))[0].status) + .toBe("expired"); + rejectRefresh = false; + // Restore this fixture's original authorization metadata to exercise + // manual replacement separately. The recovery worker is already stopped. + await db.update(toolConnections).set({ status: beforeReauthConnection.status, + enabled: beforeReauthConnection.enabled, credentialRefs: beforeReauthConnection.credentialRefs, + credentialSecretRefs: beforeReauthConnection.credentialSecretRefs, + }).where(eq(toolConnections.id, connection.id)); + await db.update(connectionGrants).set({ status: "active", credentialSecretRefs: beforeReauthGrant.credentialSecretRefs, + providerTenant: beforeReauthGrant.providerTenant }).where(eq(connectionGrants.id, grant.id)); + const accessRef = currentGrant.credentialSecretRefs.find(ref => ref.configPath === "oauth.access_token")!; + await secretService(db).rotate(accessRef.secretId, { value: "manually-replaced-token" }, { userId }); + await expect(approve(pendingActions[3].id)).rejects.toMatchObject({ reasonCode: "approved_tool_target_changed" }); + const executions = calls.filter(call => call.path === "/mcp" && JSON.parse(call.body).method === "tools/call"); + expect(executions).toHaveLength(3); + expect(executions.map(call => JSON.parse(call.body).params.arguments.action).sort()) + .toEqual(["after refresh", "connection repair", "tomorrow"]); + for (const execution of executions) expect(JSON.parse(execution.body).params.arguments.sharingPolicy) + .toEqual({ mode: "domain", domain: "paperclip.ing" }); } finally { provider.closeAllConnections(); await new Promise((resolve, reject) => provider.close((error) => error ? reject(error) : resolve())); diff --git a/server/src/__tests__/tool-gateway-service.test.ts b/server/src/__tests__/tool-gateway-service.test.ts index 32168c3a800..2772b3fcc3c 100644 --- a/server/src/__tests__/tool-gateway-service.test.ts +++ b/server/src/__tests__/tool-gateway-service.test.ts @@ -317,7 +317,11 @@ describeEmbeddedPostgres("tool gateway service", () => { const rows = (await source.select().from(toolActionRequests).where(eq(toolActionRequests.companyId, context.company.id))) .filter(row => !before.has(row.id)); expect(rows).toHaveLength(1); - return rows[0]; + // This worker group exercises explicit deadlines. Ordinary human tool + // reviews have no implicit expiry; give this fixture its intended hour. + const [pending] = await writeToolReviewChange(source, context.company.id, tx => tx.update(toolActionRequests) + .set({ expiresAt: new Date(Date.now() + 60 * 60_000) }).where(eq(toolActionRequests.id, rows[0].id)).returning()); + return pending; } async function expectExpired(id: string, invocationId: string) { expect((await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, id)))[0].status).toBe("expired"); diff --git a/server/src/__tests__/tool-gateway.test.ts b/server/src/__tests__/tool-gateway.test.ts index 03c7369bf93..c16fd503be6 100644 --- a/server/src/__tests__/tool-gateway.test.ts +++ b/server/src/__tests__/tool-gateway.test.ts @@ -3919,7 +3919,7 @@ rl.on("line", (line) => { previewMarkdown: approvalRequest.previewMarkdown, argumentsSummaryJson: expect.stringContaining('"value":"original"'), argumentsHash: approvalRequest.canonicalArgumentsHash, - expiresAt: approvalRequest.expiresAt!.toISOString(), + expiresAt: null, }, }, }); @@ -4404,7 +4404,7 @@ rl.on("line", (line) => { expect(freshRequest).toBeTruthy(); expect(freshRequest!.status).toBe("pending"); expect(freshRequest!.signedArguments).not.toBeNull(); - expect(freshRequest!.expiresAt).not.toBeNull(); + expect(freshRequest!.expiresAt).toBeNull(); // The tool never executed while the approval stayed pending. expect(fake.requests).toHaveLength(0); diff --git a/server/src/services/connection-credentials.ts b/server/src/services/connection-credentials.ts index 4af4f633883..18f73391972 100644 --- a/server/src/services/connection-credentials.ts +++ b/server/src/services/connection-credentials.ts @@ -48,6 +48,8 @@ export async function writeConnectionCredential( /** OAuth shares one definition per connection/path, with one value per owner. */ definitionKey?: string; actor?: SecretActor; + /** Only the OAuth refresh owner may retain the existing credential identity. */ + preserveOAuthIdentity?: boolean; }, ) { const vault = secretService(db); @@ -63,8 +65,9 @@ export async function writeConnectionCredential( ? existing.scope === "user" && Boolean(existing.userSecretDefinitionId) : existing.scope === "company")) { const secret = input.ownerUserId - ? await vault.rotateCurrentUserSecretValue(input.companyId, input.ownerUserId, existing.id, { value: input.value }, input.actor) - : await vault.rotate(existing.id, { value: input.value }, input.actor); + ? await vault.rotateCurrentUserSecretValue(input.companyId, input.ownerUserId, existing.id, + { value: input.value, preserveAiSessionEpoch: input.preserveOAuthIdentity }, input.actor) + : await vault.rotate(existing.id, { value: input.value, preserveAiSessionEpoch: input.preserveOAuthIdentity }, input.actor); return { secret, created: false, definitionId: null }; } // Re-entering a credential is fresh consent. Replace a legacy company diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index deb48d52673..ddde824a7f9 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -1888,12 +1888,12 @@ export function secretService(db: Db | DbTransaction) { }).sort((left, right) => left.key.localeCompare(right.key)); } - async function resolveSecretVersion( + async function resolveSecretVersionIdentity( companyId: string, secretId: string, version: number | "latest", context?: SecretBindingContext, - ): Promise { + ): Promise<{ version: number; identityEpoch: number }> { const secret = await getById(secretId); if (!secret) throw notFound("Secret not found"); if (secret.companyId !== companyId) throw unprocessable("Secret must belong to same company"); @@ -1910,7 +1910,13 @@ export function secretService(db: Db | DbTransaction) { if (versionRow.status === "disabled" || versionRow.status === "destroyed" || versionRow.revokedAt) { throw unprocessable("Secret version is not active", { code: "version_inactive" }); } - return resolvedVersion; + return { version: resolvedVersion, identityEpoch: secret.aiSessionEpoch }; + } + + async function resolveSecretVersion( + companyId: string, secretId: string, version: number | "latest", context?: SecretBindingContext, + ): Promise { + return (await resolveSecretVersionIdentity(companyId, secretId, version, context)).version; } async function normalizeEnvConfig( @@ -4098,6 +4104,8 @@ export function secretService(db: Db | DbTransaction) { externalRef?: string | null; providerVersionRef?: string | null; providerConfigId?: string | null; + /** Internal authenticated refresh only; HTTP schemas cannot set this. */ + preserveAiSessionEpoch?: boolean; }, actor?: { userId?: string | null; agentId?: string | null }, ) => { @@ -4645,6 +4653,7 @@ export function secretService(db: Db | DbTransaction) { getByKey, resolveSecretValue, resolveSecretVersion, + resolveSecretVersionIdentity, resolveSecretValueForAgentAccess, listAgentSecretAccess, resolveSecretValueForEphemeralAccess, diff --git a/server/src/services/tool-access/oauth/credential-store.ts b/server/src/services/tool-access/oauth/credential-store.ts index a4d59570235..872abd2f364 100644 --- a/server/src/services/tool-access/oauth/credential-store.ts +++ b/server/src/services/tool-access/oauth/credential-store.ts @@ -142,6 +142,7 @@ async function syncCredentialBindings( actor?: ActorInfo; existingRefs?: typeof connectionGrants.$inferSelect.credentialSecretRefs; ownerUserId?: string; + preserveOAuthIdentity?: boolean; }, context?: { dbClient: Db | DbTransaction; @@ -154,6 +155,7 @@ async function syncCredentialBindings( const write = (client: Db | DbTransaction) => writeConnectionCredential(client, { companyId: input.companyId, connectionName: input.connection.name, configPath: input.configPath, label: input.label, value: input.value, existingRef: existing ?? undefined, actor: actorForSecret(input.actor), + preserveOAuthIdentity: input.preserveOAuthIdentity, // Registration authenticates the OAuth client, never the person invoking tools. ownerUserId: input.configPath === "oauth.client_secret" ? undefined : input.ownerUserId, definitionKey: input.ownerUserId ? `tool_oauth.${input.connection.id}.${input.configPath.replace(/[^a-z0-9_:-]+/gi, "_")}` : undefined, diff --git a/server/src/services/tool-access/oauth/grant-refresh.ts b/server/src/services/tool-access/oauth/grant-refresh.ts index 096b5d4f9c9..87b2dfc5618 100644 --- a/server/src/services/tool-access/oauth/grant-refresh.ts +++ b/server/src/services/tool-access/oauth/grant-refresh.ts @@ -202,13 +202,13 @@ export function createOAuthGrantRefresh(db: Db, { now, store, leases, currentClo const txSecrets = secretService(tx); await txSecrets.rotate( accessRef.secretId, - { value: credentials.accessToken }, + { value: credentials.accessToken, preserveAiSessionEpoch: true }, actorForSecret(credentialActor), ); if (credentials.refreshToken) { await txSecrets.rotate( refreshRef.secretId, - { value: credentials.refreshToken }, + { value: credentials.refreshToken, preserveAiSessionEpoch: true }, actorForSecret(credentialActor), ); } @@ -457,6 +457,7 @@ export function createOAuthGrantRefresh(db: Db, { now, store, leases, currentClo configPath: "oauth.refresh_token", label: "OAuth refresh token", value: token.refreshToken, + preserveOAuthIdentity: true, actor: credentialActor, existingRefs: grant.credentialSecretRefs, ownerUserId: @@ -471,6 +472,7 @@ export function createOAuthGrantRefresh(db: Db, { now, store, leases, currentClo configPath: "oauth.access_token", label: "OAuth access token", value: token.accessToken, + preserveOAuthIdentity: true, actor: credentialActor, existingRefs: grant.credentialSecretRefs, ownerUserId: diff --git a/server/src/services/tool-approval-connection.ts b/server/src/services/tool-approval-connection.ts new file mode 100644 index 00000000000..f3df9d7d720 --- /dev/null +++ b/server/src/services/tool-approval-connection.ts @@ -0,0 +1,40 @@ +import { and, eq, sql } from "drizzle-orm"; +import { connectionGrants, toolConnections, type Db } from "@paperclipai/db"; + +export const TOOL_APPROVAL_CONNECTION_REQUIRED = "approved_action_connection_required"; +export const TOOL_APPROVAL_CONNECTION_MESSAGE = "Your approval is saved. Reconnect the app to continue. Paperclip will check the account and action again before it runs."; + +export function isToolApprovalConnectionError(error: unknown): boolean { + if (!error || typeof error !== "object") return false; + const typed = error as { reasonCode?: string; details?: { code?: string } }; + return ["connector_reauthorization_required", "oauth_reauthorization_required", "oauth_refresh_missing", + "user_authorization_required", "organization_authorization_required", "agent_authorization_required", + "vercel_connect_authorization_required"] + .includes(typed.reasonCode ?? typed.details?.code ?? ""); +} + +/** Read metadata only. A waiting approval retries on a committed repair, not a timer. */ +export async function toolApprovalConnectionRevision(db: Db, companyId: string, + connectionId: string | null, snapshot: unknown): Promise { + if (!connectionId) return null; + const grantId = snapshot && typeof snapshot === "object" && "credentialGrantId" in snapshot && + typeof snapshot.credentialGrantId === "string" && /^[0-9a-f]{8}-(?:[0-9a-f]{4}-){3}[0-9a-f]{12}$/i.test(snapshot.credentialGrantId) + ? snapshot.credentialGrantId : null; + const [state] = await db.select({ connectionStatus: toolConnections.status, enabled: toolConnections.enabled, + connectionUpdatedAt: toolConnections.updatedAt, grantStatus: connectionGrants.status, + grantUpdatedAt: connectionGrants.updatedAt }).from(toolConnections) + .leftJoin(connectionGrants, and(eq(connectionGrants.connectionId, toolConnections.id), + eq(connectionGrants.companyId, companyId), grantId ? eq(connectionGrants.id, grantId) : sql`false`)) + .where(and(eq(toolConnections.id, connectionId), eq(toolConnections.companyId, companyId))).limit(1); + return JSON.stringify(state ?? null); +} + +export async function toolApprovalConnectionCanResume(db: Db, companyId: string, + connectionId: string | null, snapshot: unknown, previousRevision: unknown) { + const revision = await toolApprovalConnectionRevision(db, companyId, connectionId, snapshot); + if (revision === previousRevision) return false; + const state = revision === null ? null : JSON.parse(revision) as { connectionStatus: string; enabled: boolean } | null; + // Missing connections reach normal target rejection. Draft or disabled + // connections still need repair; do not convert that wait to tool_not_found. + return !state || (state.connectionStatus === "active" && state.enabled); +} diff --git a/server/src/services/tool-approval-execution.ts b/server/src/services/tool-approval-execution.ts index f5f4fb78abf..74497ba7981 100644 --- a/server/src/services/tool-approval-execution.ts +++ b/server/src/services/tool-approval-execution.ts @@ -2,8 +2,9 @@ import { and, eq } from "drizzle-orm"; import { toolActionRequests, toolInvocations, type Db } from "@paperclipai/db"; import type { ToolRedactedValueSummary } from "@paperclipai/shared"; import { notifyToolReviewChange } from "./tool-review-work-notifications.js"; +import { TOOL_APPROVAL_CONNECTION_MESSAGE, TOOL_APPROVAL_CONNECTION_REQUIRED } from "./tool-approval-connection.js"; -export type ToolApprovalClaim = { actionRequestId: string; invocationId: string; claimedAt: Date }; +export type ToolApprovalClaim = { actionRequestId: string; invocationId: string; claimedAt: Date; enforceDeadline?: boolean }; export type ToolTestOutcome = { status: "succeeded" | "failed" | "timed_out" | "rate_limited"; result?: ToolRedactedValueSummary; @@ -37,7 +38,7 @@ export async function claimToolTestApproval(db: Db, actionRequestId: string, inv } /** Return only this still-undispatched claim to its existing approval queue. */ -export async function releaseToolApproval(db: Db, claim: ToolApprovalClaim) { +export async function releaseToolApproval(db: Db, claim: ToolApprovalClaim, connectionRevision?: string) { return db.transaction(async tx => { const [invocation] = await tx.select().from(toolInvocations) .where(eq(toolInvocations.id, claim.invocationId)).for("update"); @@ -48,6 +49,12 @@ export async function releaseToolApproval(db: Db, claim: ToolApprovalClaim) { request?.status !== "executing" || request.companyId !== invocation.companyId || request.updatedAt.getTime() !== claim.claimedAt.getTime()) return false; await notifyToolReviewChange(tx, request.companyId); + if (connectionRevision !== undefined) { + await tx.update(toolInvocations).set({ errorCode: TOOL_APPROVAL_CONNECTION_REQUIRED, + errorMessage: TOOL_APPROVAL_CONNECTION_MESSAGE, + policyExplanation: { ...invocation.policyExplanation, approvalConnectionWait: connectionRevision }, + updatedAt: new Date() }).where(eq(toolInvocations.id, invocation.id)); + } await tx.update(toolActionRequests).set({ status: "approved", updatedAt: new Date() }) .where(eq(toolActionRequests.id, request.id)); return true; @@ -87,7 +94,17 @@ export async function startToolInvocation(db: Db, invocationId: string, claim?: if (invocationId !== claim.invocationId || invocation?.status !== "awaiting_approval" || invocation.startedAt !== null || request?.status !== "executing" || request.invocationId !== invocationId || request.companyId !== invocation.companyId || request.updatedAt.getTime() !== claim.claimedAt.getTime()) return false; - await tx.update(toolInvocations).set({ status: "executing", approvalState: "approved", startedAt: now, updatedAt: now }) + if ((claim.enforceDeadline || invocation.errorCode === TOOL_APPROVAL_CONNECTION_REQUIRED) && request.expiresAt && request.expiresAt <= now) { + await notifyToolReviewChange(tx, request.companyId); + await tx.update(toolInvocations).set({ status: "failed", errorCode: "action_expired", + errorMessage: "The action deadline passed before execution.", completedAt: now, updatedAt: now }) + .where(eq(toolInvocations.id, invocationId)); + await tx.update(toolActionRequests).set({ status: "expired", resolvedAt: now, updatedAt: now }) + .where(eq(toolActionRequests.id, request.id)); + return false; + } + await tx.update(toolInvocations).set({ status: "executing", approvalState: "approved", startedAt: now, updatedAt: now, + errorCode: null, errorMessage: null }) .where(eq(toolInvocations.id, invocationId)); return true; }); diff --git a/server/src/services/tool-approval-target.test.ts b/server/src/services/tool-approval-target.test.ts new file mode 100644 index 00000000000..58718f35a50 --- /dev/null +++ b/server/src/services/tool-approval-target.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, it } from "vitest"; +import { approvalSnapshotsMatch, approvalTargetChangeMessage, oauthApprovalConfig, oauthGrantApprovalIdentity, + approvalGrantTenantIdentity, externalCredentialApprovalIdentity } from "./tool-approval-target.js"; + +function snapshot() { + return { approvalSnapshotVersion: 2, connectionConfigHash: "old expiry", connectionTransportConfigHash: "old expiry", + connectionIdentityHash: "oauth-vault-owner", connectionAuthorizationConfigHash: "same endpoint and client", + connectionAuthorizationTransportConfigHash: "same transport", credentialGrantIdentityHash: "same consent and scopes", + credentialGrantId: "owner-grant", catalogSchemaHash: "reviewed-schema", catalogVersionHash: "reviewed-tool", + credentialSecretVersions: [{ refHash: "access-token", versionSelector: "latest", resolvedVersion: 1480, identityEpoch: 1479 }], + headerCredentialVersions: [] }; +} + +describe("durable tool approval target", () => { + it.each([ + ["credentialGrantIdentityHash", "account, credentials, or permissions"], + ["catalogVersionHash", "tool definition"], + ["connectionAuthorizationConfigHash", "connection or destination"], + ] as const)("explains a changed %s without telling the person to retry", (field, reason) => { + const saved = snapshot(), live = { ...snapshot(), [field]: "changed" }; + expect(approvalTargetChangeMessage(saved, live)).toContain(reason); + expect(approvalTargetChangeMessage(saved, live)).not.toMatch(/try again/i); + }); + it("accepts refresh versions under the same authenticated credential identity", () => { + const reviewed = snapshot(), live = snapshot(); + live.connectionConfigHash = live.connectionTransportConfigHash = "new expiry"; + live.credentialSecretVersions[0].resolvedVersion = 1483; + expect(approvalSnapshotsMatch(reviewed, live)).toBe(true); + }); + + it.each(["connectionIdentityHash", "connectionAuthorizationConfigHash", "connectionAuthorizationTransportConfigHash", + "credentialGrantIdentityHash", "credentialGrantId", "catalogSchemaHash", "catalogVersionHash"] as const)( + "refuses a changed %s even after routine refresh", field => { + const reviewed = snapshot(), live = snapshot(); + live[field] = "changed"; + expect(approvalSnapshotsMatch(reviewed, live)).toBe(false); + }); + + it("refuses manual OAuth credential replacement", () => { + const reviewed = snapshot(), live = snapshot(); + live.credentialSecretVersions[0].identityEpoch++; + live.credentialSecretVersions[0].resolvedVersion++; + expect(approvalSnapshotsMatch(reviewed, live)).toBe(false); + }); + + it("keeps fixed versions and non-OAuth credentials bound to their exact versions", () => { + for (const versionSelector of ["1", "latest"]) { + const reviewed = { ...snapshot(), credentialSecretVersions: [{ refHash: "api-key", versionSelector, resolvedVersion: 1 }] }; + expect(approvalSnapshotsMatch(reviewed, { ...reviewed, + credentialSecretVersions: [{ refHash: "api-key", versionSelector, resolvedVersion: 2 }] })).toBe(false); + } + }); + + it("carries an old signed snapshot forward only while its exact versions and configuration still match", () => { + const current = snapshot(); + const { approvalSnapshotVersion, connectionIdentityHash, connectionAuthorizationConfigHash, + connectionAuthorizationTransportConfigHash, credentialGrantIdentityHash, ...legacy } = current; + const reviewed = { ...legacy, + credentialSecretVersions: [{ refHash: "access-token", versionSelector: "latest", resolvedVersion: 1480 }] }; + expect(approvalSnapshotsMatch(reviewed, current)).toBe(true); + current.credentialSecretVersions[0].resolvedVersion++; + expect(approvalSnapshotsMatch(reviewed, current)).toBe(false); + }); + + it("refuses an unknown saved snapshot version", () => { + expect(approvalSnapshotsMatch({ ...snapshot(), approvalSnapshotVersion: 3 }, snapshot())).toBe(false); + }); + + it("ignores only OAuth lifecycle bookkeeping, retaining audience, issuer, scope and consent", () => { + const config = { url: "https://notebook.example/mcp", oauth: { issuer: "https://id.example", resource: "notebook", + consentedAt: "first consent", scopes: ["write", "read"], expiresAt: "old", refreshedAt: "old" }, + providerMetadata: { oauth: { expiresAt: "old", scope: "read write" } }, methodConfig: { expiresAt: "meaningful setting" } }; + const refreshed = { ...config, oauth: { ...config.oauth, expiresAt: "new", refreshedAt: "new", scopes: ["read", "write"] }, + providerMetadata: { oauth: { ...config.providerMetadata.oauth, expiresAt: "new", scope: "write read" } } }; + expect(oauthApprovalConfig(config)).toEqual(oauthApprovalConfig(refreshed)); + for (const field of ["issuer", "resource", "consentedAt"]) { + expect(oauthApprovalConfig(config)).not.toEqual(oauthApprovalConfig({ ...refreshed, + oauth: { ...refreshed.oauth, [field]: "changed" } })); + } + expect(oauthGrantApprovalIdentity({ scopes: ["read"] }, { tokenType: "Bearer" })) + .toEqual(oauthGrantApprovalIdentity({ scopes: ["read"], strategy: "direct_oauth", tokenType: "Bearer", refreshedAt: "new" }, {})); + }); + + it("compares effective permissions when a refresh starts or stops reporting scope", () => { + const consent = { scopes: ["read", "write"], requestedScopes: ["read", "write"], scopeSource: "requested_fallback" }; + const reported = { ...consent, scopes: ["write", "read", "read"], scopeSource: "provider", scope: "write read", unrequestedScopes: [] }; + expect(oauthGrantApprovalIdentity(consent, {})).toEqual(oauthGrantApprovalIdentity(reported, {})); + expect(oauthGrantApprovalIdentity(consent, {})).not.toEqual(oauthGrantApprovalIdentity({ ...reported, scopes: ["read"] }, {})); + }); + + it("retains external account and token identity while ignoring verification timestamps", () => { + const credential = { provider: "vercel_connect", tokenId: "reviewed-token", teamId: "reviewed-team", expiresAt: 1, lastVerifiedAt: 1 }; + expect(externalCredentialApprovalIdentity(credential)).toEqual(externalCredentialApprovalIdentity({ ...credential, expiresAt: 2, lastVerifiedAt: 2 })); + for (const key of ["tokenId", "teamId"]) { + expect(externalCredentialApprovalIdentity(credential)).not.toEqual(externalCredentialApprovalIdentity({ ...credential, [key]: "changed" })); + } + }); + + it("retains installation identity while ignoring GitHub health and refresh bookkeeping", () => { + const tenant = { github: { installationId: 123, accountId: 456, lastAccessRefreshAt: "old", bindingSync: { status: "pending" } } }; + expect(approvalGrantTenantIdentity(tenant, {})).toEqual(approvalGrantTenantIdentity({ github: { + ...tenant.github, lastAccessRefreshAt: "new", bindingSync: { status: "complete" }, lastWebhookAt: "new", webhookHealth: "healthy" } }, {})); + expect(approvalGrantTenantIdentity(tenant, {})).not.toEqual(approvalGrantTenantIdentity({ github: { ...tenant.github, installationId: 789 } }, {})); + }); +}); diff --git a/server/src/services/tool-approval-target.ts b/server/src/services/tool-approval-target.ts new file mode 100644 index 00000000000..58cbdc59679 --- /dev/null +++ b/server/src/services/tool-approval-target.ts @@ -0,0 +1,109 @@ +import { canonicalToolArguments } from "./tool-content-guards.js"; + +function record(value: unknown): Record | null { + return value !== null && typeof value === "object" && !Array.isArray(value) + ? value as Record : null; +} + +/** Refresh bookkeeping does not change the OAuth authorization being reviewed. */ +export function oauthApprovalIdentity(value: unknown): Record { + const { expiresAt, accessTokenExpiresAt, refreshTokenExpiresAt, refreshedAt, refreshLease, + scope, scopeSource, unrequestedScopes, + ...identity } = record(value) ?? {}; + for (const key of ["scopes", "requestedScopes"]) { + if (Array.isArray(identity[key])) identity[key] = [...new Set(identity[key])].sort(); + } + return identity; +} + +export function oauthApprovalConfig(value: Record): Record { + const result = { ...value }; + if (record(value.oauth)) result.oauth = oauthApprovalIdentity(value.oauth); + const metadata = record(value.providerMetadata); + if (metadata && record(metadata.oauth)) { + result.providerMetadata = { ...metadata, oauth: oauthApprovalIdentity(metadata.oauth) }; + } + return result; +} + +export function oauthGrantApprovalIdentity(value: unknown, connectionOAuth: unknown) { + const grant = oauthApprovalIdentity(value); + const connection = record(connectionOAuth) ?? {}; + const tokenType = grant.tokenType ?? connection.tokenType ?? "Bearer"; + return { ...grant, strategy: grant.strategy ?? connection.strategy ?? "direct_oauth", + tokenType: typeof tokenType === "string" ? tokenType.toLowerCase() : tokenType }; +} + +export function approvalGrantTenantIdentity(value: unknown, connectionOAuth: unknown) { + const tenant: Record = { ...record(value), oauth: oauthGrantApprovalIdentity(record(value)?.oauth, connectionOAuth) }; + const github = record(tenant.github); + if (github) { + const { lastAccessRefreshAt, bindingSync, continuityRetry, lastWebhookAt, webhookHealth, ...identity } = github; + tenant.github = identity; + } + return tenant; +} + +export function externalCredentialApprovalIdentity(value: unknown) { + const fields = record(value); + if (fields?.provider !== "vercel_connect") return value; + const { expiresAt, lastVerifiedAt, ...identity } = fields; + return identity; +} + +function credentials(value: unknown, durable: boolean): unknown { + if (!Array.isArray(value)) return value; + return value.map(entry => { + const fields = record(entry); + if (!fields) return entry; + const { identityEpoch, resolvedVersion, ...rest } = fields; + // Only current, verified OAuth token identities can survive version changes. + // Fixed versions and non-OAuth secrets retain exact version comparisons. + return durable && Number.isSafeInteger(identityEpoch) && Number(identityEpoch) >= 0 + ? { ...rest, identityEpoch } + : { ...rest, resolvedVersion }; + }); +} + +function comparable(snapshot: Record, durable: boolean) { + const { approvalSnapshotVersion, connectionIdentityHash, connectionAuthorizationConfigHash, + connectionAuthorizationTransportConfigHash, credentialGrantIdentityHash, ...legacy } = snapshot; + const result = durable ? { ...snapshot } : legacy; + if (durable) { + delete result.connectionConfigHash; + delete result.connectionTransportConfigHash; + } + for (const key of ["headerCredentialVersions", "credentialSecretVersions"]) { + if (key in result) result[key] = credentials(result[key], durable); + } + return result; +} + +export function approvalSnapshotsMatch(reviewed: unknown, live: Record | null): boolean { + const saved = record(reviewed); + if (!saved || !live) return !saved && !live; + if (saved.approvalSnapshotVersion !== undefined && saved.approvalSnapshotVersion !== 2) return false; + // Old signed requests retain their exact-version contract. Never infer + // continuity for credentials that changed before this contract was recorded. + const durable = saved.approvalSnapshotVersion === 2; + if (durable && live.approvalSnapshotVersion !== 2) return false; + return canonicalToolArguments(comparable(saved, durable)) === canonicalToolArguments(comparable(live, durable)); +} + +export function approvalTargetChangeMessage(reviewed: unknown, live: Record | null): string { + const saved = record(reviewed); + if (saved && live) { + const durable = saved.approvalSnapshotVersion === 2; + const before = comparable(saved, durable), after = comparable(live, durable); + const changed = (keys: string[]) => keys.some(key => canonicalToolArguments(before[key]) !== canonicalToolArguments(after[key])); + if (changed(["credentialGrantId", "credentialGrantIdentityHash", "credentialGrantRefsHash", + "credentialSecretVersions", "headerCredentialVersions"])) + return "The connected account, credentials, or permissions changed. A new review is required."; + if (changed(["catalogEntryId", "catalogVersionHash", "catalogSchemaHash", "upstreamToolName", "riskLevel"])) + return "The tool definition changed. A new review is required."; + if (changed(["connectionId", "connectionIdentityHash", "connectionConfigHash", "connectionTransportConfigHash", + "connectionAuthorizationConfigHash", "connectionAuthorizationTransportConfigHash"])) + return "The app connection or destination changed. A new review is required."; + } + return "The reviewed action is no longer current. A new review is required."; +} diff --git a/server/src/services/tool-connection-health-notifications.ts b/server/src/services/tool-connection-health-notifications.ts index 075983d2972..1b70195e5db 100644 --- a/server/src/services/tool-connection-health-notifications.ts +++ b/server/src/services/tool-connection-health-notifications.ts @@ -15,9 +15,11 @@ export function healthScope(hint: string): HealthScope { throw new Error("Invalid tool connection health scope"); } -export const notifyToolConnectionHealth = (transaction: object, target: HealthChange) => - notifyDeliveryWork(transaction, DELIVERY_QUEUES.toolConnectionHealth, typeof target === "string" ? target +export const notifyToolConnectionHealth = async (transaction: object, target: HealthChange) => { + await notifyDeliveryWork(transaction, DELIVERY_QUEUES.toolReviewRecovery, typeof target === "string" ? target : target.companyId); + await notifyDeliveryWork(transaction, DELIVERY_QUEUES.toolConnectionHealth, typeof target === "string" ? target : "id" in target ? `${target.companyId}:connection:${target.id}` : `${target.companyId}:application:${target.applicationId}`); +}; export function writeToolConnectionHealthChange(db: Db, target: HealthChange, write: (transaction: Db) => Promise): Promise { return db.transaction(async tx => { diff --git a/server/src/services/tool-gateway.ts b/server/src/services/tool-gateway.ts index 1c1cad85772..1a507bb137a 100644 --- a/server/src/services/tool-gateway.ts +++ b/server/src/services/tool-gateway.ts @@ -1,4 +1,8 @@ import { isNewHumanIntentAfterToolRejection } from "./tool-action-replay-intent.js"; +import { approvalSnapshotsMatch, approvalTargetChangeMessage, oauthApprovalConfig, + approvalGrantTenantIdentity, externalCredentialApprovalIdentity } from "./tool-approval-target.js"; +import { isToolApprovalConnectionError, toolApprovalConnectionRevision, toolApprovalConnectionCanResume, + TOOL_APPROVAL_CONNECTION_MESSAGE, TOOL_APPROVAL_CONNECTION_REQUIRED } from "./tool-approval-connection.js"; import { notifyChatRunMilestones } from "./chat-run-milestone-notifications.js"; import { writeToolConnectionHealthChange } from "./tool-connection-health-notifications.js"; import { writeApprovalChange } from "./approval-work-notifications.js"; @@ -964,26 +968,11 @@ function stableHash(value: unknown): string { return createHash("sha256").update(stableSerialize(value)).digest("hex"); } -function normalizeSignedApprovalSnapshot( - value: unknown, -): Record | null { - return asRecord(value); -} - -function approvalSnapshotsMatch( - reviewed: unknown, - live: Record | null, -): boolean { - const reviewedRecord = normalizeSignedApprovalSnapshot(reviewed); - if (!reviewedRecord && !live) return true; - if (!reviewedRecord || !live) return false; - return stableSerialize(reviewedRecord) === stableSerialize(live); -} - type ConnectedCredentialVersionSnapshot = { refHash: string; versionSelector: string; resolvedVersion: number; + identityEpoch?: number; }; const REDACTED_ARGUMENT_SENTINEL = "***REDACTED***"; @@ -2637,7 +2626,9 @@ export function createToolGatewayService( ); } const actionRequest = input.actionRequest; - const expiresAt = new Date(Date.now() + 60 * 60 * 1000); + // Human review has no implicit token/session deadline. Explicit action + // deadlines, task closure and current authorization still gate execution. + const expiresAt = actionRequest.expiresAt; let signedArguments: ReturnType; try { @@ -2798,7 +2789,7 @@ export function createToolGatewayService( previewMarkdown, argumentsSummaryJson: input.argumentsSummary.summary, argumentsHash: canonicalArgumentsHash, - expiresAt: expiresAt.toISOString(), + expiresAt: expiresAt?.toISOString() ?? null, }, }, }, @@ -4525,7 +4516,7 @@ export function createToolGatewayService( ): Promise { const versionSelector = input.versionSelector ?? "latest"; try { - const resolvedVersion = await secrets.resolveSecretVersion( + const identity = await secrets.resolveSecretVersionIdentity( connection.companyId, input.secretId, versionSelector, @@ -4533,7 +4524,10 @@ export function createToolGatewayService( return { refHash: input.refHash, versionSelector: String(versionSelector), - resolvedVersion, + resolvedVersion: identity.version, + ...(connection.authKind === "oauth" && versionSelector === "latest" && + ["oauth.access_token", "oauth.refresh_token"].includes(input.configPath) + ? { identityEpoch: identity.identityEpoch } : {}), }; } catch { await markRemoteConnectionHealth( @@ -5926,6 +5920,18 @@ export function createToolGatewayService( }, ); return { + approvalSnapshotVersion: 2, + connectionIdentityHash: stableHash({ authKind: row.connection.authKind, + credentialSource: row.connection.credentialSource, credentialPolicy: row.connection.credentialPolicy, + externalCredential: row.connection.externalCredential }), + connectionAuthorizationConfigHash: stableHash(row.connection.authKind === "oauth" + ? oauthApprovalConfig(row.connection.config ?? {}) : row.connection.config ?? {}), + connectionAuthorizationTransportConfigHash: stableHash(row.connection.authKind === "oauth" + ? oauthApprovalConfig(row.connection.transportConfig ?? {}) : row.connection.transportConfig ?? {}), + credentialGrantIdentityHash: stableHash({ kind: grant.kind, subjectUserId: grant.subjectUserId, + subjectAgentId: grant.subjectAgentId, externalCredential: externalCredentialApprovalIdentity(grant.externalCredential), + providerTenant: row.connection.authKind === "oauth" + ? approvalGrantTenantIdentity(grant.providerTenant, row.connection.config.oauth) : grant.providerTenant }), applicationId: row.application.id, applicationKey: row.application.applicationKey ?? null, applicationStatus: row.application.status, @@ -5959,6 +5965,21 @@ export function createToolGatewayService( }; } + async function prepareApprovedConnectionAuthorization(session: ToolGatewaySession, tool: ToolGatewayDescriptor, reviewedSnapshot: unknown) { + if (tool.providerType !== "mcp_remote_http" && tool.providerType !== "provider_rest") return; + const { connection } = await resolveConnectedRemoteTool(session, tool); + if (connection.authKind !== "oauth") return; + // Never send a refresh credential to a changed endpoint. Validate the saved + // target before refresh, then let the caller revalidate its resulting grant. + const beforeRefresh = await connectedRemoteApprovalSnapshot(session, tool); + if (!approvalSnapshotsMatch(reviewedSnapshot, beforeRefresh)) + throw new ToolGatewayHttpError(409, approvalTargetChangeMessage(reviewedSnapshot, beforeRefresh), "approved_tool_target_changed"); + const grant = await resolveConnectionGrant(session, connection); + // Discover refresh/sign-in failures before startToolInvocation records a + // provider dispatch. No action is sent during this credential preparation. + await resolveCredentialHeaders(session, connection, grant); + } + function responseTooLargeError() { return new ToolGatewayHttpError( 502, @@ -7926,7 +7947,9 @@ export function createToolGatewayService( ): Promise { const claim = await claimToolTestApproval(db, actionRequestId, invocation.id); if (!claim) return; + claim.enforceDeadline = asRecord(signedPayload.approvalSnapshot)?.approvalSnapshotVersion === 2; let execution: Parameters[0]; + let connectionRevision: string | null = null; try { const agentId = invocation.agentId; if (!invocation.connectionId || !agentId) { @@ -7950,11 +7973,15 @@ export function createToolGatewayService( }, "connections:test", delegated.resource, { targetAgentIds: [agentId], afterHumanAction: true }); Object.assign(session, managementContext.workContext, { originatingAgentId: delegated.agentId, approvedInvocationId: invocation.id }); } + connectionRevision = await toolApprovalConnectionRevision(db, invocation.companyId, + invocation.connectionId, signedPayload.approvalSnapshot); const tool = (await connectedMcpToolsForConnection(invocation.companyId, invocation.connectionId)) .find(candidate => candidate.name === invocation.toolName || candidate.upstreamToolName === invocation.toolName); if (!tool) throw new ToolGatewayHttpError(404, `Tool "${invocation.toolName}" is no longer connected`, "tool_not_found"); - if (!approvalSnapshotsMatch(signedPayload.approvalSnapshot, await connectedRemoteApprovalSnapshot(session, tool))) { - throw new ToolGatewayHttpError(409, "Tool definition or connection changed; request a new review", "approved_tool_target_changed"); + await prepareApprovedConnectionAuthorization(session, tool, signedPayload.approvalSnapshot); + const liveApprovalSnapshot = await connectedRemoteApprovalSnapshot(session, tool); + if (!approvalSnapshotsMatch(signedPayload.approvalSnapshot, liveApprovalSnapshot)) { + throw new ToolGatewayHttpError(409, approvalTargetChangeMessage(signedPayload.approvalSnapshot, liveApprovalSnapshot), "approved_tool_target_changed"); } if (!(await approvedManagedArgumentsRemainCurrent(session, tool, signedPayload.arguments))) { throw new ToolGatewayHttpError(409, "Approved tool action managed arguments changed after review", "approved_tool_managed_arguments_changed"); @@ -7990,6 +8017,13 @@ export function createToolGatewayService( companyId: invocation.companyId, agentId, userId, argumentsSummary, reasonCode: "approval_granted", approvedClaim: claim, matchedPolicyIds: invocation.matchedPolicyIds ?? [] }; } catch (err) { + if (connectionRevision !== null && isToolApprovalConnectionError(err)) { + if (await releaseToolApproval(db, claim, connectionRevision)) { + await reflectToolActionInteractionLifecycle({ actionRequestId, status: "approved", + errorCode: TOOL_APPROVAL_CONNECTION_REQUIRED, errorMessage: TOOL_APPROVAL_CONNECTION_MESSAGE }); + } + return; + } if (!isApprovalPreparationRejection(err)) { // Preparation has not entered runTestToolInvocation. Preserve approval // through dependency failures; the claim owner still verifies no dispatch. @@ -8362,6 +8396,7 @@ export function createToolGatewayService( actionRequestId: claimed.id, invocationId: invocation.id, claimedAt: claimed.updatedAt, }; const agentId = invocation.agentId; + let connectionRevision: string | null = null; async function prepareExecution() { // Terminal-issue expiry revokes pending/approved requests, but a claim that // committed just before the issue closed slips past that revocation. Recheck @@ -8394,6 +8429,7 @@ export function createToolGatewayService( ); throw error; } + claim.enforceDeadline = asRecord(signedPayload.approvalSnapshot)?.approvalSnapshotVersion === 2; const session: ToolGatewaySession = { id: `approved-action:${claimed.id}`, @@ -8418,7 +8454,10 @@ export function createToolGatewayService( ); session.approvedInvocationId = invocation.id; session.approvedSlackInvocationId = invocation.id; + connectionRevision = await toolApprovalConnectionRevision(db, invocation.companyId, + invocation.connectionId, signedPayload.approvalSnapshot); const tool = await findToolForSession(session, invocation.toolName); + await prepareApprovedConnectionAuthorization(session, tool, signedPayload.approvalSnapshot); const liveApprovalSnapshot = await connectedRemoteApprovalSnapshot( session, tool, @@ -8431,7 +8470,7 @@ export function createToolGatewayService( ) { const error = new ToolGatewayHttpError( 409, - "Approved tool action target changed after review", + approvalTargetChangeMessage(signedPayload.approvalSnapshot, liveApprovalSnapshot), "approved_tool_target_changed", ); throw error; @@ -8519,7 +8558,12 @@ export function createToolGatewayService( try { prepared = await prepareExecution(); } catch (error) { - if (isApprovalPreparationRejection(error)) { + if (connectionRevision !== null && isToolApprovalConnectionError(error)) { + if (await releaseToolApproval(db, claim, connectionRevision)) { + await reflectToolActionInteractionLifecycle({ actionRequestId: claimed.id, status: "approved", + errorCode: TOOL_APPROVAL_CONNECTION_REQUIRED, errorMessage: TOOL_APPROVAL_CONNECTION_MESSAGE }); + } + } else if (isApprovalPreparationRejection(error)) { await markApprovedActionFailed({ actionRequestId: claimed.id, invocationId: invocation.id, @@ -9908,7 +9952,7 @@ export function createToolGatewayService( canonicalArgumentsSummary: argumentValidation.summary, signedArguments, previewMarkdown, - expiresAt: new Date(Date.now() + 60 * 60 * 1000), + expiresAt: pendingActionRequest.expiresAt, updatedAt: new Date(), }) .where(eq(toolActionRequests.id, pendingActionRequest.id))); @@ -10099,9 +10143,10 @@ export function createToolGatewayService( if (!isTestOriginInvocation(invocation) && payload.executionOnApprove !== true) return false; await this.approveActionRequest({ companyId: request.companyId, actionRequestId: request.id, actor: { userId: request.decidedByUserId ?? request.resolvedByUserId } }); - const [remaining] = await db.select({ status: toolActionRequests.status }).from(toolActionRequests) + const [remaining] = await db.select({ status: toolActionRequests.status, errorCode: toolInvocations.errorCode }) + .from(toolActionRequests).innerJoin(toolInvocations, eq(toolInvocations.id, toolActionRequests.invocationId)) .where(eq(toolActionRequests.id, request.id)); - return remaining?.status === "approved"; + return remaining?.status === "approved" && remaining.errorCode !== TOOL_APPROVAL_CONNECTION_REQUIRED; } catch (err) { logger.warn({ err, actionRequestId: row.request.id }, "Could not recover approved tool action"); return true; @@ -10264,6 +10309,10 @@ export function createToolGatewayService( } } if (actionRequest.status === "approved") { + if (invocation.errorCode === TOOL_APPROVAL_CONNECTION_REQUIRED && + !await toolApprovalConnectionCanResume(db, invocation.companyId, invocation.connectionId, + signedPayload.approvalSnapshot, invocation.policyExplanation?.approvalConnectionWait)) + return actionRequest; await reflectToolActionInteractionLifecycle({ actionRequestId: actionRequest.id, status: "approved", @@ -10299,75 +10348,77 @@ export function createToolGatewayService( !isTestOriginInvocation(invocation) && signedPayload.executionOnApprove === true ) { - const [issue] = await db - .select() - .from(issues) - .where( - and( - eq(issues.id, invocation.issueId!), - eq(issues.companyId, input.companyId), - ), - ) - .limit(1); - if (!issue || issue.status === "done" || issue.status === "cancelled") - throw new ToolGatewayHttpError( - 409, - "Task is closed", - "action_task_closed", - ); - const session: ToolGatewaySession = { - id: `review:${actionRequest.id}`, - token: "", - companyId: input.companyId, - agentId: invocation.agentId, - actorType: "agent", - actorId: invocation.actorId, - originatingAgentId: typeof invocation.policyExplanation?.delegatedGatewayIssuer === "string" ? invocation.policyExplanation.delegatedGatewayIssuer : invocation.actorType === "agent" && invocation.gatewayId && !invocation.runId && !invocation.gatewayTokenId ? invocation.actorId : null, - gatewayId: invocation.gatewayId, - gatewayPublicId: invocation.gatewayPublicId, - gatewayTokenId: invocation.gatewayTokenId, - runId: invocation.runId, - issueId: issue.id, - projectId: issue.projectId, - createdAt: new Date(), - expiresAt: new Date(Date.now() + DEFAULT_SESSION_TTL_MS), - }; - await restoreApprovedActionIdentity( - session, - signedPayload.identityContextId, - ); - session.approvedSlackInvocationId = invocation.id; - const tool = await findToolForSession(session, invocation.toolName); - if ( - !approvalSnapshotsMatch( - signedPayload.approvalSnapshot, - await connectedRemoteApprovalSnapshot(session, tool), - ) - ) - throw new ToolGatewayHttpError( - 409, - "Tool definition or connection changed; request a new review", - "approved_tool_target_changed", - ); - const previewHash = summarizeToolValue(signedPayload.arguments).sha256; - const access = await policyService.decide( - policyInputForTool({ + try { + const [issue] = await db + .select() + .from(issues) + .where( + and( + eq(issues.id, invocation.issueId!), + eq(issues.companyId, input.companyId), + ), + ) + .limit(1); + if (!issue || issue.status === "done" || issue.status === "cancelled") + throw new ToolGatewayHttpError( + 409, + "Task is closed", + "action_task_closed", + ); + const session: ToolGatewaySession = { + id: `review:${actionRequest.id}`, + token: "", + companyId: input.companyId, + agentId: invocation.agentId, + actorType: "agent", + actorId: invocation.actorId, + originatingAgentId: typeof invocation.policyExplanation?.delegatedGatewayIssuer === "string" ? invocation.policyExplanation.delegatedGatewayIssuer : invocation.actorType === "agent" && invocation.gatewayId && !invocation.runId && !invocation.gatewayTokenId ? invocation.actorId : null, + gatewayId: invocation.gatewayId, + gatewayPublicId: invocation.gatewayPublicId, + gatewayTokenId: invocation.gatewayTokenId, + runId: invocation.runId, + issueId: issue.id, + projectId: issue.projectId, + createdAt: new Date(), + expiresAt: new Date(Date.now() + DEFAULT_SESSION_TTL_MS), + }; + await restoreApprovedActionIdentity( session, - tool, - parameters: signedPayload.arguments, - consumeRateLimit: false, - }), - { admittedInvocationId: invocation.id, requiredGatewayAction: "tools/call", - admittedPreviewHash: [previewHash, toolRequestIdentityHash(signedPayload.arguments, options.toolActionSigningSecret)] - .includes(actionRequest.canonicalArgumentsHash) ? previewHash : undefined, - }, - ); - if (!access.allowed && access.decision !== "require_approval") - throw new ToolGatewayHttpError( - 403, - access.explanation, - access.reasonCode, + signedPayload.identityContextId, ); + session.approvedSlackInvocationId = invocation.id; + const tool = await findToolForSession(session, invocation.toolName); + const liveApprovalSnapshot = await connectedRemoteApprovalSnapshot(session, tool); + if (!approvalSnapshotsMatch(signedPayload.approvalSnapshot, liveApprovalSnapshot)) + throw new ToolGatewayHttpError( + 409, + approvalTargetChangeMessage(signedPayload.approvalSnapshot, liveApprovalSnapshot), + "approved_tool_target_changed", + ); + const previewHash = summarizeToolValue(signedPayload.arguments).sha256; + const access = await policyService.decide( + policyInputForTool({ + session, + tool, + parameters: signedPayload.arguments, + consumeRateLimit: false, + }), + { admittedInvocationId: invocation.id, requiredGatewayAction: "tools/call", + admittedPreviewHash: [previewHash, toolRequestIdentityHash(signedPayload.arguments, options.toolActionSigningSecret)] + .includes(actionRequest.canonicalArgumentsHash) ? previewHash : undefined, + }, + ); + if (!access.allowed && access.decision !== "require_approval") + throw new ToolGatewayHttpError( + 403, + access.explanation, + access.reasonCode, + ); + } catch (error) { + // Save the human decision even if the existing authorization needs + // repair. Dispatch revalidates the signed target after reconnection. + if (!isToolApprovalConnectionError(error)) throw error; + } } const updated = await commitToolActionReview(db, { ...input, @@ -10993,7 +11044,7 @@ export function createToolGatewayService( ) { throw new ToolGatewayHttpError( 409, - "Approved tool action target changed after review", + approvalTargetChangeMessage(signedPayload.approvalSnapshot, liveApprovalSnapshot), "approved_tool_target_changed", { invocationId: storedInvocation.id, diff --git a/server/src/services/tool-review-expiry.ts b/server/src/services/tool-review-expiry.ts index 8db647ce149..0e630c9def3 100644 --- a/server/src/services/tool-review-expiry.ts +++ b/server/src/services/tool-review-expiry.ts @@ -1,6 +1,8 @@ -import { and, asc, eq, gt, isNotNull, lte, sql } from "drizzle-orm"; +import { and, asc, eq, gt, isNotNull, lte, or, sql } from "drizzle-orm"; +import { unionAll } from "drizzle-orm/pg-core"; import { toolActionRequests, toolInvocations, type Db } from "@paperclipai/db"; import { writeToolReviewChange } from "./tool-review-work-notifications.js"; +import { TOOL_APPROVAL_CONNECTION_REQUIRED } from "./tool-approval-connection.js"; export function toolReviewExpiryService(db: Db, onExpired: (input: { actionRequestId: string; @@ -9,24 +11,38 @@ export function toolReviewExpiryService(db: Db, onExpired: (input: { errorCode: string; errorMessage: string; }) => Promise) { + const pending = eq(toolActionRequests.status, "pending"); + const waiting = and(eq(toolActionRequests.status, "approved"), + sql`exists (select 1 from ${toolInvocations} where ${toolInvocations.id} = ${toolActionRequests.invocationId} + and ${toolInvocations.companyId} = ${toolActionRequests.companyId} + and ${toolInvocations.status} = 'awaiting_approval' and ${toolInvocations.startedAt} is null + and ${toolInvocations.errorCode} = ${TOOL_APPROVAL_CONNECTION_REQUIRED})`); + const eligible = or(pending, waiting); return { async next(): Promise { - const [row] = await db.select({ at: sql`min(${toolActionRequests.expiresAt})` }) - .from(toolActionRequests).where(eq(toolActionRequests.status, "pending")); + const earliest = (condition: typeof pending | typeof waiting) => db.select({ + at: sql`min(${toolActionRequests.expiresAt})`.as("at"), + }).from(toolActionRequests).where(condition); + const candidates = unionAll(earliest(pending), earliest(waiting)).as("review_deadlines"); + const [row] = await db.select({ at: sql`min(${candidates.at})` }).from(candidates); return row?.at == null ? null : new Date(row.at).getTime(); }, sweep, sweepForCompany: (companyId: string) => sweep(new Date(), companyId, 1), async deadlines(companyId?: string) { - const pendingExpiry = and(eq(toolActionRequests.status, "pending"), isNotNull(toolActionRequests.expiresAt)); - const rows = companyId === undefined - ? await db.select({ companyId: toolActionRequests.companyId, - at: sql`min(${toolActionRequests.expiresAt})` }) - .from(toolActionRequests).where(pendingExpiry).groupBy(toolActionRequests.companyId) - : await db.select({ companyId: toolActionRequests.companyId, - at: sql`${toolActionRequests.expiresAt}` }) - .from(toolActionRequests).where(and(pendingExpiry, eq(toolActionRequests.companyId, companyId))) + // Keep the pending-only predicate in its own branch so PostgreSQL can + // still read one entry from the partial deadline index on large backlogs. + const deadlines = (condition: typeof pending | typeof waiting) => companyId === undefined + ? db.select({ companyId: toolActionRequests.companyId, + at: sql`min(${toolActionRequests.expiresAt})`.as("at") }) + .from(toolActionRequests).where(and(condition, isNotNull(toolActionRequests.expiresAt))).groupBy(toolActionRequests.companyId) + : db.select({ companyId: toolActionRequests.companyId, + at: sql`${toolActionRequests.expiresAt}`.as("at") }) + .from(toolActionRequests).where(and(condition, isNotNull(toolActionRequests.expiresAt), eq(toolActionRequests.companyId, companyId))) .orderBy(asc(toolActionRequests.expiresAt)).limit(1); + const candidates = unionAll(deadlines(pending), deadlines(waiting)).as("review_deadlines"); + const rows = await db.select({ companyId: candidates.companyId, + at: sql`min(${candidates.at})` }).from(candidates).groupBy(candidates.companyId); return rows.map(row => ({ companyId: row.companyId, dueAt: new Date(row.at).getTime() })); }, }; @@ -35,17 +51,18 @@ export function toolReviewExpiryService(db: Db, onExpired: (input: { let scanned = 0; for (let page = 0; page < maxPages; page++) { const rows = await db.select().from(toolActionRequests).where(and( - eq(toolActionRequests.status, "pending"), lte(toolActionRequests.expiresAt, now), + eligible, lte(toolActionRequests.expiresAt, now), cursor ? gt(toolActionRequests.id, cursor) : undefined, companyId === undefined ? undefined : eq(toolActionRequests.companyId, companyId), )).orderBy(asc(toolActionRequests.id)).limit(100); for (const row of rows) { const errorCode = "action_expired"; - const errorMessage = "The approval request expired before a decision."; + const errorMessage = row.status === "approved" ? "The action deadline passed before execution." + : "The approval request expired before a decision."; const changed = await writeToolReviewChange(db, row.companyId, async tx => { const [updated] = await tx.update(toolActionRequests) .set({ status: "expired", resolvedAt: now, updatedAt: now }).where(and( - eq(toolActionRequests.id, row.id), eq(toolActionRequests.status, "pending"), + eq(toolActionRequests.id, row.id), eq(toolActionRequests.status, row.status), eq(toolActionRequests.updatedAt, row.updatedAt), )).returning({ id: toolActionRequests.id }); if (!updated) return false; diff --git a/server/src/services/tool-review-projection.ts b/server/src/services/tool-review-projection.ts index 32644929cac..0fa8983836a 100644 --- a/server/src/services/tool-review-projection.ts +++ b/server/src/services/tool-review-projection.ts @@ -1,17 +1,20 @@ -import { and, asc, eq, gt, inArray, sql } from "drizzle-orm"; +import { and, asc, eq, gt, inArray, or, sql } from "drizzle-orm"; import { issueThreadInteractions, toolActionRequests, toolInvocations, type Db } from "@paperclipai/db"; +import { TOOL_APPROVAL_CONNECTION_REQUIRED } from "./tool-approval-connection.js"; export function toolReviewProjectionService(db: Db, reflect: (input: { actionRequestId: string; - status: "executed" | "failed" | "expired" | "cancelled"; + status: "approved" | "executed" | "failed" | "expired" | "cancelled"; errorCode: string | null; errorMessage: string | null; resultSummary: string | null; }) => Promise) { - const mismatch = and( + const mismatch = or(and( inArray(toolActionRequests.status, ["executed", "failed", "expired", "cancelled"]), sql`coalesce(${issueThreadInteractions.result}->'toolAction'->>'status', '') <> case when ${toolActionRequests.status} = 'cancelled' then 'expired' else ${toolActionRequests.status} end`, - ); + ), and(eq(toolActionRequests.status, "approved"), eq(toolInvocations.errorCode, TOOL_APPROVAL_CONNECTION_REQUIRED), + or(sql`coalesce(${issueThreadInteractions.result}->'toolAction'->>'status', '') <> 'approved'`, + sql`coalesce(${issueThreadInteractions.result}->'toolAction'->>'errorCode', '') <> ${TOOL_APPROVAL_CONNECTION_REQUIRED}`))); const candidates = (companyId?: string, cursor?: string) => db .select({ request: toolActionRequests, invocation: toolInvocations }) .from(toolActionRequests) @@ -55,7 +58,7 @@ export function toolReviewProjectionService(db: Db, reflect: (input: { if (!canContinue()) break; try { await reflect({ actionRequestId: request.id, - status: request.status as "executed" | "failed" | "expired" | "cancelled", + status: request.status as "approved" | "executed" | "failed" | "expired" | "cancelled", errorCode: invocation.errorCode, errorMessage: invocation.errorMessage, resultSummary: invocation.resultSummary?.summary ?? null }); } catch (error) { failures.push(error); } diff --git a/server/src/services/tool-review-recovery.ts b/server/src/services/tool-review-recovery.ts index 69aa98c76f0..b7fae255536 100644 --- a/server/src/services/tool-review-recovery.ts +++ b/server/src/services/tool-review-recovery.ts @@ -3,6 +3,7 @@ import { approvals, toolActionRequests, toolInvocations, type Db } from "@paperc import { readSignedToolArgumentsPayload } from "./tool-content-guards.js"; import { notifyToolReviewChange } from "./tool-review-work-notifications.js"; +import { toolApprovalConnectionCanResume, TOOL_APPROVAL_CONNECTION_REQUIRED } from "./tool-approval-connection.js"; type ReviewProjection = { actionRequestId: string; @@ -85,6 +86,10 @@ export async function prepareApprovedToolReview(db: Db, input: { return { kind: "skip" } as const; } } + // A passive connection wait cannot outlive a withdrawn governance decision. + if (invocation.errorCode === TOOL_APPROVAL_CONNECTION_REQUIRED && + !await toolApprovalConnectionCanResume(tx as unknown as Db, invocation.companyId, invocation.connectionId, + payload.approvalSnapshot, invocation.policyExplanation?.approvalConnectionWait)) return { kind: "skip" } as const; return { kind: "ready", request, invocation, payload } as const; }); } diff --git a/ui/src/components/IssueThreadInteractionCard.test.tsx b/ui/src/components/IssueThreadInteractionCard.test.tsx index 1374d3f109b..c3cc66b56c2 100644 --- a/ui/src/components/IssueThreadInteractionCard.test.tsx +++ b/ui/src/components/IssueThreadInteractionCard.test.tsx @@ -1311,6 +1311,21 @@ describe("IssueThreadInteractionCard tool-action card", () => { expect(host.textContent).toContain("Launch checklist"); }); + it("keeps an approved connection wait actionable without showing execution or another approval", () => { + const interaction = structuredClone(runningToolActionInteraction); + interaction.payload.toolAction!.expiresAt = null; + interaction.result!.toolAction = { version: 1, status: "approved", updatedAt: new Date().toISOString(), + errorCode: "approved_action_connection_required" }; + const host = renderCard({ interaction }); + expect(host.textContent).toContain("Your approval is saved"); + expect(host.textContent).toContain("Sign-in needed"); + expect(host.textContent).not.toContain("Running"); + expect(host.textContent).not.toContain("Approve & run"); + expect(host.textContent).not.toContain("Expires"); + expect(host.querySelector(`a[href="/apps/${interaction.payload.toolAction!.connectionId}/permissions"]`)?.textContent) + .toContain("Reconnect"); + }); + it("renders only the server-redacted proposal as plain text when inspected", async () => { const interaction = structuredClone(pendingToolActionWriteInteraction); interaction.payload.toolAction!.argumentsSummaryJson = JSON.stringify({ text: '', apiKey: "[redacted]" }); diff --git a/ui/src/components/IssueThreadInteractionCard.tsx b/ui/src/components/IssueThreadInteractionCard.tsx index 6ce2763aa91..46232814685 100644 --- a/ui/src/components/IssueThreadInteractionCard.tsx +++ b/ui/src/components/IssueThreadInteractionCard.tsx @@ -444,6 +444,7 @@ function connectionAuthorizationHref(interaction: RequestConfirmationInteraction type ToolActionCardState = | "pending" + | "waiting_connection" | "running" | "executed" | "failed" @@ -465,6 +466,8 @@ function toolActionCardState( if (interaction.status === "rejected") return "declined"; if (interaction.status === "cancelled") return "cancelled"; if (interaction.status === "expired") return "expired"; + if (execStatus === "approved" && interaction.result?.toolAction?.errorCode === "approved_action_connection_required") + return "waiting_connection"; // Terminal execution outcomes take precedence over the coarse interaction // status so a self-resolving "running…" advances to its real result. if (execStatus === "executed") return "executed"; @@ -484,6 +487,9 @@ function toolActionStatusClasses(state: ToolActionCardState): { dimmed?: boolean; } { switch (state) { + case "waiting_connection": + return { shell: "border-2 border-border bg-transparent", badge: "border-border bg-muted text-muted-foreground", + label: "Sign-in needed", Icon: Clock }; case "running": return { shell: "border-2 border-amber-500/70 bg-transparent", @@ -1742,6 +1748,7 @@ function ToolActionResolution({ } const labels = { pending: "Waiting for approval", + waiting_connection: "Approved · Sign-in needed", running: "Approved · Running…", executed: "Succeeded", failed: "Execution failed", @@ -1753,6 +1760,15 @@ function ToolActionResolution({ const detail = state === "failed" ? result?.errorMessage?.trim() || "The action could not complete." : state === "declined" ? interaction.result?.reason?.trim() : null; + if (state === "waiting_connection") { + const connectionId = interaction.payload.toolAction?.connectionId; + const appName = interaction.payload.toolAction?.appDisplayName || "app"; + return
+

Approved · Sign-in needed

+

Your approval is saved. Reconnect the app to continue. Paperclip will check the account and action again before it runs.

+ {connectionId ? : null} +
; + } if (state === "failed" && result?.errorCode === "github_access_denied") { const connectionId = interaction.payload.toolAction?.connectionId; return
@@ -1889,7 +1905,7 @@ function RequestToolActionCard({ ); } -type SecretProposalCardState = ToolActionCardState; +type SecretProposalCardState = Exclude; function secretProposalCardState( interaction: RequestConfirmationInteraction, diff --git a/ui/src/components/task-chat/TaskChatCompactInteractionCard.tsx b/ui/src/components/task-chat/TaskChatCompactInteractionCard.tsx index dbcb7487716..818da0ad1d3 100644 --- a/ui/src/components/task-chat/TaskChatCompactInteractionCard.tsx +++ b/ui/src/components/task-chat/TaskChatCompactInteractionCard.tsx @@ -560,12 +560,12 @@ function ReceiptDisclosure({ {interaction.payload.toolAction.toolDisplayName}{" "} · {interaction.payload.toolAction.risk} risk

-

+ {interaction.payload.toolAction.expiresAt ?

Expires{" "} {new Date( interaction.payload.toolAction.expiresAt, ).toLocaleString()} -

+

: null}
{interaction.payload.toolAction.previewMarkdown} @@ -921,12 +921,12 @@ function ConfirmationCard({ > {interaction.payload.toolAction.risk} risk - + {interaction.payload.toolAction.expiresAt ? Expires{" "} {new Date( interaction.payload.toolAction.expiresAt, ).toLocaleString()} - + : null}
{interaction.payload.toolAction.previewMarkdown} diff --git a/ui/src/lib/interaction-resolution-error.test.ts b/ui/src/lib/interaction-resolution-error.test.ts index de1e9c7ad6a..ba2dbe4eea3 100644 --- a/ui/src/lib/interaction-resolution-error.test.ts +++ b/ui/src/lib/interaction-resolution-error.test.ts @@ -58,6 +58,11 @@ describe("isInteractionAudienceDenial", () => { }); describe("describeInteractionResolutionFailure", () => { + it("does not tell a person to retry a stale tool approval", () => { + const message = "The connected account, credentials, or permissions changed. A new review is required."; + expect(describeInteractionResolutionFailure(denial(409, "approved_tool_target_changed", message))) + .toMatchObject({ kind: "settled", message }); + }); it("keeps the server denial reason and names who can respond", () => { const failure = describeInteractionResolutionFailure( denial(403, "interaction_human_only", "This issue-thread interaction is human-only"), diff --git a/ui/src/lib/interaction-resolution-error.ts b/ui/src/lib/interaction-resolution-error.ts index 8320048012d..fd4863a5832 100644 --- a/ui/src/lib/interaction-resolution-error.ts +++ b/ui/src/lib/interaction-resolution-error.ts @@ -40,6 +40,8 @@ const INTERACTION_SETTLED_CODES = [ "interaction_superseded", "interaction_stale_target", "interaction_issue_closed", + "approved_tool_target_changed", + "approved_tool_managed_arguments_changed", ] as const; export type InteractionResolutionFailureKind = "audience_denied" | "settled" | "transient"; diff --git a/ui/storybook/stories/questions-and-approvals/connection-reviews.stories.tsx b/ui/storybook/stories/questions-and-approvals/connection-reviews.stories.tsx index d3deaabfa93..bfd4c03df43 100644 --- a/ui/storybook/stories/questions-and-approvals/connection-reviews.stories.tsx +++ b/ui/storybook/stories/questions-and-approvals/connection-reviews.stories.tsx @@ -12,7 +12,7 @@ import { declinedToolActionInteraction, expiredToolActionInteraction, } from "@/fixtures/issueThreadInteractionFixtures"; -import { within, userEvent } from "storybook/test"; +import { expect, within, userEvent } from "storybook/test"; import { toolsApi } from "@/api/tools"; import { issuesApi } from "@/api/issues"; import { ReviewQueueCard } from "@/pages/apps/ReviewQueueCard"; @@ -303,6 +303,24 @@ export const Approved: Story = { export const Executing: Story = { render: () => , }; +export const ApprovedWaitingForConnection: Story = { + parameters: { paperclip: { capabilities: ["questions-and-approvals/tool-review", "connection-setup/task-request"], + implementation: "production", boundary: "fixtures", + notes: "Saved approval awaiting app sign-in. Uses the production task thread and repair link; OAuth repair and provider execution are verified separately by the server integration fixture." } }, + render: () => , + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect(canvas.getByText("Your approval is saved.", { exact: false })).toBeVisible(); + await expect(canvas.getByRole("link", { name: "Reconnect Notion" })).toBeVisible(); + await expect(canvas.queryByRole("button", { name: "Approve & run" })).not.toBeInTheDocument(); + }, +}; export const Succeeded: Story = { render: () => , };