Skip to content

Attach dropped files in every composer#1750

Open
boudra wants to merge 2 commits into
mainfrom
fix/composer-drop-attachments
Open

Attach dropped files in every composer#1750
boudra wants to merge 2 commits into
mainfrom
fix/composer-drop-attachments

Conversation

@boudra

@boudra boudra commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Linked issue

Refs #520

Type of change

  • Bug fix
  • New feature (with prior issue + design alignment)
  • Refactor / code improvement
  • Docs

What does this PR do

Dropped files now attach from the composer itself instead of depending on each parent screen to wire drag-and-drop callbacks. Non-image drops go through the existing file upload and file pill path, while raster images keep using the image attachment path.

This fixes the New Workspace case where a dropped JSON file was silently ignored because that screen only wired image drops. Active chat, New Workspace, workspace draft, and workspace setup now share the same composer-owned drop behavior.

How did you verify it

  • Ran npm run format
  • Ran npm run test --workspace=@getpaseo/app -- src/composer/attachments/drop.test.ts --bail=1
  • Ran npm run typecheck --workspace=@getpaseo/app
  • Ran targeted npm run lint -- ... on the touched files
  • Ran npm run test:e2e --workspace=@getpaseo/app -- composer-attachments.spec.ts --grep "dropped JSON file"
  • Pre-commit hook also ran targeted format/lint and full workspace npm run typecheck

Risk surface: drag/drop is web and desktop-facing; the Playwright coverage exercises desktop web for active chat and New Workspace. Native platforms should keep rendering the composer without the web drop wrapper behavior.

Checklist

  • One focused change. Unrelated cleanups split out.
  • npm run typecheck passes
  • npm run lint passes
  • npm run format ran (Biome)
  • UI changes include screenshots or video for every affected platform
  • Tests added or updated where it made sense

Composer parents had to expose attachment callbacks, so New Workspace only wired image drops and JSON files never became file attachments. Centralize dropped-file ingestion in Composer so all composer surfaces share the same image and file path.
@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown

Greptile Summary

This PR moves drag-and-drop file attachment handling from individual parent screens into the Composer component itself, fixing a long-standing bug where dropping a JSON file on the New Workspace screen was silently ignored because that screen only wired image drops.

  • composer/attachments/drop.ts is a new focused module that converts DroppedItem[] into PickedFile[], skipping raster images (which flow through the existing image path) and using an injectable runtime seam for testability — both branches are exercised in the adjacent unit test.
  • PickedFile and readDesktopFileBytes are extracted from use-file-picker.ts into attachments/picked-file.ts so both the picker and the new drop handler can import them without a circular dependency.
  • All four call sites (active chat, New Workspace, workspace draft, workspace setup dialog) remove their FileDropZone wrappers, addImagesRef/addFilesRef refs, and the onAddImages/onAddFiles callback props from Composer; two new E2E tests validate the fixed behavior end-to-end.

Confidence Score: 5/5

Safe to merge — the change is a focused refactor that centralises existing behavior with no new data paths, backed by unit tests and two new E2E scenarios.

Drop handling was previously duplicated across four screens using a fragile ref-based callback pattern; this PR collapses that into a single, well-tested module. The old Windows-path bug in fileNameFromPath was fixed in the new implementation and is now covered by a dedicated test case. No new async flows or state mutations were introduced, and the archiving-disable behavior is naturally preserved because AgentComposerSection returns null when archiving, making the embedded FileDropZone absent during that state.

No files require special attention. The use-file-picker.ts desktop dialog filename extraction retains its pre-existing Windows-path quirk, but that code was not changed in this PR.

Important Files Changed

