mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-22 03:51:18 +00:00
fix(tui): dismiss watchdog notice when response actually arrives
The streaming watchdog renders 'This response is taking longer than expected. Send another message to continue.' after 30s without a chat delta. If a delta or final then arrives — common for runs that are slow but not stuck — the notice stays in the log alongside the recovered response and contradicts what the user sees. Track the notice by runId in the chat log via a new `addPendingSystem` + `dismissPendingSystem` pair (mirroring the existing pendingUsers pattern) and dismiss it from `handleChatEvent` whenever any further chat event for that run is processed. The watchdog's internal cleanup (`activeChatRunId` reset, status idle, history reload) is unchanged. Refs #67052, #69081 (closed). Prior attempt #69026 raised the threshold and suppressed the notice entirely; this is the narrower fix that keeps the warning useful for genuinely stuck runs.
This commit is contained in:
parent
1300b22630
commit
97e4e18cb9
5 changed files with 132 additions and 8 deletions
|
|
@ -29,6 +29,7 @@ Docs: https://docs.openclaw.ai
|
|||
|
||||
### Fixes
|
||||
|
||||
- TUI/streaming watchdog: dismiss the `This response is taking longer than expected` notice as soon as a chat event for the same run arrives, so the message no longer sits next to the recovered response when the run was only briefly silent. Refs #67052, #69081 (closed), prior attempt #69026. Thanks @jpruit20 and @romneyda.
|
||||
- GitHub Copilot: drop unsafe native Responses reasoning replay items with non-replayable IDs before dispatch, preventing affected Copilot sessions from failing with `invalid_request_body`. Fixes #83220. Thanks @galiniliev.
|
||||
- QA-Lab: make runtime tool coverage fail on missing required tool exercise instead of treating pass/pass parity envelope drift as missing coverage.
|
||||
- Core/plugins: harden clawpatch-reported edge cases across gateway auth cleanup, Claude session id paths, plugin activation policy, apply-patch hunk handling, diagnostic redaction, and plugin metadata validation.
|
||||
|
|
|
|||
|
|
@ -150,6 +150,33 @@ describe("ChatLog", () => {
|
|||
expect(chatLog.countPendingUsers()).toBe(0);
|
||||
});
|
||||
|
||||
it("dismisses a pending system notice by runId", () => {
|
||||
const chatLog = new ChatLog(40);
|
||||
|
||||
chatLog.addPendingSystem("run-1", "taking longer than expected");
|
||||
let rendered = chatLog.render(120).join("\n");
|
||||
expect(rendered).toContain("taking longer than expected");
|
||||
|
||||
const dismissed = chatLog.dismissPendingSystem("run-1");
|
||||
expect(dismissed).toBe(true);
|
||||
|
||||
rendered = chatLog.render(120).join("\n");
|
||||
expect(rendered).not.toContain("taking longer than expected");
|
||||
expect(chatLog.dismissPendingSystem("run-1")).toBe(false);
|
||||
});
|
||||
|
||||
it("replaces an existing pending system notice for the same runId", () => {
|
||||
const chatLog = new ChatLog(40);
|
||||
|
||||
chatLog.addPendingSystem("run-1", "first notice");
|
||||
chatLog.addPendingSystem("run-1", "second notice");
|
||||
|
||||
const rendered = chatLog.render(120).join("\n");
|
||||
expect(rendered).not.toContain("first notice");
|
||||
expect(rendered).toContain("second notice");
|
||||
expect(chatLog.children.length).toBe(1);
|
||||
});
|
||||
|
||||
it("does not hide a new repeated prompt when only older history matches", () => {
|
||||
const chatLog = new ChatLog(40);
|
||||
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ export class ChatLog extends Container {
|
|||
createdAt: number;
|
||||
}
|
||||
>();
|
||||
private pendingSystemNotices = new Map<string, Container>();
|
||||
private btwMessage: BtwInlineMessage | null = null;
|
||||
private toolsExpanded = false;
|
||||
|
||||
|
|
@ -44,6 +45,11 @@ export class ChatLog extends Container {
|
|||
this.pendingUsers.delete(runId);
|
||||
}
|
||||
}
|
||||
for (const [runId, entry] of this.pendingSystemNotices.entries()) {
|
||||
if (entry === component) {
|
||||
this.pendingSystemNotices.delete(runId);
|
||||
}
|
||||
}
|
||||
if (this.btwMessage === component) {
|
||||
this.btwMessage = null;
|
||||
}
|
||||
|
|
@ -69,6 +75,7 @@ export class ChatLog extends Container {
|
|||
this.clear();
|
||||
this.toolById.clear();
|
||||
this.streamingRuns.clear();
|
||||
this.pendingSystemNotices.clear();
|
||||
this.btwMessage = null;
|
||||
if (!opts?.preservePendingUsers) {
|
||||
this.pendingUsers.clear();
|
||||
|
|
@ -102,6 +109,29 @@ export class ChatLog extends Container {
|
|||
this.append(this.createSystemMessage(text));
|
||||
}
|
||||
|
||||
// Tag a system notice with a runId so a later chat event for the same run
|
||||
// can dismiss it. Used by the streaming watchdog so the "taking longer than
|
||||
// expected" notice goes away if the response actually arrives afterwards.
|
||||
addPendingSystem(runId: string, text: string) {
|
||||
const existing = this.pendingSystemNotices.get(runId);
|
||||
if (existing) {
|
||||
this.removeChild(existing);
|
||||
}
|
||||
const entry = this.createSystemMessage(text);
|
||||
this.pendingSystemNotices.set(runId, entry);
|
||||
this.append(entry);
|
||||
}
|
||||
|
||||
dismissPendingSystem(runId: string) {
|
||||
const existing = this.pendingSystemNotices.get(runId);
|
||||
if (!existing) {
|
||||
return false;
|
||||
}
|
||||
this.removeChild(existing);
|
||||
this.pendingSystemNotices.delete(runId);
|
||||
return true;
|
||||
}
|
||||
|
||||
addUser(text: string) {
|
||||
this.append(new UserMessageComponent(text));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,6 +8,8 @@ type HandlerChatLog = {
|
|||
startTool: (...args: unknown[]) => void;
|
||||
updateToolResult: (...args: unknown[]) => void;
|
||||
addSystem: (...args: unknown[]) => void;
|
||||
addPendingSystem: (...args: unknown[]) => void;
|
||||
dismissPendingSystem: (...args: unknown[]) => void;
|
||||
updateAssistant: (...args: unknown[]) => void;
|
||||
finalizeAssistant: (...args: unknown[]) => void;
|
||||
dropAssistant: (...args: unknown[]) => void;
|
||||
|
|
@ -21,6 +23,8 @@ type MockChatLog = {
|
|||
startTool: MockFn;
|
||||
updateToolResult: MockFn;
|
||||
addSystem: MockFn;
|
||||
addPendingSystem: MockFn;
|
||||
dismissPendingSystem: MockFn;
|
||||
updateAssistant: MockFn;
|
||||
finalizeAssistant: MockFn;
|
||||
dropAssistant: MockFn;
|
||||
|
|
@ -36,6 +40,8 @@ function createMockChatLog(): MockChatLog & HandlerChatLog {
|
|||
startTool: vi.fn(),
|
||||
updateToolResult: vi.fn(),
|
||||
addSystem: vi.fn(),
|
||||
addPendingSystem: vi.fn(),
|
||||
dismissPendingSystem: vi.fn(),
|
||||
updateAssistant: vi.fn(),
|
||||
finalizeAssistant: vi.fn(),
|
||||
dropAssistant: vi.fn(),
|
||||
|
|
@ -1078,7 +1084,7 @@ describe("tui-event-handlers: streaming watchdog", () => {
|
|||
|
||||
expect(setActivityStatus).toHaveBeenLastCalledWith("idle");
|
||||
expect(state.activeChatRunId).toBeNull();
|
||||
expect(chatLog.addSystem).toHaveBeenCalledWith(expectedTimeoutMessage);
|
||||
expect(chatLog.addPendingSystem).toHaveBeenCalledWith("run-stuck", expectedTimeoutMessage);
|
||||
|
||||
handlers.dispose?.();
|
||||
});
|
||||
|
|
@ -1284,7 +1290,7 @@ describe("tui-event-handlers: streaming watchdog", () => {
|
|||
expect(setActivityStatus).toHaveBeenLastCalledWith("idle");
|
||||
expect(state.activeChatRunId).toBeNull();
|
||||
expect(loadHistory).toHaveBeenCalledTimes(1);
|
||||
expect(chatLog.addSystem).not.toHaveBeenCalledWith(expectedTimeoutMessage);
|
||||
expect(chatLog.addPendingSystem).not.toHaveBeenCalled();
|
||||
|
||||
handlers.dispose?.();
|
||||
});
|
||||
|
|
@ -1310,8 +1316,8 @@ describe("tui-event-handlers: streaming watchdog", () => {
|
|||
vi.advanceTimersByTime(10_000);
|
||||
|
||||
const statusCalls = setActivityStatus.mock.calls.map((c) => c[0]);
|
||||
expect(statusCalls.reduce((count, s) => count + (s === "idle" ? 1 : 0), 0)).toBe(1);
|
||||
expect(chatLog.addSystem).not.toHaveBeenCalledWith(expectedTimeoutMessage);
|
||||
expect(statusCalls.filter((s) => s === "idle").length).toBe(1);
|
||||
expect(chatLog.addPendingSystem).not.toHaveBeenCalled();
|
||||
expect(state.activeChatRunId).toBeNull();
|
||||
|
||||
handlers.dispose?.();
|
||||
|
|
@ -1332,7 +1338,7 @@ describe("tui-event-handlers: streaming watchdog", () => {
|
|||
vi.advanceTimersByTime(60_000);
|
||||
|
||||
expect(setActivityStatus).not.toHaveBeenCalledWith("idle");
|
||||
expect(chatLog.addSystem).not.toHaveBeenCalled();
|
||||
expect(chatLog.addPendingSystem).not.toHaveBeenCalled();
|
||||
expect(state.activeChatRunId).toBe("run-no-watchdog");
|
||||
|
||||
handlers.dispose?.();
|
||||
|
|
@ -1374,7 +1380,7 @@ describe("tui-event-handlers: streaming watchdog", () => {
|
|||
|
||||
expect(setActivityStatus).toHaveBeenLastCalledWith("idle");
|
||||
expect(state.activeChatRunId).toBeNull();
|
||||
expect(chatLog.addSystem).toHaveBeenCalledTimes(2);
|
||||
expect(chatLog.addPendingSystem).toHaveBeenCalledTimes(2);
|
||||
|
||||
handlers.dispose?.();
|
||||
});
|
||||
|
|
@ -1395,6 +1401,60 @@ describe("tui-event-handlers: streaming watchdog", () => {
|
|||
vi.advanceTimersByTime(10_000);
|
||||
|
||||
expect(setActivityStatus).not.toHaveBeenCalledWith("idle");
|
||||
expect(chatLog.addSystem).not.toHaveBeenCalled();
|
||||
expect(chatLog.addPendingSystem).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("dismisses the watchdog notice when a delta arrives after the watchdog fires", () => {
|
||||
const { state, chatLog, handlers } = createHarness({
|
||||
streamingWatchdogMs: 5_000,
|
||||
});
|
||||
|
||||
handlers.handleChatEvent({
|
||||
runId: "run-late",
|
||||
sessionKey: state.currentSessionKey,
|
||||
state: "delta",
|
||||
message: { content: "starting" },
|
||||
} satisfies ChatEvent);
|
||||
|
||||
vi.advanceTimersByTime(5_001);
|
||||
expect(chatLog.addPendingSystem).toHaveBeenCalledWith("run-late", expectedTimeoutMessage);
|
||||
|
||||
handlers.handleChatEvent({
|
||||
runId: "run-late",
|
||||
sessionKey: state.currentSessionKey,
|
||||
state: "delta",
|
||||
message: { content: "actually here" },
|
||||
} satisfies ChatEvent);
|
||||
|
||||
expect(chatLog.dismissPendingSystem).toHaveBeenCalledWith("run-late");
|
||||
|
||||
handlers.dispose?.();
|
||||
});
|
||||
|
||||
it("dismisses the watchdog notice when the final arrives after the watchdog fires", () => {
|
||||
const { state, chatLog, handlers } = createHarness({
|
||||
streamingWatchdogMs: 5_000,
|
||||
});
|
||||
|
||||
handlers.handleChatEvent({
|
||||
runId: "run-final-late",
|
||||
sessionKey: state.currentSessionKey,
|
||||
state: "delta",
|
||||
message: { content: "starting" },
|
||||
} satisfies ChatEvent);
|
||||
|
||||
vi.advanceTimersByTime(5_001);
|
||||
expect(chatLog.addPendingSystem).toHaveBeenCalledWith("run-final-late", expectedTimeoutMessage);
|
||||
|
||||
handlers.handleChatEvent({
|
||||
runId: "run-final-late",
|
||||
sessionKey: state.currentSessionKey,
|
||||
state: "final",
|
||||
message: { content: [{ type: "text", text: "done" }], stopReason: "stop" },
|
||||
} satisfies ChatEvent);
|
||||
|
||||
expect(chatLog.dismissPendingSystem).toHaveBeenCalledWith("run-final-late");
|
||||
|
||||
handlers.dispose?.();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -14,6 +14,8 @@ type EventHandlerChatLog = {
|
|||
options?: { partial?: boolean; isError?: boolean },
|
||||
) => void;
|
||||
addSystem: (text: string) => void;
|
||||
addPendingSystem: (runId: string, text: string) => void;
|
||||
dismissPendingSystem: (runId: string) => void;
|
||||
updateAssistant: (text: string, runId: string) => void;
|
||||
finalizeAssistant: (text: string, runId: string) => void;
|
||||
dropAssistant: (runId: string) => void;
|
||||
|
|
@ -131,7 +133,7 @@ export function createEventHandlers(context: EventHandlerContext) {
|
|||
return;
|
||||
}
|
||||
flushPendingHistoryRefreshIfIdle();
|
||||
chatLog.addSystem(STREAMING_WATCHDOG_USER_MESSAGE);
|
||||
chatLog.addPendingSystem(runId, STREAMING_WATCHDOG_USER_MESSAGE);
|
||||
tui.requestRender();
|
||||
}, streamingWatchdogMs);
|
||||
const maybeUnref = (streamingWatchdogTimer as { unref?: () => void }).unref;
|
||||
|
|
@ -391,6 +393,10 @@ export function createEventHandlers(context: EventHandlerContext) {
|
|||
if (reconnectPendingRunId === evt.runId) {
|
||||
reconnectPendingRunId = null;
|
||||
}
|
||||
// Any chat event for a run that previously tripped the streaming watchdog
|
||||
// is proof the response is no longer stuck — drop the stale notice so it
|
||||
// doesn't sit alongside the recovered content.
|
||||
chatLog.dismissPendingSystem(evt.runId);
|
||||
noteSessionRun(evt.runId);
|
||||
if (!state.activeChatRunId && !isLocalBtwRunId?.(evt.runId)) {
|
||||
state.activeChatRunId = evt.runId;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue