Persist chat attachment metadata across history reloads

[ranjeshj]

Summary

• persist attachment display metadata in a local sidecar without storing attachment bytes
• rehydrate trusted attachment chips from chat history, including attachment-only messages and first-turn thread-key fallback
• escape untrusted marker-looking text and stop rendering bare legacy markers as attachment chips

Fixes #453.

Validation

• OPENCLAW_REPO_ROOT=C:\oc453; .\build.ps1
• OPENCLAW_REPO_ROOT=C:\oc453; dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore
• OPENCLAW_REPO_ROOT=C:\oc453; dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore

Note: validation was run via the short junction path C:\oc453 to avoid the WinAppSDK PRI expansion/path issue seen from the generated long worktree path.

[clawsweeper]

Codex review: found issues before merge. Reviewed June 2, 2026, 7:32 PM ET / 23:32 UTC.

Summary
The PR adds tray chat attachment metadata sidecar persistence/rehydration, removes bare legacy marker rendering, and adds tray tests for reload, attachment-only, and spoofing cases.

Reproducibility: yes. Source inspection shows the underlying bug path and the patch findings: current main sends trimmed text while attachment chips are local, and PR head persists plaintext match text and matches repeated text by a broad time window.

Review metrics: 3 noteworthy metrics.

• Diff size: 438 added, 20 removed, 3 files. The change is concentrated but large enough that the new persistence path deserves focused review.
• Persisted stores: 1 JSON sidecar added. The PR adds new local data retention behavior outside normal chat history.
• Regression coverage: 4 attachment tests added. The added tests cover the main reload, attachment-only, and spoofing paths but not repeated same-text matching.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🌊 off-meta tidepool
Patch quality: 🧂 unranked krab
Result: blocked by patch quality or review findings.

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

Rank-up moves:

• Replace plaintext sidecar matching with a maintainer-approved non-plaintext identity strategy.
• [P2] Add a regression test where the same text appears before and after an attachment send and only the attachment send rehydrates chips.

Mantis proof suggestion
After the code blockers are repaired, visible proof of an attachment chip surviving history reload or restart would materially help review. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify a sent chat attachment still renders as an attachment chip after history reload and app restart, with private filenames/content redacted.

Risk before merge

• [P1] Merging as-is adds a new plaintext local persistence path for user message text in attachment-metadata.json.
• [P1] Same-text messages within the 24-hour match window can be associated with the wrong attachment sidecar entry during history reload.
• [P1] The final matching strategy likely needs maintainer security/product judgment because current chat.send results do not expose a stable history message id.

Maintainer options:

1. Repair sidecar identity before merge (recommended)
   Persist no user-message plaintext and add a repeated same-text regression test using a stable id or maintainer-approved non-reversible match key.
2. Accept local plaintext retention explicitly
   Maintainers could choose to accept plaintext message text in the local sidecar, but that should be an intentional documented privacy decision before merge.
3. Pause for gateway metadata
   If local matching remains too fragile, pause this PR and move the fix toward gateway-provided attachment metadata or stable message ids.

Next step before merge

• [P2] Needs maintainer/security review of the sidecar identity and privacy tradeoff before an automated repair should choose the durable match key.

Security
Needs attention: The PR introduces a new local persistence path that stores user message text in plaintext.

Review findings

• [P1] Avoid storing prompt text in the attachment sidecar — src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs:3298
• [P2] Do not attach later sidecars to earlier same-text history — src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs:3362-3363

Review details

Best possible solution:

Keep the sidecar approach only after it stores minimized non-plaintext matching data and proves repeated same-text history entries hydrate the correct attachment chips.

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

Yes. Source inspection shows the underlying bug path and the patch findings: current main sends trimmed text while attachment chips are local, and PR head persists plaintext match text and matches repeated text by a broad time window.

Is this the best way to solve the issue?

No. Writing after successful send is the right direction, but the sidecar should not store plaintext user messages and needs a safer identity/matching strategy before merge.

Full review comments:

• [P1] Avoid storing prompt text in the attachment sidecar — src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs:3298
Text = ... is serialized into attachment-metadata.json for every attachment send, which creates a new local plaintext cache of user messages even though the sidecar only needs minimized attachment metadata plus a match key. Please use a stable message id or maintainer-approved non-reversible key instead of the full message text.
  Confidence: 0.9
