Skip to content

fix(mobile): give the PR Review paste control a visible result on every tap - #9475

Merged
iscekic merged 2 commits into
mainfrom
drain/mobile-pr-review-paste-always-visible
Nov 1, 2026
Merged

iscekic merged 2 commits into
mainfrom
drain/mobile-pr-review-paste-always-visible

Conversation

@iscekic

@iscekic iscekic commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

Evidence

Evidence: demonstrated — on the Android emulator with the local dev stack, a non-production test account and the dev client at head a178640, every tap of the PR Review paste control ended in a visible result, including the refused-read case, which Android itself produced by denying this app its clipboard-read app-op: on the merge base the same tap left the screen untouched, at this head it shows the denied toast.

What the reported tap actually hit

The reported state — empty clipboard, tap, nothing — already ends in the Clipboard is empty toast on the merge base (19c6e8f2f), re-measured here. The tap that still ended in nothing was a refused clipboard read: the merge-base handlePaste awaited Clipboard.getStringAsync() and hasStringAsync() with no handling while the press handler calls it as void handlePaste(), so a rejection produced no field write, no inline state, no toast and no navigation.

Change

apps/mobile/src/lib/pr-review/pr-link-paste.ts gains readPrLinkPasteClipboard(reader), which never rejects: a rejected text read, or a rejected hasStringAsync() probe, becomes { kind: 'empty', reason: 'denied' }, while the existing decisions are unchanged. pr-review-entry-screen.tsx calls it instead of the raw clipboard API. So a PR link fills the field and opens, other text fills it with the existing inline invalid notice, an empty clipboard shows prReview.linkPasteClipboardEmpty, and a refused read shows prReview.linkPasteDenied instead of nothing. No new i18n key.

End-to-end on the emulator (real stack, no stubs)

Real device path: emulator omp36 at the default font scale and density, the CI dev-client APK, Metro serving this checkout, the local dev stack on this VM, a non-production test account. Every assertion below is a uiautomator node from a dump taken with the screenshot of the same tap.

Clipboard state Observed after the tap
empty toast Clipboard is empty; field empty, nothing opens
non-link text field replaced with that text, inline Not a pull request or merge request link, nothing opens
a pull request link the field is filled and the review route for the pasted reference opens
read refused by Android, this head toast Paste access was denied. Allow paste and try again.; field empty, nothing opens
read refused by Android, merge base nothing: no field text, no notice, no toast, nothing opens

The refusal is the platform's, not a stub: appops set com.kilocode.kiloapp READ_CLIPBOARD deny makes Android refuse this app's clipboard read — appops get records a new rejection at tap time, and logcat shows java.lang.SecurityException: … not allowed to perform READ_CLIPBOARD raised inside ClipboardService. No app source, node_modules file or bundle was modified for any step. That app-op also breaks TextView.canPaste during an accessibility dump, so it is reset right after the tap and before the dump; the toast outlives that.

Before/after used the same denial: the merge-base versions of the two changed sources were staged for Metro, the app force-stopped and relaunched, the denial applied, then the head sources restored with git status --porcelain empty.

Tests

pr-link-paste.test.ts covers each decision plus both rejection paths. pr-review-entry-screen-paste-read.test.ts covers the press handler for a rejected read and for a rejected probe; both fail at the merge base (no toast) and pass here.

Limits

Android's clipboard service normally answers the focused app, so a refusal needs the platform app-op denial used above. The declined-paste-prompt heuristic (an empty read while the text probe still sees text) is unchanged and is not reachable on this VM.

Design gate: 4 cells, 0 errors, 0 warnings — links /pr-review on the base, dark, compact and rtl cells; the merge-base run into base/ is also 4 cells, 0 errors, 0 warnings.

Production prerequisites

None.

Proxy answers

