Repository navigation
fix(mobile): fence the to-do undo delete to its auth session and cap drafts on code points - #8566
Conversation
The undo timer sends the held delete five seconds later. Capture the auth epoch when the timer is scheduled and skip the request, the failure toast and the cache refresh once that session is gone or a sign-out is under way, mirroring the fence linkStartedTodoSession already uses. Also cap a draft's title and notes on a code point: an emoji that straddles the router's code-unit limit is dropped whole instead of leaving an unpaired surrogate. Tests cover a sign-out, an account switch, and a failure that lands after a sign-out inside the undo window, plus an emoji straddling each cap.
iscekic
added
drain
|
bot: Visual proof of the to-do undo path, captured on the Android emulator against the local dev stack with a local fake account and locally seeded to-dos. Sequence:
The draft-cap change is covered by the module tests; typing an astral emoji exactly at either boundary on the emulator was not exercised.
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe follow-up replaces the raw-cache assertion with a consumer-level read through the held-delete Assumptions: Files Reviewed (4 files)
Previous Review Summary (commit 55e5c5f)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 55e5c5f)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe auth-epoch fence on the undo delete and the code-point draft cap are correct; the one finding is a test assertion that does not exercise the new held-id release path. Assumptions: Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (4 files)
Reviewed by deepseek-flash-latest · Input: 57.2K · Output: 3.2K · Cached: 176.4K Review guidance: REVIEW.md from base branch |
The undo-window test read the raw query cache, which never filters held ids, so its length assertion could not distinguish a released id. Read the list through `useTodoItems` instead: the row is hidden while held and listed again after a skipped delete.
iscekic
added
merge:blocked:insufficient-e2e
|
bot: Android verification of this branch against the local backend, with a locally seeded fake account. Captures show the list after a new-session draft was saved as a to-do, the undo toast after a delete, the signed-out state reached inside the undo window, and the item listed again after signing back in. The stored rows were checked in the local database: the sign-out run added no delete request and kept the row, while a control delete after the window removed it.
|
iscekic
removed
the
merge:blocked:insufficient-e2e
Evidence
Evidence: demonstrated — on the Android emulator the to-do delete was exercised against the running local backend and database: deleting and then signing out inside the undo window sent no delete request and kept the row, a control delete after the window sent one request and removed the row, and a new-session draft whose cap boundary fell inside an emoji was saved through the real to-do create path as a well-formed title and notes.
Android runtime evidence (this branch's JavaScript)
The app ran this branch's JavaScript from the Metro dev server, on the
omp36emulator, signed into a locally seeded fake account. No production accounts or data were used. The backend and database were the local ones.Undo-delete fence, judged from the backend request log and the stored to-do rows:
Draft caps through the real conversion path (new session, then discard, then save as to-do), with the draft text entered on the emulator:
Screenshots of the observed states are attached to a comment on this pull request.
Regression coverage
apps/mobile/src/lib/hooks/use-todos.remove-with-undo.test.tsexercises the real hook with a controlled tRPC boundary and a real query cache: same-session deletion, sign-out during the window, replacement of the auth epoch, and a request failure after sign-out. The review correction reads throughuseTodoItemsand its held-delete selector, asserting that the row is hidden while held and visible again after the skipped delete releases it.apps/mobile/src/lib/todos/todo-model.test.tscovers an astral character straddling each field cap and a title whose complete astral character ends exactly at the cap.Verification at this head:
pnpm --filter kilo-app exec vitest run src/lib/hooks/use-todos.remove-with-undo.test.ts src/lib/todos/todo-model.test.ts— 2 files, 23 tests passed.Change
removeWithUndocaptures the initiating auth epoch. The request, failure toast, and query invalidation are restricted to that session while sign-out is inactive. A skipped delete releases its held row. Leaving the screen still commits for the same session; Undo remains available during the window.todoFromDraftcaps titles and notes on code-point boundaries while preserving the existing UTF-16 code-unit budgets of 500 and 10,000. Over-limit drafts continue to truncate silently.Production prerequisites
None.
Proxy answers
The owner's proxy gave these answers in the owner's place.