ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-25 11:55:47 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix: keep chat errors out of assistant messages

Browse Source
This commit is contained in:
Shakker
2026-07-19 16:24:31 +01:00
committed by Shakker
parent 86b1961bcc
commit 9a699c7037
5 changed files with 47 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/gateway/server-chat.agent-events.test.ts
+11 -12
View File
@@ -3235,17 +3235,17 @@ describe("agent event handler", () => {
},
});
expect(
expectDefined(
chatBroadcastCalls(broadcast)[0],
"chatBroadcastCalls(broadcast)[0] test invariant",
)[1],
).toMatchObject({
const payload = expectDefined(
chatBroadcastCalls(broadcast)[0],
"chatBroadcastCalls(broadcast)[0] test invariant",
)[1] as Record<string, unknown>;
expect(payload).toMatchObject({
runId: "client-timeout",
state: "error",
stopReason: "timeout",
errorMessage: "agent provider timeout",
});
expect(payload).not.toHaveProperty("message");
});
it.each([
@@ -4126,20 +4126,17 @@ describe("agent event handler", () => {
state?: string;
errorKind?: string;
errorMessage?: string;
message?: { role?: string; content?: Array<{ type?: string; text?: string }> };
};
expect(payload.state).toBe("error");
expect(payload.errorKind).toBe("rate_limit");
expect(payload.errorMessage).toContain("Too many requests");
expect(payload.message?.role).toBe("assistant");
expect(payload.message?.content?.[0]?.text).toContain("Too many requests");
expect(payload).not.toHaveProperty("message");
const nodePayload = sessionChatCalls(nodeSendToSession).at(-1)?.[2] as {
errorKind?: string;
message?: { role?: string; content?: Array<{ type?: string; text?: string }> };
};
expect(nodePayload.errorKind).toBe("rate_limit");
expect(nodePayload.message?.content?.[0]?.text).toContain("Too many requests");
expect(nodePayload).not.toHaveProperty("message");
});
it("suppresses delayed lifecycle chat errors for active chat.send runs while still cleaning up", () => {
@@ -4204,12 +4201,14 @@ describe("agent event handler", () => {
([, payload]) => (payload as { state?: string }).state === "error",
);
expect(chatErrors).toHaveLength(1);
expectPayloadFields(chatErrors[0]?.[1], {
const errorPayload = chatErrors[0]?.[1] as Record<string, unknown>;
expectPayloadFields(errorPayload, {
runId: "run-chat-send",
sessionKey: "session-chat-send",
state: "error",
errorMessage: "chat.send failed",
});
expect(errorPayload).not.toHaveProperty("message");
expect(chatRunState.registry.peek("run-chat-send")).toBeUndefined();
expect(clearAgentRunContext).toHaveBeenCalledWith("run-chat-send");
expect(agentRunSeq.has("run-chat-send")).toBe(false);
src/gateway/server-chat.ts
-14
View File
@@ -219,19 +219,6 @@ const CHAT_ERROR_KINDS = new Set<ChatErrorKind>([
"unknown",
]);
function buildChatErrorMessage(error: unknown): Record<string, unknown> | undefined {
const raw = error ? formatForLog(error).trim() : "";
if (!raw) {
return undefined;
}
const text = raw.startsWith("⚠️") || raw.startsWith("Error:") ? raw : `Error: ${raw}`;
return {
role: "assistant",
content: [{ type: "text", text }],
timestamp: Date.now(),
};
}
function readChatErrorKind(value: unknown): ChatErrorKind | undefined {
return typeof value === "string" && CHAT_ERROR_KINDS.has(value as ChatErrorKind)
? (value as ChatErrorKind)
@@ -1120,7 +1107,6 @@ export function createAgentEventHandler({
seq,
state: "error" as const,
errorMessage: error ? formatForLog(error) : undefined,
message: buildChatErrorMessage(error),
...(errorKind && { errorKind }),
...(stopReason && { stopReason }),
};
src/gateway/server-methods/chat-broadcast.ts
-18
View File
@@ -145,7 +145,6 @@ export function broadcastChatError(params: {
}): void {
const seq = nextChatSeq(params.context, params.runId);
const payloadAgentId = params.sessionKey === "global" ? params.agentId : undefined;
const errorText = params.errorMessage?.trim();
const payload = {
runId: params.runId,
sessionKey: params.sessionKey,
@@ -153,23 +152,6 @@ export function broadcastChatError(params: {
seq,
state: "error" as const,
errorMessage: params.errorMessage,
...(errorText
? {
message: {
role: "assistant",
content: [
{
type: "text",
text:
errorText.startsWith("⚠️") || errorText.startsWith("Error:")
? errorText
: `Error: ${errorText}`,
},
],
timestamp: Date.now(),
},
}
: {}),
};
params.context.broadcast("chat", payload, {
sessionKeys: resolveGlobalAwareNodeChatDeliveryKeys({
src/gateway/server-methods/chat.directive-tags.test.ts
+13 -17
View File
@@ -2344,7 +2344,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
);
});
it("broadcasts an error terminal after an internal-ui source reply final", async () => {
it("does not broadcast an error terminal after an internal-ui source reply final", async () => {
await createTranscriptFixture("openclaw-chat-send-agent-source-reply-error-");
mockState.triggerAgentRunStart = true;
const sourceReply = setReplyPayloadMetadata(
@@ -2375,35 +2375,28 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
const respond = vi.fn();
const context = createChatContext();
await runNonStreamingChatSend({
const broadcast = await runNonStreamingChatSend({
context,
respond,
idempotencyKey: "idem-agent-source-reply-error",
message: "hello from codex",
waitFor: "dedupe",
});
const broadcasts = (context.broadcast as unknown as ReturnType<typeof vi.fn>).mock.calls.map(
([, payload]) => payload as Record<string, unknown>,
);
expect(broadcasts[0]).toMatchObject({
expect(broadcast).toMatchObject({
runId: "idem-agent-source-reply-error",
sessionKey: "main",
state: "final",
});
expect(extractFirstTextBlock(broadcasts[0])).toBe("Codex source reply");
expect(broadcasts[1]).toMatchObject({
runId: "idem-agent-source-reply-error",
sessionKey: "main",
state: "error",
errorMessage: "tool warning",
});
expect(extractFirstTextBlock(broadcast)).toBe("Codex source reply");
const errorBroadcasts = (
context.broadcast as unknown as ReturnType<typeof vi.fn>
).mock.calls.filter(([, payload]) => (payload as { state?: unknown })?.state === "error");
expect(errorBroadcasts).toStrictEqual([]);
const dedupe = context.dedupe.get("chat:idem-agent-source-reply-error");
expect(dedupe?.ok).toBe(false);
expect(dedupe?.ok).toBe(true);
expect(dedupe?.payload).toMatchObject({
runId: "idem-agent-source-reply-error",
status: "error",
summary: "tool warning",
status: "ok",
});
});
@@ -2450,6 +2443,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
state: "error",
errorMessage: "Model login expired. Re-authenticate, then try again.",
});
expect(broadcasts[0]).not.toHaveProperty("message");
const assistantEntries = await readActiveAssistantTranscriptMessages();
expect(assistantEntries).toHaveLength(1);
expect(assistantEntries[0]?.content).toStrictEqual([
@@ -2493,6 +2487,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
state: "error",
errorMessage,
});
expect(broadcast).not.toHaveProperty("message");
const finalBroadcasts = (
context.broadcast as unknown as ReturnType<typeof vi.fn>
).mock.calls.filter(([, payload]) => (payload as { state?: unknown })?.state === "final");
@@ -2528,6 +2523,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
state: "error",
errorMessage,
});
expect(broadcast).not.toHaveProperty("message");
const dedupe = context.dedupe.get("chat:idem-agent-returned-error");
expect(dedupe?.ok).toBe(false);
expect(dedupe?.payload).toMatchObject({
src/gateway/server-methods/chat.error-broadcast.test.ts
+23 -36
View File
@@ -158,25 +158,19 @@ describe("chat.send error broadcast", () => {
expect.any(Object),
);
// Verify broadcastChatError was called (via context.broadcast)
expect(ctx.broadcast).toHaveBeenCalledWith(
"chat",
expect.objectContaining({
runId: "test-run-1",
state: "error",
errorMessage: expect.stringContaining("LLM timeout"),
message: expect.objectContaining({
role: "assistant",
content: [
expect.objectContaining({
type: "text",
text: expect.stringContaining("LLM timeout"),
}),
],
}),
}),
{ sessionKeys: ["agent:main:main"] },
);
const payload = expectDefined(ctx.broadcast.mock.calls[0], "error broadcast")[1] as Record<
string,
unknown
>;
expect(payload).toMatchObject({
runId: "test-run-1",
state: "error",
errorMessage: expect.stringContaining("LLM timeout"),
});
expect(payload).not.toHaveProperty("message");
expect(ctx.broadcast).toHaveBeenCalledWith("chat", payload, {
sessionKeys: ["agent:main:main"],
});
});
it("scopes selected-agent global errors to the linked agent", async () => {
@@ -222,22 +216,15 @@ describe("chat.send error broadcast", () => {
state: "error",
}),
);
expect(ctx.nodeSendToSession).toHaveBeenCalledWith(
"global",
"chat",
expect.objectContaining({
agentId: "main",
state: "error",
message: expect.objectContaining({
role: "assistant",
content: [
expect.objectContaining({
type: "text",
text: expect.stringContaining("LLM timeout"),
}),
],
}),
}),
);
const globalPayload = expectDefined(
ctx.nodeSendToSession.mock.calls.find(([sessionKey]) => sessionKey === "global"),
"global node error payload",
)[2] as Record<string, unknown>;
expect(globalPayload).toMatchObject({
agentId: "main",
state: "error",
errorMessage: expect.stringContaining("LLM timeout"),
});
expect(globalPayload).not.toHaveProperty("message");
});
});