Skip to content

Commit 9eb580b

Browse files
committed
fix(mothership): drop the new-chat effort when the surface adopts a chat
A first send stopped before admission adopted its chat without moving the pick, so the next new chat on the same Home mount showed and sent it. adoptResolvedChatId now drops the pick when the surface leaves the new chat. The rollback restore is gone: nothing clears the pick while a send is pending, and it overwrote a pick made during the send.
1 parent 3c266bf commit 9eb580b

2 files changed

Lines changed: 125 additions & 56 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx‎

Lines changed: 117 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ import { MothershipHandoffStorage } from '@/lib/core/utils/browser-storage'
9191
import { MOTHERSHIP_STREAM_REPLAY_HEADER } from '@/lib/mothership/constants'
9292
import type { MothershipStreamV1EventEnvelope } from '@/lib/mothership/generated/mothership-stream-v1'
9393
import { getChatResourceSelectionId } from '@/lib/mothership/resources/types'
94+
import { ChatSurfaceProvider } from '@/app/workspace/[workspaceId]/home/components/chat-surface-context'
9495
import { collectCitedMessageSources } from '@/app/workspace/[workspaceId]/home/components/message-content/message-sources'
9596
import { ModelSelector } from '@/app/workspace/[workspaceId]/home/components/user-input/components/model-selector'
9697
import {
@@ -473,6 +474,77 @@ function renderHomeLikeSurface(): {
473474
}
474475
}
475476

477+
/** Holds the chat POST of the first send until the test settles it. */
478+
function holdFirstSend(): PromiseWithResolvers<Response> {
479+
const post = Promise.withResolvers<Response>()
480+
vi.stubGlobal('fetch', (input: RequestInfo | URL, init?: RequestInit) => {
481+
if (String(input) !== '/api/mothership/chat' || init?.method !== 'POST') {
482+
return fetchStub(input, init)
483+
}
484+
state.postBodies.push(JSON.parse(String(init.body)))
485+
return post.promise
486+
})
487+
return post
488+
}
489+
490+
/**
491+
* Mounts a chatless surface shaped like `home.tsx`, with real composers: the empty-state one
492+
* swaps for the chat view's once messages show, and the chat view's names the resolved chat.
493+
*/
494+
function renderComposerSwap(): {
495+
container: HTMLElement
496+
getResult: () => ReturnType<typeof useChat>
497+
shownEffort: () => string | null | undefined
498+
visit: (pathname: string) => void
499+
} {
500+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
501+
useMothershipEffortStore.getState().reset()
502+
queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } })
503+
const container = document.createElement('div')
504+
const root = createRoot(container)
505+
mountedRoots.push(root)
506+
let result: ReturnType<typeof useChat> | undefined
507+
508+
function HomeLike() {
509+
result = useChat('ws-1', undefined)
510+
return result.messages.length > 0 ? (
511+
<section key='chat'>
512+
<ChatSurfaceProvider chatId={result.resolvedChatId}>
513+
<ModelSelector />
514+
</ChatSurfaceProvider>
515+
</section>
516+
) : (
517+
<main key='empty'>
518+
<ModelSelector />
519+
</main>
520+
)
521+
}
522+
523+
const render = () =>
524+
act(() => {
525+
root.render(
526+
<QueryClientProvider client={queryClient}>
527+
<HomeLike />
528+
</QueryClientProvider>
529+
)
530+
})
531+
render()
532+
533+
return {
534+
container,
535+
getResult: () => {
536+
if (result === undefined) throw new Error('Hook result is not ready')
537+
return result
538+
},
539+
shownEffort: () =>
540+
container.querySelector('[aria-label="Reasoning effort"]')?.getAttribute('aria-description'),
541+
visit: (pathname) => {
542+
mockUsePathname.mockReturnValue(pathname)
543+
render()
544+
},
545+
}
546+
}
547+
476548
/**
477549
* Mounts the hook under StrictMode with a handoff already in storage, mirroring
478550
* `home.tsx`'s consume-and-auto-send effect. This is the production-shaped
@@ -4703,61 +4775,63 @@ describe('useChat remount send recovery', () => {
47034775
})
47044776

47054777
it('keeps the new-chat effort across the composer swap of a first send that fails', async () => {
4706-
useMothershipEffortStore.getState().reset()
4707-
const post = Promise.withResolvers<Response>()
4708-
vi.stubGlobal('fetch', (input: RequestInfo | URL, init?: RequestInit) => {
4709-
if (String(input) !== '/api/mothership/chat' || init?.method !== 'POST') {
4710-
return fetchStub(input, init)
4711-
}
4712-
state.postBodies.push(JSON.parse(String(init.body)))
4713-
return post.promise
4714-
})
4715-
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
4716-
queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } })
4717-
const container = document.createElement('div')
4718-
const root = createRoot(container)
4719-
mountedRoots.push(root)
4720-
let result: ReturnType<typeof useChat> | undefined
4721-
/** Like home.tsx: the empty-state composer swaps for the chat view's once messages show. */
4722-
function HomeLike() {
4723-
result = useChat('ws-1', undefined)
4724-
return result.messages.length > 0 ? (
4725-
<section key='chat'>
4726-
<ModelSelector />
4727-
</section>
4728-
) : (
4729-
<main key='empty'>
4730-
<ModelSelector />
4731-
</main>
4732-
)
4733-
}
4734-
act(() => {
4735-
root.render(
4736-
<QueryClientProvider client={queryClient}>
4737-
<HomeLike />
4738-
</QueryClientProvider>
4739-
)
4740-
})
4741-
const shownEffort = () =>
4742-
container.querySelector('[aria-label="Reasoning effort"]')?.getAttribute('aria-description')
4778+
const post = holdFirstSend()
4779+
const surface = renderComposerSwap()
47434780
act(() => useMothershipEffortStore.getState().setNewChatEffort('low'))
4744-
expect(shownEffort()).toBe('Low')
4781+
expect(surface.shownEffort()).toBe('Low')
47454782

