Align cleanse mode recommendations with actual server format support#27
Merged
ChrisAdamsdevelopment merged 1 commit intoMay 18, 2026
Conversation
Reviewer's GuideIntroduces a shared cleanse policy module to centralize supported formats, updates backend endpoints to use it for validation and structured error responses with explicit reasons, improves error classification/logging for server processing, and aligns frontend UI recommendations and button states with the actual server-supported formats and plan constraints. Sequence diagram for POST /api/process with cleanse policy and error reasonssequenceDiagram
actor User
participant BrowserApp
participant Server
participant cleansePolicy
participant DB
participant processor
User->>BrowserApp: Click Full Server Cleanse
BrowserApp->>Server: POST /api/process (file, metadata)
Server->>DB: SELECT plan FROM users
DB-->>Server: userPlan
Server->>cleansePolicy: isServerSupportedFormat(originalName, mime)
alt [unsupported_file_type]
Server->>Server: fs.remove(inputPath)
Server-->>BrowserApp: 422 JSON { reason: unsupported_file_type, supportedServerFormats }
else [supported_format]
alt [userPlan is free and limit reached]
Server->>Server: getMonthlyJobCount(userId)
Server->>Server: fs.remove(inputPath)
Server-->>BrowserApp: 402 JSON { reason: usage_limit }
else [allowed by plan/usage]
Server->>processor: processMediaFile(outputPath, originalName, platform, metadata)
alt [exiftool_failure]
processor->>processor: exiftool.write(...)
processor-->>Server: throw exiftoolFailureError
Server-->>BrowserApp: 500 JSON { reason: exiftool_failure }
else [other processing error]
processor-->>Server: throw error (may include reason)
Server-->>BrowserApp: status JSON { reason from error or server_processing_failure }
end
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The frontend hard-codes
isServerSupportedFormatlogic (MP4/M4A check) separately fromCLEANSE_POLICY.server.supportedExtensions; consider wiring the frontend to consume the shared policy (or an API-provided capability payload) so format support can’t drift between client and server. - In
POST /api/process, the fallbackreasonof'unsupported_file_type'for any 422 without an expliciterr.reasonmay misclassify other validation failures as format issues; it may be safer to default to a generic reason or map only known error types explicitly. - Batch processing responses mix structured errors (with
reason) and plain error strings depending on the failure path; you might want to include areasonfield consistently in all batch result error objects to make downstream handling more uniform.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The frontend hard-codes `isServerSupportedFormat` logic (MP4/M4A check) separately from `CLEANSE_POLICY.server.supportedExtensions`; consider wiring the frontend to consume the shared policy (or an API-provided capability payload) so format support can’t drift between client and server.
- In `POST /api/process`, the fallback `reason` of `'unsupported_file_type'` for any 422 without an explicit `err.reason` may misclassify other validation failures as format issues; it may be safer to default to a generic reason or map only known error types explicitly.
- Batch processing responses mix structured errors (with `reason`) and plain error strings depending on the failure path; you might want to include a `reason` field consistently in all batch result error objects to make downstream handling more uniform.
## Individual Comments
### Comment 1
<location path="server.js" line_range="489-498" />
<code_context>
+ const ext = normalizeExt(originalName);
</code_context>
<issue_to_address>
**issue (bug_risk):** Normalizing the extension to empty string can lead to extension-less server output files in some cases.
With `normalizeExt` now returning `''`, a file uploaded without an extension but with a supported MIME type will still pass `isServerSupportedFormat`, but `outputPath` will append an empty `ext`. That yields output files with no extension, which is problematic for users and some tools. Please add a fallback to a safe default extension (e.g. `.mp4`) when `normalizeExt` returns an empty string but the MIME type is supported.
</issue_to_address>
### Comment 2
<location path="server.js" line_range="548-550" />
<code_context>
+ const ext = normalizeExt(file.originalname || '');
</code_context>
<issue_to_address>
**issue (bug_risk):** Batch output files may lose their default extension, unlike the previous behavior.
Previously, when `path.extname` returned an empty string in the batch path, `ext` defaulted to `.mp4`. With `normalizeExt`, `ext` is now `''` in that case, so extension-less uploads will produce output paths with no file extension, changing the prior behavior and affecting download names. Please consider a fallback extension here (as in the single-file route) when `ext` is empty but `mime` is a supported format.
</issue_to_address>
### Comment 3
<location path="server/cleansePolicy.js" line_range="24-30" />
<code_context>
+ const safeMime = String(mime || '').toLowerCase();
+ if (CLEANSE_POLICY.server.supportedExtensions.includes(ext)) return true;
+ // Common upload MIME aliases for supported server formats.
+ return safeMime === 'video/mp4' || safeMime === 'audio/mp4' || safeMime === 'audio/x-m4a';
+}
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The server MIME checks for M4A may miss common variants such as `audio/m4a`.
The guard only allows `video/mp4`, `audio/mp4`, and `audio/x-m4a`, but some clients send M4A as `audio/m4a`. To prevent valid M4A uploads from being rejected when MIME/ext don’t line up, please add `audio/m4a` (and any other expected aliases) to this list.
```suggestion
function isServerSupportedFormat(filename = '', mime = '') {
const ext = normalizeExt(filename);
const safeMime = String(mime || '').toLowerCase();
if (CLEANSE_POLICY.server.supportedExtensions.includes(ext)) return true;
// Common upload MIME aliases for supported server formats.
return (
safeMime === 'video/mp4' ||
safeMime === 'audio/mp4' ||
safeMime === 'audio/x-m4a' ||
safeMime === 'audio/m4a'
);
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+489
to
+498
| const ext = normalizeExt(originalName); | ||
| const mime = (req.file.mimetype || '').toLowerCase(); | ||
| const isMp3 = ext === '.mp3' || mime === 'audio/mpeg'; | ||
| if (isMp3) { | ||
| await fs.remove(inputPath).catch(() => {}); | ||
| return res.status(422).json({ error: 'MP3 server cleanse is not supported', detail: 'Use Quick Cleanse (Browser) for MP3. Full Server Cleanse is best supported for MP4/M4A; WAV/FLAC may be rejected if ExifTool cannot safely rewrite them.' }); | ||
| } | ||
| const dbUser = db.prepare('SELECT plan FROM users WHERE id = ?').get(userId); | ||
| const userPlan = dbUser?.plan ?? 'free'; | ||
| console.info('[process] request', { fileName: originalName, mime, extension: ext || '(none)', mode: 'server', userPlan }); | ||
| if (!isServerSupportedFormat(originalName, mime)) { | ||
| await fs.remove(inputPath).catch(() => {}); | ||
| console.info('[process] rejected', { reason: 'unsupported_file_type', extension: ext || '(none)', mime, userPlan }); | ||
| return res.status(422).json({ | ||
| error: 'Unsupported file type for Full Server Cleanse', |
There was a problem hiding this comment.
issue (bug_risk): Normalizing the extension to empty string can lead to extension-less server output files in some cases.
With normalizeExt now returning '', a file uploaded without an extension but with a supported MIME type will still pass isServerSupportedFormat, but outputPath will append an empty ext. That yields output files with no extension, which is problematic for users and some tools. Please add a fallback to a safe default extension (e.g. .mp4) when normalizeExt returns an empty string but the MIME type is supported.
Comment on lines
+548
to
+550
| const ext = normalizeExt(file.originalname || ''); | ||
| const mime = (file.mimetype || '').toLowerCase(); | ||
| const isMp3 = ext === '.mp3' || mime === 'audio/mpeg'; | ||
| if (isMp3) { await fs.remove(file.path).catch(() => {}); results.push({ originalName: file.originalname, error: 'MP3 server cleanse is not supported. Use Quick Cleanse (Browser) for MP3.' }); continue; } | ||
| if (!isServerSupportedFormat(file.originalname || '', mime)) { await fs.remove(file.path).catch(() => {}); results.push({ originalName: file.originalname, error: 'Full Server Cleanse currently supports MP4 and M4A only. Use Quick Cleanse (Browser) for MP3, or convert WAV/FLAC to M4A/MP4.', reason: 'unsupported_file_type' }); continue; } |
There was a problem hiding this comment.
issue (bug_risk): Batch output files may lose their default extension, unlike the previous behavior.
Previously, when path.extname returned an empty string in the batch path, ext defaulted to .mp4. With normalizeExt, ext is now '' in that case, so extension-less uploads will produce output paths with no file extension, changing the prior behavior and affecting download names. Please consider a fallback extension here (as in the single-file route) when ext is empty but mime is a supported format.
Comment on lines
+24
to
+30
| function isServerSupportedFormat(filename = '', mime = '') { | ||
| const ext = normalizeExt(filename); | ||
| const safeMime = String(mime || '').toLowerCase(); | ||
| if (CLEANSE_POLICY.server.supportedExtensions.includes(ext)) return true; | ||
| // Common upload MIME aliases for supported server formats. | ||
| return safeMime === 'video/mp4' || safeMime === 'audio/mp4' || safeMime === 'audio/x-m4a'; | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): The server MIME checks for M4A may miss common variants such as audio/m4a.
The guard only allows video/mp4, audio/mp4, and audio/x-m4a, but some clients send M4A as audio/m4a. To prevent valid M4A uploads from being rejected when MIME/ext don’t line up, please add audio/m4a (and any other expected aliases) to this list.
Suggested change
| function isServerSupportedFormat(filename = '', mime = '') { | |
| const ext = normalizeExt(filename); | |
| const safeMime = String(mime || '').toLowerCase(); | |
| if (CLEANSE_POLICY.server.supportedExtensions.includes(ext)) return true; | |
| // Common upload MIME aliases for supported server formats. | |
| return safeMime === 'video/mp4' || safeMime === 'audio/mp4' || safeMime === 'audio/x-m4a'; | |
| } | |
| function isServerSupportedFormat(filename = '', mime = '') { | |
| const ext = normalizeExt(filename); | |
| const safeMime = String(mime || '').toLowerCase(); | |
| if (CLEANSE_POLICY.server.supportedExtensions.includes(ext)) return true; | |
| // Common upload MIME aliases for supported server formats. | |
| return ( | |
| safeMime === 'video/mp4' || | |
| safeMime === 'audio/mp4' || | |
| safeMime === 'audio/x-m4a' || | |
| safeMime === 'audio/m4a' | |
| ); | |
| } |
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.
Motivation
Description
server/cleansePolicy.jscontaining the canonical supported/recommended sets (quick=>.mp3,server=>.mp4,.m4a) and helpersnormalizeExt/isServerSupportedFormat.POST /api/processinserver.jsto use the policy helper, emit diagnostic logging (file name, MIME, extension, mode, user plan), and return structured 422 responses withreason: 'unsupported_file_type'andsupportedServerFormatswhen a file is rejected for format./api/processremains allowed forfreeuntil monthly limit (returns402withreason: 'usage_limit'), while batch/api/process-batchremains paid-only and now returns403withreason: 'plan_restriction'.server/processor.jsby taggingunsupportedCleanseErrorwithreason: 'unsupported_file_type'and addingexiftool_failurefor metadata rewrite failures, and propagatingreasonfields in API error responses.app.tsxUI logic to only recommend/enable Full Server Cleanse for MP4/M4A, to keep Quick Cleanse as MP3-only, and to show clear WAV/FLAC guidance instead of implying server support.server/cleansePolicy.js(new),server.js,server/processor.js,app.tsx.Testing
npm run build(automated TypeScript build); build failed with pre-existing frontend TypeScript/React dependency errors unrelated to these changes (missing React/JSX types), so no fully passing build could be produced in this environment.reasonfields to make automated verification straightforward in downstream tests.Codex Task
Summary by Sourcery
Align server cleanse behavior, error responses, and UI recommendations with a canonical cleanse policy and actual server format support.
New Features:
Bug Fixes:
Enhancements:
Tests: