feat(media-library): expanded MIME types and auto-import from watched directory#130
Open
shimondoodkin wants to merge 2 commits into
Open
feat(media-library): expanded MIME types and auto-import from watched directory#130shimondoodkin wants to merge 2 commits into
shimondoodkin wants to merge 2 commits into
Conversation
Extend supported media types to include video/matroska, video/x-msvideo, audio/x-m4a, audio/mp4, and image/svg+xml. Improve MIME detection to prefer extension-based lookup when browser-reported type is generic.
Add directory watching via File System Access API that polls for new media files and automatically imports them to the library and timeline. Includes stability detection to wait for files still being written.
|
@shimondoodkin is attempting to deploy a commit to the walterlow's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
📝 WalkthroughWalkthroughAdds auto-import functionality to the media library that monitors a selected directory for new media files, detects when files reach stable sizes, and automatically imports them into the timeline while expanding supported file formats to include additional video, audio, and image types. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Media Library UI
participant Store as Auto-Import Store
participant DirectoryAPI as Directory Picker API
participant PointerLoop as Polling Loop
participant MediaLib as Media Library Store
participant TimelineStore as Timeline Store
User->>UI: Click "Enable Auto Import"
UI->>Store: enable()
Store->>DirectoryAPI: showDirectoryPicker()
DirectoryAPI-->>Store: Directory handle
Store->>Store: Snapshot existing files into knownFiles
Store->>PointerLoop: Start setInterval polling
loop Every poll interval
PointerLoop->>Store: Scan directory for new files
Store->>Store: Check permission & supported extensions
Store->>Store: Track file size in pendingFiles map
alt File size stable (non-zero & unchanged)
Store->>MediaLib: Import media file
MediaLib->>MediaLib: Resolve blob URL & thumbnails
Store->>TimelineStore: Find first visible unlocked track
Store->>TimelineStore: Calculate duration in frames
Store->>TimelineStore: Find track end frame
Store->>TimelineStore: Build & append timeline item
Store->>Store: Update knownFiles, emit notification
else File size changing
Store->>Store: Wait for next poll
end
end
User->>UI: Click "Disable Auto Import"
UI->>Store: disable()
Store->>PointerLoop: Clear polling interval
Store->>Store: Emit disable notification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 Tip You can disable poems in the walkthrough.Disable the |
Contributor
Greptile SummaryThis PR extends the media library with two independent improvements: MIME type support for The MIME type additions and the UI wiring are clean and low-risk. The auto-import store logic, however, has two concurrency bugs that can cause duplicate clips to appear on the timeline:
Both issues share the same root fix: serialise poll execution with an Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as MediaLibrary UI
participant Store as AutoImportStore
participant Poll as pollForNewFiles (interval)
participant FS as File System API
participant ML as MediaLibraryStore
participant TL as ItemsStore (Timeline)
UI->>Store: enable()
Store->>FS: showDirectoryPicker()
FS-->>Store: directoryHandle
Store->>FS: dirHandle.values() (snapshot existing files)
Store->>Store: set({ active, knownFiles, timerId })
loop Every 3s (setInterval)
Poll->>FS: queryPermission()
FS-->>Poll: "granted"
Poll->>FS: dirHandle.values() (scan for new files)
FS-->>Poll: entries
Poll->>Poll: add new entries to pendingFiles (module-level Map)
loop For each pending file
Poll->>FS: getFileHandle(name) + getFile()
FS-->>Poll: current size
alt size unchanged & > 0
Poll->>Poll: move to stableHandles
Poll->>Poll: remove from pendingFiles
else still growing
Poll->>Poll: update pendingFiles size
end
end
Poll->>Store: setState({ knownFiles: currentKnown })
Poll->>ML: importHandles(stableHandles)
ML-->>Poll: importedMedia[]
loop For each imported media
Poll->>TL: getState().items (read end frame)
Poll->>ML: resolveMediaUrl / getThumbnailBlobUrl
Poll->>TL: addItem(timelineItem)
end
end
UI->>Store: disable()
Store->>Store: clearInterval + pendingFiles.clear() + reset state
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/features/media-library/stores/auto-import-store.ts
Line: 163-196
Comment:
**Stale `knownFiles` snapshot causes lost state on concurrent polls**
`currentKnown` is built from `state.knownFiles` — a snapshot captured at the very top of the function (line 127). Between that snapshot and line 163 several `await` points occur (permission check, full directory scan, per-file `getFile()` calls). If another concurrent poll (which can start after 3 seconds while this one is still running) completes its own stable-file cycle and calls `useAutoImportStore.setState({ knownFiles: ... })` during that window, its additions will be wiped when this poll subsequently calls `setState` with `currentKnown` derived from the stale snapshot.
This means a file that the second poll just marked as "known" (to avoid re-import) will disappear from store state, and the next poll will attempt to import it again — placing a duplicate clip on the timeline.
Fix: re-read the _current_ store state at line 163 instead of using the stale `state` snapshot:
```suggestion
const stableHandles: FileSystemFileHandle[] = [];
const currentKnown = new Set(useAutoImportStore.getState().knownFiles);
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/features/media-library/stores/auto-import-store.ts
Line: 81
Comment:
**No concurrency guard — overlapping polls can cause duplicate imports**
`setInterval` fires `pollForNewFiles` every 3 seconds regardless of whether the previous invocation has completed. A single import of a large video file can easily exceed 3 seconds, causing two (or more) polls to run simultaneously. When concurrent polls both observe the same stable file in `pendingFiles` before either one has had a chance to delete it, both polls can each:
1. Call `pendingFiles.delete(fileName)` — only one will "win" the deletion, but the iteration snapshot for the other may already hold the entry.
2. Both call `addItem(timelineItem)` — resulting in a duplicate clip on the timeline.
Add a simple boolean guard to serialise poll invocations:
```
let pollRunning = false;
async function pollForNewFiles(): Promise<void> {
if (pollRunning) return;
pollRunning = true;
try {
// ... existing body ...
} finally {
pollRunning = false;
}
}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/features/media-library/stores/auto-import-store.ts
Line: 36
Comment:
**Module-level mutable state lives outside the store**
`pendingFiles` is declared at module scope, making it invisible to Zustand devtools, not serialisable/rehydratable, and — most practically — not cleared on a hot-module-reload (HMR) cycle. If the module is re-evaluated, `pendingFiles` is reset but the Zustand store may still be active, silently leaking stale file-size entries. There's also a subtle edge case: if `disable()` is called while `pollForNewFiles` is executing between two `await` points, the `pendingFiles.clear()` on line 113 runs, but the already-captured `for...of` iterator over `pendingFiles` (line 165) will stop iterating mid-loop because Map iterators reflect live deletions — meaning some stable files may never get imported even though they were detected.
Consider moving `pendingFiles` into the Zustand state object (as a `Map<string, number>`), so its lifecycle is fully controlled by `enable`/`disable`.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/features/media-library/utils/validation.ts
Line: 65-74
Comment:
**Extension-first detection silently accepts misnamed files**
The updated `getMimeType` now returns the extension-mapped MIME type even when the browser has already reported a valid, different type in `file.type`. This means a file named `not-a-video.mp4` (e.g. a renamed PDF) will pass MIME validation as `video/mp4` because the extension wins over `file.type`. The previous behaviour — trust `file.type` first, fall back to extension only when the browser provides nothing — is generally safer.
The stated goal (normalising `video/matroska` → `video/x-matroska` for `.mkv` files) could be achieved more narrowly by only applying extension override when the browser-reported type would otherwise fail validation:
```ts
export function getMimeType(file: File): string {
const ext = file.name.toLowerCase().match(/\.[^.]+$/)?.[0];
const extMime = ext ? EXTENSION_TO_MIME[ext] : undefined;
// Normalise non-standard browser MIME types (e.g. video/matroska -> video/x-matroska)
if (file.type && extMime && file.type !== extMime) {
const allSupported = [...SUPPORTED_VIDEO_TYPES, ...SUPPORTED_AUDIO_TYPES, ...SUPPORTED_IMAGE_TYPES];
if (!allSupported.includes(file.type)) return extMime;
}
return file.type || extMime || '';
}
```
Note: the existing security comment on line 98 already acknowledges client-side MIME spoofing, so this is a best-effort concern, not a blocking one.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "feat(media-library):..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+163
to
+196
| const currentKnown = new Set(state.knownFiles); | ||
|
|
||
| for (const [fileName, lastSize] of pendingFiles) { | ||
| try { | ||
| const handle = await state.directoryHandle.getFileHandle(fileName); | ||
| const file = await handle.getFile(); | ||
| const currentSize = file.size; | ||
|
|
||
| if (currentSize === 0) { | ||
| // Still empty, keep waiting | ||
| continue; | ||
| } | ||
|
|
||
| if (currentSize === lastSize) { | ||
| // Size unchanged and > 0 — file is done being written | ||
| stableHandles.push(handle); | ||
| currentKnown.add(fileName); | ||
| pendingFiles.delete(fileName); | ||
| logger.info(`Auto-import: "${fileName}" stable at ${currentSize} bytes, importing`); | ||
| } else { | ||
| // Still growing, update the tracked size for next poll | ||
| pendingFiles.set(fileName, currentSize); | ||
| logger.info(`Auto-import: "${fileName}" still writing (${lastSize} -> ${currentSize} bytes)`); | ||
| } | ||
| } catch { | ||
| // File may have been deleted before we could read it | ||
| pendingFiles.delete(fileName); | ||
| } | ||
| } | ||
|
|
||
| if (stableHandles.length === 0) return; | ||
|
|
||
| // Update known files immediately to avoid re-processing | ||
| useAutoImportStore.setState({ knownFiles: currentKnown }); |
Contributor
There was a problem hiding this comment.
knownFiles snapshot causes lost state on concurrent polls
currentKnown is built from state.knownFiles — a snapshot captured at the very top of the function (line 127). Between that snapshot and line 163 several await points occur (permission check, full directory scan, per-file getFile() calls). If another concurrent poll (which can start after 3 seconds while this one is still running) completes its own stable-file cycle and calls useAutoImportStore.setState({ knownFiles: ... }) during that window, its additions will be wiped when this poll subsequently calls setState with currentKnown derived from the stale snapshot.
This means a file that the second poll just marked as "known" (to avoid re-import) will disappear from store state, and the next poll will attempt to import it again — placing a duplicate clip on the timeline.
Fix: re-read the current store state at line 163 instead of using the stale state snapshot:
Suggested change
| const currentKnown = new Set(state.knownFiles); | |
| for (const [fileName, lastSize] of pendingFiles) { | |
| try { | |
| const handle = await state.directoryHandle.getFileHandle(fileName); | |
| const file = await handle.getFile(); | |
| const currentSize = file.size; | |
| if (currentSize === 0) { | |
| // Still empty, keep waiting | |
| continue; | |
| } | |
| if (currentSize === lastSize) { | |
| // Size unchanged and > 0 — file is done being written | |
| stableHandles.push(handle); | |
| currentKnown.add(fileName); | |
| pendingFiles.delete(fileName); | |
| logger.info(`Auto-import: "${fileName}" stable at ${currentSize} bytes, importing`); | |
| } else { | |
| // Still growing, update the tracked size for next poll | |
| pendingFiles.set(fileName, currentSize); | |
| logger.info(`Auto-import: "${fileName}" still writing (${lastSize} -> ${currentSize} bytes)`); | |
| } | |
| } catch { | |
| // File may have been deleted before we could read it | |
| pendingFiles.delete(fileName); | |
| } | |
| } | |
| if (stableHandles.length === 0) return; | |
| // Update known files immediately to avoid re-processing | |
| useAutoImportStore.setState({ knownFiles: currentKnown }); | |
| const stableHandles: FileSystemFileHandle[] = []; | |
| const currentKnown = new Set(useAutoImportStore.getState().knownFiles); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/media-library/stores/auto-import-store.ts
Line: 163-196
Comment:
**Stale `knownFiles` snapshot causes lost state on concurrent polls**
`currentKnown` is built from `state.knownFiles` — a snapshot captured at the very top of the function (line 127). Between that snapshot and line 163 several `await` points occur (permission check, full directory scan, per-file `getFile()` calls). If another concurrent poll (which can start after 3 seconds while this one is still running) completes its own stable-file cycle and calls `useAutoImportStore.setState({ knownFiles: ... })` during that window, its additions will be wiped when this poll subsequently calls `setState` with `currentKnown` derived from the stale snapshot.
This means a file that the second poll just marked as "known" (to avoid re-import) will disappear from store state, and the next poll will attempt to import it again — placing a duplicate clip on the timeline.
Fix: re-read the _current_ store state at line 163 instead of using the stale `state` snapshot:
```suggestion
const stableHandles: FileSystemFileHandle[] = [];
const currentKnown = new Set(useAutoImportStore.getState().knownFiles);
```
How can I resolve this? If you propose a fix, please make it concise.| } | ||
| } | ||
|
|
||
| const timerId = setInterval(() => pollForNewFiles(), POLL_INTERVAL_MS); |
Contributor
There was a problem hiding this comment.
setInterval fires pollForNewFiles every 3 seconds regardless of whether the previous invocation has completed. A single import of a large video file can easily exceed 3 seconds, causing two (or more) polls to run simultaneously. When concurrent polls both observe the same stable file in pendingFiles before either one has had a chance to delete it, both polls can each:
- Call
pendingFiles.delete(fileName)— only one will "win" the deletion, but the iteration snapshot for the other may already hold the entry. - Both call
addItem(timelineItem)— resulting in a duplicate clip on the timeline.
Add a simple boolean guard to serialise poll invocations:
let pollRunning = false;
async function pollForNewFiles(): Promise<void> {
if (pollRunning) return;
pollRunning = true;
try {
// ... existing body ...
} finally {
pollRunning = false;
}
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/media-library/stores/auto-import-store.ts
Line: 81
Comment:
**No concurrency guard — overlapping polls can cause duplicate imports**
`setInterval` fires `pollForNewFiles` every 3 seconds regardless of whether the previous invocation has completed. A single import of a large video file can easily exceed 3 seconds, causing two (or more) polls to run simultaneously. When concurrent polls both observe the same stable file in `pendingFiles` before either one has had a chance to delete it, both polls can each:
1. Call `pendingFiles.delete(fileName)` — only one will "win" the deletion, but the iteration snapshot for the other may already hold the entry.
2. Both call `addItem(timelineItem)` — resulting in a duplicate clip on the timeline.
Add a simple boolean guard to serialise poll invocations:
```
let pollRunning = false;
async function pollForNewFiles(): Promise<void> {
if (pollRunning) return;
pollRunning = true;
try {
// ... existing body ...
} finally {
pollRunning = false;
}
}
```
How can I resolve this? If you propose a fix, please make it concise.| * Maps filename -> last observed file size. A file is considered stable when its | ||
| * size is >0 and unchanged between two consecutive polls. | ||
| */ | ||
| const pendingFiles = new Map<string, number>(); |
Contributor
There was a problem hiding this comment.
pendingFiles is declared at module scope, making it invisible to Zustand devtools, not serialisable/rehydratable, and — most practically — not cleared on a hot-module-reload (HMR) cycle. If the module is re-evaluated, pendingFiles is reset but the Zustand store may still be active, silently leaking stale file-size entries. There's also a subtle edge case: if disable() is called while pollForNewFiles is executing between two await points, the pendingFiles.clear() on line 113 runs, but the already-captured for...of iterator over pendingFiles (line 165) will stop iterating mid-loop because Map iterators reflect live deletions — meaning some stable files may never get imported even though they were detected.
Consider moving pendingFiles into the Zustand state object (as a Map<string, number>), so its lifecycle is fully controlled by enable/disable.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/media-library/stores/auto-import-store.ts
Line: 36
Comment:
**Module-level mutable state lives outside the store**
`pendingFiles` is declared at module scope, making it invisible to Zustand devtools, not serialisable/rehydratable, and — most practically — not cleared on a hot-module-reload (HMR) cycle. If the module is re-evaluated, `pendingFiles` is reset but the Zustand store may still be active, silently leaking stale file-size entries. There's also a subtle edge case: if `disable()` is called while `pollForNewFiles` is executing between two `await` points, the `pendingFiles.clear()` on line 113 runs, but the already-captured `for...of` iterator over `pendingFiles` (line 165) will stop iterating mid-loop because Map iterators reflect live deletions — meaning some stable files may never get imported even though they were detected.
Consider moving `pendingFiles` into the Zustand state object (as a `Map<string, number>`), so its lifecycle is fully controlled by `enable`/`disable`.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
65
to
74
| export function getMimeType(file: File): string { | ||
| if (file.type) { | ||
| return file.type; | ||
| } | ||
| // Fallback to extension-based detection | ||
| // Prefer extension-based detection for known extensions — | ||
| // browsers sometimes report non-standard MIME types (e.g. "video/matroska" | ||
| // instead of "video/x-matroska" for .mkv files). | ||
| const ext = file.name.toLowerCase().match(/\.[^.]+$/)?.[0]; | ||
| return ext ? EXTENSION_TO_MIME[ext] || '' : ''; | ||
| if (ext && EXTENSION_TO_MIME[ext]) { | ||
| return EXTENSION_TO_MIME[ext]; | ||
| } | ||
| return file.type || ''; | ||
| } |
Contributor
There was a problem hiding this comment.
The updated getMimeType now returns the extension-mapped MIME type even when the browser has already reported a valid, different type in file.type. This means a file named not-a-video.mp4 (e.g. a renamed PDF) will pass MIME validation as video/mp4 because the extension wins over file.type. The previous behaviour — trust file.type first, fall back to extension only when the browser provides nothing — is generally safer.
The stated goal (normalising video/matroska → video/x-matroska for .mkv files) could be achieved more narrowly by only applying extension override when the browser-reported type would otherwise fail validation:
export function getMimeType(file: File): string {
const ext = file.name.toLowerCase().match(/\.[^.]+$/)?.[0];
const extMime = ext ? EXTENSION_TO_MIME[ext] : undefined;
// Normalise non-standard browser MIME types (e.g. video/matroska -> video/x-matroska)
if (file.type && extMime && file.type !== extMime) {
const allSupported = [...SUPPORTED_VIDEO_TYPES, ...SUPPORTED_AUDIO_TYPES, ...SUPPORTED_IMAGE_TYPES];
if (!allSupported.includes(file.type)) return extMime;
}
return file.type || extMime || '';
}Note: the existing security comment on line 98 already acknowledges client-side MIME spoofing, so this is a best-effort concern, not a blocking one.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/media-library/utils/validation.ts
Line: 65-74
Comment:
**Extension-first detection silently accepts misnamed files**
The updated `getMimeType` now returns the extension-mapped MIME type even when the browser has already reported a valid, different type in `file.type`. This means a file named `not-a-video.mp4` (e.g. a renamed PDF) will pass MIME validation as `video/mp4` because the extension wins over `file.type`. The previous behaviour — trust `file.type` first, fall back to extension only when the browser provides nothing — is generally safer.
The stated goal (normalising `video/matroska` → `video/x-matroska` for `.mkv` files) could be achieved more narrowly by only applying extension override when the browser-reported type would otherwise fail validation:
```ts
export function getMimeType(file: File): string {
const ext = file.name.toLowerCase().match(/\.[^.]+$/)?.[0];
const extMime = ext ? EXTENSION_TO_MIME[ext] : undefined;
// Normalise non-standard browser MIME types (e.g. video/matroska -> video/x-matroska)
if (file.type && extMime && file.type !== extMime) {
const allSupported = [...SUPPORTED_VIDEO_TYPES, ...SUPPORTED_AUDIO_TYPES, ...SUPPORTED_IMAGE_TYPES];
if (!allSupported.includes(file.type)) return extMime;
}
return file.type || extMime || '';
}
```
Note: the existing security comment on line 98 already acknowledges client-side MIME spoofing, so this is a best-effort concern, not a blocking one.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/media-library/stores/auto-import-store.ts`:
- Around line 22-23: The audio extension set AUDIO_EXTENSIONS currently omits
".opus" so watched-folder auto-import never detects Opus files; update the
AUDIO_EXTENSIONS constant to include '.opus' (and any other missing audio
extensions) or, preferably, import the canonical list of supported audio file
extensions from the shared validation constant used elsewhere to avoid drift;
apply the same sourcing approach for image/audio extension lists referenced near
IMAGE_EXTENSIONS and the related sets on lines 25-29 so all extension checks use
the single shared source of truth.
- Around line 81-82: The interval starts async work via pollForNewFiles() which
can re-enter if a tick fires while a previous poll is still running, causing
races on pendingFiles/knownFiles and duplicate processing; fix by making
pollForNewFiles non-reentrant: add a local running flag or mutex (e.g.,
isPolling) checked at the top of pollForNewFiles and set/unset around the async
work (or switch from setInterval to a self-scheduling setTimeout that awaits
completion before scheduling the next run), and apply the same change to the
other polling/interval usages in this file (the other timer blocks that
reference timerId/pollForNewFiles/pendingFiles/knownFiles).
In `@src/features/media-library/utils/validation.ts`:
- Around line 66-73: The current logic in validation.ts uses EXTENSION_TO_MIME
via ext as the primary source and can override a valid file.type; change it to
prefer file.type unless it's missing or generic (e.g., empty string or
"application/octet-stream" or other known-generic values), and only then use
EXTENSION_TO_MIME[ext] as a fallback; update the return flow around ext,
EXTENSION_TO_MIME, and file.type so file.type is returned when specific,
otherwise return the mapped extension MIME or empty string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3e575f61-9473-4cb3-9703-fcedba83236d
📒 Files selected for processing (6)
src/features/media-library/components/media-library.tsxsrc/features/media-library/deps/timeline-actions.tssrc/features/media-library/deps/timeline-utils.tssrc/features/media-library/stores/auto-import-store.tssrc/features/media-library/utils/validation.tssrc/features/timeline/contracts/media-library.ts
Comment on lines
+22
to
+23
| const AUDIO_EXTENSIONS = new Set(['.mp3', '.wav', '.ogg', '.m4a', '.aac']); | ||
| const IMAGE_EXTENSIONS = new Set(['.jpg', '.jpeg', '.png', '.gif', '.webp', '.svg']); |
Contributor
There was a problem hiding this comment.
Auto-import extension filter is missing .opus.
Line 22 excludes .opus, so supported Opus files are never detected by watched-folder auto-import even though validation supports them.
💡 Suggested fix
-const AUDIO_EXTENSIONS = new Set(['.mp3', '.wav', '.ogg', '.m4a', '.aac']);
+const AUDIO_EXTENSIONS = new Set(['.mp3', '.wav', '.ogg', '.opus', '.m4a', '.aac']);Also applies to: 25-29
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/media-library/stores/auto-import-store.ts` around lines 22 - 23,
The audio extension set AUDIO_EXTENSIONS currently omits ".opus" so
watched-folder auto-import never detects Opus files; update the AUDIO_EXTENSIONS
constant to include '.opus' (and any other missing audio extensions) or,
preferably, import the canonical list of supported audio file extensions from
the shared validation constant used elsewhere to avoid drift; apply the same
sourcing approach for image/audio extension lists referenced near
IMAGE_EXTENSIONS and the related sets on lines 25-29 so all extension checks use
the single shared source of truth.
Comment on lines
+81
to
+82
| const timerId = setInterval(() => pollForNewFiles(), POLL_INTERVAL_MS); | ||
|
|
Contributor
There was a problem hiding this comment.
Prevent poll re-entrancy to avoid overlapping imports.
Line 81 starts an async poll on an interval, but interval ticks can overlap while a previous poll is still running. This can race on pendingFiles/knownFiles and duplicate processing.
💡 Suggested fix
const POLL_INTERVAL_MS = 3000;
+let pollInProgress = false;
@@
async function pollForNewFiles(): Promise<void> {
+ if (pollInProgress) return;
+ pollInProgress = true;
+
const state = useAutoImportStore.getState();
- if (!state.active || !state.directoryHandle) return;
+ if (!state.active || !state.directoryHandle) {
+ pollInProgress = false;
+ return;
+ }
@@
- try {
+ try {
@@
} catch (error) {
logger.error('Auto-import poll error:', error);
+ } finally {
+ pollInProgress = false;
}
}Also applies to: 126-272
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/media-library/stores/auto-import-store.ts` around lines 81 - 82,
The interval starts async work via pollForNewFiles() which can re-enter if a
tick fires while a previous poll is still running, causing races on
pendingFiles/knownFiles and duplicate processing; fix by making pollForNewFiles
non-reentrant: add a local running flag or mutex (e.g., isPolling) checked at
the top of pollForNewFiles and set/unset around the async work (or switch from
setInterval to a self-scheduling setTimeout that awaits completion before
scheduling the next run), and apply the same change to the other
polling/interval usages in this file (the other timer blocks that reference
timerId/pollForNewFiles/pendingFiles/knownFiles).
Comment on lines
+66
to
+73
| // Prefer extension-based detection for known extensions — | ||
| // browsers sometimes report non-standard MIME types (e.g. "video/matroska" | ||
| // instead of "video/x-matroska" for .mkv files). | ||
| const ext = file.name.toLowerCase().match(/\.[^.]+$/)?.[0]; | ||
| return ext ? EXTENSION_TO_MIME[ext] || '' : ''; | ||
| if (ext && EXTENSION_TO_MIME[ext]) { | ||
| return EXTENSION_TO_MIME[ext]; | ||
| } | ||
| return file.type || ''; |
Contributor
There was a problem hiding this comment.
Use extension mapping as fallback only (not primary).
Line 66–Line 73 currently trust extension first for all recognized extensions, which can override a valid non-generic file.type. That conflicts with the intended “fallback when generic” behavior and can misclassify files.
💡 Suggested fix
+const GENERIC_MIME_TYPES = new Set([
+ '',
+ 'application/octet-stream',
+ 'binary/octet-stream',
+]);
+
export function getMimeType(file: File): string {
- // Prefer extension-based detection for known extensions —
- // browsers sometimes report non-standard MIME types (e.g. "video/matroska"
- // instead of "video/x-matroska" for .mkv files).
+ // Use browser MIME when specific; fall back to extension for generic/empty MIME.
const ext = file.name.toLowerCase().match(/\.[^.]+$/)?.[0];
- if (ext && EXTENSION_TO_MIME[ext]) {
- return EXTENSION_TO_MIME[ext];
- }
- return file.type || '';
+ const extMime = ext ? EXTENSION_TO_MIME[ext] : undefined;
+ const reportedMime = (file.type || '').toLowerCase();
+
+ if (reportedMime && !GENERIC_MIME_TYPES.has(reportedMime)) {
+ return reportedMime;
+ }
+
+ return extMime ?? reportedMime;
}📝 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
| // Prefer extension-based detection for known extensions — | |
| // browsers sometimes report non-standard MIME types (e.g. "video/matroska" | |
| // instead of "video/x-matroska" for .mkv files). | |
| const ext = file.name.toLowerCase().match(/\.[^.]+$/)?.[0]; | |
| return ext ? EXTENSION_TO_MIME[ext] || '' : ''; | |
| if (ext && EXTENSION_TO_MIME[ext]) { | |
| return EXTENSION_TO_MIME[ext]; | |
| } | |
| return file.type || ''; | |
| const GENERIC_MIME_TYPES = new Set([ | |
| '', | |
| 'application/octet-stream', | |
| 'binary/octet-stream', | |
| ]); | |
| export function getMimeType(file: File): string { | |
| // Use browser MIME when specific; fall back to extension for generic/empty MIME. | |
| const ext = file.name.toLowerCase().match(/\.[^.]+$/)?.[0]; | |
| const extMime = ext ? EXTENSION_TO_MIME[ext] : undefined; | |
| const reportedMime = (file.type || '').toLowerCase(); | |
| if (reportedMime && !GENERIC_MIME_TYPES.has(reportedMime)) { | |
| return reportedMime; | |
| } | |
| return extMime ?? reportedMime; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/media-library/utils/validation.ts` around lines 66 - 73, The
current logic in validation.ts uses EXTENSION_TO_MIME via ext as the primary
source and can override a valid file.type; change it to prefer file.type unless
it's missing or generic (e.g., empty string or "application/octet-stream" or
other known-generic values), and only then use EXTENSION_TO_MIME[ext] as a
fallback; update the return flow around ext, EXTENSION_TO_MIME, and file.type so
file.type is returned when specific, otherwise return the mapped extension MIME
or empty string.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
Summary by CodeRabbit