Restore UriImageSource stream caching

[jfversluis]

Description

Restores disk caching for UriImageSource stream consumers. The previous IsolatedStorage implementation was disabled after GetUserStoreForApplication() failed on Android, then removed while leaving the download-only path and TODO behind. This replaces it with a shared file-system cache under FileSystem.CacheDirectory.

The cache now:

• honors CachingEnabled, CacheValidity, and HTTP Cache-Control: no-store
• uses SHA-256 URI-derived keys and coalesces concurrent requests for the same URI
• streams network responses directly to temporary files instead of buffering complete images in memory
• limits persistent storage to 100 MiB, individual entries to 25 MiB, and aggregate in-flight temporary storage to 25 MiB
• trims least-recently-used entries, removes expired entries on access, and cleans abandoned temporary files at process startup
• atomically promotes completed entries and preserves the first successful download if publication fails
• propagates cancellation through response-body reads and keeps HTTP response/client ownership tied to returned streams
• preserves seekable stream requirements across modern and compatibility Windows image services

The iOS image service shares the bounded cache for custom URI stream sources, honors CacheValidity, avoids duplicate platform cache entries for the built-in Controls UriImageSource, and keeps network reads off the captured UI context.

Android continues to use Glide and Tizen continues to use its platform URL loader. Their existing platform caches are unchanged, and exact CacheValidity enforcement on those native paths remains outside this focused stream-cache restoration.

Fixes #9138

Validation

• 5,735 Controls Core tests passed, 26 skipped, including 23 focused URI cache tests
• loopback HTTP coverage for cross-instance hits, disabled caching, expiry, slow streaming, no-store, and cancellation before and after response headers
• filesystem/concurrency coverage for request coalescing, size bounds, LRU trimming, abandoned temporary cleanup, atomic refresh with open readers, cache publication failure, and zero validity
• Controls builds passed for netstandard2.0, netstandard2.1, net10.0-android36.0, net10.0-ios26.0, and net10.0-maccatalyst26.0
• the earlier draft revision passed the full MAUI device-test pipeline across Android, iOS, Mac Catalyst, and Windows
• local Windows compilation reaches Windows-only XamlCompiler.exe/MakePri.exe, which cannot execute on macOS; the updated Windows paths are left to normal PR CI

Multiple adversarial review rounds covered storage exhaustion, memory amplification, HTTP and stream ownership, cancellation, concurrency, cache publication, and modern/compatibility platform behavior. The final round reported no remaining major or moderate findings.

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 37690

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 37690"

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

[jfversluis]

/azp run maui-pr-devicetests

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[jfversluis]

/azp run maui-pr

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[jfversluis]

/azp run maui-pr-devicetests

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[MauiBot]

Expert Review — 7 findings

See inline comments for details.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] Logic and Correctness — TryOpenCachedFile deletes the cached file as soon as it is judged stale (if (!IsCacheValid(...)) { if (File.Exists(cachePath)) File.Delete(cachePath); return null; }) and only then does the caller attempt the network download. Concrete scenario: a device is offline and the entry's CacheValidity (default 1 day) has just elapsed. The still-readable bytes are destroyed first, download(...) fails or returns null, and the image renders as nothing — whereas the pre-PR iOS path (IsImageCached(pathToImageCache) = File.Exists) served the stale file and the image kept rendering. Deleting only after a successful download, or falling back to the stale entry when download yields null/throws, preserves the offline behavior while still honoring CacheValidity when the network is reachable.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Logic and Correctness — The DateTime utcNow parameter is never read anywhere in TrimCache's body; both call sites pass DateTime.UtcNow and it is silently discarded. This is not just a dead parameter — it signals that eviction is purely size/LRU based: an entry that is past its CacheValidity is never removed by trimming and stays on disk indefinitely until the 100 MB MaxCacheSize cap happens to evict it by last-access order. For an app with a small working set of images, expired blobs accumulate permanently in FileSystem.CacheDirectory. Either use utcNow to drop expired entries during the trim pass, or remove the parameter so the size-only policy is explicit.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Logic and Correctness — The completed temp file is opened for reading (var completedStream = OpenCacheFile(temporaryPath);) before it is published via File.Replace(temporaryPath, cachePath, null) / File.Move on the following lines. File.Move tolerates the outstanding handle because OpenCacheFile passes FileShare.ReadWrite | FileShare.Delete, but Win32 ReplaceFile (which backs File.Replace) is not covered by that share mode in the same way, so on Windows the File.Exists(cachePath) branch — i.e. every refresh of an already-cached URI — can fail with a sharing violation. The catch (IOException ...) swallows it and returns a TemporaryFileStream, so the failure is invisible: the image still loads but the refreshed bytes are never written to cachePath, and the URI re-downloads on every subsequent request forever. Publishing first (move/replace) and opening cachePath afterwards removes the ambiguity on all platforms.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] Cross-Platform Behavioral Consistency — Switching to HttpCompletionOption.ResponseHeadersRead changes the contract of the stream returned by StreamWrapper.GetStreamAsync on every platform: it was a fully-buffered, seekable stream (default HttpCompletionOption.ResponseContentRead reads the body before GetAsync completes) and is now an unbuffered, non-seekable live network stream. The PR adapts only the Windows consumers (EnsureSeekableAsync added to UriImageSourceService.Windows.cs, StreamImageSourceService.Windows.cs, and the two compatibility handlers). The Android consumer src/Core/src/ImageSources/StreamImageSourceService/StreamImageSourceService.Android.cs (lines 25 and 62) takes the same IStreamImageSource.GetStreamAsync result and was not audited or adapted, and neither was Tizen. Any consumer that relied on CanSeek, Length, or on re-reading from position 0 now silently changes behavior on those platforms. Please state which non-Windows consumers were checked, or add the equivalent seekable-stream guarantee at the source.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Performance-Critical Path — EnsureSeekableAsync copies the entire response into an unbounded MemoryStream whenever !stream.CanSeek, which after the ResponseHeadersRead change is now the normal case for every remote image on Windows. This bypasses the size bounds the PR carefully establishes elsewhere (MaxCacheEntrySize = 25 MB, MaxTemporaryCacheSize = 25 MB): a large or malformed remote image is buffered in full into managed memory with no cap and no ExpectedLength pre-check, even though IImageSourceCacheStream.ExpectedLength is already available on the StreamWrapper being passed in. Consider consulting ExpectedLength / applying a ceiling before buffering.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[moderate] Async and Threading Safety / iOS Platform — NSData.FromStream(stream) reads the stream with the synchronous Stream.Read loop. Before this PR that was harmless because HttpClient had already buffered the whole body, so the reads were pure memory copies. With HttpCompletionOption.ResponseHeadersRead (StreamWrapper.cs:147), this is now blocking network I/O: GetImageData runs on a thread-pool thread (reached after ConfigureAwait(false) awaits) and that thread stays blocked for the full download duration. Concrete scenario: a page with N remote UriImageSource images that bypass the disk cache (CachingEnabled=false, Cache-Control: no-store, or an entry over MaxCacheEntrySize) blocks N pool threads simultaneously on slow connections, starving the pool. Buffering asynchronously (e.g. CopyToAsync into an NSMutableData-backed or pooled buffer) before handing bytes to NSData avoids the sync-over-network read.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] Regression Prevention and Test Coverage — internal bool IsImageCached(string path, TimeSpan cacheValidity) is added here but is never called by any production path: the caching decision now lives entirely in UriImageSourceCache.IsCacheValid (UriImageSourceCache.cs:194), and DownloadAndCacheImageAsync no longer consults IsImageCached at all. The only consumer is the new device test CachedImageHonorsCacheValidity (UriImageSourceServiceTests.iOS.cs), which therefore asserts against a dead helper and provides no coverage of the shipped validity logic — the duplicated TimeSpan.Zero / TimeSpan.MaxValue / LastWriteTimeUtc rules in IsCacheValid could regress with this test still green. This matters more than usual because the gate is INCONCLUSIVE (tests could not be built/run), so this is the PR's only iOS-side validity evidence. Please point the test at UriImageSourceCache (it is already internal and reachable) and drop the unused overload, and add a case covering the expired-entry-plus-failed-download path flagged at UriImageSourceCache.cs:208.

[MauiBot]

AI Review Summary

@jfversluis — new AI review results are available based on commit ab92585.

---

🗂️ Review Sessions — click to expand

🚦 Gate — Test Before & After Fix

Gate Result: ⚠️ INCONCLUSIVE

Platform: IOS · Base: main · Merge base: bfa57509

