feat(mobile): add per-session auto-approve toggle to the session context sheet - #6105
Conversation
|
kilo-review — independent audit of the published diff. Status: 2 Issues
|
|
bot: Accepted. Pushed b460347 (kwf What changed:
|
e0793d2 to
b460347
Compare
b460347 to
90f4e18
Compare
90f4e18 to
2d8b2ce
Compare
|
bot: Cross-platform fix. Pushed 90f4e18 (kwf What changed:
|
|
bot: Cross-platform fix. Pushed 2d8b2ce (kwf What changed:
|
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryIncremental re-review of the six files changed since the prior review: Files Reviewed (6 files)
Previous Review Summary (commit 2d8b2ce)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 2d8b2ce)Status: 3 Issues Found | Recommendation: Address before merge Executive SummaryThe new per-session auto-approve store never releases state (it is not wired into the sign-out reset) and accumulates request ids without bound, while the PR still hand-edits all 86 non-English locale catalogs against the mobile Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (17 source/test files + 87 locale catalogs)
Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
2d8b2ce to
25a3c55
Compare
25a3c55 to
4b04fe5
Compare
Changelog for users
Changelog for maintainers
autoApproveblocks fails the required catalog-parity check:tools/i18n/check-catalogs.mjs:485-489reports each translated catalog that misses anen.jsonkey, and CI runspnpm --filter kilo-app run check:i18nat.github/workflows/kilo-app-ci.yml:126. The 86 blocks stay.clearSessionAutoApprove()to drop every session's toggle and wired it intoclearSessionScopedState(), so sign-out and account switch clear the module-global map instead of leaking it.rememberRequestId(), which evicts the oldest id once a set passes a 64-id window, bounding memory for a long session.E2E proof
Superseded and replaced: the earlier no-card "tool runs" captures [p4] and the pre-repair switch-state and second-session captures are replaced by the fresh Android runs [e1], [e2], [e3], and [e4] below. The account-boundary run and the non-English catalog captures (German and Zulu) are new. The reply-failure captures [p8] and [p6] of 2026-09-12 ~16:14-16:25 UTC (head 4672beaf) are retained, and the earlier gallery images stay below. Verification ran on Android; the toggle calls one haptic API shared by iOS and Android, and the emulator reports haptics only through VibratorService, so that record is the haptic source.
[p8] With Auto-approve on, make the permission reply fail once: the permission card appears with Retry while the toggle stays on. — Android emulator-5554, 2026-09-12 ~16:14-16:25 UTC, head 4672beaf5e055f42ebce2634fc5816c8b594806d. The ingest socket was taken down while a remote CLI session held a pending "Allow External Directory? /tmp/*" ask; the auto-approve reply failed, the card returned with a tappable Retry, and the toggle stayed on.
p8-fault.mp4.trim.mp4
[p6] With Auto-approve on, make the permission reply fail once: the permission card appears with Retry while the toggle stays on. — Android emulator-5554. A real pending permission failed because cloudflare-session-ingest was down, so the card returned with Retry and the toggle stayed on.
p6.mp4.trim.mp4
[e5] Android: change the Auto-approve switch and confirm the selection haptic still fires once per commit. — Android emulator-5604: three switch commits produced exactly three VibratorService records, one per commit; the emulator emits no Vibrator/HapticFeedback logcat tag, so the VibratorService record (dumpsys vibrator_manager) is the authoritative source.
e1.mp4
e4-autoapprove-pending.trim.mp4
p6.mp4
Owner manual verification
Owner verification is pending; the excluded checks did not pass automatically.
Owner request