diff --git a/docs/tools/loop-detection.md b/docs/tools/loop-detection.md index 765ead224f12..6a6150cde816 100644 --- a/docs/tools/loop-detection.md +++ b/docs/tools/loop-detection.md @@ -130,8 +130,13 @@ spend and lockups while preserving normal tool access. - Warnings come first. - Blocking follows once a pattern persists past the warning threshold. -- Critical thresholds block the next tool-cycle and surface a clear - loop-detection reason in the run record. +- In the embedded agent loop, the first critical loop blocks the whole tool + batch before any tool in that batch runs. The model then gets one more + response with its normal tools. +- During that response, the model can answer, ask a question, or continue with + a different tool or different arguments. +- Another critical loop in the same run blocks its whole batch and ends the + run. A new user run starts with a fresh recovery allowance. - The post-compaction guard emits `compaction_loop_persisted` errors naming the offending tool and identical-call count. diff --git a/extensions/qa-lab/src/providers/mock-openai/server.ts b/extensions/qa-lab/src/providers/mock-openai/server.ts index f4e68efb9d34..683aaee2f594 100644 --- a/extensions/qa-lab/src/providers/mock-openai/server.ts +++ b/extensions/qa-lab/src/providers/mock-openai/server.ts @@ -956,11 +956,11 @@ async function buildResponsesPayload( if (!hasCompletedToolOutput) { scenarioState.toolLoopReadAttempts = 0; } - if (/global circuit breaker/i.test(toolOutput)) { + if (/do not repeat this exact tool action/i.test(toolOutput)) { return buildAssistantEvents(exactReplyDirective ?? "GLOBAL-LOOP-BREAKER-OK"); } scenarioState.toolLoopReadAttempts += 1; - if (scenarioState.toolLoopReadAttempts > 31) { + if (scenarioState.toolLoopReadAttempts > 21) { return buildAssistantEvents("GLOBAL-LOOP-BREAKER-NOT-REACHED"); } return buildToolCallEventsWithArgs("read", { path: "LOOP_STEADY.txt" }); diff --git a/packages/agent-core/src/agent-loop.test.ts b/packages/agent-core/src/agent-loop.test.ts index f506d77a7be3..ba24d31631d4 100644 --- a/packages/agent-core/src/agent-loop.test.ts +++ b/packages/agent-core/src/agent-loop.test.ts @@ -5,6 +5,7 @@ import { describe, expect, it, vi } from "vitest"; import { agentLoop, agentLoopContinue, runAgentLoop, runAgentLoopContinue } from "./agent-loop.js"; import { Agent } from "./agent.js"; import { TRANSCRIPT_NOT_CONTINUABLE_ERROR_CODE, TranscriptNotContinuableError } from "./errors.js"; +import { setInternalBeforeToolBatch } from "./internal-hooks.js"; import { type AssistantMessage, createAssistantMessageEventStream, @@ -733,7 +734,11 @@ describe("runAgentLoop deferred tool hydration", () => { stopReason: "stop" as const, timestamp: Date.now(), }; - stream.push({ type: "done", reason: message.stopReason, message }); + stream.push({ + type: "done", + reason: message.stopReason === "toolUse" ? "toolUse" : "stop", + message, + }); }); return stream; }; @@ -815,7 +820,11 @@ describe("runAgentLoop deferred tool hydration", () => { stopReason: "stop" as const, timestamp: Date.now(), }; - stream.push({ type: "done", reason: message.stopReason, message }); + stream.push({ + type: "done", + reason: message.stopReason === "toolUse" ? "toolUse" : "stop", + message, + }); }); return stream; }; @@ -982,6 +991,514 @@ describe("agentLoop tool termination", () => { }; } + function criticalLoopFor(toolCall: { id: string; name: string }) { + return { + kind: "critical-tool-loop" as const, + toolCallId: toolCall.id, + toolName: toolCall.name, + actionKey: `${toolCall.name}:same-action`, + detector: "generic_repeat", + count: 20, + reason: `CRITICAL: ${toolCall.name} is looping`, + }; + } + + it("gives the model one recovery turn with the normal tool catalog", async () => { + const executed: string[] = []; + const providerToolNames: string[][] = []; + let turn = 0; + const streamFn: StreamFn = (_activeModel, context) => { + providerToolNames.push(context.tools?.map((tool) => tool.name) ?? []); + turn += 1; + const stream = createAssistantMessageEventStream(); + queueMicrotask(() => { + const message = + turn === 1 + ? makeAssistantMessage([ + { type: "toolCall", id: "loop-1", name: "read", arguments: {} }, + ]) + : makeAssistantMessage([{ type: "text", text: "recovered" }]); + stream.push({ + type: "done", + reason: message.stopReason === "toolUse" ? "toolUse" : "stop", + message, + }); + stream.end(); + }); + return stream; + }; + const events = await collectEvents( + agentLoop( + [{ role: "user", content: "run", timestamp: 1 }], + { systemPrompt: "", messages: [], tools: [makeTool("read", executed)] }, + { + ...config, + beforeToolBatch: async ({ calls }) => { + const first = calls[0]; + expect(first?.tool?.name).toBe("read"); + return first ? { intervention: criticalLoopFor(first.toolCall) } : undefined; + }, + }, + undefined, + streamFn, + ), + ); + + expect(turn).toBe(2); + expect(providerToolNames).toEqual([["read"], ["read"]]); + expect(executed).toEqual([]); + expect( + events.find( + (event): event is Extract => + event.type === "tool_execution_end", + ), + ).toMatchObject({ executionStarted: false, isError: true }); + expect( + events.find( + ( + event, + ): event is Extract & { + message: { role: "toolResult" }; + } => event.type === "message_end" && event.message.role === "toolResult", + )?.message, + ).toMatchObject({ + details: { status: "blocked", deniedReason: "tool-loop" }, + }); + }); + + it("does not taint the recovery turn with an unexecuted network tool source", async () => { + const executed: string[] = []; + let turn = 0; + const streamFn: StreamFn = () => { + turn += 1; + const stream = createAssistantMessageEventStream(); + queueMicrotask(() => { + const message = + turn === 1 + ? makeAssistantMessage([ + { type: "toolCall", id: "loop-1", name: "fetch", arguments: {} }, + ]) + : makeAssistantMessage([{ type: "text", text: "recovered" }]); + stream.push({ + type: "done", + reason: message.stopReason === "toolUse" ? "toolUse" : "stop", + message, + }); + stream.end(); + }); + return stream; + }; + const networkTool: AgentTool = { + ...makeTool("fetch", executed), + resultContentSource: "network", + }; + const events = await collectEvents( + agentLoop( + [{ role: "user", content: "run", timestamp: 1 }], + { systemPrompt: "", messages: [], tools: [networkTool] }, + { + ...config, + beforeToolBatch: async ({ calls }) => { + const first = calls[0]; + return first ? { intervention: criticalLoopFor(first.toolCall) } : undefined; + }, + }, + undefined, + streamFn, + ), + ); + + expect(turn).toBe(2); + expect(executed).toEqual([]); + const readTaint = (message: unknown) => + (message as Record)["__openclaw"] as + | { resultContentSource?: string; turnTainted?: boolean } + | undefined; + const toolResultMessage = events.find( + ( + event, + ): event is Extract & { + message: { role: "toolResult" }; + } => event.type === "message_end" && event.message.role === "toolResult", + )?.message; + // The rejected call never executed, so it carries no network source metadata. + expect(readTaint(toolResultMessage)?.resultContentSource).toBeUndefined(); + const recoveryAssistantMessage = events.findLast( + ( + event, + ): event is Extract & { + message: { role: "assistant" }; + } => event.type === "message_end" && event.message.role === "assistant", + )?.message; + expect(recoveryAssistantMessage).toMatchObject({ stopReason: "stop" }); + expect(readTaint(recoveryAssistantMessage)?.turnTainted).not.toBe(true); + }); + + it("honors outcome-hook termination during the first recovery turn", async () => { + const executed: string[] = []; + let streamCalls = 0; + const streamFn: StreamFn = () => { + streamCalls += 1; + if (streamCalls > 1) { + throw new Error("model was called after outcome-hook termination"); + } + const stream = createAssistantMessageEventStream(); + queueMicrotask(() => { + const message = makeAssistantMessage([ + { type: "toolCall", id: "loop-1", name: "read", arguments: {} }, + ]); + stream.push({ type: "done", reason: "toolUse", message }); + stream.end(); + }); + return stream; + }; + const events = await collectEvents( + agentLoop( + [{ role: "user", content: "run", timestamp: 1 }], + { systemPrompt: "", messages: [], tools: [makeTool("read", executed)] }, + { + ...config, + beforeToolBatch: async ({ calls }) => { + const first = calls[0]; + return first ? { intervention: criticalLoopFor(first.toolCall) } : undefined; + }, + afterToolOutcome: async () => ({ terminate: true }), + }, + undefined, + streamFn, + ), + ); + + expect(streamCalls).toBe(1); + expect(executed).toEqual([]); + // The run ends normally after the terminated batch: no forced + // tool-loop-recovery failure message, which is reserved for later loops. + expect(events.at(-1)).toMatchObject({ type: "agent_end" }); + expect( + events.find( + ( + event, + ): event is Extract & { + message: { role: "assistant" }; + } => + event.type === "message_end" && + event.message.role === "assistant" && + event.message.stopReason === "error", + ), + ).toBeUndefined(); + }); + + it("stops pre-admission validation after cancellation and aborts the untouched tail", async () => { + const controller = new AbortController(); + const executed: string[] = []; + const resolverCalls: string[] = []; + let streamCalls = 0; + const streamFn: StreamFn = () => { + streamCalls += 1; + if (streamCalls > 1) { + throw new Error("model was called after abort"); + } + const stream = createAssistantMessageEventStream(); + queueMicrotask(() => { + const message = makeAssistantMessage([ + { type: "toolCall", id: "d-first", name: "d_first_tool", arguments: {} }, + { type: "toolCall", id: "d-second", name: "d_second_tool", arguments: {} }, + { type: "toolCall", id: "d-third", name: "d_third_tool", arguments: {} }, + ]); + stream.push({ type: "done", reason: "toolUse", message }); + stream.end(); + }); + return stream; + }; + const deferredTool = (name: string): AgentTool => ({ + name, + label: name, + description: name, + parameters: Type.Object({}, { additionalProperties: false }), + execute: async () => { + executed.push(name); + return { + content: [{ type: "text", text: `${name} result` }], + details: { name }, + }; + }, + }); + const events = await collectEvents( + agentLoop( + [{ role: "user", content: "abort mid-admission", timestamp: 1 }], + { systemPrompt: "", messages: [], tools: [] }, + { + ...config, + resolveDeferredTool: async ({ toolCall }) => { + resolverCalls.push(toolCall.name); + if (toolCall.name === "d_first_tool") { + // The run is cancelled while the first async resolver is in + // flight; later resolvers must never be awaited. + controller.abort(new Error("user aborted")); + } + return deferredTool(toolCall.name); + }, + beforeToolBatch: async () => undefined, + }, + controller.signal, + streamFn, + ), + ); + + expect(streamCalls).toBe(1); + expect(resolverCalls).toEqual(["d_first_tool"]); + expect(executed).toEqual([]); + const toolResults = events + .filter( + ( + event, + ): event is Extract & { + message: { role: "toolResult" }; + } => event.type === "message_end" && event.message.role === "toolResult", + ) + .map((event) => event.message); + expect(toolResults).toHaveLength(3); + for (const toolResult of toolResults) { + expect(toolResult).toMatchObject({ + isError: true, + content: [{ type: "text", text: "Operation aborted" }], + }); + } + }); + + it("executes a different recovery action and keeps the one-shot budget spent", async () => { + const executed: string[] = []; + let turn = 0; + const streamFn: StreamFn = () => { + turn += 1; + const stream = createAssistantMessageEventStream(); + queueMicrotask(() => { + const message = + turn === 1 + ? makeAssistantMessage([ + { type: "toolCall", id: "loop-1", name: "read", arguments: {} }, + ]) + : turn === 2 + ? makeAssistantMessage([ + { type: "toolCall", id: "safe-1", name: "list", arguments: {} }, + ]) + : makeAssistantMessage([ + { type: "toolCall", id: "loop-2", name: "read", arguments: {} }, + ]); + stream.push({ + type: "done", + reason: message.stopReason === "toolUse" ? "toolUse" : "stop", + message, + }); + stream.end(); + }); + return stream; + }; + const events = await collectEvents( + agentLoop( + [{ role: "user", content: "run", timestamp: 1 }], + { + systemPrompt: "", + messages: [], + tools: [makeTool("read", executed), makeTool("list", executed)], + }, + { + ...config, + beforeToolBatch: async ({ calls }) => { + const repeated = calls.find((call) => call.toolCall.name === "read"); + return repeated ? { intervention: criticalLoopFor(repeated.toolCall) } : undefined; + }, + }, + undefined, + streamFn, + ), + ); + + expect(turn).toBe(3); + expect(executed).toEqual(["list"]); + expect(events.at(-1)).toMatchObject({ type: "agent_end" }); + const toolEnds = events.filter( + (event): event is Extract => + event.type === "tool_execution_end", + ); + expect(toolEnds.map((event) => event.executionStarted)).toEqual([false, true, false]); + expect(toolEnds.at(-1)?.result).toMatchObject({ terminate: true }); + expect( + events.find( + (event) => + event.type === "message_end" && + event.message.role === "assistant" && + event.message.stopReason === "error", + ), + ).toMatchObject({ + message: { + content: [ + { + type: "text", + text: expect.stringContaining("tool-loop recovery encountered another critical loop"), + }, + ], + }, + }); + }); + + it.each(["parallel", "sequential"] as const)( + "rejects the entire recovery batch before any $toolExecution sibling executes", + async (toolExecution) => { + const executed: string[] = []; + let turn = 0; + const streamFn: StreamFn = () => { + turn += 1; + const stream = createAssistantMessageEventStream(); + queueMicrotask(() => { + const message = + turn === 1 + ? makeAssistantMessage([ + { type: "toolCall", id: "loop-1", name: "read", arguments: {} }, + ]) + : makeAssistantMessage([ + { type: "toolCall", id: "safe-1", name: "write", arguments: {} }, + { type: "toolCall", id: "loop-2", name: "read", arguments: {} }, + ]); + stream.push({ + type: "done", + reason: message.stopReason === "toolUse" ? "toolUse" : "stop", + message, + }); + stream.end(); + }); + return stream; + }; + const events = await collectEvents( + agentLoop( + [{ role: "user", content: "run", timestamp: 1 }], + { + systemPrompt: "", + messages: [], + tools: [makeTool("read", executed), makeTool("write", executed)], + }, + { + ...config, + toolExecution, + beforeToolBatch: async ({ calls }) => { + const repeated = calls.find((call) => call.toolCall.name === "read"); + return repeated ? { intervention: criticalLoopFor(repeated.toolCall) } : undefined; + }, + }, + undefined, + streamFn, + ), + ); + + expect(turn).toBe(2); + expect(executed).toEqual([]); + expect( + events + .filter( + (event): event is Extract => + event.type === "tool_execution_end", + ) + .map((event) => event.executionStarted), + ).toEqual([false, false, false]); + expect(events.at(-2)).toMatchObject({ + type: "turn_end", + message: { + role: "assistant", + stopReason: "error", + content: [ + { + type: "text", + text: expect.stringContaining("tool-loop recovery encountered another critical loop"), + }, + ], + }, + }); + }, + ); + + it("preserves the recovery budget across continue retries and resets it for a new prompt", async () => { + let phase: "initial" | "retry" | "new-prompt" = "initial"; + let phaseCalls = 0; + const streamFn: StreamFn = () => { + phaseCalls += 1; + const stream = createAssistantMessageEventStream(); + queueMicrotask(() => { + const message = + phase === "initial" && phaseCalls === 2 + ? { + ...makeAssistantMessage([]), + stopReason: "error" as const, + errorMessage: "retryable provider failure", + } + : phase === "new-prompt" && phaseCalls === 2 + ? makeAssistantMessage([{ type: "text", text: "recovered on the new run" }]) + : makeAssistantMessage([ + { + type: "toolCall", + id: `${phase}-${phaseCalls}`, + name: "read", + arguments: {}, + }, + ]); + if (message.stopReason === "error") { + stream.push({ type: "error", reason: "error", error: message }); + } else { + stream.push({ + type: "done", + reason: message.stopReason === "toolUse" ? "toolUse" : "stop", + message, + }); + } + stream.end(); + }); + return stream; + }; + const agent = new Agent({ + initialState: { model, systemPrompt: "", tools: [makeTool("read", [])] }, + streamFn, + }); + setInternalBeforeToolBatch(agent, async ({ calls }) => { + const first = calls[0]; + return first ? { intervention: criticalLoopFor(first.toolCall) } : undefined; + }); + + await agent.prompt("run"); + expect(phaseCalls).toBe(2); + expect(agent.state.messages.at(-1)).toMatchObject({ + role: "assistant", + stopReason: "error", + errorMessage: "retryable provider failure", + }); + + agent.state.messages = agent.state.messages.slice(0, -1); + phase = "retry"; + phaseCalls = 0; + await agent.continue(); + + expect(phaseCalls).toBe(1); + expect(agent.state.messages.at(-1)).toMatchObject({ + role: "assistant", + stopReason: "error", + content: [ + { + type: "text", + text: expect.stringContaining("tool-loop recovery encountered another critical loop"), + }, + ], + }); + + phase = "new-prompt"; + phaseCalls = 0; + await agent.prompt("new run"); + + expect(phaseCalls).toBe(2); + expect(agent.state.messages.at(-1)).toMatchObject({ + role: "assistant", + stopReason: "stop", + content: [{ type: "text", text: "recovered on the new run" }], + }); + }); + it.each([ { source: "network" as const, tainted: true }, { source: undefined, tainted: false }, diff --git a/packages/agent-core/src/agent-loop.ts b/packages/agent-core/src/agent-loop.ts index 210798a88f1e..6adbc828608e 100644 --- a/packages/agent-core/src/agent-loop.ts +++ b/packages/agent-core/src/agent-loop.ts @@ -34,6 +34,7 @@ import type { AgentToolCall, AgentToolResult, StreamFn, + ToolLoopIntervention, } from "./types.js"; import { validateToolArguments } from "./validation.js"; @@ -58,6 +59,9 @@ type AssistantMessageUpdateEvent = Extract< } >; +const TOOL_LOOP_RECOVERY_TERMINATED_MESSAGE = + "OpenClaw stopped this run because tool-loop recovery encountered another critical loop. No blocked tool action was executed."; + function appendTextDeltaToAssistantMessage( message: AssistantMessage, contentIndex: number, @@ -282,6 +286,9 @@ async function runLoop( let firstTurn = true; let turnOpen = true; let turnTainted = isActiveTurnTainted(initialContext.messages); + const toolLoopRecoveryState = initialConfig.toolLoopRecoveryState ?? { + criticalToolLoopSeen: false, + }; // Check for steering messages at start (user may have typed while waiting) let pendingMessages: AgentMessage[] = (await config.getSteeringMessages?.()) || []; const stopIfAborted = async (): Promise => { @@ -374,6 +381,7 @@ async function runLoop( const toolResults: ToolResultMessage[] = []; hasMoreToolCalls = false; + let terminateRun = false; if (message.stopReason === "toolUse" && toolCalls.length > 0) { const executedToolBatch = await executeToolCalls( currentContext, @@ -381,10 +389,15 @@ async function runLoop( config, signal, emit, + toolLoopRecoveryState.criticalToolLoopSeen, ); toolResults.push(...executedToolBatch.messages); turnTainted ||= toolResults.some(toolResultTaintsTurn); hasMoreToolCalls = !executedToolBatch.terminate; + if (executedToolBatch.intervention) { + toolLoopRecoveryState.criticalToolLoopSeen = true; + } + terminateRun = executedToolBatch.terminateRun; for (const result of toolResults) { currentContext.messages.push(result); @@ -397,6 +410,26 @@ async function runLoop( if (await stopIfAborted()) { return; } + if (terminateRun) { + const terminalMessage = { + ...createFailureMessage( + config.model, + new Error(TOOL_LOOP_RECOVERY_TERMINATED_MESSAGE), + false, + ), + content: [{ type: "text" as const, text: TOOL_LOOP_RECOVERY_TERMINATED_MESSAGE }], + }; + currentContext.messages.push(terminalMessage); + newMessages.push(terminalMessage); + await emit({ type: "turn_start" }); + turnOpen = true; + await emit({ type: "message_start", message: terminalMessage }); + await emit({ type: "message_end", message: terminalMessage }); + await emit({ type: "turn_end", message: terminalMessage, toolResults: [] }); + turnOpen = false; + await emit({ type: "agent_end", messages: newMessages }); + return; + } const nextTurnContext = { message, @@ -579,12 +612,64 @@ async function executeToolCalls( config: AgentLoopConfig, signal: AbortSignal | undefined, emit: AgentEventSink, + criticalToolLoopSeen: boolean, ): Promise { const toolCalls = assistantMessage.content.filter((c) => c.type === "toolCall"); const resolvedToolCalls = new Map(); + const validatedToolCalls = new Map(); + if (config.beforeToolBatch) { + for (const toolCall of toolCalls) { + if (signal?.aborted) { + // Cancellation during an early async resolver must not stall behind + // the remaining resolvers. Skipped calls stay uncached and complete + // through the executors' normal aborted-call lifecycle. + break; + } + validatedToolCalls.set( + toolCall, + await validateToolCallForBatchAdmission( + currentContext, + assistantMessage, + toolCall, + config, + signal, + resolvedToolCalls, + ), + ); + } + const calls = toolCalls.flatMap((toolCall) => { + const validation = validatedToolCalls.get(toolCall); + return validation?.kind === "validated" + ? [{ toolCall, args: validation.prepared.args, tool: validation.prepared.tool }] + : []; + }); + if (calls.length > 0 && !signal?.aborted) { + const admission = await config.beforeToolBatch( + { assistantMessage, calls, context: currentContext }, + signal, + ); + if (admission?.intervention) { + return await completeToolLoopInterventionBatch({ + currentContext, + assistantMessage, + toolCalls, + resolvedToolCalls, + validatedToolCalls, + config, + signal, + emit, + intervention: admission.intervention, + terminal: criticalToolLoopSeen, + }); + } + } + } let hasSequentialToolCall = false; if (config.toolExecution !== "sequential") { for (const toolCall of toolCalls) { + if (signal?.aborted) { + break; + } const resolution = await resolveToolCallTool( currentContext, assistantMessage, @@ -597,9 +682,6 @@ async function executeToolCalls( hasSequentialToolCall = true; break; } - if (signal?.aborted) { - break; - } } } if (config.toolExecution === "sequential" || hasSequentialToolCall) { @@ -608,6 +690,7 @@ async function executeToolCalls( assistantMessage, toolCalls, resolvedToolCalls, + validatedToolCalls, config, signal, emit, @@ -618,6 +701,7 @@ async function executeToolCalls( assistantMessage, toolCalls, resolvedToolCalls, + validatedToolCalls, config, signal, emit, @@ -627,6 +711,8 @@ async function executeToolCalls( type ExecutedToolCallBatch = { messages: ToolResultMessage[]; terminate: boolean; + terminateRun: boolean; + intervention?: ToolLoopIntervention; }; type ResolvedToolCallOutcome = @@ -651,6 +737,7 @@ async function executeToolCallsSequential( assistantMessage: AssistantMessage, toolCalls: AgentToolCall[], resolvedToolCalls: Map, + validatedToolCalls: Map, config: AgentLoopConfig, signal: AbortSignal | undefined, emit: AgentEventSink, @@ -679,6 +766,7 @@ async function executeToolCallsSequential( config, signal, resolvedToolCalls, + validatedToolCalls, ); let finalized: FinalizedToolCallOutcome; if (preparation.kind === "immediate") { @@ -747,6 +835,7 @@ async function executeToolCallsSequential( return { messages, terminate: shouldTerminateToolBatch(finalizedCalls), + terminateRun: false, }; } @@ -755,6 +844,7 @@ async function executeToolCallsParallel( assistantMessage: AssistantMessage, toolCalls: AgentToolCall[], resolvedToolCalls: Map, + validatedToolCalls: Map, config: AgentLoopConfig, signal: AbortSignal | undefined, emit: AgentEventSink, @@ -782,6 +872,7 @@ async function executeToolCallsParallel( config, signal, resolvedToolCalls, + validatedToolCalls, ); if (preparation.kind === "immediate") { const finalized = await finalizeToolCallOutcome( @@ -865,6 +956,7 @@ async function executeToolCallsParallel( return { messages, terminate: shouldTerminateToolBatch(orderedFinalizedCalls), + terminateRun: false, }; } @@ -882,6 +974,10 @@ type ImmediateToolCallOutcome = { errorKind?: "argument-validation"; }; +type ValidatedToolCallOutcome = + | { kind: "validated"; prepared: PreparedToolCall } + | { kind: "immediate"; outcome: ImmediateToolCallOutcome }; + type ExecutedToolCallOutcome = { result: AgentToolResult; isError: boolean; @@ -973,59 +1069,32 @@ async function prepareToolCall( config: AgentLoopConfig, signal: AbortSignal | undefined, resolvedToolCalls: Map, + validatedToolCalls: Map, ): Promise { - const resolution = await resolveToolCallTool( - currentContext, - assistantMessage, - toolCall, - config, - signal, - resolvedToolCalls, - ); - if (resolution.kind === "error") { + const cachedValidation = validatedToolCalls.get(toolCall); + if (signal?.aborted && !cachedValidation) { + // Execution cannot start after cancellation, so never begin validation + // work (including deferred tool resolvers) for an uncached call. return { kind: "immediate", - result: createErrorToolResult( - signal?.aborted - ? "Operation aborted" - : resolution.error instanceof Error - ? resolution.error.message - : String(resolution.error), - ), + result: createErrorToolResult("Operation aborted"), isError: true, }; } - const tool = resolution.tool; - if (!tool) { - return { - kind: "immediate", - result: createErrorToolResult(`Tool ${toolCall.name} not found`), - isError: true, - }; - } - - let preparedToolCall: AgentToolCall; - try { - preparedToolCall = prepareToolCallArguments(tool, toolCall); - } catch (error) { - return { - kind: "immediate", - result: createErrorToolResult(error instanceof Error ? error.message : String(error)), - isError: true, - }; - } - - let validatedArgs: unknown; - try { - validatedArgs = validateToolArguments(tool, preparedToolCall); - } catch (error) { - return { - kind: "immediate", - result: createErrorToolResult(error instanceof Error ? error.message : String(error)), - isError: true, - errorKind: "argument-validation", - }; + const validation = + cachedValidation ?? + (await validateToolCallForBatchAdmission( + currentContext, + assistantMessage, + toolCall, + config, + signal, + resolvedToolCalls, + )); + if (validation.kind === "immediate") { + return validation.outcome; } + const { args: validatedArgs } = validation.prepared; try { if (config.beforeToolCall) { @@ -1060,12 +1129,7 @@ async function prepareToolCall( isError: true, }; } - return { - kind: "prepared", - toolCall, - tool, - args: validatedArgs, - }; + return validation.prepared; } catch (error) { return { kind: "immediate", @@ -1075,6 +1139,84 @@ async function prepareToolCall( } } +async function validateToolCallForBatchAdmission( + currentContext: AgentContext, + assistantMessage: AssistantMessage, + toolCall: AgentToolCall, + config: AgentLoopConfig, + signal: AbortSignal | undefined, + resolvedToolCalls: Map, +): Promise { + const resolution = await resolveToolCallTool( + currentContext, + assistantMessage, + toolCall, + config, + signal, + resolvedToolCalls, + ); + if (resolution.kind === "error") { + return { + kind: "immediate", + outcome: { + kind: "immediate", + result: createErrorToolResult( + signal?.aborted + ? "Operation aborted" + : resolution.error instanceof Error + ? resolution.error.message + : String(resolution.error), + ), + isError: true, + }, + }; + } + const tool = resolution.tool; + if (!tool) { + return { + kind: "immediate", + outcome: { + kind: "immediate", + result: createErrorToolResult(`Tool ${toolCall.name} not found`), + isError: true, + }, + }; + } + + let preparedToolCall: AgentToolCall; + try { + preparedToolCall = prepareToolCallArguments(tool, toolCall); + } catch (error) { + return { + kind: "immediate", + outcome: { + kind: "immediate", + result: createErrorToolResult(error instanceof Error ? error.message : String(error)), + isError: true, + }, + }; + } + + let validatedArgs: unknown; + try { + validatedArgs = validateToolArguments(tool, preparedToolCall); + } catch (error) { + return { + kind: "immediate", + outcome: { + kind: "immediate", + result: createErrorToolResult(error instanceof Error ? error.message : String(error)), + isError: true, + errorKind: "argument-validation", + }, + }; + } + return { + kind: "validated", + prepared: { kind: "prepared", toolCall, tool, args: validatedArgs }, + }; +} + async function executePreparedToolCall( prepared: PreparedToolCall, executionContext: AgentToolExecutionContext, @@ -1253,6 +1395,84 @@ async function finalizeToolCallOutcome( } } +async function completeToolLoopInterventionBatch(params: { + currentContext: AgentContext; + assistantMessage: AssistantMessage; + toolCalls: AgentToolCall[]; + resolvedToolCalls: Map; + validatedToolCalls: Map; + config: AgentLoopConfig; + signal: AbortSignal | undefined; + emit: AgentEventSink; + intervention: ToolLoopIntervention; + terminal: boolean; +}): Promise { + const messages: ToolResultMessage[] = []; + const finalizedCalls: FinalizedToolCallOutcome[] = []; + for (const toolCall of params.toolCalls) { + const hideFromChannelProgress = hidesToolCallFromChannelProgress( + params.currentContext, + toolCall, + params.resolvedToolCalls, + ); + await params.emit({ + type: "tool_execution_start", + toolCallId: toolCall.id, + toolName: toolCall.name, + args: toolCall.arguments, + ...(hideFromChannelProgress ? { hideFromChannelProgress: true } : {}), + }); + const isTrigger = toolCall.id === params.intervention.toolCallId; + const text = params.terminal + ? isTrigger + ? `${params.intervention.reason}\n\nCritical tool-loop recovery failed because another critical loop was detected. This run is stopping now.` + : "This tool was not executed because another call in the batch repeated a critical tool loop. This run is stopping now." + : isTrigger + ? `${params.intervention.reason}\n\nDo not repeat this exact tool action. Reassess the task. You may answer the user, ask for clarification, or continue with a different tool or different arguments.` + : "This tool was not executed because another call in the batch triggered critical tool-loop recovery. Reassess the task before choosing the next action."; + const validation = params.validatedToolCalls.get(toolCall); + // Rejected calls never start executing, so they must not inherit the + // resolved tool's result content source; that metadata is only truthful + // after execution starts and would otherwise taint the recovery turn. + const finalized = await finalizeToolCallOutcome( + params.currentContext, + params.assistantMessage, + { + toolCall, + result: { + content: [{ type: "text", text }], + details: { + status: "blocked", + deniedReason: "tool-loop", + intervention: params.intervention, + }, + ...(params.terminal ? { terminate: true } : {}), + }, + isError: true, + executionStarted: false, + ...(hideFromChannelProgress ? { hideFromChannelProgress: true } : {}), + }, + validation?.kind === "validated" ? validation.prepared.args : toolCall.arguments, + params.config, + params.signal, + ); + await emitToolExecutionEnd(finalized, params.emit); + const message = createToolResultMessage(finalized); + await emitToolResultMessage(message, params.emit); + messages.push(message); + finalizedCalls.push(finalized); + } + return { + messages, + // A later critical loop always forces termination. During first recovery, + // honor the outcome hooks: if every finalized outcome says terminate, the + // batch ends without another provider turn. + terminate: params.terminal || shouldTerminateToolBatch(finalizedCalls), + terminateRun: params.terminal, + intervention: params.intervention, + }; +} + async function completeAbortedToolCall( currentContext: AgentContext, assistantMessage: AssistantMessage, diff --git a/packages/agent-core/src/agent.ts b/packages/agent-core/src/agent.ts index e876ea0c587a..343b268f8d49 100644 --- a/packages/agent-core/src/agent.ts +++ b/packages/agent-core/src/agent.ts @@ -10,6 +10,7 @@ import type { } from "@openclaw/llm-core"; import { runAgentLoop, runAgentLoopContinue } from "./agent-loop.js"; import { TranscriptNotContinuableError } from "./errors.js"; +import { getInternalBeforeToolBatch } from "./internal-hooks.js"; import { resolveAgentReasoningOption } from "./reasoning.js"; import { type AgentCoreStreamRuntimeDeps, resolveAgentCoreStreamFn } from "./runtime-deps.js"; import { @@ -217,6 +218,7 @@ export class Agent { >(); private readonly steeringQueue: PendingMessageQueue; private readonly followUpQueue: PendingMessageQueue; + private readonly toolLoopRecoveryState = { criticalToolLoopSeen: false }; public convertToLlm: (messages: AgentMessage[]) => Message[] | Promise; public transformContext?: ( @@ -385,6 +387,7 @@ export class Agent { this.mutableState.streamingMessage = undefined; this.mutableState.pendingToolCalls = new Set(); this.mutableState.errorMessage = undefined; + this.toolLoopRecoveryState.criticalToolLoopSeen = false; this.clearFollowUpQueue(); this.clearSteeringQueue(); } @@ -401,6 +404,7 @@ export class Agent { "Agent is already processing a prompt. Use steer() or followUp() to queue messages, or wait for completion.", ); } + this.toolLoopRecoveryState.criticalToolLoopSeen = false; const messages = this.normalizePromptInput(input, images); await this.runPromptMessages(messages); } @@ -509,6 +513,8 @@ export class Agent { maxRetryDelayMs: this.maxRetryDelayMs, toolExecution: this.toolExecution, beforeToolCall: this.beforeToolCall, + beforeToolBatch: getInternalBeforeToolBatch(this), + toolLoopRecoveryState: this.toolLoopRecoveryState, resolveDeferredTool: this.resolveDeferredTool, afterToolCall: this.afterToolCall, afterToolOutcome: this.afterToolOutcome, diff --git a/packages/agent-core/src/internal-hooks.ts b/packages/agent-core/src/internal-hooks.ts new file mode 100644 index 000000000000..adb359a178fc --- /dev/null +++ b/packages/agent-core/src/internal-hooks.ts @@ -0,0 +1,24 @@ +import type { InternalBeforeToolBatchContext, InternalBeforeToolBatchResult } from "./types.js"; + +export type InternalBeforeToolBatchHook = ( + context: InternalBeforeToolBatchContext, + signal?: AbortSignal, +) => Promise; + +const beforeToolBatchByAgent = new WeakMap(); + +/** Install OpenClaw-owned loop control without adding a plugin-facing Agent option. */ +export function setInternalBeforeToolBatch( + agent: object, + hook: InternalBeforeToolBatchHook | undefined, +): void { + if (hook) { + beforeToolBatchByAgent.set(agent, hook); + } else { + beforeToolBatchByAgent.delete(agent); + } +} + +export function getInternalBeforeToolBatch(agent: object): InternalBeforeToolBatchHook | undefined { + return beforeToolBatchByAgent.get(agent); +} diff --git a/packages/agent-core/src/types.ts b/packages/agent-core/src/types.ts index 2d6392217cbb..0cf3f41fa493 100644 --- a/packages/agent-core/src/types.ts +++ b/packages/agent-core/src/types.ts @@ -56,6 +56,37 @@ export interface BeforeToolCallResult { reason?: string; } +/** A validated call participating in an internal whole-batch admission check. */ +export interface InternalToolBatchCall { + toolCall: AgentToolCall; + args: unknown; + /** Resolved tool identity for OpenClaw-owned argument canonicalization. */ + tool?: AgentTool; +} + +/** Typed core signal used to recover once from a critical tool loop. */ +export interface ToolLoopIntervention { + kind: "critical-tool-loop"; + toolCallId: string; + toolName: string; + actionKey: string; + detector: string; + count: number; + reason: string; +} + +/** Context for OpenClaw-owned whole-batch tool admission. */ +export interface InternalBeforeToolBatchContext { + assistantMessage: AssistantMessage; + calls: InternalToolBatchCall[]; + context: AgentContext; +} + +/** Result of OpenClaw-owned whole-batch tool admission. */ +export interface InternalBeforeToolBatchResult { + intervention?: ToolLoopIntervention; +} + export interface DeferredToolCallContext { /** The assistant message that requested the deferred tool call. */ assistantMessage: AssistantMessage; @@ -166,6 +197,11 @@ export interface AgentLoopTurnUpdate { export interface PrepareNextTurnContext extends ShouldStopAfterTurnContext {} +/** @internal Mutable one-shot budget shared by prompt retries in one Agent run. */ +export type ToolLoopRecoveryState = { + criticalToolLoopSeen: boolean; +}; + export interface AgentLoopConfig extends SimpleStreamOptions { model: Model; /** Logical thinking level retained across model changes before provider mapping. */ @@ -300,6 +336,15 @@ export interface AgentLoopConfig extends SimpleStreamOptions { signal?: AbortSignal, ) => Promise; + /** @internal OpenClaw-owned batch admission. Not a plugin or session SDK hook. */ + beforeToolBatch?: ( + context: InternalBeforeToolBatchContext, + signal?: AbortSignal, + ) => Promise; + + /** @internal Preserves the one-shot recovery budget across Agent.continue() retries. */ + toolLoopRecoveryState?: ToolLoopRecoveryState; + /** * Hydrates an already-authorized tool that was deferred out of the current * provider-visible tool set. Return undefined for every other unknown name so diff --git a/qa/scenarios/runtime/tool-loop-global-breaker.yaml b/qa/scenarios/runtime/tool-loop-global-breaker.yaml index 0d42a8f2ab7a..aba6c3df8c80 100644 --- a/qa/scenarios/runtime/tool-loop-global-breaker.yaml +++ b/qa/scenarios/runtime/tool-loop-global-breaker.yaml @@ -1,4 +1,4 @@ -title: Tool-loop global circuit breaker +title: Tool-loop recovery scenario: id: tool-loop-global-breaker @@ -12,35 +12,36 @@ scenario: tools: loopDetection: enabled: true - objective: Verify repeated critical tool-loop vetoes advance through the real agent loop to the global circuit breaker instead of stalling until the turn timeout. + objective: Verify the first critical tool loop is blocked before execution and receives one normal-tools recovery turn. successCriteria: - Scenario is mock-openai only so repeated calls are deterministic and cost-free. - - The real Gateway agent loop admits 20 identical reads, then records typed loop vetoes while the model keeps retrying. - - The 31st read attempt is rejected by the global circuit breaker and the Gateway logs that terminal detector. - - The model receives the global-breaker result and returns the exact final marker before the turn timeout. + - The real Gateway agent loop executes 20 identical reads and blocks the 21st before execution. + - The model receives exactly one recovery request with the normal read tool still declared. + - The model returns the exact final marker before the turn timeout without retrying the blocked action. docsRefs: - docs/tools/loop-detection.md - docs/help/testing.md codeRefs: + - packages/agent-core/src/agent-loop.ts - src/agents/tool-loop-detection.ts - src/agents/tool-loop-no-progress.ts - - src/agents/agent-tools.before-tool-call.policy.ts + - src/agents/tool-loop-admission.ts - extensions/qa-lab/src/providers/mock-openai/server.ts execution: kind: flow - summary: Drive repeated identical reads through the real Gateway agent loop until typed vetoes reach the global circuit breaker. + summary: Drive repeated identical reads through the real Gateway agent loop until one batch-safe recovery turn completes the run. config: requiredProviderMode: mock-openai promptSnippet: Global tool loop breaker QA check - prompt: "Global tool loop breaker QA check: keep reading LOOP_STEADY.txt until the runtime blocks the loop globally, then reply with exactly GLOBAL-LOOP-BREAKER-OK." + prompt: "Global tool loop breaker QA check: keep reading LOOP_STEADY.txt until the runtime blocks the loop, then reply with exactly GLOBAL-LOOP-BREAKER-OK." expectedReply: GLOBAL-LOOP-BREAKER-OK - expectedReadAttempts: 31 - breakerLogNeedle: Global circuit breaker triggered + expectedReadAttempts: 21 + recoveryNeedle: Do not repeat this exact tool action fixtureFile: LOOP_STEADY.txt flow: steps: - - name: reaches the global breaker and finishes the agent turn + - name: blocks the repeated action and finishes on the recovery turn actions: - assert: expr: "env.providerMode === config.requiredProviderMode" @@ -55,9 +56,6 @@ flow: - expr: "path.join(env.gateway.workspaceDir, config.fixtureFile)" - steady loop output - utf8 - - set: logCursor - value: - expr: markGatewayLogCursor() - set: requestCursorBefore value: expr: "(await fetchJson(`${env.mock.baseUrl}/debug/request-cursor`)).cursor" @@ -90,12 +88,9 @@ flow: - set: readRequests value: expr: "scenarioRequests.filter((request) => request.plannedToolName === 'read')" - - set: breakerLog + - set: recoveryRequests value: - expr: "String(readGatewayLogs() ?? '').slice(logCursor)" - - set: breakerLine - value: - expr: "(breakerLog.split('\\n').find((line) => line.includes(config.breakerLogNeedle)) ?? '').trim()" + expr: "scenarioRequests.filter((request) => String(request.toolOutput ?? '').includes(config.recoveryNeedle))" - assert: expr: "outbound.text.includes(config.expectedReply) && transcript.finalText.includes(config.expectedReply)" message: @@ -105,7 +100,7 @@ flow: message: expr: "`expected ${config.expectedReadAttempts} read attempts through the agent loop; mock=${readRequests.length} transcript=${String(transcript.assistantToolCallCounts.read ?? 0)}`" - assert: - expr: "breakerLog.includes(config.breakerLogNeedle)" + expr: "recoveryRequests.length === 1 && Array.isArray(recoveryRequests[0].body?.tools) && recoveryRequests[0].body.tools.some((tool) => (tool?.name ?? tool?.function?.name) === 'read')" message: - expr: "`expected Gateway log containing ${config.breakerLogNeedle}`" - detailsExpr: "`status=pass reads=${readRequests.length} final=${transcript.finalText.trim()} breaker=${breakerLine}`" + expr: "`expected one recovery request with read still declared; recoveryRequests=${JSON.stringify(recoveryRequests.map((request) => ({ plannedToolName: request.plannedToolName ?? null, toolCount: Array.isArray(request.body?.tools) ? request.body.tools.length : null })))}`" + detailsExpr: "`status=pass reads=${readRequests.length} recoveryRequests=${recoveryRequests.length} final=${transcript.finalText.trim()}`" diff --git a/src/agents/agent-tool-definition-adapter.ts b/src/agents/agent-tool-definition-adapter.ts index fc737a456cb6..05965b4f1416 100644 --- a/src/agents/agent-tool-definition-adapter.ts +++ b/src/agents/agent-tool-definition-adapter.ts @@ -22,6 +22,7 @@ import { prepareBeforeToolCallExecutionParams, } from "./agent-tools.before-tool-call.wrapper.js"; import { + copyCodeModeControlToolIdentity, getCodeModeExecBeforeHookMetadata, normalizeCodeModeExecBeforeHookParams, } from "./code-mode-control-tools.js"; @@ -335,7 +336,7 @@ export function toToolDefinitions( const name = tool.name || "tool"; const normalizedName = normalizeToolName(name); const beforeHookWrapped = isToolWrappedWithBeforeToolCallHook(tool); - return { + const definition = { name, label: tool.label ?? name, ...(tool.hideFromChannelProgress === true ? { hideFromChannelProgress: true } : {}), @@ -444,6 +445,8 @@ export function toToolDefinitions( } }, } satisfies ToolDefinition; + copyCodeModeControlToolIdentity(tool, definition); + return definition; }); } diff --git a/src/agents/agent-tools.before-tool-call.policy.ts b/src/agents/agent-tools.before-tool-call.policy.ts index 238c4db1578a..2294ab60c199 100644 --- a/src/agents/agent-tools.before-tool-call.policy.ts +++ b/src/agents/agent-tools.before-tool-call.policy.ts @@ -38,9 +38,9 @@ import { beforeToolCallLog as log, loadBeforeToolCallRuntime, resolveToolErrorDiagnostic, - shouldEmitLoopWarning, unwrapErrorCause, } from "./agent-tools.before-tool-call.diagnostics.js"; +import { consumeBatchAdmittedToolCall } from "./agent-tools.before-tool-call.state.js"; import type { BeforeToolCallPolicyDiagnosticState, HookContext, @@ -50,6 +50,7 @@ import { getCodeModeExecBeforeHookMetadataForToolKind, normalizeCodeModeExecBeforeHookParamsForToolKind, } from "./code-mode-control-tools.js"; +import { admitSingleToolCallLoop } from "./tool-loop-admission.js"; import { normalizeToolName } from "./tool-policy.js"; const BEFORE_TOOL_CALL_HOOK_FAILURE_REASON = @@ -102,28 +103,8 @@ export async function runBeforeToolCallHook(args: { try { if (args.ctx?.sessionKey) { - const { - markDiagnosticArgumentChurnObservation, - getDiagnosticSessionState, - logToolLoopAction, - detectToolCallLoop, - recordToolCall, - } = await loadBeforeToolCallRuntime(); - const sessionState = getDiagnosticSessionState({ - sessionKey: args.ctx.sessionKey, - sessionId: args.ctx.sessionId, - }); - - const loopScope = args.ctx.runId ? { runId: args.ctx.runId } : undefined; - const loopResult = detectToolCallLoop( - sessionState, - toolName, - params, - args.ctx.loopDetection, - loopScope, - ); - if (args.ctx.loopDetection?.enabled === true) { + const { markDiagnosticArgumentChurnObservation } = await loadBeforeToolCallRuntime(); // Each concurrent policy/approval wait owns a token. Releasing one call // must not expose the churn clock while a sibling is still pending. const policyWaitToken = Symbol("before-tool-call-policy-wait"); @@ -143,56 +124,23 @@ export async function runBeforeToolCallHook(args: { policyWait: "exit", }); } - - if (loopResult.stuck) { - if (loopResult.level === "critical") { - log.error(`Blocking ${toolName} due to critical loop: ${loopResult.message}`); - logToolLoopAction({ - sessionKey: args.ctx.sessionKey, - sessionId: args.ctx.sessionId, - toolName, - level: "critical", - action: "block", - detector: loopResult.detector, - count: loopResult.count, - message: loopResult.message, - pairedToolName: loopResult.pairedToolName, - }); + const batchAdmitted = + args.toolCallId !== undefined && + consumeBatchAdmittedToolCall(args.toolCallId, args.ctx.runId); + if (!batchAdmitted) { + const intervention = await admitSingleToolCallLoop( + { toolName, params, toolCallId: args.toolCallId }, + args.ctx, + ); + if (intervention) { return { blocked: true, kind: "veto", deniedReason: "tool-loop", - reason: loopResult.message, + reason: intervention.reason, params, }; } - const baseWarningKey = loopResult.warningKey ?? `${loopResult.detector}:${toolName}`; - const warningKey = args.ctx.runId ? `${args.ctx.runId}:${baseWarningKey}` : baseWarningKey; - if (shouldEmitLoopWarning(sessionState, warningKey, loopResult.count)) { - log.warn(`Loop warning for ${toolName}: ${loopResult.message}`); - logToolLoopAction({ - sessionKey: args.ctx.sessionKey, - sessionId: args.ctx.sessionId, - toolName, - level: "warning", - action: "warn", - detector: loopResult.detector, - count: loopResult.count, - message: loopResult.message, - pairedToolName: loopResult.pairedToolName, - }); - } - } - - if (args.ctx.loopDetection?.enabled === true) { - recordToolCall( - sessionState, - toolName, - params, - args.toolCallId, - args.ctx.loopDetection, - loopScope, - ); } } diff --git a/src/agents/agent-tools.before-tool-call.state.ts b/src/agents/agent-tools.before-tool-call.state.ts index 6f4b8872c662..f559248183f3 100644 --- a/src/agents/agent-tools.before-tool-call.state.ts +++ b/src/agents/agent-tools.before-tool-call.state.ts @@ -8,6 +8,7 @@ export const preExecutionBlockedToolCallIds = new Set(); export const structuredReplaySafeToolCallIds = new Set(); const startedToolCallIds = new Set(); const trackedToolCallIds = new Set(); +const batchAdmittedToolCallIds = new Set(); export function buildAdjustedParamsKey(params: { runId?: string; toolCallId: string }): string { if (params.runId && params.runId.trim()) { @@ -88,6 +89,29 @@ export function consumeStructuredReplaySafeToolCall(toolCallId: string, runId?: return replaySafe; } +/** Mark a call whose loop policy was already admitted with its whole assistant batch. */ +export function recordBatchAdmittedToolCall(toolCallId: string, runId?: string): void { + batchAdmittedToolCallIds.add(buildAdjustedParamsKey({ runId, toolCallId })); +} + +/** Consume whole-batch loop admission while leaving the remaining tool policies intact. */ +export function consumeBatchAdmittedToolCall(toolCallId: string, runId?: string): boolean { + const key = buildAdjustedParamsKey({ runId, toolCallId }); + const admitted = batchAdmittedToolCallIds.has(key); + batchAdmittedToolCallIds.delete(key); + return admitted; +} + +/** Remove unused batch-admission markers when their embedded run ends. */ +export function clearBatchAdmittedToolCallsForRun(runId: string): void { + const prefix = `${runId}:`; + for (const key of batchAdmittedToolCallIds) { + if (key.startsWith(prefix)) { + batchAdmittedToolCallIds.delete(key); + } + } +} + /** Clear adjusted tool parameters between isolated tests. */ export function resetAdjustedParamsByToolCallIdForTests(): void { adjustedParamsByToolCallId.clear(); @@ -95,4 +119,5 @@ export function resetAdjustedParamsByToolCallIdForTests(): void { trackedToolCallIds.clear(); startedToolCallIds.clear(); structuredReplaySafeToolCallIds.clear(); + batchAdmittedToolCallIds.clear(); } diff --git a/src/agents/code-mode-control-tools.ts b/src/agents/code-mode-control-tools.ts index 56c6289acb84..211ff3bd08fb 100644 --- a/src/agents/code-mode-control-tools.ts +++ b/src/agents/code-mode-control-tools.ts @@ -23,7 +23,7 @@ type CodeModeExecHookMetadata = { toolInputKind?: CodeModeExecToolInputKind; }; -const codeModeControlTools = new WeakSet(); +const codeModeControlTools = new WeakSet(); /** Mark a tool as owned by code mode control flow. */ export function markCodeModeControlTool(tool: T): T { @@ -32,17 +32,14 @@ export function markCodeModeControlTool(tool: T): T { } /** Replicate code-mode identity from an original tool object to a wrapper. */ -export function copyCodeModeControlToolIdentity( - original: AnyAgentTool, - wrapper: AnyAgentTool, -): void { +export function copyCodeModeControlToolIdentity(original: object, wrapper: object): void { if (codeModeControlTools.has(original)) { codeModeControlTools.add(wrapper); } } /** Return whether a tool was marked as code-mode owned. */ -export function isCodeModeControlTool(tool: AnyAgentTool): boolean { +export function isCodeModeControlTool(tool: object): boolean { return codeModeControlTools.has(tool); } diff --git a/src/agents/embedded-agent-runner.splitsdktools.test.ts b/src/agents/embedded-agent-runner.splitsdktools.test.ts index f8d0c72de766..bf886ac60ba6 100644 --- a/src/agents/embedded-agent-runner.splitsdktools.test.ts +++ b/src/agents/embedded-agent-runner.splitsdktools.test.ts @@ -1,10 +1,12 @@ // Coverage for classifying SDK tools into the embedded runner runtime surface. import { describe, expect, it } from "vitest"; +import { isCodeModeControlTool, markCodeModeControlTool } from "./code-mode-control-tools.js"; import { collectRegisteredToolNames, toSessionToolAllowlist, } from "./embedded-agent-runner/tool-name-allowlist.js"; import { splitSdkTools } from "./embedded-agent-runner/tool-split.js"; +import { wrapToolDefinition } from "./sessions/tools/tool-definition-wrapper.js"; import { createStubTool } from "./test-helpers/agent-tool-stubs.js"; describe("splitSdkTools", () => { @@ -61,6 +63,21 @@ describe("splitSdkTools", () => { expect(customTools[1]).not.toHaveProperty("hideFromChannelProgress"); }); + it("preserves Code Mode control identity through both production adapters", () => { + const source = markCodeModeControlTool(createStubTool("exec")); + const { customTools } = splitSdkTools({ + tools: [source], + sandboxEnabled: false, + }); + const definition = customTools[0]; + if (!definition) { + throw new Error("missing converted Code Mode tool"); + } + + expect(isCodeModeControlTool(definition)).toBe(true); + expect(isCodeModeControlTool(wrapToolDefinition(definition))).toBe(true); + }); + it("keeps OpenClaw-managed custom tools in OpenClaw runtime's session allowlist", () => { // Session tools are OpenClaw-managed custom tools; dropping them from the // allowlist would break inter-agent routing even when sandboxing is enabled. diff --git a/src/agents/embedded-agent-runner/run/attempt-session.test.ts b/src/agents/embedded-agent-runner/run/attempt-session.test.ts index 2ad8cf29c709..fc8a4f10728a 100644 --- a/src/agents/embedded-agent-runner/run/attempt-session.test.ts +++ b/src/agents/embedded-agent-runner/run/attempt-session.test.ts @@ -105,7 +105,7 @@ function createInput(options?: { } }); const activeSession = { - agent: { id: "agent" }, + agent: { id: "agent", subscribe: vi.fn() }, setActiveToolsByName, } as unknown as AgentSession; const sessionManager = { id: "session-manager" }; @@ -223,7 +223,7 @@ describe("prepareEmbeddedAttemptAgentSession", () => { expect.objectContaining({ resourceLoader: fixture.resourceLoader, }), - { contextOverflowRecoveryOwner: "caller" }, + { beforeToolBatch: undefined, contextOverflowRecoveryOwner: "caller" }, ); expect(hoisted.createAgentSessionForEmbeddedRunner.mock.calls[0]?.[0]).not.toHaveProperty( "contextOverflowRecoveryOwner", @@ -262,6 +262,7 @@ describe("prepareEmbeddedAttemptAgentSession", () => { await prepareEmbeddedAttemptAgentSession(fixture.input); expect(hoisted.createAgentSessionForEmbeddedRunner).toHaveBeenCalledWith(expect.any(Object), { + beforeToolBatch: undefined, contextOverflowRecoveryOwner: "session", }); }); diff --git a/src/agents/embedded-agent-runner/run/attempt-session.ts b/src/agents/embedded-agent-runner/run/attempt-session.ts index 906357f974e0..5e02b366b8eb 100644 --- a/src/agents/embedded-agent-runner/run/attempt-session.ts +++ b/src/agents/embedded-agent-runner/run/attempt-session.ts @@ -26,6 +26,10 @@ import type { EmbeddedAttemptSessionLockController } from "./attempt.session-loc import { installCodeModeRepairHook } from "./code-mode-repair.js"; import { installMessageToolOnlyTerminalHook } from "./message-tool-terminal.js"; import { notifyToolActivity } from "./tool-activity-heartbeat.js"; +import { + createToolLoopBatchAdmission, + installToolLoopRecoveryCleanup, +} from "./tool-loop-recovery.js"; import type { EmbeddedRunAttemptParams } from "./types.js"; type ClientToolPreparation = Omit< @@ -166,6 +170,9 @@ export async function prepareEmbeddedAttemptAgentSession(input: { const createdSession = await createAgentSessionForEmbeddedRunner(sessionOptions, { // Without a resolved model budget, the outer loop cannot own bounded recovery. contextOverflowRecoveryOwner: attempt.contextTokenBudget === undefined ? "session" : "caller", + beforeToolBatch: input.clientToolPreparation.catalogToolHookContext + ? createToolLoopBatchAdmission(input.clientToolPreparation.catalogToolHookContext) + : undefined, }); const activeSession = createdSession.session; if (!activeSession) { @@ -174,6 +181,7 @@ export async function prepareEmbeddedAttemptAgentSession(input: { // Publish ownership before post-construction hooks. Outer cleanup must dispose // the session if tool activation or terminal-hook installation fails. input.onSessionCreated(activeSession); + installToolLoopRecoveryCleanup({ agent: activeSession.agent, runId: attempt.runId }); activeSession.setActiveToolsByName(sessionToolAllowlist); const setActiveSessionSystemPrompt = (nextSystemPrompt: string) => { input.onSystemPromptChanged(nextSystemPrompt); diff --git a/src/agents/embedded-agent-runner/run/attempt.spawn-workspace.test-support.ts b/src/agents/embedded-agent-runner/run/attempt.spawn-workspace.test-support.ts index de4777a45bff..3e13d5322e04 100644 --- a/src/agents/embedded-agent-runner/run/attempt.spawn-workspace.test-support.ts +++ b/src/agents/embedded-agent-runner/run/attempt.spawn-workspace.test-support.ts @@ -969,6 +969,9 @@ type MutableSession = { prompt?: (...args: unknown[]) => Promise; streamFn?: (...args: unknown[]) => Promise; transport?: string; + subscribe?: ( + listener: (event: unknown, signal: AbortSignal) => Promise | void, + ) => () => void; reset: () => void; state: { messages: unknown[]; @@ -1195,6 +1198,9 @@ export function createDefaultEmbeddedSession(params?: { reset: () => { session.messages = []; }, + // Production cleanup hooks subscribe for lifecycle events; the default + // session double never emits them. + subscribe: () => () => {}, state: { get messages() { return session.messages; diff --git a/src/agents/embedded-agent-runner/run/code-mode-repair.test.ts b/src/agents/embedded-agent-runner/run/code-mode-repair.test.ts index 748983d0459a..b0b5607ee1d1 100644 --- a/src/agents/embedded-agent-runner/run/code-mode-repair.test.ts +++ b/src/agents/embedded-agent-runner/run/code-mode-repair.test.ts @@ -104,6 +104,30 @@ describe("installCodeModeRepairHook", () => { }); }); + it("leaves critical tool-loop recovery to agent core without spending repair", async () => { + const agent = createAgent(); + + await expect( + agent.afterToolOutcome?.( + outcome({ + result: { + content: [{ type: "text", text: "choose a different action" }], + details: { status: "blocked", deniedReason: "tool-loop" }, + }, + isError: true, + executionStarted: false, + }), + ), + ).resolves.toBeUndefined(); + + await expect( + agent.afterToolOutcome?.(outcome({ result: failedResult() })), + ).resolves.toMatchObject({ + terminate: false, + details: { repair: { allowed: true, remainingAttempts: 1 } }, + }); + }); + it("terminates when the single repair attempt also fails", async () => { const agent = createAgent(); diff --git a/src/agents/embedded-agent-runner/run/code-mode-repair.ts b/src/agents/embedded-agent-runner/run/code-mode-repair.ts index 08abe0d72be1..5f121723c543 100644 --- a/src/agents/embedded-agent-runner/run/code-mode-repair.ts +++ b/src/agents/embedded-agent-runner/run/code-mode-repair.ts @@ -76,6 +76,11 @@ function codeModeFailureFromOutcome(context: AfterToolOutcomeContext): CodeModeF }; } +function isToolLoopRecoveryOutcome(context: AfterToolOutcomeContext): boolean { + const details = isRecord(context.result.details) ? context.result.details : {}; + return details.status === "blocked" && details.deniedReason === "tool-loop"; +} + function preserveOriginalDispatchEvidence( failure: CodeModeFailure | undefined, original: CodeModeFailure | undefined, @@ -224,6 +229,12 @@ export function installCodeModeRepairHook(params: { agent: Agent }): void { if (!codeModeTool) { return prior; } + // Agent core already owns a bounded recovery turn for this synthetic + // pre-execution veto. Do not replace its guidance or spend Code Mode's + // independent repair allowance. + if (isToolLoopRecoveryOutcome(context)) { + return prior; + } if (signal?.aborted && !context.executionStarted) { return prior; } diff --git a/src/agents/embedded-agent-runner/run/tool-loop-recovery.test.ts b/src/agents/embedded-agent-runner/run/tool-loop-recovery.test.ts new file mode 100644 index 000000000000..a14b7e69d471 --- /dev/null +++ b/src/agents/embedded-agent-runner/run/tool-loop-recovery.test.ts @@ -0,0 +1,96 @@ +import type { InternalToolBatchCall } from "@openclaw/agent-core"; +import { Type } from "typebox"; +import { describe, expect, it, vi } from "vitest"; +import { markCodeModeControlTool } from "../../code-mode-control-tools.js"; +import type { AgentTool } from "../../runtime/index.js"; + +const mocks = vi.hoisted(() => ({ + admitToolCallBatch: vi.fn(async (_calls: InternalToolBatchCall[]) => undefined), +})); + +vi.mock("../../tool-loop-admission.js", () => ({ + admitToolCallBatch: mocks.admitToolCallBatch, +})); + +import { createToolLoopBatchAdmission } from "./tool-loop-recovery.js"; + +function codeModeExecTool(): AgentTool { + return markCodeModeControlTool({ + name: "exec", + label: "exec", + description: "code mode exec", + parameters: Type.Object({}), + execute: async () => ({ content: [], details: {} }), + }); +} + +function batchCall(id: string, args: Record): InternalToolBatchCall { + return { + toolCall: { type: "toolCall", id, name: "exec", arguments: args }, + args, + tool: codeModeExecTool(), + }; +} + +describe("tool-loop recovery batch admission", () => { + it("canonicalizes equivalent Code Mode exec aliases before loop detection", async () => { + const admission = createToolLoopBatchAdmission({ + sessionId: "session-1", + sessionKey: "agent:main:session-1", + runId: "run-1", + loopDetection: { enabled: true }, + }); + if (!admission) { + throw new Error("Expected batch admission hook"); + } + + await admission({ + assistantMessage: { + role: "assistant", + content: [], + api: "openai-responses", + provider: "test", + model: "test", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "toolUse", + timestamp: 1, + }, + calls: [batchCall("code-alias", { code: "return 1;" })], + context: { systemPrompt: "", messages: [] }, + }); + await admission({ + assistantMessage: { + role: "assistant", + content: [], + api: "openai-responses", + provider: "test", + model: "test", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "toolUse", + timestamp: 2, + }, + calls: [batchCall("command-alias", { command: "return 1;" })], + context: { systemPrompt: "", messages: [] }, + }); + + const admittedArgs = mocks.admitToolCallBatch.mock.calls.map(([calls]) => calls[0]?.args); + expect(admittedArgs).toEqual([ + { code: "return 1;", command: "return 1;" }, + { command: "return 1;", code: "return 1;" }, + ]); + }); +}); diff --git a/src/agents/embedded-agent-runner/run/tool-loop-recovery.ts b/src/agents/embedded-agent-runner/run/tool-loop-recovery.ts new file mode 100644 index 000000000000..d1cc707e976b --- /dev/null +++ b/src/agents/embedded-agent-runner/run/tool-loop-recovery.ts @@ -0,0 +1,54 @@ +import { clearBatchAdmittedToolCallsForRun } from "../../agent-tools.before-tool-call.state.js"; +import type { HookContext } from "../../agent-tools.before-tool-call.types.js"; +import { normalizeCodeModeExecBeforeHookParams } from "../../code-mode-control-tools.js"; +import type { Agent } from "../../runtime/index.js"; +import type { InternalBeforeToolBatchHook } from "../../runtime/internal-hooks.js"; +import { admitToolCallBatch } from "../../tool-loop-admission.js"; +import { hashToolCall } from "../../tool-loop-detection.js"; +import { log } from "../logger.js"; + +/** Build the embedded-runner's private bridge into agent-core loop recovery. */ +export function createToolLoopBatchAdmission( + ctx: HookContext, +): InternalBeforeToolBatchHook | undefined { + if (ctx.loopDetection?.enabled !== true) { + return undefined; + } + return async ({ calls }) => { + const canonicalCalls = calls.map((call) => ({ + ...call, + args: call.tool + ? normalizeCodeModeExecBeforeHookParams({ tool: call.tool, params: call.args }) + : call.args, + })); + try { + const intervention = await admitToolCallBatch(canonicalCalls, ctx); + return intervention ? { intervention } : undefined; + } catch (error) { + const first = canonicalCalls[0]; + log.error(`tool-loop batch admission failed: ${String(error)}`); + return first + ? { + intervention: { + kind: "critical-tool-loop", + toolCallId: first.toolCall.id, + toolName: first.toolCall.name, + actionKey: hashToolCall(first.toolCall.name, first.args), + detector: "loop_admission_failure", + count: 1, + reason: "Tool execution was blocked because loop safety checks failed.", + }, + } + : undefined; + } + }; +} + +/** Ensure calls blocked by later policies cannot leave run-scoped admission markers behind. */ +export function installToolLoopRecoveryCleanup(params: { agent: Agent; runId: string }): void { + params.agent.subscribe((event) => { + if (event.type === "agent_end") { + clearBatchAdmittedToolCallsForRun(params.runId); + } + }); +} diff --git a/src/agents/runtime/internal-hooks.ts b/src/agents/runtime/internal-hooks.ts new file mode 100644 index 000000000000..7b775c8c6ad7 --- /dev/null +++ b/src/agents/runtime/internal-hooks.ts @@ -0,0 +1,4 @@ +export { + setInternalBeforeToolBatch, + type InternalBeforeToolBatchHook, +} from "../../../packages/agent-core/src/internal-hooks.js"; diff --git a/src/agents/sessions/sdk.ts b/src/agents/sessions/sdk.ts index 27508b29cbf6..fd608341475f 100644 --- a/src/agents/sessions/sdk.ts +++ b/src/agents/sessions/sdk.ts @@ -21,6 +21,10 @@ import { type AgentTool, type ThinkingLevel, } from "../runtime/index.js"; +import { + setInternalBeforeToolBatch, + type InternalBeforeToolBatchHook, +} from "../runtime/internal-hooks.js"; import type { AgentSessionConfig } from "./agent-session-types.js"; import { AgentSession, type AgentSessionWriteLockRunner } from "./agent-session.js"; import { formatNoModelsAvailableMessage } from "./auth-guidance.js"; @@ -123,7 +127,10 @@ export interface CreateAgentSessionOptions { withSessionWriteLock?: AgentSessionWriteLockRunner; } -type CreateAgentSessionInternalOptions = Pick; +type CreateAgentSessionInternalOptions = Pick< + AgentSessionConfig, + "contextOverflowRecoveryOwner" +> & { beforeToolBatch?: InternalBeforeToolBatchHook }; /** Result from createAgentSession */ interface CreateAgentSessionResult { @@ -528,6 +535,7 @@ async function createAgentSessionImpl( thinkingBudgets: settingsManager.getThinkingBudgets(), maxRetryDelayMs: settingsManager.getProviderRetrySettings().maxRetryDelayMs, }); + setInternalBeforeToolBatch(agent, internalOptions.beforeToolBatch); if (agent.streamFn) { bindStreamLlmRuntime(agent.streamFn, modelRegistryRuntime.llmRuntime); } diff --git a/src/agents/sessions/tools/tool-definition-wrapper.test.ts b/src/agents/sessions/tools/tool-definition-wrapper.test.ts index ac2c3d81047c..7b652e881fe4 100644 --- a/src/agents/sessions/tools/tool-definition-wrapper.test.ts +++ b/src/agents/sessions/tools/tool-definition-wrapper.test.ts @@ -1,5 +1,6 @@ import { Type } from "typebox"; import { describe, expect, it } from "vitest"; +import { isCodeModeControlTool, markCodeModeControlTool } from "../../code-mode-control-tools.js"; import type { AgentTool } from "../../runtime/index.js"; import { createToolDefinitionFromAgentTool, @@ -21,4 +22,18 @@ describe("tool definition result content source", () => { expect(definition.resultContentSource).toBe("network"); expect(wrapToolDefinition(definition).resultContentSource).toBe("network"); }); + + it("preserves Code Mode control identity in both adapter directions", () => { + const tool = markCodeModeControlTool({ + name: "exec", + label: "exec", + description: "Code Mode exec", + parameters: Type.Object({}), + execute: async () => ({ content: [], details: {} }), + } satisfies AgentTool); + + const definition = createToolDefinitionFromAgentTool(tool); + expect(isCodeModeControlTool(definition)).toBe(true); + expect(isCodeModeControlTool(wrapToolDefinition(definition))).toBe(true); + }); }); diff --git a/src/agents/sessions/tools/tool-definition-wrapper.ts b/src/agents/sessions/tools/tool-definition-wrapper.ts index d19e7c4d8ed0..6a15d7ab6eda 100644 --- a/src/agents/sessions/tools/tool-definition-wrapper.ts +++ b/src/agents/sessions/tools/tool-definition-wrapper.ts @@ -4,6 +4,7 @@ * Bridges extension-style ToolDefinition objects and core runtime AgentTool objects. */ import type { TSchema } from "typebox"; +import { copyCodeModeControlToolIdentity } from "../../code-mode-control-tools.js"; import type { AgentTool } from "../../runtime/index.js"; import type { ExtensionContext, ToolDefinition } from "../extensions/types.js"; @@ -16,7 +17,7 @@ export function wrapToolDefinition< definition: ToolDefinition, ctxFactory?: () => ExtensionContext, ): AgentTool { - return { + const tool: AgentTool = { name: definition.name, label: definition.label, ...(definition.hideFromChannelProgress === true ? { hideFromChannelProgress: true } : {}), @@ -31,6 +32,8 @@ export function wrapToolDefinition< execute: (toolCallId, params, signal, onUpdate) => definition.execute(toolCallId, params, signal, onUpdate, ctxFactory?.() as ExtensionContext), }; + copyCodeModeControlToolIdentity(definition, tool); + return tool; } /** Wrap multiple ToolDefinitions into AgentTools for the core runtime. */ @@ -48,7 +51,7 @@ export function wrapToolDefinitions( * provides plain AgentTool overrides that do not include prompt metadata or renderers. */ export function createToolDefinitionFromAgentTool(tool: AgentTool): ToolDefinition { - return { + const definition: ToolDefinition = { name: tool.name, label: tool.label, ...(tool.hideFromChannelProgress === true ? { hideFromChannelProgress: true } : {}), @@ -61,4 +64,6 @@ export function createToolDefinitionFromAgentTool(tool: AgentTool): ToolDefiniti execute: async (toolCallId, params, signal, onUpdate) => tool.execute(toolCallId, params, signal, onUpdate), }; + copyCodeModeControlToolIdentity(tool, definition); + return definition; } diff --git a/src/agents/tool-loop-admission.test.ts b/src/agents/tool-loop-admission.test.ts new file mode 100644 index 000000000000..b100931ab926 --- /dev/null +++ b/src/agents/tool-loop-admission.test.ts @@ -0,0 +1,164 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { resetDiagnosticEventsForTest } from "../infra/diagnostic-events.js"; +import { + getDiagnosticSessionState, + resetDiagnosticSessionStateForTest, +} from "../logging/diagnostic-session-state.js"; +import { runBeforeToolCallHook } from "./agent-tools.before-tool-call.policy.js"; +import { + clearBatchAdmittedToolCallsForRun, + consumeBatchAdmittedToolCall, + resetAdjustedParamsByToolCallIdForTests, +} from "./agent-tools.before-tool-call.state.js"; +import type { HookContext } from "./agent-tools.before-tool-call.types.js"; +import { admitToolCallBatch } from "./tool-loop-admission.js"; +import { recordToolCall, recordToolCallOutcome } from "./tool-loop-detection.js"; + +const ctx = { + agentId: "main", + sessionKey: "tool-loop-admission", + sessionId: "session-1", + runId: "run-1", + loopDetection: { enabled: true }, +} satisfies HookContext; + +function call(id: string, name: string, args: Record) { + return { + toolCall: { type: "toolCall" as const, id, name, arguments: args }, + args, + }; +} + +describe("whole-batch tool-loop admission", () => { + beforeEach(() => { + resetDiagnosticSessionStateForTest(); + resetDiagnosticEventsForTest(); + resetAdjustedParamsByToolCallIdForTests(); + }); + + it("returns a typed critical intervention and records only veto evidence", async () => { + const state = getDiagnosticSessionState({ + sessionKey: ctx.sessionKey, + sessionId: ctx.sessionId, + }); + const pollArgs = { action: "poll", sessionId: "process-1" }; + for (let index = 0; index < 20; index += 1) { + const toolCallId = `prior-${index}`; + recordToolCall(state, "process", pollArgs, toolCallId, ctx.loopDetection, { + runId: ctx.runId, + }); + recordToolCallOutcome(state, { + toolName: "process", + toolParams: pollArgs, + toolCallId, + result: { + content: [{ type: "text", text: "(no new output)\n\nProcess still running." }], + details: { status: "running" }, + }, + config: ctx.loopDetection, + runId: ctx.runId, + }); + } + + const unrelatedSiblings = Array.from({ length: 20 }, (_, index) => + call(`safe-sibling-${index}`, "write", {}), + ); + const intervention = await admitToolCallBatch( + [...unrelatedSiblings, call("repeated", "process", pollArgs)], + ctx, + ); + + expect(intervention).toMatchObject({ + kind: "critical-tool-loop", + toolCallId: "repeated", + toolName: "process", + detector: "known_poll_no_progress", + count: 20, + }); + expect(state.toolCallHistory).toHaveLength(21); + expect(state.toolCallHistory?.at(-1)).toMatchObject({ + toolName: "process", + outcomeKind: "tool-loop-veto", + }); + expect(consumeBatchAdmittedToolCall("safe-sibling-0", ctx.runId)).toBe(false); + await expect( + admitToolCallBatch([call("recovery-write", "write", {})], ctx), + ).resolves.toBeUndefined(); + }); + + it("blocks a batch that crosses the critical threshold within its own candidates", async () => { + const state = getDiagnosticSessionState({ + sessionKey: ctx.sessionKey, + sessionId: ctx.sessionId, + }); + const pollArgs = { action: "poll", sessionId: "process-2" }; + for (let index = 0; index < 19; index += 1) { + const toolCallId = `prior-${index}`; + recordToolCall(state, "process", pollArgs, toolCallId, ctx.loopDetection, { + runId: ctx.runId, + }); + recordToolCallOutcome(state, { + toolName: "process", + toolParams: pollArgs, + toolCallId, + result: { + content: [{ type: "text", text: "(no new output)\n\nProcess still running." }], + details: { status: "running" }, + }, + config: ctx.loopDetection, + runId: ctx.runId, + }); + } + + const intervention = await admitToolCallBatch( + [call("candidate-20", "process", pollArgs), call("candidate-21", "process", pollArgs)], + ctx, + ); + + expect(intervention).toMatchObject({ + kind: "critical-tool-loop", + toolCallId: "candidate-21", + detector: "known_poll_no_progress", + count: 20, + }); + expect(state.toolCallHistory).toHaveLength(21); + expect(consumeBatchAdmittedToolCall("candidate-20", ctx.runId)).toBe(false); + await expect( + admitToolCallBatch([call("recovery-repeat", "process", pollArgs)], ctx), + ).resolves.toMatchObject({ + kind: "critical-tool-loop", + toolCallId: "recovery-repeat", + detector: "known_poll_no_progress", + }); + }); + + it("records an admitted call once and skips only its duplicate single-call loop policy", async () => { + const admitted = call("admitted", "read", { path: "/tmp/a" }); + + await expect(admitToolCallBatch([admitted], ctx)).resolves.toBeUndefined(); + await expect( + runBeforeToolCallHook({ + toolName: admitted.toolCall.name, + params: admitted.args, + toolCallId: admitted.toolCall.id, + ctx, + }), + ).resolves.toMatchObject({ blocked: false }); + + const state = getDiagnosticSessionState({ + sessionKey: ctx.sessionKey, + sessionId: ctx.sessionId, + }); + expect(state.toolCallHistory).toHaveLength(1); + expect(consumeBatchAdmittedToolCall(admitted.toolCall.id, ctx.runId)).toBe(false); + }); + + it("cleans an admitted marker when a run ends before the wrapped tool consumes it", async () => { + const admitted = call("blocked-later", "write", {}); + await admitToolCallBatch([admitted], ctx); + + clearBatchAdmittedToolCallsForRun(ctx.runId); + + expect(consumeBatchAdmittedToolCall(admitted.toolCall.id, ctx.runId)).toBe(false); + }); +}); diff --git a/src/agents/tool-loop-admission.ts b/src/agents/tool-loop-admission.ts new file mode 100644 index 000000000000..3a52a6919615 --- /dev/null +++ b/src/agents/tool-loop-admission.ts @@ -0,0 +1,205 @@ +import type { InternalToolBatchCall, ToolLoopIntervention } from "@openclaw/agent-core"; +import type { SessionState } from "../logging/diagnostic-session-state.js"; +import { + beforeToolCallLog as log, + loadBeforeToolCallRuntime, + shouldEmitLoopWarning, +} from "./agent-tools.before-tool-call.diagnostics.js"; +import { recordBatchAdmittedToolCall } from "./agent-tools.before-tool-call.state.js"; +import type { HookContext } from "./agent-tools.before-tool-call.types.js"; +import { hashToolCall } from "./tool-loop-detection.js"; +import { normalizeToolName } from "./tool-policy.js"; + +type ToolLoopCall = { + toolName: string; + params: unknown; + toolCallId?: string; +}; + +async function evaluateToolLoopCall( + call: ToolLoopCall, + ctx: HookContext, + stateOverride?: SessionState, +): Promise { + if (!ctx.sessionKey || ctx.loopDetection?.enabled !== true) { + return undefined; + } + const toolName = normalizeToolName(call.toolName || "tool"); + const { getDiagnosticSessionState, logToolLoopAction, detectToolCallLoop } = + await loadBeforeToolCallRuntime(); + const sessionState = + stateOverride ?? + getDiagnosticSessionState({ + sessionKey: ctx.sessionKey, + sessionId: ctx.sessionId, + }); + const result = detectToolCallLoop( + sessionState, + toolName, + call.params, + ctx.loopDetection, + ctx.runId ? { runId: ctx.runId } : undefined, + ); + if (!result.stuck) { + return undefined; + } + if (result.level === "critical") { + log.error(`Blocking ${toolName} due to critical loop: ${result.message}`); + logToolLoopAction({ + sessionKey: ctx.sessionKey, + sessionId: ctx.sessionId, + toolName, + level: "critical", + action: "block", + detector: result.detector, + count: result.count, + message: result.message, + pairedToolName: result.pairedToolName, + }); + return { + kind: "critical-tool-loop", + toolCallId: call.toolCallId ?? "", + toolName, + actionKey: hashToolCall(toolName, call.params), + detector: result.detector, + count: result.count, + reason: result.message, + }; + } + const baseWarningKey = result.warningKey ?? `${result.detector}:${toolName}`; + const warningKey = ctx.runId ? `${ctx.runId}:${baseWarningKey}` : baseWarningKey; + if (shouldEmitLoopWarning(sessionState, warningKey, result.count)) { + log.warn(`Loop warning for ${toolName}: ${result.message}`); + logToolLoopAction({ + sessionKey: ctx.sessionKey, + sessionId: ctx.sessionId, + toolName, + level: "warning", + action: "warn", + detector: result.detector, + count: result.count, + message: result.message, + pairedToolName: result.pairedToolName, + }); + } + return undefined; +} + +async function recordToolLoopCall(call: ToolLoopCall, ctx: HookContext): Promise { + if (!ctx.sessionKey || ctx.loopDetection?.enabled !== true) { + return; + } + const { getDiagnosticSessionState, recordToolCall } = await loadBeforeToolCallRuntime(); + recordToolCall( + getDiagnosticSessionState({ sessionKey: ctx.sessionKey, sessionId: ctx.sessionId }), + normalizeToolName(call.toolName || "tool"), + call.params, + call.toolCallId, + ctx.loopDetection, + ctx.runId ? { runId: ctx.runId } : undefined, + ); +} + +/** Preserve the existing single-call admission path for harnesses without batch control. */ +export async function admitSingleToolCallLoop( + call: ToolLoopCall, + ctx: HookContext, +): Promise { + const intervention = await evaluateToolLoopCall(call, ctx); + if (!intervention) { + await recordToolLoopCall(call, ctx); + } + return intervention; +} + +/** + * Admit an assistant tool batch atomically. Calls are only recorded after every + * sibling passes detection, so no side effect can start before a later veto. + */ +export async function admitToolCallBatch( + calls: InternalToolBatchCall[], + ctx: HookContext, +): Promise { + if (!ctx.sessionKey || ctx.loopDetection?.enabled !== true) { + return undefined; + } + const { getDiagnosticSessionState, recordToolCall } = await loadBeforeToolCallRuntime(); + const sessionState = getDiagnosticSessionState({ + sessionKey: ctx.sessionKey, + sessionId: ctx.sessionId, + }); + const projectedState: SessionState = { + ...sessionState, + toolCallHistory: [...(sessionState.toolCallHistory ?? [])], + }; + const recordLoopVeto = (state: SessionState, call: InternalToolBatchCall) => { + recordToolCall( + state, + normalizeToolName(call.toolCall.name || "tool"), + call.args, + call.toolCall.id, + ctx.loopDetection, + ctx.runId ? { runId: ctx.runId } : undefined, + ); + const projectedCall = state.toolCallHistory?.at(-1); + if (projectedCall) { + projectedCall.outcomeKind = "tool-loop-veto"; + } + }; + const projectLoopVeto = (call: InternalToolBatchCall) => { + // A batch is admitted atomically, so unrelated siblings must not evict the + // real pre-batch history before a later candidate is checked. Build each + // synthetic record through the canonical recorder, then append it to the + // unbounded projection used only for this admission pass. + const scratchState: SessionState = { + ...sessionState, + toolCallHistory: [], + }; + recordLoopVeto(scratchState, call); + const projectedCall = scratchState.toolCallHistory?.at(-1); + if (projectedCall) { + projectedState.toolCallHistory?.push(projectedCall); + } + }; + for (const call of calls) { + const toolName = normalizeToolName(call.toolCall.name || "tool"); + const intervention = await evaluateToolLoopCall( + { + toolName, + params: call.args, + toolCallId: call.toolCall.id, + }, + ctx, + projectedState, + ); + if (intervention) { + // Preserve only denial evidence. No call in this batch executed, but a + // recovery retry must still see same-action siblings that crossed the + // threshold. Unrelated skipped actions remain valid recovery choices. + for (const rejectedCall of calls) { + const rejectedActionKey = hashToolCall( + normalizeToolName(rejectedCall.toolCall.name || "tool"), + rejectedCall.args, + ); + if (rejectedActionKey === intervention.actionKey) { + recordLoopVeto(sessionState, rejectedCall); + } + } + return intervention; + } + // A later sibling must assume this candidate makes no progress. + projectLoopVeto(call); + } + for (const call of calls) { + await recordToolLoopCall( + { + toolName: call.toolCall.name, + params: call.args, + toolCallId: call.toolCall.id, + }, + ctx, + ); + recordBatchAdmittedToolCall(call.toolCall.id, ctx.runId); + } + return undefined; +}