Skip to content

fix(messages): show native attachment download feedback - #1556

Open
BradGroux wants to merge 3 commits into
block:mainfrom
BradGroux:fix/attachment-download-feedback-1037
Open

BradGroux wants to merge 3 commits into
block:mainfrom
BradGroux:fix/attachment-download-feedback-1037

Conversation

@BradGroux

@BradGroux BradGroux commented Oct 22, 2026 •

Copy link
Copy Markdown

Successful native attachment downloads currently leave the card unchanged and give no indication that the file was saved. Repeated clicks can start duplicate requests.

Show “Downloading…” and mark the native download control unavailable until the host command settles, retaining keyboard focus. In-flight state is shared by source and filename so virtualized/remounted or mirrored cards cannot start a duplicate request. On success, use the shared completed-action Toast to confirm “File saved to Downloads”; the notification remains available if its source row disappears. Failures retain the inline error, make the control available again, and allow retry. The 100 most recent unobserved failures survive card absence; visible errors and pending operations are never evicted. Native URL validation, saving/collision policy, and external/browser download behavior stay with their existing owners.

Fixes #1037.

Verification

  • Extended the two existing native-download component journeys with explicitly held promises: pending state, duplicate suppression, success, notification ownership, failure, and retry. Both journeys fail against the unchanged component and pass with the fix.
  • 183 focused FileAttachment, MessageRow, icon-label, and shared Toast tests pass. TypeScript, design TypeScript, design guards, formatting, and diff checks pass. Required pre-commit and pre-push hooks passed, including the selected related-test suite.
  • Rendered the production component and shared Toast in Chromium and WebKit, using a mocked native IPC boundary: light/dark, 1280px/390px widths, narrow layouts at 200% text, keyboard/pointer activation, pending/success, and failure/retry. Screenshots inspected.
  • Follow-up regressions cover failures that settle while a card is absent, Strict Mode re-subscription, bounded failure retention without evicting visible errors, pending remounts on success/failure, mirrored cards, distinct filenames, and Enter/Space/pointer repeat activation. Pending remount and keyboard focus were also checked in Chromium and WebKit.
  • The final pre-push related suite passed all 1,342 tests. An earlier run hit a visibility assertion in an unchanged composer-upload case; its full 331-test file and the final related-suite rerun passed unchanged. The cause of that transient failure is unconfirmed.
  • No permanent browser cases added or removed. The state regression belongs in colocated RTL/Vitest; temporary rendered checks supplement layout evidence.

Native macOS validation

Validated head 9113975c on macOS 27.0.1 (Apple Silicon), using a release-mode Tauri host built with pinned Rust 1.98.1. An isolated app bundle loaded a diagnostic surface rendering the production FileAttachment and shared Toast. IPC, identity restoration, authenticated media reads, and filesystem saving used the real native host. No native IPC mock was used for these checks.

  • A 1,388-byte calendar fixture downloaded to the macOS Downloads directory and showed “File saved to Downloads.” Saved bytes matched the input exactly.
  • The saved files had the macOS quarantine attribute. A later intentional download preserved the original and saved a numbered copy.
  • Pending state appeared in WKWebView; a pending card remount plus repeat activation produced one save. Keyboard focus remained on the control during pending and after completion.
  • A missing media source showed the inline error and left retry available. Restoring the valid source succeeded.

At that snapshot, native validation required no additional changes. The failure-retention follow-up at 9029503e was also exercised with the current frontend and unchanged native host: an error remained visible after returning to a hidden card, retry stayed available, and a valid source still produced a byte-matching, quarantined save. This establishes the native command and filesystem path from the production card on the diagnostic surface. Full conversation navigation, a signed/notarized distributed package, and human acceptance remain separate checks. This changes no native host code and adds no dependency.

The current GitHub Actions CI run is action_required, with no jobs or logs. This gate needs maintainer action; it is not a passing full CI result. Local verification is documented above.

Overlapping attachment work

This targets current main independently of the two unmerged attachment PRs.

  • Unify attachment cards with neutral icon tiles #1548: the original patch applied cleanly to its reviewed head; this follow-up continues to preserve its icon/card styling. Recheck integration if that PR changes further.
  • [buzz-app] Preview Markdown and common text attachments #1469: its preview implementation restructures the same handler and tests, producing a merge conflict. When combining, retain its preview variants and coverage, use useFileDownload at component scope and invoke its download action from the extracted callback, and wire aria-disabled/aria-busy to both the existing card control and its new native preview/summary download controls, retaining focus and the shared operation guard. This PR does not duplicate its preview work.

Native acceptance steps

In a native macOS desktop build, open a conversation with a generic file attachment and activate Download. Confirm the pending caption and unavailable but focusable control while saving, then the Downloads confirmation and the saved file. Repeated activation during that request should not create another copy. A failed request should show the inline error and allow another attempt. Repeat with keyboard activation.

Signed-off-by: Brad Groux <3053586+BradGroux@users.noreply.github.com>
@BradGroux
BradGroux requested a review from a team as a code owner October 22, 2026 17:31

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e2a562ccce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/features/messages/FileAttachment.tsx Outdated
Comment thread src/features/messages/FileAttachment.tsx Outdated
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-23T02:23:40.401587Z 9029503 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Signed-off-by: Brad Groux <3053586+BradGroux@users.noreply.github.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9113975ce9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/features/messages/use-file-download.ts Outdated
@BradGroux

Copy link
Copy Markdown
Author

Native macOS validation completed at 9113975c on macOS 27.0.1 / Apple Silicon. Built the release-mode Tauri host with pinned Rust 1.98.1 and exercised the production FileAttachment/Toast in its real WKWebView, using real authenticated native IPC and filesystem saving.

The 1,388-byte calendar fixture saved correctly to Downloads, matched the original bytes, received the macOS quarantine mark, and showed the success notification. Pending feedback, repeat suppression through a card remount, keyboard focus retention, numbered collision handling, and missing-source error/retry availability passed. No corrective code changes were needed.

The test used an isolated diagnostic surface. It does not claim full conversation navigation, signed/notarized-package acceptance, or human acceptance. The PR remains ready for review. No attachment payload, private message, or identity values are included here.

Signed-off-by: Brad Groux <3053586+BradGroux@users.noreply.github.com>
@BradGroux

Copy link
Copy Markdown
Author

The new absent-card failure finding is fixed in 9029503e; the reply on that thread records the regression and native validation. Required local hooks passed all 1,342 related tests and the design checks.

The separate GitHub Actions CI run is action_required, with no jobs or downloadable logs. My account has read-only access to this repository, so a maintainer needs to inspect and clear that gate. This is not a full CI pass. The PR remains ready for review.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9029503ef7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

() => {
operations.delete(key);
changed(key);
notify("File saved to Downloads", "success");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Route completions through the surviving toast host

When a download is still pending and the user switches to a not-yet-opened community, select() creates a relay with restoring, so ConnectedApp takes the early return at src/app/App.tsx:221-233 and unmounts the ToastProvider declared at lines 309-502. This handler nevertheless invokes the notifier captured from that old provider; after the provider remounts with a new toaster id, Sonner targets the removed id, so the save succeeds with no visible completion feedback, and switching back shows neither state nor toast. Keep the provider mounted across restoration or dispatch completion through the currently mounted toast host.

AGENTS.md reference: AGENTS.md:L126-L129

Useful? React with 👍 / 👎.

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.

[Bug] macOS: successful attachment downloads give no visible feedback

1 participant