Skip to content

feat(storage): replace File System Access API with browser-native OPFS#260

Open
janiluuk wants to merge 1 commit into
walterlow:mainfrom
janiluuk:feat/browser-storage-no-fsa
Open

feat(storage): replace File System Access API with browser-native OPFS#260
janiluuk wants to merge 1 commit into
walterlow:mainfrom
janiluuk:feat/browser-storage-no-fsa

Conversation

@janiluuk
Copy link
Copy Markdown

@janiluuk janiluuk commented May 27, 2026

Summary

  • No more folder-picker splashWorkspaceGate auto-initializes via navigator.storage.getDirectory(). Storage-dependent routes show a blank background for <1 frame, then render immediately. No warning, no permission prompt.
  • <input type="file"> for media importshowOpenFilePicker replaced with a standard file input element. Returns File[] copied into OPFS (storageType: 'opfs'). Works in all browsers.
  • dataTransfer.files for drag-dropgetAsFileSystemHandle() removed; ExtractedMediaFileEntry.handle dropped. All call sites updated to use .file.
  • showFileHandlePicker kept as a @deprecated helper for relinking broken handle-based media from older projects.

Changes

File Change
workspace-gate/workspace-gate.tsx Replaced entire splash/picker flow with OPFS auto-init
media-library/utils/media-file-picker.ts showOpenFilePicker<input type="file">, returns File[]
media-library/stores/media-import-actions.ts importHandles/importHandlesForPlacement take File[]
media-library/services/media-library-service.ts Added public importMediaFile(file, projectId) method
media-library/utils/file-drop.ts getAsFileSystemHandledataTransfer.files
media-library/types.ts Updated importHandles/importHandlesForPlacement signatures
media-grid.tsx, missing-media-dialog.tsx Use showFileHandlePicker for legacy relink only

Test plan

  • Open app — editor/projects loads immediately with no splash screen
  • Click "Add media" button — standard browser file dialog opens (not FSA picker)
  • Drag a video file onto the media library panel — file imports correctly
  • Drag a video file onto the timeline — file imports and places on timeline
  • URL import still works
  • TypeScript: npx tsc --noEmit → 0 errors

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Refactor
    • Enhanced media import and drag-drop functionality for more consistent file handling across media placement and import workflows.
    • Simplified workspace initialization by removing legacy filesystem permission request workflows, eliminating permission prompts and workspace reconnection dialogs that previously appeared during startup.
    • Streamlined media import service to support direct file object handling throughout the application.

Review Change Stack

WorkspaceGate now auto-initializes via navigator.storage.getDirectory()
instead of showing a folder-picker splash screen. Storage-dependent
routes block for <1 frame while OPFS opens, then render immediately —
no warning, no permission prompt, straight to the editor.

Media file import switches from showOpenFilePicker to <input type="file">,
returning File[] objects that are copied into OPFS (storageType: 'opfs').
importHandles/importHandlesForPlacement now accept File[] and route through
the new public importMediaFile() service method.

Drag-drop uses dataTransfer.files instead of getAsFileSystemHandle(),
removing the dependency on the File System Access API drag-drop extension.
ExtractedMediaFileEntry.handle removed; all call sites updated to use .file.

showFileHandlePicker preserved as a @deprecated helper for relinking
broken handle-based media in existing projects (media-grid,
missing-media-dialog).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented May 27, 2026

@janiluuk is attempting to deploy a commit to the walterlow's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

📝 Walkthrough

Walkthrough

This PR migrates the media library import pipeline from File System Access API (FileSystemFileHandle) to plain File objects, and refactors WorkspaceGate to bootstrap OPFS on mount instead of managing permissions. The file picker now uses HTML input, service and action layers accept File[], drag-drop integration passes files through the pipeline, and cross-feature consumers (preview, timeline) are updated to match.

Changes

Media Library File API Migration

