fix(web): preview images outside the workspace#21064

closed4
main
GitHub fork: fix/sidebar-images-outside-workspace
opened12d agobyTtrahane
View on GitHubTwo-way sync enabledPR last imported 5d ago

Fixes #21067.

Selecting an image from an absolute folder outside a thread's workspace showed “Unable to load workspace image.” The Files preview now routes that path through the existing host-media resource. Internal files retain their existing resource, and unsent drafts retain the draft resource. The shared helper owns this decision; the browser caller no longer repeats it.

Merged pinned main, preserving its PDF browser frame and Markdown asset refresh behavior. The two focused regressions remain: external images use host media, and external draft paths retain the draft resource. All 45 preview/media-reference tests, scoped lint/formatting and web typecheck pass at 31c5a73. Removing the helper fix makes the external-image regression fail.

Fresh built-in Browser proof compared that exact head with pinned main using the same isolated thread, file, viewport and theme. Opening the external folder link and selecting its PNG fails on the base and renders on the head. Internal images and an external image in an unsent draft also render; draft text remains intact. Markdown's embedded image updated after manual refresh and tab reselection; automatic refresh was not established. PDF reached the application/pdf embed, but this browser's unavailable PDF plugin prevented checking the rendered document. No native Electron, Windows/UNC, mobile, or cross-host relay/tunnel UI pass is claimed. Draft resource kind is covered by the focused test, not directly observed from the runtime request.

Before — pinned main: External image fails in the Files preview

After — 31c5a73: The same external image renders in the Files preview

Recorded Files selection and image rendering

Original fix by @trahane in #21064. Maintainer takeover by Claude Opus 5.5 and Claude Fable 5.1 through T3 Code; independent review by Claude Opus 5.5 through Claude Code and GPT-6.1-Sol through the Codex harness. Browser proof by GPT-6.1-Sol through T3 Code.

Checks

21 successful · 13 skipped
All checks passed
31c5a73
Synced from GitHub

Approvability

Verdict: Approved at 47e4e1f

Macroscope's review found this PR approvable — This is a narrowly scoped web bug fix that reuses the existing media-file path for external workspace images while preserving internal, explicit-media, and draft behavior. Targeted tests cover POSIX, normalized, Windows, and UNC paths, with no default or static-analysis configuration changes.

You can add or adjust custom eligibility rules. Learn more.

Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7b9b76f9-3346-4ed4-93ed-955723da16eb
📥 Commits

Reviewing files that changed from the base of the PR and between 16918b57d6754d6f145e9b6367c825aa31a740da and 47e4e1fe56d554533f2cfc5642b07f674bd8eb1d.

📒 Files selected for processing: 2
  • apps/web/src/components/files/FilePreviewPanel.test.ts
  • apps/web/src/components/files/filePreviewMode.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

workspaceAssetResource now classifies non-draft workspace-file resources without a relative path as media-file resources. Tests cover workspace boundaries, Windows path casing, explicit media requests, and draft mode.

Changes: Workspace asset classification

Layer / File(s)Summary
Resource selection and path coverage
apps/web/src/components/files/filePreviewMode.ts, apps/web/src/components/files/FilePreviewPanel.test.ts
For non-draft resources, workspaceAssetResource changes the kind from workspace-file to media-file when the media reference has no relative path. Tests cover in-workspace and outside-workspace paths, Windows path casing, explicit media requests, and draft mode.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 47e4e

Outside-workspace image previews use the media-file path, and no merge-blocking issue remains after normal checks.

### Architecture Summary

Architecture risk: 🔵 Low · up to 47e4e

The change affects 1 system.

Changed systems: apps/web

Architecture concerns No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/web (ui) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/web/src/components/files/FilePreviewPanel.test.ts: Adds workspaceAssetResource test cases expecting in-workspace images to use the workspace-file resource and outside-workspace images—including sibling-prefix paths, traversal paths, and paths on another drive—to use media-file. The cases also cover case-insensitive Windows paths, explicit media requests inside the workspace, and an outside-workspace image in draft mode expecting a draft-workspace-file resource.
  • observed — Modified behavior in apps/web/src/components/files/filePreviewMode.ts: Adds the mediaFileReference import used to determine whether a workspace file should be represented as a media file.
  • observed — Modified behavior in apps/web/src/components/files/filePreviewMode.ts: For non-draft resources, workspaceAssetResource now converts "workspace-file" to "media-file" when the media reference has no relativePath; otherwise it preserves the input kind. The returned resource retains the same thread ID and absolute path.
Pre-merge checks: Passed 4
Check nameStatusExplanation
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Title checkPassedThe title clearly and concisely describes the primary fix: allowing preview of images located outside the workspace.
Description checkPassedThe description covers the problem, implementation, scope approval through the linked issue, focused tests, browser verification, screenshots, recording, limitations, and agent details.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Added 1 commit

  • fix(web): preview images outside the workspace2bb93ce

Taking over this one. I’ll reduce the tests to the useful coverage, update the implementation against current main, and verify it before publishing the changes.

Added 3 commits

  • Merge remote-tracking branch 'origin/main' into takeover/210644e89fad
  • fix(web): centralize preview asset selection and trim redundant testsdf42f9e
  • Merge commit 'c274a251510e4bb035f8479a7f1e96443cd2503c' into takeover/2106431c5a73

Thanks @trahane for the fix. Taken over in #24627, retaining your shared media-resource selection approach on current main, with minimal regression tests and verified before/after browser evidence. Closing this fork PR in favor of the credited replacement; your branch is unchanged.