Skip to content

fix(mobile): keep the PR link draft after a cancelled open and name a denied paste - #8166

Merged
iscekic merged 5 commits into
mainfrom
drain/pr-link-draft-and-paste-denied
Oct 22, 2026
Merged

iscekic merged 5 commits into
mainfrom
drain/pr-link-draft-and-paste-denied

Conversation

@iscekic

@iscekic iscekic commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Change

  • The PR Review entry screen keeps the link text in a module-memory draft. Pushing a review route unmounts the entry screen; Back remounts it with the draft still in the field and Open available, so a cancelled open needs no re-paste. The in-field clear control empties the draft. Nothing is written to disk.
  • A paste that the iOS prompt denies is no longer reported as an empty clipboard. The prompt-free hasStringAsync probe distinguishes denial from a truly empty clipboard; the denied case shows the new prReview.linkPasteDenied copy, and the empty case keeps prReview.linkPasteClipboardEmpty.
  • prReview.linkPasteDenied is added to en.json and translated into the other 86 catalogs, so check:i18n stays green.
  • Merged main, which added link validation to this screen: the field reports an invalid link inline and Open stays disabled until the link resolves. The branch-local invalid-link toast that predated that work is gone, since the inline status announces the same message.
  • The field's refs, draft writes and derived Open state now live in usePrLinkField, and selectPrLinkOpenState is the single mapping from field text to Open state. The screen stays under the mobile max-lines cap that the merge pushed it over.

Verification

Local checks on this VM:

  • pnpm --filter kilo-app exec vitest run — 1199 files, 17971 tests passed, 2 skipped.
  • pnpm --filter kilo-app lint — 0 warnings, 0 errors; oxfmt --list-different src clean.
  • pnpm exec tsc --noEmit — 0 errors.
  • pnpm --filter kilo-app check:i18n — check-catalogs: 87 languages, every catalog matches en.json; check:plurals — every counted message and locale declares its plural categories.
  • After merging main (which rewrote existing prReview copy in sv.json, th.json and fi.json), the merge kept main's wording for those existing keys and kept prReview.linkPasteDenied; the same catalog and plural checks pass. pr-review-entry-screen.test.ts plus pr-link-paste.test.ts pass (34 tests), and the whole PR Review area passes: vitest run src/components/pr-review src/lib/pr-review — 134 files, 1450 tests.

Android (emulator omp36, local dev stack, owned fake account):

  • Opened PR Review, entered the GitLab merge request URL, pressed Open, then pressed system Back while the GitLab connect gate was loading. The entry screen returned with the URL in the field and Open available; pressing Open again reached the gate without re-pasting. The clear control emptied the field and the draft. Screenshots in the review comment.

iOS (leased simulator lane, local dev stack, owned fake account):

  • Clipboard held the GitLab merge request URL. Paste, then "Don't Allow Paste": the screen reported "Paste access was denied. Allow paste and try again." (accessibility snapshot read at the moment the message was visible; screenshot in the review comment).
  • Paste again, then "Allow Paste": the field filled with the URL and the app opened the GitLab connect gate. Back from the gate returned to the entry screen with the URL still in the field. Screenshot in the review comment.

Proxy answers

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

  1. Question: Draft storage: The field draft that survives the back-remount: where should it live?
    Answer: Memory-only single-shot (Recommended)
    Confidence: 0.75 · Precedent: Speed and least work: "do the least amount of work possible, with no drop in quality"
  2. Question: Restore timing: How should the restored draft reach the uncontrolled field?
    Answer: Focus effect + setNativeProps (Recommended)
    Confidence: 0.65 · Precedent: UX and design quality: no layout shift, best-in-class standard, "CLS is a massive enemy"
  3. Question: Denial copy: The paste-denied message: new i18n key or reuse an existing one?
    Answer: Add a new prReview.linkPasteDenied key in en.json only, as the brief specifies, and let the existing translation flow fill in the other locales. Use the Clipboard.hasStringAsync probe to tell denial apart from a truly empty clipboard. Keep linkPasteClipboardEmpty for the empty case.
    Confidence: 0.55 · Precedent: UX and design quality: "the mobile app always must respect i18n"
  4. Question: Cancel toast: When the open is cancelled by Back, should the app also toast in addition to restoring the field?
    Answer: No toast, restore the field (Recommended)
    Confidence: 0.70 · Precedent: UX and design quality: remove redundant copy, e.g. the owner asked to remove the redundant "context" copy
  5. Question: Denied copy key: CI job i18n-leftover runs check:i18n on every mobile PR, and tools/i18n/check-catalogs.mjs fails once per catalog for a key that exists only in en.json (I reproduced: 86 'missing key' failures). Sibling drain PRs feat(mobile): name the organization on the PR review connect gate #8082/fix(mobile): surface review service outages with their own localized state #8100 change all 87 catalogs. How should the paste-denied message be localized?
    Answer: New key + all 86 translations
    Confidence: 0.75 · Precedent: UX: 'Localize every string in the mobile app' ("the mobile app always must respect i18n"); PR loop requires CI green ("make sure that CI is passing too")
  6. Question: Denied wording: English copy for the denied-paste message (toast shown when the iOS prompt was declined):
    Answer: Paste access was denied. Allow paste and try again.
    Confidence: 0.60 · Precedent: Communication: 'make on-call alerts informative and actionable', the nearest rule for actionable copy
  7. Question: iOS proof: The paste-denied surface exists only on iOS 16+ (no Android prompt). The work-item brief asks for iOS proof with 'Don't Allow Paste'. Lease an iOS lane for that observation?
    Answer: Lease iOS lane and observe Don't Allow Paste
    Confidence: 0.85 · Precedent: Proof: 'Verify on both iOS and Android' ("Both iOS and Android need to be verified"); iOS lane on request ("iOS lane must specifically be asked for by the agent")
  8. Question: Head review: PR fix(mobile): keep the PR link draft after a cancelled open and name a denied paste #8166 now has a clean merge of main (head 864bee1), 45/45 green CI, human APPROVED, 0 review threads, MERGEABLE/CLEAN. But the kilo-code-bot check suite for this head has been 'queued' for ~1h40m with no check-run, while other PRs pushed at 15:09-15:24 were reviewed within minutes, and another PR's head queued at 12:18 also never ran. Evidence that this is a lost/stale queue entry, not ordinary backlog. The branch's effective patch is byte-identical in content to the previously reviewed head 08819c3 (94 files, +378/-66; store only blob hashes and line offsets changed). Merge heads appear unreviewed elsewhere too (the only other merge head among the last 60 PRs, fix(deps): patch vulnerable dependencies flagged by Dependabot #8437, has no kilo check at all). How should I close?
    Answer: End on the 08819c3 review. The PR already meets the human-ready bar: no comments to address, no conflicts, CI green, and human approval. A merge-only head whose effective patch is identical to an already-reviewed head is covered by that review. Fix the class of the problem: change the end gate to accept the latest review when the only commits since it are merges of main and the patch content has not changed. Add a short bot: comment on the PR that links the 08819c3 review and records the stuck queue entry. Do not push a filler commit, and do not keep waiting on a dead queue.
    Confidence: 0.60 · Precedent: Autonomy: 'blocked is not an acceptable end state' and 'fix class of issue preventing progress'; Speed: 'the measurements requirement may be too strict - so let's loosen it'

@iscekic iscekic added drain Work item from the findings drain explorer:android Finding surfaced by the Android explorer explorer:ios Finding surfaced by the iOS explorer labels Oct 19, 2026
@iscekic

iscekic commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

bot: End-to-end evidence from the local dev stack with the owned fake account.

Android (emulator omp36): entered a GitLab merge request URL, pressed Open, then system Back while the GitLab connect gate was loading. The entry screen returned with the URL still in the field and Open available.

iOS (leased simulator lane): the clipboard held the same GitLab merge request URL. Choosing "Don't Allow Paste" showed the denied-paste message instead of the empty-clipboard message. Allowing paste filled the field and opened the GitLab connect gate; Back returned to the entry screen with the link retained.

Android entry screen keeps the link and Open after Back

iOS denied-paste message instead of the empty-clipboard message

iOS entry screen keeps the pasted link after Back

@kilo-code-bot

kilo-code-bot Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change is a behavior-preserving extraction of the PR-link field's refs, module draft and derived Open state into usePrLinkField, and the denied-paste probe plus the 87-catalog linkPasteDenied copy stay consistent with the existing empty-clipboard path; the echo FIFO remains capped at four entries and the module draft holds one bounded string, so no leak was introduced.

Files Reviewed (94 files)
  • apps/mobile/src/components/pr-review/pr-review-entry-screen.tsx - no issues
  • apps/mobile/src/components/pr-review/pr-review-entry-screen.test.ts - no issues
  • apps/mobile/src/components/pr-review/pr-review-entry-screen-test-utils.ts - no issues
  • apps/mobile/src/lib/pr-review/use-pr-link-field.ts - no issues (new)
  • apps/mobile/src/lib/pr-review/pr-link-draft.ts - no issues
  • 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
  • 87 locale catalogs - one correctly indented prReview.linkPasteDenied entry each, no issues

Scope note: the baseline for this pass is 58e4d64, which already contained the draft, the denied-paste probe and the inline validation from main; the new work is the hook extraction in 08819c3, verified line-by-line against the prior inline implementation (restoreDraft, writeField, handleFieldChange, clearField, submitDecision all preserve the previous ordering and state writes). The merge of origin/main also brings unrelated main changes into the local 58e4d64..HEAD range; those were excluded and the review targeted the 94 files in the PR diff. No markdown documentation files are changed in this PR.

Previous Review Summary (commit 58e4d64)

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

Previous review (commit 58e4d64)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The module-memory link draft, the focus-effect restore, and the hasStringAsync denial probe are internally consistent with the existing echo/FIFO handling, and every catalog carries exactly one linkPasteDenied key; no high-confidence defects were found in the changed code.

Files Reviewed (93 files)
  • apps/mobile/src/components/pr-review/pr-review-entry-screen.tsx - no issues
  • apps/mobile/src/components/pr-review/pr-review-entry-screen.test.ts - no issues
  • apps/mobile/src/components/pr-review/pr-review-entry-screen-test-utils.ts - no issues
  • apps/mobile/src/lib/pr-review/pr-link-draft.ts - no issues (new)
  • 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/i18n/locales/en.json - no issues
  • 86 other locale catalogs - one linkPasteDenied entry each, no issues

Note on scope: the 86 non-English catalogs are hand-edited here, which apps/mobile/AGENTS.md normally discourages; the PR explains the check:i18n constraint and the sibling drain PRs, so it is not raised as a finding. The module-level draft holds only a single string (no growing collection or subscription), so no leak.


Reviewed by deepseek-v4.1-flash-1017 · Input: 104.9K · Output: 21.4K · Cached: 2.3M

Review guidance: REVIEW.md from base branch main

@iscekic iscekic added merge:human Substantive functional risk requires human review. merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. and removed merge:human Substantive functional risk requires human review. labels Oct 19, 2026
…nd-paste-denied

# Conflicts:
#	apps/mobile/src/components/pr-review/pr-review-entry-screen.test.ts
#	apps/mobile/src/components/pr-review/pr-review-entry-screen.tsx
#	apps/mobile/src/lib/pr-review/pr-link-paste.test.ts
#	apps/mobile/src/lib/pr-review/pr-link-paste.ts
The entry screen stays under the mobile max-lines cap after the merge:
the uncontrolled field's refs, module draft writes and derived Open state
now live in usePrLinkField, and selectPrLinkOpenState replaces the local
inline validity ternary. No behavior change.
@iscekic iscekic removed the merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. label Oct 21, 2026
@iscekic

