Skip to content

Commit c787bca

Browse files
ryaneggzryaneggz
authored
Merge pull request #857 from ruska-ai/fix/855-files-map-not-persisted-across-threads
FROM fix/855-files-map-not-persisted-across-threads TO development
2 parents adb94d8 + c3ac004 commit c787bca

7 files changed

Lines changed: 362 additions & 15 deletions

File tree

‎frontend/src/components/buttons/NewThreadButton.test.tsx‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ describe("NewThreadButton", () => {
5555
messages: [{ id: "msg-1" }],
5656
clearMessages: vi.fn(),
5757
metadata: { assistant_id: "assistant-1" },
58+
abortQuery: vi.fn(),
5859
resetToDefault: vi.fn(),
5960
loadPersistentContextFiles: vi.fn(),
6061
clearThreadScopedFiles: vi.fn(),
@@ -68,12 +69,46 @@ describe("NewThreadButton", () => {
6869
fireEvent.click(screen.getByTitle("New Chat"));
6970

7071
const context = mockUseChatContext.mock.results[0].value;
72+
expect(context.abortQuery).toHaveBeenCalledTimes(1);
7173
expect(context.clearMessages).toHaveBeenCalledTimes(1);
7274
expect(context.clearThreadScopedFiles).toHaveBeenCalledTimes(1);
7375
expect(context.clearBackendSyncFiles).toHaveBeenCalledTimes(1);
7476
expect(context.resetToDefault).toHaveBeenCalledTimes(1);
7577
expect(context.loadPersistentContextFiles).toHaveBeenCalledTimes(1);
76-
expect(mockNavigate).toHaveBeenCalledWith("/chat");
78+
expect(mockNavigate).toHaveBeenCalledWith("/chat", {
79+
state: { staleThreadId: undefined },
80+
});
81+
});
82+
83+
it("aborts the active stream before clearing messages", () => {
84+
mockUseChatContext.mockReturnValue({
85+
messages: [{ id: "msg-1" }],
86+
clearMessages: vi.fn(),
87+
metadata: { thread_id: "thread-1", assistant_id: "assistant-1" },
88+
abortQuery: vi.fn(),
89+
resetToDefault: vi.fn(),
90+
loadPersistentContextFiles: vi.fn(),
91+
clearThreadScopedFiles: vi.fn(),
92+
clearBackendSyncFiles: vi.fn(),
93+
});
94+
95+
renderAt("/thread/thread-1");
96+
97+
fireEvent.click(screen.getByTitle("New Chat"));
98+
99+
const context = mockUseChatContext.mock.results[0].value;
100+
expect(context.abortQuery).toHaveBeenCalledTimes(1);
101+
expect(context.clearMessages).toHaveBeenCalledTimes(1);
102+
103+
// Verify abort was called before clearMessages
104+
const abortOrder = context.abortQuery.mock.invocationCallOrder[0];
105+
const clearOrder = context.clearMessages.mock.invocationCallOrder[0];
106+
expect(abortOrder).toBeLessThan(clearOrder);
107+
108+
// Verify staleThreadId is passed
109+
expect(mockNavigate).toHaveBeenCalledWith("/chat", {
110+
state: { staleThreadId: "thread-1" },
111+
});
77112
});
78113

79114
it("navigates assistant thread resets back to the assistant route", () => {
@@ -89,6 +124,7 @@ describe("NewThreadButton", () => {
89124
messages: [],
90125
clearMessages: vi.fn(),
91126
metadata: {},
127+
abortQuery: vi.fn(),
92128
resetToDefault: vi.fn(),
93129
loadPersistentContextFiles: vi.fn(),
94130
clearThreadScopedFiles: vi.fn(),
@@ -98,4 +134,25 @@ describe("NewThreadButton", () => {
98134
const { container } = renderAt("/chat");
99135
expect(container).toBeEmptyDOMElement();
100136
});
137+
138+
it("passes staleThreadId when falling back from a project route to /chat", () => {
139+
mockUseChatContext.mockReturnValue({
140+
messages: [{ id: "msg-1" }],
141+
clearMessages: vi.fn(),
142+
metadata: { thread_id: "proj-thread-1" },
143+
abortQuery: vi.fn(),
144+
resetToDefault: vi.fn(),
145+
loadPersistentContextFiles: vi.fn(),
146+
clearThreadScopedFiles: vi.fn(),
147+
clearBackendSyncFiles: vi.fn(),
148+
});
149+
150+
renderAt("/p/project-1");
151+
152+
fireEvent.click(screen.getByTitle("New Chat"));
153+
154+
expect(mockNavigate).toHaveBeenCalledWith("/chat", {
155+
state: { staleThreadId: "proj-thread-1" },
156+
});
157+
});
101158
});

‎frontend/src/components/buttons/NewThreadButton.tsx‎

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { useChatContext } from "@/context/ChatContext";
2+
import { removeActiveStreamRecovery } from "@/lib/utils/activeStreamRecovery";
23
import { Button } from "../ui/button";
34
import { Plus } from "lucide-react";
45
import { useNavigate, useLocation } from "react-router-dom";
@@ -8,6 +9,7 @@ function NewThreadButton() {
89
messages,
910
clearMessages,
1011
metadata,
12+
abortQuery,
1113
resetToDefault,
1214
loadPersistentContextFiles,
1315
clearThreadScopedFiles,
@@ -20,28 +22,44 @@ function NewThreadButton() {
2022
if (e.ctrlKey) {
2123
window.open(window.location.href, "_blank");
2224
} else {
25+
// Abort active stream BEFORE clearing to prevent race condition
26+
// where SSE handler re-populates stale thread_id after clear
27+
abortQuery();
28+
if (metadata?.thread_id) {
29+
removeActiveStreamRecovery(metadata.thread_id);
30+
}
31+
32+
// Capture navigation values before clearing metadata
33+
const pathname = location.pathname;
34+
const threadId = metadata?.thread_id;
35+
const assistantId = metadata?.assistant_id;
36+
const projectId = metadata?.project_id;
37+
38+
// Clear state eagerly. React may batch these updates, so the
39+
// destination page also guards against stale state via the
40+
// staleThreadId navigation state.
2341
clearMessages();
2442
clearThreadScopedFiles?.();
2543
clearBackendSyncFiles?.();
2644
// Reset model to user's default for new conversations
2745
resetToDefault?.();
2846
loadPersistentContextFiles?.();
29-
const pathname = location.pathname;
3047

31-
// Handle thread routes
48+
// Navigate — pass staleThreadId so the destination page can
49+
// force-clear any lingering messages that survived batching.
3250
if (pathname.startsWith("/thread/")) {
33-
// On /thread/:threadId - go back to chat
34-
navigate("/chat");
51+
navigate("/chat", {
52+
state: { staleThreadId: threadId },
53+
});
3554
} else if (pathname.startsWith("/assistant/")) {
36-
// On /assistant/:agentId/thread/:threadId - go to agent thread page
37-
navigate(`/assistant/${metadata?.assistant_id}`);
55+
navigate(`/assistant/${assistantId}`);
3856
} else if (pathname.match(/^\/p\/[^/]+\/t\//)) {
39-
// On /p/:projectId/t/:threadId - extract projectId and go to project page
40-
const projectId = pathname.split("/")[2];
41-
navigate(`/p/${projectId}`);
42-
} else if (pathname.startsWith("/p/") && !metadata?.project_id) {
43-
// On project page but no project_id in metadata - go to chat
44-
navigate("/chat");
57+
const pathProjectId = pathname.split("/")[2];
58+
navigate(`/p/${pathProjectId}`);
59+
} else if (pathname.startsWith("/p/") && !projectId) {
60+
navigate("/chat", {
61+
state: { staleThreadId: threadId },
62+
});
4563
}
4664
}
4765
};

‎frontend/src/hooks/useChat.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,10 @@ export default function useChat(): ChatContextType {
261261
const threadId = distributedStream?.getThreadId() ?? metadata?.thread_id;
262262

263263
stream.onEvent((event: StreamEvent) => {
264+
// Bail out if the stream was aborted (prevents stale SSE events from
265+
// re-populating messages/metadata after clearMessages)
266+
if (controller.signal.aborted) return;
267+
264268
if (distributedStream) {
265269
updateDistributedRecoveryCursor(distributedStream, options.route);
266270
}

‎frontend/src/hooks/useInitialThreadRedirect.test.tsx‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { renderHook } from "@testing-library/react";
33
import useInitialThreadRedirect from "./useInitialThreadRedirect";
44

55
const mockNavigate = vi.fn();
6+
const mockUseLocation = vi.fn();
67

78
vi.mock("react-router-dom", async () => {
89
const actual =
@@ -13,12 +14,15 @@ vi.mock("react-router-dom", async () => {
1314
return {
1415
...actual,
1516
useNavigate: () => mockNavigate,
17+
useLocation: () => mockUseLocation(),
1618
};
1719
});
1820

1921
describe("useInitialThreadRedirect", () => {
2022
beforeEach(() => {
2123
mockNavigate.mockReset();
24+
mockUseLocation.mockReset();
25+
mockUseLocation.mockReturnValue({ state: null });
2226
});
2327

2428
it("does not navigate when threadId is missing", () => {
@@ -99,4 +103,83 @@ describe("useInitialThreadRedirect", () => {
99103
replace: true,
100104
});
101105
});
106+
107+
it("suppresses the initial redirect when threadId matches staleThreadId", () => {
108+
mockUseLocation.mockReturnValue({
109+
state: { staleThreadId: "thread-123" },
110+
});
111+
112+
renderHook(() =>
113+
useInitialThreadRedirect({
114+
threadId: "thread-123",
115+
hasMessages: true,
116+
}),
117+
);
118+
119+
expect(mockNavigate).not.toHaveBeenCalled();
120+
});
121+
122+
it("redirects when threadId differs from staleThreadId", () => {
123+
mockUseLocation.mockReturnValue({
124+
state: { staleThreadId: "stale-thread" },
125+
});
126+
127+
const { rerender } = renderHook(
128+
({
129+
threadId,
130+
hasMessages,
131+
}: {
132+
threadId?: string;
133+
hasMessages: boolean;
134+
}) => useInitialThreadRedirect({ threadId, hasMessages }),
135+
{
136+
initialProps: {
137+
threadId: "stale-thread" as string | undefined,
138+
hasMessages: true,
139+
},
140+
},
141+
);
142+
143+
// First render: suppressed (threadId matches staleThreadId)
144+
expect(mockNavigate).not.toHaveBeenCalled();
145+
146+
// New thread arrives — different from staleThreadId, should redirect
147+
rerender({ threadId: "new-thread", hasMessages: true });
148+
expect(mockNavigate).toHaveBeenCalledTimes(1);
149+
expect(mockNavigate).toHaveBeenCalledWith("/thread/new-thread", {
150+
replace: true,
151+
});
152+
});
153+
154+
it("re-enables redirects after the stale thread clears and a new thread arrives", () => {
155+
mockUseLocation.mockReturnValue({
156+
state: { staleThreadId: "thread-123" },
157+
});
158+
159+
const { rerender } = renderHook(
160+
({
161+
threadId,
162+
hasMessages,
163+
}: {
164+
threadId?: string;
165+
hasMessages: boolean;
166+
}) => useInitialThreadRedirect({ threadId, hasMessages }),
167+
{
168+
initialProps: {
169+
threadId: "thread-123" as string | undefined,
170+
hasMessages: true,
171+
},
172+
},
173+
);
174+
175+
expect(mockNavigate).not.toHaveBeenCalled();
176+
177+
rerender({ threadId: undefined, hasMessages: false });
178+
rerender({ threadId: "thread-456", hasMessages: true });
179+
180+
expect(mockNavigate).toHaveBeenCalledTimes(1);
181+
expect(mockNavigate).toHaveBeenCalledWith("/thread/thread-456", {
182+
replace: true,
183+
});
184+
});
102185
});

‎frontend/src/hooks/useInitialThreadRedirect.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useEffect, useRef } from "react";
2-
import { useNavigate } from "react-router-dom";
2+
import { useLocation, useNavigate } from "react-router-dom";
33

44
type UseInitialThreadRedirectOptions = {
55
threadId?: string;
@@ -11,6 +11,10 @@ export default function useInitialThreadRedirect({
1111
hasMessages,
1212
}: UseInitialThreadRedirectOptions): void {
1313
const navigate = useNavigate();
14+
const location = useLocation();
15+
const staleThreadId = (
16+
location.state as { staleThreadId?: string } | null | undefined
17+
)?.staleThreadId;
1418
const lastNavigatedThreadIdRef = useRef<string | null>(null);
1519

1620
useEffect(() => {
@@ -23,11 +27,16 @@ export default function useInitialThreadRedirect({
2327
return;
2428
}
2529

30+
// Skip redirect when the threadId matches the stale one we just left
31+
if (threadId === staleThreadId) {
32+
return;
33+
}
34+
2635
if (lastNavigatedThreadIdRef.current === threadId) {
2736
return;
2837
}
2938

3039
lastNavigatedThreadIdRef.current = threadId;
3140
navigate(`/thread/${threadId}`, { replace: true });
32-
}, [hasMessages, navigate, threadId]);
41+
}, [hasMessages, navigate, staleThreadId, threadId]);
3342
}

‎frontend/src/pages/chat/chat-v2.tsx‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { useEffect } from "react";
2+
import { useLocation } from "react-router-dom";
13
import ChatLayout from "@/layouts/chat-layout-v2";
24
import ChatPanel from "./ChatPanel";
35
import { useAgentContext } from "@/context/AgentContext";
@@ -18,7 +20,22 @@ export function ChatV2Page() {
1820
metadata,
1921
messages,
2022
useModelsEffect,
23+
clearMessages,
2124
} = useChatContext();
25+
const location = useLocation();
26+
const staleThreadId = (
27+
location.state as { staleThreadId?: string } | null | undefined
28+
)?.staleThreadId;
29+
30+
// When navigating here from NewThreadButton with a staleThreadId,
31+
// force-clear any lingering messages/metadata from the previous thread.
32+
// This handles the case where clearMessages() in the button's onClick
33+
// didn't fully flush before the route change.
34+
useEffect(() => {
35+
if (staleThreadId && (messages.length > 0 || metadata?.thread_id)) {
36+
clearMessages();
37+
}
38+
}, [staleThreadId]);
2239

2340
useModelsEffect();
2441
useEffectGetAgents();

0 commit comments

Comments
 (0)