fix(agent): distinguish SMART devices sharing the same path

Co-authored-by: Digital <github@crni.xyz>

agent/smart.go

@@ -375,9 +375,15 @@ func (sm *SmartManager) parseSmartOutput(deviceInfo *DeviceInfo, output []byte)
 		Type  string
 		Parse func([]byte) (bool, int)
 	}{
-		{Type: "nvme", Parse: sm.parseSmartForNvme},
+		{Type: "nvme", Parse: func(output []byte) (bool, int) {
-		{Type: "sat", Parse: sm.parseSmartForSata},
+			return sm.parseSmartForNvme(output, deviceInfo.Type)
-		{Type: "scsi", Parse: sm.parseSmartForScsi},
+		}},
+		{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)
@@ -486,10 +492,11 @@ func (sm *SmartManager) CollectSmart(deviceInfo *DeviceInfo) error {
 		return errNoValidSmartData
 	}
 
-	// slog.Info("collecting SMART data", "device", deviceInfo.Name, "type", deviceInfo.Type, "has_existing_data", sm.hasDataForDevice(deviceInfo.Name))
+	// slog.Info("collecting SMART data", "device", deviceInfo.Name, "type", deviceInfo.Type, "has_existing_data", sm.hasDataForDevice(deviceInfo))
 
-	// Check if we have any existing data for this device
+	// Check if we have existing data for this exact device identity. Multiple
-	hasExistingData := sm.hasDataForDevice(deviceInfo.Name)
+	// bridge slots can share a path, so a name-only match is not sufficient.
+	hasExistingData := sm.hasDataForDevice(deviceInfo)
 
 	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
 	defer cancel()
@@ -592,14 +599,18 @@ func (sm *SmartManager) smartctlArgs(deviceInfo *DeviceInfo, includeStandby bool
 	return args
 }
 
-// hasDataForDevice checks if we have cached SMART data for a specific device
+// hasDataForDevice checks if we have cached SMART data for a specific device identity.
-func (sm *SmartManager) hasDataForDevice(deviceName string) bool {
+func (sm *SmartManager) hasDataForDevice(deviceInfo *DeviceInfo) bool {
+	if deviceInfo == nil {
+		return false
+	}
+
 	sm.Lock()
 	defer sm.Unlock()
 
-	// Check if any cached data has this device name
+	deviceKey := makeDeviceKey(deviceInfo.Name, deviceInfo.Type)
 	for _, data := range sm.SmartDataMap {
-		if data != nil && data.DiskName == deviceName {
+		if data != nil && makeDeviceKey(data.DiskName, data.DiskType) == deviceKey {
 			return true
 		}
 	}
@@ -747,7 +758,14 @@ func mergeDeviceLists(existing, scanned, configured []*DeviceInfo) []*DeviceInfo
 			continue
 		}
 		if existingDev := deviceIndexByName[configuredDevice.Name]; existingDev != nil {
+			oldKey := makeDeviceKey(existingDev.Name, existingDev.Type)
+			if prev := existingIndex[key]; prev != nil {
+				preserveVerifiedType(existingDev, prev)
+			}
 			applyConfiguredMetadata(existingDev, configuredDevice)
+			delete(deviceIndex, oldKey)
+			deviceIndex[makeDeviceKey(existingDev.Name, existingDev.Type)] = existingDev
+			delete(deviceIndexByName, configuredDevice.Name)
 			continue
 		}
 
@@ -851,9 +869,11 @@ 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
+// parseSmartForSata parses the output of smartctl --all -j for SATA/ATA devices and updates the SmartDataMap.
+// deviceType is the exact type used to identify and query the device; when set,
+// it takes precedence over the generic type reported by smartctl.
 // Returns hasValidData and exitStatus
-func (sm *SmartManager) parseSmartForSata(output []byte) (bool, int) {
+func (sm *SmartManager) parseSmartForSata(output []byte, deviceType string) (bool, int) {
 	var data smart.SmartInfoForSata
 
 	if err := json.Unmarshal(output, &data); err != nil {
@@ -892,6 +912,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 deviceType != "" {
+		smartData.DiskType = deviceType
+	}
 
 	// get values from ata_device_statistics if necessary
 	var ataDeviceStats smart.AtaDeviceStatistics
@@ -965,7 +988,7 @@ func findAtaDeviceStatisticsValue(data *smart.SmartInfoForSata, ataDeviceStats *
 	return nil
 }
 
-func (sm *SmartManager) parseSmartForScsi(output []byte) (bool, int) {
+func (sm *SmartManager) parseSmartForScsi(output []byte, deviceType string) (bool, int) {
 	var data smart.SmartInfoForScsi
 
 	if err := json.Unmarshal(output, &data); err != nil {
@@ -1000,6 +1023,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 deviceType != "" {
+		smartData.DiskType = deviceType
+	}
 
 	attributes := make([]*smart.SmartAttribute, 0, 10)
 	attributes = append(attributes, &smart.SmartAttribute{Name: "PowerOnHours", RawValue: data.PowerOnTime.Hours})
@@ -1097,9 +1123,11 @@ 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
+// parseSmartForNvme parses the output of smartctl --all -j /dev/nvmeX and updates the SmartDataMap.
+// deviceType is the exact type used to identify and query the device; when set,
+// it takes precedence over the generic type reported by smartctl.
 // Returns hasValidData and exitStatus
-func (sm *SmartManager) parseSmartForNvme(output []byte) (bool, int) {
+func (sm *SmartManager) parseSmartForNvme(output []byte, deviceType string) (bool, int) {
 	data := &smart.SmartInfoForNvme{}
 
 	if err := json.Unmarshal(output, &data); err != nil {
@@ -1143,6 +1171,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 deviceType != "" {
+		smartData.DiskType = deviceType
+	}
 
 	// 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)
 
@@ -112,7 +112,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 +147,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 +184,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 +223,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 +245,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)
 
@@ -268,13 +268,15 @@ func TestParseSmartForNvme(t *testing.T) {
 func TestHasDataForDevice(t *testing.T) {
 	sm := &SmartManager{
 		SmartDataMap: map[string]*smart.SmartData{
-			"serial-1": {DiskName: "/dev/sda"},
+			"serial-1": {DiskName: "/dev/sda", DiskType: "jms56x,0"},
 			"serial-2": nil,
 		},
 	}
 
-	assert.True(t, sm.hasDataForDevice("/dev/sda"))
+	assert.True(t, sm.hasDataForDevice(&DeviceInfo{Name: "/dev/sda", Type: "jms56x,0"}))
-	assert.False(t, sm.hasDataForDevice("/dev/sdb"))
+	assert.False(t, sm.hasDataForDevice(&DeviceInfo{Name: "/dev/sda", Type: "jms56x,1"}))
+	assert.False(t, sm.hasDataForDevice(&DeviceInfo{Name: "/dev/sdb", Type: "jms56x,0"}))
+	assert.False(t, sm.hasDataForDevice(nil))
 }
 
 func TestDevicesSnapshotReturnsCopy(t *testing.T) {
@@ -609,6 +611,74 @@ func TestMergeDeviceListsPrefersConfigured(t *testing.T) {
 	assert.Equal(t, "sat", byName["/dev/sdb"].Type)
 }
 
+func TestMergeDeviceListsExpandsConfiguredDevicesWithSamePath(t *testing.T) {
+	scanned := []*DeviceInfo{
+		{Name: "/dev/sdb", Type: "sat", InfoName: "scan-info", Protocol: "ATA"},
+	}
+	configured := []*DeviceInfo{
+		{Name: "/dev/sdb", Type: "jms56x,0", explicitType: true},
+		{Name: "/dev/sdb", Type: "jms56x,1", explicitType: true},
+	}
+
+	merged := mergeDeviceLists(nil, scanned, configured)
+	require.Len(t, merged, 2)
+
+	byKey := make(map[deviceKey]*DeviceInfo, len(merged))
+	for _, device := range merged {
+		byKey[makeDeviceKey(device.Name, device.Type)] = device
+	}
+
+	first := byKey[makeDeviceKey("/dev/sdb", "jms56x,0")]
+	require.NotNil(t, first)
+	assert.Equal(t, "scan-info", first.InfoName)
+	assert.Equal(t, "ATA", first.Protocol)
+	assert.True(t, first.explicitType)
+
+	second := byKey[makeDeviceKey("/dev/sdb", "jms56x,1")]
+	require.NotNil(t, second)
+	assert.True(t, second.explicitType)
+	assert.NotContains(t, byKey, makeDeviceKey("/dev/sdb", "sat"))
+}
+
+func TestMergeDeviceListsPreservesSamePathVerificationAcrossRescan(t *testing.T) {
+	existing := []*DeviceInfo{
+		{Name: "/dev/sdb", Type: "jms56x,0", parserType: "sat", typeVerified: true, explicitType: true},
+		{Name: "/dev/sdb", Type: "jms56x,1", parserType: "sat", typeVerified: true, explicitType: true},
+	}
+	scanned := []*DeviceInfo{
+		{Name: "/dev/sdb", Type: "sat", Protocol: "ATA"},
+	}
+	configured := []*DeviceInfo{
+		{Name: "/dev/sdb", Type: "jms56x,0", explicitType: true},
+		{Name: "/dev/sdb", Type: "jms56x,1", explicitType: true},
+	}
+
+	merged := mergeDeviceLists(existing, scanned, configured)
+	require.Len(t, merged, 2)
+	byKey := make(map[deviceKey]*DeviceInfo, len(merged))
+	for _, device := range merged {
+		byKey[makeDeviceKey(device.Name, device.Type)] = device
+		assert.True(t, device.typeVerified, device.Type)
+		assert.Equal(t, "sat", device.parserType, device.Type)
+		assert.True(t, device.explicitType, device.Type)
+	}
+	assert.Contains(t, byKey, makeDeviceKey("/dev/sdb", "jms56x,0"))
+	assert.Contains(t, byKey, makeDeviceKey("/dev/sdb", "jms56x,1"))
+}
+
+func TestMergeDeviceListsDeduplicatesConfiguredIdentityAfterRekey(t *testing.T) {
+	scanned := []*DeviceInfo{{Name: "/dev/sdb", Type: "sat"}}
+	configured := []*DeviceInfo{
+		{Name: "/dev/sdb", Type: "jms56x,0", explicitType: true},
+		{Name: "/dev/sdb", Type: "jms56x,0", explicitType: true},
+	}
+
+	merged := mergeDeviceLists(nil, scanned, configured)
+	require.Len(t, merged, 1)
+	assert.Equal(t, "/dev/sdb", merged[0].Name)
+	assert.Equal(t, "jms56x,0", merged[0].Type)
+}
+
 func TestMergeDeviceListsPreservesVerification(t *testing.T) {
 	existing := []*DeviceInfo{
 		{Name: "/dev/sda", Type: "sat+megaraid", parserType: "sat", typeVerified: true},
@@ -753,6 +823,20 @@ func TestParseSmartOutputKeepsCustomType(t *testing.T) {
 	assert.Equal(t, "sat+megaraid", device.Type)
 	assert.Equal(t, "sat", device.parserType)
 	assert.True(t, device.typeVerified)
+	assert.Equal(t, "sat+megaraid", sm.SmartDataMap["9C40918040082"].DiskType)
+}
+
+func TestParseSmartOutputDoesNotNormalizeDeviceIdentity(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)}
+	device := &DeviceInfo{Name: "/dev/sda", Type: "ata", explicitType: true}
+
+	require.True(t, sm.parseSmartOutput(device, data))
+	assert.Equal(t, "sat", device.parserType)
+	assert.Equal(t, "ata", sm.SmartDataMap["9C40918040082"].DiskType)
 }
 
 func TestParseSmartOutputResetsVerificationOnFailure(t *testing.T) {
@@ -1300,7 +1384,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"]
@@ -1312,7 +1396,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")
 }