Skip to content

Commit e046c28

Browse files
indulgebackindulgeback
committed
fix(outbound): ignore empty delivery receipts
1 parent 8071b06 commit e046c28

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,5 +1,5 @@
11
import "./isolated-agent.mocks.js";
2-
import { afterEach, beforeEach, describe, expect, it } from "vitest";
2+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
33
import { runSubagentAnnounceFlow } from "../agents/subagent-announce.js";
44
import type { ChannelOutboundAdapter, ChannelOutboundContext } from "../channels/plugins/types.js";
55
import type { CliDeps } from "../cli/deps.js";
@@ -454,6 +454,55 @@ describe("runCronIsolatedAgentTurn telegram forum-topic direct delivery", () =>
454454
});
455455
});
456456

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

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

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

3440+
it("does not reuse a previous payload message id for a suppressed text send", async () => {
3441+
hookMocks.runner.hasHooks.mockReturnValue(true);
3442+
const sendText = vi
3443+
.fn()
3444+
.mockResolvedValueOnce({ channel: "matrix", messageId: "mx-1" })
3445+
.mockResolvedValueOnce({ channel: "matrix", messageId: "" });
3446+
setActivePluginRegistry(
3447+
createTestRegistry([
3448+
{
3449+
pluginId: "matrix",
3450+
source: "test",
3451+
plugin: createOutboundTestPlugin({
3452+
id: "matrix",
3453+
outbound: { deliveryMode: "direct", sendText },
3454+
}),
3455+
},
3456+
]),
3457+
);
3458+
3459+
const results = await deliverOutboundPayloads({
3460+
cfg: {},
3461+
channel: "matrix",
3462+
to: "!room:1",
3463+
payloads: [{ text: "first" }, { text: "second" }],
3464+
});
3465+
3466+
expect(results).toStrictEqual([{ channel: "matrix", messageId: "mx-1" }]);
3467+
expect(hookMocks.runner.runMessageSent).toHaveBeenCalledTimes(2);
3468+
expect(hookMocks.runner.runMessageSent).toHaveBeenNthCalledWith(
3469+
1,
3470+
expect.objectContaining({
3471+
content: "first",
3472+
success: true,
3473+
messageId: "mx-1",
3474+
}),
3475+
expect.objectContaining({ channelId: "matrix" }),
3476+
);
3477+
expect(hookMocks.runner.runMessageSent).toHaveBeenNthCalledWith(
3478+
2,
3479+
expect.objectContaining({
3480+
content: "second",
3481+
success: false,
3482+
}),
3483+
expect.objectContaining({ channelId: "matrix" }),
3484+
);
3485+
expect(hookMocks.runner.runMessageSent.mock.calls[1]?.[0]).not.toHaveProperty("messageId");
3486+
});
3487+
34403488
it("emits message_sent success for sendPayload deliveries", async () => {
34413489
hookMocks.runner.hasHooks.mockReturnValue(true);
34423490
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
@@ -864,6 +864,23 @@ function hasDeliveryResultIdentity(result: OutboundDeliveryResult): boolean {
864864
);
865865
}
866866

867+
function pushIdentifiedDeliveryResult(
868+
results: OutboundDeliveryResult[],
869+
delivery: OutboundDeliveryResult,
870+
): boolean {
871+
if (!hasDeliveryResultIdentity(delivery)) {
872+
return false;
873+
}
874+
results.push(delivery);
875+
return true;
876+
}
877+
878+
function filterIdentifiedDeliveryResults(
879+
results: readonly OutboundDeliveryResult[],
880+
): OutboundDeliveryResult[] {
881+
return results.filter((result) => hasDeliveryResultIdentity(result));
882+
}
883+
867884
function normalizeDeliveryPin(payload: ReplyPayload): ReplyPayloadDeliveryPin | undefined {
868885
const pin = payload.delivery?.pin;
869886
if (pin === true) {
@@ -1507,7 +1524,7 @@ async function deliverOutboundPayloadsCore(
15071524
continue;
15081525
}
15091526
throwIfAborted(abortSignal);
1510-
results.push(await sendHandler.sendText(unit.text, unit.overrides));
1527+
pushIdentifiedDeliveryResult(results, await sendHandler.sendText(unit.text, unit.overrides));
15111528
}
15121529
};
15131530
const normalizedPayloads = normalizePayloadsForChannelDelivery(outboundPayloadPlan, handler);
@@ -1761,10 +1778,12 @@ async function deliverOutboundPayloadsCore(
17611778
const beforeCount = results.length;
17621779
if (deliveryHandler.sendFormattedText) {
17631780
results.push(
1764-
...(await deliveryHandler.sendFormattedText(
1765-
payloadSummary.text,
1766-
applySendReplyToConsumption(sendOverrides),
1767-
)),
1781+
...filterIdentifiedDeliveryResults(
1782+
await deliveryHandler.sendFormattedText(
1783+
payloadSummary.text,
1784+
applySendReplyToConsumption(sendOverrides),
1785+
),
1786+
),
17681787
);
17691788
} else {
17701789
await sendTextChunks(deliveryHandler, payloadSummary.text, sendOverrides);
@@ -1785,7 +1804,7 @@ async function deliverOutboundPayloadsCore(
17851804
}),
17861805
);
17871806
}
1788-
const messageId = results.at(-1)?.messageId;
1807+
const messageId = deliveredResults.at(-1)?.messageId;
17891808
const pinMessageId = deliveredResults.find((entry) => entry.messageId)?.messageId;
17901809
await maybePinDeliveredMessage({
17911810
handler: deliveryHandler,
@@ -1802,7 +1821,7 @@ async function deliverOutboundPayloadsCore(
18021821
});
18031822
completeDeliveryDiagnostics(deliveredResults.length);
18041823
emitMessageSent({
1805-
success: results.length > beforeCount,
1824+
success: deliveredResults.length > 0,
18061825
content: payloadSummary.hookContent ?? payloadSummary.text,
18071826
messageId,
18081827
});
@@ -1842,7 +1861,7 @@ async function deliverOutboundPayloadsCore(
18421861
}),
18431862
);
18441863
}
1845-
const messageId = results.at(-1)?.messageId;
1864+
const messageId = deliveredResults.at(-1)?.messageId;
18461865
const pinMessageId = deliveredResults.find((entry) => entry.messageId)?.messageId;
18471866
await maybePinDeliveredMessage({
18481867
handler: deliveryHandler,
@@ -1859,7 +1878,7 @@ async function deliverOutboundPayloadsCore(
18591878
});
18601879
completeDeliveryDiagnostics(deliveredResults.length);
18611880
emitMessageSent({
1862-
success: results.length > beforeCount,
1881+
success: deliveredResults.length > 0,
18631882
content: payloadSummary.hookContent ?? payloadSummary.text,
18641883
messageId,
18651884
});
@@ -1887,9 +1906,10 @@ async function deliverOutboundPayloadsCore(
18871906
unit.overrides,
18881907
)
18891908
: await deliveryHandler.sendMedia(unit.caption ?? "", unit.mediaUrl, unit.overrides);
1890-
results.push(delivery);
1891-
firstMessageId ??= delivery.messageId;
1892-
lastMessageId = delivery.messageId;
1909+
if (pushIdentifiedDeliveryResult(results, delivery)) {
1910+
firstMessageId ??= delivery.messageId;
1911+
lastMessageId = delivery.messageId;
1912+
}
18931913
}
18941914
await maybePinDeliveredMessage({
18951915
handler: deliveryHandler,
@@ -1922,7 +1942,7 @@ async function deliverOutboundPayloadsCore(
19221942
}
19231943
completeDeliveryDiagnostics(results.length - beforeCount);
19241944
emitMessageSent({
1925-
success: true,
1945+
success: results.length > beforeCount,
19261946
content: payloadSummary.hookContent ?? payloadSummary.text,
19271947
messageId: lastMessageId,
19281948
});

0 commit comments

Comments
 (0)