Skip to content

Commit 422a137

Browse files
authored
Fix silent success for non-deliverable Bedrock Telegram turns (#82905)
* fix: handle non-deliverable terminal turns * chore: add changelog for non-deliverable turns * fix: align telegram message cache types
1 parent ee10fe1 commit 422a137

14 files changed

Lines changed: 531 additions & 38 deletions

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ Docs: https://docs.openclaw.ai
129129
- Providers/xAI: continue polling video generations while xAI reports in-flight jobs as `pending`, so Grok video requests no longer fail before the final `done` response. (#82610) Thanks @Manzojunior.
130130
- Logs: redact raw Basic auth and named security headers from `logs.tail` output before returning lines to read-scoped clients. Fixes #66832. Thanks @Magicray1217.
131131
- CLI/gateway: emit structured JSON for gateway transport close/timeout failures when `--json` is requested by health, gateway health, and devices list commands. Fixes #79108. Thanks @TurboTheTurtle.
132+
- Agents/Telegram: retry Bedrock non-visible terminal turns and mark non-deliverable attempts as trajectory errors instead of silent success. Fixes #82394. (#82905) Thanks @joshavant.
132133
- Telegram: normalize announce group targets via a new `resolveSessionTarget` channel hook so scheduled announcements resolve consistently against the same Telegram session conversation registry as inbound turns. Fixes #81229. Thanks @giodl73-repo.
133134
- QA/RTT: let `pnpm rtt` lease Convex-backed Telegram credentials while preserving RTT sample counts, sample timeouts, and result stats on the RTT harness path.
134135
- Discord: bind delayed gateway `identify` retries to the originating socket generation so retries triggered after a reconnect do not identify against a fresh socket. Fixes #82225. Thanks @giodl73-repo.

extensions/telegram/src/message-cache.ts

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ type TelegramCachedMessageObservation = {
7070
mode: TelegramMessageObservationMode;
7171
};
7272

73+
type TelegramEmbeddedReplyMessage = NonNullable<Message["reply_to_message"]>;
74+
7375
const DEFAULT_MAX_MESSAGES = 5000;
7476
const COMPACT_THRESHOLD_RATIO = 2;
7577
const persistedMessageCacheBuckets = new Map<string, TelegramMessageCacheBucket>();
@@ -344,24 +346,27 @@ function trimMessages(messages: Map<string, TelegramCachedMessageNode>, maxMessa
344346
function mergeTelegramSourceMessage(existing: Message, incoming: Message): Message {
345347
const existingReply = resolveEmbeddedReplyMessage(existing);
346348
const incomingReply = resolveEmbeddedReplyMessage(incoming);
347-
const merged = { ...existing, ...incoming };
348349
if (existingReply?.message_id != null && incomingReply?.message_id === existingReply.message_id) {
349-
return {
350-
...merged,
351-
reply_to_message: mergeTelegramSourceMessage(existingReply, incomingReply),
352-
};
350+
return Object.assign({}, existing, incoming, {
351+
reply_to_message: mergeTelegramSourceMessage(
352+
existingReply,
353+
incomingReply,
354+
) as TelegramEmbeddedReplyMessage,
355+
}) as Message;
353356
}
354-
return merged;
357+
return Object.assign({}, existing, incoming);
355358
}
356359

357360
function mergeAuthoritativeTelegramSourceMessage(existing: Message, incoming: Message): Message {
358361
const existingReply = resolveEmbeddedReplyMessage(existing);
359362
const incomingReply = resolveEmbeddedReplyMessage(incoming);
360363
if (existingReply?.message_id != null && incomingReply?.message_id === existingReply.message_id) {
361-
return {
362-
...incoming,
363-
reply_to_message: mergeTelegramSourceMessage(existingReply, incomingReply),
364-
};
364+
return Object.assign({}, incoming, {
365+
reply_to_message: mergeTelegramSourceMessage(
366+
existingReply,
367+
incomingReply,
368+
) as TelegramEmbeddedReplyMessage,
369+
}) as Message;
365370
}
366371
return incoming;
367372
}
@@ -376,9 +381,7 @@ function mergeCachedMessageNode(
376381
mode === "authoritative"
377382
? mergeAuthoritativeTelegramSourceMessage(existing.sourceMessage, incoming.sourceMessage)
378383
: mergeTelegramSourceMessage(existing.sourceMessage, incoming.sourceMessage);
379-
return normalizeRequiredMessageNode(sourceMessage, {
380-
...(Number.isFinite(threadId) ? { threadId } : {}),
381-
});
384+
return normalizeRequiredMessageNode(sourceMessage, Number.isFinite(threadId) ? { threadId } : {});
382385
}
383386

384387
function upsertCachedMessageNode(params: {
@@ -559,7 +562,11 @@ export function createTelegramMessageCache(params?: {
559562
}
560563
let recordedEntry: TelegramCachedMessageNode | null = null;
561564
for (const { node, mode } of observations) {
562-
const key = telegramMessageCacheKey({ accountId, chatId, messageId: node.messageId });
565+
const { messageId } = node;
566+
if (!messageId) {
567+
continue;
568+
}
569+
const key = telegramMessageCacheKey({ accountId, chatId, messageId });
563570
const cachedNode = upsertCachedMessageNode({ messages, key, node, mode });
564571
if (node.messageId === currentObservation.node.messageId) {
565572
recordedEntry = cachedNode;

src/agents/pi-embedded-runner/run.incomplete-turn.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1350,6 +1350,34 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => {
13501350
expect(retryInstruction).toBe(REASONING_ONLY_RETRY_INSTRUCTION);
13511351
});
13521352

1353+
it("retries signed reasoning-only Bedrock Converse turns with a visible-answer continuation", () => {
1354+
const retryInstruction = resolveReasoningOnlyRetryInstruction({
1355+
provider: "amazon-bedrock",
1356+
modelId: "openai.gpt-oss-120b-1:0",
1357+
modelApi: "bedrock-converse-stream",
1358+
aborted: false,
1359+
timedOut: false,
1360+
attempt: makeAttemptResult({
1361+
assistantTexts: [],
1362+
lastAssistant: {
1363+
role: "assistant",
1364+
stopReason: "stop",
1365+
provider: "amazon-bedrock",
1366+
model: "openai.gpt-oss-120b-1:0",
1367+
content: [
1368+
{
1369+
type: "thinking",
1370+
thinking: "internal reasoning",
1371+
thinkingSignature: "bedrock-reasoning-signature",
1372+
},
1373+
],
1374+
} as unknown as EmbeddedRunAttemptResult["lastAssistant"],
1375+
}),
1376+
});
1377+
1378+
expect(retryInstruction).toBe(REASONING_ONLY_RETRY_INSTRUCTION);
1379+
});
1380+
13531381
it("does not apply planning-only or ack fast paths to Ollama runs", () => {
13541382
const retryInstruction = resolvePlanningOnlyRetryInstruction({
13551383
provider: "ollama",
@@ -1862,6 +1890,30 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => {
18621890
expect(DEFAULT_EMPTY_RESPONSE_RETRY_LIMIT).toBe(1);
18631891
});
18641892

1893+
it("retries generic empty Bedrock Converse turns without visible text", () => {
1894+
const retryInstruction = resolveEmptyResponseRetryInstruction({
1895+
provider: "amazon-bedrock",
1896+
modelId: "openai.gpt-oss-120b-1:0",
1897+
modelApi: "bedrock-converse-stream",
1898+
payloadCount: 0,
1899+
aborted: false,
1900+
timedOut: false,
1901+
attempt: makeAttemptResult({
1902+
assistantTexts: [],
1903+
lastAssistant: {
1904+
role: "assistant",
1905+
stopReason: "stop",
1906+
provider: "amazon-bedrock",
1907+
model: "openai.gpt-oss-120b-1:0",
1908+
content: [{ type: "text", text: "" }],
1909+
usage: { input: 950, output: 103, totalTokens: 1053 },
1910+
} as unknown as EmbeddedRunAttemptResult["lastAssistant"],
1911+
}),
1912+
});
1913+
1914+
expect(retryInstruction).toBe(EMPTY_RESPONSE_RETRY_INSTRUCTION);
1915+
});
1916+
18651917
it("treats clean empty assistant turns as silent only when the caller allows it", () => {
18661918
const attempt = makeAttemptResult({
18671919
assistantTexts: [],
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
import { describe, expect, it } from "vitest";
2+
import {
3+
NON_DELIVERABLE_TERMINAL_TURN_REASON,
4+
resolveAttemptTrajectoryTerminal,
5+
resolveTerminalAssistantTexts,
6+
type ResolveAttemptTrajectoryTerminalParams,
7+
} from "./attempt-trajectory-status.js";
8+
9+
function baseParams(
10+
overrides: Partial<ResolveAttemptTrajectoryTerminalParams> = {},
11+
): ResolveAttemptTrajectoryTerminalParams {
12+
return {
13+
aborted: false,
14+
timedOut: false,
15+
assistantTexts: [],
16+
toolMetas: [],
17+
didSendViaMessagingTool: false,
18+
didSendDeterministicApprovalPrompt: false,
19+
messagingToolSentTexts: [],
20+
messagingToolSentMediaUrls: [],
21+
messagingToolSentTargets: [],
22+
successfulCronAdds: 0,
23+
synthesizedPayloadCount: 0,
24+
...overrides,
25+
};
26+
}
27+
28+
describe("attempt trajectory status", () => {
29+
it("marks a terminal turn without visible text, tools, or delivery as an error", () => {
30+
expect(resolveAttemptTrajectoryTerminal(baseParams())).toEqual({
31+
status: "error",
32+
terminalError: NON_DELIVERABLE_TERMINAL_TURN_REASON,
33+
});
34+
});
35+
36+
it("keeps visible assistant text as success", () => {
37+
expect(
38+
resolveAttemptTrajectoryTerminal(baseParams({ assistantTexts: ["Visible answer."] })),
39+
).toEqual({ status: "success" });
40+
});
41+
42+
it("keeps committed messaging tool delivery as success even without assistant text", () => {
43+
expect(
44+
resolveAttemptTrajectoryTerminal(
45+
baseParams({
46+
didSendViaMessagingTool: true,
47+
messagingToolSentTargets: [{ channel: "telegram" }],
48+
}),
49+
),
50+
).toEqual({ status: "success" });
51+
});
52+
53+
it("does not treat an uncommitted messaging tool attempt as delivery", () => {
54+
expect(
55+
resolveAttemptTrajectoryTerminal(
56+
baseParams({
57+
didSendViaMessagingTool: true,
58+
messagingToolSentTexts: [" "],
59+
messagingToolSentMediaUrls: [" "],
60+
}),
61+
),
62+
).toEqual({
63+
status: "error",
64+
terminalError: NON_DELIVERABLE_TERMINAL_TURN_REASON,
65+
});
66+
});
67+
68+
it("does not treat tool metadata alone as terminal progress", () => {
69+
expect(
70+
resolveAttemptTrajectoryTerminal(
71+
baseParams({
72+
toolMetas: [{ toolName: "read" }],
73+
}),
74+
),
75+
).toEqual({
76+
status: "error",
77+
terminalError: NON_DELIVERABLE_TERMINAL_TURN_REASON,
78+
});
79+
});
80+
81+
it("keeps synthesized terminal payloads as success", () => {
82+
expect(resolveAttemptTrajectoryTerminal(baseParams({ synthesizedPayloadCount: 1 }))).toEqual({
83+
status: "success",
84+
});
85+
});
86+
87+
it("keeps heartbeat responses as success", () => {
88+
expect(
89+
resolveAttemptTrajectoryTerminal(
90+
baseParams({
91+
heartbeatToolResponse: { notify: false, summary: "ok" },
92+
}),
93+
),
94+
).toEqual({
95+
status: "success",
96+
});
97+
});
98+
99+
it("does not treat expected silent turns as non-deliverable failures", () => {
100+
expect(resolveAttemptTrajectoryTerminal(baseParams({ silentExpected: true }))).toEqual({
101+
status: "success",
102+
});
103+
});
104+
105+
it("does not treat eligible empty silent replies as non-deliverable failures", () => {
106+
expect(
107+
resolveAttemptTrajectoryTerminal(baseParams({ emptyAssistantReplyIsSilent: true })),
108+
).toEqual({
109+
status: "success",
110+
});
111+
});
112+
113+
it("does not let the raw silent policy hide ineligible empty failures", () => {
114+
expect(
115+
resolveAttemptTrajectoryTerminal(baseParams({ emptyAssistantReplyIsSilent: false })),
116+
).toEqual({
117+
status: "error",
118+
terminalError: NON_DELIVERABLE_TERMINAL_TURN_REASON,
119+
});
120+
});
121+
122+
it("uses safe last-assistant fallback text for terminal delivery status", () => {
123+
expect(
124+
resolveTerminalAssistantTexts({
125+
assistantTexts: [],
126+
lastAssistantStopReason: "stop",
127+
lastAssistantVisibleText: "Fallback answer.",
128+
}),
129+
).toEqual(["Fallback answer."]);
130+
expect(
131+
resolveTerminalAssistantTexts({
132+
assistantTexts: [],
133+
lastAssistantStopReason: "error",
134+
lastAssistantVisibleText: "Raw provider error",
135+
}),
136+
).toEqual([]);
137+
});
138+
139+
it("marks terminal tool-use attempts as non-deliverable without explicit delivery", () => {
140+
expect(
141+
resolveAttemptTrajectoryTerminal(
142+
baseParams({
143+
assistantTexts: ["I will update that file."],
144+
toolMetas: [{ toolName: "write" }],
145+
lastAssistantStopReason: "toolUse",
146+
}),
147+
),
148+
).toEqual({
149+
status: "error",
150+
terminalError: NON_DELIVERABLE_TERMINAL_TURN_REASON,
151+
});
152+
expect(
153+
resolveAttemptTrajectoryTerminal(
154+
baseParams({
155+
assistantTexts: ["I sent the reply."],
156+
didSendViaMessagingTool: true,
157+
messagingToolSentTexts: ["sent"],
158+
lastAssistantStopReason: "toolUse",
159+
}),
160+
),
161+
).toEqual({ status: "success" });
162+
});
163+
164+
it("preserves prompt errors and interrupts", () => {
165+
expect(
166+
resolveAttemptTrajectoryTerminal(baseParams({ promptError: new Error("boom") })),
167+
).toEqual({ status: "error" });
168+
expect(resolveAttemptTrajectoryTerminal(baseParams({ timedOut: true }))).toEqual({
169+
status: "interrupted",
170+
});
171+
});
172+
});

0 commit comments

Comments
 (0)