feat(chat): render user and agent media in native chat - #1119
Merged
karkarl merged 2 commits intoAug 19, 2026
Conversation
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: needs changes before merge. Reviewed August 19, 2026, 7:18 PM ET / 23:18 UTC. ClawSweeper reviewWhat this changesThis branch adds typed media parsing, Gateway-backed media retrieval, attachment-echo correlation, and native chat rendering for user and assistant media. Merge readiness⛔ Blocked by patch quality or review findings - 6 items remain This PR has credible current-head UI proof, but it still resolves assistant media when its independently authorized HTTP credential is absent. That conflicts with the stated fail-closed media boundary and is a P1 merge blocker. Priority: P2 Review scores
Verification
How this fits togetherThe tray chat converts Gateway messages and local attachments into native conversation rows. This change carries media metadata through connection-scoped retrieval and safe WinUI rendering so users see previews instead of raw media paths or duplicate echoes. flowchart LR
A[Gateway chat events] --> B[Typed media projection]
C[Local attachments] --> B
B --> D[Connection-scoped authorization]
D --> E[Bounded media retrieval]
E --> F[Native chat timeline]
B --> F
Before merge
Findings
Agent review detailsSecurityNeeds attention: The new credentialed media path is fail-open when its separate HTTP credential is absent. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Require a current, independently authorized assistant-media credential before either media-resolution path starts, and retain the existing safe unavailable-card behavior when it is absent. Do we have a high-confidence way to reproduce the issue? Yes. The included handler test constructs a connected client without an assistant-media credential and demonstrates that legacy media still resolves successfully with no Authorization header. Is this the best way to solve the issue? No. The rendering and provenance design is sound in direction, but the resolver must reject missing authorized media credentials before sending either structured or legacy retrieval requests. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 428db773d8be. LabelsLabel changes:
Label justifications:
EvidenceSecurity concerns:
Acceptance criteria:
What I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (5 earlier review cycles)
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
calebeden
changed the title
clawsweeper
Bot
removed
the
merge-risk: 🚨 compatibility
calebeden
marked this pull request as ready for review
August 10, 2026 18:51
|
🦞👀 Pull request received. I will update this pull request when review starts. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Bound local preview memory, enforce guarded local image decoding, preserve media metadata across history and reset reconciliation, and keep runtime V2 credential upgrades without allowing downgrades. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f74b3dc-62de-4bf8-abee-aae3915b2f89
karkarl
added
the
status: 🚢 actively landing
karkarl
force-pushed
the
calebeden-investigate-media-rendering
branch
from
August 19, 2026 23:11
a5f614e to
bf76398
Compare
clawsweeper
Bot
added
proof: 📸 screenshot
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Problem This Solves
Fixes two native chat media problems:
MEDIA:path instead of rendering as native chat content.Why This Change Was Made
The chat pipeline now preserves typed user and assistant media metadata across live events, reset, backfill, and history. Assistant media is resolved only through authenticated Gateway contracts. Model-provided paths are never treated as Windows filesystem paths or arbitrary URLs.
Assistant-media HTTP authentication is connection-owned and separate from WebSocket authentication. Requests remain scoped to the active Gateway authority and connection generation.
User Impact
Users can send image attachments without duplicate media echoes and view assistant-generated images directly in native chat. Images have bounded inline previews, accessible loading and unavailable states, Retry, and a lightbox. Unsupported media remains visible as a safe fallback card without exposing raw Gateway paths.
Evidence
Prepared current head:
bf763987a45c0b5859226537c7524c72b650d77cRebased onto
origin/mainat428db773d8beb7d1c7452a68f4224dd43e3c76e4.Picture Proof
The screenshot is intentionally cropped to the media cards and generic connection state. It contains no endpoint, token, device name, filesystem or WSL path, or PII.
Change Type
Scope
winnodeValidation
./build.ps1dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restoredotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restoreKnown timing noise was cleared rather than waived. A queued-send timing test passed five targeted reruns and the subsequent complete suite. Prior telemetry, virtualization, and camera timing flakes also passed targeted and complete reruns.
Real Behavior Proof
bf763987a45c0b5859226537c7524c72b650d77cattach.grantand same-origin HTTP, decoded, and rendered inlineSecurity Impact
Yes/No): NoYes/No): YesYes/No): YesYes/No): NoYes/No): YesAssistant media retrieval uses same-Gateway authenticated requests. Shared token is preferred, followed only by a current non-bootstrap device token; otherwise media HTTP remains disabled. Strong credentials are endpoint-provenance gated and cleared before reconnect reauthorization. Redirects are disabled. Managed ticket paths, exact host and port, traversal, MIME families, connection generation, encoded and streamed byte sizes, and decoded image dimensions are validated. File URIs, UNC paths, authority changes, fragments, backslashes, and traversal are rejected. Credentials, tickets, paths, response bodies, and media bytes are not logged or displayed.
Local previews are bounded to 32 entries, 64 MiB total, and 10 MiB per item with FIFO eviction and an encoded-length precheck. Gateway references cannot access local preview bytes.
Compatibility and Migration
Yes/No): YesYes/No): NoYes/No): NoReview Conversations