fix(agent): honor explicit SMART_DEVICES type hint instead of scan-detected type (#2102)

When SMART_DEVICES specifies an explicit type (e.g. /dev/sda:scsi), the
agent resolved the device type correctly but smartctlArgs dropped the -d
flag for scsi/ata (the #1345 scan-misdetection workaround), so smartctl
re-detected the wrong type (sat) and collection failed on USB drives
whose bridge does not support SAT passthrough.

Mark types that come from an explicit SMART_DEVICES hint and always pass
them through via -d, while still letting scan-detected scsi/ata
auto-detect as before. Adds regression tests for the arg building, the
full parse -> merge -> args path, and flag preservation across rescans.

Fixes #2072

agent/smart.go

@@ -55,6 +55,11 @@ type DeviceInfo struct {
 	typeVerified bool
 	// parserType holds the parser type (nvme, sat, scsi) that last succeeded.
 	parserType string
+	// explicitType reports whether Type came from an explicit ":type" hint in
+	// SMART_DEVICES. Such a type is a deliberate user override and must always be
+	// passed to smartctl via -d, even for scsi/ata where a scan-detected type is
+	// otherwise left off (see smartctlArgs and issue #1345).
+	explicitType bool
 }
 
 // deviceKey is a composite key for a device, used to identify a device uniquely.
@@ -251,8 +256,9 @@ func (sm *SmartManager) parseConfiguredDevices(config string) ([]*DeviceInfo, er
 		}
 
 		devices = append(devices, &DeviceInfo{
-			Name: name,
-			Type: devType,
+			Name:         name,
+			Type:         devType,
+			explicitType: devType != "",
 		})
 	}
 
@@ -558,7 +564,9 @@ func (sm *SmartManager) smartctlArgs(deviceInfo *DeviceInfo, includeStandby bool
 		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" {
+		// An explicit SMART_DEVICES ":type" hint is a deliberate override, so always
+		// pass it through; otherwise scsi/ata are left off so smartctl can auto-detect.
+		if deviceType != "" && (deviceInfo.explicitType || (deviceType != "scsi" && deviceType != "ata")) {
 			args = append(args, "-d", deviceInfo.Type)
 		}
 	}
@@ -663,6 +671,9 @@ func mergeDeviceLists(existing, scanned, configured []*DeviceInfo) []*DeviceInfo
 		target.Type = prev.Type
 		target.typeVerified = true
 		target.parserType = prev.parserType
+		if prev.explicitType {
+			target.explicitType = true
+		}
 	}
 
 	// applyConfiguredMetadata updates a matched device with any configured
@@ -676,6 +687,9 @@ func mergeDeviceLists(existing, scanned, configured []*DeviceInfo) []*DeviceInfo
 			existingDev.typeVerified = false
 			existingDev.parserType = normalizeParserType(newType)
 		}
+		if configuredDev.explicitType {
+			existingDev.explicitType = true
+		}
 		if configuredDev.InfoName != "" {
 			existingDev.InfoName = configuredDev.InfoName
 		}

agent/smart_test.go

@@ -392,6 +392,81 @@ func TestSmartctlArgs(t *testing.T) {
 	)
 }
 
+// TestSmartctlArgsExplicitType verifies that an explicit SMART_DEVICES type hint
+// is always passed to smartctl via -d, while a scan-detected scsi/ata type is
+// still left off so smartctl can auto-detect it (see issue #1345).
+func TestSmartctlArgsExplicitType(t *testing.T) {
+	sm := &SmartManager{}
+
+	// Scan-detected scsi: -d is intentionally omitted.
+	scanScsi := &DeviceInfo{Name: "/dev/sda", Type: "scsi"}
+	assert.Equal(t,
+		[]string{"-a", "--json=c", "/dev/sda"},
+		sm.smartctlArgs(scanScsi, false),
+	)
+
+	// Explicit scsi from SMART_DEVICES: -d scsi must be passed.
+	explicitScsi := &DeviceInfo{Name: "/dev/sda", Type: "scsi", explicitType: true}
+	assert.Equal(t,
+		[]string{"-d", "scsi", "-a", "--json=c", "/dev/sda"},
+		sm.smartctlArgs(explicitScsi, false),
+	)
+
+	// Explicit ata from SMART_DEVICES: -d ata must be passed (devstat still added).
+	explicitAta := &DeviceInfo{Name: "/dev/sdb", Type: "ata", explicitType: true}
+	assert.Equal(t,
+		[]string{"-d", "ata", "-a", "--json=c", "-l", "devstat", "/dev/sdb"},
+		sm.smartctlArgs(explicitAta, false),
+	)
+}
+
+// TestSmartDevicesExplicitTypeFlowsToSmartctlArgs is a regression test for
+// issue #2072: an explicit SMART_DEVICES type (e.g. /dev/sda:scsi) must win over
+// a wrong scan-detected type (sat) and be handed to smartctl as -d scsi.
+func TestSmartDevicesExplicitTypeFlowsToSmartctlArgs(t *testing.T) {
+	sm := &SmartManager{}
+
+	configured, err := sm.parseConfiguredDevices("/dev/sda:scsi")
+	require.NoError(t, err)
+	require.Len(t, configured, 1)
+	assert.True(t, configured[0].explicitType)
+
+	// smartctl --scan misreports this USB drive as sat, which fails on it.
+	scanned := []*DeviceInfo{
+		{Name: "/dev/sda", Type: "sat", Protocol: "ATA"},
+	}
+
+	merged := mergeDeviceLists(nil, scanned, configured)
+	require.Len(t, merged, 1)
+
+	device := merged[0]
+	assert.Equal(t, "scsi", device.Type, "configured type should win over scan-detected sat")
+	assert.True(t, device.explicitType, "explicit hint must survive the merge")
+
+	assert.Equal(t,
+		[]string{"-d", "scsi", "-a", "--json=c", "/dev/sda"},
+		sm.smartctlArgs(device, false),
+		"explicit scsi type must be passed to smartctl, not dropped",
+	)
+}
+
+// TestMergeDeviceListsPreservesExplicitTypeAcrossRescan ensures a verified,
+// explicitly-typed device keeps its explicit flag when a later scan re-reports
+// it with a different auto-detected type.
+func TestMergeDeviceListsPreservesExplicitTypeAcrossRescan(t *testing.T) {
+	existing := []*DeviceInfo{
+		{Name: "/dev/sda", Type: "scsi", parserType: "scsi", typeVerified: true, explicitType: true},
+	}
+	scanned := []*DeviceInfo{
+		{Name: "/dev/sda", Type: "sat"},
+	}
+
+	merged := mergeDeviceLists(existing, scanned, nil)
+	require.Len(t, merged, 1)
+	assert.Equal(t, "scsi", merged[0].Type)
+	assert.True(t, merged[0].explicitType, "explicit type flag should survive a rescan")
+}
+
 func TestResolveRefreshError(t *testing.T) {
 	scanErr := errors.New("scan failed")
 	collectErr := errors.New("collect failed")