fix(desktop): stage dropped files into the remote session workspace

Finder/OS drops became `@file:/Users/...` refs that only resolve when the gateway shares the local disk, so on a remote gateway non-image files (PDF/CSV/Markdown/...) never reached the agent. Route OS drops through the file.attach / image.attach_bytes upload pipeline — in-app project-tree and gutter drags stay inline workspace-relative refs — across every drop surface: the conversation area, the composer form, the contenteditable input, and the message-edit composer (which still reproduced the bug). Also: - upload dropped files eagerly when a session exists, so the card shows a spinner instead of stalling the send (images stay submit-time to avoid racing their thumbnail write); - round the attachment card and drop the monospace detail; - render image previews from the bytes we already hold, so a pasted/dropped screenshot shows its thumbnail and previews even when its only on-disk copy is a transient path (the data URL is not persisted to localStorage). Supersedes #38615, #41203. Co-authored-by: LeonSGP <154585401+LeonSGP43@users.noreply.github.com> Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
2026-06-09 16:50:08 -05:00
parent 02f878ec5a
commit 4906dcfc25
12 changed files with 584 additions and 142 deletions
@@ -1,11 +1,11 @@
-import { cleanup, render } from '@testing-library/react'
+import { cleanup, render, waitFor } from '@testing-library/react'
 import type { MutableRefObject } from 'react'
 import { useEffect } from 'react'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'

 import { $sessions, setSessions } from '@/store/session'
 import { $connection } from '@/store/session'
-import type { ComposerAttachment } from '@/store/composer'
+import { $composerAttachments, type ComposerAttachment } from '@/store/composer'
 import type { SessionInfo } from '@/types/hermes'

 import { usePromptActions } from './use-prompt-actions'
@@ -385,6 +385,49 @@ describe('usePromptActions file attachment sync', () => {
    })
  })

+  it('passes a path-less @file: ref straight through (no path = nothing to upload)', async () => {
+    // Submit-layer contract: only attachments that carry a `path` are upload
+    // candidates. A path-less ref (an @-mention/context ref or pasted text)
+    // has no bytes to send, so syncAttachments leaves it untouched and the ref
+    // reaches the gateway as-is — correct for workspace-relative refs.
+    //
+    // The MahmoudR drag-drop bug (a Finder PDF that became a local-path text
+    // ref in remote mode) is fixed upstream at the DROP layer: OS drops now
+    // carry a path and route through the upload pipeline instead of becoming a
+    // path-less inline ref. See partitionDroppedFiles in use-composer-actions.
+    $connection.set({ mode: 'remote' } as never)
+    const readFileDataUrl = vi.fn(async () => 'data:application/pdf;base64,JVBERi0=')
+    Object.defineProperty(window, 'hermesDesktop', {
+      configurable: true,
+      value: { readFileDataUrl }
+    })
+
+    const pathlessRef: ComposerAttachment = {
+      id: 'file:devis',
+      kind: 'file',
+      label: 'DEVIS_signed.pdf',
+      // NOTE: no `path` field — only the pre-baked local @file: ref.
+      refText: '@file:`/Users/mahmoud/Downloads/DEVIS_signed.pdf`'
+    }
+
+    const calls: { method: string; params?: Record<string, unknown> }[] = []
+    const requestGateway = vi.fn(async (method: string, params?: Record<string, unknown>) => {
+      calls.push({ method, params })
+      return {} as never
+    })
+
+    let handle: HarnessHandle | null = null
+    render(<Harness onReady={h => (handle = h)} refreshSessions={async () => undefined} requestGateway={requestGateway} />)
+
+    const ok = await handle!.submitText('read this file', { attachments: [pathlessRef] })
+
+    expect(ok).toBe(true)
+    // No path → no file.attach, no byte read: the ref passes through unchanged.
+    expect(calls.map(c => c.method)).toEqual(['prompt.submit'])
+    expect(readFileDataUrl).not.toHaveBeenCalled()
+    expect(calls[0]?.params?.text).toContain('@file:`/Users/mahmoud/Downloads/DEVIS_signed.pdf`')
+  })
+
  it('passes the path directly via file.attach in local mode (no byte upload)', async () => {
    $connection.set({ mode: 'local' } as never)

@@ -518,3 +561,89 @@ describe('usePromptActions sleep/wake session recovery', () => {
  })
 })

