Skip to content

fix(email): retain files in attachment and inline-image pickers - #6935

Merged
alteixeira20 merged 5 commits into
odysseus-dev:devfrom
redpersongpt:codex/email-paperclip-files
Oct 30, 2026
Merged

alteixeira20 merged 5 commits into
odysseus-dev:devfrom
redpersongpt:codex/email-paperclip-files

Conversation

@redpersongpt

@redpersongpt redpersongpt commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The email paperclip handler retained the input's live FileList, then reset the input before handing the files to the uploader. That reset emptied the list, so choosing files produced no upload while drag-and-drop still worked. Snapshot the files before resetting the input in both the attachment and inline-image handlers. The input still resets so selecting the same file again works.

Target branch

  • This PR targets the correct integration branch: dev in the public repository. main remains release-curated.

Linked Issue

Fixes #2227.

Part of #2271: the shared inline-image picker has the same live-list reset bug. Its repair is included here, with actual email rich-body image validation; regular Markdown and other document-format insertion semantics are unchanged and were not separately validated.

Earlier #2258 closed in favor of #2296; #2296 was later approved but is closed without merge. Current dev still contains the live-FileList/reset sequence. This carries the narrow email selection fix, without adding the document image-insertion features discussed in that older review. Current open/closed PR searches found no equivalent open fix; #6019's gallery changes do not modify this handler.

Type of Change

  • Bug fix (non-breaking — fixes a confirmed issue)
  • New feature
  • Breaking change
  • Refactor / cleanup
  • Documentation only
  • CI / tooling / configuration

Checklist

  • I searched open issues and open/closed PRs; related work is linked above.
  • This PR targets the correct integration branch (dev).
  • My changes are limited to the scope described above.
  • I actually ran the app with authenticated Uvicorn, the actual paperclip/upload-menu/file-chooser controls and real compose-upload storage at desktop/mobile widths, before and after the fix.
  • I did not run the app/runtime validation and stated that gap in How to Test.

AI assistance was used. The non-agent attestation below is intentionally unchecked.

How to Test

With Node dependencies and Chromium installed:

python -m pytest -q tests/test_email_paperclip_files_browser.py \
  tests/test_frontend_module_version_parity.py tests/test_security_regressions.py
node --check static/js/document.js
node --check static/sw.js
git diff --check

Original validation: 111 passed, one existing SQLAlchemy deprecation warning. All eight modified JavaScript files were syntax checked. Eight new browser cases execute both actual handlers extracted with the repository's document-source helper, using Chromium's native file input and real File objects. Single/multiple/repeated attachment selection fail on the original 57a1859 base; the same three inline-image cases fail before the follow-up fix. Empty selection already passes for both. The uploader boundaries record names and SHA-256 hashes of real repository PNG fixtures, without replacing FileList with a mock. All cases verify the input reset is retained.

Full-app verification at 1440/390 px:

  1. Open an email draft, click the actual paperclip, choose Upload from computer, and select a synthetic text file through the native chooser.
  2. Before: zero compose-upload POSTs and no staged attachment. After: one real POST to /api/email/compose-upload, successful filename/size metadata and a visible paperclip-example.txt chip.
  3. Read the staged file under the fixture app's configured compose directory and compare it byte-for-byte with the selected 43-byte file: identical in both layouts. Page-error arrays are empty. No email was sent.

The after drafts were opened through the running app's public documentModule.openEmailDraft entry point; paperclip/menu/chooser/upload/rendering remained actual app behavior. On mobile, the existing email-fields toggle exposes the attachment chip. Initial harness setup used the received-attachment container instead of the compose container, and encountered sidebar/short-navigation timing issues; those attempts are not passing evidence. The final runs use the correct compose container, real field toggle and a cold-load navigation timeout of 60 seconds. No force-click or CSS override was used.

The app was stopped before changing production source and restarted afterward, then stopped again; temporary dependencies were removed. Limits: native macOS/Python/Chromium, no Windows, Docker, SMTP delivery, model inference or full repository suite. Optional vector services ran degraded. Backend upload limits, storage, authorization and drag-and-drop are unchanged.

Upstream refresh — 29 October 2026

Merged current dev at 8b8fc62449057888e20c74314b8e2924a6c02adf. Kept the current upstream network-first worker and removed obsolete cache-only/query-version churn where applicable; the production behavior fix is retained. 116 focused tests passed after the merge, with the existing SQLAlchemy deprecation warning. Changed JS files passed syntax checks and the merge diff passed whitespace checks. Original full-app evidence predates this upstream UI refresh; no new full-app run was performed for this mechanical merge.

CI fixture follow-up

The refreshed shard also needed the shared photo-routing fixture to explicitly exclude unrelated external MCP registry state. Its routing assertions and production code are unchanged. The file-picker checks plus photo-family/cache checks passed 145 tests after this test-only correction.

Visual / UI changes

  • Screenshot or short clip of the running app is attached below, including mobile.
  • Style match — existing attachment chips, composer, buttons and typography; no styling changes.
  • No new component patterns.
  • I am not an LLM agent submitting a bulk PR. — intentionally unchecked; AI assistance is disclosed above.

Screenshots / clips

Immutable evidence-only fork images are outside the code diff. The file is deliberately tiny, so the existing size formatter shows 0 KB; the API and stored bytes were verified as 43 bytes.

Before: choosing a file creates no attachment
After: staged file shown in the composer
After: native selection and chip in the narrow composer

Shared inline-image picker validation

The same reset bug was also reproduced through the real inline-image toolbar and native chooser in the email rich-body editor. Before: selecting a valid repository PNG triggered “Choose an image file”, zero /api/upload POSTs and no inserted image. After: one real upload, a rendered image with natural width 192, and bytes read back over authenticated HTTP identical to the selected fixture, at both widths with no page errors. This is part of the same PR; the count is unchanged. The module version and service-worker cache were bumped again for this follow-up.

After: inline image uploaded and rendered
After: inline image in the narrow editor

CI cache-identity follow-up

The first follow-up CI run found two existing tests pinned to the old document-module cache token. Their expectations now match the new token. All 14 tests in the two affected files plus module-version parity pass locally; the 111-test browser/security run above predates this test-only adjustment.

@github-actions github-actions Bot added the ready for review Description complete — ready for maintainer review label Oct 28, 2026
@redpersongpt redpersongpt changed the title fix(email): retain paperclip files when resetting the input fix(email): retain files in attachment and inline-image pickers Oct 28, 2026

@alteixeira20 alteixeira20 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @redpersongpt for this important fix.

Summary:

  • Defect Resolution: Confirmed. Converting file picker inputs into an array prior to resetting the input value prevents the browser from clearing the live FileList before asynchronous upload processing completes.
  • Safety & Isolation: Shallow copying preserves File blob references for asynchronous dispatch while enabling users to immediately re-select the same file.
  • Compatibility: Integrates cleanly with current dev (0e49c4ef) and passes syntax verification across static/js/document.js.
  • Validation: Verified with 8 Playwright headless Chromium tests exercising the upload event handlers and input reset, 37 focused tests in 4.64s, and 529 email suite tests passing in 25.36s.

@alteixeira20
alteixeira20 merged commit c811f76 into odysseus-dev:dev Oct 30, 2026
22 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Description complete — ready for maintainer review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Paperclip button in Email Module not Attaching Files

2 participants