From df2308c1cde9cfbad7b135f5bcdbeafafe51da82 Mon Sep 17 00:00:00 2001 From: Marcus Chandra Date: Thu, 6 Aug 2026 17:53:24 -0700 Subject: [PATCH 1/9] feat(files): let the user set a file's type from the header dropdown The file-detail filename dropdown gains a Type submenu offering the text-editable types (nine document formats plus a nested Code group). Picking one swaps the file's extension and its stored contentType in a single write, leaving the bytes untouched, so a file created as untitled.md can become untitled.json and open in the right editor. Renaming previously never touched contentType, so name and type could silently diverge; the retype path keeps them in agreement and the server re-derives the pairing rather than trusting the client. --- .gitignore | 1 + .../[id]/files/[fileId]/route.test.ts | 166 ++++++++++++++++++ .../workspaces/[id]/files/[fileId]/route.ts | 24 ++- .../[workspaceId]/components/index.ts | 4 + .../components/resource-header/index.ts | 4 + .../resource-header/resource-header.tsx | 100 ++++++++++- .../workspace/[workspaceId]/files/files.tsx | 111 ++++++++++-- .../files/untitled-title.test.ts | 32 +++- .../[workspaceId]/files/untitled-title.ts | 28 ++- .../hooks/queries/workspace-files.test.tsx | 146 ++++++++++++++- apps/sim/hooks/queries/workspace-files.ts | 16 +- apps/sim/lib/api/contracts/workspace-files.ts | 36 +++- apps/sim/lib/posthog/events.ts | 5 + .../workspace/workspace-file-manager.ts | 34 +++- .../workspace/workspace-file-retype.test.ts | 135 ++++++++++++++ apps/sim/lib/uploads/utils/file-utils.ts | 2 + .../lib/uploads/utils/text-file-types.test.ts | 136 ++++++++++++++ apps/sim/lib/uploads/utils/text-file-types.ts | 154 ++++++++++++++++ .../orchestration/file-folder-lifecycle.ts | 15 +- 19 files changed, 1094 insertions(+), 55 deletions(-) create mode 100644 apps/sim/app/api/workspaces/[id]/files/[fileId]/route.test.ts create mode 100644 apps/sim/lib/uploads/contexts/workspace/workspace-file-retype.test.ts create mode 100644 apps/sim/lib/uploads/utils/text-file-types.test.ts create mode 100644 apps/sim/lib/uploads/utils/text-file-types.ts diff --git a/.gitignore b/.gitignore index 6e9bdf2c99c..137b3e34bd8 100644 --- a/.gitignore +++ b/.gitignore @@ -111,3 +111,4 @@ __pycache__/ # `apps/sim/lib/uploads/` — 61 files of tracked source — and silently ignore # anything added there later. /uploads +.gstack/ diff --git a/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.test.ts b/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.test.ts new file mode 100644 index 00000000000..829085e1ec3 --- /dev/null +++ b/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.test.ts @@ -0,0 +1,166 @@ +/** + * @vitest-environment node + */ +import { authMockFns, permissionsMock, permissionsMockFns } from '@sim/testing' +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockPerformRenameWorkspaceFile, mockPerformDeleteWorkspaceFileItems } = vi.hoisted(() => ({ + mockPerformRenameWorkspaceFile: vi.fn(), + mockPerformDeleteWorkspaceFileItems: vi.fn(), +})) + +vi.mock('@/lib/workspace-files/orchestration', () => ({ + performDeleteWorkspaceFileItems: mockPerformDeleteWorkspaceFileItems, + performRenameWorkspaceFile: mockPerformRenameWorkspaceFile, +})) + +vi.mock('@/lib/posthog/server', () => ({ + captureServerEvent: vi.fn(), +})) + +vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock) + +const WS = '7727ef3f-8cf6-4686-b063-2bb006a10785' +const FILE_ID = 'ec28e5d5-898a-48f0-aa6f-2fd7427c9563' + +import { captureServerEvent } from '@/lib/posthog/server' +import { PATCH } from '@/app/api/workspaces/[id]/files/[fileId]/route' + +const params = () => ({ params: Promise.resolve({ id: WS, fileId: FILE_ID }) }) + +const patchRequest = (body: unknown) => + new NextRequest(`http://localhost/api/workspaces/${WS}/files/${FILE_ID}`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }) + +const RENAMED_FILE = { + id: FILE_ID, + workspaceId: WS, + name: 'untitled.json', + key: `workspace/${WS}/mock-key`, + path: '/api/files/serve/mock-key?context=workspace', + size: 0, + type: 'application/json', + uploadedBy: 'user-1', + folderId: null, + uploadedAt: new Date('2026-04-13T00:00:00.000Z'), + updatedAt: new Date('2026-04-13T00:00:00.000Z'), +} + +describe('PATCH /api/workspaces/[id]/files/[fileId]', () => { + beforeEach(() => { + vi.clearAllMocks() + authMockFns.mockGetSession.mockResolvedValue({ + user: { id: 'user-1', name: 'User One', email: 'u@example.com' }, + }) + permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('write') + mockPerformRenameWorkspaceFile.mockResolvedValue({ success: true, file: RENAMED_FILE }) + }) + + describe('auth', () => { + it('returns 401 when unauthenticated', async () => { + authMockFns.mockGetSession.mockResolvedValueOnce(null) + const res = await PATCH(patchRequest({ name: 'notes.md' }), params()) + expect(res.status).toBe(401) + }) + + it('returns 403 for a read-only member', async () => { + permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValueOnce('read') + const res = await PATCH(patchRequest({ name: 'notes.md' }), params()) + expect(res.status).toBe(403) + expect(mockPerformRenameWorkspaceFile).not.toHaveBeenCalled() + }) + }) + + describe('rename only', () => { + it('forwards the name with no contentType', async () => { + const res = await PATCH(patchRequest({ name: 'notes.md' }), params()) + + expect(res.status).toBe(200) + expect(mockPerformRenameWorkspaceFile).toHaveBeenCalledWith( + expect.objectContaining({ name: 'notes.md', contentType: undefined }) + ) + expect(captureServerEvent).toHaveBeenCalledWith( + 'user-1', + 'file_renamed', + expect.anything(), + expect.anything() + ) + }) + + it('returns 409 on a name conflict', async () => { + mockPerformRenameWorkspaceFile.mockResolvedValueOnce({ + success: false, + error: 'A file named "notes.md" already exists', + errorCode: 'conflict', + }) + const res = await PATCH(patchRequest({ name: 'notes.md' }), params()) + expect(res.status).toBe(409) + }) + }) + + describe('retype', () => { + it('forwards a valid name and contentType pair', async () => { + const res = await PATCH( + patchRequest({ name: 'untitled.json', contentType: 'application/json' }), + params() + ) + + expect(res.status).toBe(200) + expect(await res.json()).toMatchObject({ + success: true, + file: expect.objectContaining({ name: 'untitled.json', type: 'application/json' }), + }) + expect(mockPerformRenameWorkspaceFile).toHaveBeenCalledWith( + expect.objectContaining({ name: 'untitled.json', contentType: 'application/json' }) + ) + }) + + it('reports a retype separately from a rename', async () => { + await PATCH( + patchRequest({ name: 'untitled.json', contentType: 'application/json' }), + params() + ) + + expect(captureServerEvent).toHaveBeenCalledWith( + 'user-1', + 'file_type_changed', + expect.objectContaining({ content_type: 'application/json' }), + expect.anything() + ) + }) + + it('rejects a contentType outside the selectable allowlist', async () => { + const res = await PATCH( + patchRequest({ name: 'installer.exe', contentType: 'application/x-msdownload' }), + params() + ) + + expect(res.status).toBe(400) + expect(mockPerformRenameWorkspaceFile).not.toHaveBeenCalled() + }) + + it('rejects a contentType that disagrees with the name extension', async () => { + const res = await PATCH( + patchRequest({ name: 'untitled.json', contentType: 'text/markdown' }), + params() + ) + + expect(res.status).toBe(400) + expect(mockPerformRenameWorkspaceFile).not.toHaveBeenCalled() + }) + + it('rejects a contentType paired with an extension no type writes', async () => { + const res = await PATCH( + patchRequest({ name: 'notes.yml', contentType: 'application/x-yaml' }), + params() + ) + + expect(res.status).toBe(400) + expect(mockPerformRenameWorkspaceFile).not.toHaveBeenCalled() + }) + }) +}) diff --git a/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.ts b/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.ts index 1826988ea08..be62e1cfbd8 100644 --- a/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.ts +++ b/apps/sim/app/api/workspaces/[id]/files/[fileId]/route.ts @@ -37,7 +37,7 @@ export const PATCH = withRouteHandler( const parsed = await parseRequest(renameWorkspaceFileContract, request, context) if (!parsed.success) return parsed.response const { id: workspaceId, fileId } = parsed.data.params - const { name } = parsed.data.body + const { name, contentType } = parsed.data.body const userPermission = await getUserEntityPermissions( session.user.id, @@ -56,6 +56,7 @@ export const PATCH = withRouteHandler( fileId, name, userId: session.user.id, + contentType, }) if (!result.success || !result.file) { return NextResponse.json( @@ -66,12 +67,21 @@ export const PATCH = withRouteHandler( logger.info(`[${requestId}] Renamed workspace file: ${fileId} to "${result.file.name}"`) - captureServerEvent( - session.user.id, - 'file_renamed', - { workspace_id: workspaceId }, - { groups: { workspace: workspaceId } } - ) + if (contentType) { + captureServerEvent( + session.user.id, + 'file_type_changed', + { workspace_id: workspaceId, content_type: contentType }, + { groups: { workspace: workspaceId } } + ) + } else { + captureServerEvent( + session.user.id, + 'file_renamed', + { workspace_id: workspaceId }, + { groups: { workspace: workspaceId } } + ) + } return NextResponse.json({ success: true, file: result.file, diff --git a/apps/sim/app/workspace/[workspaceId]/components/index.ts b/apps/sim/app/workspace/[workspaceId]/components/index.ts index 16570ad0070..99d90c32a31 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/index.ts +++ b/apps/sim/app/workspace/[workspaceId]/components/index.ts @@ -13,7 +13,11 @@ export { export type { BreadcrumbEditing, BreadcrumbItem, + DropdownMenuOption, DropdownOption, + DropdownRadioGroup, + DropdownRadioItem, + DropdownSubmenuOption, ResourceAction, } from './resource/components/resource-header' export type { diff --git a/apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-header/index.ts b/apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-header/index.ts index eaa02307577..af40241b08e 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-header/index.ts +++ b/apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-header/index.ts @@ -1,7 +1,11 @@ export type { BreadcrumbEditing, BreadcrumbItem, + DropdownMenuOption, DropdownOption, + DropdownRadioGroup, + DropdownRadioItem, + DropdownSubmenuOption, ResourceAction, } from './resource-header' export { ResourceHeader } from './resource-header' diff --git a/apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-header/resource-header.tsx b/apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-header/resource-header.tsx index d70a12049b8..72f546053fb 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-header/resource-header.tsx +++ b/apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-header/resource-header.tsx @@ -20,6 +20,11 @@ import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, + DropdownMenuRadioGroup, + DropdownMenuRadioItem, + DropdownMenuSub, + DropdownMenuSubContent, + DropdownMenuSubTrigger, DropdownMenuTrigger, FloatingTooltip, POPOVER_ANIMATION_CLASSES, @@ -44,6 +49,77 @@ export interface DropdownOption { disabled?: boolean } +export interface DropdownRadioItem { + /** Selection value, matched against {@link DropdownSubmenuOption.value}. */ + id: string + label: string +} + +export interface DropdownRadioGroup { + /** + * Nests the group behind a submenu of its own when set. Use it to keep a long tail of options + * (a language list, say) off the top level without losing them. + */ + submenuLabel?: string + items: DropdownRadioItem[] +} + +/** + * A dropdown entry that opens a submenu of mutually exclusive choices rather than firing an action. + * Distinguished from {@link DropdownOption} by `groups`, so the two can share one list. + */ +export interface DropdownSubmenuOption { + label: string + icon?: React.ElementType + groups: DropdownRadioGroup[] + /** The currently selected `id`, or undefined when the value is not one of the offered choices. */ + value?: string + onValueChange: (id: string) => void + disabled?: boolean +} + +export type DropdownMenuOption = DropdownOption | DropdownSubmenuOption + +function isSubmenuOption(option: DropdownMenuOption): option is DropdownSubmenuOption { + return 'groups' in option +} + +interface DropdownRadioGroupContentProps { + group: DropdownRadioGroup + value?: string + onValueChange: (id: string) => void +} + +/** + * One radio group inside a {@link DropdownSubmenuOption}, nested behind its own submenu when the + * group carries a `submenuLabel`. Every group is handed the same `value`; only the one that owns it + * renders a selected indicator, which is what lets the selection read correctly across groups. + */ +function DropdownRadioGroupContent({ + group, + value, + onValueChange, +}: DropdownRadioGroupContentProps) { + const items = ( + + {group.items.map((item) => ( + + {item.label} + + ))} + + ) + + if (!group.submenuLabel) return items + + return ( + + {group.submenuLabel} + {items} + + ) +} + export interface BreadcrumbEditing { isEditing: boolean value: string @@ -63,7 +139,7 @@ export interface BreadcrumbItem { label: string icon?: React.ElementType onClick?: () => void - dropdownItems?: DropdownOption[] + dropdownItems?: DropdownMenuOption[] editing?: BreadcrumbEditing /** * Marks a non-navigable trailing crumb (e.g. "New Chunk", "Loading...") so the @@ -266,7 +342,7 @@ interface BreadcrumbSegmentProps { icon?: React.ElementType label: string onClick?: () => void - dropdownItems?: DropdownOption[] + dropdownItems?: DropdownMenuOption[] editing?: BreadcrumbEditing className?: string } @@ -327,6 +403,26 @@ const BreadcrumbSegment = memo(function BreadcrumbSegment({ {dropdownItems.map((item) => { const ItemIcon = item.icon + if (isSubmenuOption(item)) { + return ( + + + {ItemIcon && } + {item.label} + + + {item.groups.map((group) => ( + + ))} + + + ) + } return ( {ItemIcon && } diff --git a/apps/sim/app/workspace/[workspaceId]/files/files.tsx b/apps/sim/app/workspace/[workspaceId]/files/files.tsx index 92dca7e8221..928b0074cf5 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/files.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/files.tsx @@ -18,7 +18,7 @@ import { toast, Upload, } from '@sim/emcn' -import { Download, Send } from '@sim/emcn/icons' +import { Download, FileText, Send } from '@sim/emcn/icons' import { createLogger } from '@sim/logger' import { getErrorMessage, toError } from '@sim/utils/errors' import { useParams, useRouter } from 'next/navigation' @@ -38,6 +38,12 @@ import { isVideoFileType, resolveEffectiveMimeType, } from '@/lib/uploads/utils/file-utils' +import { + findTextFileTypeById, + resolveTextFileType, + SELECTABLE_TEXT_FILE_TYPES, + withFileTypeExtension, +} from '@/lib/uploads/utils/text-file-types' import { isSupportedExtension, SUPPORTED_AUDIO_EXTENSIONS, @@ -48,6 +54,7 @@ import { } from '@/lib/uploads/utils/validation' import type { BreadcrumbItem, + DropdownRadioGroup, FilterTag, ResourceAction, ResourceColumn, @@ -98,7 +105,7 @@ import { DEFAULT_UNTITLED_NAME, deriveMarkdownFileName, isUntitledName, - uniqueMarkdownName, + uniqueFileName, } from '@/app/workspace/[workspaceId]/files/untitled-title' import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider' import { useContextMenu } from '@/app/workspace/[workspaceId]/w/components/sidebar/hooks' @@ -166,6 +173,11 @@ const COLUMNS: ResourceColumn[] = [ { id: 'updated', header: 'Last Updated' }, ] +/** + * Labels for the binary formats. The text formats are labelled by the selectable-type registry + * instead, so the Type column and the header's type picker can never disagree about what a file is + * called. + */ const MIME_TYPE_LABELS: Record = { 'application/pdf': 'PDF', 'application/msword': 'Word', @@ -174,14 +186,25 @@ const MIME_TYPE_LABELS: Record = { 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet': 'Excel', 'application/vnd.ms-powerpoint': 'PowerPoint', 'application/vnd.openxmlformats-officedocument.presentationml.presentation': 'PowerPoint', - 'application/json': 'JSON', - 'application/x-yaml': 'YAML', - 'text/csv': 'CSV', - 'text/plain': 'Text', - 'text/html': 'HTML', - 'text/markdown': 'Markdown', } +/** Hoisted: derived from a frozen registry, so there is nothing to rebuild per render. */ +const TEXT_FILE_TYPE_MENU_GROUPS: DropdownRadioGroup[] = [ + { + items: SELECTABLE_TEXT_FILE_TYPES.filter((type) => type.group === 'document').map((type) => ({ + id: type.id, + label: type.label, + })), + }, + { + submenuLabel: 'Code', + items: SELECTABLE_TEXT_FILE_TYPES.filter((type) => type.group === 'code').map((type) => ({ + id: type.id, + label: type.label, + })), + }, +] + const EMPTY_WORKSPACE_FILES: WorkspaceFileRecord[] = [] const EMPTY_WORKSPACE_FILE_FOLDERS: WorkspaceFileFolderApi[] = [] @@ -197,6 +220,9 @@ const hasExternalFiles = (dataTransfer: DataTransfer): boolean => dataTransfer.types.includes('Files') function formatFileType(storedType: string | null, filename: string): string { + const textType = resolveTextFileType(storedType, filename) + if (textType) return textType.label + const mimeType = resolveEffectiveMimeType(storedType, filename) if (MIME_TYPE_LABELS[mimeType]) { @@ -393,11 +419,17 @@ export function Files() { * While a file is still untitled, name it after the leading heading the user types in its editor. The * editor reports the heading text (debounced); here we re-check the file is still untitled, derive a * unique `.md` name among its folder siblings, and rename. A no-op once the file has a real name. + * + * The markdown check is what keeps this from retyping a file behind the user's back: + * `deriveMarkdownFileName` always appends `.md`, so an untitled file of another type would be + * silently converted. Only the rich markdown editor reports headings today, so the guard is + * belt-and-braces — but it keeps the invariant local instead of three files away. */ const handleDeriveTitleFromHeading = useCallback( (headingText: string) => { const currentFile = selectedFileRef.current if (!currentFile || !isUntitledName(currentFile.name)) return + if (!isMarkdownFile(currentFile)) return const derived = deriveMarkdownFileName(headingText) if (!derived) return const siblingNames = new Set( @@ -408,7 +440,7 @@ export function Files() { ) .map((f) => f.name) ) - const name = uniqueMarkdownName(derived, siblingNames) + const name = uniqueFileName(derived, siblingNames) if (name === currentFile.name) return renameFile .mutateAsync({ workspaceId, fileId: currentFile.id, name }) @@ -1154,6 +1186,49 @@ export function Files() { if (file) headerRename.startRename(file.id, file.name) }, [headerRename.startRename]) + /** + * Retypes the open file: swaps its extension for the chosen type's and stores that type's MIME in + * the same write, leaving the bytes untouched. + * + * Unsaved edits are flushed first rather than blocked. `showUnsavedChangesAlert` exists for + * navigating away, where discarding is a real choice; here the user stays on the file and expects + * their text to survive — and the markdown editor unmounts the moment the file stops being + * markdown, taking its pending debounce with it. Awaiting the save also orders the content write + * ahead of the metadata write, so the two cannot race. + */ + const handleChangeFileType = useCallback( + async (typeId: string) => { + const file = selectedFileRef.current + if (!file) return + + const type = findTextFileTypeById(typeId) + if (!type) return + + const nextName = withFileTypeExtension(file.name, type) + if (type.mimeType === file.type && nextName === file.name) return + + if (isDirtyRef.current) await saveRef.current?.() + + const siblingNames = new Set( + filesRef.current + .filter((f) => (f.folderId ?? null) === (file.folderId ?? null) && f.id !== file.id) + .map((f) => f.name) + ) + + try { + await renameFile.mutateAsync({ + workspaceId, + fileId: file.id, + name: uniqueFileName(nextName, siblingNames), + contentType: type.mimeType, + }) + } catch (err) { + logger.error('Failed to change file type:', err) + } + }, + [workspaceId] + ) + const handleDownloadSelected = useCallback(() => { const file = selectedFileRef.current if (file) handleDownload(file) @@ -1268,6 +1343,21 @@ export function Files() { ...(canEdit ? [ { label: 'Rename', icon: Pencil, onClick: handleStartHeaderRename }, + /** + * Offered only for files the in-app editor already opens. Retyping is a metadata + * edit, not a conversion, so a PDF or an image has nothing it could be changed to. + */ + ...(isTextEditable(selectedFile) + ? [ + { + label: 'Type', + icon: FileText, + groups: TEXT_FILE_TYPE_MENU_GROUPS, + value: resolveTextFileType(selectedFile.type, selectedFile.name)?.id, + onValueChange: handleChangeFileType, + }, + ] + : []), { label: 'Share', icon: Send, onClick: handleShareSelected }, { label: 'Delete', icon: Trash, onClick: handleDeleteSelected }, ] @@ -1284,6 +1374,7 @@ export function Files() { headerRename.editingId, headerRename.editValue, handleStartHeaderRename, + handleChangeFileType, handleDownloadSelected, handleShareSelected, handleDeleteSelected, @@ -1316,7 +1407,7 @@ export function Files() { const existingNames = new Set( filesRef.current.filter((f) => (f.folderId ?? null) === currentFolderId).map((f) => f.name) ) - const name = uniqueMarkdownName(DEFAULT_UNTITLED_NAME, existingNames) + const name = uniqueFileName(DEFAULT_UNTITLED_NAME, existingNames) const mimeType = getMimeTypeFromExtension('md') const blob = new Blob([''], { type: mimeType }) diff --git a/apps/sim/app/workspace/[workspaceId]/files/untitled-title.test.ts b/apps/sim/app/workspace/[workspaceId]/files/untitled-title.test.ts index e9ee0ba2e43..c052f0ba96f 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/untitled-title.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/untitled-title.test.ts @@ -3,15 +3,15 @@ import { DEFAULT_UNTITLED_NAME, deriveMarkdownFileName, isUntitledName, - uniqueMarkdownName, + uniqueFileName, } from './untitled-title' describe('untitled format single-source-of-truth', () => { - // Guards against DEFAULT_UNTITLED_NAME / uniqueMarkdownName drifting from the isUntitledName regex: + // Guards against DEFAULT_UNTITLED_NAME / uniqueFileName drifting from the isUntitledName regex: // the default name and its deduped siblings must always read back as "untitled". it('recognizes the default name and its deduped siblings as untitled', () => { expect(isUntitledName(DEFAULT_UNTITLED_NAME)).toBe(true) - const second = uniqueMarkdownName(DEFAULT_UNTITLED_NAME, new Set([DEFAULT_UNTITLED_NAME])) + const second = uniqueFileName(DEFAULT_UNTITLED_NAME, new Set([DEFAULT_UNTITLED_NAME])) expect(second).toBe('untitled (1).md') expect(isUntitledName(second)).toBe(true) }) @@ -22,9 +22,11 @@ describe('isUntitledName', () => { ['untitled.md', true], ['untitled (1).md', true], ['untitled (23).md', true], + ['untitled.txt', true], + ['untitled (2).json', true], ['Untitled.md', false], - ['untitled.txt', false], ['untitled', false], + ['untitled.', false], ['my notes.md', false], ['untitled draft.md', false], ['untitled ().md', false], @@ -64,14 +66,26 @@ describe('deriveMarkdownFileName', () => { }) }) -describe('uniqueMarkdownName', () => { +describe('uniqueFileName', () => { it('returns the name unchanged when free', () => { - expect(uniqueMarkdownName('notes.md', new Set())).toBe('notes.md') + expect(uniqueFileName('notes.md', new Set())).toBe('notes.md') }) it('appends an incrementing suffix before the extension when taken', () => { - expect(uniqueMarkdownName('notes.md', new Set(['notes.md']))).toBe('notes (1).md') - expect(uniqueMarkdownName('notes.md', new Set(['notes.md', 'notes (1).md']))).toBe( - 'notes (2).md' + expect(uniqueFileName('notes.md', new Set(['notes.md']))).toBe('notes (1).md') + expect(uniqueFileName('notes.md', new Set(['notes.md', 'notes (1).md']))).toBe('notes (2).md') + }) + it('suffixes before the last extension only', () => { + expect(uniqueFileName('report.final.csv', new Set(['report.final.csv']))).toBe( + 'report.final (1).csv' ) }) + it('suffixes at the end when there is no extension', () => { + expect(uniqueFileName('notes', new Set(['notes']))).toBe('notes (1)') + }) + it('treats a leading dot as part of the name, not an extension', () => { + expect(uniqueFileName('.gitignore', new Set(['.gitignore']))).toBe('.gitignore (1)') + }) + it('suffixes a non-markdown extension in place', () => { + expect(uniqueFileName('data.json', new Set(['data.json']))).toBe('data (1).json') + }) }) diff --git a/apps/sim/app/workspace/[workspaceId]/files/untitled-title.ts b/apps/sim/app/workspace/[workspaceId]/files/untitled-title.ts index 4800fb13e9a..11cc599f741 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/untitled-title.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/untitled-title.ts @@ -8,7 +8,14 @@ import { truncate } from '@sim/utils/string' */ export const DEFAULT_UNTITLED_NAME = 'untitled.md' -const UNTITLED_NAME_RE = /^untitled(?: \(\d+\))?\.md$/ +/** + * Any extension, not just `.md`: a file created as `untitled.md` and immediately retyped to JSON is + * still an unnamed file, and should keep naming itself from its first heading if it is retyped back. + * A bare `untitled` with no extension is a name the user chose, so it does not match. Still + * case-sensitive: only the lowercase name this app generates counts as unnamed, so a file a user + * deliberately called `Untitled.md` is left alone. + */ +const UNTITLED_NAME_RE = /^untitled(?: \(\d+\))?\.[a-z0-9]+$/ /** Longest title kept when deriving a file name from a heading, before the `.md` extension. */ const MAX_DERIVED_TITLE_LENGTH = 100 @@ -19,7 +26,7 @@ const MAX_DERIVED_TITLE_LENGTH = 100 */ const ILLEGAL_FILENAME_CHARS = /[\\/:*?"<>|\x00-\x1f]/g -/** True when `name` is still the auto-assigned untitled markdown name (`untitled.md`, `untitled (2).md`). */ +/** True when `name` is still the auto-assigned untitled name (`untitled.md`, `untitled (2).json`). */ export function isUntitledName(name: string): boolean { return UNTITLED_NAME_RE.test(name) } @@ -41,17 +48,22 @@ export function deriveMarkdownFileName(headingText: string): string | null { } /** - * Makes `name` unique among `existingNames` by appending ` (n)` before the `.md` extension — the same - * scheme `handleCreateFile` uses for the default untitled name. + * Makes `name` unique among `existingNames` by inserting ` (n)` before the extension — the same + * scheme `handleCreateFile` uses for the default untitled name, and the same last-dot rule the + * server's `withCopySuffix` applies, so `report.final.csv` becomes `report.final (2).csv`. A name + * with no extension takes the suffix at the end: `notes` becomes `notes (2)`. */ -export function uniqueMarkdownName(name: string, existingNames: ReadonlySet): string { +export function uniqueFileName(name: string, existingNames: ReadonlySet): string { if (!existingNames.has(name)) return name - const withoutExt = name.replace(/\.md$/i, '') + const lastDot = name.lastIndexOf('.') + const hasExtension = lastDot > 0 && lastDot < name.length - 1 + const base = hasExtension ? name.slice(0, lastDot) : name + const extension = hasExtension ? name.slice(lastDot) : '' let counter = 1 - let candidate = `${withoutExt} (${counter}).md` + let candidate = `${base} (${counter})${extension}` while (existingNames.has(candidate)) { counter++ - candidate = `${withoutExt} (${counter}).md` + candidate = `${base} (${counter})${extension}` } return candidate } diff --git a/apps/sim/hooks/queries/workspace-files.test.tsx b/apps/sim/hooks/queries/workspace-files.test.tsx index db51e9fc452..e64e5ac8788 100644 --- a/apps/sim/hooks/queries/workspace-files.test.tsx +++ b/apps/sim/hooks/queries/workspace-files.test.tsx @@ -12,7 +12,11 @@ import { sleep } from '@sim/utils/helpers' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { useWorkspaceFileContent } from '@/hooks/queries/workspace-files' +import { + useRenameWorkspaceFile, + useWorkspaceFileContent, + workspaceFilesKeys, +} from '@/hooks/queries/workspace-files' let fetchCount = 0 @@ -102,3 +106,143 @@ describe('useWorkspaceFileContent refetchInterval passthrough', () => { unmount() }) }) + +/** + * The optimistic patch in `useRenameWorkspaceFile.onMutate`: a retype must move BOTH the name and + * the stored type in the cache, because the viewer picks its editor from `type` and would otherwise + * keep rendering the old one until the invalidation lands. + */ +describe('useRenameWorkspaceFile optimistic cache patch', () => { + const WS = 'ws-1' + const FILE_ID = 'file-1' + + const existingFile = { + id: FILE_ID, + workspaceId: WS, + name: 'untitled.md', + key: `workspace/${WS}/123-abc-untitled.md`, + path: '/api/files/serve/mock-key?context=workspace', + size: 0, + type: 'text/markdown', + uploadedBy: 'user-1', + folderId: null, + uploadedAt: new Date('2026-04-13T00:00:00.000Z'), + updatedAt: new Date('2026-04-13T00:00:00.000Z'), + } + + function renderRenameHook(): { + rename: () => ReturnType + queryClient: QueryClient + unmount: () => void + } { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }) + queryClient.setQueryData(workspaceFilesKeys.list(WS, 'active'), [existingFile]) + + const container = document.createElement('div') + const root: Root = createRoot(container) + let result: ReturnType | null = null + + function Probe() { + result = useRenameWorkspaceFile() + return null + } + + act(() => { + root.render( + + + + ) + }) + + return { + rename: () => { + if (!result) throw new Error('hook did not render') + return result + }, + queryClient, + unmount: () => { + act(() => root.unmount()) + queryClient.clear() + }, + } + } + + function cachedFile(queryClient: QueryClient) { + const list = queryClient.getQueryData<(typeof existingFile)[]>( + workspaceFilesKeys.list(WS, 'active') + ) + return list?.[0] + } + + /** + * Leaves the request in flight, so the assertion sees the optimistic write rather than whatever + * `onSettled`'s invalidation refetches over it. + */ + function stubPendingFetch() { + vi.stubGlobal( + 'fetch', + vi.fn(() => new Promise(() => {})) + ) + } + + it('patches name and type together on a retype', async () => { + stubPendingFetch() + const { rename, queryClient, unmount } = renderRenameHook() + + await act(async () => { + rename().mutate({ + workspaceId: WS, + fileId: FILE_ID, + name: 'untitled.json', + contentType: 'application/json', + }) + await sleep(20) + }) + + expect(cachedFile(queryClient)).toMatchObject({ + name: 'untitled.json', + type: 'application/json', + }) + unmount() + }) + + it('leaves type untouched on a plain rename', async () => { + stubPendingFetch() + const { rename, queryClient, unmount } = renderRenameHook() + + await act(async () => { + rename().mutate({ workspaceId: WS, fileId: FILE_ID, name: 'notes.md' }) + await sleep(20) + }) + + expect(cachedFile(queryClient)).toMatchObject({ name: 'notes.md', type: 'text/markdown' }) + unmount() + }) + + it('rolls the type back when the request fails', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response(JSON.stringify({ error: 'boom' }), { status: 500 })) + ) + const { rename, queryClient, unmount } = renderRenameHook() + + await act(async () => { + rename().mutate({ + workspaceId: WS, + fileId: FILE_ID, + name: 'untitled.json', + contentType: 'application/json', + }) + await sleep(50) + }) + + expect(cachedFile(queryClient)).toMatchObject({ + name: 'untitled.md', + type: 'text/markdown', + }) + unmount() + }) +}) diff --git a/apps/sim/hooks/queries/workspace-files.ts b/apps/sim/hooks/queries/workspace-files.ts index 65469cbe202..9791694b44f 100644 --- a/apps/sim/hooks/queries/workspace-files.ts +++ b/apps/sim/hooks/queries/workspace-files.ts @@ -587,18 +587,24 @@ interface RenameFileParams { workspaceId: string fileId: string name: string + /** + * Set only when the rename is a type change, in which case `name` already carries the matching + * extension. Patched into the cache alongside the name so the viewer swaps editors immediately + * rather than waiting for the invalidation to land. + */ + contentType?: string } export function useRenameWorkspaceFile() { const queryClient = useQueryClient() return useMutation({ - mutationFn: async ({ workspaceId, fileId, name }: RenameFileParams) => + mutationFn: async ({ workspaceId, fileId, name, contentType }: RenameFileParams) => requestJson(renameWorkspaceFileContract, { params: { id: workspaceId, fileId }, - body: { name }, + body: contentType ? { name, contentType } : { name }, }), - onMutate: async ({ workspaceId, fileId, name }) => { + onMutate: async ({ workspaceId, fileId, name, contentType }) => { await queryClient.cancelQueries({ queryKey: workspaceFilesKeys.workspaceLists(workspaceId) }) const previous = queryClient.getQueryData( workspaceFilesKeys.list(workspaceId, 'active') @@ -606,7 +612,9 @@ export function useRenameWorkspaceFile() { if (previous) { queryClient.setQueryData( workspaceFilesKeys.list(workspaceId, 'active'), - previous.map((f) => (f.id === fileId ? { ...f, name } : f)) + previous.map((f) => + f.id === fileId ? { ...f, name, ...(contentType ? { type: contentType } : {}) } : f + ) ) } return { previous } diff --git a/apps/sim/lib/api/contracts/workspace-files.ts b/apps/sim/lib/api/contracts/workspace-files.ts index 0e5cadaaa75..37a23e6e9f6 100644 --- a/apps/sim/lib/api/contracts/workspace-files.ts +++ b/apps/sim/lib/api/contracts/workspace-files.ts @@ -2,6 +2,11 @@ import { z } from 'zod' import { inlineFileRefQuerySchema } from '@/lib/api/contracts/primitives' import { shareRecordSchema } from '@/lib/api/contracts/public-shares' import { defineRouteContract } from '@/lib/api/contracts/types' +import { getFileExtension } from '@/lib/uploads/utils/file-utils' +import { + findTextFileTypeByExtension, + SELECTABLE_TEXT_MIME_TYPES, +} from '@/lib/uploads/utils/text-file-types' /** * Client-reachable listing scopes. `all` is deliberately excluded: it drops the @@ -47,9 +52,34 @@ const workspaceFileNameSchema = z 'Name cannot contain path separators or dot segments' ) -export const renameWorkspaceFileBodySchema = z.object({ - name: workspaceFileNameSchema, -}) +/** + * Renames a file and, when `contentType` is present, retypes it in the same write. + * + * Only the text-editable MIMEs are accepted, and the name's extension must be the one that MIME + * writes. The client computes both sides; the server re-derives the pairing here and rejects any + * disagreement, so a crafted request cannot label arbitrary bytes `text/plain`. The check runs + * extension to MIME rather than the reverse because the extension is the registry's unique key — + * several types share a MIME. + */ +export const renameWorkspaceFileBodySchema = z + .object({ + name: workspaceFileNameSchema, + contentType: z + .string() + .refine((mimeType) => SELECTABLE_TEXT_MIME_TYPES.includes(mimeType), 'Unsupported file type') + .optional(), + }) + .refine( + (body) => + body.contentType === undefined || + findTextFileTypeByExtension(getFileExtension(body.name))?.mimeType === body.contentType, + { + path: ['name'], + error: 'File name extension does not match the selected type', + } + ) + +export type RenameWorkspaceFileBody = z.input export const updateWorkspaceFileContentBodySchema = z.object({ content: z.string(), diff --git a/apps/sim/lib/posthog/events.ts b/apps/sim/lib/posthog/events.ts index 48462b7be1b..cb9322fa184 100644 --- a/apps/sim/lib/posthog/events.ts +++ b/apps/sim/lib/posthog/events.ts @@ -301,6 +301,11 @@ export interface PostHogEventMap { workspace_id: string } + file_type_changed: { + workspace_id: string + content_type: string + } + file_moved: { workspace_id: string file_count: number diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts index 4bae36791dc..11087e5a8c7 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts @@ -1523,12 +1523,20 @@ export async function updateWorkspaceFileContent( } /** - * Rename a workspace file (updates the display name in the database) + * Rename a workspace file (updates the display name in the database), and optionally retype it in + * the same write. + * + * A retype is always accompanied by a rename — the extension carries the type — so both land in one + * row update, behind one conflict check, and there is never a moment where the name and the stored + * `contentType` disagree. `contentUpdatedAt` is deliberately left alone: it is the collaborative + * persist's optimistic-concurrency token, and advancing it on a metadata write would invalidate an + * in-flight editor save. */ export async function renameWorkspaceFile( workspaceId: string, fileId: string, - newName: string + newName: string, + options?: { contentType?: string } ): Promise { logger.info(`Renaming workspace file: ${fileId} to "${newName}" in workspace ${workspaceId}`) @@ -1540,20 +1548,31 @@ export async function renameWorkspaceFile( throw new Error('File not found') } - if (fileRecord.name === normalizedName) { + const nextContentType = + options?.contentType && options.contentType !== fileRecord.type + ? options.contentType + : undefined + + if (fileRecord.name === normalizedName && !nextContentType) { return fileRecord } - const exists = await fileExistsInWorkspace(workspaceId, normalizedName, fileRecord.folderId) - if (exists) { - throw new FileConflictError(normalizedName) + if (fileRecord.name !== normalizedName) { + const exists = await fileExistsInWorkspace(workspaceId, normalizedName, fileRecord.folderId) + if (exists) { + throw new FileConflictError(normalizedName) + } } let updated: { id: string }[] try { updated = await db .update(workspaceFiles) - .set({ originalName: normalizedName, updatedAt: new Date() }) + .set({ + originalName: normalizedName, + ...(nextContentType ? { contentType: nextContentType } : {}), + updatedAt: new Date(), + }) .where( and( eq(workspaceFiles.id, fileId), @@ -1578,6 +1597,7 @@ export async function renameWorkspaceFile( return { ...fileRecord, name: normalizedName, + ...(nextContentType ? { type: nextContentType } : {}), } } diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-retype.test.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-retype.test.ts new file mode 100644 index 00000000000..d296eb928cf --- /dev/null +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-retype.test.ts @@ -0,0 +1,135 @@ +/** + * @vitest-environment node + */ +import { dbChainMockFns, resetDbChainMock } from '@sim/testing' +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockFileNameExistsInWorkspaceFolder } = vi.hoisted(() => ({ + mockFileNameExistsInWorkspaceFolder: vi.fn(), +})) + +vi.mock('@/lib/uploads', () => ({ + getServePathPrefix: vi.fn(() => '/api/files/serve/'), +})) + +vi.mock('@/lib/uploads/contexts/workspace/workspace-file-folder-manager', () => ({ + assertWorkspaceFileFolderTarget: vi.fn(async () => null), + buildWorkspaceFileFolderPathMap: vi.fn(() => new Map()), + fileNameExistsInWorkspaceFolder: mockFileNameExistsInWorkspaceFolder, + findWorkspaceFileFolderIdByPath: vi.fn(), + getWorkspaceFileFolderPath: vi.fn(), + listWorkspaceFileFolders: vi.fn(async () => []), + normalizeWorkspaceFileItemName: vi.fn((name: string) => name), +})) + +import { renameWorkspaceFile } from '@/lib/uploads/contexts/workspace/workspace-file-manager' + +const WORKSPACE_ID = 'ws_123' +const FILE_ID = 'ec28e5d5-898a-48f0-aa6f-2fd7427c9563' + +/** A `workspace_files` row as drizzle returns it, before the DTO mapping. */ +function makeRow(overrides: Record = {}) { + return { + id: FILE_ID, + workspaceId: WORKSPACE_ID, + originalName: 'untitled.md', + key: `workspace/${WORKSPACE_ID}/1738000000000-a3f9k2b-untitled.md`, + size: 0, + contentType: 'text/markdown', + uploadedBy: 'user_123', + folderId: null, + context: 'workspace', + deletedAt: null, + uploadedAt: new Date('2026-04-13T00:00:00.000Z'), + updatedAt: new Date('2026-04-13T00:00:00.000Z'), + ...overrides, + } +} + +/** The column values handed to the single `db.update(...).set(...)` a rename performs. */ +function lastUpdateSet(): Record { + const calls = dbChainMockFns.set.mock.calls + expect(calls.length).toBeGreaterThan(0) + return calls[calls.length - 1][0] as Record +} + +afterAll(resetDbChainMock) + +describe('renameWorkspaceFile — retype', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mockFileNameExistsInWorkspaceFolder.mockResolvedValue(false) + // The lone select is getWorkspaceFile's `.limit(1)` read of the current row. + dbChainMockFns.limit.mockResolvedValue([makeRow()]) + dbChainMockFns.returning.mockResolvedValue([{ id: FILE_ID }]) + }) + + it('writes the new contentType alongside the new name', async () => { + const file = await renameWorkspaceFile(WORKSPACE_ID, FILE_ID, 'untitled.json', { + contentType: 'application/json', + }) + + expect(lastUpdateSet()).toMatchObject({ + originalName: 'untitled.json', + contentType: 'application/json', + }) + expect(file).toMatchObject({ name: 'untitled.json', type: 'application/json' }) + }) + + it('never touches contentUpdatedAt — it is the collab persist concurrency token', async () => { + await renameWorkspaceFile(WORKSPACE_ID, FILE_ID, 'untitled.json', { + contentType: 'application/json', + }) + + expect(lastUpdateSet()).not.toHaveProperty('contentUpdatedAt') + }) + + it('leaves contentType alone on a plain rename', async () => { + const file = await renameWorkspaceFile(WORKSPACE_ID, FILE_ID, 'notes.md') + + expect(lastUpdateSet()).not.toHaveProperty('contentType') + expect(file).toMatchObject({ name: 'notes.md', type: 'text/markdown' }) + }) + + it('still writes when only the contentType changes', async () => { + dbChainMockFns.limit.mockResolvedValue([makeRow({ originalName: 'config.yaml' })]) + + const file = await renameWorkspaceFile(WORKSPACE_ID, FILE_ID, 'config.yaml', { + contentType: 'application/x-yaml', + }) + + expect(lastUpdateSet()).toMatchObject({ contentType: 'application/x-yaml' }) + expect(file.type).toBe('application/x-yaml') + }) + + it('skips the conflict probe when the name is unchanged', async () => { + dbChainMockFns.limit.mockResolvedValue([makeRow({ originalName: 'config.yaml' })]) + + await renameWorkspaceFile(WORKSPACE_ID, FILE_ID, 'config.yaml', { + contentType: 'application/x-yaml', + }) + + expect(mockFileNameExistsInWorkspaceFolder).not.toHaveBeenCalled() + }) + + it('short-circuits when neither the name nor the type changes', async () => { + const file = await renameWorkspaceFile(WORKSPACE_ID, FILE_ID, 'untitled.md', { + contentType: 'text/markdown', + }) + + expect(dbChainMockFns.set).not.toHaveBeenCalled() + expect(file).toMatchObject({ name: 'untitled.md', type: 'text/markdown' }) + }) + + it('rejects a retype whose new name is already taken', async () => { + mockFileNameExistsInWorkspaceFolder.mockResolvedValue(true) + + await expect( + renameWorkspaceFile(WORKSPACE_ID, FILE_ID, 'untitled.json', { + contentType: 'application/json', + }) + ).rejects.toThrow(/untitled\.json/) + expect(dbChainMockFns.set).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/uploads/utils/file-utils.ts b/apps/sim/lib/uploads/utils/file-utils.ts index da2c51c0f91..29740b3a1fd 100644 --- a/apps/sim/lib/uploads/utils/file-utils.ts +++ b/apps/sim/lib/uploads/utils/file-utils.ts @@ -312,6 +312,7 @@ const EXTENSION_TO_MIME: Record = { xls: 'application/vnd.ms-excel', ppt: 'application/vnd.ms-powerpoint', md: 'text/markdown', + mmd: 'text/x-mermaid', yaml: 'application/x-yaml', yml: 'application/x-yaml', rtf: 'application/rtf', @@ -554,6 +555,7 @@ const MIME_TO_EXTENSION: Record = { 'application/vnd.ms-excel': 'xls', 'application/vnd.ms-powerpoint': 'ppt', 'text/markdown': 'md', + 'text/x-mermaid': 'mmd', 'application/rtf': 'rtf', // Audio diff --git a/apps/sim/lib/uploads/utils/text-file-types.test.ts b/apps/sim/lib/uploads/utils/text-file-types.test.ts new file mode 100644 index 00000000000..b028b657110 --- /dev/null +++ b/apps/sim/lib/uploads/utils/text-file-types.test.ts @@ -0,0 +1,136 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils' +import { + findTextFileTypeByExtension, + findTextFileTypeById, + resolveTextFileType, + SELECTABLE_TEXT_FILE_TYPES, + SELECTABLE_TEXT_MIME_TYPES, + type TextFileType, + withFileTypeExtension, +} from '@/lib/uploads/utils/text-file-types' +import { resolveFileCategory } from '@/app/workspace/[workspaceId]/files/components/file-viewer/file-category' + +function typeById(id: string): TextFileType { + const type = findTextFileTypeById(id) + if (!type) throw new Error(`Unknown test fixture type: ${id}`) + return type +} + +describe('SELECTABLE_TEXT_FILE_TYPES invariants', () => { + it('gives every entry a unique extension', () => { + const extensions = SELECTABLE_TEXT_FILE_TYPES.map((type) => type.extension) + expect(new Set(extensions).size).toBe(extensions.length) + }) + + it('gives every entry a unique id', () => { + const ids = SELECTABLE_TEXT_FILE_TYPES.map((type) => type.id) + expect(new Set(ids).size).toBe(ids.length) + }) + + it.each(SELECTABLE_TEXT_FILE_TYPES)( + 'resolves $label to the shared extension-to-MIME map', + (type) => { + expect(getMimeTypeFromExtension(type.extension)).toBe(type.mimeType) + } + ) + + it.each(SELECTABLE_TEXT_FILE_TYPES)('opens $label in the text editor', (type) => { + expect(resolveFileCategory(type.mimeType, `example.${type.extension}`)).toBe('text-editable') + }) + + it('exposes every entry MIME in the allowlist', () => { + for (const type of SELECTABLE_TEXT_FILE_TYPES) { + expect(SELECTABLE_TEXT_MIME_TYPES).toContain(type.mimeType) + } + }) + + it('deduplicates MIMEs shared by several entries in the allowlist', () => { + expect(new Set(SELECTABLE_TEXT_MIME_TYPES).size).toBe(SELECTABLE_TEXT_MIME_TYPES.length) + }) +}) + +describe('withFileTypeExtension', () => { + it('swaps a simple extension', () => { + expect(withFileTypeExtension('notes.md', typeById('json'))).toBe('notes.json') + }) + + it('swaps only the last segment of a multi-dot name', () => { + expect(withFileTypeExtension('report.final.md', typeById('csv'))).toBe('report.final.csv') + }) + + it('appends to a name with no extension', () => { + expect(withFileTypeExtension('notes', typeById('json'))).toBe('notes.json') + }) + + it('appends to a leading-dot name rather than consuming it', () => { + expect(withFileTypeExtension('.gitignore', typeById('text'))).toBe('.gitignore.txt') + }) + + it('appends to a trailing-dot name rather than producing a double dot', () => { + expect(withFileTypeExtension('notes.', typeById('markdown'))).toBe('notes..md') + }) + + it('preserves the base name case while writing a lowercase extension', () => { + expect(withFileTypeExtension('NOTES.MD', typeById('json'))).toBe('NOTES.json') + }) + + it('is a no-op in effect when the type is unchanged', () => { + expect(withFileTypeExtension('notes.md', typeById('markdown'))).toBe('notes.md') + }) +}) + +describe('resolveTextFileType', () => { + it('resolves from the extension', () => { + expect(resolveTextFileType('text/markdown', 'notes.md')?.id).toBe('markdown') + }) + + it('prefers the extension over a MIME shared by several types', () => { + expect(resolveTextFileType('text/typescript', 'component.tsx')?.id).toBe('tsx') + expect(resolveTextFileType('text/typescript', 'client.ts')?.id).toBe('typescript') + }) + + it('prefers the extension when a stale MIME disagrees with it', () => { + expect(resolveTextFileType('text/markdown', 'data.csv')?.id).toBe('csv') + }) + + it('falls back to the MIME when the extension is unknown', () => { + expect(resolveTextFileType('application/json', 'payload.unknownext')?.id).toBe('json') + }) + + it('falls back to the MIME when the name has no extension', () => { + expect(resolveTextFileType('text/plain', 'README')?.id).toBe('text') + }) + + it('is case-insensitive on the extension', () => { + expect(resolveTextFileType(null, 'NOTES.MD')?.id).toBe('markdown') + }) + + it('returns null for a type outside the registry', () => { + expect(resolveTextFileType('application/pdf', 'report.pdf')).toBeNull() + expect(resolveTextFileType(null, 'photo.png')).toBeNull() + expect(resolveTextFileType(null, 'README')).toBeNull() + }) +}) + +describe('findTextFileTypeByExtension', () => { + it('resolves a known extension', () => { + expect(findTextFileTypeByExtension('yaml')?.mimeType).toBe('application/x-yaml') + }) + + it('is case-insensitive', () => { + expect(findTextFileTypeByExtension('JSON')?.id).toBe('json') + }) + + it('returns null for an alias the registry never writes', () => { + expect(findTextFileTypeByExtension('yml')).toBeNull() + }) + + it('returns null for an unknown extension', () => { + expect(findTextFileTypeByExtension('pdf')).toBeNull() + expect(findTextFileTypeByExtension('')).toBeNull() + }) +}) diff --git a/apps/sim/lib/uploads/utils/text-file-types.ts b/apps/sim/lib/uploads/utils/text-file-types.ts new file mode 100644 index 00000000000..d931da2141f --- /dev/null +++ b/apps/sim/lib/uploads/utils/text-file-types.ts @@ -0,0 +1,154 @@ +import { getFileExtension } from '@/lib/uploads/utils/file-utils' + +/** + * A file type the user can switch a workspace file to from the file-detail header. + * + * Every entry is text-editable — see `resolveFileCategory` — so switching between them only + * rewrites the name's extension and the stored `contentType`. The bytes are never touched and no + * binary format is selectable, which is what keeps a "change type" action from being a lossy + * format conversion in disguise. + */ +export interface TextFileType { + /** Stable key, and the value the header's radio group selects on. */ + id: string + /** Shown in the type picker, and used as the label in the file list's Type column. */ + label: string + /** Canonical extension, no leading dot. Unique across the registry. */ + extension: string + /** Not unique — `ts` and `tsx` are both `text/typescript`. See {@link resolveTextFileType}. */ + mimeType: string + /** Which submenu group the type is offered in. */ + group: 'document' | 'code' +} + +/** + * The selectable types, in menu order. Every `mimeType` here is taken from `EXTENSION_TO_MIME` in + * `file-utils.ts` rather than invented, so a retype produces a name and type the shared upload + * helpers already agree on. + * + * Code extensions that `EXTENSION_TO_MIME` does not know (`fish`, `graphql`, `dockerfile`, + * `makefile`, `mdx`, …) are deliberately absent: they resolve to `application/octet-stream`, which + * would store a file the viewer then refuses to open. + */ +export const SELECTABLE_TEXT_FILE_TYPES = [ + { + id: 'markdown', + label: 'Markdown', + extension: 'md', + mimeType: 'text/markdown', + group: 'document', + }, + { id: 'text', label: 'Text', extension: 'txt', mimeType: 'text/plain', group: 'document' }, + { id: 'csv', label: 'CSV', extension: 'csv', mimeType: 'text/csv', group: 'document' }, + { id: 'json', label: 'JSON', extension: 'json', mimeType: 'application/json', group: 'document' }, + { + id: 'yaml', + label: 'YAML', + extension: 'yaml', + mimeType: 'application/x-yaml', + group: 'document', + }, + { id: 'html', label: 'HTML', extension: 'html', mimeType: 'text/html', group: 'document' }, + { id: 'xml', label: 'XML', extension: 'xml', mimeType: 'application/xml', group: 'document' }, + { + id: 'mermaid', + label: 'Mermaid', + extension: 'mmd', + mimeType: 'text/x-mermaid', + group: 'document', + }, + { id: 'svg', label: 'SVG', extension: 'svg', mimeType: 'image/svg+xml', group: 'document' }, + + { + id: 'typescript', + label: 'TypeScript', + extension: 'ts', + mimeType: 'text/typescript', + group: 'code', + }, + { id: 'tsx', label: 'TSX', extension: 'tsx', mimeType: 'text/typescript', group: 'code' }, + { + id: 'javascript', + label: 'JavaScript', + extension: 'js', + mimeType: 'text/javascript', + group: 'code', + }, + { id: 'jsx', label: 'JSX', extension: 'jsx', mimeType: 'text/javascript', group: 'code' }, + { id: 'python', label: 'Python', extension: 'py', mimeType: 'text/x-python', group: 'code' }, + { id: 'go', label: 'Go', extension: 'go', mimeType: 'text/x-go', group: 'code' }, + { id: 'rust', label: 'Rust', extension: 'rs', mimeType: 'text/x-rust', group: 'code' }, + { id: 'java', label: 'Java', extension: 'java', mimeType: 'text/x-java', group: 'code' }, + { id: 'kotlin', label: 'Kotlin', extension: 'kt', mimeType: 'text/x-kotlin', group: 'code' }, + { id: 'swift', label: 'Swift', extension: 'swift', mimeType: 'text/x-swift', group: 'code' }, + { id: 'c', label: 'C', extension: 'c', mimeType: 'text/x-c', group: 'code' }, + { id: 'cpp', label: 'C++', extension: 'cpp', mimeType: 'text/x-c++', group: 'code' }, + { id: 'csharp', label: 'C#', extension: 'cs', mimeType: 'text/x-csharp', group: 'code' }, + { id: 'ruby', label: 'Ruby', extension: 'rb', mimeType: 'text/x-ruby', group: 'code' }, + { id: 'php', label: 'PHP', extension: 'php', mimeType: 'text/x-php', group: 'code' }, + { id: 'shell', label: 'Shell', extension: 'sh', mimeType: 'text/x-shellscript', group: 'code' }, + { id: 'sql', label: 'SQL', extension: 'sql', mimeType: 'text/x-sql', group: 'code' }, + { id: 'toml', label: 'TOML', extension: 'toml', mimeType: 'text/x-toml', group: 'code' }, + { id: 'css', label: 'CSS', extension: 'css', mimeType: 'text/css', group: 'code' }, + { id: 'scss', label: 'SCSS', extension: 'scss', mimeType: 'text/x-scss', group: 'code' }, +] as const satisfies readonly TextFileType[] + +/** Every MIME a retype may store. The allowlist the rename contract validates against. */ +export const SELECTABLE_TEXT_MIME_TYPES: readonly string[] = Array.from( + new Set(SELECTABLE_TEXT_FILE_TYPES.map((type) => type.mimeType)) +) + +const TYPE_BY_ID = new Map( + SELECTABLE_TEXT_FILE_TYPES.map((type) => [type.id, type]) +) + +const TYPE_BY_EXTENSION = new Map( + SELECTABLE_TEXT_FILE_TYPES.map((type) => [type.extension, type]) +) + +/** First entry wins, so the MIME fallback resolves to the type listed first for a shared MIME. */ +const TYPE_BY_MIME = new Map() +for (const type of SELECTABLE_TEXT_FILE_TYPES) { + if (!TYPE_BY_MIME.has(type.mimeType)) TYPE_BY_MIME.set(type.mimeType, type) +} + +export function findTextFileTypeById(id: string): TextFileType | null { + return TYPE_BY_ID.get(id) ?? null +} + +export function findTextFileTypeByExtension(extension: string): TextFileType | null { + return TYPE_BY_EXTENSION.get(extension.toLowerCase()) ?? null +} + +/** + * The registry entry a file currently is, or null when it is not one of the selectable types. + * + * Extension first, MIME second — the inverse of `resolveFileCategory`, and deliberately so. The + * extension is the registry's unique key and always resolves a single entry; the MIME does not + * (`ts` and `tsx` are both `text/typescript`, and `sh` stands in for `bash`/`zsh`). Since the + * server keeps name and `contentType` in agreement on every write, the two orders can only differ + * for older rows where a rename left them diverged — and there the extension is the one the user + * can actually see. + */ +export function resolveTextFileType( + mimeType: string | null | undefined, + filename: string +): TextFileType | null { + const byExtension = findTextFileTypeByExtension(getFileExtension(filename)) + if (byExtension) return byExtension + return mimeType ? (TYPE_BY_MIME.get(mimeType) ?? null) : null +} + +/** + * Swaps a file name's extension for `type`'s. + * + * The last dot wins, matching `withCopySuffix` in the workspace file manager, so + * `report.final.md` becomes `report.final.json` rather than `report.json`. A name with no + * extension, a leading-dot name, or a trailing-dot name gains the extension instead of losing a + * segment: `notes` becomes `notes.json`, and `.gitignore` becomes `.gitignore.json`. + */ +export function withFileTypeExtension(name: string, type: TextFileType): string { + const lastDot = name.lastIndexOf('.') + const hasExtension = lastDot > 0 && lastDot < name.length - 1 + return `${hasExtension ? name.slice(0, lastDot) : name}.${type.extension}` +} diff --git a/apps/sim/lib/workspace-files/orchestration/file-folder-lifecycle.ts b/apps/sim/lib/workspace-files/orchestration/file-folder-lifecycle.ts index 4abfd980bcf..5cdf48e5381 100644 --- a/apps/sim/lib/workspace-files/orchestration/file-folder-lifecycle.ts +++ b/apps/sim/lib/workspace-files/orchestration/file-folder-lifecycle.ts @@ -71,6 +71,11 @@ export interface PerformRenameWorkspaceFileParams { fileId: string name: string userId: string + /** + * Present only when the caller is changing the file's type, in which case `name` already carries + * the matching extension — the contract enforces that pairing before this is reached. + */ + contentType?: string } export interface PerformRenameWorkspaceFileResult { @@ -284,12 +289,12 @@ export async function performMoveWorkspaceFileItems( export async function performRenameWorkspaceFile( params: PerformRenameWorkspaceFileParams ): Promise { - const { workspaceId, fileId, name, userId } = params + const { workspaceId, fileId, name, userId, contentType } = params try { - const file = await renameWorkspaceFile(workspaceId, fileId, name) + const file = await renameWorkspaceFile(workspaceId, fileId, name, { contentType }) - logger.info('Renamed workspace file', { workspaceId, fileId, name: file.name }) + logger.info('Renamed workspace file', { workspaceId, fileId, name: file.name, contentType }) recordAudit({ workspaceId, @@ -298,7 +303,9 @@ export async function performRenameWorkspaceFile( resourceType: AuditResourceType.FILE, resourceId: fileId, resourceName: file.name, - description: `Renamed file to "${file.name}"`, + description: contentType + ? `Changed file type to "${file.type}" and renamed to "${file.name}"` + : `Renamed file to "${file.name}"`, }) await notifyWorkspaceFilesChanged(workspaceId) From 3131f78e3285ed72fc2e0faa413a1c3b33a800e4 Mon Sep 17 00:00:00 2001 From: Marcus Chandra Date: Thu, 6 Aug 2026 18:11:28 -0700 Subject: [PATCH 2/9] fix(files): stop a content save from resurrecting a stale contentType updateWorkspaceFileContent read the row before taking the FOR UPDATE lock, then wrote that read's contentType back inside the transaction. A save overlapping a type change therefore restored the pre-change type, leaving the file named .txt while still stored as text/markdown. A content write carries no opinion about the file's type unless the caller says so, so the column is now only written when a contentType is supplied. The live-doc markdown gate reads the committed row for the same reason. --- .../workspace/workspace-file-manager.ts | 9 ++++- .../workspace-file-storage-accounting.test.ts | 39 +++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts index 11087e5a8c7..a2b72e52539 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts @@ -1389,7 +1389,12 @@ export async function updateWorkspaceFileContent( .set({ key: uploadResult.key, size: content.length, - contentType: nextContentType, + // Only written when the caller actually declared a type. `nextContentType` falls back to + // a read taken BEFORE this row was locked, so writing it unconditionally lets a content + // save that overlaps a retype resurrect the pre-retype type — the file ends up named + // `.txt` while still stored as `text/markdown`. A content write carries no opinion about + // the file's type unless it says so, so leave the committed value alone. + ...(contentType ? { contentType } : {}), // Replaced bytes: drop the old image's dimensions so the row never describes stale content. // The next view reserves nothing (the baseline first-load reflow) rather than a wrong-sized // box, then the browser's measurement backfills the correct value. No server-side decode here @@ -1479,7 +1484,7 @@ export async function updateWorkspaceFileContent( // persist and empty-shell creates pass `syncLiveDoc: false` to stay out of it. if ( options?.syncLiveDoc !== false && - isMarkdownFile({ type: nextContentType, name: finalized.file.originalName }) + isMarkdownFile({ type: finalized.file.contentType, name: finalized.file.originalName }) ) { // Pass the new CONTENT version this write produced, so the relay records that its live doc now // incorporates this durable version — the collab persist's optimistic-concurrency guard then won't diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts index 23c2d3b6b37..8bed3175b4a 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts @@ -407,6 +407,45 @@ describe('workspace file metadata and storage accounting', () => { expect(dbChainMockFns.transaction).not.toHaveBeenCalled() }) + /** + * A content write reads the row BEFORE taking the FOR UPDATE lock. If it wrote that stale + * `contentType` back, a save overlapping a type change would resurrect the pre-change type and + * leave the file named `.txt` while stored as `text/markdown`. + */ + it('leaves contentType alone when the caller declares none', async () => { + const retypedFile = { ...FILE_ROW, originalName: 'note.md', contentType: 'text/markdown' } + dbChainMockFns.limit.mockResolvedValueOnce([FILE_ROW]).mockResolvedValueOnce([retypedFile]) + dbChainMockFns.returning.mockResolvedValueOnce([{ ...retypedFile, size: 10 }]) + mockUploadFile.mockResolvedValueOnce({ key: `${FILE_ROW.key}-replacement` }) + + const updated = await updateWorkspaceFileContent( + FILE_ROW.workspaceId, + FILE_ROW.id, + FILE_ROW.userId, + Buffer.alloc(10) + ) + + const written = dbChainMockFns.set.mock.calls.at(-1)?.[0] as Record + expect(written).not.toHaveProperty('contentType') + expect(updated.type).toBe('text/markdown') + }) + + it('writes contentType when the caller declares one', async () => { + dbChainMockFns.limit.mockResolvedValueOnce([FILE_ROW]).mockResolvedValueOnce([FILE_ROW]) + dbChainMockFns.returning.mockResolvedValueOnce([{ ...FILE_ROW, contentType: 'text/csv' }]) + mockUploadFile.mockResolvedValueOnce({ key: `${FILE_ROW.key}-replacement` }) + + await updateWorkspaceFileContent( + FILE_ROW.workspaceId, + FILE_ROW.id, + FILE_ROW.userId, + Buffer.alloc(10), + 'text/csv' + ) + + expect(dbChainMockFns.set.mock.calls.at(-1)?.[0]).toMatchObject({ contentType: 'text/csv' }) + }) + it('uploads an overwrite before atomically swapping the locked row and exact delta', async () => { const concurrentFile = { ...FILE_ROW, size: 7 } const replacementKey = `${FILE_ROW.key}-replacement` From 2a1452d037a99091f844a7d153a50d0674a97a6f Mon Sep 17 00:00:00 2001 From: Marcus Chandra Date: Thu, 6 Aug 2026 18:17:09 -0700 Subject: [PATCH 3/9] fix(files): nest both type groups so neither is hidden by the menu cap --- apps/sim/app/workspace/[workspaceId]/files/files.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/apps/sim/app/workspace/[workspaceId]/files/files.tsx b/apps/sim/app/workspace/[workspaceId]/files/files.tsx index 928b0074cf5..b782e02b3ee 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/files.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/files.tsx @@ -188,9 +188,16 @@ const MIME_TYPE_LABELS: Record = { 'application/vnd.openxmlformats-officedocument.presentationml.presentation': 'PowerPoint', } -/** Hoisted: derived from a frozen registry, so there is nothing to rebuild per render. */ +/** + * Both groups are nested behind their own submenu rather than one being listed inline. Menus cap at + * a fixed height, and a flat document list plus a Code entry overflows it — which would push the + * entry gating every code type out of sight on open. Two rows always fit. + * + * Hoisted: derived from a frozen registry, so there is nothing to rebuild per render. + */ const TEXT_FILE_TYPE_MENU_GROUPS: DropdownRadioGroup[] = [ { + submenuLabel: 'Document', items: SELECTABLE_TEXT_FILE_TYPES.filter((type) => type.group === 'document').map((type) => ({ id: type.id, label: type.label, From 3a06d38712baa41fdedf6931f954344a23d162e1 Mon Sep 17 00:00:00 2001 From: Marcus Chandra Date: Thu, 6 Aug 2026 19:54:05 -0700 Subject: [PATCH 4/9] feat(files): flush the live doc before a retype reads its bytes back Changing a collaborative markdown file's type unmounts its editor and mounts one that reads the file's durable bytes. The relay owns durability for that document and persists on a 5s debounce, so the read raced the write and returned the content from before the last edits. The client cannot close this itself: its save path is disabled for a collaborative doc by design, and isDirty is pinned false. Adds a FLUSH/FLUSH_COMPLETE round trip so the client can ask the relay to project the document now and wait for the answer. flushPersist grows a mode and returns an outcome: only a debounced flush may be coalesced away by the cross-task dedup window, because a deduped no-op acked as success would ship exactly the staleness this closes. The client wait is bounded well under the persist budget and a lapsed wait proceeds with the rename rather than blocking the user. --- apps/realtime/src/handlers/file-doc.test.ts | 114 ++++++++++++++++ apps/realtime/src/handlers/file-doc.ts | 126 +++++++++++++++--- .../collaboration/file-doc-provider.test.ts | 80 +++++++++++ .../collaboration/file-doc-provider.ts | 51 +++++++ .../collaboration/file-doc-room-context.tsx | 74 +++++++++- .../use-file-doc-collaboration.ts | 7 +- .../workspace/[workspaceId]/files/files.tsx | 28 +++- .../realtime-protocol/src/file-doc.test.ts | 8 ++ packages/realtime-protocol/src/file-doc.ts | 55 +++++++- 9 files changed, 509 insertions(+), 34 deletions(-) diff --git a/apps/realtime/src/handlers/file-doc.test.ts b/apps/realtime/src/handlers/file-doc.test.ts index 938092d4484..5b53aab3401 100644 --- a/apps/realtime/src/handlers/file-doc.test.ts +++ b/apps/realtime/src/handlers/file-doc.test.ts @@ -368,6 +368,120 @@ describe('setupWorkspaceFileDocHandlers', () => { expect(mockFetchFileDocPersist).toHaveBeenCalled() }) + describe('FLUSH', () => { + /** Joins, seeds, and lands one real user edit so the room is `edited` and worth persisting. */ + async function joinAndEdit(handlers: Record) { + await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 }) + await flushMicrotasks() + const edit = new Y.Doc() + edit.getText(FILE_DOC_FIELD).insert(0, 'user typed this') + handlers[FILE_DOC_EVENTS.MESSAGE]( + frame(FILE_DOC_MESSAGE_TYPE.SYNC, (e) => + syncProtocol.writeUpdate(e, Y.encodeStateAsUpdate(edit)) + ) + ) + await flushMicrotasks() + } + + function flushAcks(socket: { emit: ReturnType }) { + return socket.emit.mock.calls + .filter((call: unknown[]) => call[0] === FILE_DOC_EVENTS.FLUSH_COMPLETE) + .map((call: unknown[]) => call[1]) + } + + it('persists immediately and acks with the resulting version', async () => { + mockFetchFileDocSeed.mockResolvedValue(seedResult('# From server')) + mockFetchFileDocPersist.mockResolvedValue({ status: 'persisted', version: 77 }) + const { io } = createIo() + const { handlers, socket } = setup('socket-1', io) + await joinAndEdit(handlers) + + await handlers[FILE_DOC_EVENTS.FLUSH]({ fileId: 'file-1' }) + + expect(mockFetchFileDocPersist).toHaveBeenCalled() + expect(flushAcks(socket)).toEqual([{ fileId: 'file-1', status: 'persisted', version: 77 }]) + }) + + it('acks unchanged — never persisted — for a doc nobody edited', async () => { + mockFetchFileDocSeed.mockResolvedValue(seedResult('# From server')) + const { io } = createIo() + const { handlers, socket } = setup('socket-1', io) + await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 }) + await flushMicrotasks() + + await handlers[FILE_DOC_EVENTS.FLUSH]({ fileId: 'file-1' }) + + // Projecting an unedited seed back over the file is the clobber the `edited` gate exists to + // prevent, so a flush must not force one. + expect(mockFetchFileDocPersist).not.toHaveBeenCalled() + expect(flushAcks(socket)).toEqual([{ fileId: 'file-1', status: 'unchanged' }]) + }) + + it('acks skipped — not persisted — when the durable file advanced out-of-band', async () => { + mockFetchFileDocSeed.mockResolvedValue(seedResult('# From server')) + mockFetchFileDocPersist.mockResolvedValue({ status: 'conflict' }) + const { io } = createIo() + const { handlers, socket } = setup('socket-1', io) + await joinAndEdit(handlers) + + await handlers[FILE_DOC_EVENTS.FLUSH]({ fileId: 'file-1' }) + + // The caller must be able to tell this from a real write: the durable bytes are NOT current. + expect(flushAcks(socket)).toEqual([{ fileId: 'file-1', status: 'skipped' }]) + }) + + it('acks skipped when the persist request fails', async () => { + mockFetchFileDocSeed.mockResolvedValue(seedResult('# From server')) + mockFetchFileDocPersist.mockRejectedValue(new Error('app unreachable')) + const { io } = createIo() + const { handlers, socket } = setup('socket-1', io) + await joinAndEdit(handlers) + + await handlers[FILE_DOC_EVENTS.FLUSH]({ fileId: 'file-1' }) + + expect(flushAcks(socket)).toEqual([{ fileId: 'file-1', status: 'skipped' }]) + }) + + it('cancels the pending debounce so no redundant second write follows', async () => { + mockFetchFileDocSeed.mockResolvedValue(seedResult('# From server')) + mockFetchFileDocPersist.mockResolvedValue({ status: 'persisted', version: 5 }) + vi.useFakeTimers() + try { + const { io } = createIo() + const { handlers } = setup('socket-1', io) + await joinAndEdit(handlers) + // The edit armed the debounce; the flush must disarm it. + await handlers[FILE_DOC_EVENTS.FLUSH]({ fileId: 'file-1' }) + expect(mockFetchFileDocPersist).toHaveBeenCalledTimes(1) + + await vi.advanceTimersByTimeAsync(30_000) + expect(mockFetchFileDocPersist).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + } + }) + + it('refuses a flush for a file this socket never joined', async () => { + mockFetchFileDocSeed.mockResolvedValue(seedResult('# From server')) + const { io } = createIo() + const { handlers, socket } = setup('socket-1', io) + await joinAndEdit(handlers) + + await handlers[FILE_DOC_EVENTS.FLUSH]({ fileId: 'someone-elses-file' }) + + // Membership IS the authorization here — a socket must not force a write to another document. + expect(mockFetchFileDocPersist).not.toHaveBeenCalled() + expect(flushAcks(socket)).toEqual([{ fileId: 'someone-elses-file', status: 'unchanged' }]) + }) + + it('ignores a payload with no fileId', async () => { + const { io } = createIo() + const { handlers, socket } = setup('socket-1', io) + await handlers[FILE_DOC_EVENTS.FLUSH]({}) + expect(flushAcks(socket)).toEqual([]) + }) + }) + it('drops document frames and evicts once the editor loses write access mid-session', async () => { // The join-time check is not a standing right: a collaborator downgraded to `read` // (or removed) must stop landing durable edits on the socket they already hold. diff --git a/apps/realtime/src/handlers/file-doc.ts b/apps/realtime/src/handlers/file-doc.ts index a0152fd85f6..dc578ef7225 100644 --- a/apps/realtime/src/handlers/file-doc.ts +++ b/apps/realtime/src/handlers/file-doc.ts @@ -31,6 +31,8 @@ import { FILE_DOC_SEED, FILE_DOC_TIMEOUTS, type FileDocPresenceUser, + type FlushFileDocPayload, + type FlushFileDocResult, type JoinFileDocPayload, type LeaveFileDocPayload, toFileDocBytes, @@ -260,27 +262,52 @@ function schedulePersist(name: string, room: FileDocRoom): void { room.persistTimer = setTimeout(() => { room.persistTimer = null room.persistDeadline = null - void flushPersist(name, room, false) + void flushPersist(name, room, 'debounced') }, delay) } /** - * Project the live doc to markdown and write it durably via the app. `final` (last collaborator - * leaving) always writes; a debounced mid-edit flush first claims a best-effort cross-task dedup WINDOW - * (a TTL key that just expires, so at most ~one persist per window cluster-wide) so concurrent tasks - * editing the same file don't each write a redundant blob version. Best-effort: never throws (a failure + * Why a flush is running. Only `debounced` is subject to the cross-task dedup window: the other two + * have a waiter that would mistake a deduped no-op for a completed write. + * + * - `debounced` — the mid-edit timer fired. Coalescable, nobody is waiting. + * - `final` — last collaborator leaving, or shutdown. Last chance before teardown. + * - `requested` — a client asked for it and is waiting on the outcome ({@link FILE_DOC_EVENTS.FLUSH}). + */ +type FlushMode = 'debounced' | 'final' | 'requested' + +/** What a {@link flushPersist} call actually did. Mirrors {@link FlushFileDocResult}'s status. */ +type FlushPersistOutcome = + | { status: 'persisted'; version: number } + | { status: 'unchanged' } + | { status: 'skipped' } + +/** + * Project the live doc to markdown and write it durably via the app. A `debounced` mid-edit flush + * first claims a best-effort cross-task dedup WINDOW (a TTL key that just expires, so at most ~one + * persist per window cluster-wide) so concurrent tasks editing the same file don't each write a + * redundant blob version; `final` and `requested` always write. Best-effort: never throws (a failure * is retried on the next debounce; the stream holds the state meanwhile). * + * Returns what actually happened so a `requested` flush can be acked truthfully — several paths here + * complete having written nothing, and a caller that treats "returned" as "persisted" would ship + * exactly the staleness the flush exists to prevent. + * * Persists the AUTHORITATIVE shared state (the stream), not this task's local doc: a copilot merge — or * a peer's edit — published by another task may not be integrated into `room.doc` yet (and the stream * holds content even when THIS task's doc was never locally seeded), so a last-disconnect flush can't * clobber the durable file with a lagging projection. The local doc is captured SYNCHRONOUSLY as a - * fallback before any await, so a `void flushPersist(name, room, true)` fired immediately before the + * fallback before any await, so a `void flushPersist(name, room, 'final')` fired immediately before the * caller destroys `room.doc` never encodes a destroyed doc, and the disabled path stays authoritative. */ -async function flushPersist(name: string, room: FileDocRoom, final: boolean): Promise { +async function flushPersist( + name: string, + room: FileDocRoom, + mode: FlushMode +): Promise { // Never project a doc no user actually edited back over the file (see {@link FileDocRoom.edited}). - if (!room.edited || !room.workspaceId || !room.lastEditorUserId) return + // Nothing to write is not a failure — the durable content is already current. + if (!room.edited || !room.workspaceId || !room.lastEditorUserId) return { status: 'unchanged' } const store = getFileDocStore() const workspaceId = room.workspaceId const userId = room.lastEditorUserId @@ -327,18 +354,24 @@ async function flushPersist(name: string, room: FileDocRoom, final: boolean): Pr } try { - if (!final && !(await store.tryClaimPersistWindow(name, FILE_DOC_TIMEOUTS.persistRequestMs))) - return + // Only a debounced flush may be coalesced away. A `requested` flush has a client waiting on the + // outcome, so losing the claim must not report back as a completed write. + if ( + mode === 'debounced' && + !(await store.tryClaimPersistWindow(name, FILE_DOC_TIMEOUTS.persistRequestMs)) + ) + return { status: 'skipped' } // The If-Match token: the durable content version the live doc is synced to. let ifMatch = await currentVersion() - // FINAL flush = last chance before teardown: if the version read momentarily fails (Redis blip) for a - // peer-seeded/tail-only task that never cached it, retry briefly rather than defer and strand the - // edits in the TTL'd stream (the version is cluster-wide + heartbeat-refreshed). Bounded — a genuinely - // unset version never appears, and the flush must not stall teardown. + // A flush with no second chance (last-leave teardown) or with a waiter (`requested`): if the version + // read momentarily fails (Redis blip) for a peer-seeded/tail-only task that never cached it, retry + // briefly rather than defer and strand the edits in the TTL'd stream (the version is cluster-wide + + // heartbeat-refreshed). Bounded — a genuinely unset version never appears, the flush must not stall + // teardown, and 2x100ms stays far inside the client's flush budget. for ( let i = 0; - ifMatch === undefined && final && store.enabled && i < FINAL_VERSION_RETRIES; + ifMatch === undefined && mode !== 'debounced' && store.enabled && i < FINAL_VERSION_RETRIES; i++ ) { await sleep(FINAL_VERSION_RETRY_MS) @@ -349,19 +382,21 @@ async function flushPersist(name: string, room: FileDocRoom, final: boolean): Pr // still at the version the live doc synced from, so a projection can never silently clobber an // out-of-band edit. A single attempt — on conflict we STOP rather than retry (see below). const docState = await captureState() - if (!docState) return // nothing seeded/authoritative to persist yet + // Nothing seeded/authoritative to persist yet. + if (!docState) return { status: 'skipped' } const result = await fetchFileDocPersist(workspaceId, room.fileId, userId, docState, ifMatch) - if (result.status === 'missing') return // the file was deleted; nothing to write + // The file was deleted; nothing to write. + if (result.status === 'missing') return { status: 'skipped' } if (result.status === 'deferred') { // No version token available (momentarily — a Redis blip on a peer-seeded task). Leave the edits in // the stream; a later persist writes them once the version is re-established. logger.warn(`Persist deferred for file ${room.fileId} (no synced version available yet)`) - return + return { status: 'skipped' } } if (result.status === 'persisted') { room.syncedVersion = Math.max(room.syncedVersion ?? 0, result.version) void store.setSyncedVersion(name, result.version) - return + return { status: 'persisted', version: result.version } } // status === 'conflict': the durable file advanced out-of-band since our If-Match token. We do NOT // re-persist against the current stream: an external write commits durable BEFORE its chokepoint merge @@ -375,8 +410,10 @@ async function flushPersist(name: string, room: FileDocRoom, final: boolean): Pr logger.warn( `Persist conflict for file ${room.fileId}; durable content advanced out-of-band, left authoritative` ) + return { status: 'skipped' } } catch (error) { logger.warn(`Persist failed for file ${room.fileId}`, { error: getErrorMessage(error) }) + return { status: 'skipped' } } } @@ -446,7 +483,7 @@ function destroyRoomIfIdle(name: string) { } // Final durable flush BEFORE teardown — `flushPersist` encodes the doc synchronously (before the // destroy below) and awaits the write in the background. Best-effort; never throws. - void flushPersist(name, room, true) + void flushPersist(name, room, 'final') getFileDocStore().detachRoom(name) room.awareness.destroy() room.doc.destroy() @@ -461,9 +498,9 @@ function destroyRoomIfIdle(name: string) { * process is exiting); only their durable state is secured. */ export async function flushAllFileDocRooms(): Promise { - const flushes: Promise[] = [] + const flushes: Promise[] = [] for (const [name, room] of fileDocRooms) { - if (room.edited) flushes.push(flushPersist(name, room, true)) + if (room.edited) flushes.push(flushPersist(name, room, 'final')) } await Promise.all(flushes) } @@ -1231,6 +1268,51 @@ export function setupWorkspaceFileDocHandlers( socket.on(FILE_DOC_EVENTS.MESSAGE, (data: unknown) => handleMessage(socket, io, data)) + /** + * Persist the live document now, ahead of the debounce, and report what happened. + * + * The membership check is the authorization: `socketToRoomName` is only populated by a join that + * already passed the room's permission middleware, and the payload's file must match the room this + * socket actually holds — so a socket cannot force a write to a document it never joined. + * + * The pending debounce is cancelled first. Leaving it armed would fire a second, redundant blob + * version moments after this one for content that is already durable. + */ + socket.on(FILE_DOC_EVENTS.FLUSH, async (payload?: FlushFileDocPayload) => { + const fileId = payload?.fileId + if (!fileId) return + const ack = (status: FlushFileDocResult['status'], version?: number) => { + socket.emit(FILE_DOC_EVENTS.FLUSH_COMPLETE, { + fileId, + status, + ...(version !== undefined ? { version } : {}), + } satisfies FlushFileDocResult) + } + + try { + const name = socketToRoomName.get(socket.id) + // Not in a room, or in a different file's room: nothing of this client's is unpersisted here. + // Acked as `unchanged` rather than left silent so the caller's wait always resolves. + if (!name || roomName(fileDocRoom(fileId)) !== name) return ack('unchanged') + const room = fileDocRooms.get(name) + if (!room) return ack('unchanged') + + if (room.persistTimer) { + clearTimeout(room.persistTimer) + room.persistTimer = null + } + room.persistDeadline = null + + const outcome = await flushPersist(name, room, 'requested') + ack(outcome.status, outcome.status === 'persisted' ? outcome.version : undefined) + } catch (error) { + logger.error('Error flushing file-doc room:', error) + // `flushPersist` never throws, so reaching here means the room lookup did. The write did not + // happen, and the caller must not read the ack as durable. + ack('skipped') + } + }) + socket.on(FILE_DOC_EVENTS.LEAVE, (payload?: LeaveFileDocPayload) => { try { // Cancel an in-flight join whose file the client is now leaving (or an unscoped leave): a diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.test.ts index 689c2ee6c77..15d4e7a81c0 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.test.ts @@ -5,6 +5,7 @@ import { FILE_DOC_EVENTS, FILE_DOC_MESSAGE_TYPE, FILE_DOC_SEED, + FILE_DOC_TIMEOUTS, } from '@sim/realtime-protocol/file-doc' import * as encoding from 'lib0/encoding' import type { Socket } from 'socket.io-client' @@ -429,3 +430,82 @@ describe('FileDocProvider', () => { } }) }) + +describe('FileDocProvider.flush', () => { + it('emits FLUSH and resolves with the server outcome', async () => { + const { provider, emit, fire } = createProvider() + + const pending = provider.flush() + expect(emit).toHaveBeenCalledWith(FILE_DOC_EVENTS.FLUSH, { fileId: 'file-1' }) + + fire(FILE_DOC_EVENTS.FLUSH_COMPLETE, { fileId: 'file-1', status: 'persisted', version: 42 }) + await expect(pending).resolves.toEqual({ + fileId: 'file-1', + status: 'persisted', + version: 42, + }) + provider.destroy() + }) + + it('ignores an ack for a different file', async () => { + vi.useFakeTimers() + try { + const { provider, fire } = createProvider() + const pending = provider.flush() + + fire(FILE_DOC_EVENTS.FLUSH_COMPLETE, { + fileId: 'other-file', + status: 'persisted', + version: 1, + }) + await vi.advanceTimersByTimeAsync(FILE_DOC_TIMEOUTS.flushRequestMs) + + // The foreign ack must not settle this waiter — only the timeout does. + await expect(pending).resolves.toEqual({ fileId: 'file-1', status: 'skipped' }) + provider.destroy() + } finally { + vi.useRealTimers() + } + }) + + it('resolves skipped when the ack never arrives', async () => { + vi.useFakeTimers() + try { + const { provider } = createProvider() + const pending = provider.flush() + await vi.advanceTimersByTimeAsync(FILE_DOC_TIMEOUTS.flushRequestMs) + await expect(pending).resolves.toEqual({ fileId: 'file-1', status: 'skipped' }) + provider.destroy() + } finally { + vi.useRealTimers() + } + }) + + it('settles a pending flush on destroy rather than stranding it', async () => { + const { provider } = createProvider() + const pending = provider.flush() + provider.destroy() + await expect(pending).resolves.toEqual({ fileId: 'file-1', status: 'skipped' }) + }) + + it('settles every concurrent waiter from one ack', async () => { + const { provider, fire } = createProvider() + const first = provider.flush() + const second = provider.flush() + + fire(FILE_DOC_EVENTS.FLUSH_COMPLETE, { fileId: 'file-1', status: 'unchanged' }) + + await expect(first).resolves.toMatchObject({ status: 'unchanged' }) + await expect(second).resolves.toMatchObject({ status: 'unchanged' }) + provider.destroy() + }) + + it('resolves skipped without emitting once destroyed', async () => { + const { provider, emit } = createProvider() + provider.destroy() + emit.mockClear() + + await expect(provider.flush()).resolves.toEqual({ fileId: 'file-1', status: 'skipped' }) + expect(emit).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.ts index 84bab0733bc..82315b80e64 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.ts @@ -7,6 +7,7 @@ import { FILE_DOC_MESSAGE_TYPE, FILE_DOC_SEED, FILE_DOC_TIMEOUTS, + type FlushFileDocResult, type JoinFileDocError, type JoinFileDocSuccess, toFileDocBytes, @@ -107,6 +108,8 @@ export class FileDocProvider extends ObservableV2 { joinError: JoinFileDocError | null = null private disposed = false + /** Resolvers for in-flight {@link flush} calls, so a single ack settles every concurrent waiter. */ + private pendingFlushes = new Set<(result: FlushFileDocResult) => void>() /** Set on a non-retryable join rejection (e.g. lost write access) so the * provider stops attempting to (re)join until the owner tears it down. */ private fatal = false @@ -136,6 +139,7 @@ export class FileDocProvider extends ObservableV2 { socket.on(FILE_DOC_EVENTS.MESSAGE, this.handleMessage) socket.on(FILE_DOC_EVENTS.JOIN_SUCCESS, this.handleJoinSuccess) socket.on(FILE_DOC_EVENTS.JOIN_ERROR, this.handleJoinError) + socket.on(FILE_DOC_EVENTS.FLUSH_COMPLETE, this.handleFlushComplete) socket.on(ROOM_ACCESS_REVOKED_EVENT, this.handleAccessRevoked) socket.on('connect', this.handleConnect) doc.on('update', this.handleDocUpdate) @@ -222,6 +226,49 @@ export class FileDocProvider extends ObservableV2 { this.sendLocalAwareness() } + private handleFlushComplete = (data: FlushFileDocResult) => { + if (data.fileId !== this.fileId) return + this.settleFlushes(data) + } + + private settleFlushes(result: FlushFileDocResult) { + if (this.pendingFlushes.size === 0) return + const waiters = [...this.pendingFlushes] + this.pendingFlushes.clear() + for (const resolve of waiters) resolve(result) + } + + /** + * Ask the server to project this document to durable markdown NOW and wait for the outcome. + * + * For a caller that is about to read the file's durable content back — changing the file's type + * swaps the editor, so this one unmounts and a plain-text editor fetches the stored bytes. Without + * this the read races the relay's 5s persist debounce and returns the pre-edit content. + * + * Always resolves, never rejects: the wait is bounded by {@link FILE_DOC_TIMEOUTS.flushRequestMs} + * and a lapsed wait resolves `skipped`. Callers must branch on `status` — `unchanged` and `skipped` + * both mean "the durable bytes may not include the latest edits", and only `persisted` guarantees + * they do. A timed-out flush is not cancelled server-side; it completes, just unobserved. + */ + flush(): Promise { + if (this.disposed || this.fatal) { + return Promise.resolve({ fileId: this.fileId, status: 'skipped' }) + } + return new Promise((resolve) => { + const settle = (result: FlushFileDocResult) => { + clearTimeout(timer) + this.pendingFlushes.delete(settle) + resolve(result) + } + const timer = setTimeout( + () => settle({ fileId: this.fileId, status: 'skipped' }), + FILE_DOC_TIMEOUTS.flushRequestMs + ) + this.pendingFlushes.add(settle) + this.socket.emit(FILE_DOC_EVENTS.FLUSH, { fileId: this.fileId }) + }) + } + /** * Handle a join rejection. A non-retryable rejection (access denied, invalid) * won't succeed on retry, so latch {@link fatal} to stop (re)joining and let the @@ -389,9 +436,13 @@ export class FileDocProvider extends ObservableV2 { if (releaseRoomMembership(this.socket, this.fileId)) { this.socket.emit(FILE_DOC_EVENTS.LEAVE, { fileId: this.fileId }) } + // Settle any waiter before the listener goes away, or a `flush()` awaited across a teardown would + // hang until its own timeout. `skipped` because this provider can no longer observe the outcome. + this.settleFlushes({ fileId: this.fileId, status: 'skipped' }) this.socket.off(FILE_DOC_EVENTS.MESSAGE, this.handleMessage) this.socket.off(FILE_DOC_EVENTS.JOIN_SUCCESS, this.handleJoinSuccess) this.socket.off(FILE_DOC_EVENTS.JOIN_ERROR, this.handleJoinError) + this.socket.off(FILE_DOC_EVENTS.FLUSH_COMPLETE, this.handleFlushComplete) this.socket.off(ROOM_ACCESS_REVOKED_EVENT, this.handleAccessRevoked) this.socket.off('connect', this.handleConnect) this.doc.off('update', this.handleDocUpdate) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context.tsx index 2afc31ae7f1..6db566b4593 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context.tsx @@ -1,6 +1,15 @@ 'use client' -import { createContext, type ReactNode, useContext, useState } from 'react' +import { + createContext, + type ReactNode, + type RefObject, + useContext, + useEffect, + useRef, + useState, +} from 'react' +import type { FlushFileDocResult } from '@sim/realtime-protocol/file-doc' import type { PresenceAvatarUser } from '@/app/workspace/[workspaceId]/components/presence/presence-avatars' const EMPTY_OTHERS: PresenceAvatarUser[] = [] @@ -13,6 +22,29 @@ const noop = () => {} const FileDocOthersContext = createContext(EMPTY_OTHERS) const FileDocSetOthersContext = createContext<(users: PresenceAvatarUser[]) => void>(noop) +/** + * The open document's flush, or `null` when nothing collaborative is mounted. + * + * Same publish-upward direction as the roster, and for the same reason: the editor owns the realtime + * provider but sits below the file-detail header that acts on it. Carried in a ref rather than state + * so publishing it never re-renders anything — it is only ever read from an event handler. + */ +export type FileDocFlush = () => Promise +const FileDocFlushContext = createContext<{ current: FileDocFlush | null }>({ current: null }) + +interface FileDocRoomProviderProps { + children: ReactNode + /** + * Optional ref for the open document's flush, owned by an ANCESTOR of this provider. + * + * `useFileDocFlush` only reaches descendants, which is the wrong shape for the file-detail page: + * it renders this provider itself, so its own handlers sit above the context. Passing the ref in + * lets that owner read the flush the editor publishes without being a descendant. Omit it and the + * provider keeps its own ref, so descendant-only consumers work unchanged. + */ + flushRef?: RefObject +} + /** * Scopes "who's in this file" presence to the open document — the `RoomProvider` + * `useOthers` pattern (Liveblocks / y-presence) adapted to our component tree. The editor @@ -21,15 +53,31 @@ const FileDocSetOthersContext = createContext<(users: PresenceAvatarUser[]) => v * ({@link useReportFileDocOthers}) and the header reads it ({@link useFileDocOthers}). * Presence is ephemeral and room-scoped, so it lives in this provider, not a global store. */ -export function FileDocRoomProvider({ children }: { children: ReactNode }) { +export function FileDocRoomProvider({ children, flushRef }: FileDocRoomProviderProps) { const [others, setOthers] = useState(EMPTY_OTHERS) + // A ref, not state: the flush is read at call time by an event handler, never rendered. Storing it + // in state would re-render the whole file detail every time a provider mounts or tears down. + const ownFlushRef = useRef(null) return ( - - {children} - + + + {children} + + ) } +/** + * Calls the flush behind `flushRef`, for an owner that passed its own ref to + * {@link FileDocRoomProvider}. Resolves `skipped` when nothing collaborative is mounted, so the + * caller needs no null check. + */ +export function flushFileDocRef( + flushRef: RefObject +): Promise { + return flushRef.current?.() ?? Promise.resolve({ fileId: '', status: 'skipped' as const }) +} + /** The roster of collaborators currently in the open file, for an avatar stack. Empty * outside a {@link FileDocRoomProvider}. */ export function useFileDocOthers(): PresenceAvatarUser[] { @@ -41,3 +89,19 @@ export function useFileDocOthers(): PresenceAvatarUser[] { export function useReportFileDocOthers(): (users: PresenceAvatarUser[]) => void { return useContext(FileDocSetOthersContext) } + +/** + * Publishes the open document's flush into the room context (editor side). Pass `null` on teardown + * so a consumer can never call into a destroyed provider. + */ +export function useReportFileDocFlush(flush: FileDocFlush | null): void { + const flushRef = useContext(FileDocFlushContext) + useEffect(() => { + flushRef.current = flush + return () => { + // Only clear if still ours: a remount can install the next provider's flush before this + // cleanup runs, and blindly nulling would drop the live one. + if (flushRef.current === flush) flushRef.current = null + } + }, [flush, flushRef]) +} diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts index 5afd9470a21..19b092e4482 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts @@ -7,7 +7,7 @@ import * as Y from 'yjs' import { getUserColor } from '@/lib/workspaces/colors' import { useSocket } from '@/app/workspace/providers/socket-provider' import { FileDocProvider } from './file-doc-provider' -import { useReportFileDocOthers } from './file-doc-room-context' +import { useReportFileDocFlush, useReportFileDocOthers } from './file-doc-room-context' /** The live collaboration binding the editor wires into TipTap's Collaboration * (the {@link Y.Doc}) and CollaborationCaret (the awareness). */ @@ -110,6 +110,11 @@ export function useFileDocCollaboration({ } }, [enabled, socket, fileId]) + // Publish this document's flush so the file-detail header can force durability before it reads the + // file's stored bytes back (changing the file's type unmounts this editor and swaps in one that + // reads them). Cleared with the provider, so it can never outlive the socket it wraps. + useReportFileDocFlush(useMemo(() => (provider ? () => provider.flush() : null), [provider])) + const reportOthers = useReportFileDocOthers() const reportOthersRef = useRef(reportOthers) reportOthersRef.current = reportOthers diff --git a/apps/sim/app/workspace/[workspaceId]/files/files.tsx b/apps/sim/app/workspace/[workspaceId]/files/files.tsx index b782e02b3ee..56b707847fe 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/files.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/files.tsx @@ -90,7 +90,11 @@ import { isTextEditable, } from '@/app/workspace/[workspaceId]/files/components/file-viewer' import { FileDocAvatars } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-avatars' -import { FileDocRoomProvider } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context' +import { + type FileDocFlush, + FileDocRoomProvider, + flushFileDocRef, +} from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context' import { FilesListContextMenu } from '@/app/workspace/[workspaceId]/files/components/files-list-context-menu' import { ShareModal } from '@/app/workspace/[workspaceId]/files/components/share-modal' import { useWorkspaceFilesRoom } from '@/app/workspace/[workspaceId]/files/hooks/use-workspace-files-room' @@ -249,6 +253,12 @@ function formatFileType(storedType: string | null, filename: string): string { export function Files() { const fileInputRef = useRef(null) const saveRef = useRef<(() => Promise) | null>(null) + /** + * The open collaborative document's flush, published by the editor through + * {@link FileDocRoomProvider}. Owned here rather than read via context because this component + * renders that provider, so its handlers sit above it. + */ + const fileDocFlushRef = useRef(null) const discardRef = useRef<(() => void) | null>(null) const params = useParams() @@ -1202,6 +1212,19 @@ export function Files() { * their text to survive — and the markdown editor unmounts the moment the file stops being * markdown, taking its pending debounce with it. Awaiting the save also orders the content write * ahead of the metadata write, so the two cannot race. + * + * Two different durability owners have to be settled, and only one of them is the client's: + * + * - An editor that owns its own durability autosaves, so flushing its pending debounce is enough. + * `isDirty` is meaningful there. + * - A COLLABORATIVE markdown document's durability belongs to the relay — the client's save path + * is disabled outright and `isDirty` is pinned false — so the just-typed text is only in the + * live doc. Without asking the relay to project it, the incoming editor reads the bytes from + * before the edits. Hence the flush. + * + * Both are best-effort and neither blocks the retype: a flush that cannot confirm durability still + * lets the rename proceed, because the alternative is refusing an action the user asked for over a + * display concern. The bytes are safe either way. */ const handleChangeFileType = useCallback( async (typeId: string) => { @@ -1215,6 +1238,7 @@ export function Files() { if (type.mimeType === file.type && nextName === file.name) return if (isDirtyRef.current) await saveRef.current?.() + await flushFileDocRef(fileDocFlushRef) const siblingNames = new Set( filesRef.current @@ -2153,7 +2177,7 @@ export function Files() { {/* The room provider scopes "who's in this file" presence to the open document: the editor (inside FileViewer) publishes the server-authenticated roster and the header's FileDocAvatars reads it — both must be descendants. */} - + { // read-only fallback, or a late-but-successful seed can never reach the client. expect(FILE_DOC_TIMEOUTS.seedRequestMs).toBeLessThan(FILE_DOC_TIMEOUTS.readinessDeadlineMs) }) + + it('keeps the client flush wait well under the write budget it triggers', () => { + // Deliberately NOT an "inner finishes before outer" invariant: the flush wait is an interaction + // budget and the persist is a durable write budget. The client gives up FIRST and continues; the + // write it started still completes server-side. Asserting the ordering pins that intent, so + // raising the flush wait to "cover" a slow persist is a conscious change, not a drift. + expect(FILE_DOC_TIMEOUTS.flushRequestMs).toBeLessThan(FILE_DOC_TIMEOUTS.persistRequestMs) + }) }) diff --git a/packages/realtime-protocol/src/file-doc.ts b/packages/realtime-protocol/src/file-doc.ts index 3e50cf097df..0072e6958ac 100644 --- a/packages/realtime-protocol/src/file-doc.ts +++ b/packages/realtime-protocol/src/file-doc.ts @@ -23,6 +23,15 @@ export const FILE_DOC_EVENTS = { JOIN_ERROR: 'join-file-doc-error', /** Client → server: leave the session ({@link LeaveFileDocPayload}). */ LEAVE: 'leave-file-doc', + /** + * Client → server: project the live document to durable markdown NOW, ahead of the debounce + * ({@link FlushFileDocPayload}). For a client that is about to stop being a collaborator on this + * file while STAYING on the page — changing the file's type swaps the editor, so the markdown + * editor unmounts — and needs the durable content to be current before it reads it back. + */ + FLUSH: 'flush-file-doc', + /** Server → client: the outcome of a {@link FILE_DOC_EVENTS.FLUSH} ({@link FlushFileDocResult}). */ + FLUSH_COMPLETE: 'flush-file-doc-complete', /** Both directions: a framed Yjs message (binary), tagged by {@link FILE_DOC_MESSAGE_TYPE}. */ MESSAGE: 'file-doc-message', /** @@ -102,10 +111,16 @@ export const FILE_DOC_SEED = { * The seed request gets more headroom than the merge because it reads a (possibly cold) blob before * converting; the merge is a pure in-memory conversion the caller fully supplies. * - * `persistRequestMs` (relay → app `/persist`) stands alone — no client waits on it (the relay flushes - * the live doc to durable markdown debounced during editing and on the last collaborator leaving), so - * it forms no ordering invariant. It gets seed-level headroom because, like the seed, it crosses a - * durable blob write (Yjs → markdown → storage), not just an in-memory conversion. + * `persistRequestMs` (relay → app `/persist`) forms no ordering invariant with the others: the relay + * flushes the live doc to durable markdown debounced during editing and on the last collaborator + * leaving, and neither path has a waiter. It gets seed-level headroom because, like the seed, it + * crosses a durable blob write (Yjs → markdown → storage), not just an in-memory conversion. + * + * A {@link FILE_DOC_EVENTS.FLUSH} is the one path a client DOES wait on. It is bounded separately by + * {@link FILE_DOC_TIMEOUTS.flushRequestMs} rather than by `persistRequestMs`, because the budget that + * keeps a background write from being abandoned is far longer than a user will wait on an + * interaction. The two are independent by design: a flush that outruns its client budget still + * completes server-side and its result is simply no longer awaited. */ export const FILE_DOC_TIMEOUTS = { seedRequestMs: 8_000, @@ -113,6 +128,9 @@ export const FILE_DOC_TIMEOUTS = { applyEditMs: 6_000, readinessDeadlineMs: 12_000, persistRequestMs: 8_000, + /** How long a client waits for {@link FILE_DOC_EVENTS.FLUSH_COMPLETE} before giving up and + * continuing without it. Sized for an interaction, not for the write it triggers. */ + flushRequestMs: 2_000, } as const /** Client → server join request. `fileId` is the `workspace_files.id`. */ @@ -144,6 +162,35 @@ export interface LeaveFileDocPayload { fileId: string } +/** Client → server request to persist the live document immediately. */ +export interface FlushFileDocPayload { + fileId: string +} + +/** + * Server → client outcome of a {@link FILE_DOC_EVENTS.FLUSH}. + * + * `version` is present ONLY on `persisted`, and is the new durable content version + * (`content_updated_at` epoch ms). A caller that needs to know the durable content actually moved + * must check the status — several outcomes complete normally having written nothing: + * + * - `persisted` — projected and written; `version` advanced. + * - `unchanged` — nobody edited the document this session, so there is nothing to project. The + * durable content is already current. + * - `skipped` — a write was attempted or considered and did not land: an out-of-band edit won the + * optimistic-concurrency check, the synced version was momentarily unresolvable, the file was + * deleted, or the request failed. The edits remain in the relay's stream and a later flush + * (debounced, or on last leave) writes them. + * + * `skipped` is deliberately NOT an error: the caller's own next step is usually still safe, it just + * cannot assume the durable bytes are current. + */ +export interface FlushFileDocResult { + fileId: string + status: 'persisted' | 'unchanged' | 'skipped' + version?: number +} + /** One collaborator session in a {@link FileDocPresence} roster — server-authenticated identity. * Keyed per socket (session), not per user: the client excludes its OWN `socketId` and then * dedupes the rest per user for the avatar stack, so a second tab of the same account still From 214bf611ddd522f923ceabe99c8813dc91e57541 Mon Sep 17 00:00:00 2001 From: Marcus Chandra Date: Fri, 7 Aug 2026 00:55:51 -0700 Subject: [PATCH 5/9] fix(files): publish a stable file-doc flush and trace its outcome MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The published flush was bound to the provider's identity, so every socket churn republished it — and a churn ending on null left the file-detail header with nothing to call, silently degrading a retype back to a stale read. It now publishes once and resolves the provider at call time. Adds a log on both sides of the flush. Its outcome decides whether the caller may treat the durable bytes as current, and unchanged/skipped are both silent no-writes, so a stale read after a retype is otherwise indistinguishable from a rendering bug. --- apps/realtime/src/handlers/file-doc.ts | 6 +++++ .../use-file-doc-collaboration.ts | 25 +++++++++++++++---- .../workspace/[workspaceId]/files/files.tsx | 10 +++++++- 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/apps/realtime/src/handlers/file-doc.ts b/apps/realtime/src/handlers/file-doc.ts index dc578ef7225..4b13dd9d180 100644 --- a/apps/realtime/src/handlers/file-doc.ts +++ b/apps/realtime/src/handlers/file-doc.ts @@ -1304,6 +1304,12 @@ export function setupWorkspaceFileDocHandlers( room.persistDeadline = null const outcome = await flushPersist(name, room, 'requested') + // The caller's next step depends on this outcome, and `unchanged`/`skipped` are both silent + // no-writes — worth a line so a stale read after a retype can be traced without a repro. + logger.info(`Requested flush for file ${fileId}: ${outcome.status}`, { + edited: room.edited, + hasWorkspace: Boolean(room.workspaceId), + }) ack(outcome.status, outcome.status === 'persisted' ? outcome.version : undefined) } catch (error) { logger.error('Error flushing file-doc room:', error) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts index 19b092e4482..4e5cd5584f0 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts @@ -1,6 +1,6 @@ 'use client' -import { useEffect, useMemo, useRef, useState } from 'react' +import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { FILE_DOC_EVENTS, type FileDocPresence } from '@sim/realtime-protocol/file-doc' import { Awareness } from 'y-protocols/awareness' import * as Y from 'yjs' @@ -110,10 +110,25 @@ export function useFileDocCollaboration({ } }, [enabled, socket, fileId]) - // Publish this document's flush so the file-detail header can force durability before it reads the - // file's stored bytes back (changing the file's type unmounts this editor and swaps in one that - // reads them). Cleared with the provider, so it can never outlive the socket it wraps. - useReportFileDocFlush(useMemo(() => (provider ? () => provider.flush() : null), [provider])) + /** + * Publish this document's flush so the file-detail header can force durability before it reads the + * file's stored bytes back (changing the file's type unmounts this editor and swaps in one that + * reads them). + * + * The published function is STABLE and resolves the provider at call time through a ref. Binding it + * to the provider's identity instead looked equivalent and was not: the provider is torn down and + * rebuilt on every socket change, so each churn republished — and any churn ending on `null` left + * the header with nothing to call, silently degrading the retype back to a stale read. A stable + * identity publishes once and always sees the live provider. + */ + const providerRef = useRef(null) + providerRef.current = provider + useReportFileDocFlush( + useCallback( + () => providerRef.current?.flush() ?? Promise.resolve({ fileId, status: 'skipped' as const }), + [fileId] + ) + ) const reportOthers = useReportFileDocOthers() const reportOthersRef = useRef(reportOthers) diff --git a/apps/sim/app/workspace/[workspaceId]/files/files.tsx b/apps/sim/app/workspace/[workspaceId]/files/files.tsx index 56b707847fe..3d3dbc16f63 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/files.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/files.tsx @@ -1238,7 +1238,15 @@ export function Files() { if (type.mimeType === file.type && nextName === file.name) return if (isDirtyRef.current) await saveRef.current?.() - await flushFileDocRef(fileDocFlushRef) + const flushed = await flushFileDocRef(fileDocFlushRef) + if (flushed.status !== 'persisted') { + // Not an error — `unchanged` means there was nothing to write, and `skipped` means the write + // did not land in time. The retype proceeds either way; this is the breadcrumb for a stale + // first paint, which is otherwise indistinguishable from a rendering bug. + logger.info('Changing file type without a confirmed durable flush', { + status: flushed.status, + }) + } const siblingNames = new Set( filesRef.current From d6cafadb76a5b82a8686d8cec7c9456b2acfde83 Mon Sep 17 00:00:00 2001 From: Marcus Chandra Date: Fri, 7 Aug 2026 01:17:00 -0700 Subject: [PATCH 6/9] fix(realtime): bound a client-requested flush to edits it has not written A requested flush deliberately bypasses the cross-task dedup window, because a deduped no-op acked as success would reintroduce the staleness the flush exists to prevent. But room.edited is set on the first edit and never cleared, so every repeat still performed a full projection: a Yjs-to-markdown conversion, a fresh blob upload, and a delete of the previous key. A client emitting flush in a loop could drive that unbounded. Pairs a monotonic edit counter with the sequence the last successful persist covered, so a flush with nothing new to write acks unchanged instead. The sequence is captured before the projection and stored only on success, so an edit arriving mid-write stays pending and a conflict is never mistaken for a completed write. --- apps/realtime/src/handlers/file-doc.test.ts | 57 ++++++++ apps/realtime/src/handlers/file-doc.ts | 28 +++- .../file-doc-room-context.test.tsx | 128 ++++++++++++++++++ 3 files changed, 212 insertions(+), 1 deletion(-) create mode 100644 apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context.test.tsx diff --git a/apps/realtime/src/handlers/file-doc.test.ts b/apps/realtime/src/handlers/file-doc.test.ts index 5b53aab3401..0df825bd3f8 100644 --- a/apps/realtime/src/handlers/file-doc.test.ts +++ b/apps/realtime/src/handlers/file-doc.test.ts @@ -461,6 +461,63 @@ describe('setupWorkspaceFileDocHandlers', () => { } }) + it('does not re-project edits the durable file already has', async () => { + mockFetchFileDocSeed.mockResolvedValue(seedResult('# From server')) + mockFetchFileDocPersist.mockResolvedValue({ status: 'persisted', version: 9 }) + const { io } = createIo() + const { handlers, socket } = setup('socket-1', io) + await joinAndEdit(handlers) + + await handlers[FILE_DOC_EVENTS.FLUSH]({ fileId: 'file-1' }) + await handlers[FILE_DOC_EVENTS.FLUSH]({ fileId: 'file-1' }) + await handlers[FILE_DOC_EVENTS.FLUSH]({ fileId: 'file-1' }) + + // `edited` never clears, so without an edit-sequence check each repeat would mint another blob + // version. Only the first has anything to write; the rest are honest no-ops. + expect(mockFetchFileDocPersist).toHaveBeenCalledTimes(1) + expect(flushAcks(socket)).toEqual([ + { fileId: 'file-1', status: 'persisted', version: 9 }, + { fileId: 'file-1', status: 'unchanged' }, + { fileId: 'file-1', status: 'unchanged' }, + ]) + }) + + it('writes again once a new edit lands after a flush', async () => { + mockFetchFileDocSeed.mockResolvedValue(seedResult('# From server')) + mockFetchFileDocPersist.mockResolvedValue({ status: 'persisted', version: 9 }) + const { io } = createIo() + const { handlers } = setup('socket-1', io) + await joinAndEdit(handlers) + await handlers[FILE_DOC_EVENTS.FLUSH]({ fileId: 'file-1' }) + + const more = new Y.Doc() + more.getText(FILE_DOC_FIELD).insert(0, 'and more typing') + handlers[FILE_DOC_EVENTS.MESSAGE]( + frame(FILE_DOC_MESSAGE_TYPE.SYNC, (e) => + syncProtocol.writeUpdate(e, Y.encodeStateAsUpdate(more)) + ) + ) + await flushMicrotasks() + await handlers[FILE_DOC_EVENTS.FLUSH]({ fileId: 'file-1' }) + + // The dedup must bound redundant writes without ever swallowing real edits. + expect(mockFetchFileDocPersist).toHaveBeenCalledTimes(2) + }) + + it('leaves the edits pending when a persist did not land', async () => { + mockFetchFileDocSeed.mockResolvedValue(seedResult('# From server')) + mockFetchFileDocPersist.mockResolvedValue({ status: 'conflict' }) + const { io } = createIo() + const { handlers } = setup('socket-1', io) + await joinAndEdit(handlers) + + await handlers[FILE_DOC_EVENTS.FLUSH]({ fileId: 'file-1' }) + await handlers[FILE_DOC_EVENTS.FLUSH]({ fileId: 'file-1' }) + + // A conflict wrote nothing, so the second attempt must NOT be deduped away as already-durable. + expect(mockFetchFileDocPersist).toHaveBeenCalledTimes(2) + }) + it('refuses a flush for a file this socket never joined', async () => { mockFetchFileDocSeed.mockResolvedValue(seedResult('# From server')) const { io } = createIo() diff --git a/apps/realtime/src/handlers/file-doc.ts b/apps/realtime/src/handlers/file-doc.ts index 4b13dd9d180..9ae6c1071a3 100644 --- a/apps/realtime/src/handlers/file-doc.ts +++ b/apps/realtime/src/handlers/file-doc.ts @@ -148,6 +148,19 @@ interface FileDocRoom { * is last to leave — even one that only tailed the edits. */ edited: boolean + /** + * Monotonic count of edits applied here. Paired with {@link FileDocRoom.persistedEditSeq} to answer + * "is there anything new to write?" — `edited` alone only answers "was this doc EVER edited", which + * stays true forever and would let a client force an unbounded run of redundant blob versions by + * repeatedly asking for a flush. + */ + editSeq: number + /** + * The {@link FileDocRoom.editSeq} value the last successful persist projected. Equal to `editSeq` + * means the durable file already reflects every edit this room has seen. Captured BEFORE the + * projection and stored only on success, so an edit arriving mid-persist is never marked durable. + */ + persistedEditSeq: number /** Whether this room has observed its doc become seeded — so a post-seed update counts as an edit but * the seed transition itself does not. See the `doc.on('update')` edit-tracking below. */ seededObserved: boolean @@ -308,6 +321,12 @@ async function flushPersist( // Never project a doc no user actually edited back over the file (see {@link FileDocRoom.edited}). // Nothing to write is not a failure — the durable content is already current. if (!room.edited || !room.workspaceId || !room.lastEditorUserId) return { status: 'unchanged' } + // Nor re-project edits the durable file already has. `edited` never clears, so without this a + // caller that can ask for a flush could force an unbounded run of identical blob versions — each + // one a fresh upload plus a delete of the old key. Captured here, before any await, so an edit + // landing mid-persist is compared against the value this projection actually covers. + const projectedEditSeq = room.editSeq + if (projectedEditSeq === room.persistedEditSeq) return { status: 'unchanged' } const store = getFileDocStore() const workspaceId = room.workspaceId const userId = room.lastEditorUserId @@ -395,6 +414,9 @@ async function flushPersist( } if (result.status === 'persisted') { room.syncedVersion = Math.max(room.syncedVersion ?? 0, result.version) + // Only the edits this projection actually carried. An edit that arrived while the write was in + // flight keeps `editSeq` ahead, so the next flush still has something to do. + room.persistedEditSeq = Math.max(room.persistedEditSeq, projectedEditSeq) void store.setSyncedVersion(name, result.version) return { status: 'persisted', version: result.version } } @@ -771,6 +793,8 @@ function getOrCreateRoom(io: Server, ref: RoomRef): FileDocRoom { workspaceId: null, lastEditorUserId: null, edited: false, + editSeq: 0, + persistedEditSeq: 0, seededObserved: false, persistTimer: null, persistDeadline: null, @@ -837,8 +861,10 @@ function getOrCreateRoom(io: Server, ref: RoomRef): FileDocRoom { originSocketId(origin) || origin === REDIS_SNAPSHOT_ORIGIN || (seededBefore && origin === REDIS_ORIGIN) - ) + ) { room.edited = true + room.editSeq++ + } // Debounce a persist for LOCAL user edits only (peers debounce their own). if (originSocketId(origin)) schedulePersist(name, room) }) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context.test.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context.test.tsx new file mode 100644 index 00000000000..53254694fe7 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context.test.tsx @@ -0,0 +1,128 @@ +/** + * @vitest-environment jsdom + * + * The flush hand-off between the editor (which owns the realtime provider) and the file-detail + * header (which acts on it). The header renders the provider, so it owns the ref rather than + * reading it through context — these cover that the published flush is actually reachable from + * there, and stays reachable. + */ +import { act, type ReactNode, useRef } from 'react' +import type { FlushFileDocResult } from '@sim/realtime-protocol/file-doc' +import { createRoot, type Root } from 'react-dom/client' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + type FileDocFlush, + FileDocRoomProvider, + flushFileDocRef, + useReportFileDocFlush, +} from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context' + +beforeEach(() => { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true +}) + +const PERSISTED: FlushFileDocResult = { fileId: 'file-1', status: 'persisted', version: 1 } + +/** + * Mounts an owner that passes its own ref into the provider — the shape `files.tsx` uses — with a + * publisher underneath standing in for the editor. `read()` is what the header's retype handler does. + */ +function renderOwner(initialFlush: FileDocFlush | null) { + const container = document.createElement('div') + const root: Root = createRoot(container) + let read: (() => Promise) | null = null + + function Publisher({ flush }: { flush: FileDocFlush | null }) { + useReportFileDocFlush(flush) + return null + } + + function Owner({ flush, children }: { flush: FileDocFlush | null; children?: ReactNode }) { + const flushRef = useRef(null) + read = () => flushFileDocRef(flushRef) + return ( + + + {children} + + ) + } + + const render = (flush: FileDocFlush | null) => { + act(() => { + root.render() + }) + } + render(initialFlush) + + return { + render, + read: () => { + if (!read) throw new Error('owner did not render') + return read() + }, + unmount: () => act(() => root.unmount()), + } +} + +describe('file-doc flush hand-off', () => { + it('reaches the ancestor-owned ref, not just descendants', async () => { + const flush = vi.fn(async () => PERSISTED) + const owner = renderOwner(flush) + + await expect(owner.read()).resolves.toEqual(PERSISTED) + expect(flush).toHaveBeenCalledTimes(1) + owner.unmount() + }) + + it('resolves skipped when nothing collaborative is mounted', async () => { + const owner = renderOwner(null) + + // The caller needs no null check: a non-collaborative file must cost nothing and never throw. + await expect(owner.read()).resolves.toMatchObject({ status: 'skipped' }) + owner.unmount() + }) + + /** + * The regression that motivated the stable-publish design. A flush bound to the provider's + * identity republished on every socket churn, and a churn ending on `null` silently left the + * header with nothing to call — degrading a retype back to reading pre-edit bytes. + */ + it('survives publisher churn that ends on a live flush', async () => { + const first = vi.fn(async () => PERSISTED) + const second = vi.fn(async () => PERSISTED) + const owner = renderOwner(first) + + owner.render(null) + owner.render(second) + + await expect(owner.read()).resolves.toEqual(PERSISTED) + expect(second).toHaveBeenCalledTimes(1) + expect(first).not.toHaveBeenCalled() + owner.unmount() + }) + + it('goes quiet once the publisher reports no flush', async () => { + const flush = vi.fn(async () => PERSISTED) + const owner = renderOwner(flush) + + owner.render(null) + + await expect(owner.read()).resolves.toMatchObject({ status: 'skipped' }) + expect(flush).not.toHaveBeenCalled() + owner.unmount() + }) + + it('stops resolving a torn-down publisher', async () => { + const flush = vi.fn(async () => PERSISTED) + const owner = renderOwner(flush) + owner.unmount() + + // Nothing to assert against the ref after unmount, but the published function must not be + // retained by a later mount — a fresh owner starts empty. + const next = renderOwner(null) + await expect(next.read()).resolves.toMatchObject({ status: 'skipped' }) + expect(flush).not.toHaveBeenCalled() + next.unmount() + }) +}) From e278ae382d00a1a00fe7e636812ff82c6933c7ff Mon Sep 17 00:00:00 2001 From: Marcus Chandra Date: Fri, 7 Aug 2026 01:25:51 -0700 Subject: [PATCH 7/9] chore: drop an unrelated .gitignore line from this branch The .gstack/ entry was created by local tooling during QA on this branch and got swept in by a broad stage. It is unrelated to the file-type work and the repo references .gstack nowhere, so it does not belong in this PR. --- .gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitignore b/.gitignore index 137b3e34bd8..6e9bdf2c99c 100644 --- a/.gitignore +++ b/.gitignore @@ -111,4 +111,3 @@ __pycache__/ # `apps/sim/lib/uploads/` — 61 files of tracked source — and silently ignore # anything added there later. /uploads -.gstack/ From e1775025efd99342d1dff8852d5d895db1de8c7d Mon Sep 17 00:00:00 2001 From: Marcus Chandra Date: Fri, 7 Aug 2026 01:36:42 -0700 Subject: [PATCH 8/9] fix(files): refresh the file list after a flush so the retype reads a live key A collaborative flush persists through a versioned object swap: it mints a new storage key and deletes the previous blob. The retype then swaps editors from the optimistic rename patch, so the newly mounted viewer read `key` off a record the flush had already invalidated - a 404, or pre-edit text from the content cache keyed on that dead key, until the rename's own invalidation landed. Awaits a list refetch between a confirmed `persisted` flush and the rename. `refetchType: 'all'`, because the caller awaits this for a usable key and the default `active` resolves immediately against an unobserved list. Also moves this file's two sibling imports onto the `@/` alias per the repo's absolute-import rule. --- .../use-file-doc-collaboration.ts | 7 +- .../workspace/[workspaceId]/files/files.tsx | 13 ++- .../hooks/queries/workspace-files.test.tsx | 110 ++++++++++++++++++ apps/sim/hooks/queries/workspace-files.ts | 26 ++++- 4 files changed, 150 insertions(+), 6 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts index 4e5cd5584f0..d0be1be8ca8 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts @@ -5,9 +5,12 @@ import { FILE_DOC_EVENTS, type FileDocPresence } from '@sim/realtime-protocol/fi import { Awareness } from 'y-protocols/awareness' import * as Y from 'yjs' import { getUserColor } from '@/lib/workspaces/colors' +import { FileDocProvider } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider' +import { + useReportFileDocFlush, + useReportFileDocOthers, +} from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context' import { useSocket } from '@/app/workspace/providers/socket-provider' -import { FileDocProvider } from './file-doc-provider' -import { useReportFileDocFlush, useReportFileDocOthers } from './file-doc-room-context' /** The live collaboration binding the editor wires into TipTap's Collaboration * (the {@link Y.Doc}) and CollaborationCaret (the awareness). */ diff --git a/apps/sim/app/workspace/[workspaceId]/files/files.tsx b/apps/sim/app/workspace/[workspaceId]/files/files.tsx index 3d3dbc16f63..e70374ff38a 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/files.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/files.tsx @@ -125,6 +125,7 @@ import { } from '@/hooks/queries/workspace-file-folders' import { useDeleteWorkspaceFile, + useRefreshWorkspaceFiles, useRenameWorkspaceFile, useUploadWorkspaceFile, useWorkspaceFiles, @@ -305,6 +306,7 @@ export function Files() { const notifyLimit = useLimitUpgradeToast() const deleteFile = useDeleteWorkspaceFile() const renameFile = useRenameWorkspaceFile() + const refreshFiles = useRefreshWorkspaceFiles() const createFolder = useCreateWorkspaceFileFolder() const updateFolder = useUpdateWorkspaceFileFolder() const moveItems = useMoveWorkspaceFileItems() @@ -1239,8 +1241,13 @@ export function Files() { if (isDirtyRef.current) await saveRef.current?.() const flushed = await flushFileDocRef(fileDocFlushRef) - if (flushed.status !== 'persisted') { - // Not an error — `unchanged` means there was nothing to write, and `skipped` means the write + if (flushed.status === 'persisted') { + // The persist minted a new storage key and deleted the previous blob, so the cached record + // the viewer renders from now points at a key that 404s. Wait for the refreshed list before + // the rename swaps editors, or the newly mounted viewer reads the dead key. + await refreshFiles(workspaceId) + } else { + // Not an error - `unchanged` means there was nothing to write, and `skipped` means the write // did not land in time. The retype proceeds either way; this is the breadcrumb for a stale // first paint, which is otherwise indistinguishable from a rendering bug. logger.info('Changing file type without a confirmed durable flush', { @@ -1265,7 +1272,7 @@ export function Files() { logger.error('Failed to change file type:', err) } }, - [workspaceId] + [workspaceId, refreshFiles] ) const handleDownloadSelected = useCallback(() => { diff --git a/apps/sim/hooks/queries/workspace-files.test.tsx b/apps/sim/hooks/queries/workspace-files.test.tsx index e64e5ac8788..c54b0cd0071 100644 --- a/apps/sim/hooks/queries/workspace-files.test.tsx +++ b/apps/sim/hooks/queries/workspace-files.test.tsx @@ -13,6 +13,7 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { + useRefreshWorkspaceFiles, useRenameWorkspaceFile, useWorkspaceFileContent, workspaceFilesKeys, @@ -246,3 +247,112 @@ describe('useRenameWorkspaceFile optimistic cache patch', () => { unmount() }) }) + +/** + * A collaborative flush mints a new storage key and deletes the previous blob, so a retype has to + * wait for the refreshed list before it swaps editors - the newly mounted viewer reads `key` off + * that record, and the pre-flush one 404s. + */ +describe('useRefreshWorkspaceFiles', () => { + const WS = 'ws-1' + + function renderRefresh(): { + refresh: () => (workspaceId: string) => Promise + queryClient: QueryClient + unmount: () => void + } { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }) + const container = document.createElement('div') + const root: Root = createRoot(container) + let result: ((workspaceId: string) => Promise) | null = null + + function Probe() { + result = useRefreshWorkspaceFiles() + return null + } + + act(() => { + root.render( + + + + ) + }) + + return { + refresh: () => { + if (!result) throw new Error('hook did not render') + return result + }, + queryClient, + unmount: () => { + act(() => root.unmount()) + queryClient.clear() + }, + } + } + + it('resolves only after the refetched list has landed', async () => { + const keys = ['workspace/ws-1/old-key', 'workspace/ws-1/new-key'] + let call = 0 + const queryFn = vi.fn(async () => { + const key = keys[Math.min(call, keys.length - 1)] + call += 1 + await sleep(10) + return [{ id: 'file-1', key }] + }) + + const { refresh, queryClient, unmount } = renderRefresh() + const queryKey = workspaceFilesKeys.list(WS, 'active') + await act(async () => { + await queryClient.fetchQuery({ queryKey, queryFn }) + }) + expect(queryClient.getQueryData<{ key: string }[]>(queryKey)?.[0].key).toBe( + 'workspace/ws-1/old-key' + ) + + await act(async () => { + await refresh()(WS) + }) + + // The awaited call must have already replaced the dead key, not merely marked it stale. + expect(queryClient.getQueryData<{ key: string }[]>(queryKey)?.[0].key).toBe( + 'workspace/ws-1/new-key' + ) + expect(queryFn).toHaveBeenCalledTimes(2) + unmount() + }) + + it('refetches a cached list that no component is observing', async () => { + // `refetchType: 'all'` is load-bearing: the retype awaits this before mounting the next viewer, + // and the default `active` would resolve instantly against an unobserved list. + const queryFn = vi.fn(async () => [{ id: 'file-1', key: 'k' }]) + const { refresh, queryClient, unmount } = renderRefresh() + + await act(async () => { + await queryClient.fetchQuery({ queryKey: workspaceFilesKeys.list(WS, 'active'), queryFn }) + await refresh()(WS) + }) + + expect(queryFn).toHaveBeenCalledTimes(2) + unmount() + }) + + it('leaves another workspace list alone', async () => { + const otherQueryFn = vi.fn(async () => [{ id: 'file-2', key: 'k2' }]) + const { refresh, queryClient, unmount } = renderRefresh() + + await act(async () => { + await queryClient.fetchQuery({ + queryKey: workspaceFilesKeys.list('ws-2', 'active'), + queryFn: otherQueryFn, + }) + await refresh()(WS) + }) + + expect(otherQueryFn).toHaveBeenCalledTimes(1) + unmount() + }) +}) diff --git a/apps/sim/hooks/queries/workspace-files.ts b/apps/sim/hooks/queries/workspace-files.ts index 9791694b44f..0bd27d6d09d 100644 --- a/apps/sim/hooks/queries/workspace-files.ts +++ b/apps/sim/hooks/queries/workspace-files.ts @@ -1,4 +1,4 @@ -import { useMemo } from 'react' +import { useCallback, useMemo } from 'react' import { toast } from '@sim/emcn' import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' @@ -580,6 +580,30 @@ export function useUpdateWorkspaceFileContent() { }) } +/** + * Refetch the workspace file list and resolve once the fresh records have landed. + * + * Every content write mints a new storage key and deletes the previous blob, so a cached record's + * `key` is dead the moment one lands. A caller that is about to mount a viewer from that record - + * a retype, which swaps editors optimistically - has to wait for the refreshed list, or the new + * viewer fetches a key the store has already deleted. + */ +export function useRefreshWorkspaceFiles() { + const queryClient = useQueryClient() + + return useCallback( + (workspaceId: string) => + queryClient.invalidateQueries({ + queryKey: workspaceFilesKeys.workspaceLists(workspaceId), + // `all`, not the default `active`: the caller awaits this to get a usable key back, and an + // invalidation that only marks an unobserved list stale resolves immediately with the dead + // key still cached - the exact staleness this exists to close. + refetchType: 'all', + }), + [queryClient] + ) +} + /** * Rename a workspace file */ From 2c1518edac9ed1d20f1d14a2b3b31cbe1d44c243 Mon Sep 17 00:00:00 2001 From: Marcus Chandra Date: Fri, 7 Aug 2026 01:48:37 -0700 Subject: [PATCH 9/9] fix(files): surface a failed pre-retype list refresh instead of swallowing it `invalidateQueries` resolves whether or not the refetch succeeded, so the refresh reported success while leaving the dead storage key in the cache - the caller awaiting a usable key could not tell the two apart. The hook now rejects on a failed refetch. The retype logs and proceeds rather than aborting: the edits are already durable, the type change is explicit, and the rename's own invalidation refetches straight after, so the cost of a failed refresh is one stale first paint - the pre-fix behaviour - not a lost change. --- .../workspace/[workspaceId]/files/files.tsx | 10 +++++- .../hooks/queries/workspace-files.test.tsx | 34 +++++++++++++++++++ apps/sim/hooks/queries/workspace-files.ts | 24 ++++++++----- 3 files changed, 59 insertions(+), 9 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/files/files.tsx b/apps/sim/app/workspace/[workspaceId]/files/files.tsx index e70374ff38a..5f6c3ac358c 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/files.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/files.tsx @@ -1245,7 +1245,15 @@ export function Files() { // The persist minted a new storage key and deleted the previous blob, so the cached record // the viewer renders from now points at a key that 404s. Wait for the refreshed list before // the rename swaps editors, or the newly mounted viewer reads the dead key. - await refreshFiles(workspaceId) + try { + await refreshFiles(workspaceId) + } catch (err) { + // Proceed rather than abort: the edits are already durable, the retype is an explicit + // action, and the rename's own invalidation refetches immediately after. The cost of a + // failed refresh is one stale first paint, which is the pre-fix behaviour - not losing + // the user's type change on a transient list fetch. + logger.warn('Retyping against a stale file list; the first read may 404', { err }) + } } else { // Not an error - `unchanged` means there was nothing to write, and `skipped` means the write // did not land in time. The retype proceeds either way; this is the breadcrumb for a stale diff --git a/apps/sim/hooks/queries/workspace-files.test.tsx b/apps/sim/hooks/queries/workspace-files.test.tsx index c54b0cd0071..d49be596220 100644 --- a/apps/sim/hooks/queries/workspace-files.test.tsx +++ b/apps/sim/hooks/queries/workspace-files.test.tsx @@ -340,6 +340,40 @@ describe('useRefreshWorkspaceFiles', () => { unmount() }) + /** + * react-query resolves an invalidation whether or not the refetch succeeded. A caller awaiting + * this for a usable storage key would otherwise read the dead one back as if it were fresh. + */ + it('rejects when the refetch fails instead of resolving on the stale cache', async () => { + let call = 0 + const queryFn = vi.fn(async () => { + call += 1 + if (call > 1) throw new Error('network down') + return [{ id: 'file-1', key: 'workspace/ws-1/old-key' }] + }) + + const { refresh, queryClient, unmount } = renderRefresh() + const queryKey = workspaceFilesKeys.list(WS, 'active') + await act(async () => { + await queryClient.fetchQuery({ queryKey, queryFn }) + }) + + let rejection: unknown = null + await act(async () => { + await refresh()(WS).catch((err) => { + rejection = err + }) + }) + + expect(rejection).toBeInstanceOf(Error) + // The stale record is still cached — the caller has to decide what to do about it, not be told + // the refresh worked. + expect(queryClient.getQueryData<{ key: string }[]>(queryKey)?.[0].key).toBe( + 'workspace/ws-1/old-key' + ) + unmount() + }) + it('leaves another workspace list alone', async () => { const otherQueryFn = vi.fn(async () => [{ id: 'file-2', key: 'k2' }]) const { refresh, queryClient, unmount } = renderRefresh() diff --git a/apps/sim/hooks/queries/workspace-files.ts b/apps/sim/hooks/queries/workspace-files.ts index 0bd27d6d09d..4d43f970553 100644 --- a/apps/sim/hooks/queries/workspace-files.ts +++ b/apps/sim/hooks/queries/workspace-files.ts @@ -581,25 +581,33 @@ export function useUpdateWorkspaceFileContent() { } /** - * Refetch the workspace file list and resolve once the fresh records have landed. + * Refetch the workspace file list, resolving once the fresh records have landed and **rejecting** + * if the refetch failed. * * Every content write mints a new storage key and deletes the previous blob, so a cached record's * `key` is dead the moment one lands. A caller that is about to mount a viewer from that record - * a retype, which swaps editors optimistically - has to wait for the refreshed list, or the new * viewer fetches a key the store has already deleted. + * + * The rejection is the point: react-query resolves an invalidation whether or not the refetch + * succeeded, so a caller awaiting a usable key cannot otherwise tell fresh records from the dead + * ones still sitting in the cache. Callers decide what a failure means for them. */ export function useRefreshWorkspaceFiles() { const queryClient = useQueryClient() return useCallback( (workspaceId: string) => - queryClient.invalidateQueries({ - queryKey: workspaceFilesKeys.workspaceLists(workspaceId), - // `all`, not the default `active`: the caller awaits this to get a usable key back, and an - // invalidation that only marks an unobserved list stale resolves immediately with the dead - // key still cached - the exact staleness this exists to close. - refetchType: 'all', - }), + queryClient.invalidateQueries( + { + queryKey: workspaceFilesKeys.workspaceLists(workspaceId), + // `all`, not the default `active`: the caller awaits this to get a usable key back, and an + // invalidation that only marks an unobserved list stale resolves immediately with the dead + // key still cached - the exact staleness this exists to close. + refetchType: 'all', + }, + { throwOnError: true } + ), [queryClient] ) }