Skip to content

feat(android): og:image thumbnails on link preview cards#101396

Merged
steipete merged 1 commit into
mainfrom
feat/android-link-preview-image-nash
Jul 7, 2026
Merged

feat(android): og:image thumbnails on link preview cards#101396
steipete merged 1 commit into
mainfrom
feat/android-link-preview-image-nash

Conversation

@steipete

@steipete steipete commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Related: #100699

What Problem This Solves

Android's tap-to-expand link-preview cards landed text-only in #100898; this delivers the declared og:image follow-up, matching the iOS sibling (#101387).

Why This Change Was Made

The expanded card now loads the OpenGraph image through the same locked-down OkHttp fetcher as the metadata — no new dependency, no ambient traffic. The image request runs only on the post-expansion path with Loaded metadata; the image URL independently passes the non-public-host policy (DNS-pinned client) before any request. Image mode refactors the fetch into one shared body path with per-mode rules: image/jpeg|png|webp allowlist, 1 MiB hard cap (oversized rejected, not truncated), same redirect/deadline/no-cookie contract. A review-hardening round added byte-level format sniffing so a spoofed content-type cannot smuggle another decoder path. Decoding is bounds-first with inSampleSize targeting ≤600px, with Runtime/OOM guards; failures cache negatively and the card stays text-only. Thumbnails render decorative (no content description) at ≤120dp; text renders immediately with no reserved placeholder, keeping failed/text-only cards compact at the cost of one bounded shift when an image arrives.

User Impact

Android link-preview cards show the page's image when available, under the same zero-ambient-fetch contract.

Evidence

  • ChatLinkPreviewTest — 22 tests green: post-expansion-only fetch gating, host policy on image URLs, MIME + byte-format spoof rejection, size cap, redirect rules in image mode, downsampling bounds, corruption → negative cache, cache-hit request counting.
  • Play-debug Kotlin compile green; ktlint clean on changed files; corepack pnpm native:i18n:sync run.
  • Structured review (codex): one finding accepted and fixed (encoded-format validation); fixed-height suggestion rejected per the max-height contract; final rerun clean.
  • Unverified: on-device visual pass (an attached device was available but installing an unreviewed build was not authorized).

@openclaw-barnacle openclaw-barnacle Bot added app: android App: android size: M maintainer Maintainer-authored PR labels Jul 7, 2026
@clawsweeper

clawsweeper Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 7, 2026, 2:03 AM ET / 06:03 UTC.

Summary
The PR adds Android OpenGraph thumbnail fetching, bounded bitmap decoding/caching, thumbnail rendering in expanded link-preview cards, i18n line updates, and focused Android unit tests.

PR surface: Other +288. Total +288 across 4 files.

Reproducibility: not applicable. this is a feature PR, not a bug report with a failing current-main reproduction path. Source inspection confirms current main is text-only and the PR adds the requested Android thumbnail path.

Review metrics: 1 noteworthy metric.

  • Remote image fetch mode: 1 added. The PR adds one new user-initiated network fetch and bitmap decode path, which maintainers should review as a privacy and security boundary before merge.

Stored data model
Persistent data-model change detected: vector/embedding metadata: apps/android/app/src/main/java/ai/openclaw/app/ui/chat/ChatMessageViews.kt. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #101396
Summary: This PR is the Android og:image thumbnail implementation candidate; the related items are a closed umbrella tracker, the merged Android text-preview base, and a sibling iOS thumbnail PR rather than duplicates.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add a redacted Android device or emulator screenshot/recording showing tap-to-expand rendering a thumbnail, with private URLs or endpoints hidden.
  • Get maintainer acceptance of the remote image fetch/decode boundary after proof is posted.
  • Refresh the branch and clear the Android unit-test lane if the unrelated gateway auth failure persists.

Proof guidance:

  • [P1] Needs real behavior proof before merge: No after-fix real Android proof is present; the PR body lists tests and compile/lint evidence and explicitly says the on-device visual pass was unverified. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Real Android behavior proof is missing for the visible thumbnail UI; tests and compile output do not show the post-expansion card rendering on a device or emulator.
  • [P1] The PR adds a second user-initiated network request plus untrusted remote bitmap decoding, so the narrowed MIME, byte cap, DNS, redirect, and no-cookie policy should get explicit maintainer review before merge.
  • [P1] The exact head is behind current main and has an Android unit-test lane red in an apparently unrelated gateway auth test; it still needs a refreshed green merge gate before landing.

Maintainer options:

  1. Require proof plus boundary review (recommended)
    Merge only after redacted Android device/emulator evidence shows the thumbnail rendering and a maintainer accepts the narrowed remote-image fetch/decode policy.
  2. Keep Android text-only for now
    Pause or close this branch if maintainers are not ready to add remote image decoding to Android chat previews.

