fix(auto-reply): count routed block deliveries as visible before falling back

A routed media-only block reaches the user without touching blockCount or
dispatcher queue counts; the no-visible-reply fallback then sent misleading
failure text after valid media. Expose the settled routed-block delivery fact
from the dispatch route and require it clear before synthesizing the fallback.
Also reset the routeReply mock in describe2BeforeEach0 like sibling suite
setups so queued once-values cannot leak between tests.
This commit is contained in:
Ayaan Zaidi
2026-07-27 20:38:31 +09:00
parent d628bdd8b4
commit 21ba3bfcca
4 changed files with 56 additions and 4 deletions
@@ -250,6 +250,9 @@ export async function chooseDispatchRoute(state: PrepareDispatchOperationReadySt
deliveredBlockContentKeys.add(createBlockReplyContentKey(payload));
}
};
// Routed blocks bypass dispatcher queue counts entirely; this is the only
// settled-delivery fact for them (media-only blocks never touch blockCount).
const hasDeliveredRoutedBlockReply = (): boolean => deliveredBlockContentKeys.size > 0;
const wasReplyDeliveredAsBlock = async (
payload: ReplyPayload,
abortSignal?: AbortSignal,
@@ -610,6 +613,7 @@ export async function chooseDispatchRoute(state: PrepareDispatchOperationReadySt
sendTrackedBlockReply,
recordRoutedBlockReplyDelivery,
wasReplyDeliveredAsBlock,
hasDeliveredRoutedBlockReply,
sendFinalPayload,
},
{
@@ -40,6 +40,7 @@ export async function finalizeDispatchAndAudit(state: ExecuteDispatchReadyState)
flushPendingCommentaryProgress,
getDispatchAbortSignal,
getObservedReplyDelivery,
hasDeliveredRoutedBlockReply,
isRoutedReplyDelivered,
markIdle,
markInboundDedupeReplayUnsafe,
@@ -303,6 +304,7 @@ export async function finalizeDispatchAndAudit(state: ExecuteDispatchReadyState)
!getObservedReplyDelivery() &&
!emptyFinalAllowedAsSilent &&
!sawDedupedAgainstBlock &&
!hasDeliveredRoutedBlockReply() &&
state.blockCount === 0 &&
counts.tool === 0 &&
counts.block === 0 &&
@@ -575,8 +575,7 @@ describe("sendPolicy deny — suppress delivery, not processing (#53328)", () =>
it("delivers routed fallback when routing drops an empty final without sending", async () => {
setNoAbort();
mocks.routeReply.mockResolvedValueOnce({ ok: true });
mocks.routeReply.mockResolvedValueOnce({ ok: true, messageId: "fallback-1" });
mocks.routeReply.mockResolvedValue({ ok: true, messageId: "fallback-1" });
const dispatcher = createDispatcher();
const replyResolver = vi.fn(async () => ({ text: "" }));
const ctx = buildTestCtx({
@@ -617,8 +616,7 @@ describe("sendPolicy deny — suppress delivery, not processing (#53328)", () =>
it("keeps eligibility when an empty routed final precedes a suppressed fallback", async () => {
setNoAbort();
mocks.routeReply.mockResolvedValueOnce({ ok: true });
mocks.routeReply.mockResolvedValueOnce({ ok: true, suppressed: true });
mocks.routeReply.mockResolvedValue({ ok: true, suppressed: true });
const dispatcher = createDispatcher();
const replyResolver = vi.fn(async () => ({ text: "" }));
const ctx = buildTestCtx({
@@ -685,6 +683,50 @@ describe("sendPolicy deny — suppress delivery, not processing (#53328)", () =>
expect(result.noVisibleReplyFallbackEligible).toBe(true);
});
it("does not deliver no-visible fallback after a routed media-only block", async () => {
setNoAbort();
mocks.routeReply.mockResolvedValue({ ok: true, messageId: "media-block-1" });
const dispatcher = createDispatcher();
const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => {
await opts?.onBlockReply?.({ mediaUrl: "https://example.com/seatmap.png" });
return undefined;
});
const ctx = buildTestCtx({
ChatType: "group",
Surface: "slack",
Provider: "slack",
OriginatingChannel: "telegram",
OriginatingTo: "telegram:999",
SessionKey: "agent:main:slack:group:oc_group",
});
const result = await dispatchReplyFromConfig({
ctx,
cfg: {
agents: {
defaults: {
silentReply: {
group: "disallow",
},
},
},
} as OpenClawConfig,
dispatcher,
replyResolver,
});
// A routed media-only block increments neither blockCount (text-only counter)
// nor dispatcher counts; the delivered-routed-block fact must suppress the
// fallback so users never see failure text after valid media.
const fallbackCall = mocks.routeReply.mock.calls.find(
(call) =>
(call[0] as { payload?: { text?: string } }).payload?.text ===
NO_VISIBLE_REPLY_FALLBACK_TEXT,
);
expect(fallbackCall).toBeUndefined();
expect(result.noVisibleReplyFallbackDelivered).toBeUndefined();
});
it("does not deliver no-visible fallback after streamed blocks invisible to dispatcher counts", async () => {
setNoAbort();
const dispatcher = createDispatcher();
@@ -594,6 +594,10 @@ export const describe1BeforeEach0 = () => {
export const describe2BeforeEach0 = () => {
resetInboundDedupe();
// Same routeReply reset as the sibling suite setups: queued once-values and
// persistent overrides must not leak between tests.
mocks.routeReply.mockReset();
mocks.routeReply.mockResolvedValue({ ok: true, messageId: "mock" });
sessionStoreMocks.currentEntry = undefined;
sessionBindingMocks.resolveByConversation.mockReset();
sessionBindingMocks.resolveByConversation.mockReturnValue(null);