feat: Cache MPRIS artwork from HTTPS URLs to local files and update t… by josueygp · Pull Request #33 · wimpysworld/sidra

josueygp · 2026-03-24T00:01:58Z

Description

This Pull Request overhauls the MPRIS artwork integration to ensure compatibility with strict desktop environments like GNOME Shell, which previously failed to display any album art from Sidra. The fix targets three core architectural issues related to how the D-Bus Metadata payload is handled.

Key Changes & Justification

Fixed Metadata Property Name (xesam:artUrl → mpris:artUrl)
According to the official MPRIS v2 D-Bus Interface Specification, while most track information utilizes the xesam: ontology (like xesam:title, xesam:artist), the album art URL is an exception and must be populated under the mpris:artUrl key. The previous code was incorrectly assigning it to xesam:artUrl, causing strict MPRIS clients to discard the property entirely.
Implemented Local Image Caching (file:// URIs)
Many Linux desktop environments (most notably GNOME's built-in media controls) deliberately reject remote https:// URLs for security and performance reasons.
- This PR implements asynchronous downloading of the artwork from the remote CDN to app.getPath('userData')/art (e.g., ~/.config/sidra/art).
- The original https:// URL is still emitted immediately as a fallback for more tolerant environments like KDE Plasma.
- Once the image is cached, the metadata is seamlessly updated with a compliant file:///... URI.
- We specifically use userData instead of cache to prevent AppImage FUSE mounting bugs, ensuring the host OS can successfully traverse and read the cached image path.
Resolved GNOME Shell's Premature Caching Bug
GNOME Shell contains an aggressive optimization behavior where it ignores subsequent D-Bus metadata updates if the mpris:trackid remains unchanged. To bypass this, the onNowPlayingItemDidChange property emission is now smartly wrapped in a setMetadata() orchestrator. The initial PropertiesChanged payload is emitted only after the local artwork is verified or downloaded, guaranteeing GNOME receives the complete payload (with the local file:// URI) on its very first read.

Testing

Verified on Linux (AppImage).
Confirmed instantaneous artwork rendering in GNOME Shell's top bar and extensions (like Dynamic Music Pill) without missing the cache hit.
Confirmed fallback compatibility if the client allows internet CDNs.

…he metadata key to `mpris:artUrl`.

cubic-dev-ai

3 issues found across 1 file

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/integrations/mpris/index.ts">

<violation number="1" location="src/integrations/mpris/index.ts:645">
P2: Artwork cache file extension is derived unsafely from full URL, allowing path separators/traversal-like tokens in generated cache paths.</violation>

<violation number="2" location="src/integrations/mpris/index.ts:652">
P2: Async artwork callbacks can race and overwrite MPRIS metadata/trackId with stale track data because no track-change guard is checked before calling setMetadata.</violation>

<violation number="3" location="src/integrations/mpris/index.ts:657">
P2: HTTPS artwork download path lacks timeout and stream/response error handling, risking hangs and unhandled stream failures.</violation>
</file>

Since this is your first cubic review, here's how it works:

cubic automatically reviews your code and comments on bugs and improvements
Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
Add one-off context when rerunning by tagging @cubic-dev-ai with guidance or docs links (including llms.txt)
Ask questions if you need clarification on any suggestion

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

cubic-dev-ai · 2026-03-24T00:14:30Z

src/integrations/mpris/index.ts

+        require('fs').mkdirSync(cacheDir, { recursive: true });
+      }
+      const hash = crypto.createHash('md5').update(p.artworkUrl).digest('hex');
+      const ext = p.artworkUrl.split('.').pop()?.split('?')[0] || 'jpg';


P2: Artwork cache file extension is derived unsafely from full URL, allowing path separators/traversal-like tokens in generated cache paths.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/integrations/mpris/index.ts, line 645: <comment>Artwork cache file extension is derived unsafely from full URL, allowing path separators/traversal-like tokens in generated cache paths.</comment> <file context> @@ -609,14 +622,62 @@ function onNowPlayingItemDidChange(payload: unknown): void { + require('fs').mkdirSync(cacheDir, { recursive: true }); + } + const hash = crypto.createHash('md5').update(p.artworkUrl).digest('hex'); + const ext = p.artworkUrl.split('.').pop()?.split('?')[0] || 'jpg'; + const filepath = path.join(cacheDir, `sidra-art-${hash}.${ext}`); + const fileUrl = `file://${filepath}`; </file context>

cubic-dev-ai · 2026-03-24T00:14:30Z

src/integrations/mpris/index.ts

+      fsp.stat(filepath)
+        .then(() => {
+          metadata['mpris:artUrl'] = new Variant('s', fileUrl);
+          setMetadata();


P2: Async artwork callbacks can race and overwrite MPRIS metadata/trackId with stale track data because no track-change guard is checked before calling setMetadata.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/integrations/mpris/index.ts, line 652: <comment>Async artwork callbacks can race and overwrite MPRIS metadata/trackId with stale track data because no track-change guard is checked before calling setMetadata.</comment> <file context> @@ -609,14 +622,62 @@ function onNowPlayingItemDidChange(payload: unknown): void { + fsp.stat(filepath) + .then(() => { + metadata['mpris:artUrl'] = new Variant('s', fileUrl); + setMetadata(); + }) + .catch(() => { </file context>

cubic-dev-ai · 2026-03-24T00:14:31Z

src/integrations/mpris/index.ts

+        .catch(() => {
+          https.get(p.artworkUrl as string, (res: any) => {
+            if (res.statusCode === 200) {
+              const stream = createWriteStream(filepath);


P2: HTTPS artwork download path lacks timeout and stream/response error handling, risking hangs and unhandled stream failures.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/integrations/mpris/index.ts, line 657: <comment>HTTPS artwork download path lacks timeout and stream/response error handling, risking hangs and unhandled stream failures.</comment> <file context> @@ -609,14 +622,62 @@ function onNowPlayingItemDidChange(payload: unknown): void { + .catch(() => { + https.get(p.artworkUrl as string, (res: any) => { + if (res.statusCode === 200) { + const stream = createWriteStream(filepath); + res.pipe(stream); + stream.on('finish', () => { </file context>

flexiondotorg

Thanks for this, the fixes look useful and are welcome.

I'd ask that you split this into separate, focused pull requests, one per concern:

Fix MPRIS metadata property name (xesam:artUrl → mpris:artUrl)
Add local artwork caching (download HTTPS URLs to file:// URIs)
Resolve GNOME Shell premature caching (orchestrate metadata emission after artwork is ready)

Each change is independently reviewable and revertable, which keeps the git history clean and makes bisecting easier. Please reference this PR in each new one for context, but I'll close this pull request.

feat: Cache MPRIS artwork from HTTPS URLs to local files and update t…

0072a16

…he metadata key to `mpris:artUrl`.

cubic-dev-ai bot reviewed Mar 24, 2026

View reviewed changes

flexiondotorg reviewed Mar 24, 2026

View reviewed changes

flexiondotorg closed this Mar 24, 2026

This was referenced Mar 24, 2026

fix(mpris): Rename xesam:artUrl to compliant mpris:artUrl #37

Merged

Feat: local artwork cache #38

Closed

Fix: gnome premature caching #39

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Cache MPRIS artwork from HTTPS URLs to local files and update t…#33

feat: Cache MPRIS artwork from HTTPS URLs to local files and update t…#33
josueygp wants to merge 1 commit intowimpysworld:mainfrom
josueygp:main

josueygp commented Mar 24, 2026

Uh oh!

cubic-dev-ai bot left a comment

Uh oh!

cubic-dev-ai bot Mar 24, 2026

Uh oh!

cubic-dev-ai bot Mar 24, 2026

Uh oh!

cubic-dev-ai bot Mar 24, 2026

Uh oh!

flexiondotorg left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

josueygp commented Mar 24, 2026

Description

Key Changes & Justification

Testing

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

flexiondotorg left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants