Skip to content

Commit 0318750

Browse files
anubra266github-actions[bot]claude
authored
fix: key durable approval decisions by toolCallId instead of toolName (#3062)
* fix: key durable approval decisions by toolCallId instead of toolName Eliminates ordering ambiguity when the same tool is called multiple times in one execution. Previously used Record<toolName, Array<decision>> with shift() which could mismatch if the LLM reorders calls on replay. Closes PRD-6452 * refactor: inline effectiveToolCallId alias to toolCallId Now that approvals are keyed by toolCallId directly, effectiveToolCallId is always equal to toolCallId. Remove the dead alias per review feedback. * chore: re-add changeset after rebase * chore: incorporate origin/main constants extraction into PR branch Manually applies the changes from origin/main commit 01a960d (refactor: extract magic string literals into shared constants) on top of the PR's toolCallId-keying changes, equivalent to rebasing the PR onto origin/main. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 9b60e24 commit 0318750

9 files changed

Lines changed: 37 additions & 80 deletions

File tree

‎.changeset/standard-lavender-dormouse.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@inkeep/agents-api": patch
3+
---
4+
5+
Fix approval queue ordering by keying on toolCallId instead of toolName

‎agents-api/src/domains/run/agents/Agent.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,7 @@ export class Agent {
152152
}
153153

154154
setApprovedToolCalls(
155-
approvedToolCalls:
156-
| Record<string, Array<{ approved: boolean; reason?: string; originalToolCallId?: string }>>
157-
| undefined
155+
approvedToolCalls: Record<string, { approved: boolean; reason?: string }> | undefined
158156
) {
159157
this.ctx.approvedToolCalls = approvedToolCalls;
160158
}

‎agents-api/src/domains/run/agents/agent-types.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -278,10 +278,7 @@ export interface AgentRunContext {
278278
functionToolRelationshipIdByName: Map<string, string>;
279279
taskDenialRedirects: Array<{ toolName: string; toolCallId: string; reason: string }>;
280280
durableWorkflowRunId?: string;
281-
approvedToolCalls?: Record<
282-
string,
283-
Array<{ approved: boolean; reason?: string; originalToolCallId?: string }>
284-
>;
281+
approvedToolCalls?: Record<string, { approved: boolean; reason?: string }>;
285282
pendingDurableApproval?: PendingDurableApproval;
286283
delegatedToolApproval?: {
287284
toolCallId: string;

‎agents-api/src/domains/run/agents/generateTaskHandler.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -404,12 +404,9 @@ export const createTaskHandler = (
404404
? typeof approvedToolCallsRaw === 'string'
405405
? (JSON.parse(approvedToolCallsRaw) as Record<
406406
string,
407-
Array<{ approved: boolean; reason?: string; originalToolCallId?: string }>
408-
>)
409-
: (approvedToolCallsRaw as Record<
410-
string,
411-
Array<{ approved: boolean; reason?: string; originalToolCallId?: string }>
407+
{ approved: boolean; reason?: string }
412408
>)
409+
: (approvedToolCallsRaw as Record<string, { approved: boolean; reason?: string }>)
413410
: undefined;
414411

415412
agent.setDurableWorkflowRunId(durableWorkflowRunId);

‎agents-api/src/domains/run/agents/relationTools.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -446,13 +446,10 @@ export function createDelegateToAgentTool({
446446
...(agentRunContext?.delegatedToolApproval
447447
? {
448448
approved_tool_calls: JSON.stringify({
449-
[agentRunContext.delegatedToolApproval.toolName]: [
450-
{
451-
approved: agentRunContext.delegatedToolApproval.approved,
452-
reason: agentRunContext.delegatedToolApproval.reason,
453-
originalToolCallId: agentRunContext.delegatedToolApproval.toolCallId,
454-
},
455-
],
449+
[agentRunContext.delegatedToolApproval.toolCallId]: {
450+
approved: agentRunContext.delegatedToolApproval.approved,
451+
reason: agentRunContext.delegatedToolApproval.reason,
452+
},
456453
}),
457454
}
458455
: {}),

‎agents-api/src/domains/run/agents/tools/tool-approval.ts‎

Lines changed: 3 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -45,41 +45,10 @@ export async function waitForToolApproval(
4545
if (ctx.durableWorkflowRunId) {
4646
const approvedToolCalls = ctx.approvedToolCalls;
4747
if (approvedToolCalls) {
48-
const queue = approvedToolCalls[toolName];
49-
const preApproved = queue?.shift();
50-
if (queue?.length === 0) {
51-
delete approvedToolCalls[toolName];
52-
}
48+
const preApproved = approvedToolCalls[toolCallId];
5349
if (preApproved !== undefined) {
54-
if (preApproved.originalToolCallId && preApproved.originalToolCallId !== toolCallId) {
55-
const deniedResult = tracer.startActiveSpan(
56-
'tool.approval_denied',
57-
{
58-
attributes: {
59-
...baseSpanAttributes,
60-
'tool.approval.reason': 'originalToolCallId mismatch',
61-
'tool.approval.originalToolCallId': preApproved.originalToolCallId,
62-
},
63-
},
64-
(denialSpan: Span) => {
65-
logger.warn(
66-
{
67-
toolName,
68-
toolCallId,
69-
originalToolCallId: preApproved.originalToolCallId,
70-
},
71-
'Durable approval rejected: originalToolCallId mismatch — tool call may have changed since approval'
72-
);
73-
denialSpan.setStatus({ code: SpanStatusCode.OK });
74-
denialSpan.end();
75-
return createDeniedToolResult(
76-
toolCallId,
77-
'Tool approval rejected: the tool call changed since it was approved.'
78-
);
79-
}
80-
);
81-
return { approved: false, deniedResult };
82-
}
50+
delete approvedToolCalls[toolCallId];
51+
8352
if (!preApproved.approved) {
8453
const deniedResult = tracer.startActiveSpan(
8554
'tool.approval_denied',

‎agents-api/src/domains/run/agents/tools/tool-wrapper.ts‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -142,25 +142,24 @@ export function wrapToolWithStreaming(
142142
const needsApproval = options?.needsApproval || false;
143143

144144
const preApprovedEntry = ctx.durableWorkflowRunId
145-
? ctx.approvedToolCalls?.[toolName]?.[0]
145+
? ctx.approvedToolCalls?.[toolCallId]
146146
: undefined;
147-
const effectiveToolCallId = preApprovedEntry?.originalToolCallId ?? toolCallId;
148147
const isPreApproved = !!preApprovedEntry;
149148

150149
if (streamRequestId && streamHelper && !isInternalToolForUi && !isPreApproved) {
151150
const inputText = JSON.stringify(args ?? {});
152151

153-
await streamHelper.writeToolInputStart({ toolCallId: effectiveToolCallId, toolName });
152+
await streamHelper.writeToolInputStart({ toolCallId: toolCallId, toolName });
154153

155154
for (const part of chunkString(inputText, 16)) {
156155
await streamHelper.writeToolInputDelta({
157-
toolCallId: effectiveToolCallId,
156+
toolCallId: toolCallId,
158157
inputTextDelta: part,
159158
});
160159
}
161160

162161
await streamHelper.writeToolInputAvailable({
163-
toolCallId: effectiveToolCallId,
162+
toolCallId: toolCallId,
164163
toolName,
165164
input: args ?? {},
166165
providerMetadata: context?.providerMetadata,
@@ -171,7 +170,7 @@ export function wrapToolWithStreaming(
171170
const toolCallData: ToolCallData = {
172171
toolName,
173172
input: args,
174-
toolCallId: effectiveToolCallId,
173+
toolCallId: toolCallId,
175174
relationshipId,
176175
inDelegatedAgent: ctx.isDelegatedAgent,
177176
};
@@ -272,7 +271,7 @@ export function wrapToolWithStreaming(
272271
}
273272

274273
ctx.pendingDurableApproval = {
275-
toolCallId: effectiveToolCallId,
274+
toolCallId: toolCallId,
276275
toolName,
277276
args: resolvedArgs,
278277
delegatedApproval: {
@@ -300,7 +299,7 @@ export function wrapToolWithStreaming(
300299
toolName,
301300
args,
302301
result,
303-
effectiveToolCallId,
302+
toolCallId,
304303
toolResultConversationId,
305304
messageId
306305
);
@@ -317,7 +316,7 @@ export function wrapToolWithStreaming(
317316
metadata: {
318317
a2a_metadata: {
319318
toolName,
320-
toolCallId: effectiveToolCallId,
319+
toolCallId: toolCallId,
321320
toolArgs: args,
322321
toolOutput: result,
323322
timestamp: Date.now(),
@@ -332,7 +331,7 @@ export function wrapToolWithStreaming(
332331
{
333332
error,
334333
toolName,
335-
toolCallId: effectiveToolCallId,
334+
toolCallId: toolCallId,
336335
conversationId: toolResultConversationId,
337336
},
338337
'Failed to store tool result in conversation history'
@@ -348,7 +347,7 @@ export function wrapToolWithStreaming(
348347
{
349348
toolName,
350349
output: result,
351-
toolCallId: effectiveToolCallId,
350+
toolCallId: toolCallId,
352351
duration,
353352
relationshipId,
354353
needsApproval,
@@ -361,10 +360,10 @@ export function wrapToolWithStreaming(
361360

362361
if (streamRequestId && streamHelper && !isInternalToolForUi) {
363362
if (isDeniedResult) {
364-
await streamHelper.writeToolOutputDenied({ toolCallId: effectiveToolCallId });
363+
await streamHelper.writeToolOutputDenied({ toolCallId: toolCallId });
365364
} else {
366365
await streamHelper.writeToolOutputAvailable({
367-
toolCallId: effectiveToolCallId,
366+
toolCallId: toolCallId,
368367
output: result,
369368
});
370369
}
@@ -388,7 +387,7 @@ export function wrapToolWithStreaming(
388387
{
389388
toolName,
390389
output: null,
391-
toolCallId: effectiveToolCallId,
390+
toolCallId: toolCallId,
392391
duration,
393392
error: errorMessage,
394393
relationshipId,
@@ -400,7 +399,7 @@ export function wrapToolWithStreaming(
400399

401400
if (streamRequestId && streamHelper && !isInternalToolForUi) {
402401
await streamHelper.writeToolOutputError({
403-
toolCallId: effectiveToolCallId,
402+
toolCallId: toolCallId,
404403
errorText: errorMessage,
405404
});
406405
}

‎agents-api/src/domains/run/handlers/executionHandler.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,8 @@ export interface ExecutionHandlerParams {
5656
responseMessageId?: string;
5757
/** Durable workflow run ID — present when running inside a WDK workflow */
5858
durableWorkflowRunId?: string;
59-
/** Pre-approved tool decisions keyed by toolName — accumulated across approval loops */
60-
approvedToolCalls?: Record<
61-
string,
62-
Array<{ approved: boolean; reason?: string; originalToolCallId?: string }>
63-
>;
59+
/** Pre-approved tool decisions keyed by toolCallId — accumulated across approval loops */
60+
approvedToolCalls?: Record<string, { approved: boolean; reason?: string }>;
6461
}
6562

6663
interface ExecutionResult {

‎agents-api/src/domains/run/workflow/steps/agentExecutionSteps.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { FullExecutionContext, McpTool, Part, ResolvedRef } from '@inkeep/agents-core';
2-
import { SPAN_NAMES } from '@inkeep/agents-core';
2+
import { SPAN_NAMES, TRANSFER_TOOL_PREFIX } from '@inkeep/agents-core';
33
import { context as otelContext, propagation } from '@opentelemetry/api';
44
import { getWritable } from 'workflow';
55
import { env } from '../../../../env';
@@ -671,7 +671,7 @@ export async function callLlmStep(params: CallLlmStepParams): Promise<CallLlmRes
671671
);
672672
}
673673

674-
if (hasToolCallWithPrefix('transfer_to_')(response)) {
674+
if (hasToolCallWithPrefix(TRANSFER_TOOL_PREFIX)(response)) {
675675
const transferReason =
676676
response.steps?.[response.steps.length - 1]?.text || response.text || '';
677677

@@ -683,9 +683,9 @@ export async function callLlmStep(params: CallLlmStepParams): Promise<CallLlmRes
683683
)?.toolCalls ?? [];
684684

685685
const transferToolCall = lastStepToolCallsForTransfer.find((tc) =>
686-
tc.toolName.startsWith('transfer_to_')
686+
tc.toolName.startsWith(TRANSFER_TOOL_PREFIX)
687687
);
688-
const targetSubAgentId = transferToolCall?.toolName.slice('transfer_to_'.length);
688+
const targetSubAgentId = transferToolCall?.toolName.slice(TRANSFER_TOOL_PREFIX.length);
689689

690690
logger.info(
691691
{ targetSubAgentId, transferToolName: transferToolCall?.toolName },
@@ -969,9 +969,7 @@ export async function executeToolStep(params: ExecuteToolStepParams): Promise<Ex
969969

970970
if (preApproved !== undefined) {
971971
agent.setApprovedToolCalls({
972-
[toolName]: [
973-
{ approved: preApproved, reason: approvalReason, originalToolCallId: toolCallId },
974-
],
972+
[toolCallId]: { approved: preApproved, reason: approvalReason },
975973
});
976974
}
977975

0 commit comments

Comments
 (0)