Skip to content

feat(ui): add prompt attachment picker#417

Closed
pascalandr wants to merge 4 commits into
NeuralNomadsAI:devfrom
Pagecran:feat/upload-attachment-button
Closed

feat(ui): add prompt attachment picker#417
pascalandr wants to merge 4 commits into
NeuralNomadsAI:devfrom
Pagecran:feat/upload-attachment-button

Conversation

@pascalandr
Copy link
Copy Markdown
Contributor

@pascalandr pascalandr commented May 9, 2026

Summary

  • Add a paperclip action to the prompt composer that opens the native multi-file picker.
  • Reuse the existing attachment pipeline so selected files behave like drag-and-drop attachments, including image/text previews where available.
  • Localize the new attach action labels across supported UI languages.

Closes #380

Validation

  • npm run typecheck --workspace @codenomad/ui
  • npm run build:tauri
  • Feature tested manually
image

Add a visible paperclip action to the prompt so users can choose local files instead of relying only on paste or drag and drop.

The picker reuses the existing attachment pipeline, including image and text previews, while reading browser-selected files as data URLs when no native file path is available.

All user-visible labels are localized across the existing message catalogs. Validated with UI typecheck, UI build, and git diff whitespace checks.
@pascalandr
Copy link
Copy Markdown
Contributor Author

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot Bot commented May 9, 2026

Gatekeeper review for latest PR state.

Request understood: review this PR ruthlessly for regressions and implementation quality. Assumption: no code changes were requested, only a gatekeeper review.

