fix(ethercat): prevent slave name collisions across masters [DOPE-281]

src/frontend/components/_features/[workspace]/editor/device/ethercat/ethercat-device-editor.tsx

@@ -1,3 +1,3 @@
 import * as Tabs from '@radix-ui/react-tabs'
 import { collectUsedIecAddresses } from '@root/backend/shared/ethercat'
 import { useDeviceConfiguration } from '@root/frontend/hooks/use-device-configuration'
@@ -224,6 +224,9 @@
       {/* Header */}
       <div className='mb-4 shrink-0'>
         <h2 className='text-lg font-semibold text-neutral-1000 dark:text-neutral-100'>{device.name}</h2>
+        {esiDevice?.name && esiDevice.name !== device.name && (
+          <p className='mt-0.5 text-xs text-neutral-500 dark:text-neutral-400'>{esiDevice.name}</p>
+        )}
       </div>
 
       {/* Tabs */}

src/frontend/components/_features/[workspace]/editor/device/ethercat/index.tsx

@@ -1,3 +1,3 @@
 import * as Tabs from '@radix-ui/react-tabs'
 import { collectUsedIecAddresses } from '@root/backend/shared/ethercat/collect-used-iec-addresses'
 import { createDefaultSlaveConfig } from '@root/backend/shared/ethercat/device-config-defaults'
@@ -7,6 +7,8 @@
 import { Modal, ModalContent, ModalTitle } from '@root/frontend/components/_molecules/modal'
 import { useOpenPLCStore } from '@root/frontend/store'
 import { cn } from '@root/frontend/utils/cn'
+import { getShortDeviceName } from '@root/frontend/utils/short-device-name'
+import { collectAllSlaveNames, generateUniqueSlaveName } from '@root/frontend/utils/unique-slave-name'
 import type {
   ConfiguredEtherCATDevice,
   ESIDeviceRef,
@@ -379,6 +381,9 @@
     const unmatched: ScannedDeviceMatch['device'][] = []
     const existingPositions = new Set(configuredDevices.map((d) => d.position))
     const usedAddresses = collectUsedIecAddresses(project.data.remoteDevices)
+    // Names already taken across every master — extended as we add each
+    // device so the batch can't collide with itself either.
+    const takenNames = collectAllSlaveNames(project.data.remoteDevices)
 
     for (const position of selectedScannedDevices) {
       // Skip devices already configured at this position
@@ -406,10 +411,14 @@
         for (const m of enriched.channelMappings ?? []) usedAddresses.add(m.iecLocation)
       }
 
+      const baseName = getShortDeviceName(bestMatch.esiDevice)
+      const uniqueName = generateUniqueSlaveName(baseName, takenNames)
+      takenNames.add(uniqueName)
+
       newDevices.push({
         id: uuidv4(),
         position: match.device.position,
-        name: bestMatch.esiDevice.name || match.device.name,
+        name: uniqueName,
         esiDeviceRef: {
           repositoryItemId: bestMatch.repositoryItemId,
           deviceIndex: bestMatch.deviceIndex,
@@ -473,10 +482,13 @@
       const nextPosition =
         configuredDevices.length > 0 ? Math.max(...configuredDevices.map((d) => d.position ?? 0)) + 1 : 1
 
+      const baseName = getShortDeviceName(device)
+      const uniqueName = generateUniqueSlaveName(baseName, collectAllSlaveNames(project.data.remoteDevices))
+
       const newDevice: ConfiguredEtherCATDevice = {
         id: uuidv4(),
         position: nextPosition,
-        name: device.name,
+        name: uniqueName,
         esiDeviceRef: ref,
         vendorId: repoItem.vendor.id,
         productCode: device.type.productCode,

src/frontend/store/__tests__/shared-slice.test.ts

@@ -1,3 +1,3 @@
 import { createStore } from 'zustand/vanilla'
 
 import type { PLCVariable } from '../../../middleware/shared/ports/types'
@@ -773,6 +773,132 @@
     })
   })
 
+  // =========================================================================
+  // ethercatDeviceActions
+  // =========================================================================
+  describe('ethercatDeviceActions', () => {
+    function addEthercatBus(name: string, slaves: Array<{ id: string; name: string }>) {
+      store.getState().projectActions.createRemoteDevice({
+        data: { name, protocol: 'ethercat' },
+      })
+      store.getState().projectActions.updateEthercatConfig(name, {
+        masterConfig: { networkInterface: 'eth0', cycleTimeUs: 1000, watchdogTimeoutCycles: 3 },
+        devices: slaves as never,
+      })
+      for (const slave of slaves) {
+        store
+          .getState()
+          .editorActions.addModel({ type: 'plc-remote-device', meta: { name: slave.name, protocol: 'ethercat' } })
+        store.getState().fileActions.addFile({ name: slave.name, type: 'ethercat-device', filePath: name })
+        store.getState().tabsActions.updateTabs({
+          name: slave.name,
+          elementType: { type: 'ethercat-device', busName: name, deviceId: slave.id },
+        })
+      }
+    }
+
+    // -----------------------------------------------------------------------
+    // delete
+    // -----------------------------------------------------------------------
+    describe('delete', () => {
+      beforeEach(() => {
+        addEthercatBus('bus1', [{ id: 'slave-1', name: 'EK1100' }])
+      })
+
+      it('removes the slave from project, files, tabs and editor', () => {
+        store.getState().editorActions.setEditor({
+          type: 'plc-remote-device',
+          meta: { name: 'EK1100', protocol: 'ethercat' },
+        })
+
+        const result = store.getState().ethercatDeviceActions.delete('bus1', 'slave-1')
+        expect(result).toEqual({ ok: true })
+
+        const state = store.getState()
+        const bus = state.project.data.remoteDevices?.find((d) => d.name === 'bus1')
+        expect(bus?.ethercatConfig?.devices).toHaveLength(0)
+        expect(state.files['EK1100']).toBeUndefined()
+        expect(state.tabs.some((t) => t.name === 'EK1100')).toBe(false)
+        expect(state.editor.type).toBe('available')
+      })
+
+      it('returns error when the bus does not exist', () => {
+        const result = store.getState().ethercatDeviceActions.delete('missing-bus', 'slave-1')
+        expect(result).toEqual({ ok: false, message: 'Bus not found' })
+      })
+
+      it('returns error when the slave id does not exist', () => {
+        const result = store.getState().ethercatDeviceActions.delete('bus1', 'missing-slave')
+        expect(result).toEqual({ ok: false, message: 'EtherCAT device not found' })
+      })
+
+      it('does not clear the editor when a different slave is active', () => {
+        store.getState().editorActions.setEditor({
+          type: 'plc-remote-device',
+          meta: { name: 'other-device', protocol: 'ethercat' },
+        })
+        store.getState().ethercatDeviceActions.delete('bus1', 'slave-1')
+        expect(store.getState().editor.meta.name).toBe('other-device')
+      })
+    })
+
+    // -----------------------------------------------------------------------
+    // rename
+    // -----------------------------------------------------------------------
+    describe('rename', () => {
+      beforeEach(() => {
+        addEthercatBus('bus1', [
+          { id: 'slave-1', name: 'EK1100' },
+          { id: 'slave-2', name: 'EL1809' },
+        ])
+        addEthercatBus('bus2', [{ id: 'slave-3', name: 'EL1809_01' }])
+      })
+
+      it('renames the slave across project, files and tabs', () => {
+        const result = store.getState().ethercatDeviceActions.rename('bus1', 'slave-1', 'EK1100-renamed')
+        expect(result).toEqual({ ok: true })
+
+        const state = store.getState()
+        const bus = state.project.data.remoteDevices?.find((d) => d.name === 'bus1')
+        const slave = bus?.ethercatConfig?.devices?.find((d) => d.id === 'slave-1')
+        expect(slave?.name).toBe('EK1100-renamed')
+        expect(state.files['EK1100-renamed']).toBeDefined()
+        expect(state.files['EK1100']).toBeUndefined()
+        expect(state.tabs.some((t) => t.name === 'EK1100-renamed')).toBe(true)
+      })
+
+      it('rejects renaming to a name already used by another slave in the same bus', () => {
+        const result = store.getState().ethercatDeviceActions.rename('bus1', 'slave-1', 'EL1809')
+        expect(result.ok).toBe(false)
+        expect(result.message).toContain('EL1809')
+        const state = store.getState()
+        const bus = state.project.data.remoteDevices?.find((d) => d.name === 'bus1')
+        expect(bus?.ethercatConfig?.devices?.find((d) => d.id === 'slave-1')?.name).toBe('EK1100')
+      })
+
+      it('rejects renaming to a name already used by a slave on a different bus', () => {
+        const result = store.getState().ethercatDeviceActions.rename('bus1', 'slave-2', 'EL1809_01')
+        expect(result.ok).toBe(false)
+        expect(result.message).toContain('EL1809_01')
+      })
+
+      it('allows renaming to the same name (no-op)', () => {
+        const result = store.getState().ethercatDeviceActions.rename('bus1', 'slave-1', 'EK1100')
+        expect(result).toEqual({ ok: true })
+      })
+
+      it('returns error when the bus does not exist', () => {
+        const result = store.getState().ethercatDeviceActions.rename('missing-bus', 'slave-1', 'X')
+        expect(result).toEqual({ ok: false, message: 'Bus not found' })
+      })
+
+      it('returns error when the slave id does not exist', () => {
+        const result = store.getState().ethercatDeviceActions.rename('bus1', 'missing-slave', 'X')
+        expect(result).toEqual({ ok: false, message: 'EtherCAT device not found' })
+      })
+    })
+  })
+
   // =========================================================================
   // snapshotActions
   // =========================================================================

src/frontend/store/slices/shared/slice.ts

@@ -1,3 +1,3 @@
 import { produce } from 'immer'
 import { StateCreator } from 'zustand'
 
@@ -6,6 +6,7 @@
 import { generateIecVariablesToString } from '../../../utils/generate-iec-variables-to-string'
 import { syncNodesWithVariables, syncNodesWithVariablesFBD } from '../../../utils/graphical/sync-nodes-with-variables'
 import { toast } from '../../../utils/toast'
+import { collectAllSlaveNames } from '../../../utils/unique-slave-name'
 import type { FBDFlowType } from '../fbd'
 import type { FileSliceDataObject } from '../file'
 import type { HistorySnapshot } from '../history'
@@ -345,6 +346,13 @@
       if (!device) return { ok: false, message: 'EtherCAT device not found' }
 
       const oldName = device.name
+      // Only *rejecting* enforcement of slave-name uniqueness — scan-bus add
+      // auto-suffixes instead. Tabs/editor/file slices are name-keyed and break
+      // silently on duplicates, so new write paths must replicate one strategy.
+      // Same-name rename is allowed (the action stays idempotent).
+      if (newName !== oldName && collectAllSlaveNames(state.project.data.remoteDevices).has(newName)) {
+        return { ok: false, message: `An EtherCAT slave named "${newName}" already exists in this project` }
+      }
       const updatedDevices = devices.map((d) => (d.id === deviceId ? { ...d, name: newName } : d))
       state.projectActions.updateEthercatConfig(busName, {
         masterConfig: remoteDevice.ethercatConfig?.masterConfig ?? {

src/frontend/utils/__tests__/short-device-name.test.ts

@@ -0,0 +1,102 @@
+import { getShortDeviceName } from '../short-device-name'
+
+const make = (
+  overrides: Partial<{ typeName: string; name: string; productCode: string; revisionNo: string }> = {},
+) => ({
+  type: {
+    name: overrides.typeName ?? '',
+    productCode: overrides.productCode ?? '0x07212C52',
+    revisionNo: overrides.revisionNo ?? '0x00110000',
+  },
+  name: overrides.name ?? '',
+})
+
+describe('getShortDeviceName', () => {
+  describe('P1: <Type> text as short code', () => {
+    it('returns <Type> text when it looks like an SKU', () => {
+      expect(getShortDeviceName(make({ typeName: 'EL1809' }))).toBe('EL1809')
+    })
+
+    it('accepts SKUs with hyphens up to 24 chars', () => {
+      expect(getShortDeviceName(make({ typeName: 'EL2521-0124-0010' }))).toBe('EL2521-0124-0010')
+    })
+
+    it('trims surrounding whitespace before evaluating', () => {
+      expect(getShortDeviceName(make({ typeName: '  EL1809  ' }))).toBe('EL1809')
+    })
+
+    it('falls through to P2 when <Type> text contains internal whitespace', () => {
+      // P1 rejects (whitespace), P2 takes first token "EK1100" — SKU-shaped, returned
+      expect(getShortDeviceName(make({ typeName: 'Generic Coupler', name: 'EK1100 EtherCAT Coupler' }))).toBe('EK1100')
+    })
+
+    it('falls through to P3 when both P1 and P2 reject but <Type> text exists', () => {
+      // P1 rejects (whitespace), P2 rejects ("Generic" has no digit), P3 returns <Type> text
+      expect(getShortDeviceName(make({ typeName: 'Generic Coupler', name: 'Generic Coupler description' }))).toBe(
+        'Generic Coupler',
+      )
+    })
+
+    it('falls through when <Type> text is longer than 24 chars', () => {
+      expect(
+        getShortDeviceName(make({ typeName: 'ExtraLongDescriptiveTypeName123', name: 'EL1809 2Ch. Digital Input' })),
+      ).toBe('EL1809')
+    })
+  })
+
+  describe('P2: first token of <Name> as SKU', () => {
+    it('extracts SKU when it leads the long name', () => {
+      expect(getShortDeviceName(make({ name: 'EL1809 2Ch. Digital Input 24V, 3ms' }))).toBe('EL1809')
+    })
+
+    it('handles comma and semicolon separators', () => {
+      expect(getShortDeviceName(make({ name: 'EK1100,EtherCAT Coupler' }))).toBe('EK1100')
+    })
+
+    it('rejects digit-leading tokens like "2-Channel"', () => {
+      // P2 rejects, P3 unavailable, P4 returns the long name as-is
+      expect(getShortDeviceName(make({ name: '2-Channel Digital Input' }))).toBe('2-Channel Digital Input')
+    })
+
+    it('rejects pure-letter tokens like "EtherCAT"', () => {
+      expect(getShortDeviceName(make({ name: 'EtherCAT Generic Slave' }))).toBe('EtherCAT Generic Slave')
+    })
+
+    it('rejects tokens shorter than 3 chars even if SKU-shaped', () => {
+      expect(getShortDeviceName(make({ name: 'A1 short token here' }))).toBe('A1 short token here')
+    })
+
+    it('rejects tokens longer than 24 chars', () => {
+      const longToken = 'X' + '1'.repeat(24)
+      expect(getShortDeviceName(make({ name: `${longToken} description` }))).toBe(`${longToken} description`)
+    })
+
+    it('accepts SKUs with mixed digits and hyphens (e.g. Omron R88D-1SN02H-ECT)', () => {
+      expect(getShortDeviceName(make({ name: 'R88D-1SN02H-ECT Servo Drive' }))).toBe('R88D-1SN02H-ECT')
+    })
+  })
+
+  describe('P3: <Type> text as last readable fallback', () => {
+    it('returns <Type> text when it has whitespace and P2 finds nothing usable', () => {
+      expect(getShortDeviceName(make({ typeName: 'Generic Coupler' }))).toBe('Generic Coupler')
+    })
+  })
+
+  describe('P4: long name as-is', () => {
+    it('returns the long name when both P1 and P2 reject', () => {
+      expect(getShortDeviceName(make({ name: 'Generic EtherCAT Slave' }))).toBe('Generic EtherCAT Slave')
+    })
+  })
+
+  describe('P5: canonical identity fallback', () => {
+    it('returns Device_{productCode}_{revisionNo} when no name is available', () => {
+      expect(getShortDeviceName(make({ productCode: '0x07212C52', revisionNo: '0x00110000' }))).toBe(
+        'Device_0x07212C52_0x00110000',
+      )
+    })
+
+    it('treats whitespace-only names as empty', () => {
+      expect(getShortDeviceName(make({ typeName: '   ', name: '   ' }))).toBe('Device_0x07212C52_0x00110000')
+    })
+  })
+})

src/frontend/utils/__tests__/unique-slave-name.test.ts

@@ -0,0 +1,57 @@
+import { collectAllSlaveNames, generateUniqueSlaveName } from '../unique-slave-name'
+
+describe('collectAllSlaveNames', () => {
+  it('returns empty set when remoteDevices is undefined', () => {
+    expect(collectAllSlaveNames(undefined)).toEqual(new Set())
+  })
+
+  it('returns empty set when there are no remote devices', () => {
+    expect(collectAllSlaveNames([])).toEqual(new Set())
+  })
+
+  it('returns empty set when remote devices have no ethercat config', () => {
+    expect(collectAllSlaveNames([{}, { ethercatConfig: undefined }])).toEqual(new Set())
+  })
+
+  it('returns empty set when ethercat config has no devices array', () => {
+    expect(collectAllSlaveNames([{ ethercatConfig: {} }])).toEqual(new Set())
+  })
+
+  it('collects names from a single master', () => {
+    const result = collectAllSlaveNames([{ ethercatConfig: { devices: [{ name: 'EL1809' }, { name: 'EL2008' }] } }])
+    expect(result).toEqual(new Set(['EL1809', 'EL2008']))
+  })
+
+  it('collects names across multiple masters and deduplicates', () => {
+    const result = collectAllSlaveNames([
+      { ethercatConfig: { devices: [{ name: 'EL1809' }, { name: 'EL2008' }] } },
+      { ethercatConfig: { devices: [{ name: 'EL1809' }, { name: 'EL3104' }] } },
+    ])
+    expect(result).toEqual(new Set(['EL1809', 'EL2008', 'EL3104']))
+  })
+})
+
+describe('generateUniqueSlaveName', () => {
+  it('returns base when not taken', () => {
+    expect(generateUniqueSlaveName('EL1809', [])).toBe('EL1809')
+    expect(generateUniqueSlaveName('EL1809', ['EL2008'])).toBe('EL1809')
+  })
+
+  it('returns _01 suffix on first collision', () => {
+    expect(generateUniqueSlaveName('EL1809', ['EL1809'])).toBe('EL1809_01')
+  })
+
+  it('skips taken suffixes and picks the next free one', () => {
+    expect(generateUniqueSlaveName('EL1809', ['EL1809', 'EL1809_01', 'EL1809_02'])).toBe('EL1809_03')
+  })
+
+  it('pads single digits to two digits and widens past 99', () => {
+    const taken = new Set<string>(['EL1809'])
+    for (let i = 1; i <= 99; i++) taken.add(`EL1809_${String(i).padStart(2, '0')}`)
+    expect(generateUniqueSlaveName('EL1809', taken)).toBe('EL1809_100')
+  })
+
+  it('accepts a Set directly as the existing argument', () => {
+    expect(generateUniqueSlaveName('EL1809', new Set(['EL1809']))).toBe('EL1809_01')
+  })
+})