fix(agent): ensure -d sat is passed for SCSI-identified SATA devices (#1791)

agent/smart.go

@@ -553,8 +553,16 @@ func (sm *SmartManager) smartctlArgs(deviceInfo *DeviceInfo, includeStandby bool
 	if deviceInfo != nil {
 		deviceType = strings.ToLower(deviceInfo.Type)
 		parserType = strings.ToLower(deviceInfo.parserType)
-		// types sometimes misidentified in scan; see github.com/henrygd/beszel/issues/1345
-		if deviceType != "" && deviceType != "scsi" && deviceType != "ata" {
+		// When the scan reported "scsi"/"ata" but we have already verified this device
+		// speaks SAT, pass "-d sat" explicitly. Without it, ATA-specific flags like
+		// "-l devstat" are added (see below) while the device is still addressed as
+		// SCSI, causing smartctl to return SCSI-format output that lacks a serial
+		// number and therefore makes the sat parser fail. See #1791.
+		if deviceInfo.typeVerified && (parserType == "sat" || parserType == "ata") &&
+			(deviceType == "scsi" || deviceType == "ata") {
+			args = append(args, "-d", "sat")
+		} else if deviceType != "" && deviceType != "scsi" && deviceType != "ata" {
+			// types sometimes misidentified in scan; see github.com/henrygd/beszel/issues/1345
 			args = append(args, "-d", deviceInfo.Type)
 		}
 	}

agent/smart_test.go

@@ -320,6 +320,51 @@ func TestSmartctlArgs(t *testing.T) {
 	)
 }
 
+// TestSmartctlArgsScsiVerifiedAsSat covers the case where smartctl --scan
+// reports a SATA drive as type "scsi" (SCSI translation layer), but a previous
+// successful parse has confirmed it speaks SAT (typeVerified=true,
+// parserType="sat"). Without "-d sat", subsequent calls would add "-l devstat"
+// (ATA-specific) while addressing the device as SCSI, causing the sat parser to
+// fail because smartctl returns SCSI-format output without a serial number.
+// See github.com/henrygd/beszel/issues/1791.
+func TestSmartctlArgsScsiVerifiedAsSat(t *testing.T) {
+	sm := &SmartManager{}
+
+	// Simulate a device whose scan type is "scsi" but has been verified as sat.
+	scsiVerifiedDevice := &DeviceInfo{
+		Name:         "/dev/sda",
+		Type:         "scsi",
+		parserType:   "sat",
+		typeVerified: true,
+	}
+
+	// With existing data (hasExistingData=true), should use -d sat and -l devstat,
+	// not rely on the "scsi" scan type.
+	assert.Equal(t,
+		[]string{"-d", "sat", "-a", "--json=c", "-l", "devstat", "-n", "standby", "/dev/sda"},
+		sm.smartctlArgs(scsiVerifiedDevice, true),
+	)
+
+	// Without standby flag, same -d sat and -l devstat.
+	assert.Equal(t,
+		[]string{"-d", "sat", "-a", "--json=c", "-l", "devstat", "/dev/sda"},
+		sm.smartctlArgs(scsiVerifiedDevice, false),
+	)
+
+	// An unverified scsi device should still omit -d (existing behavior: scan can
+	// misidentify; see #1345).
+	scsiUnverifiedDevice := &DeviceInfo{
+		Name:         "/dev/sdb",
+		Type:         "scsi",
+		parserType:   "sat",
+		typeVerified: false,
+	}
+	assert.Equal(t,
+		[]string{"-a", "--json=c", "-l", "devstat", "/dev/sdb"},
+		sm.smartctlArgs(scsiUnverifiedDevice, false),
+	)
+}
+
 func TestResolveRefreshError(t *testing.T) {
 	scanErr := errors.New("scan failed")
 	collectErr := errors.New("collect failed")