Code quality improvements: robust error handling, HA best practices, stale translation cleanup

.github/workflows/hassfest.yaml

@@ -2,13 +2,17 @@ name: Validate with hassfest
 
 on:
   push:
+    branches:
+      - main
   pull_request:
   schedule:
     - cron: "0 0 * * *"
 
 jobs:
   validate:
     runs-on: "ubuntu-latest"
+    permissions:
+      contents: read
     steps:
       - name: "Check out repository"
         uses: actions/checkout@v4

.github/workflows/validate.yaml

@@ -2,6 +2,8 @@ name: Validate
 
 on:
   push:
+    branches:
+      - main
   pull_request:
   schedule:
     - cron: "0 0 * * *"

config_flow.py

@@ -2,26 +2,23 @@
 
 from __future__ import annotations
 
-import logging
 from typing import Any
 
 from homeassistant.config_entries import ConfigFlow, ConfigFlowResult
 from .const import DOMAIN
 
-_LOGGER = logging.getLogger(__name__)
-
 
 class SshCommandConfigFlow(ConfigFlow, domain=DOMAIN):
     """Handle a config flow for SSH Command."""
 
     VERSION = 1
+    single_instance_allowed = True
 
     async def async_step_user(
             self, _user_input: dict[str, Any] | None = None
     ) -> ConfigFlowResult:
         """Handle the initial step."""
-        # Check if already configured
-        if self._async_current_entries() or self.hass.data.get(DOMAIN):  # pylint: disable=no-member
+        if self._async_current_entries():
             return self.async_abort(reason="single_instance_allowed")
 
         return self.async_create_entry(title="SSH Command", data={})

coordinator.py

@@ -11,6 +11,7 @@
 from __future__ import annotations
 
 import logging
+import socket
 from pathlib import Path
 from typing import Any
 
@@ -88,30 +89,34 @@ async def async_execute(self, data: dict[str, Any]) -> dict[str, Any]:
             async with connect(**conn_kwargs) as conn:
                 result = await conn.run(**run_kwargs)
         except HostKeyNotVerifiable as exc:
+            _LOGGER.warning("Host key not verifiable for %s: %s", host, exc)
             raise ServiceValidationError(
                 "The host key could not be verified.",
                 translation_domain=DOMAIN,
                 translation_key="host_key_not_verifiable",
             ) from exc
         except PermissionDenied as exc:
+            _LOGGER.warning("SSH login failed for %s@%s: %s", username, host, exc)
             raise ServiceValidationError(
                 "SSH login failed.",
                 translation_domain=DOMAIN,
                 translation_key="login_failed",
             ) from exc
         except TimeoutError as exc:
+            _LOGGER.warning("SSH connection to %s timed out: %s", host, exc)
             raise ServiceValidationError(
                 "Connection timed out.",
                 translation_domain=DOMAIN,
                 translation_key="connection_timed_out",
             ) from exc
-        except OSError as e:
-            if e.strerror == 'Temporary failure in name resolution':
+        except OSError as exc:
+            if isinstance(exc, socket.gaierror):
+                _LOGGER.warning("Host %s is not reachable: %s", host, exc)
                 raise ServiceValidationError(
                     "Host is not reachable.",
                     translation_domain=DOMAIN,
                     translation_key="host_not_reachable",
-                ) from e
+                ) from exc
             raise
 
         return {
@@ -125,7 +130,7 @@ async def _resolve_known_hosts(self, check_known_hosts: bool, known_hosts: str |
         if not check_known_hosts:
             return None
         if not known_hosts:
-            known_hosts = str(Path('~', '.ssh', CONF_KNOWN_HOSTS).expanduser())
+            known_hosts = str(Path("~", ".ssh", "known_hosts").expanduser())
         if await exists(known_hosts):
             return await self.hass.async_add_executor_job(read_known_hosts, known_hosts)
         return known_hosts

strings.json

@@ -18,6 +18,10 @@
                 "name": "Username",
                 "description": "Username for SSH authentication."
             },
+            "password": {
+                "name": "Password",
+                "description": "Password for SSH authentication (optional if using key-based authentication)."
+            },
             "key_file": {
                 "name": "Key File",
                 "description": "Path to the SSH private key file for key-based authentication."
@@ -26,9 +30,9 @@
                 "name": "Command",
                 "description": "The command to execute on the machine."
             },
-            "script_file": {
-                "name": "Script File",
-                "description": "Path to the script file to execute on the machine."
+            "input": {
+                "name": "Input",
+                "description": "Input to send to the standard input of the remote process."
             },
             "check_known_hosts": {
                 "name": "Check Known Hosts",
@@ -50,17 +54,17 @@
       "message": "Either password or key file must be provided."
     },
     "command_or_input": {
-      "message": "Either command or script file must be provided."
+      "message": "Either command or input must be provided."
     },
     "key_file_not_found": {
       "message": "Could not find key file."
     },
-    "script_file_not_found": {
-      "message": "Could not find script file."
-    },
     "known_hosts_with_check_disabled": {
       "message": "Known hosts provided while check known hosts is disabled."
     },
+    "integration_not_set_up": {
+      "message": "SSH Command integration is not set up."
+    },
     "host_key_not_verifiable": {
       "message": "The host could not be verified."
     },

test/homeassistant_mock/homeassistant/config_entries.py

@@ -6,6 +6,8 @@ class ConfigEntry:
 
 
 class ConfigFlow:
+    single_instance_allowed = False
+
     def __init_subclass__(cls, domain=None, **kwargs):
         super().__init_subclass__(**kwargs)
         if domain is not None:

test/test_async_execute.py

@@ -1,4 +1,5 @@
 import os
+import socket
 import sys
 import tempfile
 import unittest
@@ -122,8 +123,7 @@ async def test_timeout(self):
         self.assertEqual(ctx.exception.translation_key, "connection_timed_out")
 
     async def test_name_resolution_failure(self):
-        err = OSError()
-        err.strerror = "Temporary failure in name resolution"
+        err = socket.gaierror("Name or service not known")
         service_call = self._make_service_call(SERVICE_DATA_BASE)
 
         with patch("ssh_command.coordinator.connect", return_value=_MockConnectRaises(err)):
@@ -135,7 +135,6 @@ async def test_name_resolution_failure(self):
 
     async def test_other_oserror_is_reraised(self):
         err = OSError("something else")
-        err.strerror = "something else"
         service_call = self._make_service_call(SERVICE_DATA_BASE)
 
         with patch("ssh_command.coordinator.connect", return_value=_MockConnectRaises(err)):

test/test_config_flow.py

@@ -42,11 +42,5 @@ async def test_aborts_when_entry_already_exists(self):
         self.assertEqual(result["type"], "abort")
         self.assertEqual(result["reason"], "single_instance_allowed")
 
-    async def test_aborts_when_domain_in_hass_data(self):
-        flow = self._make_flow()
-        flow.hass.data[DOMAIN] = object()
-
-        result = await flow.async_step_user()
-
-        self.assertEqual(result["type"], "abort")
-        self.assertEqual(result["reason"], "single_instance_allowed")
+    def test_single_instance_allowed_is_set(self):
+        self.assertTrue(SshCommandConfigFlow.single_instance_allowed)

test/test_coordinator.py

@@ -1,3 +1,4 @@
+import socket
 import sys
 import unittest
 from pathlib import Path
@@ -103,8 +104,7 @@ async def test_async_execute_timeout(self):
         self.assertEqual(ctx.exception.translation_key, "connection_timed_out")
 
     async def test_async_execute_name_resolution_failure(self):
-        err = OSError()
-        err.strerror = "Temporary failure in name resolution"
+        err = socket.gaierror("Name or service not known")
 
         with patch("ssh_command.coordinator.connect", return_value=_MockConnectRaises(err)):
             with patch("ssh_command.coordinator.exists", return_value=False):
@@ -115,7 +115,6 @@ async def test_async_execute_name_resolution_failure(self):
 
     async def test_async_execute_other_oserror_reraised(self):
         err = OSError("something else")
-        err.strerror = "something else"
 
         with patch("ssh_command.coordinator.connect", return_value=_MockConnectRaises(err)):
             with patch("ssh_command.coordinator.exists", return_value=False):

translations/de.json

@@ -50,14 +50,17 @@
       "message": "Entweder Passwort oder Schlüsseldatei muss angegeben werden."
     },
     "command_or_input": {
-      "message": "Entweder Befehl oder Skriptdatei muss angegeben werden."
+      "message": "Entweder Befehl oder Eingabe muss angegeben werden."
     },
     "key_file_not_found": {
       "message": "Konnte Schlüsseldatei nicht finden."
     },
     "known_hosts_with_check_disabled": {
       "message": "Bekannte Hosts wurden angegeben, obwohl die Überprüfung deaktiviert ist."
     },
+    "integration_not_set_up": {
+      "message": "Die SSH-Command-Integration ist nicht eingerichtet."
+    },
     "host_key_not_verifiable": {
       "message": "Der Host konnte nicht verifiziert werden."
     },

translations/en.json

@@ -50,14 +50,17 @@
       "message": "Either password or key file must be provided."
     },
     "command_or_input": {
-      "message": "Either command or script file must be provided."
+      "message": "Either command or input must be provided."
     },
     "key_file_not_found": {
       "message": "Could not find key file."
     },
     "known_hosts_with_check_disabled": {
       "message": "Known hosts provided while check known hosts is disabled."
     },
+    "integration_not_set_up": {
+      "message": "SSH Command integration is not set up."
+    },
     "host_key_not_verifiable": {
       "message": "The host could not be verified."
     },