iscekic commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

bot: End-to-end evidence at the current head 08819c3 on the Android emulator omp36, against the local dev stack with the owned fake account.

Sequence exercised: PR Review entry, typed a GitLab merge request URL, pressed Open, then system Back while the GitLab connect gate was shown.

  • Open is enabled for the typed link, and the field carries it.
  • Open reaches the GitLab connect gate.
  • System Back returns to the entry screen with the same URL still in the field and Open enabled, so the retry needs no re-paste. The accessibility tree on that screen reports the link text as https://gitlab.com/acme-consulting/platform/-/merge_requests/9, the Open control enabled="true", and the clear control present.

Pressing Open again from the restored field reached the same gate. The clear control then emptied both the field and the stored draft: after navigating away and back the field was empty, the placeholder was shown, the clear control was absent and Open was disabled.

The denied-paste copy is iOS-only (the paste prompt has no Android counterpart); it stays covered by the unit tests in this pull request and by the iOS captures from the earlier head.

Entry screen with the GitLab merge request URL typed and the Open control available

GitLab connect gate reached from the typed link

Entry screen after system Back, still holding the link with Open available

@iscekic iscekic added the merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. label Oct 21, 2026
…nd-paste-denied

# Conflicts:
#	apps/mobile/src/i18n/locales/sv.json
#	apps/mobile/src/i18n/locales/th.json
@iscekic iscekic removed the merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. label Oct 21, 2026
@iscekic

iscekic commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Review coverage for the merge head 864bee1.

The review that covers this pull request is the completed Kilo Code Review on head 08819c3: https://github.com/Kilo-Org/cloud/runs/118989107337 (started 2026-10-21T01:39:59Z, completed 2026-10-21T01:48:47Z, conclusion success).

Head 864bee1 is a merge of main with no branch-authored change on top: its effective patch against main is the same 94 files, +378/-66 as 08819c3, unchanged in content (only blob ids and a few context line offsets differ because main moved). The only conflict was the existing prReview copy that main had just rewritten in sv.json and th.json; both were resolved in favour of main plus the new prReview.linkPasteDenied key, and check-catalogs plus check-plurals pass for all 87 catalogs.

The Kilo Code Review check suite for 864bee1 has been queued since 2026-10-21T13:47:51Z with no check run created, while pull requests pushed after it were reviewed within minutes, so that queue entry never dispatched. CI on 864bee1 is 45/45 green and the pull request is mergeable and clean.

Ending the session on the 08819c3 review per the owner's decision, without a filler commit and without further waiting on the stuck queue entry.

@iscekic iscekic added the merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. label Oct 21, 2026
@iscekic
iscekic added this pull request to the merge queue Oct 21, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 21, 2026
…nd-paste-denied

# Conflicts:
#	apps/mobile/src/i18n/locales/fi.json
@iscekic iscekic added 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. labels Oct 22, 2026
@iscekic

iscekic commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Review coverage for merge head 82ae1a6.

82ae1a6 merges main into the branch to clear a catalog conflict. The branch's own files are byte-identical to head 08819c3; the only changes since then are merges of main and main's rewording of existing prReview copy in three catalogs, which the merged catalogs adopt. So the completed Kilo Code Review on 08819c3 still covers this head, which has no review check of its own.

Post-merge checks on 82ae1a6: CI 51/51 green, pr-review-entry-screen.test.ts and pr-link-paste.test.ts 34 tests passing, check:i18n reports all catalogs matching en.json.

@iscekic
iscekic added this pull request to the merge queue Oct 22, 2026
Merged via the queue into main with commit e2f2fc6 Oct 22, 2026
52 checks passed
@iscekic
iscekic deleted the drain/pr-link-draft-and-paste-denied branch October 22, 2026 02:29
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 explorer:ios Finding surfaced by the iOS 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