The owner's proxy gave these answers in the owner's place.

  1. Question: Scope: I could not reproduce the reported silence: on current main the empty-clipboard tap already shows the "Clipboard is empty" toast on the Android emulator (uiautomator node + screenshots, with and without the keyboard open). The only tap that still ends in nothing is a rejected clipboard read, swallowed by void handlePaste(). What should this work item do?
    Answer: Make every read failure end in a toast
    Confidence: 0.80 · Precedent: Autonomy: apply the reasonable fix when a defect does not reproduce; speed: choose the simplest design
  2. Question: Failure copy: Which existing copy should a failed clipboard read show? There is no dedicated failure string today (only empty and denied).
    Answer: prReview.linkPasteDenied
    Confidence: 0.60 · Precedent: Speed and least work: the brief asks to reuse the existing toast copy

…ry tap

The paste control runs as a fire-and-forget press handler, and it read the
clipboard with no error handling. A read the platform refuses therefore
disappeared: no text in the field, no inline validation, no toast, nothing.

Read the clipboard through one helper that never rejects. A refused text
read, or a refused text probe on the empty path, now reports the paste as
denied, so the tap always ends in the field being replaced or a toast that
names why nothing was pasted.
@iscekic iscekic added drain Work item from the findings drain explorer:android Finding surfaced by the Android explorer labels Nov 1, 2026
@kilo-code-bot

kilo-code-bot Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The formatter pass fixes both prior findings (over-width import and missing trailing newline) and the only other change collapses an equivalent ternary; no new defects found in the paste-read hardening.

Assumptions: none.

Files Reviewed (5 files)
  • apps/mobile/src/lib/pr-review/pr-link-paste.ts — no issues
  • apps/mobile/src/lib/pr-review/pr-link-paste.test.ts — no issues
  • apps/mobile/src/components/pr-review/pr-review-entry-screen.tsx — no issues (ternary collapse is behavior-preserving, line fits print width)
  • apps/mobile/src/components/pr-review/pr-review-entry-screen-test-utils.ts — no issues
  • apps/mobile/src/components/pr-review/pr-review-entry-screen-paste-read.test.ts — no issues (import reflowed, trailing newline added)
Previous Review Summary (commit 5ad6197)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 5ad6197)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The clipboard-read hardening is sound; the new test file fails the repo's blocking format:check on two counts (over-100-column import and missing trailing newline).

Assumptions: none.

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/mobile/src/components/pr-review/pr-review-entry-screen-paste-read.test.ts 7 Import line exceeds the 100-column printWidth; oxfmt reflows it and CI's format:check fails
apps/mobile/src/components/pr-review/pr-review-entry-screen-paste-read.test.ts 50 File lacks a trailing newline, which oxfmt normalises; CI's format:check fails
Files Reviewed (5 files)
  • apps/mobile/src/lib/pr-review/pr-link-paste.ts - no issues
  • apps/mobile/src/lib/pr-review/pr-link-paste.test.ts - no issues
  • apps/mobile/src/components/pr-review/pr-review-entry-screen.tsx - no issues
  • apps/mobile/src/components/pr-review/pr-review-entry-screen-test-utils.ts - no issues (defaults keep existing suites unaffected)
  • apps/mobile/src/components/pr-review/pr-review-entry-screen-paste-read.test.ts - 2 issues

Fix these issues in Kilo Cloud


Reviewed by deepseek-flash-latest · Input: 34.8K · Output: 2.7K · Cached: 221.7K

Review guidance: REVIEW.md from base branch main

@iscekic iscekic added merge:blocked:insufficient-e2e Verification does not cover the change merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. labels Nov 1, 2026
@iscekic

iscekic commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Android emulator proof of the PR Review paste control, on the local dev stack with an owned non-production test account.

Each capture is a real tap on the paste control in the PR link field row:

  1. Empty clipboard — the tap ends in the "Clipboard is empty" toast, not silence.
  2. Non-PR text on the clipboard — the field is filled and the inline "Not a pull request or merge request link" notice appears; nothing navigates.
  3. PR link on the clipboard — the field is filled and the review route for the pasted reference opens.
  4. Refused clipboard read (after) — with the clipboard read made to throw, the tap ends in the "Paste access was denied. Allow paste and try again." toast.
  5. Refused clipboard read (before) — the same injected failure on the base commit: the tap leaves the screen untouched, no toast, no field text — the silent outcome this change removes.
  6. Design gate contact sheet for the head run (4 cells, 0 errors, 0 warnings; base run also 0/0).

