lib/view/page/server/edit/actions.dart (1)

186-191: Consider adding a hint about the Settings page for permission errors.

On macOS with sandbox restrictions, users seeing "permission denied" may not know they can manually import from Settings. A slightly more informative message could help.

💡 Optional enhancement

       if (e is PathAccessException ||
           e.toString().contains('Operation not permitted')) {
-        context.showSnackBar(l10n.sshConfigPermissionDenied);
+        // Consider: l10n.sshConfigPermissionDeniedHint that mentions Settings > SSH
+        context.showSnackBar(l10n.sshConfigPermissionDenied);
       } else {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/view/page/server/edit/actions.dart` around lines 186 - 191, The
permission-denied branch should surface a hint directing users to the Settings
import flow: when detecting PathAccessException or
e.toString().contains('Operation not permitted'), update the message passed to
context.showSnackBar (currently using l10n.sshConfigPermissionDenied) to include
guidance about importing SSH config via the Settings page (or call a different
localized string that appends that hint); modify the branch handling around
PathAccessException / permission check to use the new/extended localized message
so users on macOS know to open Settings → Import SSH config.

lib/core/utils/server_dedup.dart (2)

85-93: Document the expected usage pattern for the resolvedServers-only path.

When only resolvedServers is passed and it's empty, source.length equals 0, making the allExistMessage('0') semantically odd ("0 servers already exist"). The current callers handle this correctly by checking emptiness before calling, but this assumption isn't documented.

Consider either:

1. Adding a doc comment clarifying that callers should pre-filter empty lists when passing only resolvedServers, or
2. Storing the original count separately if you want accurate "X already exist" messaging.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/core/utils/server_dedup.dart` around lines 85 - 93, The branch in
server_dedup.dart that computes source = servers ?? resolvedServers! and then
uses source.length for the allExistMessage produces misleading output when
callers pass only resolvedServers (because source.length will be 0 even if
resolvedServers was non-empty but resolved is empty); update the code or docs:
either add a doc comment on the function (or the parameter resolvedServers)
describing that callers must pre-filter and not pass an empty resolvedServers
list, or change the logic in the method (around source, resolved, and the
resolved.isEmpty check) to capture the original input count (e.g., store
originalCount = (servers != null ? servers.length : resolvedServers!.length))
and use that count in context.showSnackBar(allExistMessage(...)) so the message
reflects the original number; reference symbols: servers, resolvedServers,
source, resolved, _resolveServers, context.showSnackBar, allExistMessage.

---

96-98: Consider batch import to reduce persistence overhead.

Each addServer call (lines 96-98) performs individual persistence writes via Stores.server.put(spi) and Stores.setting.serverOrder.put(newOrder), resulting in 2N writes for N servers. For larger SSH config imports, this creates unnecessary I/O overhead. No batch add method currently exists in the codebase, but implementing one would consolidate these writes into a single operation.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/core/utils/server_dedup.dart` around lines 96 - 98, The loop calling
ref.read(serversProvider.notifier).addServer(server) causes 2N persistence
writes (Stores.server.put and Stores.setting.serverOrder.put); replace with a
new batch API on the servers notifier (e.g., addServers(List<Server>) or
importServers(List<Server>)) that accepts the resolved list, performs in-memory
state updates for all servers and computes/updates server order once, then does
a single Stores.server.put (or bulk put) and a single
Stores.setting.serverOrder.put; change the loop to a single call to this new
batch method and ensure existing dedup/update logic from addServer is preserved
inside the batch implementation.

lib/view/page/setting/entries/ssh.dart (1)

130-136: ⚠️ Potential issue | 🟡 Minor

Localize the discovery success copy as a single message.

Line 135 builds the snackbar text from raw fragments, which hard-codes English word order and skips pluralization. Please reuse one parameterized l10n entry here, or add a dedicated discovery-import message if none already fits.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/view/page/setting/entries/ssh.dart` around lines 130 - 136, The snackbar
success text passed into ServerDeduplication.importServersWithNotification uses
concatenated fragments (importedMessage: (count) => '${libL10n.success}: $count
${libL10n.servers}') which hardcodes word order and ignores pluralization;
update the call to pass a single localized, parameterized string from your l10n
(or add a new entry like discoveryImportSuccess(count)) and use that key for
importedMessage, keeping the same arguments (servers, ref, context,
allExistMessage) and preserving pluralization rules in the localization file.

lib/view/page/server/edit/actions.dart (1)

141-194: ⚠️ Potential issue | 🟡 Minor

Clear firstTimeReadSSHCfg on every exit path.

Line 145, Line 169, and the permission-denied branch all exit before the flag is updated, so this “first-time” check can keep running on later visits to the edit page. Moving Stores.setting.firstTimeReadSSHCfg.put(false) to right after the first parse attempt finishes, or into a finally, avoids the repeat prompt/snackbar paths.

Suggested fix

   Future<void> _checkSSHConfigImport() async {
     try {
       final servers = await SSHConfig.parseConfig();
+      Stores.setting.firstTimeReadSSHCfg.put(false);
       if (!mounted) return;
       if (servers.isEmpty) {
         return;
       }
@@
     } catch (e) {
+      Stores.setting.firstTimeReadSSHCfg.put(false);
       if (!mounted) return;
       if (e is PathAccessException ||
           e.toString().contains('Operation not permitted')) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/view/page/server/edit/actions.dart` around lines 141 - 194, The
_checkSSHConfigImport method fails to clear the firstTimeReadSSHCfg flag on all
exit paths; move the Stores.setting.firstTimeReadSSHCfg.put(false) call into a
finally block (or execute it immediately after the SSHConfig.parseConfig() call)
so it always runs regardless of early returns or exceptions; update references
in this function (remove the existing scattered calls at the top/inside the try
and inside the catch) and ensure the finally runs after SSHConfig.parseConfig(),
preserving the existing behavior for showing dialogs/snackbars and the
ServerDeduplication.importServersWithNotification call.

lib/view/page/setting/entries/ssh.dart (1)

23-261: Split the new SSH settings flow into build/actions helpers.

This extension now owns tiles, scanner/discovery handlers, dialogs, file picking, parsing, and import orchestration. Pulling the _onTap*, _process*, and _handle* methods into companion actions/utils extensions would keep the settings entry easier to scan and aligned with the repo’s view structure. As per coding guidelines, "Split UI into build, actions, and utils; use extension on to separate concerns".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/view/page/setting/entries/ssh.dart` around lines 23 - 261, The settings
entry is doing too much—extract the action and util logic out of the widget file
by moving the tap handlers and processing helpers into companion
extensions/helpers: create an extension or separate helper class that implements
_onTapQrScan, _onTapSSHDiscovery, _processDiscoveredServers,
_onTapSSHConfigImport, _processSSHServers, _handleImportSSHCfgPermissionIssue,
and _onTapSSHImportWithFilePicker (keeping the same signatures) and have the
widget file only call those methods; ensure you inject or pass needed
context/ref dependencies into the new helpers (or accept BuildContext and
WidgetRef parameters) and update imports and references so the ListTile builders
(_buildSSHConfigImport, _buildQrScan, _buildSSHDiscovery) only contain UI code
and forward actions to the extracted helpers.