Skip to content

Commit 9a735be

Browse files
indulgebackindulgeback
authored
fix(outbound): ignore empty delivery receipts (#79811)
1 parent 81e5320 commit 9a735be

3 files changed

Lines changed: 131 additions & 14 deletions

File tree

‎src/cron/isolated-agent.direct-delivery-core-channels.test.ts‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Direct delivery tests cover isolated agent delivery through core channel targets.
22
import "./isolated-agent.mocks.js";
3-
import { afterEach, beforeEach, describe, expect, it } from "vitest";
3+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
44
import { runSubagentAnnounceFlow } from "../agents/subagent-announce.js";
55
import type { ChannelOutboundAdapter, ChannelOutboundContext } from "../channels/plugins/types.js";
66
import type { CliDeps } from "../cli/deps.js";
@@ -455,6 +455,55 @@ describe("runCronIsolatedAgentTurn telegram forum-topic direct delivery", () =>
455455
});
456456
});
457457

458+
it("does not report delivered when telegram announce produces no platform result", async () => {
459+
await withTempCronHome(async (home) => {
460+
const storePath = await writeSessionStore(home, { lastProvider: "webchat", lastTo: "" });
461+
const sendText = vi.fn(async () => ({ channel: "telegram", messageId: "" }));
462+
setActivePluginRegistry(
463+
createTestRegistry([
464+
{
465+
pluginId: "telegram",
466+
plugin: createOutboundTestPlugin({
467+
id: "telegram",
468+
outbound: {
469+
deliveryMode: "direct",
470+
preferFinalAssistantVisibleText: true,
471+
sendText,
472+
resolveTarget: ({ to }) =>
473+
to?.trim()
474+
? { ok: true, to: to.trim() }
475+
: { ok: false, error: new Error("target is required") },
476+
},
477+
messaging: {
478+
parseExplicitTarget: ({ raw }) => ({ to: raw.trim() }),
479+
},
480+
}),
481+
source: "test",
482+
},
483+
]),
484+
);
485+
const deps = createCliDeps();
486+
mockAgentPayloads([{ text: "cron message with no platform receipt" }]);
487+
488+
const res = await runTelegramAnnounceTurn({
489+
home,
490+
storePath,
491+
deps,
492+
delivery: { mode: "announce", channel: "telegram", to: "123" },
493+
});
494+
495+
expect(res.status).toBe("ok");
496+
expect(res.delivered).toBe(false);
497+
expect(res.deliveryAttempted).toBe(true);
498+
expect(res.delivery).toMatchObject({
499+
fallbackUsed: true,
500+
delivered: false,
501+
});
502+
expect(sendText).toHaveBeenCalledTimes(1);
503+
expect(deps.sendMessageTelegram).not.toHaveBeenCalled();
504+
});
505+
});
506+
458507
it("delivers only the final assistant-visible text to forum-topic telegram targets", async () => {
459508
await expectTelegramAnnounceDelivery({
460509
to: "123:topic:42",

‎src/infra/outbound/deliver.test.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3598,6 +3598,54 @@ describe("deliverOutboundPayloads", () => {
35983598
expect(mocks.appendAssistantMessageToSessionTranscript).not.toHaveBeenCalled();
35993599
});
36003600

3601+
it("does not reuse a previous payload message id for a suppressed text send", async () => {
3602+
hookMocks.runner.hasHooks.mockReturnValue(true);
3603+
const sendText = vi
3604+
.fn()
3605+
.mockResolvedValueOnce({ channel: "matrix", messageId: "mx-1" })
3606+
.mockResolvedValueOnce({ channel: "matrix", messageId: "" });
3607+
setActivePluginRegistry(
3608+
createTestRegistry([
3609+
{
3610+
pluginId: "matrix",
3611+
source: "test",
3612+
plugin: createOutboundTestPlugin({
3613+
id: "matrix",
3614+
outbound: { deliveryMode: "direct", sendText },
3615+
}),
3616+
},
3617+
]),
3618+
);
3619+
3620+
const results = await deliverOutboundPayloads({
3621+
cfg: {},
3622+
channel: "matrix",
3623+
to: "!room:1",
3624+
payloads: [{ text: "first" }, { text: "second" }],
3625+
});
3626+
3627+
expect(results).toStrictEqual([{ channel: "matrix", messageId: "mx-1" }]);
3628+
expect(hookMocks.runner.runMessageSent).toHaveBeenCalledTimes(2);
3629+
expect(hookMocks.runner.runMessageSent).toHaveBeenNthCalledWith(
3630+
1,
3631+
expect.objectContaining({
3632+
content: "first",
3633+
success: true,
3634+
messageId: "mx-1",
3635+
}),
3636+
expect.objectContaining({ channelId: "matrix" }),
3637+
);
3638+
expect(hookMocks.runner.runMessageSent).toHaveBeenNthCalledWith(
3639+
2,
3640+
expect.objectContaining({
3641+
content: "second",
3642+
success: false,
3643+
}),
3644+
expect.objectContaining({ channelId: "matrix" }),
3645+
);
3646+
expect(hookMocks.runner.runMessageSent.mock.calls[1]?.[0]).not.toHaveProperty("messageId");
3647+
});
3648+
36013649
it("emits message_sent success for sendPayload deliveries", async () => {
36023650
hookMocks.runner.hasHooks.mockReturnValue(true);
36033651
const sendPayload = vi.fn().mockResolvedValue({ channel: "matrix", messageId: "mx-1" });

‎src/infra/outbound/deliver.ts‎

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -868,6 +868,23 @@ function hasDeliveryResultIdentity(result: OutboundDeliveryResult): boolean {
868868
);
869869
}
870870

871+
function pushIdentifiedDeliveryResult(
872+
results: OutboundDeliveryResult[],
873+
delivery: OutboundDeliveryResult,
874+
): boolean {
875+
if (!hasDeliveryResultIdentity(delivery)) {
876+
return false;
877+
}
878+
results.push(delivery);
879+
return true;
880+
}
881+
882+
function filterIdentifiedDeliveryResults(
883+
results: readonly OutboundDeliveryResult[],
884+
): OutboundDeliveryResult[] {
885+
return results.filter((result) => hasDeliveryResultIdentity(result));
886+
}
887+
871888
function normalizeDeliveryPin(payload: ReplyPayload): ReplyPayloadDeliveryPin | undefined {
872889
const pin = payload.delivery?.pin;
873890
if (pin === true) {
@@ -1529,7 +1546,7 @@ async function deliverOutboundPayloadsCore(
15291546
continue;
15301547
}
15311548
throwIfAborted(abortSignal);
1532-
results.push(await sendHandler.sendText(unit.text, unit.overrides));
1549+
pushIdentifiedDeliveryResult(results, await sendHandler.sendText(unit.text, unit.overrides));
15331550
}
15341551
};
15351552
const normalizedPayloads = normalizePayloadsForChannelDelivery(outboundPayloadPlan, handler);
@@ -1783,10 +1800,12 @@ async function deliverOutboundPayloadsCore(
17831800
const beforeCount = results.length;
17841801
if (deliveryHandler.sendFormattedText) {
17851802
results.push(
1786-
...(await deliveryHandler.sendFormattedText(
1787-
payloadSummary.text,
1788-
applySendReplyToConsumption(sendOverrides),
1789-
)),
1803+
...filterIdentifiedDeliveryResults(
1804+
await deliveryHandler.sendFormattedText(
1805+
payloadSummary.text,
1806+
applySendReplyToConsumption(sendOverrides),
1807+
),
1808+
),
17901809
);
17911810
} else {
17921811
await sendTextChunks(deliveryHandler, payloadSummary.text, sendOverrides);
@@ -1807,7 +1826,7 @@ async function deliverOutboundPayloadsCore(
18071826
}),
18081827
);
18091828
}
1810-
const messageId = results.at(-1)?.messageId;
1829+
const messageId = deliveredResults.at(-1)?.messageId;
18111830
const pinMessageId = deliveredResults.find((entry) => entry.messageId)?.messageId;
18121831
await maybePinDeliveredMessage({
18131832
handler: deliveryHandler,
@@ -1824,7 +1843,7 @@ async function deliverOutboundPayloadsCore(
18241843
});
18251844
completeDeliveryDiagnostics(deliveredResults.length);
18261845
emitMessageSent({
1827-
success: results.length > beforeCount,
1846+
success: deliveredResults.length > 0,
18281847
content: payloadSummary.hookContent ?? payloadSummary.text,
18291848
messageId,
18301849
});
@@ -1864,7 +1883,7 @@ async function deliverOutboundPayloadsCore(
18641883
}),
18651884
);
18661885
}
1867-
const messageId = results.at(-1)?.messageId;
1886+
const messageId = deliveredResults.at(-1)?.messageId;
18681887
const pinMessageId = deliveredResults.find((entry) => entry.messageId)?.messageId;
18691888
await maybePinDeliveredMessage({
18701889
handler: deliveryHandler,
@@ -1881,7 +1900,7 @@ async function deliverOutboundPayloadsCore(
18811900
});
18821901
completeDeliveryDiagnostics(deliveredResults.length);
18831902
emitMessageSent({
1884-
success: results.length > beforeCount,
1903+
success: deliveredResults.length > 0,
18851904
content: payloadSummary.hookContent ?? payloadSummary.text,
18861905
messageId,
18871906
});
@@ -1909,9 +1928,10 @@ async function deliverOutboundPayloadsCore(
19091928
unit.overrides,
19101929
)
19111930
: await deliveryHandler.sendMedia(unit.caption ?? "", unit.mediaUrl, unit.overrides);
1912-
results.push(delivery);
1913-
firstMessageId ??= delivery.messageId;
1914-
lastMessageId = delivery.messageId;
1931+
if (pushIdentifiedDeliveryResult(results, delivery)) {
1932+
firstMessageId ??= delivery.messageId;
1933+
lastMessageId = delivery.messageId;
1934+
}
19151935
}
19161936
await maybePinDeliveredMessage({
19171937
handler: deliveryHandler,
@@ -1944,7 +1964,7 @@ async function deliverOutboundPayloadsCore(
19441964
}
19451965
completeDeliveryDiagnostics(results.length - beforeCount);
19461966
emitMessageSent({
1947-
success: true,
1967+
success: results.length > beforeCount,
19481968
content: payloadSummary.hookContent ?? payloadSummary.text,
19491969
messageId: lastMessageId,
19501970
});

0 commit comments

Comments
 (0)