Filename Overview
packages/app/src/composer/attachments/drop.ts New module centralizing dropped-item-to-PickedFile conversion; correctly uses split(/[/\]/) loop to handle Windows paths, skips raster images, exposes injectable runtime seam for testing.
packages/app/src/composer/attachments/drop.test.ts Unit tests cover both web-file and desktop-path branches; the desktop-path test injects readDesktopFileBytes via the runtime port — clean adapter pattern, no vi.mock used.
packages/app/src/composer/index.tsx FileDropZone moved inside Composer; onAddImages/onAddFiles prop callbacks removed; uploadPickedFiles extracted for reuse by both handlePickFile and handleGenericFilesDropped; error handling correctly scoped in each callback.
packages/app/src/attachments/picked-file.ts PickedFile interface and readDesktopFileBytes extracted from use-file-picker into a shared module; allows drop.ts to import without creating a circular dependency.
packages/app/src/panels/agent-panel.tsx Removed ref-based addImagesRef/addFilesRef, FileDropZone wrapper, and all handleGenericFilesDropped logic; drop handling now flows through Composer. AgentComposerSection still returns null when isArchivingCurrentAgent, so the embedded FileDropZone is absent during archiving — correct behavior.
packages/app/src/screens/new-workspace-screen.tsx Removed addImagesRef, handleFilesDropped, handleAddImagesCallback, and the wrapping FileDropZone; drop handling now owned by Composer, fixing the silent-JSON-drop bug on this screen.
packages/app/src/composer/draft/workspace-tab.tsx Removed outer FileDropZone wrapper and addImagesRef/handleAddImagesCallback plumbing; the workspace draft tab now relies on Composer's embedded drop zone.
packages/app/src/components/workspace-setup-dialog.tsx Removed addImagesRef, handleFilesDropped, handleAddImagesCallback, and onFilesDropped/onAddImages props; clean deletion, no leftover references.
packages/app/src/components/file-drop-zone.tsx Added style prop to allow callers to override the default flex:1 container; containerStyle correctly merges via useMemo. Non-web path still renders children directly and ignores the style prop.
packages/app/src/hooks/use-file-picker.ts PickedFile and readDesktopFileBytes moved to picked-file.ts; desktop dialog path now delegates to the shared helper.
packages/app/e2e/composer-attachments.spec.ts Two new E2E tests: active-chat JSON drop (clean, 5-line body) and New Workspace JSON drop (uses try/finally for seedWorkspace cleanup, consistent with existing patterns in this file).
packages/app/e2e/helpers/composer.ts New dropFileOnComposer helper builds a DataTransfer in-browser via evaluateHandle and dispatches dragenter/dragover/drop on the message-input-root; correctly disposes the handle after use.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User drops files onto Composer] --> B{IS_WEB?}
    B -- No --> C[Native: no-op, children rendered directly]
    B -- Yes --> D[FileDropZone captures dragenter/dragover/drop]
    D --> E{Item type}
    E -- web-file image --> F[addImages → ImageAttachment path]
    E -- web-file non-image --> G[droppedItemsToPickedFiles]
    E -- desktop-path image --> H[skip raster image]
    E -- desktop-path non-image --> G
    G --> I[uploadPickedFiles]
    I --> J{client connected?}
    J -- No --> K[toast error]
    J -- Yes --> L{oversized?}
    L -- Yes --> M[toast size error]
    L -- No --> N[uploadFileAttachments IPC]
    N --> O[addFiles → UserComposerAttachment pill]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[User drops files onto Composer] --> B{IS_WEB?}
    B -- No --> C[Native: no-op, children rendered directly]
    B -- Yes --> D[FileDropZone captures dragenter/dragover/drop]
    D --> E{Item type}
    E -- web-file image --> F[addImages → ImageAttachment path]
    E -- web-file non-image --> G[droppedItemsToPickedFiles]
    E -- desktop-path image --> H[skip raster image]
    E -- desktop-path non-image --> G
    G --> I[uploadPickedFiles]
    I --> J{client connected?}
    J -- No --> K[toast error]
    J -- Yes --> L{oversized?}
    L -- Yes --> M[toast size error]
    L -- No --> N[uploadFileAttachments IPC]
    N --> O[addFiles → UserComposerAttachment pill]
Loading

Reviews (2): Last reviewed commit: "fix(app): handle desktop dropped attachm..." | Re-trigger Greptile

Comment thread packages/app/src/composer/attachments/drop.ts
Comment thread packages/app/src/composer/attachments/drop.test.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant