bumb base#4
Conversation
Removes [tar](https://github.com/isaacs/node-tar). It's no longer used after updating ancestor dependency [npm](https://github.com/npm/cli). These dependencies need to be updated together. Removes `tar` Updates `npm` from 11.11.0 to 11.11.1 - [Release notes](https://github.com/npm/cli/releases) - [Changelog](https://github.com/npm/cli/blob/latest/CHANGELOG.md) - [Commits](npm/cli@v11.11.0...v11.11.1) --- updated-dependencies: - dependency-name: tar dependency-version: dependency-type: indirect - dependency-name: npm dependency-version: 11.11.1 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
…bot/npm_and_yarn/multi-cc382f683c chore(deps): bump tar and npm
This is resolved by using ffmpeg to copy the audio data into a new mp4 container file before passing it to taglib.
…logging for ffmpeg
…ract-duplicated-code-to-ts Extract shared download utilities and abstract bulk download handlers + new folder picker support
- FolderPickerWriter: throw AbortError on cancel instead of returning null - FolderPickerWriter: add try/catch with abort() to release file locks on failure - ZipNeutralinoWriter: move writeBinaryFile after response.body validation - bulkDownloadSettings: migrate legacy key and validate stored values - download-utils: catch ffmpeg cancellation via signal.aborted - downloads.js: use consistent Neutralino detection with bridge module - download-utils: use strict equality for flac extension check
…laylist fix(library): share playlist with the correct url
Fix local folder
Made-with: Cursor
HOTFIX: Fix track validation in loveTrack method
…rid on narrow viewports Made-with: Cursor
Made-with: Cursor
…es, clear-btn, styles) and lint workflow; keep scroll on list/grid toggle Made-with: Cursor
…brary-fullscreen Fix(ui): video library cards, fullscreen layout, and search UX
…ace-avatar fix(api): format image url for the api to recognize it and to save the user details
📝 WalkthroughWalkthroughThe pull request introduces significant infrastructure and feature enhancements: adds iOS/Capacitor support with native app configuration, refactors Docker deployment to use multi-stage Nginx builds, implements a new HiFi client API wrapper for direct Tidal integration, introduces podcast streaming via PodcastIndex, replaces DASH.js with Shaka Player for adaptive streaming, adds modern settings and bulk download infrastructure with multiple writer strategies, removes Qobuz support, removes smooth scrolling, updates authentication to support GitHub/Spotify/Discord, and refactors the icon system to use parameterized SVG factories. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Vite as Vite Dev Server
participant HiFiClient
participant TidalAPI as Tidal API
participant PodcastAPI as PodcastIndex API
participant ShakaPlayer
participant Download as Download Manager
Browser->>Vite: Request /track/123
Vite->>Browser: Serve app with Player.instance
Browser->>HiFiClient: initialize()
HiFiClient->>TidalAPI: getToken() via OAuth
TidalAPI-->>HiFiClient: Bearer token + refresh
Browser->>HiFiClient: query(/track/?id=123)
HiFiClient->>TidalAPI: fetch track metadata
TidalAPI-->>HiFiClient: track JSON
HiFiClient-->>Browser: TidalResponse with track
Browser->>HiFiClient: getTrackManifest(123)
HiFiClient->>TidalAPI: /trackManifests endpoint
TidalAPI-->>HiFiClient: manifest + ReplayGain
HiFiClient-->>Browser: {url, rgInfo}
Browser->>ShakaPlayer: attach(source)
ShakaPlayer->>ShakaPlayer: load DASH/HLS stream
ShakaPlayer-->>Browser: adaptive quality selection
Browser->>Download: downloadTrack(track)
Download->>Download: enrichTrack() + applyAudioPostProcessing()
Download->>HiFiClient: getStreamUrl()
Download->>FFmpeg: transcode if needed
FFmpeg-->>Download: Blob output
Download-->>Browser: download complete
sequenceDiagram
participant Worker as Cloudflare Worker
participant Bot as Bot Request
participant DirectAPI as Direct Tidal API
participant ServerAPI as ServerAPI Proxy
participant Meta as Meta Tags
Bot->>Worker: GET /album/123 with bot UA
Worker->>Worker: isBot check
alt Bot Request
Worker->>DirectAPI: TidalAPI.getAlbumMetadata()
alt Success
DirectAPI-->>Worker: album metadata + cover
Worker->>Meta: build OpenGraph tags
Worker-->>Bot: HTML with meta tags
else Failure
Worker->>ServerAPI: fallback instance lookup
ServerAPI-->>Worker: metadata
Worker->>Meta: build meta tags
Worker-->>Bot: HTML response
end
else Non-Bot
Worker-->>Bot: Serve SPA from assets
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (15)
public/manifest.json (1)
62-68:⚠️ Potential issue | 🔴 CriticalFix mime type for WebP icons and consider updating shortcut icon for consistency.
The shortcut icon file
assets/96.pngexists and is valid. However, there are two issues:
Critical: The main icons use
.webpfiles but declaretype: "image/png"- this is a mime type mismatch. Change totype: "image/webp"for all icon entries with.webpsources.Optional: The shortcut icon still references the old PNG file while main icons use WebP. Consider updating to
assets/icons/icon-96.webpfor consistency with the main icons array.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/manifest.json` around lines 62 - 68, The manifest's icons array has mime-type mismatches and an inconsistent shortcut icon: update every icon entry whose "src" ends with ".webp" to use type "image/webp" instead of "image/png" (look for the "icons" array and its "type" fields), and optionally change the shortcut icon "src" that currently points to "assets/96.png" to the WebP file used by the main icons (e.g., "assets/icons/icon-96.webp") so the shortcut and main icons are consistent..github/workflows/lint.yml (1)
24-53:⚠️ Potential issue | 🔴 CriticalCorrect step indentation; current YAML is invalid.
From Line 24 onward, steps are misindented outside
jobs.lint.steps, which causes parse failure and blocks the workflow.Suggested fix (reindent steps under
steps:)- - name: Setup Node - uses: actions/setup-node@v4 - with: - node-version: '24' - cache: 'npm' + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version: '24' + cache: 'npm' - - name: Install dependencies - run: npm ci + - name: Install dependencies + run: npm ci - - name: Run JS Lint - run: npm run lint:js -- --fix - continue-on-error: true + - name: Run JS Lint + run: npm run lint:js -- --fix + continue-on-error: true - - name: Run CSS Lint - run: npm run lint:css -- --fix - continue-on-error: true + - name: Run CSS Lint + run: npm run lint:css -- --fix + continue-on-error: true - - name: Format with Prettier - run: npm run format - continue-on-error: true + - name: Format with Prettier + run: npm run format + continue-on-error: true - name: Commit and Push lint fixes uses: stefanzweifel/git-auto-commit-action@v5 with: commit_message: 'style: auto-fix linting issues' commit_user_name: 'github-actions[bot]' commit_user_email: 'github-actions[bot]@users.noreply.github.com' - - name: Run HTML Lint - run: npm run lint:html + - name: Run HTML Lint + run: npm run lint:html🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/lint.yml around lines 24 - 53, The workflow's steps block has incorrect indentation causing the steps (Setup Node, Install dependencies, Run JS Lint, Run CSS Lint, Format with Prettier, Commit and Push lint fixes, Run HTML Lint) to be defined outside jobs.lint.steps; re-indent all these step entries so they are children of jobs.lint.steps (ensure each "- name:" line aligns under the steps: key), fix the nested Commit and Push action indentation so its uses/with keys are indented under that step, and validate the YAML (jobs.lint.steps should be a single list containing all listed steps).functions/artist/[id].js (1)
74-93:⚠️ Potential issue | 🟠 MajorKeep the artist proxy list aligned with the new squid blacklist.
This handler still returns
*.squid.wtfinstances from discovery and still hardcodeshttps://triton.squid.wtfas a fallback. The playlist handler in this same PR now filters those out, so artist previews can still route to the proxy family the rollout is trying to retire.Suggested fix
if (data) { - this.apiInstances = (data.api || []).map((item) => item.url || item); + this.apiInstances = (data.api || []) + .map((item) => item.url || item) + .filter((url) => !/\.squid\.wtf/i.test(url)); return this.apiInstances; } @@ return [ 'https://eu-central.monochrome.tf', 'https://us-west.monochrome.tf', 'https://arran.monochrome.tf', - 'https://triton.squid.wtf', 'https://api.monochrome.tf', 'https://monochrome-api.samidy.com', 'https://maus.qqdl.site',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/artist/`[id].js around lines 74 - 93, The artist handler currently includes and falls back to squid.wtf proxies; update the logic in the assignment to this.apiInstances in functions/artist/[id].js so any discovered or fallback URLs matching *.squid.wtf are excluded. Specifically, when mapping (data.api || []).map(...) replace with a map+filter (or filter before map) to remove hosts matching /\.squid\.wtf$/i (or /^https?:\/\/.*\.squid\.wtf/i), and remove or replace the hardcoded 'https://triton.squid.wtf' entry from the fallback array so the returned array contains no squid.wtf domains. Ensure the rest of the return value and variable name this.apiInstances remain unchanged.functions/album/[id].js (1)
178-207:⚠️ Potential issue | 🟠 MajorSecurity: HTML template vulnerable to XSS - same issue as
track/[id].js.Album metadata (
title,artist,description) is interpolated without HTML escaping. Apply the sameescapeHtmlfunction fix suggested fortrack/[id].js.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/album/`[id].js around lines 178 - 207, The metaHtml template interpolates unescaped album fields (title, artist, description, imageUrl, pageUrl, album.releaseDate) and is vulnerable to XSS; update the template construction to pass every interpolated value through the existing escapeHtml helper (e.g., use escapeHtml(title), escapeHtml(artist), escapeHtml(description), escapeHtml(imageUrl), escapeHtml(pageUrl), and escapeHtml(album.releaseDate) or a safe formatter) so both HTML element content and attribute values are escaped before building the metaHtml string.functions/track/[id].js (1)
218-249:⚠️ Potential issue | 🟠 MajorSecurity: HTML template is vulnerable to XSS via unescaped metadata.
Track metadata (
title,artist,description,album.title) is interpolated directly into HTML without escaping. If a track's metadata contains characters like<,>,", or&, it could break the HTML or enable XSS attacks.🛡️ Proposed fix - add an escape function
+function escapeHtml(str) { + if (!str) return ''; + return String(str) + .replace(/&/g, '&') + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +}Then use it in the template:
- <title>${title} by ${artist}</title> - <meta name="description" content="${description}"> + <title>${escapeHtml(title)} by ${escapeHtml(artist)}</title> + <meta name="description" content="${escapeHtml(description)}">Apply similar escaping to all interpolated values in the HTML template.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/track/`[id].js around lines 218 - 249, The metaHtml template is vulnerable to XSS because track metadata (title, artist, description, track.album.title, imageUrl, trackUrl, audioMeta, etc.) are interpolated raw; add a small HTML-escape helper (e.g., escapeHtml) and call it for every interpolated value used in the template (title, artist, description, track.album.title, imageUrl, trackUrl, track.duration if string, and any strings inside audioMeta) before building metaHtml so all special characters (& < > " ') are properly escaped; update uses of title, artist, description, imageUrl, trackUrl, track.duration, track.album.title and audioMeta in the metaHtml construction to use the escaped variants.js/visualizer.js (2)
142-144:⚠️ Potential issue | 🟡 MinorMissing
awaitfor asyncinit()call.
init()was changed toasync(line 61), butstart()calls it withoutawait. This could cause race conditions wherethis.analyseris accessed before audio context initialization completes.🔧 Proposed fix
- start() { + async start() { if (this.isActive) return; if (!this.ctx) { this.initContext(); } if (!this.audioContext) { - this.init(); + await this.init(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/visualizer.js` around lines 142 - 144, The start() method calls the now-async init() without awaiting, which can let this.analyser be used before initialization finishes; update start() to await init() (i.e., make start() async and use await this.init()) or otherwise wait for the promise returned by init() before accessing this.audioContext or this.analyser so initialization completes first.
57-59:⚠️ Potential issue | 🟡 MinorAdd defensive check for uninitialized presets.
The
activePresetgetter accessesthis.presetswhich isundefineduntilinitPresets()completes. If accessed prematurely (e.g., during startup before async initialization), this will throw a TypeError.🛡️ Proposed defensive fix
get activePreset() { + if (!this.presets) return null; return this.presets[this.activePresetKey] || this.presets['lcd']; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/visualizer.js` around lines 57 - 59, The activePreset getter can throw when this.presets is undefined before initPresets() finishes; update the getter (activePreset) to defensively check that this.presets is an object (and that this.activePresetKey exists) and return a safe fallback (e.g., null or an empty preset, or this.presets['lcd'] only if presets exists) so accessing activePreset during startup won't raise a TypeError; reference the activePreset getter, this.presets, initPresets(), and this.activePresetKey when making the change.js/audio-context.js (1)
703-708:⚠️ Potential issue | 🟡 MinorMalformed JSDoc comment block.
The JSDoc comment on lines 703-704 is incomplete and not properly closed before the next JSDoc block starts on line 705. This creates invalid documentation.
📝 Proposed fix
/** * Called when the app enters the background (screen lock, app switch). - /** + */ + + /** * Export equalizer settings to text format * `@returns` {string} Exported settings in text format */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/audio-context.js` around lines 703 - 708, Fix the malformed JSDoc by closing the incomplete comment that begins with "Called when the app enters the background (screen lock, app switch)."—add the missing "*/" (and any missing JSDoc tags) before the next JSDoc block that documents "Export equalizer settings to text format"; ensure each JSDoc block properly precedes and documents its intended function (the background handler comment and the exportEqualizerSettings / export function) so the documentation is valid and not merged.js/lyrics.js (2)
3-15:⚠️ Potential issue | 🔴 CriticalCritical: Duplicate declaration shadows imported
SVG_GENIUS_ACTIVE.
SVG_GENIUS_ACTIVEis imported from./icons.json line 5, but then redeclared as a constant on line 15. The local declaration shadows the import, and the hardcoded SVG string will be used instead of the icon factory function.Remove line 15 to use the imported icon function.
🐛 Proposed fix
import { SVG_CLOSE, SVG_GENIUS_ACTIVE, SVG_GENIUS_INACTIVE, SVG_MINUS, SVG_PLUS, SVG_RESET, SVG_GLOBE, } from './icons.js'; import { sidePanelManager } from './side-panel.js'; import '@uimaxbai/am-lyrics/am-lyrics.js'; -const SVG_GENIUS_ACTIVE = `<svg width="20" height="20" viewBox="0 0 24 24" fill="none"><path d="M12 24c6.627 0 12-5.373 12-12S18.627 0 12 0 0 5.373 0 12s5.373 12 12 12z" fill="#ffff64"/><path d="M6.3 6.3h11.4v11.4H6.3z" fill="#000"/></svg>`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/lyrics.js` around lines 3 - 15, The file redeclares SVG_GENIUS_ACTIVE locally which shadows the imported SVG_GENIUS_ACTIVE from icons.js; remove the local const declaration (the const SVG_GENIUS_ACTIVE = `<svg ...>`) so all references use the imported symbol, and verify there are no other duplicate declarations of SVG_GENIUS_ACTIVE in this module; if any code relied on the hardcoded string, replace those usages with the imported value or call the imported factory as intended.
58-59:⚠️ Potential issue | 🟠 MajorHardcoded API token in client-side code.
Similar to the PodcastIndex credentials, this Genius API token is exposed in the client bundle. Consider proxying through your backend or using environment variables.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/lyrics.js` around lines 58 - 59, The getToken() method currently returns a hardcoded Genius API token (in js/lyrics.js -> getToken); remove the hardcoded string and stop exposing credentials in the bundle. Replace getToken with a call to your backend (e.g., fetch('/api/genius/token') or, better, proxy all Genius requests through an endpoint like '/api/genius/search') so the server holds process.env.GENIUS_TOKEN and performs the external requests, or read a build-time environment variable injected securely rather than a literal string. Ensure you remove the hardcoded token from the repo and update any code paths expecting getToken() to use the new backend endpoint.js/events.js (1)
1658-1671:⚠️ Potential issue | 🟡 MinorNormalize
user-playlistURLs here too.
copy-linkalready mapsuser-playlistto/userplaylist/..., but this fallback still opens/${type}/.... For user playlists that produces/user-playlist/..., which does not match the router.🔧 Minimal fix
const contextMenu = document.getElementById('context-menu'); const storedHref = contextMenu?._contextHref; + const typeForUrl = type === 'user-playlist' ? 'userplaylist' : type; const url = storedHref ? `${window.location.origin}${storedHref}` - : `${window.location.origin}/${type}/${item.id || item.uuid}`; + : `${window.location.origin}/${typeForUrl}/${item.id || item.uuid}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/events.js` around lines 1658 - 1671, The open-in-new-tab branch builds a fallback URL using the raw type which misformats 'user-playlist'; introduce the same normalization used earlier (e.g., const typeForUrl = type === 'user-playlist' ? 'userplaylist' : type) and use typeForUrl when constructing the fallback URL (the branch that sets url to window.location.origin/... when contextMenu?._contextHref is absent) so the router receives /userplaylist/..., referencing the existing variables contextMenu?._contextHref and item.id || item.uuid.js/app.js (1)
422-523:⚠️ Potential issue | 🔴 CriticalFinish the singleton migration before dropping the old locals.
This block switches the file to
Player.instance/UIRenderer.instance, but later paths still referenceplayerandui(mediaNext,initDesktop, and the local-folder rerender path). Since neither local is defined anymore, those flows now fail withReferenceError.🔧 Minimal fix
await Player.initialize(audioPlayer, MusicAPI.instance, currentQuality); + const player = Player.instance; @@ await UIRenderer.initialize(MusicAPI.instance, Player.instance); + const ui = UIRenderer.instance;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app.js` around lines 422 - 523, The code migrated to Player.instance and UIRenderer.instance but still references the removed locals (player, ui) in event handlers and initialization calls; replace all remaining uses of the old locals with the singletons (e.g., use Player.instance instead of player for playNext/playPrev/handlePlayPause/audio/currentTime and pass Player.instance into desktopModule.initDesktop) and replace any ui references with UIRenderer.instance (including any local-folder rerender paths), ensuring event callbacks reference the singleton properties (Player.instance.audio, Player.instance.activeElement, etc.) so no ReferenceError occurs.js/metadata.js (1)
139-152:⚠️ Potential issue | 🟡 MinorPotential runtime error:
file.lastModifiedmay be undefined for some input types.The function accepts
Uint8Array | Blob | File | FileSystemFileHandle | FileSystemFileEntry, butlastModifiedonly exists onFileobjects. For other types, this will beundefined, resulting in IDs likelocal-filename-undefined.✏️ Proposed fix
export async function readTrackMetadata(file, { filename = file?.name || 'Unknown Title', siblings } = {}) { const metadata = { title: filename?.replace(/\.[^/.]+$/, ''), artists: [], artist: { name: 'Unknown Artist' }, // For fallback/compatibility album: { title: 'Unknown Album', cover: 'assets/appicon.png', releaseDate: null }, duration: 0, isrc: null, copyright: null, explicit: false, isLocal: true, file: file, - id: `local-${filename}-${file.lastModified}`, + id: `local-${filename}-${file?.lastModified ?? Date.now()}`, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/metadata.js` around lines 139 - 152, readTrackMetadata currently builds metadata.id using file.lastModified which exists only on File objects and can be undefined for other accepted types; change the id generation in readTrackMetadata (metadata.id) to use a safe fallback when file.lastModified is missing (e.g., use file.lastModified ?? Date.now() or another deterministic fallback like a generated short UID or file.size combined with Date.now()) so IDs never contain "undefined"; update only the metadata.id expression to reference the chosen fallback.js/api.js (1)
1911-1914:⚠️ Potential issue | 🔴 CriticalWeb downloads are no longer triggered.
After building the blob, this only attempts the Capacitor save path and returns. On the web,
saveTrackToNativeMusicDirectory()returnsfalse, so the user gets no file despite the method contract and the importedtriggerDownload.Suggested fix
const savedToNativeMusic = await this.saveTrackToNativeMusicDirectory(blob, finalFilename, track, { forceVideo: isVideo, }); + if (!savedToNativeMusic && options.triggerDownload !== false) { + triggerDownload(blob, finalFilename); + } return blob;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/api.js` around lines 1911 - 1914, The code calls saveTrackToNativeMusicDirectory(blob, finalFilename, track, { forceVideo: isVideo }) and immediately returns the blob, but on web that save method returns false so no file is produced; update the flow so that after awaiting savedToNativeMusic you check its value and, if falsy, call the existing triggerDownload(blob, finalFilename) to perform a web download (keeping the same finalFilename and blob), then return the blob; reference saveTrackToNativeMusicDirectory, savedToNativeMusic, triggerDownload, finalFilename, blob and isVideo to locate and implement the conditional fallback.styles.css (1)
5280-5294:⚠️ Potential issue | 🟠 MajorResizable side panels need the fullscreen offsets to use the same width variable.
This now introduces
--side-panel-width, but the fullscreen shift rules later in the file still hard-code600px/50vwat Lines 7605-7632. After resizing, the fullscreen content and buttons will drift out of alignment with the panel.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@styles.css` around lines 5280 - 5294, The fullscreen shift rules later in the stylesheet still use hard-coded 600px / 50vw values, which will desync after the panel is resized; update those selectors to use the same CSS variable used here (use var(--side-panel-width, 600px) for the desktop fallback and var(--side-panel-width, 50vw) where 50vw is currently used) so fullscreen offsets, content and buttons align with the resizable panel (reference the --side-panel-width variable and the sidepanel container-name in your edits).
🟡 Minor comments (18)
public/editors-picks.json-233-243 (1)
233-243:⚠️ Potential issue | 🟡 MinorInconsistent data type for
artist.idfield.The
artist.idat line 237 is a string ("17300439") while all other artist IDs in the file are numbers. This inconsistency could cause issues if consuming code performs strict type comparisons or expects numeric IDs.Note: There's also a pre-existing inconsistency at line 224 where the album
idis a string ("344201347"), but that wasn't introduced by this PR.🔧 Proposed fix
{ "type": "album", "id": 365819314, "title": "One Life", - "artist": { "id": "17300439", "name": "1oneam" }, + "artist": { "id": 17300439, "name": "1oneam" }, "releaseDate": "2024-05-30", "cover": "eb5d74f6-7403-4404-8452-9b68713445fe", "explicit": true, "audioQuality": "LOSSLESS", "mediaMetadata": { "tags": ["LOSSLESS"] } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/editors-picks.json` around lines 233 - 243, The artist.id in the editors-picks entry for album "One Life" is a string ("17300439") whereas other artist IDs are numeric; update the artist.id value to a number (remove quotes) so it matches the numeric type used elsewhere (e.g., other artist.id fields) and run a quick scan of the surrounding entries (including album.id occurrences like the pre-existing "344201347") to ensure consistent numeric types across the file..github/pull_request_template.md-15-15 (1)
15-15:⚠️ Potential issue | 🟡 MinorImprove wording and capitalization of the AI checkbox.
The new checkbox has several text quality issues:
- Inconsistent capitalization: "Pull request" and "Using AI" should be lowercase
- Awkward phrasing: "Is this..." format doesn't work well for checkboxes
- Unclear terminology: "Vibecoded" may not be immediately clear to all contributors
- Two questions merged into one checkbox
📝 Suggested rewording
-- [ ] Is this Pull request Using AI/Is Vibecoded? +- [ ] This pull request uses AI assistance or is "vibecoded"Alternatively, if these should be separate concerns:
-- [ ] Is this Pull request Using AI/Is Vibecoded? +- [ ] This pull request uses AI assistance +- [ ] This pull request is "vibecoded"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/pull_request_template.md at line 15, Replace the awkward checkbox "[ ] Is this Pull request Using AI/Is Vibecoded?" with clearer, lowercased, and separated options: change it to two checkboxes such as "[ ] uses AI-generated content" and "[ ] uses Vibecoded" (or "[ ] uses Vibecoded tooling" if you want clearer intent), keeping "pull request" lowercase and removing the "Is this..." question wording; update the template line that contains the original checkbox text accordingly.public/manifest.json-13-53 (1)
13-53:⚠️ Potential issue | 🟡 MinorMIME type mismatch: WebP files declared as PNG.
All icon entries reference
.webpfiles but declare"type": "image/png". The correct MIME type for WebP images is"image/webp". This mismatch may cause browsers to mishandle icon loading or caching.🐛 Proposed fix
{ "src": "assets/icons/icon-48.webp", - "type": "image/png", + "type": "image/webp", "sizes": "48x48", "purpose": "any maskable" }, { "src": "assets/icons/icon-72.webp", - "type": "image/png", + "type": "image/webp", "sizes": "72x72", "purpose": "any maskable" }, { "src": "assets/icons/icon-96.webp", - "type": "image/png", + "type": "image/webp", "sizes": "96x96", "purpose": "any maskable" }, { "src": "assets/icons/icon-128.webp", - "type": "image/png", + "type": "image/webp", "sizes": "128x128", "purpose": "any maskable" }, { "src": "assets/icons/icon-192.webp", - "type": "image/png", + "type": "image/webp", "sizes": "192x192", "purpose": "any maskable" }, { "src": "assets/icons/icon-256.webp", - "type": "image/png", + "type": "image/webp", "sizes": "256x256", "purpose": "any maskable" }, { "src": "assets/icons/icon-512.webp", - "type": "image/png", + "type": "image/webp", "sizes": "512x512", "purpose": "any maskable" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/manifest.json` around lines 13 - 53, Update the icon entries in public/manifest.json so the MIME type matches the .webp assets: change each "type": "image/png" to "type": "image/webp" for the entries with "src": "assets/icons/icon-48.webp", "assets/icons/icon-72.webp", "assets/icons/icon-96.webp", "assets/icons/icon-128.webp", "assets/icons/icon-192.webp", "assets/icons/icon-256.webp", and "assets/icons/icon-512.webp" so the manifest correctly declares WebP images.package.json-19-19 (1)
19-19:⚠️ Potential issue | 🟡 MinorAdd explicit
packageManagerdeclaration to clarify Bun requirement.The
lintscript usesbun run, but package.json lacks an explicitpackageManagerorenginesfield. While Bun is properly configured in the CI/Dockerfile/devcontainer, adding this declaration will prevent confusion for developers and tools like Corepack.Suggested fix
"scripts": { "lint": "bun run lint:js && bun run lint:css && bun run lint:html",+ "packageManager": "bun@1.2.15", "name": "monochrome",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 19, The package.json currently uses the "lint" script with bun ("bun run lint:js") but lacks an explicit package manager declaration; add a top-level "packageManager" entry (for example "packageManager": "bun@<version>" matching the Bun version used in CI/devcontainer) to package.json so tools like Corepack and contributors know Bun is required, or alternatively add an "engines" field specifying bun and its version; update the package.json's top-level metadata (not inside scripts) to include this declaration and ensure the version matches the CI/Dockerfile/devcontainer configuration.CONTRIBUTING.md-308-308 (1)
308-308:⚠️ Potential issue | 🟡 MinorGrammatical error: missing preposition.
The sentence "Changes are live a minute" is incomplete. It should read "Changes are live in a minute" or "Changes are live within a minute".
📝 Proposed fix
-3. Changes are live a minute +3. Changes are live in a minute🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.md` at line 308, The sentence "Changes are live a minute" is missing a preposition; update that sentence (the line containing "Changes are live a minute") to read either "Changes are live in a minute" or "Changes are live within a minute" for correct grammar and clarity, and run a quick proofread of nearby lines to ensure consistent phrasing.js/downloadProgressUtils.js-11-14 (1)
11-14:⚠️ Potential issue | 🟡 MinorPotential bug:
parseIntreturningNaNwon't trigger nullish fallback.If
contentLengthFromGetis present but malformed (e.g.,"abc"),parseIntreturnsNaN. SinceNaNis notnullorundefined, the nullish coalescing (??) won't fall back toheadContentLength, and the function will returnNaN.🐛 Proposed fix
export function resolveDownloadTotalBytes(contentLengthFromGet, headContentLength) { - const fromGet = contentLengthFromGet ? parseInt(contentLengthFromGet, 10) : null; - return fromGet ?? headContentLength ?? 0; + const parsed = contentLengthFromGet ? parseInt(contentLengthFromGet, 10) : NaN; + const fromGet = Number.isNaN(parsed) ? null : parsed; + return fromGet ?? headContentLength ?? 0; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/downloadProgressUtils.js` around lines 11 - 14, In resolveDownloadTotalBytes, parseInt(contentLengthFromGet, 10) can produce NaN which the nullish coalescing won't catch; change the logic to parse and then check for a valid finite number (e.g., const parsed = parseInt(...); if Number.isFinite(parsed) use parsed) and otherwise fall back to headContentLength and then 0, so the function returns a numeric download total even when contentLengthFromGet is malformed.functions/album/[id].js-74-78 (1)
74-78:⚠️ Potential issue | 🟡 MinorInconsistent instance filtering across handlers.
The
getInstances()method filters out.squid.wtfURLs inalbum/[id].js(line 77) andplaylist/[id].js, buttrack/[id].js(line 97) andartist/[id].jshave no such filter. This causes different behavior between handlers when processing API instances, and is inconsistent despite these being parallel implementations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/album/`[id].js around lines 74 - 78, The getInstances() implementation is inconsistent: album/[id].js (and playlist) filter out .squid.wtf URLs but track/[id].js and artist/[id].js do not. Update the getInstances() logic in track/[id].js and artist/[id].js to match album's behavior by mapping items to item.url || item and adding .filter(url => !/\.squid\.wtf/i.test(url)) before assigning this.apiInstances (or alternatively remove the filter from album/playlist if you prefer the opposite behavior) so all handlers use the same instance filtering.vite-plugin-blob.ts-78-88 (1)
78-88:⚠️ Potential issue | 🟡 MinorAdd response status check before processing the fetch result.
The generated client code doesn't verify
res.okbefore processing. A failed fetch (404, 500, etc.) would silently produce corrupted data or cryptic decompression errors.🔧 Proposed fix
return blobPromise = (async () => { try { const res = await fetch(${JSON.stringify(assetUrl)}); + if (!res.ok) { + throw new Error(\`Failed to fetch blob asset: \${res.status}\`); + } const compressed = await res.arrayBuffer();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vite-plugin-blob.ts` around lines 78 - 88, The fetch result is not checked for non-OK HTTP status, so update the async blobPromise generator (the function creating blobPromise that calls fetch(${JSON.stringify(assetUrl)})) to verify res.ok after await fetch(...) and before calling res.arrayBuffer(); if !res.ok throw or reject a clear Error that includes res.status and res.statusText (and ideally the assetUrl) so that decompress(...) and Blob creation (using mime.getType(filepath) and URL.createObjectURL) never run on an invalid response.js/listenbrainz.js-112-129 (1)
112-129:⚠️ Potential issue | 🟡 MinorShadowed
paramsvariable in retry block.The
paramsvariable is re-declared withconston line 115, shadowing theparamsfrom line 96. While this works, it can be confusing and is typically flagged by linters.🔧 Proposed fix
if (!response.ok) { console.warn(`[ListenBrainz] MBID lookup failed, trying without album`); with_album = false; - const params = new URLSearchParams({ + const retryParams = new URLSearchParams({ recording_name: metadata.track_name, artist_name: metadata.artist_name, }); - response = await fetch(`${apiUrl}/1/metadata/lookup/?${params}`, { + response = await fetch(`${apiUrl}/1/metadata/lookup/?${retryParams}`, {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/listenbrainz.js` around lines 112 - 129, The retry block is redeclaring params which shadows the earlier params; change the retry to reuse and modify the existing URLSearchParams instead of creating a new const. For example, remove the `const` and either call params.delete('album_name') or rebuild by assigning to the existing params variable (e.g., params = new URLSearchParams({...}) if params is let) so the same params object is reused before the second fetch; keep the same headers usage with this.getToken(), and ensure with_album is still set to false and response is reassigned as shown.js/accounts/auth.js-221-228 (1)
221-228:⚠️ Potential issue | 🟡 MinorMissing UI wiring for Spotify sign-in button.
The
signInWithSpotify()method was added (line 74-85), but there's no corresponding UI button handling inupdateUI(). The GitHub and Discord buttons are wired, but Spotify is missing.Is this intentional, or should a
#auth-spotify-btnbe handled similarly?🔧 Proposed fix to add Spotify button handling
const githubBtn = document.getElementById('auth-github-btn'); const discordBtn = document.getElementById('auth-discord-btn'); + const spotifyBtn = document.getElementById('auth-spotify-btn');And in the signed-in block:
if (githubBtn) githubBtn.style.display = 'none'; if (discordBtn) discordBtn.style.display = 'none'; + if (spotifyBtn) spotifyBtn.style.display = 'none';And in the signed-out block:
if (discordBtn) { discordBtn.style.display = 'inline-block'; discordBtn.onclick = () => this.signInWithDiscord(); } + if (spotifyBtn) { + spotifyBtn.style.display = 'inline-block'; + spotifyBtn.onclick = () => this.signInWithSpotify(); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/accounts/auth.js` around lines 221 - 228, The UI update is missing wiring for the Spotify button: in updateUI(), add a spotifyBtn lookup for '#auth-spotify-btn' alongside githubBtn and discordBtn and set spotifyBtn.style.display = 'inline-block' and spotifyBtn.onclick = () => this.signInWithSpotify() where the signed-out buttons are shown (and hide/adjust it in the signed-in block the same way you handle githubBtn/discordBtn); reference the signInWithSpotify() method and the '#auth-spotify-btn' element so the Spotify button is shown and invokes the existing signInWithSpotify handler.js/ffmpeg.js-83-87 (1)
83-87:⚠️ Potential issue | 🟡 MinorProgress callback may fire twice for the same event.
The conditions on lines 83 and 85-86 can both be true simultaneously:
- Line 83:
type === 'progress' && message- Line 85:
type === 'progress' && stage != 'loading' && progress !== nullWhen
messageis truthy,stage != 'loading', andprogress !== null, both conditions pass, causing duplicateonProgresscalls.🔧 Proposed fix to make conditions mutually exclusive
- } else if (type === 'progress' && message) { - onProgress?.(new FfmpegProgress(stage, progress || 0, message)); - } else if (type === 'progress' && stage != 'loading' && progress !== null) { + } else if (type === 'progress') { + if (message) { + onProgress?.(new FfmpegProgress(stage, progress || 0, message)); + } else if (stage !== 'loading' && progress !== null) { + onProgress?.(new FfmpegProgress(stage, progress || 0, message)); + } + } else if (type === 'log') {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/ffmpeg.js` around lines 83 - 87, The progress handler can call onProgress twice because both branches for type === 'progress' overlap; update the first branch condition so they are mutually exclusive (e.g., only treat it as the "message-only" case when message is present AND either stage === 'loading' or progress is null), leaving the second branch for numeric-progress cases (stage != 'loading' && progress !== null); modify the conditions around the onProgress calls that construct new FfmpegProgress (referencing variables type, message, stage, progress and the onProgress and FfmpegProgress symbols) so only one branch can fire for a single event.js/commandPalette.js-498-505 (1)
498-505:⚠️ Potential issue | 🟡 MinorTypo in theme name:
'machiatto'should be'macchiato'.The label says "Macchiato" but the action sets theme to
'machiatto'(missing a 'c'). This will likely fail to apply the correct theme.📝 Proposed fix
{ id: 'theme-macchiato', group: 'Theme', icon: 'palette', label: 'Theme: Macchiato', keywords: ['theme', 'macchiato', 'catppuccin'], - action: () => this.setTheme('machiatto'), + action: () => this.setTheme('macchiato'), },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/commandPalette.js` around lines 498 - 505, The theme entry with id 'theme-macchiato' has a typo in its action: it calls this.setTheme('machiatto') (missing a 'c'), so change the argument to 'macchiato' to match the label and theme key; update the action in the object where id is 'theme-macchiato' to call this.setTheme('macchiato') so the correct theme is applied.js/playlist-importer.js-416-423 (1)
416-423:⚠️ Potential issue | 🟡 MinorJSDoc typo: missing closing brace in
@returns.📝 Proposed fix
-/** - * Imports CSV result to library - * `@param` {Object} csvResult - Result from parseDynamicCSV - * `@param` {Object} db - Database instance - * `@param` {Function} onProgress - Progress callback - * `@param` {Object} options - Import options - * `@returns` {Promise<Object} - Results summary - */ +/** + * Imports CSV result to library + * `@param` {Object} csvResult - Result from parseDynamicCSV + * `@param` {Object} db - Database instance + * `@param` {Function} onProgress - Progress callback + * `@param` {Object} options - Import options + * `@returns` {Promise<Object>} - Results summary + */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/playlist-importer.js` around lines 416 - 423, The JSDoc for the CSV import function is missing the closing brace in the `@returns` tag; update the comment (the JSDoc block above the function that "Imports CSV result to library") so the `@returns` line reads "@returns {Promise<Object>} - Results summary" (ensure the closing } is added) to fix the typo and keep the JSDoc valid.js/player.js-128-131 (1)
128-131:⚠️ Potential issue | 🟡 MinorThe
setIntervalfor cross-codec ABR evaluation has no cleanup mechanism.This interval runs indefinitely every 3 seconds. If the player is ever re-initialized or destroyed, this could lead to multiple concurrent intervals or memory leaks.
✏️ Proposed fix - store interval reference for cleanup
this.shakaPlayer.addEventListener('variantchanged', this.updateAdaptiveQualityBadge.bind(this)); this.shakaInitialized = false; // Monitor and bridge different codec groups (e.g. AAC to FLAC) since native ABR isolates them - setInterval(this.evaluateCrossCodecAbr.bind(this), 3000); + this.crossCodecAbrInterval = setInterval(this.evaluateCrossCodecAbr.bind(this), 3000); } else {Then add cleanup in an appropriate teardown method:
// In a destroy/cleanup method: if (this.crossCodecAbrInterval) { clearInterval(this.crossCodecAbrInterval); this.crossCodecAbrInterval = null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/player.js` around lines 128 - 131, The setInterval call that repeatedly invokes evaluateCrossCodecAbr every 3s needs a stored interval handle and explicit cleanup to avoid duplicate timers/memory leaks; assign the return value of setInterval to a property (e.g. this.crossCodecAbrInterval) where the interval is created, and in the player teardown/destroy method (e.g. destroy, dispose, or equivalent cleanup function) call clearInterval(this.crossCodecAbrInterval) and set it to null so the interval is removed when the player is re-initialized or destroyed.js/HiFi.ts-975-982 (1)
975-982:⚠️ Potential issue | 🟡 MinorBoolean coercion of
toLowerCase()is always truthy for non-empty strings.
Boolean(qp.adaptive?.toLowerCase())returnstruefor any non-empty string (including"false"). The intent appears to be parsing a boolean query parameter.🐛 Proposed fix
case '/trackManifests': return new TidalResponse( await this.getTrackManifest(Number(qp.id), { ...qp, formats: formats.length > 0 ? formats : undefined, - adaptive: Boolean(qp.adaptive?.toLowerCase()) || undefined, + adaptive: qp.adaptive === 'true' || qp.adaptive === '1' ? true : qp.adaptive === 'false' || qp.adaptive === '0' ? false : undefined, }) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/HiFi.ts` around lines 975 - 982, The adaptive query flag is coerced incorrectly—Boolean(qp.adaptive?.toLowerCase()) treats any non-empty string (including "false") as true; update the case '/trackManifests' call so adaptive is explicitly parsed from qp.adaptive (e.g., use qp.adaptive?.toLowerCase() === 'true' ? true : undefined) before passing into getTrackManifest, referencing qp.adaptive and the getTrackManifest call to locate the change; ensure the existing formats logic is preserved.js/ModernSettings.ts-154-159 (1)
154-159:⚠️ Potential issue | 🟡 MinorEmpty catch block silently swallows database errors.
If
db.getSettingthrows an error other than "key not found", it will be silently ignored. Consider logging the error or only catching specific expected exceptions.✏️ Proposed fix
try { value = (await db.getSetting(backingKey ?? key)) ?? defaultValue; - } catch { + } catch (err) { + console.warn(`Failed to load setting "${backingKey ?? key}":`, err); value = defaultValue; } - }).catch(console.trace); + }).catch(console.error);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/ModernSettings.ts` around lines 154 - 159, The try/catch around db.getSetting in the async block (where value is assigned from (await db.getSetting(backingKey ?? key)) ?? defaultValue) is swallowing all errors; update the catch to either filter for the expected "key not found" error and fall back to defaultValue, or log unexpected errors before falling back. Specifically, modify the catch to inspect the thrown error (from db.getSetting) and if it is the known missing-key case set value = defaultValue, otherwise call the module logger (or console.error) with the error and context (key/backingKey) and then set value = defaultValue; ensure this change is applied in the same async block and preserve the outer .catch(console.trace) behavior.js/HiFi.ts-876-992 (1)
876-992:⚠️ Potential issue | 🟡 MinorThe
querymethod lacks signal propagation for several routes.The
signalparameter is passed to the method but not forwarded to several internal calls likegetInfo,getTrack,getRecommendations,getMix,getLyrics,getVideo,getTopVideos,getWidevine, andgetTrackManifest. This prevents proper cancellation.✏️ Example fix for a few routes
case '/info': - return new TidalResponse(await this.getInfo(Number(qp.id))); + return new TidalResponse(await this.getInfo(Number(qp.id), signal)); case '/track': - return new TidalResponse(await this.getTrack(Number(qp.id), qp.quality || undefined)); + return new TidalResponse(await this.getTrack(Number(qp.id), qp.quality || undefined, false, signal));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/HiFi.ts` around lines 876 - 992, The query method accepts an AbortSignal but does not forward it to many downstream calls, so update the switch cases to pass the signal through to the internal methods that support cancellation (e.g. call this.getInfo(..., signal), this.getTrack(..., signal), this.getRecommendations(..., signal), this.getMix(..., signal), this.getLyrics(..., signal), this.getVideo(..., signal), this.getTopVideos({... , signal}), this.getWidevine(signal) and include signal in the options passed to this.getTrackManifest(..., { ..., signal }) ); locate these calls in query and add the signal argument or include it in the options object so cancellations propagate correctly.styles.css-1773-1779 (1)
1773-1779:⚠️ Potential issue | 🟡 MinorMerge this into the existing
.settings-tab-contentrule.Stylelint already flags this as a duplicate selector against Line 1764, so the stylesheet will keep failing lint until these declarations are folded into the earlier block.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@styles.css` around lines 1773 - 1779, Merge the duplicate .settings-tab-content block into the existing .settings-tab-content rule: move the declarations max-width: 100%, min-width: 0, overflow-x: auto, and overflow-y: visible into the original .settings-tab-content rule (replace or add them there as needed) and remove the later duplicate block so stylelint no longer reports a duplicate selector.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ca55660e-672d-4396-879c-72c300bc9506
⛔ Files ignored due to path filters (45)
assets/icon-background.pngis excluded by!**/*.pngassets/icon-foreground.pngis excluded by!**/*.pngassets/icon-only.pngis excluded by!**/*.pngassets/splash-dark.pngis excluded by!**/*.pngassets/splash.pngis excluded by!**/*.pngimages/animate-spin.svgis excluded by!**/*.svgimages/apple.svgis excluded by!**/*.svgimages/atmos.svgis excluded by!**/*.svgimages/discord.svgis excluded by!**/*.svgimages/facebook.svgis excluded by!**/*.svgimages/genius-active.svgis excluded by!**/*.svgimages/genius-inactive.svgis excluded by!**/*.svgimages/github.svgis excluded by!**/*.svgimages/gitlab.svgis excluded by!**/*.svgimages/google.svgis excluded by!**/*.svgimages/instagram.svgis excluded by!**/*.svgimages/linux.svgis excluded by!**/*.svgimages/mic.svgis excluded by!**/*.svgimages/mix.svgis excluded by!**/*.svgimages/monochrome-logo.svgis excluded by!**/*.svgimages/pause-large.svgis excluded by!**/*.svgimages/pause.svgis excluded by!**/*.svgimages/play-large.svgis excluded by!**/*.svgimages/play.svgis excluded by!**/*.svgimages/recent.svgis excluded by!**/*.svgimages/side-menu.svgis excluded by!**/*.svgimages/sort.svgis excluded by!**/*.svgimages/soundcloud.svgis excluded by!**/*.svgimages/spotify.svgis excluded by!**/*.svgimages/squares.svgis excluded by!**/*.svgimages/twitter.svgis excluded by!**/*.svgimages/windows.svgis excluded by!**/*.svgimages/youtube.svgis excluded by!**/*.svgios/App/App/Assets.xcassets/AppIcon.appiconset/AppIcon-512@2x.pngis excluded by!**/*.pngios/App/App/Assets.xcassets/Splash.imageset/Default@1x~universal~anyany-dark.pngis excluded by!**/*.pngios/App/App/Assets.xcassets/Splash.imageset/Default@1x~universal~anyany.pngis excluded by!**/*.pngios/App/App/Assets.xcassets/Splash.imageset/Default@2x~universal~anyany-dark.pngis excluded by!**/*.pngios/App/App/Assets.xcassets/Splash.imageset/Default@2x~universal~anyany.pngis excluded by!**/*.pngios/App/App/Assets.xcassets/Splash.imageset/Default@3x~universal~anyany-dark.pngis excluded by!**/*.pngios/App/App/Assets.xcassets/Splash.imageset/Default@3x~universal~anyany.pngis excluded by!**/*.pngios/App/App/Assets.xcassets/Splash.imageset/splash-2732x2732-1.pngis excluded by!**/*.pngios/App/App/Assets.xcassets/Splash.imageset/splash-2732x2732-2.pngis excluded by!**/*.pngios/App/App/Assets.xcassets/Splash.imageset/splash-2732x2732.pngis excluded by!**/*.pngpackage-lock.jsonis excluded by!**/package-lock.jsonpublic/lib/butterchurnPresets.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (116)
.env.example.github/dependabot.yml.github/pull_request_template.md.github/workflows/lint.yml.github/workflows/update-lockfile.yml.gitmodules.htmlhintignore.prettierignore.vscode/tasks.jsonCONTRIBUTING.mdDockerfileINSTANCES.mdREADME.mdcapacitor.config.tsdocker-compose.ymlfunctions/album/[id].jsfunctions/artist/[id].jsfunctions/playlist/[id].jsfunctions/podcasts/[id].jsfunctions/track/[id].jsfunctions/upload.jsfunctions/user/@[username].jsfunctions/userplaylist/[id].jsindex.htmlios/.gitignoreios/App/App.xcodeproj/project.pbxprojios/App/App.xcodeproj/project.xcworkspace/xcshareddata/IDEWorkspaceChecks.plistios/App/App.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolvedios/App/App/AppDelegate.swiftios/App/App/Assets.xcassets/AppIcon.appiconset/Contents.jsonios/App/App/Assets.xcassets/Contents.jsonios/App/App/Assets.xcassets/Splash.imageset/Contents.jsonios/App/App/Base.lproj/LaunchScreen.storyboardios/App/App/Base.lproj/Main.storyboardios/App/App/Info.plistios/App/CapApp-SPM/.gitignoreios/App/CapApp-SPM/Package.swiftios/App/CapApp-SPM/README.mdios/App/CapApp-SPM/Sources/CapApp-SPM/CapApp-SPM.swiftios/debug.xcconfigjs/HiFi.tsjs/METADATA_STRINGS.jsjs/ModernSettings.tsjs/accounts/auth.jsjs/accounts/pocketbase.jsjs/api.jsjs/app.jsjs/audio-context.jsjs/bulk-download-writer.tsjs/commandPalette.jsjs/container-classes.tsjs/dash-downloader.tsjs/db.jsjs/doTimed.tsjs/download-utils.tsjs/downloadProgressUtils.jsjs/errorTypes.tsjs/events.jsjs/ffmpeg.jsjs/ffmpeg.types.tsjs/ffmpeg.worker.jsjs/ffmpegFormats.tsjs/global.d.tsjs/haptics.jsjs/hls-downloader.jsjs/icons.tsjs/listenbrainz.jsjs/lyrics.jsjs/metadata.flac.jsjs/metadata.jsjs/mp3-encoder.jsjs/multi-scrobbler.jsjs/music-api.jsjs/platform-detection.tsjs/player.jsjs/playlist-generator.jsjs/playlist-importer.jsjs/podcasts-api.jsjs/profile.jsjs/progressEvents.tsjs/qobuz-api.jsjs/readableStreamIterator.tsjs/router.jsjs/settings.jsjs/side-panel.jsjs/smooth-scrolling.jsjs/storage.jsjs/taglib.tsjs/taglib.types.tsjs/taglib.worker.tsjs/themeStore.jsjs/tracker.jsjs/ui-interactions.jsjs/ui.jsjs/utils.jsjs/visualizer.jslicensenginx.confpackage.jsonpublic/assets/icons/icon-128.webppublic/assets/icons/icon-192.webppublic/assets/icons/icon-256.webppublic/assets/icons/icon-48.webppublic/assets/icons/icon-512.webppublic/assets/icons/icon-72.webppublic/assets/icons/icon-96.webppublic/editors-picks.jsonpublic/instances.jsonpublic/manifest.jsonstream-stub.jsstyles.csstest-search.jsvite-plugin-auth-gate.jsvite-plugin-blob.tsvite-plugin-svg-use.tsvite.config.ts
💤 Files with no reviewable changes (7)
- .htmlhintignore
- .prettierignore
- .env.example
- functions/upload.js
- .vscode/tasks.json
- js/smooth-scrolling.js
- js/qobuz-api.js
| class TidalAPI { | ||
| static CLIENT_ID = 'txNoH4kkV41MfH25'; | ||
| static CLIENT_SECRET = 'dQjy0MinCEvxi1O4UmxvxWnDjt4cgHBPw8ll6nYBk98='; | ||
|
|
There was a problem hiding this comment.
Critical: Same hardcoded credentials issue as in track/[id].js.
OAuth credentials are hardcoded here as well. This TidalAPI class is duplicated across multiple files (track/[id].js, album/[id].js, likely others). Both the security issue and the duplication should be addressed.
Consider extracting TidalAPI into a shared module and using environment variables for credentials.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@functions/album/`[id].js around lines 3 - 6, TidalAPI currently contains
hardcoded CLIENT_ID and CLIENT_SECRET constants; extract the TidalAPI class into
a single shared module (e.g., export class TidalAPI from a new tidal-api
module), replace the hardcoded values with process.env.TIDAL_CLIENT_ID and
process.env.TIDAL_CLIENT_SECRET, update all consumers (the duplicated classes in
other files) to import the shared TidalAPI, and ensure you validate the env vars
at module initialization (throw or log a clear error if missing) so credentials
are not embedded in source.
| class TidalAPI { | ||
| static CLIENT_ID = 'txNoH4kkV41MfH25'; | ||
| static CLIENT_SECRET = 'dQjy0MinCEvxi1O4UmxvxWnDjt4cgHBPw8ll6nYBk98='; |
There was a problem hiding this comment.
Don't commit the TIDAL OAuth secret.
This ships a live client secret in plaintext, and the same value is duplicated across the new bot-preview handlers. Anyone who can read the repo can mint tokens with it; move both values into Worker secrets/env and rotate the current secret.
Suggested fix
class TidalAPI {
- static CLIENT_ID = 'txNoH4kkV41MfH25';
- static CLIENT_SECRET = 'dQjy0MinCEvxi1O4UmxvxWnDjt4cgHBPw8ll6nYBk98=';
+ constructor(env) {
+ this.clientId = env.TIDAL_CLIENT_ID;
+ this.clientSecret = env.TIDAL_CLIENT_SECRET;
+ }
async getToken() {
const params = new URLSearchParams({
- client_id: TidalAPI.CLIENT_ID,
- client_secret: TidalAPI.CLIENT_SECRET,
+ client_id: this.clientId,
+ client_secret: this.clientSecret,
grant_type: 'client_credentials',
});
const res = await fetch('https://auth.tidal.com/v1/oauth2/token', {
method: 'POST',
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
- Authorization: 'Basic ' + btoa(`${TidalAPI.CLIENT_ID}:${TidalAPI.CLIENT_SECRET}`),
+ Authorization: 'Basic ' + btoa(`${this.clientId}:${this.clientSecret}`),
},
body: params,
});- api = new TidalAPI();
+ api = new TidalAPI(env);Also applies to: 8-18, 149-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@functions/playlist/`[id].js around lines 3 - 5, The Tidal OAuth client secret
and client ID are hard-coded in the TidalAPI class (static CLIENT_ID / static
CLIENT_SECRET) and duplicated in the bot-preview handlers; remove these
plaintext values and read them from environment/Worker secrets instead (e.g.,
process.env.TIDAL_CLIENT_ID and process.env.TIDAL_CLIENT_SECRET or the
platform-specific secrets API), update the TidalAPI class and the bot-preview
handlers to reference those env variables, ensure the repo no longer contains
the secret, and rotate the exposed secret immediately.
| const PODCAST_API_KEY = 'YU5HMSDYBQQVYDF6QN4P'; | ||
| const PODCAST_API_SECRET = '8hCvpjSL7T$S7^5ftnf5MhqQwYUYVjM^fmUL3Ld$'; |
There was a problem hiding this comment.
Critical: API credentials hardcoded in source code.
The PodcastIndex API key and secret are exposed in the repository. These credentials should be stored as environment variables/secrets and accessed via env.PODCAST_API_KEY and env.PODCAST_API_SECRET.
Hardcoded secrets in version control can be scraped by bots and abused. Consider rotating these credentials after moving them to environment variables.
Suggested fix
-const PODCAST_API_KEY = 'YU5HMSDYBQQVYDF6QN4P';
-const PODCAST_API_SECRET = '8hCvpjSL7T$S7^5ftnf5MhqQwYUYVjM^fmUL3Ld$';
+// Move to Cloudflare Workers secrets/environment variables
async function getAuthHeaders() {
+async function getAuthHeaders(env) {
+ const PODCAST_API_KEY = env.PODCAST_API_KEY;
+ const PODCAST_API_SECRET = env.PODCAST_API_SECRET;
const apiHeaderTime = Math.floor(Date.now() / 1000).toString();Then update the call site:
- const headers = await getAuthHeaders();
+ const headers = await getAuthHeaders(env);📝 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.
| const PODCAST_API_KEY = 'YU5HMSDYBQQVYDF6QN4P'; | |
| const PODCAST_API_SECRET = '8hCvpjSL7T$S7^5ftnf5MhqQwYUYVjM^fmUL3Ld$'; | |
| // Move to Cloudflare Workers secrets/environment variables | |
| async function getAuthHeaders(env) { | |
| const PODCAST_API_KEY = env.PODCAST_API_KEY; | |
| const PODCAST_API_SECRET = env.PODCAST_API_SECRET; | |
| const apiHeaderTime = Math.floor(Date.now() / 1000).toString(); | |
| // ... rest of function | |
| } | |
| // Updated call site: | |
| const headers = await getAuthHeaders(env); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@functions/podcasts/`[id].js around lines 4 - 5, Remove the hardcoded
credentials PODCAST_API_KEY and PODCAST_API_SECRET and read them from
environment variables instead; replace usages of the constants with
process.env.PODCAST_API_KEY and process.env.PODCAST_API_SECRET (or your
runtime's env accessor) and add a runtime check in the handler (e.g., in the
function that calls the PodcastIndex API) to throw or return a clear error if
those env vars are missing so deployment fails fast; rotate the exposed keys
after deploying the change.
| const metaHtml = ` | ||
| <!DOCTYPE html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8"> | ||
| <title>${title}</title> | ||
| <meta name="description" content="${description}"> | ||
| <meta name="theme-color" content="#000000"> | ||
|
|
||
| <meta property="og:site_name" content="Monochrome"> | ||
| <meta property="og:title" content="${title}"> | ||
| <meta property="og:description" content="${description}"> | ||
| <meta property="og:image" content="${imageUrl}"> | ||
| <meta property="og:type" content="website"> | ||
| <meta property="og:url" content="${pageUrl}"> | ||
|
|
||
| <meta name="twitter:card" content="summary_large_image"> | ||
| <meta name="twitter:title" content="${title}"> | ||
| <meta name="twitter:description" content="${description}"> | ||
| <meta name="twitter:image" content="${imageUrl}"> | ||
| </head> | ||
| <body> | ||
| <h1>${title}</h1> | ||
| <p>${description}</p> | ||
| <img src="${imageUrl}" alt="Podcast Cover"> | ||
| </body> | ||
| </html> | ||
| `; |
There was a problem hiding this comment.
XSS vulnerability: User-controlled data interpolated into HTML without escaping.
The title, description, author, and imageUrl values from the PodcastIndex API are directly interpolated into the HTML response without sanitization. A malicious podcast could have a title like </title><script>malicious()</script> which would execute in the bot's context (or any user who directly visits this URL).
Suggested fix
Add an escape function and use it for all interpolated values:
+function escapeHtml(str) {
+ return str
+ .replace(/&/g, '&')
+ .replace(/</g, '<')
+ .replace(/>/g, '>')
+ .replace(/"/g, '"')
+ .replace(/'/g, ''');
+}
// Then in the HTML generation:
- <title>${title}</title>
+ <title>${escapeHtml(title)}</title>
- <meta name="description" content="${description}">
+ <meta name="description" content="${escapeHtml(description)}">
// ... and so on for all user-controlled values🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@functions/podcasts/`[id].js around lines 60 - 87, The HTML template built in
metaHtml directly interpolates untrusted fields (title, description, author,
imageUrl, pageUrl) causing an XSS risk; fix by creating and using a safe
escaping function (e.g., escapeHtml that replaces &,<,>,",',` with entities) for
all text interpolations (title, description, author and any attribute values
like alt), and validate/sanitize URL-like values (imageUrl, pageUrl) before
interpolation (e.g., allow only http(s) and run through encodeURI or strict
whitelist) so every interpolated variable in the metaHtml template is
escaped/validated.
| class TidalAPI { | ||
| static CLIENT_ID = 'txNoH4kkV41MfH25'; | ||
| static CLIENT_SECRET = 'dQjy0MinCEvxi1O4UmxvxWnDjt4cgHBPw8ll6nYBk98='; | ||
|
|
There was a problem hiding this comment.
Critical: Hardcoded OAuth credentials must be moved to environment variables.
Embedding CLIENT_ID and CLIENT_SECRET directly in source code is a significant security risk. These credentials are exposed in version control, build artifacts, and client-side bundles. Even if this is a serverless function, the source may be visible.
Move these to environment variables and access via context.env:
🔒 Proposed fix
class TidalAPI {
- static CLIENT_ID = 'txNoH4kkV41MfH25';
- static CLIENT_SECRET = 'dQjy0MinCEvxi1O4UmxvxWnDjt4cgHBPw8ll6nYBk98=';
+ constructor(env) {
+ this.clientId = env.TIDAL_CLIENT_ID;
+ this.clientSecret = env.TIDAL_CLIENT_SECRET;
+ }
async getToken() {
const params = new URLSearchParams({
- client_id: TidalAPI.CLIENT_ID,
- client_secret: TidalAPI.CLIENT_SECRET,
+ client_id: this.clientId,
+ client_secret: this.clientSecret,
grant_type: 'client_credentials',
});
const res = await fetch('https://auth.tidal.com/v1/oauth2/token', {
method: 'POST',
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
- Authorization: 'Basic ' + btoa(`${TidalAPI.CLIENT_ID}:${TidalAPI.CLIENT_SECRET}`),
+ Authorization: 'Basic ' + btoa(`${this.clientId}:${this.clientSecret}`),
},
body: params,
});Then update usage:
- api = new TidalAPI();
+ api = new TidalAPI(env);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@functions/track/`[id].js around lines 15 - 18, Replace the hardcoded
credentials in the TidalAPI class by reading them from environment variables:
remove static CLIENT_ID and static CLIENT_SECRET constants and instead read
values from context.env (e.g., context.env.TIDAL_CLIENT_ID and
context.env.TIDAL_CLIENT_SECRET) where TidalAPI is constructed or where its
methods are called; validate presence and throw or log a clear error if the env
vars are missing so the function fails fast rather than using embedded secrets.
| const PODCAST_API_KEY = 'YU5HMSDYBQQVYDF6QN4P'; | ||
| const PODCAST_API_SECRET = '8hCvpjSL7T$S7^5ftnf5MhqQwYUYVjM^fmUL3Ld$'; |
There was a problem hiding this comment.
Critical: API credentials hardcoded in source code.
The PodcastIndex API key and secret are exposed in the client-side code. This is a security risk as:
- Anyone can extract these credentials from the browser/bundle
- Credentials cannot be rotated without code changes
- Usage can be abused by malicious actors
Move these to environment variables injected at build time, or proxy requests through your backend server.
🔐 Recommended approach
-const PODCAST_API_KEY = 'YU5HMSDYBQQVYDF6QN4P';
-const PODCAST_API_SECRET = '8hCvpjSL7T$S7^5ftnf5MhqQwYUYVjM^fmUL3Ld$';
+// Inject at build time via environment variables
+const PODCAST_API_KEY = import.meta.env.VITE_PODCAST_API_KEY || '';
+const PODCAST_API_SECRET = import.meta.env.VITE_PODCAST_API_SECRET || '';Or better yet, proxy these requests through your backend to keep secrets server-side.
📝 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.
| const PODCAST_API_KEY = 'YU5HMSDYBQQVYDF6QN4P'; | |
| const PODCAST_API_SECRET = '8hCvpjSL7T$S7^5ftnf5MhqQwYUYVjM^fmUL3Ld$'; | |
| // Inject at build time via environment variables | |
| const PODCAST_API_KEY = import.meta.env.VITE_PODCAST_API_KEY || ''; | |
| const PODCAST_API_SECRET = import.meta.env.VITE_PODCAST_API_SECRET || ''; |
🧰 Tools
🪛 Betterleaks (1.1.1)
[high] 6-6: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/podcasts-api.js` around lines 6 - 7, Replace the hardcoded PODCAST_API_KEY
and PODCAST_API_SECRET constants with values injected at build/runtime or
fetched from your backend: remove the literals for PODCAST_API_KEY and
PODCAST_API_SECRET and read them from environment/config (e.g.,
process.env.PODCAST_API_KEY and process.env.PODCAST_API_SECRET for server-side
code or a build-time injected variable for client bundles), or instead stop
calling the PodcastIndex API directly from the client and route requests through
a backend proxy endpoint (e.g., /api/podcasts) that uses server-side env vars to
sign requests; update any code referencing PODCAST_API_KEY/PODCAST_API_SECRET to
use the new env/config or proxy flow and ensure secrets are never included in
the client bundle or committed to source.
| const fileNameWithoutSpace = file.name.replace(/\s/g, '_'); | ||
| const response = await fetch(`https://worker.uploads.monochrome.qzz.io/${fileNameWithoutSpace}`, { | ||
| method: 'PUT', | ||
| headers: { | ||
| 'x-api-key': 'if_youre_reading_this_fuck_off', | ||
| 'Content-Type': file.type || 'application/octet-stream', | ||
| }, | ||
| body: file, | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| if (response.status === 413) throw new Error('File exceeds 10MB'); | ||
| throw new Error(`Upload failed: ${response.status}`); | ||
| } | ||
|
|
||
| return `https://images.monochrome.qzz.io/${await response.text()}`; |
There was a problem hiding this comment.
Critical: Hardcoded API key exposed in client-side code.
The API key 'if_youre_reading_this_fuck_off' is embedded directly in the client-side JavaScript. This key is visible to anyone who inspects the source code or network requests, allowing unauthorized access to the upload endpoint.
Consider:
- Move API authentication to a backend proxy that holds the key server-side
- Use short-lived tokens generated by your backend
- If the endpoint must be public, implement rate limiting and other abuse prevention on the worker side
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/profile.js` around lines 42 - 57, Replace the hardcoded API key used in
the client-side upload request: remove the literal 'x-api-key' value in the
fetch call in js/profile.js (the block that builds fileNameWithoutSpace and
calls fetch to https://worker.uploads.monochrome.qzz.io/); instead have the
client call your backend (or request a short-lived upload token) which injects
the key server-side, then use that token or proxied endpoint in the fetch;
update references around fileNameWithoutSpace and the fetch/headers logic to
accept a backend-provided token or URL and do NOT keep any secret value in the
client code.
| for (const [key, fmt] of Object.entries(customFormats)) { | ||
| allOptions.push({ value: key, text: fmt.displayName, category: fmt.category }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "References inside js/settings.js:"
rg -n -C1 '\b(customFormats|containerFormats|modernSettings)\b' js/settings.js
echo
echo "Local imports/declarations inside js/settings.js:"
rg -n -C1 'import .*customFormats|import .*containerFormats|import .*modernSettings|\b(const|let|var|class|function)\s+(customFormats|containerFormats|modernSettings)\b' js/settings.js
echo
echo "Definitions or global assignments elsewhere:"
rg -n -C2 'export\s+(const|let|var|class|function)\s+(customFormats|containerFormats|modernSettings)\b|window\.(customFormats|containerFormats|modernSettings)\s*=|globalThis\.(customFormats|containerFormats|modernSettings)\s*=' jsRepository: subby2006/monochrome-android
Length of output: 3926
🏁 Script executed:
rg -n 'customFormats\s*=|containerFormats\s*=|modernSettings\s*=' --type jsRepository: subby2006/monochrome-android
Length of output: 54
🏁 Script executed:
# Also search in HTML files to see how scripts are loaded
find . -name "*.html" -type f | head -20Repository: subby2006/monochrome-android
Length of output: 149
🏁 Script executed:
# Search for where these might be imported/defined in any file
rg -n 'const\s+(customFormats|containerFormats|modernSettings)\s*=' --type jsRepository: subby2006/monochrome-android
Length of output: 54
🏁 Script executed:
cat -n index.html | head -100Repository: subby2006/monochrome-android
Length of output: 5808
🏁 Script executed:
# Search for any assignments to window or globalThis
rg -n 'window\.|globalThis\.' js/settings.js | head -20Repository: subby2006/monochrome-android
Length of output: 1481
🏁 Script executed:
# Search for where these might be fetched or populated dynamically
rg -n 'customFormats|containerFormats|modernSettings' . --type htmlRepository: subby2006/monochrome-android
Length of output: 54
🏁 Script executed:
# See full index.html to understand script loading
tail -n +100 index.html | head -150Repository: subby2006/monochrome-android
Length of output: 7800
🏁 Script executed:
# Find all .js files in repository
find . -name "*.js" -type f | grep -v node_modulesRepository: subby2006/monochrome-android
Length of output: 1720
🏁 Script executed:
# Search ALL .js files for where these variables are defined
rg -n 'customFormats\s*=' . --type js -A 2 -B 2Repository: subby2006/monochrome-android
Length of output: 54
🏁 Script executed:
# Search for these in key utility files
rg -n 'customFormats|containerFormats|modernSettings' js/downloads.js js/ffmpeg.js js/storage.js js/db.js 2>/dev/nullRepository: subby2006/monochrome-android
Length of output: 54
🏁 Script executed:
# Try searching without assignment - maybe they're exported from somewhere
rg -n 'export.*customFormats|export.*containerFormats|export.*modernSettings' . --type jsRepository: subby2006/monochrome-android
Length of output: 54
🏁 Script executed:
# Check if settings.js is an ES module or has any imports at the top
head -50 js/settings.jsRepository: subby2006/monochrome-android
Length of output: 1516
🏁 Script executed:
# Check if these variables are exported from storage.js
rg -n 'customFormats|containerFormats|modernSettings' js/storage.jsRepository: subby2006/monochrome-android
Length of output: 54
🏁 Script executed:
# Check downloads.js and ffmpeg.js for format definitions
rg -n 'customFormats|containerFormats|modernSettings' js/downloads.js js/ffmpeg.jsRepository: subby2006/monochrome-android
Length of output: 54
🏁 Script executed:
# Search entire codebase one more time just to be absolutely sure
rg 'customFormats|containerFormats|modernSettings' . --type js --countRepository: subby2006/monochrome-android
Length of output: 121
🏁 Script executed:
# Check context around references in app.js
rg -n -B 2 -A 2 'customFormats|containerFormats|modernSettings' js/app.jsRepository: subby2006/monochrome-android
Length of output: 376
🏁 Script executed:
# Check context around references in utils.js
rg -n -B 2 -A 2 'customFormats|containerFormats|modernSettings' js/utils.jsRepository: subby2006/monochrome-android
Length of output: 496
🏁 Script executed:
# Let's also check if there's any global initialization or setup file
ls -la | grep -E '\.(html|js)$' | head -20Repository: subby2006/monochrome-android
Length of output: 552
🏁 Script executed:
# Found it! Check if ModernSettings.js exists
ls -la js/ModernSettings.jsRepository: subby2006/monochrome-android
Length of output: 139
🏁 Script executed:
# Now search for customFormats and containerFormats definitions
rg -n 'export.*customFormats|const customFormats|let customFormats|var customFormats' . --type jsRepository: subby2006/monochrome-android
Length of output: 54
🏁 Script executed:
# Search for where customFormats and containerFormats might be defined
find . -name "*.js" -type f | xargs grep -l 'customFormats\s*=' 2>/dev/nullRepository: subby2006/monochrome-android
Length of output: 56
🏁 Script executed:
# Let's see if there's a FFmpeg or formats config file
fd -e js . | grep -i 'format\|ffmpeg\|config'Repository: subby2006/monochrome-android
Length of output: 169
🏁 Script executed:
# Search case-insensitively for ModernSettings
find . -iname "*modernsettings*" -type fRepository: subby2006/monochrome-android
Length of output: 94
🏁 Script executed:
# List files with "modern" in them
find js -iname "*modern*" -type fRepository: subby2006/monochrome-android
Length of output: 92
🏁 Script executed:
# Search for where customFormats and containerFormats might be initialized
rg -n 'customFormats\s*=' . --type js -A 5Repository: subby2006/monochrome-android
Length of output: 54
🏁 Script executed:
# Check if settings.js file has these variables defined anywhere - broader search
rg -n 'customFormats|containerFormats' js/settings.js -B 5 -A 5 | head -80Repository: subby2006/monochrome-android
Length of output: 1138
🏁 Script executed:
# Found it! Check ModernSettings.ts
head -100 js/ModernSettings.tsRepository: subby2006/monochrome-android
Length of output: 3333
🏁 Script executed:
# Search for customFormats and containerFormats in ModernSettings.ts
rg -n 'customFormats|containerFormats' js/ModernSettings.tsRepository: subby2006/monochrome-android
Length of output: 54
🏁 Script executed:
# Check entire ModernSettings.ts
wc -l js/ModernSettings.tsRepository: subby2006/monochrome-android
Length of output: 96
🏁 Script executed:
# Search for where customFormats might be defined - check all files now
rg -n 'const customFormats|let customFormats|var customFormats|customFormats\s*=' . --type ts --type jsRepository: subby2006/monochrome-android
Length of output: 157
🏁 Script executed:
# Found customFormats in ffmpegFormats.ts! Let's check for containerFormats and modernSettings there too
rg -n 'export.*customFormats|export.*containerFormats' js/ffmpegFormats.tsRepository: subby2006/monochrome-android
Length of output: 206
🏁 Script executed:
# Verify settings.js imports - what does it actually import?
head -50 js/settings.js | grep importRepository: subby2006/monochrome-android
Length of output: 547
🏁 Script executed:
# Check if ffmpegFormats is imported anywhere in settings.js
rg -n 'ffmpegFormats' js/settings.jsRepository: subby2006/monochrome-android
Length of output: 54
🏁 Script executed:
# Let's see the full end of the imports in settings.js
sed -n '1,60p' js/settings.jsRepository: subby2006/monochrome-android
Length of output: 2131
Import missing dependencies in js/settings.js.
Lines 849, 914, and 980+ use customFormats, containerFormats, and modernSettings respectively, but none are imported. These are exported from js/ffmpegFormats.ts and js/ModernSettings.ts and must be added to the import statements at the top of the file, or the settings page will fail with ReferenceError when those sections execute.
Add the following imports:
import { customFormats, containerFormats } from './ffmpegFormats.js';
import { modernSettings } from './ModernSettings.js';Also applies to: 914-918, 980-983
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/settings.js` around lines 849 - 850, The settings page references missing
exports causing ReferenceError: add imports for the missing symbols at the top
of js/settings.js — import customFormats and containerFormats from
'./ffmpegFormats.js' and import modernSettings from './ModernSettings.js' so the
code using customFormats (used around the allOptions push), containerFormats
(used in the container formats block), and modernSettings (used for the modern
settings section) can resolve; place these imports with the other module imports
near the file header.
| async function getFileRefFromAudioData( | ||
| audioData: Uint8Array | Blob | File | FileSystemFileHandle | FileSystemFileEntry | ||
| ): Promise<FileRef | null> { | ||
| if (audioData instanceof Blob || audioData instanceof File) { | ||
| const stream = new BlobStream(audioData); | ||
| return await FileRef.open(stream, true, ReadStyle.Average); | ||
| } else if (audioData instanceof FileSystemFileHandle) { | ||
| const stream = await FileSystemFileHandleStream.open(audioData, true); | ||
| return await FileRef.open(stream, true, ReadStyle.Average); | ||
| } else if ('FileSystemFileEntry' in globalThis && audioData instanceof FileSystemFileEntry) { | ||
| const file = await new Promise<File>((resolve) => audioData.file((f) => resolve(f))); | ||
| const stream = new BlobStream(file); | ||
| return await FileRef.open(stream, true, ReadStyle.Average); | ||
| } else if (audioData instanceof Uint8Array) { | ||
| const stream = new ChunkedByteVectorStream(audioData); | ||
| return await FileRef.open(stream, true, ReadStyle.Average); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the file in question
fd -t f "taglib.worker.ts" | head -20Repository: subby2006/monochrome-android
Length of output: 91
🏁 Script executed:
# Read the specific lines mentioned in the review
cat -n js/taglib.worker.ts | sed -n '270,295p'Repository: subby2006/monochrome-android
Length of output: 1373
🏁 Script executed:
# Check if there are other similar guards in the codebase for global API checks
rg "in globalThis" --context=2Repository: subby2006/monochrome-android
Length of output: 1741
🏁 Script executed:
# Search for any references to browser compatibility or known issues with FileSystemFileHandle
rg "FileSystemFileHandle" -B 2 -A 2Repository: subby2006/monochrome-android
Length of output: 3852
🌐 Web query:
does instanceof throw on undefined class javascript
💡 Result:
Yes, the instanceof operator in JavaScript throws a TypeError when the right-hand side (the "class" or constructor) is undefined. The official MDN documentation confirms this behavior: the right-hand side must be a constructor object (callable with a prototype property) or an object with Symbol.hasInstance. When it's undefined, it fails validation and throws "TypeError: invalid 'instanceof' operand 'undefined'" (or similar messages across engines like V8, Firefox, Safari). Examples from MDN: - 42 instanceof 0; → TypeError - x instanceof f; where f is undefined → TypeError This is consistent across JavaScript engines and ECMAScript specifications.
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/invalid_right_hand_side_instanceof_operand
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/instanceof
Add guard for FileSystemFileHandle instanceof check to prevent errors in non-Chromium browsers.
audioData instanceof FileSystemFileHandle throws a TypeError in browsers without the File System Access API (Firefox, Safari, etc.), blocking execution and breaking taglib operations even for basic Uint8Array buffers.
Add the same guard pattern already used for FileSystemFileEntry on line 284:
Fix
- } else if (audioData instanceof FileSystemFileHandle) {
+ } else if ('FileSystemFileHandle' in globalThis && audioData instanceof FileSystemFileHandle) {
const stream = await FileSystemFileHandleStream.open(audioData, true);
return await FileRef.open(stream, true, ReadStyle.Average);📝 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.
| async function getFileRefFromAudioData( | |
| audioData: Uint8Array | Blob | File | FileSystemFileHandle | FileSystemFileEntry | |
| ): Promise<FileRef | null> { | |
| if (audioData instanceof Blob || audioData instanceof File) { | |
| const stream = new BlobStream(audioData); | |
| return await FileRef.open(stream, true, ReadStyle.Average); | |
| } else if (audioData instanceof FileSystemFileHandle) { | |
| const stream = await FileSystemFileHandleStream.open(audioData, true); | |
| return await FileRef.open(stream, true, ReadStyle.Average); | |
| } else if ('FileSystemFileEntry' in globalThis && audioData instanceof FileSystemFileEntry) { | |
| const file = await new Promise<File>((resolve) => audioData.file((f) => resolve(f))); | |
| const stream = new BlobStream(file); | |
| return await FileRef.open(stream, true, ReadStyle.Average); | |
| } else if (audioData instanceof Uint8Array) { | |
| const stream = new ChunkedByteVectorStream(audioData); | |
| return await FileRef.open(stream, true, ReadStyle.Average); | |
| } | |
| async function getFileRefFromAudioData( | |
| audioData: Uint8Array | Blob | File | FileSystemFileHandle | FileSystemFileEntry | |
| ): Promise<FileRef | null> { | |
| if (audioData instanceof Blob || audioData instanceof File) { | |
| const stream = new BlobStream(audioData); | |
| return await FileRef.open(stream, true, ReadStyle.Average); | |
| } else if ('FileSystemFileHandle' in globalThis && audioData instanceof FileSystemFileHandle) { | |
| const stream = await FileSystemFileHandleStream.open(audioData, true); | |
| return await FileRef.open(stream, true, ReadStyle.Average); | |
| } else if ('FileSystemFileEntry' in globalThis && audioData instanceof FileSystemFileEntry) { | |
| const file = await new Promise<File>((resolve) => audioData.file((f) => resolve(f))); | |
| const stream = new BlobStream(file); | |
| return await FileRef.open(stream, true, ReadStyle.Average); | |
| } else if (audioData instanceof Uint8Array) { | |
| const stream = new ChunkedByteVectorStream(audioData); | |
| return await FileRef.open(stream, true, ReadStyle.Average); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/taglib.worker.ts` around lines 275 - 291, The instanceof check for
FileSystemFileHandle in getFileRefFromAudioData can throw in browsers that don't
define that API; wrap that check with a guard like the FileSystemFileEntry
branch does (e.g., check 'FileSystemFileHandle' in globalThis && audioData
instanceof FileSystemFileHandle) so the code only performs the instanceof when
the constructor exists, then open the FileSystemFileHandleStream and FileRef as
before; update the branch that references FileSystemFileHandle accordingly to
avoid TypeError in non‑Chromium browsers.
| function getGitCommitHash() { | ||
| try { | ||
| return execSync('git rev-parse --short HEAD').toString().trim(); | ||
| } catch { | ||
| return 'unknown'; | ||
| } | ||
| } | ||
|
|
||
| export default defineConfig(() => { | ||
| return { | ||
| base: './', | ||
| define: { | ||
| __COMMIT_HASH__: JSON.stringify(commitHash), | ||
| }, |
There was a problem hiding this comment.
Undefined variable commitHash will cause build failure.
getGitCommitHash() is defined but never called. The define block references commitHash which doesn't exist, causing a ReferenceError at build time.
🐛 Proposed fix
function getGitCommitHash() {
try {
return execSync('git rev-parse --short HEAD').toString().trim();
} catch {
return 'unknown';
}
}
+const commitHash = getGitCommitHash();
+
export default defineConfig(() => {
return {
base: './',
define: {
__COMMIT_HASH__: JSON.stringify(commitHash),
},📝 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.
| function getGitCommitHash() { | |
| try { | |
| return execSync('git rev-parse --short HEAD').toString().trim(); | |
| } catch { | |
| return 'unknown'; | |
| } | |
| } | |
| export default defineConfig(() => { | |
| return { | |
| base: './', | |
| define: { | |
| __COMMIT_HASH__: JSON.stringify(commitHash), | |
| }, | |
| function getGitCommitHash() { | |
| try { | |
| return execSync('git rev-parse --short HEAD').toString().trim(); | |
| } catch { | |
| return 'unknown'; | |
| } | |
| } | |
| const commitHash = getGitCommitHash(); | |
| export default defineConfig(() => { | |
| return { | |
| base: './', | |
| define: { | |
| __COMMIT_HASH__: JSON.stringify(commitHash), | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vite.config.ts` around lines 10 - 23, The define block references an
undefined variable commitHash causing build failure; call the existing
getGitCommitHash() and assign its result to commitHash before returning the
config so define: { __COMMIT_HASH__: JSON.stringify(commitHash) } uses a real
value—e.g., invoke getGitCommitHash() at the start of the default export (or
inline the call) so commitHash is defined when define is constructed.
19 participants
Description
Type of Change
Checklist
By submitting this PR, I agree to follow the guidelines. I understand that the final decision to merge rests with the maintainers and that not all contributions can be accepted.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Chores