From 2bd426d576ae39bd30ca40ae8683ee9469eb763d Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Mon, 26 Oct 2026 20:47:19 -0700 Subject: [PATCH 01/15] openai-agents: keep the caller's run context when resuming a RunState TemporalOpenAIRunner rebuilt a resumed RunState with RunState.fromString, which gives the resumed run a JSON copy of its context. Tools that ran after an approval resume wrote that copy, so the caller's context object (or one injected with RunState.fromStringWithContext) and the next turn missed their writes. The upstream Runner uses the state's own context. Rebuild the state with RunState.fromStringWithContext and the state's own RunContext. Runs that resumed before this change keep the JSON copy on replay, behind patched('openai-agents.keep-resume-context'). --- .../approval-resume-context-before-fix.json | 361 ++++++++++++++++++ .../src/__tests__/test-openai-agents.ts | 44 +++ .../src/__tests__/workflows/openai-agents.ts | 50 ++- .../openai-agents/src/workflow/runner.ts | 17 +- 4 files changed, 469 insertions(+), 3 deletions(-) create mode 100644 typescript/openai-agents/src/__tests__/histories/approval-resume-context-before-fix.json diff --git a/typescript/openai-agents/src/__tests__/histories/approval-resume-context-before-fix.json b/typescript/openai-agents/src/__tests__/histories/approval-resume-context-before-fix.json new file mode 100644 index 00000000..8eb18265 --- /dev/null +++ b/typescript/openai-agents/src/__tests__/histories/approval-resume-context-before-fix.json @@ -0,0 +1,361 @@ +{ + "events": [ + { + "eventId": "1", + "eventTime": "2026-10-27T03:56:21.632106554Z", + "eventType": "EVENT_TYPE_WORKFLOW_EXECUTION_STARTED", + "taskId": "1048587", + "workflowExecutionStartedEventAttributes": { + "workflowType": { + "name": "approvalResumeContextWorkflow" + }, + "taskQueue": { + "name": "approval-resume-context-fixture-1793073379926", + "kind": "TASK_QUEUE_KIND_NORMAL" + }, + "input": {}, + "workflowExecutionTimeout": "30s", + "workflowRunTimeout": "30s", + "workflowTaskTimeout": "10s", + "originalExecutionRunId": "01a17b80-8100-719c-a4ea-38bd76452b7d", + "identity": "323@3badd016993a", + "firstExecutionRunId": "01a17b80-8100-719c-a4ea-38bd76452b7d", + "attempt": 1, + "workflowExecutionExpirationTime": "2026-10-27T03:56:51.629Z", + "firstWorkflowTaskBackoff": "0s", + "header": {}, + "workflowId": "approval-resume-context-fixture-1793073379926" + } + }, + { + "eventId": "2", + "eventTime": "2026-10-27T03:56:21.634941679Z", + "eventType": "EVENT_TYPE_WORKFLOW_TASK_SCHEDULED", + "taskId": "1048588", + "workflowTaskScheduledEventAttributes": { + "taskQueue": { + "name": "approval-resume-context-fixture-1793073379926", + "kind": "TASK_QUEUE_KIND_NORMAL" + }, + "startToCloseTimeout": "10s", + "attempt": 1 + } + }, + { + "eventId": "3", + "eventTime": "2026-10-27T03:56:21.690752429Z", + "eventType": "EVENT_TYPE_WORKFLOW_TASK_STARTED", + "taskId": "1048594", + "workflowTaskStartedEventAttributes": { + "scheduledEventId": "2", + "identity": "323@3badd016993a", + "requestId": "7abc93b7-a197-4c54-91f9-a784cd8a7d94", + "historySizeBytes": "376", + "workerVersion": { + "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" + } + } + }, + { + "eventId": "4", + "eventTime": "2026-10-27T03:56:21.933503221Z", + "eventType": "EVENT_TYPE_WORKFLOW_TASK_COMPLETED", + "taskId": "1048599", + "workflowTaskCompletedEventAttributes": { + "scheduledEventId": "2", + "startedEventId": "3", + "identity": "323@3badd016993a", + "workerVersion": { + "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" + }, + "sdkMetadata": { + "coreUsedFlags": [ + 2, + 3, + 1 + ], + "sdkName": "temporal-typescript", + "sdkVersion": "1.24.0" + }, + "meteringMetadata": {} + } + }, + { + "eventId": "5", + "eventTime": "2026-10-27T03:56:21.935907638Z", + "eventType": "EVENT_TYPE_ACTIVITY_TASK_SCHEDULED", + "taskId": "1048600", + "activityTaskScheduledEventAttributes": { + "activityId": "1", + "activityType": { + "name": "invokeModelActivity" + }, + "taskQueue": { + "name": "approval-resume-context-fixture-1793073379926", + "kind": "TASK_QUEUE_KIND_NORMAL" + }, + "header": { + "fields": { + "__openai_span": { + "metadata": { + "encoding": "anNvbi9wbGFpbg==" + }, + "data": "eyJ0cmFjZU5hbWUiOiJBZ2VudCB3b3JrZmxvdyIsInNwYW5JZCI6InNwYW5fYTgyYTU2NGVjYTI0MzczMGJmNzRmOTM2IiwidHJhY2VJZCI6InRyYWNlX2M2N2I2ZTlkZGZhMWQ1YmViMzk0NmYwZDdiOWM5NTIyIn0=" + } + } + }, + "input": { + "payloads": [ + { + "metadata": { + "encoding": "anNvbi9wbGFpbg==" + }, + "data": "eyJtb2RlbE5hbWUiOiJncHQtNG8tbWluaSIsInJlcXVlc3QiOnsic3lzdGVtSW5zdHJ1Y3Rpb25zIjoiU2V0IHRoZSB2YWx1ZS4iLCJpbnB1dCI6W3sidHlwZSI6Im1lc3NhZ2UiLCJyb2xlIjoidXNlciIsImNvbnRlbnQiOiJnbyJ9XSwibW9kZWxTZXR0aW5ncyI6e30sInRvb2xzIjpbeyJ0eXBlIjoiZnVuY3Rpb24iLCJuYW1lIjoic2V0X2FmdGVyIiwiZGVzY3JpcHRpb24iOiJTZXQgYSB2YWx1ZSBhZnRlciBhcHByb3ZhbC4iLCJwYXJhbWV0ZXJzIjp7InR5cGUiOiJvYmplY3QiLCJwcm9wZXJ0aWVzIjp7InZhbHVlIjp7InR5cGUiOiJzdHJpbmcifX0sInJlcXVpcmVkIjpbInZhbHVlIl0sImFkZGl0aW9uYWxQcm9wZXJ0aWVzIjpmYWxzZX0sInN0cmljdCI6dHJ1ZSwiZGVmZXJMb2FkaW5nIjpmYWxzZX1dLCJ0b29sc0V4cGxpY2l0bHlQcm92aWRlZCI6dHJ1ZSwib3V0cHV0VHlwZSI6InRleHQiLCJoYW5kb2ZmcyI6W10sInRyYWNpbmciOnRydWUsIm92ZXJyaWRlUHJvbXB0TW9kZWwiOnRydWV9fQ==" + } + ] + }, + "scheduleToCloseTimeout": "30s", + "scheduleToStartTimeout": "30s", + "startToCloseTimeout": "30s", + "heartbeatTimeout": "0s", + "workflowTaskCompletedEventId": "4", + "retryPolicy": { + "initialInterval": "1s", + "backoffCoefficient": 2, + "maximumInterval": "100s" + }, + "useWorkflowBuildId": true + } + }, + { + "eventId": "6", + "eventTime": "2026-10-27T03:56:21.945662179Z", + "eventType": "EVENT_TYPE_ACTIVITY_TASK_STARTED", + "taskId": "1048603", + "activityTaskStartedEventAttributes": { + "scheduledEventId": "5", + "identity": "323@3badd016993a", + "requestId": "c744554c-0943-4b08-aa9e-50889b8f3a1a", + "attempt": 1, + "workerVersion": { + "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" + } + } + }, + { + "eventId": "7", + "eventTime": "2026-10-27T03:56:21.974155638Z", + "eventType": "EVENT_TYPE_ACTIVITY_TASK_COMPLETED", + "taskId": "1048604", + "activityTaskCompletedEventAttributes": { + "result": { + "payloads": [ + { + "metadata": { + "encoding": "anNvbi9wbGFpbg==" + }, + "data": "eyJ1c2FnZSI6eyJyZXF1ZXN0cyI6MSwiaW5wdXRUb2tlbnMiOjEwLCJvdXRwdXRUb2tlbnMiOjIwLCJ0b3RhbFRva2VucyI6MzAsImlucHV0VG9rZW5zRGV0YWlscyI6W10sIm91dHB1dFRva2Vuc0RldGFpbHMiOltdfSwib3V0cHV0IjpbeyJ0eXBlIjoiZnVuY3Rpb25fY2FsbCIsIm5hbWUiOiJzZXRfYWZ0ZXIiLCJhcmd1bWVudHMiOiJ7XCJ2YWx1ZVwiOlwiUy1hZnRlclwifSIsImNhbGxJZCI6ImNhbGxfZmFrZV9zZXRfYWZ0ZXIiLCJzdGF0dXMiOiJjb21wbGV0ZWQifV19" + } + ] + }, + "scheduledEventId": "5", + "startedEventId": "6", + "identity": "323@3badd016993a" + } + }, + { + "eventId": "8", + "eventTime": "2026-10-27T03:56:21.974741388Z", + "eventType": "EVENT_TYPE_WORKFLOW_TASK_SCHEDULED", + "taskId": "1048605", + "workflowTaskScheduledEventAttributes": { + "taskQueue": { + "name": "323@3badd016993a-d4b771c1d6f042329e11492aa03eb013", + "kind": "TASK_QUEUE_KIND_STICKY", + "normalName": "approval-resume-context-fixture-1793073379926" + }, + "startToCloseTimeout": "10s", + "attempt": 1 + } + }, + { + "eventId": "9", + "eventTime": "2026-10-27T03:56:21.979498763Z", + "eventType": "EVENT_TYPE_WORKFLOW_TASK_STARTED", + "taskId": "1048609", + "workflowTaskStartedEventAttributes": { + "scheduledEventId": "8", + "identity": "323@3badd016993a", + "requestId": "7d1cdb1f-283d-4f58-94b6-d5502aa5e1a7", + "historySizeBytes": "2271", + "workerVersion": { + "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" + } + } + }, + { + "eventId": "10", + "eventTime": "2026-10-27T03:56:22.017579346Z", + "eventType": "EVENT_TYPE_WORKFLOW_TASK_COMPLETED", + "taskId": "1048614", + "workflowTaskCompletedEventAttributes": { + "scheduledEventId": "8", + "startedEventId": "9", + "identity": "323@3badd016993a", + "workerVersion": { + "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" + }, + "sdkMetadata": {}, + "meteringMetadata": {} + } + }, + { + "eventId": "11", + "eventTime": "2026-10-27T03:56:22.017617013Z", + "eventType": "EVENT_TYPE_ACTIVITY_TASK_SCHEDULED", + "taskId": "1048615", + "activityTaskScheduledEventAttributes": { + "activityId": "2", + "activityType": { + "name": "invokeModelActivity" + }, + "taskQueue": { + "name": "approval-resume-context-fixture-1793073379926", + "kind": "TASK_QUEUE_KIND_NORMAL" + }, + "header": { + "fields": { + "__openai_span": { + "metadata": { + "encoding": "anNvbi9wbGFpbg==" + }, + "data": "eyJ0cmFjZU5hbWUiOiJBZ2VudCB3b3JrZmxvdyIsInNwYW5JZCI6InNwYW5fYzJiMDVkMjRlYTk5ZWNlOGEwNmZkMWE0IiwidHJhY2VJZCI6InRyYWNlX2MwMmM3ZTc1MThjZTY3OWY3YmUzZjk5NDUwN2FmN2E4In0=" + } + } + }, + "input": { + "payloads": [ + { + "metadata": { + "encoding": "anNvbi9wbGFpbg==" + }, + "data": "eyJtb2RlbE5hbWUiOiJncHQtNG8tbWluaSIsInJlcXVlc3QiOnsic3lzdGVtSW5zdHJ1Y3Rpb25zIjoiU2V0IHRoZSB2YWx1ZS4iLCJpbnB1dCI6W3sidHlwZSI6Im1lc3NhZ2UiLCJyb2xlIjoidXNlciIsImNvbnRlbnQiOiJnbyJ9LHsidHlwZSI6ImZ1bmN0aW9uX2NhbGwiLCJjYWxsSWQiOiJjYWxsX2Zha2Vfc2V0X2FmdGVyIiwibmFtZSI6InNldF9hZnRlciIsInN0YXR1cyI6ImNvbXBsZXRlZCIsImFyZ3VtZW50cyI6IntcInZhbHVlXCI6XCJTLWFmdGVyXCJ9In0seyJ0eXBlIjoiZnVuY3Rpb25fY2FsbF9yZXN1bHQiLCJuYW1lIjoic2V0X2FmdGVyIiwiY2FsbElkIjoiY2FsbF9mYWtlX3NldF9hZnRlciIsInN0YXR1cyI6ImNvbXBsZXRlZCIsIm91dHB1dCI6eyJ0eXBlIjoidGV4dCIsInRleHQiOiJvayJ9fV0sIm1vZGVsU2V0dGluZ3MiOnt9LCJ0b29scyI6W3sidHlwZSI6ImZ1bmN0aW9uIiwibmFtZSI6InNldF9hZnRlciIsImRlc2NyaXB0aW9uIjoiU2V0IGEgdmFsdWUgYWZ0ZXIgYXBwcm92YWwuIiwicGFyYW1ldGVycyI6eyJ0eXBlIjoib2JqZWN0IiwicHJvcGVydGllcyI6eyJ2YWx1ZSI6eyJ0eXBlIjoic3RyaW5nIn19LCJyZXF1aXJlZCI6WyJ2YWx1ZSJdLCJhZGRpdGlvbmFsUHJvcGVydGllcyI6ZmFsc2V9LCJzdHJpY3QiOnRydWUsImRlZmVyTG9hZGluZyI6ZmFsc2V9XSwidG9vbHNFeHBsaWNpdGx5UHJvdmlkZWQiOnRydWUsIm91dHB1dFR5cGUiOiJ0ZXh0IiwiaGFuZG9mZnMiOltdLCJ0cmFjaW5nIjp0cnVlLCJvdmVycmlkZVByb21wdE1vZGVsIjp0cnVlfX0=" + } + ] + }, + "scheduleToCloseTimeout": "30s", + "scheduleToStartTimeout": "30s", + "startToCloseTimeout": "30s", + "heartbeatTimeout": "0s", + "workflowTaskCompletedEventId": "10", + "retryPolicy": { + "initialInterval": "1s", + "backoffCoefficient": 2, + "maximumInterval": "100s" + }, + "useWorkflowBuildId": true + } + }, + { + "eventId": "12", + "eventTime": "2026-10-27T03:56:22.017644138Z", + "eventType": "EVENT_TYPE_ACTIVITY_TASK_STARTED", + "taskId": "1048617", + "activityTaskStartedEventAttributes": { + "scheduledEventId": "11", + "identity": "323@3badd016993a", + "requestId": "f6267073-cdc4-4ec4-9d43-c68c2db2ee4c", + "attempt": 1, + "workerVersion": { + "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" + } + } + }, + { + "eventId": "13", + "eventTime": "2026-10-27T03:56:22.026585721Z", + "eventType": "EVENT_TYPE_ACTIVITY_TASK_COMPLETED", + "taskId": "1048618", + "activityTaskCompletedEventAttributes": { + "result": { + "payloads": [ + { + "metadata": { + "encoding": "anNvbi9wbGFpbg==" + }, + "data": "eyJ1c2FnZSI6eyJyZXF1ZXN0cyI6MSwiaW5wdXRUb2tlbnMiOjEwLCJvdXRwdXRUb2tlbnMiOjQsInRvdGFsVG9rZW5zIjoxNCwiaW5wdXRUb2tlbnNEZXRhaWxzIjpbXSwib3V0cHV0VG9rZW5zRGV0YWlscyI6W119LCJvdXRwdXQiOlt7InR5cGUiOiJtZXNzYWdlIiwiaWQiOiJtc2dfZmFrZV8wMDEiLCJyb2xlIjoiYXNzaXN0YW50IiwiY29udGVudCI6W3sidHlwZSI6Im91dHB1dF90ZXh0IiwidGV4dCI6ImRvbmUifV0sInN0YXR1cyI6ImNvbXBsZXRlZCJ9XX0=" + } + ] + }, + "scheduledEventId": "11", + "startedEventId": "12", + "identity": "323@3badd016993a" + } + }, + { + "eventId": "14", + "eventTime": "2026-10-27T03:56:22.026598179Z", + "eventType": "EVENT_TYPE_WORKFLOW_TASK_SCHEDULED", + "taskId": "1048619", + "workflowTaskScheduledEventAttributes": { + "taskQueue": { + "name": "323@3badd016993a-d4b771c1d6f042329e11492aa03eb013", + "kind": "TASK_QUEUE_KIND_STICKY", + "normalName": "approval-resume-context-fixture-1793073379926" + }, + "startToCloseTimeout": "10s", + "attempt": 1 + } + }, + { + "eventId": "15", + "eventTime": "2026-10-27T03:56:22.031897596Z", + "eventType": "EVENT_TYPE_WORKFLOW_TASK_STARTED", + "taskId": "1048623", + "workflowTaskStartedEventAttributes": { + "scheduledEventId": "14", + "identity": "323@3badd016993a", + "requestId": "7f37a533-9655-47d3-af4a-57d47afebc52", + "historySizeBytes": "4393", + "workerVersion": { + "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" + } + } + }, + { + "eventId": "16", + "eventTime": "2026-10-27T03:56:22.055352096Z", + "eventType": "EVENT_TYPE_WORKFLOW_TASK_COMPLETED", + "taskId": "1048627", + "workflowTaskCompletedEventAttributes": { + "scheduledEventId": "14", + "startedEventId": "15", + "identity": "323@3badd016993a", + "workerVersion": { + "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" + }, + "sdkMetadata": {}, + "meteringMetadata": {} + } + }, + { + "eventId": "17", + "eventTime": "2026-10-27T03:56:22.055412554Z", + "eventType": "EVENT_TYPE_WORKFLOW_EXECUTION_COMPLETED", + "taskId": "1048628", + "workflowExecutionCompletedEventAttributes": { + "result": { + "payloads": [ + { + "metadata": { + "encoding": "anNvbi9wbGFpbg==" + }, + "data": "eyJyZXN1bWVkVXNlc0NhbGxlck9iamVjdCI6ZmFsc2V9" + } + ] + }, + "workflowTaskCompletedEventId": "16" + } + } + ] +} diff --git a/typescript/openai-agents/src/__tests__/test-openai-agents.ts b/typescript/openai-agents/src/__tests__/test-openai-agents.ts index cf660dee..ddd36eb8 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-agents.ts @@ -1,10 +1,13 @@ // Integration tests that need a real Worker. Pure-function tests live in // the sibling `*.test.ts` files. +import { readFileSync } from 'node:fs'; +import path from 'node:path'; import { setTracingDisabled } from '@openai/agents-core'; import type { MCPServer } from '@openai/agents-core'; import { WorkflowClient } from '@temporalio/client'; import { temporal } from '@temporalio/proto'; import { helpers } from '@temporalio/test-helpers'; +import { Worker } from '@temporalio/worker'; import { WorkflowStreamClient } from '@temporalio/workflow-streams/client'; import { OpenAIAgentsPlugin, @@ -13,6 +16,7 @@ import { StatelessMCPServerProvider, } from '..'; import { + approvalResumeContextWorkflow, localActivityAgentWorkflow, mcpFactoryArgWorkflow, summaryOverrideStringWorkflow, @@ -524,3 +528,43 @@ test('Streaming: useLocalActivity fails fast before scheduling an Activity', asy t.is(result, 'StreamingLocalActivityUnsupported'); }); }); + +test('Approval resume keeps the caller context object', async (t) => { + const { createWorker, executeWorkflow } = helpers(t); + const worker = await createWorker({ + plugins: [ + new OpenAIAgentsPlugin({ + modelProvider: new FakeModelProvider([ + toolCallResponse('set_after', { value: 'S-after' }), + textResponse('done'), + ]), + }), + ], + }); + + const result = await worker.runUntil( + executeWorkflow(approvalResumeContextWorkflow, { workflowExecutionTimeout: '30 seconds' }) + ); + + t.deepEqual(result, { callerAfter: 'S-after', resumedUsesCallerObject: true }); +}); + +test('Approval resume recorded before the context fix still replays', async (t) => { + // Recorded with the plugin before this change: the write missed the caller's object, so the + // Workflow did not sleep. + const history = JSON.parse( + readFileSync( + path.resolve(__dirname, '../../src/__tests__/histories/approval-resume-context-before-fix.json'), + 'utf8' + ) + ); + await t.notThrowsAsync( + Worker.runReplayHistory( + { + workflowBundle: t.context.workflowBundle, + plugins: [new OpenAIAgentsPlugin({ modelProvider: new FakeModelProvider([]) })], + }, + history + ) + ); +}); diff --git a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts index b739c0d1..fe630e09 100644 --- a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts @@ -1,5 +1,5 @@ -import { Agent } from '@openai/agents-core'; -import { ApplicationFailure } from '@temporalio/workflow'; +import { Agent, tool } from '@openai/agents-core'; +import { ApplicationFailure, sleep } from '@temporalio/workflow'; import { WorkflowStream } from '@temporalio/workflow-streams/workflow'; import { TemporalOpenAIRunner, statelessMcpServer, statefulMcpServer } from '../../workflow'; @@ -176,3 +176,49 @@ export async function statefulMcpSlowConnectHeartbeatWorkflow(): Promise await server.cleanup(); } } + +interface ResumeContext { + original: string; + after?: string; +} + +export interface ApprovalResumeContextResult { + callerAfter: string | undefined; + resumedUsesCallerObject: boolean; +} + +/** + * Approves a tool that writes the run context, resumes the same `RunState`, then reads the + * caller's context object. It sleeps only when the write reached that object, so a change in + * where the write lands also changes the Workflow's commands. + */ +export async function approvalResumeContextWorkflow(): Promise { + const setAfter = tool({ + name: 'set_after', + description: 'Set a value after approval.', + parameters: { + type: 'object' as const, + properties: { value: { type: 'string' } }, + required: ['value'] as const, + additionalProperties: false as const, + }, + needsApproval: true, + execute: async (args, runContext) => { + (runContext!.context as ResumeContext).after = (args as { value: string }).value; + return 'ok'; + }, + }); + const agent = new Agent({ + name: 'ResumeContextAgent', + instructions: 'Set the value.', + model: 'gpt-4o-mini', + tools: [setAfter], + }); + const runner = new TemporalOpenAIRunner(); + const ctx: ResumeContext = { original: 'orig' }; + const first = await runner.run(agent, 'go', { context: ctx }); + first.state.approve(first.interruptions[0]!); + const resumed = await runner.run(agent, first.state); + if (ctx.after !== undefined) await sleep(1); + return { callerAfter: ctx.after, resumedUsesCallerObject: resumed.runContext.context === ctx }; +} diff --git a/typescript/openai-agents/src/workflow/runner.ts b/typescript/openai-agents/src/workflow/runner.ts index 732cfedc..b1bd5c25 100644 --- a/typescript/openai-agents/src/workflow/runner.ts +++ b/typescript/openai-agents/src/workflow/runner.ts @@ -19,6 +19,7 @@ import { } from '@openai/agents-core'; import { SandboxAgent, type SandboxRunConfig } from '@openai/agents-core/sandbox'; import { ApplicationFailure } from '@temporalio/common'; +import { patched } from '@temporalio/workflow'; import { DEFAULT_MODEL_ACTIVITY_OPTIONS, STREAMING_LOCAL_ACTIVITY_UNSUPPORTED, @@ -35,6 +36,13 @@ import { getCurrentPluginConfig } from './plugin-config-store'; const agentAsToolSandboxConfig = Symbol.for('@temporalio/openai-agents/agentAsToolSandboxConfig'); +/** + * Runs that resumed a `RunState` before the resumed run kept the state's own context keep + * resuming with a JSON copy on replay: a tool that writes the caller's context object can + * change the commands that follow. + */ +const KEEP_RESUME_CONTEXT_PATCH = 'openai-agents.keep-resume-context'; + export interface TemporalRunOptions { /** Run context passed to agents and tools */ context?: TContext; @@ -282,7 +290,14 @@ export class TemporalOpenAIRunner { // agent (with ActivityBackedModel); setCurrentAgent only swaps the top-level ref, not nested ones. // It also drops upstream's live-session identity, which would otherwise resolve manifest // environment references on the Workflow thread rather than in the resume Activity. - const restored = (await RunState.fromString(converted, input.toString())) as RunState; + // Pass the state's own context, as the upstream Runner uses it, so resumed tools write the + // caller's object rather than a JSON copy. + const serialized = input.toString(); + const restored = ( + patched(KEEP_RESUME_CONTEXT_PATCH) + ? await RunState.fromStringWithContext(converted, serialized, input._context) + : await RunState.fromString(converted, serialized) + ) as RunState; // Suppress upstream Runner.run's withTrace(state._trace, ...) branch; we want the current Workflow's trace. restored.clearTrace(); preparedInput = restored; From f6c865b2a5d28b9687f358b7973f5caa972b8b3d Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Tue, 27 Oct 2026 16:54:40 -0700 Subject: [PATCH 02/15] openai-agents: keep the resume context without a version gate Fixes add no version gates for now, because the 1.x plugin releases may break backward compatibility. Remove the patched('openai-agents.keep-resume-context') check, so every RunState resume keeps the state's own context. Remove the replay test, its History and the test Workflow's sleep, which only tested the gate. Breaking: a run that resumed a RunState on an earlier version and then took a step that depends on the context object a resumed tool writes fails replay with [TMPRL1100]. Let such runs finish before upgrading, or use Worker Versioning. --- .../approval-resume-context-before-fix.json | 361 ------------------ .../src/__tests__/test-openai-agents.ts | 23 -- .../src/__tests__/workflows/openai-agents.ts | 6 +- .../openai-agents/src/workflow/runner.ts | 18 +- 4 files changed, 6 insertions(+), 402 deletions(-) delete mode 100644 typescript/openai-agents/src/__tests__/histories/approval-resume-context-before-fix.json diff --git a/typescript/openai-agents/src/__tests__/histories/approval-resume-context-before-fix.json b/typescript/openai-agents/src/__tests__/histories/approval-resume-context-before-fix.json deleted file mode 100644 index 8eb18265..00000000 --- a/typescript/openai-agents/src/__tests__/histories/approval-resume-context-before-fix.json +++ /dev/null @@ -1,361 +0,0 @@ -{ - "events": [ - { - "eventId": "1", - "eventTime": "2026-10-27T03:56:21.632106554Z", - "eventType": "EVENT_TYPE_WORKFLOW_EXECUTION_STARTED", - "taskId": "1048587", - "workflowExecutionStartedEventAttributes": { - "workflowType": { - "name": "approvalResumeContextWorkflow" - }, - "taskQueue": { - "name": "approval-resume-context-fixture-1793073379926", - "kind": "TASK_QUEUE_KIND_NORMAL" - }, - "input": {}, - "workflowExecutionTimeout": "30s", - "workflowRunTimeout": "30s", - "workflowTaskTimeout": "10s", - "originalExecutionRunId": "01a17b80-8100-719c-a4ea-38bd76452b7d", - "identity": "323@3badd016993a", - "firstExecutionRunId": "01a17b80-8100-719c-a4ea-38bd76452b7d", - "attempt": 1, - "workflowExecutionExpirationTime": "2026-10-27T03:56:51.629Z", - "firstWorkflowTaskBackoff": "0s", - "header": {}, - "workflowId": "approval-resume-context-fixture-1793073379926" - } - }, - { - "eventId": "2", - "eventTime": "2026-10-27T03:56:21.634941679Z", - "eventType": "EVENT_TYPE_WORKFLOW_TASK_SCHEDULED", - "taskId": "1048588", - "workflowTaskScheduledEventAttributes": { - "taskQueue": { - "name": "approval-resume-context-fixture-1793073379926", - "kind": "TASK_QUEUE_KIND_NORMAL" - }, - "startToCloseTimeout": "10s", - "attempt": 1 - } - }, - { - "eventId": "3", - "eventTime": "2026-10-27T03:56:21.690752429Z", - "eventType": "EVENT_TYPE_WORKFLOW_TASK_STARTED", - "taskId": "1048594", - "workflowTaskStartedEventAttributes": { - "scheduledEventId": "2", - "identity": "323@3badd016993a", - "requestId": "7abc93b7-a197-4c54-91f9-a784cd8a7d94", - "historySizeBytes": "376", - "workerVersion": { - "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" - } - } - }, - { - "eventId": "4", - "eventTime": "2026-10-27T03:56:21.933503221Z", - "eventType": "EVENT_TYPE_WORKFLOW_TASK_COMPLETED", - "taskId": "1048599", - "workflowTaskCompletedEventAttributes": { - "scheduledEventId": "2", - "startedEventId": "3", - "identity": "323@3badd016993a", - "workerVersion": { - "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" - }, - "sdkMetadata": { - "coreUsedFlags": [ - 2, - 3, - 1 - ], - "sdkName": "temporal-typescript", - "sdkVersion": "1.24.0" - }, - "meteringMetadata": {} - } - }, - { - "eventId": "5", - "eventTime": "2026-10-27T03:56:21.935907638Z", - "eventType": "EVENT_TYPE_ACTIVITY_TASK_SCHEDULED", - "taskId": "1048600", - "activityTaskScheduledEventAttributes": { - "activityId": "1", - "activityType": { - "name": "invokeModelActivity" - }, - "taskQueue": { - "name": "approval-resume-context-fixture-1793073379926", - "kind": "TASK_QUEUE_KIND_NORMAL" - }, - "header": { - "fields": { - "__openai_span": { - "metadata": { - "encoding": "anNvbi9wbGFpbg==" - }, - "data": "eyJ0cmFjZU5hbWUiOiJBZ2VudCB3b3JrZmxvdyIsInNwYW5JZCI6InNwYW5fYTgyYTU2NGVjYTI0MzczMGJmNzRmOTM2IiwidHJhY2VJZCI6InRyYWNlX2M2N2I2ZTlkZGZhMWQ1YmViMzk0NmYwZDdiOWM5NTIyIn0=" - } - } - }, - "input": { - "payloads": [ - { - "metadata": { - "encoding": "anNvbi9wbGFpbg==" - }, - "data": "eyJtb2RlbE5hbWUiOiJncHQtNG8tbWluaSIsInJlcXVlc3QiOnsic3lzdGVtSW5zdHJ1Y3Rpb25zIjoiU2V0IHRoZSB2YWx1ZS4iLCJpbnB1dCI6W3sidHlwZSI6Im1lc3NhZ2UiLCJyb2xlIjoidXNlciIsImNvbnRlbnQiOiJnbyJ9XSwibW9kZWxTZXR0aW5ncyI6e30sInRvb2xzIjpbeyJ0eXBlIjoiZnVuY3Rpb24iLCJuYW1lIjoic2V0X2FmdGVyIiwiZGVzY3JpcHRpb24iOiJTZXQgYSB2YWx1ZSBhZnRlciBhcHByb3ZhbC4iLCJwYXJhbWV0ZXJzIjp7InR5cGUiOiJvYmplY3QiLCJwcm9wZXJ0aWVzIjp7InZhbHVlIjp7InR5cGUiOiJzdHJpbmcifX0sInJlcXVpcmVkIjpbInZhbHVlIl0sImFkZGl0aW9uYWxQcm9wZXJ0aWVzIjpmYWxzZX0sInN0cmljdCI6dHJ1ZSwiZGVmZXJMb2FkaW5nIjpmYWxzZX1dLCJ0b29sc0V4cGxpY2l0bHlQcm92aWRlZCI6dHJ1ZSwib3V0cHV0VHlwZSI6InRleHQiLCJoYW5kb2ZmcyI6W10sInRyYWNpbmciOnRydWUsIm92ZXJyaWRlUHJvbXB0TW9kZWwiOnRydWV9fQ==" - } - ] - }, - "scheduleToCloseTimeout": "30s", - "scheduleToStartTimeout": "30s", - "startToCloseTimeout": "30s", - "heartbeatTimeout": "0s", - "workflowTaskCompletedEventId": "4", - "retryPolicy": { - "initialInterval": "1s", - "backoffCoefficient": 2, - "maximumInterval": "100s" - }, - "useWorkflowBuildId": true - } - }, - { - "eventId": "6", - "eventTime": "2026-10-27T03:56:21.945662179Z", - "eventType": "EVENT_TYPE_ACTIVITY_TASK_STARTED", - "taskId": "1048603", - "activityTaskStartedEventAttributes": { - "scheduledEventId": "5", - "identity": "323@3badd016993a", - "requestId": "c744554c-0943-4b08-aa9e-50889b8f3a1a", - "attempt": 1, - "workerVersion": { - "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" - } - } - }, - { - "eventId": "7", - "eventTime": "2026-10-27T03:56:21.974155638Z", - "eventType": "EVENT_TYPE_ACTIVITY_TASK_COMPLETED", - "taskId": "1048604", - "activityTaskCompletedEventAttributes": { - "result": { - "payloads": [ - { - "metadata": { - "encoding": "anNvbi9wbGFpbg==" - }, - "data": "eyJ1c2FnZSI6eyJyZXF1ZXN0cyI6MSwiaW5wdXRUb2tlbnMiOjEwLCJvdXRwdXRUb2tlbnMiOjIwLCJ0b3RhbFRva2VucyI6MzAsImlucHV0VG9rZW5zRGV0YWlscyI6W10sIm91dHB1dFRva2Vuc0RldGFpbHMiOltdfSwib3V0cHV0IjpbeyJ0eXBlIjoiZnVuY3Rpb25fY2FsbCIsIm5hbWUiOiJzZXRfYWZ0ZXIiLCJhcmd1bWVudHMiOiJ7XCJ2YWx1ZVwiOlwiUy1hZnRlclwifSIsImNhbGxJZCI6ImNhbGxfZmFrZV9zZXRfYWZ0ZXIiLCJzdGF0dXMiOiJjb21wbGV0ZWQifV19" - } - ] - }, - "scheduledEventId": "5", - "startedEventId": "6", - "identity": "323@3badd016993a" - } - }, - { - "eventId": "8", - "eventTime": "2026-10-27T03:56:21.974741388Z", - "eventType": "EVENT_TYPE_WORKFLOW_TASK_SCHEDULED", - "taskId": "1048605", - "workflowTaskScheduledEventAttributes": { - "taskQueue": { - "name": "323@3badd016993a-d4b771c1d6f042329e11492aa03eb013", - "kind": "TASK_QUEUE_KIND_STICKY", - "normalName": "approval-resume-context-fixture-1793073379926" - }, - "startToCloseTimeout": "10s", - "attempt": 1 - } - }, - { - "eventId": "9", - "eventTime": "2026-10-27T03:56:21.979498763Z", - "eventType": "EVENT_TYPE_WORKFLOW_TASK_STARTED", - "taskId": "1048609", - "workflowTaskStartedEventAttributes": { - "scheduledEventId": "8", - "identity": "323@3badd016993a", - "requestId": "7d1cdb1f-283d-4f58-94b6-d5502aa5e1a7", - "historySizeBytes": "2271", - "workerVersion": { - "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" - } - } - }, - { - "eventId": "10", - "eventTime": "2026-10-27T03:56:22.017579346Z", - "eventType": "EVENT_TYPE_WORKFLOW_TASK_COMPLETED", - "taskId": "1048614", - "workflowTaskCompletedEventAttributes": { - "scheduledEventId": "8", - "startedEventId": "9", - "identity": "323@3badd016993a", - "workerVersion": { - "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" - }, - "sdkMetadata": {}, - "meteringMetadata": {} - } - }, - { - "eventId": "11", - "eventTime": "2026-10-27T03:56:22.017617013Z", - "eventType": "EVENT_TYPE_ACTIVITY_TASK_SCHEDULED", - "taskId": "1048615", - "activityTaskScheduledEventAttributes": { - "activityId": "2", - "activityType": { - "name": "invokeModelActivity" - }, - "taskQueue": { - "name": "approval-resume-context-fixture-1793073379926", - "kind": "TASK_QUEUE_KIND_NORMAL" - }, - "header": { - "fields": { - "__openai_span": { - "metadata": { - "encoding": "anNvbi9wbGFpbg==" - }, - "data": "eyJ0cmFjZU5hbWUiOiJBZ2VudCB3b3JrZmxvdyIsInNwYW5JZCI6InNwYW5fYzJiMDVkMjRlYTk5ZWNlOGEwNmZkMWE0IiwidHJhY2VJZCI6InRyYWNlX2MwMmM3ZTc1MThjZTY3OWY3YmUzZjk5NDUwN2FmN2E4In0=" - } - } - }, - "input": { - "payloads": [ - { - "metadata": { - "encoding": "anNvbi9wbGFpbg==" - }, - "data": "eyJtb2RlbE5hbWUiOiJncHQtNG8tbWluaSIsInJlcXVlc3QiOnsic3lzdGVtSW5zdHJ1Y3Rpb25zIjoiU2V0IHRoZSB2YWx1ZS4iLCJpbnB1dCI6W3sidHlwZSI6Im1lc3NhZ2UiLCJyb2xlIjoidXNlciIsImNvbnRlbnQiOiJnbyJ9LHsidHlwZSI6ImZ1bmN0aW9uX2NhbGwiLCJjYWxsSWQiOiJjYWxsX2Zha2Vfc2V0X2FmdGVyIiwibmFtZSI6InNldF9hZnRlciIsInN0YXR1cyI6ImNvbXBsZXRlZCIsImFyZ3VtZW50cyI6IntcInZhbHVlXCI6XCJTLWFmdGVyXCJ9In0seyJ0eXBlIjoiZnVuY3Rpb25fY2FsbF9yZXN1bHQiLCJuYW1lIjoic2V0X2FmdGVyIiwiY2FsbElkIjoiY2FsbF9mYWtlX3NldF9hZnRlciIsInN0YXR1cyI6ImNvbXBsZXRlZCIsIm91dHB1dCI6eyJ0eXBlIjoidGV4dCIsInRleHQiOiJvayJ9fV0sIm1vZGVsU2V0dGluZ3MiOnt9LCJ0b29scyI6W3sidHlwZSI6ImZ1bmN0aW9uIiwibmFtZSI6InNldF9hZnRlciIsImRlc2NyaXB0aW9uIjoiU2V0IGEgdmFsdWUgYWZ0ZXIgYXBwcm92YWwuIiwicGFyYW1ldGVycyI6eyJ0eXBlIjoib2JqZWN0IiwicHJvcGVydGllcyI6eyJ2YWx1ZSI6eyJ0eXBlIjoic3RyaW5nIn19LCJyZXF1aXJlZCI6WyJ2YWx1ZSJdLCJhZGRpdGlvbmFsUHJvcGVydGllcyI6ZmFsc2V9LCJzdHJpY3QiOnRydWUsImRlZmVyTG9hZGluZyI6ZmFsc2V9XSwidG9vbHNFeHBsaWNpdGx5UHJvdmlkZWQiOnRydWUsIm91dHB1dFR5cGUiOiJ0ZXh0IiwiaGFuZG9mZnMiOltdLCJ0cmFjaW5nIjp0cnVlLCJvdmVycmlkZVByb21wdE1vZGVsIjp0cnVlfX0=" - } - ] - }, - "scheduleToCloseTimeout": "30s", - "scheduleToStartTimeout": "30s", - "startToCloseTimeout": "30s", - "heartbeatTimeout": "0s", - "workflowTaskCompletedEventId": "10", - "retryPolicy": { - "initialInterval": "1s", - "backoffCoefficient": 2, - "maximumInterval": "100s" - }, - "useWorkflowBuildId": true - } - }, - { - "eventId": "12", - "eventTime": "2026-10-27T03:56:22.017644138Z", - "eventType": "EVENT_TYPE_ACTIVITY_TASK_STARTED", - "taskId": "1048617", - "activityTaskStartedEventAttributes": { - "scheduledEventId": "11", - "identity": "323@3badd016993a", - "requestId": "f6267073-cdc4-4ec4-9d43-c68c2db2ee4c", - "attempt": 1, - "workerVersion": { - "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" - } - } - }, - { - "eventId": "13", - "eventTime": "2026-10-27T03:56:22.026585721Z", - "eventType": "EVENT_TYPE_ACTIVITY_TASK_COMPLETED", - "taskId": "1048618", - "activityTaskCompletedEventAttributes": { - "result": { - "payloads": [ - { - "metadata": { - "encoding": "anNvbi9wbGFpbg==" - }, - "data": "eyJ1c2FnZSI6eyJyZXF1ZXN0cyI6MSwiaW5wdXRUb2tlbnMiOjEwLCJvdXRwdXRUb2tlbnMiOjQsInRvdGFsVG9rZW5zIjoxNCwiaW5wdXRUb2tlbnNEZXRhaWxzIjpbXSwib3V0cHV0VG9rZW5zRGV0YWlscyI6W119LCJvdXRwdXQiOlt7InR5cGUiOiJtZXNzYWdlIiwiaWQiOiJtc2dfZmFrZV8wMDEiLCJyb2xlIjoiYXNzaXN0YW50IiwiY29udGVudCI6W3sidHlwZSI6Im91dHB1dF90ZXh0IiwidGV4dCI6ImRvbmUifV0sInN0YXR1cyI6ImNvbXBsZXRlZCJ9XX0=" - } - ] - }, - "scheduledEventId": "11", - "startedEventId": "12", - "identity": "323@3badd016993a" - } - }, - { - "eventId": "14", - "eventTime": "2026-10-27T03:56:22.026598179Z", - "eventType": "EVENT_TYPE_WORKFLOW_TASK_SCHEDULED", - "taskId": "1048619", - "workflowTaskScheduledEventAttributes": { - "taskQueue": { - "name": "323@3badd016993a-d4b771c1d6f042329e11492aa03eb013", - "kind": "TASK_QUEUE_KIND_STICKY", - "normalName": "approval-resume-context-fixture-1793073379926" - }, - "startToCloseTimeout": "10s", - "attempt": 1 - } - }, - { - "eventId": "15", - "eventTime": "2026-10-27T03:56:22.031897596Z", - "eventType": "EVENT_TYPE_WORKFLOW_TASK_STARTED", - "taskId": "1048623", - "workflowTaskStartedEventAttributes": { - "scheduledEventId": "14", - "identity": "323@3badd016993a", - "requestId": "7f37a533-9655-47d3-af4a-57d47afebc52", - "historySizeBytes": "4393", - "workerVersion": { - "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" - } - } - }, - { - "eventId": "16", - "eventTime": "2026-10-27T03:56:22.055352096Z", - "eventType": "EVENT_TYPE_WORKFLOW_TASK_COMPLETED", - "taskId": "1048627", - "workflowTaskCompletedEventAttributes": { - "scheduledEventId": "14", - "startedEventId": "15", - "identity": "323@3badd016993a", - "workerVersion": { - "buildId": "@temporalio/worker@1.24.0+ad9f638f8a2c67b50e68cd483576c5ab19f810dfa831f5e9f9f162ec36239584" - }, - "sdkMetadata": {}, - "meteringMetadata": {} - } - }, - { - "eventId": "17", - "eventTime": "2026-10-27T03:56:22.055412554Z", - "eventType": "EVENT_TYPE_WORKFLOW_EXECUTION_COMPLETED", - "taskId": "1048628", - "workflowExecutionCompletedEventAttributes": { - "result": { - "payloads": [ - { - "metadata": { - "encoding": "anNvbi9wbGFpbg==" - }, - "data": "eyJyZXN1bWVkVXNlc0NhbGxlck9iamVjdCI6ZmFsc2V9" - } - ] - }, - "workflowTaskCompletedEventId": "16" - } - } - ] -} diff --git a/typescript/openai-agents/src/__tests__/test-openai-agents.ts b/typescript/openai-agents/src/__tests__/test-openai-agents.ts index ddd36eb8..742af0aa 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-agents.ts @@ -1,13 +1,10 @@ // Integration tests that need a real Worker. Pure-function tests live in // the sibling `*.test.ts` files. -import { readFileSync } from 'node:fs'; -import path from 'node:path'; import { setTracingDisabled } from '@openai/agents-core'; import type { MCPServer } from '@openai/agents-core'; import { WorkflowClient } from '@temporalio/client'; import { temporal } from '@temporalio/proto'; import { helpers } from '@temporalio/test-helpers'; -import { Worker } from '@temporalio/worker'; import { WorkflowStreamClient } from '@temporalio/workflow-streams/client'; import { OpenAIAgentsPlugin, @@ -548,23 +545,3 @@ test('Approval resume keeps the caller context object', async (t) => { t.deepEqual(result, { callerAfter: 'S-after', resumedUsesCallerObject: true }); }); - -test('Approval resume recorded before the context fix still replays', async (t) => { - // Recorded with the plugin before this change: the write missed the caller's object, so the - // Workflow did not sleep. - const history = JSON.parse( - readFileSync( - path.resolve(__dirname, '../../src/__tests__/histories/approval-resume-context-before-fix.json'), - 'utf8' - ) - ); - await t.notThrowsAsync( - Worker.runReplayHistory( - { - workflowBundle: t.context.workflowBundle, - plugins: [new OpenAIAgentsPlugin({ modelProvider: new FakeModelProvider([]) })], - }, - history - ) - ); -}); diff --git a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts index fe630e09..590dd116 100644 --- a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts @@ -1,5 +1,5 @@ import { Agent, tool } from '@openai/agents-core'; -import { ApplicationFailure, sleep } from '@temporalio/workflow'; +import { ApplicationFailure } from '@temporalio/workflow'; import { WorkflowStream } from '@temporalio/workflow-streams/workflow'; import { TemporalOpenAIRunner, statelessMcpServer, statefulMcpServer } from '../../workflow'; @@ -189,8 +189,7 @@ export interface ApprovalResumeContextResult { /** * Approves a tool that writes the run context, resumes the same `RunState`, then reads the - * caller's context object. It sleeps only when the write reached that object, so a change in - * where the write lands also changes the Workflow's commands. + * caller's context object. */ export async function approvalResumeContextWorkflow(): Promise { const setAfter = tool({ @@ -219,6 +218,5 @@ export async function approvalResumeContextWorkflow(): Promise { /** Run context passed to agents and tools */ context?: TContext; @@ -292,12 +284,10 @@ export class TemporalOpenAIRunner { // environment references on the Workflow thread rather than in the resume Activity. // Pass the state's own context, as the upstream Runner uses it, so resumed tools write the // caller's object rather than a JSON copy. - const serialized = input.toString(); - const restored = ( - patched(KEEP_RESUME_CONTEXT_PATCH) - ? await RunState.fromStringWithContext(converted, serialized, input._context) - : await RunState.fromString(converted, serialized) - ) as RunState; + const restored = (await RunState.fromStringWithContext(converted, input.toString(), input._context)) as RunState< + TContext, + TAgent + >; // Suppress upstream Runner.run's withTrace(state._trace, ...) branch; we want the current Workflow's trace. restored.clearTrace(); preparedInput = restored; From e84f9eaf1c7a896997080b959275448f1759a088 Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Thu, 29 Oct 2026 23:11:13 -0700 Subject: [PATCH 03/15] Address review findings --- .../__tests__/stubs/openai-agents-fakes.ts | 8 ++- .../src/__tests__/test-openai-agents.ts | 15 +++- .../src/__tests__/workflows/openai-agents.ts | 69 ++++++++++++++++--- .../openai-agents/src/workflow/runner.ts | 39 +++++++++-- 4 files changed, 115 insertions(+), 16 deletions(-) diff --git a/typescript/openai-agents/src/__tests__/stubs/openai-agents-fakes.ts b/typescript/openai-agents/src/__tests__/stubs/openai-agents-fakes.ts index c72420d8..7e5e7989 100644 --- a/typescript/openai-agents/src/__tests__/stubs/openai-agents-fakes.ts +++ b/typescript/openai-agents/src/__tests__/stubs/openai-agents-fakes.ts @@ -140,13 +140,17 @@ export function textResponse(text: string): ModelResponse { return { output, usage: fakeUsage(text.length) }; } -export function toolCallResponse(toolName: string, args: Record): ModelResponse { +export function toolCallResponse( + toolName: string, + args: Record, + callId = `call_fake_${toolName}` +): ModelResponse { const output: AgentOutputItem[] = [ { type: 'function_call', name: toolName, arguments: JSON.stringify(args), - callId: `call_fake_${toolName}`, + callId, status: 'completed', }, ]; diff --git a/typescript/openai-agents/src/__tests__/test-openai-agents.ts b/typescript/openai-agents/src/__tests__/test-openai-agents.ts index 742af0aa..b8da01aa 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-agents.ts @@ -532,7 +532,9 @@ test('Approval resume keeps the caller context object', async (t) => { plugins: [ new OpenAIAgentsPlugin({ modelProvider: new FakeModelProvider([ - toolCallResponse('set_after', { value: 'S-after' }), + toolCallResponse('set_after', { value: 'S-1' }, 'call_set_after_1'), + toolCallResponse('set_after', { value: 'S-2' }, 'call_set_after_2'), + toolCallResponse('set_after', { value: 'S-after' }, 'call_set_after_3'), textResponse('done'), ]), }), @@ -543,5 +545,14 @@ test('Approval resume keeps the caller context object', async (t) => { executeWorkflow(approvalResumeContextWorkflow, { workflowExecutionTimeout: '30 seconds' }) ); - t.deepEqual(result, { callerAfter: 'S-after', resumedUsesCallerObject: true }); + // Each resume keys the approvals under a new converted agent graph; only the last one keeps them. + t.deepEqual(result, { + callerAfter: 'S-after', + resumedUsesCallerObject: true, + writes: ['S-1', 'S-2', 'S-after'], + approvalRounds: 3, + failedResumeThrew: true, + approvalOwners: 1, + approvedCallIds: 3, + }); }); diff --git a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts index 590dd116..0863bc85 100644 --- a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts @@ -180,16 +180,48 @@ export async function statefulMcpSlowConnectHeartbeatWorkflow(): Promise interface ResumeContext { original: string; after?: string; + writes: string[]; } export interface ApprovalResumeContextResult { callerAfter: string | undefined; resumedUsesCallerObject: boolean; + writes: string[]; + approvalRounds: number; + failedResumeThrew: boolean; + approvalOwners: number; + approvedCallIds: number; +} + +/** Serialized approval record, as the `RunContext` internal below returns it. */ +interface ApprovalRecord { + approved: boolean | string[]; +} + +/** Counts the agents that own function approvals in `context`, and the call IDs they approve. */ +function countApprovalOwners(context: unknown): { approvalOwners: number; approvedCallIds: number } { + let approvalOwners = 0; + // _toJSONForRunState is internal to @openai/agents-core; it looks up each owner in the map it's given. + const json = ( + context as { + _toJSONForRunState(keys: { get(agent: unknown): string }): { + functionApprovals?: Array<{ approvals: Record }>; + }; + } + )._toJSONForRunState({ get: () => `owner-${++approvalOwners}` }); + let approvedCallIds = 0; + for (const { approvals } of json.functionApprovals ?? []) { + for (const record of Object.values(approvals)) { + approvedCallIds += Array.isArray(record.approved) ? record.approved.length : 0; + } + } + return { approvalOwners, approvedCallIds }; } /** - * Approves a tool that writes the run context, resumes the same `RunState`, then reads the - * caller's context object. + * Approves a tool that writes the run context and resumes the same `RunState` until the run + * finishes, then reads the caller's context object. The first resume uses an agent the state + * doesn't match, which must fail without losing the approval. */ export async function approvalResumeContextWorkflow(): Promise { const setAfter = tool({ @@ -203,7 +235,9 @@ export async function approvalResumeContextWorkflow(): Promise { - (runContext!.context as ResumeContext).after = (args as { value: string }).value; + const context = runContext!.context as ResumeContext; + context.after = (args as { value: string }).value; + context.writes.push(context.after); return 'ok'; }, }); @@ -213,10 +247,29 @@ export async function approvalResumeContextWorkflow(): Promise({ name: 'OtherAgent', model: 'gpt-4o-mini', tools: [setAfter] }); const runner = new TemporalOpenAIRunner(); - const ctx: ResumeContext = { original: 'orig' }; - const first = await runner.run(agent, 'go', { context: ctx }); - first.state.approve(first.interruptions[0]!); - const resumed = await runner.run(agent, first.state); - return { callerAfter: ctx.after, resumedUsesCallerObject: resumed.runContext.context === ctx }; + const ctx: ResumeContext = { original: 'orig', writes: [] }; + let result = await runner.run(agent, 'go', { context: ctx }); + let approvalRounds = 0; + let failedResumeThrew = false; + while (result.interruptions.length > 0 && approvalRounds < 10) { + result.state.approve(result.interruptions[0]!); + if (approvalRounds++ === 0) { + try { + await runner.run(otherAgent, result.state); + } catch { + failedResumeThrew = true; + } + } + result = await runner.run(agent, result.state); + } + return { + callerAfter: ctx.after, + resumedUsesCallerObject: result.runContext.context === ctx, + writes: ctx.writes, + approvalRounds, + failedResumeThrew, + ...countApprovalOwners(result.runContext), + }; } diff --git a/typescript/openai-agents/src/workflow/runner.ts b/typescript/openai-agents/src/workflow/runner.ts index d4f7e6d2..69743755 100644 --- a/typescript/openai-agents/src/workflow/runner.ts +++ b/typescript/openai-agents/src/workflow/runner.ts @@ -4,6 +4,7 @@ import { Handoff, MemorySession, Runner, + RunContext, RunState, type AgentOutputType, type CallModelInputFilter, @@ -35,6 +36,39 @@ import { getCurrentPluginConfig } from './plugin-config-store'; const agentAsToolSandboxConfig = Symbol.for('@temporalio/openai-agents/agentAsToolSandboxConfig'); +/** `RunContext` approval methods that `@openai/agents-core` marks internal and omits from its types. */ +interface RunContextApprovalInternals { + _rebuildFunctionApprovals(approvals: unknown[], agentsByIdentity: Map>): void; + _mergeApprovalState(source: RunContext): void; +} + +/** + * Rebuilds `state` over `converted` while keeping the state's own `RunContext`. + * + * Function-tool approvals are keyed by agent object, and the merge keys the state's approvals + * under the new converted agents. The old owners are dropped first, or every resume would keep + * the previous converted graph with a full copy of its approvals. The serialized state carries + * every approval owned within its agent graph, so the approvals of the resumed run are kept. + */ +async function restoreWithOwnContext( + converted: Agent, + state: RunState +): Promise> { + const serialized = state.toString(); + const context = state._context as RunContext & RunContextApprovalInternals; + const previous = new RunContext() as RunContext & RunContextApprovalInternals; + previous._mergeApprovalState(context); + context._rebuildFunctionApprovals([], new Map()); + try { + return await RunState.fromStringWithContext(converted, serialized, context); + } catch (error) { + // Leave the caller's context as it was, so a later resume of the same state keeps its approvals. + context._rebuildFunctionApprovals([], new Map()); + context._mergeApprovalState(previous); + throw error; + } +} + export interface TemporalRunOptions { /** Run context passed to agents and tools */ context?: TContext; @@ -284,10 +318,7 @@ export class TemporalOpenAIRunner { // environment references on the Workflow thread rather than in the resume Activity. // Pass the state's own context, as the upstream Runner uses it, so resumed tools write the // caller's object rather than a JSON copy. - const restored = (await RunState.fromStringWithContext(converted, input.toString(), input._context)) as RunState< - TContext, - TAgent - >; + const restored = (await restoreWithOwnContext(converted, input)) as RunState; // Suppress upstream Runner.run's withTrace(state._trace, ...) branch; we want the current Workflow's trace. restored.clearTrace(); preparedInput = restored; From a7dbfc99498bcbad3baf8174a1554212c82d8d6e Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Fri, 30 Oct 2026 01:06:53 -0700 Subject: [PATCH 04/15] Address review findings --- .../src/__tests__/test-openai-agents.ts | 7 +++ .../src/__tests__/workflows/openai-agents.ts | 26 ++++++++-- .../openai-agents/src/workflow/runner.ts | 51 ++++++++++++++----- 3 files changed, 69 insertions(+), 15 deletions(-) diff --git a/typescript/openai-agents/src/__tests__/test-openai-agents.ts b/typescript/openai-agents/src/__tests__/test-openai-agents.ts index b8da01aa..2270d297 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-agents.ts @@ -536,6 +536,10 @@ test('Approval resume keeps the caller context object', async (t) => { toolCallResponse('set_after', { value: 'S-2' }, 'call_set_after_2'), toolCallResponse('set_after', { value: 'S-after' }, 'call_set_after_3'), textResponse('done'), + toolCallResponse('set_after', { value: 'A' }, 'call_shared_a'), + toolCallResponse('set_after', { value: 'B' }, 'call_shared_b'), + textResponse('done A'), + textResponse('done B'), ]), }), ], @@ -546,6 +550,7 @@ test('Approval resume keeps the caller context object', async (t) => { ); // Each resume keys the approvals under a new converted agent graph; only the last one keeps them. + // Resuming state A drops only A's previous graph, so state B still runs its approved call. t.deepEqual(result, { callerAfter: 'S-after', resumedUsesCallerObject: true, @@ -554,5 +559,7 @@ test('Approval resume keeps the caller context object', async (t) => { failedResumeThrew: true, approvalOwners: 1, approvedCallIds: 3, + sharedContextWrites: ['A', 'B'], + sharedContextInterruptions: 0, }); }); diff --git a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts index 0863bc85..a907b6b0 100644 --- a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts @@ -1,4 +1,4 @@ -import { Agent, tool } from '@openai/agents-core'; +import { Agent, RunState, tool } from '@openai/agents-core'; import { ApplicationFailure } from '@temporalio/workflow'; import { WorkflowStream } from '@temporalio/workflow-streams/workflow'; import { TemporalOpenAIRunner, statelessMcpServer, statefulMcpServer } from '../../workflow'; @@ -191,6 +191,8 @@ export interface ApprovalResumeContextResult { failedResumeThrew: boolean; approvalOwners: number; approvedCallIds: number; + sharedContextWrites: string[]; + sharedContextInterruptions: number; } /** Serialized approval record, as the `RunContext` internal below returns it. */ @@ -221,7 +223,8 @@ function countApprovalOwners(context: unknown): { approvalOwners: number; approv /** * Approves a tool that writes the run context and resumes the same `RunState` until the run * finishes, then reads the caller's context object. The first resume uses an agent the state - * doesn't match, which must fail without losing the approval. + * doesn't match, which must fail without losing the approval. Then two paused states share one + * `RunContext`, both are approved, and resuming the first must keep the second's approval. */ export async function approvalResumeContextWorkflow(): Promise { const setAfter = tool({ @@ -264,12 +267,29 @@ export async function approvalResumeContextWorkflow(): Promise({ name: 'SharedAgentA', model: 'gpt-4o-mini', tools: [setAfter] }); + const sharedAgentB = new Agent({ name: 'SharedAgentB', model: 'gpt-4o-mini', tools: [setAfter] }); + const sharedCtx: ResumeContext = { original: 'shared', writes: [] }; + const pausedA = await runner.run(sharedAgentA, 'go', { context: sharedCtx }); + const pausedB = await runner.run(sharedAgentB, 'go', { context: sharedCtx }); + // State A keeps the converted graph its run produced; state B is rehydrated onto A's context. + const stateA = pausedA.state; + const stateB = await RunState.fromStringWithContext(sharedAgentB, pausedB.state.toString(), stateA._context); + stateA.approve(stateA.getInterruptions()[0]!); + stateB.approve(stateB.getInterruptions()[0]!); + const resumedA = await runner.run(sharedAgentA, stateA); + const resumedB = await runner.run(sharedAgentB, stateB); + return { callerAfter: ctx.after, resumedUsesCallerObject: result.runContext.context === ctx, writes: ctx.writes, approvalRounds, failedResumeThrew, - ...countApprovalOwners(result.runContext), + ...owners, + sharedContextWrites: sharedCtx.writes, + sharedContextInterruptions: resumedA.interruptions.length + resumedB.interruptions.length, }; } diff --git a/typescript/openai-agents/src/workflow/runner.ts b/typescript/openai-agents/src/workflow/runner.ts index 69743755..c7338b45 100644 --- a/typescript/openai-agents/src/workflow/runner.ts +++ b/typescript/openai-agents/src/workflow/runner.ts @@ -4,8 +4,8 @@ import { Handoff, MemorySession, Runner, - RunContext, RunState, + type RunContext, type AgentOutputType, type CallModelInputFilter, type HandoffInputData, @@ -36,19 +36,33 @@ import { getCurrentPluginConfig } from './plugin-config-store'; const agentAsToolSandboxConfig = Symbol.for('@temporalio/openai-agents/agentAsToolSandboxConfig'); +/** Each converted agent, mapped to every agent its `convertAgent` call produced. */ +const convertedGraphs = new WeakMap, ReadonlySet>>(); + +interface SerializedFunctionApprovals { + agentIdentity: string; + approvals: Record; +} + /** `RunContext` approval methods that `@openai/agents-core` marks internal and omits from its types. */ interface RunContextApprovalInternals { - _rebuildFunctionApprovals(approvals: unknown[], agentsByIdentity: Map>): void; - _mergeApprovalState(source: RunContext): void; + _toJSONForRunState(agentIdentityKeys: { get(agent: Agent): string | undefined }): { + functionApprovals?: SerializedFunctionApprovals[]; + }; + _rebuildFunctionApprovals( + approvals: SerializedFunctionApprovals[], + agentsByIdentity: Map> + ): void; } /** * Rebuilds `state` over `converted` while keeping the state's own `RunContext`. * * Function-tool approvals are keyed by agent object, and the merge keys the state's approvals - * under the new converted agents. The old owners are dropped first, or every resume would keep - * the previous converted graph with a full copy of its approvals. The serialized state carries - * every approval owned within its agent graph, so the approvals of the resumed run are kept. + * under the new converted agents. The approvals owned by the state's previous converted graph + * are dropped first, or every resume would keep that graph with a full copy of its approvals. + * The serialized state carries them, so the resumed run keeps them. Approvals owned by other + * agents, such as those of another state sharing this context, stay where they are. */ async function restoreWithOwnContext( converted: Agent, @@ -56,15 +70,25 @@ async function restoreWithOwnContext( ): Promise> { const serialized = state.toString(); const context = state._context as RunContext & RunContextApprovalInternals; - const previous = new RunContext() as RunContext & RunContextApprovalInternals; - previous._mergeApprovalState(context); - context._rebuildFunctionApprovals([], new Map()); + const superseded = convertedGraphs.get(state._currentAgent as Agent) ?? new Set(); + // The internal serializer asks for each owner's identity, which lists every owner. + const owners = new Map>(); + const approvals = + context._toJSONForRunState({ + get: (agent) => { + const identity = String(owners.size); + owners.set(identity, agent); + return identity; + }, + }).functionApprovals ?? []; + const kept = approvals.filter(({ agentIdentity }) => !superseded.has(owners.get(agentIdentity)!)); + // Copy the kept records, so the merge below can't change those the restore puts back. + context._rebuildFunctionApprovals(JSON.parse(JSON.stringify(kept)), owners); try { return await RunState.fromStringWithContext(converted, serialized, context); } catch (error) { // Leave the caller's context as it was, so a later resume of the same state keeps its approvals. - context._rebuildFunctionApprovals([], new Map()); - context._mergeApprovalState(previous); + context._rebuildFunctionApprovals(approvals, owners); throw error; } } @@ -305,7 +329,10 @@ export class TemporalOpenAIRunner { const { model: modelOverride, ...runnerConfigOverrides } = options?.runConfig ?? {}; - const converted = convertAgent(agent, this.modelParams, undefined, modelOverride); + const convertedBySource = new Map, Agent>(); + const converted = convertAgent(agent, this.modelParams, convertedBySource, modelOverride); + const convertedGraph = new Set(convertedBySource.values()); + for (const convertedAgent of convertedGraph) convertedGraphs.set(convertedAgent, convertedGraph); if (options?.runConfig?.sandbox !== undefined) { propagateAgentToolSandboxConfig(converted, options.runConfig.sandbox); } From 005219d898e2b1c1fff747e90365762bec09dbb4 Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Fri, 30 Oct 2026 09:08:47 -0700 Subject: [PATCH 05/15] Address review findings --- .../src/__tests__/test-openai-agents.ts | 21 ++- .../src/__tests__/workflows/openai-agents.ts | 114 ++++++++++++-- .../openai-agents/src/workflow/runner.ts | 141 +++++++++++------- 3 files changed, 210 insertions(+), 66 deletions(-) diff --git a/typescript/openai-agents/src/__tests__/test-openai-agents.ts b/typescript/openai-agents/src/__tests__/test-openai-agents.ts index 2270d297..50f33305 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-agents.ts @@ -538,8 +538,16 @@ test('Approval resume keeps the caller context object', async (t) => { textResponse('done'), toolCallResponse('set_after', { value: 'A' }, 'call_shared_a'), toolCallResponse('set_after', { value: 'B' }, 'call_shared_b'), + toolCallResponse('set_after', { value: 'C' }, 'call_shared_c'), textResponse('done A'), textResponse('done B'), + textResponse('done C'), + toolCallResponse('flaky', { value: 'R' }, 'call_flaky'), + textResponse('done R'), + toolCallResponse('gated', { value: 'D' }, 'call_gated_d'), + toolCallResponse('gated', { value: 'E1' }, 'call_gated_e1'), + toolCallResponse('gated', { value: 'E2' }, 'call_gated_e2'), + textResponse('done E'), ]), }), ], @@ -549,8 +557,8 @@ test('Approval resume keeps the caller context object', async (t) => { executeWorkflow(approvalResumeContextWorkflow, { workflowExecutionTimeout: '30 seconds' }) ); - // Each resume keys the approvals under a new converted agent graph; only the last one keeps them. - // Resuming state A drops only A's previous graph, so state B still runs its approved call. + // Each resume reuses the converted graph its state runs on, so the approvals keep one owner, and + // no resume drops approvals that another state on the context, or a retry of the same state, needs. t.deepEqual(result, { callerAfter: 'S-after', resumedUsesCallerObject: true, @@ -559,7 +567,14 @@ test('Approval resume keeps the caller context object', async (t) => { failedResumeThrew: true, approvalOwners: 1, approvedCallIds: 3, - sharedContextWrites: ['A', 'B'], + sharedContextWrites: ['A', 'B', 'C'], sharedContextInterruptions: 0, + retryThrew: true, + retryAttempts: 2, + retryWrites: ['R'], + retryInterruptions: 0, + concurrentFailedResumeThrew: true, + concurrentWrites: ['E1', 'E2'], + concurrentInterruptions: 0, }); }); diff --git a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts index a907b6b0..206afcbb 100644 --- a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts @@ -1,5 +1,5 @@ import { Agent, RunState, tool } from '@openai/agents-core'; -import { ApplicationFailure } from '@temporalio/workflow'; +import { ApplicationFailure, condition } from '@temporalio/workflow'; import { WorkflowStream } from '@temporalio/workflow-streams/workflow'; import { TemporalOpenAIRunner, statelessMcpServer, statefulMcpServer } from '../../workflow'; @@ -193,6 +193,13 @@ export interface ApprovalResumeContextResult { approvedCallIds: number; sharedContextWrites: string[]; sharedContextInterruptions: number; + retryThrew: boolean; + retryAttempts: number; + retryWrites: string[]; + retryInterruptions: number; + concurrentFailedResumeThrew: boolean; + concurrentWrites: string[]; + concurrentInterruptions: number; } /** Serialized approval record, as the `RunContext` internal below returns it. */ @@ -223,8 +230,10 @@ function countApprovalOwners(context: unknown): { approvalOwners: number; approv /** * Approves a tool that writes the run context and resumes the same `RunState` until the run * finishes, then reads the caller's context object. The first resume uses an agent the state - * doesn't match, which must fail without losing the approval. Then two paused states share one - * `RunContext`, both are approved, and resuming the first must keep the second's approval. + * doesn't match, which must fail without losing the approval. Then three paused states share one + * `RunContext`, all are approved, and resuming one must keep the others' approvals. Last, an approved + * tool throws once, and resuming the same state again must run it without asking again. Then a + * resume fails while another state on its context advances and is approved again. */ export async function approvalResumeContextWorkflow(): Promise { const setAfter = tool({ @@ -274,13 +283,93 @@ export async function approvalResumeContextWorkflow(): Promise { + const context = runContext!.context as ResumeContext & { attempts: number }; + if (context.attempts++ === 0) throw new Error('flaky failed'); + context.writes.push((args as { value: string }).value); + return 'ok'; + }, + }); + const retryAgent = new Agent({ name: 'RetryAgent', model: 'gpt-4o-mini', tools: [flaky] }); + const retryCtx = { original: 'retry', writes: [] as string[], attempts: 0 }; + const pausedRetry = await runner.run(retryAgent, 'go', { context: retryCtx }); + pausedRetry.state.approve(pausedRetry.interruptions[0]!); + let retryThrew = false; + try { + await runner.run(retryAgent, pausedRetry.state); + } catch { + retryThrew = true; + } + const retried = await runner.run(retryAgent, pausedRetry.state); + + // State D's resume waits in a tool's isEnabled check, then throws. Meanwhile state E, which shares + // D's context, resumes, interrupts on its next call, and has that call approved. + const gate = { holdD: false, eApproved: false }; + const gated = tool({ + name: 'gated', + description: 'Record a value after approval.', + parameters: { + type: 'object' as const, + properties: { value: { type: 'string' } }, + required: ['value'] as const, + additionalProperties: false as const, + }, + needsApproval: true, + isEnabled: async ({ agent }) => { + if (gate.holdD && agent.name === 'ConcurrentAgentD') { + await condition(() => gate.eApproved); + throw new Error('isEnabled failed'); + } + return true; + }, + execute: async (args, runContext) => { + (runContext!.context as ResumeContext).writes.push((args as { value: string }).value); + return 'ok'; + }, + }); + const agentD = new Agent({ name: 'ConcurrentAgentD', model: 'gpt-4o-mini', tools: [gated] }); + const agentE = new Agent({ name: 'ConcurrentAgentE', model: 'gpt-4o-mini', tools: [gated] }); + const concurrentCtx: ResumeContext = { original: 'concurrent', writes: [] }; + const pausedD = await runner.run(agentD, 'go', { context: concurrentCtx }); + const pausedE = await runner.run(agentE, 'go', { context: concurrentCtx }); + const stateD = pausedD.state; + const stateE = await RunState.fromStringWithContext(agentE, pausedE.state.toString(), stateD._context); + stateE.approve(stateE.getInterruptions()[0]!); + gate.holdD = true; + const failedD = runner.run(agentD, stateD).then( + () => false, + () => true + ); + const pausedAgainE = await runner.run(agentE, stateE); + pausedAgainE.state.approve(pausedAgainE.interruptions[0]!); + gate.eApproved = true; + const concurrentFailedResumeThrew = await failedD; + gate.holdD = false; + const finishedE = await runner.run(agentE, pausedAgainE.state); return { callerAfter: ctx.after, @@ -290,6 +379,13 @@ export async function approvalResumeContextWorkflow(): Promise count + interruptions.length, 0), + retryThrew, + retryAttempts: retryCtx.attempts, + retryWrites: retryCtx.writes, + retryInterruptions: retried.interruptions.length, + concurrentFailedResumeThrew, + concurrentWrites: concurrentCtx.writes, + concurrentInterruptions: finishedE.interruptions.length, }; } diff --git a/typescript/openai-agents/src/workflow/runner.ts b/typescript/openai-agents/src/workflow/runner.ts index c7338b45..29b4be1e 100644 --- a/typescript/openai-agents/src/workflow/runner.ts +++ b/typescript/openai-agents/src/workflow/runner.ts @@ -5,7 +5,6 @@ import { MemorySession, Runner, RunState, - type RunContext, type AgentOutputType, type CallModelInputFilter, type HandoffInputData, @@ -36,61 +35,94 @@ import { getCurrentPluginConfig } from './plugin-config-store'; const agentAsToolSandboxConfig = Symbol.for('@temporalio/openai-agents/agentAsToolSandboxConfig'); -/** Each converted agent, mapped to every agent its `convertAgent` call produced. */ -const convertedGraphs = new WeakMap, ReadonlySet>>(); +/** A `convertAgent` call, with the inputs that decide whether a later run would convert the same graph. */ +interface Conversion { + source: Agent; + converted: Agent; + modelParams: ModelActivityOptions; + modelOverride: string | undefined; + sandbox: SandboxRunConfig | undefined; + /** Each source agent in the graph, with its fields when it was converted. */ + sourceFields: Map, [string, unknown][]>; +} + +/** Each converted agent, mapped to the conversion that produced it. */ +const conversions = new WeakMap, Conversion>(); + +function agentFields(agent: Agent): [string, unknown][] { + return Object.entries(agent).map(([key, value]) => [key, Array.isArray(value) ? [...value] : value]); +} -interface SerializedFunctionApprovals { - agentIdentity: string; - approvals: Record; +function isPlainObject(value: unknown): value is Record { + if (typeof value !== 'object' || value === null) return false; + const prototype = Object.getPrototypeOf(value); + return prototype === Object.prototype || prototype === null; +} + +/** Compares arrays and plain objects by their members, and any other value by identity. */ +function sameValue(a: unknown, b: unknown): boolean { + if (Object.is(a, b)) return true; + if (Array.isArray(a) && Array.isArray(b)) { + return a.length === b.length && a.every((value, index) => sameValue(value, b[index])); + } + if (isPlainObject(a) && isPlainObject(b)) { + const keys = Object.keys(a); + return keys.length === Object.keys(b).length && keys.every((key) => key in b && sameValue(a[key], b[key])); + } + return false; } -/** `RunContext` approval methods that `@openai/agents-core` marks internal and omits from its types. */ -interface RunContextApprovalInternals { - _toJSONForRunState(agentIdentityKeys: { get(agent: Agent): string | undefined }): { - functionApprovals?: SerializedFunctionApprovals[]; +function convertAgentGraph( + agent: Agent, + modelParams: ModelActivityOptions, + modelOverride: string | undefined, + sandbox: SandboxRunConfig | undefined +): Agent { + const convertedBySource = new Map, Agent>(); + const converted = convertAgent(agent, modelParams, convertedBySource, modelOverride); + if (sandbox !== undefined) { + propagateAgentToolSandboxConfig(converted, sandbox); + } + const conversion: Conversion = { + source: agent, + converted, + modelParams, + modelOverride, + sandbox, + sourceFields: new Map([...convertedBySource.keys()].map((source) => [source, agentFields(source)])), }; - _rebuildFunctionApprovals( - approvals: SerializedFunctionApprovals[], - agentsByIdentity: Map> - ): void; + for (const convertedAgent of convertedBySource.values()) conversions.set(convertedAgent, conversion); + return converted; } /** - * Rebuilds `state` over `converted` while keeping the state's own `RunContext`. + * Returns the converted graph that `state` runs on, if converting `agent` again would give the same graph. * - * Function-tool approvals are keyed by agent object, and the merge keys the state's approvals - * under the new converted agents. The approvals owned by the state's previous converted graph - * are dropped first, or every resume would keep that graph with a full copy of its approvals. - * The serialized state carries them, so the resumed run keeps them. Approvals owned by other - * agents, such as those of another state sharing this context, stay where they are. + * Function-tool approvals are keyed by agent object on the state's `RunContext`, which the resume keeps. + * Rebuilding the state over a new graph would add a copy of its approvals under every resume's graph, and + * the earlier copies can't be dropped, because saved states that still point at those agents read them. */ -async function restoreWithOwnContext( - converted: Agent, - state: RunState -): Promise> { - const serialized = state.toString(); - const context = state._context as RunContext & RunContextApprovalInternals; - const superseded = convertedGraphs.get(state._currentAgent as Agent) ?? new Set(); - // The internal serializer asks for each owner's identity, which lists every owner. - const owners = new Map>(); - const approvals = - context._toJSONForRunState({ - get: (agent) => { - const identity = String(owners.size); - owners.set(identity, agent); - return identity; - }, - }).functionApprovals ?? []; - const kept = approvals.filter(({ agentIdentity }) => !superseded.has(owners.get(agentIdentity)!)); - // Copy the kept records, so the merge below can't change those the restore puts back. - context._rebuildFunctionApprovals(JSON.parse(JSON.stringify(kept)), owners); - try { - return await RunState.fromStringWithContext(converted, serialized, context); - } catch (error) { - // Leave the caller's context as it was, so a later resume of the same state keeps its approvals. - context._rebuildFunctionApprovals(approvals, owners); - throw error; +function reusableConversion( + state: RunState, + agent: Agent, + modelParams: ModelActivityOptions, + modelOverride: string | undefined, + sandbox: SandboxRunConfig | undefined +): Agent | undefined { + const conversion = conversions.get(state._currentAgent as Agent); + if ( + conversion === undefined || + conversion.source !== agent || + conversion.modelOverride !== modelOverride || + !sameValue(conversion.sandbox, sandbox) || + !sameValue(conversion.modelParams, modelParams) + ) { + return undefined; + } + for (const [source, fields] of conversion.sourceFields) { + if (!sameValue(fields, agentFields(source))) return undefined; } + return conversion.converted; } export interface TemporalRunOptions { @@ -329,13 +361,11 @@ export class TemporalOpenAIRunner { const { model: modelOverride, ...runnerConfigOverrides } = options?.runConfig ?? {}; - const convertedBySource = new Map, Agent>(); - const converted = convertAgent(agent, this.modelParams, convertedBySource, modelOverride); - const convertedGraph = new Set(convertedBySource.values()); - for (const convertedAgent of convertedGraph) convertedGraphs.set(convertedAgent, convertedGraph); - if (options?.runConfig?.sandbox !== undefined) { - propagateAgentToolSandboxConfig(converted, options.runConfig.sandbox); - } + const sandbox = options?.runConfig?.sandbox; + const converted = + (input instanceof RunState + ? reusableConversion(input, agent, this.modelParams, modelOverride, sandbox) + : undefined) ?? convertAgentGraph(agent, this.modelParams, modelOverride, sandbox); let preparedInput: string | AgentInputItem[] | RunState; if (input instanceof RunState) { @@ -345,7 +375,10 @@ export class TemporalOpenAIRunner { // environment references on the Workflow thread rather than in the resume Activity. // Pass the state's own context, as the upstream Runner uses it, so resumed tools write the // caller's object rather than a JSON copy. - const restored = (await restoreWithOwnContext(converted, input)) as RunState; + const restored = (await RunState.fromStringWithContext(converted, input.toString(), input._context)) as RunState< + TContext, + TAgent + >; // Suppress upstream Runner.run's withTrace(state._trace, ...) branch; we want the current Workflow's trace. restored.clearTrace(); preparedInput = restored; From ec00865f398a950b068ada815bf010c0e90fe84f Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Fri, 30 Oct 2026 14:52:53 -0700 Subject: [PATCH 06/15] Address review findings --- .../src/__tests__/test-openai-agents.ts | 18 ++++- .../src/__tests__/workflows/openai-agents.ts | 65 +++++++++++++++- .../openai-agents/src/workflow/runner.ts | 74 +++++++++++++++---- 3 files changed, 139 insertions(+), 18 deletions(-) diff --git a/typescript/openai-agents/src/__tests__/test-openai-agents.ts b/typescript/openai-agents/src/__tests__/test-openai-agents.ts index 50f33305..9669faf0 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-agents.ts @@ -26,7 +26,7 @@ import { streamingNoTopicWorkflow, } from './workflows/openai-agents'; import { makeTestFunction } from './helpers/test-fn'; -import { FakeModelProvider, textResponse, toolCallResponse } from './stubs/openai-agents'; +import { FakeModelProvider, handoffResponse, textResponse, toolCallResponse } from './stubs/openai-agents'; import { StreamingFakeModelProvider, streamingTextEvents } from './stubs/openai-agents-fakes'; import EventType = temporal.api.enums.v1.EventType; @@ -548,6 +548,16 @@ test('Approval resume keeps the caller context object', async (t) => { toolCallResponse('gated', { value: 'E1' }, 'call_gated_e1'), toolCallResponse('gated', { value: 'E2' }, 'call_gated_e2'), textResponse('done E'), + toolCallResponse('set_after', { value: 'R-1' }, 'call_rehydrated_1'), + toolCallResponse('set_after', { value: 'R-2' }, 'call_rehydrated_2'), + toolCallResponse('set_after', { value: 'R-3' }, 'call_rehydrated_3'), + textResponse('done rehydrated'), + toolCallResponse('set_after', { value: 'H-1' }, 'call_route_1'), + handoffResponse('transfer_to_FirstTarget'), + textResponse('done routed'), + toolCallResponse('set_after', { value: 'H-2' }, 'call_route_2'), + handoffResponse('transfer_to_FirstTarget'), + textResponse('done disabled'), ]), }), ], @@ -576,5 +586,11 @@ test('Approval resume keeps the caller context object', async (t) => { concurrentFailedResumeThrew: true, concurrentWrites: ['E1', 'E2'], concurrentInterruptions: 0, + rehydratedWrites: ['R-1', 'R-2', 'R-3'], + rehydratedApprovalOwners: 2, + rehydratedApprovedCallIds: 6, + handoffTarget: 'SecondTarget', + handoffFilters: ['second'], + disabledHandoffThrew: true, }); }); diff --git a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts index 206afcbb..e2637f78 100644 --- a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts @@ -1,4 +1,4 @@ -import { Agent, RunState, tool } from '@openai/agents-core'; +import { Agent, RunState, handoff, tool } from '@openai/agents-core'; import { ApplicationFailure, condition } from '@temporalio/workflow'; import { WorkflowStream } from '@temporalio/workflow-streams/workflow'; import { TemporalOpenAIRunner, statelessMcpServer, statefulMcpServer } from '../../workflow'; @@ -200,6 +200,12 @@ export interface ApprovalResumeContextResult { concurrentFailedResumeThrew: boolean; concurrentWrites: string[]; concurrentInterruptions: number; + rehydratedWrites: string[]; + rehydratedApprovalOwners: number; + rehydratedApprovedCallIds: number; + handoffTarget: string | undefined; + handoffFilters: string[]; + disabledHandoffThrew: boolean; } /** Serialized approval record, as the `RunContext` internal below returns it. */ @@ -233,7 +239,9 @@ function countApprovalOwners(context: unknown): { approvalOwners: number; approv * doesn't match, which must fail without losing the approval. Then three paused states share one * `RunContext`, all are approved, and resuming one must keep the others' approvals. Last, an approved * tool throws once, and resuming the same state again must run it without asking again. Then a - * resume fails while another state on its context advances and is approved again. + * resume fails while another state on its context advances and is approved again. Then each resume + * rehydrates the saved state onto the original agent and context. Last, a handoff changes between an + * interruption and its resume. */ export async function approvalResumeContextWorkflow(): Promise { const setAfter = tool({ @@ -371,6 +379,53 @@ export async function approvalResumeContextWorkflow(): Promise({ name: 'FirstTarget', model: 'gpt-4o-mini' }); + const secondTarget = new Agent({ name: 'SecondTarget', model: 'gpt-4o-mini' }); + const routing = handoff(firstTarget, { + inputFilter: (data) => { + handoffFilters.push('first'); + return data; + }, + }); + const routingAgent = new Agent({ + name: 'RoutingAgent', + model: 'gpt-4o-mini', + tools: [setAfter], + handoffs: [routing], + }); + const rehydratedCtx: ResumeContext = { original: 'rehydrated', writes: [] }; + let rehydrated = await runner.run(routingAgent, 'go', { context: rehydratedCtx }); + const rehydratedRunContext = rehydrated.runContext; + for (let round = 0; rehydrated.interruptions.length > 0 && round < 10; round++) { + const state = await RunState.fromStringWithContext(routingAgent, rehydrated.state.toString(), rehydratedRunContext); + state.approve(state.getInterruptions()[0]!); + rehydrated = await runner.run(routingAgent, state); + } + const rehydratedOwners = countApprovalOwners(rehydratedRunContext); + + // The handoff's target and input filter change between the interruption and the resume, and then + // the handoff is disabled before another resume. Each resume must use the handoff as it is now. + const pausedRoute = await runner.run(routingAgent, 'go', { context: { original: 'route', writes: [] } }); + pausedRoute.state.approve(pausedRoute.interruptions[0]!); + routing.agent = secondTarget; + routing.inputFilter = (data) => { + handoffFilters.push('second'); + return data; + }; + const routed = await runner.run(routingAgent, pausedRoute.state); + const pausedDisabled = await runner.run(routingAgent, 'go', { context: { original: 'disabled', writes: [] } }); + pausedDisabled.state.approve(pausedDisabled.interruptions[0]!); + routing.isEnabled = async () => false; + let disabledHandoffThrew = false; + try { + await runner.run(routingAgent, pausedDisabled.state); + } catch { + disabledHandoffThrew = true; + } + return { callerAfter: ctx.after, resumedUsesCallerObject: result.runContext.context === ctx, @@ -387,5 +442,11 @@ export async function approvalResumeContextWorkflow(): Promise, Conversion>(); +/** + * For each `RunContext`, the conversion its latest resume used for each source agent. A state rehydrated + * onto the source agent doesn't point at a converted agent, so this is how its resume finds the graph that + * already owns the context's approvals. It's keyed by context so Workflows that share module-level agents + * under `reuseV8Context` never share a graph. + */ +const resumeConversions = new WeakMap, Conversion>>(); + +/** Snapshots an agent's fields, including those of each `Handoff`, which `convertAgent` clones. */ function agentFields(agent: Agent): [string, unknown][] { - return Object.entries(agent).map(([key, value]) => [key, Array.isArray(value) ? [...value] : value]); + return Object.entries(agent).map(([key, value]) => [ + key, + Array.isArray(value) + ? value.map((member) => + member instanceof Handoff ? { handoff: member, fields: Object.entries(member) } : member + ) + : value, + ]); } function isPlainObject(value: unknown): value is Record { @@ -95,21 +111,13 @@ function convertAgentGraph( return converted; } -/** - * Returns the converted graph that `state` runs on, if converting `agent` again would give the same graph. - * - * Function-tool approvals are keyed by agent object on the state's `RunContext`, which the resume keeps. - * Rebuilding the state over a new graph would add a copy of its approvals under every resume's graph, and - * the earlier copies can't be dropped, because saved states that still point at those agents read them. - */ -function reusableConversion( - state: RunState, +function convertsTheSame( + conversion: Conversion | undefined, agent: Agent, modelParams: ModelActivityOptions, modelOverride: string | undefined, sandbox: SandboxRunConfig | undefined -): Agent | undefined { - const conversion = conversions.get(state._currentAgent as Agent); +): conversion is Conversion { if ( conversion === undefined || conversion.source !== agent || @@ -117,12 +125,45 @@ function reusableConversion( !sameValue(conversion.sandbox, sandbox) || !sameValue(conversion.modelParams, modelParams) ) { - return undefined; + return false; } for (const [source, fields] of conversion.sourceFields) { - if (!sameValue(fields, agentFields(source))) return undefined; + if (!sameValue(fields, agentFields(source))) return false; + } + return true; +} + +/** + * Returns the converted graph that `state` runs on, or else the one the latest resume on its context used, + * if converting `agent` again would give the same graph. + * + * Function-tool approvals are keyed by agent object on the state's `RunContext`, which the resume keeps. + * Rebuilding the state over a new graph would add a copy of its approvals under every resume's graph, and + * the earlier copies can't be dropped, because saved states that still point at those agents read them. + */ +function reusableConversion( + state: RunState, + agent: Agent, + modelParams: ModelActivityOptions, + modelOverride: string | undefined, + sandbox: SandboxRunConfig | undefined +): Agent | undefined { + const candidates = [ + conversions.get(state._currentAgent as Agent), + resumeConversions.get(state._context)?.get(agent), + ]; + return candidates.find((conversion) => convertsTheSame(conversion, agent, modelParams, modelOverride, sandbox)) + ?.converted; +} + +function recordResumeConversion(state: RunState, agent: Agent, converted: Agent): void { + const conversion = conversions.get(converted)!; + let byAgent = resumeConversions.get(state._context); + if (byAgent === undefined) { + byAgent = new Map(); + resumeConversions.set(state._context, byAgent); } - return conversion.converted; + byAgent.set(agent, conversion); } export interface TemporalRunOptions { @@ -369,6 +410,9 @@ export class TemporalOpenAIRunner { let preparedInput: string | AgentInputItem[] | RunState; if (input instanceof RunState) { + // Record before restoring, since the restore merges the state's approvals under this graph even if + // the resume then fails. + recordResumeConversion(input, agent, converted); // Round-trip through fromString so the rehydrated state's agent graph carries our converted // agent (with ActivityBackedModel); setCurrentAgent only swaps the top-level ref, not nested ones. // It also drops upstream's live-session identity, which would otherwise resolve manifest From b583a641dfd0034211f46d35eccc7236d4b450e6 Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Fri, 30 Oct 2026 16:10:19 -0700 Subject: [PATCH 07/15] Address review findings --- .../src/__tests__/test-openai-sandbox.ts | 35 +++++++++++++ .../src/__tests__/workflows/openai-sandbox.ts | 49 ++++++++++++++++++- .../openai-agents/src/workflow/runner.ts | 46 ++++++++++++++--- 3 files changed, 123 insertions(+), 7 deletions(-) diff --git a/typescript/openai-agents/src/__tests__/test-openai-sandbox.ts b/typescript/openai-agents/src/__tests__/test-openai-sandbox.ts index 4dcab3fd..29f1aa0a 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-sandbox.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-sandbox.ts @@ -11,6 +11,7 @@ import { sandboxApprovalResumeWorkflow, sandboxArchiveLimitsWorkflow, sandboxExecWorkflow, + sandboxHandoffConfigResumeWorkflow, sandboxManifestResumeWorkflow, sandboxValidationWorkflow, } from './workflows/openai-sandbox'; @@ -114,6 +115,40 @@ test.serial('a SandboxAgent whose manifest carries a workerEnvValue survives an t.true(client.resumeCalls >= 1, 'the preserved session is re-established through resume()'); }); +test('a SandboxAgent manifest or runAs change before the resume reaches the sandbox', async (t) => { + const { createWorker, executeWorkflow } = helpers(t); + + const client = new FakeSandboxClient(); + const responses = () => [ + toolCallResponse('confirm', {}), + toolCallResponse('transfer_to_sandbox_target', {}), + toolCallResponse('run_command', { cmd: 'echo hello' }), + textResponse('Done.'), + ]; + const worker = await createWorker({ + plugins: [ + new OpenAIAgentsPlugin({ + modelProvider: new FakeModelProvider([...responses(), ...responses()]), + modelParams: { startToCloseTimeout: '30s' }, + sandboxClientProviders: [new SandboxClientProvider('fake', client)], + }), + ], + }); + + const users = () => client.session.state.manifest.users.map((user) => user.name); + await worker.runUntil(async () => { + t.is(await executeWorkflow(sandboxHandoffConfigResumeWorkflow, { args: ['environment'] }), 'Done.'); + t.is(client.createCalls.length, 1); + t.is(client.session.state.environment?.VERSION, 'after'); + t.deepEqual(users(), ['before']); + + t.is(await executeWorkflow(sandboxHandoffConfigResumeWorkflow, { args: ['runAs'] }), 'Done.'); + t.is(client.createCalls.length, 2); + t.is(client.session.state.environment?.VERSION, 'before'); + t.deepEqual(users(), ['after']); + }); +}); + test('setArchiveLimits is forwarded to the persistWorkspace and hydrateWorkspace Activities', async (t) => { const { createWorker, executeWorkflow } = helpers(t); diff --git a/typescript/openai-agents/src/__tests__/workflows/openai-sandbox.ts b/typescript/openai-agents/src/__tests__/workflows/openai-sandbox.ts index f259645d..46330a00 100644 --- a/typescript/openai-agents/src/__tests__/workflows/openai-sandbox.ts +++ b/typescript/openai-agents/src/__tests__/workflows/openai-sandbox.ts @@ -1,6 +1,7 @@ -import { Agent, type FunctionTool, type RunContext, type Tool } from '@openai/agents-core'; +import { Agent, tool, type FunctionTool, type RunContext, type Tool } from '@openai/agents-core'; import { Capability, + Environment, Manifest, SandboxAgent, isEnvValueReference, @@ -120,6 +121,52 @@ export async function sandboxApprovalResumeWorkflow(): Promise { return `${resumed.finalOutput}`; } +/** + * A root agent pauses for approval before it hands off to a `SandboxAgent`. Then the target's manifest + * environment entry or its `runAs` user changes in place, and the resume must create the sandbox with the + * new value. + */ +export async function sandboxHandoffConfigResumeWorkflow(change: 'environment' | 'runAs'): Promise { + const target = new SandboxAgent({ + name: 'sandbox_target', + model: 'gpt-4o-mini', + capabilities: [new TestSandboxCapability()], + defaultManifest: new Manifest({ + entries: { 'data.bin': { type: 'file', content: new Uint8Array([0, 1, 2]) } }, + environment: { VERSION: 'before' }, + }), + runAs: { name: 'before' }, + }); + const root = new Agent({ + name: 'root', + model: 'gpt-4o-mini', + tools: [ + tool({ + name: 'confirm', + description: 'confirm', + parameters: { type: 'object', properties: {}, required: [], additionalProperties: false }, + needsApproval: true, + execute: async () => 'confirmed', + }), + ], + handoffs: [target], + }); + const runner = new TemporalOpenAIRunner(); + const runConfig = { sandbox: { client: temporalSandboxClient('fake') } }; + + const result = await runner.run(root, 'go', { runConfig }); + if (result.interruptions.length === 0) return 'no-interruption'; + if (change === 'environment') { + target.defaultManifest!.environment.VERSION = new Environment('after'); + } else { + (target.runAs as { name: string }).name = 'after'; + } + for (const interruption of result.interruptions) result.state.approve(interruption); + + const resumed = await runner.run(root, result.state, { runConfig }); + return `${resumed.finalOutput}`; +} + export async function sandboxManifestResumeWorkflow(): Promise { const client = temporalSandboxClient('fake'); const session = await client.create( diff --git a/typescript/openai-agents/src/workflow/runner.ts b/typescript/openai-agents/src/workflow/runner.ts index 97c3cb99..91b8971b 100644 --- a/typescript/openai-agents/src/workflow/runner.ts +++ b/typescript/openai-agents/src/workflow/runner.ts @@ -57,15 +57,46 @@ const conversions = new WeakMap, Conversion>(); */ const resumeConversions = new WeakMap, Conversion>>(); +/** + * The `SandboxAgent` fields that `SandboxAgent.clone` copies by value (`cloneManifest`, and a new user object + * for `runAs`), so a change inside them doesn't reach a converted graph. `Agent.clone` and `Handoff.clone` + * share every other field by reference. + */ +const sandboxAgentFieldsCopiedByValue = new Set(['defaultManifest', 'runAs']); + +/** A deep copy of `value`'s contents that `sameValue` compares member by member. Functions stay by identity. */ +function contentSnapshot(value: unknown, ancestors: object[] = []): unknown { + if (typeof value !== 'object' || value === null) return value; + if (ArrayBuffer.isView(value) || value instanceof ArrayBuffer) { + const bytes = ArrayBuffer.isView(value) + ? new Uint8Array(value.buffer, value.byteOffset, value.byteLength) + : new Uint8Array(value); + return { prototype: Object.getPrototypeOf(value), bytes: bytes.slice() }; + } + const cycle = ancestors.indexOf(value); + if (cycle !== -1) return { cycle }; + ancestors.push(value); + const entries = + value instanceof Map ? [...value] : value instanceof Set ? [...value].entries() : Object.entries(value); + const snapshot = { + prototype: Object.getPrototypeOf(value), + members: [...entries].map(([key, member]) => [contentSnapshot(key, ancestors), contentSnapshot(member, ancestors)]), + }; + ancestors.pop(); + return snapshot; +} + /** Snapshots an agent's fields, including those of each `Handoff`, which `convertAgent` clones. */ function agentFields(agent: Agent): [string, unknown][] { return Object.entries(agent).map(([key, value]) => [ key, - Array.isArray(value) - ? value.map((member) => - member instanceof Handoff ? { handoff: member, fields: Object.entries(member) } : member - ) - : value, + agent instanceof SandboxAgent && sandboxAgentFieldsCopiedByValue.has(key) + ? contentSnapshot(value) + : Array.isArray(value) + ? value.map((member) => + member instanceof Handoff ? { handoff: member, fields: Object.entries(member) } : member + ) + : value, ]); } @@ -75,9 +106,12 @@ function isPlainObject(value: unknown): value is Record { return prototype === Object.prototype || prototype === null; } -/** Compares arrays and plain objects by their members, and any other value by identity. */ +/** Compares arrays, byte arrays and plain objects by their members, and any other value by identity. */ function sameValue(a: unknown, b: unknown): boolean { if (Object.is(a, b)) return true; + if (a instanceof Uint8Array && b instanceof Uint8Array) { + return a.length === b.length && a.every((byte, index) => byte === b[index]); + } if (Array.isArray(a) && Array.isArray(b)) { return a.length === b.length && a.every((value, index) => sameValue(value, b[index])); } From e408f48cfc7fa34892178ba3d6acf15ccd872dd5 Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Fri, 30 Oct 2026 16:55:21 -0700 Subject: [PATCH 08/15] Address review findings --- .../src/__tests__/stubs/private-env-value.ts | 22 ++ .../src/__tests__/stubs/sandbox-fakes.ts | 3 + .../src/__tests__/test-openai-agents.ts | 8 +- .../src/__tests__/test-openai-sandbox.ts | 7 +- .../src/__tests__/workflows/openai-agents.ts | 56 +++- .../src/__tests__/workflows/openai-sandbox.ts | 13 +- .../src/workflow/convert-agent.ts | 27 +- .../openai-agents/src/workflow/runner.ts | 263 +++++++----------- 8 files changed, 216 insertions(+), 183 deletions(-) create mode 100644 typescript/openai-agents/src/__tests__/stubs/private-env-value.ts diff --git a/typescript/openai-agents/src/__tests__/stubs/private-env-value.ts b/typescript/openai-agents/src/__tests__/stubs/private-env-value.ts new file mode 100644 index 00000000..14b9aae1 --- /dev/null +++ b/typescript/openai-agents/src/__tests__/stubs/private-env-value.ts @@ -0,0 +1,22 @@ +import { EnvValueReference, registerEnvValueReference } from '@openai/agents-core/sandbox'; + +/** A Manifest environment reference that keeps its value in a private field, which only `serialize` exposes. */ +export class PrivateEnvValue extends EnvValueReference { + static readonly type = 'test.private-env-value'; + readonly #value: string; + + constructor(value: string) { + super(); + this.#value = value; + } + + serialize(): Record { + return { secret: this.#value }; + } + + async resolve(): Promise { + return this.#value; + } +} + +registerEnvValueReference(PrivateEnvValue, (payload) => new PrivateEnvValue(String(payload.secret))); diff --git a/typescript/openai-agents/src/__tests__/stubs/sandbox-fakes.ts b/typescript/openai-agents/src/__tests__/stubs/sandbox-fakes.ts index 193d6636..5c889866 100644 --- a/typescript/openai-agents/src/__tests__/stubs/sandbox-fakes.ts +++ b/typescript/openai-agents/src/__tests__/stubs/sandbox-fakes.ts @@ -18,6 +18,9 @@ import { } from '@openai/agents-core/sandbox'; import { deserializeManifest, mergeManifestDelta, mergeManifestEntryDelta } from '@openai/agents-core/sandbox/internal'; +// Re-exported so the Worker can deserialize this reference in sandbox Activities. +export { PrivateEnvValue } from './private-env-value'; + export class FakeSandboxSession implements SandboxSession { state: SandboxSessionState; execCalls: ExecCommandArgs[] = []; diff --git a/typescript/openai-agents/src/__tests__/test-openai-agents.ts b/typescript/openai-agents/src/__tests__/test-openai-agents.ts index 9669faf0..5b373379 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-agents.ts @@ -557,7 +557,9 @@ test('Approval resume keeps the caller context object', async (t) => { textResponse('done routed'), toolCallResponse('set_after', { value: 'H-2' }, 'call_route_2'), handoffResponse('transfer_to_FirstTarget'), - textResponse('done disabled'), + toolCallResponse('extend', {}, 'call_extend'), + toolCallResponse('extra', { value: 'X' }, 'call_extra'), + textResponse('done extended'), ]), }), ], @@ -567,7 +569,7 @@ test('Approval resume keeps the caller context object', async (t) => { executeWorkflow(approvalResumeContextWorkflow, { workflowExecutionTimeout: '30 seconds' }) ); - // Each resume reuses the converted graph its state runs on, so the approvals keep one owner, and + // Each resume converts into the agent objects its state runs on, so the approvals keep one owner, and // no resume drops approvals that another state on the context, or a retry of the same state, needs. t.deepEqual(result, { callerAfter: 'S-after', @@ -592,5 +594,7 @@ test('Approval resume keeps the caller context object', async (t) => { handoffTarget: 'SecondTarget', handoffFilters: ['second'], disabledHandoffThrew: true, + sharedFieldsWrites: ['X'], + sharedFieldsTemperatures: [0.1, 0.9, 0.9], }); }); diff --git a/typescript/openai-agents/src/__tests__/test-openai-sandbox.ts b/typescript/openai-agents/src/__tests__/test-openai-sandbox.ts index 29f1aa0a..cd447f5d 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-sandbox.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-sandbox.ts @@ -128,7 +128,7 @@ test('a SandboxAgent manifest or runAs change before the resume reaches the sand const worker = await createWorker({ plugins: [ new OpenAIAgentsPlugin({ - modelProvider: new FakeModelProvider([...responses(), ...responses()]), + modelProvider: new FakeModelProvider([...responses(), ...responses(), ...responses()]), modelParams: { startToCloseTimeout: '30s' }, sandboxClientProviders: [new SandboxClientProvider('fake', client)], }), @@ -146,6 +146,11 @@ test('a SandboxAgent manifest or runAs change before the resume reaches the sand t.is(client.createCalls.length, 2); t.is(client.session.state.environment?.VERSION, 'before'); t.deepEqual(users(), ['after']); + + t.is(await executeWorkflow(sandboxHandoffConfigResumeWorkflow, { args: ['reference'] }), 'Done.'); + t.is(client.createCalls.length, 3); + t.is(client.session.state.environment?.VERSION, 'after'); + t.deepEqual(users(), ['before']); }); }); diff --git a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts index e2637f78..5f766d91 100644 --- a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts @@ -206,6 +206,8 @@ export interface ApprovalResumeContextResult { handoffTarget: string | undefined; handoffFilters: string[]; disabledHandoffThrew: boolean; + sharedFieldsWrites: string[]; + sharedFieldsTemperatures: Array; } /** Serialized approval record, as the `RunContext` internal below returns it. */ @@ -240,8 +242,9 @@ function countApprovalOwners(context: unknown): { approvalOwners: number; approv * `RunContext`, all are approved, and resuming one must keep the others' approvals. Last, an approved * tool throws once, and resuming the same state again must run it without asking again. Then a * resume fails while another state on its context advances and is approved again. Then each resume - * rehydrates the saved state onto the original agent and context. Last, a handoff changes between an - * interruption and its resume. + * rehydrates the saved state onto the original agent and context. Then a handoff changes between an + * interruption and its resume. Last, an agent's tools and model settings are replaced with equal copies + * before a resume, and the approved tool changes them. */ export async function approvalResumeContextWorkflow(): Promise { const setAfter = tool({ @@ -426,6 +429,53 @@ export async function approvalResumeContextWorkflow(): Promise { + (runContext!.context as ResumeContext).writes.push((args as { value: string }).value); + return 'ok'; + }, + }); + const sharedFieldsTemperatures: Array = []; + const sharedFieldsAgent: Agent = new Agent({ + name: 'SharedFieldsAgent', + model: 'gpt-4o-mini', + instructions: (_runContext, current) => { + sharedFieldsTemperatures.push(current.modelSettings.temperature); + return 'Extend yourself.'; + }, + modelSettings: { temperature: 0.1 }, + tools: [ + tool({ + name: 'extend', + description: 'Add a tool and change a setting after approval.', + parameters: { type: 'object', properties: {}, required: [], additionalProperties: false }, + needsApproval: true, + execute: async () => { + sharedFieldsAgent.tools.push(extra); + sharedFieldsAgent.modelSettings.temperature = 0.9; + return 'ok'; + }, + }), + ], + }); + const sharedFieldsCtx: ResumeContext = { original: 'shared-fields', writes: [] }; + const pausedSharedFields = await runner.run(sharedFieldsAgent, 'go', { context: sharedFieldsCtx }); + pausedSharedFields.state.approve(pausedSharedFields.interruptions[0]!); + sharedFieldsAgent.tools = [...sharedFieldsAgent.tools]; + sharedFieldsAgent.modelSettings = { ...sharedFieldsAgent.modelSettings }; + await runner.run(sharedFieldsAgent, pausedSharedFields.state); + return { callerAfter: ctx.after, resumedUsesCallerObject: result.runContext.context === ctx, @@ -448,5 +498,7 @@ export async function approvalResumeContextWorkflow(): Promise { /** * A root agent pauses for approval before it hands off to a `SandboxAgent`. Then the target's manifest - * environment entry or its `runAs` user changes in place, and the resume must create the sandbox with the - * new value. + * environment entry, an environment reference that keeps its value private, or its `runAs` user changes in + * place, and the resume must create the sandbox with the new value. */ -export async function sandboxHandoffConfigResumeWorkflow(change: 'environment' | 'runAs'): Promise { +export async function sandboxHandoffConfigResumeWorkflow( + change: 'environment' | 'reference' | 'runAs' +): Promise { const target = new SandboxAgent({ name: 'sandbox_target', model: 'gpt-4o-mini', capabilities: [new TestSandboxCapability()], defaultManifest: new Manifest({ entries: { 'data.bin': { type: 'file', content: new Uint8Array([0, 1, 2]) } }, - environment: { VERSION: 'before' }, + environment: { VERSION: change === 'reference' ? new PrivateEnvValue('before') : 'before' }, }), runAs: { name: 'before' }, }); @@ -158,6 +161,8 @@ export async function sandboxHandoffConfigResumeWorkflow(change: 'environment' | if (result.interruptions.length === 0) return 'no-interruption'; if (change === 'environment') { target.defaultManifest!.environment.VERSION = new Environment('after'); + } else if (change === 'reference') { + target.defaultManifest!.environment.VERSION = new PrivateEnvValue('after'); } else { (target.runAs as { name: string }).name = 'after'; } diff --git a/typescript/openai-agents/src/workflow/convert-agent.ts b/typescript/openai-agents/src/workflow/convert-agent.ts index c699db37..f26cd8ea 100644 --- a/typescript/openai-agents/src/workflow/convert-agent.ts +++ b/typescript/openai-agents/src/workflow/convert-agent.ts @@ -7,12 +7,17 @@ import { ActivityBackedModel } from './activity-backed-model'; * Recursively converts an agent graph: validate tools, replace each agent's * model with an ActivityBackedModel, and clone handoffs. Uses a seen-map to * handle circular handoff references. + * + * When `identities` maps a source agent to an agent an earlier conversion returned, that object is + * returned again, with the fields of this conversion's clone. Each agent then has the same fields as + * after a fresh conversion, but keeps its object identity, which approvals on a `RunContext` are keyed by. */ export function convertAgent( agent: Agent, modelParams: ModelActivityOptions, seen?: Map, Agent>, - modelNameOverride?: string + modelNameOverride?: string, + identities?: ReadonlyMap, Agent> ): Agent { seen = seen ?? new Map(); if (seen.has(agent)) return seen.get(agent)!; @@ -57,17 +62,31 @@ export function convertAgent( // user-declared name and instructions. activityBackedModel.setAgent(agent); - const converted = agent.clone({ model: activityBackedModel }); + const clone = agent.clone({ model: activityBackedModel }); + const identity = identities?.get(agent); + const converted = identity === undefined ? clone : replaceFields(identity, clone); seen.set(agent, converted); const convertedHandoffs = (agent.handoffs ?? []).map((h) => { if (h instanceof Handoff) { - const convertedHandoffAgent = convertAgent(h.agent, modelParams, seen, modelNameOverride); + const convertedHandoffAgent = convertAgent(h.agent, modelParams, seen, modelNameOverride, identities); return h.clone({ agent: convertedHandoffAgent }); } - return convertAgent(h, modelParams, seen, modelNameOverride); + return convertAgent(h, modelParams, seen, modelNameOverride, identities); }); converted.handoffs = convertedHandoffs; return converted; } + +/** Gives `target` exactly the own properties and prototype of `source`, and returns `target`. */ +function replaceFields(target: Agent, source: Agent): Agent { + for (const key of Reflect.ownKeys(target)) { + delete (target as any)[key]; + } + Object.defineProperties(target, Object.getOwnPropertyDescriptors(source)); + if (Object.getPrototypeOf(target) !== Object.getPrototypeOf(source)) { + Object.setPrototypeOf(target, Object.getPrototypeOf(source)); + } + return target; +} diff --git a/typescript/openai-agents/src/workflow/runner.ts b/typescript/openai-agents/src/workflow/runner.ts index 91b8971b..32d84002 100644 --- a/typescript/openai-agents/src/workflow/runner.ts +++ b/typescript/openai-agents/src/workflow/runner.ts @@ -35,163 +35,68 @@ import { getCurrentPluginConfig } from './plugin-config-store'; const agentAsToolSandboxConfig = Symbol.for('@temporalio/openai-agents/agentAsToolSandboxConfig'); -/** A `convertAgent` call, with the inputs that decide whether a later run would convert the same graph. */ +/** A converted agent graph. */ interface Conversion { source: Agent; - converted: Agent; - modelParams: ModelActivityOptions; - modelOverride: string | undefined; - sandbox: SandboxRunConfig | undefined; - /** Each source agent in the graph, with its fields when it was converted. */ - sourceFields: Map, [string, unknown][]>; + /** Each source agent the graph has converted, mapped to its converted agent. */ + agents: Map, Agent>; + /** Runs that are using the graph. A resume converts into the graph only while no run uses it. */ + activeRuns: number; } -/** Each converted agent, mapped to the conversion that produced it. */ +/** Each converted agent, mapped to the graph it belongs to. */ const conversions = new WeakMap, Conversion>(); /** - * For each `RunContext`, the conversion its latest resume used for each source agent. A state rehydrated - * onto the source agent doesn't point at a converted agent, so this is how its resume finds the graph that + * For each `RunContext`, the graph its latest resume used for each source agent. A state rehydrated onto + * the source agent doesn't point at a converted agent, so this is how its resume finds the graph that * already owns the context's approvals. It's keyed by context so Workflows that share module-level agents * under `reuseV8Context` never share a graph. */ const resumeConversions = new WeakMap, Conversion>>(); /** - * The `SandboxAgent` fields that `SandboxAgent.clone` copies by value (`cloneManifest`, and a new user object - * for `runAs`), so a change inside them doesn't reach a converted graph. `Agent.clone` and `Handoff.clone` - * share every other field by reference. + * Converts `agent`'s graph. With `into`, the conversion reuses `into`'s agent objects: each one gets the + * fields a fresh conversion would give it, and keeps its identity. */ -const sandboxAgentFieldsCopiedByValue = new Set(['defaultManifest', 'runAs']); - -/** A deep copy of `value`'s contents that `sameValue` compares member by member. Functions stay by identity. */ -function contentSnapshot(value: unknown, ancestors: object[] = []): unknown { - if (typeof value !== 'object' || value === null) return value; - if (ArrayBuffer.isView(value) || value instanceof ArrayBuffer) { - const bytes = ArrayBuffer.isView(value) - ? new Uint8Array(value.buffer, value.byteOffset, value.byteLength) - : new Uint8Array(value); - return { prototype: Object.getPrototypeOf(value), bytes: bytes.slice() }; - } - const cycle = ancestors.indexOf(value); - if (cycle !== -1) return { cycle }; - ancestors.push(value); - const entries = - value instanceof Map ? [...value] : value instanceof Set ? [...value].entries() : Object.entries(value); - const snapshot = { - prototype: Object.getPrototypeOf(value), - members: [...entries].map(([key, member]) => [contentSnapshot(key, ancestors), contentSnapshot(member, ancestors)]), - }; - ancestors.pop(); - return snapshot; -} - -/** Snapshots an agent's fields, including those of each `Handoff`, which `convertAgent` clones. */ -function agentFields(agent: Agent): [string, unknown][] { - return Object.entries(agent).map(([key, value]) => [ - key, - agent instanceof SandboxAgent && sandboxAgentFieldsCopiedByValue.has(key) - ? contentSnapshot(value) - : Array.isArray(value) - ? value.map((member) => - member instanceof Handoff ? { handoff: member, fields: Object.entries(member) } : member - ) - : value, - ]); -} - -function isPlainObject(value: unknown): value is Record { - if (typeof value !== 'object' || value === null) return false; - const prototype = Object.getPrototypeOf(value); - return prototype === Object.prototype || prototype === null; -} - -/** Compares arrays, byte arrays and plain objects by their members, and any other value by identity. */ -function sameValue(a: unknown, b: unknown): boolean { - if (Object.is(a, b)) return true; - if (a instanceof Uint8Array && b instanceof Uint8Array) { - return a.length === b.length && a.every((byte, index) => byte === b[index]); - } - if (Array.isArray(a) && Array.isArray(b)) { - return a.length === b.length && a.every((value, index) => sameValue(value, b[index])); - } - if (isPlainObject(a) && isPlainObject(b)) { - const keys = Object.keys(a); - return keys.length === Object.keys(b).length && keys.every((key) => key in b && sameValue(a[key], b[key])); - } - return false; -} - function convertAgentGraph( agent: Agent, modelParams: ModelActivityOptions, modelOverride: string | undefined, - sandbox: SandboxRunConfig | undefined -): Agent { + sandbox: SandboxRunConfig | undefined, + into?: Conversion +): Conversion { const convertedBySource = new Map, Agent>(); - const converted = convertAgent(agent, modelParams, convertedBySource, modelOverride); + const converted = convertAgent(agent, modelParams, convertedBySource, modelOverride, into?.agents); if (sandbox !== undefined) { propagateAgentToolSandboxConfig(converted, sandbox); } - const conversion: Conversion = { - source: agent, - converted, - modelParams, - modelOverride, - sandbox, - sourceFields: new Map([...convertedBySource.keys()].map((source) => [source, agentFields(source)])), - }; - for (const convertedAgent of convertedBySource.values()) conversions.set(convertedAgent, conversion); - return converted; -} - -function convertsTheSame( - conversion: Conversion | undefined, - agent: Agent, - modelParams: ModelActivityOptions, - modelOverride: string | undefined, - sandbox: SandboxRunConfig | undefined -): conversion is Conversion { - if ( - conversion === undefined || - conversion.source !== agent || - conversion.modelOverride !== modelOverride || - !sameValue(conversion.sandbox, sandbox) || - !sameValue(conversion.modelParams, modelParams) - ) { - return false; - } - for (const [source, fields] of conversion.sourceFields) { - if (!sameValue(fields, agentFields(source))) return false; + const conversion = into ?? { source: agent, agents: new Map(), activeRuns: 0 }; + for (const [source, convertedAgent] of convertedBySource) { + conversion.agents.set(source, convertedAgent); + conversions.set(convertedAgent, conversion); } - return true; + return conversion; } /** * Returns the converted graph that `state` runs on, or else the one the latest resume on its context used, - * if converting `agent` again would give the same graph. + * if it was converted from `agent` and no run is using it. * * Function-tool approvals are keyed by agent object on the state's `RunContext`, which the resume keeps. - * Rebuilding the state over a new graph would add a copy of its approvals under every resume's graph, and - * the earlier copies can't be dropped, because saved states that still point at those agents read them. + * Rebuilding the state over new agent objects would add a copy of its approvals under every resume's + * graph, and the earlier copies can't be dropped, because saved states that still point at those agents + * read them. */ -function reusableConversion( - state: RunState, - agent: Agent, - modelParams: ModelActivityOptions, - modelOverride: string | undefined, - sandbox: SandboxRunConfig | undefined -): Agent | undefined { +function reusableConversion(state: RunState, agent: Agent): Conversion | undefined { const candidates = [ conversions.get(state._currentAgent as Agent), resumeConversions.get(state._context)?.get(agent), ]; - return candidates.find((conversion) => convertsTheSame(conversion, agent, modelParams, modelOverride, sandbox)) - ?.converted; + return candidates.find((conversion) => conversion?.source === agent && conversion.activeRuns === 0); } -function recordResumeConversion(state: RunState, agent: Agent, converted: Agent): void { - const conversion = conversions.get(converted)!; +function recordResumeConversion(state: RunState, agent: Agent, conversion: Conversion): void { let byAgent = resumeConversions.get(state._context); if (byAgent === undefined) { byAgent = new Map(); @@ -437,58 +342,76 @@ export class TemporalOpenAIRunner { const { model: modelOverride, ...runnerConfigOverrides } = options?.runConfig ?? {}; const sandbox = options?.runConfig?.sandbox; - const converted = - (input instanceof RunState - ? reusableConversion(input, agent, this.modelParams, modelOverride, sandbox) - : undefined) ?? convertAgentGraph(agent, this.modelParams, modelOverride, sandbox); - - let preparedInput: string | AgentInputItem[] | RunState; - if (input instanceof RunState) { - // Record before restoring, since the restore merges the state's approvals under this graph even if - // the resume then fails. - recordResumeConversion(input, agent, converted); - // Round-trip through fromString so the rehydrated state's agent graph carries our converted - // agent (with ActivityBackedModel); setCurrentAgent only swaps the top-level ref, not nested ones. - // It also drops upstream's live-session identity, which would otherwise resolve manifest - // environment references on the Workflow thread rather than in the resume Activity. - // Pass the state's own context, as the upstream Runner uses it, so resumed tools write the - // caller's object rather than a JSON copy. - const restored = (await RunState.fromStringWithContext(converted, input.toString(), input._context)) as RunState< - TContext, - TAgent - >; - // Suppress upstream Runner.run's withTrace(state._trace, ...) branch; we want the current Workflow's trace. - restored.clearTrace(); - preparedInput = restored; - } else { - preparedInput = input; - } - - const innerRunner = new Runner({ ...runnerConfigOverrides }); - - const sharedRunOptions = { - maxTurns: options?.maxTurns, - context: options?.context, - previousResponseId: options?.previousResponseId, - conversationId: options?.conversationId, - session: options?.session, - sessionInputCallback: options?.sessionInputCallback, - callModelInputFilter: options?.callModelInputFilter, - tracing: options?.tracing, - }; - + const conversion = convertAgentGraph( + agent, + this.modelParams, + modelOverride, + sandbox, + input instanceof RunState ? reusableConversion(input, agent) : undefined + ); + const converted = conversion.agents.get(agent)!; + + conversion.activeRuns++; + // A streamed run keeps using the graph after `run` returns, until its result completes. + let releasedOnCompletion = false; try { - if (options?.stream === true) { - return (await innerRunner.run(converted, preparedInput as any, { - ...sharedRunOptions, - stream: true, - })) as StreamedRunResult; + let preparedInput: string | AgentInputItem[] | RunState; + if (input instanceof RunState) { + // Record before restoring, since the restore merges the state's approvals under this graph even if + // the resume then fails. + recordResumeConversion(input, agent, conversion); + // Round-trip through fromString so the rehydrated state's agent graph carries our converted + // agent (with ActivityBackedModel); setCurrentAgent only swaps the top-level ref, not nested ones. + // It also drops upstream's live-session identity, which would otherwise resolve manifest + // environment references on the Workflow thread rather than in the resume Activity. + // Pass the state's own context, as the upstream Runner uses it, so resumed tools write the + // caller's object rather than a JSON copy. + const restored = (await RunState.fromStringWithContext( + converted, + input.toString(), + input._context + )) as RunState; + // Suppress upstream Runner.run's withTrace(state._trace, ...) branch; we want the current Workflow's trace. + restored.clearTrace(); + preparedInput = restored; + } else { + preparedInput = input; + } + + const innerRunner = new Runner({ ...runnerConfigOverrides }); + + const sharedRunOptions = { + maxTurns: options?.maxTurns, + context: options?.context, + previousResponseId: options?.previousResponseId, + conversationId: options?.conversationId, + session: options?.session, + sessionInputCallback: options?.sessionInputCallback, + callModelInputFilter: options?.callModelInputFilter, + tracing: options?.tracing, + }; + + try { + if (options?.stream === true) { + const result = (await innerRunner.run(converted, preparedInput as any, { + ...sharedRunOptions, + stream: true, + })) as StreamedRunResult; + const release = () => { + conversion.activeRuns--; + }; + result.completed.then(release, release); + releasedOnCompletion = true; + return result; + } + return (await innerRunner.run(converted, preparedInput as any, sharedRunOptions)) as RunResult; + } catch (error) { + throw normalizeAgentsRunError(error); + } finally { + await flushOpenSpans(); } - return (await innerRunner.run(converted, preparedInput as any, sharedRunOptions)) as RunResult; - } catch (error) { - throw normalizeAgentsRunError(error); } finally { - await flushOpenSpans(); + if (!releasedOnCompletion) conversion.activeRuns--; } } } From 977c005e0a5a176db145f8de7df97bb5f89ac78f Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Sat, 31 Oct 2026 02:06:03 -0700 Subject: [PATCH 09/15] Address review findings --- .../src/__tests__/test-openai-agents.ts | 10 +++ .../src/__tests__/workflows/openai-agents.ts | 79 ++++++++++++++++++- .../src/workflow/convert-agent.ts | 63 +++++++++++++-- 3 files changed, 141 insertions(+), 11 deletions(-) diff --git a/typescript/openai-agents/src/__tests__/test-openai-agents.ts b/typescript/openai-agents/src/__tests__/test-openai-agents.ts index 5b373379..64edea58 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-agents.ts @@ -560,6 +560,11 @@ test('Approval resume keeps the caller context object', async (t) => { toolCallResponse('extend', {}, 'call_extend'), toolCallResponse('extra', { value: 'X' }, 'call_extra'), textResponse('done extended'), + handoffResponse('transfer_to_FailChild'), + toolCallResponse('set_after', { value: 'F' }, 'call_fail_child'), + textResponse('done failed conversion'), + toolCallResponse('set_after', { value: 'V' }, 'call_versioned'), + textResponse('done versioned'), ]), }), ], @@ -596,5 +601,10 @@ test('Approval resume keeps the caller context object', async (t) => { disabledHandoffThrew: true, sharedFieldsWrites: ['X'], sharedFieldsTemperatures: [0.1, 0.9, 0.9], + failedConversionThrew: true, + failedConversionWrites: ['F'], + failedConversionInterruptions: 0, + versions: ['before', 'after'], + lastAgentVersion: 'after', }); }); diff --git a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts index 5f766d91..ffd1ba2a 100644 --- a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts @@ -1,4 +1,4 @@ -import { Agent, RunState, handoff, tool } from '@openai/agents-core'; +import { Agent, type AgentOptions, RunState, handoff, tool } from '@openai/agents-core'; import { ApplicationFailure, condition } from '@temporalio/workflow'; import { WorkflowStream } from '@temporalio/workflow-streams/workflow'; import { TemporalOpenAIRunner, statelessMcpServer, statefulMcpServer } from '../../workflow'; @@ -208,6 +208,33 @@ export interface ApprovalResumeContextResult { disabledHandoffThrew: boolean; sharedFieldsWrites: string[]; sharedFieldsTemperatures: Array; + failedConversionThrew: boolean; + failedConversionWrites: string[]; + failedConversionInterruptions: number; + versions: string[]; + lastAgentVersion: string | undefined; +} + +/** An agent whose `clone` carries a private field, which can't be copied onto another object. */ +class VersionedAgent extends Agent { + #version: string; + + constructor(config: AgentOptions, version: string) { + super(config); + this.#version = version; + } + + get version(): string { + return this.#version; + } + + setVersion(version: string): void { + this.#version = version; + } + + override clone(config: Partial>): VersionedAgent { + return new VersionedAgent({ ...this, ...config } as AgentOptions, this.#version); + } } /** Serialized approval record, as the `RunContext` internal below returns it. */ @@ -243,8 +270,10 @@ function countApprovalOwners(context: unknown): { approvalOwners: number; approv * tool throws once, and resuming the same state again must run it without asking again. Then a * resume fails while another state on its context advances and is approved again. Then each resume * rehydrates the saved state onto the original agent and context. Then a handoff changes between an - * interruption and its resume. Last, an agent's tools and model settings are replaced with equal copies - * before a resume, and the approved tool changes them. + * interruption and its resume. Then an agent's tools and model settings are replaced with equal copies + * before a resume, and the approved tool changes them. Then a resume fails to convert a handoff target, + * and the saved state must still carry its approval. Last, an agent class whose clone keeps a private + * field changes that field before the resume. */ export async function approvalResumeContextWorkflow(): Promise { const setAfter = tool({ @@ -476,6 +505,45 @@ export async function approvalResumeContextWorkflow(): Promise({ name: 'FailChild', model: 'gpt-4o-mini', tools: [setAfter] }); + const failRoot = new Agent({ name: 'FailRoot', model: 'gpt-4o-mini', handoffs: [failChild] }); + const failedConversionCtx: ResumeContext = { original: 'failed-conversion', writes: [] }; + const pausedFail = await runner.run(failRoot, 'go', { context: failedConversionCtx }); + pausedFail.state.approve(pausedFail.interruptions[0]!); + failChild.tools = [setAfter, (() => 'raw') as any]; + let failedConversionThrew = false; + try { + await runner.run(failRoot, pausedFail.state); + } catch { + failedConversionThrew = true; + } + failChild.tools = [setAfter]; + const failRehydrated = await RunState.fromString(failRoot, pausedFail.state.toString()); + const failedConversionResumed = await runner.run(failRoot, failRehydrated); + + // The version changes between the interruption and the resume, and the agent the resumed run uses must + // have the new one, as a fresh clone does. + const versions: string[] = []; + const versionedAgent = new VersionedAgent( + { + name: 'VersionedAgent', + model: 'gpt-4o-mini', + instructions: (_runContext, current) => { + versions.push((current as VersionedAgent).version); + return 'Set the value.'; + }, + tools: [setAfter], + }, + 'before' + ); + const pausedVersioned = await runner.run(versionedAgent, 'go', { context: { original: 'versioned', writes: [] } }); + pausedVersioned.state.approve(pausedVersioned.interruptions[0]!); + versionedAgent.setVersion('after'); + const versioned = await runner.run(versionedAgent, pausedVersioned.state); + return { callerAfter: ctx.after, resumedUsesCallerObject: result.runContext.context === ctx, @@ -500,5 +568,10 @@ export async function approvalResumeContextWorkflow(): Promise, @@ -19,7 +23,35 @@ export function convertAgent( modelNameOverride?: string, identities?: ReadonlyMap, Agent> ): Agent { - seen = seen ?? new Map(); + const replacements = new Map, Agent>(); + const converted = cloneAgentGraph( + agent, + modelParams, + seen ?? new Map(), + new Map(), + replacements, + modelNameOverride, + identities + ); + for (const [target, clone] of replacements) { + replaceFields(target, clone); + } + return converted; +} + +/** + * Converts `agent`'s graph like `convertAgent`, without changing the objects in `identities`. Each reused + * object is added to `replacements`, mapped to the clone whose fields it gets. + */ +function cloneAgentGraph( + agent: Agent, + modelParams: ModelActivityOptions, + seen: Map, Agent>, + clones: Map, Agent>, + replacements: Map, Agent>, + modelNameOverride: string | undefined, + identities: ReadonlyMap, Agent> | undefined +): Agent { if (seen.has(agent)) return seen.get(agent)!; // Reject raw functions: authoring mistake — use tool() or activityAsTool(). @@ -64,21 +96,36 @@ export function convertAgent( const clone = agent.clone({ model: activityBackedModel }); const identity = identities?.get(agent); - const converted = identity === undefined ? clone : replaceFields(identity, clone); + const converted = identity !== undefined && canReplaceFields(identity, clone) ? identity : clone; seen.set(agent, converted); + clones.set(agent, clone); - const convertedHandoffs = (agent.handoffs ?? []).map((h) => { + const convertHandoffAgent = (handoffAgent: Agent) => + cloneAgentGraph(handoffAgent, modelParams, seen, clones, replacements, modelNameOverride, identities); + clone.handoffs = (agent.handoffs ?? []).map((h) => { if (h instanceof Handoff) { - const convertedHandoffAgent = convertAgent(h.agent, modelParams, seen, modelNameOverride, identities); - return h.clone({ agent: convertedHandoffAgent }); + const convertedHandoffAgent = convertHandoffAgent(h.agent); + // A reused target still has its earlier fields here, so take the name from its new clone. + return h.clone({ agent: convertedHandoffAgent, agentName: clones.get(h.agent)!.name }); } - return convertAgent(h, modelParams, seen, modelNameOverride, identities); + return convertHandoffAgent(h); }); - converted.handoffs = convertedHandoffs; + if (converted !== clone) replacements.set(converted, clone); return converted; } +/** + * Whether `target` can take `source`'s fields. `Agent.clone` and `SandboxAgent.clone` return instances of + * exactly those classes, which keep all their state in own properties. + */ +function canReplaceFields(target: Agent, source: Agent): boolean { + const plainPrototypes: readonly object[] = [Agent.prototype, SandboxAgent.prototype]; + return ( + plainPrototypes.includes(Object.getPrototypeOf(target)) && plainPrototypes.includes(Object.getPrototypeOf(source)) + ); +} + /** Gives `target` exactly the own properties and prototype of `source`, and returns `target`. */ function replaceFields(target: Agent, source: Agent): Agent { for (const key of Reflect.ownKeys(target)) { From 3747794a19425f3397e579f3ef7ef2e103219c0e Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Sat, 31 Oct 2026 11:16:22 -0700 Subject: [PATCH 10/15] Address review findings --- .../src/__tests__/test-openai-agents.ts | 21 ++++ .../src/__tests__/workflows/openai-agents.ts | 97 ++++++++++++++++++- .../src/workflow/convert-agent.ts | 31 ++++-- .../openai-agents/src/workflow/runner.ts | 39 +++++--- 4 files changed, 167 insertions(+), 21 deletions(-) diff --git a/typescript/openai-agents/src/__tests__/test-openai-agents.ts b/typescript/openai-agents/src/__tests__/test-openai-agents.ts index 64edea58..d84751cf 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-agents.ts @@ -565,6 +565,17 @@ test('Approval resume keeps the caller context object', async (t) => { textResponse('done failed conversion'), toolCallResponse('set_after', { value: 'V' }, 'call_versioned'), textResponse('done versioned'), + handoffResponse('transfer_to_CustomChild'), + toolCallResponse('set_after', { value: 'C-1' }, 'call_custom_child'), + textResponse('done custom child'), + handoffResponse('transfer_to_ReplacedChild'), + toolCallResponse('set_after', { value: 'C-2' }, 'call_replaced_child'), + textResponse('done replaced child'), + handoffResponse('transfer_to_RestoreChild'), + toolCallResponse('set_after', { value: 'D-1' }, 'call_restore_child'), + textResponse('done restore child'), + toolCallResponse('set_after', { value: 'W' }, 'call_registered'), + textResponse('done registered'), ]), }), ], @@ -606,5 +617,15 @@ test('Approval resume keeps the caller context object', async (t) => { failedConversionInterruptions: 0, versions: ['before', 'after'], lastAgentVersion: 'after', + customChildWrites: ['C-1'], + customChildInterruptions: 0, + customChildSavedApprovalOwners: 1, + replacedChildWrites: ['C-2'], + replacedChildInterruptions: 0, + failedRestoreThrew: true, + failedRestoreWrites: ['D-1'], + failedRestoreInterruptions: 0, + registeredVersions: ['before', 'after'], + lastAgentRegisteredVersion: 'after', }); }); diff --git a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts index ffd1ba2a..4bdacacd 100644 --- a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts @@ -213,6 +213,16 @@ export interface ApprovalResumeContextResult { failedConversionInterruptions: number; versions: string[]; lastAgentVersion: string | undefined; + customChildWrites: string[]; + customChildInterruptions: number; + customChildSavedApprovalOwners: number; + replacedChildWrites: string[]; + replacedChildInterruptions: number; + failedRestoreThrew: boolean; + failedRestoreWrites: string[]; + failedRestoreInterruptions: number; + registeredVersions: Array; + lastAgentRegisteredVersion: string | undefined; } /** An agent whose `clone` carries a private field, which can't be copied onto another object. */ @@ -237,6 +247,18 @@ class VersionedAgent extends Agent { } } +/** Versions that `RegisteredVersionAgent.clone` registers for each clone. */ +const registeredVersions = new WeakMap, string>(); + +/** An agent whose `clone` returns a plain `Agent` and registers its version outside the clone. */ +class RegisteredVersionAgent extends Agent { + override clone(config: Partial>): Agent { + const clone = super.clone(config); + registeredVersions.set(clone, registeredVersions.get(this)!); + return clone; + } +} + /** Serialized approval record, as the `RunContext` internal below returns it. */ interface ApprovalRecord { approved: boolean | string[]; @@ -272,8 +294,10 @@ function countApprovalOwners(context: unknown): { approvalOwners: number; approv * rehydrates the saved state onto the original agent and context. Then a handoff changes between an * interruption and its resume. Then an agent's tools and model settings are replaced with equal copies * before a resume, and the approved tool changes them. Then a resume fails to convert a handoff target, - * and the saved state must still carry its approval. Last, an agent class whose clone keeps a private - * field changes that field before the resume. + * and the saved state must still carry its approval. Then an agent class whose clone keeps a private + * field changes that field before the resume. Then a resume replaces a handoff target's object, because + * its class overrides `clone` or the target itself was replaced. Last, a `clone` that returns a plain + * `Agent` keeps state outside it, which changes before the resume. */ export async function approvalResumeContextWorkflow(): Promise { const setAfter = tool({ @@ -544,6 +568,65 @@ export async function approvalResumeContextWorkflow(): Promise({ name: 'CustomRoot', model: 'gpt-4o-mini', handoffs: [customChild] }); + const customChildCtx: ResumeContext = { original: 'custom-child', writes: [] }; + const pausedCustomChild = await runner.run(customRoot, 'go', { context: customChildCtx }); + pausedCustomChild.state.approve(pausedCustomChild.interruptions[0]!); + const customChildResumed = await runner.run(customRoot, pausedCustomChild.state); + const customChildSaved = await RunState.fromString(customRoot, pausedCustomChild.state.toString()); + + // The handoff target is replaced by another agent with the same name before the resume. + const replacedChild = new Agent({ name: 'ReplacedChild', model: 'gpt-4o-mini', tools: [setAfter] }); + const replacedRoot = new Agent({ + name: 'ReplacedRoot', + model: 'gpt-4o-mini', + handoffs: [replacedChild], + }); + const replacedChildCtx: ResumeContext = { original: 'replaced-child', writes: [] }; + const pausedReplacedChild = await runner.run(replacedRoot, 'go', { context: replacedChildCtx }); + pausedReplacedChild.state.approve(pausedReplacedChild.interruptions[0]!); + replacedRoot.handoffs = [replacedChild.clone({})]; + const replacedChildResumed = await runner.run(replacedRoot, pausedReplacedChild.state); + + // The handoff to the paused target is removed, so the restore throws after the conversion. Once the + // handoff is back, the state saved as a string must still carry the approval. + const restoreChild = new Agent({ name: 'RestoreChild', model: 'gpt-4o-mini', tools: [setAfter] }); + const restoreRoot = new Agent({ name: 'RestoreRoot', model: 'gpt-4o-mini', handoffs: [restoreChild] }); + const pausedRestore = await runner.run(restoreRoot, 'go', { context: { original: 'restore', writes: [] } }); + pausedRestore.state.approve(pausedRestore.interruptions[0]!); + restoreRoot.handoffs = []; + let failedRestoreThrew = false; + try { + await runner.run(restoreRoot, pausedRestore.state); + } catch { + failedRestoreThrew = true; + } + restoreRoot.handoffs = [restoreChild]; + const restoreRehydrated = await RunState.fromString(restoreRoot, pausedRestore.state.toString()); + const failedRestoreResumed = await runner.run(restoreRoot, restoreRehydrated); + + // The registered version changes between the interruption and the resume, and the agent the resumed run + // uses must have the new one, as a fresh clone does. + const versionsSeen: Array = []; + const registeredAgent = new RegisteredVersionAgent({ + name: 'RegisteredVersionAgent', + model: 'gpt-4o-mini', + instructions: (_runContext, current) => { + versionsSeen.push(registeredVersions.get(current)); + return 'Set the value.'; + }, + tools: [setAfter], + }); + registeredVersions.set(registeredAgent, 'before'); + const pausedRegistered = await runner.run(registeredAgent, 'go', { context: { original: 'registered', writes: [] } }); + pausedRegistered.state.approve(pausedRegistered.interruptions[0]!); + registeredVersions.set(registeredAgent, 'after'); + const registered = await runner.run(registeredAgent, pausedRegistered.state); + return { callerAfter: ctx.after, resumedUsesCallerObject: result.runContext.context === ctx, @@ -573,5 +656,15 @@ export async function approvalResumeContextWorkflow(): Promise, modelParams: ModelActivityOptions, seen?: Map, Agent>, modelNameOverride?: string, - identities?: ReadonlyMap, Agent> + identities?: ReadonlyMap, Agent>, + previousFields?: Map, Agent> ): Agent { const replacements = new Map, Agent>(); const converted = cloneAgentGraph( @@ -34,11 +37,20 @@ export function convertAgent( identities ); for (const [target, clone] of replacements) { + const previous = Object.create(Object.getPrototypeOf(target), Object.getOwnPropertyDescriptors(target)); + previousFields?.set(target, previous); replaceFields(target, clone); } return converted; } +/** Gives each agent `convertAgent` changed the fields it had before, as recorded in `previousFields`. */ +export function restoreFields(previousFields: ReadonlyMap, Agent>): void { + for (const [target, previous] of previousFields) { + replaceFields(target, previous); + } +} + /** * Converts `agent`'s graph like `convertAgent`, without changing the objects in `identities`. Each reused * object is added to `replacements`, mapped to the clone whose fields it gets. @@ -96,7 +108,7 @@ function cloneAgentGraph( const clone = agent.clone({ model: activityBackedModel }); const identity = identities?.get(agent); - const converted = identity !== undefined && canReplaceFields(identity, clone) ? identity : clone; + const converted = identity !== undefined && canReplaceFields(agent, identity, clone) ? identity : clone; seen.set(agent, converted); clones.set(agent, clone); @@ -116,13 +128,16 @@ function cloneAgentGraph( } /** - * Whether `target` can take `source`'s fields. `Agent.clone` and `SandboxAgent.clone` return instances of - * exactly those classes, which keep all their state in own properties. + * Whether `target` can take the fields of `clone`, which `agent.clone` returned. `Agent.clone` and + * `SandboxAgent.clone` return instances of exactly those classes, which keep all their state in own + * properties. Any other `clone`, even one that returns such an instance, can keep state elsewhere. */ -function canReplaceFields(target: Agent, source: Agent): boolean { +function canReplaceFields(agent: Agent, target: Agent, clone: Agent): boolean { const plainPrototypes: readonly object[] = [Agent.prototype, SandboxAgent.prototype]; return ( - plainPrototypes.includes(Object.getPrototypeOf(target)) && plainPrototypes.includes(Object.getPrototypeOf(source)) + (agent.clone === Agent.prototype.clone || agent.clone === SandboxAgent.prototype.clone) && + plainPrototypes.includes(Object.getPrototypeOf(target)) && + plainPrototypes.includes(Object.getPrototypeOf(clone)) ); } diff --git a/typescript/openai-agents/src/workflow/runner.ts b/typescript/openai-agents/src/workflow/runner.ts index 32d84002..4d6bce93 100644 --- a/typescript/openai-agents/src/workflow/runner.ts +++ b/typescript/openai-agents/src/workflow/runner.ts @@ -27,7 +27,7 @@ import { type SerializableModelActivityOptions, } from '../common/model-activity-options'; import { unwrapTemporalFailure } from '../common/errors'; -import { convertAgent } from './convert-agent'; +import { convertAgent, restoreFields } from './convert-agent'; import { TemporalSandboxClient } from './sandbox-client'; import { ensureTracingProcessorRegistered } from './tracing'; import { flushOpenSpans } from './agent-sink-processor'; @@ -57,7 +57,8 @@ const resumeConversions = new WeakMap, Conversion>>( /** * Converts `agent`'s graph. With `into`, the conversion reuses `into`'s agent objects: each one gets the - * fields a fresh conversion would give it, and keeps its identity. + * fields a fresh conversion would give it, and keeps its identity. The returned `undo` gives `into` back + * the agents and fields it had before. */ function convertAgentGraph( agent: Agent, @@ -65,18 +66,25 @@ function convertAgentGraph( modelOverride: string | undefined, sandbox: SandboxRunConfig | undefined, into?: Conversion -): Conversion { +): { conversion: Conversion; undo: () => void } { const convertedBySource = new Map, Agent>(); - const converted = convertAgent(agent, modelParams, convertedBySource, modelOverride, into?.agents); + const previousFields = new Map, Agent>(); + const converted = convertAgent(agent, modelParams, convertedBySource, modelOverride, into?.agents, previousFields); if (sandbox !== undefined) { propagateAgentToolSandboxConfig(converted, sandbox); } const conversion = into ?? { source: agent, agents: new Map(), activeRuns: 0 }; + const previousAgents = new Map(conversion.agents); for (const [source, convertedAgent] of convertedBySource) { conversion.agents.set(source, convertedAgent); conversions.set(convertedAgent, conversion); } - return conversion; + const undo = () => { + restoreFields(previousFields); + conversion.agents.clear(); + for (const [source, convertedAgent] of previousAgents) conversion.agents.set(source, convertedAgent); + }; + return { conversion, undo }; } /** @@ -342,7 +350,10 @@ export class TemporalOpenAIRunner { const { model: modelOverride, ...runnerConfigOverrides } = options?.runConfig ?? {}; const sandbox = options?.runConfig?.sandbox; - const conversion = convertAgentGraph( + // Serialize the state before converting: a reused agent can then point at new objects for its + // handoffs, which the state's approvals aren't keyed by, so serializing after would drop them. + const serializedState = input instanceof RunState ? input.toString() : undefined; + const { conversion, undo } = convertAgentGraph( agent, this.modelParams, modelOverride, @@ -366,11 +377,17 @@ export class TemporalOpenAIRunner { // environment references on the Workflow thread rather than in the resume Activity. // Pass the state's own context, as the upstream Runner uses it, so resumed tools write the // caller's object rather than a JSON copy. - const restored = (await RunState.fromStringWithContext( - converted, - input.toString(), - input._context - )) as RunState; + let restored: RunState; + try { + restored = (await RunState.fromStringWithContext(converted, serializedState!, input._context)) as RunState< + TContext, + TAgent + >; + } catch (error) { + // The state's approvals weren't merged under the new graph, so keep the graph that owns them. + undo(); + throw error; + } // Suppress upstream Runner.run's withTrace(state._trace, ...) branch; we want the current Workflow's trace. restored.clearTrace(); preparedInput = restored; From 5aa3fe153803265593827065dec7ce87a5e74496 Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Sat, 31 Oct 2026 11:43:30 -0700 Subject: [PATCH 11/15] Address review findings --- .../src/__tests__/test-openai-agents.ts | 17 +++ .../src/__tests__/workflows/openai-agents.ts | 111 +++++++++++++++- .../src/workflow/convert-agent.ts | 121 ++++++++++-------- .../openai-agents/src/workflow/runner.ts | 43 +++---- 4 files changed, 209 insertions(+), 83 deletions(-) diff --git a/typescript/openai-agents/src/__tests__/test-openai-agents.ts b/typescript/openai-agents/src/__tests__/test-openai-agents.ts index d84751cf..4c6e4120 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-agents.ts @@ -576,6 +576,14 @@ test('Approval resume keeps the caller context object', async (t) => { textResponse('done restore child'), toolCallResponse('set_after', { value: 'W' }, 'call_registered'), textResponse('done registered'), + handoffResponse('transfer_to_RejectCustomChild'), + toolCallResponse('rejectable', { value: 'J-1' }, 'call_reject_custom_child'), + textResponse('done rejected custom child'), + handoffResponse('transfer_to_RejectChild'), + toolCallResponse('rejectable', { value: 'J-2' }, 'call_reject_child'), + textResponse('done rejected child'), + toolCallResponse('guarded', { value: 'G' }, 'call_guarded'), + textResponse('done guarded'), ]), }), ], @@ -627,5 +635,14 @@ test('Approval resume keeps the caller context object', async (t) => { failedRestoreInterruptions: 0, registeredVersions: ['before', 'after'], lastAgentRegisteredVersion: 'after', + rejectedResumesThrew: [true, true], + rejectedExecutions: ['J-1', 'J-2'], + rejectedInterruptions: 0, + failedRestores: 3, + failedRestoreApprovalOwners: 2, + failedRestoreApprovedCallIds: 2, + recoveredWrites: ['G'], + recoveredApprovalOwners: 2, + recoveredApprovedCallIds: 2, }); }); diff --git a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts index 4bdacacd..5fbbac7f 100644 --- a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts @@ -223,6 +223,15 @@ export interface ApprovalResumeContextResult { failedRestoreInterruptions: number; registeredVersions: Array; lastAgentRegisteredVersion: string | undefined; + rejectedResumesThrew: boolean[]; + rejectedExecutions: string[]; + rejectedInterruptions: number; + failedRestores: number; + failedRestoreApprovalOwners: number; + failedRestoreApprovedCallIds: number; + recoveredWrites: string[]; + recoveredApprovalOwners: number; + recoveredApprovedCallIds: number; } /** An agent whose `clone` carries a private field, which can't be copied onto another object. */ @@ -297,7 +306,10 @@ function countApprovalOwners(context: unknown): { approvalOwners: number; approv * and the saved state must still carry its approval. Then an agent class whose clone keeps a private * field changes that field before the resume. Then a resume replaces a handoff target's object, because * its class overrides `clone` or the target itself was replaced. Last, a `clone` that returns a plain - * `Agent` keeps state outside it, which changes before the resume. + * `Agent` keeps state outside it, which changes before the resume. Then an approved call fails after + * the resume replaced its agent's object, and is rejected through the original state, whose retry must + * not run it. Last, the restore of a saved state fails repeatedly after merging its approvals, which must + * not add a copy of them each time. */ export async function approvalResumeContextWorkflow(): Promise { const setAfter = tool({ @@ -627,6 +639,94 @@ export async function approvalResumeContextWorkflow(): Promise { + const { value } = args as { value: string }; + const firstRun = !rejectedExecutions.includes(value); + rejectedExecutions.push(value); + if (firstRun) throw new Error('rejectable failed'); + return 'ok'; + }, + }); + const rejectAfterFailedResume = async (child: Agent, replacement?: Agent) => { + const root = new Agent({ name: `${child.name}Root`, model: 'gpt-4o-mini', handoffs: [child] }); + const paused = await runner.run(root, 'go', { context: { original: child.name, writes: [] } }); + const interruption = paused.interruptions[0]!; + paused.state.approve(interruption); + if (replacement !== undefined) root.handoffs = [replacement]; + let threw = false; + try { + await runner.run(root, paused.state); + } catch { + threw = true; + } + paused.state.reject(interruption, { alwaysReject: true }); + const retried = await runner.run(root, paused.state); + return { threw, interruptions: retried.interruptions.length }; + }; + const rejectChild = new Agent({ name: 'RejectChild', model: 'gpt-4o-mini', tools: [rejectable] }); + const rejected = [ + await rejectAfterFailedResume( + new VersionedAgent({ name: 'RejectCustomChild', model: 'gpt-4o-mini', tools: [rejectable] }, 'v') + ), + await rejectAfterFailedResume(rejectChild, rejectChild.clone({})), + ]; + + // A saved state is rehydrated onto the original agent. Its restore merges the approvals, then fails in a + // tool's isEnabled check, three times. Neither the failures nor the resume that then succeeds may add a + // copy of the approvals. + const guard = { fail: false }; + const guarded = tool({ + name: 'guarded', + description: 'Record a value after approval.', + parameters: { + type: 'object' as const, + properties: { value: { type: 'string' } }, + required: ['value'] as const, + additionalProperties: false as const, + }, + needsApproval: true, + isEnabled: async () => { + if (guard.fail) throw new Error('isEnabled failed'); + return true; + }, + execute: async (args, runContext) => { + (runContext!.context as ResumeContext).writes.push((args as { value: string }).value); + return 'ok'; + }, + }); + const guardedAgent = new Agent({ name: 'GuardedAgent', model: 'gpt-4o-mini', tools: [guarded] }); + const pausedGuarded = await runner.run(guardedAgent, 'go', { context: { original: 'guarded', writes: [] } }); + pausedGuarded.state.approve(pausedGuarded.interruptions[0]!); + const guardedState = await RunState.fromString(guardedAgent, pausedGuarded.state.toString()); + guard.fail = true; + let failedRestores = 0; + for (let attempt = 0; attempt < 3; attempt++) { + try { + await runner.run(guardedAgent, guardedState); + } catch { + failedRestores++; + } + } + const failedRestoreOwners = countApprovalOwners(guardedState._context); + guard.fail = false; + const recovered = await runner.run(guardedAgent, guardedState); + const recoveredOwners = countApprovalOwners(guardedState._context); + return { callerAfter: ctx.after, resumedUsesCallerObject: result.runContext.context === ctx, @@ -666,5 +766,14 @@ export async function approvalResumeContextWorkflow(): Promise threw), + rejectedExecutions, + rejectedInterruptions: rejected.reduce((count, { interruptions }) => count + interruptions, 0), + failedRestores, + failedRestoreApprovalOwners: failedRestoreOwners.approvalOwners, + failedRestoreApprovedCallIds: failedRestoreOwners.approvedCallIds, + recoveredWrites: recovered.runContext.context.writes, + recoveredApprovalOwners: recoveredOwners.approvalOwners, + recoveredApprovedCallIds: recoveredOwners.approvedCallIds, }; } diff --git a/typescript/openai-agents/src/workflow/convert-agent.ts b/typescript/openai-agents/src/workflow/convert-agent.ts index b8d5afd3..42aa4181 100644 --- a/typescript/openai-agents/src/workflow/convert-agent.ts +++ b/typescript/openai-agents/src/workflow/convert-agent.ts @@ -12,59 +12,52 @@ import { ActivityBackedModel } from './activity-backed-model'; * When `identities` maps a source agent to an agent an earlier conversion returned, that object is * returned again, with the fields of this conversion's clone. Each agent then has the same fields as * after a fresh conversion, but keeps its object identity, which approvals on a `RunContext` are keyed by. - * Those objects only change once the whole graph has converted, so a conversion that throws leaves them - * as they were. With `previousFields`, each changed object is mapped to a copy of its earlier fields, which - * `restoreFields` puts back. An agent whose class overrides `clone` gets its fresh clone instead, since its - * clone can hold state, such as private fields or entries keyed by the clone, that can't be moved onto - * another object. + * An object is only reused if its handoffs lead to the same objects as before, so a state saved on the + * earlier graph still reaches every agent it reached, and serializes the decisions those agents own. + * Other agents, and agents whose class overrides `clone`, get their fresh clone. Such a clone can hold + * state, such as private fields or entries keyed by the clone, that can't be moved onto another object. + * The objects only change once the whole graph has converted, so a conversion that throws leaves them + * as they were. */ export function convertAgent( agent: Agent, modelParams: ModelActivityOptions, seen?: Map, Agent>, modelNameOverride?: string, - identities?: ReadonlyMap, Agent>, - previousFields?: Map, Agent> + identities?: ReadonlyMap, Agent> ): Agent { - const replacements = new Map, Agent>(); - const converted = cloneAgentGraph( - agent, - modelParams, - seen ?? new Map(), - new Map(), - replacements, - modelNameOverride, - identities - ); - for (const [target, clone] of replacements) { - const previous = Object.create(Object.getPrototypeOf(target), Object.getOwnPropertyDescriptors(target)); - previousFields?.set(target, previous); - replaceFields(target, clone); + seen = seen ?? new Map(); + if (seen.has(agent)) return seen.get(agent)!; + const clones = new Map, Agent>(); + cloneAgentGraph(agent, modelParams, seen, clones, modelNameOverride); + const reused = reusableIdentities(clones, identities); + for (const [source, clone] of clones) { + seen.set(source, reused.get(source) ?? clone); } - return converted; -} - -/** Gives each agent `convertAgent` changed the fields it had before, as recorded in `previousFields`. */ -export function restoreFields(previousFields: ReadonlyMap, Agent>): void { - for (const [target, previous] of previousFields) { - replaceFields(target, previous); + for (const [source, clone] of clones) { + clone.handoffs = (source.handoffs ?? []).map((h) => { + const handoffAgent = h instanceof Handoff ? h.agent : h; + const converted = seen.get(handoffAgent)!; + if (!(h instanceof Handoff)) return converted; + // A reused target still has its earlier fields here, so take the name from its new clone. + return h.clone({ agent: converted, agentName: (clones.get(handoffAgent) ?? converted).name }); + }); } + for (const [source, target] of reused) { + replaceFields(target, clones.get(source)!); + } + return seen.get(agent)!; } -/** - * Converts `agent`'s graph like `convertAgent`, without changing the objects in `identities`. Each reused - * object is added to `replacements`, mapped to the clone whose fields it gets. - */ +/** Validates and clones each agent in `agent`'s graph that isn't in `seen`, into `clones`. */ function cloneAgentGraph( agent: Agent, modelParams: ModelActivityOptions, - seen: Map, Agent>, + seen: ReadonlyMap, Agent>, clones: Map, Agent>, - replacements: Map, Agent>, - modelNameOverride: string | undefined, - identities: ReadonlyMap, Agent> | undefined -): Agent { - if (seen.has(agent)) return seen.get(agent)!; + modelNameOverride: string | undefined +): void { + if (seen.has(agent) || clones.has(agent)) return; // Reject raw functions: authoring mistake — use tool() or activityAsTool(). const tools = agent.tools ?? []; @@ -106,25 +99,45 @@ function cloneAgentGraph( // user-declared name and instructions. activityBackedModel.setAgent(agent); - const clone = agent.clone({ model: activityBackedModel }); - const identity = identities?.get(agent); - const converted = identity !== undefined && canReplaceFields(agent, identity, clone) ? identity : clone; - seen.set(agent, converted); - clones.set(agent, clone); + clones.set(agent, agent.clone({ model: activityBackedModel })); + for (const h of agent.handoffs ?? []) { + cloneAgentGraph(h instanceof Handoff ? h.agent : h, modelParams, seen, clones, modelNameOverride); + } +} - const convertHandoffAgent = (handoffAgent: Agent) => - cloneAgentGraph(handoffAgent, modelParams, seen, clones, replacements, modelNameOverride, identities); - clone.handoffs = (agent.handoffs ?? []).map((h) => { - if (h instanceof Handoff) { - const convertedHandoffAgent = convertHandoffAgent(h.agent); - // A reused target still has its earlier fields here, so take the name from its new clone. - return h.clone({ agent: convertedHandoffAgent, agentName: clones.get(h.agent)!.name }); +/** + * The objects in `identities` this conversion reuses, keyed by source agent: those whose fields + * `canReplaceFields` allows replacing, and whose handoffs, once converted, lead to the same objects as + * before. Rewiring a reused object to a new one would hide the agent it led to from states saved on the + * earlier graph, which then lose that agent's approvals and rejections when serialized. + */ +function reusableIdentities( + clones: ReadonlyMap, Agent>, + identities: ReadonlyMap, Agent> | undefined +): Map, Agent> { + const reused = new Map, Agent>(); + for (const [source, clone] of clones) { + const identity = identities?.get(source); + if (identity !== undefined && canReplaceFields(source, identity, clone)) reused.set(source, identity); + } + // Dropping an agent can change the targets of agents that hand off to it, so repeat until none changes. + let dropped = true; + while (dropped) { + dropped = false; + for (const [source, identity] of reused) { + const before = new Set(handoffAgents(identity)); + const after = new Set(handoffAgents(source).map((target) => reused.get(target))); + if (after.size !== before.size || [...after].some((target) => target === undefined || !before.has(target))) { + reused.delete(source); + dropped = true; + } } - return convertHandoffAgent(h); - }); + } + return reused; +} - if (converted !== clone) replacements.set(converted, clone); - return converted; +function handoffAgents(agent: Agent): Agent[] { + return (agent.handoffs ?? []).map((h) => (h instanceof Handoff ? h.agent : h)); } /** diff --git a/typescript/openai-agents/src/workflow/runner.ts b/typescript/openai-agents/src/workflow/runner.ts index 4d6bce93..b706793a 100644 --- a/typescript/openai-agents/src/workflow/runner.ts +++ b/typescript/openai-agents/src/workflow/runner.ts @@ -27,7 +27,7 @@ import { type SerializableModelActivityOptions, } from '../common/model-activity-options'; import { unwrapTemporalFailure } from '../common/errors'; -import { convertAgent, restoreFields } from './convert-agent'; +import { convertAgent } from './convert-agent'; import { TemporalSandboxClient } from './sandbox-client'; import { ensureTracingProcessorRegistered } from './tracing'; import { flushOpenSpans } from './agent-sink-processor'; @@ -56,9 +56,10 @@ const conversions = new WeakMap, Conversion>(); const resumeConversions = new WeakMap, Conversion>>(); /** - * Converts `agent`'s graph. With `into`, the conversion reuses `into`'s agent objects: each one gets the - * fields a fresh conversion would give it, and keeps its identity. The returned `undo` gives `into` back - * the agents and fields it had before. + * Converts `agent`'s graph. With `into`, the conversion reuses those of `into`'s agent objects that + * `convertAgent` can: each one gets the fields a fresh conversion would give it, and keeps its identity. + * `into` then maps each source agent to its latest converted agent. It keeps them even if the resume + * fails, since the restore may already have merged the state's approvals under them. */ function convertAgentGraph( agent: Agent, @@ -66,25 +67,18 @@ function convertAgentGraph( modelOverride: string | undefined, sandbox: SandboxRunConfig | undefined, into?: Conversion -): { conversion: Conversion; undo: () => void } { +): Conversion { const convertedBySource = new Map, Agent>(); - const previousFields = new Map, Agent>(); - const converted = convertAgent(agent, modelParams, convertedBySource, modelOverride, into?.agents, previousFields); + const converted = convertAgent(agent, modelParams, convertedBySource, modelOverride, into?.agents); if (sandbox !== undefined) { propagateAgentToolSandboxConfig(converted, sandbox); } const conversion = into ?? { source: agent, agents: new Map(), activeRuns: 0 }; - const previousAgents = new Map(conversion.agents); for (const [source, convertedAgent] of convertedBySource) { conversion.agents.set(source, convertedAgent); conversions.set(convertedAgent, conversion); } - const undo = () => { - restoreFields(previousFields); - conversion.agents.clear(); - for (const [source, convertedAgent] of previousAgents) conversion.agents.set(source, convertedAgent); - }; - return { conversion, undo }; + return conversion; } /** @@ -350,10 +344,9 @@ export class TemporalOpenAIRunner { const { model: modelOverride, ...runnerConfigOverrides } = options?.runConfig ?? {}; const sandbox = options?.runConfig?.sandbox; - // Serialize the state before converting: a reused agent can then point at new objects for its - // handoffs, which the state's approvals aren't keyed by, so serializing after would drop them. + // Serialize the state before converting, from the graph it was saved on, which the conversion changes. const serializedState = input instanceof RunState ? input.toString() : undefined; - const { conversion, undo } = convertAgentGraph( + const conversion = convertAgentGraph( agent, this.modelParams, modelOverride, @@ -377,17 +370,11 @@ export class TemporalOpenAIRunner { // environment references on the Workflow thread rather than in the resume Activity. // Pass the state's own context, as the upstream Runner uses it, so resumed tools write the // caller's object rather than a JSON copy. - let restored: RunState; - try { - restored = (await RunState.fromStringWithContext(converted, serializedState!, input._context)) as RunState< - TContext, - TAgent - >; - } catch (error) { - // The state's approvals weren't merged under the new graph, so keep the graph that owns them. - undo(); - throw error; - } + const restored = (await RunState.fromStringWithContext( + converted, + serializedState!, + input._context + )) as RunState; // Suppress upstream Runner.run's withTrace(state._trace, ...) branch; we want the current Workflow's trace. restored.clearTrace(); preparedInput = restored; From 1c511efbd8a470c260c36dfb058c04bec129b54e Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Sat, 31 Oct 2026 12:14:35 -0700 Subject: [PATCH 12/15] Address review findings --- .../src/__tests__/test-openai-agents.ts | 9 ++- .../src/__tests__/workflows/openai-agents.ts | 32 +++++++++- .../openai-agents/src/workflow/runner.ts | 60 +++++++++++++++++-- 3 files changed, 92 insertions(+), 9 deletions(-) diff --git a/typescript/openai-agents/src/__tests__/test-openai-agents.ts b/typescript/openai-agents/src/__tests__/test-openai-agents.ts index 4c6e4120..5a013d9f 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-agents.ts @@ -582,6 +582,11 @@ test('Approval resume keeps the caller context object', async (t) => { handoffResponse('transfer_to_RejectChild'), toolCallResponse('rejectable', { value: 'J-2' }, 'call_reject_child'), textResponse('done rejected child'), + handoffResponse('transfer_to_AlwaysChild'), + toolCallResponse('rejectable', { value: 'J-3' }, 'call_always_child'), + textResponse('done always child'), + toolCallResponse('rejectable', { value: 'J-4' }, 'call_persisted'), + textResponse('done persisted'), toolCallResponse('guarded', { value: 'G' }, 'call_guarded'), textResponse('done guarded'), ]), @@ -635,8 +640,8 @@ test('Approval resume keeps the caller context object', async (t) => { failedRestoreInterruptions: 0, registeredVersions: ['before', 'after'], lastAgentRegisteredVersion: 'after', - rejectedResumesThrew: [true, true], - rejectedExecutions: ['J-1', 'J-2'], + rejectedResumesThrew: [true, true, true, true], + rejectedExecutions: ['J-1', 'J-2', 'J-3', 'J-4'], rejectedInterruptions: 0, failedRestores: 3, failedRestoreApprovalOwners: 2, diff --git a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts index 5fbbac7f..7a4fd5b2 100644 --- a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts @@ -641,7 +641,9 @@ export async function approvalResumeContextWorkflow(): Promise, replacement?: Agent) => { + const rejectAfterFailedResume = async ( + child: Agent, + replacement?: Agent, + alwaysApprove = false + ) => { const root = new Agent({ name: `${child.name}Root`, model: 'gpt-4o-mini', handoffs: [child] }); const paused = await runner.run(root, 'go', { context: { original: child.name, writes: [] } }); const interruption = paused.interruptions[0]!; - paused.state.approve(interruption); + paused.state.approve(interruption, { alwaysApprove }); if (replacement !== undefined) root.handoffs = [replacement]; let threw = false; try { @@ -679,12 +685,32 @@ export async function approvalResumeContextWorkflow(): Promise({ name: 'RejectChild', model: 'gpt-4o-mini', tools: [rejectable] }); + const alwaysChild = new Agent({ name: 'AlwaysChild', model: 'gpt-4o-mini', tools: [rejectable] }); const rejected = [ await rejectAfterFailedResume( new VersionedAgent({ name: 'RejectCustomChild', model: 'gpt-4o-mini', tools: [rejectable] }, 'v') ), await rejectAfterFailedResume(rejectChild, rejectChild.clone({})), + await rejectAfterFailedResume(alwaysChild, alwaysChild.clone({}), true), ]; + const persistedAgent = new Agent({ + name: 'PersistedAgent', + model: 'gpt-4o-mini', + tools: [rejectable], + }); + const pausedPersisted = await runner.run(persistedAgent, 'go', { context: { original: 'persisted', writes: [] } }); + const persistedState = await RunState.fromString(persistedAgent, pausedPersisted.state.toString()); + const persistedInterruption = persistedState.getInterruptions()[0]!; + persistedState.approve(persistedInterruption, { alwaysApprove: true }); + let persistedThrew = false; + try { + await runner.run(persistedAgent, persistedState); + } catch { + persistedThrew = true; + } + persistedState.reject(persistedInterruption, { alwaysReject: true }); + const persistedRetried = await runner.run(persistedAgent, persistedState); + rejected.push({ threw: persistedThrew, interruptions: persistedRetried.interruptions.length }); // A saved state is rehydrated onto the original agent. Its restore merges the approvals, then fails in a // tool's isEnabled check, three times. Neither the failures nor the resume that then succeeds may add a diff --git a/typescript/openai-agents/src/workflow/runner.ts b/typescript/openai-agents/src/workflow/runner.ts index b706793a..75dff7e6 100644 --- a/typescript/openai-agents/src/workflow/runner.ts +++ b/typescript/openai-agents/src/workflow/runner.ts @@ -83,19 +83,71 @@ function convertAgentGraph( /** * Returns the converted graph that `state` runs on, or else the one the latest resume on its context used, - * if it was converted from `agent` and no run is using it. + * if it was converted from `agent`, no run is using it, and it doesn't override one of the state's + * rejections. * * Function-tool approvals are keyed by agent object on the state's `RunContext`, which the resume keeps. * Rebuilding the state over new agent objects would add a copy of its approvals under every resume's * graph, and the earlier copies can't be dropped, because saved states that still point at those agents * read them. */ -function reusableConversion(state: RunState, agent: Agent): Conversion | undefined { +function reusableConversion( + state: RunState, + agent: Agent, + serializedState: string +): Conversion | undefined { const candidates = [ conversions.get(state._currentAgent as Agent), resumeConversions.get(state._context)?.get(agent), ]; - return candidates.find((conversion) => conversion?.source === agent && conversion.activeRuns === 0); + return candidates.find( + (conversion) => + conversion?.source === agent && + conversion.activeRuns === 0 && + !overridesRejection(conversion, serializedState, state._context) + ); +} + +/** A function-tool approval record, as a serialized `RunState` and `RunContext` store it. */ +interface ApprovalRecord { + approved: boolean | string[]; + rejected: boolean | string[]; +} + +interface FunctionApprovals { + agentIdentity: string; + approvals: Record; +} + +/** + * Whether one of `conversion`'s agents always approves a tool that the serialized state always rejects for + * the agent with its name. The restore merges the state's decisions into the decisions the agent already + * owns, and an approval for good wins that merge. So a state that always approved a tool, was resumed, and + * then always rejected it would run it again. A fresh graph owns only the state's decisions. + */ +function overridesRejection(conversion: Conversion, serializedState: string, context: object): boolean { + const stateApprovals: FunctionApprovals[] = JSON.parse(serializedState).context.functionApprovals ?? []; + const rejections = stateApprovals.flatMap(({ agentIdentity, approvals }) => + Object.entries(approvals) + .filter(([, record]) => record.rejected === true && record.approved !== true) + .map(([toolName]) => ({ agentIdentity, toolName })) + ); + if (rejections.length === 0) return false; + const names = new Map, string>(); + for (const [source, converted] of conversion.agents) names.set(converted, source.name); + // _toJSONForRunState is internal to @openai/agents-core; it serializes the approvals of each agent it's given. + const { functionApprovals = [] } = ( + context as { + _toJSONForRunState(keys: ReadonlyMap, string>): { functionApprovals?: FunctionApprovals[] }; + } + )._toJSONForRunState(names); + // A state's agent identity is its name, followed by `#n` if other agents in the graph share it. + return rejections.some(({ agentIdentity, toolName }) => + functionApprovals.some( + ({ agentIdentity: name, approvals }) => + (agentIdentity === name || agentIdentity.startsWith(`${name}#`)) && approvals[toolName]?.approved === true + ) + ); } function recordResumeConversion(state: RunState, agent: Agent, conversion: Conversion): void { @@ -351,7 +403,7 @@ export class TemporalOpenAIRunner { this.modelParams, modelOverride, sandbox, - input instanceof RunState ? reusableConversion(input, agent) : undefined + input instanceof RunState ? reusableConversion(input, agent, serializedState!) : undefined ); const converted = conversion.agents.get(agent)!; From 433b0ad5ba7b0e14385abbcb2c79ad956b787a73 Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Sat, 31 Oct 2026 12:31:24 -0700 Subject: [PATCH 13/15] Address review findings --- .../src/__tests__/test-openai-agents.ts | 16 +++ .../src/__tests__/workflows/openai-agents.ts | 66 ++++++++++++- .../openai-agents/src/workflow/runner.ts | 97 ++++++++++++++----- 3 files changed, 152 insertions(+), 27 deletions(-) diff --git a/typescript/openai-agents/src/__tests__/test-openai-agents.ts b/typescript/openai-agents/src/__tests__/test-openai-agents.ts index 5a013d9f..6ecbecfb 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-agents.ts @@ -589,6 +589,12 @@ test('Approval resume keeps the caller context object', async (t) => { textResponse('done persisted'), toolCallResponse('guarded', { value: 'G' }, 'call_guarded'), textResponse('done guarded'), + toolCallResponse('vetoed', { value: 'V-1' }, 'call_veto_persisted'), + textResponse('done veto persisted'), + toolCallResponse('vetoed', { value: 'V-2' }, 'call_veto_persisted_always'), + textResponse('done veto persisted always'), + toolCallResponse('vetoed', { value: 'V-3' }, 'call_veto_direct'), + textResponse('done veto direct'), ]), }), ], @@ -649,5 +655,15 @@ test('Approval resume keeps the caller context object', async (t) => { recoveredWrites: ['G'], recoveredApprovalOwners: 2, recoveredApprovedCallIds: 2, + rejectionMessages: [ + 'VetoPersisted before', + 'VetoPersisted after', + 'VetoPersistedAlways before', + 'VetoPersistedAlways after', + 'VetoDirect before', + 'VetoDirect after', + ], + revisedRejectionWrites: [], + revisedRejectionInterruptions: 0, }); }); diff --git a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts index 7a4fd5b2..728d7b25 100644 --- a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts @@ -1,4 +1,4 @@ -import { Agent, type AgentOptions, RunState, handoff, tool } from '@openai/agents-core'; +import { Agent, type AgentOptions, type CallModelInputFilter, RunState, handoff, tool } from '@openai/agents-core'; import { ApplicationFailure, condition } from '@temporalio/workflow'; import { WorkflowStream } from '@temporalio/workflow-streams/workflow'; import { TemporalOpenAIRunner, statelessMcpServer, statefulMcpServer } from '../../workflow'; @@ -232,6 +232,9 @@ export interface ApprovalResumeContextResult { recoveredWrites: string[]; recoveredApprovalOwners: number; recoveredApprovedCallIds: number; + rejectionMessages: string[]; + revisedRejectionWrites: string[]; + revisedRejectionInterruptions: number; } /** An agent whose `clone` carries a private field, which can't be copied onto another object. */ @@ -308,8 +311,9 @@ function countApprovalOwners(context: unknown): { approvalOwners: number; approv * its class overrides `clone` or the target itself was replaced. Last, a `clone` that returns a plain * `Agent` keeps state outside it, which changes before the resume. Then an approved call fails after * the resume replaced its agent's object, and is rejected through the original state, whose retry must - * not run it. Last, the restore of a saved state fails repeatedly after merging its approvals, which must - * not add a copy of them each time. + * not run it. Then the restore of a saved state fails repeatedly after merging its approvals, which must + * not add a copy of them each time. Last, a rejection message changes after a failed resume, and the retry + * must send the model the new one. */ export async function approvalResumeContextWorkflow(): Promise { const setAfter = tool({ @@ -753,6 +757,59 @@ export async function approvalResumeContextWorkflow(): Promise { + (runContext!.context as ResumeContext).writes.push((args as { value: string }).value); + return 'ok'; + }, + }); + const rejectionMessages: string[] = []; + const vetoedWrites: string[] = []; + const reviseRejection = async (name: string, rehydrate: boolean, alwaysReject: boolean) => { + const vetoAgent = new Agent({ name, model: 'gpt-4o-mini', tools: [vetoed] }); + const vetoCtx: ResumeContext = { original: name, writes: vetoedWrites }; + const paused = await runner.run(vetoAgent, 'go', { context: vetoCtx }); + const state = rehydrate ? await RunState.fromString(vetoAgent, paused.state.toString()) : paused.state; + const interruption = state.getInterruptions()[0]!; + let failModel = true; + const callModelInputFilter: CallModelInputFilter = ({ modelData }) => { + for (const item of modelData.input) { + if (item.type === 'function_call_result') { + const { output } = item; + rejectionMessages.push( + typeof output === 'string' ? output : 'text' in output ? output.text : JSON.stringify(output) + ); + } + } + if (failModel) { + failModel = false; + throw new Error('model failed'); + } + return modelData; + }; + state.reject(interruption, { alwaysReject, message: `${name} before` }); + await runner.run(vetoAgent, state, { callModelInputFilter }).catch(() => undefined); + state.reject(interruption, { alwaysReject, message: `${name} after` }); + return (await runner.run(vetoAgent, state, { callModelInputFilter })).interruptions.length; + }; + const revisedInterruptions = [ + await reviseRejection('VetoPersisted', true, false), + await reviseRejection('VetoPersistedAlways', true, true), + await reviseRejection('VetoDirect', false, false), + ]; + return { callerAfter: ctx.after, resumedUsesCallerObject: result.runContext.context === ctx, @@ -801,5 +858,8 @@ export async function approvalResumeContextWorkflow(): Promise count + interruptions, 0), }; } diff --git a/typescript/openai-agents/src/workflow/runner.ts b/typescript/openai-agents/src/workflow/runner.ts index 75dff7e6..de756bec 100644 --- a/typescript/openai-agents/src/workflow/runner.ts +++ b/typescript/openai-agents/src/workflow/runner.ts @@ -3,6 +3,7 @@ import { type AgentInputItem, Handoff, MemorySession, + RunContext, Runner, RunState, type AgentOutputType, @@ -83,8 +84,8 @@ function convertAgentGraph( /** * Returns the converted graph that `state` runs on, or else the one the latest resume on its context used, - * if it was converted from `agent`, no run is using it, and it doesn't override one of the state's - * rejections. + * if it was converted from `agent`, no run is using it, and restoring the state into it keeps the state's + * decisions and rejection messages. * * Function-tool approvals are keyed by agent object on the state's `RunContext`, which the resume keeps. * Rebuilding the state over new agent objects would add a copy of its approvals under every resume's @@ -104,7 +105,7 @@ function reusableConversion( (conversion) => conversion?.source === agent && conversion.activeRuns === 0 && - !overridesRejection(conversion, serializedState, state._context) + !losesStateDecisions(conversion, serializedState, state._context) ); } @@ -112,6 +113,8 @@ function reusableConversion( interface ApprovalRecord { approved: boolean | string[]; rejected: boolean | string[]; + messages?: Record; + stickyRejectMessage?: string; } interface FunctionApprovals { @@ -119,37 +122,83 @@ interface FunctionApprovals { approvals: Record; } +/** The `RunContext` internals of @openai/agents-core that read and merge per-agent approvals. */ +interface ApprovalContext { + _toJSONForRunState(keys: ReadonlyMap): { functionApprovals?: FunctionApprovals[] }; + _mergeFunctionApprovals(approvals: FunctionApprovals[], agentsByIdentity: ReadonlyMap): void; +} + /** - * Whether one of `conversion`'s agents always approves a tool that the serialized state always rejects for - * the agent with its name. The restore merges the state's decisions into the decisions the agent already - * owns, and an approval for good wins that merge. So a state that always approved a tool, was resumed, and - * then always rejected it would run it again. A fresh graph owns only the state's decisions. + * Whether the restore would lose one of the serialized state's decisions or rejection messages if it merged + * them into the decisions `conversion`'s agents already own. The merge keeps an approval for good over a + * rejection, and an existing rejection message over the state's. So a state that always approved a tool + * and then always rejected it would run it again, and a state whose rejection message changed after a + * failed resume would send the model the earlier message. A fresh graph owns only the state's decisions. */ -function overridesRejection(conversion: Conversion, serializedState: string, context: object): boolean { +function losesStateDecisions(conversion: Conversion, serializedState: string, context: object): boolean { const stateApprovals: FunctionApprovals[] = JSON.parse(serializedState).context.functionApprovals ?? []; - const rejections = stateApprovals.flatMap(({ agentIdentity, approvals }) => - Object.entries(approvals) - .filter(([, record]) => record.rejected === true && record.approved !== true) - .map(([toolName]) => ({ agentIdentity, toolName })) - ); - if (rejections.length === 0) return false; + if (stateApprovals.length === 0) return false; const names = new Map, string>(); for (const [source, converted] of conversion.agents) names.set(converted, source.name); - // _toJSONForRunState is internal to @openai/agents-core; it serializes the approvals of each agent it's given. - const { functionApprovals = [] } = ( - context as { - _toJSONForRunState(keys: ReadonlyMap, string>): { functionApprovals?: FunctionApprovals[] }; - } - )._toJSONForRunState(names); + const { functionApprovals: owned = [] } = (context as ApprovalContext)._toJSONForRunState(names); // A state's agent identity is its name, followed by `#n` if other agents in the graph share it. - return rejections.some(({ agentIdentity, toolName }) => - functionApprovals.some( - ({ agentIdentity: name, approvals }) => - (agentIdentity === name || agentIdentity.startsWith(`${name}#`)) && approvals[toolName]?.approved === true + return stateApprovals.some(({ agentIdentity, approvals }) => + owned.some( + ({ agentIdentity: name, approvals: ownedApprovals }) => + (agentIdentity === name || agentIdentity.startsWith(`${name}#`)) && + Object.entries(approvals).some( + ([toolName, record]) => + ownedApprovals[toolName] !== undefined && + !keepsDecisions(record, mergeApprovalRecords(ownedApprovals[toolName], record, toolName)) + ) ) ); } +/** Merges `incoming` into `current` as the restore does, in a scratch `RunContext`. */ +function mergeApprovalRecords(current: ApprovalRecord, incoming: ApprovalRecord, toolName: string): ApprovalRecord { + const owner = {}; + const scratch = new RunContext() as unknown as ApprovalContext; + for (const record of [current, incoming]) { + scratch._mergeFunctionApprovals( + [{ agentIdentity: 'owner', approvals: { [toolName]: record } }], + new Map([['owner', owner]]) + ); + } + return scratch._toJSONForRunState(new Map([[owner, 'owner']])).functionApprovals![0]!.approvals[toolName]!; +} + +/** Whether `merged` resolves every call that `record` decides, and its rejection messages, as `record` does. */ +function keepsDecisions(record: ApprovalRecord, merged: ApprovalRecord): boolean { + const callIds = new Set([ + undefined, + ...(Array.isArray(record.approved) ? record.approved : []), + ...(Array.isArray(record.rejected) ? record.rejected : []), + ...Object.keys(record.messages ?? {}), + ]); + return [...callIds].every((callId) => { + const decision = approvalDecision(record, callId); + if (decision === undefined) return true; + if (approvalDecision(merged, callId) !== decision) return false; + return decision || rejectionMessage(merged, callId) === rejectionMessage(record, callId); + }); +} + +/** The decision a record gives a call, or a call it doesn't name if `callId` is undefined, as agents-core resolves it. */ +function approvalDecision(record: ApprovalRecord, callId: string | undefined): boolean | undefined { + if (record.approved === true) return true; + if (record.rejected === true) return false; + if (callId !== undefined && Array.isArray(record.approved) && record.approved.includes(callId)) return true; + if (callId !== undefined && Array.isArray(record.rejected) && record.rejected.includes(callId)) return false; + return undefined; +} + +/** The rejection message a record gives a call, as agents-core resolves it. */ +function rejectionMessage(record: ApprovalRecord, callId: string | undefined): string | undefined { + if (callId !== undefined && typeof record.messages?.[callId] === 'string') return record.messages[callId]; + return record.rejected === true ? record.stickyRejectMessage : undefined; +} + function recordResumeConversion(state: RunState, agent: Agent, conversion: Conversion): void { let byAgent = resumeConversions.get(state._context); if (byAgent === undefined) { From b3bd4767b954035ea79f2fc948e884806f1861a0 Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Sat, 31 Oct 2026 12:52:30 -0700 Subject: [PATCH 14/15] Address review findings --- .../__tests__/stubs/openai-agents-fakes.ts | 6 +- .../src/__tests__/test-openai-agents.ts | 43 ++++++++- .../src/__tests__/workflows/openai-agents.ts | 88 ++++++++++++++++++- .../openai-agents/src/workflow/runner.ts | 80 +++++++++-------- 4 files changed, 177 insertions(+), 40 deletions(-) diff --git a/typescript/openai-agents/src/__tests__/stubs/openai-agents-fakes.ts b/typescript/openai-agents/src/__tests__/stubs/openai-agents-fakes.ts index 7e5e7989..bf3d8e15 100644 --- a/typescript/openai-agents/src/__tests__/stubs/openai-agents-fakes.ts +++ b/typescript/openai-agents/src/__tests__/stubs/openai-agents-fakes.ts @@ -161,12 +161,14 @@ export function handoffResponse(handoffToolName: string, args: Record }>): ModelResponse { +export function multiToolCallResponse( + calls: Array<{ name: string; args: Record; callId?: string }> +): ModelResponse { const output: AgentOutputItem[] = calls.map((c) => ({ type: 'function_call' as const, name: c.name, arguments: JSON.stringify(c.args), - callId: `call_fake_${c.name}`, + callId: c.callId ?? `call_fake_${c.name}`, status: 'completed' as const, })); return { output, usage: fakeUsage(20) }; diff --git a/typescript/openai-agents/src/__tests__/test-openai-agents.ts b/typescript/openai-agents/src/__tests__/test-openai-agents.ts index 6ecbecfb..d95ebe1b 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-agents.ts @@ -26,7 +26,13 @@ import { streamingNoTopicWorkflow, } from './workflows/openai-agents'; import { makeTestFunction } from './helpers/test-fn'; -import { FakeModelProvider, handoffResponse, textResponse, toolCallResponse } from './stubs/openai-agents'; +import { + FakeModelProvider, + handoffResponse, + multiToolCallResponse, + textResponse, + toolCallResponse, +} from './stubs/openai-agents'; import { StreamingFakeModelProvider, streamingTextEvents } from './stubs/openai-agents-fakes'; import EventType = temporal.api.enums.v1.EventType; @@ -595,6 +601,28 @@ test('Approval resume keeps the caller context object', async (t) => { textResponse('done veto persisted always'), toolCallResponse('vetoed', { value: 'V-3' }, 'call_veto_direct'), textResponse('done veto direct'), + multiToolCallResponse([ + { name: 'vetoed', args: { value: 'K-A' }, callId: 'call_batch_persisted_a' }, + { name: 'vetoed', args: { value: 'K-B' }, callId: 'call_batch_persisted_b' }, + ]), + textResponse('done batch persisted'), + multiToolCallResponse([ + { name: 'vetoed', args: { value: 'L-A' }, callId: 'call_batch_direct_a' }, + { name: 'vetoed', args: { value: 'L-B' }, callId: 'call_batch_direct_b' }, + ]), + textResponse('done batch direct'), + toolCallResponse('same_name', { value: 'N-0' }, 'call_same_direct_0'), + handoffResponse('transfer_to_Same'), + toolCallResponse('same_name', { value: 'N-1' }, 'call_same_direct_1'), + toolCallResponse('same_name', { value: 'N-2' }, 'call_same_direct_2'), + toolCallResponse('same_name', { value: 'N-3' }, 'call_same_direct_3'), + textResponse('done same name direct'), + toolCallResponse('same_name', { value: 'M-0' }, 'call_same_rehydrated_0'), + handoffResponse('transfer_to_Same'), + toolCallResponse('same_name', { value: 'M-1' }, 'call_same_rehydrated_1'), + toolCallResponse('same_name', { value: 'M-2' }, 'call_same_rehydrated_2'), + toolCallResponse('same_name', { value: 'M-3' }, 'call_same_rehydrated_3'), + textResponse('done same name rehydrated'), ]), }), ], @@ -665,5 +693,18 @@ test('Approval resume keeps the caller context object', async (t) => { ], revisedRejectionWrites: [], revisedRejectionInterruptions: 0, + batchMessages: [ + 'call_batch_persisted_a: ok', + 'call_batch_persisted_b: BatchPersisted before', + 'call_batch_persisted_a: BatchPersisted after', + 'call_batch_persisted_b: BatchPersisted after', + 'call_batch_direct_a: ok', + 'call_batch_direct_b: BatchDirect before', + 'call_batch_direct_a: BatchDirect after', + 'call_batch_direct_b: BatchDirect after', + ], + batchInterruptions: 0, + sameNameWrites: ['N-0', 'M-0'], + sameNameApprovalOwners: [2, 4], }); }); diff --git a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts index 728d7b25..0dfb43a6 100644 --- a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts @@ -235,6 +235,10 @@ export interface ApprovalResumeContextResult { rejectionMessages: string[]; revisedRejectionWrites: string[]; revisedRejectionInterruptions: number; + batchMessages: string[]; + batchInterruptions: number; + sameNameWrites: string[]; + sameNameApprovalOwners: number[]; } /** An agent whose `clone` carries a private field, which can't be copied onto another object. */ @@ -312,8 +316,9 @@ function countApprovalOwners(context: unknown): { approvalOwners: number; approv * `Agent` keeps state outside it, which changes before the resume. Then an approved call fails after * the resume replaced its agent's object, and is rejected through the original state, whose retry must * not run it. Then the restore of a saved state fails repeatedly after merging its approvals, which must - * not add a copy of them each time. Last, a rejection message changes after a failed resume, and the retry - * must send the model the new one. + * not add a copy of them each time. Then a rejection message changes after a failed resume, and the retry + * must send the model the new one, also for a call that a later rejection for good covers. Last, a root and + * its handoff target share a name, and their resumes must not add a copy of the approvals each time. */ export async function approvalResumeContextWorkflow(): Promise { const setAfter = tool({ @@ -810,6 +815,81 @@ export async function approvalResumeContextWorkflow(): Promise { + const batchAgent = new Agent({ name, model: 'gpt-4o-mini', tools: [vetoed] }); + const paused = await runner.run(batchAgent, 'go', { context: { original: name, writes: [] } }); + const state = rehydrate ? await RunState.fromString(batchAgent, paused.state.toString()) : paused.state; + const [callA, callB] = state.getInterruptions(); + let failModel = true; + const callModelInputFilter: CallModelInputFilter = ({ modelData }) => { + for (const item of modelData.input) { + if (item.type === 'function_call_result') { + const { output } = item; + batchMessages.push( + `${item.callId}: ${typeof output === 'string' ? output : 'text' in output ? output.text : JSON.stringify(output)}` + ); + } + } + if (failModel) { + failModel = false; + throw new Error('model failed'); + } + return modelData; + }; + state.approve(callA!); + state.reject(callB!, { message: `${name} before` }); + await runner.run(batchAgent, state, { callModelInputFilter }).catch(() => undefined); + state.reject(callA!, { alwaysReject: true, message: `${name} after` }); + return (await runner.run(batchAgent, state, { callModelInputFilter })).interruptions.length; + }; + const batchInterruptions = [await rejectBatch('BatchPersisted', true), await rejectBatch('BatchDirect', false)]; + + // A root and its handoff target share a name. The root's call is approved for good, and the target's + // calls are then rejected one at a time. The resumes must keep converting into the same agents, whether + // each resumes the run's state or one rehydrated onto the original agent and the run's context. + const sameNameTool = tool({ + name: 'same_name', + description: 'Record a value after approval.', + parameters: { + type: 'object' as const, + properties: { value: { type: 'string' } }, + required: ['value'] as const, + additionalProperties: false as const, + }, + needsApproval: true, + execute: async (args, runContext) => { + (runContext!.context as ResumeContext).writes.push((args as { value: string }).value); + return 'ok'; + }, + }); + const sameNameWrites: string[] = []; + const resumeSameName = async (rehydrate: boolean) => { + const target = new Agent({ name: 'Same', model: 'gpt-4o-mini', tools: [sameNameTool] }); + const root = new Agent({ + name: 'Same', + model: 'gpt-4o-mini', + tools: [sameNameTool], + handoffs: [target], + }); + let paused = await runner.run(root, 'go', { context: { original: 'same-name', writes: sameNameWrites } }); + const runContext = paused.runContext; + for (let round = 0; paused.interruptions.length > 0 && round < 10; round++) { + const state = rehydrate + ? await RunState.fromStringWithContext(root, paused.state.toString(), runContext) + : paused.state; + const interruption = state.getInterruptions()[0]!; + if (round === 0) state.approve(interruption, { alwaysApprove: true }); + else state.reject(interruption); + paused = await runner.run(root, state); + } + return countApprovalOwners(runContext).approvalOwners; + }; + const sameNameApprovalOwners = [await resumeSameName(false), await resumeSameName(true)]; + return { callerAfter: ctx.after, resumedUsesCallerObject: result.runContext.context === ctx, @@ -861,5 +941,9 @@ export async function approvalResumeContextWorkflow(): Promise count + interruptions, 0), + batchMessages, + batchInterruptions: batchInterruptions.reduce((count, interruptions) => count + interruptions, 0), + sameNameWrites, + sameNameApprovalOwners, }; } diff --git a/typescript/openai-agents/src/workflow/runner.ts b/typescript/openai-agents/src/workflow/runner.ts index de756bec..4bdcce99 100644 --- a/typescript/openai-agents/src/workflow/runner.ts +++ b/typescript/openai-agents/src/workflow/runner.ts @@ -105,7 +105,7 @@ function reusableConversion( (conversion) => conversion?.source === agent && conversion.activeRuns === 0 && - !losesStateDecisions(conversion, serializedState, state._context) + !losesStateDecisions(conversion.agents.get(agent)!, serializedState, state._context) ); } @@ -130,29 +130,30 @@ interface ApprovalContext { /** * Whether the restore would lose one of the serialized state's decisions or rejection messages if it merged - * them into the decisions `conversion`'s agents already own. The merge keeps an approval for good over a - * rejection, and an existing rejection message over the state's. So a state that always approved a tool - * and then always rejected it would run it again, and a state whose rejection message changed after a - * failed resume would send the model the earlier message. A fresh graph owns only the state's decisions. + * them into the decisions the agents of the graph rooted at `root` already own. The merge keeps an approval + * for good over a rejection, and an existing rejection message over the state's. So a state that always + * approved a tool and then always rejected it would run it again, and a state whose rejection message + * changed after a failed resume would send the model the earlier message. A fresh graph owns only the + * state's decisions. */ -function losesStateDecisions(conversion: Conversion, serializedState: string, context: object): boolean { +function losesStateDecisions(root: Agent, serializedState: string, context: RunContext): boolean { const stateApprovals: FunctionApprovals[] = JSON.parse(serializedState).context.functionApprovals ?? []; if (stateApprovals.length === 0) return false; - const names = new Map, string>(); - for (const [source, converted] of conversion.agents) names.set(converted, source.name); - const { functionApprovals: owned = [] } = (context as ApprovalContext)._toJSONForRunState(names); - // A state's agent identity is its name, followed by `#n` if other agents in the graph share it. - return stateApprovals.some(({ agentIdentity, approvals }) => - owned.some( - ({ agentIdentity: name, approvals: ownedApprovals }) => - (agentIdentity === name || agentIdentity.startsWith(`${name}#`)) && - Object.entries(approvals).some( - ([toolName, record]) => - ownedApprovals[toolName] !== undefined && - !keepsDecisions(record, mergeApprovalRecords(ownedApprovals[toolName], record, toolName)) - ) - ) - ); + // Serializing a state that starts at `root` keys the decisions the graph's agents own by the same agent + // identities (`name`, or `name#n` for agents that share a name) the restore resolves the state's against. + const owned: FunctionApprovals[] = new RunState(context, '', root, null).toJSON().context.functionApprovals ?? []; + const ownedByIdentity = new Map(owned.map(({ agentIdentity, approvals }) => [agentIdentity, approvals])); + return stateApprovals.some(({ agentIdentity, approvals }) => { + const ownedApprovals = ownedByIdentity.get(agentIdentity); + return ( + ownedApprovals !== undefined && + Object.entries(approvals).some( + ([toolName, record]) => + ownedApprovals[toolName] !== undefined && + !keepsDecisions(record, mergeApprovalRecords(ownedApprovals[toolName], record, toolName)) + ) + ); + }); } /** Merges `incoming` into `current` as the restore does, in a scratch `RunContext`. */ @@ -168,14 +169,12 @@ function mergeApprovalRecords(current: ApprovalRecord, incoming: ApprovalRecord, return scratch._toJSONForRunState(new Map([[owner, 'owner']])).functionApprovals![0]!.approvals[toolName]!; } -/** Whether `merged` resolves every call that `record` decides, and its rejection messages, as `record` does. */ +/** + * Whether `merged` resolves every call that `record` decides, and its rejection messages, as `record` does. + * That includes calls only `merged` names, which a decision for good in `record` also covers. + */ function keepsDecisions(record: ApprovalRecord, merged: ApprovalRecord): boolean { - const callIds = new Set([ - undefined, - ...(Array.isArray(record.approved) ? record.approved : []), - ...(Array.isArray(record.rejected) ? record.rejected : []), - ...Object.keys(record.messages ?? {}), - ]); + const callIds = new Set([undefined, ...namedCallIds(record), ...namedCallIds(merged)]); return [...callIds].every((callId) => { const decision = approvalDecision(record, callId); if (decision === undefined) return true; @@ -184,6 +183,14 @@ function keepsDecisions(record: ApprovalRecord, merged: ApprovalRecord): boolean }); } +function namedCallIds(record: ApprovalRecord): string[] { + return [ + ...(Array.isArray(record.approved) ? record.approved : []), + ...(Array.isArray(record.rejected) ? record.rejected : []), + ...Object.keys(record.messages ?? {}), + ]; +} + /** The decision a record gives a call, or a call it doesn't name if `callId` is undefined, as agents-core resolves it. */ function approvalDecision(record: ApprovalRecord, callId: string | undefined): boolean | undefined { if (record.approved === true) return true; @@ -447,13 +454,16 @@ export class TemporalOpenAIRunner { const sandbox = options?.runConfig?.sandbox; // Serialize the state before converting, from the graph it was saved on, which the conversion changes. const serializedState = input instanceof RunState ? input.toString() : undefined; - const conversion = convertAgentGraph( - agent, - this.modelParams, - modelOverride, - sandbox, - input instanceof RunState ? reusableConversion(input, agent, serializedState!) : undefined - ); + const reusable = input instanceof RunState ? reusableConversion(input, agent, serializedState!) : undefined; + let conversion = convertAgentGraph(agent, this.modelParams, modelOverride, sandbox, reusable); + // The conversion can change which agents of the graph share a name identity, so check the graph it + // gave again. + if ( + reusable !== undefined && + losesStateDecisions(conversion.agents.get(agent)!, serializedState!, (input as RunState)._context) + ) { + conversion = convertAgentGraph(agent, this.modelParams, modelOverride, sandbox); + } const converted = conversion.agents.get(agent)!; conversion.activeRuns++; From 60e6226ac6d64b6894937eebca4933b34b63583a Mon Sep 17 00:00:00 2001 From: Francesc Campoy Date: Sat, 31 Oct 2026 13:11:32 -0700 Subject: [PATCH 15/15] Address review findings --- .../src/__tests__/test-openai-agents.ts | 6 ++ .../src/__tests__/workflows/openai-agents.ts | 56 ++++++++++++++++++- .../openai-agents/src/workflow/runner.ts | 37 +++++++----- 3 files changed, 85 insertions(+), 14 deletions(-) diff --git a/typescript/openai-agents/src/__tests__/test-openai-agents.ts b/typescript/openai-agents/src/__tests__/test-openai-agents.ts index d95ebe1b..f299c508 100644 --- a/typescript/openai-agents/src/__tests__/test-openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/test-openai-agents.ts @@ -623,6 +623,8 @@ test('Approval resume keeps the caller context object', async (t) => { toolCallResponse('same_name', { value: 'M-2' }, 'call_same_rehydrated_2'), toolCallResponse('same_name', { value: 'M-3' }, 'call_same_rehydrated_3'), textResponse('done same name rehydrated'), + toolCallResponse('long_history', { value: 'H' }, 'call_long_history'), + textResponse('done long history'), ]), }), ], @@ -706,5 +708,9 @@ test('Approval resume keeps the caller context object', async (t) => { batchInterruptions: 0, sameNameWrites: ['N-0', 'M-0'], sameNameApprovalOwners: [2, 4], + longHistoryThrew: true, + longHistoryWrites: ['H'], + longHistoryInterruptions: 0, + longHistoryApprovedCallIds: 100_002, }); }); diff --git a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts index 0dfb43a6..27cf35e2 100644 --- a/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts +++ b/typescript/openai-agents/src/__tests__/workflows/openai-agents.ts @@ -239,6 +239,10 @@ export interface ApprovalResumeContextResult { batchInterruptions: number; sameNameWrites: string[]; sameNameApprovalOwners: number[]; + longHistoryThrew: boolean; + longHistoryWrites: string[]; + longHistoryInterruptions: number; + longHistoryApprovedCallIds: number; } /** An agent whose `clone` carries a private field, which can't be copied onto another object. */ @@ -318,7 +322,8 @@ function countApprovalOwners(context: unknown): { approvalOwners: number; approv * not run it. Then the restore of a saved state fails repeatedly after merging its approvals, which must * not add a copy of them each time. Then a rejection message changes after a failed resume, and the retry * must send the model the new one, also for a call that a later rejection for good covers. Last, a root and - * its handoff target share a name, and their resumes must not add a copy of the approvals each time. + * its handoff target share a name, and their resumes must not add a copy of the approvals each time. Last, + * the retry of a saved state with a long approval history must finish. */ export async function approvalResumeContextWorkflow(): Promise { const setAfter = tool({ @@ -890,6 +895,51 @@ export async function approvalResumeContextWorkflow(): Promise { + const context = runContext!.context as ResumeContext & { attempts: number }; + if (context.attempts++ === 0) throw new Error('long history failed'); + context.writes.push((args as { value: string }).value); + return 'ok'; + }, + }); + const longHistoryAgent = new Agent({ + name: 'LongHistoryAgent', + model: 'gpt-4o-mini', + tools: [longHistoryTool], + }); + const pausedLongHistory = await runner.run(longHistoryAgent, 'go', { context: longHistoryCtx }); + pausedLongHistory.state.approve(pausedLongHistory.interruptions[0]!); + const longHistoryJson = JSON.parse(pausedLongHistory.state.toString()); + for (const { approvals } of longHistoryJson.context.functionApprovals) { + for (const record of Object.values(approvals) as ApprovalRecord[]) { + if (Array.isArray(record.approved)) { + record.approved.push(...Array.from({ length: 50_000 }, (_, index) => `call_earlier_${index}`)); + } + } + } + const longHistoryState = await RunState.fromString(longHistoryAgent, JSON.stringify(longHistoryJson)); + let longHistoryThrew = false; + try { + await runner.run(longHistoryAgent, longHistoryState); + } catch { + longHistoryThrew = true; + } + const longHistoryRetried = await runner.run(longHistoryAgent, longHistoryState); + return { callerAfter: ctx.after, resumedUsesCallerObject: result.runContext.context === ctx, @@ -945,5 +995,9 @@ export async function approvalResumeContextWorkflow(): Promise count + interruptions, 0), sameNameWrites, sameNameApprovalOwners, + longHistoryThrew, + longHistoryWrites: longHistoryRetried.runContext.context.writes, + longHistoryInterruptions: longHistoryRetried.interruptions.length, + longHistoryApprovedCallIds: countApprovalOwners(longHistoryState._context).approvedCallIds, }; } diff --git a/typescript/openai-agents/src/workflow/runner.ts b/typescript/openai-agents/src/workflow/runner.ts index 4bdcce99..006da968 100644 --- a/typescript/openai-agents/src/workflow/runner.ts +++ b/typescript/openai-agents/src/workflow/runner.ts @@ -174,13 +174,17 @@ function mergeApprovalRecords(current: ApprovalRecord, incoming: ApprovalRecord, * That includes calls only `merged` names, which a decision for good in `record` also covers. */ function keepsDecisions(record: ApprovalRecord, merged: ApprovalRecord): boolean { + // Index each record's call IDs once, so long approval histories compare in linear time. + const recordDecision = approvalDecisions(record); + const mergedDecision = approvalDecisions(merged); const callIds = new Set([undefined, ...namedCallIds(record), ...namedCallIds(merged)]); - return [...callIds].every((callId) => { - const decision = approvalDecision(record, callId); - if (decision === undefined) return true; - if (approvalDecision(merged, callId) !== decision) return false; - return decision || rejectionMessage(merged, callId) === rejectionMessage(record, callId); - }); + for (const callId of callIds) { + const decision = recordDecision(callId); + if (decision === undefined) continue; + if (mergedDecision(callId) !== decision) return false; + if (!decision && rejectionMessage(merged, callId) !== rejectionMessage(record, callId)) return false; + } + return true; } function namedCallIds(record: ApprovalRecord): string[] { @@ -191,13 +195,20 @@ function namedCallIds(record: ApprovalRecord): string[] { ]; } -/** The decision a record gives a call, or a call it doesn't name if `callId` is undefined, as agents-core resolves it. */ -function approvalDecision(record: ApprovalRecord, callId: string | undefined): boolean | undefined { - if (record.approved === true) return true; - if (record.rejected === true) return false; - if (callId !== undefined && Array.isArray(record.approved) && record.approved.includes(callId)) return true; - if (callId !== undefined && Array.isArray(record.rejected) && record.rejected.includes(callId)) return false; - return undefined; +/** + * Resolves the decision a record gives a call, or a call it doesn't name if `callId` is undefined, as + * agents-core resolves it. + */ +function approvalDecisions(record: ApprovalRecord): (callId: string | undefined) => boolean | undefined { + const approved = new Set(Array.isArray(record.approved) ? record.approved : []); + const rejected = new Set(Array.isArray(record.rejected) ? record.rejected : []); + return (callId) => { + if (record.approved === true) return true; + if (record.rejected === true) return false; + if (callId !== undefined && approved.has(callId)) return true; + if (callId !== undefined && rejected.has(callId)) return false; + return undefined; + }; } /** The rejection message a record gives a call, as agents-core resolves it. */