refactor(ui): remove obsolete chat media plumbing by steipete · Pull Request #139346 · openclaw/openclaw

steipete · 2026-09-05T20:11:57Z

What Problem This Solves

Chat media still carried a browser-side preview-folder list through Gateway bootstrap, application config, panes, transcripts, and sidebars after preview authorization moved to the Gateway. That unused state retained redundant invalidation logic and test scaffolding. Availability and retry paths also repeated the same credential, session, policy, and callback arguments.

Why This Change Was Made

Remove the unused localMediaPreviewRoots field end to end. The Gateway continues to derive allowed folders from current session/config state; the browser continues to revalidate through the existing media policy key. Availability and retry now accept the shared render-options object and use one resource lookup, preserving their existing cache keys and callback ownership.

Consolidate repeated pane, ticket-response, and Blob-URL fixtures without dropping the 23 media-lifecycle cases. Remove the single 129-line test that enforced obsolete bootstrap-root propagation; retain the actual session-permission and configuration-driven denial/restoration tests. No new caches, flags, compatibility paths, or dependencies.

Area	Net lines removed
Production	108
Tests and test support	288
Total	396

User Impact

Preserves the uploaded-image continuity fix and existing attachment behavior while reducing the state and code needed to maintain it. Uploads remain visible through worktree hydration and history handoff. Permission changes still trigger server checks, and explicit denial still removes a preview.

Evidence

841 selected UI tests passed across media lifecycle, handoff, invalidation, attachment rendering, chat views/state, pane identity, and plugin-page coverage.
All 13 selected browser scenarios passed: initial-session and ordinary-send image handoff, authenticated media metadata, configuration-driven denial/restoration, rejected media, and tilde/POSIX/Windows paths.
162 Gateway HTTP tests passed. Two unchanged avatar-authentication cases hit their 15-second first-request timeout locally; both timeouts reproduce on unchanged baseline de2c4b1768d9 with the pinned dependency setup. Both cases passed in the warmed candidate rerun with unchanged timeout and auth assertions.
The initial CI lint findings in the consolidated URL mock fixture are fixed, and its 23 cases plus targeted lint pass. The unrelated dashboard A2UI event test passes in isolation on macOS and in both light-only and ordered dark→light Linux tests using the CI Chromium binary, with no source or assertion changes. A full Linux shard also reproduced the intermittent failure; its cause remains unproven. The unchanged dashboard and maintainer-tooling tests passed on rerun, and exact-head CI is green.
Independent reviews of the cleanup and fixture correction, plus ClawSweeper review of the final head, found no actionable findings. The full check:changed gate passed in an isolated normal checkout with its own pinned install; all 33 edited files match this commit.
Inspected synthetic captures verify that the cleanup preserves the same loaded-image presentation.

Before cleanup

After cleanup

clawsweeper · 2026-09-05T20:12:00Z

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

steipete · 2026-09-05T20:30:38Z

@clawsweeper re-review

Current head: 907a9a6. The follow-up fixes the two URL-mock lint findings without changing production code. All 23 media lifecycle cases and targeted lint pass; the unchanged dashboard case also passed in isolation. The PR body contains the full cleanup evidence and final LOC totals.

clawsweeper · 2026-09-05T20:30:44Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

State: Review in progress
Detail: The exact-review queue leased this run; Codex is reviewing the item.
Run: https://github.com/openclaw/clawsweeper/actions/runs/33991720635
Updated: 2026-09-05T21:01:29.223Z

clawsweeper · 2026-09-05T21:05:29Z

Codex review: needs maintainer review before merge. Reviewed September 5, 2026, 5:16 PM ET / 21:16 UTC.

ClawSweeper review

What this changes

Removes unused browser preview-folder state and consolidates attachment availability arguments and media test fixtures without changing intended chat behavior.

Merge readiness

✅ Ready for maintainer review

Keep open as a useful, distinct cleanup following the merged image-continuity fix. The reviewed refactor preserves media authorization and invalidation behavior; no actionable introduced defect was found.

Priority: P3
Reviewed head: 907a9a6003c57e2d5f6044a32a7b414791239d8e

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused, maintainable cleanup with preserved behavioral coverage and no blocking findings.
Proof confidence	🌊 off-meta tidepool	Not applicable: The supplied MEMBER association exempts this cleanup from external-contributor proof requirements, and no material authority change triggers an exception. Reported browser scenarios cover media metadata and denial/restoration, but linked captures could not be inspected and tests were not rerun.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: The supplied MEMBER association exempts this cleanup from external-contributor proof requirements, and no material authority change triggers an exception. Reported browser scenarios cover media metadata and denial/restoration, but linked captures could not be inspected and tests were not rerun.
Evidence reviewed	8 items	Introduced production refactor: Compared the pinned merge base with the reviewed head. The shared resource lookup preserves source, base path, trimmed credential, session, agent, and policy cache identity; image callers preserve their directive-owned callback. Authorization remains server-owned: The unchanged media endpoint checks authenticated metadata requests, administrator-only explicit allowance, session-bound tickets, and current policy after asynchronous preparation. Removing the bootstrap folder list does not remove these checks or materially change authority. Retained policy invalidation: Configuration events still increment mediaPolicyEpoch; the policy key includes session permission and workspace facts. The retained browser scenario verifies configuration-driven denial, preview removal, restoration, and three metadata requests.
Findings	None	None.
Security	None	None.

How this fits together

Control UI renders chat attachments using authenticated Gateway metadata and short-lived media tickets. Session and configuration changes invalidate browser resources, while the Gateway decides whether each file may be served.

flowchart TD
  A[Chat attachments] --> B[Browser media resources]
  C[Session and configuration changes] --> B
  B --> D[Authenticated metadata request]
  D --> E[Gateway file access policy]
  E --> F[Media ticket or denial]
  F --> G[Preview or unavailable card]

Before merge

None.

Agent review details

Security

None.

PR surface

Source -108, Tests -288. Total -396 across 33 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	19	31	139	-108
Tests	14	212	500	-288
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	33	243	639	-396

Review metrics

None.

Technical review

Best possible solution:

Keep one server-owned media authorization path and reuse the existing browser policy invalidation and resource lifecycle.

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

Not applicable: this PR removes obsolete plumbing rather than claiming a new bug fix; source comparison supports the stated equivalence.

Is this the best way to solve the issue?

Yes: removing the unused browser state and migrating internal callers together is narrower than retaining a parallel folder list or introducing another authorization abstraction.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against fcebd5d5a19b.

Labels

Label justifications:

P3: This is behavior-preserving internal cleanup with no established urgent user-facing regression.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The supplied MEMBER association exempts this cleanup from external-contributor proof requirements, and no material authority change triggers an exception. Reported browser scenarios cover media metadata and denial/restoration, but linked captures could not be inspected and tests were not rerun.

Evidence

What I checked:

Introduced production refactor: Compared the pinned merge base with the reviewed head. The shared resource lookup preserves source, base path, trimmed credential, session, agent, and policy cache identity; image callers preserve their directive-owned callback. (ui/src/pages/chat/components/chat-message-attachment-availability.ts:270, 907a9a6003c5)
Authorization remains server-owned: The unchanged media endpoint checks authenticated metadata requests, administrator-only explicit allowance, session-bound tickets, and current policy after asynchronous preparation. Removing the bootstrap folder list does not remove these checks or materially change authority. (src/gateway/control-ui.ts:559, 907a9a6003c5)
Retained policy invalidation: Configuration events still increment mediaPolicyEpoch; the policy key includes session permission and workspace facts. The retained browser scenario verifies configuration-driven denial, preview removal, restoration, and three metadata requests. (ui/src/e2e/chat-flow.local-media-policy.e2e.test.ts:173, 907a9a6003c5)
Test consolidation retains behavioral coverage: Read the complete lifecycle suite: session-scoped allowance, renewal limits, split-pane sharing, disconnect cleanup, credential rotation, and credential-free artifact requests remain covered. These are inspected tests, not reviewer-executed results. (ui/src/pages/chat/components/chat-message-media-lifecycle.test.ts:123, 907a9a6003c5)
Existing product contract: The documented media flow assigns permission decisions to the Gateway and distinguishes retained decoded images from authorization for new reads. The UI-scoped policy also establishes that the UI and Gateway ship together, without older-Gateway compatibility fallbacks. Public docs: docs/web/control-ui.md. (docs/web/control-ui.md:1189, 907a9a6003c5)
Cleanup remains distinct from current main: Fetched main still emits and propagates localMediaPreviewRoots. The supplied GitHub context identifies fix(ui): keep uploaded images visible during worktree startup #139323 as merged, but that PR fixes image continuity rather than performing this cleanup. Latest-release source inspection failed because a required blob could not be fetched; no release inclusion is claimed. (src/gateway/control-ui.ts:1090, fcebd5d5a19b)

Likely related people:

steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

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.

History

Review history (1 earlier review cycle)

reviewed 2026-09-05T21:04:16.929Z sha 907a9a6 :: needs maintainer review before merge. :: none

steipete · 2026-09-05T21:05:42Z

@clawsweeper status

clawsweeper · 2026-09-05T21:05:46Z

🦞👀
ClawSweeper status:

Current PR: 139346
Branch: codex/simplify-chat-media-lifecycle
ClawSweeper PR: no
Automerge: not-enabled
Automerge job: jobs/openclaw/inbox/automerge-openclaw-openclaw-139346.md
Merge state: BLOCKED
Checks: SKIPPED:41 SUCCESS:137 IN_PROGRESS:1 NEUTRAL:1
Check blockers: checks-ui-e2e (6/7):IN_PROGRESS

steipete · 2026-09-05T21:23:59Z

Merged via squash.

Prepared head SHA: 907a9a6003c57e2d5f6044a32a7b414791239d8e
Landed commit: 69a993ad7873228ed40f5b2afe3bacab07fff51f

* refactor(ui): simplify chat media availability and lifecycle tests * test(ui): return URL mocks from the media fixture

refactor(ui): simplify chat media availability and lifecycle tests

00f2cd4

steipete self-assigned this Sep 5, 2026

openclaw-barnacle Bot added app: web-ui App: web-ui gateway Gateway runtime size: L maintainer Maintainer-authored PR labels Sep 5, 2026

test(ui): return URL mocks from the media fixture

907a9a6

steipete merged commit 69a993a into main Sep 5, 2026
464 of 471 checks passed

steipete deleted the codex/simplify-chat-media-lifecycle branch September 5, 2026 21:23

github-actions Bot mentioned this pull request Sep 5, 2026

📡 Upstream Digest — 2026-09-05 21:49 UTC curtismercier/openclaw-mods#1793

Open

Navigation Menu

Repository navigation

refactor(ui): remove obsolete chat media plumbing - #139346

refactor(ui): remove obsolete chat media plumbing#139346
steipete merged 2 commits into
mainfrom
codex/simplify-chat-media-lifecycle

steipete commented Sep 5, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Sep 5, 2026 •

edited

Loading

Uh oh!

steipete commented Sep 5, 2026

Uh oh!

clawsweeper Bot commented Sep 5, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Sep 5, 2026 •

edited

Loading

Security

PR surface

Review metrics

Technical review

Labels

Evidence

Rating scale

Workflow

History

Uh oh!

steipete commented Sep 5, 2026

Uh oh!

clawsweeper Bot commented Sep 5, 2026 •

edited

Loading

Uh oh!

Uh oh!

steipete commented Sep 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

steipete commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Before cleanup

After cleanup

Uh oh!

clawsweeper Bot commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ClawSweeper review complete

Uh oh!

steipete commented Sep 5, 2026

Uh oh!

clawsweeper Bot commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ClawSweeper review

What this changes

Merge readiness

Review scores

Verification

How this fits together

Before merge

Security

PR surface

Review metrics

Technical review

Labels

Evidence

Rating scale

Workflow

History

Uh oh!

steipete commented Sep 5, 2026

Uh oh!

clawsweeper Bot commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

steipete commented Sep 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

steipete commented Sep 5, 2026 •

edited

Loading

clawsweeper Bot commented Sep 5, 2026 •

edited

Loading

clawsweeper Bot commented Sep 5, 2026 •

edited

Loading

clawsweeper Bot commented Sep 5, 2026 •

edited

Loading

clawsweeper Bot commented Sep 5, 2026 •

edited

Loading