Layer / File(s) Summary
File picker abstraction: input-based picker with deprecated handle fallback
src/features/media-library/utils/media-file-picker.ts, src/features/media-library/utils/media-file-picker.test.ts
showMediaFilePicker reimplemented to use HTML <input type="file"> and return Promise<File[]> instead of calling window.showOpenFilePicker. New deprecated showFileHandlePicker preserves legacy handle-based behavior. hasMediaFilePickerSupport now always returns true.
File-drop data shape: handle field removed, file and mediaType retained
src/features/media-library/utils/file-drop.ts
ExtractedMediaFileEntry drops handle field, now contains only file and mediaType. extractValidMediaFileEntriesFromDataTransfer rewritten to extract files directly from dataTransfer.files without FileSystemHandle resolution.
Service layer: new importMediaFile entry point for File-based imports
src/features/media-library/services/media-library-service.ts
New importMediaFile(file, projectId) method validates projectId and delegates to importMediaFileToOpfs, returning MediaMetadata with optional duplicate/codec flags.
Import actions refactor: File[] pipeline with optimistic OPFS storage
src/features/media-library/stores/media-import-actions.ts, src/features/media-library/stores/media-import-actions.test.ts
Import task shape changed to { tempId, file } without handle; optimistic metadata uses storageType: 'opfs'. importMedia picker calls showMediaFilePicker directly and treats empty selection as cancelled success. Internal import runner calls mediaLibraryService.importMediaFile(file, projectId). Public importHandles and importHandlesForPlacement now accept File[] and route through shared pipeline. Test suite switched from handle-based mocks to importMediaFile mocks.
Import actions API types: File[] parameter signatures
src/features/media-library/types.ts
MediaLibraryActions.importHandles and importHandlesForPlacement now accept File[] instead of FileSystemFileHandle[].
Component wiring: drag-drop and relink flows updated to File APIs
src/features/media-library/components/media-library.tsx, src/features/media-library/components/media-grid.tsx, src/features/media-library/components/missing-media-dialog.tsx
Media library drag-drop handler passes entry.file to handleImportHandles(File[]). Relink and restore flows use deprecated showFileHandlePicker for backward compatibility, routing through File-based import pipeline.
Contract re-exports: removed supportsFileSystemDragDrop, expose extractValidMediaFileEntriesFromDataTransfer
src/features/media-library/contracts/timeline.ts, src/features/preview/deps/media-library-contract.ts, src/features/timeline/deps/media-library-resolver.ts
Barrel re-exports updated to remove supportsFileSystemDragDrop and expose extractValidMediaFileEntriesFromDataTransfer.
Cross-feature integration: timeline and preview hooks use File-based drop handling
src/features/timeline/utils/drop-execution.ts, src/features/preview/hooks/use-canvas-media-drop.ts, src/features/timeline/utils/external-file-project-match.test.ts
importHandlesForPlacement callback type updated to accept File[]; drop handlers now pass entry.file instead of entry.handle. Test helper removes handle field from entry construction.

Workspace Gate OPFS Bootstrap Refactor

Layer / File(s) Summary
OPFS bootstrap on mount: initialization without permission workflow
src/features/workspace-gate/workspace-gate.tsx
Refactored from permission/handle-state machine to direct OPFS bootstrap. Removed handles-db integration, permission workflow (request, picker, reconnect), and splash UI. Now initializes OPFS via navigator.storage.getDirectory(), bootstraps workspace, schedules background trash auto-purge, and uses single ready boolean to gate protected route rendering.

Sequence Diagram(s)

sequenceDiagram
  participant User as User/UI
  participant Picker as File Picker
  participant Actions as Import Actions
  participant Service as MediaLibraryService
  participant OPFS as OPFS Storage
  
  User->>Picker: Click import or drag files
  Picker->>Picker: Show file dialog or extract from drop
  Picker-->>Actions: File[]
  Actions->>Actions: Create optimistic metadata (storageType: opfs)
  Actions->>Service: importMediaFile(file, projectId)
  Service->>OPFS: importMediaFileToOpfs(file)
  OPFS-->>Service: MediaMetadata
  Service-->>Actions: metadata + metadata
  Actions->>Actions: Resolve optimistic, emit success/error
  Actions-->>User: Imported media list
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • walterlow/freecut#69: Both PRs modify the drag-and-drop import flow in media-library.tsx—main switches from FileSystemFileHandle[] to File[] in the drop handler, while the retrieved PR adds drop-zone panel-level integration wiring with handle-based imports.
  • walterlow/freecut#41: Both PRs touch src/features/media-library/stores/media-import-actions.ts around File System Access API support gating and error messaging—main removes the old support gate while the retrieved PR adds Brave-specific fallback guidance.
  • walterlow/freecut#172: Both PRs refactor src/features/media-library/services/media-library-service.ts to modularize helper logic—main adds new importMediaFile entry point while the retrieved PR extracts image/thumbnail/permission handling into separate modules.