Findings:

  1. Blocking: picker-selected files can fall back to data URLs/fake paths instead of real filesystem attachments.
    packages/ui/src/components/prompt-input/usePromptAttachments.ts:313-333

    The new picker path only reads (file as File & { path?: string }).path. If that is absent, it falls back to file.name, then shouldReadDataUrl becomes true for every file because !nativePath. That means arbitrary selected binaries can be read fully into renderer memory and sent as data: URLs, while source.path is just the basename. This is a regression risk versus the existing path-based attachment model: large files can freeze/bloat the UI/request payload, duplicate basenames become ambiguous, and the backend no longer receives a stable file:// path for normal file attachments.

    This is especially risky because the repo already has platform-specific path extraction logic for dropped files in packages/ui/src/lib/native/desktop-file-drop.ts:20-31, including the Electron electronAPI.getPathForFile(file) fallback. The picker implementation bypasses that helper entirely. A safer implementation would reuse/export a shared getFilePath(file) path resolver, or preferably use the native dialog API to return selected filesystem paths directly and create the same path-backed attachments as the existing picker/drop flows.

  2. High: large non-previewable files are eagerly read into memory when no native path is exposed.
    packages/ui/src/components/prompt-input/usePromptAttachments.ts:326-333

    The condition !nativePath || mime.startsWith("image/") || mime.startsWith("text/") reads every selected file as a data URL whenever the platform does not expose File.path. That includes archives, videos, PDFs, and other large binaries. There is no size guard, no MIME allowlist for this fallback, and no error handling for FileReader.onerror. This can make a simple attachment action lock up the composer or silently fail to attach on read errors. At minimum, the data URL fallback should be restricted to the same preview cases (image/*, text/*) or bounded by size, with non-previewable files requiring a real native path.

  3. Medium: hidden input remains enabled while the composer can become disabled.
    packages/ui/src/components/prompt-input.tsx:723-735

    The visible attach button respects props.disabled, but the underlying file input is not disabled and handleFileInputChange does not re-check props.disabled. If the session/composer becomes disabled while the OS picker is open, selecting files will still enqueue attachments. This is a small race, but composer disabled state should be enforced at the change handler/input level too.

Implementation notes:

  • The UI placement and i18n coverage are generally tight and match existing patterns.
  • The refactor that routes drag/drop through handleFileSelection is reasonable, but it also changes unknown MIME fallback from text/plain to application/octet-stream; that seems defensible, but it makes the path-resolution issue more important.
  • Consider adding a targeted unit/helper test around file path resolution and fallback behavior if the attachment pipeline has a test harness available.

Validation:

  • git diff --check origin/dev...HEAD: passed.
  • npm run typecheck --workspace @codenomad/ui: could not validate in this checkout because dependencies are not installed (node_modules is absent), causing missing-module errors such as solid-js, @tauri-apps/api, and SDK packages before PR-specific type errors can be evaluated.

File size note:

  • packages/ui/src/components/prompt-input.tsx is about 827 lines after this PR, above the project’s target limit. I would not block this PR solely for that, but future prompt composer work should probably split more behavior out instead of growing this file further.

Gatekeeper verdict: request changes before merge. The feature intent is good, but the picker should not ship until file selections reliably preserve real filesystem paths or have a bounded, explicit upload/data fallback.

--
Yours,
CodeNomadBot

Reuse the existing desktop file path resolver so picker-selected files stay path-backed whenever the runtime exposes a native filesystem path.

Only fall back to data URLs for bounded image and text previews, skip non-previewable files without a native path instead of reading arbitrary binaries into renderer memory, and keep path-backed attachments when preview reads fail.

Also enforce the disabled composer state at the hidden file input and selection handler to prevent attachments being queued after the prompt becomes disabled.

Validation: git diff --check; npm run typecheck --workspace @codenomad/ui; npm run build --workspace @codenomad/ui
@pascalandr
Copy link
Copy Markdown
Contributor Author

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot Bot commented May 9, 2026

Updated gatekeeper review for latest PR state (ca22929674ba27d25bff7eade129d85a29e9dd4a).

Request understood: re-run the ruthless PR gatekeeper review against the updated branch. Assumption: this is review-only; no code changes requested.

Findings:

  1. High: pathless selected files can still fail silently, so the picker does not reliably attach what the user selected.
    packages/ui/src/components/prompt-input/usePromptAttachments.ts:317-326

    The update fixes the worst prior behavior by sharing getFilePath(file) and bounding inline reads to 5 MiB, but the fallback path is now:

    if (!nativePath && !canInlinePreview) {
      continue
    }

    That means any selected file without an exposed native path and without an image/* or text/* MIME under 5 MiB is silently ignored. This is not just theoretical: browser file inputs often report an empty/unknown MIME for source files and other useful attachments (.ts, .tsx, .json, .md, extensionless config files, archives, etc.). If the host does not expose File.path or the Electron bridge cannot resolve it, the user picks a file, the dialog closes, and nothing appears. That is a bad failure mode for a primary attach action.

    Better implementation: use the platform-native dialog path flow for the attach button where possible, returning filesystem paths directly, then keep the HTML File fallback only for explicitly supported inline previews and surface a visible error/notification for skipped files. Silent continue should not be the final behavior for user-selected files.

  2. Medium: disabled drag-over no longer prevents browser default file handling.
    packages/ui/src/components/prompt-input/usePromptAttachments.ts:297-301

    handleDragOver now returns immediately when disabled, before preventDefault(). For file drops, the browser typically requires dragover.preventDefault() to allow the controlled drop handler path. If the composer is disabled and the user drops a file over it, the browser/webview may perform default file handling instead of the app safely ignoring the drop. handleDrop already prevents default before checking disabled, so handleDragOver should probably do the same: prevent/stop the event, clear/avoid drag state, then return without accepting files.

  3. Medium: exporting getFilePath from desktop-file-drop.ts couples prompt attachment code to the whole desktop drop module.
    packages/ui/src/components/prompt-input/usePromptAttachments.ts:5
    packages/ui/src/lib/native/desktop-file-drop.ts:1-25

    The hook now imports a small path resolver from a module that also imports Tauri event APIs and owns folder-drop listeners. That works structurally, but it is not a clean dependency boundary: opening the prompt composer attachment hook now pulls in folder-drop/Tauri-listener concerns just to ask “can I resolve this File to a path?”. A better implementation is a tiny shared helper module, e.g. lib/native/file-path.ts, containing only getFilePath(file), used by both desktop drop and prompt attachments. This keeps the file picker feature from depending on unrelated folder-drop machinery.

What improved since the previous review:

  • The prior disabled-input race was addressed with disabled={props.disabled} and a disabled guard in handleFileInputChange.
  • The prior unbounded binary data URL read was addressed with MAX_INLINE_PICKED_FILE_BYTES and canInlinePreview.
  • The prior bypass of Electron path resolution was partially addressed by exporting/reusing getFilePath(file).

Validation:

  • git diff --check origin/dev...HEAD: passed.
  • npm run typecheck --workspace @codenomad/ui: still not meaningfully runnable in this checkout because dependencies are missing (node_modules is absent), causing missing-module errors for solid-js, @tauri-apps/api, SDK packages, etc. before PR-specific type errors can be isolated.

File size note:

  • packages/ui/src/components/prompt-input.tsx is about 833 lines after this PR, above the project’s target limit. I would not block on that alone, but future composer work should split behavior out rather than keep growing this file.

Gatekeeper verdict: request changes before merge. The update fixes the most dangerous memory/path issues, but the attach action still has an unacceptable silent-failure path for legitimate user-selected files and should use a cleaner native path resolution boundary.

--
Yours,
CodeNomadBot

Route the prompt attachment button through the platform-native multi-file dialog when available so selected files are attached by filesystem path instead of relying on HTML File metadata that may omit paths or MIME types.

Keep the HTML file input as a fallback for supported inline previews, surface a localized warning when fallback files cannot be attached, and keep disabled drag-over events from falling through to browser default file handling.

Move file path resolution into a small native helper shared by prompt attachments and desktop folder drops, avoiding a dependency from the prompt composer to the folder-drop listener module.

Validated with git diff --check, npm run typecheck --workspace @codenomad/ui, and npm run build --workspace @codenomad/ui.
@pascalandr
Copy link
Copy Markdown
Contributor Author

@CodeNomadBot
same review again

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot Bot commented May 9, 2026

Updated gatekeeper review for latest PR state (459c1d408169b2cc084965ec99498f5a4c70bf2e).

Request understood: run the same ruthless gatekeeper review again against the current branch. Assumption: review-only; no code changes requested.

Findings:

  1. Blocking: Electron native multi-file picker still cannot select multiple files.
    packages/ui/src/lib/native/electron/functions.ts:33-35
    packages/electron-app/electron/main/ipc.ts:47-61

    The UI now calls openNativeFileDialogs({ multiple: true }), and the renderer-side Electron wrapper expects result.paths to contain multiple paths. But the Electron main IPC handler still builds dialog properties as only ['openFile'] for file mode:

    const properties = request.mode === "directory" ? ["openDirectory", "createDirectory"] : ["openFile"]

    It never accepts request.multiple, never adds Electron’s multiSelections property, and the DialogOpenRequest interface does not include multiple. Result: on Electron, the new “native multi-file picker” opens a single-file dialog despite the PR’s core feature claim. Tauri was updated for multiple; Electron main must be updated too, otherwise this ships inconsistent desktop behavior.

  2. Medium: native path attachments lose MIME/preview behavior for selected image/text files.
    packages/ui/src/components/prompt-input/usePromptAttachments.ts:326-331

    The native dialog path now creates every selected path-backed attachment as application/octet-stream. That is safe for path delivery, but it means image files picked through the new native path flow will not be recognized by message-item.tsx as images (mime.startsWith("image/") or data:image/), and text/image preview behavior is worse than drag/drop or the HTML input fallback. The PR summary explicitly says selected files behave like drag-and-drop attachments, including image/text previews where available. Native path selection currently discards the available filename extension signal entirely.

    Better implementation: infer MIME from extension for common image/text types before creating the attachment, or preserve a preview path for images/text when the picker returns a path and the environment can safely expose/read preview data.

  3. Low/Medium: native dialog cancellation and empty selections are indistinguishable from successful empty selection.
    packages/ui/src/components/prompt-input.tsx:399-401
    packages/ui/src/lib/native/native-functions.ts:46-49

    openNativeFileDialogs collapses cancellation, unsupported handlers, and empty results into [], and handleAttachFiles always calls handleNativeFilePathSelection(paths). This is not user-visible most of the time, but it means telemetry/debugging and future UX cannot distinguish “user cancelled” from “native bridge failed/returned malformed data”. If the native bridge fails, this silently does nothing. Consider logging native dialog failures at the wrapper boundary or preserving a result shape that can represent cancellation vs failure vs selected paths.

What improved since the previous review:

  • The path resolver has been split into packages/ui/src/lib/native/file-path.ts, fixing the prior coupling complaint.
  • Disabled drag-over now prevents/stops the event before returning, fixing the browser-default drop risk.
  • Pathless HTML-input fallback now warns via localized toast instead of silently skipping files.
  • Native dialog selection is now preferred where supported, which is the right direction for stable filesystem paths.

Validation:

  • git diff --check origin/dev...HEAD: passed.
  • npm run typecheck --workspace @codenomad/ui: still not meaningfully runnable in this checkout because dependencies are missing (node_modules is absent), causing missing-module errors for solid-js, @tauri-apps/api, SDK packages, etc. before PR-specific type errors can be isolated.

File size note:

  • packages/ui/src/components/prompt-input.tsx is about 840 lines after this PR, above the project’s target limit. I would not block on that alone, but future composer work should split behavior out rather than keep growing this file.

Gatekeeper verdict: request changes before merge. The implementation is much closer, but Electron still does not implement the promised native multi-file selection, so the main feature is incomplete on one desktop runtime.

--
Yours,
CodeNomadBot

Honor multi-file selection in Electron's native dialog bridge so the prompt attachment button behaves consistently with Tauri.

Infer MIME types for path-based native attachments from common image and text extensions, preserving previews when file contents are not read in the renderer.

Validated with UI and Electron typechecks plus the UI production build before committing.
@pascalandr
Copy link
Copy Markdown
Contributor Author

Addressed in 49c6161. Electron now passes \multiSelections\ when the native prompt attachment picker requests multiple files, and path-based native attachments infer common image/text MIME types by extension so previews remain available without reading file contents in the renderer.

@pascalandr
Copy link
Copy Markdown
Contributor Author

@CodeNomadBot
same review again

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot Bot commented May 9, 2026

Updated gatekeeper review for latest PR state (49c6161e7c7f065d25a0f66a5bf95f9a24712e4c).

Request understood: run the same ruthless gatekeeper review again after the author noted fixes in 49c6161e. Assumption: review-only; no code changes requested.

Findings:

  1. Low/Medium: native dialog failures are still silent from the attach button path.
    packages/ui/src/components/prompt-input.tsx:398-401
    packages/ui/src/lib/native/native-functions.ts:46-50

    openNativeFileDialogs still collapses cancellation, unsupported handlers, native bridge failures, and malformed empty results into [], and handleAttachFiles passes that straight into handleNativeFilePathSelection. User cancellation should be quiet, but actual native dialog failure should not be indistinguishable from cancellation. The lower-level Electron/Tauri wrappers log caught exceptions, so this is not a blocker, but if the native handler is absent or returns an unexpected empty result, the attach action silently does nothing. A small result shape or explicit logging at openNativeFileDialogs would make this easier to diagnose.

  2. Low: native path MIME inference is hand-maintained and intentionally incomplete.
    packages/ui/src/components/prompt-input/usePromptAttachments.ts:313-360

    The new extension map restores common image/text behavior and fixes the prior application/octet-stream regression. The remaining risk is that preview behavior now depends on this local allowlist. That is acceptable for this PR scope, but it should be treated as a maintenance point: files like .svelte, .vue, .ini, .lock, .dockerfile, and extensionless text files will still be generic unless added later. I would not block merge on this because the path attachment itself still works.

What improved since the previous review:

  • Electron IPC now accepts multiple and pushes multiSelections, so the native multi-file picker feature is implemented for Electron as well as Tauri.
  • Native path attachments now infer common image/text MIME types by extension, restoring image preview behavior for common selected files without renderer file reads.
  • Shared file path resolution remains isolated in packages/ui/src/lib/native/file-path.ts.
  • Disabled drag/drop handling and pathless fallback warning behavior remain fixed.

Validation:

  • git diff --check origin/dev...HEAD: passed.
  • npm run typecheck --workspace @codenomad/ui: still not meaningfully runnable in this checkout because dependencies are missing (node_modules is absent), causing missing-module errors for solid-js, @tauri-apps/api, SDK packages, etc. before PR-specific type errors can be isolated.

File size note:

  • packages/ui/src/components/prompt-input.tsx is about 840 lines, above the project’s target limit.
  • packages/ui/src/components/prompt-input/usePromptAttachments.ts is about 471 lines, approaching the source-file warning threshold.
  • I would not block this PR on either size note, but the prompt composer/attachment area is becoming a clear refactor candidate for future work.

Gatekeeper verdict: no blocking findings in the latest state. The previous blockers appear addressed. I would still prefer better native dialog result diagnostics, but that is not enough to hold this PR.

--
Yours,
CodeNomadBot

@shantur
Copy link
Copy Markdown
Collaborator

shantur commented May 9, 2026

Screenshot 2026-05-09 at 22 41 59

When voice controls are enabled. I think the Prompt input needs a re-think now

@pascalandr
Copy link
Copy Markdown
Contributor Author

Screenshot 2026-05-09 at 22 41 59

When voice controls are enabled. I think the Prompt input needs a re-think now

Aaarg, didn't think about that.

@shantur
Copy link
Copy Markdown
Collaborator

shantur commented May 9, 2026

Aaarg, didn't think about that.

Actually, we don't need to do it as part of this PR. That should be a separate work that is now needed.
I will test and review this

@shantur
Copy link
Copy Markdown
Collaborator

shantur commented May 9, 2026

Before I do, has this been tested on all - Electron, Tauri and web interfaces?

@pascalandr
Copy link
Copy Markdown
Contributor Author

nope only on tauri, ok will try with the other.

@shantur
Copy link
Copy Markdown
Collaborator

shantur commented May 11, 2026

@pascalandr - The attachment is handled a bit differently in CodeNomad / opencode. The current PR uses a standard local attachment behavior but we need to support

  1. Remote server attachments
  2. Browser
  3. Attachments as data not file path

I have created another PR on top of this with the changes. #432
Would you be able to check the Tauri for that?

@shantur
Copy link
Copy Markdown
Collaborator

shantur commented May 11, 2026

Superseded by #432

@shantur shantur closed this May 11, 2026
@shantur
Copy link
Copy Markdown
Collaborator

shantur commented May 11, 2026

Thanks for working on this @pascalandr

shantur added a commit that referenced this pull request May 11, 2026
## Summary
- Reuses the directory browser for web file attachment selection and
removes the separate filesystem browser dialog.
- Reads web-selected files through the filesystem browser API so
selection respects unrestricted and configured workspace roots.
- Keeps Electron/Tauri picker selection on the FileList ingestion path
so picked files behave like dropped files with byte-backed attachments.

## Validation
- npm run typecheck --workspace @codenomad/ui
- npm run typecheck --workspace @neuralnomads/codenomad
- npm run typecheck --workspace @neuralnomads/codenomad-electron-app
- cargo check
- git diff --check

## Notes
- Follow-up to #417; this branch currently includes the commits from
that PR plus the attachment picker parity fix.

---------

Co-authored-by: Pascal André <pascalandr@gmail.com>
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.

Allow upload image by button

2 participants