mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(chat): rearm safety timer on sub-agent tool / task-board updates (#2857)
Co-authored-by: zahica1234 <i.milev001@gmail.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
co-authored by
zahica1234
Claude Opus 4.7
parent
977cbf959b
commit
f89501312a
@@ -40,6 +40,7 @@ import {
|
||||
beginInferenceTurn,
|
||||
clearRuntimeForThread,
|
||||
fetchAndHydrateTurnState,
|
||||
type InferenceStatus,
|
||||
setTaskBoardForThread,
|
||||
setToolTimelineForThread,
|
||||
} from '../store/chatRuntimeSlice';
|
||||
@@ -296,6 +297,10 @@ const Conversations = ({
|
||||
// from `selectedThreadId` so switching threads mid-turn doesn't move the
|
||||
// timer's reference point.
|
||||
const sendingThreadIdRef = useRef<string | null>(null);
|
||||
// Previous inference status for the sending thread; lets the rearm effect
|
||||
// distinguish "status was just cleared (chat_done / chat_error)" from
|
||||
// "status was never set yet (in-flight turn pre-status)".
|
||||
const prevInferenceStatusRef = useRef<InferenceStatus | undefined>(undefined);
|
||||
|
||||
const getAudioExtension = (mimeType: string): string => {
|
||||
const lower = mimeType.toLowerCase();
|
||||
@@ -493,32 +498,65 @@ const Conversations = ({
|
||||
dispatch(setActiveThread(null));
|
||||
sendingTimeoutRef.current = null;
|
||||
sendingThreadIdRef.current = null;
|
||||
// Reset so the NEXT send starts from a clean "never had a status"
|
||||
// baseline — otherwise the rearm effect could read this turn's last
|
||||
// status as a stale "previous" and falsely treat the next send's
|
||||
// first signal as a chat-done transition.
|
||||
prevInferenceStatusRef.current = undefined;
|
||||
pendingSendRef.current = null;
|
||||
setPendingSendingThreadId(null);
|
||||
}, 120_000);
|
||||
};
|
||||
|
||||
// Rearm the silence timer on every inference signal for the sending
|
||||
// thread. Tool / iteration / subagent events bump `inferenceStatusByThread`;
|
||||
// pure-text streams (no tools) only bump `streamingAssistantByThread`, so
|
||||
// both must be watched — otherwise a long text stream would trip the
|
||||
// safety timer mid-reply. When the status is cleared (chat_done /
|
||||
// chat_error), drop the timer — the completion handlers own UI cleanup.
|
||||
// thread. Top-level tool / iteration events bump `inferenceStatusByThread`;
|
||||
// pure-text streams (no tools) only bump `streamingAssistantByThread`;
|
||||
// sub-agent activity (a delegated `Research`/`Tools Agent`/`Memory Tree`
|
||||
// turn whose tools run in a child task) bumps `toolTimelineByThread` and
|
||||
// `taskBoardByThread` without necessarily re-emitting a top-level status
|
||||
// change, so all four must be watched — otherwise a long sub-agent loop
|
||||
// would trip the safety timer mid-run even though the user can see the
|
||||
// delegated tools firing in the timeline. When the status is cleared
|
||||
// (chat_done / chat_error), drop the timer — the completion handlers
|
||||
// own UI cleanup.
|
||||
//
|
||||
// `prevInferenceStatusRef` distinguishes "status was just cleared
|
||||
// (chat_done / chat_error transition: defined → undefined)" from "status
|
||||
// was never set yet (the Send handler also dispatches
|
||||
// `setToolTimelineForThread({ entries: [] })` to reset the timeline,
|
||||
// which fires this effect immediately after `armSilenceTimer` — at
|
||||
// that instant the inference status hasn't been published yet)". Only
|
||||
// the real transition should clear our timer.
|
||||
useEffect(() => {
|
||||
const threadId = sendingThreadIdRef.current;
|
||||
if (!threadId || !sendingTimeoutRef.current) return;
|
||||
const status = inferenceStatusByThread[threadId];
|
||||
if (status === undefined) {
|
||||
if (status === undefined && prevInferenceStatusRef.current !== undefined) {
|
||||
clearTimeout(sendingTimeoutRef.current);
|
||||
sendingTimeoutRef.current = null;
|
||||
sendingThreadIdRef.current = null;
|
||||
prevInferenceStatusRef.current = undefined;
|
||||
return;
|
||||
}
|
||||
prevInferenceStatusRef.current = status;
|
||||
armSilenceTimer(threadId);
|
||||
// armSilenceTimer is stable (refs + dispatch); depending on the
|
||||
// selector references is enough to rearm on every progress event.
|
||||
// Scope the dependencies to the SENDING thread's slices only, keyed by the
|
||||
// reactive `activeThreadId` (set on send, cleared on done/error/timeout —
|
||||
// so it tracks the in-flight turn for the timer's whole lifetime, unlike
|
||||
// `pendingSendingThreadId` which is released the moment the backend accepts
|
||||
// the send). Depending on the whole maps would rearm this thread's timer
|
||||
// whenever ANY other thread's state changed — unrelated background activity
|
||||
// shouldn't keep a foreground turn's timer alive. armSilenceTimer is stable
|
||||
// (refs + dispatch), so listing the per-thread values is enough to rearm on
|
||||
// every progress event for this thread.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [inferenceStatusByThread, streamingAssistantByThread]);
|
||||
}, [
|
||||
activeThreadId,
|
||||
activeThreadId ? inferenceStatusByThread[activeThreadId] : undefined,
|
||||
activeThreadId ? streamingAssistantByThread[activeThreadId] : undefined,
|
||||
activeThreadId ? toolTimelineByThread[activeThreadId] : undefined,
|
||||
activeThreadId ? taskBoardByThread[activeThreadId] : undefined,
|
||||
]);
|
||||
|
||||
useEffect(() => {
|
||||
if (
|
||||
@@ -745,6 +783,10 @@ const Conversations = ({
|
||||
// changes for `sendingThreadId`, so long-running agent turns stay alive
|
||||
// as long as the backend is emitting signals. A truly hung server still
|
||||
// fails fast.
|
||||
// Fresh send: clear the previous-status baseline before arming so the
|
||||
// first inference signal of this turn isn't misread as a chat-done
|
||||
// transition (defined → undefined) left over from the prior turn.
|
||||
prevInferenceStatusRef.current = undefined;
|
||||
armSilenceTimer(sendingThreadId);
|
||||
dispatch(setToolTimelineForThread({ threadId: sendingThreadId, entries: [] }));
|
||||
dispatch(beginInferenceTurn({ threadId: sendingThreadId }));
|
||||
@@ -777,6 +819,7 @@ const Conversations = ({
|
||||
sendingTimeoutRef.current = null;
|
||||
}
|
||||
sendingThreadIdRef.current = null;
|
||||
prevInferenceStatusRef.current = undefined;
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
if (
|
||||
msg.toLowerCase().includes('blocked by a security policy') ||
|
||||
|
||||
@@ -18,7 +18,10 @@ import { threadApi } from '../../services/api/threadApi';
|
||||
import { chatSend } from '../../services/chatService';
|
||||
import { CoreRpcError } from '../../services/coreRpcClient';
|
||||
import agentProfileReducer from '../../store/agentProfileSlice';
|
||||
import chatRuntimeReducer from '../../store/chatRuntimeSlice';
|
||||
import chatRuntimeReducer, {
|
||||
setInferenceStatusForThread,
|
||||
setToolTimelineForThread,
|
||||
} from '../../store/chatRuntimeSlice';
|
||||
import socketReducer from '../../store/socketSlice';
|
||||
import threadReducer, { setSelectedThread } from '../../store/threadSlice';
|
||||
import type { Thread } from '../../types/thread';
|
||||
@@ -802,6 +805,124 @@ describe('Conversations — smoke render (#1123 welcome-lock removal)', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('rearms the silence timer on sub-agent tool-timeline updates', async () => {
|
||||
// Regression: when a delegated sub-agent (`Research`, `Tools Agent`,
|
||||
// …) is running, the parent thread's `inferenceStatusByThread` and
|
||||
// `streamingAssistantByThread` references can stay put while
|
||||
// `toolTimelineByThread` and `taskBoardByThread` tick. The rearm
|
||||
// effect must watch all four — otherwise a long sub-agent loop
|
||||
// trips the 120s safety timer even though the user can see tools
|
||||
// firing in the timeline.
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true });
|
||||
try {
|
||||
const { textarea, store, thread } = await renderSelectedConversation();
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.change(textarea, { target: { value: 'kick off a sub-agent loop' } });
|
||||
});
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Send message' }));
|
||||
});
|
||||
await waitFor(() => {
|
||||
expect(chatSend).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// Two-thirds of the way through the safety window, the parent
|
||||
// status is already in `subagent` phase and a delegated tool
|
||||
// posts a timeline update. After the fix this re-arms the timer.
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(80_000);
|
||||
});
|
||||
await act(async () => {
|
||||
store!.dispatch(
|
||||
setInferenceStatusForThread({
|
||||
threadId: thread.id,
|
||||
status: { phase: 'subagent', iteration: 1, maxIterations: 8 },
|
||||
})
|
||||
);
|
||||
store!.dispatch(
|
||||
setToolTimelineForThread({
|
||||
threadId: thread.id,
|
||||
entries: [{ id: 'tl-1', name: 'web_fetch', round: 1, status: 'running' }],
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
// Advance another 80s (total elapsed 160s, well past the 120s
|
||||
// window). The tool-timeline dispatch should have re-armed the
|
||||
// timer at the 80s mark, so the silence timer is now at 80s of
|
||||
// its fresh 120s budget and has NOT fired. The pending guard
|
||||
// therefore still holds and Send stays disabled — proof the
|
||||
// rearm effect ran on a toolTimelineByThread change.
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(80_000);
|
||||
});
|
||||
await act(async () => {
|
||||
fireEvent.change(textarea, { target: { value: 'still typing while sub-agent runs' } });
|
||||
});
|
||||
expect(screen.getByRole('button', { name: 'Send message' })).toBeDisabled();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it('does NOT rearm the silence timer on an unrelated thread’s updates', async () => {
|
||||
// Regression for the per-thread dependency scoping: the rearm effect must
|
||||
// react only to the SENDING thread's slices. A different thread churning
|
||||
// (background triage, another conversation) must not keep the foreground
|
||||
// turn's 120s timer alive — otherwise a truly hung send never fails fast.
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true });
|
||||
try {
|
||||
const { textarea, store } = await renderSelectedConversation();
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.change(textarea, { target: { value: 'send on the foreground thread' } });
|
||||
});
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Send message' }));
|
||||
});
|
||||
await waitFor(() => {
|
||||
expect(chatSend).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// Churn an UNRELATED thread the whole time the foreground send is open.
|
||||
// None of these dispatches target the sending thread ('send-thread'),
|
||||
// so they must not rearm its timer.
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(80_000);
|
||||
});
|
||||
await act(async () => {
|
||||
store!.dispatch(
|
||||
setInferenceStatusForThread({
|
||||
threadId: 'some-other-thread',
|
||||
status: { phase: 'subagent', iteration: 3, maxIterations: 8 },
|
||||
})
|
||||
);
|
||||
store!.dispatch(
|
||||
setToolTimelineForThread({
|
||||
threadId: 'some-other-thread',
|
||||
entries: [{ id: 'other-1', name: 'web_fetch', round: 1, status: 'running' }],
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
// Cross the original 120s deadline (80s + 50s = 130s). Because the
|
||||
// unrelated-thread churn did NOT rearm, the safety timer fires: the
|
||||
// pending guard is released and Send re-enables once the user types.
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(50_000);
|
||||
});
|
||||
await act(async () => {
|
||||
fireEvent.change(textarea, { target: { value: 'retry after timeout' } });
|
||||
});
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole('button', { name: 'Send message' })).not.toBeDisabled();
|
||||
});
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it('releases the pending-send lock when chatSend rejects', async () => {
|
||||
vi.mocked(chatSend).mockRejectedValueOnce(new Error('emit failed'));
|
||||
const { textarea } = await renderSelectedConversation();
|
||||
|
||||
Reference in New Issue
Block a user