Skip to content

fix(ui): keep history-backed user image messages visible#68415

Merged
steipete merged 5 commits intoopenclaw:mainfrom
mraleko:fix/ui-user-image-history-normalization
Apr 18, 2026
Merged

fix(ui): keep history-backed user image messages visible#68415
steipete merged 5 commits intoopenclaw:mainfrom
mraleko:fix/ui-user-image-history-normalization

Conversation

@mraleko
Copy link
Copy Markdown
Contributor

@mraleko mraleko commented Apr 18, 2026

Summary

  • Problem: Control UI dropped user image messages after history reload because transcript-backed media fields were not rendered.
  • Why it matters: image uploads appeared during the optimistic send path, then disappeared once the assistant finished and chat history reloaded.
  • What changed: taught chat image extraction/rendering to preserve history-backed user images, including transcript MediaPath fields and input_image blocks.
  • What did NOT change (scope boundary): no gateway transcript format changes, no assistant attachment behavior changes outside this rendering path.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Root Cause (if applicable)

  • Root cause: chat history reload returned user messages in a transcript-backed media shape that the Control UI image extractor did not recognize.
  • Missing detection / guardrail: no UI regression test covered history-backed user image messages.
  • Contributing context (if known): optimistic send and history reload used different message representations.

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: ui/src/ui/views/chat.test.ts
  • Scenario the test should lock in: a user history message with MediaPath still renders a visible image after reload.
  • Why this is the smallest reliable guardrail: it exercises the exact renderer path that regressed without pulling in gateway/integration setup.
  • Existing test that already covers this (if any): None
  • If no new test is added, why not: N/A

User-visible / Behavior Changes

  • User-uploaded images remain visible in Control UI after chat history reload/final reconciliation.

Diagram (if applicable)

Before:
[user sends image] -> [optimistic image renders] -> [assistant finishes] -> [history reload] -> [image disappears]

After:
[user sends image] -> [optimistic image renders] -> [assistant finishes] -> [history reload] -> [image still visible]

Security Impact (required)

  • New permissions/capabilities? (No)
  • Secrets/tokens handling changed? (No)
  • New/changed network calls? (No)
  • Command/tool execution surface changed? (No)
  • Data access scope changed? (No)
  • If any Yes, explain risk + mitigation:

Repro + Verification

Environment

  • OS: macOS 15.x
  • Runtime/container: local dev checkout
  • Model/provider: N/A for renderer regression test
  • Integration/channel (if any): Control UI / webchat
  • Relevant config (redacted): localMediaPreviewRoots includes the uploaded image path root

Steps

  1. Open Control UI chat.
  2. Send an image.
  3. Wait for the assistant reply to finish and history to reload.

Expected

  • The user image remains visible after the assistant response completes.

Actual

  • Before this change, the user image disappeared after final/history reconciliation.

Evidence

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

  • Verified scenarios: history-backed user image rendering via transcript media fields; scoped Control UI test file.
  • Edge cases checked: local path images proxied through the existing Control UI media route; input_image blocks preserved.
  • What you did not verify: live manual browser interaction against a running gateway.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? (Yes)
  • Config/env changes? (No)
  • Migration needed? (No)
  • If yes, exact upgrade steps:

Risks and Mitigations

  • Risk: history-backed local image paths now reuse the existing Control UI media proxy path.
  • Mitigation: the change stays within the existing preview allowlist and auth-token flow.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 18, 2026

Greptile Summary

Fixes a regression where user-uploaded images disappeared from the Control UI after chat history reloaded, by teaching extractImages to handle transcript MediaPath/MediaPaths fields and input_image content blocks, and by forwarding proxy opts into renderMessageImages so those local paths are served through the existing assistant-attachment route.

The fix is focused and consistent with the existing proxy guard (isLocalAssistantAttachmentSource + isLocalAttachmentPreviewAllowed). The new regression test covers the primary MediaPath scenario; the MediaPaths (plural array) branch and non-image media files in those fields are minor gaps worth noting but do not block this change.

Confidence Score: 5/5

Safe to merge — all findings are P2 style suggestions with no blocking correctness or security issues.

The fix is narrowly scoped to the rendering path, reuses existing proxy infrastructure, and is guarded by the existing allowlist check. No P0/P1 issues found.

No files require special attention.

Comments Outside Diff (1)

  1. ui/src/ui/views/chat.test.ts, line 952 (link)

    P2 MediaPaths (plural) variant not covered by new test

    The new regression test exercises MediaPath (singular) but not the MediaPaths: string[] array branch added in extractImages. A parallel test case for the array variant would give the same deduplication guarantee for the array path — the two branches share no code and either could regress independently.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: ui/src/ui/views/chat.test.ts
    Line: 952
    
    Comment:
    **`MediaPaths` (plural) variant not covered by new test**
    
    The new regression test exercises `MediaPath` (singular) but not the `MediaPaths: string[]` array branch added in `extractImages`. A parallel test case for the array variant would give the same deduplication guarantee for the array path — the two branches share no code and either could regress independently.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: ui/src/ui/views/chat.test.ts
Line: 952

Comment:
**`MediaPaths` (plural) variant not covered by new test**

The new regression test exercises `MediaPath` (singular) but not the `MediaPaths: string[]` array branch added in `extractImages`. A parallel test case for the array variant would give the same deduplication guarantee for the array path — the two branches share no code and either could regress independently.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: ui/src/ui/chat/grouped-render.ts
Line: 114-121

Comment:
**No image-type guard on `MediaPath`/`MediaPaths` fields**

The field name is generic (`MediaPath`, not `ImagePath`), so if the gateway ever stores a video, audio, or PDF path there, the code feeds it into `appendImageBlock` and renders it as `<img src="...">`. The browser will issue a request and show a broken-image placeholder rather than silently skipping it. If the contract is "always an image," a comment saying so (or a convention like checking the extension) would protect future readers from widening it unintentionally.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix(ui): keep history-backed user image ..." | Re-trigger Greptile

Comment thread ui/src/ui/chat/grouped-render.ts
Copy link
Copy Markdown

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

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: d01bba84d7

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread ui/src/ui/chat/grouped-render.ts Outdated
Copy link
Copy Markdown

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

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: 63cabccf52

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread ui/src/ui/chat/grouped-render.ts
Copy link
Copy Markdown

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

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: 18a4e1304c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread ui/src/ui/chat/grouped-render.ts Outdated
@steipete steipete force-pushed the fix/ui-user-image-history-normalization branch from fe2fdb9 to 2c557c1 Compare April 18, 2026 19:42
@steipete steipete force-pushed the fix/ui-user-image-history-normalization branch from 2c557c1 to e3fc13f Compare April 18, 2026 19:43
@steipete steipete merged commit 6d40de4 into openclaw:main Apr 18, 2026
9 checks passed
@steipete
Copy link
Copy Markdown
Contributor

Thanks @mraleko, landed this with the image rendering fix plus a couple of robustness cleanups around the flaky/slow test paths.

Validation run:

  • focused chat image tests: pass
  • setup-promotion/config regression tests: pass
  • pnpm check: pass
  • full pnpm test: pass (61 shards)
  • pnpm build: pass after refreshing stale node_modules from latest main
  • final post-rebase affected tests + git diff --check: pass

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Control UI user image messages disappear when assistant response completes

2 participants