Skip to content

fix(mobile): bound sign-in email input and code destination - #8670

Merged
iscekic merged 4 commits into
mainfrom
drain/email-code-entry
Oct 24, 2026
Merged

iscekic merged 4 commits into
mainfrom
drain/email-code-entry

Conversation

@iscekic

@iscekic iscekic commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Evidence

Evidence: demonstrated — on the Android emulator against the isolated local stack, a 3,508-character address and a malformed address stayed on the email step with the localized inline error and created no sign-in token; a 249-character fake address and a normal fake address reached a code step whose destination was middle-abbreviated (or complete) with the code input, Verify, Resend and Back all visible, and the normal fake address completed sign-in with an OTP read from the local email sink. On iOS, a 74-character fixture showed the abbreviated destination with the code input and actions visible, and six digits enabled Verify. New regression cases fail against the previous behavior and pass with the change; the mobile auth and form suites pass (163 tests), and the same checks were repeated after merging main.

Change

The mobile email step validates the trimmed, lowercased address against the existing syntax rule and a 254-character maximum, keeping the localized inline message beside the field and never sending an ineligible address. The code step's destination prompt abbreviates addresses longer than 48 characters in the middle, so the code input, Verify, Resend and Back stay in the normal layout. Delivery, challenge binding and verification still use the full address; server eligibility, SSO and token policy are unchanged.

The full-address layout assertions that pinned the previous rendering are replaced by a boundary case that keeps submission intact.

Verification

  • pnpm --filter kilo-app exec vitest run src/lib/auth/use-native-auth.test.ts src/components/login/__tests__/email-otp-form.mounted.test.tsx: 163 passed, also after the main merge.
  • Reverting only the two behavior lines reproduced the new failures (255 and 3,508 characters accepted; long destination not abbreviated), then the fix restored them.
  • pnpm --filter kilo-app typecheck, changed-file oxlint, and oxfmt are clean, also after the main merge.
  • Android: headless emulator, local backend, local database volume, locally captured email delivery.
  • iOS: leased simulator lane against the same local backend and Metro.
  • Merge-head re-run on the same local stack: a 3,508-character address and a 255-character syntactically valid address each stayed on the email step with the inline message, the field and Continue visible, and no code request reached the backend; a 249-character address reached the code step with a middle-abbreviated destination and visible code input, Verify, Resend and Back; a short fixture address reached the code step with the complete destination and enabled Verify once six digits were entered, and finished sign-in through the local mail sink.
  • The 255-character address is syntactically valid, so its rejection shows the length rule rather than only the syntax rule.

Known limits: the 254-character maximum is a client rule; the shared OTP route is unchanged. The destination abbreviation is display-only.

Proxy answers

  1. Question: Email contract: use the requested 254-character cap only in mobile, alongside the existing syntax check, and shorten only the displayed destination to 48 characters with a middle ellipsis, leaving server eligibility, SSO and verification rules unchanged?
    Answer: Client cap and bounded display
    Confidence: 0.60 · Precedent: Put each separate concern in its own PR: session debabfb38c82 asked for the flag removal "in a separate PR". The brief scopes this item to the mobile files, and a server cap on the shared OTP auth API is a separate concern.
  2. Question: The two added commits to PR fix(mobile): bound sign-in email input and code destination #8670 are main-merge commits. CI is green, the human review is approved, and there are no remarks, but the PR loop confirms there is no Kilobot check on this merge head and does not converge. The prior Kilobot summary reviewed the unchanged implementation before the merge. How should I proceed?
    Answer: Accept the previous review
    Confidence: 0.72 · Precedent: Regrade a PR only after meaningful changes since the last grade: session 55c9c80332a7, "regrading should only happen if there were meaningful changes"; main-merge commits do not change the reviewed implementation, and human ready is met (clean Kilobot review, CI green, no conflicts).

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

iscekic commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

bot: E2E captures from the isolated local stack, using owned fake test accounts and the local email sink.

  • Oversized input: the email step stayed visible with its inline address error and created no sign-in token.
  • Long accepted address: the destination is middle-abbreviated, while the code input, Verify, Resend and Back all remain visible.
  • Normal fake address: the complete destination, code input and Verify are visible, and sign-in completed with the code from the local email sink.
  • iOS lane: the same abbreviated destination with the code input and Verify action visible; entering six digits enabled Verify.

Oversized input refused inline

Abbreviated destination with code controls

Normal destination with code controls

Abbreviated destination on iOS

@kilo-code-bot

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

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change extracts the destination-abbreviation literals into named constants with unchanged behavior (24 + ellipsis + 23 = the 48-character display bound) and adds regression assertions that the destination text carries no truncation props; no new defects found.

Assumptions: [INFERENCE] The review was read-only, so vitest, typecheck, and lint were not executed here; conclusions rest on the incremental diff, the full changed component and test, and the unchanged files verified as untouched since the previous review at c577c90. No memory-leak risk in the changed code: the diff adds only module-level constants and test assertions, with no subscriptions, timers, listeners, or retained references.

Files Reviewed (5 files)
  • apps/mobile/src/components/login/email-otp-form.tsx
  • apps/mobile/src/components/login/__tests__/email-otp-form.mounted.test.tsx
  • apps/mobile/README.md (unchanged since previous review)
  • apps/mobile/src/lib/auth/use-native-auth.ts (unchanged since previous review)
  • apps/mobile/src/lib/auth/use-native-auth.test.ts (unchanged since previous review)
Previous Review Summary (commit c577c90)

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

Previous review (commit c577c90)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The mobile email step now caps the trimmed, lowercased address at 254 characters before any delivery request, and the code-step destination shortens only its display beyond 48 characters while submission and verification keep the full address; the accompanying regression tests cover both the rejection and the 254-character boundary.

Assumptions: [INFERENCE] The review was read-only, so the vitest, typecheck, and lint commands were not executed here; conclusions rest on the diff, the full changed files, the unchanged OTP route, and the verification reported in the PR description. No memory-leak risk was found in the changed code (no new subscriptions, timers, listeners, or retained references).

Files Reviewed (5 files)
  • apps/mobile/README.md
  • apps/mobile/src/components/login/email-otp-form.tsx
  • apps/mobile/src/components/login/__tests__/email-otp-form.mounted.test.tsx
  • apps/mobile/src/lib/auth/use-native-auth.ts
  • apps/mobile/src/lib/auth/use-native-auth.test.ts

Reviewed by deepseek-flash-latest · Input: 32.3K · Output: 2.3K · Cached: 116K

Review guidance: REVIEW.md from base branch main

@iscekic iscekic added the merge:human Substantive functional risk requires human review. label Oct 22, 2026
@iscekic iscekic removed the merge:human Substantive functional risk requires human review. label Oct 23, 2026
@iscekic

iscekic commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Merge-head re-check of the email sign-in flow on the isolated local stack: headless Android emulator, local backend, local mail sink, owned fake test account. The captures come from the email step and the code step only.

  • A 3,412-character address and a 255-character syntactically valid address each stayed on the email step with the inline invalid-address message, the email field and Continue both visible, and no sign-in token row was created.
  • A 249-character address reached the code step with the destination abbreviated in the middle, beside a visible code input, Verify, Resend and Back.
  • A short fixture address reached the code step with the destination shown in full, and six digits enabled Verify.

The re-check stops at the code step: the local device check refused the token exchange, which is unrelated to these two steps.

Email step with an over-long address and the inline invalid-address message, field and Continue visible

Email step with a syntactically valid address above the length bound and the same inline message

Code step with the destination abbreviated in the middle above a code input, Verify, Resend and Back

Code step with the complete destination, code input, disabled Verify, Resend and Back

@iscekic iscekic added the merge:human Substantive functional risk requires human review. label Oct 23, 2026
@iscekic iscekic removed the merge:human Substantive functional risk requires human review. label Oct 23, 2026
@iscekic

iscekic commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Android proof on the current head, against the isolated local stack (headless emulator, local backend, local database, locally captured mail).

  • A 3,508-character address and a 255-character syntactically valid address each kept the email step: the field showed the inline message "Check your email address and try again." with the address field and Continue still present, and the backend log recorded no code request for either submission (only the app-version check from the launches).
  • A 249-character address reached the code step with its destination abbreviated in the middle, and the code input, Verify, Resend and Back all in the normal layout.
  • A short fake test address reached the code step with its destination complete; six digits enabled Verify, and Verify answered the code request, the admission challenge and the token request with 200, then the app landed on onboarding as the seeded fake user.

Each capture was taken right after a uiautomator dump that reported the quoted text, enabled state and bounds; the assertions are those dumps.

Email step showing the inline validation message after a 3,508-character address

Email step showing the same validation message for a 255-character address

Code step with the middle-abbreviated destination and the code controls

Code step with a complete destination and Verify enabled after six digits

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:human Substantive functional risk requires human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants