fix(wireless,topology): CAPsMAN write guards, MAC neighbour resolution, unified panel (v0.23.0-beta)

Closes #95. Addresses the second round of #94 feedback. Includes a regression
introduced in v0.22.0.

Wireless write paths were not CAPsMAN-aware (regression)
  v0.22.0 widened getAP() and the bulk-SSID selector to include cap/controller
  roles so a controller — normally tagged a router — would stop being invisible to
  the wireless section. Those same helpers gate the write endpoints, so the change
  also allowed writes to devices whose radios a controller owns. Reads needed
  widening; writes needed a guard, and did not get one.

  Five paths now refuse or skip provisioned radios: bulk SSID deployment, single
  interface create, interface edit, interface delete, and guest network setup.
  Delete was the worst of them — RouterOS will not remove a provisioned interface,
  but the cached row was deleted anyway, so it vanished from the UI until the next
  poll brought it back.

  The check is shared: partitionRadios() splits physical radios into locally
  controllable and controller-owned, assertNotCapsmanManaged() refuses an edit to a
  provisioned interface and names the controller.

Neighbours identified only by MAC (#95)
  Topology resolved a neighbour to a managed device by IP address. A trunk port has
  no IP, so LLDP across one reports a MAC and an identity but no address — and
  those links stayed unresolved, drawing an external node beside the very device it
  already had on the map. Two directly connected devices appeared four times.

  Resolution by MAC now runs first, before address matching, because a MAC is
  hardware identity and unique fleet-wide where an address is not. This is the same
  join the CAPsMAN radio matching uses.

CAPsMAN panel, from the reporter's testing
  - Radios and provisioned interfaces were two tables describing the same things
    from different angles; they are now one, per interface, with the access point,
    SSID, band, frequency, channel, clients and status.
  - Provisioning rules sit inside the configuration they apply. Rules referencing a
    configuration we did not collect are listed separately rather than dropped,
    since that usually means a stale reference.
  - The Wireless header reported zero clients while the Clients page reported 32:
    it summed the CAP-local count, which is empty under central forwarding. It now
    takes the greater of that and the controller's per-radio counts, so a controller
    that is also a CAP is not double-counted.
  - A remote radio could be attributed to the controller by the five-octet prefix
    fallback when the two shared a MAC block. Prefix matches now refuse the
    controller for a non-local radio; exact matches are still honoured.

Radios tab followed device_type
  device_type is user-supplied tagging, and a router with radios had no way to
  display them. The tab now follows the detected wifi_role, which is a fact about
  the hardware.

48 tests on the CAPsMAN module, 22 on topology, including a fixture built from the
device output in #95. Verified against the fleet: topology output unchanged, so MAC
resolution merged nothing it should not have; guards correctly permissive on a
standalone AP.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

backend/src/routes/devices.ts

@@ -302,7 +302,7 @@ router.get('/:id', async (req: Request, res: Response) => {
             notes, location_address,
             location_lat::float8 AS location_lat,
             location_lng::float8 AS location_lng,
-            rack_name, rack_slot, created_at, updated_at
+            rack_name, rack_slot, wifi_role, created_at, updated_at
      FROM devices WHERE id = $1`,
     [req.params.id]
   );
@@ -339,11 +339,11 @@ router.patch('/:id/location', requireWrite, async (req: Request, res: Response)
 
   const updated = await queryOne(
     `SELECT id, name, ip_address, api_port, api_username, ssh_port, ssh_username, model,
-            serial_number, firmware_version, ros_version, device_type, status, last_seen,
+            serial_number, firmware_version, ros_version, device_type, status, last_seen, wifi_role,
             notes, location_address,
             location_lat::float8 AS location_lat,
             location_lng::float8 AS location_lng,
-            rack_name, rack_slot, created_at, updated_at
+            rack_name, rack_slot, wifi_role, created_at, updated_at
      FROM devices WHERE id = $1`,
     [req.params.id]
   );

backend/src/routes/topology.ts

@@ -20,7 +20,7 @@ export function setPollerService(p: PollerService): void {
 
 // GET /api/topology
 router.get('/', async (_req: Request, res: Response) => {
-  const [devices, allLinks, manualLinks] = await Promise.all([
+  const [devices, allLinks, manualLinks, deviceMacs] = await Promise.all([
     query<TopoDevice>(
       `SELECT id, name, ip_address, model, device_type, status, ros_version, ip_addresses_jsonb
        FROM devices ORDER BY name ASC`
@@ -40,9 +40,16 @@ router.get('/', async (_req: Request, res: Response) => {
        JOIN devices fd ON fd.id = ml.from_device_id
        JOIN devices td ON td.id = ml.to_device_id`
     ),
+    // Every interface MAC in the fleet, so a neighbour seen only by MAC — an LLDP
+    // sighting across a trunk port carries no address — still resolves to its device.
+    query<{ device_id: number; mac_address: string }>(
+      `SELECT device_id, mac_address FROM interfaces WHERE mac_address IS NOT NULL
+       UNION
+       SELECT device_id, mac_address FROM wireless_interfaces WHERE mac_address IS NOT NULL`
+    ),
   ]);
 