• [P2] Do not attach later sidecars to earlier same-text history — src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs:3362-3363
  The matcher consumes the first unused sidecar entry whenever the text matches within 24 hours. If a user sends the same text without an attachment and then later with an attachment, a reload can put the attachment chip on the earlier no-attachment bubble and leave the real attachment message bare; add a stable identity or stricter timestamp matching plus a regression test.
  Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against bfeb3b5c90be.

Label changes

Label changes:

• add P2: This is a bounded chat attachment persistence fix with concrete merge blockers, not an emergency runtime failure.
• add merge-risk: 🚨 session-state: The sidecar matcher can mis-associate attachments with the wrong history message when the same text repeats within the match window.
• add merge-risk: 🚨 security-boundary: The diff would persist user message text in a new local sidecar file for attachment sends.
• add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The contributor proof gate is not applied to this collaborator-authored PR; the body reports the repository-required build and test commands.

Label justifications:

• P2: This is a bounded chat attachment persistence fix with concrete merge blockers, not an emergency runtime failure.
• merge-risk: 🚨 security-boundary: The diff would persist user message text in a new local sidecar file for attachment sends.
• merge-risk: 🚨 session-state: The sidecar matcher can mis-associate attachments with the wrong history message when the same text repeats within the match window.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The contributor proof gate is not applied to this collaborator-authored PR; the body reports the repository-required build and test commands.

Evidence reviewed

Security concerns:

• [medium] Plaintext message text in attachment sidecar — src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs:3298
attachment-metadata.json serializes the full user message text for attachment sends, increasing local sensitive-data retention beyond attachment display metadata.
  Confidence: 0.9

What I checked:

• PR sidecar persists user message text: PR head serializes CachedAttachmentMeta.Text from the sent user message into attachment-metadata.json for every attachment send. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs:3298, c81d59e177b6)
• PR matcher uses text plus broad time window: AttachmentMetaMatcher accepts the first unused entry with equal text unless the timestamps differ by more than 24 hours, with no stable message identity. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs:3362, c81d59e177b6)
• Failed-send sidecar blocker is addressed: The PR calls CacheAttachmentMeta only after SendChatMessageAsync completes successfully, avoiding the earlier bot PR's pre-send stale-entry bug. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs:299, c81d59e177b6)
• Current main explains the bug being fixed: Current main appends local attachment marker lines to display text but sends only the trimmed message plus attachments to the gateway, so history reloads need a metadata rehydration path. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs:239, bfeb3b5c90be)
• Linked issue and prior bot PR context: Issue discussion for Persist chat attachment metadata across history reloads #453 asks for structured attachment metadata persistence and spoof-prevention coverage; prior bot PR [Repo Assist] fix(chat): persist attachment metadata in sidecar for history reload #615 was blocked on plaintext sidecar content and failed-send staleness.
• Feature history provenance: git blame ties the current attachment send/display and history reconstruction code in the central files to recent chat commits, including v0.6.0-era chat hardening and native chat work. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs:208, 7d9152f427a3)

Likely related people:

• Régis Brid: Local history shows the native FunctionalUI chat provider/timeline was introduced in commit 08cab69, and git blame points the current attachment send and history reconstruction lines at the v0.6.0 chat hardening commit. (role: introduced behavior and recent area contributor; confidence: high; commits: 08cab69a038c, 7d9152f427a3; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs, src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs)
• christineyan4: The linked issue names this account as the requested follow-up owner, and merged PR Chat revamp: file attachments, abort, session switching, voice UI, mute sync, overlay disable #424 was the chat revamp that included file attachment support. (role: attachment feature contributor and requested follow-up owner; confidence: medium; commits: cd4acf013e38; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs, src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs, src/OpenClaw.Shared/ChatAttachment.cs)
• Ranjesh Jaganathan: Beyond authoring this PR, merged history shows prior native chat UX work touching OpenClawChatTimeline in commit 131154c. (role: recent adjacent chat contributor; confidence: medium; commits: 131154c13013, c81d59e177b6; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs, 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.