From 99578c704209188bfd11800f077f77d263589e79 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 6 Aug 2026 14:39:05 -0700 Subject: [PATCH 1/4] fix(chat): stop chats storing a resource they can never send with A chat resource persisted with a blank id made every later message fail: the write contract accepted `id: ''` while the send schema required `min(1)`, so the request 400d before a stream existed and the client's reconnect 404d. The tab could not be removed either, since the delete route requires a non-empty id. Twelve production chats were in this state. The id came from an agent-written file chip that carried only a filename: the client filled the missing id with `''` when the file was absent from its list, which it always is for a file the agent just created. - model the unresolved state (`WorkspaceResourceRef`) instead of faking an id, and resolve chip refs at one choke point that may refuse - close the stale-cache race by fetching the file list before giving up, so clicking a just-created file opens it instead of doing nothing - reject blank ids at the stream, write and send boundaries, and drop them wherever stored resources are read, which self-heals affected chats - collapse the 5-6 duplicate POSTs every resource add was firing - log rejected chat bodies, which previously left no trace at all --- .../app/api/copilot/chat/resources/route.ts | 8 +- .../mothership/chats/[chatId]/fork/route.ts | 7 +- .../chat-surface-context.tsx | 8 +- .../components/chat-content/chat-content.tsx | 25 ++++-- .../components/special-tags/special-tags.tsx | 20 ++--- .../mothership-chat/mothership-chat.tsx | 4 +- .../app/workspace/[workspaceId]/home/home.tsx | 78 ++++++++++------- .../[workspaceId]/home/hooks/use-chat.ts | 23 ++++- .../home/resolve-resource-ref.test.ts | 84 +++++++++++++++++++ .../home/resolve-resource-ref.ts | 53 ++++++++++++ .../app/workspace/[workspaceId]/home/types.ts | 1 + .../utils/find-workspace-file-by-src.ts | 17 ++++ apps/sim/hooks/queries/workspace-files.ts | 21 ++++- apps/sim/lib/api/contracts/copilot.ts | 4 +- apps/sim/lib/copilot/chat/post.ts | 34 +++++++- .../copilot/request/session/contract.test.ts | 13 +++ .../lib/copilot/request/session/contract.ts | 20 +++-- apps/sim/lib/copilot/resources/persistence.ts | 23 ++--- apps/sim/lib/copilot/resources/types.test.ts | 42 +++++++++- apps/sim/lib/copilot/resources/types.ts | 60 ++++++++++++- 20 files changed, 445 insertions(+), 100 deletions(-) create mode 100644 apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.test.ts create mode 100644 apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.ts diff --git a/apps/sim/app/api/copilot/chat/resources/route.ts b/apps/sim/app/api/copilot/chat/resources/route.ts index ae87ae73219..85b81323a65 100644 --- a/apps/sim/app/api/copilot/chat/resources/route.ts +++ b/apps/sim/app/api/copilot/chat/resources/route.ts @@ -19,8 +19,8 @@ import { import type { ChatResource } from '@/lib/copilot/resources/persistence' import { canonicalizeDesktopSessionResource, - canonicalizeDesktopSessionResources, GENERIC_RESOURCE_TITLES, + sanitizeChatResources, } from '@/lib/copilot/resources/types' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -67,7 +67,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => { return createNotFoundResponse('Chat not found or unauthorized') } - const existing = canonicalizeDesktopSessionResources( + const existing = sanitizeChatResources( Array.isArray(chat.resources) ? (chat.resources as ChatResource[]) : [] ) const key = `${resource.type}:${resource.id}` @@ -141,10 +141,10 @@ export const PATCH = withRouteHandler(async (req: NextRequest) => { return createNotFoundResponse('Chat not found or unauthorized') } - const existing = canonicalizeDesktopSessionResources( + const existing = sanitizeChatResources( Array.isArray(chat.resources) ? (chat.resources as ChatResource[]) : [] ) - const canonicalOrder = canonicalizeDesktopSessionResources(newOrder) + const canonicalOrder = sanitizeChatResources(newOrder) const existingKeys = new Set(existing.map((r) => `${r.type}:${r.id}`)) const newKeys = new Set(canonicalOrder.map((r) => `${r.type}:${r.id}`)) diff --git a/apps/sim/app/api/mothership/chats/[chatId]/fork/route.ts b/apps/sim/app/api/mothership/chats/[chatId]/fork/route.ts index 5d18c9be56d..4f407ffea71 100644 --- a/apps/sim/app/api/mothership/chats/[chatId]/fork/route.ts +++ b/apps/sim/app/api/mothership/chats/[chatId]/fork/route.ts @@ -29,10 +29,7 @@ import { createUnauthorizedResponse, } from '@/lib/copilot/request/http' import { removeChatResources } from '@/lib/copilot/resources/persistence' -import { - canonicalizeDesktopSessionResources, - type MothershipResource, -} from '@/lib/copilot/resources/types' +import { type MothershipResource, sanitizeChatResources } from '@/lib/copilot/resources/types' import { getMothershipBaseURL, getMothershipSourceEnvHeaders } from '@/lib/copilot/server/agent-url' import { env } from '@/lib/core/config/env' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -118,7 +115,7 @@ export const POST = withRouteHandler( // file resources whose chat-owned file is NOT copied (uploads born // after the cut) are dropped in the rewrite below; everything else is // copied. - const parentResources = canonicalizeDesktopSessionResources( + const parentResources = sanitizeChatResources( Array.isArray(parent.resources) ? (parent.resources as MothershipResource[]) : [] ) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/chat-surface-context/chat-surface-context.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/chat-surface-context/chat-surface-context.tsx index c2d0a2146b0..6864df15943 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/chat-surface-context/chat-surface-context.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/chat-surface-context/chat-surface-context.tsx @@ -10,7 +10,7 @@ import { useRef, } from 'react' import { noop } from '@sim/utils/helpers' -import type { MothershipResource } from '@/app/workspace/[workspaceId]/home/types' +import type { WorkspaceResourceRef } from '@/app/workspace/[workspaceId]/home/types' import type { ChatContext } from '@/stores/panel' /** @@ -34,7 +34,7 @@ interface ChatSurfaceContextValue { */ onContextRemove: (context: ChatContext, remaining: ChatContext[]) => void /** Opens a workspace resource referenced from rendered message content. */ - onWorkspaceResourceSelect: (resource: MothershipResource) => void + onWorkspaceResourceSelect: (resource: WorkspaceResourceRef) => void } const ChatSurfaceContext = createContext({ @@ -48,7 +48,7 @@ interface ChatSurfaceProviderProps { userId?: string onContextAdd?: (context: ChatContext) => void onContextRemove?: (context: ChatContext, remaining: ChatContext[]) => void - onWorkspaceResourceSelect?: (resource: MothershipResource) => void + onWorkspaceResourceSelect?: (resource: WorkspaceResourceRef) => void children: ReactNode } @@ -82,7 +82,7 @@ export function ChatSurfaceProvider({ const stableOnContextRemove = useCallback((context: ChatContext, remaining: ChatContext[]) => { onContextRemoveRef.current?.(context, remaining) }, []) - const stableOnWorkspaceResourceSelect = useCallback((resource: MothershipResource) => { + const stableOnWorkspaceResourceSelect = useCallback((resource: WorkspaceResourceRef) => { onWorkspaceResourceSelectRef.current?.(resource) }, []) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/chat-content/chat-content.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/chat-content/chat-content.tsx index 17b24c9cd17..c0c235455f5 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/chat-content/chat-content.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/chat-content/chat-content.tsx @@ -20,7 +20,10 @@ import { parseSpecialTags, SpecialTags, } from '@/app/workspace/[workspaceId]/home/components/message-content/components/special-tags' -import type { ChatContextKind, MothershipResource } from '@/app/workspace/[workspaceId]/home/types' +import type { + ChatContextKind, + WorkspaceResourceRef, +} from '@/app/workspace/[workspaceId]/home/types' import { useSmoothText } from '@/hooks/use-smooth-text' import { sanitizeChatDisplayContent } from './chat-sanitize' import { ExternalLink, externalLinkHostname } from './external-link' @@ -278,12 +281,16 @@ const MARKDOWN_COMPONENTS = { e.preventDefault() if (!type || !ref) return const linkText = label || ref + // A file link carries whichever the tag had (`path ?? id`), so + // classify before forwarding: a canonical VFS path is always + // `files/…`, and an id never contains a separator. Labelling an id + // as a path would throw away the only thing that identifies it. + const isVfsPath = type === 'file' && ref.includes('/') window.dispatchEvent( new CustomEvent('wsres-click', { - detail: - type === 'file' - ? { type, path: ref, title: linkText } - : { type, id: ref, title: linkText }, + detail: isVfsPath + ? { type, path: ref, title: linkText } + : { type, id: ref, title: linkText }, }) ) }} @@ -393,7 +400,7 @@ interface ChatContentProps { questionAnswers?: string[] onOptionSelect?: (id: string) => void onQuestionDismiss?: () => void - onWorkspaceResourceSelect?: (resource: MothershipResource) => void + onWorkspaceResourceSelect?: (resource: WorkspaceResourceRef) => void onRevealStateChange?: (isRevealing: boolean) => void /** Reports whether this segment is actively painting text. */ onStreamActivityChange?: (active: boolean) => void @@ -520,10 +527,12 @@ function ChatContentInner({ useEffect(() => { const handler = (e: Event) => { const { type, id, path, title } = (e as CustomEvent).detail + // A link built from a path carries no id. Forward what the tag actually + // had; the select handler resolves it rather than guessing here. onWorkspaceResourceSelectRef.current?.({ type, - id: id ?? '', - path, + ...(id ? { id } : {}), + ...(path ? { path } : {}), title: title || id || path || '', }) } diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx index 67708c96aee..fbfdf7a8fdd 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx @@ -21,7 +21,6 @@ import { useSession } from '@/lib/auth/auth-client' import { buildHostedUpgradeUrl, HOSTED_BILLING_SETTINGS_URL } from '@/lib/billing/upgrade-reasons' import { canManageWorkspaceBilling } from '@/lib/billing/workspace-permissions' import { isBrowserAgentAvailable, sendBrowserPanelAction } from '@/lib/browser-agent/transport' -import { canonicalWorkspaceFilePath } from '@/lib/copilot/vfs/path-utils' import { isHosted } from '@/lib/core/config/env-flags' import { isSafeHttpUrl } from '@/lib/core/utils/urls' import { getDesktopBridge } from '@/lib/desktop' @@ -39,6 +38,7 @@ import { QuestionDisplay } from '@/app/workspace/[workspaceId]/home/components/m import type { ChatMessageContext, MothershipResource, + WorkspaceResourceRef, } from '@/app/workspace/[workspaceId]/home/types' // Deep import, not the barrel: the barrel also re-exports // ConnectServiceAccountModal, and that edge would pull the modal into this @@ -54,6 +54,7 @@ import { } from '@/hooks/queries/environment' import { useKnowledgeBasesQuery } from '@/hooks/queries/kb/knowledge' import { useTablesList } from '@/hooks/queries/tables' +import { findWorkspaceFileByPath } from '@/hooks/queries/utils/find-workspace-file-by-src' import { useWorkflows } from '@/hooks/queries/workflows' import { useWorkspaceFiles } from '@/hooks/queries/workspace-files' import { useSettingsNavigation } from '@/hooks/use-settings-navigation' @@ -1300,7 +1301,7 @@ interface SpecialTagsProps { questionAnswers?: string[] onOptionSelect?: (id: string) => void onQuestionDismiss?: () => void - onWorkspaceResourceSelect?: (resource: MothershipResource) => void + onWorkspaceResourceSelect?: (resource: WorkspaceResourceRef) => void } /** @@ -1455,7 +1456,7 @@ export function WorkspaceResourceDisplay({ onSelect, }: { data: WorkspaceResourceTagData - onSelect?: (resource: MothershipResource) => void + onSelect?: (resource: WorkspaceResourceRef) => void }) { const { workspaceId } = useParams<{ workspaceId: string }>() const { data: workflows = [] } = useWorkflows(workspaceId) @@ -1463,15 +1464,9 @@ export function WorkspaceResourceDisplay({ const { data: files = [] } = useWorkspaceFiles(workspaceId) const { data: knowledgeBases = [] } = useKnowledgeBasesQuery(workspaceId) - const resource = useMemo(() => { + const resource = useMemo(() => { const fileFromPath = - data.type === 'file' && data.path - ? files.find( - (file) => - canonicalWorkspaceFilePath({ folderPath: file.folderPath, name: file.name }) === - data.path - ) - : undefined + data.type === 'file' ? findWorkspaceFileByPath(files, data.path) : undefined const title = data.type === 'workflow' ? (workflows.find((workflow) => workflow.id === data.id)?.name ?? @@ -1487,9 +1482,10 @@ export function WorkspaceResourceDisplay({ : (knowledgeBases.find((knowledgeBase) => knowledgeBase.id === data.id)?.name ?? fallbackWorkspaceResourceTitle(data.type)) + const id = data.id ?? fileFromPath?.id return { type: toMothershipResourceType(data.type), - id: data.id ?? fileFromPath?.id ?? data.path ?? '', + ...(id ? { id } : {}), title, ...(data.type === 'file' && data.path ? { path: data.path } : {}), } diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/mothership-chat.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/mothership-chat.tsx index 16e9553545c..4ed0236df5a 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/mothership-chat.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/mothership-chat.tsx @@ -35,8 +35,8 @@ import type { ChatMessageContext, ContentBlock, FileAttachmentForApi, - MothershipResource, QueuedMessage, + WorkspaceResourceRef, } from '@/app/workspace/[workspaceId]/home/types' import { useAutoScroll } from '@/hooks/use-auto-scroll' import type { ChatContext } from '@/stores/panel' @@ -70,7 +70,7 @@ interface MothershipChatProps { * `ChatSurfaceContextValue`, which this forwards to. */ onContextRemove?: (context: ChatContext, remaining: ChatContext[]) => void - onWorkspaceResourceSelect?: (resource: MothershipResource) => void + onWorkspaceResourceSelect?: (resource: WorkspaceResourceRef) => void draftScopeKey?: string layout?: 'mothership-view' | 'copilot-view' initialScrollBlocked?: boolean diff --git a/apps/sim/app/workspace/[workspaceId]/home/home.tsx b/apps/sim/app/workspace/[workspaceId]/home/home.tsx index d27ffc2423f..14ffc118908 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/home.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/home.tsx @@ -15,12 +15,12 @@ import { import { Button, cn } from '@sim/emcn' import { PanelLeft } from '@sim/emcn/icons' import { createLogger } from '@sim/logger' +import { useQueryClient } from '@tanstack/react-query' import { useParams, useRouter } from 'next/navigation' import { useQueryState } from 'nuqs' import { usePostHog } from 'posthog-js/react' import { requestJson } from '@/lib/api/client/request' import { createWorkflowContract } from '@/lib/api/contracts' -import { canonicalWorkspaceFilePath } from '@/lib/copilot/vfs/path-utils' import { LandingPromptStorage, type LandingWorkflowSeed, @@ -36,6 +36,7 @@ import { import { captureEvent } from '@/lib/posthog/client' import { persistImportedWorkflow } from '@/lib/workflows/operations/import-export' import { RESOURCE_HEADER_CLASSES } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tab-controls' +import { resolveWorkspaceResourceRef } from '@/app/workspace/[workspaceId]/home/resolve-resource-ref' import { resourceParam, resourceUrlKeys } from '@/app/workspace/[workspaceId]/home/search-params' import { useFolders } from '@/hooks/queries/folders' import { @@ -43,7 +44,7 @@ import { useMothershipChatHistory, } from '@/hooks/queries/mothership-chats' import { useWorkflows } from '@/hooks/queries/workflows' -import { useWorkspaceFiles } from '@/hooks/queries/workspace-files' +import { getWorkspaceFilesQueryOptions, useWorkspaceFiles } from '@/hooks/queries/workspace-files' import { useOAuthReturnRouter } from '@/hooks/use-oauth-return' import type { ChatContext } from '@/stores/panel' import { @@ -56,7 +57,12 @@ import { type UserInputHandle, } from './components' import { getMothershipUseChatOptions, useChat, useMothershipResize } from './hooks' -import type { FileAttachmentForApi, MothershipResource, MothershipResourceType } from './types' +import type { + FileAttachmentForApi, + MothershipResource, + MothershipResourceType, + WorkspaceResourceRef, +} from './types' const logger = createLogger('Home') const subscribeToDesktopApp = () => () => {} @@ -90,6 +96,7 @@ export function Home({ chatId, userName, userId, tableViewsEnabled }: HomeProps) ) const { workspaceId } = useParams<{ workspaceId: string }>() const router = useRouter() + const queryClient = useQueryClient() /** * URL is the single source of truth for the selected resource. `Home` renders * client-side, so nuqs reads `?resource=` from the URL on mount — the same @@ -432,39 +439,48 @@ export function Home({ chatId, userName, userId, tableViewsEnabled }: HomeProps) removeResource(resolved.type, resolved.id) } - const resolveFileResource = useCallback( - (resource: MothershipResource): MothershipResource => { - if (resource.type !== 'file') return resource - - const reference = (resource.path || resource.id).trim() - - const file = workspaceFiles.find((candidate) => { - const candidatePath = canonicalWorkspaceFilePath({ - folderPath: candidate.folderPath, - name: candidate.name, - }) - return candidate.id === reference || candidatePath === reference - }) - - if (!file) return resource - return { - ...resource, - id: file.id, - title: resource.title || file.name, - } - }, - [workspaceFiles] - ) - - function handleWorkspaceResourceSelect(resource: MothershipResource) { - const resolvedResource = resolveFileResource(resource) - const wasAdded = addResource(resolvedResource) + function openWorkspaceResource(resource: MothershipResource) { + const wasAdded = addResource(resource) if (!wasAdded) { - setActiveResourceId(resolvedResource.id) + setActiveResourceId(resource.id) } handleResourceEvent() } + /** + * Opens the resource a message chip points at, resolving it first. A chip may + * carry only a filename — the agent names a file before the client's file + * list knows it exists — so one forced refetch closes that window. What still + * resolves to nothing opens nothing, rather than a tab that cannot be + * viewed or removed. + */ + async function handleWorkspaceResourceSelect(ref: WorkspaceResourceRef) { + const immediate = resolveWorkspaceResourceRef(ref, workspaceFiles) + if (immediate) { + openWorkspaceResource(immediate) + return + } + if (ref.type !== 'file') return + + // `staleTime: 0` forces the fetch this branch exists for — the cached list + // is what already failed to resolve. `fetchQuery` rejects on error, and + // this handler is invoked as a void callback, so the catch keeps a failed + // refetch on the same "resolved nothing" path as an empty result. + const files = await queryClient + .fetchQuery({ ...getWorkspaceFilesQueryOptions(workspaceId), staleTime: 0 }) + .catch(() => []) + const resolved = resolveWorkspaceResourceRef(ref, files) + if (resolved) { + openWorkspaceResource(resolved) + return + } + logger.warn('Ignored a resource chip that names nothing in this workspace', { + type: ref.type, + title: ref.title, + hasPath: Boolean(ref.path), + }) + } + const hasMessages = messages.length > 0 const showChatSkeleton = Boolean(chatId) && !hasMessages && isChatHistoryPending const draftScopeKey = `${workspaceId}:${chatId ?? 'new'}` diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts index 43e890b8d7d..e3ce3327301 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts @@ -64,8 +64,9 @@ import type { StreamBatchEvent } from '@/lib/copilot/request/session/types' import { canDisplayResource } from '@/lib/copilot/resources/availability' import { BROWSER_SESSION_RESOURCE_ID, - canonicalizeDesktopSessionResources, + isAddressableResource, isEphemeralResource, + sanitizeChatResources, TERMINAL_SESSION_RESOURCE_ID, } from '@/lib/copilot/resources/types' import { executeBrowserToolOnClient } from '@/lib/copilot/tools/client/browser-tool-execution' @@ -1655,6 +1656,11 @@ export function useChat( return source.map((m) => restoreRevealedSimKeysForMessage(m, revealedSimKeysRef.current)) }, [chatHistory, pendingMessages]) const addResource = useCallback((resource: MothershipResource): boolean => { + // The single fan-in for tab creation, so the invariant lives here. + if (!isAddressableResource(resource)) { + logger.warn('Ignored a resource with no id', { type: resource.type, title: resource.title }) + return false + } if (resourcesRef.current.some((r) => r.type === resource.type && r.id === resource.id)) { return false } @@ -1674,6 +1680,15 @@ export function useChat( const persistChatId = chatIdRef.current ?? selectedChatIdRef.current const key = `${resource.type}:${resource.id}` + // `resourcesRef` is written during render, so adds of the same resource in + // one tick all read the pre-render list and all pass the check above. State + // converges (the updater is idempotent) but each fired its own POST — 5-6 + // per resource in production. + const alreadyPersisting = + inFlightResourceAddsRef.current.has(key) || pendingPersistResourceKeysRef.current.has(key) + if (alreadyPersisting) { + return true + } if (persistChatId) { const promise = requestJson(addMothershipChatResourceContract, { body: { chatId: persistChatId, resource }, @@ -1714,6 +1729,10 @@ export function useChat( }) } if (inFlightAdd) { + // Drop the entry now, not when the add settles: an add being deleted must + // not suppress a fresh add of the same resource. The chained delete keeps + // its own reference to the promise. + inFlightResourceAddsRef.current.delete(key) inFlightAdd.finally(fireDelete) } else { fireDelete() @@ -2150,7 +2169,7 @@ export function useChat( // Older clients persisted each live browser page as a top-level resource // during new-chat creation. Collapse those legacy rows into the one // restorable Browser panel so page titles never appear beside Browser. - const persistedResources = canonicalizeDesktopSessionResources( + const persistedResources = sanitizeChatResources( chatHistory.resources.filter((r) => r.id !== 'streaming-file') ) // A stored panel this client cannot open is kept out of the tab strip diff --git a/apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.test.ts b/apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.test.ts new file mode 100644 index 00000000000..e1bbe8cdb69 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.test.ts @@ -0,0 +1,84 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import type { WorkspaceResourceRef } from '@/lib/copilot/resources/types' +import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace' +import { resolveWorkspaceResourceRef } from './resolve-resource-ref' + +function file(overrides: Partial & { id: string; name: string }) { + return { + workspaceId: 'ws-1', + key: `k/${overrides.id}`, + path: `/api/files/serve/${overrides.id}`, + size: 1, + type: 'text/plain', + uploadedBy: 'user-1', + uploadedAt: new Date(0), + updatedAt: new Date(0), + ...overrides, + } as WorkspaceFileRecord +} + +function ref(overrides: Partial = {}): WorkspaceResourceRef { + return { type: 'file', title: 'notes.md', ...overrides } +} + +describe('resolveWorkspaceResourceRef', () => { + it('trusts an explicit id even when the file list has not caught up', () => { + expect(resolveWorkspaceResourceRef(ref({ id: 'wf_abc' }), [])).toEqual({ + type: 'file', + id: 'wf_abc', + title: 'notes.md', + }) + }) + + it('resolves a path against the known files', () => { + const files = [file({ id: 'wf_abc', name: 'notes.md', folderPath: 'docs' })] + expect(resolveWorkspaceResourceRef(ref({ path: 'files/docs/notes.md' }), files)).toEqual({ + type: 'file', + id: 'wf_abc', + title: 'notes.md', + path: 'files/docs/notes.md', + }) + }) + + it('resolves a title when exactly one file answers to it', () => { + const files = [file({ id: 'wf_abc', name: 'notes.md' }), file({ id: 'wf_x', name: 'other.md' })] + expect(resolveWorkspaceResourceRef(ref(), files)?.id).toBe('wf_abc') + }) + + it('refuses an ambiguous title rather than opening the wrong file', () => { + const files = [ + file({ id: 'wf_a', name: 'notes.md', folderPath: 'a' }), + file({ id: 'wf_b', name: 'notes.md', folderPath: 'b' }), + ] + expect(resolveWorkspaceResourceRef(ref(), files)).toBeNull() + }) + + it('refuses a file it cannot identify, instead of inventing an empty id', () => { + expect(resolveWorkspaceResourceRef(ref(), [])).toBeNull() + expect(resolveWorkspaceResourceRef(ref({ path: 'files/gone.md' }), [])).toBeNull() + }) + + it('never resolves a non-file resource without an id', () => { + expect(resolveWorkspaceResourceRef(ref({ type: 'table', title: 'Sales' }), [])).toBeNull() + expect( + resolveWorkspaceResourceRef(ref({ type: 'table', title: 'Sales', id: 't1' }), []) + ).toEqual({ type: 'table', id: 't1', title: 'Sales' }) + }) + + it('only ever returns resources that can be addressed', () => { + const cases: WorkspaceResourceRef[] = [ + ref(), + ref({ id: '' }), + ref({ path: '' }), + ref({ title: '' }), + ref({ type: 'workflow', title: '' }), + ] + for (const candidate of cases) { + const resolved = resolveWorkspaceResourceRef(candidate, []) + expect(resolved === null || resolved.id.trim().length > 0).toBe(true) + } + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.ts b/apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.ts new file mode 100644 index 00000000000..d1a0f849117 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.ts @@ -0,0 +1,53 @@ +import type { MothershipResource, WorkspaceResourceRef } from '@/lib/copilot/resources/types' +import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace' +import { findWorkspaceFileByPath } from '@/hooks/queries/utils/find-workspace-file-by-src' + +/** + * The one file whose name is exactly {@link name}, or undefined. Two files can + * share a name in different folders, and opening the wrong one is worse than + * opening none, so an ambiguous name does not resolve. + */ +function findFileByUniqueName( + files: readonly WorkspaceFileRecord[], + name: string +): WorkspaceFileRecord | undefined { + if (!name) return undefined + let match: WorkspaceFileRecord | undefined + for (const file of files) { + if (file.name !== name) continue + if (match) return undefined + match = file + } + return match +} + +/** + * Turns a chip's best-effort reference into a resource the panel can open, or + * null when nothing identifies it. + * + * An explicit id is authoritative even when the file list has not caught up + * with it — the id is addressable regardless of what this client has cached. + * Everything else has to match a known file, by path first and then by a unique + * name, because a path or a title is only an id if something answers to it. + */ +export function resolveWorkspaceResourceRef( + ref: WorkspaceResourceRef, + files: readonly WorkspaceFileRecord[] +): MothershipResource | null { + const id = ref.id?.trim() + const path = ref.path?.trim() + if (ref.type !== 'file') { + return id ? { type: ref.type, id, title: ref.title } : null + } + + const withPath = path ? { path } : {} + if (id) { + const title = ref.title || files.find((file) => file.id === id)?.name || 'File' + return { type: 'file', id, title, ...withPath } + } + + const match = + findWorkspaceFileByPath(files, path) ?? findFileByUniqueName(files, ref.title.trim()) + if (!match) return null + return { type: 'file', id: match.id, title: ref.title || match.name, ...withPath } +} diff --git a/apps/sim/app/workspace/[workspaceId]/home/types.ts b/apps/sim/app/workspace/[workspaceId]/home/types.ts index 96e131e3ea2..e6d21c27765 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/types.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/types.ts @@ -7,6 +7,7 @@ const RUN_SUBAGENT_ID = 'run' export type { MothershipResource, MothershipResourceType, + WorkspaceResourceRef, } from '@/lib/copilot/resources/types' /** Union of all valid context kind strings, derived from {@link ChatContext}. */ diff --git a/apps/sim/hooks/queries/utils/find-workspace-file-by-src.ts b/apps/sim/hooks/queries/utils/find-workspace-file-by-src.ts index 2c70b1e67d1..2a52f7c8259 100644 --- a/apps/sim/hooks/queries/utils/find-workspace-file-by-src.ts +++ b/apps/sim/hooks/queries/utils/find-workspace-file-by-src.ts @@ -1,6 +1,23 @@ +import { canonicalWorkspaceFilePath } from '@/lib/copilot/vfs/path-utils' import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace' import { extractEmbeddedFileRef } from '@/lib/uploads/utils/embedded-image-ref' +/** + * Resolve the workspace file a canonical VFS path points at (`files/docs/a.md`). + * The path is rebuilt per candidate with the same helper the VFS serves, so a + * caller cannot drift from the encoding the agent's tags use. + */ +export function findWorkspaceFileByPath( + records: readonly WorkspaceFileRecord[] | undefined, + path: string | undefined +): WorkspaceFileRecord | undefined { + if (!path || !records) return undefined + return records.find( + (record) => + canonicalWorkspaceFilePath({ folderPath: record.folderPath, name: record.name }) === path + ) +} + /** * Resolve the workspace file record an embedded image `src` points at, matching the persisted serve-URL * shape by storage key or file id. Returns `undefined` for external / `data:` / unrecognized srcs, and diff --git a/apps/sim/hooks/queries/workspace-files.ts b/apps/sim/hooks/queries/workspace-files.ts index 1c37bfc2215..65469cbe202 100644 --- a/apps/sim/hooks/queries/workspace-files.ts +++ b/apps/sim/hooks/queries/workspace-files.ts @@ -109,6 +109,23 @@ async function fetchWorkspaceFiles( return data.success ? data.files : [] } +/** + * Shared options for the workspace-file list, so an imperative caller can + * `fetchQuery` the same cache entry {@link useWorkspaceFiles} populates instead + * of refetching by key and reading the result back out of the cache. + */ +export function getWorkspaceFilesQueryOptions( + workspaceId: string, + scope: WorkspaceFileQueryScope = 'active' +) { + return { + queryKey: workspaceFilesKeys.list(workspaceId, scope), + queryFn: ({ signal }: { signal?: AbortSignal }) => + fetchWorkspaceFiles(workspaceId, scope, signal), + staleTime: WORKSPACE_FILES_LIST_STALE_TIME, // 30 seconds - files can change frequently + } +} + /** * Hook to fetch workspace files */ @@ -118,10 +135,8 @@ export function useWorkspaceFiles( options?: { enabled?: boolean } ) { return useQuery({ - queryKey: workspaceFilesKeys.list(workspaceId, scope), - queryFn: ({ signal }) => fetchWorkspaceFiles(workspaceId, scope, signal), + ...getWorkspaceFilesQueryOptions(workspaceId, scope), enabled: !!workspaceId && (options?.enabled ?? true), - staleTime: WORKSPACE_FILES_LIST_STALE_TIME, // 30 seconds - files can change frequently placeholderData: keepPreviousData, // Show cached data immediately }) } diff --git a/apps/sim/lib/api/contracts/copilot.ts b/apps/sim/lib/api/contracts/copilot.ts index ff2df7e8b13..59dfe32e07a 100644 --- a/apps/sim/lib/api/contracts/copilot.ts +++ b/apps/sim/lib/api/contracts/copilot.ts @@ -1,4 +1,5 @@ import { z } from 'zod' +import { requiredFieldSchema } from '@/lib/api/contracts/primitives' import { type ContractJsonResponse, defineRouteContract } from '@/lib/api/contracts/types' import { cleanedWorkflowStateSchema } from '@/lib/api/contracts/workflows' import { @@ -102,7 +103,8 @@ export const addCopilotChatResourceBodySchema = z.object({ chatId: z.string(), resource: z.object({ type: copilotResourceTypeSchema, - id: z.string(), + // Matches the bound the chat-send path enforces. + id: requiredFieldSchema('resource.id cannot be empty'), title: z.string(), }), }) diff --git a/apps/sim/lib/copilot/chat/post.ts b/apps/sim/lib/copilot/chat/post.ts index 24da5a81104..0dad00dc5d0 100644 --- a/apps/sim/lib/copilot/chat/post.ts +++ b/apps/sim/lib/copilot/chat/post.ts @@ -54,8 +54,9 @@ import { import type { ExecutionContext, OrchestratorResult } from '@/lib/copilot/request/types' import { persistChatResources } from '@/lib/copilot/resources/persistence' import { - canonicalizeDesktopSessionResources, + hasAddressableId, isEphemeralResource, + sanitizeChatResources, } from '@/lib/copilot/resources/types' import { prepareExecutionContext } from '@/lib/copilot/tools/handlers/context' import { captureServerEvent } from '@/lib/posthog/server' @@ -148,6 +149,20 @@ function isPersistableAttachment(resource: z.infer { + const id = (resource as { id?: unknown } | null)?.id + return typeof id !== 'string' || hasAddressableId(id) + }) +} + /** Non-strings pass through for the schema to reject; strings are sanitized. */ function sanitizeBrowserSelectionUrl(value: unknown): unknown { return typeof value === 'string' ? safeBrowserSelectionUrl(value) : value @@ -257,7 +272,9 @@ const ChatMessageSchema = z.object({ createNewChat: z.boolean().optional().default(false), implicitFeedback: z.string().optional(), fileAttachments: z.array(FileAttachmentSchema).optional(), - resourceAttachments: z.array(ResourceAttachmentSchema).optional(), + resourceAttachments: z + .preprocess(dropUnaddressableAttachments, z.array(ResourceAttachmentSchema)) + .optional(), provider: z.string().optional(), contexts: z.array(ChatContextSchema).optional(), commands: z.array(z.string()).optional(), @@ -1090,7 +1107,10 @@ export async function handleUnifiedChatPost(req: NextRequest) { } if (chatIsNew && actualChatId && body.resourceAttachments?.length) { - const persistable = canonicalizeDesktopSessionResources( + // Canonicalizes here, not just inside `persistChatResources`: several + // browser tabs collapse onto the one Browser panel before they are + // stored, so the chat reopens with a single tab rather than one per page. + const persistable = sanitizeChatResources( body.resourceAttachments.filter(isPersistableAttachment).map((resource) => ({ type: resource.type, id: resource.id, @@ -1398,6 +1418,14 @@ export async function handleUnifiedChatPost(req: NextRequest) { otelRoot?.finish('error', error) if (isZodError(error)) { + // A rejected body otherwise leaves no trace: the client sees a 400 and + // its stream reconnect 404s, which reads as the stream dying for no reason. + logger.warn(`[${requestId}] Rejected chat request as invalid`, { + issues: error.issues.map((issue) => ({ + path: issue.path.join('.'), + message: issue.message, + })), + }) return validationErrorResponse(error, 'Invalid request data') } diff --git a/apps/sim/lib/copilot/request/session/contract.test.ts b/apps/sim/lib/copilot/request/session/contract.test.ts index 3d50fdd64eb..06661e275da 100644 --- a/apps/sim/lib/copilot/request/session/contract.test.ts +++ b/apps/sim/lib/copilot/request/session/contract.test.ts @@ -126,6 +126,19 @@ describe('stream session contract parser', () => { expect(parsePersistedStreamEventEnvelope(event).ok).toBe(true) }) + it('rejects a resource event whose id names nothing', () => { + for (const id of ['', ' ']) { + const event = { + ...BASE_ENVELOPE, + type: 'resource' as const, + payload: { op: 'upsert' as const, resource: { id, type: 'file', title: 'test.md' } }, + } + + expect(isContractStreamEventEnvelope(event)).toBe(false) + expect(parsePersistedStreamEventEnvelope(event).ok).toBe(false) + } + }) + it('accepts contract run events', () => { const event = { ...BASE_ENVELOPE, diff --git a/apps/sim/lib/copilot/request/session/contract.ts b/apps/sim/lib/copilot/request/session/contract.ts index 947b162a0ca..dde683b966c 100644 --- a/apps/sim/lib/copilot/request/session/contract.ts +++ b/apps/sim/lib/copilot/request/session/contract.ts @@ -15,6 +15,7 @@ import { MothershipStreamV1TextChannel, MothershipStreamV1ToolPhase, } from '@/lib/copilot/generated/mothership-stream-v1' +import { hasAddressableId } from '@/lib/copilot/resources/types' import type { FilePreviewTargetKind } from './file-preview-session-contract' type JsonRecord = Record @@ -265,13 +266,18 @@ function isValidSpanPayload(payload: JsonRecord): boolean { } function isValidResourcePayload(payload: JsonRecord): boolean { - return ( - (payload.op === MothershipStreamV1ResourceOp.upsert || - payload.op === MothershipStreamV1ResourceOp.remove) && - isRecordLike(payload.resource) && - typeof (payload.resource as JsonRecord).id === 'string' && - typeof (payload.resource as JsonRecord).type === 'string' - ) + if ( + payload.op !== MothershipStreamV1ResourceOp.upsert && + payload.op !== MothershipStreamV1ResourceOp.remove + ) { + return false + } + if (!isRecordLike(payload.resource)) return false + const resource = payload.resource as JsonRecord + // Dropping a blank id here is the only guard covering both branches + // downstream: the handler adds a suppressed file resource to the tab strip + // directly, bypassing the checks in `addResource`. + return hasAddressableId(resource.id) && typeof resource.type === 'string' } function isValidRunPayload(payload: JsonRecord): boolean { diff --git a/apps/sim/lib/copilot/resources/persistence.ts b/apps/sim/lib/copilot/resources/persistence.ts index 7cc19e75a9c..f4e0e98251b 100644 --- a/apps/sim/lib/copilot/resources/persistence.ts +++ b/apps/sim/lib/copilot/resources/persistence.ts @@ -3,11 +3,7 @@ import { copilotChats } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { eq, sql } from 'drizzle-orm' -import { - canonicalizeDesktopSessionResources, - GENERIC_RESOURCE_TITLES, - type MothershipResource, -} from './types' +import { GENERIC_RESOURCE_TITLES, type MothershipResource, sanitizeChatResources } from './types' export { extractDeletedResourcesFromToolResult, @@ -44,7 +40,7 @@ export async function persistChatResources( if (!chat) return - const existing = canonicalizeDesktopSessionResources( + const existing = sanitizeChatResources( Array.isArray(chat.resources) ? (chat.resources as ChatResource[]) : [] ) const map = new Map() @@ -53,7 +49,7 @@ export async function persistChatResources( map.set(`${r.type}:${r.id}`, r) } - for (const r of canonicalizeDesktopSessionResources(toMerge)) { + for (const r of sanitizeChatResources(toMerge)) { const key = `${r.type}:${r.id}` const prev = map.get(key) if ( @@ -93,15 +89,14 @@ export async function removeChatResources(chatId: string, toRemove: ChatResource if (!chat) return - const existing = canonicalizeDesktopSessionResources( - Array.isArray(chat.resources) ? (chat.resources as ChatResource[]) : [] - ) - const removeKeys = new Set( - canonicalizeDesktopSessionResources(toRemove).map((r) => `${r.type}:${r.id}`) - ) + const stored = Array.isArray(chat.resources) ? (chat.resources as ChatResource[]) : [] + const existing = sanitizeChatResources(stored) + const removeKeys = new Set(sanitizeChatResources(toRemove).map((r) => `${r.type}:${r.id}`)) const filtered = existing.filter((r) => !removeKeys.has(`${r.type}:${r.id}`)) - if (filtered.length === existing.length) return + const removedSomething = filtered.length !== existing.length + const sanitizedSomething = existing.length !== stored.length + if (!removedSomething && !sanitizedSomething) return await db .update(copilotChats) diff --git a/apps/sim/lib/copilot/resources/types.test.ts b/apps/sim/lib/copilot/resources/types.test.ts index b7d2cc39b43..fa142c19298 100644 --- a/apps/sim/lib/copilot/resources/types.test.ts +++ b/apps/sim/lib/copilot/resources/types.test.ts @@ -3,12 +3,13 @@ import { addCopilotChatResourceBodySchema } from '@/lib/api/contracts/copilot' import { BROWSER_SESSION_RESOURCE_ID, canonicalizeDesktopSessionResource, - canonicalizeDesktopSessionResources, + isAddressableResource, isDesktopOnlyResource, isEphemeralResource, type MothershipResource, MothershipResourceType, PERSISTED_RESOURCE_TYPES, + sanitizeChatResources, TERMINAL_SESSION_RESOURCE_ID, } from './types' @@ -55,7 +56,7 @@ describe('isDesktopOnlyResource', () => { describe('desktop session resource identity', () => { it('keeps browser pages as inner tabs of one canonical Browser resource', () => { expect( - canonicalizeDesktopSessionResources([ + sanitizeChatResources([ resource({ type: 'browser', id: 'browser-session:slack-tab', @@ -115,3 +116,40 @@ describe('client and server agree on what can be persisted', () => { expect([...PERSISTED_RESOURCE_TYPES, ...ephemeral].sort()).toEqual([...all].sort()) }) }) + +describe('unaddressable resources', () => { + it('recognizes a resource that points at nothing', () => { + expect(isAddressableResource(resource({ id: '' }))).toBe(false) + expect(isAddressableResource(resource({ id: ' ' }))).toBe(false) + expect(isAddressableResource(resource({ id: 'file-1' }))).toBe(true) + }) + + it('drops a stored blank-id resource, which would otherwise 400 every send', () => { + const stored = [ + resource({ id: '', title: 'reporte-russell.md' }), + resource({ type: 'table', id: 'tbl_1', title: 'kb_agent_queries' }), + ] + expect(sanitizeChatResources(stored)).toEqual([ + { type: 'table', id: 'tbl_1', title: 'kb_agent_queries' }, + ]) + }) + + it('keeps the desktop panels, which are given their ids by canonicalization', () => { + const sanitized = sanitizeChatResources([ + resource({ type: 'browser', id: '', title: 'Browser' }), + resource({ type: 'terminal', id: '', title: 'Terminal' }), + ]) + expect(sanitized.map((r) => r.id)).toEqual([ + BROWSER_SESSION_RESOURCE_ID, + TERMINAL_SESSION_RESOURCE_ID, + ]) + }) + + it('refuses a blank id at the write boundary, matching the send path', () => { + const parsed = addCopilotChatResourceBodySchema.safeParse({ + chatId: 'chat-1', + resource: { type: 'file', id: '', title: 'reporte-russell.md' }, + }) + expect(parsed.success).toBe(false) + }) +}) diff --git a/apps/sim/lib/copilot/resources/types.ts b/apps/sim/lib/copilot/resources/types.ts index 6e78ddd5ebd..7edc630d199 100644 --- a/apps/sim/lib/copilot/resources/types.ts +++ b/apps/sim/lib/copilot/resources/types.ts @@ -22,6 +22,21 @@ export interface MothershipResource { path?: string } +/** + * What a chip in an assistant message knows about the resource it points at, + * before it has been resolved. The agent writes these tags as text, so a file + * it just created is usually named but not yet identified. A ref becomes a + * {@link MothershipResource} only through resolution, which may fail — see + * {@link isAddressableResource} for why the unresolved state is modelled rather + * than filled in. + */ +export interface WorkspaceResourceRef { + type: MothershipResourceType + id?: string + path?: string + title: string +} + interface ResourcePolicy { /** Stored with the chat, so the tab is still there when the chat is reopened. */ persisted: boolean @@ -120,8 +135,36 @@ export function canonicalizeDesktopSessionResource( return resource } -/** Canonicalizes and deduplicates the singleton desktop panels in display order. */ -export function canonicalizeDesktopSessionResources( +/** + * Whether an id value names something the app can act on. + * + * This is the definition every layer defers to, so they cannot disagree about + * whitespace. Takes `unknown` because two of its callers validate untrusted + * input — a stream payload and a chat request body — before it has a type. + */ +export function hasAddressableId(id: unknown): boolean { + return typeof id === 'string' && id.trim().length > 0 +} + +/** + * True when the resource names something the app can actually act on. + * + * A blank id points at nothing: it cannot be opened, resolved into agent + * context, or even removed, since the resources API requires a non-empty + * `resourceId` to delete. Storing one used to be possible, and it made the chat + * reject every later message — the write contract accepted `id: ''` while the + * send schema required `min(1)`. + */ +export function isAddressableResource(resource: MothershipResource): boolean { + return hasAddressableId(resource.id) +} + +/** + * Canonicalizes and deduplicates the singleton desktop panels in display order. + * Module-private: callers want {@link sanitizeChatResources}, which also drops + * unaddressable resources. + */ +function canonicalizeDesktopSessionResources( resources: readonly MothershipResource[] ): MothershipResource[] { const seenDesktopTypes = new Set<'browser' | 'terminal'>() @@ -138,6 +181,19 @@ export function canonicalizeDesktopSessionResources( return canonical } +/** + * The canonical form of a chat's resource list: singleton desktop panels + * collapsed, unaddressable resources dropped. Every path that reads or writes + * stored resources goes through this, which is what heals chats that already + * hold one. Canonicalization runs first, so the browser and terminal panels — + * which are given their ids there — are never dropped for arriving without one. + */ +export function sanitizeChatResources( + resources: readonly MothershipResource[] +): MothershipResource[] { + return canonicalizeDesktopSessionResources(resources).filter(isAddressableResource) +} + /** Placeholder resource titles that a more specific title may overwrite during dedup. */ export const GENERIC_RESOURCE_TITLES = new Set([ 'Table', From 9b69b03356ec3256c5460137ed1568bcce35420b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 6 Aug 2026 14:48:28 -0700 Subject: [PATCH 2/4] fix(chat): require a file chip's reference to resolve before opening it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A rendered link collapses a resource's id and path into one href, so the click handler cannot tell them apart. Classifying on a separator got a bare filename in `path` wrong, and the resolver then trusted it as an id — opening and persisting a tab pointing at nothing. Drop the classifier and let the resolver try each candidate as an id, a VFS path and a unique name. A file ref must now match a record the workspace actually has; the stale-list case is covered by the refetch, so an id that never resolves was never an id. --- .../components/chat-content/chat-content.tsx | 15 +++--- .../home/resolve-resource-ref.test.ts | 22 ++++++++- .../home/resolve-resource-ref.ts | 46 ++++++++++++++----- 3 files changed, 61 insertions(+), 22 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/chat-content/chat-content.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/chat-content/chat-content.tsx index c0c235455f5..1fd9961a504 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/chat-content/chat-content.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/chat-content/chat-content.tsx @@ -281,16 +281,15 @@ const MARKDOWN_COMPONENTS = { e.preventDefault() if (!type || !ref) return const linkText = label || ref - // A file link carries whichever the tag had (`path ?? id`), so - // classify before forwarding: a canonical VFS path is always - // `files/…`, and an id never contains a separator. Labelling an id - // as a path would throw away the only thing that identifies it. - const isVfsPath = type === 'file' && ref.includes('/') + // A file link carries whichever the tag had (`path ?? id`) with no + // way to tell them apart here, so it is forwarded as-is and the + // resolver tries every interpretation against the real file list. window.dispatchEvent( new CustomEvent('wsres-click', { - detail: isVfsPath - ? { type, path: ref, title: linkText } - : { type, id: ref, title: linkText }, + detail: + type === 'file' + ? { type, path: ref, title: linkText } + : { type, id: ref, title: linkText }, }) ) }} diff --git a/apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.test.ts b/apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.test.ts index e1bbe8cdb69..1d65cbd47ae 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.test.ts @@ -25,14 +25,20 @@ function ref(overrides: Partial = {}): WorkspaceResourceRe } describe('resolveWorkspaceResourceRef', () => { - it('trusts an explicit id even when the file list has not caught up', () => { - expect(resolveWorkspaceResourceRef(ref({ id: 'wf_abc' }), [])).toEqual({ + it('resolves an explicit id against the known files', () => { + const files = [file({ id: 'wf_abc', name: 'notes.md' })] + expect(resolveWorkspaceResourceRef(ref({ id: 'wf_abc' }), files)).toEqual({ type: 'file', id: 'wf_abc', title: 'notes.md', + path: 'files/notes.md', }) }) + it('refuses an id no file answers to, rather than opening a tab pointing at nothing', () => { + expect(resolveWorkspaceResourceRef(ref({ id: 'wf_gone' }), [])).toBeNull() + }) + it('resolves a path against the known files', () => { const files = [file({ id: 'wf_abc', name: 'notes.md', folderPath: 'docs' })] expect(resolveWorkspaceResourceRef(ref({ path: 'files/docs/notes.md' }), files)).toEqual({ @@ -43,6 +49,18 @@ describe('resolveWorkspaceResourceRef', () => { }) }) + it('resolves a bare filename handed over as a path', () => { + // A rendered link collapses id and path into one value, and the agent may + // write a bare name into `path` — neither may be taken at face value. + const files = [file({ id: 'wf_abc', name: 'notes.md', folderPath: 'docs' })] + expect(resolveWorkspaceResourceRef(ref({ path: 'notes.md' }), files)?.id).toBe('wf_abc') + }) + + it('resolves an id handed over as a path', () => { + const files = [file({ id: 'wf_abc', name: 'notes.md' })] + expect(resolveWorkspaceResourceRef(ref({ path: 'wf_abc' }), files)?.id).toBe('wf_abc') + }) + it('resolves a title when exactly one file answers to it', () => { const files = [file({ id: 'wf_abc', name: 'notes.md' }), file({ id: 'wf_x', name: 'other.md' })] expect(resolveWorkspaceResourceRef(ref(), files)?.id).toBe('wf_abc') diff --git a/apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.ts b/apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.ts index d1a0f849117..c09836a0510 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/resolve-resource-ref.ts @@ -1,4 +1,5 @@ import type { MothershipResource, WorkspaceResourceRef } from '@/lib/copilot/resources/types' +import { canonicalWorkspaceFilePath } from '@/lib/copilot/vfs/path-utils' import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace' import { findWorkspaceFileByPath } from '@/hooks/queries/utils/find-workspace-file-by-src' @@ -21,14 +22,30 @@ function findFileByUniqueName( return match } +/** A file answering to `reference` as an id, as a VFS path, or by name. */ +function findFileByReference( + files: readonly WorkspaceFileRecord[], + reference: string | undefined +): WorkspaceFileRecord | undefined { + if (!reference) return undefined + return ( + files.find((file) => file.id === reference) ?? + findWorkspaceFileByPath(files, reference) ?? + findFileByUniqueName(files, reference) + ) +} + /** * Turns a chip's best-effort reference into a resource the panel can open, or * null when nothing identifies it. * - * An explicit id is authoritative even when the file list has not caught up - * with it — the id is addressable regardless of what this client has cached. - * Everything else has to match a known file, by path first and then by a unique - * name, because a path or a title is only an id if something answers to it. + * A file chip's reference is lossy — the agent writes the tag by hand, and + * rendering it as a link collapses id and path into one value — so each + * candidate is tried against every interpretation. The match must be a file + * this workspace actually has: an id the client's list has not caught up with + * resolves once the caller refetches, and one that never resolves was never an + * id. Trusting an unverified id would open a tab pointing at nothing, which is + * the state this whole path exists to prevent. */ export function resolveWorkspaceResourceRef( ref: WorkspaceResourceRef, @@ -40,14 +57,19 @@ export function resolveWorkspaceResourceRef( return id ? { type: ref.type, id, title: ref.title } : null } - const withPath = path ? { path } : {} - if (id) { - const title = ref.title || files.find((file) => file.id === id)?.name || 'File' - return { type: 'file', id, title, ...withPath } + let match: WorkspaceFileRecord | undefined + for (const candidate of [id, path, ref.title.trim()]) { + match = findFileByReference(files, candidate) + if (match) break } - - const match = - findWorkspaceFileByPath(files, path) ?? findFileByUniqueName(files, ref.title.trim()) if (!match) return null - return { type: 'file', id: match.id, title: ref.title || match.name, ...withPath } + + return { + type: 'file', + id: match.id, + title: ref.title || match.name, + // Always the matched file's canonical path, never the reference we were + // handed — the viewer reads this as a real path. + path: canonicalWorkspaceFilePath({ folderPath: match.folderPath, name: match.name }), + } } From 791c098e8b778214ebe665384ae25e20e4bdf48c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 6 Aug 2026 15:00:35 -0700 Subject: [PATCH 3/4] fix(chat): tell the user when a resource chip resolves to nothing The chip renders as a button with a hover state, so refusing to open it silently reads as a broken control. Say what happened instead. --- apps/sim/app/workspace/[workspaceId]/home/home.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/home.tsx b/apps/sim/app/workspace/[workspaceId]/home/home.tsx index 14ffc118908..0263611b878 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/home.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/home.tsx @@ -12,7 +12,7 @@ import { useState, useSyncExternalStore, } from 'react' -import { Button, cn } from '@sim/emcn' +import { Button, cn, toast } from '@sim/emcn' import { PanelLeft } from '@sim/emcn/icons' import { createLogger } from '@sim/logger' import { useQueryClient } from '@tanstack/react-query' @@ -474,6 +474,8 @@ export function Home({ chatId, userName, userId, tableViewsEnabled }: HomeProps) openWorkspaceResource(resolved) return } + // The chip looks clickable, so refusing silently reads as a broken button. + toast.error(`Couldn't find "${ref.title}" in this workspace`) logger.warn('Ignored a resource chip that names nothing in this workspace', { type: ref.type, title: ref.title, From 6bea695614d07ec1c699e864b977d1e28f31c503 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 6 Aug 2026 15:09:07 -0700 Subject: [PATCH 4/4] fix(chat): do not report an unreachable workspace as a missing file A failed refetch and a successful one that found nothing were both collapsed to an empty list, so a network blip told the user the file does not exist. Keep the two apart and say which happened. --- .../app/workspace/[workspaceId]/home/home.tsx | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/home.tsx b/apps/sim/app/workspace/[workspaceId]/home/home.tsx index 0263611b878..492b5659464 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/home.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/home.tsx @@ -463,23 +463,29 @@ export function Home({ chatId, userName, userId, tableViewsEnabled }: HomeProps) if (ref.type !== 'file') return // `staleTime: 0` forces the fetch this branch exists for — the cached list - // is what already failed to resolve. `fetchQuery` rejects on error, and - // this handler is invoked as a void callback, so the catch keeps a failed - // refetch on the same "resolved nothing" path as an empty result. + // is what already failed to resolve. `fetchQuery` rejects on error and this + // handler is invoked as a void callback, so failure becomes null rather + // than an unhandled rejection — and stays distinct from an empty list, so + // "we could not look" is never reported as "it is not there". const files = await queryClient .fetchQuery({ ...getWorkspaceFilesQueryOptions(workspaceId), staleTime: 0 }) - .catch(() => []) - const resolved = resolveWorkspaceResourceRef(ref, files) + .catch(() => null) + const resolved = files && resolveWorkspaceResourceRef(ref, files) if (resolved) { openWorkspaceResource(resolved) return } // The chip looks clickable, so refusing silently reads as a broken button. - toast.error(`Couldn't find "${ref.title}" in this workspace`) - logger.warn('Ignored a resource chip that names nothing in this workspace', { + toast.error( + files + ? `Couldn't find "${ref.title}" in this workspace` + : `Couldn't open "${ref.title}" — check your connection and try again` + ) + logger.warn('Ignored a resource chip that did not resolve', { type: ref.type, title: ref.title, hasPath: Boolean(ref.path), + reachedWorkspace: files !== null, }) }