🩺 Could not verify — environment/infrastructure error. The gate ran the tests but hit an environment error (an emulator/simulator/Appium/XHarness flake, a device that would not boot, or an empty/invalid result file), so it could not record a real pass/fail. The ⚠️ ENV ERROR marks below are infrastructure, not test failures — this is not a problem with your PR. Comment /review to retry on a fresh agent.

XHarness did not produce the expected fresh result 'testResults.xml' for requested class(es) 'Microsoft.Maui.DeviceTests.UriImageSourceServiceTests' (the target tests did not run).

Test	Without Fix (expect FAIL)	With Fix (expect PASS)
🧪 UriImageSourceTests UriImageSourceTests	🛠️ BUILD ERROR	✅ PASS — 24s
📱 UriImageSourceServiceTests (CachedImageHonorsCacheValidity) Category=ImageSource	🛠️ BUILD ERROR	⚠️ ENV ERROR

🔴 Without fix — 🧪 UriImageSourceTests: 🛠️ BUILD ERROR · 19s

Error-relevant lines (filtered from the build log):

/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs(377,29): error CS0103: The name 'UriImageSourceCache' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs(397,20): error CS0103: The name 'UriImageSourceCache' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs(399,21): error CS0103: The name 'UriImageSourceCache' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs(425,20): error CS0103: The name 'UriImageSourceCache' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs(429,21): error CS0103: The name 'UriImageSourceCache' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs(452,5): error CS0103: The name 'UriImageSourceCache' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs(454,29): error CS0103: The name 'UriImageSourceCache' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs(466,29): error CS0103: The name 'UriImageSourceCache' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs(483,20): error CS0103: The name 'UriImageSourceCache' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs(487,23): error CS0103: The name 'UriImageSourceCache' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs(525,5): error CS0103: The name 'UriImageSourceCache' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs(526,5): error CS0103: The name 'UriImageSourceCache' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs(533,5): error CS0103: The name 'UriImageSourceCache' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs(534,5): error CS0103: The name 'UriImageSourceCache' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]

🟢 With fix — 🧪 UriImageSourceTests: PASS ✅ · 24s

(no coded error found; showing last 1200 chars)

oreResponseIsReturnedButNotCached [3 ms]
  Passed CancellationAfterHeadersAbortsUncachedResponseBody [2 s]
  Passed CachingEnabledControlsHttpRequests(cachingEnabled: True, expectedRequests: 1) [4 ms]
  Passed CachingEnabledControlsHttpRequests(cachingEnabled: False, expectedRequests: 2) [2 ms]
  Passed HttpCancellationIsPropagatedAndNotCached [2 ms]
  Passed CacheWriteFailureStillReturnsDownloadedImage [1 ms]
  Passed SlowStreamingResponseIsFullyCached [107 ms]
  Passed NullUriDoesNotCrash [< 1 ms]
  Passed CachePublishFailureReturnsFirstDownload [4 ms]
  Passed SameUriHasSameCachePath [< 1 ms]
[xUnit.net 00:00:02.98]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Passed DifferentUrisHaveDifferentCachePaths [< 1 ms]
  Passed OversizedImageIsReturnedButNotCached [3 ms]
  Passed ZeroValidityAlwaysDownloads [< 1 ms]
  Passed CancellationDoesNotCreateCacheEntry [1 ms]
  Passed TrimCacheRemovesOldestFilesAndStaleTemporaryFiles [1 ms]
  Passed SecondCallLoadsFromCache [2 ms]
  Passed OpenCachedStreamDoesNotPreventRefresh [2 ms]
  Passed CacheMissReturnsFileStreamWithoutBufferingImageInMemory [2 ms]
Test Run Successful.
Total tests: 23
     Passed: 23
 Total time: 3.2562 Seconds

🔴 Without fix — 📱 UriImageSourceServiceTests (CachedImageHonorsCacheValidity): 🛠️ BUILD ERROR · 38s

Error-relevant lines (filtered from the build log):

/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Services/ImageSource/UriImageSourceServiceTests.iOS.cs(61,25): error CS1501: No overload for method 'IsImageCached' takes 2 arguments [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Services/ImageSource/UriImageSourceServiceTests.iOS.cs(65,26): error CS1501: No overload for method 'IsImageCached' takes 2 arguments [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Services/ImageSource/UriImageSourceServiceTests.iOS.cs(66,26): error CS1501: No overload for method 'IsImageCached' takes 2 arguments [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Services/ImageSource/UriImageSourceServiceTests.iOS.cs(67,25): error CS1501: No overload for method 'IsImageCached' takes 2 arguments [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
Build FAILED.

🟢 With fix — 📱 UriImageSourceServiceTests (CachedImageHonorsCacheValidity): ⚠️ ENV ERROR · 51s

No log file found

⚠️ Failure Details

• 🛠️ UriImageSourceTests without fix: build failed before tests could run
  • /Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs(44,46): error CS0103: The name 'UriImageSourceCache' does not exist in the current context [/Users/cloudtest/vss...
• 🛠️ UriImageSourceServiceTests (CachedImageHonorsCacheValidity) without fix: build failed before tests could run
  • /Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Services/ImageSource/UriImageSourceServiceTests.iOS.cs(61,25): error CS1501: No overload for method 'IsImageCached' takes 2 arguments [/Users/...
• ⚠️ UriImageSourceServiceTests (CachedImageHonorsCacheValidity) with fix: XHarness did not produce the expected fresh result 'testResults.xml' for requested class(es) 'Microsoft.Maui.DeviceTests.UriImageSourceServiceTests' (the target tests did not run).

📁 Fix files reverted (10 files)

• src/Compatibility/Core/src/GTK/GtkPlatformServices.cs
• src/Compatibility/Core/src/Windows/StreamImagesourceHandler.cs
• src/Compatibility/Core/src/Windows/UriImageSourceHandler.cs
• src/Controls/src/Core/Image/ImageSource.cs
• src/Controls/src/Core/StreamWrapper.cs
• src/Controls/src/Core/UriImageSource.cs
• src/Core/src/ImageSources/IStreamImageSource.cs
• src/Core/src/ImageSources/StreamImageSourceService/StreamImageSourceService.Windows.cs
• src/Core/src/ImageSources/UriImageSourceService/UriImageSourceService.Windows.cs
• src/Core/src/ImageSources/UriImageSourceService/UriImageSourceService.iOS.cs

New files (not reverted):

• src/Core/src/ImageSources/ImageSourceServiceExtensions.Windows.cs
• src/Core/src/ImageSources/UriImageSourceCache.cs

---

📋 Pre-Flight — Context & Validation

PR #37690 Pre-Flight

Context

• PR: Restore UriImageSource stream caching
• URL: #37690
• Base: main
• Materialized review commit: ae361cd2d458b8f001ea6c850864cbeed424312a
• Linked issue: #9138, which reports that UriImageSource caching was left as a TODO/download-only path after the former isolated-storage implementation was removed.
• Gate: Inconclusive because the existing gate could not build/run the test. This is an environment/build blocker, not evidence that the PR fix fails. Gate verification must not be rerun.

Existing PR Approach

The PR implements a shared file-system cache under FileSystem.CacheDirectory and routes Controls and the iOS custom-stream path through it. Its main mechanisms are:

• SHA-256 URI keys, validity checks, per-key request coalescing, atomic temporary-file publication, LRU trimming, and bounded entry/cache/in-flight temporary sizes in a new UriImageSourceCache.
• streamed HTTP response handling with explicit HttpClient/HttpResponseMessage ownership and cancellation propagation through StreamWrapper.
• Cache-Control: no-store and expected response length metadata exposed through internal stream interfaces.
• iOS avoidance of duplicate caching for Controls sources that already own the shared cache.
• seekable-stream preservation for modern and compatibility Windows image services.

The checked-out diff changes 14 files (+1490/-144), including two added production files:

• src/Core/src/ImageSources/UriImageSourceCache.cs
• src/Core/src/ImageSources/ImageSourceServiceExtensions.Windows.cs

Any alternative must differ at the root-cause/mechanism level rather than merely relocating the same cache checks. Per the try-fix restoration contract, each attempt must inspect .github/.baseline-state.json after establishing the broken baseline and report Blocked without editing if its NewFiles array is non-empty.

Candidate Boundary

The relevant existing changed files are:

• src/Controls/src/Core/UriImageSource.cs
• src/Controls/src/Core/StreamWrapper.cs
• src/Controls/src/Core/Image/ImageSource.cs
• src/Core/src/ImageSources/IStreamImageSource.cs
• src/Core/src/ImageSources/UriImageSourceCache.cs
• src/Core/src/ImageSources/UriImageSourceService/UriImageSourceService.iOS.cs
• src/Core/tests/DeviceTests/Services/ImageSource/UriImageSourceServiceTests.iOS.cs
• src/Controls/tests/Core.UnitTests/UriImageSourceTests.cs
• the changed Windows and compatibility image-source files listed by git diff origin/main...HEAD

Only files listed in baseline state's RevertedFiles may be edited by an attempt.

Bounded Validation

Platform: iOS

Detected primary test: Microsoft.Maui.DeviceTests.UriImageSourceServiceTests.CachedImageHonorsCacheValidity

pwsh .github/skills/run-device-tests/scripts/Run-DeviceTests.ps1 -Project Core -Platform ios -IncludeClasses "Microsoft.Maui.DeviceTests.UriImageSourceServiceTests" -IncludeMethods "CachedImageHonorsCacheValidity"

No mandatory regression tests were enumerated in the STEP 5a request, so attempts must not expand to a class, project, or full-suite run. Each candidate gets one implementation/test pass and at most one focused correction/retest. Environment/device unavailability is Blocked; a completed failing test or compile failure after the correction budget is Fail.

Repository Constraints

• iOS-specific files also compile for Mac Catalyst.
• Preserve cancellation through asynchronous image reads and keep shared mutable cache state synchronized.
• The PR adds only internal interfaces/helpers, so an alternative must not introduce an unnecessary public API.
• Perform the required inline expert self-review; do not launch a separate reviewer agent.
• Restore only with pwsh .github/scripts/EstablishBrokenBaseline.ps1 -Restore.

---

🔬 Code Review — Deep Analysis

Expert Evaluation of the Submitted PR

Verdict: NEEDS_CHANGES
Confidence: Medium

The submitted fix has a strong overall design: it restores a bounded shared disk cache, coalesces same-URI requests, streams downloads to temporary files, publishes entries atomically, preserves cancellation and HTTP ownership, and adapts Windows image consumers that require seekable streams. The inconclusive Gate is an environment/build gap, not evidence that the fix fails.

The expert review nevertheless identified three major correctness or coverage gaps and four moderate issues:

1. An expired cache entry is deleted before replacement succeeds, so an offline or failed refresh destroys the last usable image instead of allowing stale fallback.
2. ResponseHeadersRead changes the shared stream contract to a live, usually non-seekable network stream, while only Windows consumers were adapted; Android and Tizen consumers still receive the changed contract directly.
3. The new iOS validity test exercises a production-dead IsImageCached(path, validity) helper rather than UriImageSourceCache.IsCacheValid, so it does not cover the shipped validity path.
4. The cache opens a completed temporary file before File.Replace, risking failed refresh publication on Windows.
5. TrimCache accepts but ignores utcNow, leaving expired entries until size-based LRU eviction.
6. Windows seekability adaptation copies an unbounded remote stream into memory despite the cache's 25 MiB limits and available expected-length metadata.
7. iOS passes a live network stream to synchronous NSData.FromStream, potentially blocking one thread-pool thread per uncached download.

These findings are recorded in inline-findings.json against the raw PR diff. A single consolidated pr-plus-reviewer candidate should address the concrete cache publication/stale fallback, stream-consumption, bounded buffering, and test-coverage concerns, then run only the required focused iOS validation once.

---

🛠️ Try-Fix — Analysis & Comparison

STEP 5a Alternative Fix Candidates for PR #37690

Candidate 1 — HTTP-Layer Revalidation and Bounded Memory Cache

Model: claude-opus-5
Result: Blocked
Artifacts: CustomAgentLogsTmp/PRState/37690/PRAgent/try-fix/attempt-1/
Individual narrative: CustomAgentLogsTmp/PRState/37690/PRAgent/try-fix-1/content.md

Approach

Replace the PR's disk-backed byte store with a shared HttpClient and a bounded, size-accounted in-process map from URI to Task<byte[]>, including HTTP validators. Each consumer would receive an independent seekable MemoryStream.

Cancellation would be per consumer without cancelling a shared in-flight request; response and client ownership would end before a stream reaches the caller; ETag, Last-Modified, Cache-Control, and UriImageSource.CacheValidity would govern freshness; memoized tasks would coalesce concurrent requests; and LRU byte accounting would bound memory.

Prior Approach Avoided and Mechanism-Level Difference

The PR treats the missing persistent byte store as the root cause and adds UriImageSourceCache: SHA-256 file keys, per-key locks, temporary-file publication, LRU disk trimming, bounded entries and temporary storage, stream metadata interfaces, and an iOS marker to prevent double caching.

Candidate 1 instead hypothesizes that the central failure is the throwaway HttpClient and discarded response. Buffering once at the HTTP layer would end HTTP ownership before consumers receive independent streams, use a single cache layer, eliminate iOS double-cache markers, and enforce freshness through HTTP revalidation rather than file timestamps.

Diff and Files Changed

No code was changed. The captured fix.diff is empty.

Test Result

The only permitted command was:

pwsh .github/skills/run-device-tests/scripts/Run-DeviceTests.ps1 -Project Core -Platform ios -IncludeClasses "Microsoft.Maui.DeviceTests.UriImageSourceServiceTests" -IncludeMethods "CachedImageHonorsCacheValidity"

It was not executed because no candidate fix could be applied; running it would only repeat gate verification. Test executions consumed: 0 of 2.

Failure Analysis

EstablishBrokenBaseline.ps1 rejected the pre-existing dirty worktree before creating .github/.baseline-state.json. The unrelated .github/scripts, .github/skills, and eng changes were explicitly out of scope and could not be reverted, while try-fix rules prohibit alternate cleanup commands. The missing baseline state therefore required Blocked before editing.

There is also an independent deterministic blocker: the PR adds src/Core/src/ImageSources/UriImageSourceCache.cs and src/Core/src/ImageSources/ImageSourceServiceExtensions.Windows.cs. On a clean worktree these would populate baseline state's NewFiles, which the restoration contract also requires treating as Blocked.

Self-Review and Restore

Inline self-review recorded 0 findings because the candidate diff is empty. The exact restore command was run:

pwsh .github/scripts/EstablishBrokenBaseline.ps1 -Restore

It returned No baseline state found / Restored False, the expected no-state outcome because baseline creation failed before any candidate edit. Pre-existing worktree changes and untracked paths were left untouched.

Candidate 2 — iOS Native Protocol Cache

Model: gpt-5.6-sol
Result: Blocked
Artifacts: CustomAgentLogsTmp/PRState/37690/PRAgent/try-fix/attempt-2/
Individual narrative: CustomAgentLogsTmp/PRState/37690/PRAgent/try-fix-2/content.md

Approach

Preserve the URI for the standard Controls UriImageSource until it reaches an internal opt-in iOS path, then use NSUrlSession backed by a capacity-limited NSUrlCache. CacheValidity would cap acceptable cached-response age; disabled or zero validity would force reload, while native protocol policy would retain authority for validators and Cache-Control: no-store.

Managed synchronization would retain only active NSUrlSessionDataTask operations, not completed image payloads. Each waiter would have independent cancellation, with the native request cancelled only after its final waiter leaves. Custom URI/stream implementations would remain on the existing stream path to preserve authentication and custom behavior. The iOS validity helper for legacy files would remain solely for compatibility and the targeted regression test.

Prior Approaches Avoided and Mechanism-Level Difference

The PR's custom disk cache and Candidate 1's managed in-memory cache both make the framework own completed encoded payloads, requiring managed freshness, synchronization, cancellation boundaries, and eviction.

Candidate 2 instead hypothesizes that iOS caching became unreliable because the URI was converted to an opaque managed stream before native networking could apply protocol caching. Keeping the URI visible to NSUrlSession delegates validators, no-store, freshness, and bounded eviction to NSUrlCache; managed state exists only during active requests. It is therefore neither a custom disk store nor completed-payload memoization.

Diff and Files Changed

No code was changed. The captured fix.diff is empty.

Test Result

The only permitted command was:

pwsh .github/skills/run-device-tests/scripts/Run-DeviceTests.ps1 -Project Core -Platform ios -IncludeClasses "Microsoft.Maui.DeviceTests.UriImageSourceServiceTests" -IncludeMethods "CachedImageHonorsCacheValidity"

It was not executed because no candidate fix could be applied; running it would only repeat gate verification. Test executions consumed: 0 of 2.

Failure Analysis

EstablishBrokenBaseline.ps1 rejected the pre-existing dirty worktree before creating .github/.baseline-state.json. Without the authoritative RevertedFiles allow-list, candidate edits were prohibited. The unrelated worktree changes could not be altered and manual Git cleanup is forbidden.

The PR's two added production files would independently produce a non-empty NewFiles list on a clean baseline, which also requires Blocked. Neither blocker evaluates the native-cache design, and the prior gate remains Inconclusive rather than failing.

Self-Review and Restore

Inline self-review recorded 0 findings because the candidate diff is empty. The exact restore command was run:

pwsh .github/scripts/EstablishBrokenBaseline.ps1 -Restore

It returned No baseline state found / Restored False, the expected no-state outcome because baseline creation failed before any candidate edit. Pre-existing worktree changes and untracked paths were left untouched.

Aggregate Outcome

Two mechanism-level alternatives were bounded and recorded. Both are Blocked before implementation or testing by restoration-safety preconditions, not shown to fail:

1. HTTP-layer conditional revalidation with bounded managed memory payloads.
2. iOS-native NSUrlSession/NSUrlCache protocol caching with active-request-only managed coalescing.

No candidate code diff exists, no test command was executed, no gate file was created or overwritten, and both attempts completed the required no-state restore path.

---

📝 PR Finalize — Recommended Title & Description

Assessment: ✏️ Recommend updating — the title does not follow the component-oriented format, and the description's claim that the final review found no major or moderate issues is now stale.

Recommended title

[Core] UriImageSource: Restore stream caching

Recommended description

## Description

Restores disk caching for `UriImageSource` stream consumers. The previous IsolatedStorage implementation was disabled after `GetUserStoreForApplication()` failed on Android, then removed while leaving the download-only path and TODO behind. This replaces it with a shared file-system cache under `FileSystem.CacheDirectory`.

The cache now:
- honors `CachingEnabled`, `CacheValidity`, and HTTP `Cache-Control: no-store`
- uses SHA-256 URI-derived keys and coalesces concurrent requests for the same URI
- streams network responses directly to temporary files instead of buffering complete images in memory
- limits persistent storage to 100 MiB, individual entries to 25 MiB, and aggregate in-flight temporary storage to 25 MiB
- trims least-recently-used entries, removes expired entries on access, and cleans abandoned temporary files at process startup
- atomically promotes completed entries and preserves the first successful download if publication fails
- propagates cancellation through response-body reads and keeps HTTP response/client ownership tied to returned streams
- preserves seekable stream requirements across modern and compatibility Windows image services

The iOS image service shares the bounded cache for custom URI stream sources, honors `CacheValidity`, avoids duplicate platform cache entries for the built-in Controls `UriImageSource`, and keeps network reads off the captured UI context.

Android continues to use Glide and Tizen continues to use its platform URL loader. Their existing platform caches are unchanged, and exact `CacheValidity` enforcement on those native paths remains outside this focused stream-cache restoration.

Fixes #9138

## Validation

- 5,735 Controls Core tests passed, 26 skipped, including 23 focused URI cache tests
- loopback HTTP coverage for cross-instance hits, disabled caching, expiry, slow streaming, `no-store`, and cancellation before and after response headers
- filesystem/concurrency coverage for request coalescing, size bounds, LRU trimming, abandoned temporary cleanup, atomic refresh with open readers, cache publication failure, and zero validity
- Controls builds passed for `netstandard2.0`, `netstandard2.1`, `net10.0-android36.0`, `net10.0-ios26.0`, and `net10.0-maccatalyst26.0`
- the earlier draft revision passed the full MAUI device-test pipeline across Android, iOS, Mac Catalyst, and Windows
- local Windows compilation reaches Windows-only `XamlCompiler.exe`/`MakePri.exe`, which cannot execute on macOS; the updated Windows paths are left to normal PR CI

Multiple adversarial review rounds covered storage exhaustion, memory amplification, HTTP and stream ownership, cancellation, concurrency, cache publication, and modern/compatibility platform behavior.

---

🏁 Report — Final Recommendation

⚠️ Final Recommendation: REQUEST CHANGES

Winner: pr-plus-reviewer

pr-plus-reviewer is the strongest candidate because it preserves the submitted fix's bounded shared disk-cache design while correcting two concrete review findings: the iOS validity test now exercises the shipped UriImageSourceCache path, and iOS no longer performs synchronous reads from a live network stream. It also removes the misleading unused TrimCache clock parameter. The focused iOS build reached candidate sources but was blocked by the machine's Xcode 26.5 / SDK-required Xcode 26.0 mismatch, so its test result is Blocked, not Fail.

Comparative Ranking

Rank	Candidate	Implementation	Regression validation	Assessment
1	pr-plus-reviewer	Complete reviewer patch on the PR fix	Blocked by Xcode version mismatch after candidate assemblies compiled	Best implemented option; fixes the dead-test and iOS sync-I/O findings without replacing the PR's core design. Remaining expert concerns are explicit uncertainties.
2	pr	Complete submitted fix	Gate Inconclusive due build/environment error	Strong cache architecture and broad author-reported coverage, but the expert review found three major and four moderate concerns. In particular, the targeted iOS validity test covers a dead helper and uncached iOS responses can be synchronously consumed.
3	try-fix-1	Design only; empty diff	Not run because baseline establishment was blocked	Cross-platform HTTP revalidation plus a bounded in-memory cache is coherent, but it is unimplemented, unvalidated, non-persistent, and would replace a working candidate with a materially different cache model.
4	try-fix-2	Design only; empty diff	Not run because baseline establishment was blocked	Native NSUrlSession/NSUrlCache is plausible for standard iOS URIs, but it is unimplemented, iOS-specific, and does not by itself restore the shared stream-cache behavior addressed by the PR.

No candidate completed a regression-test run, and no candidate has a regression-test failure. Therefore the mandatory rule that failed candidates rank below passing candidates does not alter the ordering; validation uncertainty is retained rather than treated as failure.

Expert Findings Reconciliation

• Addressed by the winner: production-dead iOS validity test; synchronous iOS reads from live network streams; unused TrimCache clock parameter.
• Unresolved and requiring author judgment: whether an expired entry should be retained or served after a failed refresh; whether Android/Tizen consumers need an explicit seekability guarantee; whether the current FileShare.Delete contract is sufficient for Windows replacement; and whether Windows should avoid unbounded buffering for oversized non-seekable images.
• Gate status: Inconclusive and not a reason by itself to reject the fix.

Because the winning changes are not present in the submitted PR HEAD, the recommendation must be REQUEST CHANGES. Apply pr-plus-reviewer/reviewer.patch to the PR and retain the unresolved concerns as review discussion rather than claiming full verification.

---

📱 UI Tests — Button,Label,Layout

Detected UI test categories: Button,Label,Layout

❌ Deep UI tests — 354 passed, 1 failed, 8 skipped across 3 categories on platform-pool agent (replaces in-process counts above).

🧪 UI Test Execution Results (deep, platform pool)

Category	Tests	Snapshot diffs
Button	71/72 (1 skipped) ✓	—
Label	90/92 (2 skipped) ✓	—
Layout	193/199 (1 ❌, 5 skipped)	—

🔍 AI analysis of failures — PR-related vs unrelated

🔍 AI-generated triage (GitHub Copilot CLI) — a heuristic judgement of whether each deep UI test failure is connected to this PR's changes. Verify before relying on it.

Likely unrelated: the failures appear pre-existing, flaky, or infrastructure.

• ● Unrelated — iOS editor keyboard-scrolling layout (~1 test): EditorsScrollingPageTest failed while locating a UI element, whereas the PR changes URI/stream image loading and caching rather than Editor, keyboard, scrolling, or layout behavior.

❌ Layout — 1 failed test

EditorsScrollingPageTest

OpenQA.Selenium.NoSuchElementException : An element could not be located on the page using the given search parameters.; For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception
at OpenQA.Selenium.WebDriver.UnpackAndThrowOnError(Response errorResponse, String commandToExecute)
   at OpenQA.Selenium.WebDriver.ExecuteAsync(String driverCommandToExecute, Dictionary`2 parameters)
   at OpenQA.Selenium.Appium.AppiumDriver.Execute(String driverCommandToExecute, Dictionary`2 parameters)
   at OpenQA.Selenium.WebDriver.FindElement(String mechanism, String value)
   at OpenQA.Selenium.Appium.AppiumDriver.FindElement(String by, String value)
   at OpenQA.Selenium.Appium.MobileBy.FindElement(ISearchContext context)
   at OpenQA.Selenium.WebDriver.FindElement(By by)
   at OpenQA.Selenium.Appium.AppiumDriver.FindElement(By by)
   at Microsoft.Maui.TestCases.Tests.KeyboardScrolling.CloseiOSEditorKeyboard(AppiumDr
...

📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)

---

🧭 Next Steps — reviewer changes required

The reviewer-enhanced candidate identified changes that are not yet in the submitted PR.

Why: The reviewer-enhanced PR preserves the submitted disk-cache design while replacing a dead-path validity test with production-path coverage and preventing synchronous iOS network reads. Its focused validation was blocked by an Xcode/SDK version mismatch rather than a test failure.

Address the actionable findings in this review before merging.