Empty clipboard shows the clipboard-empty toast

Non-PR text fills the field with the inline invalid-link notice

PR link fills the field and opens the review route

Refused clipboard read shows the denied toast after the fix

Same refused read on the base commit leaves the screen untouched

Design gate contact sheet for the head run

Reformat the new test's import and the toast selection to the shared
print width, and end the file with a newline.
@iscekic iscekic added merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:blocked:insufficient-e2e Verification does not cover the change merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. and removed merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. merge:blocked:insufficient-e2e Verification does not cover the change merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head labels Nov 1, 2026
@iscekic

iscekic commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Android emulator proof re-run at the current head (local dev stack, owned non-production test account, fresh Metro bundle).

Every capture is a real tap on the paste control in the PR link field row:

  1. Empty clipboard — the tap ends in the "Clipboard is empty" toast.
  2. Non-PR text on the clipboard — the field is filled and the inline "Not a pull request or merge request link" notice appears; nothing navigates.
  3. PR link on the clipboard — the field is filled and the review route for the pasted reference opens.
  4. Refused clipboard read (after) — with the clipboard read made to throw, the tap ends in the "Paste access was denied. Allow paste and try again." toast.
  5. Refused clipboard read (before) — the same injected failure on the base commit: the tap leaves the screen untouched, with no toast and no field text.
  6. Design gate contact sheet for this head (4 cells, 0 errors, 0 warnings; the merge-base run is 0/0 as well).

Empty clipboard shows the clipboard-empty toast

Non-PR text fills the field with the inline invalid-link notice

PR link fills the field and opens the review route

Refused clipboard read shows the denied toast after the fix

Same refused read on the base commit leaves the screen untouched

Design gate contact sheet for this head

@iscekic iscekic added merge:blocked:design-review Design review has not cleared this change and removed merge:blocked:insufficient-e2e Verification does not cover the change merge:blocked:design-review Design review has not cleared this change labels Nov 1, 2026
@iscekic iscekic added merge:blocked:insufficient-e2e Verification does not cover the change merge:blocked:design-review Design review has not cleared this change and removed merge:blocked:design-review Design review has not cleared this change labels Nov 1, 2026
@iscekic

iscekic commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Android emulator proof re-run at head a178640 (local dev stack, non-production test account, dev client on Metro serving this checkout). Each capture is one real tap on the paste control in the PR Review link row, with the uiautomator node quoted in the pull request body.

  1. Empty clipboard — the tap ends in the "Clipboard is empty" toast.
  2. Non-link text on the clipboard — the field is replaced and the inline "Not a pull request or merge request link" notice appears; nothing opens.
  3. Pull request link on the clipboard — the field is filled and the review route for the pasted reference opens.
  4. Read refused by Android (this head) — the platform denied the app its clipboard-read app-op; the tap ends in "Paste access was denied. Allow paste and try again."
  5. Read refused by Android (merge base) — the same denial on the merge-base sources: the tap leaves the screen untouched.
  6. Design gate contact sheet for this head (4 cells, 0 errors, 0 warnings; the merge-base run is 0/0 as well).

No app source, node_modules file or bundle was modified for these captures: the refusal comes from the platform app-op, and the app-op is reset before each accessibility dump.

Paste with an empty clipboard shows the clipboard-empty toast

Paste with non-link text fills the field and shows the invalid-link notice

Paste with a pull request link opens the review route

Refused clipboard read shows the paste-denied toast at this head

Same refused clipboard read on the merge base leaves the screen unchanged

Design gate contact sheet for this head

@iscekic iscekic added merge:blocked:design-review Design review has not cleared this change and removed merge:blocked:insufficient-e2e Verification does not cover the change merge:blocked:design-review Design review has not cleared this change labels Nov 1, 2026
@iscekic
iscekic added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit 4665417 Nov 1, 2026
54 checks passed
@iscekic
iscekic deleted the drain/mobile-pr-review-paste-always-visible branch November 1, 2026 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

drain Work item from the findings drain explorer:android Finding surfaced by the Android explorer merge:bot Bot-owned review; merge when ready once readiness passes and a review approves.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants