Uh oh!
There was an error while loading. Please reload this page.
fix(chat): show Codex generated images in the thread - #6441
fix(chat): show Codex generated images in the thread#6441sambhavthakkar wants to merge 4 commits into
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
0f61eab to
3052999CompareThere was a problem hiding this comment.
One finding on Effect error modeling in apps/server/src/assets/AssetAccess.ts. Everything else in the changed Effect code (namespace imports from effect/*, Effect.catchTags for the single WorkspacePathOutsideRootError tag, Path.Path acquired from the environment in resolveCodexGeneratedImagesRoots, no new Layer.succeed/ManagedRuntime/runPromise in service code) follows the conventions.
Posted via Macroscope — Effect Service Conventions
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
One finding on Effect error modeling in apps/server/src/assets/AssetAccess.ts. Everything else in the changed Effect code (namespace imports from effect/*, Effect.catchTags for the single WorkspacePathOutsideRootError tag, Path.Path acquired from the environment in resolveCodexGeneratedImagesRoots, no new Layer.succeed/ManagedRuntime/runPromise in service code) follows the conventions.
Posted via Macroscope — Effect Service Conventions
Uh oh!
There was an error while loading. Please reload this page.
| function* (settings: ServerSettingsValue) { | ||
| const path = yield* Path.Path; | ||
| const roots: string[] = []; | ||
| const instances = Object.values(settings.providerInstances) |
There was a problem hiding this comment.
🔴 Criticalsrc/ws.ts:142
A client authorized for one thread can mint a signed URL for a known image from another configured Codex instance or thread. resolveCodexGeneratedImagesRoots returns generated_images roots for every Codex instance, while asset issuance validates only the requested thread, so an absolute path under any returned root bypasses thread/workspace isolation. Restrict resolution to the thread's modelSelection.instanceId (and its session provider instance) instead of returning all configured roots.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/ws.ts around line 142:
A client authorized for one thread can mint a signed URL for a known image from another configured Codex instance or thread. `resolveCodexGeneratedImagesRoots` returns `generated_images` roots for every Codex instance, while asset issuance validates only the requested thread, so an absolute path under any returned root bypasses thread/workspace isolation. Restrict resolution to the thread's `modelSelection.instanceId` (and its session provider instance) instead of returning all configured roots.
| ...(event.payload.status ? { status: event.payload.status } : {}), | ||
| ...(event.payload.detail ? { detail: truncateDetail(event.payload.detail) } : {}), | ||
| ...(event.payload.detail | ||
| ? { detail: toolLifecycleDetail(event.payload.itemType, event.payload.detail) } |
There was a problem hiding this comment.
🟡 MediumLayers/ProviderRuntimeIngestion.ts:805
image_view activity details are persisted and sent to clients without the 180-character cap, so OpenCode image-tool outputs—including large or base64 text—can create oversized activity records and websocket payloads. toolLifecycleDetail disables truncation for every image_view, even though OpenCode maps any tool name containing “image” to that type and detailFromToolPart supplies the full output or error text; preserve untruncated details only after verifying they are file paths (or only for the Codex generated-image case).
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts around line 805:
`image_view` activity details are persisted and sent to clients without the 180-character cap, so OpenCode image-tool outputs—including large or base64 text—can create oversized activity records and websocket payloads. `toolLifecycleDetail` disables truncation for every `image_view`, even though OpenCode maps any tool name containing “image” to that type and `detailFromToolPart` supplies the full output or error text; preserve untruncated details only after verifying they are file paths (or only for the Codex generated-image case).
| if (trimmed.includes("\n") || /\s/.test(trimmed)) { | ||
| return undefined; | ||
| } | ||
| if (trimmed.includes("/") || trimmed.includes("\\")) { |
There was a problem hiding this comment.
🟡 MediumLayers/CodexAdapter.ts:283
imageGenerationResultPath returns short base64 image data containing / as detail, so image rows treat the payload as a filesystem path and render a failed preview instead of falling back to revisedPrompt. Validate that result is an actual path rather than relying on / alone.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/CodexAdapter.ts around line 283:
`imageGenerationResultPath` returns short base64 image data containing `/` as `detail`, so image rows treat the payload as a filesystem path and render a failed preview instead of falling back to `revisedPrompt`. Validate that `result` is an actual path rather than relying on `/` alone.
| return ( | ||
| trimText(item.savedPath ?? undefined) ?? | ||
| imageGenerationResultPath(item.result) ?? | ||
| trimText(item.revisedPrompt ?? undefined) | ||
| ); |
There was a problem hiding this comment.
🟡 MediumLayers/CodexAdapter.ts:291
Pathless imageGeneration items return revisedPrompt as detail, so extractImageViewPath sends prompt text such as a ceramic mug to the workspace-asset loader as imagePath, producing a broken image preview. Only actual image paths should populate this detail channel; remove the revisedPrompt fallback.
| return( | |
| trimText(item.savedPath??undefined)?? | |
| imageGenerationResultPath(item.result)?? | |
| trimText(item.revisedPrompt??undefined) | |
| ); | |
| return( | |
| trimText(item.savedPath??undefined)?? | |
| imageGenerationResultPath(item.result) | |
| ); |
Also found in 1 other location(s)
apps/mobile/src/lib/threadActivity.ts:966
extractImageViewPathtreats every non-emptypayload.detailas an image path. For animageGenerationitem withoutsavedPath(the protocol makes it optional),CodexAdapter.itemDetailcan populatedetailwithrevisedPrompt; this then becomesimagePath, so mobile attempts to request and render the prompt text as a workspace asset and shows a broken image preview. The fallback should only acceptdetailwhen it is known to be a path (for example, an actualimageViewitem), not for pathless image-generation results.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/CodexAdapter.ts around lines 291-295:
Pathless `imageGeneration` items return `revisedPrompt` as `detail`, so `extractImageViewPath` sends prompt text such as `a ceramic mug` to the workspace-asset loader as `imagePath`, producing a broken image preview. Only actual image paths should populate this detail channel; remove the `revisedPrompt` fallback.
Also found in 1 other location(s):
- apps/mobile/src/lib/threadActivity.ts:966 -- `extractImageViewPath` treats every non-empty `payload.detail` as an image path. For an `imageGeneration` item without `savedPath` (the protocol makes it optional), `CodexAdapter.itemDetail` can populate `detail` with `revisedPrompt`; this then becomes `imagePath`, so mobile attempts to request and render the prompt text as a workspace asset and shows a broken image preview. The fallback should only accept `detail` when it is known to be a path (for example, an actual `imageView` item), not for pathless image-generation results.
| /> | ||
| ); | ||
| } | ||
| if (src && /^https?:\/\//i.test(src)) { |
There was a problem hiding this comment.
🟡 Mediumcomponents/ChatMarkdown.tsx:1564
Protocol-relative images such as  fail to load and, when a thread exists, are sent to ChatWorkspaceImage as a workspace path. The img renderer only identifies http:// and https:// as remote, so resolveMarkdownFileLinkMeta classifies //cdn.example.com/logo.png as a file link; treat // URLs as remote as well.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ChatMarkdown.tsx around line 1564:
Protocol-relative images such as `` fail to load and, when a thread exists, are sent to `ChatWorkspaceImage` as a workspace path. The `img` renderer only identifies `http://` and `https://` as remote, so `resolveMarkdownFileLinkMeta` classifies `//cdn.example.com/logo.png` as a file link; treat `//` URLs as remote as well.
| renderFeedEntry(info, { | ||
| environmentId: props.environmentId, | ||
| threadId: props.threadId, | ||
| workspaceRoot: props.workspaceRoot, |
There was a problem hiding this comment.
🟠 Highthreads/ThreadFeed.tsx:1969
Visible image rows keep using the previous workspaceRoot when threadCwd changes without a feed-data change, so images remain missing or resolve against the wrong root until another list invalidation occurs. workspaceRoot is captured by renderItem but is absent from listAppearanceData/extraData, and LegendList does not repaint visible rows when only that closure changes; include it in extraData (or otherwise invalidate/remount the rows).
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 1969:
Visible image rows keep using the previous `workspaceRoot` when `threadCwd` changes without a feed-data change, so images remain missing or resolve against the wrong root until another list invalidation occurs. `workspaceRoot` is captured by `renderItem` but is absent from `listAppearanceData`/`extraData`, and LegendList does not repaint visible rows when only that closure changes; include it in `extraData` (or otherwise invalidate/remount the rows).
| const [failed, setFailed] = useState(false); | ||
| const [expanded, setExpanded] = useState(false); | ||
| const name = props.alt.trim().length > 0 ? props.alt : "Image"; | ||
| if (failed) { | ||
| return ( | ||
| <div className="my-2 rounded-lg border border-border/80 bg-background/70 px-3 py-2 text-secondary-label text-xs"> | ||
| Unable to load image{props.alt ? ` “${props.alt}”` : ""}. | ||
| </div> | ||
| ); | ||
| } | ||
| return ( | ||
| <> | ||
| <button | ||
| type="button" | ||
| className={cn( | ||
| "my-2 block max-w-full cursor-zoom-in overflow-hidden rounded-lg border border-border/80 bg-background/70", | ||
| props.className, | ||
| )} | ||
| aria-label={`Preview ${name}`} | ||
| onClick={() => setExpanded(true)} | ||
| > | ||
| <img | ||
| src={props.src} | ||
| alt={name} | ||
| className="block h-auto max-h-[360px] max-w-full object-contain" | ||
| onError={() => setFailed(true)} |
There was a problem hiding this comment.
🟡 Mediumchat/ChatWorkspaceImage.tsx:15
After ChatRemoteImage's image load fails, changing props.src still renders the old error state, so corrected remote URLs remain permanently hidden. Because failed is a component-wide boolean, it is never cleared for the new URL; track the failed URL instead (or reset the state when props.src changes).
- const [failed, setFailed] = useState(false);+ const [failed, setFailed] = useState<string | null>(null);- if (failed) {+ if (failed === props.src) {
return (
<div className="my-2 rounded-lg border border-border/80 bg-background/70 px-3 py-2 text-secondary-label text-xs">
Unable to load image{props.alt ? ` “${props.alt}”` : ""}.
@@
- onError={() => setFailed(true)}+ onError={() => setFailed(props.src)}🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/ChatWorkspaceImage.tsx around lines 15-42:
After `ChatRemoteImage`'s image load fails, changing `props.src` still renders the old error state, so corrected remote URLs remain permanently hidden. Because `failed` is a component-wide boolean, it is never cleared for the new URL; track the failed URL instead (or reset the state when `props.src` changes).
| </div> | ||
| </div> | ||
| {imagePath && threadRef ? ( | ||
| <div className="ms-7 mt-1" onClick={stopRowToggle} onPointerDown={stopRowToggle}> |
There was a problem hiding this comment.
🟡 Mediumchat/MessagesTimeline.tsx:2398
When ChatWorkspaceImage is focused, pressing Enter or Space bubbles to the row's onKeyDown, which calls preventDefault() and toggles expanded instead of activating the image button. Add an onKeyDown handler to this wrapper that stops propagation so keyboard users can open the generated-image dialog.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/MessagesTimeline.tsx around line 2398:
When `ChatWorkspaceImage` is focused, pressing Enter or Space bubbles to the row's `onKeyDown`, which calls `preventDefault()` and toggles `expanded` instead of activating the image button. Add an `onKeyDown` handler to this wrapper that stops propagation so keyboard users can open the generated-image dialog.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3052999. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
ApprovabilityVerdict: Needs human review 8 blocking correctness issues found. This PR introduces new capability for displaying Codex-generated images with cross-cutting changes to mobile, web, and server-side authorization logic. A Critical finding identifies a potential thread isolation bypass in asset URL minting that requires human review. You can customize Macroscope's approvability policy. Learn more. |
What Changed
Codex imagegen already writes a file and emits
savedPath. T3 dropped that path, labeled the row Image view, and only served workspace files, so generated images never showed in chat.savedPathon the Codex item and label it Generated image$CODEX_HOME/generated_imagesthrough signed asset URLs (not the base64resultover the websocket)![]()and the work row)Why
The file is already on disk and the protocol already names it. Chat should show the picture, including remote clients.
UI Changes
![]()does not loadChecklist
Note
Medium Risk
Adds host-path asset access outside the workspace with root allowlisting—security-sensitive but scoped to Codex
generated_imagesand exact basename tokens; broad touch across server assets, ingestion, and both clients.Overview
Codex-generated images now render inline in web and mobile chat (markdown images and work-log rows) instead of showing only a path or a generic “Image view” label.
On the server, Codex
imageGeneration/imageViewitems keep fullsavedPath(no truncation forimage_view), map to Generated image where appropriate, and workspace asset URLs can fall back tohost-image-exacttokens for files under configured Codexgenerated_imagesroots when the path is outside the project workspace. WebSocket asset issuance passes those roots from server settings.Clients add custom markdown image rendering (
renderImageon native markdown;ChatWorkspaceImage/MarkdownWorkspaceImage) with signed workspace URLs, loading/error UI, and tap-to-preview. Work-log activities exposeimagePathfrom lifecycle payloads for inline previews. Mobile asset hooks gainuseAssetUrlStatefor non-binary loading/failure handling; list sizing skips fixed height when a row has an image preview.Reviewed by Cursor Bugbot for commit a1816fc. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Show Codex-generated images inline in chat threads on web and mobile
image_viewwork log entries as inline image previews in both the webMessagesTimelineand mobileThreadFeed, with click-to-expand dialogs and loading/failure states.host-image-exactasset claim inAssetAccess.tsso the server can securely serve generated images that live outside the workspace root, validated against configuredgeneratedImagesRoots.CodexAdapter.tsto populatedetailonimageGeneration/imageViewitems with the saved file path, and preserves full (untruncated) paths through the ingestion pipeline forimage_viewevents.renderImageprop toNativeMarkdownBlockandSelectableMarkdownTextso custom image renderers can be injected into the markdown tree on mobile.image_viewdetail strings are no longer truncated anywhere in the pipeline; very long paths are passed through as-is.Macroscope summarized a1816fc.