Skip to content

Support multiple chat attachments - #783

Merged
shanselman merged 2 commits into
openclaw:mainfrom
TurboTheTurtle:tt/windows-node-779-multiple-attachments
Jun 25, 2026
Merged

Support multiple chat attachments#783
shanselman merged 2 commits into
openclaw:mainfrom
TurboTheTurtle:tt/windows-node-779-multiple-attachments

Conversation

@TurboTheTurtle

Copy link
Copy Markdown
Contributor

Closes #779.

Adds multi-select attachment picking in the native chat surfaces and keeps pending attachments as an ordered list through the composer before sending them to the gateway. Multiple attachments now render and rehydrate as separate attachment markers in chat history.

@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 18, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper

clawsweeper Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed June 22, 2026, 3:09 AM ET / 07:09 UTC.

Summary
The branch changes native WinUI chat attachment picking and composer state from one pending attachment to an ordered list, sends all selected attachments, and adds provider tests for multiple marker rendering and history rehydration.

Reproducibility: yes. source inspection gives a high-confidence path: current main's ChatPage and ChatWindow call PickSingleFileAsync and the composer has a single PendingAttachment, while the linked issue asks for native multi-file upload parity. I did not run the Windows UI in this read-only sweep.

Review metrics: 3 noteworthy metrics.

  • Changed surface: 6 production files, 1 test file; +316/-104. The diff spans native chat UI state, Win32 picker interop, and provider tests, so UI proof matters alongside CI.
  • Attachment coverage: 2 tests added, 1 test strengthened. Provider send and history rehydration behavior now have multi-attachment coverage, while picker behavior remains runtime-proven rather than unit-tested.
  • Proof artifacts: 4 screenshots inspected. The latest proof directly shows pending, ready, sent, and rehydrated native chat attachment states.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #779
Summary: This PR is the candidate fix for the linked native-app multiple-file attachment issue.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] The PR refactors the native file-picker path used by existing single-file attachment workflows; maintainers should explicitly accept the posted current-head Windows proof because the added tests cover provider behavior, not the COM picker UI itself.

Maintainer options:

  1. Land with current-head native proof (recommended)
    Maintainers can accept the remaining Win32 picker compatibility risk because the latest screenshots and CI cover the intended multi-attachment path on f0a82fc.
  2. Ask for a one-file smoke
    If maintainers want extra upgrade confidence, ask for a short current-head proof that selecting one file through the new picker still attaches and sends normally.

Next step before merge

  • [P2] No repair job is needed; the remaining action is ordinary maintainer review and possible merge of a proof-positive feature PR.

Security
Cleared: Cleared: the diff touches local WinUI chat UI, COM file-picker interop, and tests, with no dependency, workflow, secret, permission, or supply-chain changes.

Review details

Best possible solution:

Land this focused native multi-attachment support after routine maintainer review, preserving existing single-file and paste behavior while extending the composer to ordered attachment lists.

Do we have a high-confidence way to reproduce the issue?

Yes, source inspection gives a high-confidence path: current main's ChatPage and ChatWindow call PickSingleFileAsync and the composer has a single PendingAttachment, while the linked issue asks for native multi-file upload parity. I did not run the Windows UI in this read-only sweep.

Is this the best way to solve the issue?

Yes, the PR follows the existing provider/gateway attachment-list contract and confines the change to native picker/composer state plus provider tests. No alternate core API appears needed.

AGENTS.md: found, but no applicable review policy affected this item.

Codex review notes: model internal, reasoning high; reviewed against 6283fb174ead.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. Sufficient: the contributor posted inspected current-head Windows screenshots showing two attachments pending, sent, and rehydrated as separate markers, plus build and proof-gateway notes.
  • add proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. Sufficient: the contributor posted inspected current-head Windows screenshots showing two attachments pending, sent, and rehydrated as separate markers, plus build and proof-gateway notes.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): Sufficient: the contributor posted inspected current-head Windows screenshots showing two attachments pending, sent, and rehydrated as separate markers, plus build and proof-gateway notes.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This is a normal-priority native chat feature-parity improvement with limited blast radius and no confirmed security, data-loss, or crash impact.
  • merge-risk: 🚨 compatibility: The PR changes the native file-picker path used by existing single-file attachment workflows, and automated tests do not exercise the Win32 picker UI.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): Sufficient: the contributor posted inspected current-head Windows screenshots showing two attachments pending, sent, and rehydrated as separate markers, plus build and proof-gateway notes.
  • proof: sufficient: Contributor real behavior proof is sufficient. Sufficient: the contributor posted inspected current-head Windows screenshots showing two attachments pending, sent, and rehydrated as separate markers, plus build and proof-gateway notes.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. Sufficient: the contributor posted inspected current-head Windows screenshots showing two attachments pending, sent, and rehydrated as separate markers, plus build and proof-gateway notes.
Evidence reviewed

What I checked:

Likely related people:

  • shanselman: Blame ties the current single PendingAttachment composer/root contract and Win32 picker helper to b637369; this person also posted recent triage guidance on the PR. (role: introduced native chat attachment/composer surface and recent reviewer; confidence: high; commits: b637369fbc56; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawComposer.cs, src/OpenClaw.Tray.WinUI/Chat/OpenClawChatRoot.cs, src/OpenClaw.Tray.WinUI/Helpers/Win32FilePickerHelper.cs)
  • Ranjesh: Blame ties attachment marker rendering, cache persistence, and history rehydration helpers that this PR extends to commit 9a3a7a6. (role: attachment metadata and history contributor; confidence: high; commits: 9a3a7a6131a8; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs, tests/OpenClaw.Tray.Tests/OpenClawChatDataProviderTests.cs)
  • RBrid: Recent commits and blame show reset/send lifecycle and logging work near the same provider SendMessageAsync and attachment metadata paths. (role: recent chat provider contributor; confidence: medium; commits: 753828f63e96, 3fdfbfad837f; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. labels Jun 18, 2026
@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@openclaw-mantis visual task: verify native chat can select two attachments, send them, and show separate attachment markers in history.

@shanselman

Copy link
Copy Markdown
Collaborator

Maintainer triage: this is a useful feature area, but the branch is currently conflicting and ClawSweeper is still asking for runtime UI proof of the native picker/composer path. Please rebase on current main and add the Mantis/manual proof showing two attachments selected, sent, and rehydrated as separate markers before this is merge-ready.

@TurboTheTurtle
TurboTheTurtle force-pushed the tt/windows-node-779-multiple-attachments branch from d87f6cb to 9d49010 Compare June 22, 2026 04:48
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. label Jun 22, 2026
@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

Native Windows verification on f0a82fc7:

  • selected pr783-notes.txt and pr783-diagram.png through the native Win32 picker; both appeared pending in the composer
  • sent the message; the local proof gateway received chat.send with attachmentCount: 2
  • restarted the tray against the same gateway/data dir; history rehydrated separate markers for both attachments
  • C:\Users\andyl\.dotnet10\dotnet.exe build src\OpenClaw.Tray.WinUI\OpenClaw.Tray.WinUI.csproj -c Debug -r win-x64 passed: 0 warnings, 0 errors

While running this, the shell dialog returned a null IShellItemArray on this desktop, so f0a82fc7 adds the defensive fallback/skip path instead of letting the picker crash.

Pending composer with two attachments:
pending composer with two attachments

Ready to send:
ready to send two attachments

Sent message:
sent message with attachment markers

Rehydrated after tray restart:
rehydrated history with separate attachment markers

@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 22, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 22, 2026
@shanselman
shanselman merged commit c5aa130 into openclaw:main Jun 25, 2026
12 checks passed
@TurboTheTurtle
TurboTheTurtle deleted the tt/windows-node-779-multiple-attachments branch June 25, 2026 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to upload multiple files in a single message

2 participants