+describe('usePromptActions eager attachment upload (drop-time)', () => {
+  afterEach(() => {
+    cleanup()
+    vi.restoreAllMocks()
+    $connection.set(null)
+    $composerAttachments.set([])
+  })
+
+  it('uploads a dropped file the moment it lands (active session) and rewrites the chip with the gateway ref', async () => {
+    // A Finder drop adds a chip with a local path but no attachedSessionId. With
+    // a session already open, the hook should stage it right away — so the send
+    // is instant and the card can show a spinner while bytes upload — instead of
+    // waiting for submit.
+    $connection.set({ mode: 'remote' } as never)
+    const readFileDataUrl = vi.fn(async () => 'data:application/pdf;base64,JVBERi0=')
+    Object.defineProperty(window, 'hermesDesktop', { configurable: true, value: { readFileDataUrl } })
+
+    const calls: string[] = []
+    const requestGateway = vi.fn(async (method: string) => {
+      calls.push(method)
+      if (method === 'file.attach') {
+        return { attached: true, ref_text: '@file:.hermes/desktop-attachments/DEVIS_signed.pdf', uploaded: true } as never
+      }
+      return {} as never
+    })
+
+    $composerAttachments.set([
+      { id: 'file:devis', kind: 'file', label: 'DEVIS_signed.pdf', path: '/Users/mahmoud/Downloads/DEVIS_signed.pdf' }
+    ])
+
+    render(<Harness onReady={() => undefined} refreshSessions={async () => undefined} requestGateway={requestGateway} />)
+
+    await waitFor(() => expect(calls).toContain('file.attach'))
+    await waitFor(() => expect($composerAttachments.get()[0]?.attachedSessionId).toBe(RUNTIME_SESSION_ID))
+
+    const chip = $composerAttachments.get()[0]!
+    expect(chip.refText).toBe('@file:.hermes/desktop-attachments/DEVIS_signed.pdf')
+    expect(chip.uploadState).toBeUndefined()
+    expect(readFileDataUrl).toHaveBeenCalledWith('/Users/mahmoud/Downloads/DEVIS_signed.pdf')
+  })
+
+  it('flags the chip uploadState=error when the eager upload fails, keeping the path so submit can retry', async () => {
+    $connection.set({ mode: 'remote' } as never)
+    Object.defineProperty(window, 'hermesDesktop', {
+      configurable: true,
+      value: { readFileDataUrl: vi.fn(async () => 'data:application/pdf;base64,JVBERi0=') }
+    })
+
+    const requestGateway = vi.fn(async (method: string) => {
+      if (method === 'file.attach') {
+        throw new Error('[Errno 13] Permission denied')
+      }
+      return {} as never
+    })
+
+    $composerAttachments.set([{ id: 'file:x', kind: 'file', label: 'x.pdf', path: '/abs/x.pdf' }])
+
+    render(<Harness onReady={() => undefined} refreshSessions={async () => undefined} requestGateway={requestGateway} />)
+
+    await waitFor(() => expect($composerAttachments.get()[0]?.uploadState).toBe('error'))
+    expect($composerAttachments.get()[0]?.attachedSessionId).toBeUndefined()
+    expect($composerAttachments.get()[0]?.path).toBe('/abs/x.pdf')
+  })
+
+  it('does not eagerly re-upload a chip already attached to this session', async () => {
+    $connection.set({ mode: 'remote' } as never)
+    const requestGateway = vi.fn(async () => ({}) as never)
+
+    $composerAttachments.set([
+      {
+        id: 'file:done',
+        kind: 'file',
+        label: 'done.pdf',
+        path: '/abs/done.pdf',
+        refText: '@file:data/done.pdf',
+        attachedSessionId: RUNTIME_SESSION_ID
+      }
+    ])
+
+    render(<Harness onReady={() => undefined} refreshSessions={async () => undefined} requestGateway={requestGateway} />)
+
+    await Promise.resolve()
+    expect(requestGateway).not.toHaveBeenCalledWith('file.attach', expect.anything())
+  })
+})
+
@@ -1,5 +1,6 @@
 import type { AppendMessage, ThreadMessage } from '@assistant-ui/react'
-import { type MutableRefObject, useCallback } from 'react'
+import { useStore } from '@nanostores/react'
+import { type MutableRefObject, useCallback, useEffect, useRef } from 'react'

 import { getProfiles, transcribeAudio } from '@/hermes'
 import { translateNow, type Translations, useI18n } from '@/i18n'
@@ -27,6 +28,7 @@ import {
  addComposerAttachment,
  clearComposerAttachments,
  type ComposerAttachment,
+  setComposerAttachmentUploadState,
  terminalContextBlocksFromDraft
 } from '@/store/composer'
 import { clearNotifications, notify, notifyError } from '@/store/notifications'
@@ -118,6 +120,87 @@ async function readFileDataUrlForAttach(filePath: string): Promise<string | null
  return dataUrl || null
 }

+type GatewayRequest = <T>(method: string, params?: Record<string, unknown>) => Promise<T>
+
+/**
+ * Stage one file/image attachment into the session workspace and return the
+ * attachment rewritten with the gateway-side ref. Images upload their bytes in
+ * remote mode (so vision works) and pass the path locally; non-image files
+ * upload bytes remotely and pass the path locally. Throws on failure so callers
+ * can surface an error. Shared by submit-time sync, the eager drop-time upload,
+ * and the message-edit composer drop — keep them in lockstep.
+ */
+export async function uploadComposerAttachment(
+  attachment: ComposerAttachment,
+  opts: { remote: boolean; requestGateway: GatewayRequest; sessionId: string }
+): Promise<ComposerAttachment> {
+  const { remote, requestGateway, sessionId } = opts
+  const path = attachment.path ?? ''
+  const label = attachment.label || pathLabel(path)
+
+  if (attachment.kind === 'image') {
+    let result: ImageAttachResponse
+
+    if (remote) {
+      const payload = await readImageForRemoteAttach(path)
+
+      if (!payload) {
+        throw new Error(`Could not read ${label}`)
+      }
+
+      result = await requestGateway<ImageAttachResponse>('image.attach_bytes', {
+        session_id: sessionId,
+        content_base64: payload.contentBase64,
+        filename: payload.filename
+      })
+    } else {
+      result = await requestGateway<ImageAttachResponse>('image.attach', {
+        path,
+        session_id: sessionId
+      })
+    }
+
+    if (!result.attached) {
+      throw new Error(result.message || `Could not attach ${label}`)
+    }
+
+    const attachedPath = result.path || path
+
+    return {
+      ...attachment,
+      attachedSessionId: sessionId,
+      label: attachedPath ? pathLabel(attachedPath) : attachment.label,
+      path: attachedPath,
+      uploadState: undefined
+    }
+  }
+
+  // Non-image file.
+  const dataUrl = remote ? await readFileDataUrlForAttach(path) : null
+
+  if (remote && !dataUrl) {
+    throw new Error(`Could not read ${label}`)
+  }
+
+  const result = await requestGateway<FileAttachResponse>('file.attach', {
+    name: label,
+    path,
+    session_id: sessionId,
+    ...(dataUrl ? { data_url: dataUrl } : {})
+  })
+
+  if (!result.attached || !result.ref_text) {
+    throw new Error(result.message || `Could not attach ${label}`)
+  }
+
+  return {
+    ...attachment,
+    attachedSessionId: sessionId,
+    refText: result.ref_text,
+    uploadState: undefined
+  }
+}
+
 interface PromptActionsOptions {
  activeSessionId: string | null
  activeSessionIdRef: MutableRefObject<string | null>
@@ -239,95 +322,23 @@ export function usePromptActions({

      for (const attachment of attachments) {
        // Already-synced or pathless refs (terminal, url, etc.) pass through.
+        // A drop-time eager upload may already have staged this one (matching
+        // attachedSessionId) — don't re-upload it.
        if (!attachment.path || attachment.attachedSessionId === sessionId) {
          synced.push(attachment)
+
          continue
        }

-        if (attachment.kind === 'image') {
-          let result: ImageAttachResponse
-
-          if (remote) {
-            // The gateway is on another machine — it can't read attachment.path
-            // (a path on THIS disk). Upload the bytes via image.attach_bytes.
-            const payload = await readImageForRemoteAttach(attachment.path)
-
-            if (!payload) {
-              const label = attachment.label || pathLabel(attachment.path)
-              throw new Error(`Could not read ${label}`)
-            }
-
-            result = await requestGateway<ImageAttachResponse>('image.attach_bytes', {
-              session_id: sessionId,
-              content_base64: payload.contentBase64,
-              filename: payload.filename
-            })
-          } else {
-            result = await requestGateway<ImageAttachResponse>('image.attach', {
-              session_id: sessionId,
-              path: attachment.path
-            })
-          }
-
-          if (!result.attached) {
-            const label = attachment.label || pathLabel(attachment.path)
-            throw new Error(result.message || `Could not attach ${label}`)
-          }
-
-          const attachedPath = result.path || attachment.path
-          const nextAttachment: ComposerAttachment = {
-            ...attachment,
-            id: attachment.id,
-            label: attachedPath ? pathLabel(attachedPath) : attachment.label,
-            path: attachedPath,
-            attachedSessionId: sessionId
-          }
+        if (attachment.kind === 'image' || attachment.kind === 'file') {
+          const nextAttachment = await uploadComposerAttachment(attachment, { remote, requestGateway, sessionId })

          if (updateComposerAttachments) {
            addComposerAttachment(nextAttachment)
          }

          synced.push(nextAttachment)
-          continue
-        }

-        if (attachment.kind === 'file') {
-          // Non-image file refs are @file: paths the gateway reads with its file
-          // tools. On a remote gateway the desktop path doesn't exist there, so
-          // upload the bytes; the gateway stages them into the session workspace
-          // and hands back a workspace-relative ref that actually resolves.
-          // Local mode can pass the path directly (gateway shares this disk).
-          const dataUrl = remote ? await readFileDataUrlForAttach(attachment.path) : null
-
-          if (remote && !dataUrl) {
-            const label = attachment.label || pathLabel(attachment.path)
-            throw new Error(`Could not read ${label}`)
-          }
-
-          const result = await requestGateway<FileAttachResponse>('file.attach', {
-            session_id: sessionId,
-            path: attachment.path,
-            name: attachment.label || pathLabel(attachment.path),
-            ...(dataUrl ? { data_url: dataUrl } : {})
-          })
-
-          if (!result.attached || !result.ref_text) {
-            const label = attachment.label || pathLabel(attachment.path)
-            throw new Error(result.message || `Could not attach ${label}`)
-          }
-
-          const nextAttachment: ComposerAttachment = {
-            ...attachment,
-            id: attachment.id,
-            refText: result.ref_text,
-            attachedSessionId: sessionId
-          }
-
-          if (updateComposerAttachments) {
-            addComposerAttachment(nextAttachment)
-          }
-
-          synced.push(nextAttachment)
          continue
        }

@@ -339,6 +350,62 @@ export function usePromptActions({
    [requestGateway]
  )

+  // Stage a freshly dropped file as soon as it lands (when a session already
+  // exists), so the upload runs while the user is still typing rather than
+  // stalling the send. The card shows a spinner via `uploadState`; on success
+  // the chip carries its gateway-side ref so submit skips re-uploading.
+  //
+  // Images are intentionally NOT eager-uploaded: attachImagePath adds the chip
+  // and then fills in `previewUrl` (the base64 thumbnail) on a second tick, so
+  // an eager upload would race that write — clobbering the thumbnail and
+  // swapping `path` to a gateway path the local preview can't read. Images are
+  // small and still byte-upload at submit via image.attach_bytes.
+  const eagerlyUploadAttachment = useCallback(
+    async (sessionId: string, attachment: ComposerAttachment) => {
+      const remote = $connection.get()?.mode === 'remote'
+
+      setComposerAttachmentUploadState(attachment.id, 'uploading')
+
+      try {
+        addComposerAttachment(await uploadComposerAttachment(attachment, { remote, requestGateway, sessionId }))
+      } catch (err) {
+        // Leave the chip in place so submit-time sync can retry (or the user can
+        // remove it) and flag the card; also toast so a hard failure (unreadable
+        // file, gateway perms) isn't swallowed while the user keeps typing.
+        setComposerAttachmentUploadState(attachment.id, 'error')
+        notifyError(err, copy.dropFiles)
+      }
+    },
+    [copy.dropFiles, requestGateway]
+  )
+
+  const composerAttachments = useStore($composerAttachments)
+  const eagerUploadInFlight = useRef<Set<string>>(new Set())
+
+  useEffect(() => {
+    if (!activeSessionId) {
+      return
+    }
+
+    for (const attachment of composerAttachments) {
+      const needsUpload =
+        attachment.kind === 'file' &&
+        Boolean(attachment.path) &&
+        !attachment.attachedSessionId &&
+        !attachment.uploadState &&
+        !eagerUploadInFlight.current.has(attachment.id)
+
+      if (!needsUpload) {
+        continue
+      }
+
+      eagerUploadInFlight.current.add(attachment.id)
+      void eagerlyUploadAttachment(activeSessionId, attachment).finally(() =>
+        eagerUploadInFlight.current.delete(attachment.id)
+      )
+    }
+  }, [activeSessionId, composerAttachments, eagerlyUploadAttachment])
+
  const submitPromptText = useCallback(
    async (rawText: string, options?: SubmitTextOptions) => {
      const visibleText = rawText.trim()