feat: panel side setting, hidden-file browsing, media previews, and video attachments - #8047
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 |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
UI consistency review found one regression in the attachment lightbox (two call sites) and one toggle-affordance inconsistency in the new Files header control. The panel-side layout work (PreviewPanelShell, RightPanelResizeHandle, RightPanelTabs, FilePreviewPanel explorer side, titlebar-inset conditions) looks coherent: borders, resize edge, titlebar insets and the collapsed-sidebar inset are all switched together, and the new settings row matches the neighbouring Select row exactly.
Posted via Macroscope — UI Consistency
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces multiple cross-cutting user-facing capabilities, including video uploads, media playback, filesystem walking, and configurable panel layout, with substantial server, client, and desktop changes. Unresolved substantive findings also affect attachment handling, filesystem errors, and settings restoration, so the changes need human review. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
There was a problem hiding this comment.
UI consistency review of the changed web surfaces. Two findings, both in the new video-attachment / left-dock paths. The rest (panel side plumbing in PreviewPanelShell / RightPanelResizeHandle / FilePreviewPanel, the Button-based hidden-files toggle, the settings Select row) follows the existing primitive and Tailwind ownership conventions.
Posted via Macroscope — UI Consistency
1e1fbd1 to
66bc1ae
Compare
There was a problem hiding this comment.
UI consistency review of the panel-side / hidden-files / video-attachment changes. Two findings: a desktop CSP gap that blocks the new <video>/<audio> playback in the Electron renderer, and an icon-geometry mismatch in the new Files-tab toggle. The earlier lightbox and header-inset findings look resolved.
Posted via Macroscope — UI Consistency
| onError={() => setFailedUrl(assetUrl.url)} | ||
| /> | ||
| ) : props.kind === "video" ? ( | ||
| <video |
There was a problem hiding this comment.
These are the app's first <video>/<audio> elements, and the desktop window's CSP declares no media-src: makeDesktopContentSecurityPolicy emits default-src 'self' plus an explicit img-src 'self' <scheme>: blob: data: http: https: (apps/desktop/src/electron/ElectronProtocol.ts), so media falls back to default-src 'self'. The signed asset URL here lives on the environment's http(s) origin, so in the desktop app the fetch is refused before a byte moves and this surface renders "Unable to load workspace video" — while the sibling <img> branch works because img-src enumerates those schemes. The same applies to the blob-backed previews added in ChatComposer and MessagesTimeline (blob: does not match 'self'); PullRequestMarkdown already documents this exact constraint as the reason it links out instead of playing.
Consider adding a matching media-src directive next to img-src in makeDesktopContentSecurityPolicy (and the corresponding assertion in ElectronProtocol.test.ts) so video/audio resolve from the same sources images already do.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review of the panel-side / hidden-files / video-attachment changes. Two findings: a desktop CSP gap that blocks the new <video>/<audio> playback in the Electron renderer, and an icon-geometry mismatch in the new Files-tab toggle. The earlier lightbox and header-inset findings look resolved.
Posted via Macroscope — UI Consistency
| onError={() => setFailedUrl(assetUrl.url)} | ||
| /> | ||
| ) : props.kind === "video" ? ( | ||
| <video |
There was a problem hiding this comment.
These are the app's first <video>/<audio> elements, and the desktop window's CSP declares no media-src: makeDesktopContentSecurityPolicy emits default-src 'self' plus an explicit img-src 'self' <scheme>: blob: data: http: https: (apps/desktop/src/electron/ElectronProtocol.ts), so media falls back to default-src 'self'. The signed asset URL here lives on the environment's http(s) origin, so in the desktop app the fetch is refused before a byte moves and this surface renders "Unable to load workspace video" — while the sibling <img> branch works because img-src enumerates those schemes. The same applies to the blob-backed previews added in ChatComposer and MessagesTimeline (blob: does not match 'self'); PullRequestMarkdown already documents this exact constraint as the reason it links out instead of playing.
Consider adding a matching media-src directive next to img-src in makeDesktopContentSecurityPolicy (and the corresponding assertion in ElectronProtocol.test.ts) so video/audio resolve from the same sources images already do.
Posted via Macroscope — UI Consistency
…ideo attachments Adds a Settings > Appearance 'Panel side' preference to dock the tool panel (files, terminal, diff, preview) on the left with chat on the right; a Files-panel toggle that lists gitignored and dot-prefixed entries via a direct filesystem walk; inline video/audio preview for workspace files; and end-to-end video attachments in chat (stored with the thread, played inline, handed to providers as a file path). Also fixes the revert prune deleting non-image attachments from disk, stops a file dropped outside the composer from navigating the app, and includes review-bot fixes for mixed image/video blob lifecycle, the image lightbox, video MIME validation, and left-dock titlebar insets.
… media CSP, eye icon size - Advance the handoff preview index for every attachment so mixed video+image messages pair blob URLs correctly. - Title video-only first sends 'Video: <name>' instead of 'Image:'. - Allow media-src in the desktop CSP so video/audio previews play in the Electron app. - Match the hidden-files eye icon size to the neighbouring refresh glyph.
66bc1ae to
c176294
Compare
| let dirents: ReadonlyArray<{ readonly name: string; isDirectory(): boolean }>; | ||
| try { | ||
| dirents = await NodeFSP.readdir(join(root, relativeDir), { withFileTypes: true }); | ||
| } catch { |
There was a problem hiding this comment.
🟡 Medium workspace/WorkspaceEntries.ts:160
When the normalized workspace root is unreadable or disappears, includeHidden listing returns a successful empty entries array instead of reporting WorkspaceEntriesReadDirectoryError (or the corresponding index failure). The catch at line 160 also handles the root directory, so distinguish the root read from nested-directory reads and propagate the root failure.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/workspace/WorkspaceEntries.ts around line 160:
When the normalized workspace root is unreadable or disappears, `includeHidden` listing returns a successful empty `entries` array instead of reporting `WorkspaceEntriesReadDirectoryError` (or the corresponding index failure). The catch at line 160 also handles the root directory, so distinguish the root read from nested-directory reads and propagate the root failure.
| message. Up to 8 files per message; images over 10 MB are compressed to fit, videos can be up to | ||
| 25 MB. Attached videos play inline in the conversation. Coding agents cannot watch video directly, |
There was a problem hiding this comment.
🟢 Low user/composer.md:26
Images over 10 MB can still be rejected when compressImageToByteLimit returns too_large, so the documentation's claim that they are compressed to fit is incorrect. Describe compression as an attempt and note that very large or poorly compressible images may still be refused.
| message. Up to 8 files per message; images over 10 MB are compressed to fit, videos can be up to | |
| 25 MB. Attached videos play inline in the conversation. Coding agents cannot watch video directly, | |
| message. Up to 8 files per message; images over 10 MB are compressed when possible, but very large or poorly compressible images can still be refused; videos can be up to | |
| 25 MB. Attached videos play inline in the conversation. Coding agents cannot watch video directly, |
🤖 Copy this AI Prompt to have your agent fix this:
In file @docs/user/composer.md around lines 26-27:
Images over 10 MB can still be rejected when `compressImageToByteLimit` returns `too_large`, so the documentation's claim that they are compressed to fit is incorrect. Describe compression as an attempt and note that very large or poorly compressible images may still be refused.
| for (const file of acceptedFiles) { | ||
| if (file.type.startsWith("video/")) { | ||
| // Videos ship byte-for-byte; there is no client-side recompression. | ||
| nextImages.push({ |
There was a problem hiding this comment.
🟠 High chat/ChatComposer.tsx:2535
Videos are treated as images when restored from the prompt stash, so a stashed video is dispatched with type: "image" and its video MIME type; larger videos are also cleared despite failing image compression. The stash flow must exclude videos or persist and hydrate their type and data end-to-end.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/ChatComposer.tsx around line 2535:
Videos are treated as images when restored from the prompt stash, so a stashed video is dispatched with `type: "image"` and its video MIME type; larger videos are also cleared despite failing image compression. The stash flow must exclude videos or persist and hydrate their type and data end-to-end.
|
|
||
| for (const attachment of input.attachments ?? []) { | ||
| if (attachment.type !== "image") { | ||
| // Claude ingests no raw video; hand the agent the stored file's path. |
There was a problem hiding this comment.
🟠 High Layers/ClaudeAdapter.ts:1268
A non-image attachment is silently omitted when resolveAttachmentPath returns null, so sendTurn succeeds without sending the attachment or notifying either the user or agent. Unlike the image branch, this path does not validate or read the file; return ProviderAdapterRequestError for an invalid path and propagate filesystem read failures before adding the prompt.
Also found in 1 other location(s)
apps/server/src/provider/Layers/GrokAdapter.ts:975
The new video branch returns a text prompt immediately without checking that
attachmentPathexists or is readable.resolveAttachmentPathonly performs path normalization/containment and does not touch the filesystem, whereas the image branch'sfileSystem.readFilesurfaces missing-file errors. If attachment metadata survives while its stored video is missing or inaccessible,sendTurnsucceeds and tells Grok the file is available at a nonexistent path, leaving the agent unable to process the attachment instead of returning a clear request error.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/ClaudeAdapter.ts around line 1268:
A non-image attachment is silently omitted when `resolveAttachmentPath` returns `null`, so `sendTurn` succeeds without sending the attachment or notifying either the user or agent. Unlike the image branch, this path does not validate or read the file; return `ProviderAdapterRequestError` for an invalid path and propagate filesystem read failures before adding the prompt.
Also found in 1 other location(s):
- apps/server/src/provider/Layers/GrokAdapter.ts:975 -- The new video branch returns a text prompt immediately without checking that `attachmentPath` exists or is readable. `resolveAttachmentPath` only performs path normalization/containment and does not touch the filesystem, whereas the image branch's `fileSystem.readFile` surfaces missing-file errors. If attachment metadata survives while its stored video is missing or inaccessible, `sendTurn` succeeds and tells Grok the file is available at a nonexistent path, leaving the agent unable to process the attachment instead of returning a clear request error.
| The Files tab lists your workspace. The listing follows your `.gitignore`, so ignored files and | ||
| some dotfiles are hidden by default. Use the eye toggle in the Files tab header to show hidden and | ||
| ignored files (`.git` and `node_modules` stay hidden). Selecting an image, video, or audio file | ||
| previews it inline; other files open as text. |
There was a problem hiding this comment.
🟢 Low user/workspace-panel.md:12
Selecting .htm, .html, or .pdf files opens the browser preview rather than the documented text view. The final sentence says every non-media file opens as text, so it should explicitly mention these browser-previewed formats.
| previews it inline; other files open as text. | |
| previews it inline; `.htm`, `.html`, and `.pdf` files open in the browser preview; other files open as text. |
🤖 Copy this AI Prompt to have your agent fix this:
In file @docs/user/workspace-panel.md around line 12:
Selecting `.htm`, `.html`, or `.pdf` files opens the browser preview rather than the documented text view. The final sentence says every non-media file opens as text, so it should explicitly mention these browser-previewed formats.
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.
Reviewed by Cursor Bugbot for commit c176294. Configure here.
| } | ||
| /> | ||
| ) : null} | ||
| <SettingsRow |
There was a problem hiding this comment.
Restore omits panel side
Medium Severity
The new rightPanelSide setting has a per-row reset on Appearance, but global Restore defaults never lists or resets it. Other Appearance prefs such as wordWrap and glassOpacity are included, so a left-docked panel survives a full restore while the confirmation dialog implies a complete reset.
Additional Locations (1)
Triggered by learned rule: Restore defaults on General settings page is intentionally global — not scoped per-page
Reviewed by Cursor Bugbot for commit c176294. Configure here.
There was a problem hiding this comment.
One finding: the new window-level drag/drop preventDefault in apps/web/src/routes/__root.tsx cancels the browser default for every drag, not just file drags, which suppresses native text drops into the app's <textarea>/<input> surfaces. Details inline.
The panel-side, hidden-files toggle, and media-preview changes reviewed in earlier runs look addressed: the desktop CSP now declares media-src, the eye icon is sized at the call site to match the neighbouring icon-xs refresh button, buildExpandedImagePreview excludes videos from the <img> gallery, and the chat header's collapsed-sidebar inset is now opt-out via reserveCollapsedSidebarInset when the panel is docked left. Side-aware geometry in PreviewPanelShell / RightPanelResizeHandle / FilePreviewPanel (border edge, resize edge, flex-row-reverse) is consistent with the useResizableWidth edge contract.
Posted via Macroscope — UI Consistency
| const preventWindowFileDrop = (event: DragEvent) => { | ||
| event.preventDefault(); | ||
| }; |
There was a problem hiding this comment.
This cancels the default action for all drags, not just file drags. preventDefault() from a window listener still applies after the target's own handler runs, so drags the target intentionally leaves to the browser lose their default too — dropping selected text into a native <textarea>/<input> (PR description editor, diff/review comment fields) no longer inserts anything, and every non-target area now reports a valid drop target on dragover.
Consider gating on the file payload, which is the only case that triggers the navigation this guards against:
| const preventWindowFileDrop = (event: DragEvent) => { | |
| event.preventDefault(); | |
| }; | |
| const preventWindowFileDrop = (event: DragEvent) => { | |
| // Only file drags navigate; leave text drags to their drop target's default. | |
| if (!event.dataTransfer?.types.includes("Files")) return; | |
| event.preventDefault(); | |
| }; |
Posted via Macroscope — UI Consistency
1 participant
What changed
1. Panel side preference (Settings → Appearance → "Panel side")
rightPanelSideclient setting inpackages/contracts(synced on web and desktop, searchable in settings search).2. Hidden files in the workspace file browser
.gitignore— so ignored files and some dotfiles could never be shown, and the indexer exposes no option to include them.includeHiddenflag onprojectsListEntries: the server walks the filesystem directly (breadth-first,.gitandnode_modulesstill skipped, same 25k entry cap). Ships with a server test.3. Video/audio preview in the Files panel
.mp4/.mov/.webm/.m4vused to fail the binary-file check; media files now render inline players via signed asset URLs (audio too: mp3, wav, ogg, flac, m4a). Extension allowlists extended inpackages/shared/filePreviewand the asset access layer.4. Video attachments in chat
ChatVideoAttachmentin the wire contract: MP4/MOV/WebM up to 25 MB per file (kept as a one-shot data-URL upload, same transport as images).<video>player that persists with the thread.Bugs found and fixed along the way
ProjectionPipelineskipped any attachment whose type wasn't"image", so any future non-image attachment was deleted on the next thread revert. The skip is removed;attachmentRelativePathnow covers every attachment type.dragover/droppreventDefault guard in the root route suppresses the default without interfering with real drop targets.Notes / known ceilings
Note
Medium Risk
Touches attachment persistence, provider adapters, asset signing, CSP, and a new filesystem walker for hidden files. Video uploads are larger (25 MB data URLs) and skip client compression/draft persistence.
Overview
Adds video chat attachments (MP4/MOV/WebM, 25 MB), a panel-side setting, hidden-file browsing, and inline video/audio previews in Files.
Composer drop/paste now accepts videos; they persist on the server and play in the timeline. Providers do not ingest raw video — adapters send a text note with the stored file path so the agent can use tools. Videos skip localStorage drafts. Revert prune now keeps non-image attachments on disk.
Settings → Appearance → Panel side docks the tool panel left or right (resize handle, borders, titlebar insets, and the Files explorer follow). Files gets an eye toggle that walks the filesystem (
includeHidden) so gitignored and dotfiles show (still skipping.git/node_modules). Selecting video/audio in Files plays via signed asset URLs; desktop CSP addsmedia-src. Window-level droppreventDefaultstops Chromium from navigating away on stray file drops.Reviewed by Cursor Bugbot for commit c176294. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add video attachments, panel side setting, hidden-file browsing, and media previews
ChatVideoAttachmentwith size/MIME limits, server normalizer persists them, and provider adapters (ClaudeAdapter,CodexAdapter,CursorAdapter,GrokAdapter) send videos as text content blocks with the stored file path.rightPanelSideclient setting ('left' | 'right', default'right') with an Appearance settings control;PreviewPanelShell,RightPanelTabs,WorkspacePageHeader, and resize handles adapt borders, insets, and drag edge to the chosen side.FileBrowserPanelgains a persistedshowHiddentoggle,WorkspaceEntries.listbranches onincludeHiddento use a bounded BFS walker (HIDDEN_WALK_MAX_ENTRIES25000, skips.gitandnode_modules), andProjectListEntriesInputacceptsincludeHidden.FilePreviewPanelfor images, videos, and audio via<img>,<video>, and<audio>elements; CSPmedia-srcupdated to allow blob and signed-asset URLs.WorkspaceEntries.listwithincludeHidden: trueperforms direct filesystem BFS and reportstruncatedbeyond 25,000 entries;node_modulesand.gitare always skipped. Video attachments are not persisted to local drafts and rely on<video>element buffering without client-side preload.Macroscope summarized c176294.