mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-07 01:21:36 +00:00
fix: harden typing lifecycle and cross-channel suppression
This commit is contained in:
@@ -286,6 +286,45 @@ describe("dispatchReplyFromConfig", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("forces suppressTyping when routing to a different originating channel", async () => {
|
||||
setNoAbort();
|
||||
const cfg = emptyConfig;
|
||||
const dispatcher = createDispatcher();
|
||||
const ctx = buildTestCtx({
|
||||
Provider: "slack",
|
||||
OriginatingChannel: "telegram",
|
||||
OriginatingTo: "telegram:999",
|
||||
});
|
||||
|
||||
const replyResolver = async (_ctx: MsgContext, opts?: GetReplyOptions) => {
|
||||
expect(opts?.suppressTyping).toBe(true);
|
||||
expect(opts?.typingPolicy).toBe("system_event");
|
||||
return { text: "hi" } satisfies ReplyPayload;
|
||||
};
|
||||
|
||||
await dispatchReplyFromConfig({ ctx, cfg, dispatcher, replyResolver });
|
||||
});
|
||||
|
||||
it("forces suppressTyping for internal webchat turns", async () => {
|
||||
setNoAbort();
|
||||
const cfg = emptyConfig;
|
||||
const dispatcher = createDispatcher();
|
||||
const ctx = buildTestCtx({
|
||||
Provider: "webchat",
|
||||
Surface: "webchat",
|
||||
OriginatingChannel: "webchat",
|
||||
OriginatingTo: "session:abc",
|
||||
});
|
||||
|
||||
const replyResolver = async (_ctx: MsgContext, opts?: GetReplyOptions) => {
|
||||
expect(opts?.suppressTyping).toBe(true);
|
||||
expect(opts?.typingPolicy).toBe("internal_webchat");
|
||||
return { text: "hi" } satisfies ReplyPayload;
|
||||
};
|
||||
|
||||
await dispatchReplyFromConfig({ ctx, cfg, dispatcher, replyResolver });
|
||||
});
|
||||
|
||||
it("routes media-only tool results when summaries are suppressed", async () => {
|
||||
setNoAbort();
|
||||
mocks.routeReply.mockClear();
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
import { getGlobalHookRunner } from "../../plugins/hook-runner-global.js";
|
||||
import { resolveSendPolicy } from "../../sessions/send-policy.js";
|
||||
import { maybeApplyTtsToPayload, normalizeTtsAutoMode, resolveTtsConfig } from "../../tts/tts.js";
|
||||
import { INTERNAL_MESSAGE_CHANNEL } from "../../utils/message-channel.js";
|
||||
import { getReplyFromConfig } from "../reply.js";
|
||||
import type { FinalizedMsgContext } from "../templating.js";
|
||||
import type { GetReplyOptions, ReplyPayload } from "../types.js";
|
||||
@@ -253,6 +254,8 @@ export async function dispatchReplyFromConfig(params: {
|
||||
const shouldRouteToOriginating = Boolean(
|
||||
isRoutableChannel(originatingChannel) && originatingTo && originatingChannel !== currentSurface,
|
||||
);
|
||||
const shouldSuppressTyping =
|
||||
shouldRouteToOriginating || originatingChannel === INTERNAL_MESSAGE_CHANNEL;
|
||||
const ttsChannel = shouldRouteToOriginating ? originatingChannel : currentSurface;
|
||||
|
||||
/**
|
||||
@@ -397,6 +400,14 @@ export async function dispatchReplyFromConfig(params: {
|
||||
ctx,
|
||||
{
|
||||
...params.replyOptions,
|
||||
typingPolicy:
|
||||
params.replyOptions?.typingPolicy ??
|
||||
(originatingChannel === INTERNAL_MESSAGE_CHANNEL
|
||||
? "internal_webchat"
|
||||
: shouldRouteToOriginating
|
||||
? "system_event"
|
||||
: undefined),
|
||||
suppressTyping: params.replyOptions?.suppressTyping === true || shouldSuppressTyping,
|
||||
onToolResult: (payload: ReplyPayload) => {
|
||||
const run = async () => {
|
||||
const ttsPayload = await maybeApplyTtsToPayload({
|
||||
|
||||
@@ -81,6 +81,7 @@ vi.mock("./typing-mode.js", () => ({
|
||||
|
||||
import { runReplyAgent } from "./agent-runner.js";
|
||||
import { routeReply } from "./route-reply.js";
|
||||
import { resolveTypingMode } from "./typing-mode.js";
|
||||
|
||||
function baseParams(
|
||||
overrides: Partial<Parameters<typeof runPreparedReply>[0]> = {},
|
||||
@@ -249,4 +250,48 @@ describe("runPreparedReply media-only handling", () => {
|
||||
|
||||
expect(vi.mocked(routeReply)).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("uses inbound origin channel for run messageProvider", async () => {
|
||||
await runPreparedReply(
|
||||
baseParams({
|
||||
ctx: {
|
||||
Body: "",
|
||||
RawBody: "",
|
||||
CommandBody: "",
|
||||
ThreadHistoryBody: "Earlier message in this thread",
|
||||
OriginatingChannel: "webchat",
|
||||
OriginatingTo: "session:abc",
|
||||
ChatType: "group",
|
||||
},
|
||||
sessionCtx: {
|
||||
Body: "",
|
||||
BodyStripped: "",
|
||||
ThreadHistoryBody: "Earlier message in this thread",
|
||||
MediaPath: "/tmp/input.png",
|
||||
Provider: "telegram",
|
||||
ChatType: "group",
|
||||
OriginatingChannel: "telegram",
|
||||
OriginatingTo: "telegram:123",
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const call = vi.mocked(runReplyAgent).mock.calls[0]?.[0];
|
||||
expect(call?.followupRun.run.messageProvider).toBe("webchat");
|
||||
});
|
||||
|
||||
it("passes suppressTyping through typing mode resolution", async () => {
|
||||
await runPreparedReply(
|
||||
baseParams({
|
||||
opts: {
|
||||
suppressTyping: true,
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const call = vi.mocked(resolveTypingMode).mock.calls[0]?.[0] as
|
||||
| { suppressTyping?: boolean }
|
||||
| undefined;
|
||||
expect(call?.suppressTyping).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -18,6 +18,7 @@ import {
|
||||
import { logVerbose } from "../../globals.js";
|
||||
import { clearCommandLane, getQueueSize } from "../../process/command-queue.js";
|
||||
import { normalizeMainKey } from "../../routing/session-key.js";
|
||||
import { INTERNAL_MESSAGE_CHANNEL } from "../../utils/message-channel.js";
|
||||
import { isReasoningTagProvider } from "../../utils/provider-utils.js";
|
||||
import { hasControlCommand } from "../command-detection.js";
|
||||
import { buildInboundMediaNote } from "../media-note.js";
|
||||
@@ -233,11 +234,21 @@ export async function runPreparedReply(
|
||||
const isGroupChat = sessionCtx.ChatType === "group";
|
||||
const wasMentioned = ctx.WasMentioned === true;
|
||||
const isHeartbeat = opts?.isHeartbeat === true;
|
||||
const typingPolicy =
|
||||
opts?.typingPolicy ??
|
||||
(isHeartbeat
|
||||
? "heartbeat"
|
||||
: ctx.OriginatingChannel === INTERNAL_MESSAGE_CHANNEL
|
||||
? "internal_webchat"
|
||||
: "auto");
|
||||
const suppressTyping = opts?.suppressTyping === true;
|
||||
const typingMode = resolveTypingMode({
|
||||
configured: sessionCfg?.typingMode ?? agentCfg?.typingMode,
|
||||
isGroupChat,
|
||||
wasMentioned,
|
||||
isHeartbeat,
|
||||
typingPolicy,
|
||||
suppressTyping,
|
||||
});
|
||||
const shouldInjectGroupIntro = Boolean(
|
||||
isGroupChat && (isFirstTurnInSession || sessionEntry?.groupActivationNeedsSystemIntro),
|
||||
@@ -462,8 +473,8 @@ export async function runPreparedReply(
|
||||
sessionId: sessionIdFinal,
|
||||
sessionKey,
|
||||
messageProvider: resolveOriginMessageProvider({
|
||||
originatingChannel: sessionCtx.OriginatingChannel,
|
||||
provider: sessionCtx.Provider,
|
||||
originatingChannel: ctx.OriginatingChannel ?? sessionCtx.OriginatingChannel,
|
||||
provider: ctx.Surface ?? ctx.Provider ?? sessionCtx.Provider,
|
||||
}),
|
||||
agentAccountId: sessionCtx.AccountId,
|
||||
groupId: resolveGroupSessionKey(sessionCtx)?.id ?? undefined,
|
||||
|
||||
@@ -257,6 +257,28 @@ describe("resolveTypingMode", () => {
|
||||
},
|
||||
expected: "never",
|
||||
},
|
||||
{
|
||||
name: "suppressTyping forces never",
|
||||
input: {
|
||||
configured: "instant" as const,
|
||||
isGroupChat: false,
|
||||
wasMentioned: false,
|
||||
isHeartbeat: false,
|
||||
suppressTyping: true,
|
||||
},
|
||||
expected: "never",
|
||||
},
|
||||
{
|
||||
name: "typingPolicy system_event forces never",
|
||||
input: {
|
||||
configured: "instant" as const,
|
||||
isGroupChat: false,
|
||||
wasMentioned: false,
|
||||
isHeartbeat: false,
|
||||
typingPolicy: "system_event" as const,
|
||||
},
|
||||
expected: "never",
|
||||
},
|
||||
] as const;
|
||||
|
||||
for (const testCase of cases) {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { TypingMode } from "../../config/types.js";
|
||||
import { isSilentReplyText, SILENT_REPLY_TOKEN } from "../tokens.js";
|
||||
import type { TypingPolicy } from "../types.js";
|
||||
import type { TypingController } from "./typing.js";
|
||||
|
||||
export type TypingModeContext = {
|
||||
@@ -7,6 +8,8 @@ export type TypingModeContext = {
|
||||
isGroupChat: boolean;
|
||||
wasMentioned: boolean;
|
||||
isHeartbeat: boolean;
|
||||
typingPolicy?: TypingPolicy;
|
||||
suppressTyping?: boolean;
|
||||
};
|
||||
|
||||
export const DEFAULT_GROUP_TYPING_MODE: TypingMode = "message";
|
||||
@@ -16,8 +19,16 @@ export function resolveTypingMode({
|
||||
isGroupChat,
|
||||
wasMentioned,
|
||||
isHeartbeat,
|
||||
typingPolicy,
|
||||
suppressTyping,
|
||||
}: TypingModeContext): TypingMode {
|
||||
if (isHeartbeat) {
|
||||
if (
|
||||
isHeartbeat ||
|
||||
typingPolicy === "heartbeat" ||
|
||||
typingPolicy === "system_event" ||
|
||||
typingPolicy === "internal_webchat" ||
|
||||
suppressTyping
|
||||
) {
|
||||
return "never";
|
||||
}
|
||||
if (configured) {
|
||||
|
||||
@@ -13,6 +13,13 @@ export type ModelSelectedContext = {
|
||||
thinkLevel: string | undefined;
|
||||
};
|
||||
|
||||
export type TypingPolicy =
|
||||
| "auto"
|
||||
| "user_message"
|
||||
| "system_event"
|
||||
| "internal_webchat"
|
||||
| "heartbeat";
|
||||
|
||||
export type GetReplyOptions = {
|
||||
/** Override run id for agent events (defaults to random UUID). */
|
||||
runId?: string;
|
||||
@@ -27,6 +34,10 @@ export type GetReplyOptions = {
|
||||
onTypingCleanup?: () => void;
|
||||
onTypingController?: (typing: TypingController) => void;
|
||||
isHeartbeat?: boolean;
|
||||
/** Policy-level typing control for run classes (user/system/internal/heartbeat). */
|
||||
typingPolicy?: TypingPolicy;
|
||||
/** Force-disable typing indicators for this run (system/internal/cross-channel routes). */
|
||||
suppressTyping?: boolean;
|
||||
/** Resolved heartbeat model override (provider/model string from merged per-agent config). */
|
||||
heartbeatModelOverride?: string;
|
||||
/** If true, suppress tool error warning payloads for this run. */
|
||||
|
||||
@@ -73,6 +73,80 @@ describe("createTypingCallbacks", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("stops keepalive after consecutive start failures", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const start = vi.fn().mockRejectedValue(new Error("gone"));
|
||||
const onStartError = vi.fn();
|
||||
const callbacks = createTypingCallbacks({ start, onStartError });
|
||||
|
||||
await callbacks.onReplyStart();
|
||||
expect(start).toHaveBeenCalledTimes(1);
|
||||
expect(onStartError).toHaveBeenCalledTimes(1);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(3_000);
|
||||
expect(start).toHaveBeenCalledTimes(2);
|
||||
expect(onStartError).toHaveBeenCalledTimes(2);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(9_000);
|
||||
expect(start).toHaveBeenCalledTimes(2);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("does not restart keepalive when breaker trips on initial start", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const start = vi.fn().mockRejectedValue(new Error("gone"));
|
||||
const onStartError = vi.fn();
|
||||
const callbacks = createTypingCallbacks({
|
||||
start,
|
||||
onStartError,
|
||||
maxConsecutiveFailures: 1,
|
||||
});
|
||||
|
||||
await callbacks.onReplyStart();
|
||||
expect(start).toHaveBeenCalledTimes(1);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(9_000);
|
||||
expect(start).toHaveBeenCalledTimes(1);
|
||||
expect(onStartError).toHaveBeenCalledTimes(1);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("resets failure counter after a successful keepalive tick", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
let callCount = 0;
|
||||
const start = vi.fn().mockImplementation(async () => {
|
||||
callCount += 1;
|
||||
if (callCount % 2 === 1) {
|
||||
throw new Error("flaky");
|
||||
}
|
||||
});
|
||||
const onStartError = vi.fn();
|
||||
const callbacks = createTypingCallbacks({
|
||||
start,
|
||||
onStartError,
|
||||
maxConsecutiveFailures: 2,
|
||||
});
|
||||
|
||||
await callbacks.onReplyStart(); // fail
|
||||
await vi.advanceTimersByTimeAsync(3_000); // success
|
||||
await vi.advanceTimersByTimeAsync(3_000); // fail
|
||||
await vi.advanceTimersByTimeAsync(3_000); // success
|
||||
await vi.advanceTimersByTimeAsync(3_000); // fail
|
||||
|
||||
expect(start).toHaveBeenCalledTimes(5);
|
||||
expect(onStartError).toHaveBeenCalledTimes(3);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("deduplicates stop across idle and cleanup", async () => {
|
||||
const start = vi.fn().mockResolvedValue(undefined);
|
||||
const stop = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
@@ -13,6 +13,8 @@ export type CreateTypingCallbacksParams = {
|
||||
onStartError: (err: unknown) => void;
|
||||
onStopError?: (err: unknown) => void;
|
||||
keepaliveIntervalMs?: number;
|
||||
/** Stop keepalive after this many consecutive start() failures. Default: 2 */
|
||||
maxConsecutiveFailures?: number;
|
||||
/** Maximum duration for typing indicator before auto-cleanup (safety TTL). Default: 60s */
|
||||
maxDurationMs?: number;
|
||||
};
|
||||
@@ -20,19 +22,31 @@ export type CreateTypingCallbacksParams = {
|
||||
export function createTypingCallbacks(params: CreateTypingCallbacksParams): TypingCallbacks {
|
||||
const stop = params.stop;
|
||||
const keepaliveIntervalMs = params.keepaliveIntervalMs ?? 3_000;
|
||||
const maxConsecutiveFailures = Math.max(1, params.maxConsecutiveFailures ?? 2);
|
||||
const maxDurationMs = params.maxDurationMs ?? 60_000; // Default 60s TTL
|
||||
let stopSent = false;
|
||||
let closed = false;
|
||||
let consecutiveFailures = 0;
|
||||
let breakerTripped = false;
|
||||
let ttlTimer: ReturnType<typeof setTimeout> | undefined;
|
||||
|
||||
const fireStart = async () => {
|
||||
const fireStart = async (): Promise<void> => {
|
||||
if (closed) {
|
||||
return;
|
||||
}
|
||||
if (breakerTripped) {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
await params.start();
|
||||
consecutiveFailures = 0;
|
||||
} catch (err) {
|
||||
consecutiveFailures += 1;
|
||||
params.onStartError(err);
|
||||
if (consecutiveFailures >= maxConsecutiveFailures) {
|
||||
breakerTripped = true;
|
||||
keepaliveLoop.stop();
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
@@ -67,9 +81,14 @@ export function createTypingCallbacks(params: CreateTypingCallbacksParams): Typi
|
||||
return;
|
||||
}
|
||||
stopSent = false;
|
||||
breakerTripped = false;
|
||||
consecutiveFailures = 0;
|
||||
keepaliveLoop.stop();
|
||||
clearTtlTimer();
|
||||
await fireStart();
|
||||
if (breakerTripped) {
|
||||
return;
|
||||
}
|
||||
keepaliveLoop.start();
|
||||
startTtlTimer(); // Start TTL safety timer
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user