fix: SMART_DEVICES with same path and different -d types only shows one device (#2053)

* fix: SMART_DEVICES with same path and different -d types only shows one device

* test: add unit test for parseSmartForSata with configuredType

* refactor: replace string manipulation with normalizeParserType for disk type handling

* fix: handle transient failures for USB bridge passthrough device types in CollectSmart

agent/smart.go

@@ -65,6 +65,14 @@ type deviceKey struct {
 
 var errNoValidSmartData = fmt.Errorf("no valid SMART data found") // Error for missing data
 
+// isBridgePassthroughType reports whether the device type uses a USB bridge
+// passthrough driver that may fail transiently (exit 2) due to wakeup data
+// or other bridge-specific quirks. A single retry is attempted in CollectSmart.
+func isBridgePassthroughType(deviceType string) bool {
+	dt := strings.ToLower(deviceType)
+	return strings.HasPrefix(dt, "jms56x") || strings.HasPrefix(dt, "jmb39x")
+}
+
 // Refresh updates SMART data for all known devices
 func (sm *SmartManager) Refresh(forceScan bool) error {
 	sm.refreshMutex.Lock()
@@ -368,9 +376,15 @@ func (sm *SmartManager) parseSmartOutput(deviceInfo *DeviceInfo, output []byte)
 		Type  string
 		Parse func([]byte) (bool, int)
 	}{
-		{Type: "nvme", Parse: sm.parseSmartForNvme},
-		{Type: "sat", Parse: sm.parseSmartForSata},
-		{Type: "scsi", Parse: sm.parseSmartForScsi},
+		{Type: "nvme", Parse: func(output []byte) (bool, int) {
+			return sm.parseSmartForNvme(output, deviceInfo.Type)
+		}},
+		{Type: "sat", Parse: func(output []byte) (bool, int) {
+			return sm.parseSmartForSata(output, deviceInfo.Type)
+		}},
+		{Type: "scsi", Parse: func(output []byte) (bool, int) {
+			return sm.parseSmartForScsi(output, deviceInfo.Type)
+		}},
 	}
 
 	deviceType := normalizeParserType(deviceInfo.parserType)
@@ -495,15 +509,22 @@ func (sm *SmartManager) CollectSmart(deviceInfo *DeviceInfo) error {
 	// Check if device is in standby (exit status 2)
 	if exitErr, ok := errors.AsType[*exec.ExitError](err); ok && exitErr.ExitCode() == 2 {
 		if hasExistingData {
-			// Device is in standby and we have cached data, keep using cache
 			return nil
 		}
-		// No cached data, need to collect initial data by bypassing standby
-		ctx2, cancel2 := context.WithTimeout(context.Background(), 15*time.Second)
-		defer cancel2()
+		// No cached data, retry without -n standby
 		args = sm.smartctlArgs(deviceInfo, false)
-		cmd = exec.CommandContext(ctx2, sm.smartctlPath, args...)
+		cmd = exec.CommandContext(ctx, sm.smartctlPath, args...)
 		output, err = cmd.CombinedOutput()
+
+		// Bridge passthrough drivers (jms56x, jmb39x) can fail siently due
+		// to wakeup data left by the bridge firmware. A second retry succeeds
+		// after the first attempt has cleared the bridge state.
+		if deviceInfo != nil && isBridgePassthroughType(deviceInfo.Type) {
+			if exitErr2, ok2 := errors.AsType[*exec.ExitError](err); ok2 && exitErr2.ExitCode() == 2 {
+				cmd = exec.CommandContext(ctx, sm.smartctlPath, args...)
+				output, err = cmd.CombinedOutput()
+			}
+		}
 	}
 
 	hasValidData := sm.parseSmartOutput(deviceInfo, output)
@@ -733,6 +754,7 @@ func mergeDeviceLists(existing, scanned, configured []*DeviceInfo) []*DeviceInfo
 		}
 		if existingDev := deviceIndexByName[configuredDevice.Name]; existingDev != nil {
 			applyConfiguredMetadata(existingDev, configuredDevice)
+			delete(deviceIndexByName, configuredDevice.Name)
 			continue
 		}
 
@@ -836,9 +858,12 @@ func (sm *SmartManager) isVirtualDeviceFromStrings(fields ...string) bool {
 	return false
 }
 
-// parseSmartForSata parses the output of smartctl --all -j for SATA/ATA devices and updates the SmartDataMap
-// Returns hasValidData and exitStatus
-func (sm *SmartManager) parseSmartForSata(output []byte) (bool, int) {
+// parseSmartForSata parses the output of smartctl --all -j for SATA/ATA devices and updates the SmartDataMap.
+// configuredType is the user-specified device type (e.g., "sat", "jms56x,0") from SMART_DEVICES.
+// When non-empty, it overrides the disk type reported by smartctl in SmartData.DiskType so that
+// updateSmartDevices can match entries by the configured composite (name, type) key.
+// Returns hasValidData and exitStatus.
+func (sm *SmartManager) parseSmartForSata(output []byte, configuredType string) (bool, int) {
 	var data smart.SmartInfoForSata
 
 	if err := json.Unmarshal(output, &data); err != nil {
@@ -877,6 +902,9 @@ func (sm *SmartManager) parseSmartForSata(output []byte) (bool, int) {
 	smartData.SmartStatus = getSmartStatus(smartData.Temperature, data.SmartStatus.Passed)
 	smartData.DiskName = data.Device.Name
 	smartData.DiskType = data.Device.Type
+	if configuredType != "" {
+		smartData.DiskType = normalizeParserType(configuredType)
+	}
 
 	// get values from ata_device_statistics if necessary
 	var ataDeviceStats smart.AtaDeviceStatistics
@@ -950,7 +978,7 @@ func findAtaDeviceStatisticsValue(data *smart.SmartInfoForSata, ataDeviceStats *
 	return nil
 }
 
-func (sm *SmartManager) parseSmartForScsi(output []byte) (bool, int) {
+func (sm *SmartManager) parseSmartForScsi(output []byte, configuredType string) (bool, int) {
 	var data smart.SmartInfoForScsi
 
 	if err := json.Unmarshal(output, &data); err != nil {
@@ -985,6 +1013,9 @@ func (sm *SmartManager) parseSmartForScsi(output []byte) (bool, int) {
 	smartData.SmartStatus = getSmartStatus(smartData.Temperature, data.SmartStatus.Passed)
 	smartData.DiskName = data.Device.Name
 	smartData.DiskType = data.Device.Type
+	if configuredType != "" {
+		smartData.DiskType = normalizeParserType(configuredType)
+	}
 
 	attributes := make([]*smart.SmartAttribute, 0, 10)
 	attributes = append(attributes, &smart.SmartAttribute{Name: "PowerOnHours", RawValue: data.PowerOnTime.Hours})
@@ -1082,9 +1113,12 @@ func (sm *SmartManager) lookupDarwinNvmeCapacity(serial string) uint64 {
 	return sm.darwinNvmeCapacity[serial]
 }
 
-// parseSmartForNvme parses the output of smartctl --all -j /dev/nvmeX and updates the SmartDataMap
-// Returns hasValidData and exitStatus
-func (sm *SmartManager) parseSmartForNvme(output []byte) (bool, int) {
+// parseSmartForNvme parses the output of smartctl --all -j /dev/nvmeX and updates the SmartDataMap.
+// configuredType is the user-specified device type (e.g., "nvme") from SMART_DEVICES.
+// When non-empty, it overrides the disk type reported by smartctl in SmartData.DiskType so that
+// updateSmartDevices can match entries by the configured composite (name, type) key.
+// Returns hasValidData and exitStatus.
+func (sm *SmartManager) parseSmartForNvme(output []byte, configuredType string) (bool, int) {
 	data := &smart.SmartInfoForNvme{}
 
 	if err := json.Unmarshal(output, &data); err != nil {
@@ -1128,6 +1162,9 @@ func (sm *SmartManager) parseSmartForNvme(output []byte) (bool, int) {
 	smartData.SmartStatus = getSmartStatus(smartData.Temperature, data.SmartStatus.Passed)
 	smartData.DiskName = data.Device.Name
 	smartData.DiskType = data.Device.Type
+	if configuredType != "" {
+		smartData.DiskType = normalizeParserType(configuredType)
+	}
 
 	// nvme attributes does not follow the same format as ata attributes,
 	// so we manually map each field to SmartAttributes

agent/smart_test.go

@@ -24,7 +24,7 @@ func TestParseSmartForScsi(t *testing.T) {
 		SmartDataMap: make(map[string]*smart.SmartData),
 	}
 
-	hasData, exitStatus := sm.parseSmartForScsi(data)
+	hasData, exitStatus := sm.parseSmartForScsi(data, "")
 	if !hasData {
 		t.Fatalf("expected SCSI data to parse successfully")
 	}
@@ -69,7 +69,7 @@ func TestParseSmartForSata(t *testing.T) {
 		SmartDataMap: make(map[string]*smart.SmartData),
 	}
 
-	hasData, exitStatus := sm.parseSmartForSata(data)
+	hasData, exitStatus := sm.parseSmartForSata(data, "")
 	require.True(t, hasData)
 	assert.Equal(t, 64, exitStatus)
 
@@ -88,6 +88,26 @@ func TestParseSmartForSata(t *testing.T) {
 	}
 }
 
+func TestParseSmartForSataWithConfiguredType(t *testing.T) {
+	fixturePath := filepath.Join("test-data", "smart", "sda.json")
+	data, err := os.ReadFile(fixturePath)
+	require.NoError(t, err)
+
+	sm := &SmartManager{
+		SmartDataMap: make(map[string]*smart.SmartData),
+	}
+
+	hasData, exitStatus := sm.parseSmartForSata(data, "sat+megaraid")
+	require.True(t, hasData)
+	assert.Equal(t, 64, exitStatus)
+
+	deviceData, ok := sm.SmartDataMap["9C40918040082"]
+	require.True(t, ok, "expected smart data entry for serial 9C40918040082")
+
+	assert.Equal(t, "sat+megaraid", deviceData.DiskType,
+		"DiskType should be overridden by configuredType")
+}
+
 func TestParseSmartForSataDeviceStatisticsTemperature(t *testing.T) {
 	jsonPayload := []byte(`{
 		"smartctl": {"exit_status": 0},
@@ -112,7 +132,7 @@ func TestParseSmartForSataDeviceStatisticsTemperature(t *testing.T) {
 	}`)
 
 	sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)}
-	hasData, exitStatus := sm.parseSmartForSata(jsonPayload)
+	hasData, exitStatus := sm.parseSmartForSata(jsonPayload, "")
 	require.True(t, hasData)
 	assert.Equal(t, 0, exitStatus)
 
@@ -147,7 +167,7 @@ func TestParseSmartForSataAtaDeviceStatistics(t *testing.T) {
 	}`)
 
 	sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)}
-	hasData, exitStatus := sm.parseSmartForSata(jsonPayload)
+	hasData, exitStatus := sm.parseSmartForSata(jsonPayload, "")
 	require.True(t, hasData)
 	assert.Equal(t, 0, exitStatus)
 
@@ -184,7 +204,7 @@ func TestParseSmartForSataNegativeDeviceStatistics(t *testing.T) {
 	}`)
 
 	sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)}
-	hasData, exitStatus := sm.parseSmartForSata(jsonPayload)
+	hasData, exitStatus := sm.parseSmartForSata(jsonPayload, "")
 	require.True(t, hasData)
 	assert.Equal(t, 0, exitStatus)
 
@@ -223,7 +243,7 @@ func TestParseSmartForSataParentheticalRawValue(t *testing.T) {
 
 	sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)}
 
-	hasData, exitStatus := sm.parseSmartForSata(jsonPayload)
+	hasData, exitStatus := sm.parseSmartForSata(jsonPayload, "")
 	require.True(t, hasData)
 	assert.Equal(t, 0, exitStatus)
 
@@ -245,7 +265,7 @@ func TestParseSmartForNvme(t *testing.T) {
 		SmartDataMap: make(map[string]*smart.SmartData),
 	}
 
-	hasData, exitStatus := sm.parseSmartForNvme(data)
+	hasData, exitStatus := sm.parseSmartForNvme(data, "")
 	require.True(t, hasData)
 	assert.Equal(t, 0, exitStatus)
 
@@ -1225,7 +1245,7 @@ func TestParseSmartForNvmeAppleSSD(t *testing.T) {
 		darwinNvmeProvider: fakeProvider,
 	}
 
-	hasData, _ := sm.parseSmartForNvme(data)
+	hasData, _ := sm.parseSmartForNvme(data, "")
 	require.True(t, hasData)
 
 	deviceData, ok := sm.SmartDataMap["0ba0147940253c15"]
@@ -1237,7 +1257,7 @@ func TestParseSmartForNvmeAppleSSD(t *testing.T) {
 	assert.Equal(t, 1, providerCalls, "system_profiler should be called once")
 
 	// Second parse: provider should NOT be called again (cache hit)
-	_, _ = sm.parseSmartForNvme(data)
+	_, _ = sm.parseSmartForNvme(data, "")
 	assert.Equal(t, 1, providerCalls, "system_profiler should not be called again after caching")
 }