Harden SCSI health fallback (Greptile review)

- _scsi_health: treat any unrecognised status (e.g. "UNKNOWN", "N/A") as
  unreadable (None) instead of a false FAIL. Only "OK" -> PASS and "FAIL*"
  -> FAIL; anything else omits the drive rather than showing a red FAIL that
  contradicts the smart_limited disclaimer.
- get_all_smart_status: isolate the SCSI supplement loop in its own
  try/except so an error there cannot discard the pySMART results already
  collected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

controller/lib/smart.py

@@ -50,21 +50,25 @@ def get_all_smart_status() -> list[dict]:
         for device in DeviceList().devices:
             results.append(_get_device_smart(device))
             covered.add(f"/dev/{device.name}")
+    except Exception:
+        return []
 
-        # Drives behind USB enclosures that reject ATA pass-through (e.g.
-        # Seagate Expansion, WD easystore) are invisible to pySMART/DeviceList.
-        # smartctl --scan still lists them; add a coarse SCSI health entry so
-        # the drive is at least represented instead of silently unmonitored.
+    # Drives behind USB enclosures that reject ATA pass-through (e.g. Seagate
+    # Expansion, WD easystore) are invisible to pySMART/DeviceList.  smartctl
+    # --scan still lists them; add a coarse SCSI health entry so the drive is
+    # at least represented instead of silently unmonitored.  Isolated from the
+    # pySMART read above: a failure here must not discard those results.
+    try:
         for path, _interface in _scan_devices():
             if path in covered:
                 continue
             limited = _limited_device_smart(path)
             if limited is not None:
                 results.append(limited)
-
-        return results
     except Exception:
-        return []
+        pass
+
+    return results
 
 
 def _get_device_smart(device) -> dict:
@@ -229,10 +233,15 @@ def _scsi_health(path: str) -> Optional[str]:
     for line in result.stdout.splitlines():
         key, sep, val = line.partition(":")
         if sep and key.strip().lower() == "smart health status":
-            val = val.strip()
-            if not val:
-                return None
-            return "PASS" if val.upper() == "OK" else "FAIL"
+            val = val.strip().upper()
+            # Standard SCSI health is "OK" or "FAILED!".  Some bridges emit
+            # non-standard strings (e.g. "UNKNOWN", "N/A"); treat anything we
+            # do not recognise as unreadable (None) rather than a false FAIL.
+            if val == "OK":
+                return "PASS"
+            if val.startswith("FAIL"):
+                return "FAIL"
+            return None
     return None
 
 

controller/test/test_smart.py

@@ -133,6 +133,29 @@ class TestGetAllSmartStatus:
         devices = {d["device"] for d in result}
         assert devices == {"sdc", "sda"}
 
+    def test_supplement_failure_preserves_pysmart_results(self):
+        """An error in the SCSI supplement must not discard the drives already
+        collected from pySMART."""
+        ata = mock.Mock()
+        ata.name = "sdb"
+        ata.model = "Seagate"
+        ata.serial = "S1"
+        ata.capacity = "8.0 TB"
+        ata.assessment = "PASS"
+        ata.smart_enabled = True
+        ata.attributes = []
+
+        device_list = mock.Mock()
+        device_list.devices = [ata]
+
+        with mock.patch("controller.lib.smart.is_smart_available", return_value=True), \
+                mock.patch("controller.lib.smart.DeviceList", return_value=device_list), \
+                mock.patch("controller.lib.smart._scan_devices",
+                           side_effect=RuntimeError("scan blew up")):
+            result = get_all_smart_status()
+
+        assert [d["device"] for d in result] == ["sdb"]
+
 
 class TestGetDeviceSmart:
     """Tests for _get_device_smart function."""
@@ -466,6 +489,17 @@ class TestReadScsiLimited:
         # Only the health read happened; identity was skipped.
         run_fn.assert_called_once()
 
+    def test_unrecognised_status_is_treated_as_unreadable(self):
+        """A non-standard bridge status (e.g. UNKNOWN) must NOT show as a red
+        FAIL; treat it as unreadable so the drive is omitted, not falsely
+        failed."""
+        run = mock.Mock(stdout="SMART Health Status: UNKNOWN\n")
+        with mock.patch("controller.lib.smart.subprocess.run",
+                        return_value=run) as run_fn:
+            result = _read_scsi_limited("/dev/sda")
+        assert result == (None, None, None, None)
+        run_fn.assert_called_once()
+
     def test_smartctl_error_returns_all_none(self):
         with mock.patch("controller.lib.smart.subprocess.run",
                         side_effect=OSError("boom")):