fix(cron): deliver full announce output instead of last chunk only (#57322)

resolveCronPayloadOutcome() collapsed announce delivery to the last
deliverable payload. Replace with pickDeliverablePayloads() that
preserves all successful text payloads. Error-only runs fall back to
the last error payload only.

Extract shared isDeliverablePayload() helper. Keep
deliveryPayloadHasStructuredContent scoped to the last payload
to preserve downstream finalizeTextDelivery safeguards.

Fixes #13812
This commit is contained in:
Robin Waslander
2026-03-30 01:24:45 +02:00
committed by GitHub
parent 0a4c11061d
commit bdd9bc93f1
6 changed files with 143 additions and 19 deletions

View File

@@ -39,6 +39,42 @@ describe("runCronIsolatedAgentTurn forum topic delivery", () => {
});
});
it("delivers all successful text chunks to forum-topic telegram targets", async () => {
await withTempCronHome(async (home) => {
const storePath = await writeSessionStore(home, { lastProvider: "webchat", lastTo: "" });
const deps = createCliDeps();
mockAgentPayloads([
{ text: "section 1" },
{ text: "temporary error", isError: true },
{ text: "section 2" },
]);
const res = await runTelegramAnnounceTurn({
home,
storePath,
deps,
delivery: { mode: "announce", channel: "telegram", to: "123:topic:42" },
});
expect(res.status).toBe("ok");
expect(res.delivered).toBe(true);
expect(runSubagentAnnounceFlow).not.toHaveBeenCalled();
expect(deps.sendMessageTelegram).toHaveBeenCalledTimes(2);
expect(deps.sendMessageTelegram).toHaveBeenNthCalledWith(
1,
"123",
"section 1",
expect.objectContaining({ messageThreadId: 42 }),
);
expect(deps.sendMessageTelegram).toHaveBeenNthCalledWith(
2,
"123",
"section 2",
expect.objectContaining({ messageThreadId: 42 }),
);
});
});
it("routes plain telegram targets through the correct delivery path", async () => {
await withTempCronHome(async (home) => {
const storePath = await writeSessionStore(home, { lastProvider: "webchat", lastTo: "" });

View File

@@ -59,4 +59,41 @@ describe("resolveCronPayloadOutcome", () => {
expect(String(result.summary ?? "")).toMatch(/…$/);
});
it("preserves all successful deliverable payloads for announce delivery", () => {
const result = resolveCronPayloadOutcome({
payloads: [
{ text: "line 1" },
{ text: "temporary error", isError: true },
{ text: "line 2" },
],
});
expect(result.deliveryPayloads).toEqual([{ text: "line 1" }, { text: "line 2" }]);
expect(result.deliveryPayload).toEqual({ text: "line 2" });
});
it("keeps structured-content detection scoped to the last delivery payload", () => {
const result = resolveCronPayloadOutcome({
payloads: [{ mediaUrl: "https://example.com/report.png" }, { text: "final text" }],
});
expect(result.deliveryPayloads).toEqual([
{ mediaUrl: "https://example.com/report.png" },
{ text: "final text" },
]);
expect(result.deliveryPayloadHasStructuredContent).toBe(false);
});
it("returns only the last error payload when all payloads are errors", () => {
const result = resolveCronPayloadOutcome({
payloads: [
{ text: "first error", isError: true },
{ text: "last error", isError: true },
],
});
expect(result.deliveryPayloads).toEqual([{ text: "last error", isError: true }]);
expect(result.deliveryPayload).toEqual({ text: "last error", isError: true });
});
});

View File

@@ -244,7 +244,7 @@ async function assertExplicitTelegramTargetDelivery(params: {
storePath: string;
deps: CliDeps;
payloads: Array<Record<string, unknown>>;
expectedText: string;
expectedTexts: string[];
}): Promise<void> {
mockAgentPayloads(params.payloads);
const res = await runExplicitTelegramAnnounceTurn({
@@ -255,10 +255,22 @@ async function assertExplicitTelegramTargetDelivery(params: {
expectDeliveredOk(res);
expect(runSubagentAnnounceFlow).not.toHaveBeenCalled();
expectDirectTelegramDelivery(params.deps, {
chatId: "123",
text: params.expectedText,
});
if (params.expectedTexts.length === 1) {
expectDirectTelegramDelivery(params.deps, {
chatId: "123",
text: params.expectedTexts[0] ?? "",
});
return;
}
expect(params.deps.sendMessageTelegram).toHaveBeenCalledTimes(params.expectedTexts.length);
for (const [index, text] of params.expectedTexts.entries()) {
expect(params.deps.sendMessageTelegram).toHaveBeenNthCalledWith(
index + 1,
"123",
text,
expect.objectContaining({ cfg: expect.any(Object) }),
);
}
}
describe("runCronIsolatedAgentTurn", () => {
@@ -274,19 +286,19 @@ describe("runCronIsolatedAgentTurn", () => {
storePath,
deps,
payloads: [{ text: "hello from cron" }],
expectedText: "hello from cron",
expectedTexts: ["hello from cron"],
});
});
});
it("delivers explicit targets with final-payload text", async () => {
it("delivers explicit targets with all successful payload text", async () => {
await withTelegramAnnounceFixture(async ({ home, storePath, deps }) => {
await assertExplicitTelegramTargetDelivery({
home,
storePath,
deps,
payloads: [{ text: "Working on it..." }, { text: "Final weather summary" }],
expectedText: "Final weather summary",
expectedTexts: ["Working on it...", "Final weather summary"],
});
});
});

View File

@@ -1,6 +1,7 @@
import { describe, expect, it } from "vitest";
import {
isHeartbeatOnlyResponse,
pickDeliverablePayloads,
pickLastDeliverablePayload,
pickLastNonEmptyTextFromPayloads,
pickSummaryFromPayloads,
@@ -86,6 +87,27 @@ describe("pickLastDeliverablePayload", () => {
});
});
describe("pickDeliverablePayloads", () => {
it("preserves all successful deliverable payloads", () => {
const payloads = [
{ text: "line 1" },
{ text: "temporary error", isError: true as const },
{ text: "line 2" },
];
expect(pickDeliverablePayloads(payloads)).toEqual([{ text: "line 1" }, { text: "line 2" }]);
});
it("returns only the last error payload when all payloads are errors", () => {
const payloads = [
{ text: "first error", isError: true as const },
{ text: "last error", isError: true as const },
];
expect(pickDeliverablePayloads(payloads)).toEqual([{ text: "last error", isError: true }]);
});
});
describe("isHeartbeatOnlyResponse", () => {
const ACK_MAX = 300;

View File

@@ -71,28 +71,43 @@ export function pickLastNonEmptyTextFromPayloads(
return undefined;
}
function isDeliverablePayload(payload: DeliveryPayload | null | undefined): boolean {
if (!payload) return false;
const hasInteractive = (payload.interactive?.blocks?.length ?? 0) > 0;
const hasChannelData = Object.keys(payload.channelData ?? {}).length > 0;
return (
hasOutboundReplyContent(payload, { trimText: true }) || hasInteractive || hasChannelData
);
}
export function pickLastDeliverablePayload(payloads: DeliveryPayload[]) {
const isDeliverable = (p: DeliveryPayload) => {
const hasInteractive = (p?.interactive?.blocks?.length ?? 0) > 0;
const hasChannelData = Object.keys(p?.channelData ?? {}).length > 0;
return hasOutboundReplyContent(p, { trimText: true }) || hasInteractive || hasChannelData;
};
for (let i = payloads.length - 1; i >= 0; i--) {
if (payloads[i]?.isError) {
continue;
}
if (isDeliverable(payloads[i])) {
if (isDeliverablePayload(payloads[i])) {
return payloads[i];
}
}
for (let i = payloads.length - 1; i >= 0; i--) {
if (isDeliverable(payloads[i])) {
if (isDeliverablePayload(payloads[i])) {
return payloads[i];
}
}
return undefined;
}
export function pickDeliverablePayloads(payloads: DeliveryPayload[]): DeliveryPayload[] {
const successfulDeliverablePayloads = payloads.filter(
(payload) => payload != null && payload.isError !== true && isDeliverablePayload(payload),
);
if (successfulDeliverablePayloads.length > 0) {
return successfulDeliverablePayloads;
}
const lastDeliverablePayload = pickLastDeliverablePayload(payloads);
return lastDeliverablePayload ? [lastDeliverablePayload] : [];
}
/**
* Check if delivery should be skipped because the agent signaled no user-visible update.
* Returns true when any payload is a heartbeat ack token and no payload contains media.
@@ -115,9 +130,10 @@ export function resolveCronPayloadOutcome(params: {
const outputText = pickLastNonEmptyTextFromPayloads(params.payloads);
const synthesizedText = outputText?.trim() || summary?.trim() || undefined;
const deliveryPayload = pickLastDeliverablePayload(params.payloads);
const deliveryPayloads =
deliveryPayload !== undefined
? [deliveryPayload]
const selectedDeliveryPayloads = pickDeliverablePayloads(params.payloads);
const resolvedDeliveryPayloads =
selectedDeliveryPayloads.length > 0
? selectedDeliveryPayloads
: synthesizedText
? [{ text: synthesizedText }]
: [];
@@ -146,7 +162,7 @@ export function resolveCronPayloadOutcome(params: {
outputText,
synthesizedText,
deliveryPayload,
deliveryPayloads,
deliveryPayloads: resolvedDeliveryPayloads,
deliveryPayloadHasStructuredContent,
hasFatalErrorPayload,
embeddedRunError: hasFatalErrorPayload