Skip to content

fix(mobile): confirm host revoke and keep the model on refetch failure - #6758

Merged
iscekic merged 2 commits into
mainfrom
kwf/janitor-mobile-settings-d4123721a9
Sep 28, 2026
Merged

iscekic merged 2 commits into
mainfrom
kwf/janitor-mobile-settings-d4123721a9

Conversation

@iscekic

@iscekic iscekic commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

Fix proof

A failed background catalogue refetch (remount after staleTime, or reconnect, both default-enabled) sets catalogueError while the models cache is still populated, so the tool-summary translation Model

Asserted value: apps/mobile/src/components/tool-summary-translation-settings-screen.tsx. Sense check (jev): probability 0.95

The scripts were proven on an earlier base, so only the head ran.

Head d81fb29c4612

Head log: backend-assert 9879eb864dd0 exited 0
$ git diff --unified=0 d72f1f366dabae2752f480d1c65dff586f12d90f d81fb29c46120b6286b25eb9372892f4f33a54b6 -- apps/mobile/src/components/tool-summary-translation-settings-screen.tsx
diff --git a/apps/mobile/src/components/tool-summary-translation-settings-screen.tsx b/apps/mobile/src/components/tool-summary-translation-settings-screen.tsx
--- a/apps/mobile/src/components/tool-summary-translation-settings-screen.tsx
+++ b/apps/mobile/src/components/tool-summary-translation-settings-screen.tsx
+  //
+  // Only a catalogue with nothing to fall back on is an error: a failed
+  // background refetch (remount after `staleTime`, or reconnect) still holds
+  // the populated cache, so the stored model and the loaded catalogue keep
+  // working. Blanking the row to `—` and disabling the picker there would drop
+  // a working selection, which the voice-input sibling never does (it treats a
+  // failed refetch as `error` only while `models` is empty).
+  const catalogueError = (isError || (isLoading && isFetched)) && models.length === 0;

Revoking a trusted host fires on a single tap of the row's X with no confirmation, while the app's other removal rows (passkeys, device sessions) confirm through a destructive dialog first; the mobile

Asserted value: apps/mobile/src/components/trusted-hosts-screen.tsx. Sense check (jev): probability 0.95

The scripts were proven on an earlier base, so only the head ran.

Head d81fb29c4612

Head log: backend-assert 7477c9a0f53d exited 0
$ git diff --unified=0 d72f1f366dabae2752f480d1c65dff586f12d90f d81fb29c46120b6286b25eb9372892f4f33a54b6 -- apps/mobile/src/components/trusted-hosts-screen.tsx
diff --git a/apps/mobile/src/components/trusted-hosts-screen.tsx b/apps/mobile/src/components/trusted-hosts-screen.tsx
--- a/apps/mobile/src/components/trusted-hosts-screen.tsx
+++ b/apps/mobile/src/components/trusted-hosts-screen.tsx
+        },
+      },
+    ]);
+  };
+
@@ -70 +86 @@ export function TrustedHostsScreen() {
-                      revokeHost(host);
+                      confirmRevoke(host);

Changelog for users

  • Revoking a trusted host asks for confirmation before the host is removed.
  • The translation Model row keeps its stored model and stays selectable when a background catalogue refresh fails.

Changelog for maintainers

  • The catalogue error now requires an empty models cache, so a failed background refetch keeps the loaded catalogue; check the loading branch first.
  • Revocation goes through a destructive confirmation dialog, matching the passkey and device-session removal rows.
  • Only the English locale gained the revoke message, so other locales fall back until translated.

E2E proof

A failed background catalogue refetch (remount after staleTime, or reconnect, both default-enabled) sets catalogueError while the models cache is still populated, so the tool-summary translation Model row is blanked to '—' and the picker disabled even though the stored model and loaded catalogue still work; the voice-input sibling keeps its model in the same state.

Code trace: apps/mobile/src/components/tool-summary-translation-settings-screen.tsx:48 changed in 7004b4b54360db99d523a520e06f45d3b5c06bce. Sense check (jev): probability 0.95

Changed lines
-  const catalogueError = isError || (isLoading && isFetched);
+  //
+  // Only a catalogue with nothing to fall back on is an error: a failed
+  // background refetch (remount after `staleTime`, or reconnect) still holds
+  // the populated cache, so the stored model and the loaded catalogue keep
+  // working. Blanking the row to `—` and disabling the picker there would drop
+  // a working selection, which the voice-input sibling never does (it treats a
+  // failed refetch as `error` only while `models` is empty).
+  const catalogueError = (isError || (isLoading && isFetched)) && models.length === 0;

Revoking a trusted host fires on a single tap of the row's X with no confirmation, while the app's other removal rows (passkeys, device sessions) confirm through a destructive dialog first; the mobile AGENTS rule requires destructive actions to be confirmed.

Code trace: apps/mobile/src/components/trusted-hosts-screen.tsx:3 changed in 7004b4b54360db99d523a520e06f45d3b5c06bce. Sense check (jev): probability 0.94

Changed lines
-import { Pressable, View } from 'react-native';
+import { Alert, Pressable, View } from 'react-native';
+  // Revoking is destructive and one tap away on a single row, so it confirms
+  // first, the way the passkey and device-session removal rows do
+  // (apps/mobile/AGENTS.md).
+  const confirmRevoke = (host: string) => {
+    Alert.alert(t('trustedHosts.revoke', { host }), t('trustedHosts.revokeMessage'), [
+      { text: t('common.cancel'), style: 'cancel' },
+      {
+        text: t('organization.members.revokeConfirm'),
+        style: 'destructive',
+        onPress: () => {
+          revokeHost(host);
+        },
+      },
+    ]);
+  };
+
-                      revokeHost(host);
+                      confirmRevoke(host);
Owner request

Fix 2 janitor findings in mobile/settings. Fix every one; the proof covers each.

  1. A failed background catalogue refetch (remount after staleTime, or reconnect, both default-enabled) sets catalogueError while the models cache is still populated, so the tool-summary translation Model row is blanked to '—' and the picker disabled even though the stored model and loaded catalogue still work; the voice-input sibling keeps its model in the same state.
    Trace: apps/mobile/src/components/tool-summary-translation-settings-screen.tsx:52: A failed background catalogue refetch (remount after staleTime, or reconnect, both default-enabled) sets catalogueError while the models cache is still populated, so the tool-summary translation Model row is blanked to '—' and the picker disabled even though the stored model and loaded catalogue still work; the voice-input sibling keeps its model in the same state. (janitor area freestyle).
    Files: apps/mobile/src/components/tool-summary-translation-settings-screen.tsx.
  2. Revoking a trusted host fires on a single tap of the row's X with no confirmation, while the app's other removal rows (passkeys, device sessions) confirm through a destructive dialog first; the mobile AGENTS rule requires destructive actions to be confirmed.
    Trace: apps/mobile/src/components/trusted-hosts-screen.tsx:70: Revoking a trusted host fires on a single tap of the row's X with no confirmation, while the app's other removal rows (passkeys, device sessions) confirm through a destructive dialog first; the mobile AGENTS rule requires destructive actions to be confirmed. (janitor area freestyle).
    Files: apps/mobile/src/components/trusted-hosts-screen.tsx.

@iscekic iscekic added the kwf-janitor Admitted to the workflow from a janitor finding label Sep 26, 2026
Comment thread apps/mobile/src/i18n/locales/en.json
@kilo-code-bot

kilo-code-bot Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (86 files)
  • apps/mobile/src/i18n/locales/*.json - all 86 non-English catalogs gained the trustedHosts.revokeMessage translation, each inserted directly after trustedHosts.revoke with matching key order and no placeholders; 87/87 catalogs now contain the key, resolving the previously reported check-catalogs parity failure
Previous Review Summary (commit 80fdc48)

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

Previous review (commit 80fdc48)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

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

CRITICAL

File Line Issue
apps/mobile/src/i18n/locales/en.json 2727 New key trustedHosts.revokeMessage was added only to en.json; tools/i18n/check-catalogs.mjs requires parity across all 87 catalogs, so pnpm check:i18n fails with 86 missing key errors (en.json has 3166 key lines, every other catalog 3165).
Files Reviewed (3 files)
  • apps/mobile/src/components/tool-summary-translation-settings-screen.tsx
  • apps/mobile/src/components/trusted-hosts-screen.tsx
  • apps/mobile/src/i18n/locales/en.json - 1 issue

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

@iscekic iscekic self-assigned this Sep 26, 2026
@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 26, 2026
@iscekic iscekic added merge-by-human the merge bot routed this PR to a human merge-by-bot and removed human-ready The PR is ready for human review. merge-by-human the merge bot routed this PR to a human labels Sep 26, 2026
@iscekic
iscekic marked this pull request as draft September 28, 2026 08:40
@iscekic
iscekic marked this pull request as ready for review September 28, 2026 08:44
@iscekic
iscekic marked this pull request as draft September 28, 2026 08:52
@iscekic
iscekic marked this pull request as ready for review September 28, 2026 09:01
@iscekic
iscekic marked this pull request as draft September 28, 2026 09:05
@iscekic
iscekic marked this pull request as ready for review September 28, 2026 09:10
@iscekic
iscekic marked this pull request as draft September 28, 2026 09:21
@iscekic
iscekic marked this pull request as ready for review September 28, 2026 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kwf-janitor Admitted to the workflow from a janitor finding merge-by-bot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants