Skip to content

fix(web): preserve file attachments after input reset - #8786

Closed
mikeyperes wants to merge 1 commit into
pingdotgg:mainfrom
mikeyperes:fix/upstream-attachment-c01ff86f2
Closed

mikeyperes wants to merge 1 commit into
pingdotgg:mainfrom
mikeyperes:fix/upstream-attachment-c01ff86f2

Conversation

@mikeyperes

@mikeyperes mikeyperes commented Aug 30, 2026 •

Copy link
Copy Markdown

Focused upstream fix for native file attachment bytes being cleared before preservation.

This branch is based on upstream/main at 2daff8c and contains only the attachment helper, its tests, and ChatComposer wiring. Local validation passed: 18 attachment assertions.


Note

Low Risk
Client-only composer attachment handling with a targeted workaround; large files are still bounded by existing staging limits, with extra in-memory copies only for generic attachments at pick time.

Overview
Fixes generic file attachments in the chat composer losing their bytes on Electron/macOS when the hidden file input is cleared immediately after selection.

Adds snapshotComposerFilesBeforeInputReset, which copies generic files into in-memory File objects via arrayBuffer() before the input value is reset, while leaving images unchanged so they still go through the existing compression path. ChatComposer’s file onChange now awaits that snapshot, then passes the result to addComposerAttachments. A unit test asserts bytes are read before reset and the snapshot matches name, type, and content.

Reviewed by Cursor Bugbot for commit a83a712. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Preserve file attachments after input reset in ChatComposer

  • Adds snapshotComposerFilesBeforeInputReset to read generic files into new File objects detached from the input element before it is cleared, fixing attachment loss on Electron/macOS where input reset invalidates File handles
  • Updates the ChatComposer onChange handler to call this snapshot utility asynchronously before clearing input.value; image files continue on their existing unmodified path
  • The resetInput callback runs in a finally block so the input clears even if snapshotting fails
  • Risk: generic files are now fully read into memory via arrayBuffer before the input resets; very large files may consume more memory during the snapshot step
📊 Macroscope summarized a83a712. 2 files reviewed, 2 issues evaluated, 0 issues filtered, 2 comments posted

🗂️ Filtered Issues

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 56c8a7f2-83dc-4c83-8e87-fa664a9b3acf

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Aug 30, 2026
void addComposerAttachments(files);
focusComposer();
const input = event.currentTarget;
void snapshotComposerFilesBeforeInputReset(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 High chat/ChatComposer.tsx:4058

A second file selection can be cleared by the first snapshotComposerFilesBeforeInputReset call, causing the second upload to retain an invalid input-owned File on Electron/macOS and fail. Because the shared input is not reset or otherwise locked until the first asynchronous snapshot completes, a subsequent picker interaction reuses the same input; serialize these snapshots or disable/reject new selections until the prior reset finishes.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/ChatComposer.tsx around line 4058:

A second file selection can be cleared by the first `snapshotComposerFilesBeforeInputReset` call, causing the second upload to retain an invalid input-owned `File` on Electron/macOS and fail. Because the shared input is not reset or otherwise locked until the first asynchronous snapshot completes, a subsequent picker interaction reuses the same input; serialize these snapshots or disable/reject new selections until the prior reset finishes.

return file;
}
try {
const bytes = await file.arrayBuffer();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 High chat/composerAttachmentFiles.ts:95

Selecting an oversized generic file makes snapshotComposerFilesBeforeInputReset read the entire file and allocate a second in-memory File before addComposerAttachments rejects it. A multi-GB selection can therefore exhaust renderer memory or make Electron unresponsive; validate against the staging limit before calling file.arrayBuffer(), or use a bounded copy path.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/composerAttachmentFiles.ts around line 95:

Selecting an oversized generic file makes `snapshotComposerFilesBeforeInputReset` read the entire file and allocate a second in-memory `File` before `addComposerAttachments` rejects it. A multi-GB selection can therefore exhaust renderer memory or make Electron unresponsive; validate against the staging limit before calling `file.arrayBuffer()`, or use a bounded copy path.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a83a712. Configure here.

return file;
}
}),
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Snapshot reads files before size check

High Severity

snapshotComposerFilesBeforeInputReset calls arrayBuffer on every generic file before addComposerAttachments applies fileStagingLimit. A picker selection larger than the 50 MB cap is fully loaded into the renderer and only then rejected, which can freeze or OOM the app.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a83a712. Configure here.

@macroscopeapp

macroscopeapp Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This focused client-side fix snapshots generic attachments before clearing the hidden input, but it also introduces asynchronous full-file reads and a race window for rapid successive selections. Unresolved high-severity findings about potential renderer memory exhaustion and lost selections require human review.

Not approved because:

  • 2 blocking correctness issues found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

Copy link
Copy Markdown
Member

Note

This comment is posted by Julius' dot

Closing under the verification requirement. The PR reports 18 attachment assertions, but its new test mocks file reads and checks snapshot ordering; it does not establish the claimed Electron/macOS file loss or show a successful upload after the change. Please provide the affected app and OS versions, reproduction steps, and observed bytes before and after the fix, then request reconsideration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants