From 996ec2dd76b910243575de9cea07f6be6c733cf6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 25 Apr 2026 07:06:41 +0100 Subject: [PATCH] fix(agents): key fallback on committed delivery --- CHANGELOG.md | 2 +- ...pi-agent.auth-profile-rotation.e2e.test.ts | 8 ++- .../run.incomplete-turn.test.ts | 56 ++++++++++++++++++- .../run.overflow-compaction.fixture.ts | 8 ++- src/agents/pi-embedded-runner/run/attempt.ts | 2 + .../pi-embedded-runner/run/incomplete-turn.ts | 45 ++++++++++----- .../pi-embedded-runner-e2e-fixtures.ts | 8 ++- 7 files changed, 104 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a503f40407c..32dc00a622e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,7 +74,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Signal: preserve sender attachment filenames and resolve missing MIME types from those filenames, so Linux `signal-cli` voice notes without `contentType` still enter audio transcription. Fixes #48614. Thanks @mindfury. -- Telegram/agents: suppress the phantom "Agent couldn't generate a response" fallback after a reply was already delivered through the messaging tool on clean non-error terminal turns. (#70623) Thanks @chinar-amrutkar. +- Telegram/agents: suppress the phantom "Agent couldn't generate a response" fallback after a reply was already committed through the messaging tool. (#70623) Thanks @chinar-amrutkar. - Dashboard/security: avoid writing tokenized Control UI URLs or SSH hints to runtime logs, keeping gateway bearer fragments out of console-captured logs readable through `logs.tail`. (#70029) Thanks @Ziy1-Tan. - Providers/OpenRouter: treat DeepSeek refs as cache-TTL eligible without injecting Anthropic cache-control markers, aligning context pruning with OpenRouter-managed prompt caching. (#51983) Thanks @QuinnH496. - Control UI/browser: defer temp-dir access-mode constants until Node-only temp-dir resolution runs, preventing browser bundles from crashing when `node:fs` constants are stubbed. (#48930) Thanks @Valentinws. diff --git a/src/agents/pi-embedded-runner.run-embedded-pi-agent.auth-profile-rotation.e2e.test.ts b/src/agents/pi-embedded-runner.run-embedded-pi-agent.auth-profile-rotation.e2e.test.ts index ca438b76769..b242e091993 100644 --- a/src/agents/pi-embedded-runner.run-embedded-pi-agent.auth-profile-rotation.e2e.test.ts +++ b/src/agents/pi-embedded-runner.run-embedded-pi-agent.auth-profile-rotation.e2e.test.ts @@ -182,6 +182,8 @@ const buildAssistant = (overrides: Partial): AssistantMessage const makeAttempt = (overrides: Partial): EmbeddedRunAttemptResult => { const toolMetas = overrides.toolMetas ?? []; const didSendViaMessagingTool = overrides.didSendViaMessagingTool ?? false; + const messagingToolSentTexts = overrides.messagingToolSentTexts ?? []; + const messagingToolSentMediaUrls = overrides.messagingToolSentMediaUrls ?? []; const successfulCronAdds = overrides.successfulCronAdds; return { aborted: false, @@ -202,11 +204,13 @@ const makeAttempt = (overrides: Partial): EmbeddedRunA buildAttemptReplayMetadata({ toolMetas, didSendViaMessagingTool, + messagingToolSentTexts, + messagingToolSentMediaUrls, successfulCronAdds, }), didSendViaMessagingTool, - messagingToolSentTexts: [], - messagingToolSentMediaUrls: [], + messagingToolSentTexts, + messagingToolSentMediaUrls, messagingToolSentTargets: [], cloudCodeAssistFormatError: false, itemLifecycle: { startedCount: 0, completedCount: 0, activeCount: 0 }, diff --git a/src/agents/pi-embedded-runner/run.incomplete-turn.test.ts b/src/agents/pi-embedded-runner/run.incomplete-turn.test.ts index f308e388780..6a4191ecc49 100644 --- a/src/agents/pi-embedded-runner/run.incomplete-turn.test.ts +++ b/src/agents/pi-embedded-runner/run.incomplete-turn.test.ts @@ -302,6 +302,7 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => { makeAttemptResult({ assistantTexts: [], didSendViaMessagingTool: true, + messagingToolSentTexts: ["Delivered through the message tool."], lastAssistant: { role: "assistant", stopReason: "stop", @@ -801,7 +802,7 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => { expect(incompleteTurnText).toBeNull(); }); - it("suppresses the incomplete-turn warning when a messaging tool delivered and the turn ended cleanly", () => { + it("suppresses the incomplete-turn warning after committed messaging text delivery", () => { const incompleteTurnText = resolveIncompleteTurnPayloadText({ payloadCount: 0, aborted: false, @@ -809,6 +810,7 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => { attempt: makeAttemptResult({ assistantTexts: [], didSendViaMessagingTool: true, + messagingToolSentTexts: ["Delivered through the message tool."], lastAssistant: { role: "assistant", stopReason: "stop", @@ -822,7 +824,7 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => { expect(incompleteTurnText).toBeNull(); }); - it("suppresses the incomplete-turn warning when a messaging tool delivered before end_turn", () => { + it("suppresses the incomplete-turn warning after committed messaging delivery before end_turn", () => { const incompleteTurnText = resolveIncompleteTurnPayloadText({ payloadCount: 0, aborted: false, @@ -830,6 +832,7 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => { attempt: makeAttemptResult({ assistantTexts: [], didSendViaMessagingTool: true, + messagingToolSentTexts: ["Delivered through the message tool."], lastAssistant: { role: "assistant", stopReason: "end_turn", @@ -849,7 +852,52 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => { expect(incompleteTurnText).toBeNull(); }); - it("still surfaces the incomplete-turn warning after a messaging tool when the provider signalled an error", () => { + it("suppresses the incomplete-turn warning after committed media-only messaging delivery", () => { + const incompleteTurnText = resolveIncompleteTurnPayloadText({ + payloadCount: 0, + aborted: false, + timedOut: false, + attempt: makeAttemptResult({ + assistantTexts: [], + didSendViaMessagingTool: false, + messagingToolSentMediaUrls: ["file:///tmp/render.png"], + lastAssistant: { + role: "assistant", + stopReason: "end_turn", + provider: "openai", + model: "gpt-5.4", + content: [], + } as unknown as EmbeddedRunAttemptResult["lastAssistant"], + }), + }); + + expect(incompleteTurnText).toBeNull(); + }); + + it("suppresses the incomplete-turn warning after committed messaging delivery even when the provider errored", () => { + const incompleteTurnText = resolveIncompleteTurnPayloadText({ + payloadCount: 0, + aborted: false, + timedOut: false, + attempt: makeAttemptResult({ + assistantTexts: [], + didSendViaMessagingTool: true, + messagingToolSentTexts: ["Delivered before the provider error."], + lastAssistant: { + role: "assistant", + stopReason: "error", + provider: "ollama", + model: "kimi-k2.6:cloud", + errorMessage: "provider failed after delivery", + content: [], + } as unknown as EmbeddedRunAttemptResult["lastAssistant"], + }), + }); + + expect(incompleteTurnText).toBeNull(); + }); + + it("still surfaces the incomplete-turn warning when no messaging delivery was committed", () => { const incompleteTurnText = resolveIncompleteTurnPayloadText({ payloadCount: 0, aborted: false, @@ -1136,6 +1184,8 @@ describe("resolvePlanningOnlyRetryInstruction single-action loophole", () => { replayMetadata: buildAttemptReplayMetadata({ toolMetas, didSendViaMessagingTool: false, + messagingToolSentTexts: [], + messagingToolSentMediaUrls: [], }), clientToolCall: null, yieldDetected: false, diff --git a/src/agents/pi-embedded-runner/run.overflow-compaction.fixture.ts b/src/agents/pi-embedded-runner/run.overflow-compaction.fixture.ts index 79a6ca753d9..dbebd4004f9 100644 --- a/src/agents/pi-embedded-runner/run.overflow-compaction.fixture.ts +++ b/src/agents/pi-embedded-runner/run.overflow-compaction.fixture.ts @@ -31,6 +31,8 @@ export function makeAttemptResult( ): EmbeddedRunAttemptResult { const toolMetas = overrides.toolMetas ?? []; const didSendViaMessagingTool = overrides.didSendViaMessagingTool ?? false; + const messagingToolSentTexts = overrides.messagingToolSentTexts ?? []; + const messagingToolSentMediaUrls = overrides.messagingToolSentMediaUrls ?? []; const successfulCronAdds = overrides.successfulCronAdds; return { aborted: false, @@ -50,6 +52,8 @@ export function makeAttemptResult( buildAttemptReplayMetadata({ toolMetas, didSendViaMessagingTool, + messagingToolSentTexts, + messagingToolSentMediaUrls, successfulCronAdds, }), itemLifecycle: { @@ -58,8 +62,8 @@ export function makeAttemptResult( activeCount: 0, }, didSendViaMessagingTool, - messagingToolSentTexts: [], - messagingToolSentMediaUrls: [], + messagingToolSentTexts, + messagingToolSentMediaUrls, messagingToolSentTargets: [], cloudCodeAssistFormatError: false, ...overrides, diff --git a/src/agents/pi-embedded-runner/run/attempt.ts b/src/agents/pi-embedded-runner/run/attempt.ts index c2bc4162b86..6acbf5749e4 100644 --- a/src/agents/pi-embedded-runner/run/attempt.ts +++ b/src/agents/pi-embedded-runner/run/attempt.ts @@ -2969,6 +2969,8 @@ export async function runEmbeddedAttempt( const observedReplayMetadata = buildAttemptReplayMetadata({ toolMetas: toolMetasNormalized, didSendViaMessagingTool: didSendViaMessagingTool(), + messagingToolSentTexts: getMessagingToolSentTexts(), + messagingToolSentMediaUrls: getMessagingToolSentMediaUrls(), successfulCronAdds: getSuccessfulCronAdds(), }); const replayMetadata = replayMetadataFromState( diff --git a/src/agents/pi-embedded-runner/run/incomplete-turn.ts b/src/agents/pi-embedded-runner/run/incomplete-turn.ts index 1050a5bfa1c..0dde8fc53e3 100644 --- a/src/agents/pi-embedded-runner/run/incomplete-turn.ts +++ b/src/agents/pi-embedded-runner/run/incomplete-turn.ts @@ -13,7 +13,11 @@ import type { EmbeddedRunAttemptResult } from "./types.js"; type ReplayMetadataAttempt = Pick< EmbeddedRunAttemptResult, - "toolMetas" | "didSendViaMessagingTool" | "successfulCronAdds" + | "toolMetas" + | "didSendViaMessagingTool" + | "messagingToolSentTexts" + | "messagingToolSentMediaUrls" + | "successfulCronAdds" >; type IncompleteTurnAttempt = Pick< @@ -24,6 +28,8 @@ type IncompleteTurnAttempt = Pick< | "yieldDetected" | "didSendDeterministicApprovalPrompt" | "didSendViaMessagingTool" + | "messagingToolSentTexts" + | "messagingToolSentMediaUrls" | "lastToolError" | "lastAssistant" | "replayMetadata" @@ -155,12 +161,31 @@ export type PlanningOnlyPlanDetails = { steps: string[]; }; +function hasStringEntry(values: readonly unknown[] | undefined): boolean { + return ( + Array.isArray(values) && + values.some((value) => typeof value === "string" && value.trim().length > 0) + ); +} + +export function hasCommittedUserVisibleToolDelivery( + attempt: Pick, +): boolean { + return ( + hasStringEntry(attempt.messagingToolSentTexts) || + hasStringEntry(attempt.messagingToolSentMediaUrls) + ); +} + export function buildAttemptReplayMetadata( params: ReplayMetadataAttempt, ): EmbeddedRunAttemptResult["replayMetadata"] { const hadMutatingTools = params.toolMetas.some((t) => isLikelyMutatingToolName(t.toolName)); const hadPotentialSideEffects = - hadMutatingTools || params.didSendViaMessagingTool || (params.successfulCronAdds ?? 0) > 0; + hadMutatingTools || + params.didSendViaMessagingTool || + hasCommittedUserVisibleToolDelivery(params) || + (params.successfulCronAdds ?? 0) > 0; return { hadPotentialSideEffects, replaySafe: !hadPotentialSideEffects, @@ -189,21 +214,11 @@ export function resolveIncompleteTurnPayloadText(params: { return null; } - const stopReason = params.attempt.lastAssistant?.stopReason; - // If the assistant already delivered user-visible content via a messaging - // tool during this turn and did not end in a hard error/interrupted tool-use - // state, do not surface an incomplete-turn warning. The user has received the - // reply; a follow-up "couldn't generate a response" bubble is a false positive. - // Provider-side failures and interrupted tool-use still fall through to the - // normal incomplete-turn paths below; tool-error cases are already handled by - // the lastToolError early return above. - if ( - params.attempt.didSendViaMessagingTool && - stopReason !== "error" && - stopReason !== "toolUse" - ) { + if (hasCommittedUserVisibleToolDelivery(params.attempt)) { return null; } + + const stopReason = params.attempt.lastAssistant?.stopReason; const incompleteTerminalAssistant = isIncompleteTerminalAssistantTurn({ hasAssistantVisibleText: params.payloadCount > 0, lastAssistant: params.attempt.lastAssistant, diff --git a/src/agents/test-helpers/pi-embedded-runner-e2e-fixtures.ts b/src/agents/test-helpers/pi-embedded-runner-e2e-fixtures.ts index 53826239e11..4fed384fff1 100644 --- a/src/agents/test-helpers/pi-embedded-runner-e2e-fixtures.ts +++ b/src/agents/test-helpers/pi-embedded-runner-e2e-fixtures.ts @@ -99,6 +99,8 @@ export function makeEmbeddedRunnerAttempt( ): EmbeddedRunAttemptResult { const toolMetas = overrides.toolMetas ?? []; const didSendViaMessagingTool = overrides.didSendViaMessagingTool ?? false; + const messagingToolSentTexts = overrides.messagingToolSentTexts ?? []; + const messagingToolSentMediaUrls = overrides.messagingToolSentMediaUrls ?? []; const successfulCronAdds = overrides.successfulCronAdds; return { aborted: false, @@ -119,11 +121,13 @@ export function makeEmbeddedRunnerAttempt( buildAttemptReplayMetadata({ toolMetas, didSendViaMessagingTool, + messagingToolSentTexts, + messagingToolSentMediaUrls, successfulCronAdds, }), didSendViaMessagingTool, - messagingToolSentTexts: [], - messagingToolSentMediaUrls: [], + messagingToolSentTexts, + messagingToolSentMediaUrls, messagingToolSentTargets: [], cloudCodeAssistFormatError: false, itemLifecycle: { startedCount: 0, completedCount: 0, activeCount: 0 },