media/02-album-art#33
Conversation
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
ec77763 to
3a492d6
Compare
3a492d6 to
2eecd49
Compare
Greptile SummaryAdds end-to-end Windows album-art support for media widgets.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported session-removal, transient-refresh, oversized-artwork, and cross-session artwork paths now resolve without exposing stale artwork or indefinitely withholding replacement metadata.
|
| Filename | Overview |
|---|---|
| host/src/windows_providers.cpp | Adds SMTC dirty-event coalescing, thumbnail reading and normalization, bounded refresh retries, and explicit session/artwork outcomes. |
| host/src/media.zig | Integrates artwork outcomes with media polling while preventing artwork from crossing session boundaries and retaining cache pins across transient failures. |
| host/src/art_cache.zig | Implements bounded hash-deduplicated artwork storage with atomic publication, temporary-file cleanup, pinning, and LRU pruning. |
| runtime/src/image_paths.zig | Adds canonicalization and containment validation for widget assets and host-owned cache images. |
| runtime/src/main.zig | Connects validated image paths and dynamic source updates to runtime image registration. |
| sdk/src/reconciler.ts | Updates image reconciliation so dynamic sources re-register after mutating render turns while unchanged static trees remain idle. |
| examples/now-playing/widget.tsx | Expands the Now Playing layout and renders optional album artwork with a placeholder fallback. |
Sequence Diagram
sequenceDiagram
participant SMTC as Windows SMTC
participant Host as Media Provider
participant Cache as Artwork Cache
participant Runtime as Widget Runtime
participant Widget as Now Playing Widget
SMTC->>Host: Session or properties event
Host->>Host: Coalesce event into 1 Hz poll
Host->>Host: Read and normalize thumbnail
alt Artwork available
Host->>Cache: Hash and atomically publish
Cache-->>Host: Complete contained art path
else Confirmed unavailable
Host->>Host: Resolve frame without stale artwork
end
Host->>Runtime: Media frame with optional artPath
Runtime->>Runtime: Canonicalize and validate path
Runtime->>Widget: Register image and render frame
Reviews (6): Last reviewed commit: "fix: bound unresolved artwork replacemen..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (8)
host/src/media.zig (1)
100-102: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winZero-initialize
artworkbefore handing it to the C boundary.
weaver_media_pollreturns-1on its null-argument check before thememset, so the unconditionaldefer weaver_media_artwork_release(&artwork)woulddelete[]a garbage pointer. That path is unreachable today (session/output/artwork are all non-null here), but the safety of anundefinedstruct crossing into C depends entirely on the callee's internal ordering. One line removes the coupling.🛡️ Proposed change
- var artwork: native.WeaverMediaArtwork = undefined; + var artwork: native.WeaverMediaArtwork = std.mem.zeroes(native.WeaverMediaArtwork); const result = native.weaver_media_poll(self.session, &source, &artwork); defer native.weaver_media_artwork_release(&artwork);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@host/src/media.zig` around lines 100 - 102, Zero-initialize the artwork variable in the media polling flow before passing it to native.weaver_media_poll, replacing the undefined initialization while preserving the existing release defer and result handling.host/src/art_cache.zig (1)
128-150: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueHoist the directory open/iterate out of the prune loop.
Each deletion reopens the root and re-
statFiles every entry, so overshoot by k files costs k full directory scans. Collecting(name, mtime)once and deleting the oldest non-pinned entries keeps this to a single scan, which matches the low-CPU goal for a 1 Hz poll path.As per coding guidelines, "Optimize for very low memory and CPU usage."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@host/src/art_cache.zig` around lines 128 - 150, Refactor Cache.prune so it opens and iterates the root directory only once, collecting each eligible non-pinned image filename with its modification time. Sort or select these entries by oldest mtime, then delete only the required excess files without repeating statFile calls or directory scans; preserve the ArtCachePinExceedsLimit error when insufficient deletable files exist.Source: Coding guidelines
runtime/src/image_paths.zig (2)
125-149: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe resolver test skips everywhere except Windows despite being platform-neutral.
Directory creation,
realPathFileAlloc, and containment all work on POSIX; only the case-insensitive comparison differs. Dropping the skip would give the canonical-root acceptance path coverage on non-Windows CI too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@runtime/src/image_paths.zig` around lines 125 - 149, Remove the Windows-only skip from the test “resolver accepts only a real file under the canonical host art root” so it runs on all supported platforms. Preserve the existing setup and assertions, including the platform-specific path comparison behavior handled by Resolver.
107-123: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
windowsLexicallyContainedis only exercised by its own test, not byresolve.Production containment goes through
validAbsoluteArtPath+hasParentComponent+componentContainedinsideresolve, so the traversal/UNC/device-namespace guarantees this test asserts are proven for a helper that never runs. Either routeresolve's host-art branch through this helper, or drop it and assert the same cases againstResolver.resolve.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@runtime/src/image_paths.zig` around lines 107 - 123, Ensure the production resolve path exercises the Windows containment logic tested by windowsLexicallyContained. Route resolve’s host-art branch through windowsLexicallyContained, or remove that helper and add equivalent traversal, UNC, and device-namespace assertions using Resolver.resolve while preserving existing containment behavior.runtime/src/main.zig (3)
289-289: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
drainProviderFramestakes an ignored*Effects.All three call sites were updated to pass
effects, but the body discards it. Either use it (e.g.syncTimersafter a drain) or revert the signature.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@runtime/src/main.zig` at line 289, Update drainProviderFrames to either use the passed *Effects value, such as synchronizing timers after draining frames, or remove the parameter and adjust all three call sites accordingly. Avoid retaining an ignored effects argument.
807-818: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
seedImageStatesmakes a startup resolve failure fatal for the widget.Any error from
resolver.resolveon a bundled asset (unexpected symlink layout, canonicalization quirk) propagates out ofmainand the widget never starts, even thoughloadLocalImagesalready read the bytes successfully andsynchronizeImageshandles the same errors gracefully at runtime. Logging and skipping the seed for that node would keep startup resilient.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@runtime/src/main.zig` around lines 807 - 818, Update seedImageStates to handle resolver.resolve failures per image instead of propagating them: log the error, skip seeding that node, and continue processing remaining images. Preserve successful state initialization and image_tree_generation updates, matching the resilient error handling used by synchronizeImages.
851-859: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSmall path strings on
std.heap.page_allocatorcost a full page each.
rememberRawSourceand the resolved paths held inImageState.observedare a few hundred bytes but each takes a page-granular mapping (up to 16 live, re-allocated on every track change). A small fixed inline buffer ([max_art_path_bytes]u8is already the contract bound) or a shared arena would remove both the footprint and the map/unmap churn on the 1 Hz media path.As per coding guidelines, "Optimize for very low memory and CPU usage."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@runtime/src/main.zig` around lines 851 - 859, Update ImageState path storage and the replaceObserved/rememberRawSource flow to avoid page_allocator allocations for short paths. Use the existing max_art_path_bytes contract with inline fixed-capacity storage, or a shared arena, while preserving replacement, null termination, and observed_valid behavior across track changes.Source: Coding guidelines
host/src/windows_host.zig (1)
617-620: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winArt-cache setup is fatal on both sides of the wiring. Artwork is an optional enhancement, but a filesystem or permission problem with the cache root aborts process startup in both the host and the widget runtime, taking down CPU/memory/audio widgets and non-artwork rendering with it.
host/src/windows_host.zig#L617-L620: replacetry art_cache.Cache.init(...)with acatchthat logs, leavesmedia_provider.cacheunset, and skips exportingWEAVER_ART_CACHE.runtime/src/main.zig#L1024-L1030: when the failure comes from theWEAVER_ART_CACHEroot (error.InvalidArtCacheRoot, canonicalization errors), fall back to constructing the resolver with a null art root instead of returning frommain; keep failures on the widget directory itself fatal.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@host/src/windows_host.zig` around lines 617 - 620, The art-cache initialization failure must no longer abort startup. In host/src/windows_host.zig lines 617-620, update the Cache.init call to catch and log errors, leave media_provider.cache unset, and skip exporting WEAVER_ART_CACHE; in runtime/src/main.zig lines 1024-1030, treat WEAVER_ART_CACHE root errors such as error.InvalidArtCacheRoot and canonicalization failures by constructing the resolver with a null art root, while keeping widget-directory failures fatal.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/media-run-status.md`:
- Around line 66-72: Reconcile the Layer 02 status between the recorded gate
results and the “Next executable task” section: state whether the complete
layer-02 gate has passed and only F9 commit/push remains, or name the specific
outstanding gate. Update the task wording to match that status while preserving
the subsequent layer-03 restack and remediation steps.
In `@host/src/art_cache.zig`:
- Line 110: Update the publish flow around self.prune() so pruning errors are
treated as non-fatal after the file is durable, renamed, and pinned. Preserve
the successful publication and return success to applyArtwork in media.zig,
while retaining the existing prune behavior for best-effort housekeeping.
- Around line 63-68: Update the hash formatting in the digest-to-filename flow
around digest and hash to pass the SHA-256 bytes as a slice through
std.fmt.fmtSliceHexLower, ensuring hash receives the lowercase hexadecimal
representation before constructing the .img filename.
In `@host/src/windows_providers.cpp`:
- Around line 368-371: Update host/src/windows_providers.cpp lines 368-371 in
read_thumbnail to set too_large and changed, then return true for oversized
artwork so it is treated as a definite unavailable-artwork result; update
host/src/media.zig lines 148-153 in the media polling flow to handle too_large
before the changed == 0 or refresh_failed guard, preserving the warning and
artwork-retention path.
- Around line 366-378: Bound the asynchronous waits in the artwork-loading flow
around OpenReadAsync and LoadAsync so neither .get() can block the host poll
thread indefinitely. Apply a finite timeout and route timeout or
failed-operation results through the existing refresh_failed path, preserving
prior artwork and retry behavior; keep the current size checks and successful
loading behavior unchanged.
- Around line 286-301: Update the callback setup associated with
WeaverMediaSession so CurrentSessionChanged and MediaPropertiesChanged do not
capture a raw WeaverMediaSession pointer. Capture shared ownership or a
std::weak_ptr, lock it before handling each event, and return when the session
is already destroyed; ensure destruction and handler revocation cannot leave
late callbacks accessing freed state.
In `@runtime/src/image_paths.zig`:
- Around line 24-30: Change the resolved path types in the image path handling,
including Resolved.path, widget_root, and art_root, from [:0]u8 to []u8 to match
the return types of realPathFileAlloc and realPathFileAbsoluteAlloc. Preserve
the existing allocation and allocator.free cleanup behavior.
In `@runtime/src/main.zig`:
- Around line 897-914: Track transient reload failures separately from
successful observations in the image reload state. Update both read and register
failure paths near effects.registerImageBytes to mark the path as failed rather
than calling replaceObserved with a valid observation, and gate the
observedEquals fast path in the reload function on !state.load_failed. Clear the
failure state after a successful load, with bounded retry behavior for
permanently unreadable paths.
---
Nitpick comments:
In `@host/src/art_cache.zig`:
- Around line 128-150: Refactor Cache.prune so it opens and iterates the root
directory only once, collecting each eligible non-pinned image filename with its
modification time. Sort or select these entries by oldest mtime, then delete
only the required excess files without repeating statFile calls or directory
scans; preserve the ArtCachePinExceedsLimit error when insufficient deletable
files exist.
In `@host/src/media.zig`:
- Around line 100-102: Zero-initialize the artwork variable in the media polling
flow before passing it to native.weaver_media_poll, replacing the undefined
initialization while preserving the existing release defer and result handling.
In `@host/src/windows_host.zig`:
- Around line 617-620: The art-cache initialization failure must no longer abort
startup. In host/src/windows_host.zig lines 617-620, update the Cache.init call
to catch and log errors, leave media_provider.cache unset, and skip exporting
WEAVER_ART_CACHE; in runtime/src/main.zig lines 1024-1030, treat
WEAVER_ART_CACHE root errors such as error.InvalidArtCacheRoot and
canonicalization failures by constructing the resolver with a null art root,
while keeping widget-directory failures fatal.
In `@runtime/src/image_paths.zig`:
- Around line 125-149: Remove the Windows-only skip from the test “resolver
accepts only a real file under the canonical host art root” so it runs on all
supported platforms. Preserve the existing setup and assertions, including the
platform-specific path comparison behavior handled by Resolver.
- Around line 107-123: Ensure the production resolve path exercises the Windows
containment logic tested by windowsLexicallyContained. Route resolve’s host-art
branch through windowsLexicallyContained, or remove that helper and add
equivalent traversal, UNC, and device-namespace assertions using
Resolver.resolve while preserving existing containment behavior.
In `@runtime/src/main.zig`:
- Line 289: Update drainProviderFrames to either use the passed *Effects value,
such as synchronizing timers after draining frames, or remove the parameter and
adjust all three call sites accordingly. Avoid retaining an ignored effects
argument.
- Around line 807-818: Update seedImageStates to handle resolver.resolve
failures per image instead of propagating them: log the error, skip seeding that
node, and continue processing remaining images. Preserve successful state
initialization and image_tree_generation updates, matching the resilient error
handling used by synchronizeImages.
- Around line 851-859: Update ImageState path storage and the
replaceObserved/rememberRawSource flow to avoid page_allocator allocations for
short paths. Use the existing max_art_path_bytes contract with inline
fixed-capacity storage, or a shared arena, while preserving replacement, null
termination, and observed_valid behavior across track changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ec87d9d1-3e11-489e-8c0e-251262654ae3
⛔ Files ignored due to path filters (3)
docs/media-evidence/pr02-now-playing-paused.pngis excluded by!**/*.pngdocs/media-evidence/pr02-now-playing-track-change.pngis excluded by!**/*.pngdocs/media-evidence/pr02-now-playing.pngis excluded by!**/*.png
📒 Files selected for processing (16)
docs/media-evidence/pr02-visual.mddocs/media-run-status.mdexamples/now-playing/widget.tsxhost/build.zighost/src/art_cache.zighost/src/media.zighost/src/windows_host.zighost/src/windows_providers.cpphost/src/windows_providers.hruntime/src/image_paths.zigruntime/src/main.zigruntime/src/tree.zigsdk/CONTRACT.mdsdk/index.d.tssdk/src/reconciler.tssdk/test/reconciler.test.mjs
| The 15-finding adversarial remediation is in progress bottom-up. F9 is fixed | ||
| and tested in layer 02; its complete per-layer gate and publication are next. | ||
|
|
||
| ## Next executable task | ||
|
|
||
| Create `media/02-album-art` from layer 01 and trace SMTC thumbnail ownership, | ||
| host cache lifecycle, runtime path containment, and dynamic image | ||
| re-registration before editing. | ||
| Run the complete layer-02 gate, commit/push F9, then restack layer 03 and fix | ||
| F1/F2/F3/F4/F10/F11/F12 at the transport-owning layer. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reconcile the Layer 02 gate status.
Lines 36-55 record the Layer 02 gates as passing, but lines 66-72 still say the complete gate is pending. Clarify whether only F9 commit/push remains, or identify the specific gate still outstanding.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/media-run-status.md` around lines 66 - 72, Reconcile the Layer 02 status
between the recorded gate results and the “Next executable task” section: state
whether the complete layer-02 gate has passed and only F9 commit/push remains,
or name the specific outstanding gate. Update the task wording to match that
status while preserving the subsequent layer-03 restack and remediation steps.
| var digest: [std.crypto.hash.sha2.Sha256.digest_length]u8 = undefined; | ||
| std.crypto.hash.sha2.Sha256.hash(bytes, &digest, .{}); | ||
| var hash: [hash_hex_bytes]u8 = undefined; | ||
| _ = try std.fmt.bufPrint(&hash, "{x}", .{digest}); | ||
| var filename_buffer: [hash_hex_bytes + ".img".len]u8 = undefined; | ||
| const filename = try std.fmt.bufPrint(&filename_buffer, "{s}.img", .{hash}); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Zig std.fmt {x} format specifier byte array vs slice hex formatting 0.15
💡 Result:
In Zig, including version 0.15, the {x} and {X} format specifiers are intended strictly for numeric types (such as integers), where they output the numeric value in hexadecimal notation [1][2][3]. Using {x} or {X} with slices of bytes (e.g., []u8) is not supported and will result in a compile-time error [4][5]. Specifically, the standard library includes checks that will explicitly trigger a compile error if you attempt to use these specifiers with types that are not integers, advising you to use alternative formatting functions [4][5]. To format a byte slice or byte array as a hexadecimal string, you must use the following dedicated functions provided in std.fmt: 1. std.fmt.fmtSliceHexLower: Formats bytes as lowercase hexadecimal [1][2][6]. 2. std.fmt.fmtSliceHexUpper: Formats bytes as uppercase hexadecimal [1][2][6]. Example usage: const std = @import("std"); test "hex formatting" { const data = "Zig!"; // Incorrect: try std.debug.print("{x}",.{data}); // Causes compile error // Correct: try std.debug.print("{}\n",.{std.fmt.fmtSliceHexLower(data)}); } This separation was implemented to avoid ambiguity, as the same specifier previously attempted to handle two fundamentally different types of data (single integers vs. collections of bytes), which led to confusion and design limitations [4]. Using the explicit function wrappers ensures clear, type-safe formatting [4].
Citations:
- 1: https://github.com/Sobeston/zig.guide/blob/master/website/versioned_docs/version-0.15.x/02-standard-library/14-formatting-specifiers.mdx
- 2: https://zig.guide/standard-library/formatting-specifiers/
- 3: https://github.com/ziglang/zig/blob/905c85be968e6823305887474d2a821dac82f16c/lib/std/fmt.zig
- 4: https://ziggit.dev/t/is-x-format-deprecated-or-is-it-not/2207
- 5: https://github.com/ziglang/zig/blob/6d1f0eca773e688c802e441589495b7bde2f9e3f/lib/std/fmt.zig
- 6: https://zig.guide/0.12/standard-library/formatting-specifiers/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Zig-related files ==\n'
git ls-files | rg '(^|/)(build\.zig|build\.zig\.zon|zig\.version|\.zig-version|host/src/art_cache\.zig)$' || true
printf '\n== build.zig excerpt ==\n'
if [ -f build.zig ]; then
cat -n build.zig | sed -n '1,220p'
fi
printf '\n== build.zig.zon excerpt ==\n'
if [ -f build.zig.zon ]; then
cat -n build.zig.zon | sed -n '1,220p'
fi
printf '\n== host/src/art_cache.zig excerpt ==\n'
cat -n host/src/art_cache.zig | sed -n '55,80p'
printf '\n== std.fmt references in repo ==\n'
rg -n 'fmtSliceHexLower|fmtSliceHexUpper|\\{x\\}' -g '!**/dist/**' -g '!**/build/**' -g '!**/node_modules/**' .Repository: SunkenInTime/weaver
Length of output: 1930
🌐 Web query:
Zig std.fmt "{x}" fixed-size byte array [32]u8 compile error fmtSliceHexLower
💡 Result:
The compile error occurs because the {x} and {X} format specifiers are no longer supported for slices or byte arrays in Zig [1][2]. Previously, these specifiers could be used to print bytes as hexadecimal, but this behavior was deprecated and removed to avoid ambiguity and provide clearer formatting options [1][2]. To print a byte array or slice as hexadecimal, you must explicitly use the std.fmt.fmtSliceHexLower or std.fmt.fmtSliceHexUpper formatter [1][2]. Here is the correct way to print a fixed-size byte array: const std = @import("std"); test "print hex" { const bytes: u8 = []u8{0xAB, 0xCD, 0xEF} ++ ([]u8{0} 29); // Use {} as the format specifier and pass the hex formatter as the argument std.debug.print("{}\n",.{std.fmt.fmtSliceHexLower(&bytes)}); } By passing std.fmt.fmtSliceHexLower(&bytes) into the format string with {}, you ensure the byte slice is correctly interpreted and formatted as a lowercase hexadecimal string [3][4][2]. Note that if you are passing a fixed-size array, you should take a slice of it (e.g., &array or array[0..]) to pass to the formatter [4][2].
Citations:
- 1: https://ziggit.dev/t/is-x-format-deprecated-or-is-it-not/2207
- 2: add format option
{xs}/{Xs}to print u8 arrays as bytes ziglang/zig#17611 - 3: https://zig.guide/standard-library/formatting-specifiers/
- 4: https://zig.guide/0.11/standard-library/formatting-specifiers/
🌐 Web query:
Zig std.fmt formatting arrays of u8 with {x} versus fmtSliceHexLower
💡 Result:
In modern versions of Zig, the use of the {x} and {X} format specifiers to format slices or arrays of u8 has been deprecated and replaced with the explicit use of std.fmt.fmtSliceHexLower and std.fmt.fmtSliceHexUpper [1][2][3]. If you attempt to use {x} or {X} directly on a slice of u8 (e.g., std.debug.print("{x}",.{my_slice})), the compiler will trigger a compile error, typically advising you to use the corresponding fmtSliceHex function instead [1][2]. The recommended approach to format a u8 slice or array as a hexadecimal string is as follows [4][2]: std.fmt.format(writer, "{s}",.{std.fmt.fmtSliceHexLower(my_slice)}); Key points: - Deprecation: The {x} and {X} specifiers for u8 slices were special cases in earlier versions of Zig's standard library [3]. They were removed to simplify the formatting system and reduce ambiguity, as {x} for integers and {x} for byte arrays performed fundamentally different operations [1][3]. - Explicit Formatting: Using std.fmt.fmtSliceHexLower or std.fmt.fmtSliceHexUpper makes the intent explicit and avoids the issues associated with overloaded format specifiers [1][3]. - Behavior: These functions return a Formatter object that handles the conversion of each byte in the slice into two hexadecimal characters, which is then correctly processed by the formatting system [2].
Citations:
- 1: https://ziggit.dev/t/is-x-format-deprecated-or-is-it-not/2207
- 2: https://github.com/ziglang/zig/blob/79628d48a4429818bddef2e86e2d7073d1955302/lib/std/fmt.zig
- 3: std: Deprecate 'x'/'X'/'e'/'E' special cases for u8 slices ziglang/zig#8007
- 4: add format option
{xs}/{Xs}to print u8 arrays as bytes ziglang/zig#17611
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== host/build.zig ==\n'
cat -n host/build.zig | sed -n '1,220p'
printf '\n== host/build.zig.zon ==\n'
cat -n host/build.zig.zon | sed -n '1,220p'Repository: SunkenInTime/weaver
Length of output: 9326
Use fmtSliceHexLower for the SHA-256 bytes
{x} doesn’t format a [32]u8 here; use std.fmt.fmtSliceHexLower(digest[0..]) (or another slice) when filling hash.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@host/src/art_cache.zig` around lines 63 - 68, Update the hash formatting in
the digest-to-filename flow around digest and hash to pass the SHA-256 bytes as
a slice through std.fmt.fmtSliceHexLower, ensuring hash receives the lowercase
hexadecimal representation before constructing the .img filename.
| } | ||
| self.published_hash = hash; | ||
| self.published = true; | ||
| try self.prune(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
A prune failure discards a publication that already succeeded.
By Line 110 the file is durable, renamed, and pinned. If prune returns an error (e.g. error.ArtCachePinExceedsLimit when every non-pinned statFile fails, or a delete error), publish propagates it and applyArtwork in host/src/media.zig logs "retaining prior art" and drops the fresh path — even though the artwork is on disk and pinned. Pruning is housekeeping; let it fail without invalidating the publication.
🛠️ Proposed fix
self.published_hash = hash;
self.published = true;
- try self.prune();
+ self.prune() catch |err| std.log.warn("art cache prune failed: {s}", .{`@errorName`(err)});📝 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.
| try self.prune(); | |
| self.prune() catch |err| std.log.warn("art cache prune failed: {s}", .{`@errorName`(err)}); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@host/src/art_cache.zig` at line 110, Update the publish flow around
self.prune() so pruning errors are treated as non-fatal after the file is
durable, renamed, and pinned. Preserve the successful publication and return
success to applyArtwork in media.zig, while retaining the existing prune
behavior for best-effort housekeeping.
| ~WeaverMediaSession() { | ||
| shutting_down.store(true, std::memory_order_release); | ||
| try { | ||
| if (properties_subscribed && current) current.MediaPropertiesChanged(properties_token); | ||
| } catch (...) { | ||
| } | ||
| try { | ||
| if (manager_subscribed && manager) manager.CurrentSessionChanged(manager_token); | ||
| } catch (...) { | ||
| } | ||
| properties_subscribed = false; | ||
| manager_subscribed = false; | ||
| current = nullptr; | ||
| manager = nullptr; | ||
| if (apartment_initialized) winrt::uninit_apartment(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
WinRT event revoke token in-flight handler completion guarantee
💡 Result:
In the Windows Runtime (WinRT), there is no guarantee that an in-flight event handler will cease execution immediately upon calling the revoke function with an EventRegistrationToken [1][2][3]. When you revoke a delegate, the event source removes that delegate from its invocation list [1][4][5]. However, if an event was already in the process of being raised (i.e., it is "in-flight"), the event source may have already acquired a copy of the invocation list or begun iterating through it [6][3]. Consequently, an in-flight event handler can still be invoked even after the revocation call has successfully returned [6][3]. Key considerations regarding this behavior include: 1. Asynchronous Events: This race condition is particularly relevant for asynchronous events [1][2]. If you revoke an event handler within a destructor, an in-flight event might reach your object after the destruction process has begun, potentially leading to crashes or access violations [1][2]. 2. Synchronous Events: For events raised synchronously, revoking the handler generally ensures that you will not receive further events from that point forward [1][2]. 3. Safety Patterns: To handle these scenarios safely, especially in C++/WinRT, it is recommended to use weak references (such as winrt::event_revoker) [1][2][7]. These mechanisms help manage the lifetime of the event recipient and provide a safer way to revoke handlers automatically as objects go out of scope [1][2]. 4. Redundancy: If an event handler is invoked after it has been unregistered, the call is considered redundant but harmless [3]. The event source is typically designed to handle these cases gracefully [3]. Developers should not rely on revocation to immediately terminate a handler that has already been dispatched by the event source [6][3]. If your logic requires ensuring that a handler does not execute, you must implement your own guard, such as a boolean flag or a weak reference check, within the handler itself [1][2][3].
Citations:
- 1: https://github.com/MicrosoftDocs/windows-dev-docs/blob/docs/uwp/cpp-and-winrt-apis/handle-events.md
- 2: https://learn.microsoft.com/en-us/windows/uwp/cpp-and-winrt-apis/handle-events
- 3: https://devblogs.microsoft.com/oldnewthing/20190521-00/?p=102505
- 4: https://learn.microsoft.com/en-us/windows/uwp/winrt-components/custom-events-and-event-accessors-in-windows-runtime-components
- 5: https://learn.microsoft.com/en-us/uwp/cpp-ref-for-winrt/event
- 6: https://learn.microsoft.com/en-us/cpp/cppcx/delegates-c-cx?view=msvc-170
- 7: https://learn.microsoft.com/en-us/uwp/cpp-ref-for-winrt/event-revoker
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file around the destructor and callback setup.
file='host/src/windows_providers.cpp'
wc -l "$file"
sed -n '220,460p' "$file" | cat -n
# Locate the destroy path and any raw-pointer captures in this file.
rg -n "delete session|CurrentSessionChanged|MediaPropertiesChanged|shutting_down|mark_session|WeaverMediaSession" "$file"Repository: SunkenInTime/weaver
Length of output: 13004
Handler revocation doesn’t fence in-flight callbacks
CurrentSessionChanged / MediaPropertiesChanged can still be executing after revocation, and both handlers capture a raw WeaverMediaSession*. That leaves a use-after-free window when weaver_media_destroy deletes the session; capture shared ownership or a std::weak_ptr in the callbacks so late events can’t touch freed memory.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@host/src/windows_providers.cpp` around lines 286 - 301, Update the callback
setup associated with WeaverMediaSession so CurrentSessionChanged and
MediaPropertiesChanged do not capture a raw WeaverMediaSession pointer. Capture
shared ownership or a std::weak_ptr, lock it before handling each event, and
return when the session is already destroyed; ensure destruction and handler
revocation cannot leave late callbacks accessing freed state.
| const auto stream = reference.OpenReadAsync().get(); | ||
| const uint64_t size = stream.Size(); | ||
| if (size > max_artwork_bytes) { | ||
| artwork->too_large = 1; | ||
| return false; | ||
| } | ||
| if (size == 0) { | ||
| artwork->changed = 1; | ||
| return true; | ||
| } | ||
| const auto input = stream.GetInputStreamAt(0); | ||
| winrt::Windows::Storage::Streams::DataReader reader(input); | ||
| const uint32_t loaded = reader.LoadAsync(static_cast<uint32_t>(size)).get(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Unbounded blocking on the host poll thread.
OpenReadAsync().get() and LoadAsync(...).get() block the caller with no timeout, and this runs inline in the 1 Hz weaver_media_poll on the host loop thread. A media app whose thumbnail stream stalls (a remote/album-art fetch behind the SMTC provider is not unusual) hangs the whole daemon loop, not just the media provider — every widget stops updating.
Bounding the wait keeps a misbehaving source contained; a timeout maps naturally onto the existing refresh_failed path, which already retains prior art and retries on the next property change.
⏱️ Sketch
- const auto stream = reference.OpenReadAsync().get();
+ auto open = reference.OpenReadAsync();
+ if (open.wait_for(std::chrono::seconds(2)) != winrt::Windows::Foundation::AsyncStatus::Completed) {
+ return false;
+ }
+ const auto stream = open.GetResults();C++/WinRT IAsyncOperation wait_for AsyncStatus GetResults timeout pattern
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@host/src/windows_providers.cpp` around lines 366 - 378, Bound the
asynchronous waits in the artwork-loading flow around OpenReadAsync and
LoadAsync so neither .get() can block the host poll thread indefinitely. Apply a
finite timeout and route timeout or failed-operation results through the
existing refresh_failed path, preserving prior artwork and retry behavior; keep
the current size checks and successful loading behavior unchanged.
| if (size > max_artwork_bytes) { | ||
| artwork->too_large = 1; | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Oversized artwork is conflated with a refresh failure across the C/Zig boundary, making the too_large handling dead code. read_thumbnail reports the oversized case by returning false, which weaver_media_poll turns into state->refresh_failed = true; the Zig consumer then checks refresh_failed before too_large and returns early. Net effect: the dedicated 1 MiB warning never fires, and a permanently-oversized thumbnail leaves the session in a sticky "failed" state instead of a settled "art unavailable" one.
host/src/windows_providers.cpp#L368-L371: settoo_large = 1andchanged = 1, then returntrue— oversized is a definite outcome, not a failure to observe.host/src/media.zig#L148-L153: evaluate thetoo_largebranch before thechanged == 0 or refresh_failed != 0guard so the warning and retention path is reachable regardless of the producer-side fix.
A regression test for the oversized path would have caught this; the new tests in host/src/media.zig cover refresh-failure and no-art but not too_large.
📍 Affects 2 files
host/src/windows_providers.cpp#L368-L371(this comment)host/src/media.zig#L148-L153
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@host/src/windows_providers.cpp` around lines 368 - 371, Update
host/src/windows_providers.cpp lines 368-371 in read_thumbnail to set too_large
and changed, then return true for oversized artwork so it is treated as a
definite unavailable-artwork result; update host/src/media.zig lines 148-153 in
the media polling flow to handle too_large before the changed == 0 or
refresh_failed guard, preserving the warning and artwork-retention path.
| const widget_root = try std.Io.Dir.cwd().realPathFileAlloc(io, widget_directory, allocator); | ||
| errdefer allocator.free(widget_root); | ||
| var art_root: ?[:0]u8 = null; | ||
| if (art_cache_environment) |root| { | ||
| if (!validAbsoluteArtPath(root)) return error.InvalidArtCacheRoot; | ||
| art_root = try std.Io.Dir.cwd().realPathFileAlloc(io, root, allocator); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the Zig toolchain version pinned by the repo and any other uses of these APIs.
fd -H -i -t f '(\.zigversion|\.tool-versions|build\.zig\.zon|flake\.nix|Dockerfile.*)' -x sed -n '1,60p' {} \;
rg -n 'realPathFileAlloc|realPathFileAbsoluteAlloc' -C2Repository: SunkenInTime/weaver
Length of output: 931
🌐 Web query:
Zig std.Io.Dir realPathFileAlloc return type sentinel
💡 Result:
The function realpathAlloc returns a slice of bytes, specifically []u8 [1][2]. It does not return a null-terminated sentinel string [2]. The returned slice contains the canonicalized path, and the caller is responsible for freeing this memory using the provided allocator [1][2]. Because it returns a standard Zig slice ([]u8), the data is not guaranteed to be null-terminated; if you require a null-terminated string, you must ensure it is handled separately or convert it to a sentinel-terminated type, though realpathAlloc itself provides a standard dynamic slice [2].
Citations:
- 1: https://mintlify.wiki/ziglang/zig/api/fs
- 2: https://github.com/ziglang/zig/blob/master/lib/std/fs.zig
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the affected source locations and nearby declarations.
sed -n '1,120p' runtime/src/image_paths.zig
printf '\n--- std API references in repo ---\n'
rg -n 'realPathFileAlloc|realPathFileAbsoluteAlloc|allocator\.free\(' runtime/src src -C2 || true
printf '\n--- Zig version pins ---\n'
git ls-files | rg '(^|/)(build\.zig\.zon|\.zigversion|\.tool-versions|Dockerfile.*|flake\.nix)$' || true
for f in $(git ls-files | rg '(^|/)(build\.zig\.zon|\.zigversion|\.tool-versions|Dockerfile.*|flake\.nix)$' || true); do
echo "### $f"
sed -n '1,80p' "$f"
doneRepository: SunkenInTime/weaver
Length of output: 24297
🌐 Web query:
Zig std.Io.Dir.cwd().realPathFileAlloc return type [:0]u8 realPathFileAbsoluteAlloc return type [:0]u8
💡 Result:
In the Zig standard library, the functions std.fs.Dir.realPathFileAlloc and std.fs.Dir.realPathFileAbsoluteAlloc return a slice of bytes, specifically []u8 [1][2]. The return type of these functions is not explicitly defined as a null-terminated slice ([:0]u8) in the standard library's public API for file path resolution [3][4]. Instead, they return a standard []u8 slice, meaning the caller receives an owned slice of bytes allocated via the provided allocator, which must be freed by the caller [5][1][6]. While some internal components or third-party codebases may utilize these functions [1][2], the functions are part of the std.fs.Dir interface (and accessible via std.fs.Dir.cwd) intended for resolving paths [7][5]. They resolve paths to their canonical form and return an allocated buffer containing the path as a sequence of bytes [1]. On Windows, this result is encoded as WTF-8, while on other platforms, it is an opaque sequence of bytes with no particular encoding [3][8]. If you require a null-terminated string, you would typically need to manually ensure or append a sentinel, though these specific standard library functions return standard []u8 slices [1].
Citations:
- 1: https://github.com/valthon/zigbase/blob/main/src/static_files.zig
- 2: KurtWagner/zlinter@c44f11b
- 3: https://github.com/ziglang/zig/blob/master/lib/std/fs/Dir.zig
- 4: https://github.com/ziglang/zig/blob/master/lib/std/fs.zig
- 5: https://zenofzig.com/book/c9-directories.html
- 6: https://git.lerch.org/lobo/zfin/src/commit/b31db345adad036806631bd5b6df554c04c34860/src/git.zig
- 7: https://ziggit.dev/t/get-path-const-u8-from-std-fs-dir/5870
- 8: https://raw.githubusercontent.com/ziglang/zig/master/lib/std/fs.zig
🌐 Web query:
site:github.com zig std.Io.Dir.realPathFileAlloc return type sentinel 0.16-dev
💡 Result:
In Zig 0.16-dev, the return type of std.fs.Dir.realpathAlloc is ![]u8 (an error union containing a slice of unsigned 8-bit integers) [1]. The function returns a []u8 slice which represents the canonicalized absolute path [1]. This slice is not null-terminated; it is a standard Zig slice containing the path data, and the caller is responsible for freeing the allocated memory [1]. As of the current development version, realpathAlloc is implemented by allocating a buffer and calling std.fs.Dir.realpath [1]. It does not utilize sentinel-termination in the returned slice [1].
Citations:
🌐 Web query:
zig lib/std/fs/Dir.zig realPathFileAlloc return type source
💡 Result:
In the Zig standard library (specifically within the std.fs.Dir interface), realPathFileAlloc is a function that returns an allocated slice of bytes representing the absolute canonical path [1]. The return type of realPathFileAlloc is: RealPathFileAllocError![]u8 This indicates that the function returns a []u8 (a slice of bytes) on success, or an error of type RealPathFileAllocError on failure [1]. Because the function allocates memory to hold the path, the caller is responsible for freeing the returned slice using the provided allocator [1]. Note that Zig's standard library APIs related to realpath have evolved, and the use of such functions is sometimes discouraged in favor of alternative approaches, such as working with relative paths or file descriptors, due to limitations with path lengths and portability across different operating systems [2][3]. Always check the documentation or source code for your specific version of Zig, as internal APIs are subject to change [4][5].
Citations:
- 1: https://zenofzig.com/book/c9-directories.html
- 2: remove realpath() from the standard library ziglang/zig#19353
- 3: stop calling realpath in the compiler ziglang/zig#16571
- 4: https://github.com/ziglang/zig/blob/master/lib/std/fs/Dir.zig
- 5: https://github.com/ziglang/zig/blob/master/lib/std/fs.zig
Use []u8 for the resolved paths
runtime/src/image_paths.zig:24-30,50-57
realPathFileAlloc / realPathFileAbsoluteAlloc return []u8, so Resolved.path, widget_root, and art_root can’t be [:0]u8 here. Switching these to []u8 should make the assignments type-check; the existing free(...) calls can stay as-is.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@runtime/src/image_paths.zig` around lines 24 - 30, Change the resolved path
types in the image path handling, including Resolved.path, widget_root, and
art_root, from [:0]u8 to []u8 to match the return types of realPathFileAlloc and
realPathFileAbsoluteAlloc. Preserve the existing allocation and allocator.free
cleanup behavior.
| const bytes = std.Io.Dir.cwd().readFileAlloc( | ||
| model.io orelse return error.MissingIo, | ||
| resolved.path, | ||
| std.heap.page_allocator, | ||
| .limited(1024 * 1024), | ||
| ) catch |err| { | ||
| replaceObserved(state, resolved.path, true); | ||
| std.log.err("image reload read failed; keeping the prior image: {s}", .{@errorName(err)}); | ||
| return; | ||
| }; | ||
| defer std.heap.page_allocator.free(bytes); | ||
| _ = effects.registerImageBytes(id, bytes) catch |err| { | ||
| replaceObserved(state, resolved.path, true); | ||
| std.log.err("image reload decode/register failed; keeping the prior image: {s}", .{@errorName(err)}); | ||
| return; | ||
| }; | ||
| replaceObserved(state, resolved.path, true); | ||
| state.registered = true; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
A transient read/register failure permanently pins that art path out.
Both failure paths call replaceObserved(state, resolved.path, true). On the next generation the same path resolves, observedEquals(state, resolved.path, true) matches at Line 893, and the function returns early — so the image is never retried even after the sharing violation / partial write clears. The artwork stays stale until the host publishes a different path (i.e. until the next track change).
Record the failure distinctly so a retry happens, ideally bounded so a permanently unreadable path isn't re-read on every generation.
🐛 Sketch: track the last failed path instead of marking it observed-valid
const ImageState = struct {
id: tree_mod.NodeId = 0,
lifetime: u64 = 0,
epoch: u64 = 0,
observed: ?[:0]u8 = null,
observed_valid: bool = false,
registered: bool = false,
+ /// Set when `observed` was resolved but could not be loaded. Cleared on
+ /// a successful load so a transient failure retries exactly once more.
+ load_failed: bool = false,
}; ) catch |err| {
replaceObserved(state, resolved.path, true);
+ state.load_failed = !state.load_failed;
std.log.err("image reload read failed; keeping the prior image: {s}", .{`@errorName`(err)});
return;
};and gate the Line 893 fast path on !state.load_failed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@runtime/src/main.zig` around lines 897 - 914, Track transient reload failures
separately from successful observations in the image reload state. Update both
read and register failure paths near effects.registerImageBytes to mark the path
as failed rather than calling replaceObserved with a valid observation, and gate
the observedEquals fast path in the reload function on !state.load_failed. Clear
the failure state after a successful load, with bounded retry behavior for
permanently unreadable paths.
|
@greptileai review |
|
Resolved the cross-session artwork P1 in commit 03c9056. A session change now marks the retained cache snapshot ineligible for the replacement frame; if the replacement thumbnail refresh fails, the provider returns no new frame, preserving the prior complete metadata+art frame atomically. The replacement frame is published only after its artwork publishes successfully or no-art is confirmed. The cache pin remains untouched on refresh failure, preserving F9. Added the regression test session replacement refresh failure suppresses prior art without unpinning it, and the final Windows live check viewed 250/750 ms old-frame retention followed by title+cover changing together at 1500 ms with no blank or mismatched frame. @greptileai review |
|
Resolved the follow-up indefinite-retention P1 in c7393f. The Windows provider now retries media properties/artwork while |
|
Resolved the permanent-thumbnail P1 in �153e6c. Oversized artwork is now a definite unavailable outcome instead of a transient failure. Other refresh failures retry on the existing subscription-only 1 Hz path; after three failed polls, refreshed metadata is released explicitly without art while the prior cache path/pin remains untouched. Later subscribed polls keep retrying and can publish recovered art. This prevents both stale metadata/art pairing and indefinite retention. Cache publication failure uses the same artless-metadata outcome. Added tests for the three-attempt bound and permanent-unavailable frame state; all exact-head local gates pass. @greptileai review |
1 participant
Stack: 02/05
What becomes usable
MediaData.artPathpoints only at a complete host-owned cache object.<image>sources re-register after every mutating JS turn while unchanged static-image trees exit on one generation comparison.Deliberately missing
media-transportcapability land in 03/05.Verification
npm test? PASS, 62/62.npm run typecheck? PASS.runtime: zig build test -Dweb-layer=exclude -Dtrace=off? PASS (Zig 0.16.0).host: zig build test? PASS (Zig 0.16.0).runtime: zig build -Dweb-layer=exclude -Dtrace=off? PASS.host: zig build? PASS.node cli/bin/weaver.js check examples/now-playing? PASS.3f6a68b606e110087b5992cbe75f700051f1b7f3.Live and visual gate
Spotify art was captured from the real desktop-layer widget after minimizing other windows. The accepted settled, track-change, and pause PNGs were each reopened at original resolution and graded element-by-element in
docs/media-evidence/pr02-visual.md.ImageTooLargefor Spotify's 300x300 source; bounded host normalization repaired it before the accepted captures.Performance claim
No observed static-image idle CPU regression. With identical ReleaseFast
examples/styling-imagesbuilds and matched ~60.014 s windows after 41?42 s settles:2feb700: 0.000 ms CPU; private 42.719 -> 42.684 MiB.8f66767: 0.000 ms CPU; private 42.758 -> 41.570 MiB.Raw values and the deliberately bounded claim are recorded in
docs/media-evidence/pr02-visual.md; no generalized memory claim is made.Adversarial review update (2026-07-25)
F9 is fixed at this owning layer. A transient SMTC thumbnail read, normalization, size, or cache-publication failure now retains the prior art path and cache pin; only a successful replacement or a confirmed no-art state clears it. The new retention/no-art test passes, as do the full layer gates: npm 62/62, typecheck, runtime/host
zig build test, production builds, andweaver check examples/now-playing.Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Final artwork association review (2026-07-25)
Final implementation head
a153e6c; GitHub Actions run30193379848is PASS for gate, Intel headless, Apple-silicon headless, and hosted Apple-silicon. Greptile is PASS.A replacement media item is now atomic with its artwork outcome. Transient refresh failure retains the prior complete metadata+art frame and retries only on the existing subscription-driven 1 Hz poll. Oversized art resolves immediately as unavailable; other unresolved refreshes release refreshed metadata artless after three attempts while retaining the previous cache path/pin. Later subscribed retries can still recover and publish art. Tests cover the retry truth table, three-attempt bound, cache-pin retention, cross-session suppression, and permanent-unavailable outcome.
The final Windows visual recheck delivered a real Spotify Next command and viewed five exact-region captures: the old title/cover remained paired at 250/750 ms and the replacement title/cover appeared together at 1500 ms, with no blank, black, fallback, or mismatched frame. Evidence is in
docs/media-evidence/pr05-visual.md.