Skip to content

Commit 8dc7054

Browse files
committed
fix(project-files): protect drafts and align file operations
1 parent e5a1288 commit 8dc7054

11 files changed

Lines changed: 229 additions & 39 deletions

File tree

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,13 @@ it.each([
249249
mocks.projects = projects
250250
mocks.projectFiles = projectFiles
251251
vi.spyOn(toast, 'error').mockReturnValue('lookup-error')
252-
const fetch = vi.fn().mockRejectedValue(new Error('Project lookup is disabled'))
253-
vi.stubGlobal('fetch', fetch)
252+
const requests: unknown[] = []
253+
const resources: unknown[] = []
254+
vi.stubGlobal('fetch', async (input: unknown) => {
255+
requests.push(input)
256+
throw new Error('Project lookup is disabled')
257+
})
258+
mocks.addResource.mockImplementation((resource: unknown) => resources.push(resource))
254259
await act(async () => renderHome(<OrganizationHome chatId='chat-a' />))
255260
const selectResource = mocks.renderer.mock.lastCall?.[0].onWorkspaceResourceSelect
256261
if (!selectResource) throw new Error('Chat resource selection is unavailable')
@@ -262,7 +267,7 @@ it.each([
262267
owner: { entityType: 'project', entityId: 'project-a' },
263268
})
264269
)
265-
expect(fetch).not.toHaveBeenCalled()
266-
expect(mocks.addResource).not.toHaveBeenCalled()
270+
expect(requests).toEqual([])
271+
expect(resources).toEqual([])
267272
}
268273
)

‎apps/sim/app/workspace/[workspaceId]/files/components/file-detail/navigation.tsx‎

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
'use client'
22

33
import { createContext, type ReactNode, useCallback, useContext, useRef, useState } from 'react'
4-
import { ChipConfirmModal } from '@sim/emcn'
54
import { useRouter } from 'next/navigation'
5+
import { useSettingsUnsavedGuard } from '@/components/settings/use-settings-unsaved-guard'
66
import type { FileDownloadSource } from '@/lib/uploads/client/download'
77
import type { EditableFileOwner } from '@/lib/workspace-files/ownership'
88

@@ -40,7 +40,6 @@ export function FileNavigationProvider({ owner, fileId, children }: FileNavigati
4040
const router = useRouter()
4141
const [isDirty, updateIsDirty] = useState(false)
4242
const [saveStatus, updateSaveStatus] = useState<SaveStatus>('idle')
43-
const [pendingUrl, setPendingUrl] = useState<string | null>(null)
4443
const setIsDirty = useCallback((dirty: boolean) => {
4544
dirtyRef.current = dirty
4645
updateIsDirty(dirty)
@@ -49,28 +48,25 @@ export function FileNavigationProvider({ owner, fileId, children }: FileNavigati
4948
savingRef.current = status
5049
updateSaveStatus(status)
5150
}, [])
52-
const navigate = useCallback(
53-
(url: string) => {
54-
if (dirtyRef.current) setPendingUrl(url)
55-
else router.push(url)
51+
const { guardBack } = useSettingsUnsavedGuard({
52+
isDirty,
53+
navigationBlocked: saveStatus === 'saving',
54+
onDiscard: () => {
55+
discardRef.current?.()
56+
setIsDirty(false)
57+
setSaveStatus('idle')
5658
},
57-
[router]
59+
})
60+
const navigate = useCallback(
61+
(url: string) => guardBack(() => router.push(url)),
62+
[guardBack, router]
5863
)
5964
const save = useCallback(async () => {
6065
if (saveRef.current && dirtyRef.current && savingRef.current !== 'saving') {
6166
await saveRef.current()
6267
}
6368
}, [])
6469

65-
function discardAndNavigate() {
66-
if (!pendingUrl) return
67-
discardRef.current?.()
68-
setIsDirty(false)
69-
setSaveStatus('idle')
70-
setPendingUrl(null)
71-
router.push(pendingUrl)
72-
}
73-
7470
return (
7571
<FileNavigationContext.Provider
7672
value={{
@@ -88,17 +84,6 @@ export function FileNavigationProvider({ owner, fileId, children }: FileNavigati
8884
}}
8985
>
9086
{children}
91-
<ChipConfirmModal
92-
open={pendingUrl !== null}
93-
onOpenChange={(open) => {
94-
if (!open) setPendingUrl(null)
95-
}}
96-
srTitle='Unsaved Changes'
97-
title='Unsaved Changes'
98-
text='You have unsaved changes. Are you sure you want to discard them?'
99-
dismissLabel='Keep editing'
100-
confirm={{ label: 'Discard Changes', onClick: discardAndNavigate }}
101-
/>
10287
</FileNavigationContext.Provider>
10388
)
10489
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import {
3232
} from '@/app/workspace/[workspaceId]/home/hooks/use-resource-panel'
3333
import { useRestoredChatEntry } from '@/app/workspace/[workspaceId]/home/hooks/use-restored-chat-entry'
3434
import { resolveWorkspaceResourceRef } from '@/app/workspace/[workspaceId]/home/resolve-resource-ref'
35+
import { useFeatureFlag } from '@/app/workspace/[workspaceId]/providers/feature-flags-provider'
3536
import { PermissionAccessBoundary } from '@/ee/access-requests/components/permission-access-boundary'
3637
import { useMarkMothershipChatRead } from '@/hooks/queries/mothership-chats'
3738
import { getProjectFileQueryOptions } from '@/hooks/queries/project-files'
@@ -69,6 +70,8 @@ export function Home(props: HomeProps) {
6970

7071
function HomeContent({ chatId, userName, userId }: HomeProps) {
7172
useOAuthReturnRouter()
73+
const projectsEnabled = useFeatureFlag('projects')
74+
const projectFilesEnabled = useFeatureFlag('project-files')
7275
const { workspaceId } = useParams<{ workspaceId: string }>()
7376
const queryClient = useQueryClient()
7477
const controller = useResourcePanelController()
@@ -382,6 +385,7 @@ function HomeContent({ chatId, userName, userId }: HomeProps) {
382385
*/
383386
async function handleWorkspaceResourceSelect(ref: WorkspaceResourceRef) {
384387
if (ref.type === 'file' && ref.owner?.entityType === 'project') {
388+
if (!projectsEnabled || !projectFilesEnabled) return
385389
if (!ref.id) {
386390
toast.error('This Project file reference is missing its ID')
387391
return

‎apps/sim/components/settings/use-settings-browser-navigation.test.tsx‎

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
/** @vitest-environment jsdom */
22

3-
import { act, useState } from 'react'
3+
import { act, useEffect, useState } from 'react'
44
import { nextNavigationMockFns } from '@sim/testing/mocks/next-navigation.mock'
55
import { createRoot, type Root } from 'react-dom/client'
66
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
77
import { SettingsIntentLink } from '@/components/settings/settings-intent-link'
88
import { SettingsNavigationGuard } from '@/components/settings/settings-navigation-guard'
99
import { useSettingsBrowserNavigation } from '@/components/settings/use-settings-browser-navigation'
1010
import { useSettingsUnsavedGuard } from '@/components/settings/use-settings-unsaved-guard'
11+
import {
12+
FileNavigationProvider,
13+
useFileNavigation,
14+
} from '@/app/workspace/[workspaceId]/files/components/file-detail/navigation'
1115
import { useSettingsDirtyStore } from '@/stores/settings/dirty/store'
1216

1317
vi.mock(
@@ -587,3 +591,63 @@ describe('native settings navigation', () => {
587591
expect(unload.defaultPrevented).toBe(false)
588592
})
589593
})
594+
595+
function FileDraft() {
596+
const [draft, setDraft] = useState('')
597+
const { setIsDirty, discardRef } = useFileNavigation({
598+
owner: { entityType: 'project', entityId: 'project' },
599+
})
600+
useEffect(() => {
601+
discardRef.current = () => setDraft('')
602+
return () => {
603+
discardRef.current = null
604+
}
605+
}, [discardRef])
606+
return (
607+
<input
608+
value={draft}
609+
onChange={(event) => {
610+
setDraft(event.target.value)
611+
setIsDirty(Boolean(event.target.value))
612+
}}
613+
/>
614+
)
615+
}
616+
617+
it.each([-1, 1])(
618+
'keeps a file draft through native history cancellation and discards only on confirmed traversal %s',
619+
async (delta) => {
620+
window.history.pushState({ router: 'future' }, '', '/future')
621+
nativeGo.call(window.history, -1)
622+
await settle()
623+
act(() =>
624+
root.render(
625+
<FileNavigationProvider
626+
owner={{ entityType: 'project', entityId: 'project' }}
627+
fileId='file'
628+
>
629+
<FileDraft />
630+
</FileNavigationProvider>
631+
)
632+
)
633+
const field = container.querySelector('input')
634+
const setter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')?.set
635+
if (!field || !setter) throw new Error('Missing file draft')
636+
act(() => {
637+
setter.call(field, 'unsaved file draft')
638+
field.dispatchEvent(new Event('input', { bubbles: true }))
639+
})
640+
nativeGo.call(window.history, delta)
641+
await settle()
642+
expect(window.location.pathname).toBe('/editor')
643+
expect(field.value).toBe('unsaved file draft')
644+
act(() => useSettingsDirtyStore.getState().cancelLeave())
645+
expect(field.value).toBe('unsaved file draft')
646+
nativeGo.call(window.history, delta)
647+
await settle()
648+
act(() => useSettingsDirtyStore.getState().confirmLeave())
649+
await settle()
650+
expect(window.location.pathname).toBe(delta === -1 ? '/prior' : '/future')
651+
expect(field.value).toBe('')
652+
}
653+
)

‎apps/sim/hooks/queries/workspace-files.test.tsx‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
*/
1010

1111
import { act, type ReactNode } from 'react'
12+
import { toast } from '@sim/emcn'
1213
import { flushMacrotask } from '@sim/testing/helpers/async'
1314
import { createDeferred } from '@sim/testing/helpers/deferred'
1415
import {
@@ -32,6 +33,7 @@ import {
3233
useAddressedWorkspaceFileRecord,
3334
useReloadWorkspaceFileContent,
3435
useUpdateWorkspaceFileContent,
36+
useUploadWorkspaceFile,
3537
useWorkspaceFileContent,
3638
useWorkspaceFiles,
3739
type WorkspaceFileContentResult,
@@ -430,3 +432,51 @@ describe('addressed file metadata fallback', () => {
430432
}
431433
})
432434
})
435+
436+
it.each(['abort-error', 'aborted-signal', 'failure'] as const)(
437+
'suppresses only intentional upload cancellation notifications (%s)',
438+
async (scenario) => {
439+
const notifications: unknown[] = []
440+
vi.spyOn(toast, 'error').mockImplementation((message) => {
441+
notifications.push(message)
442+
return 'toast'
443+
})
444+
const controller = new AbortController()
445+
if (scenario === 'aborted-signal') controller.abort()
446+
const failure =
447+
scenario === 'abort-error'
448+
? new DOMException('Cancelled', 'AbortError')
449+
: new Error('Provider failed')
450+
mockRequestJson.mockRejectedValueOnce(failure)
451+
const client = new QueryClient({ defaultOptions: { mutations: { retry: false } } })
452+
const container = document.createElement('div')
453+
const root = createRoot(container)
454+
let upload: ReturnType<typeof useUploadWorkspaceFile> | undefined
455+
function Probe() {
456+
upload = useUploadWorkspaceFile()
457+
return null
458+
}
459+
try {
460+
await act(async () =>
461+
root.render(
462+
<QueryClientProvider client={client}>
463+
<Probe />
464+
</QueryClientProvider>
465+
)
466+
)
467+
await act(async () => {
468+
await expect(
469+
upload?.mutateAsync({
470+
workspaceId: 'workspace',
471+
file: new File(['content'], 'upload.txt'),
472+
signal: controller.signal,
473+
})
474+
).rejects.toBe(failure)
475+
})
476+
expect(notifications.length).toBe(scenario === 'failure' ? 1 : 0)
477+
} finally {
478+
await act(async () => root.unmount())
479+
client.clear()
480+
}
481+
}
482+
)

‎apps/sim/hooks/queries/workspace-files.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,7 @@ export function useUploadWorkspaceFile() {
543543
}
544544
},
545545
onError: (error, variables) => {
546+
if (error.name === 'AbortError' || variables.signal?.aborted) return
546547
logger.error('Failed to upload file:', error)
547548
if (!variables.skipToast) {
548549
toast.error(`Failed to upload "${variables.file.name}": ${error.message}`, {

‎apps/sim/lib/mothership/agent-cli/engines/universal-grep.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,11 @@ async function listAll(runtime: GrepRuntime, path: string): Promise<Record<strin
145145
return out
146146
}
147147

148+
/** File adapters share bounded reads and wait for every provenance import before returning. */
149+
export function mapGrepFileReads<T, R>(items: T[], read: (item: T) => Promise<R>): Promise<R[]> {
150+
return mapConcurrent(items, FILE_READ_CONCURRENCY, read)
151+
}
152+
148153
async function mapConcurrent<T, R>(
149154
items: T[],
150155
limit: number,

‎apps/sim/lib/mothership/agent-cli/project-file-grep.test.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
/** @vitest-environment node */
2+
3+
import { flushMacrotask } from '@sim/testing/helpers/async'
4+
import { createDeferred } from '@sim/testing/helpers/deferred'
25
import { beforeEach, describe, expect, it, vi } from 'vitest'
36

47
const { list, artifact } = vi.hoisted(() => ({ list: vi.fn(), artifact: vi.fn() }))
@@ -80,3 +83,46 @@ describe('Project grep corpus', () => {
8083
expect(result.stdout).toBe('')
8184
})
8285
})
86+
87+
it('reads five Project artifacts concurrently while preserving inventory result order', async () => {
88+
const gates = Array.from({ length: 6 }, () => createDeferred<void>())
89+
let active = 0
90+
let peak = 0
91+
list.mockResolvedValue({
92+
files: Array.from({ length: 6 }, (_, index) => ({
93+
id: `file-${index}`,
94+
name: `notes-${index}.txt`,
95+
folderPath: '/',
96+
})),
97+
nextKeys: null,
98+
})
99+
artifact.mockImplementation(async ({ fileId }: { fileId: string }) => {
100+
active++
101+
peak = Math.max(peak, active)
102+
await gates[Number(fileId.slice(5))].promise
103+
active--
104+
return {
105+
file: { id: fileId, name: `${fileId}.txt` },
106+
contentType: 'text/plain',
107+
buffer: Buffer.from('needle'),
108+
secretProvenance: { status: 'exact', entries: [] },
109+
}
110+
})
111+
const pending = grepProjectFiles(invocation, context(), 'project')
112+
try {
113+
await flushMacrotask()
114+
expect(active).toBe(5)
115+
for (const index of [4, 3, 2, 1, 0, 5]) {
116+
gates[index].resolve()
117+
await flushMacrotask()
118+
}
119+
} finally {
120+
for (const gate of gates) gate.resolve()
121+
await pending
122+
}
123+
const result = await pending
124+
expect(peak).toBe(5)
125+
expect(result.stdout.split('\n')).toEqual(
126+
Array.from({ length: 6 }, (_, index) => `files/notes-${index}.txt (file-${index}):1: needle`)
127+
)
128+
})

0 commit comments

Comments
 (0)