Skip to content

Commit e93524a

Browse files
Bill LeoutsakosBill Leoutsakos
Bill Leoutsakos
authored and
Bill Leoutsakos
committed
fix(mothership): protect resource drafts across navigation
1 parent 6225308 commit e93524a

7 files changed

Lines changed: 204 additions & 44 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,7 @@ export const ResourceContent = memo(function ResourceContent({
362362
workspaceId={workspaceId}
363363
serverId={resource.id}
364364
onBack={() => onNotFound?.(resource.id)}
365+
onDirtyChange={handleDirtyChange}
365366
/>
366367
</EmbeddedSettingsShell>
367368
)

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx‎

Lines changed: 4 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import {
2222
import { Columns3, Eye, Pencil } from '@sim/emcn/icons'
2323
import { sendBrowserPanelAction } from '@/lib/browser-agent/transport'
2424
import { SIM_RESOURCE_DRAG_TYPE, SIM_RESOURCES_DRAG_TYPE } from '@/lib/copilot/resource-types'
25-
import { isEphemeralResource } from '@/lib/copilot/resources/types'
2625
import { openTerminal } from '@/lib/terminal/transport'
2726
import type { PreviewMode } from '@/app/workspace/[workspaceId]/files/components/file-viewer'
2827
import { useMothershipResources } from '@/app/workspace/[workspaceId]/home/components/mothership-resources-context'
@@ -41,11 +40,6 @@ import { useCustomTools } from '@/hooks/queries/custom-tools'
4140
import { useFolders } from '@/hooks/queries/folders'
4241
import { useKnowledgeBasesQuery } from '@/hooks/queries/kb/knowledge'
4342
import { useMcpServers } from '@/hooks/queries/mcp'
44-
import {
45-
useAddChatResource,
46-
useRemoveChatResource,
47-
useReorderChatResources,
48-
} from '@/hooks/queries/mothership-chats'
4943
import { useSkills } from '@/hooks/queries/skills'
5044
import { useTablesList } from '@/hooks/queries/tables'
5145
import { useWorkflows } from '@/hooks/queries/workflows'
@@ -276,10 +270,6 @@ export function ResourceTabs({
276270
requestResourceTransition,
277271
} = useMothershipResources()
278272

279-
const addResource = useAddChatResource(chatId)
280-
const removeResource = useRemoveChatResource(chatId)
281-
const reorderResources = useReorderChatResources(chatId)
282-
283273
const [selectedIds, setSelectedIds] = useState<Set<string>>(new Set())
284274
const anchorIdRef = useRef<string | null>(null)
285275
const prevChatIdRef = useRef(chatId)
@@ -324,19 +314,9 @@ export function ResourceTabs({
324314

325315
const handleAdd = useCallback(
326316
(resource: MothershipResource) => {
327-
requestResourceTransition(() => {
328-
// Opening a resource before the first message is sent is allowed: there
329-
// is simply no chat to attach it to yet. `onAddResource` queues it and
330-
// persists once the chat exists, so only the server call is conditional.
331-
// Synthetic result/preview panels are in-memory only either way.
332-
if (chatId && !isEphemeralResource(resource)) {
333-
addResource.mutate({ chatId, resource })
334-
}
335-
onAddResource(resource)
336-
})
317+
requestResourceTransition(() => onAddResource(resource))
337318
},
338-
// eslint-disable-next-line react-hooks/exhaustive-deps
339-
[chatId, onAddResource, requestResourceTransition]
319+
[onAddResource, requestResourceTransition]
340320
)
341321

342322
const handleOpenExisting = useCallback(
@@ -436,21 +416,11 @@ export function ResourceTabs({
436416
if (anchorIdRef.current && removedIds.has(anchorIdRef.current)) {
437417
anchorIdRef.current = null
438418
}
439-
// Mirrors `handleAdd`: a resource opened while composing the first prompt
440-
// has to be closable before there is a chat to attach it to. Only the
441-
// server call is conditional — the local removal above also drops the
442-
// queued write, so nothing resurrects it once the chat exists.
443-
if (!chatId) return
444-
for (const r of targets) {
445-
if (isEphemeralResource(r)) continue
446-
removeResource.mutate({ chatId, resourceType: r.type, resourceId: r.id })
447-
}
448419
}
449420
if (targets.some((target) => target.id === activeId)) requestResourceTransition(close)
450421
else close()
451422
},
452-
// eslint-disable-next-line react-hooks/exhaustive-deps
453-
[activeId, chatId, onRemoveResource, requestResourceTransition, resources, selectedIds]
423+
[activeId, onRemoveResource, requestResourceTransition, resources, selectedIds]
454424
)
455425

456426
const handleTabDragStart = useCallback(
@@ -497,15 +467,8 @@ export function ResourceTabs({
497467
const [moved] = reordered.splice(fromIndex, 1)
498468
reordered.splice(targetIndex, 0, moved)
499469
onReorderResources(reordered)
500-
if (chatId) {
501-
const persistable = reordered.filter((r) => !isEphemeralResource(r))
502-
if (persistable.length > 0) {
503-
reorderResources.mutate({ chatId, resources: persistable })
504-
}
505-
}
506470
},
507-
// eslint-disable-next-line react-hooks/exhaustive-deps
508-
[chatId, resources, onReorderResources]
471+
[resources, onReorderResources]
509472
)
510473

511474
const previewToggle =

‎apps/sim/app/workspace/[workspaceId]/home/hooks/use-resource-transition-guard.test.tsx‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import { act } from 'react'
66
import { createRoot, type Root } from 'react-dom/client'
7-
import { describe, expect, it, vi } from 'vitest'
7+
import { afterEach, describe, expect, it, vi } from 'vitest'
88
import { useResourceTransitionGuard } from '@/app/workspace/[workspaceId]/home/hooks/use-resource-transition-guard'
99

1010
function renderGuard() {
@@ -22,6 +22,10 @@ function renderGuard() {
2222
}
2323

2424
describe('useResourceTransitionGuard', () => {
25+
afterEach(() => {
26+
document.body.replaceChildren()
27+
})
28+
2529
it('keeps a dirty draft when cancelled and performs the complete deferred action on discard', () => {
2630
const selectAnotherTab = vi.fn()
2731
const closeSelectedTabs = vi.fn()
@@ -62,4 +66,37 @@ describe('useResourceTransitionGuard', () => {
6266
expect(guard.result().showDiscardConfirmation).toBe(false)
6367
guard.unmount()
6468
})
69+
70+
it('guards browser unload and replays app-link navigation only after discard', () => {
71+
const guard = renderGuard()
72+
const link = document.createElement('a')
73+
link.href = '/workspace/ws-1/chat/chat-2'
74+
link.textContent = 'Another chat'
75+
const navigate = vi.fn((event: MouseEvent) => event.preventDefault())
76+
link.addEventListener('click', navigate)
77+
document.body.appendChild(link)
78+
79+
act(() => guard.result().reportResourceDirty('custom-tool-1', true))
80+
81+
const beforeUnload = new Event('beforeunload', { cancelable: true })
82+
window.dispatchEvent(beforeUnload)
83+
expect(beforeUnload.defaultPrevented).toBe(true)
84+
85+
act(() => link.click())
86+
expect(navigate).not.toHaveBeenCalled()
87+
expect(guard.result().showDiscardConfirmation).toBe(true)
88+
89+
act(() => guard.result().dismissDiscardConfirmation())
90+
expect(navigate).not.toHaveBeenCalled()
91+
92+
act(() => link.click())
93+
act(() => guard.result().confirmDiscard())
94+
expect(navigate).toHaveBeenCalledOnce()
95+
expect(guard.result().showDiscardConfirmation).toBe(false)
96+
97+
const cleanBeforeUnload = new Event('beforeunload', { cancelable: true })
98+
window.dispatchEvent(cleanBeforeUnload)
99+
expect(cleanBeforeUnload.defaultPrevented).toBe(false)
100+
guard.unmount()
101+
})
65102
})

‎apps/sim/app/workspace/[workspaceId]/home/hooks/use-resource-transition-guard.ts‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useCallback, useRef, useState } from 'react'
1+
import { useCallback, useEffect, useRef, useState } from 'react'
22

33
interface ResourceTransitionGuard {
44
showDiscardConfirmation: boolean
@@ -80,6 +80,64 @@ export function useResourceTransitionGuard(): ResourceTransitionGuard {
8080
setShowDiscardConfirmation(false)
8181
}, [])
8282

83+
useEffect(() => {
84+
const handleBeforeUnload = (event: BeforeUnloadEvent) => {
85+
if (!dirtyResourceIdRef.current) return
86+
event.preventDefault()
87+
}
88+
89+
/**
90+
* Next.js handles links before a history listener can block them. Capture
91+
* same-window app links first, then replay the original click after the
92+
* user confirms so the link keeps its own routing and selection behavior.
93+
*/
94+
const handleDocumentClick = (event: MouseEvent) => {
95+
if (
96+
!dirtyResourceIdRef.current ||
97+
event.defaultPrevented ||
98+
event.button !== 0 ||
99+
event.metaKey ||
100+
event.ctrlKey ||
101+
event.shiftKey ||
102+
event.altKey ||
103+
!(event.target instanceof Element)
104+
) {
105+
return
106+
}
107+
108+
const anchor = event.target.closest<HTMLAnchorElement>('a[href]')
109+
if (
110+
!anchor ||
111+
anchor.hasAttribute('download') ||
112+
(anchor.target && anchor.target !== '_self')
113+
) {
114+
return
115+
}
116+
117+
const destination = new URL(anchor.href, window.location.href)
118+
const current = new URL(window.location.href)
119+
if (
120+
destination.origin !== current.origin ||
121+
(destination.pathname === current.pathname &&
122+
destination.search === current.search &&
123+
destination.hash === current.hash)
124+
) {
125+
return
126+
}
127+
128+
event.preventDefault()
129+
event.stopPropagation()
130+
requestResourceTransition(() => anchor.click())
131+
}
132+
133+
window.addEventListener('beforeunload', handleBeforeUnload)
134+
document.addEventListener('click', handleDocumentClick, true)
135+
return () => {
136+
window.removeEventListener('beforeunload', handleBeforeUnload)
137+
document.removeEventListener('click', handleDocumentClick, true)
138+
}
139+
}, [requestResourceTransition])
140+
83141
return {
84142
showDiscardConfirmation,
85143
reportResourceDirty,

‎apps/sim/app/workspace/[workspaceId]/settings/components/mcp/components/mcp-server-form-modal/mcp-server-form-modal.test.tsx‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
5+
import { act } from 'react'
6+
import { createRoot, type Root } from 'react-dom/client'
7+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
8+
9+
const { clearTestResult } = vi.hoisted(() => ({ clearTestResult: vi.fn() }))
10+
11+
vi.mock('@/hooks/queries/mcp', () => ({
12+
useMcpServerTest: () => ({
13+
testResult: null,
14+
isTestingConnection: false,
15+
testConnection: vi.fn(),
16+
clearTestResult,
17+
}),
18+
}))
19+
20+
import { McpServerFormModal } from '@/app/workspace/[workspaceId]/settings/components/mcp/components/mcp-server-form-modal/mcp-server-form-modal'
21+
22+
describe('McpServerFormModal dirty state', () => {
23+
let container: HTMLDivElement
24+
let root: Root
25+
26+
beforeEach(() => {
27+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
28+
container = document.createElement('div')
29+
document.body.appendChild(container)
30+
root = createRoot(container)
31+
})
32+
33+
afterEach(() => {
34+
act(() => root.unmount())
35+
container.remove()
36+
clearTestResult.mockReset()
37+
})
38+
39+
it('reports edits only while the embedded edit form is open', () => {
40+
const onDirtyChange = vi.fn()
41+
const renderModal = (open: boolean) => {
42+
act(() => {
43+
root.render(
44+
<McpServerFormModal
45+
open={open}
46+
onOpenChange={vi.fn()}
47+
onDirtyChange={onDirtyChange}
48+
mode='edit'
49+
initialData={{
50+
name: 'DeepWiki',
51+
transport: 'streamable-http',
52+
url: 'https://mcp.deepwiki.com/mcp',
53+
timeout: 30000,
54+
headers: [{ key: '', value: '' }],
55+
}}
56+
onSubmit={vi.fn()}
57+
workspaceId='workspace-1'
58+
allowedMcpDomains={null}
59+
/>
60+
)
61+
})
62+
}
63+
64+
renderModal(true)
65+
expect(onDirtyChange).toHaveBeenLastCalledWith(false)
66+
67+
const nameInput = document.querySelector<HTMLInputElement>(
68+
'input[placeholder="e.g., My MCP Server"]'
69+
)
70+
if (!nameInput) throw new Error('MCP server name input was not rendered')
71+
72+
act(() => {
73+
const valueSetter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')?.set
74+
valueSetter?.call(nameInput, 'Renamed server')
75+
nameInput.dispatchEvent(new Event('input', { bubbles: true }))
76+
})
77+
expect(onDirtyChange).toHaveBeenLastCalledWith(true)
78+
79+
renderModal(false)
80+
expect(onDirtyChange).toHaveBeenLastCalledWith(false)
81+
})
82+
})

‎apps/sim/app/workspace/[workspaceId]/settings/components/mcp/components/mcp-server-form-modal/mcp-server-form-modal.tsx‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ export interface McpServerFormConfig {
5757
export interface McpServerFormModalProps {
5858
open: boolean
5959
onOpenChange: (open: boolean) => void
60+
/** Reports unsaved edits when a host surface owns navigation protection. */
61+
onDirtyChange?: (dirty: boolean) => void
6062
mode: 'add' | 'edit'
6163
initialData?: McpServerFormData
6264
onSubmit: (config: McpServerFormConfig) => Promise<void>
@@ -308,6 +310,7 @@ function updateHeadersArray(
308310
export function McpServerFormModal({
309311
open,
310312
onOpenChange,
313+
onDirtyChange,
311314
mode,
312315
initialData,
313316
onSubmit,
@@ -452,6 +455,14 @@ export function McpServerFormModal({
452455
}
453456
const hasChanges = computeHasChanges()
454457

458+
useEffect(() => {
459+
onDirtyChange?.(open && hasChanges)
460+
}, [hasChanges, onDirtyChange, open])
461+
462+
useEffect(() => {
463+
return () => onDirtyChange?.(false)
464+
}, [onDirtyChange])
465+
455466
const parseJsonConfig = (
456467
json: string
457468
): { name: string; url: string; headers: Record<string, string> } | null => {

‎apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,16 @@ interface MCPProps {
177177
workspaceId?: string
178178
serverId?: string
179179
onBack?: () => void
180+
/** Reports edits in the server form to an embedding navigation guard. */
181+
onDirtyChange?: (dirty: boolean) => void
180182
}
181183

182-
export function MCP({ workspaceId: explicitWorkspaceId, serverId, onBack }: MCPProps = {}) {
184+
export function MCP({
185+
workspaceId: explicitWorkspaceId,
186+
serverId,
187+
onBack,
188+
onDirtyChange,
189+
}: MCPProps = {}) {
183190
const params = useParams()
184191
const workspaceId = explicitWorkspaceId ?? (params.workspaceId as string)
185192
const workspacePermissions = useUserPermissionsContext()
@@ -665,6 +672,7 @@ export function MCP({ workspaceId: explicitWorkspaceId, serverId, onBack }: MCPP
665672
onOpenChange={(open) => {
666673
if (!open) setEditingServerId(null)
667674
}}
675+
onDirtyChange={onDirtyChange}
668676
mode='edit'
669677
initialData={editInitialData}
670678
onSubmit={async (config) => {

0 commit comments

Comments
 (0)