Next step before merge

  • [P1] Protected maintainer labeling, missing real Android proof, and the remote-image security boundary make this a human review item rather than a safe automated repair lane.

Maintainer decision needed

  • Question: Is the Android remote-thumbnail policy acceptable to ship once real device or emulator proof is added?
  • Rationale: The PR adds remote image fetching and bitmap decoding for untrusted URLs, which is a privacy/security boundary that unit tests alone cannot settle.
  • Likely owner: steipete — He authored the Android link-preview base PR and this follow-up, and is the clearest owner for the mobile polish/security tradeoff.
  • Options:
    • Ship narrowed policy after proof (recommended): Keep JPEG/PNG/WebP only, public-host DNS validation, no cookies, hard caps, and merge after redacted Android visual proof confirms the tap-to-expand thumbnail path.
    • Require stricter image policy: Ask for additional format, dimension, or rendering constraints before accepting the new remote decode surface.
    • Pause Android thumbnails: Keep Android link previews text-only until maintainers are ready to own remote image loading in the mobile transcript.

Security
Needs attention: The diff adds a security-sensitive remote thumbnail fetch/decode path; I found no discrete code defect, but the boundary needs maintainer review and real behavior proof before merge.

Review details

Best possible solution:

Keep the Android thumbnail implementation narrow and user-initiated, land it only after redacted device/emulator proof and maintainer acceptance of the remote-image boundary, and leave current text-only previews as the fallback when image fetch or decode fails.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this is a feature PR, not a bug report with a failing current-main reproduction path. Source inspection confirms current main is text-only and the PR adds the requested Android thumbnail path.

Is this the best way to solve the issue?

Yes directionally: Android chat UI is the right owner boundary and the patch reuses the existing locked-down preview fetcher. It is not merge-ready until real Android visual proof and security-boundary review are complete.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against b66220948882.

Label changes

Label changes:

  • add P3: This is mobile chat polish for an optional expanded-card thumbnail, not a blocked setup path or urgent runtime regression.
  • add merge-risk: 🚨 security-boundary: Merging adds remote thumbnail fetching and untrusted bitmap decoding under a new policy that tests alone do not fully settle.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix real Android proof is present; the PR body lists tests and compile/lint evidence and explicitly says the on-device visual pass was unverified. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P3: This is mobile chat polish for an optional expanded-card thumbnail, not a blocked setup path or urgent runtime regression.
  • merge-risk: 🚨 security-boundary: Merging adds remote thumbnail fetching and untrusted bitmap decoding under a new policy that tests alone do not fully settle.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix real Android proof is present; the PR body lists tests and compile/lint evidence and explicitly says the on-device visual pass was unverified. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Other +288. Total +288 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 4 356 68 +288
Total 4 356 68 +288

Security concerns:

  • [medium] Remote thumbnail fetch/decode boundary — apps/android/app/src/main/java/ai/openclaw/app/ui/chat/ChatLinkPreview.kt:154
    The new image path fetches and decodes remote JPEG/PNG/WebP bytes from page metadata; the PR includes caps, MIME sniffing, no cookies, redirect limits, and public-host policy, but maintainers should explicitly accept this boundary before merge.
    Confidence: 0.78

What I checked:

Likely related people:

  • steipete: He authored the merged Android text-only link-preview PR, the current thumbnail follow-up, and current main blame attributes the checked-out Android link-preview files to him. (role: feature owner and recent area contributor; confidence: high; commits: 989df7362532, cd25de98b1b9, ad5b81e88493; files: apps/android/app/src/main/java/ai/openclaw/app/ui/chat/ChatLinkPreview.kt, apps/android/app/src/main/java/ai/openclaw/app/ui/chat/ChatMessageViews.kt, apps/android/app/src/test/java/ai/openclaw/app/ui/chat/ChatLinkPreviewTest.kt)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jul 7, 2026
@steipete steipete self-assigned this Jul 7, 2026
@steipete
steipete merged commit 6125dd9 into main Jul 7, 2026
305 of 315 checks passed
@steipete
steipete deleted the feat/android-link-preview-image-nash branch July 7, 2026 09:42
@steipete

steipete commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