-  const graph = buildTopology(devices, allLinks, manualLinks);
+  const graph = buildTopology(devices, allLinks, manualLinks, deviceMacs);
 
   res.json({
     devices,

backend/src/routes/wireless.ts

@@ -42,6 +42,18 @@ router.post('/ssid/bulk', requireWrite, async (req: Request, res: Response) => {
       try {
         await collector.connect();
         const pkg = await collector.detectWifiPackage();
+
+        // Widening the device selector so CAPsMAN controllers stop being invisible
+        // also let CAPs through here. An SSID created on a provisioned radio is
+        // discarded by the controller, so refuse rather than report success.
+        const { local: localRadios, capsmanOwned } = await collector.partitionRadios();
+        if (localRadios.length === 0 && capsmanOwned.length > 0) {
+          throw new Error(
+            `${ap.name}: every radio is provisioned by CAPsMAN (${capsmanOwned.join(', ')}). `
+            + `Add this SSID to the controller's configuration instead.`
+          );
+        }
+
         const autoName = await collector.getNextInterfaceName();
 
         // Normalize mode to the correct value for this AP's package type
@@ -98,7 +110,15 @@ router.get('/', async (_req: Request, res: Response) => {
     SELECT d.id, d.name, d.ip_address, d.model, d.device_type, d.status, d.last_seen,
            d.ros_version, d.firmware_version, d.serial_number, d.rack_name, d.rack_slot,
            COUNT(DISTINCT wi.id)                           AS radio_count,
-           COALESCE(SUM(wi.registered_clients), 0)        AS client_count,
+           -- A CAP's own registration table is empty when the controller processes
+           -- traffic centrally, so its per-interface counts read zero. GREATEST
+           -- prefers whichever source actually has the clients without adding the
+           -- two together on a controller that is also a CAP.
+           GREATEST(
+             COALESCE(SUM(wi.registered_clients), 0),
+             COALESCE((SELECT SUM(cr.registered_peers) FROM capsman_radios cr
+                        WHERE cr.matched_device_id = d.id), 0)
+           )                                              AS client_count,
            COUNT(DISTINCT wi.id) FILTER (WHERE wi.ssid IS NOT NULL AND wi.disabled = false) AS ssid_count
     FROM devices d
     LEFT JOIN wireless_interfaces wi ON wi.device_id = d.id
@@ -158,8 +178,8 @@ router.get('/capsman', async (_req: Request, res: Response) => {
     `, [ids]),
   ]);
 
-  // SSIDs actually broadcasting, so the view shows what is live rather than only
-  // what was configured.
+  // Interfaces provisioned onto the CAPs, joined to their radio so the UI can show
+  // one row per interface with the access point it lives on.
   const interfaces = await query(`
     SELECT wi.device_id, wi.name, wi.ssid, wi.band, wi.frequency, wi.disabled, wi.running,
            wi.managed_by_capsman, wi.radio_mac, wi.registered_clients, d.name AS device_name
@@ -208,8 +228,17 @@ router.get('/capsman', async (_req: Request, res: Response) => {
           ...g,
           client_count: peers(g.radios as { registered_peers?: number | null }[]),
         })),
-        configurations: byController(configs as { controller_device_id: number }[], c.id),
-        provisioning: byController(provisioning as { controller_device_id: number }[], c.id),
+        configurations: byController(configs as { controller_device_id: number; name: string }[], c.id)
+          .map((cfg) => ({
+            ...cfg,
+            // The rule and the configuration it applies are one idea; showing them in
+            // separate boxes made the reader join them by eye (#94).
+            rules: byController(provisioning as { controller_device_id: number; master_configuration: string | null }[], c.id)
+              .filter((p) => p.master_configuration === cfg.name),
+          })),
+        provisioning: byController(provisioning as { controller_device_id: number; master_configuration: string | null }[], c.id)
+          .filter((p) => !configs.some((cfg) => (cfg as { controller_device_id: number; name: string }).controller_device_id === c.id
+            && (cfg as { name: string }).name === p.master_configuration)),
         ssids,
       };
     }),
@@ -278,6 +307,16 @@ router.post('/:id/interfaces', requireWrite, async (req: Request, res: Response)
   try {
     await collector.connect();
 
+    // A virtual AP created on a CAPsMAN-provisioned radio is discarded by the
+    // controller at the next provision, so refuse instead of reporting success.
+    const { local: localRadios, capsmanOwned } = await collector.partitionRadios();
+    if (localRadios.length === 0 && capsmanOwned.length > 0) {
+      return res.status(409).json({
+        error: `Every radio on ${ap.name} is provisioned by CAPsMAN (${capsmanOwned.join(', ')}). `
+          + `Add this SSID to the controller's configuration so it is provisioned to the access points.`,
+      });
+    }
+
     // Auto-generate the next available interface name (e.g. wifi3, wifi4 …)
     const autoName = await collector.getNextInterfaceName();
 
@@ -358,6 +397,7 @@ router.put('/:id/interfaces/:name', requireWrite, async (req: Request, res: Resp
   const collector = new DeviceCollector(ap);
   try {
     await collector.connect();
+    await collector.assertNotCapsmanManaged(ifaceName);
     await collector.setWirelessInterface(ifaceName, params);
     // Bridge port membership — only touched when 'bridge' key is present in the request
     if (body.bridge !== undefined) {
@@ -386,6 +426,7 @@ router.delete('/:id/interfaces/:name', requireWrite, async (req: Request, res: R
   const collector = new DeviceCollector(ap);
   try {
     await collector.connect();
+    await collector.assertNotCapsmanManaged(req.params.name);
     await collector.removeWirelessInterface(req.params.name);
     await query(
       `DELETE FROM wireless_interfaces WHERE device_id=$1 AND name=$2`,

backend/src/services/mikrotik/DeviceCollector.ts

@@ -1344,6 +1344,63 @@ export class DeviceCollector {
   }
 
   // ─── CAPsMAN ──────────────────────────────────────────────────────────────
+  /**
+   * Split this device's physical radios into ones we may configure locally and ones
+   * a CAPsMAN controller owns.
+   *
+   * Writing to a provisioned radio does not reliably take effect: the change is
+   * either rejected or silently replaced the next time the controller provisions the
+   * CAP, while the API reports success. Callers use this to skip those radios and
+   * say so rather than claim a change that will not survive.
+   */
+  async partitionRadios(): Promise<{ local: Record<string, string>[]; capsmanOwned: string[] }> {
+    const pkg = await this.detectWifiPackage();
+    if (pkg === 'none') return { local: [], capsmanOwned: [] };
+
+    const raw = pkg === 'wifi'
+      ? await this.client.execute('/interface/wifi/print', { detail: '' }).catch(() => [] as Record<string, string>[])
+      : await this.client.execute('/interface/wireless/print', { detail: '' }).catch(() => [] as Record<string, string>[]);
+
+    const physicals = raw.filter((r) => !r['master-interface'] && r['name']);
+    if (pkg !== 'wifi') return { local: physicals, capsmanOwned: [] };
+
+    const role = await this.detectWifiRole();
+    const owned = physicals.filter((r) => isCapsmanManaged(r, role));
+    return {
+      local: physicals.filter((r) => !isCapsmanManaged(r, role)),
+      capsmanOwned: owned.map((r) => r['name']).filter(Boolean),
+    };
+  }
+
+  /**
+   * Refuse a local edit to an interface a controller owns.
+   *
+   * RouterOS will not let a provisioned interface be changed or removed locally, and
+   * where it appears to accept one the controller undoes it. Failing here with an
+   * explanation beats reporting success for a change that never happened — which is
+   * what the delete path did, removing the cached row while the interface stayed.
+   */
+  async assertNotCapsmanManaged(interfaceName: string): Promise<void> {
+    const pkg = await this.detectWifiPackage();
+    if (pkg !== 'wifi') return;
+    const role = await this.detectWifiRole();
+    if (role !== 'cap' && role !== 'controller_cap') return;
+
+    const rows = await this.client.execute('/interface/wifi/print', { detail: '' })
+      .catch(() => [] as Record<string, string>[]);
+    const row = rows.find((r) => r['name'] === interfaceName);
+    if (!row || !isCapsmanManaged(row, role)) return;
+
+    const status = parseCapsmanStatus(row);
+    throw new Error(
+      `${interfaceName} is provisioned by CAPsMAN`
+      + (status?.controllerMac ? ` from controller ${status.controllerMac}` : '')
+      + `, so it cannot be changed on this device. Edit it in the controller's configuration `
+      + `instead — a local change would be discarded the next time the CAP is provisioned.`
+    );
+  }
+
+
 
   private wifiRoleCache: WifiRole | null = null;
 
@@ -1446,9 +1503,12 @@ export class DeviceCollector {
           seen.push(radio.radioMac);
           const capInfo = parseCapStatus(radio.raw);
           const st = radio.interfaceName ? live.get(radio.interfaceName) : undefined;
+          // A remote radio belongs to a CAP, never to the controller — so a
+          // near-miss prefix match against the controller's own MAC block must not
+          // pull it back onto the controller.
           const matched = radio.local
             ? this.device.id
-            : lookupDeviceForMac(radio.radioMac, macIndex);
+            : lookupDeviceForMac(radio.radioMac, macIndex, this.device.id);
 
           await query(
             `INSERT INTO capsman_radios
@@ -2429,21 +2489,15 @@ export class DeviceCollector {
       const pkg = await this.detectWifiPackage();
       if (pkg === 'none') throw new Error('No wireless package detected on this device — cannot create a guest SSID');
       const raw = pkg === 'wifi'
-        ? await this.client.execute('/interface/wifi/print').catch(() => [] as Record<string, string>[])
-        : await this.client.execute('/interface/wireless/print').catch(() => [] as Record<string, string>[]);
-      let physicals = raw.filter(r => !r['master-interface'] && r['name']);
-      if (physicals.length === 0) throw new Error('No physical radios found to attach the guest SSID to');
+        ? await this.client.execute('/interface/wifi/print', { detail: '' }).catch(() => [] as Record<string, string>[])
+        : await this.client.execute('/interface/wireless/print', { detail: '' }).catch(() => [] as Record<string, string>[]);
 
-      // A CAPsMAN-provisioned radio is owned by the controller. Adding a local
-      // virtual AP to it does not reliably take effect and can be wiped or
-      // contradicted the next time the controller provisions the CAP — so skip
-      // those radios and say so, rather than reporting success for something that
-      // silently will not work. Raised by the reporter of issue #94.
-      const role = await this.detectWifiRole();
-      const managedRadios = physicals.filter(r => isCapsmanManaged(r, role));
-      if (managedRadios.length > 0) {
-        physicals = physicals.filter(r => !isCapsmanManaged(r, role));
-        const names = managedRadios.map(r => r['name']).join(', ');
+      const { local: physicals, capsmanOwned } = await this.partitionRadios();
+      if (physicals.length === 0 && capsmanOwned.length === 0) {
+        throw new Error('No physical radios found to attach the guest SSID to');
+      }
+      if (capsmanOwned.length > 0) {
+        const names = capsmanOwned.join(', ');
         if (physicals.length === 0) {
           throw new Error(
             `Every radio on this device (${names}) is provisioned by CAPsMAN, so a guest SSID `

backend/src/services/mikrotik/__tests__/capsman.test.ts

@@ -267,3 +267,30 @@ describe('clientsPerRadio', () => {
     expect(clientsPerRadio(interfaces, []).size).toBe(0);
   });
 });
+
+describe('lookupDeviceForMac — prefix must not steal a CAP for the controller (#94)', () => {
+  it('refuses a prefix match against the excluded device', () => {
+    // Controller (id 3) owns AA:BB:CC:00:00:01; a CAP radio one octet away must not
+    // be attributed to it.
+    const idx = new Map(macIndexKeys('AA:BB:CC:00:00:01').map((k) => [k, 3] as [string, number]));
+    expect(lookupDeviceForMac('AA:BB:CC:00:00:09', idx, 3)).toBeNull();
+  });
+
+  it('still honours an exact match even against the excluded device', () => {
+    const idx = new Map(macIndexKeys('AA:BB:CC:00:00:01').map((k) => [k, 3] as [string, number]));
+    expect(lookupDeviceForMac('AA:BB:CC:00:00:01', idx, 3)).toBe(3);
+  });
+
+  it('still prefix-matches a different device', () => {
+    const idx = new Map(macIndexKeys('AA:BB:CC:00:00:01').map((k) => [k, 7] as [string, number]));
+    expect(lookupDeviceForMac('AA:BB:CC:00:00:09', idx, 3)).toBe(7);
+  });
+
+  it('prefers an exact match over a prefix match', () => {
+    const idx = new Map<string, number>([
+      ['AA:BB:CC:00:00', 7],          // prefix → device 7
+      ['AA:BB:CC:00:00:09', 9],       // exact  → device 9
+    ]);
+    expect(lookupDeviceForMac('AA:BB:CC:00:00:09', idx)).toBe(9);
+  });
+});

backend/src/services/mikrotik/capsman.ts

@@ -212,15 +212,31 @@ export function macIndexKeys(mac: string): string[] {
   return [norm, parts.slice(0, 5).join(':')];
 }
 
-/** Look up a device for a radio MAC, falling back to the five-octet prefix. */
+/**
+ * Look up a device for a radio MAC, falling back to the five-octet prefix.
+ *
+ * `excludeOnPrefix` refuses a *prefix* match against a given device — used to stop a
+ * remote CAP being attributed to the controller when the two happen to share the
+ * first five octets. An exact match is still honoured, since that is hardware
+ * identity rather than a near-miss. Reported on #94, where a CAP's radio was grouped
+ * under the controller instead of its access point.
+ */
 export function lookupDeviceForMac(
   mac: string | null,
-  index: Map<string, number>
+  index: Map<string, number>,
+  excludeOnPrefix?: number
 ): number | null {
   if (!mac) return null;
-  for (const key of macIndexKeys(mac)) {
+  const keys = macIndexKeys(mac);
+
+  const exact = index.get(keys[0]);
+  if (exact !== undefined) return exact;
+
+  for (const key of keys.slice(1)) {
     const hit = index.get(key);
-    if (hit !== undefined) return hit;
+    if (hit === undefined) continue;
+    if (excludeOnPrefix !== undefined && hit === excludeOnPrefix) continue;
+    return hit;
   }
   return null;
 }

backend/src/services/topology/__tests__/buildTopology.test.ts

@@ -209,3 +209,71 @@ describe('buildTopology — behaviour preserved for unambiguous fleets', () => {
     expect(graph.links[0].to_device_id).toBeNull();
   });
 });
+
+/**
+ * Issue #95. Two managed devices connected by a trunk port. LLDP across a trunk
+ * carries no IP, so address matching has nothing to work with — but it does carry
+ * the peer's port MAC. Without MAC resolution both devices were drawn twice: once
+ * as themselves and once as an external node.
+ */
+describe('buildTopology — neighbours identified only by MAC (#95)', () => {
+  const devices = [
+    dev(12, 'legacy-core-rt-001', '10.99.0.1'),
+    dev(13, 'legacy-core-sw-001', '10.99.0.2'),
+  ];
+  // Trunk-port MACs, which differ from the management-VLAN MACs the devices answer on.
+  const macs = [
+    { device_id: 12, mac_address: '04:F4:1C:9F:3D:A9' },
+    { device_id: 13, mac_address: '04:F4:1C:36:B5:E9' },
+  ];
+  const trunkLinks = () => [
+    link({
+      from_device_id: 12, from_interface: 'trunk1', link_type: 'lldp',
+      neighbor_mac: '04:F4:1C:36:B5:E9', neighbor_identity: 'legacy-core-sw-001',
+      neighbor_address: null, from_device_name: 'legacy-core-rt-001',
+    }),
+    link({
+      from_device_id: 13, from_interface: 'sfp-sfpplus2', link_type: 'lldp',
+      neighbor_mac: '04:F4:1C:9F:3D:A9', neighbor_identity: 'legacy-core-rt-001',
+      neighbor_address: null, from_device_name: 'legacy-core-sw-001',
+    }),
+  ];
+
+  it('resolves both directions to the managed devices', () => {
+    const graph = buildTopology(devices, trunkLinks(), [], macs);
+    for (const l of graph.links) expect(l.to_device_id).not.toBeNull();
+  });
+
+  it('draws no external node duplicating a managed device', () => {
+    const graph = buildTopology(devices, trunkLinks(), [], macs);
+    expect(graph.externalNodes.filter((n) => n.caps !== 'segment')).toHaveLength(0);
+  });
+
+  it('collapses the bidirectional pair into one link', () => {
+    const graph = buildTopology(devices, trunkLinks(), [], macs);
+    expect(graph.links).toHaveLength(1);
+    expect(graph.links[0].from_device_id).toBe(12);
+    expect(graph.links[0].to_device_id).toBe(13);
+  });
+
+  it('still produces external nodes for genuinely unmanaged neighbours', () => {
+    const graph = buildTopology(devices, [
+      link({ from_device_id: 12, from_interface: 'ether5', link_type: 'lldp',
+             neighbor_mac: 'FF:FF:FF:00:00:01', neighbor_identity: 'some-switch' }),
+    ], [], macs);
+    expect(graph.externalNodes.filter((n) => n.caps !== 'segment')).toHaveLength(1);
+  });
+
+  it('never resolves a device to itself through its own MAC', () => {
+    const graph = buildTopology(devices, [
+      link({ from_device_id: 12, from_interface: 'ether1', link_type: 'lldp',
+             neighbor_mac: '04:F4:1C:9F:3D:A9' }),
+    ], [], macs);
+    expect(graph.links[0].to_device_id).toBeNull();
+  });
+
+  it('works without a MAC index, as before', () => {
+    const graph = buildTopology(devices, trunkLinks(), []);
+    expect(graph.links.every((l) => l.to_device_id === null)).toBe(true);
+  });
+});

backend/src/services/topology/buildTopology.ts

@@ -188,11 +188,39 @@ function makeNeighborKey() {
 export function buildTopology(
   devices: TopoDevice[],
   allLinks: LinkRow[],
-  manualLinks: ManualLinkRow[]
+  manualLinks: ManualLinkRow[],
+  deviceMacs: { device_id: number; mac_address: string }[] = []
 ): TopologyGraph {
   const amb = findAmbiguousIdentifiers(devices, allLinks);
   const neighborKey = makeNeighborKey();
 
+  // ── Step 0: Resolve neighbours by MAC ──────────────────────────────────────
+  //
+  // Runs before everything else because a MAC identifies hardware and is unique
+  // fleet-wide, which no other neighbour field is. It also covers the case address
+  // matching cannot: a trunk port carries no IP, so LLDP across one reports a MAC
+  // and an identity but no address. Those links stayed unresolved and rendered as
+  // external nodes duplicating devices already on the map
+  // (github.com/2GT-Media-Group-LLC/mikrotik-manager/issues/95).
+  const macToDevice = new Map<string, { id: number; name: string }>();
+  const deviceNames = new Map(devices.map((d) => [d.id, d.name]));
+  for (const row of deviceMacs) {
+    if (!row.mac_address) continue;
+    const key = row.mac_address.toUpperCase();
+    if (!macToDevice.has(key)) {
+      macToDevice.set(key, { id: row.device_id, name: deviceNames.get(row.device_id) ?? '' });
+    }
+  }
+
+  for (const link of allLinks) {
+    if (link.to_device_id || !link.neighbor_mac) continue;
+    const hit = macToDevice.get(link.neighbor_mac.toUpperCase());
+    if (hit && hit.id !== link.from_device_id) {
+      link.to_device_id = hit.id;
+      link.to_device_name = hit.name || link.to_device_name;
+    }
+  }
+
   // ── Step 1: Per-(device, neighbor) best-protocol dedup ─────────────────────
   const bestByPair = new Map<string, LinkRow>();
   for (const link of allLinks) {