mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-11 01:24:31 +00:00
feat(typing): add TTL safety-net for stuck indicators (land #27428, thanks @Crpdim)
Co-authored-by: Crpdim <crpdim@users.noreply.github.com>
This commit is contained in:
@@ -21,6 +21,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
- Telegram/Inline buttons: allow callback-query button handling in groups (including `/models` follow-up buttons) when group policy authorizes the sender, by removing the redundant callback allowlist gate that blocked open-policy groups. (#27343) Thanks @GodsBoy.
|
- Telegram/Inline buttons: allow callback-query button handling in groups (including `/models` follow-up buttons) when group policy authorizes the sender, by removing the redundant callback allowlist gate that blocked open-policy groups. (#27343) Thanks @GodsBoy.
|
||||||
- Telegram/Streaming preview: when finalizing without an existing preview message, prime pending preview text with final answer before stop-flush so users do not briefly see stale 1-2 word fragments (for example `no` before `no problem`). (#27449) Thanks @emanuelst for the original fix direction in #19673.
|
- Telegram/Streaming preview: when finalizing without an existing preview message, prime pending preview text with final answer before stop-flush so users do not briefly see stale 1-2 word fragments (for example `no` before `no problem`). (#27449) Thanks @emanuelst for the original fix direction in #19673.
|
||||||
- Telegram/sendChatAction 401 handling: add bounded exponential backoff + temporary local typing suppression after repeated unauthorized failures to stop unbounded `sendChatAction` retry loops that can trigger Telegram abuse enforcement and bot deletion. (#27415) Thanks @widingmarcus-cyber.
|
- Telegram/sendChatAction 401 handling: add bounded exponential backoff + temporary local typing suppression after repeated unauthorized failures to stop unbounded `sendChatAction` retry loops that can trigger Telegram abuse enforcement and bot deletion. (#27415) Thanks @widingmarcus-cyber.
|
||||||
|
- Typing/TTL safety net: add max-duration guardrails to shared typing callbacks so stuck lifecycle edges auto-stop typing indicators even when explicit idle/cleanup signals are missed. (#27428) Thanks @Crpdim.
|
||||||
- Typing/Main reply pipeline: always mark dispatch idle in `agent-runner` finalization so typing cleanup runs even when dispatcher `onIdle` does not fire, preventing stuck typing indicators after run completion. (#27250) Thanks @Sid-Qin.
|
- Typing/Main reply pipeline: always mark dispatch idle in `agent-runner` finalization so typing cleanup runs even when dispatcher `onIdle` does not fire, preventing stuck typing indicators after run completion. (#27250) Thanks @Sid-Qin.
|
||||||
- Typing/Run completion race: prevent post-run keepalive ticks from re-triggering typing callbacks by guarding `triggerTyping()` with `runComplete`, with regression coverage for no-restart behavior during run-complete/dispatch-idle boundaries. (#27413) Thanks @widingmarcus-cyber.
|
- Typing/Run completion race: prevent post-run keepalive ticks from re-triggering typing callbacks by guarding `triggerTyping()` with `runComplete`, with regression coverage for no-restart behavior during run-complete/dispatch-idle boundaries. (#27413) Thanks @widingmarcus-cyber.
|
||||||
- Daemon/macOS launchd: forward proxy env vars into supervised service environments, keep LaunchAgent `KeepAlive=true` semantics, and harden restart sequencing to `print -> bootout -> wait old pid exit -> bootstrap -> kickstart`. (#27276) thanks @frankekn.
|
- Daemon/macOS launchd: forward proxy env vars into supervised service environments, keep LaunchAgent `KeepAlive=true` semantics, and harden restart sequencing to `print -> bootout -> wait old pid exit -> bootstrap -> kickstart`. (#27276) thanks @frankekn.
|
||||||
|
|||||||
@@ -109,4 +109,156 @@ describe("createTypingCallbacks", () => {
|
|||||||
vi.useRealTimers();
|
vi.useRealTimers();
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// ========== TTL Safety Tests ==========
|
||||||
|
describe("TTL safety", () => {
|
||||||
|
it("auto-stops typing after maxDurationMs", async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
try {
|
||||||
|
const consoleWarn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||||
|
const start = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const stop = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const onStartError = vi.fn();
|
||||||
|
const callbacks = createTypingCallbacks({
|
||||||
|
start,
|
||||||
|
stop,
|
||||||
|
onStartError,
|
||||||
|
maxDurationMs: 10_000,
|
||||||
|
});
|
||||||
|
|
||||||
|
await callbacks.onReplyStart();
|
||||||
|
expect(start).toHaveBeenCalledTimes(1);
|
||||||
|
expect(stop).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
// Advance past TTL
|
||||||
|
await vi.advanceTimersByTimeAsync(10_000);
|
||||||
|
|
||||||
|
// Should auto-stop
|
||||||
|
expect(stop).toHaveBeenCalledTimes(1);
|
||||||
|
expect(consoleWarn).toHaveBeenCalledWith(expect.stringContaining("TTL exceeded"));
|
||||||
|
|
||||||
|
consoleWarn.mockRestore();
|
||||||
|
} finally {
|
||||||
|
vi.useRealTimers();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not auto-stop if idle is called before TTL", async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
try {
|
||||||
|
const consoleWarn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||||
|
const start = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const stop = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const onStartError = vi.fn();
|
||||||
|
const callbacks = createTypingCallbacks({
|
||||||
|
start,
|
||||||
|
stop,
|
||||||
|
onStartError,
|
||||||
|
maxDurationMs: 10_000,
|
||||||
|
});
|
||||||
|
|
||||||
|
await callbacks.onReplyStart();
|
||||||
|
|
||||||
|
// Stop before TTL
|
||||||
|
await vi.advanceTimersByTimeAsync(5_000);
|
||||||
|
callbacks.onIdle?.();
|
||||||
|
await flushMicrotasks();
|
||||||
|
|
||||||
|
expect(stop).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
// Advance past original TTL
|
||||||
|
await vi.advanceTimersByTimeAsync(10_000);
|
||||||
|
|
||||||
|
// Should not have triggered TTL warning
|
||||||
|
expect(consoleWarn).not.toHaveBeenCalled();
|
||||||
|
// Stop should still be called only once
|
||||||
|
expect(stop).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
consoleWarn.mockRestore();
|
||||||
|
} finally {
|
||||||
|
vi.useRealTimers();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("uses default 60s TTL when not specified", async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
try {
|
||||||
|
const start = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const stop = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const onStartError = vi.fn();
|
||||||
|
const callbacks = createTypingCallbacks({ start, stop, onStartError });
|
||||||
|
|
||||||
|
await callbacks.onReplyStart();
|
||||||
|
|
||||||
|
// Should not stop at 59s
|
||||||
|
await vi.advanceTimersByTimeAsync(59_000);
|
||||||
|
expect(stop).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
// Should stop at 60s
|
||||||
|
await vi.advanceTimersByTimeAsync(1_000);
|
||||||
|
expect(stop).toHaveBeenCalledTimes(1);
|
||||||
|
} finally {
|
||||||
|
vi.useRealTimers();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("disables TTL when maxDurationMs is 0", async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
try {
|
||||||
|
const start = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const stop = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const onStartError = vi.fn();
|
||||||
|
const callbacks = createTypingCallbacks({
|
||||||
|
start,
|
||||||
|
stop,
|
||||||
|
onStartError,
|
||||||
|
maxDurationMs: 0,
|
||||||
|
});
|
||||||
|
|
||||||
|
await callbacks.onReplyStart();
|
||||||
|
|
||||||
|
// Should not auto-stop even after long time
|
||||||
|
await vi.advanceTimersByTimeAsync(300_000);
|
||||||
|
expect(stop).not.toHaveBeenCalled();
|
||||||
|
} finally {
|
||||||
|
vi.useRealTimers();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("resets TTL timer on restart after idle", async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
try {
|
||||||
|
const start = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const stop = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const onStartError = vi.fn();
|
||||||
|
const callbacks = createTypingCallbacks({
|
||||||
|
start,
|
||||||
|
stop,
|
||||||
|
onStartError,
|
||||||
|
maxDurationMs: 10_000,
|
||||||
|
});
|
||||||
|
|
||||||
|
// First start
|
||||||
|
await callbacks.onReplyStart();
|
||||||
|
await vi.advanceTimersByTimeAsync(5_000);
|
||||||
|
|
||||||
|
// Idle and restart
|
||||||
|
callbacks.onIdle?.();
|
||||||
|
await flushMicrotasks();
|
||||||
|
expect(stop).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
// Reset mock to track second start
|
||||||
|
stop.mockClear();
|
||||||
|
|
||||||
|
// After stop, callbacks are closed, so new onReplyStart should be no-op
|
||||||
|
await callbacks.onReplyStart();
|
||||||
|
await vi.advanceTimersByTimeAsync(15_000);
|
||||||
|
|
||||||
|
// Should not trigger stop again since it's closed
|
||||||
|
expect(stop).not.toHaveBeenCalled();
|
||||||
|
} finally {
|
||||||
|
vi.useRealTimers();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -3,21 +3,27 @@ import { createTypingKeepaliveLoop } from "./typing-lifecycle.js";
|
|||||||
export type TypingCallbacks = {
|
export type TypingCallbacks = {
|
||||||
onReplyStart: () => Promise<void>;
|
onReplyStart: () => Promise<void>;
|
||||||
onIdle?: () => void;
|
onIdle?: () => void;
|
||||||
/** Called when the typing controller is cleaned up (e.g., on NO_REPLY). */
|
/** Called when the typing controller is cleaned up (e.g. on NO_REPLY). */
|
||||||
onCleanup?: () => void;
|
onCleanup?: () => void;
|
||||||
};
|
};
|
||||||
|
|
||||||
export function createTypingCallbacks(params: {
|
export type CreateTypingCallbacksParams = {
|
||||||
start: () => Promise<void>;
|
start: () => Promise<void>;
|
||||||
stop?: () => Promise<void>;
|
stop?: () => Promise<void>;
|
||||||
onStartError: (err: unknown) => void;
|
onStartError: (err: unknown) => void;
|
||||||
onStopError?: (err: unknown) => void;
|
onStopError?: (err: unknown) => void;
|
||||||
keepaliveIntervalMs?: number;
|
keepaliveIntervalMs?: number;
|
||||||
}): TypingCallbacks {
|
/** Maximum duration for typing indicator before auto-cleanup (safety TTL). Default: 60s */
|
||||||
|
maxDurationMs?: number;
|
||||||
|
};
|
||||||
|
|
||||||
|
export function createTypingCallbacks(params: CreateTypingCallbacksParams): TypingCallbacks {
|
||||||
const stop = params.stop;
|
const stop = params.stop;
|
||||||
const keepaliveIntervalMs = params.keepaliveIntervalMs ?? 3_000;
|
const keepaliveIntervalMs = params.keepaliveIntervalMs ?? 3_000;
|
||||||
|
const maxDurationMs = params.maxDurationMs ?? 60_000; // Default 60s TTL
|
||||||
let stopSent = false;
|
let stopSent = false;
|
||||||
let closed = false;
|
let closed = false;
|
||||||
|
let ttlTimer: ReturnType<typeof setTimeout> | undefined;
|
||||||
|
|
||||||
const fireStart = async () => {
|
const fireStart = async () => {
|
||||||
if (closed) {
|
if (closed) {
|
||||||
@@ -35,19 +41,43 @@ export function createTypingCallbacks(params: {
|
|||||||
onTick: fireStart,
|
onTick: fireStart,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// TTL safety: auto-stop typing after maxDurationMs
|
||||||
|
const startTtlTimer = () => {
|
||||||
|
if (maxDurationMs <= 0) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
clearTtlTimer();
|
||||||
|
ttlTimer = setTimeout(() => {
|
||||||
|
if (!closed) {
|
||||||
|
console.warn(`[typing] TTL exceeded (${maxDurationMs}ms), auto-stopping typing indicator`);
|
||||||
|
fireStop();
|
||||||
|
}
|
||||||
|
}, maxDurationMs);
|
||||||
|
};
|
||||||
|
|
||||||
|
const clearTtlTimer = () => {
|
||||||
|
if (ttlTimer) {
|
||||||
|
clearTimeout(ttlTimer);
|
||||||
|
ttlTimer = undefined;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
const onReplyStart = async () => {
|
const onReplyStart = async () => {
|
||||||
if (closed) {
|
if (closed) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
stopSent = false;
|
stopSent = false;
|
||||||
keepaliveLoop.stop();
|
keepaliveLoop.stop();
|
||||||
|
clearTtlTimer();
|
||||||
await fireStart();
|
await fireStart();
|
||||||
keepaliveLoop.start();
|
keepaliveLoop.start();
|
||||||
|
startTtlTimer(); // Start TTL safety timer
|
||||||
};
|
};
|
||||||
|
|
||||||
const fireStop = () => {
|
const fireStop = () => {
|
||||||
closed = true;
|
closed = true;
|
||||||
keepaliveLoop.stop();
|
keepaliveLoop.stop();
|
||||||
|
clearTtlTimer(); // Clear TTL timer on normal stop
|
||||||
if (!stop || stopSent) {
|
if (!stop || stopSent) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user