mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-13 02:36:04 +00:00
fix(agents): repair context-engine tool-result pairing
Context-engine assembly could replace an already-repaired attempt transcript with a view containing orphaned tool results. Reuse the attempt-level tool-use/tool-result pairing repair after initial assembly and ownsCompaction loop assembly so strict OpenAI-compatible replay does not emit free-floating tool messages. Fixes #88561. Verification: - .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main - git diff --check origin/main...HEAD - PR CI: https://github.com/openclaw/openclaw/actions/runs/26766745554 - Real behavior proof: https://github.com/openclaw/openclaw/actions/runs/26767235053 Co-authored-by: Ted Li <tl2493@columbia.edu>
This commit is contained in:
@@ -1740,6 +1740,44 @@ describe("runEmbeddedAttempt context engine sessionKey forwarding", () => {
|
||||
expect(hoisted.preemptiveCompactionCalls.at(-1)).not.toHaveProperty("unwindowedMessages");
|
||||
});
|
||||
|
||||
it("repairs tool-result pairing after context engine assembly", async () => {
|
||||
let promptMessages: AgentMessage[] = [];
|
||||
|
||||
await createContextEngineAttemptRunner({
|
||||
contextEngine: createTestContextEngine({
|
||||
assemble: async () => ({
|
||||
messages: [
|
||||
{ role: "user", content: "assembled context", timestamp: 1 },
|
||||
{
|
||||
role: "toolResult",
|
||||
toolCallId: "call_orphan",
|
||||
toolUseId: "call_orphan",
|
||||
toolName: "read",
|
||||
content: [{ type: "text", text: "orphaned result" }],
|
||||
timestamp: 2,
|
||||
},
|
||||
] as AgentMessage[],
|
||||
estimatedTokens: 8,
|
||||
}),
|
||||
}),
|
||||
sessionKey,
|
||||
tempPaths,
|
||||
sessionPrompt: async (session) => {
|
||||
promptMessages = session.messages.map((message) => message as AgentMessage);
|
||||
session.messages = [
|
||||
...session.messages,
|
||||
{ role: "assistant", content: "done", timestamp: 3 },
|
||||
];
|
||||
},
|
||||
});
|
||||
|
||||
expect(promptMessages).toContainEqual(
|
||||
expect.objectContaining({ role: "user", content: "assembled context" }),
|
||||
);
|
||||
expect(promptMessages.some((message) => message.role === "toolResult")).toBe(false);
|
||||
expect(JSON.stringify(promptMessages)).not.toContain("orphaned result");
|
||||
});
|
||||
|
||||
it("honors context engines that opt into preassembly overflow authority", async () => {
|
||||
const lockEvents = trackSessionWriteLocks();
|
||||
let sawPrompt = false;
|
||||
|
||||
@@ -682,6 +682,7 @@ vi.mock("../../tool-policy.js", async (importOriginal) => {
|
||||
vi.mock("../../transcript-policy.js", () => ({
|
||||
resolveTranscriptPolicy: () => ({
|
||||
allowSyntheticToolResults: false,
|
||||
repairToolUseResultPairing: true,
|
||||
}),
|
||||
shouldAllowProviderOwnedThinkingReplay: () => false,
|
||||
}));
|
||||
|
||||
@@ -604,6 +604,16 @@ function flushSessionManagerFile(sessionManager: ReturnType<typeof guardSessionM
|
||||
(sessionManager as unknown as { rewriteFile?: () => void }).rewriteFile?.();
|
||||
}
|
||||
|
||||
function repairAttemptToolUseResultPairing(
|
||||
messages: AgentMessage[],
|
||||
isOpenAIResponsesApi: boolean,
|
||||
): AgentMessage[] {
|
||||
return sanitizeToolUseResultPairing(messages, {
|
||||
erroredAssistantResultPolicy: "drop",
|
||||
...(isOpenAIResponsesApi ? { missingToolResultText: "aborted" } : {}),
|
||||
});
|
||||
}
|
||||
|
||||
function shouldPreservePromptErrorAfterCleanupError(params: {
|
||||
promptError: unknown;
|
||||
cleanupError: unknown;
|
||||
@@ -1963,6 +1973,10 @@ export async function runEmbeddedAttempt(
|
||||
config: params.config,
|
||||
env: process.env,
|
||||
});
|
||||
const isOpenAIResponsesApi =
|
||||
params.model.api === "openai-responses" ||
|
||||
params.model.api === "azure-openai-responses" ||
|
||||
params.model.api === "openai-chatgpt-responses";
|
||||
|
||||
await prewarmSessionFile(params.sessionFile);
|
||||
const preparedUserTurnMessage = await params.userTurnTranscriptRecorder?.resolveMessage();
|
||||
@@ -2356,6 +2370,12 @@ export async function runEmbeddedAttempt(
|
||||
sessionFile: params.sessionFile,
|
||||
tokenBudget: params.contextTokenBudget,
|
||||
modelId: params.modelId,
|
||||
...(transcriptPolicy.repairToolUseResultPairing
|
||||
? {
|
||||
repairAssembledMessages: (messages) =>
|
||||
repairAttemptToolUseResultPairing(messages, isOpenAIResponsesApi),
|
||||
}
|
||||
: {}),
|
||||
getPrePromptMessageCount: () => prePromptMessageCount,
|
||||
onAfterTurnCheckpoint: (messageCount) => {
|
||||
contextEngineAfterTurnCheckpoint = messageCount;
|
||||
@@ -2653,11 +2673,6 @@ export async function runEmbeddedAttempt(
|
||||
// historical messages at attempt start, but the agent loop's internal tool call →
|
||||
// tool result cycles bypass that path. Wrap streamFn so every outbound request
|
||||
// sees sanitized tool call IDs.
|
||||
const isOpenAIResponsesApi =
|
||||
params.model.api === "openai-responses" ||
|
||||
params.model.api === "azure-openai-responses" ||
|
||||
params.model.api === "openai-chatgpt-responses";
|
||||
|
||||
const replayToolCallIdSanitizerDecision = {
|
||||
sanitizeToolCallIds: transcriptPolicy.sanitizeToolCallIds,
|
||||
toolCallIdMode: transcriptPolicy.toolCallIdMode,
|
||||
@@ -2920,10 +2935,7 @@ export async function runEmbeddedAttempt(
|
||||
// limitHistoryTurns can orphan tool_result blocks by removing the
|
||||
// assistant message that contained the matching tool_use.
|
||||
const limited = transcriptPolicy.repairToolUseResultPairing
|
||||
? sanitizeToolUseResultPairing(truncated, {
|
||||
erroredAssistantResultPolicy: "drop",
|
||||
...(isOpenAIResponsesApi ? { missingToolResultText: "aborted" } : {}),
|
||||
})
|
||||
? repairAttemptToolUseResultPairing(truncated, isOpenAIResponsesApi)
|
||||
: truncated;
|
||||
cacheTrace?.recordStage("session:limited", { messages: limited });
|
||||
if (limited.length > 0 || prior.length > 0) {
|
||||
@@ -2952,8 +2964,11 @@ export async function runEmbeddedAttempt(
|
||||
if (!assembled) {
|
||||
throw new Error("context engine assemble returned no result");
|
||||
}
|
||||
if (assembled.messages !== activeSession.messages) {
|
||||
activeSession.agent.state.messages = assembled.messages;
|
||||
const assembledMessages = transcriptPolicy.repairToolUseResultPairing
|
||||
? repairAttemptToolUseResultPairing(assembled.messages, isOpenAIResponsesApi)
|
||||
: assembled.messages;
|
||||
if (assembledMessages !== activeSession.messages) {
|
||||
activeSession.agent.state.messages = assembledMessages;
|
||||
}
|
||||
contextEnginePromptAuthority = assembled.promptAuthority ?? "assembled";
|
||||
if (contextEnginePromptAuthority === "preassembly_may_overflow") {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import type { AgentMessage } from "openclaw/plugin-sdk/agent-core";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { ContextEngine } from "../../context-engine/types.js";
|
||||
import { sanitizeToolUseResultPairing } from "../session-transcript-repair.js";
|
||||
import { castAgentMessage } from "../test-helpers/agent-message-fixtures.js";
|
||||
import { MidTurnPrecheckSignal } from "./run/midturn-precheck.js";
|
||||
import {
|
||||
@@ -787,6 +788,61 @@ describe("installContextEngineLoopHook", () => {
|
||||
expect(transformed).toBe(compactedView);
|
||||
});
|
||||
|
||||
it("repairs tool-result pairing in ownsCompaction assembled loop views", async () => {
|
||||
const agent = makeGuardableAgent();
|
||||
const assembledView = [makeUser("compacted"), makeToolResult("call_orphan", "stale")];
|
||||
const engine = makeMockEngine({
|
||||
assemble: async () => ({ messages: assembledView, estimatedTokens: 0 }),
|
||||
});
|
||||
installContextEngineLoopHook({
|
||||
agent,
|
||||
contextEngine: engine,
|
||||
sessionId,
|
||||
sessionKey,
|
||||
sessionFile,
|
||||
tokenBudget,
|
||||
modelId,
|
||||
repairAssembledMessages: sanitizeToolUseResultPairing,
|
||||
});
|
||||
|
||||
const { transformed } = await callAfterInitialToolResult(agent, {
|
||||
includeSecondUser: false,
|
||||
firstResultText: "r",
|
||||
});
|
||||
|
||||
expect(transformed).toEqual([expect.objectContaining({ role: "user", content: "compacted" })]);
|
||||
expect((transformed as AgentMessage[]).some((message) => message.role === "toolResult")).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
it("repairs same-reference ownsCompaction assembled loop views", async () => {
|
||||
const agent = makeGuardableAgent();
|
||||
const engine = makeMockEngine();
|
||||
installContextEngineLoopHook({
|
||||
agent,
|
||||
contextEngine: engine,
|
||||
sessionId,
|
||||
sessionKey,
|
||||
sessionFile,
|
||||
tokenBudget,
|
||||
modelId,
|
||||
repairAssembledMessages: sanitizeToolUseResultPairing,
|
||||
});
|
||||
|
||||
const { transformed, withNew } = await callAfterInitialToolResult(agent, {
|
||||
includeSecondUser: false,
|
||||
firstResultText: "r",
|
||||
});
|
||||
|
||||
expect(recordMockArg(engine.assemble).messages).toBe(withNew);
|
||||
expect(transformed).not.toBe(withNew);
|
||||
expect(transformed).toEqual([expect.objectContaining({ role: "user", content: "first" })]);
|
||||
expect((transformed as AgentMessage[]).some((message) => message.role === "toolResult")).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
it("clears an assembled view when the engine fails on a later source", async () => {
|
||||
const agent = makeGuardableAgent();
|
||||
const compactedView = [makeUser("compacted")];
|
||||
|
||||
@@ -324,6 +324,7 @@ export function installContextEngineLoopHook(params: {
|
||||
sessionFile: string;
|
||||
tokenBudget?: number;
|
||||
modelId: string;
|
||||
repairAssembledMessages?: (messages: AgentMessage[]) => AgentMessage[];
|
||||
getPrePromptMessageCount?: () => number;
|
||||
onAfterTurnCheckpoint?: (messageCount: number) => void;
|
||||
getRuntimeContext?: (params: {
|
||||
@@ -427,11 +428,14 @@ export function installContextEngineLoopHook(params: {
|
||||
});
|
||||
if (
|
||||
assembled &&
|
||||
Array.isArray(assembled.messages) &&
|
||||
assembled.messages !== providerMessages
|
||||
Array.isArray(assembled.messages)
|
||||
) {
|
||||
lastAssembledView = assembled.messages;
|
||||
return assembled.messages;
|
||||
const repairedMessages =
|
||||
params.repairAssembledMessages?.(assembled.messages) ?? assembled.messages;
|
||||
if (repairedMessages !== providerMessages || assembled.messages !== providerMessages) {
|
||||
lastAssembledView = repairedMessages;
|
||||
return repairedMessages;
|
||||
}
|
||||
}
|
||||
lastAssembledView = null;
|
||||
} catch {
|
||||
|
||||
Reference in New Issue
Block a user