Skip to content

fix(images): resolve artwork against its supplying server - #308

Merged
Quick104 merged 1 commit into
mainfrom
t3code/1882e391
Sep 16, 2026
Merged

Quick104 merged 1 commit into
mainfrom
t3code/1882e391

Conversation

@Quick104

@Quick104 Quick104 commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Local artwork responses can contain signed root-relative URLs. Android passed those values to Coil and media-session consumers without a server origin, so local-storage posters and backdrops could fail to load.

Resolve artwork fields in shared API response decoding, using the origin of the request that supplied the response. The resolved URLs retain their encoded paths and signature queries, ignore configured base paths, and remain tied to their supplying server after a switch. Absolute S3 URLs pass through unchanged. This covers phone and TV image components, direct prefetch requests, Cast artwork, and media-session artwork before those values enter caches or UI state.

Probe GET /api/v2/images/capabilities at startup and active-session changes, with captured authentication scope and cancellation of obsolete probes. Optional storage_backend and delivery fields tolerate older and future responses. Artwork resolution works independently of probe success or timing.

Closes #306. Related: Silo-Server/silo-server#935.

Validation:

  • All 4,728 debug unit tests passed across shared, android-shared, phone, and TV modules.
  • Phone and TV debug APKs built successfully.
  • Debug lint passed for android-shared, phone, and TV; release vital lint passed for phone and TV. Existing baseline warnings remain. Release compilation required a 6 GB Kotlin compiler heap after the default heap was exhausted.
  • The seven shared regression tests passed again after the final cleanup; both debug APKs were rebuilt from the final commit.
  • Regression tests cover signed and encoded URLs, base paths, IPv6, absolute storage URLs, delayed responses from different servers with identical artwork paths, capability compatibility, switching, and sign-out.
  • Robolectric tests with phone and TV configurations load artwork through the production Coil and media-session loaders against local HTTP fixtures. Requests preserve signed queries, use distinct server cache keys, and carry no Silo authorization, profile, or cookie headers.
  • Build supply-chain checks and their self-tests passed.

Limitations: physical phone/TV rendering against live local-storage and S3-backed Silo servers remains unverified. The integration fixtures exercise local and direct delivery but are not a live S3 service. Future artwork wire fields must be added to the shared field list.

AI disclosure: implemented with OpenAI gpt-6-astra in the Codex agent harness through T3 Code. No other AI tooling or sub-agents were used.

Note

Resolve API V2 artwork URLs against their supplying server origin

  • Root-relative artwork values in API V2 JSON responses are rewritten to origin-absolute URLs (scheme, host, port) inside safeApiV2Call before model decoding, preserving signed path and query encoding; absolute, protocol-relative, file, content, and empty values are left unchanged.
  • Adds ImageCapabilitiesApi and ImageCapabilitiesSession wired through the shared DI graph, with both phone and TV apps starting the session coordinator in a background IO scope on startup.
  • ImageCapabilitiesSession probes the V2 image capabilities endpoint on startup and after identity changes, clears state during transitions, and only publishes a result if the probed identity still matches.
  • The image client carries no Silo login or profile credentials; tests verify signed and direct artwork loads without Authorization, profile, or Cookie headers across phone and TV configurations.
  • Risk: resolveArtworkUrl in ArtworkUrls.kt discards the server URL base path — any deployment serving API artwork under a sub-path will now produce origin-rooted URLs instead of preserving that base path.

Macroscope summarized 3921492.

Summary by CodeRabbit

  • New Features
    • Added automatic image capability discovery for signed-in sessions on mobile and TV.
    • Artwork links now resolve correctly against the server that supplied them, including nested media artwork and poster collections.
  • Bug Fixes
    • Improved artwork loading across phone and TV devices, including media-session playback.
    • Image requests no longer include Silo login, profile, or cookie credentials, improving compatibility with signed and direct storage URLs.
    • Prevented stale artwork capabilities from carrying over after switching servers or signing out.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick104 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: a35b075d-034a-4976-a7db-8d0ea1c71995

📥 Commits

Reviewing files that changed from the base of the PR and between a5c9329 and 3921492.

📒 Files selected for processing (12)
  • android-shared/src/androidMain/kotlin/org/siloserver/silo/common/images/SiloImageLoader.kt
  • android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/images/ArtworkLoadingTest.kt
  • androidApp/src/androidMain/kotlin/org/siloserver/silo/android/SiloApplication.kt
  • androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/SiloTvApplication.kt
  • shared/src/commonMain/kotlin/org/siloserver/silo/di/NetworkModule.kt
  • shared/src/commonMain/kotlin/org/siloserver/silo/network/ArtworkUrls.kt
  • shared/src/commonMain/kotlin/org/siloserver/silo/network/api/ImageCapabilitiesApi.kt
  • shared/src/commonMain/kotlin/org/siloserver/silo/network/apiv2/ApiV2Call.kt
  • shared/src/commonMain/kotlin/org/siloserver/silo/repository/ImageCapabilitiesSession.kt
  • shared/src/commonTest/kotlin/org/siloserver/silo/network/ArtworkUrlsTest.kt
  • shared/src/commonTest/kotlin/org/siloserver/silo/network/ImageCapabilitiesSessionTest.kt
  • shared/src/commonTest/kotlin/org/siloserver/silo/network/apiv2/SimilarCardsV2Test.kt

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change resolves server-relative artwork URLs against their response origin, adds tests for phone and TV image loading, and starts session-scoped image capability discovery in phone and TV applications.

Changes

Artwork URL resolution and capability discovery

Layer / File(s) Summary
Response-origin artwork resolution
shared/src/commonMain/kotlin/org/siloserver/silo/network/ArtworkUrls.kt, shared/src/commonMain/kotlin/org/siloserver/silo/network/apiv2/ApiV2Call.kt, shared/src/commonTest/kotlin/org/siloserver/silo/network/*, android-shared/src/androidMain/kotlin/org/siloserver/silo/common/images/SiloImageLoader.kt, android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/images/ArtworkLoadingTest.kt
Relative artwork paths are resolved against the request origin during API decoding. Absolute and unrelated URLs remain unchanged. Tests cover encoded paths, server changes, Coil loading, media-session loading, and missing login headers.
Session-scoped capability discovery
shared/src/commonMain/kotlin/org/siloserver/silo/network/api/ImageCapabilitiesApi.kt, shared/src/commonMain/kotlin/org/siloserver/silo/repository/ImageCapabilitiesSession.kt, shared/src/commonTest/kotlin/org/siloserver/silo/network/ImageCapabilitiesSessionTest.kt
The client models and fetches image capabilities. The session resets state across identity transitions and discards results from stale owners.
Android startup wiring
shared/src/commonMain/kotlin/org/siloserver/silo/di/NetworkModule.kt, androidApp/src/androidMain/kotlin/org/siloserver/silo/android/SiloApplication.kt, androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/SiloTvApplication.kt
Koin registers the capability API and session as singletons. Phone and TV application startup begins the session on IO-backed coroutine scopes.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant SiloAPI
  participant safeApiV2Call
  participant ArtworkURLResolver
  participant ImageConsumer
  SiloAPI->>safeApiV2Call: Return JSON with artwork paths
  safeApiV2Call->>ArtworkURLResolver: Resolve paths against response origin
  ArtworkURLResolver-->>safeApiV2Call: Return transformed JSON
  safeApiV2Call-->>ImageConsumer: Decode model with resolved artwork URLs
Loading

Merge Risk: ⚪ Minimal · up to 39214

The artwork URL and session-capability changes preserve server association across switches without retaining stale capability data. No actionable merge risk was identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 12 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: resolving artwork URLs against the server that supplied them.
Linked Issues check ✅ Passed The PR meets the coding requirements in [#306]. safeApiV2Call resolves artwork fields from the response request origin before model decoding. resolveArtworkUrl preserves encoded paths and queries,…
Out of Scope Changes check ✅ Passed The changes stay within [#306]. The application startup registrations start the required capability session. DI registrations provide the required API and session. KDoc and the updated poster assertio…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch t3code/1882e391

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-16T19:01:58.017867Z 3921492 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@macroscopeapp

macroscopeapp Bot commented Sep 16, 2026

Copy link
Copy Markdown

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The PR correctly targets server-specific artwork resolution and adds strong regression coverage, but it also changes shared API response decoding and introduces an automatically launched authenticated capability probe in both applications. Those cross-cutting runtime changes warrant human review.

You can add or adjust custom eligibility rules. Learn more.

@Quick104
Quick104 merged commit c459bde into main Sep 16, 2026
6 checks passed
@Quick104
Quick104 deleted the t3code/1882e391 branch September 16, 2026 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(artwork): resolve server-relative artwork URLs

1 participant