diff --git a/.changeset/steady-authorization-challenges.md b/.changeset/steady-authorization-challenges.md new file mode 100644 index 0000000000..992c23f7d4 --- /dev/null +++ b/.changeset/steady-authorization-challenges.md @@ -0,0 +1,5 @@ +--- +"eve": patch +--- + +Reuse pending authorization challenges when multi-connection turns resume. Existing device codes remain valid and no longer appear as failed attempts. diff --git a/packages/eve/src/context/providers/pending-authorization.test.ts b/packages/eve/src/context/providers/pending-authorization.test.ts new file mode 100644 index 0000000000..d275e46e06 --- /dev/null +++ b/packages/eve/src/context/providers/pending-authorization.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from "vitest"; + +import { ContextContainer } from "#context/container.js"; +import { pendingAuthorizationProvider } from "#context/providers/pending-authorization.js"; +import { getPendingAuthorization, setPendingAuthorization } from "#harness/authorization.js"; +import type { HarnessSession } from "#harness/types.js"; + +describe("pendingAuthorizationProvider", () => { + it("rebuilds pending challenges from durable session state", async () => { + const state = setPendingAuthorization(undefined, { + challenges: [ + { + attemptId: "attempt-linear", + challenge: { userCode: "OLD-CODE", url: "https://idp.example/linear" }, + hookUrl: "https://agent.example/linear", + name: "linear", + principal: { id: "user-1", issuer: "idp", type: "user" }, + }, + ], + }); + + const result = await pendingAuthorizationProvider.create(new ContextContainer(), { + state, + } as HarnessSession); + + expect(result?.value).toEqual(getPendingAuthorization(state)); + }); +}); diff --git a/packages/eve/src/context/providers/pending-authorization.ts b/packages/eve/src/context/providers/pending-authorization.ts new file mode 100644 index 0000000000..ed26b91c72 --- /dev/null +++ b/packages/eve/src/context/providers/pending-authorization.ts @@ -0,0 +1,15 @@ +import type { FrameworkContextProvider } from "#context/provider.js"; +import { + getPendingAuthorization, + PendingAuthorizationStateKey, + type PendingAuthorizationState, +} from "#harness/authorization.js"; + +/** Rebuilds active authorization state from the durable session for each step. */ +export const pendingAuthorizationProvider: FrameworkContextProvider = { + key: PendingAuthorizationStateKey, + create(_ctx, session) { + const pending = getPendingAuthorization(session.state); + return pending === undefined ? undefined : { value: pending }; + }, +}; diff --git a/packages/eve/src/context/run-step.ts b/packages/eve/src/context/run-step.ts index efaeac6c98..967e0439cb 100644 --- a/packages/eve/src/context/run-step.ts +++ b/packages/eve/src/context/run-step.ts @@ -2,6 +2,7 @@ import type { HarnessSession, StepResult } from "#harness/types.js"; import { type ContextContainer, contextStorage } from "#context/container.js"; import type { FrameworkContextProvider } from "#context/provider.js"; import { connectionProvider } from "#context/providers/connection.js"; +import { pendingAuthorizationProvider } from "#context/providers/pending-authorization.js"; import { sandboxProvider } from "#context/providers/sandbox.js"; import { sessionProvider } from "#context/providers/session.js"; @@ -13,6 +14,7 @@ import { sessionProvider } from "#context/providers/session.js"; */ const frameworkProviders: readonly FrameworkContextProvider[] = [ sessionProvider, + pendingAuthorizationProvider, connectionProvider, sandboxProvider, ]; diff --git a/packages/eve/src/harness/authorization.test.ts b/packages/eve/src/harness/authorization.test.ts index 16b9d19dd8..01e4c5a16b 100644 --- a/packages/eve/src/harness/authorization.test.ts +++ b/packages/eve/src/harness/authorization.test.ts @@ -8,7 +8,11 @@ import { consumeAuthorizationResult, getPendingAuthorization, getHookUrl, + getReusableAuthorizationChallenge, + getSupersededAuthorizationChallenges, + isPendingAuthorizationChallenge, PendingAuthorizationResultKey, + PendingAuthorizationStateKey, setPendingAuthorization, } from "#harness/authorization.js"; import type { ConnectionPrincipal } from "#runtime/connections/types.js"; @@ -161,4 +165,63 @@ describe("pending authorization attempts", () => { getPendingAuthorization(clearPendingAuthorization(state, ["linear-2"]))?.challenges, ).toEqual([challenge("github", "github-1")]); }); + + it("reuses a still-valid challenge for the same principal", () => { + const ctx = new ContextContainer(); + const principal = { id: "user-a", issuer: "idp", type: "user" } as const; + const pending = challenge("linear", "linear-1", principal); + ctx.set(PendingAuthorizationStateKey, { challenges: [pending] }); + + expect( + contextStorage.run(ctx, () => getReusableAuthorizationChallenge("linear", principal)), + ).toBe(pending); + + const state = setPendingAuthorization(undefined, { challenges: [pending] }); + expect( + getPendingAuthorization(setPendingAuthorization(state, { challenges: [pending] })) + ?.challenges, + ).toEqual([pending]); + }); + + it("does not reuse an expired challenge", () => { + const ctx = new ContextContainer(); + const principal = { id: "user-a", issuer: "idp", type: "user" } as const; + ctx.set(PendingAuthorizationStateKey, { + challenges: [ + { + ...challenge("linear", "linear-1", principal), + challenge: { + expiresAt: "2000-01-01T00:00:00.000Z", + url: "https://idp.example/linear-1", + }, + }, + ], + }); + + expect( + contextStorage.run(ctx, () => getReusableAuthorizationChallenge("linear", principal)), + ).toBeUndefined(); + }); + + it("treats a reusable legacy challenge without an attempt ID as the same attempt", () => { + const ctx = new ContextContainer(); + const pending = { + challenge: { url: "https://idp.example/linear" }, + hookUrl: "https://agent.example/linear", + name: "linear", + principal: { type: "app" } as const, + }; + const state = setPendingAuthorization(undefined, { challenges: [pending] }); + ctx.set(PendingAuthorizationStateKey, { challenges: [pending] }); + + expect( + contextStorage.run(ctx, () => getReusableAuthorizationChallenge("linear", pending.principal)), + ).toBe(pending); + expect(getSupersededAuthorizationChallenges(state, [pending])).toEqual([]); + expect(isPendingAuthorizationChallenge(state, pending)).toBe(true); + + const replacement = { ...pending, hookUrl: "https://agent.example/linear-replacement" }; + expect(getSupersededAuthorizationChallenges(state, [replacement])).toEqual([pending]); + expect(isPendingAuthorizationChallenge(state, replacement)).toBe(false); + }); }); diff --git a/packages/eve/src/harness/authorization.ts b/packages/eve/src/harness/authorization.ts index 6adeb53da5..5c57e688d9 100644 --- a/packages/eve/src/harness/authorization.ts +++ b/packages/eve/src/harness/authorization.ts @@ -283,17 +283,24 @@ export interface PendingAuthorizationState { readonly challenges: readonly AuthorizationChallenge[]; } +/** Active challenges rebuilt from session state for each harness step. */ +export const PendingAuthorizationStateKey = new ContextKey( + "eve.pendingAuthorizationState", +); + export function setPendingAuthorization( sessionState: Record | undefined, value: PendingAuthorizationState, ): Record { const previous = getPendingAuthorization(sessionState)?.challenges ?? []; - const superseded = getSupersededAuthorizationChallenges(sessionState, value.challenges); + const replaced = previous.filter((candidate) => + value.challenges.some((replacement) => sameAuthorizationScope(candidate, replacement)), + ); return { ...sessionState, [PENDING_AUTHORIZATION_KEY]: { challenges: [ - ...previous.filter((challenge) => !superseded.includes(challenge)), + ...previous.filter((challenge) => !replaced.includes(challenge)), ...value.challenges, ], }, @@ -309,12 +316,42 @@ export function getSupersededAuthorizationChallenges( return previous.filter((candidate) => replacements.some( (replacement) => - candidate.name === replacement.name && - samePrincipal(candidate.principal, replacement.principal), + sameAuthorizationScope(candidate, replacement) && + !sameAuthorizationAttempt(candidate, replacement), ), ); } +/** Whether this exact attempt is already pending in durable session state. */ +export function isPendingAuthorizationChallenge( + sessionState: Record | undefined, + challenge: AuthorizationChallenge, +): boolean { + return ( + getPendingAuthorization(sessionState)?.challenges.some((pending) => + sameAuthorizationAttempt(pending, challenge), + ) ?? false + ); +} + +function sameAuthorizationScope( + left: AuthorizationChallenge, + right: AuthorizationChallenge, +): boolean { + return left.name === right.name && samePrincipal(left.principal, right.principal); +} + +function sameAuthorizationAttempt( + left: AuthorizationChallenge, + right: AuthorizationChallenge, +): boolean { + if (!sameAuthorizationScope(left, right)) return false; + if (left.attemptId !== undefined || right.attemptId !== undefined) { + return left.attemptId !== undefined && left.attemptId === right.attemptId; + } + return left.hookUrl === right.hookUrl; +} + function samePrincipal( left: ConnectionPrincipal | undefined, right: ConnectionPrincipal | undefined, @@ -364,6 +401,27 @@ export function getPendingAuthorization( return v as PendingAuthorizationState; } +/** Returns a still-valid pending challenge for the same scope and principal. */ +export function getReusableAuthorizationChallenge( + name: string, + principal: ConnectionPrincipal, +): AuthorizationChallenge | undefined { + return loadContext() + .get(PendingAuthorizationStateKey) + ?.challenges.find( + (challenge) => + challenge.name === name && + samePrincipal(challenge.principal, principal) && + !authorizationChallengeExpired(challenge.challenge), + ); +} + +function authorizationChallengeExpired(challenge: ConnectionAuthorizationChallenge): boolean { + if (challenge.expiresAt === undefined) return false; + const expiresAt = Date.parse(challenge.expiresAt); + return Number.isNaN(expiresAt) || expiresAt <= Date.now(); +} + export function hasPendingAuthorization( sessionState: Record | undefined, ): boolean { diff --git a/packages/eve/src/harness/tool-loop.test.ts b/packages/eve/src/harness/tool-loop.test.ts index 8963d99553..67717abcc1 100644 --- a/packages/eve/src/harness/tool-loop.test.ts +++ b/packages/eve/src/harness/tool-loop.test.ts @@ -46,6 +46,7 @@ import { getPendingAuthorization, modelFacingAuthorizationOutput, requestAuthorization, + setPendingAuthorization, } from "#harness/authorization.js"; import { getPendingInputRequestIds, @@ -5869,15 +5870,15 @@ describe("createToolLoopHarness", () => { }); describe("authorization signal detection", () => { - function createAuthSignals() { + function createAuthSignals(attemptId = "attempt-protected-action", userCode = "GFI-QLM") { const full = requestAuthorization([ { - attemptId: "attempt-protected-action", + attemptId, name: "protected_action", challenge: { url: "https://idp.example/auth", instructions: "Sign in to continue", - userCode: "GFI-QLM", + userCode, }, hookUrl: "https://app.example/callback", principal: { type: "app" }, @@ -6043,6 +6044,95 @@ describe("createToolLoopHarness", () => { const actionResults = events.filter((event) => event.type === "action.result"); expect(actionResults).toHaveLength(0); }); + + it("does not report or re-emit a reused pending authorization", async () => { + const { full, modelFacing } = createAuthSignals("attempt-existing", "OLD-CODE"); + + setupMockAgent({ + finishReason: "tool-calls", + fullStreamParts: [ + { + input: { action: "run" }, + toolCallId: "call-1", + toolName: "protected_action", + type: "tool-call", + }, + { + output: modelFacing, + toolCallId: "call-1", + toolName: "protected_action", + type: "tool-result", + }, + { finishReason: "tool-calls", type: "finish-step" }, + ], + response: { + messages: [ + { + content: [ + { + input: { action: "run" }, + toolCallId: "call-1", + toolName: "protected_action", + type: "tool-call", + }, + ], + role: "assistant", + }, + ], + }, + text: "", + toolCalls: [ + { + input: { action: "run" }, + toolCallId: "call-1", + toolName: "protected_action", + type: "tool-call", + }, + ], + toolResults: [ + { + output: modelFacing, + toolCallId: "call-1", + toolName: "protected_action", + type: "tool-result", + }, + ], + }); + + const { emit, events } = createEventCollector(); + const runStep = createToolLoopHarness( + createTestConfig("conversation", emit, { + tools: new Map([ + [ + "protected_action", + { + description: "Run a protected action", + execute: vi.fn(), + inputSchema: jsonSchema({ type: "object" }), + name: "protected_action", + }, + ], + ]), + }), + ); + const ctx = new ContextContainer(); + stashToolInterrupt(ctx, "call-1", full); + const session = createTestSession({ + state: setPendingAuthorization(undefined, { challenges: full.challenges }), + }); + + const result = await contextStorage.run(ctx, () => + runStep(session, { message: "run protected action" }), + ); + + expect( + events.filter( + (event) => event.type === "authorization.completed" && event.data.outcome === "failed", + ), + ).toHaveLength(0); + expect(events.filter((event) => event.type === "authorization.required")).toHaveLength(0); + expect(getPendingAuthorization(result.session.state)?.challenges).toEqual(full.challenges); + }); }); it("persists the SDK's accumulated approval-resume messages into session history", async () => { diff --git a/packages/eve/src/harness/tool-loop.ts b/packages/eve/src/harness/tool-loop.ts index 787d689792..89120d2c9c 100644 --- a/packages/eve/src/harness/tool-loop.ts +++ b/packages/eve/src/harness/tool-loop.ts @@ -186,6 +186,7 @@ import { normalizeProviderToolHistory } from "#harness/provider-tool-history.js" import { type AuthorizationSignal, getSupersededAuthorizationChallenges, + isPendingAuthorizationChallenge, isAuthorizationSignal, setPendingAuthorization, } from "#harness/authorization.js"; @@ -2789,6 +2790,7 @@ async function handleStepResult(input: { ); } for (const ch of challenges) { + if (isPendingAuthorizationChallenge(baseSession.state, ch)) continue; await emit( createAuthorizationRequiredEvent({ attemptId: ch.attemptId, diff --git a/packages/eve/src/runtime/connections/scoped-authorization.ts b/packages/eve/src/runtime/connections/scoped-authorization.ts index c8dad53e0a..91b6f6879a 100644 --- a/packages/eve/src/runtime/connections/scoped-authorization.ts +++ b/packages/eve/src/runtime/connections/scoped-authorization.ts @@ -16,6 +16,7 @@ import { type AuthorizationSignal, consumeAuthorizationResult, createAuthorizationAttempt, + getReusableAuthorizationChallenge, requestAuthorization, } from "#harness/authorization.js"; import type { JsonValue } from "#public/types/json.js"; @@ -173,11 +174,14 @@ export async function startScopedAuthorization( const { scope, authorization, connection } = input; if (!supportsInteractiveAuthorization(authorization)) return undefined; + const principal = resolveScopedPrincipal(input); + const pending = getReusableAuthorizationChallenge(scope, principal); + if (pending !== undefined) return requestAuthorization([pending]); + const attempt = createAuthorizationAttempt(scope); if (attempt === undefined) return undefined; const interactive = authorization as InteractiveAuthorizationDefinition; - const principal = resolveScopedPrincipal(input); const callbackUrl = resolveAuthorizationCallbackUrl({ authorization, callbackUrl: attempt.hookUrl, diff --git a/packages/eve/src/runtime/framework-tools/connection-search-dynamic.test.ts b/packages/eve/src/runtime/framework-tools/connection-search-dynamic.test.ts index 2ce85b3c6f..644149a89f 100644 --- a/packages/eve/src/runtime/framework-tools/connection-search-dynamic.test.ts +++ b/packages/eve/src/runtime/framework-tools/connection-search-dynamic.test.ts @@ -7,6 +7,7 @@ import { CallbackBaseUrlKey, isAuthorizationSignal, PendingAuthorizationResultKey, + PendingAuthorizationStateKey, } from "#harness/authorization.js"; import { ConnectionAuthorizationRequiredError } from "#public/connections/errors.js"; import type { ToolContext } from "#public/definitions/tool.js"; @@ -394,6 +395,98 @@ describe("connection_search", () => { ]); }); + it("reuses another connection's pending challenge after one authorization completes", async () => { + let notionCompletions = 0; + let dataHubStarts = 0; + const principal = { id: "user-1", issuer: "test-idp", type: "user" } as const; + const notion: ResolvedConnectionDefinition = { + ...connection("notion"), + authorization: { + completeAuthorization: async () => { + notionCompletions += 1; + return { token: "notion-token" }; + }, + getToken: async () => ({ token: "notion-token" }), + principalType: "user", + startAuthorization: async () => ({ + challenge: { url: "https://idp.example.com/notion" }, + }), + }, + }; + const dataHub: ResolvedConnectionDefinition = { + ...connection("data-hub"), + authorization: { + completeAuthorization: async () => ({ token: "data-hub-token" }), + getToken: async () => { + throw new ConnectionAuthorizationRequiredError("data-hub"); + }, + principalType: "user", + startAuthorization: async () => { + dataHubStarts += 1; + return { + challenge: { + url: "https://idp.example.com/data-hub/new", + userCode: "NEW-CODE", + }, + }; + }, + }, + }; + const pendingDataHubChallenge = { + attemptId: "attempt-data-hub", + challenge: { + url: "https://idp.example.com/data-hub/existing", + userCode: "OLD-CODE", + }, + hookUrl: "https://agent.example.com/eve/v1/connections/data-hub/callback/attempt-data-hub", + name: "data-hub", + principal, + }; + const connectionRegistry = registry({ + connections: [notion, dataHub], + loadTools: { + notion: async () => [], + "data-hub": async () => { + throw new ConnectionAuthorizationRequiredError("data-hub"); + }, + }, + }); + + const result = await executeConnectionSearch( + connectionRegistry, + { keywords: "search records" }, + (ctx) => { + ctx.set(SessionIdKey, "session-auth"); + ctx.set(CallbackBaseUrlKey, "https://agent.example.com"); + ctx.set(AuthKey, { + attributes: {}, + authenticator: "test-idp", + issuer: "test-idp", + principalId: "user-1", + principalType: "user", + }); + ctx.set(PendingAuthorizationResultKey, [ + { + attemptId: "attempt-notion", + callback: { method: "GET", params: {} }, + hookUrl: "https://agent.example.com/eve/v1/connections/notion/callback/attempt-notion", + name: "notion", + principal, + }, + ]); + ctx.set(PendingAuthorizationStateKey, { + challenges: [pendingDataHubChallenge], + }); + }, + ); + + expect(notionCompletions).toBe(1); + expect(dataHubStarts).toBe(0); + expect(isAuthorizationSignal(result)).toBe(true); + if (!isAuthorizationSignal(result)) throw new Error("expected authorization signal"); + expect(result.challenges).toEqual([pendingDataHubChallenge]); + }); + it("replays authorization from the step-scoped durable execute descriptor", async () => { const salesforce: ResolvedConnectionDefinition = { ...connection("salesforce"), diff --git a/packages/eve/src/runtime/framework-tools/connection-search-dynamic.ts b/packages/eve/src/runtime/framework-tools/connection-search-dynamic.ts index 20bdf4b733..b0c426d1a7 100644 --- a/packages/eve/src/runtime/framework-tools/connection-search-dynamic.ts +++ b/packages/eve/src/runtime/framework-tools/connection-search-dynamic.ts @@ -6,7 +6,6 @@ import { type AuthorizationChallenge, type AuthorizationSignal, consumeAuthorizationResult, - createAuthorizationAttempt, requestAuthorization, } from "#harness/authorization.js"; import { @@ -27,10 +26,7 @@ import { stampDurableDynamicToolCallbacks } from "#shared/durable-dynamic-tool-c import { writeCachedToken } from "#runtime/connections/authorization-tokens.js"; import { principalKey, resolveConnectionPrincipal } from "#runtime/connections/principal.js"; import { resolveConnectionAuthorization } from "#runtime/connections/resolve-authorization.js"; -import { - resolveAuthorizationCallbackUrl, - stampChallengeDisplayName, -} from "#runtime/connections/scoped-authorization.js"; +import { startScopedAuthorization } from "#runtime/connections/scoped-authorization.js"; import { type ConnectionRegistry, type ConnectionToolMetadata, @@ -235,40 +231,27 @@ async function executeConnectionSearch( const auth = await resolveInteractiveAuth(registry, conn.connectionName); if (auth) { - const attempt = createAuthorizationAttempt(conn.connectionName); - if (attempt) { - const principal = resolveConnectionPrincipal(conn.connectionName, auth); - const callbackUrl = resolveAuthorizationCallbackUrl({ + try { + const signal = await startScopedAuthorization({ authorization: auth, - callbackUrl: attempt.hookUrl, + connection: { url: conn.url ?? "" }, + scope: conn.connectionName, }); - try { - const { challenge, resume } = await auth.startAuthorization({ - callbackUrl, - connection: { url: conn.url ?? "" }, - principal, - }); - authChallenges.push({ - attemptId: attempt.attemptId, - name: conn.connectionName, - challenge: stampChallengeDisplayName(challenge, auth), - hookUrl: callbackUrl, - principal, - resume, - }); - } catch (startErr) { - const error = toError(startErr); - logger.warn("startAuthorization failed", { - connection: conn.connectionName, - error, - }); - failedConnections.push({ - connection: conn.connectionName, - description: conn.description, - error: `Failed to start authorization for "${conn.connectionName}": ${error.message}`, - }); - continue; + if (signal !== undefined) { + authChallenges.push(...signal.challenges); } + } catch (startErr) { + const error = toError(startErr); + logger.warn("startAuthorization failed", { + connection: conn.connectionName, + error, + }); + failedConnections.push({ + connection: conn.connectionName, + description: conn.description, + error: `Failed to start authorization for "${conn.connectionName}": ${error.message}`, + }); + continue; } } failedConnections.push({ @@ -464,28 +447,13 @@ async function executeDiscoveredConnectionTool( }); } - const attempt = createAuthorizationAttempt(connectionName); - if (!attempt) throw error; - const principal = resolveConnectionPrincipal(connectionName, interactiveAuth); - const callbackUrl = resolveAuthorizationCallbackUrl({ + const signal = await startScopedAuthorization({ authorization: interactiveAuth, - callbackUrl: attempt.hookUrl, - }); - const { challenge, resume } = await interactiveAuth.startAuthorization({ - callbackUrl, connection: { url: conn?.url ?? "" }, - principal, + scope: connectionName, }); - return requestAuthorization([ - { - attemptId: attempt.attemptId, - name: connectionName, - challenge: stampChallengeDisplayName(challenge, interactiveAuth), - hookUrl: callbackUrl, - principal, - resume, - }, - ]); + if (signal === undefined) throw error; + return signal; } }