Poem

🐰 From handles so complex to Files so plain,
OPFS boots with nary a permission's pain,
Drag-drop and picker both sing the same song,
Your media flows freely—where it all belongs! 📁✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(storage): replace File System Access API with browser-native OPFS' clearly and concisely captures the primary architectural change across the entire changeset—moving away from the File System Access API to browser-native OPFS and File-based workflows.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

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

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 27, 2026

Greptile Summary

This PR replaces the File System Access API (folder-picker + FileSystemFileHandle) with browser-native OPFS (navigator.storage.getDirectory()) for workspace storage, and swaps showOpenFilePicker for a standard <input type="file"> element for media import. The result is a zero-splash, no-permission-prompt startup flow and broader browser compatibility for file picking.

  • WorkspaceGate now auto-initializes via navigator.storage.getDirectory() and removes all picker/splash UI; setReady(true) is placed after the catch block and fires even on initialization failure.
  • showMediaFilePicker becomes a programmatic <input type="file"> that resolves with File[]; cancellation relies on the cancel event, which is not universally supported in older browsers.
  • All FileSystemFileHandle[] call sites across drag-drop, timeline, and preview have been updated to File[], and the handle field has been removed from ExtractedMediaFileEntry.

Confidence Score: 3/5

The core OPFS migration is clean and the drag-drop/timeline call-site updates are correct, but two initialization paths leave users in a broken or hung state without any UI feedback.

When navigator.storage.getDirectory() fails (private browsing in Firefox/Safari, quota restrictions), setReady(true) still fires and renders protected routes against an uninitialized workspace — every storage operation will silently fail. Separately, dismissing the file picker on Safari older than 17.4 leaves the showMediaFilePicker promise permanently unresolved, hanging the import UI with no way to recover. Both paths can be hit in normal use.

workspace-gate.tsx and media-file-picker.ts each have a scenario where the app proceeds without a valid state; everything else in the PR looks correct.

Important Files Changed

Filename Overview
src/features/workspace-gate/workspace-gate.tsx Simplified to auto-init via OPFS; setReady(true) is called even on initialization failure, leaving protected routes unblocked with no valid workspace root.
src/features/media-library/utils/media-file-picker.ts Replaced showOpenFilePicker with input[type=file]; cancel event not universally supported — promise hangs forever on older Safari when user dismisses dialog without selecting a file.
src/features/media-library/stores/media-import-actions.ts Handle-based imports replaced with File[] throughout; minor logging regression passes full PromiseSettledResult instead of result.reason to logger.error.
src/features/media-library/utils/file-drop.ts Simplified to use dataTransfer.files directly, removing getAsFileSystemHandle; logic is correct and more broadly compatible.
src/features/media-library/services/media-library-service.ts Added public importMediaFile(file, projectId) wrapper that delegates to the existing importMediaFileToOpfs; straightforward and correct.
src/features/media-library/components/media-grid.tsx Swapped showMediaFilePicker import to showFileHandlePicker for legacy relink flow; change is correct.
src/features/timeline/utils/drop-execution.ts Updated importHandlesForPlacement signature from FileSystemFileHandle[] to File[] and call sites updated accordingly; correct.

Sequence Diagram

sequenceDiagram
    participant App
    participant WorkspaceGate
    participant OPFS as navigator.storage (OPFS)
    participant MediaLib as MediaLibrary
    participant FilePicker as input[type=file]
    participant DragDrop as file-drop.ts

    App->>WorkspaceGate: mount
    WorkspaceGate->>OPFS: getDirectory()
    OPFS-->>WorkspaceGate: FileSystemDirectoryHandle
    WorkspaceGate->>WorkspaceGate: setWorkspaceRoot(handle)
    WorkspaceGate->>WorkspaceGate: bootstrapWorkspace(handle)
    WorkspaceGate->>WorkspaceGate: setReady(true) [always, even on error]
    WorkspaceGate-->>App: render children

    App->>MediaLib: importMedia()
    MediaLib->>FilePicker: createElement('input') + click()
    FilePicker-->>MediaLib: change event → File[]
    Note over FilePicker,MediaLib: cancel event may not fire on older Safari
    MediaLib->>MediaLib: importMediaFile(file, projectId) → OPFS

    App->>DragDrop: extractValidMediaFileEntriesFromDataTransfer(dataTransfer)
    DragDrop->>DragDrop: Array.from(dataTransfer.files)
    DragDrop-->>App: "ExtractedMediaFileEntry[] {file, mediaType}"
    App->>MediaLib: importHandlesForPlacement(files) → OPFS
Loading
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
src/features/workspace-gate/workspace-gate.tsx:41-44
`setReady(true)` is called unconditionally even when the outer `catch` fires. If `navigator.storage.getDirectory()` rejects (private-browsing mode in some browsers, or permissions denied), `setWorkspaceRoot` is never called, yet the gate immediately unblocks protected routes — every subsequent OPFS read/write will fail without any error UI shown to the user.

```suggestion
      } catch (error) {
        logger.error('OPFS initialization failed', error)
        return
      }
      setReady(true)
```

### Issue 2 of 3
src/features/media-library/utils/media-file-picker.ts:19-22
The `cancel` event on `<input type="file">` is only supported from Safari 17.4+. On older Safari (and any browser that never fires `cancel`), dismissing the file picker without selecting a file leaves the returned Promise permanently unresolved — the caller in `importMedia` hangs forever and the UI never recovers. A common fix is to register a one-shot `focus` listener on `window` as a fallback cancellation signal.

```suggestion
    input.addEventListener('change', () => done(Array.from(input.files ?? [])))
    input.addEventListener('cancel', () => done([]))

    // Fallback for browsers that don't fire 'cancel' (Safari < 17.4):
    // window regains focus after the dialog closes without a selection.
    const onFocus = () => {
      // Give the 'change' event a tick to arrive first.
      setTimeout(() => done([]), 300)
    }
    window.addEventListener('focus', onFocus, { once: true })

    input.click()
```

### Issue 3 of 3
src/features/media-library/stores/media-import-actions.ts:132
The second argument passed to `logger.error` is now `importResults[index]` — the entire `PromiseSettledResult` object (`{ status: 'rejected', reason: Error }`) — rather than the underlying error. The test reflects this by relaxing the assertion from `expect.any(Error)` to `expect.anything()`. Log consumers and error-tracking integrations that expect an `Error` instance here will receive a plain object instead.

```suggestion
      logger.error(`Failed to import ${importTask.file.name}`, result.reason)
```

Reviews (1): Last reviewed commit: "feat(storage): replace File System Acces..." | Re-trigger Greptile

Comment on lines +41 to +44
} catch (error) {
logger.error('OPFS initialization failed', error)
}
})().catch((error) => {
logger.error('Gate initialization failed', error)
if (!cancelled) setStatus({ kind: 'pick' })
})
return () => {
cancelled = true
}
}, [activate])

// Permission-lost mid-session → flip to reconnect.
useEffect(() => {
const unsubscribe = onPermissionLost(() => {
void (async () => {
const record = await getWorkspaceHandleRecord()
setStatus({ kind: 'reconnect', handleName: record?.name ?? 'workspace' })
})()
})
return () => {
unsubscribe()
}
setReady(true)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 setReady(true) is called unconditionally even when the outer catch fires. If navigator.storage.getDirectory() rejects (private-browsing mode in some browsers, or permissions denied), setWorkspaceRoot is never called, yet the gate immediately unblocks protected routes — every subsequent OPFS read/write will fail without any error UI shown to the user.

Suggested change
} catch (error) {
logger.error('OPFS initialization failed', error)
}
})().catch((error) => {
logger.error('Gate initialization failed', error)
if (!cancelled) setStatus({ kind: 'pick' })
})
return () => {
cancelled = true
}
}, [activate])
// Permission-lost mid-session → flip to reconnect.
useEffect(() => {
const unsubscribe = onPermissionLost(() => {
void (async () => {
const record = await getWorkspaceHandleRecord()
setStatus({ kind: 'reconnect', handleName: record?.name ?? 'workspace' })
})()
})
return () => {
unsubscribe()
}
setReady(true)
} catch (error) {
logger.error('OPFS initialization failed', error)
return
}
setReady(true)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/workspace-gate/workspace-gate.tsx
Line: 41-44

Comment:
`setReady(true)` is called unconditionally even when the outer `catch` fires. If `navigator.storage.getDirectory()` rejects (private-browsing mode in some browsers, or permissions denied), `setWorkspaceRoot` is never called, yet the gate immediately unblocks protected routes — every subsequent OPFS read/write will fail without any error UI shown to the user.

```suggestion
      } catch (error) {
        logger.error('OPFS initialization failed', error)
        return
      }
      setReady(true)
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +19 to +22
input.addEventListener('change', () => done(Array.from(input.files ?? [])))
input.addEventListener('cancel', () => done([]))

input.click()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 The cancel event on <input type="file"> is only supported from Safari 17.4+. On older Safari (and any browser that never fires cancel), dismissing the file picker without selecting a file leaves the returned Promise permanently unresolved — the caller in importMedia hangs forever and the UI never recovers. A common fix is to register a one-shot focus listener on window as a fallback cancellation signal.

Suggested change
input.addEventListener('change', () => done(Array.from(input.files ?? [])))
input.addEventListener('cancel', () => done([]))
input.click()
input.addEventListener('change', () => done(Array.from(input.files ?? [])))
input.addEventListener('cancel', () => done([]))
// Fallback for browsers that don't fire 'cancel' (Safari < 17.4):
// window regains focus after the dialog closes without a selection.
const onFocus = () => {
// Give the 'change' event a tick to arrive first.
setTimeout(() => done([]), 300)
}
window.addEventListener('focus', onFocus, { once: true })
input.click()
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/media-library/utils/media-file-picker.ts
Line: 19-22

Comment:
The `cancel` event on `<input type="file">` is only supported from Safari 17.4+. On older Safari (and any browser that never fires `cancel`), dismissing the file picker without selecting a file leaves the returned Promise permanently unresolved — the caller in `importMedia` hangs forever and the UI never recovers. A common fix is to register a one-shot `focus` listener on `window` as a fallback cancellation signal.

```suggestion
    input.addEventListener('change', () => done(Array.from(input.files ?? [])))
    input.addEventListener('cancel', () => done([]))

    // Fallback for browsers that don't fire 'cancel' (Safari < 17.4):
    // window regains focus after the dialog closes without a selection.
    const onFocus = () => {
      // Give the 'change' event a tick to arrive first.
      setTimeout(() => done([]), 300)
    }
    window.addEventListener('focus', onFocus, { once: true })

    input.click()
```

How can I resolve this? If you propose a fix, please make it concise.

failedCount++
removeImportPlaceholder(set, importTask.tempId)
logger.error(`Failed to import ${importTask.file.name}`, result.reason)
logger.error(`Failed to import ${importTask.file.name}`, importResults[index])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The second argument passed to logger.error is now importResults[index] — the entire PromiseSettledResult object ({ status: 'rejected', reason: Error }) — rather than the underlying error. The test reflects this by relaxing the assertion from expect.any(Error) to expect.anything(). Log consumers and error-tracking integrations that expect an Error instance here will receive a plain object instead.

Suggested change
logger.error(`Failed to import ${importTask.file.name}`, importResults[index])
logger.error(`Failed to import ${importTask.file.name}`, result.reason)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/media-library/stores/media-import-actions.ts
Line: 132

Comment:
The second argument passed to `logger.error` is now `importResults[index]` — the entire `PromiseSettledResult` object (`{ status: 'rejected', reason: Error }`) — rather than the underlying error. The test reflects this by relaxing the assertion from `expect.any(Error)` to `expect.anything()`. Log consumers and error-tracking integrations that expect an `Error` instance here will receive a plain object instead.

```suggestion
      logger.error(`Failed to import ${importTask.file.name}`, result.reason)
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/features/media-library/utils/media-file-picker.ts`:
- Around line 30-31: Wrap the unguarded call to window.showOpenFilePicker with a
feature-detection check and fallback to the existing controlled error/relink
flow: before calling window.showOpenFilePicker (the expression currently
returned), check if typeof window.showOpenFilePicker === "function"; if true,
call and return it as before, otherwise invoke the fallback path used by this
module (e.g., return a rejected Promise or call the module's relink/error
handler) so unsupported browsers (Safari/Firefox) don't throw a TypeError;
update the code around the existing return window.showOpenFilePicker(...)
expression to implement this guard.

In `@src/features/workspace-gate/workspace-gate.tsx`:
- Around line 28-46: The effect currently sets setReady(true) even when
navigator.storage.getDirectory() fails, allowing protected routes to render
without a workspace; modify the async IIFE so that setReady(true) is only called
on successful OPFS initialization (after setWorkspaceRoot and
bootstrapWorkspace), and on catch of getDirectory() set an error state (e.g.,
setOpfsError or reuse setReady(false)) and either render an error/redirect
instead of children; update references in this function
(navigator.storage.getDirectory, setWorkspaceRoot, bootstrapWorkspace,
autoPurgeExpiredTrash, setReady, logger) to ensure errors are logged via
logger.error and the component does not set ready when OPFS initialization
fails.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 98b3dcb0-8dd7-497a-b08e-1a08c79be17c

📥 Commits

Reviewing files that changed from the base of the PR and between eaf4160 and 2aaf2cd.

📒 Files selected for processing (17)
  • src/features/media-library/components/media-grid.tsx
  • src/features/media-library/components/media-library.tsx
  • src/features/media-library/components/missing-media-dialog.tsx
  • src/features/media-library/contracts/timeline.ts
  • src/features/media-library/services/media-library-service.ts
  • src/features/media-library/stores/media-import-actions.test.ts
  • src/features/media-library/stores/media-import-actions.ts
  • src/features/media-library/types.ts
  • src/features/media-library/utils/file-drop.ts
  • src/features/media-library/utils/media-file-picker.test.ts
  • src/features/media-library/utils/media-file-picker.ts
  • src/features/preview/deps/media-library-contract.ts
  • src/features/preview/hooks/use-canvas-media-drop.ts
  • src/features/timeline/deps/media-library-resolver.ts
  • src/features/timeline/utils/drop-execution.ts
  • src/features/timeline/utils/external-file-project-match.test.ts
  • src/features/workspace-gate/workspace-gate.tsx
💤 Files with no reviewable changes (4)
  • src/features/media-library/contracts/timeline.ts
  • src/features/timeline/deps/media-library-resolver.ts
  • src/features/timeline/utils/external-file-project-match.test.ts
  • src/features/preview/deps/media-library-contract.ts

Comment on lines 30 to +31
return window.showOpenFilePicker({
multiple: options?.multiple ?? true,
types: MEDIA_FILE_PICKER_TYPES,
multiple: options?.multiple ?? false,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Which browsers currently support window.showOpenFilePicker (File System Access API), and what is the recommended fallback behavior when unsupported?

💡 Result:

Currently, window.showOpenFilePicker (File System Access API “picker” for opening local files) is supported in Chromium-based desktop browsers: Chrome and Microsoft Edge support it from version 86 onward, and Opera supports it from version 72 onward (per Can I use’s extracted MDN compatibility data) [1]. In contrast, Firefox and Safari do not support showOpenFilePicker (and the wider File System Access “picker” methods) [2]. Recommended fallback when unsupported: use a classic file-picking/upload fallback such as (and/or an download flow for saving). The browser-fs-access project describes this as a “transparent fallback to the and legacy methods,” and it feature-detects support for the File System Access API and otherwise uses the fallback implementation [3]. Practical guidance: detect support before calling, e.g. check whether 'showOpenFilePicker' exists in self, and if not, use the fallback file input approach [4][3].

Citations:


Guard showOpenFilePicker behind feature detection to avoid TypeError in unsupported browsers.

window.showOpenFilePicker is supported only in Chromium-based desktop browsers (e.g., Chrome/Edge/Opera); Firefox and Safari don’t implement it, so the current unguarded call can throw a runtime TypeError instead of a controlled relink/error path.

Suggested fix
 export async function showFileHandlePicker(options?: {
   multiple?: boolean
 }): Promise<FileSystemFileHandle[]> {
+  if (typeof window === 'undefined' || typeof window.showOpenFilePicker !== 'function') {
+    throw new Error('File relinking is not supported in this browser.')
+  }
   return window.showOpenFilePicker({
     multiple: options?.multiple ?? false,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return window.showOpenFilePicker({
multiple: options?.multiple ?? true,
types: MEDIA_FILE_PICKER_TYPES,
multiple: options?.multiple ?? false,
export async function showFileHandlePicker(options?: {
multiple?: boolean
}): Promise<FileSystemFileHandle[]> {
if (typeof window === 'undefined' || typeof window.showOpenFilePicker !== 'function') {
throw new Error('File relinking is not supported in this browser.')
}
return window.showOpenFilePicker({
multiple: options?.multiple ?? false,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/media-library/utils/media-file-picker.ts` around lines 30 - 31,
Wrap the unguarded call to window.showOpenFilePicker with a feature-detection
check and fallback to the existing controlled error/relink flow: before calling
window.showOpenFilePicker (the expression currently returned), check if typeof
window.showOpenFilePicker === "function"; if true, call and return it as before,
otherwise invoke the fallback path used by this module (e.g., return a rejected
Promise or call the module's relink/error handler) so unsupported browsers
(Safari/Firefox) don't throw a TypeError; update the code around the existing
return window.showOpenFilePicker(...) expression to implement this guard.

Comment on lines 28 to 46
useEffect(() => {
let cancelled = false
;(async () => {
if (!isFileSystemAccessSupported()) {
if (!cancelled) setStatus({ kind: 'unavailable' })
return
}
// Promote any legacy `workspace:current` into a proper known-workspace
// record before we read it, so the indicator's "known workspaces" list
// includes the one the user is about to use.
await ensureKnownWorkspaceForCurrent()
const record = await getWorkspaceHandleRecord()
if (!record) {
if (!cancelled) setStatus({ kind: 'pick' })
return
}
const handle = record.handle as FileSystemDirectoryHandle
const permission = await queryHandlePermission(handle)
if (cancelled) return
if (permission === 'granted') {
await activate(handle)
} else {
setStatus({ kind: 'reconnect', handleName: record.name })
try {
const handle = await navigator.storage.getDirectory()
setWorkspaceRoot(handle)
try {
await bootstrapWorkspace(handle)
} catch (error) {
logger.warn('bootstrapWorkspace failed', error)
}
setTimeout(() => {
void autoPurgeExpiredTrash()
}, 0)
} catch (error) {
logger.error('OPFS initialization failed', error)
}
})().catch((error) => {
logger.error('Gate initialization failed', error)
if (!cancelled) setStatus({ kind: 'pick' })
})
return () => {
cancelled = true
}
}, [activate])

// Permission-lost mid-session → flip to reconnect.
useEffect(() => {
const unsubscribe = onPermissionLost(() => {
void (async () => {
const record = await getWorkspaceHandleRecord()
setStatus({ kind: 'reconnect', handleName: record?.name ?? 'workspace' })
})()
})
return () => {
unsubscribe()
}
setReady(true)
})()
}, [])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

OPFS failure still allows protected routes to render without a valid workspace.

If navigator.storage.getDirectory() throws (private browsing mode, unsupported browser, storage quota exceeded), the error is logged but setReady(true) still executes on line 44. Protected routes will render children without setWorkspaceRoot ever being called, causing cascading failures when storage-dependent code runs.

Consider keeping ready false on OPFS failure and rendering an error state, or redirecting to a safe route:

Proposed fix with error state
 export function WorkspaceGate({ children }: { children: React.ReactNode }) {
   const [ready, setReady] = useState(false)
+  const [initError, setInitError] = useState<Error | null>(null)
   const pathname = usePathname()
   const needsWorkspace = isStorageProtectedPath(pathname)

   useEffect(() => {
     ;(async () => {
       try {
         const handle = await navigator.storage.getDirectory()
         setWorkspaceRoot(handle)
         try {
           await bootstrapWorkspace(handle)
         } catch (error) {
           logger.warn('bootstrapWorkspace failed', error)
         }
         setTimeout(() => {
           void autoPurgeExpiredTrash()
         }, 0)
+        setReady(true)
       } catch (error) {
         logger.error('OPFS initialization failed', error)
+        setInitError(error instanceof Error ? error : new Error(String(error)))
       }
-      setReady(true)
     })()
   }, [])

   if (!needsWorkspace) return <>{children}</>
+  if (initError) {
+    return (
+      <div className="min-h-screen bg-background flex items-center justify-center">
+        <p>Storage initialization failed. Please try refreshing or use a supported browser.</p>
+      </div>
+    )
+  }
   if (!ready) return <div className="min-h-screen bg-background" aria-hidden="true" />
   return <>{children}</>
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
let cancelled = false
;(async () => {
if (!isFileSystemAccessSupported()) {
if (!cancelled) setStatus({ kind: 'unavailable' })
return
}
// Promote any legacy `workspace:current` into a proper known-workspace
// record before we read it, so the indicator's "known workspaces" list
// includes the one the user is about to use.
await ensureKnownWorkspaceForCurrent()
const record = await getWorkspaceHandleRecord()
if (!record) {
if (!cancelled) setStatus({ kind: 'pick' })
return
}
const handle = record.handle as FileSystemDirectoryHandle
const permission = await queryHandlePermission(handle)
if (cancelled) return
if (permission === 'granted') {
await activate(handle)
} else {
setStatus({ kind: 'reconnect', handleName: record.name })
try {
const handle = await navigator.storage.getDirectory()
setWorkspaceRoot(handle)
try {
await bootstrapWorkspace(handle)
} catch (error) {
logger.warn('bootstrapWorkspace failed', error)
}
setTimeout(() => {
void autoPurgeExpiredTrash()
}, 0)
} catch (error) {
logger.error('OPFS initialization failed', error)
}
})().catch((error) => {
logger.error('Gate initialization failed', error)
if (!cancelled) setStatus({ kind: 'pick' })
})
return () => {
cancelled = true
}
}, [activate])
// Permission-lost mid-session → flip to reconnect.
useEffect(() => {
const unsubscribe = onPermissionLost(() => {
void (async () => {
const record = await getWorkspaceHandleRecord()
setStatus({ kind: 'reconnect', handleName: record?.name ?? 'workspace' })
})()
})
return () => {
unsubscribe()
}
setReady(true)
})()
}, [])
export function WorkspaceGate({ children }: { children: React.ReactNode }) {
const [ready, setReady] = useState(false)
const [initError, setInitError] = useState<Error | null>(null)
const pathname = usePathname()
const needsWorkspace = isStorageProtectedPath(pathname)
useEffect(() => {
;(async () => {
try {
const handle = await navigator.storage.getDirectory()
setWorkspaceRoot(handle)
try {
await bootstrapWorkspace(handle)
} catch (error) {
logger.warn('bootstrapWorkspace failed', error)
}
setTimeout(() => {
void autoPurgeExpiredTrash()
}, 0)
setReady(true)
} catch (error) {
logger.error('OPFS initialization failed', error)
setInitError(error instanceof Error ? error : new Error(String(error)))
}
})()
}, [])
if (!needsWorkspace) return <>{children}</>
if (initError) {
return (
<div className="min-h-screen bg-background flex items-center justify-center">
<p>Storage initialization failed. Please try refreshing or use a supported browser.</p>
</div>
)
}
if (!ready) return <div className="min-h-screen bg-background" aria-hidden="true" />
return <>{children}</>
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/workspace-gate/workspace-gate.tsx` around lines 28 - 46, The
effect currently sets setReady(true) even when navigator.storage.getDirectory()
fails, allowing protected routes to render without a workspace; modify the async
IIFE so that setReady(true) is only called on successful OPFS initialization
(after setWorkspaceRoot and bootstrapWorkspace), and on catch of getDirectory()
set an error state (e.g., setOpfsError or reuse setReady(false)) and either
render an error/redirect instead of children; update references in this function
(navigator.storage.getDirectory, setWorkspaceRoot, bootstrapWorkspace,
autoPurgeExpiredTrash, setReady, logger) to ensure errors are logged via
logger.error and the component does not set ready when OPFS initialization
fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant