fix(ui): proxy canonical inbound media previews#100725
Conversation
openclaw-barnacle
Bot
added
app: web-ui
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a7cbe865c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| activeRestartRequest = upgradedRequest; | ||
| forceActiveRestartExit?.(); |
There was a problem hiding this comment.
When a managed update SIGUSR1 arrives while a regular restart is already draining active tasks, this branch only replaces activeRestartRequest and calls forceActiveRestartExit, whose callback just re-arms the shutdown watchdog. The drain loop above is already running with the original captured restartIntent, so waitForActiveTasks/waitForActiveEmbeddedRuns keep waiting for the old deferral window (or indefinitely) until the watchdog exits with code 1, instead of cleanly reaching the update respawn path. Please make this upgrade actually interrupt/force the active drain rather than only arming the timeout.
Useful? React with 👍 / 👎.
|
Land-ready verification for
Known proof gaps: no live Telegram credential roundtrip; the browser flow uses the real Control UI with a mocked Gateway plus the separately proven real Gateway HTTP route. The replacement preserves @sweetcornna's co-author and changelog credit from #90172. |
openclaw-barnacle
Bot
added
channel: discord
d2cd869 to
5b3e65e
Compare
openclaw-barnacle
Bot
removed
channel: discord
openclaw-barnacle
Bot
added
size: S
and removed
extensions: qa-lab
extensions: codex
extensions: openrouter
size: XL
labels
|
Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 4:36 AM ET / 08:36 UTC. Summary PR surface: Source +23, Tests +152. Total +175 across 3 files. Reproducibility: yes. Source inspection shows current main extracts image Review metrics: none identified. Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Next step before merge
Security Review detailsBest possible solution: Land this focused renderer change once required exact-head checks finish; it should supersede #90172 and close #89591. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main extracts image Is this the best way to solve the issue? Yes. Reusing the existing authenticated, ticketed assistant-media route in the renderer is narrower and safer than adding a new public inbound media route, and the PR adds malformed-ref coverage around that boundary. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against e7ca90e3afaf. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +23, Tests +152. Total +175 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (1 earlier review cycle)
|
clawsweeper
Bot
added
proof: sufficient
5b3e65e to
1bbd24e
Compare
Co-authored-by: Cornna <[email protected]>
1bbd24e to
3073f7d
Compare
clawsweeper
Bot
added
proof: sufficient
|
Land-ready verification for
Known proof gaps: no live Telegram credential roundtrip; the browser flow uses the real Control UI with a mocked Gateway plus the separately proven real Gateway HTTP route. The replacement preserves @sweetcornna's co-author credit from #90172; maintainer changelog credit is isolated in #100740 to avoid coupling this exact-head UI proof to the high-traffic release-notes hunk. |
|
Merged via squash.
|
Co-authored-by: Cornna <[email protected]>
Co-authored-by: Cornna <[email protected]>
1 participant
What Problem This Solves
Inbound channel images persisted as
media://inbound/<id>are extracted from chat history but left as an unloadable custom-scheme URL in the Control UI. The user sees only the attachment placeholder after reload. Fixes #89591 and salvages the valid intent from #90172.Why This Change Was Made
Recognize only the raw one-segment canonical inbound URI shape at the existing renderer proxy decision, then reuse the authenticated assistant-media metadata/ticket route. The raw predicate rejects nested, dot-segment, cross-bucket, query, fragment, encoded-separator, and NUL forms before they can bypass local preview-root checks. No new public media route or core policy is added.
User Impact
Images received through Telegram, iMessage, and other channels render inline in WebUI chat history after reload. Malformed media references and ordinary local files remain blocked by the existing containment policy.
Evidence
tbx_01kwv3e6wmqvd6m1bsh7x9jcgz: chat renderer plus Gateway assistant-media contract — 320 assertions passed across UI, Gateway variants, and real HTTP ticket flow.git diff --checkpassed.Co-authored-by: Cornna [email protected]