47464783
await act(async () => {
4747-
void result?.sendMessage('Plan the launch')
4784+
void surface.getResult().sendMessage('Plan the launch')
47484785
})
47494786
await waitFor(() => state.postBodies.length === 1)
47504787
expect(state.postBodies[0].effort).toBe('low')
4751-
expect(container.querySelector('section')).not.toBeNull()
4752-
expect(shownEffort()).toBe('Low')
4788+
expect(surface.container.querySelector('section')).not.toBeNull()
4789+
expect(surface.shownEffort()).toBe('Low')
47534790

47544791
await act(async () => {
47554792
post.reject(new TypeError('Failed to fetch'))
47564793
})
4757-
await waitFor(() => container.querySelector('main') !== null)
4794+
await waitFor(() => surface.container.querySelector('main') !== null)
47584795

47594796
expect(useMothershipEffortStore.getState().newChatEffort).toBe('low')
4760-
expect(shownEffort()).toBe('Low')
4797+
expect(surface.shownEffort()).toBe('Low')
4798+
})
4799+
4800+
it('keeps a new-chat effort picked while the first send is pending when that send fails', async () => {
4801+
const post = holdFirstSend()
4802+
const surface = renderComposerSwap()
4803+
act(() => useMothershipEffortStore.getState().setNewChatEffort('low'))
4804+
await act(async () => {
4805+
void surface.getResult().sendMessage('Plan the launch')
4806+
})
4807+
await waitFor(() => state.postBodies.length === 1)
4808+
act(() => useMothershipEffortStore.getState().setNewChatEffort('medium'))
4809+
4810+
await act(async () => {
4811+
post.reject(new TypeError('Failed to fetch'))
4812+
})
4813+
await waitFor(() => surface.container.querySelector('main') !== null)
4814+
4815+
expect(surface.shownEffort()).toBe('Medium')
4816+
})
4817+
4818+
it('starts the next new chat at the default after a first send stopped before admission', async () => {
4819+
const surface = renderComposerSwap()
4820+
act(() => useMothershipEffortStore.getState().setNewChatEffort('low'))
4821+
await act(async () => {
4822+
void surface.getResult().sendMessage('Plan the launch')
4823+
})
4824+
await waitFor(() => state.postBodies.length === 1)
4825+
await act(async () => {
4826+
await surface.getResult().stopGeneration()
4827+
})
4828+
await waitFor(() => surface.getResult().resolvedChatId === DEDUPED_CHAT_ID)
4829+
4830+
surface.visit(`/workspace/ws-1/chat/${DEDUPED_CHAT_ID}`)
4831+
surface.visit('/workspace/ws-1/home')
4832+
await waitFor(() => surface.container.querySelector('main') !== null)
4833+
4834+
expect(surface.shownEffort()).toBe('High')
47614835
})
47624836

47634837
it.each(['leaves the page', 'opens another chat'] as const)(

‎apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,7 +1001,7 @@ export function useChat(
10011001
new Set())
10021002
const streamReaderRef = useRef<ReadableStreamDefaultReader<Uint8Array> | null>(null)
10031003
const chatIdRef = useRef<string | undefined>(initialChatId)
1004-
/** Cleared on unmount, so a late rollback cannot hand a pick to a surface the user left. */
1004+
/** Cleared on unmount, so late async work cannot act on a surface the user left. */
10051005
const surfaceMountedRef = useRef(true)
10061006
useEffect(() => {
10071007
surfaceMountedRef.current = true
@@ -1010,8 +1010,9 @@ export function useChat(
10101010
}
10111011
}, [])
10121012
/* The new-chat effort pick belongs to this surface, not to one composer: it outlives the swap
1013-
from the empty-state composer to the chat view during a first send, and drops only when the
1014-
surface leaves the new chat. */
1013+
from the empty-state composer to the chat view during a first send, so a withdrawn send
1014+
leaves it in place. It drops when the surface unmounts or switches chats, and when it adopts
1015+
a chat (`adoptResolvedChatId`). */
10151016
useEffect(() => {
10161017
if (initialChatId) return
10171018
return () => useMothershipEffortStore.getState().setNewChatEffort(null)
@@ -1285,6 +1286,10 @@ export function useChat(
12851286
const resolvedDesktopScopeId = desktopChatScopeId(scopeKey, chatId)
12861287
if (wasPending) {
12871288
useChatPanelStore.getState().migrate(pendingDesktopScopeId, resolvedDesktopScopeId)
1289+
// Leaving the new chat. An admitted send has already moved the pick onto its chat; any
1290+
// other way out (a Stop before admission, a recovered handoff) must not carry it into
1291+
// the next new chat.
1292+
useMothershipEffortStore.getState().setNewChatEffort(null)
12881293
}
12891294
const activeActivityTracker = resourceActivityTrackerRef.current
12901295
if (activeActivityTracker?.generation === streamGenRef.current) {
@@ -3743,16 +3748,6 @@ export function useChat(
37433748
}
37443749

37453750
const rollbackOptimisticSend = () => {
3746-
// A withdrawn first send hands its pick back to the new-chat composer for the retry,
3747-
// only while that surface is still open on the new chat.
3748-
if (
3749-
!requestChatId &&
3750-
effortChoice &&
3751-
surfaceMountedRef.current &&
3752-
!chatIdRef.current &&
3753-
!selectedChatIdRef.current
3754-
)
3755-
useMothershipEffortStore.getState().setNewChatEffort(effortChoice)
37563751
if (requestChatId) {
37573752
upsertChatHistory(requestChatId, (current) => ({
37583753
...current,

0 commit comments

Comments
 (0)