Skip to content

Commit 8f7e91d

Browse files
committed
fix(chat): resolve a new chat's history entry to the chat route on Back, and say when an effort save fails
1 parent d59e923 commit 8f7e91d

6 files changed

Lines changed: 82 additions & 11 deletions

File tree

‎apps/sim/app/o/[organizationId]/home/organization-home.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
useChatResourcePanel,
3636
useResourcePanelController,
3737
} from '@/app/workspace/[workspaceId]/home/hooks/use-resource-panel'
38+
import { useRestoredChatEntry } from '@/app/workspace/[workspaceId]/home/hooks/use-restored-chat-entry'
3839
import { resolveWorkspaceResourceRef } from '@/app/workspace/[workspaceId]/home/resolve-resource-ref'
3940
import { searchFiltersFromParams } from '@/app/workspace/[workspaceId]/home/search-params'
4041
import type {
@@ -66,9 +67,10 @@ export function OrganizationHome(props: OrganizationHomeProps) {
6667
const { organization, searchAccess, canBuild, mothershipAvailable } = useOrganizationContext()
6768
const { data: session } = useSession()
6869
const isClient = useSyncExternalStore(subscribeToClient, clientSnapshot, serverSnapshot)
70+
const isRestoredChatEntry = useRestoredChatEntry(props.chatId)
6971
if (!mothershipAvailable || (!canBuild && !searchAccess.memberScoped)) return null
7072
/** Preferences are browser-persisted and keyed by user; never paint a guessed mode first. */
71-
if (!isClient || !session?.user?.id) return <HomeFallback />
73+
if (isRestoredChatEntry || !isClient || !session?.user?.id) return <HomeFallback />
7274
return (
7375
<OrganizationHomeContent
7476
key={`${session.user.id}:${organization.id}:${props.chatId ?? 'new'}`}

‎apps/sim/app/workspace/[workspaceId]/home/home.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,12 @@ import { persistImportedWorkflow } from '@/lib/workflows/operations/import-expor
2424
import { ChatResourcePanel } from '@/app/workspace/[workspaceId]/home/components/chat-resource-panel'
2525
import { RESOURCE_HEADER_CLASSES } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tab-controls'
2626
import { SuggestedActions } from '@/app/workspace/[workspaceId]/home/components/suggested-actions'
27+
import { HomeFallback } from '@/app/workspace/[workspaceId]/home/home-fallback'
2728
import {
2829
useChatResourcePanel,
2930
useResourcePanelController,
3031
} from '@/app/workspace/[workspaceId]/home/hooks/use-resource-panel'
32+
import { useRestoredChatEntry } from '@/app/workspace/[workspaceId]/home/hooks/use-restored-chat-entry'
3133
import { resolveWorkspaceResourceRef } from '@/app/workspace/[workspaceId]/home/resolve-resource-ref'
3234
import { PermissionAccessBoundary } from '@/ee/access-requests/components/permission-access-boundary'
3335
import { useMarkMothershipChatRead } from '@/hooks/queries/mothership-chats'
@@ -58,6 +60,8 @@ interface HomeProps {
5860
}
5961

6062
export function Home(props: HomeProps) {
63+
const isRestoredChatEntry = useRestoredChatEntry(props.chatId)
64+
if (isRestoredChatEntry) return <HomeFallback />
6165
return (
6266
<PermissionAccessBoundary configKey='hideCopilot'>
6367
<HomeContent {...props} />
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { useEffect, useState } from 'react'
2+
import { usePathname, useRouter, useSearchParams } from 'next/navigation'
3+
4+
const CHAT_PATH = /^\/(?:workspace|o)\/[^/]+\/chat\/[^/]+$/
5+
6+
/**
7+
* Hands a restored new-chat history entry back to the router.
8+
*
9+
* A new chat moves its URL from the home route to `/chat/<id>` in place, through
10+
* `history.replaceState`, so the turn streaming on that surface stays mounted. Next keeps
11+
* the home route's tree in that history entry, so Back or Forward to it mounts the home
12+
* route at the chat's URL. Replacing the entry through the router resolves the chat route
13+
* and stores its tree, so later visits to the entry render the chat directly. Only the URL
14+
* at mount counts: the surface that moved its own URL keeps rendering.
15+
*
16+
* @param chatId - The chat the surface was opened for; the new-chat surface has none.
17+
* @returns Whether this mount is a restored entry; the caller renders its fallback until
18+
* the chat route replaces it.
19+
*/
20+
export function useRestoredChatEntry(chatId: string | undefined): boolean {
21+
const router = useRouter()
22+
const pathname = usePathname()
23+
const searchParams = useSearchParams()
24+
const [restoredChatUrl] = useState(() => {
25+
if (chatId || !CHAT_PATH.test(pathname)) return null
26+
const search = searchParams.toString()
27+
return search ? `${pathname}?${search}` : pathname
28+
})
29+
30+
useEffect(() => {
31+
if (restoredChatUrl) router.replace(restoredChatUrl, { scroll: false })
32+
}, [restoredChatUrl, router])
33+
34+
return restoredChatUrl !== null
35+
}

‎apps/sim/hooks/queries/mothership-chats.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { jsonResponse } from '@sim/testing/helpers/http'
2+
import { emcnMock, emcnMockFns } from '@sim/testing/mocks/emcn.mock'
23
import { reactQueryMock, reactQueryMockFns } from '@sim/testing/mocks/react-query.mock'
34
import { sleep } from '@sim/utils/helpers'
45
import type { MutationObserverOptions } from '@tanstack/react-query'
@@ -15,6 +16,7 @@ vi.mock('@/stores/mothership-queue/store', () => ({
1516
}))
1617

1718
vi.mock('@tanstack/react-query', () => reactQueryMock)
19+
vi.mock('@sim/emcn', () => emcnMock)
1820

1921
vi.mock('@/lib/browser-agent/transport', () => ({
2022
suspendBrowserScope,
@@ -134,5 +136,27 @@ describe('tasks query boundary parsing', () => {
134136
saves[2].resolve(jsonResponse({ success: true }))
135137
await Promise.all(outcomes)
136138
expect(useMothershipEffortStore.getState().chatEfforts['chat-1']?.effort).toBe('low')
139+
expect(emcnMockFns.mockToast.error).not.toHaveBeenCalled()
140+
})
141+
142+
it('tells the user when a failed save rolls their pick back', async () => {
143+
const tanstack =
144+
await vi.importActual<typeof import('@tanstack/react-query')>('@tanstack/react-query')
145+
const observer = new tanstack.MutationObserver(
146+
new tanstack.QueryClient(),
147+
useSetMothershipChatEffort('chat-1') as unknown as MutationObserverOptions<
148+
void,
149+
Error,
150+
MothershipEffort,
151+
{ pick: number }
152+
>
153+
)
154+
vi.mocked(fetch).mockResolvedValueOnce(new Response('save failed', { status: 500 }))
155+
useMothershipEffortStore.getState().reset()
156+
157+
await observer.mutate('xhigh').catch(() => undefined)
158+
159+
expect(useMothershipEffortStore.getState().chatEfforts['chat-1']).toBeUndefined()
160+
expect(emcnMockFns.mockToast.error).toHaveBeenCalledTimes(1)
137161
})
138162
})

‎apps/sim/hooks/queries/mothership-chats.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { toast } from '@sim/emcn'
12
import { toError } from '@sim/utils/errors'
23
import { isRecordLike } from '@sim/utils/object'
34
import {
@@ -641,8 +642,10 @@ function chatEffortMutationOptions(queryClient: QueryClient, chatId: string | un
641642
return { pick: useMothershipEffortStore.getState().setChatEffort(chatId, effort) }
642643
},
643644
onError: (_error, _effort, context) => {
644-
if (chatId && context)
645-
useMothershipEffortStore.getState().dropChatEffort(chatId, context.pick)
645+
if (!chatId || !context) return
646+
if (useMothershipEffortStore.getState().dropChatEffort(chatId, context.pick)) {
647+
toast.error("Couldn't change reasoning effort")
648+
}
646649
},
647650
onSuccess: (_data, effort) => {
648651
queryClient.setQueryData<MothershipChatHistory>(

‎apps/sim/stores/mothership-effort/store.ts‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,11 @@ interface MothershipEffortState {
3030
chatEfforts: Record<string, ChatEffortPick>
3131
/** Records a pick and returns its token for {@link MothershipEffortState.dropChatEffort}. */
3232
setChatEffort: (chatId: string, effort: MothershipEffort) => number
33-
/** Drops a pick whose save failed, unless a newer pick replaced it, even one of the same value. */
34-
dropChatEffort: (chatId: string, pick: number) => void
33+
/**
34+
* Drops a pick whose save failed, unless a newer pick replaced it, even one of the same value.
35+
* Returns whether it dropped the pick, which rolls the chat back to its saved effort.
36+
*/
37+
dropChatEffort: (chatId: string, pick: number) => boolean
3538
/** Moves the new-chat pick onto the chat its first send created. */
3639
adoptNewChatEffort: (chatId: string, effort: MothershipEffort) => void
3740
reset: () => void
@@ -58,7 +61,7 @@ function withModelSelection(
5861
export const useMothershipEffortStore = create<MothershipEffortState>()(
5962
devtools(
6063
persist(
61-
(set) => ({
64+
(set, get) => ({
6265
...initialState,
6366
setFastMode: (fastMode) =>
6467
set((state) => withModelSelection({ ...state.modelSelection, fastMode })),
@@ -70,11 +73,11 @@ export const useMothershipEffortStore = create<MothershipEffortState>()(
7073
set((state) => ({ chatEfforts: { ...state.chatEfforts, [chatId]: { effort, pick } } }))
7174
return pick
7275
},
73-
dropChatEffort: (chatId, pick) =>
74-
set((state) => {
75-
if (state.chatEfforts[chatId]?.pick !== pick) return state
76-
return { chatEfforts: omit(state.chatEfforts, [chatId]) }
77-
}),
76+
dropChatEffort: (chatId, pick) => {
77+
if (get().chatEfforts[chatId]?.pick !== pick) return false
78+
set((state) => ({ chatEfforts: omit(state.chatEfforts, [chatId]) }))
79+
return true
80+
},
7881
adoptNewChatEffort: (chatId, effort) => {
7982
const pick = ++lastChatEffortPick
8083
set((state) => ({

0 commit comments

Comments
 (0)