feat(android): tap-to-expand link previews in chat transcript#100898
Conversation
fc2239f to
989df73
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc2239fb8f
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| val title = sanitizeMetadataText(ogTitle ?: findTitle(html), LINK_PREVIEW_TITLE_MAX_CHARS) | ||
| val description = sanitizeMetadataText(ogDescription, LINK_PREVIEW_DESCRIPTION_MAX_CHARS) | ||
| val imageUrl = resolveSafeWebUrl(baseUrl, decodeHtmlEntities(ogImage.orEmpty())) | ||
| if (title == null && description == null && imageUrl == null) return LinkPreviewResult.Failed |
There was a problem hiding this comment.
Treat image-only metadata as unavailable until it is rendered
When a page only exposes og:image but no title or description, this condition returns Loaded because imageUrl is non-null. The card UI only renders title and description, so those expanded previews show just the domain and never display the existing “No preview available” state; either ignore image-only metadata for now or render the image before counting it as a successful preview.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 9:40 AM ET / 13:40 UTC. Summary PR surface: Other +915. Total +915 across 6 files. Reproducibility: yes. from source for the review finding: Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land this only after image-only metadata either fails with the existing unavailable state or is visibly rendered, and after a redacted Android run or screenshot shows tap-to-expand behavior with no ambient fetch. Do we have a high-confidence way to reproduce the issue? Yes from source for the review finding: Is this the best way to solve the issue? No: the branch is close, but the maintainable fix is to fail image-only metadata until image rendering exists, or actually render the image. The PR also needs real Android behavior proof before merge. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 88f1ec38d4a5. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Other +915. Total +915 across 6 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
Merged via squash.
|
Related: #100699
What Problem This Solves
Bare URLs in the Android chat transcript render as plain tappable links with no way to see what's behind them before leaving the app.
Why This Change Was Made
Adds tap-to-expand link previews: a message whose text contains a safe web link shows a compact chip (domain + expand affordance) under the body; tapping it fetches OpenGraph metadata and expands into a card (title + description), and tapping the card opens the URL exactly like tapping the link. Privacy is the design driver: no ambient fetches, ever — network activity happens only on an explicit user tap (see #22060 for why automatic preview fetches on message content are an injection/exfiltration hazard). The preview fetch is strictly less capable than tapping the link itself: same URL, same user gesture, display-only result — fetched with a locked-down client (no cookies, no auth/proxy authenticators, no auto-redirects; each of ≤3 manual redirect hops re-validated http/https; 6s total deadline; 512KB body cap;
text/htmlonly). Metadata is untrusted plain text: HTML entities decoded, control characters stripped, whitespace collapsed, length-capped, rendered as plainText. Results (including failures) sit in a 64-entry in-memory LRU — nothing persists to disk. The card is text-only for now: the Android transcript has no remote-image facility and this PR deliberately adds no dependency for one (og:image is parsed and safe-resolved for when one exists).User Impact
Android users can peek at a link's title and description before opening it, with zero background network traffic from message content. Failed lookups show a plain "No preview available" state.
Evidence
ChatLinkPreviewTestgreen (12 tests): OG parsing (full set,<title>fallback, attribute-order tolerance, relative og:image resolution, control-char stripping and caps), first-URL extraction (bare URLs, markdown links, code-fence/inline-code exclusion, unsafe schemes skipped), MockWebServer fetcher (redirect chain, >3 hops rejected, non-HTML rejected, body cap, timeout), cache hit avoids refetch (request-count assertion).libs.okhttp, used by the gateway session) — no new dependency.corepack pnpm native:i18n:syncrun for the new strings.