Fall back to LPMetadataProvider for link previews

[onefovrth]

Summary

• x.com/twitter.com links now show rich link previews using Apple's LPMetadataProvider (same approach as iMessage)
• When the standard OpenGraph tag fetch returns nothing (as with x.com's JS-rendered pages), the system falls back to LPMetadataProvider which uses WebKit to render the page and extract metadata
• Follows the existing ConvosCore/ConvosCoreiOS bridge pattern: protocol in ConvosCore, iOS implementation using LinkPresentation framework, configured at app launch

What changed

• OpenGraphService: Refactored fetch into fetchOpenGraphTags helper; added RichLinkMetadata.provider fallback when OG parsing returns nil
• RichLinkMetadataProviding (new): Protocol + singleton accessor in ConvosCore
• IOSRichLinkMetadataProvider (new): iOS implementation using LPMetadataProvider, extracts title + image, pre-caches images in ImageCache
• ConvosCoreiOS: Configures RichLinkMetadata alongside other platform providers

Next steps

• New social media post card format for x.com, threads.net, bsky.app (design pending)

Test plan

• Unit tests pass (swift test --package-path ConvosCore)
• Paste an x.com tweet URL in composer — verify preview shows title, image, and "x.com" subtitle
• Paste a normal URL (e.g. apple.com) — verify standard OG preview still works
• Verify link preview in message bubble after sending

🤖 Generated with Claude Code

Note

Fall back to LPMetadataProvider for link previews when OpenGraph tags are missing

• Adds a RichLinkMetadataProviding protocol and a global RichLinkMetadata registry so platform-specific providers can supply metadata for links that lack OpenGraph tags.
• Implements IOSRichLinkMetadataProvider using LPMetadataProvider to fetch title and image, validate image data/size, cache via ImageCache, and reject private hosts.
• OpenGraphService now falls back to the configured provider when OG tag parsing yields no result, returning metadata where it previously returned nil.
• Makes LinkPreview.isPrivateHost public so the iOS provider can reuse it.
• Behavioral Change: links without OG tags that previously returned nil may now return metadata on iOS.

^{Macroscope summarized 289c3b4.}

[claude]

Claude finished @onefovrth's task —— View job

---

Code Review Complete

I've reviewed PR #685 and have comprehensive feedback on code quality, potential bugs, performance, security, and test coverage.

Tasks:

• Read and analyze all changed files
• Evaluate architecture and code quality
• Identify potential bugs and issues
• Review security considerations
• Assess performance implications
• Review test coverage
• Provide comprehensive feedback

---

Architecture & Code Quality ✅

Excellent architecture choices:

• Protocol bridge pattern: The RichLinkMetadataProviding protocol in ConvosCore with IOSRichLinkMetadataProvider implementation in ConvosCoreiOS follows the established codebase pattern perfectly (matches PushNotificationRegistering pattern)
• Thread-safe singleton: RichLinkMetadata registry uses NSLock for safe configuration across threads
• Proper separation: iOS-specific LinkPresentation framework is isolated in ConvosCoreiOS, keeping ConvosCore platform-independent
• Clean fallback logic: OpenGraphService.fetchMetadata tries OG tags first, then falls back to provider only when OG parsing returns nil (lines 61-73 in OpenGraphService.swift:48)

Code quality notes:

• Clean async/await throughout
• Proper error handling with logging
• Good use of @unchecked Sendable for ExtractedMetadata wrapper around non-Sendable NSItemProvider

---

Critical Bug 🔴 - Cache Key vs URL Issue

The Macroscope review correctly identified a critical bug. However, I've analyzed the code and found that the bug was already fixed in the current commit (289c3b4).

Original problematic code (mentioned in Macroscope review):

// IOSRichLinkMetadataProvider.swift lines 95-96 (OLD)
let cacheKey = "richlink:\(originalURL.absoluteString)"
return cacheKey  // ❌ Returns cache key as imageURL

Current code (289c3b4):

// IOSRichLinkMetadataProvider.swift lines 95-96 (CURRENT)
let imageURL = originalURL.absoluteString
ImageCache.shared.cacheImage(image, for: imageURL, storageTier: .cache)
return imageURL  // ✅ Returns actual URL

Why this works correctly now:

1. IOSRichLinkMetadataProvider returns the actual URL (originalURL.absoluteString) in imageURL field
2. Image is cached using the same URL as the key (line 96)
3. LinkPreviewBubbleView.loadImage(from:) checks cache using url.absoluteString (line 138)
4. Cache hit succeeds, avoiding network fetch
5. On cache miss, OpenGraphService.loadImage(from:) can fetch the URL (it's a valid HTTPS URL)

The fix was already applied in commit 289c3b4, so this issue is resolved.

---

Security ✅

Well-protected against common security issues:

1. SSRF protection: isPrivateHost() correctly rejects localhost, private IPs (10.x, 192.168.x, 172.16-31.x), link-local (169.254.x.x), IPv6 loopback/link-local/ULA

   • Checked in both LinkPreview.from(text:) (line 49 in LinkPreview+Detection.swift)
   • AND in IOSRichLinkMetadataProvider (line 12)
   • Prevents fetching metadata from internal services

2. Redirect validation: SafeRedirectDelegate blocks redirects to private hosts (lines 294-295 in OpenGraphService.swift)

3. Resource limits:

   • HTML fetch capped at 200KB (line 109)
   • Image data capped at 5MB (line 144-146)
   • Image dimensions validated (32-4096px, lines 164-167)
   • Valid image formats only (JPEG/PNG/GIF/WebP magic byte check)

4. Timeout protection: 8-second timeout on metadata fetch (line 44, line 93)

5. No secrets exposed: No API keys or sensitive data in code

Minor suggestion: Consider adding URL scheme validation in IOSRichLinkMetadataProvider.fetchMetadata to ensure only HTTPS URLs are processed (currently relies on upstream validation).

---

Performance Considerations ⚠️

Concerns to monitor:

1. LPMetadataProvider latency: As noted in the PR description, LPMetadataProvider uses WebKit to render pages, which adds significant latency for JS-heavy sites like x.com

   • Mitigation in place: Only triggers on OG tag failure (fallback path)
   • Good: Result is cached in OpenGraphService.cache (max 100 entries, LRU eviction)

2. Image extraction overhead: NSItemProvider.loadDataRepresentation is async but happens on every fallback (lines 73-88 in IOSRichLinkMetadataProvider.swift)

   • Concern: For a conversation with many x.com links, initial load could be slow
   • Mitigation: Images are pre-cached in ImageCache before returning metadata (line 96)
   • Future optimization: Consider background prefetching for visible link previews

3. Main thread blocking: LPMetadataProvider.startFetchingMetadata callback may run on main thread

   • Current code is safe: Wrapped in withCheckedThrowingContinuation, allows suspension
   • Good practice: Consider explicitly dispatching to background queue if performance issues arise

Recommendation: Monitor real-world latency with Logger entries. If fallback path is slow, consider:

• Background prefetch queue for link previews in visible messages
• Shorter timeout for LPMetadataProvider (currently inherits 8s from OpenGraphService)

---

Test Coverage ✅

Strong test coverage added:

New tests in LinkPreviewTests.swift (lines 387-413):

• RichLinkMetadataFallbackTests suite covers:
  • Default nil provider ✅
  • Configuration ✅
  • Reset for testing ✅
  • Mock provider pattern ✅

What's tested:

• Protocol registration and reset
• Mock provider implementation example

What's NOT tested (acceptable for this PR):

• IOSRichLinkMetadataProvider implementation (would require LinkPresentation framework, iOS Simulator)
• Integration test with actual x.com URL (would require network, flaky)
• Image caching behavior in fallback path (covered by existing ImageCache tests)

Recommendation: Consider adding integration tests in a manual test plan or screenshot tests for x.com link previews in the PR description's "Test plan" section.

---

Additional Issues Found

None. The code is clean, well-structured, and follows codebase conventions.

---

Minor Suggestions (Non-blocking)

1. Logging clarity: Line 67 logs "falling back to LPMetadataProvider" but could include URL for easier debugging:

   Log.info("OpenGraph tags missing for \(urlString), falling back to RichLinkMetadataProvider")

2. Error domain: Custom error domain "IOSRichLinkMetadataProvider" (lines 60, 82) could use a constant to avoid typos

3. Image validation consistency: IOSRichLinkMetadataProvider uses OpenGraphService.isValidImageData/Size (lines 90-92) - good reuse, but creates a dependency from ConvosCoreiOS → ConvosCore service. Consider moving validation helpers to a shared utility if more providers need them.

4. Test URL suggestion: Add a comment in test plan showing example x.com URL to test with (e.g., https://x.com/elonmusk/status/...)

---

Summary

Overall assessment: Excellent PR with one critical bug that was already fixed. ✅

Strengths:

• Follows established architecture patterns
• Strong security posture with SSRF protection
• Clean error handling and logging
• Good test coverage for core protocol logic
• Image caching optimizes repeated loads

Fixed in latest commit:

• ✅ Cache key vs URL bug (Macroscope finding) - resolved in 289c3b4

Minor recommendations:

• Monitor LPMetadataProvider latency in production
• Consider adding manual test examples to PR description
• Consider extracting image validation to shared utility

Verdict: Ready to merge pending manual testing with x.com URLs. The architecture is solid, security is well-handled, and the critical bug was already addressed.

---

[macroscopeapp]

Approvability

Verdict: Needs human review

This PR introduces a new fallback mechanism that changes user-facing behavior: links without OpenGraph tags will now attempt to fetch previews via iOS's LPMetadataProvider. While well-implemented with good test coverage, this is a new feature that expands what content displays to users, warranting human review.

^{You can customize Macroscope's approvability policy. Learn more.}