From 51f305f1d16ea5bce71ee32540caa1942f3ff350 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 26 Oct 2026 04:37:35 -0500 Subject: [PATCH] fix(server): notify native recovery from OAuth card changes Co-Authored-By: Paperclip --- ...2026-10-24-recurring-server-sleep-stack.md | 10 ++ ...-native-status-reassessment-sleep-audit.md | 46 +++++++ .../src/__tests__/tool-access-service.test.ts | 40 ++++++ .../tool-access/oauth/authorization-start.ts | 123 +++++++++--------- 4 files changed, 160 insertions(+), 59 deletions(-) diff --git a/doc/plans/2026-10-24-recurring-server-sleep-stack.md b/doc/plans/2026-10-24-recurring-server-sleep-stack.md index e8f9aff047f..445a351c558 100644 --- a/doc/plans/2026-10-24-recurring-server-sleep-stack.md +++ b/doc/plans/2026-10-24-recurring-server-sleep-stack.md @@ -2556,3 +2556,13 @@ classifier consumer cases and server types pass. The audit preserves before-fix failures and the remaining identity/producer and cost limits. Startup wiring, periodic removal, remote gates, older stack findings and all remaining recurring work are still incomplete. Nothing merged. + + +Slice 50 OAuth producer checkpoint: authorization-card creation/reopening now +signals both native review workers from the same short transaction as its card +and state link. Three missing-settlement-signal regressions were reproduced; +all 472 tool-access and 30 worker lifecycle tests plus server typecheck pass. +The audit retains before/after evidence and the remaining handoff/authority +coverage limits. Master is freshly verified at 1fbbef45a8. Discovery/cost proof, +startup wiring, periodic removal, remote acceptance, earlier stack findings and +the full original recurring-work scope remain incomplete. Nothing merged. diff --git a/doc/plans/2026-10-26-native-status-reassessment-sleep-audit.md b/doc/plans/2026-10-26-native-status-reassessment-sleep-audit.md index fd252f21855..78e730c29dd 100644 --- a/doc/plans/2026-10-26-native-status-reassessment-sleep-audit.md +++ b/doc/plans/2026-10-26-native-status-reassessment-sleep-audit.md @@ -781,3 +781,49 @@ cases still run without a locking API in their fake query builder. Remaining: producer/authority coverage, query/history/backlog cost, startup integration, removal of this periodic duty, PR publication and remote acceptance. The full recurring-work stack and its older unresolved gates remain open. + + +## OAuth authorization-start producer checkpoint (2026-10-26) + +The direct OAuth authorization-start owner creates a confirmation or reopens its +previous accepted/rejected card by idempotency key. That bypassed the interaction +service's native-review notifications. Three real tool-access service regressions +(creation, reopening, outer rollback) reproduced absent settled notifications: +/tmp/sleep50-oauth-before.log/.status, three failed assertions, exit 1. + +The existing card lookup/mutation and OAuth-state interaction link now share a +short database transaction. It awaits notifyNativeReviewState before writing, +with company/task scope. The shared hook wakes both governed review response and +status reassessment; consumers still select only actionable durable state. +Provider discovery, client setup and authorization remain outside the new short +transaction. Browser/user flows that do not create an agent authorization card +keep their existing path. Caller-owned transactions retain their outer boundary. +No provider-specific logic moved into recovery or scheduler code. + +All 472 tool-access consumer cases pass, including the new real nested-transaction +tests. They assert the inside/outside card states, no pre-commit notification, +one settlement notification per consumer, and rollback retaining the prior card +and no OAuth state. Rollback settlement releases the intent for reconciliation; +it is not represented as a committed card. All 30 reassessment lifecycle cases +pass against retained history, and server typecheck passes. Evidence (all terminal): +/tmp/sleep50-oauth-after.log/.status, /tmp/sleep50-oauth-lifecycle.log/.status and +/tmp/sleep50-oauth-types.log/.status, all exit 0; 502 distinct tests in this checkpoint. + +A fresh origin/master fetch still resolves to +1fbbef45a8d399771add7a27a04b12f3a20e264c. No source changed during acceptance. + +Additional handoff audit evidence: direct operator cancellation flows through +cancelPendingCaseHandoffsForIssue; run-control/task cancellation uses +heartbeat/case-handoff-lifecycle; delivery can cancel a fenced claim in its own +settlement transaction. The classifier accepts all persisted non-cancelled +handoff states. Those delivery/lease transitions are not independently positive +evidence triggers, so do not fan out every lease renewal. Creation assigns its +UUID inside the owning transaction. The previous checkpoint now fences stale +handoff evidence at commit. Complete activation/quiet-state proof for the +remaining handoff/authority paths is still outstanding; no blanket producer- +coverage claim is made from this search. + +Remaining acceptance continues to include producer/authority audit, discovery +and commit costs, backlog and lifecycle coverage, startup registration, periodic +removal, PR publication and exact-head remote gates. The full original stack +scope and older unresolved gates remain active. diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index 38c88536a13..1a11bf82e6b 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -1,3 +1,4 @@ +import { DELIVERY_QUEUES, subscribeDeliveryWork } from "../services/delivery-work-notifications.js"; import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js"; import * as fs from "node:fs/promises"; import { createHash, generateKeyPairSync, randomUUID } from "node:crypto"; @@ -10078,6 +10079,45 @@ describeEmbeddedPostgres("tool access service", () => { }); + it.each(["creation", "reopening", "rollback"] as const)("signals native review recovery after OAuth card %s settles", async mode => { + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_SLACK_CLIENT_ID", "slack-client-id"); + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_SLACK_CLIENT_SECRET", "slack-client-secret"); + const company = await createCompany(db); + const agent = await createAgent(db, company.id); + const { issue, run } = await createIssueAndRun(db, company.id, agent.id); + const connected = await createTestToolAccessService(db).connectGalleryApp(company.id, { galleryKey: "slack", name: "Slack user auth" }); + if (mode !== "creation") await db.insert(issueThreadInteractions).values({ companyId: company.id, + issueId: issue.id, kind: "request_confirmation", status: "accepted", payload: { version: 1, prompt: "Previous authorization" }, + idempotencyKey: `connection-authorization:${connected.connectionId}:user-for-run`, sourceRunId: run.id }); + const reassessment = vi.fn(), reviewResponse = vi.fn(); + const unsubscribe = [subscribeDeliveryWork(db, DELIVERY_QUEUES.nativeStatusReassessment, reassessment), + subscribeDeliveryWork(db, DELIVERY_QUEUES.nativeReviewResponse, reviewResponse)]; + const rollback = new Error("rollback OAuth reopening"); + try { + const operation = db.transaction(async tx => { + const started = await createTestToolAccessService(tx as unknown as typeof db).startOAuth(company.id, connected.connectionId, { + redirectUri: "https://paperclip.example/api/tools/oauth/callback", actor: { actorType: "agent", actorId: agent.id, sessionId: run.id }, + issueId: issue.id, subjectUserId: "user-for-run", scopes: ["channels:read"], + }); + const [inside] = await tx.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.issueId, issue.id)); + expect(inside.status).toBe("pending"); + expect(inside.payload).toMatchObject({ target: { href: started.authorizationUrl } }); + expect(reassessment).not.toHaveBeenCalled(); expect(reviewResponse).not.toHaveBeenCalled(); + const [outside] = await db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.issueId, issue.id)); + expect(outside?.status).toBe(mode === "creation" ? undefined : "accepted"); + if (mode === "rollback") throw rollback; + }); + if (mode === "rollback") await expect(operation).rejects.toBe(rollback); else await operation; + // Rollback still releases the intent for reconciliation; no uncommitted + // card becomes visible and neither consumer can run before outer settlement. + expect(reassessment).toHaveBeenCalledTimes(1); expect(reviewResponse).toHaveBeenCalledTimes(1); + const [persisted] = await db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.issueId, issue.id)); + expect(persisted.status).toBe(mode === "rollback" ? "accepted" : "pending"); + expect(await db.select().from(toolOauthStates).where(eq(toolOauthStates.issueId, issue.id))) + .toHaveLength(mode === "rollback" ? 0 : 1); + } finally { unsubscribe.forEach(stop => stop()); } + }); + it("creates and resolves an agent-initiated user authorization grant card", async () => { vi.stubEnv("PAPERCLIP_TOOL_OAUTH_SLACK_CLIENT_ID", "slack-client-id"); vi.stubEnv( diff --git a/server/src/services/tool-access/oauth/authorization-start.ts b/server/src/services/tool-access/oauth/authorization-start.ts index 823b79eb5ef..6c024e92bf1 100644 --- a/server/src/services/tool-access/oauth/authorization-start.ts +++ b/server/src/services/tool-access/oauth/authorization-start.ts @@ -1,3 +1,4 @@ +import { notifyNativeReviewState } from "../../native-review-state-notifications.js"; import { vercelConnectHttpError, vercelCredentialFor, vercelConnectResourcesFor } from "../vercel-credentials.js"; import { credentialScope, managedConnectorProfile } from "./client-settings.js"; import { createOAuthClientRegistration } from "./client-registration.js"; @@ -532,65 +533,69 @@ export function createOAuthAuthorizationStart(db: Db, dependencies: Dependencies href: authorizationUrl.toString(), }, }; - const [existingInteraction] = await db - .select() - .from(issueThreadInteractions) - .where( - and( - eq(issueThreadInteractions.companyId, companyId), - eq(issueThreadInteractions.issueId, input.issueId), - eq(issueThreadInteractions.idempotencyKey, idempotencyKey), - ), - ) - .limit(1); - const [interaction] = existingInteraction - ? await db - .update(issueThreadInteractions) - .set({ - status: "pending", - requestedResolverPolicy: "human_only", - effectiveResolverPolicy: "human_only", - resolverPolicyProvenance: "explicit", - effectiveResolverPolicySource: "requested", - addresseeUserId: authorizationSubjectUserId, - payload, - result: null, - resolvedAt: null, - updatedAt: new Date(), - }) - .where(eq(issueThreadInteractions.id, existingInteraction.id)) - .returning() - : await db - .insert(issueThreadInteractions) - .values({ - companyId, - issueId: input.issueId, - kind: "request_confirmation", - status: "pending", - continuationPolicy: "none", - requestedResolverPolicy: "human_only", - effectiveResolverPolicy: "human_only", - resolverPolicyProvenance: "explicit", - effectiveResolverPolicySource: "requested", - addresseeUserId: authorizationSubjectUserId, - idempotencyKey, - sourceRunId: - binding.actorType === "agent" - ? (input.actor.sessionId ?? null) - : null, - title: `Connect your ${providerName} to continue`, - summary: `${requestingAgent?.name ?? "An agent"} needs your ${providerName} identity for work running as you.`, - createdByAgentId: - binding.actorType === "agent" ? binding.actorId : null, - payload, - }) - .returning(); - if (interaction) { - await db - .update(toolOauthStates) - .set({ interactionId: interaction.id }) - .where(eq(toolOauthStates.state, state)); - } + const issueId = input.issueId; + await db.transaction(async tx => { + await notifyNativeReviewState(tx, { companyId, issueId }); + const [existingInteraction] = await tx + .select() + .from(issueThreadInteractions) + .where( + and( + eq(issueThreadInteractions.companyId, companyId), + eq(issueThreadInteractions.issueId, issueId), + eq(issueThreadInteractions.idempotencyKey, idempotencyKey), + ), + ) + .limit(1); + const [interaction] = existingInteraction + ? await tx + .update(issueThreadInteractions) + .set({ + status: "pending", + requestedResolverPolicy: "human_only", + effectiveResolverPolicy: "human_only", + resolverPolicyProvenance: "explicit", + effectiveResolverPolicySource: "requested", + addresseeUserId: authorizationSubjectUserId, + payload, + result: null, + resolvedAt: null, + updatedAt: new Date(), + }) + .where(eq(issueThreadInteractions.id, existingInteraction.id)) + .returning() + : await tx + .insert(issueThreadInteractions) + .values({ + companyId, + issueId: issueId, + kind: "request_confirmation", + status: "pending", + continuationPolicy: "none", + requestedResolverPolicy: "human_only", + effectiveResolverPolicy: "human_only", + resolverPolicyProvenance: "explicit", + effectiveResolverPolicySource: "requested", + addresseeUserId: authorizationSubjectUserId, + idempotencyKey, + sourceRunId: + binding.actorType === "agent" + ? (input.actor.sessionId ?? null) + : null, + title: `Connect your ${providerName} to continue`, + summary: `${requestingAgent?.name ?? "An agent"} needs your ${providerName} identity for work running as you.`, + createdByAgentId: + binding.actorType === "agent" ? binding.actorId : null, + payload, + }) + .returning(); + if (interaction) { + await tx + .update(toolOauthStates) + .set({ interactionId: interaction.id }) + .where(eq(toolOauthStates.state, state)); + } + }); } const nextConfig = {