steipete added a commit that referenced this pull request Jul 7, 2026
…plicit intents (#101799)

* fix(android): keep cold-start gateway auto-connect from overriding explicit intents

NodeRuntime's init-time auto-connect now atomically claims the first gateway
lifecycle intent (CAS 0->1) and stands down permanently when any explicit
connect/disconnect/switch intent already exists, so a late discovery emission
can no longer override a user decision. This was also the root cause of the
CI-only flakes in GatewayBootstrapAuthTest (refreshGatewayConnection_* at
:616/:627 and switchToUndiscoveredGateway* on PRs #101002/#101387/#101396).

GatewaySession.Connection.sendRequestFrame now starts its response watcher
with CoroutineStart.ATOMIC: disconnect teardown cancels connectionJob right
after failPending(), and a DEFAULT-start watcher still queued for dispatch was
cancelled without running, silently dropping the terminal UNAVAILABLE onError
owed to fire-and-forget callers (root cause of the flaky
GatewaySessionInvokeTest.disconnectReportsUnknownOutcomeForFireAndForgetRpc).

Tests neutralize runtime background work (cancel+join the runtime scope)
before arming the registry and scripting lifecycle steps, and add regression
coverage for the auto-connect intent claim.

Proof: refresh flake reproduced at iteration 1 under taskset -c 0,1 on a
Blacksmith Linux box; post-fix 3 pinned stress rounds (120/150 looped
iterations) plus full :app:testPlayDebugUnitTest (109 classes, 0 failures)
and :app:ktlintCheck green on the same box.

* chore(i18n): resync native inventory line positions after NodeRuntime edits
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 8, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 8, 2026
…plicit intents (openclaw#101799)

* fix(android): keep cold-start gateway auto-connect from overriding explicit intents

NodeRuntime's init-time auto-connect now atomically claims the first gateway
lifecycle intent (CAS 0->1) and stands down permanently when any explicit
connect/disconnect/switch intent already exists, so a late discovery emission
can no longer override a user decision. This was also the root cause of the
CI-only flakes in GatewayBootstrapAuthTest (refreshGatewayConnection_* at
:616/:627 and switchToUndiscoveredGateway* on PRs openclaw#101002/openclaw#101387/openclaw#101396).

GatewaySession.Connection.sendRequestFrame now starts its response watcher
with CoroutineStart.ATOMIC: disconnect teardown cancels connectionJob right
after failPending(), and a DEFAULT-start watcher still queued for dispatch was
cancelled without running, silently dropping the terminal UNAVAILABLE onError
owed to fire-and-forget callers (root cause of the flaky
GatewaySessionInvokeTest.disconnectReportsUnknownOutcomeForFireAndForgetRpc).

Tests neutralize runtime background work (cancel+join the runtime scope)
before arming the registry and scripting lifecycle steps, and add regression
coverage for the auto-connect intent claim.

Proof: refresh flake reproduced at iteration 1 under taskset -c 0,1 on a
Blacksmith Linux box; post-fix 3 pinned stress rounds (120/150 looped
iterations) plus full :app:testPlayDebugUnitTest (109 classes, 0 failures)
and :app:ktlintCheck green on the same box.

* chore(i18n): resync native inventory line positions after NodeRuntime edits
giodl73-repo pushed a commit to giodl73-repo/openclaw that referenced this pull request Jul 8, 2026
giodl73-repo pushed a commit to giodl73-repo/openclaw that referenced this pull request Jul 8, 2026
…plicit intents (openclaw#101799)

* fix(android): keep cold-start gateway auto-connect from overriding explicit intents

NodeRuntime's init-time auto-connect now atomically claims the first gateway
lifecycle intent (CAS 0->1) and stands down permanently when any explicit
connect/disconnect/switch intent already exists, so a late discovery emission
can no longer override a user decision. This was also the root cause of the
CI-only flakes in GatewayBootstrapAuthTest (refreshGatewayConnection_* at
:616/:627 and switchToUndiscoveredGateway* on PRs openclaw#101002/openclaw#101387/openclaw#101396).

GatewaySession.Connection.sendRequestFrame now starts its response watcher
with CoroutineStart.ATOMIC: disconnect teardown cancels connectionJob right
after failPending(), and a DEFAULT-start watcher still queued for dispatch was
cancelled without running, silently dropping the terminal UNAVAILABLE onError
owed to fire-and-forget callers (root cause of the flaky
GatewaySessionInvokeTest.disconnectReportsUnknownOutcomeForFireAndForgetRpc).

Tests neutralize runtime background work (cancel+join the runtime scope)
before arming the registry and scripting lifecycle steps, and add regression
coverage for the auto-connect intent claim.

Proof: refresh flake reproduced at iteration 1 under taskset -c 0,1 on a
Blacksmith Linux box; post-fix 3 pinned stress rounds (120/150 looped
iterations) plus full :app:testPlayDebugUnitTest (109 classes, 0 failures)
and :app:ktlintCheck green on the same box.

* chore(i18n): resync native inventory line positions after NodeRuntime edits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: android App: android maintainer Maintainer-authored PR merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant