From 63ed9adfe90a1619b60e420f0b86b2bb8eda3a1a Mon Sep 17 00:00:00 2001 From: "clawsweeper[bot]" <274271284+clawsweeper[bot]@users.noreply.github.com> Date: Tue, 2 Jun 2026 04:16:58 +0000 Subject: [PATCH] fix(auto-reply): guard missing dispatcher getFailedCounts without weakening the SDK type (#89318) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: - Adds defensive failed-count reads in auto-reply/ACP accounting and Feishu fallback paths, plus a focused regression test, while keeping `ReplyDispatcher.getFailedCounts` required. - PR surface: Source +24, Tests +35. Total +59 across 5 files. - Reproducibility: yes. from source inspection. Current main calls `dispatcher.getFailedCounts().final` and si ... issing that method follows a clear TypeError path; the source PR also supplied terminal before/after proof. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(auto-reply): guard missing dispatcher getFailedCounts without wea… Validation: - ClawSweeper review passed for head 0bdfb4adebc64c0bf4700516e6d3e118ef6519bd. - Required merge gates passed before the squash merge. Prepared head SHA: 0bdfb4adebc64c0bf4700516e6d3e118ef6519bd Review: https://github.com/openclaw/openclaw/pull/89318#issuecomment-4598624344 Co-authored-by: Alix-007 Co-authored-by: Claude Opus 4.8 Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com> --- extensions/feishu/src/bot.ts | 4 +-- src/auto-reply/reply/dispatch-acp-delivery.ts | 3 +- ...h-from-config.final-outcome-counts.test.ts | 35 +++++++++++++++++++ src/auto-reply/reply/dispatch-from-config.ts | 13 ++++--- .../reply/reply-dispatcher.types.ts | 18 ++++++++++ 5 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 src/auto-reply/reply/dispatch-from-config.final-outcome-counts.test.ts diff --git a/extensions/feishu/src/bot.ts b/extensions/feishu/src/bot.ts index d47c6d191b02..2ce55d6c4050 100644 --- a/extensions/feishu/src/bot.ts +++ b/extensions/feishu/src/bot.ts @@ -1575,7 +1575,7 @@ export async function handleFeishuMessage(params: { turnResult.dispatched && shouldSendNoVisibleReplyFallback({ ...turnResult.dispatchResult, - failedCounts: dispatcher.getFailedCounts(), + failedCounts: dispatcher.getFailedCounts?.() ?? { tool: 0, block: 0, final: 0 }, }) ) { await ensureNoVisibleReplyFallback("broadcast-dispatch-complete-no-visible-reply"); @@ -1771,7 +1771,7 @@ export async function handleFeishuMessage(params: { if ( shouldSendNoVisibleReplyFallback({ ...dispatchResult, - failedCounts: dispatcher.getFailedCounts(), + failedCounts: dispatcher.getFailedCounts?.() ?? { tool: 0, block: 0, final: 0 }, }) ) { await ensureNoVisibleReplyFallback("dispatch-complete-no-visible-reply"); diff --git a/src/auto-reply/reply/dispatch-acp-delivery.ts b/src/auto-reply/reply/dispatch-acp-delivery.ts index 274ebd67696c..04d20375045b 100644 --- a/src/auto-reply/reply/dispatch-acp-delivery.ts +++ b/src/auto-reply/reply/dispatch-acp-delivery.ts @@ -16,6 +16,7 @@ import type { FinalizedMsgContext } from "../templating.js"; import type { ReplyPayload } from "../types.js"; import { waitForReplyDispatcherIdle } from "./reply-dispatcher.js"; import type { ReplyDispatchKind, ReplyDispatcher } from "./reply-dispatcher.types.js"; +import { readDispatcherFailedCounts } from "./reply-dispatcher.types.js"; import { resolveRoutedDeliveryThreadId } from "./routed-delivery-thread.js"; const routeReplyRuntimeLoader = createLazyImportLoader(() => import("./route-reply.runtime.js")); @@ -257,7 +258,7 @@ export function createAcpDispatchDeliveryCoordinator(params: { state.settledDirectVisibleText = true; hasPendingDirectBlockReplyDelivery = false; await params.dispatcher.waitForIdle(); - const failedCounts = params.dispatcher.getFailedCounts(); + const failedCounts = readDispatcherFailedCounts(params.dispatcher); const failedVisibleCount = failedCounts.block + failedCounts.final; if (failedVisibleCount > 0) { state.failedVisibleTextDelivery = true; diff --git a/src/auto-reply/reply/dispatch-from-config.final-outcome-counts.test.ts b/src/auto-reply/reply/dispatch-from-config.final-outcome-counts.test.ts new file mode 100644 index 000000000000..fccf77ea6855 --- /dev/null +++ b/src/auto-reply/reply/dispatch-from-config.final-outcome-counts.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from "vitest"; +import { getDispatcherFinalOutcomeCounts } from "./dispatch-from-config.js"; +import type { ReplyDispatcher } from "./reply-dispatcher.types.js"; + +describe("getDispatcherFinalOutcomeCounts (#89116)", () => { + it("returns failed: 0 when the dispatcher does not implement getFailedCounts", () => { + // Some ReplyDispatcher variants omit the optional count methods entirely; the + // previous code called dispatcher.getFailedCounts() unguarded and threw + // "TypeError: dispatcher.getFailedCounts is not a function". + const dispatcher = { + getCancelledCounts: () => ({ tool: 0, block: 0, final: 2 }), + // getFailedCounts intentionally absent + } as unknown as ReplyDispatcher; + + expect(() => getDispatcherFinalOutcomeCounts(dispatcher)).not.toThrow(); + expect(getDispatcherFinalOutcomeCounts(dispatcher)).toEqual({ cancelled: 2, failed: 0 }); + }); + + it("returns cancelled: 0 when getCancelledCounts is absent (existing behavior preserved)", () => { + const dispatcher = { + getFailedCounts: () => ({ tool: 0, block: 1, final: 3 }), + } as unknown as ReplyDispatcher; + + expect(getDispatcherFinalOutcomeCounts(dispatcher)).toEqual({ cancelled: 0, failed: 3 }); + }); + + it("uses the real final counts when both methods are present", () => { + const dispatcher = { + getCancelledCounts: () => ({ tool: 0, block: 0, final: 1 }), + getFailedCounts: () => ({ tool: 0, block: 0, final: 5 }), + } as unknown as ReplyDispatcher; + + expect(getDispatcherFinalOutcomeCounts(dispatcher)).toEqual({ cancelled: 1, failed: 5 }); + }); +}); diff --git a/src/auto-reply/reply/dispatch-from-config.ts b/src/auto-reply/reply/dispatch-from-config.ts index b9714a7b9eae..6b5f99771344 100644 --- a/src/auto-reply/reply/dispatch-from-config.ts +++ b/src/auto-reply/reply/dispatch-from-config.ts @@ -132,7 +132,12 @@ import { withFullRuntimeReplyConfig } from "./get-reply-fast-path.js"; import { claimInboundDedupe, commitInboundDedupe, releaseInboundDedupe } from "./inbound-dedupe.js"; import { resolveOriginMessageProvider } from "./origin-routing.js"; import { waitForReplyDispatcherIdle } from "./reply-dispatcher.js"; -import type { ReplyDispatchKind, ReplyDispatcher } from "./reply-dispatcher.types.js"; +import type { + DispatcherOutcomeCountsView, + ReplyDispatchKind, + ReplyDispatcher, +} from "./reply-dispatcher.types.js"; +import { readDispatcherFailedCounts } from "./reply-dispatcher.types.js"; import { replyRunRegistry, type ReplyOperation } from "./reply-run-registry.js"; import { isReplyProfilerEnabled } from "./reply-timing-tracker.js"; import { admitReplyTurn, resolveReplyTurnKind } from "./reply-turn-admission.js"; @@ -774,13 +779,13 @@ async function mirrorInternalSourceReplyToTranscript(params: { } } -function getDispatcherFinalOutcomeCounts(dispatcher: ReplyDispatcher): { +export function getDispatcherFinalOutcomeCounts(dispatcher: DispatcherOutcomeCountsView): { cancelled: number; failed: number; } { return { cancelled: dispatcher.getCancelledCounts?.().final ?? 0, - failed: dispatcher.getFailedCounts().final, + failed: readDispatcherFailedCounts(dispatcher).final, }; } @@ -903,7 +908,7 @@ function createAbortAwareDispatcher(params: { sendFinalReply: sendIfActive(params.dispatcher.sendFinalReply), waitForIdle: () => params.dispatcher.waitForIdle(), getQueuedCounts: () => params.dispatcher.getQueuedCounts(), - getFailedCounts: () => params.dispatcher.getFailedCounts(), + getFailedCounts: () => readDispatcherFailedCounts(params.dispatcher), markComplete: () => { if (!params.isAborted()) { params.dispatcher.markComplete(); diff --git a/src/auto-reply/reply/reply-dispatcher.types.ts b/src/auto-reply/reply/reply-dispatcher.types.ts index 1dd77582d263..c7d4e1064664 100644 --- a/src/auto-reply/reply/reply-dispatcher.types.ts +++ b/src/auto-reply/reply/reply-dispatcher.types.ts @@ -18,3 +18,21 @@ export type ReplyDispatcher = { getFailedCounts: () => Record; markComplete: () => void; }; + +/** + * Internal view for defensive outcome-count accounting. Some non-conforming + * runtime dispatcher variants (for example plugin-provided dispatchers) may omit + * these readers even though the public ReplyDispatcher contract requires + * getFailedCounts. Read the counters through this view so the guards stay + * type-correct without weakening the SDK-visible ReplyDispatcher type. + */ +export type DispatcherOutcomeCountsView = { + getCancelledCounts?: () => Record; + getFailedCounts?: () => Record; +}; + +export function readDispatcherFailedCounts( + dispatcher: DispatcherOutcomeCountsView, +): Record { + return dispatcher.getFailedCounts?.() ?? { tool: 0, block: 0, final: 0 }; +}