fix(chat): only deliver proactive messages to a fresh thread (#3713)

Gate proactive (morning brief / subconscious) delivery on thread freshness via threadHasMessages(); reuse a thread only when provably empty, else open a new thread. Unknown/unloaded metadata fails closed. Closes #3713.
This commit is contained in:
sanil-23
2026-06-17 13:10:20 +05:30
committed by GitHub
parent 9abeda33c3
commit cdadf316aa
2 changed files with 119 additions and 11 deletions
+40 -8
View File
@@ -65,6 +65,34 @@ const MAX_SEGMENT_DELIVERIES = 100;
type SegmentDelivery = { segments: Map<number, string>; createdAt: number; lastSeenAt: number };
type ThreadSliceState = ReturnType<typeof store.getState>['thread'];
/**
* Whether a thread should be treated as occupied for proactive delivery — a
* thread is only a valid target when it is provably "fresh" (holds no
* messages). We check the in-memory message cache and the persisted
* `messageCount` snapshot, so a thread that already has a conversation
* server-side (but whose messages aren't loaded into the cache yet) is still
* treated as occupied and never reused.
*
* Crucially, *unknown* thread metadata counts as occupied: when only a
* rehydrated `selectedThreadId` is present (e.g. before `loadThreads`
* resolves, or after caches were reset while selection was preserved),
* `state.threads.find` returns `undefined`. Treating that as fresh would let
* a proactive event append into a conversation we simply haven't loaded yet.
* We fail closed and open a new thread instead. See #3713.
*/
function threadHasMessages(state: ThreadSliceState, threadId: string): boolean {
const cached = state.messagesByThreadId[threadId];
if (cached && cached.length > 0) return true;
if (threadId === state.selectedThreadId && state.messages.length > 0) return true;
const thread = state.threads.find(t => t.id === threadId);
// Unknown metadata → fail closed (occupied) rather than risk interrupting an
// unloaded conversation.
if (!thread) return true;
return thread.messageCount > 0;
}
function rtLog(message: string, fields?: Record<string, string | number | null | undefined>) {
if (IS_PROD) return;
if (fields && Object.keys(fields).length > 0) {
@@ -262,14 +290,18 @@ const ChatRuntimeProvider = ({ children }: { children: React.ReactNode }) => {
}
const state = store.getState().thread;
// Resolution priority: selected > any in-flight inference thread > first
// thread. With parallel inference there may be several active threads;
// any one is an acceptable proactive target when nothing is selected.
const firstActiveThreadId = Object.keys(state.activeThreadIds)[0] ?? null;
const targetFromState =
state.selectedThreadId ?? firstActiveThreadId ?? state.threads[0]?.id ?? null;
if (targetFromState) {
return targetFromState;
// Reuse an existing thread for proactive delivery ONLY when it is
// fresh (no messages). Injecting a morning brief / subconscious
// update into a thread that already holds a conversation interrupts
// the active chat flow (#3713). Candidate priority is selected >
// first thread; if the candidate already has messages we fall
// through and open a dedicated new thread instead. An in-flight
// inference thread always has at least the user's message, so it is
// never considered fresh — that is why `activeThreadIds` is no
// longer used as a target here.
const candidateThreadId = state.selectedThreadId ?? state.threads[0]?.id ?? null;
if (candidateThreadId && !threadHasMessages(state, candidateThreadId)) {
return candidateThreadId;
}
if (proactiveThreadCreationPromiseRef.current) {
@@ -421,10 +421,10 @@ describe('ChatRuntimeProvider — dedupe, proactive resolution, mid-turn invaria
});
describe('proactive thread resolution', () => {
it('reuses the selected thread when resolving a proactive: sender', async () => {
it('reuses the selected thread when it is fresh (no messages)', async () => {
store.dispatch(
loadThreads.fulfilled(
{ threads: [{ id: 'visible-thread', title: 'x' }] as never, count: 1 },
{ threads: [{ id: 'visible-thread', title: 'x', messageCount: 0 }] as never, count: 1 },
'req-id',
undefined
)
@@ -440,7 +440,7 @@ describe('ChatRuntimeProvider — dedupe, proactive resolution, mid-turn invaria
});
});
// createNewThread must NOT be invoked when a visible thread already exists.
// createNewThread must NOT be invoked when a fresh visible thread exists.
expect(threadApi.createNewThread).not.toHaveBeenCalled();
await waitFor(() =>
expect(threadApi.appendMessage).toHaveBeenCalledWith(
@@ -450,6 +450,82 @@ describe('ChatRuntimeProvider — dedupe, proactive resolution, mid-turn invaria
);
});
it('opens a new thread instead of interrupting a selected thread that has messages (#3713)', async () => {
vi.mocked(threadApi.createNewThread).mockResolvedValue({
id: 'fresh-thread',
title: 'new',
} as never);
vi.mocked(threadApi.getThreads).mockResolvedValue({
threads: [{ id: 'fresh-thread', title: 'new' }] as never,
count: 1,
});
// The user is mid-conversation: the selected thread already holds
// messages, so a proactive morning brief / subconscious update must
// NOT be injected into it.
store.dispatch(
loadThreads.fulfilled(
{ threads: [{ id: 'busy-thread', title: 'chat', messageCount: 4 }] as never, count: 1 },
'req-id',
undefined
)
);
store.dispatch(setSelectedThread('busy-thread'));
const listeners = renderProvider();
await act(async () => {
listeners.onProactiveMessage?.({
thread_id: 'proactive:morning_briefing',
request_id: 'req-mb',
full_response: "good morning! here's your briefing",
});
});
// The active conversation must be left untouched; delivery goes to a
// dedicated new thread.
await waitFor(() => expect(threadApi.createNewThread).toHaveBeenCalledTimes(1));
expect(threadApi.appendMessage).toHaveBeenCalledWith(
'fresh-thread',
expect.objectContaining({ content: "good morning! here's your briefing" })
);
expect(threadApi.appendMessage).not.toHaveBeenCalledWith('busy-thread', expect.anything());
});
it('treats a selected thread with unknown metadata as occupied and opens a new thread', async () => {
vi.mocked(threadApi.createNewThread).mockResolvedValue({
id: 'fresh-thread',
title: 'new',
} as never);
vi.mocked(threadApi.getThreads).mockResolvedValue({
threads: [{ id: 'fresh-thread', title: 'new' }] as never,
count: 1,
});
// Only a rehydrated selection is present — the thread list hasn't loaded,
// so its message metadata is unknown. We must NOT assume it is fresh
// (it could already hold a server-side conversation). See #3713.
store.dispatch(setSelectedThread('rehydrated-thread'));
const listeners = renderProvider();
await act(async () => {
listeners.onProactiveMessage?.({
thread_id: 'proactive:morning_briefing',
request_id: 'req-unknown',
full_response: 'briefing',
});
});
await waitFor(() => expect(threadApi.createNewThread).toHaveBeenCalledTimes(1));
expect(threadApi.appendMessage).toHaveBeenCalledWith(
'fresh-thread',
expect.objectContaining({ content: 'briefing' })
);
expect(threadApi.appendMessage).not.toHaveBeenCalledWith(
'rehydrated-thread',
expect.anything()
);
});
it('creates a new thread when no visible thread exists for proactive handoff', async () => {
vi.mocked(threadApi.createNewThread).mockResolvedValue({
id: 'created-thread',